agents: show draft cards only for drafts that exist on disk - #176
Open
morgmart wants to merge 4 commits into
Open
agents: show draft cards only for drafts that exist on disk#176morgmart wants to merge 4 commits into
morgmart wants to merge 4 commits into
Conversation
The Agents gallery used to build draft cards from open build-agent chat sessions plus an in-memory cache of draft metadata. When a draft file was moved or deleted out from under the app, the card stayed behind and Delete failed with `Source "…" not found`, leaving a card that could never be removed. Drafts are now read from disk like finished agents: `listAgentGallery()` splits a single `listAgentSources()` call into personas and drafts, the agent store keeps `draftSources`, and `AgentsView` renders a card per draft file, joining it to its builder session when one is still open. File gone -> card gone on the next refresh. Deleting a draft whose chat is gone discards the file directly instead of going through a session. Untouched drafts no longer pile up or prompt. `modelProviderId` is seeded on every new draft and was being counted as user content, so leaving a fresh "New agent" draft asked "Save this agent draft?" and kept an `untitled-agent-*.md` around. It is now exempt from the placeholder check, and the navigation guard silently discards a draft with no user content (navigating first so closing the empty chat does not redirect home). Editing an existing agent without changes still just navigates away. `findAgentBuilderSource` drops a cached draft from the in-memory cache when its file is no longer listed by the backend and cannot be read, so a missing file can't deadlock delete again. Also removes a duplicate `useVoiceConversationStore` import in the AppShell navigation test that was failing lint on main. Co-Authored-By: Claude <noreply@anthropic.com>
morgmart
force-pushed
the
fix/stale-agent-draft-delete
branch
from
August 23, 2026 22:25
0467a75 to
85babd0
Compare
Navigation guard: the "is this draft untouched?" decision was made, then another lookup awaited, then the draft deleted — anything typed in that gap was discarded silently. `discardUntouchedDraftAgentSession` now owns re-check-then-discard: the user-content check is the last step before the file goes, and a draft that picked up content returns "kept" so the caller shows the save/discard prompt instead. Both the Back guard and the New agent button use it; `isDiscardableAgentBuilderSession` is folded in. Gallery refresh: `usePersonas` fenced stale disk listings behind a private mutation counter, but draft deletion in AgentsView and the promotion writes in AgentBuilderCapability bypassed it, so a focus/interval refresh that began before Delete could land afterwards and repaint the deleted card. The fence now lives in the agent store as `refreshGallery` / `mutateGallery`; every gallery writer goes through one of the two. Tests: deferred-lookup race for the guard (type while pending → kept, no delete), store fence semantics (stale snapshot dropped, in-flight mutation blocks apply, fence released on throw), and an AgentsView test where a refresh started before Delete resolves afterwards and the card stays gone. Co-Authored-By: Claude <noreply@anthropic.com>
…ion refresh Review of efb8993 found two gaps in the race fixes. The "final" user-content check still awaited a disk read after its in-memory look, so text typed during that read was invisible to it and the draft was still deleted. The in-memory look is now its own synchronous helper (`hasLocalAgentBuilderUserContent`) and `discardUntouchedDraftAgentSession` runs it once more with no await between it and the delete. Test holds the read inside the content check, types during it, and asserts "kept" with no delete/navigate/close; it fails without the re-check. `completeBuilder` started its disk refresh before the seeding mutation had released the gallery fence, so the fence (correctly) dropped every post-promotion refresh and the gallery stayed on the optimistic copy until the next timed refresh. The refresh now chains after the mutation. A capability test drives a real save through the real store and asserts the listing from disk is applied; the ChatRightRail store mock now models the fence instead of always applying, so it can no longer mask ordering bugs. Co-Authored-By: Claude <noreply@anthropic.com>
The New agent path discarded the untouched draft first and started the new builder afterwards, while the Back path navigated first. Both now use the helper's `onBeforeDiscard` transition, so the old chat is no longer the active session while its file is being deleted and closing it cannot redirect home. The helper test pins the order: navigate, delete, close. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Category
Bug fix / UX behavior (Agents gallery, agent builder)
User Impact
Source "…" not found. The gallery now shows exactly the drafts that exist on disk — file gone, card gone on the next refresh.untitled-agent-*.mdfile (and an empty chat) behind. Untouched drafts are discarded silently.Editing an existing agent without changes still shows the save/discard modal — that is intentionally out of scope here and will be a separate PR.
Problem
Draft cards were built from open
build-agentchat sessions plus an in-memory cache of draft metadata (localDraftSourcesByPath). When the draft file disappeared out from under the app, the card survived on the cache, and Delete went through Goose'ssources/delete, which fails when the file can't be canonicalized. The lookup infindAgentBuilderSourcekept returning the cached entry, so the card could never be removed.Separately, every new draft is seeded with
modelProviderId, and the placeholder check treated that as user content. So a fresh "New agent" draft was never considered empty: leaving it prompted to save, and declining still left the file on disk. Over time this piled upuntitled-agent-*files — the exact files that become stuck cards when cleaned up by hand.Solution
Gallery drafts come from disk.
listAgentGallery()makes a singlelistAgentSources()call and splits it intopersonasanddraftsbyproperties.draft === true. The agent store gainsdraftSources;usePersonasandAgentBuilderCapability.refreshPersonaspopulate both lists from that one call.AgentsViewrenders one card per draft file (hiding untouched placeholders) and joins each to its builder session by path orbuilderSessionIdwhen one is still open.PersonaDraftCardrenders from the source entry rather than a session.Placeholder rule.
modelProviderIdis exempt fromisPlaceholderDraftForSession, so a draft with only seeded metadata counts as untouched.Silent discard of untouched drafts.
guardNavigationchecksisDiscardableAgentBuilderSession(draft or no file) when there's no user content, runs the navigation first, then discards the draft and closes its chat. Navigating first matters: closing the active chat redirects home, which would stomp on where the user was going. Existing-agent edits with no changes just navigate away.Lookup no longer deadlocks.
findAgentBuilderSourcecompares against the backend's listed paths; a cached draft that isn't listed and can't be read is dropped from the cache and treated as missing, so delete/discard paths can complete.Delete without a session.
handleDeleteDraftusesdeleteDraftAgentSessionwhen a chat exists, otherwisediscardAgentBuilderSource(path), then removes the draft from the store immediately.Also removes a duplicate
useVoiceConversationStoreimport inAppShell.navigation.test.tsxthat was failing lint onmain.Review follow-up: two timing races (second commit)
Review surfaced two gaps where the promises above could break under timing:
A draft could be deleted based on a stale "it's empty" check. The guard decided "untouched", awaited one more lookup, then deleted — anything typed in that gap was lost silently.
discardUntouchedDraftAgentSessionnow owns re-check-then-discard: the user-content check is the last step before the file goes, and a draft that picked up content returns"kept"so the save/discard prompt shows instead. Both the Back guard and the New agent button use it (isDiscardableAgentBuilderSessionfolded in).A slow gallery refresh could repaint a just-deleted card.
usePersonasalready fenced stale disk listings behind a mutation counter, but that fence was private to the hook; draft deletion inAgentsViewand promotion writes inAgentBuilderCapabilitybypassed it. The fence now lives in the agent store asrefreshGallery(fetch)/mutateGallery(work), and every gallery writer goes through one of the two. A refresh that started before a delete or promotion is dropped when it lands.Review follow-up 2 (third commit)
hasLocalAgentBuilderUserContent), re-run with no await between it and the delete. Test types during the held read inside the content check and asserts"kept".completeBuilderstarted its disk refresh before the seeding mutation released the fence, so the fence dropped every post-promotion refresh. The refresh now chains after the mutation; a capability test drives a real save through the real store and asserts the disk listing is applied. TheChatRightRailstore mock now models the fence instead of always applying.Testing
just checkpasses.pnpm vitest run: 576 files, 6822 passed, 1 skipped."kept", nothing deleted), store fence semantics (stale snapshot dropped, in-flight mutation blocks apply, fence released on throw), and anAgentsViewtest where a refresh started before Delete resolves afterwards and the card stays gone.just devwas done against the earlier version of this branch (moved a draft file out of~/.agents/agents, confirmed Delete worked). The reworked behavior (card disappears on refresh, untouched draft leaves no prompt/file) is covered by the tests above; a fresh manual pass is still worth doing before merge.File changes
src/shared/api/agents.ts—AgentGalleryListing,listAgentGallery(),refreshAgentGallery();listPersonas()delegates to the gallery call.src/features/agents/stores/agentStore.ts—draftSources,setDraftSources,removeDraftSource; gallery fence (galleryRevision,galleryMutationsInFlight,refreshGallery,mutateGallery).src/features/agents/hooks/usePersonas.ts— loads personas and drafts from one gallery fetch through the store fence (private mutation refs removed).src/features/agents/capabilities/AgentBuilderCapability.tsx—completeBuilderwrites and its follow-up refresh go through the store fence.src/features/agents/ui/PersonaGallery.tsx—GalleryDrafttype;PersonaDraftCardrenders from the source entry.src/features/agents/ui/AgentsView.tsx— drafts derived fromdraftSources, joined to sessions; continue/delete handlers work with or without a session; delete runs as a gallery mutation.src/features/agents/lib/agentBuilderIdentity.ts—modelProviderIdexempt from placeholder detection.src/features/agents/lib/agentBuilderSession.ts—discardUntouchedDraftAgentSession(re-check-then-discard).src/features/agents/lib/agentBuilderSourceLifecycle.ts—findAgentBuilderSourcedrops unlisted, unreadable cached drafts.src/features/agents/hooks/useAgentBuilderCoordinator.ts—guardNavigationandstartusediscardUntouchedDraftAgentSession.AppShell.navigation.test.tsx,usePersonas.test.ts,AgentBuilderCapability.test.tsx,AgentsView.entry.test.tsx,agentBuilderSession.test.ts,agentStore.test.ts,ChatRightRail.test.tsx.