From b8a3887bbc174f3b5c3e69798f4c664e4def8122 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Wed, 23 Sep 2026 12:48:03 +0200 Subject: [PATCH] fix: follow a deployed draft's preview, and search drafts by chosen name Review round findings: - `list_workspace_items`' `query` filter read only `path` and `summary`, so a draft listed under its storage path stopped matching the name the user searches by. - `read_workspace_item` rebuilt app and flow payloads without `draftPath`, so a read gave the model the storage path and no chosen name. - A session preview is opened on the path the draft is stored at, which the deploy then removes. The deploy notification now carries both that path and the deployed one, and the session follows the tab to the item's new path. - The name table is loaded only for a tool call that names a path, and concurrent callers share one request rather than each making their own. - The app deploy guard refused the retry of a deploy that created the app and then failed to remove its draft; `force` now passes through it. Co-Authored-By: Claude Opus 5 (1M context) --- .../copilot/chat/global/core.test.ts | 50 ++++++++++++++++++- .../components/copilot/chat/global/core.ts | 34 +++++++++---- .../copilot/chat/global/userDraftAdapter.ts | 15 +++++- .../sessions/sessionPreviewTabs.svelte.ts | 18 +++++++ .../sessions/sessionRuntime.svelte.ts | 11 +++- 5 files changed, 114 insertions(+), 14 deletions(-) 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 21550f8ea1..8574a9cdf2 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -2446,6 +2446,53 @@ describe('global AI tools', () => { expect(getBackendDraft('raw_app', 'u/admin/staged_app')).toBeUndefined() }) + // The chosen name is what the user searches by and what the model should call the app, + // and it is the only place it survives once the draft is listed under its storage path. + it('finds and names a staged app by its chosen name', async () => { + seedBackendDraft( + 'raw_app', + 'u/admin/draft_named', + { + summary: 'Quote tool', + draft_path: 'f/sales/quote_app', + files: {}, + runnables: {}, + data: { tables: [] } + }, + { workspace: WORKSPACE } + ) + + const listed = JSON.parse( + await callGlobalTool('list_workspace_items', { types: ['app'], query: 'quote_app' }) + ) + expect(listed).toContainEqual( + expect.objectContaining({ path: 'u/admin/draft_named', draftPath: 'f/sales/quote_app' }) + ) + const read = JSON.parse( + await callGlobalTool('read_workspace_item', { type: 'app', path: 'f/sales/quote_app' }) + ) + expect(read).toMatchObject({ path: 'u/admin/draft_named', draftPath: 'f/sales/quote_app' }) + }) + + it('opens a preview on the draft a chosen name belongs to', async () => { + seedBackendDraft( + 'raw_app', + 'u/admin/draft_previewed', + { summary: 'Previewed', draft_path: 'f/sales/preview_app', files: {}, runnables: {} }, + { workspace: WORKSPACE } + ) + const handler = vi.fn(async () => 'opened') + setOpenPreviewHandler(handler) + try { + await callGlobalTool('open_preview', { kind: 'raw_app', path: 'f/sales/preview_app' }) + expect(handler).toHaveBeenCalledWith( + expect.objectContaining({ kind: 'raw_app', path: 'u/admin/draft_previewed' }) + ) + } finally { + setOpenPreviewHandler(undefined) + } + }) + 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( @@ -4739,7 +4786,8 @@ describe('global AI tools', () => { expect(onDeployed).toHaveBeenCalledWith({ sessionId: 'sess-123', kind: 'raw_app', - path: 'f/apps/report' + path: 'f/apps/report', + deployedPath: 'f/apps/report' }) } finally { setDeployedInSessionHandler(undefined) diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index 87cf3fabf9..3dc3fa3058 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -1468,13 +1468,16 @@ function getRequestedTypes(types: WorkspaceItemType[] | undefined): WorkspaceIte } function itemMatches( - item: Pick, + item: Pick, query: string | undefined ): boolean { const normalized = query?.trim().toLowerCase() if (!normalized) return true return ( item.path.toLowerCase().includes(normalized) || + // A draft is listed under its storage path, so its chosen name — the one the user + // searches by — is only in `draftPath`. + (item.draftPath?.toLowerCase().includes(normalized) ?? false) || (item.summary?.toLowerCase().includes(normalized) ?? false) ) } @@ -1532,6 +1535,7 @@ function serializeWorkspaceItemForRead(item: WorkspaceItem): unknown { return { type: 'app', path: item.path, + draftPath: item.draftPath, summary: item.summary, value: summarizeAppValue(item.value as AppDraftValue), rawApp: item.rawApp, @@ -1547,6 +1551,7 @@ function serializeWorkspaceItemForRead(item: WorkspaceItem): unknown { return { type: 'flow', path: item.path, + draftPath: item.draftPath, summary: item.summary, value: editable, isDraft: item.isDraft @@ -4454,8 +4459,10 @@ const unroutedGlobalTools: Tool<{}>[] = [ // Every tool reaches drafts through the adapter's resolver, which can route a draft's // chosen name to where it is stored only with the user's current drafts loaded. export const globalTools: Tool<{}>[] = unroutedGlobalTools.map((tool) => { + // `path` is the only argument a tool addresses a draft by, so a call without one — a + // doc search, a screenshot — needs no names and pays no request for them. const load = (p: { workspace: string; args: any }) => - loadDraftNames(p.workspace, typeof p.args?.path === 'string' ? p.args.path : undefined) + typeof p.args?.path === 'string' ? loadDraftNames(p.workspace, p.args.path) : Promise.resolve() return { ...tool, // Also on the pre-confirmation check: it reads drafts too, and runs before `fn`. @@ -4804,7 +4811,10 @@ function getSessionScreenshot(sessionId: string | undefined): Promise void let deployedInSessionHandler: DeployedInSessionHandler | undefined @@ -7961,6 +7971,9 @@ async function deployDraft( }) let actions: ToolDisplayAction[] | undefined + // Resolved before the deploy removes the draft, since both callbacks below name the + // path the draft was addressable at, not the one it was addressed by. + const draftStoragePath = getGlobalDraftStoragePath(workspace, type, path, triggerKind) // Where the deploy actually lands — the app branch can resolve a different // target from the draft's own path fields; the mask rename below must track it. let deployedPath = path @@ -8205,10 +8218,13 @@ async function deployDraft( const targetExists = await AppService.existsApp({ workspace, path: targetPath }) // A draft of a deployed app is stored at that app's path, so a target elsewhere that // already exists is another app: nothing checks a chosen name before deploy. - if (targetExists && targetPath !== storagePath) { + // `force` is the way through for the one case this cannot tell apart: a deploy + // that created the app and then failed to clean up its draft, retried. + if (targetExists && targetPath !== storagePath && !force) { throw new Error( `Cannot deploy app "${path}" to "${targetPath}": another app is already deployed there. ` + - `Ask the user for a different path for this draft.` + `Ask the user for a different path for this draft, or pass force: true if this draft ` + + `is what is deployed there.` ) } if (targetExists) { @@ -8270,11 +8286,7 @@ async function deployDraft( // stop matching anything after the draft is gone. const deployedKind = itemKindFor(type, triggerKind) if (deployedKind) { - toolCallbacks.onItemDeployed?.( - deployedKind, - getGlobalDraftStoragePath(workspace, type, path, triggerKind), - deployedPath - ) + toolCallbacks.onItemDeployed?.(deployedKind, draftStoragePath, deployedPath) } // Reload the session preview if it's open on the deployed item. Map the @@ -8286,7 +8298,9 @@ async function deployDraft( app: 'raw_app' } const kind = previewKindByType[type] - if (kind) deployedInSessionHandler?.({ sessionId, kind, path }) + if (kind) { + deployedInSessionHandler?.({ sessionId, kind, path: draftStoragePath, deployedPath }) + } toolCallbacks.setToolStatus(toolId, { content: `Deployed ${type} "${path}"`, diff --git a/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts b/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts index 63854c9761..353098c79a 100644 --- a/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts +++ b/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts @@ -338,7 +338,20 @@ const deployedExists: Partial< * use is often not where the draft lives. Called before every chat tool call; `path` is * the path that call addresses, checked against deployed items only when it is a name. */ -export async function loadDraftNames(workspace: string, path?: string): Promise { +export function loadDraftNames(workspace: string, path?: string): Promise { + // A tool and its own pre-confirmation check ask for the same names back to back, and + // tools in one turn can overlap; they share the request rather than repeating it. + const key = `${workspace}:${path ?? ''}` + const started = loadingDraftNames.get(key) + if (started) return started + const loading = readDraftNames(workspace, path).finally(() => loadingDraftNames.delete(key)) + loadingDraftNames.set(key, loading) + return loading +} + +const loadingDraftNames = new Map>() + +async function readDraftNames(workspace: string, path?: string): Promise { const names: DraftNames = { stored: new Set(), byName: new Map(), deployedAt: new Set() } const add = ( itemKind: UserDraftItemKind, diff --git a/frontend/src/lib/components/sessions/sessionPreviewTabs.svelte.ts b/frontend/src/lib/components/sessions/sessionPreviewTabs.svelte.ts index a90eb83cc4..079dc5c5ff 100644 --- a/frontend/src/lib/components/sessions/sessionPreviewTabs.svelte.ts +++ b/frontend/src/lib/components/sessions/sessionPreviewTabs.svelte.ts @@ -604,6 +604,24 @@ export class SessionPreviewTabs { this.#flush() } + /** Follow an editor item to the path it deployed under, in place. A draft is hosted + * under the path it is stored at, which the deploy then removes. */ + retargetEditorItem(from: SessionTarget, to: SessionTarget): void { + if (from.kind === to.kind && from.path === to.path) return + const tab = this.#tabs.find((t) => isEditorTabFor(t.url, from)) + if (!tab) return + // Two tabs on one item would mount two editors racing the same cell, as in + // open()/navigate(); the tab already there wins and this one is closed. + if (this.#tabs.some((t) => t.id !== tab.id && isEditorTabFor(t.url, to))) { + this.close(tab.id) + return + } + const target = previewTargetForSessionTarget(to.kind, to.path) + if (!target) return + retargetTab(tab, targetUrl(target)) + this.#flush() + } + /** Follow a page item its editor saved under a new path, in place. */ retargetPageItem(from: PageItemRef, to: PageItemRef): void { const fromUrl = pageItemUrl(from) diff --git a/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts b/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts index fbb84e1431..0fbc92c125 100644 --- a/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts +++ b/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts @@ -1273,7 +1273,7 @@ setClosePreviewTabsHandler(({ sessionId: callerSessionId, all, match }) => { // After a chat deploy, reload the calling session's preview — only if it's open // showing that exact item. -setDeployedInSessionHandler(({ sessionId: callerSessionId, kind, path }) => { +setDeployedInSessionHandler(({ sessionId: callerSessionId, kind, path, deployedPath }) => { const sessionId = callerSessionId ?? sessionState.currentSessionId if (!sessionId) return const session = sessionState.sessions.find((s) => s.id === sessionId) @@ -1282,7 +1282,14 @@ setDeployedInSessionHandler(({ sessionId: callerSessionId, kind, path }) => { // Peek without creating a cell: a deploy for an item with no open editor tab // must not allocate an empty cell that lingers until the next prune. if (runtime.loadedEditorPath(kind, path) !== path) return - runtime.syncPreviewWithDeployed(session.workspace_id, kind, path) + if (deployedPath !== path) { + // The draft the tab is open on is gone: drop it and follow the item to the path + // it now lives at, so the tab doesn't sit on a path nothing resolves to. + UserDraft.stopSync(kind, path, { workspace: session.workspace_id }) + UserDraft.discard(kind, path, undefined, { workspace: session.workspace_id }) + runtime.previewTabs.retargetEditorItem({ kind, path }, { kind, path: deployedPath }) + } + runtime.syncPreviewWithDeployed(session.workspace_id, kind, deployedPath) }) setGetRuntimeLogsHandler(async ({ sessionId: callerSessionId, limit }) => {