diff --git a/frontend/src/lib/components/copilot/chat/global/core.test.ts b/frontend/src/lib/components/copilot/chat/global/core.test.ts index 19050bb7da..5c069f189f 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -2136,6 +2136,30 @@ describe('global AI tools', () => { ).toBeUndefined() }) + // The removal marker tells a session's hosted editor that its item is gone, and + // it is read by the tool-completion listener — which a throw never reaches. Left + // behind by a failed discard it would be consumed by some later action on the + // same item and send that editor away while the item is still there. + it('records the draft-only removal marker only once the discard has landed', async () => { + const path = 'f/resources/discard-marker' + await callGlobalTool('write_resource', { + path, + resource_type: 'postgresql', + value: { host: 'localhost' } + }) + + // The delete is a `value: null` draft write, so failing that write fails it. + failingWrites.add(`resource:${path}`) + await expect(callGlobalTool('discard_local_draft', { type: 'resource', path })).rejects.toThrow() + expect(UserDraft.takeDraftOnlyDiscard('resource', path, { workspace: WORKSPACE })).toBe(false) + + // The same discard, allowed through, does publish it (so the check above is + // about the failure, not about the marker never being written at all). + failingWrites.delete(`resource:${path}`) + await callGlobalTool('discard_local_draft', { type: 'resource', path }) + expect(UserDraft.takeDraftOnlyDiscard('resource', path, { workspace: WORKSPACE })).toBe(true) + }) + // "Create a resource, then never mind": delete_workspace_item must reject a path // that was never deployed, before the confirmation card — otherwise the user // confirms a workspace mutation that 404s past the draft cleanup, leaving the diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index d0594a6a5b..18dbf9c077 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -6170,17 +6170,23 @@ async function discardLocalDraft( throw new Error(`No draft found for ${type} "${path}".`) } - // Before the delete, while both sides are still knowable: a draft with nothing - // deployed under it IS the item, so discarding it removes the item rather than - // reverting it — and anything showing that item has to stop. + // Probed before the delete, while both sides are still knowable: a draft with + // nothing deployed under it IS the item, so discarding it removes the item + // rather than reverting it — and anything showing that item has to stop. const discardedKind = itemKindFor(type, triggerKind) const storagePath = getGlobalDraftStoragePath(workspace, type, path, triggerKind) - if (discardedKind && !(await hasDeployedItem(workspace, type, path))) { - UserDraft.recordDraftOnlyDiscard(discardedKind, storagePath, { workspace }) - } + const removesItem = !!discardedKind && !(await hasDeployedItem(workspace, type, path)) await deleteGlobalDraft(workspace, type, path, triggerKind) + // Published only now: the delete above can throw, and the marker is read by the + // tool-completion listener, which a throw never reaches — leaving it to be + // consumed by some later action on this item, which would send its editor away + // while the item is still there. + if (removesItem && discardedKind) { + UserDraft.recordDraftOnlyDiscard(discardedKind, storagePath, { workspace }) + } + // The chat's touch on the item is undone — drop it from the mask so a // pre-existing deployed item doesn't keep reading as this chat's edit. if (discardedKind) { diff --git a/frontend/src/lib/components/sessions/previewReload.ts b/frontend/src/lib/components/sessions/previewReload.ts index dfbf70eeff..8d92eab21b 100644 --- a/frontend/src/lib/components/sessions/previewReload.ts +++ b/frontend/src/lib/components/sessions/previewReload.ts @@ -2,6 +2,13 @@ import type { SessionPreviewTab } from './sessionState.svelte' import { whereIs } from './sessionPreviewTabs.svelte' import { stripBase, TRIGGER_PAGES, type TriggerKind } from './previewPaths' +/** What a tool asks of a hosted entity editor (see previewRouter's + * `IN_REALM_ENTITY_PAGES`) showing one of the affected pages. A plain write + * reaches it on its own — it holds the draft cell the write seeds — but the + * tools that clear or replace that cell go behind it, and the item can be gone + * altogether. `none` is what makes the live-editing case live. */ +export type EntityToolEffect = 'none' | 'refresh' | 'close' + // Which list pages a completed chat tool can change, as base-stripped paths // (e.g. `/schedules`). This allowlist is the single source of truth for "does // this tool change a list page a preview tab might show". A new mutating tool @@ -15,13 +22,6 @@ import { stripBase, TRIGGER_PAGES, type TriggerKind } from './previewPaths' // no list page we preview lists open drafts. They fall through to NO_RELOAD. // This "live editors self-sync, only list pages reload" invariant is the reason // the callers below and in the sessions page reload nothing for item tabs. -/** What a tool asks of a hosted entity editor (see previewRouter's - * `IN_REALM_ENTITY_PAGES`) showing one of the affected pages. A plain write - * reaches it on its own — it holds the draft cell the write seeds — but the - * tools that clear or replace that cell go behind it, and the item can be gone - * altogether. `none` is what makes the live-editing case live. */ -export type EntityToolEffect = 'none' | 'refresh' | 'close' - export type ToolReloadEffect = { pages: string[] entity: EntityToolEffect diff --git a/frontend/src/lib/userDraft.svelte.ts b/frontend/src/lib/userDraft.svelte.ts index 2ea187c014..6a5385b2c7 100644 --- a/frontend/src/lib/userDraft.svelte.ts +++ b/frontend/src/lib/userDraft.svelte.ts @@ -129,6 +129,21 @@ const entries = new Map() const seedMisses = new Set() // Cells whose last discard removed the item itself (see `takeDraftOnlyDiscard`). const draftOnlyDiscards = new Set() + +// Both sets above are read by the tool-completion listener right after the write +// that wrote them — but only while something is listening, and nothing is when the +// user is not in a session. Capped so an unread marker cannot accumulate: the +// oldest is dropped, since a marker is only ever meaningful to the action that +// immediately follows it. +const MAX_WRITE_MARKERS = 64 +function noteMarker(set: Set, key: string): void { + set.add(key) + while (set.size > MAX_WRITE_MARKERS) { + const oldest = set.values().next().value + if (oldest === undefined || oldest === key) break + set.delete(oldest) + } +} const liveEditorDrafts = new Map() /** * Map keys whose entry should start `syncSuspended` on acquire. Lets @@ -409,7 +424,7 @@ export const UserDraft = { const mk = mapKey(ws, itemKind, path) const entry = entries.get(mk) if (!entry) { - seedMisses.add(mk) + noteMarker(seedMisses, mk) return } seedMisses.delete(mk) @@ -424,7 +439,7 @@ export const UserDraft = { * showing it. Set at the discard, where the deployed side is known. */ recordDraftOnlyDiscard(itemKind: UserDraftItemKind, path: string, opts?: UserDraftOptions): void { - draftOnlyDiscards.add(mapKey(resolveWorkspace(opts), itemKind, path)) + noteMarker(draftOnlyDiscards, mapKey(resolveWorkspace(opts), itemKind, path)) }, /** Whether the last discard for this cell removed the item outright (see