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 72a4cb33b2..4721fba229 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -345,7 +345,8 @@ import { import { UserDraft, __resetUserDraftForTesting, - registerLiveItemBridge + registerLiveItemBridge, + type LiveItemBridge } from '$lib/userDraft.svelte' import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte' import { @@ -2310,6 +2311,39 @@ describe('global AI tools', () => { expect(res.status).toBe('conflict') }) + /** A bridge with nobody holding any key. Spread it and override only what the test is about, so + * a member added to the bridge later cannot quietly answer for a fixture that never meant to. */ + function noLiveItems(): LiveItemBridge { + return { + seed: () => false, + rowFor: () => undefined, + read: () => undefined, + refresh: () => undefined, + noteRow: () => {}, + itemDeleted: () => Promise.resolve(), + discard: () => undefined, + list: () => [] + } + } + + // The chat can write a value that turns out to be the deployed one. A live editor takes it and + // stays clean, keeping no row — so this save has to clear the row rather than persist a draft + // its own editor does not have: nothing would show it, and nothing short of an edit would + // remove it, while it still lists and goes stale once the deployed item moves on. + it('clears the row when the chat writes the value the live editor already deploys', async () => { + const path = 'f/res/noop' + const deployed = { path, value: { a: 1 } } + seedBackendDraft('resource', path, { path, value: { a: 2 } }) + registerLiveItemBridge({ ...noLiveItems(), seed: () => true, rowFor: () => null }) + try { + const res = await persistGlobalDraft(WORKSPACE, 'resource', path, deployed) + expect(res.status).toBe('saved') + expect(getBackendDraft('resource', path)).toBeUndefined() + } finally { + registerLiveItemBridge(noLiveItems()) + } + }) + // An open editor owns its own row: the chat must defer to that editor's discard rather than // send a delete of its own, which would be a second unconditional request. it('leaves the delete to a live editor that owns the draft', async () => { @@ -2322,32 +2356,19 @@ describe('global AI tools', () => { }) let discards = 0 registerLiveItemBridge({ - seed: () => false, - read: () => undefined, - refresh: () => undefined, - noteRow: () => {}, - itemDeleted: () => Promise.resolve(), + ...noLiveItems(), // Claims the key and removes nothing, so anything still gone was deleted by the chat. discard: () => { discards++ return Promise.resolve('done' as const) - }, - list: () => [] + } }) try { await deleteGlobalDraft(WORKSPACE, 'script', path) expect(discards).toBe(1) expect(getBackendDraft('script', path)).toBeDefined() } finally { - registerLiveItemBridge({ - seed: () => false, - read: () => undefined, - refresh: () => undefined, - noteRow: () => {}, - itemDeleted: () => Promise.resolve(), - discard: () => undefined, - list: () => [] - }) + registerLiveItemBridge(noLiveItems()) } }) @@ -2359,13 +2380,11 @@ describe('global AI tools', () => { seedBackendDraft('resource', path, { path, value: { a: 2 } }) const calls: string[] = [] registerLiveItemBridge({ - seed: () => false, - read: () => undefined, + ...noLiveItems(), refresh: () => { calls.push('refresh') return Promise.resolve('done' as const) }, - noteRow: () => {}, itemDeleted: () => { calls.push('itemDeleted') return Promise.resolve() @@ -2373,8 +2392,7 @@ describe('global AI tools', () => { discard: () => { calls.push('discard') return Promise.resolve({ removed: true }) - }, - list: () => [] + } }) try { await deleteGlobalDraft(WORKSPACE, 'resource', path, undefined, { @@ -2385,15 +2403,7 @@ describe('global AI tools', () => { // The chat sends no delete of its own either: the row is the editor's to settle. expect(getBackendDraft('resource', path)).toBeDefined() } finally { - registerLiveItemBridge({ - seed: () => false, - read: () => undefined, - refresh: () => undefined, - noteRow: () => {}, - itemDeleted: () => Promise.resolve(), - discard: () => undefined, - list: () => [] - }) + registerLiveItemBridge(noLiveItems()) } }) @@ -2403,14 +2413,10 @@ describe('global AI tools', () => { const path = 'u/admin/notyetloaded' seedBackendDraft('resource', path, { path, value: { a: 2 } }) registerLiveItemBridge({ - seed: () => false, - read: () => undefined, + ...noLiveItems(), // Registered, but never had the item: it has not dealt with the row. refresh: () => Promise.resolve('absent' as const), - noteRow: () => {}, - itemDeleted: () => Promise.resolve(), - discard: () => Promise.resolve('absent' as const), - list: () => [] + discard: () => Promise.resolve('absent' as const) }) try { await deleteGlobalDraft(WORKSPACE, 'resource', path, undefined, { @@ -2419,15 +2425,7 @@ describe('global AI tools', () => { }) expect(getBackendDraft('resource', path)).toBeUndefined() } finally { - registerLiveItemBridge({ - seed: () => false, - read: () => undefined, - refresh: () => undefined, - noteRow: () => {}, - itemDeleted: () => Promise.resolve(), - discard: () => undefined, - list: () => [] - }) + registerLiveItemBridge(noLiveItems()) } }) @@ -2438,10 +2436,8 @@ describe('global AI tools', () => { seedBackendDraft('resource', path, { path, value: { a: 2 } }) const calls: string[] = [] registerLiveItemBridge({ - seed: () => false, - read: () => undefined, + ...noLiveItems(), refresh: () => Promise.resolve('done' as const), - noteRow: () => {}, itemDeleted: () => { calls.push('itemDeleted') return Promise.resolve() @@ -2449,23 +2445,14 @@ describe('global AI tools', () => { discard: () => { calls.push('discard') return Promise.resolve('done' as const) - }, - 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, - noteRow: () => {}, - itemDeleted: () => Promise.resolve(), - discard: () => undefined, - list: () => [] - }) + registerLiveItemBridge(noLiveItems()) } }) @@ -2476,27 +2463,15 @@ describe('global AI tools', () => { const path = 'u/admin/stale_res' seedBackendDraft('resource', path, { path, value: { a: 2 } }) registerLiveItemBridge({ - seed: () => false, - read: () => undefined, + ...noLiveItems(), refresh: () => Promise.resolve('failed' as const), - noteRow: () => {}, - itemDeleted: () => Promise.resolve(), - discard: () => Promise.resolve('failed' as const), - list: () => [] + discard: () => Promise.resolve('failed' as const) }) try { await expect(deleteGlobalDraft(WORKSPACE, 'resource', path)).rejects.toThrow() expect(getBackendDraft('resource', path)).toBeDefined() } finally { - registerLiveItemBridge({ - seed: () => false, - read: () => undefined, - refresh: () => undefined, - noteRow: () => {}, - itemDeleted: () => Promise.resolve(), - discard: () => undefined, - list: () => [] - }) + registerLiveItemBridge(noLiveItems()) } }) @@ -2507,13 +2482,11 @@ describe('global AI tools', () => { const path = 'u/admin/kept_res' seedBackendDraft('resource', path, { path, value: { a: 2 } }) registerLiveItemBridge({ - seed: () => false, + ...noLiveItems(), // 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: () => [] + discard: () => Promise.resolve('done' as const) }) try { const cleanup = await deleteGlobalDraft(WORKSPACE, 'resource', path, undefined, { @@ -2522,15 +2495,7 @@ describe('global AI tools', () => { }) expect(cleanup).toEqual({ removed: false }) } finally { - registerLiveItemBridge({ - seed: () => false, - read: () => undefined, - refresh: () => undefined, - noteRow: () => {}, - itemDeleted: () => Promise.resolve(), - discard: () => undefined, - list: () => [] - }) + registerLiveItemBridge(noLiveItems()) } }) @@ -2540,28 +2505,16 @@ describe('global AI tools', () => { const path = 'u/admin/kept_on_discard' seedBackendDraft('resource', path, { path, value: { a: 2 } }) registerLiveItemBridge({ - seed: () => false, + ...noLiveItems(), // Still holding a draft after the discard: the user typed after it was asked for. read: () => ({ value: { path, value: { a: 3 } } }), - refresh: () => undefined, - noteRow: () => {}, - itemDeleted: () => Promise.resolve(), - discard: () => Promise.resolve('done' as const), - list: () => [] + discard: () => Promise.resolve('done' as const) }) try { const out = await deleteGlobalDraft(WORKSPACE, 'resource', path) expect(out).toEqual({ removed: false }) } finally { - registerLiveItemBridge({ - seed: () => false, - read: () => undefined, - refresh: () => undefined, - noteRow: () => {}, - itemDeleted: () => Promise.resolve(), - discard: () => undefined, - list: () => [] - }) + registerLiveItemBridge(noLiveItems()) } }) @@ -2573,11 +2526,8 @@ describe('global AI tools', () => { seedBackendDraft('resource', path, { path, value: { a: 2 } }) let discards = 0 registerLiveItemBridge({ - seed: () => false, + ...noLiveItems(), read: () => ({ value: { path, value: { a: 3 } } }), - refresh: () => undefined, - noteRow: () => {}, - itemDeleted: () => Promise.resolve(), discard: () => { discards++ return Promise.resolve('done' as const) @@ -2592,15 +2542,7 @@ describe('global AI tools', () => { clearGlobalDrafts(WORKSPACE, new Set([path])) expect(discards).toBe(1) } finally { - registerLiveItemBridge({ - seed: () => false, - read: () => undefined, - refresh: () => undefined, - noteRow: () => {}, - itemDeleted: () => Promise.resolve(), - discard: () => undefined, - list: () => [] - }) + registerLiveItemBridge(noLiveItems()) } }) diff --git a/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts b/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts index 2b96dd64e0..91cdb23fd1 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 { + liveItemRow, markLiveItemDeleted, noteLiveItemRow, refreshLiveItem, @@ -428,7 +429,13 @@ export async function persistGlobalDraft( const itemKind = itemKindFor(type, opts.triggerKind) if (!itemKind) throw new Error(`Unsupported draft type "${type}".`) const storagePath = resolveDraftStoragePath(workspace, itemKind, path) - UserDraft.seed(itemKind, storagePath, value, { workspace }) + // A live item that took the value decides what belongs in the row, and answers `null` when the + // value it now holds is the deployed one. Persisting `value` regardless would leave a row for a + // draft that item does not have, invisible to its banner and undeletable by its Save. Asked + // only of an item that took this value, so the answer is about it and not what it held before. + const took = UserDraft.seed(itemKind, storagePath, value, { workspace }) + const held = took ? liveItemRow(workspace, itemKind, storagePath) : undefined + const row = held === undefined ? value : held === null ? null : held.value // An open editor took that value and may have written its own row for it; this save is a // second row for the same value, so say so rather than leave it looking like someone else's. noteLiveItemRow(workspace, itemKind, storagePath) @@ -436,7 +443,7 @@ export async function persistGlobalDraft( workspace, itemKind, path: storagePath, - value, + value: row, immediate: true, force: opts.force }) @@ -768,10 +775,7 @@ export async function flushGlobalDraftSaves( * changed after that discard was asked for — would have that newer edit taken on this pass, * undoing exactly what the first one preserved. `keep` names the paths to leave alone. */ -export function clearGlobalDrafts( - workspace: string, - keep: ReadonlySet = new Set() -): void { +export function clearGlobalDrafts(workspace: string, keep: ReadonlySet = new Set()): void { for (const draft of UserDraft.list({ workspace, itemKinds: [...GLOBAL_DRAFT_KINDS] })) { if (keep.has(draft.path)) continue UserDraft.forgetLocal(draft.itemKind, draft.path, { workspace }) diff --git a/frontend/src/lib/itemStore.svelte.ts b/frontend/src/lib/itemStore.svelte.ts index 31b9cc0198..84388faf47 100644 --- a/frontend/src/lib/itemStore.svelte.ts +++ b/frontend/src/lib/itemStore.svelte.ts @@ -1242,6 +1242,19 @@ export function createItemStore(ports: ItemRowPort) { entry.applyExternal(value) return true }, + /** The row this entry keeps for the key, by the same rule `reconcile` writes by: `{ value }` + * when its value diverges from the deployed one, `null` when it does not and so nothing + * belongs in the row. `undefined` when nobody can answer — no entry, one that has not read + * yet, or one whose baseline is known to be behind the server. */ + rowFor( + workspace: string, + kind: UserDraftItemKind, + path: string + ): { value: unknown } | null | undefined { + const entry = find(workspace, kind, path) + if (!entry || !entry.loaded || entry.stale) return undefined + return entry.dirty ? { value: snapshot(entry.value) } : null + }, /** The draft as this tab knows it: the value when it diverges, nothing otherwise. */ read( workspace: string, diff --git a/frontend/src/lib/userDraft.svelte.ts b/frontend/src/lib/userDraft.svelte.ts index 25575d251f..401dd002c8 100644 --- a/frontend/src/lib/userDraft.svelte.ts +++ b/frontend/src/lib/userDraft.svelte.ts @@ -135,6 +135,13 @@ const liveEditorDrafts = new Map() export type LiveItemBridge = { /** Apply `value` as an outside write. False when no live item holds the key. */ seed(workspace: string, itemKind: UserDraftItemKind, path: string, value: unknown): boolean + /** The row a live item keeps for the key: `{ value }` when its value diverges from the deployed + * one, `null` when it does not. `undefined` when no live item can answer. */ + rowFor( + workspace: string, + itemKind: UserDraftItemKind, + path: string + ): { value: unknown } | null | undefined /** `undefined` when no live item holds the key; else the draft, if it has one. */ read( workspace: string, @@ -176,6 +183,18 @@ export function refreshLiveItem( return liveItems?.refresh(workspace, itemKind, path) } +/** What a live item keeps in the key's row, for a caller about to write that row itself: the + * store's rule is that a row exists exactly while the value diverges from the deployed one, so + * persisting the raw value instead would leave a row behind for a value that matches it. + * `undefined` when no live item can answer and the caller's own value is all there is. */ +export function liveItemRow( + workspace: string, + itemKind: UserDraftItemKind, + path: string +): { value: unknown } | null | undefined { + return liveItems?.rowFor(workspace, itemKind, path) +} + /** Tell a live item that a row just written for it was written by whoever handed it that same * value through `seed`, not by some other editor whose value it does not have. */ export function noteLiveItemRow( @@ -537,11 +556,15 @@ export const UserDraft = { * editor's `initContent` cascading into the bound value). * * No-op if the entry isn't live yet (acquire via `use`/`useMany` first). + * + * Returns whether a live item took the value, for a caller about to persist it: only then does + * `liveItemRow` describe that value rather than whatever the item held already. */ - seed(itemKind: UserDraftItemKind, path: string, value: V, opts?: UserDraftOptions): void { + seed(itemKind: UserDraftItemKind, path: string, value: V, opts?: UserDraftOptions): boolean { const ws = resolveWorkspace(opts) - liveItems?.seed(ws, itemKind, path, value) + const took = liveItems?.seed(ws, itemKind, path, value) ?? false seedLocalMirror(mapKey(ws, itemKind, path), value) + return took }, /**