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 82c565c311..efa371ab3c 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -2325,6 +2325,7 @@ describe('global AI tools', () => { seed: () => false, read: () => undefined, refresh: () => undefined, + itemDeleted: () => Promise.resolve(), // Claims the key and removes nothing, so anything still gone was deleted by the chat. discard: () => { discards++ @@ -2341,6 +2342,7 @@ describe('global AI tools', () => { seed: () => false, read: () => undefined, refresh: () => undefined, + itemDeleted: () => Promise.resolve(), discard: () => undefined, list: () => [] }) @@ -2359,6 +2361,10 @@ describe('global AI tools', () => { read: () => undefined, refresh: () => { calls.push('refresh') + return Promise.resolve(true) + }, + itemDeleted: () => { + calls.push('itemDeleted') return Promise.resolve() }, discard: () => { @@ -2380,6 +2386,75 @@ describe('global AI tools', () => { seed: () => false, read: () => undefined, refresh: () => undefined, + itemDeleted: () => Promise.resolve(), + discard: () => undefined, + list: () => [] + }) + } + }) + + // A cleanup that the editor could not do — its first read still in flight, or failed — means + // the row is nobody's, so the chat must still delete it rather than report success. + it('deletes the draft itself when the live editor could not refresh', async () => { + const path = 'u/admin/notyetloaded' + seedBackendDraft('resource', path, { path, value: { a: 2 } }) + registerLiveItemBridge({ + seed: () => false, + read: () => undefined, + // Registered, but not loaded: it has not dealt with the row. + refresh: () => Promise.resolve(false), + itemDeleted: () => Promise.resolve(), + discard: () => undefined, + list: () => [] + }) + try { + await deleteGlobalDraft(WORKSPACE, 'resource', path, undefined, { + preserveLiveDraft: true, + deployed: true + }) + expect(getBackendDraft('resource', path)).toBeUndefined() + } finally { + registerLiveItemBridge({ + seed: () => false, + read: () => undefined, + refresh: () => undefined, + itemDeleted: () => Promise.resolve(), + discard: () => undefined, + list: () => [] + }) + } + }) + + // Deleting the deployed item leaves the editor with no baseline to reset to, so it is told + // the item is gone rather than handed a draft discard. + it('tells a live editor its item was deleted rather than discarding its draft', async () => { + const path = 'u/admin/goneitem' + seedBackendDraft('resource', path, { path, value: { a: 2 } }) + const calls: string[] = [] + registerLiveItemBridge({ + seed: () => false, + read: () => undefined, + refresh: () => Promise.resolve(true), + itemDeleted: () => { + calls.push('itemDeleted') + return Promise.resolve() + }, + discard: () => { + calls.push('discard') + return Promise.resolve({ removed: true }) + }, + list: () => [] + }) + try { + await deleteGlobalDraft(WORKSPACE, 'resource', path, undefined, { itemDeleted: true }) + expect(calls).toEqual(['itemDeleted']) + expect(getBackendDraft('resource', path)).toBeUndefined() + } finally { + registerLiveItemBridge({ + seed: () => false, + read: () => undefined, + refresh: () => undefined, + itemDeleted: () => Promise.resolve(), discard: () => undefined, list: () => [] }) diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index 25903fccdd..893fdbe72d 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -8178,7 +8178,7 @@ async function deleteWorkspaceItem( // are no longer trustworthy (same rule as deploy success). Before the // draft cleanup: a cleanup failure must not leave stale comparisons. invalidateWorkspaceComparison(workspace) - await deleteGlobalDraft(workspace, type, path, triggerKind) + await deleteGlobalDraft(workspace, type, path, triggerKind, { itemDeleted: true }) // Record the deletion in the chat's modified-items mask. In a fork this leaves a // reviewable "removed" diff vs the parent that stays scoped to this chat. Keyed diff --git a/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts b/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts index a6dabe4400..b268882f14 100644 --- a/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts +++ b/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts @@ -3,6 +3,7 @@ import { DraftService } from '$lib/gen' import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte' import { DEFAULT_DATA as DEFAULT_RAW_APP_DATA } from '$lib/components/raw_apps/dataTableRefUtils' import { + markLiveItemDeleted, refreshLiveItem, UserDraft, type UserDraftEntry, @@ -583,6 +584,9 @@ type DeleteGlobalDraftOptions = { * baseline from before that, and discarding the draft resets it to exactly that — showing * the pre-deploy value, clean, ready to be saved back over the deployment. */ deployed?: boolean + /** The deployed item itself was deleted. An editor open on it has nothing to reset to, so it + * is told the item is gone rather than handed draft-discard semantics. */ + itemDeleted?: boolean } export async function deleteGlobalDraft( @@ -595,19 +599,36 @@ export async function deleteGlobalDraft( const itemKind = itemKindFor(type, triggerKind) if (!itemKind) return 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 + // deleted here rather than by an editor that no longer has a baseline to discard to. + await markLiveItemDeleted(workspace, itemKind, storagePath) + UserDraft.forgetLocal(itemKind, storagePath, { workspace }) + await UserDraftDbSyncer.save({ + workspace, + itemKind, + path: storagePath, + value: null, + immediate: true + }) + await assertDraftCleanupLanded(workspace, itemKind, path, storagePath) + return + } if (options.deployed) { - const refreshing = refreshLiveItem(workspace, itemKind, storagePath) - if (refreshing) { - // Re-reading the open editor is the whole cleanup, and discarding on top of it is - // wrong: the deploy carried the draft as it stood when it started, so anything typed - // since is a divergence from what was deployed and belongs in the row. The editor's - // own rule already drops the row when the two match and keeps it when they do not. - await refreshing + // Re-reading the open editor is the whole cleanup, and discarding on top of it is wrong: + // the deploy carried the draft as it stood when it started, so anything typed since is a + // divergence from what was deployed and belongs in the row. The editor's own rule drops + // the row when the two match and keeps it when they do not. + const refreshed = await refreshLiveItem(workspace, itemKind, storagePath) + if (refreshed) { // Its reconcile only queued that; send it, so a caller awaiting this sees the server // settled — with no draft, or with the newer edit. await UserDraftDbSyncer.flush({ workspace, itemKind, path: storagePath }) + await assertDraftCleanupLanded(workspace, itemKind, path, storagePath) return } + // It could not read after all (still loading, or the read failed), so it has not dealt + // with the row and this caller still has to. } const liveDraft = UserDraft.getLiveEditorDraft(itemKind, { workspace }) const live = @@ -630,9 +651,20 @@ export async function deleteGlobalDraft( immediate: true }) } - // A failed (network/5xx) or conflicted delete is recorded in the syncer state, - // not thrown — surface it so callers don't report the draft as removed while - // the DB-backed source of truth still has it (same guard as the write path). + await assertDraftCleanupLanded(workspace, itemKind, path, storagePath) +} + +/** + * A failed (network/5xx) or conflicted delete is recorded in the syncer state, not thrown — + * surface it so callers don't report the draft as removed while the DB-backed source of truth + * still has it (same guard as the write path), and only then invalidate the draft readers. + */ +async function assertDraftCleanupLanded( + workspace: string, + itemKind: UserDraftItemKind, + path: string, + storagePath: string +): Promise { const state = UserDraftDbSyncer.getState({ workspace, itemKind, path: storagePath }) if (state.state === 'failed') { throw new Error(state.failureMessage ?? `Failed to delete draft "${path}".`) diff --git a/frontend/src/lib/itemStore.svelte.ts b/frontend/src/lib/itemStore.svelte.ts index 7275faf44f..da3edbf322 100644 --- a/frontend/src/lib/itemStore.svelte.ts +++ b/frontend/src/lib/itemStore.svelte.ts @@ -446,6 +446,35 @@ class Entry { return this.load(this.adapter, asOf, true) } + /** + * Re-read for a caller that has just written the deployed item, taking a turn in the queue + * first: asked for during the first load, `reread` would find nothing loaded and return + * without doing anything. Resolves false when it could not read after all, so the caller + * knows this editor has not dealt with the row and it must. + */ + async refreshed(): Promise { + await this.run(async () => {}) + if (!this.loaded || this.retired || !this.adapter) return false + return (await this.reread()).ok + } + + /** + * The deployed item itself was deleted elsewhere. Not a discard: there is no baseline left to + * reset to, so the editor reports it gone rather than sitting clean on something whose next + * save would 404. The server removes the row with the item, so none is written. + */ + itemDeleted(): Promise { + return this.run(async () => { + if (this.retired || !this.loaded) return + this.serverDeployed = undefined + this.patched = {} + this.replaceValue(undefined) + this.row = null + this.removed = true + this.ports.hint(this.key, false) + }) + } + /** An outside write (the AI chat, another editor): a real divergence, never settling. */ applyExternal(value: V): number { if (this.loaded && serialize(value) === serialize(this.value)) return this.revision @@ -1093,8 +1122,11 @@ export function createItemStore(ports: ItemRowPort) { workspace: string, kind: UserDraftItemKind, path: string - ): Promise | undefined { - return find(workspace, kind, path)?.reread() + ): Promise | undefined { + return find(workspace, kind, path)?.refreshed() + }, + itemDeleted(workspace: string, kind: UserDraftItemKind, path: string): Promise { + return find(workspace, kind, path)?.itemDeleted() ?? Promise.resolve() }, discard( workspace: string, diff --git a/frontend/src/lib/itemStore.test.ts b/frontend/src/lib/itemStore.test.ts index a8a668a0a5..cac2436a0f 100644 --- a/frontend/src/lib/itemStore.test.ts +++ b/frontend/src/lib/itemStore.test.ts @@ -752,6 +752,28 @@ describe('item store: origins', () => { expect(item.value).toEqual(typed) }) + it('reports an item deleted elsewhere as gone, not as clean', async () => { + const rows = fakeRows() + const store = createItemStore(rows.port) + const { handle: item } = store.acquire( + { workspace: 'w', kind: 'resource', path: 'u/me/r' }, + { workspace: 'w', path: 'u/me/r' }, + adapter({ deployed: deployedRes }) + ) + await settle() + item.value = { ...deployedRes, description: 'edited' } + expect(item.dirty).toBe(true) + + await store.bridge.itemDeleted('w', 'resource', 'u/me/r') + + // Resetting to the deployed baseline would leave the editor clean on something that no + // longer exists, and its next save would 404. + expect(item.removed).toBe(true) + expect(item.value).toBeUndefined() + expect(item.deployed).toBeUndefined() + expect(rows.hints.get('u/me/r')).toBe(false) + }) + it('keeps a draft-only item an outside write put back while its delete was going', async () => { const rows = fakeRows() const draft = { ...deployedRes, description: 'only a draft' } diff --git a/frontend/src/lib/userDraft.svelte.ts b/frontend/src/lib/userDraft.svelte.ts index d6f9f9ede5..e789c0815e 100644 --- a/frontend/src/lib/userDraft.svelte.ts +++ b/frontend/src/lib/userDraft.svelte.ts @@ -147,7 +147,9 @@ export type LiveItemBridge = { workspace: string, itemKind: UserDraftItemKind, path: string - ): Promise | undefined + ): Promise | undefined + /** The deployed item was deleted: the live item reports itself gone. */ + itemDeleted(workspace: string, itemKind: UserDraftItemKind, path: string): Promise /** Discard the live item's draft, resolving once its delete has landed. `undefined` when no * live item holds the key, so the caller issues the delete itself. */ discard( @@ -166,10 +168,20 @@ export function refreshLiveItem( workspace: string, itemKind: UserDraftItemKind, path: string -): Promise | undefined { +): Promise | undefined { return liveItems?.refresh(workspace, itemKind, path) } +/** Tell a live item that the deployed item under it has been deleted, so it stops showing a + * baseline that no longer exists. Resolves once it has. */ +export function markLiveItemDeleted( + workspace: string, + itemKind: UserDraftItemKind, + path: string +): Promise { + return liveItems?.itemDeleted(workspace, itemKind, path) ?? Promise.resolve() +} + export function registerLiveItemBridge(bridge: LiveItemBridge): void { liveItems = bridge }