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 8574a9cdf2..55cdf5e48a 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -3316,6 +3316,55 @@ describe('global AI tools', () => { expect(ScriptService.createScript).not.toHaveBeenCalled() }) + // The shared deployer deploys a script at the path inside the draft, so the deployed + // path is the chosen name — what the preview follows and the mask entry moves to. + it('reports where a draft-only script landed, not the key it was stored at', async () => { + seedBackendDraft('script', 'u/admin/draft_s1', { + path: 'f/team/new_script', + summary: 's', + description: '', + content: 'export async function main() { return 1 }', + language: 'bun', + kind: 'script', + schema: {} + }) + vi.mocked(ScriptService.existsScriptByPath).mockResolvedValue(false) + // What the shared deployer reads to deploy at the draft's own path. + vi.mocked(ScriptService.getScriptByPath).mockResolvedValue({ + hash: 'h1', + draft: { + path: 'f/team/new_script', + summary: 's', + content: 'export async function main() { return 1 }', + language: 'bun', + kind: 'script', + schema: {} + } + } as any) + const onDeployed = vi.fn() + setDeployedInSessionHandler(onDeployed) + try { + await callGlobalTool('deploy_workspace_item', { + type: 'script', + path: 'u/admin/draft_s1' + }) + expect(ScriptService.createScript).toHaveBeenCalledWith( + expect.objectContaining({ + requestBody: expect.objectContaining({ path: 'f/team/new_script' }) + }) + ) + expect(onDeployed).toHaveBeenCalledWith( + expect.objectContaining({ + kind: 'script', + path: 'u/admin/draft_s1', + deployedPath: 'f/team/new_script' + }) + ) + } finally { + setDeployedInSessionHandler(undefined) + } + }) + it('deploys a stale script draft when force is set', async () => { seedStaleScriptDraft('f/scripts/stale', 'base-hash') mockDeployedScript('f/scripts/stale', 'new-hash') diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index 3dc3fa3058..3c8bafc8cb 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -226,6 +226,7 @@ import { getGlobalDraft, getGlobalDraftStoragePath, itemKindFor, + chosenDraftName, listGlobalDrafts, loadDraftNames, persistGlobalDraft, @@ -8022,6 +8023,11 @@ async function deployDraft( type === 'flow' ? !(await FlowService.existsFlowByPath({ workspace, path: storagePath })) : false + // The shared deployer deploys at the path inside the draft and doesn't report it + // back, so read it here — post-flush, since a rename may have been parked. This is + // where a draft-only or renamed item lands, and what the callbacks below must name. + deployedPath = + chosenDraftName(type, await readGlobalDraftValue(workspace, type, storagePath)) ?? storagePath const result = await deployDraftToWorkspace(type, storagePath, workspace, { draftOnly, deploymentMessage @@ -8218,13 +8224,15 @@ 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. - // `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) { + // `force` means "deploy over a newer version of this item" and cannot answer + // this: the app at the target is a different item, so there is nothing to + // overwrite on purpose. Discarding is the way out of the one ambiguous case, + // a deploy that created the app and then failed to remove its draft. + if (targetExists && targetPath !== storagePath) { 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, or pass force: true if this draft ` + - `is what is deployed there.` + `Ask the user for a different path for this draft. If instead this draft is what is ` + + `already deployed there, discard_local_draft removes the leftover draft.` ) } if (targetExists) { diff --git a/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts b/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts index 353098c79a..3eb1cf3f92 100644 --- a/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts +++ b/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts @@ -134,10 +134,10 @@ function scriptDraftToWorkspaceItem(path: string, draft: NewScript): WorkspaceIt return { type: 'script', path, - // The session editor parks a rename in the draft's `draft_path` (see - // sessionDraftCodecs.ts); surface it so lists/pickers show the friendly - // name instead of the `draft_` storage key. - draftPath: (draft as NewScript & { draft_path?: string }).draft_path, + // A script's chosen name is the value's own `path` — the editor binds the Path + // widget to it — so a rename or a `draft_` storage key shows up there, and + // the session editor may also park one in `draft_path`. + draftPath: chosenDraftName('script', draft) ?? (draft as { draft_path?: string }).draft_path, summary: draft.summary, language: draft.language, value: draft.content, @@ -318,7 +318,7 @@ const draftNamesByWorkspace = new Map() // Scripts keep their chosen path in the value's own `path`; the other kinds in `draft_path` // (the same split listDrafts applies server-side). -function chosenDraftName(itemKind: UserDraftItemKind, value: unknown): string | undefined { +export function chosenDraftName(itemKind: UserDraftItemKind, value: unknown): string | undefined { const v = value as { path?: string; draft_path?: string } | null | undefined return (itemKind === 'script' ? v?.path : v?.draft_path) || undefined }