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 b711fbe50f..841c83d5e6 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -2490,6 +2490,39 @@ describe('global AI tools', () => { } }) + // A deploy carries the draft as it stood when it started. An editor changed since has a + // divergence from that which is still undeployed, so saying "Draft removed" sends the model + // and the user away from an edit that still needs deploying. + it('reports a draft the deploy deliberately left in place', async () => { + const path = 'u/admin/kept_res' + seedBackendDraft('resource', path, { path, value: { a: 2 } }) + registerLiveItemBridge({ + seed: () => false, + // The editor kept a row: it diverged from what was deployed while the deploy ran. + read: () => ({ value: { path, value: { a: 3 } } }), + refresh: () => Promise.resolve('done' as const), + itemDeleted: () => Promise.resolve(), + discard: () => Promise.resolve('done' as const), + list: () => [] + }) + try { + const cleanup = await deleteGlobalDraft(WORKSPACE, 'resource', path, undefined, { + preserveLiveDraft: true, + deployed: true + }) + expect(cleanup).toEqual({ removed: false }) + } finally { + registerLiveItemBridge({ + seed: () => false, + read: () => undefined, + refresh: () => undefined, + itemDeleted: () => Promise.resolve(), + discard: () => undefined, + list: () => [] + }) + } + }) + // A failed server delete must surface (throw), not silently report removed — // the same guard the write path got, applied to the delete path. it('deleteGlobalDraft throws when the server delete fails', async () => { diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index 893fdbe72d..d5a95e142d 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -7994,7 +7994,7 @@ async function deployDraft( // fork comparisons before the fallible draft cleanup below. invalidateWorkspaceComparison(workspace) - await deleteGlobalDraft(workspace, type, path, triggerKind, { + const cleanup = await deleteGlobalDraft(workspace, type, path, triggerKind, { preserveLiveDraft: true, deployed: true }) @@ -8030,9 +8030,11 @@ async function deployDraft( return JSON.stringify( { success: true, - message: `Deployed draft ${type} "${path}" to the workspace. Draft removed.${ - deployNote ? ` ${deployNote}` : '' - }`, + message: `Deployed draft ${type} "${path}" to the workspace. ${ + cleanup.removed + ? 'Draft removed.' + : 'Its editor was changed while the deploy ran, so a draft of those later changes remains and is not deployed.' + }${deployNote ? ` ${deployNote}` : ''}`, type, path, triggerKind diff --git a/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts b/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts index b2f08155c9..f7fd7510dd 100644 --- a/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts +++ b/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts @@ -589,15 +589,17 @@ type DeleteGlobalDraftOptions = { itemDeleted?: boolean } +/** `removed` is false when a draft is deliberately left in place: an open editor diverged from + * what was deployed while the deploy was running, and that divergence is still to be deployed. */ export async function deleteGlobalDraft( workspace: string, type: WorkspaceItemType, path: string, triggerKind?: TriggerKind, options: DeleteGlobalDraftOptions = {} -): Promise { +): Promise<{ removed: boolean }> { const itemKind = itemKindFor(type, triggerKind) - if (!itemKind) return + if (!itemKind) return { removed: false } const storagePath = resolveDraftStoragePath(workspace, itemKind, path) if (options.itemDeleted) { // Nothing to discard against: the item is gone. The editor is told that, and the row is @@ -612,7 +614,7 @@ export async function deleteGlobalDraft( immediate: true }) await assertDraftCleanupLanded(workspace, itemKind, path, storagePath) - return + return { removed: true } } if (options.deployed) { // Re-reading the open editor is the whole cleanup, and discarding on top of it is wrong: @@ -625,7 +627,9 @@ export async function deleteGlobalDraft( // settled — with no draft, or with the newer edit. await UserDraftDbSyncer.flush({ workspace, itemKind, path: storagePath }) await assertDraftCleanupLanded(workspace, itemKind, path, storagePath) - return + // The editor keeps a row when it diverged from what was deployed while the deploy was + // running. Saying it was removed would send the caller away from an undeployed edit. + return { removed: !UserDraft.has(itemKind, storagePath, { workspace }) } } if (refreshed === 'failed') { // It has the item and a baseline from before the deploy. Discarding its row would @@ -664,6 +668,7 @@ export async function deleteGlobalDraft( }) } await assertDraftCleanupLanded(workspace, itemKind, path, storagePath) + return { removed: true } } /** diff --git a/frontend/src/lib/itemStore.svelte.ts b/frontend/src/lib/itemStore.svelte.ts index d08e1499e1..c7a5668ca5 100644 --- a/frontend/src/lib/itemStore.svelte.ts +++ b/frontend/src/lib/itemStore.svelte.ts @@ -414,16 +414,16 @@ class Entry { // nothing can send it, so a read nobody asked for must not put the server's value in // its place, nor answer the conflict on the user's behalf. Checked here rather than // before the read, because the flush that precedes one is itself a way to find out. - // A row was handed over while this read was out, so its response predates it. The value - // is still shown, for want of a better one, but writing from it would post it back - // over that row on the baseline the row advanced: the newer write lost to an older - // read. Nothing goes out until a reload settles which is which. - this.stale = this.ports.rowMark(key) !== rowsAtRead const standOff = unasked && this.ports.conflicted(key) // Whatever landed since the read was asked for — an external write, an edit — is // newer than what it read, so the read only moves the deployed side under it. const keepValue = standOff || this.edits !== at.edits || this.externals !== at.externals + // A row landed while the read was out and no newer value came with it, so it was + // written by someone this entry cannot see. Its response predates that row, and + // writing from it would post it back over it on the baseline that row advanced — so + // nothing goes out, and nothing is saved or discarded, until a reload. + this.stale = this.ports.rowMark(key) !== rowsAtRead && !keepValue this.meta = res.meta this.error = undefined if (isTemporaryPath(key.path)) { diff --git a/frontend/src/lib/itemStore.test.ts b/frontend/src/lib/itemStore.test.ts index ff67705878..bb9d13aac6 100644 --- a/frontend/src/lib/itemStore.test.ts +++ b/frontend/src/lib/itemStore.test.ts @@ -713,6 +713,8 @@ describe('item store: origins', () => { // The chat writes after asking for the discard, so its value is the newer of the two. const fromChat = { ...deployedRes, description: 'from the chat' } item.applyExternal(fromChat) + // `persistGlobalDraft` hands the value here and writes its own row through the syncer. + rows.handExternally('u/me/r') read.resolve({ deployed: deployedRes }) expect(await discarding).toEqual({ removed: false }) @@ -843,6 +845,34 @@ describe('item store: origins', () => { expect(item.value?.description).toBe('typed during the deploy') }) + it('still autosaves after the chat wrote a row during its first read', async () => { + const rows = fakeRows() + const read = deferred>() + const store = createItemStore(rows.port) + const key: ItemKey = { workspace: 'w', kind: 'resource', path: 'u/me/r' } + const { handle: item } = store.acquire( + key, + { workspace: 'w', path: 'u/me/r' }, + adapter(() => read.promise) + ) + await settle() + // The chat writes while the GET is out. Its row is this entry's own doing, and its value + // is right here — so the read is behind the server, but not in a way that loses anything. + const fromChat = { ...deployedRes, description: 'from the chat' } + item.applyExternal(fromChat) + // `persistGlobalDraft` hands the value here and writes its own row through the syncer. + rows.handExternally('u/me/r') + read.resolve({ deployed: deployedRes }) + await settle() + + expect(item.value).toEqual(fromChat) + // The editor opened on the chat's draft; typing from here has to keep reaching the row. + expect(item.canSave).toBe(true) + const typed = { ...deployedRes, description: 'typed after opening' } + item.value = typed + expect(rows.writes.at(-1)).toEqual({ path: 'u/me/r', value: typed }) + }) + it('does not post a read back over a row another editor landed during it', async () => { const rows = fakeRows() const read = deferred>()