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 bbc0997995..aafede147f 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -2538,6 +2538,58 @@ describe('global AI tools', () => { } }) + // A draft can exist only as a local cell — a second session tab, or an editor with + // autosave off — with no backend row to fall back to. + it('reads a cell-only draft by its chosen name', async () => { + UserDraft.save( + 'raw_app', + 'u/admin/draft_cell_only', + { summary: 'Cell only', draft_path: 'f/sales/cell_app', files: {}, runnables: {} }, + { workspace: WORKSPACE } + ) + + const read = JSON.parse( + await callGlobalTool('read_workspace_item', { type: 'app', path: 'f/sales/cell_app' }) + ) + expect(read).toMatchObject({ path: 'u/admin/draft_cell_only', summary: 'Cell only' }) + }) + + // Deploying by the chosen name creates the app at that name, which would then win the + // name back and leave the draft the deploy came from listed forever. + it('removes the source draft of an app deployed by its chosen name', async () => { + const storageKey = 'u/admin/draft_deployed_by_name' + seedBackendDraft( + 'raw_app', + storageKey, + { + summary: 'By name', + draft_path: 'f/sales/by_name', + files: { '/App.tsx': 'export default () => null' }, + runnables: {}, + data: { tables: [] } + }, + { workspace: WORKSPACE } + ) + const deployed = new Set() + vi.mocked(AppService.existsApp).mockImplementation(async ({ path }) => deployed.has(path)) + vi.mocked(AppService.createAppRaw).mockImplementation(async ({ formData }: any) => { + deployed.add(formData.app.path) + return 1 as any + }) + vi.mocked(AppService.getAppByPath).mockResolvedValueOnce({ + draft: { draft_path: 'f/sales/by_name' } + } as any) + + try { + await callGlobalTool('deploy_workspace_item', { type: 'app', path: 'f/sales/by_name' }) + expect(deployed.has('f/sales/by_name')).toBe(true) + expect(getBackendDraft('raw_app', storageKey, { workspace: WORKSPACE })).toBeUndefined() + } finally { + vi.mocked(AppService.existsApp).mockImplementation(async () => false) + vi.mocked(AppService.createAppRaw).mockImplementation(async () => 1 as any) + } + }) + it('refuses a draft_path that two drafts are staged under', async () => { for (const storage of ['u/admin/draft_a', 'u/admin/draft_b']) { seedBackendDraft( diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index aaf907ae6f..3265351e4f 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -6834,16 +6834,16 @@ async function discardLocalDraft( throw new Error(`No draft found for ${type} "${path}".`) } - await deleteGlobalDraft(workspace, type, path, triggerKind) + // Resolved once: the mask records a draft under the path it is stored at, so clearing + // it by the name the call used would leave the entry behind. + const storagePath = await resolveGlobalDraftStoragePath(workspace, type, path, triggerKind) + await deleteGlobalDraft(workspace, type, storagePath, triggerKind) // 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. const discardedKind = itemKindFor(type, triggerKind) if (discardedKind) { - toolCallbacks.onItemDiscarded?.( - discardedKind, - liveGlobalDraftStoragePath(workspace, type, path, triggerKind) - ) + toolCallbacks.onItemDiscarded?.(discardedKind, storagePath) } toolCallbacks.setToolStatus(toolId, { @@ -8274,7 +8274,11 @@ async function deployDraft( // fork comparisons before the fallible draft cleanup below. invalidateWorkspaceComparison(workspace) - await deleteGlobalDraft(workspace, type, path, triggerKind, { preserveLiveDraft: true }) + // By the path resolved before the deploy: the app now deployed at the draft's chosen + // name would otherwise win the name back, and the draft would be left behind. + await deleteGlobalDraft(workspace, type, draftStoragePath, triggerKind, { + preserveLiveDraft: true + }) // Move the chat's mask entry to the deployed path: a draft-only item's // synthetic storage key never exists deployed, so the entry would otherwise diff --git a/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts b/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts index 886242f578..992c4965a2 100644 --- a/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts +++ b/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts @@ -336,7 +336,10 @@ type ResolvedDraft = { storagePath: string; value?: unknown } async function resolveDraft( workspace: string, itemKind: UserDraftItemKind, - path: string + path: string, + /** A write must know which draft a name belongs to; a read can fall back to the path + * itself, which is what it addressed before names were resolved at all. */ + opts: { forWrite?: boolean } = {} ): Promise { const live = liveEditorStoragePath(workspace, itemKind, path) if (live !== path) return { storagePath: live } @@ -345,13 +348,14 @@ async function resolveDraft( if (UserDraft.get(itemKind, path, { workspace }) !== undefined) return { storagePath: path } const own = await fetchBackendDraftValue(workspace, itemKind, path) if (own !== undefined) return { storagePath: path, value: own } - return { storagePath: await storagePathForChosenName(workspace, itemKind, path) } + return { storagePath: await storagePathForChosenName(workspace, itemKind, path, opts) } } async function storagePathForChosenName( workspace: string, itemKind: UserDraftItemKind, - path: string + path: string, + opts: { forWrite?: boolean } ): Promise { const staged: StagedDraft[] = [] const add = (storagePath: string, summary: string | undefined) => { @@ -365,7 +369,9 @@ async function storagePathForChosenName( rows = await DraftService.listDrafts({ workspace }) } catch (e) { // Nothing is stored at this path, so it may be a name. Without the listing that - // cannot be told, and guessing the path itself would write a second draft. + // cannot be told, and a write that guessed the path itself would create a second + // draft; a read of whatever is deployed there is harmless. + if (!opts.forWrite) return path throw new Error( `Could not load this workspace's drafts, so "${path}" cannot be matched to the draft ` + `it may name: ${e instanceof Error ? e.message : String(e)}. Try again.` @@ -494,11 +500,11 @@ export async function readGlobalDraftValue( ): Promise { const itemKind = itemKindFor(type, triggerKind) if (!itemKind) return undefined - const cell = UserDraft.get(itemKind, liveEditorStoragePath(workspace, itemKind, path), { - workspace - }) - if (cell !== undefined) return cell const resolved = await resolveDraft(workspace, itemKind, path) + // The cell at the RESOLVED path: it is the freshest state when a save is parked, failed + // or conflicted, and a draft that exists only as a cell has no backend row behind it. + const cell = UserDraft.get(itemKind, resolved.storagePath, { workspace }) + if (cell !== undefined) return cell if (resolved.value !== undefined) return resolved.value as V return (await fetchBackendDraftValue(workspace, itemKind, resolved.storagePath)) as V | undefined } @@ -538,7 +544,8 @@ export async function persistGlobalDraft( if (!itemKind) throw new Error(`Unsupported draft type "${type}".`) // Resolved rather than taken as given: a write under a chosen name that did not resolve // would create a second draft beside the one it meant to edit. - const storagePath = (await resolveDraft(workspace, itemKind, path)).storagePath + const storagePath = (await resolveDraft(workspace, itemKind, path, { forWrite: true })) + .storagePath UserDraft.seed(itemKind, storagePath, value, { workspace }) await UserDraftDbSyncer.save({ workspace, @@ -596,7 +603,12 @@ export async function getGlobalDraft( if (!itemKind) return undefined const resolved = await resolveDraft(workspace, itemKind, path) const storagePath = resolved.storagePath - const value = resolved.value ?? (await fetchBackendDraftValue(workspace, itemKind, storagePath)) + // A draft can live only as a local cell — a second session tab, or an editor with + // autosave off — with no backend row behind it. + const value = + UserDraft.get(itemKind, storagePath, { workspace }) ?? + resolved.value ?? + (await fetchBackendDraftValue(workspace, itemKind, storagePath)) if (value === undefined || value === null) return undefined const { displayPath, isLiveDraft } = liveDisplayPath(workspace, itemKind, storagePath) return userDraftEntryToWorkspaceItem(