From c71733681fc7dc43e15d71b141131e79d8e38d93 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Thu, 24 Sep 2026 08:14:33 +0200 Subject: [PATCH] refactor: resolve a draft's key at the tool boundary, once per call A tool declares the draft it addresses with `draftTarget`; the dispatch resolves it before the body runs and hands down the storage key. The adapter's read, write and delete functions now take that key, so a tool's read and the write that follows it cannot disagree about which draft they mean. `resolveDraftTarget` is the only way to turn a path into a key. Two lookups stay below the boundary and say so: a flow step's script and a subflow are other items than the tool's target. Co-Authored-By: Claude Opus 5 (1M context) --- .../copilot/chat/global/core.test.ts | 105 +++++- .../components/copilot/chat/global/core.ts | 339 +++++++++++------- .../copilot/chat/global/userDraftAdapter.ts | 154 +++----- .../src/lib/components/copilot/chat/shared.ts | 25 +- 4 files changed, 367 insertions(+), 256 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 7d1d493262..43f31cb61a 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -395,7 +395,8 @@ import { deleteGlobalDraft, listGlobalDrafts, persistGlobalDraft, - readGlobalDraftValueAt, + readGlobalDraftValue, + resolveDraftTarget, saveGlobalAppDraft } from './userDraftAdapter' import { bundleRawAppDraft } from './rawAppBundlerBridge' @@ -466,12 +467,27 @@ async function callGlobalTool( callbacks: ToolCallbacks = toolCallbacks, helpers: Record = {} ): Promise { - return getGlobalTool(name).fn({ + return runGlobalTool(name, args, WORKSPACE, callbacks, helpers) +} + +/** Mirrors the dispatch in shared.ts: the tool's declared draft is resolved once, before + * its body runs. Calling `fn` without this would hand the body no storage key at all. */ +async function runGlobalTool( + name: string, + args: Record, + workspace: string, + callbacks: ToolCallbacks = toolCallbacks, + helpers: Record = {} +): Promise { + const tool = getGlobalTool(name) + const target = await tool.draftTarget?.({ args, workspace }) + return tool.fn({ args, - workspace: WORKSPACE, + workspace, helpers, toolCallbacks: callbacks, - toolId: `test-${name}` + toolId: `test-${name}`, + target }) } @@ -2502,7 +2518,7 @@ describe('global AI tools', () => { it('refuses to write to an unresolvable path when the drafts cannot be listed', async () => { const workspace = 'ws-unlistable-drafts' const call = (name: string, args: Record) => - getGlobalTool(name).fn({ args, workspace, helpers: {}, toolCallbacks, toolId: 'no-names' }) + runGlobalTool(name, args, workspace) const listing = vi.mocked(DraftService.listDrafts).getMockImplementation() vi.mocked(DraftService.listDrafts).mockRejectedValue(new Error('server error')) @@ -2592,7 +2608,7 @@ describe('global AI tools', () => { // Deleting the deployed item removes the reason its path outranks a draft staged under // that name, so a cleanup resolved afterwards would delete that unrelated draft. - it('leaves alone a draft staged under a deleted item\'s path', async () => { + it("leaves alone a draft staged under a deleted item's path", async () => { seedBackendDraft( 'raw_app', 'u/admin/draft_namesake', @@ -2928,7 +2944,7 @@ describe('global AI tools', () => { // directly: a non-force save whose recorded baseline is older than the // server row is rejected with `status:'conflict'`, and `override` (force) // pushes our version through. NB: this targets persistGlobalDraft, not the - // write_* tools — those re-read the backend first (readGlobalDraftValueAt -> + // write_* tools — those re-read the backend first (readGlobalDraftValue -> // recordRemoteSync), which re-seeds the baseline and so can only surface a // conflict when a live editor cell is mounted (not the case in unit tests). it('persistGlobalDraft surfaces a conflict on a stale baseline and override forces it', async () => { @@ -2950,7 +2966,8 @@ describe('global AI tools', () => { ) const v2 = { ...v1, summary: 'v2', content: 'export function main() { return 1 }' } - const conflict = await persistGlobalDraft(WORKSPACE, 'script', path, v2) + const target = await resolveDraftTarget(WORKSPACE, { type: 'script', path }) + const conflict = await persistGlobalDraft(WORKSPACE, 'script', target, v2) expect(conflict.status).toBe('conflict') if (conflict.status === 'conflict') { expect(conflict.serverTimestamp).toBe('2026-06-15T00:01:00Z') @@ -2961,7 +2978,7 @@ describe('global AI tools', () => { }) // override:true bypasses the check and persists our version. - const forced = await persistGlobalDraft(WORKSPACE, 'script', path, v2, { force: true }) + const forced = await persistGlobalDraft(WORKSPACE, 'script', target, v2, { force: true }) expect(forced.status).toBe('saved') expect(getBackendDraft('script', path, { workspace: WORKSPACE })).toMatchObject({ summary: 'v2', @@ -2981,7 +2998,12 @@ describe('global AI tools', () => { content: 'export function main() {}', language: 'bun' } - const res = await persistGlobalDraft(WORKSPACE, 'script', path, v) + const res = await persistGlobalDraft( + WORKSPACE, + 'script', + await resolveDraftTarget(WORKSPACE, { type: 'script', path }), + v + ) expect(res.status).toBe('error') if (res.status === 'error') expect(res.message).toBeTruthy() // Nothing was persisted. @@ -2993,7 +3015,9 @@ describe('global AI tools', () => { it('a non-404 backend read failure propagates instead of returning undefined', async () => { const path = 'f/scripts/readfail' failingReads.add(`script:${path}`) - await expect(readGlobalDraftValueAt(WORKSPACE, 'script', path)).rejects.toThrow() + await expect( + readGlobalDraftValue(WORKSPACE, 'script', { path, storagePath: path, fetched: false }) + ).rejects.toThrow() }) // Raw-app writes go through saveGlobalAppDraft, which must carry the conflict @@ -3007,11 +3031,11 @@ describe('global AI tools', () => { { workspace: WORKSPACE, itemKind: 'raw_app', path }, '2026-06-15T00:00:00Z' ) - const res = await saveGlobalAppDraft(WORKSPACE, path, { - summary: 'v2', - files: {}, - runnables: {} - } as any) + const res = await saveGlobalAppDraft( + WORKSPACE, + await resolveDraftTarget(WORKSPACE, { type: 'app', path }), + { summary: 'v2', files: {}, runnables: {} } as any + ) expect(res.status).toBe('conflict') }) @@ -3026,7 +3050,13 @@ describe('global AI tools', () => { language: 'bun' }) failingWrites.add(`script:${path}`) - await expect(deleteGlobalDraft(WORKSPACE, 'script', path)).rejects.toThrow() + await expect( + deleteGlobalDraft( + WORKSPACE, + 'script', + await resolveDraftTarget(WORKSPACE, { type: 'script', path }) + ) + ).rejects.toThrow() }) // `override` is a tool-only conflict flag and must not leak into the persisted @@ -3092,6 +3122,47 @@ describe('global AI tools', () => { ).toBeUndefined() }) + // write_trigger is the one tool whose path is nested inside another argument, so its + // target declaration has to reach into `config` to find the name to resolve. + it('routes a trigger write nested in config to the draft staged under that name', async () => { + seedBackendDraft( + 'trigger_http', + 'u/admin/draft_http_0001', + { + path: 'u/admin/draft_http_0001', + draft_path: 'u/admin/staged_route', + script_path: 'f/scripts/handler', + is_flow: false, + route_path: 'api/staged', + http_method: 'get', + authentication_method: 'none', + is_static_website: false + }, + { workspace: WORKSPACE } + ) + + await callGlobalTool('write_trigger', { + kind: 'http', + config: { + path: 'u/admin/staged_route', + script_path: 'f/scripts/handler', + is_flow: false, + route_path: 'api/renamed', + http_method: 'get', + authentication_method: 'none', + is_static_website: false + } + }) + + expect( + getBackendDraft('trigger_http', 'u/admin/draft_http_0001', { workspace: WORKSPACE }) + ?.route_path + ).toBe('api/renamed') + expect( + getBackendDraft('trigger_http', 'u/admin/staged_route', { workspace: WORKSPACE }) + ).toBeUndefined() + }) + // Same private-owner read path as schedules, for the trigger drawer kinds. it('reads and deploys a trigger draft written by the chat', async () => { await callGlobalTool('write_trigger', { diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index b9612a9760..311cc2e207 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -134,6 +134,7 @@ import { SPECIAL_MODULE_IDS, type CreatedResourceTriggerKind, type PreviewCardKind, + type ResolvedDraftTarget, type RunFormDisplay, type Tool, type ToolCallbacks, @@ -226,12 +227,11 @@ import { getGlobalDraft, itemKindFor, chosenDraftName, - getGlobalDraftAt, listGlobalDrafts, liveGlobalDraftStoragePath, - readGlobalDraftValueAt, - resolveGlobalDraft, - resolveGlobalDraftStoragePath, + type DraftTarget, + readGlobalDraftValue, + resolveDraftTarget, persistGlobalDraft, readLocalDraftCellByKind, resolveGlobalDraftStoragePathByKind, @@ -1727,12 +1727,6 @@ type AppMetadata = { data?: any } -type LoadedAppDraftValue = { - value: AppDraftValue - /** The key this value came from, which the write that follows must save back to. */ - storagePath: string -} - function summarizeAppValue(value: AppDraftValue): AppMetadata { const frontend: AppFrontendFileMetadata[] = Object.entries(value.files).map( ([path, content]) => ({ @@ -1857,36 +1851,19 @@ async function getRawAppByPath(workspace: string, path: string): Promise { - const draft = await getGlobalDraft(workspace, 'app', path) +async function loadAppDraftValue( + target: ResolvedDraftTarget, + workspace: string +): Promise { + const draft = await getGlobalDraft(workspace, 'app', target) if (draft && draft.value && typeof draft.value === 'object' && 'files' in draft.value) { return draft.value as AppDraftValue } - const app = await getRawAppByPath(workspace, path) + const app = await getRawAppByPath(workspace, target.path) return appSourceToDraftValue(app, app) } -async function loadAppDraftValue( - path: string, - workspace: string, - /** A write must know which draft a name belongs to; a test run reads whatever the path - * reaches, as it did before names were resolved at all. */ - opts: { forWrite?: boolean } = {} -): Promise { - // One resolution for the read and the write that follows it: resolving again at save - // time could answer differently and write the app's files onto another draft. - const resolved = await resolveGlobalDraft(workspace, 'app', path, undefined, opts) - const storagePath = resolved.storagePath - const draft = await getGlobalDraftAt(workspace, 'app', storagePath, undefined, resolved) - if (draft && draft.value && typeof draft.value === 'object' && 'files' in draft.value) { - return { value: draft.value as AppDraftValue, storagePath } - } - - const app = await getRawAppByPath(workspace, path) - return { value: appSourceToDraftValue(app, app), storagePath } -} - /** * A live editor's draft is displayed under its chosen name, which resolves to the draft only * while that editor stays open. The model is handed the storage path instead, which keeps @@ -1902,11 +1879,10 @@ function addressedByStoragePath(workspace: string, item: WorkspaceItem): Workspa async function saveAppDraft( workspace: string, - path: string, - value: AppDraftValue, - storagePath?: string + target: ResolvedDraftTarget, + value: AppDraftValue ): Promise { - return saveGlobalAppDraft(workspace, path, value, { storagePath }) + return saveGlobalAppDraft(workspace, target, value) } type TriggerLike = { path: string; summary?: string | null } @@ -3553,18 +3529,22 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'read_workspace_item', 'Read one workspace item or draft. Prefers your draft when one exists; pass version: "deployed" to read the deployed state instead.' ), + draftTarget: draftTarget((args) => + args.version === 'deployed' || isHubPath(args.path) + ? undefined + : { type: args.type, path: args.path, triggerKind: args.trigger_kind } + ), planModeSafe: true, - fn: async ({ args, workspace, toolId, toolCallbacks }) => { + fn: async ({ args, workspace, toolId, toolCallbacks, target }) => { const parsed = readWorkspaceItemSchema.parse(args) if (parsed.type === 'trigger' && !parsed.trigger_kind) { const message = 'trigger_kind is required when type is trigger.' toolCallbacks.setToolStatus(toolId, { content: message, error: message }) return JSON.stringify({ success: false, error: message }) } - const draft = - parsed.version === 'deployed' || isHubPath(parsed.path) - ? null - : await getGlobalDraft(workspace, parsed.type, parsed.path, parsed.trigger_kind) + const draft = target + ? await getGlobalDraft(workspace, parsed.type, target, parsed.trigger_kind) + : null if (draft) { toolCallbacks.setToolStatus(toolId, { content: `Read draft ${parsed.type} "${parsed.path}"` @@ -3647,6 +3627,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ }, { def: createToolDef(writeScriptSchema, 'write_script', 'Create or overwrite a draft script.'), + draftTarget: draftTarget((args) => ({ type: 'script', path: args.path, forWrite: true })), showDetails: true, streamArguments: true, showFade: true, @@ -3657,6 +3638,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ }, { def: createToolDef(writeFlowSchema, 'write_flow', 'Create or overwrite a draft flow.'), + draftTarget: draftTarget((args) => ({ type: 'flow', path: args.path, forWrite: true })), showDetails: true, streamArguments: true, showFade: true, @@ -3679,7 +3661,11 @@ const unroutedGlobalTools: Tool<{}>[] = [ }, { aiProviders, aiProviderWarnings } ) - const resolved = await resolveWriteFlowInlineScripts(parsed.path, editable, ctx.workspace) + const resolved = await resolveWriteFlowInlineScripts( + draftTargetOf(ctx), + editable, + ctx.workspace + ) const result = await writeFlowDraft( { path: parsed.path, @@ -3703,6 +3689,12 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'Create or overwrite a draft schedule.', { strict: false } ), + draftTarget: draftTarget((args) => ({ + type: 'schedule', + // `advanced` may carry a key the definition already lists, including this one. + path: args.path ?? (args.advanced as { path?: string } | undefined)?.path, + forWrite: true + })), showDetails: true, streamArguments: true, showFade: true, @@ -3728,6 +3720,12 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'Create or overwrite a draft trigger.', { strict: false } ), + draftTarget: draftTarget((args) => ({ + type: 'trigger', + path: (args.config as { path?: string } | undefined)?.path, + triggerKind: args.kind, + forWrite: true + })), showDetails: true, streamArguments: true, showFade: true, @@ -3775,6 +3773,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'edit_script', 'Find/replace exact text in a script and save a draft.' ), + draftTarget: draftTarget((args) => ({ type: 'script', path: args.path, forWrite: true })), showDetails: true, streamArguments: true, showFade: true, @@ -3789,6 +3788,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'patch_flow_json', 'Find/replace exact text in compact flow JSON and save a draft.' ), + draftTarget: draftTarget((args) => ({ type: 'flow', path: args.path, forWrite: true })), showDetails: true, streamArguments: true, showFade: true, @@ -3799,6 +3799,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ }, { def: testRunScriptToolDef, + draftTarget: draftTarget((args) => ({ type: 'script', path: args.path })), fn: async (ctx) => { const parsed = testRunScriptSchema.parse(ctx.args) return testRunScriptByPath(parsed, ctx) @@ -3830,6 +3831,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ }, { def: testRunFlowToolDef, + draftTarget: draftTarget((args) => ({ type: 'flow', path: args.path })), fn: async (ctx) => { const parsed = testRunFlowSchema.parse(ctx.args) return testRunFlowByPath(parsed, ctx) @@ -3857,6 +3859,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ }, { def: testRunStepToolDef, + draftTarget: draftTarget((args) => ({ type: 'flow', path: args.path })), fn: async (ctx) => { const parsed = testRunStepSchema.parse(ctx.args) return testRunFlowStepByPath(parsed, ctx) @@ -4004,6 +4007,11 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'Deploy a draft to the workspace. Mutates the workspace.', { strict: false } ), + draftTarget: draftTarget((args) => ({ + type: args.type, + path: args.path, + triggerKind: args.trigger_kind + })), showDetails: true, showFade: true, requiresConfirmation: true, @@ -4020,6 +4028,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'Discard a stale script, flow, or app draft and return your changes as a diff to re-apply on the latest deployed version. Use when deploy_workspace_item reports the draft was started from an older deployed version.', { strict: false } ), + draftTarget: draftTarget((args) => ({ type: args.type, path: args.path })), showDetails: true, showFade: true, fn: async (ctx) => { @@ -4056,6 +4065,11 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'delete_workspace_item', 'Delete an item that is already deployed in the workspace. Mutates the workspace. FAILS if the path has no deployed item, so never call it to undo something you created in this chat — that is a draft; use discard_local_draft instead.' ), + draftTarget: draftTarget((args) => ({ + type: args.type, + path: args.path, + triggerKind: args.trigger_kind + })), showDetails: true, showFade: true, requiresConfirmation: true, @@ -4072,6 +4086,11 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'discard_local_draft', 'Discard a draft only — the tool to undo an item you created or edited in this chat and have not deployed. Does not mutate deployed workspace items, but clears the matching open editor draft if one is mounted.' ), + draftTarget: draftTarget((args) => ({ + type: args.type, + path: args.path, + triggerKind: args.trigger_kind + })), showDetails: true, showFade: true, requiresConfirmation: true, @@ -4088,6 +4107,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'Create or overwrite a draft resource.', { strict: false } ), + draftTarget: draftTarget((args) => ({ type: 'resource', path: args.path, forWrite: true })), showDetails: true, streamArguments: true, showFade: true, @@ -4103,6 +4123,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'Create or edit a draft variable. Editing is a partial update: pass only the fields you are changing. You can never read the value of a variable, so to edit a secret variable you MUST omit value — any value you pass replaces the stored secret. FAILS if you pass a "$var:" self-reference as the value, or un-secret a variable without giving its new value.', { strict: false } ), + draftTarget: draftTarget((args) => ({ type: 'variable', path: args.path, forWrite: true })), showDetails: true, streamArguments: true, showFade: true, @@ -4152,6 +4173,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'read_flow_module_code', 'Read inline script code from one flow module.' ), + draftTarget: draftTarget((args) => ({ type: 'flow', path: args.path })), planModeSafe: true, fn: async (ctx) => { const parsed = readFlowModuleCodeSchema.parse(ctx.args) @@ -4164,6 +4186,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'set_flow_module_code', 'Overwrite inline script code in one flow module and save a draft.' ), + draftTarget: draftTarget((args) => ({ type: 'flow', path: args.path, forWrite: true })), showDetails: true, streamArguments: true, showFade: true, @@ -4179,6 +4202,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'Initialize a draft raw app from a framework template.', { strict: false } ), + draftTarget: draftTarget((args) => ({ type: 'app', path: args.path, forWrite: true })), showDetails: true, showFade: true, fn: async (ctx) => { @@ -4192,6 +4216,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'read_app_file', 'Read one raw app frontend file or inline backend runnable. Large files are truncated to a head slice; pass offset/limit to page through the rest.' ), + draftTarget: draftTarget((args) => ({ type: 'app', path: args.path })), planModeSafe: true, fn: async (ctx) => { const parsed = readAppFileSchema.parse(ctx.args) @@ -4204,6 +4229,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'search_app', "Grep across all of a raw app's frontend files and inline backend runnables in one call. Returns matching file:line rows (capped), not file bodies — use it to locate a symbol or string before read_app_file instead of reading whole files one by one." ), + draftTarget: draftTarget((args) => ({ type: 'app', path: args.path })), planModeSafe: true, fn: async (ctx) => { const parsed = searchAppSchema.parse(ctx.args) @@ -4216,6 +4242,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'write_app_file', 'Create or overwrite a frontend file in a local app draft.' ), + draftTarget: draftTarget((args) => ({ type: 'app', path: args.path, forWrite: true })), showDetails: true, streamArguments: true, showFade: true, @@ -4230,6 +4257,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'delete_app_file', 'Remove a frontend file from a local app draft.' ), + draftTarget: draftTarget((args) => ({ type: 'app', path: args.path, forWrite: true })), fn: async (ctx) => { const parsed = deleteAppFileSchema.parse(ctx.args) return deleteAppFile(parsed, ctx) @@ -4241,6 +4269,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'patch_app_file', 'Find/replace exact text in a raw app file and save a draft.' ), + draftTarget: draftTarget((args) => ({ type: 'app', path: args.path, forWrite: true })), showDetails: true, streamArguments: true, showFade: true, @@ -4256,6 +4285,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'Create or overwrite a backend runnable in a local app draft.', { strict: false } ), + draftTarget: draftTarget((args) => ({ type: 'app', path: args.path, forWrite: true })), showDetails: true, streamArguments: true, showFade: true, @@ -4270,6 +4300,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'delete_app_runnable', 'Remove a backend runnable from a local app draft.' ), + draftTarget: draftTarget((args) => ({ type: 'app', path: args.path, forWrite: true })), fn: async (ctx) => { const parsed = deleteAppRunnableSchema.parse(ctx.args) return deleteAppRunnable(parsed, ctx) @@ -4277,6 +4308,7 @@ const unroutedGlobalTools: Tool<{}>[] = [ }, { def: testRunAppRunnableToolDef, + draftTarget: draftTarget((args) => ({ type: 'app', path: args.path })), fn: async (ctx) => { const parsed = testRunAppRunnableSchema.parse(ctx.args) return testRunAppRunnable(parsed, ctx) @@ -4295,9 +4327,14 @@ const unroutedGlobalTools: Tool<{}>[] = [ 'open_preview', 'Open the live preview / editor for a workspace item in the side panel next to the chat. ONLY works inside an AI session — call this after writing or editing a script, flow, or raw app to let the user see and interact with it. The path you pass is the path of the item; for code-based apps use kind="raw_app" (legacy drag-and-drop apps are not previewable). Returns an error if there is no active session.' ), + draftTarget: draftTarget((args) => + args.kind === 'pipeline' + ? undefined + : { type: args.kind === 'raw_app' ? 'app' : args.kind, path: args.path } + ), fn: async (ctx) => { const parsed = openPreviewSchema.parse(ctx.args) - return openSessionPreview(parsed, ctx.workspace, sessionIdFromCtx(ctx)) + return openSessionPreview(parsed, ctx.target, sessionIdFromCtx(ctx)) } }, { @@ -4522,6 +4559,34 @@ type WriteDraftCtx = { // Present when the ctx is the raw tool `fn` context — session chats carry // their id here (see SessionToolHelpers / sessionIdFromCtx). helpers?: unknown + // Resolved by the dispatch from the tool's `draftTarget`, before the body runs. + target?: ResolvedDraftTarget +} + +/** + * Declares the draft a tool addresses. The dispatch calls this once, before the body runs, + * and hands the body the storage key as `ctx.target` — so a tool's read, its write and its + * cleanup cannot end up on different drafts. Return undefined for a call that addresses no + * draft, such as a read pinned to the deployed version. + */ +function draftTarget( + describe: (args: any) => (Omit & { path?: string }) | undefined +): (p: { args: any; workspace: string }) => Promise { + return async ({ args, workspace }) => { + // A malformed call has no path to resolve; the body's own schema parse reports it. + const target = describe(args ?? {}) + if (!target?.path) return undefined + return resolveDraftTarget(workspace, target as DraftTarget) + } +} + +/** The draft this call addresses. Absent means the tool did not declare `draftTarget`; + * failing here beats writing to a path that was never resolved to a storage key. */ +function draftTargetOf(ctx: WriteDraftCtx): ResolvedDraftTarget { + if (!ctx.target) { + throw new Error('Internal error: this tool did not declare the draft it addresses.') + } + return ctx.target } // Sessions are the only context where `open_preview` makes sense — the global @@ -4608,7 +4673,7 @@ export function setOpenPreviewHandler(handler: OpenPreviewHandler | undefined): async function openSessionPreview( args: { kind: 'script' | 'flow' | 'raw_app' | 'pipeline'; path: string }, - workspace: string, + target: ResolvedDraftTarget | undefined, sessionId: string | undefined ): Promise { if (!openPreviewHandler) { @@ -4618,14 +4683,7 @@ async function openSessionPreview( // For a pipeline the handler awaits the editor's tool registration, so the // model's next build_pipeline_node call can't race the async canvas mount. // The preview loads the literal path, so a draft's chosen name is routed here. - const path = - args.kind === 'pipeline' - ? args.path - : await resolveGlobalDraftStoragePath( - workspace, - args.kind === 'raw_app' ? 'app' : args.kind, - args.path - ) + const path = args.kind === 'pipeline' ? args.path : (target?.storagePath ?? args.path) return await openPreviewHandler({ ...args, path, sessionId }) } @@ -5143,18 +5201,12 @@ async function writeDraft( opts: { triggerKind?: TriggerKind; override?: boolean } = {} ): Promise { const { workspace } = ctx + const target = draftTargetOf(ctx) startDraftWrite(ctx, type, path) - // One resolution for the merge base and the write: resolving twice lets a listing that - // fails in between base the merge on the deployed item and write that over the draft. - const resolved = await resolveGlobalDraft(workspace, type, path, opts.triggerKind, { - forWrite: true + const existingDraft = await readGlobalDraftValue(workspace, type, target, { + triggerKind: opts.triggerKind }) - const existingDraft = ( - resolved.fetched - ? resolved.value - : await readGlobalDraftValueAt(workspace, type, resolved.storagePath, opts.triggerKind) - ) as T | undefined let base = existingDraft let existed = existingDraft !== undefined if (base === undefined && (await spec.probe(workspace, path))) { @@ -5164,10 +5216,9 @@ async function writeDraft( const draft = await spec.buildDraft(base, args, path) - const result = await persistGlobalDraft(workspace, type, path, draft, { + const result = await persistGlobalDraft(workspace, type, target, draft, { triggerKind: opts.triggerKind, - force: opts.override, - storagePath: resolved.storagePath + force: opts.override }) return finishDraftWrite(result, existed, ctx) } @@ -5370,7 +5421,7 @@ function writeVariableDraft(args: WriteVariableArgs, ctx: WriteDraftCtx): Promis } async function loadScriptForEdit( - path: string, + target: ResolvedDraftTarget, workspace: string ): Promise<{ content: string @@ -5378,7 +5429,8 @@ async function loadScriptForEdit( summary?: string schema?: Record }> { - const draft = await getGlobalDraft(workspace, 'script', path) + const path = target.path + const draft = await getGlobalDraft(workspace, 'script', target) if (draft) { if (typeof draft.value !== 'string' || !draft.language) { throw new Error(`Draft script "${path}" is missing content or language.`) @@ -5431,7 +5483,7 @@ async function editScript( const { path, old_string: oldString, new_string: newString, replace_all: replaceAll } = args ctx.toolCallbacks.setToolStatus(ctx.toolId, { content: `Editing script "${path}"...` }) - const base = await loadScriptForEdit(path, ctx.workspace) + const base = await loadScriptForEdit(draftTargetOf(ctx), ctx.workspace) const updated = findAndReplace(base.content, oldString, newString, replaceAll, 'script source') return writeScriptDraft( { @@ -5445,12 +5497,13 @@ async function editScript( } async function loadFlowDraftValue( - path: string, + target: ResolvedDraftTarget, workspace: string // `isDraft` says which of the two this came from. Reported here because the draft lookup // is a request of its own: a caller that needs to know would otherwise repeat it. ): Promise<{ flow: FlowDraftValue; summary?: string; isDraft: boolean }> { - const draft = await getGlobalDraft(workspace, 'flow', path) + const path = target.path + const draft = await getGlobalDraft(workspace, 'flow', target) if (draft) { if (draft.value === undefined || typeof draft.value === 'string') { throw new Error(`Draft flow "${path}" has no value.`) @@ -5473,7 +5526,7 @@ async function loadFlowDraftValue( * (to fill via set_flow_module_code), and anything else rejects the write. */ async function resolveWriteFlowInlineScripts( - path: string, + target: ResolvedDraftTarget, editable: EditableFlowJson, workspace: string ): Promise { @@ -5489,10 +5542,10 @@ async function resolveWriteFlowInlineScripts( const session = createInlineScriptSession() if ( - (await getGlobalDraft(workspace, 'flow', path)) || - (await FlowService.existsFlowByPath({ workspace, path })) + (await getGlobalDraft(workspace, 'flow', target)) || + (await FlowService.existsFlowByPath({ workspace, path: target.path })) ) { - const base = await loadFlowDraftValue(path, workspace) + const base = await loadFlowDraftValue(target, workspace) buildEditableFlowJson(flowDraftAsEditableInput(base.flow), session) } const resolved: EditableFlowJson = { @@ -5516,7 +5569,7 @@ async function patchFlowJson( // bodies don't appear in the JSON the model sees or patches. Real script // content is preserved through the patch via the InlineScriptSession; the // model uses set_flow_module_code to change inline script bodies. - const base = await loadFlowDraftValue(path, ctx.workspace) + const base = await loadFlowDraftValue(draftTargetOf(ctx), ctx.workspace) const session = createInlineScriptSession() const editable = buildEditableFlowJson(flowDraftAsEditableInput(base.flow), session) const currentJson = JSON.stringify(editable) @@ -5578,7 +5631,7 @@ async function readFlowModuleCode( toolCallbacks.setToolStatus(toolId, { content: `Reading inline script for module "${args.module_id}" from flow "${args.path}"...` }) - const base = await loadFlowDraftValue(args.path, workspace) + const base = await loadFlowDraftValue(draftTargetOf(ctx), workspace) const session = createInlineScriptSession() buildEditableFlowJson(flowDraftAsEditableInput(base.flow), session) const content = session.get(args.module_id) @@ -5601,7 +5654,7 @@ async function setFlowModuleCode( toolCallbacks.setToolStatus(toolId, { content: `Updating inline script for module "${args.module_id}" in flow "${args.path}"...` }) - const base = await loadFlowDraftValue(args.path, workspace) + const base = await loadFlowDraftValue(draftTargetOf(ctx), workspace) const session = createInlineScriptSession() const editable = buildEditableFlowJson(flowDraftAsEditableInput(base.flow), session) if (!session.has(args.module_id)) { @@ -5643,7 +5696,10 @@ async function loadScriptForFlowStep( moduleValue: { path: string; hash?: string }, workspace: string ): Promise<{ content: string; language: ScriptLang; schema?: Record }> { - const draft = await getGlobalDraft(workspace, 'script', moduleValue.path) + // Another item than the tool's target: a step points at a script of its own, so this + // is the one place below the boundary that resolves a path. + const target = await resolveDraftTarget(workspace, { type: 'script', path: moduleValue.path }) + const draft = await getGlobalDraft(workspace, 'script', target) if (draft) { if (typeof draft.value !== 'string' || !draft.language) { throw new Error(`Draft script "${moduleValue.path}" is missing content or language.`) @@ -5671,7 +5727,10 @@ async function loadSubflowForFlowStep( path: string, workspace: string ): Promise<{ previewValue?: FlowValue; schema?: Record }> { - const nestedFlow = await loadFlowDraftValue(path, workspace) + const nestedFlow = await loadFlowDraftValue( + await resolveDraftTarget(workspace, { type: 'flow', path }), + workspace + ) return { // Only a draft is previewed; a deployed subflow is run by path, as its parent flow // would run it. The schema describes whichever of the two that leaves. @@ -5692,7 +5751,7 @@ async function testRunScriptByPath( ctx: WriteDraftCtx ): Promise { const { workspace } = ctx - const script = await loadScriptForEdit(args.path, workspace) + const script = await loadScriptForEdit(draftTargetOf(ctx), workspace) const schema = await schemaForTestRun(script) return runThroughForm( @@ -6076,7 +6135,7 @@ async function testRunFlowByPath( // The schema must be in hand before the form is built, and the value rides along from the // same read so the fields and the previewed flow are one version. With an editor open on // this path this reads its in-memory cell rather than the network. - const flow = await loadFlowDraftValue(args.path, workspace) + const flow = await loadFlowDraftValue(draftTargetOf(ctx), workspace) const schema = (flow.flow.schema as Record | null | undefined) ?? {} return runThroughForm( @@ -6129,7 +6188,7 @@ async function testRunFlowStepByPath( ctx: WriteDraftCtx ): Promise { const { workspace, toolId, toolCallbacks } = ctx - const flow = await loadFlowDraftValue(args.path, workspace) + const flow = await loadFlowDraftValue(draftTargetOf(ctx), workspace) const resolved = await resolveFlowStepRun({ flowValue: flowDraftValueForPreview(flow.flow), stepId: args.stepId, @@ -6198,7 +6257,8 @@ async function initApp( const { workspace, toolId, toolCallbacks } = ctx const { path, summary, framework } = args - if (await getGlobalDraft(workspace, 'app', path)) { + const target = draftTargetOf(ctx) + if (await getGlobalDraft(workspace, 'app', target)) { throw new Error( `A draft for app "${path}" already exists. Use write_app_file / write_app_runnable to modify it, or delete the existing draft first.` ) @@ -6220,7 +6280,7 @@ async function initApp( runnables: { [STARTER_RUNNABLE_KEY]: { ...STARTER_RUNNABLE } } } await recomputeAppPolicy(value) - const result = await saveAppDraft(workspace, path, value) + const result = await saveAppDraft(workspace, target, value) return finishAppDraftWrite(result, ctx, () => ({ content: `Saved app "${path}" draft (${framework})`, message: `Initialized a per-user draft app "${path}" from the ${framework} template with a starter runnable "${STARTER_RUNNABLE_KEY}" (saved server-side, not a deployed workspace item). Use write_app_file / write_app_runnable to evolve it.` @@ -6318,7 +6378,7 @@ async function readAppFile( content: `Reading ${target.filePath} from app "${args.path}"...` }) - const value = await loadAppValueForRead(args.path, workspace) + const value = await loadAppDraftValue(draftTargetOf(ctx), workspace) let content: string if (target.kind === 'frontend') { @@ -6408,7 +6468,7 @@ async function searchApp( content: `Searching app "${args.path}" for "${query}"...` }) - const value = await loadAppValueForRead(args.path, workspace) + const value = await loadAppDraftValue(draftTargetOf(ctx), workspace) const maxMatches = Math.min( args.max_matches ?? SEARCH_APP_DEFAULT_MAX_MATCHES, SEARCH_APP_MAX_MATCHES_CEILING @@ -6523,9 +6583,10 @@ async function writeAppFile( content: `Writing ${target.filePath} to app "${args.path}"...` }) - const { value, storagePath } = await loadAppDraftValue(args.path, workspace, { forWrite: true }) + const draft = draftTargetOf(ctx) + const value = await loadAppDraftValue(draft, workspace) value.files = { ...value.files, [target.filePath]: args.content } - const result = await saveAppDraft(workspace, args.path, value, storagePath) + const result = await saveAppDraft(workspace, draft, value) return finishAppDraftWrite(result, ctx, () => ({ content: `Updated ${target.filePath} in app "${args.path}"`, message: `Updated draft app "${args.path}" with frontend file "${target.filePath}".` @@ -6549,13 +6610,14 @@ async function deleteAppFile( content: `Deleting ${target.filePath} from app "${args.path}"...` }) - const { value, storagePath } = await loadAppDraftValue(args.path, workspace, { forWrite: true }) + const draft = draftTargetOf(ctx) + const value = await loadAppDraftValue(draft, workspace) if (!(target.filePath in value.files)) { throw new Error(`Frontend file "${target.filePath}" not found in app "${args.path}".`) } const { [target.filePath]: _removed, ...remaining } = value.files value.files = remaining - const result = await saveAppDraft(workspace, args.path, value, storagePath) + const result = await saveAppDraft(workspace, draft, value) return finishAppDraftWrite(result, ctx, () => ({ content: `Removed ${target.filePath} from app "${args.path}"`, message: `Removed "${target.filePath}" from draft app "${args.path}".` @@ -6589,7 +6651,8 @@ async function patchAppFile( content: `Patching ${target.filePath} in app "${path}"...` }) - const { value, storagePath } = await loadAppDraftValue(path, workspace, { forWrite: true }) + const draft = draftTargetOf(ctx) + const value = await loadAppDraftValue(draft, workspace) let currentContent: string let runnable: PersistedRunnable | undefined @@ -6629,7 +6692,7 @@ async function patchAppFile( } } - const result = await saveAppDraft(workspace, path, value, storagePath) + const result = await saveAppDraft(workspace, draft, value) return finishAppDraftWrite(result, ctx, () => ({ content: `Patched ${target.filePath} in app "${path}"`, message: `Patched "${target.filePath}" in draft app "${path}".` @@ -6657,13 +6720,14 @@ async function writeAppRunnable( content: `Writing runnable "${key}" to app "${path}"...` }) - const { value, storagePath } = await loadAppDraftValue(path, workspace, { forWrite: true }) + const target = draftTargetOf(ctx) + const value = await loadAppDraftValue(target, workspace) const existing = value.runnables[key] as PersistedRunnable | undefined const persisted = buildPersistedRunnable(input, existing) value.runnables = { ...value.runnables, [key]: persisted } await recomputeAppPolicy(value) const undeployed = await undeployedRunnableTargets(workspace, { [key]: persisted }) - const result = await saveAppDraft(workspace, path, value, storagePath) + const result = await saveAppDraft(workspace, target, value) return finishAppDraftWrite(result, ctx, () => ({ content: `Updated runnable "${key}" in app "${path}"`, message: `Updated draft app "${path}" with runnable "${key}".`, @@ -6687,7 +6751,7 @@ async function testRunAppRunnable( const { workspace, toolId, toolCallbacks } = ctx const { path, key } = args - const { value } = await loadAppDraftValue(path, workspace) + const value = await loadAppDraftValue(draftTargetOf(ctx), workspace) const runnable = value.runnables?.[key] as PersistedRunnable | undefined if (!runnable) { const known = Object.keys(value.runnables ?? {}) @@ -6768,14 +6832,15 @@ async function deleteAppRunnable( content: `Removing runnable "${key}" from app "${path}"...` }) - const { value, storagePath } = await loadAppDraftValue(path, workspace, { forWrite: true }) + const target = draftTargetOf(ctx) + const value = await loadAppDraftValue(target, workspace) if (!(key in value.runnables)) { throw new Error(`Backend runnable "${key}" not found in app "${path}".`) } const { [key]: _removed, ...remaining } = value.runnables value.runnables = remaining await recomputeAppPolicy(value) - const result = await saveAppDraft(workspace, path, value, storagePath) + const result = await saveAppDraft(workspace, target, value) return finishAppDraftWrite(result, ctx, () => ({ content: `Removed runnable "${key}" from app "${path}"`, message: `Removed runnable "${key}" from draft app "${path}".` @@ -6854,21 +6919,21 @@ async function discardLocalDraft( throw new Error('trigger_kind is required when discarding a trigger draft.') } - // 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) - const draft = await getGlobalDraftAt(workspace, type, storagePath, triggerKind) + const target = draftTargetOf(ctx) + const draft = await getGlobalDraft(workspace, type, target, triggerKind) if (!draft) { throw new Error(`No draft found for ${type} "${path}".`) } - await deleteGlobalDraft(workspace, type, path, triggerKind, { storagePath }) + await deleteGlobalDraft(workspace, type, target, 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, storagePath) + // 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. + toolCallbacks.onItemDiscarded?.(discardedKind, target.storagePath) } toolCallbacks.setToolStatus(toolId, { @@ -6934,8 +6999,9 @@ async function rebaseScriptDraft(path: string, ctx: WriteDraftCtx): Promise // The deployed version a draft is based on sits at the path it is stored at: a rename // is staged over the old path, and nothing is deployed yet at the new name. - const basePath = await resolveGlobalDraftStoragePath(workspace, 'app', path) - const draft = await getGlobalDraftAt(workspace, 'app', basePath) + const target = draftTargetOf(ctx) + const basePath = target.storagePath + const draft = await getGlobalDraft(workspace, 'app', target) if (!draft || !draft.value || typeof draft.value === 'string' || !('files' in draft.value)) { throw new Error(`No app draft found for "${path}".`) } @@ -7139,7 +7207,7 @@ async function rebaseAppDraft(path: string, ctx: WriteDraftCtx): Promise // Discard the stale draft (see rebaseScriptDraft): the next write re-projects // the deployed app into a fresh draft (re-pinning parent_version to the head), // and a premature deploy fails cleanly instead of shipping the latest unchanged. - await deleteGlobalDraft(workspace, 'app', path, undefined, { storagePath: basePath }) + await deleteGlobalDraft(workspace, 'app', target) toolCallbacks.setToolStatus(toolId, { content: `Discarded stale draft "${path}"`, @@ -7976,10 +8044,9 @@ async function deployDraft( throw new Error('trigger_kind is required when deploying a trigger.') } - // Resolved once for the whole deploy: the read, the flush, the deployed-path lookup and - // the cleanup all name the same draft, whatever the listing says in between. - const draftStoragePath = await resolveGlobalDraftStoragePath(workspace, type, path, triggerKind) - const draft = await getGlobalDraftAt(workspace, type, draftStoragePath, triggerKind) + const target = draftTargetOf(ctx) + const { storagePath } = target + const draft = await getGlobalDraft(workspace, type, target, triggerKind) if (!draft) { throw new Error(`No draft found for ${type} "${path}".`) } @@ -8018,7 +8085,6 @@ async function deployDraft( // getScriptByPath/getFlowByPath at the path we pass (then deploys at the // draft's own `path`), so passing the display/chosen path would 404. For a // draft on a deployed item the storage path is just the item path. - const storagePath = draftStoragePath // The shared deployer re-reads the persisted DB draft, but an open editor's // edit may still be parked in a debounced/disabled autosave. Flush it first so // we deploy the latest value (not a stale persisted one) — and so the @@ -8048,8 +8114,12 @@ async function deployDraft( // 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 readGlobalDraftValueAt(workspace, type, storagePath)) ?? - storagePath + chosenDraftName( + type, + // `fresh`: the flush above may have persisted a rename parked in the editor, + // which the value read while resolving predates. + await readGlobalDraftValue(workspace, type, target, { fresh: true }) + ) ?? storagePath const result = await deployDraftToWorkspace(type, storagePath, workspace, { draftOnly, deploymentMessage @@ -8125,7 +8195,7 @@ async function deployDraft( // a fork base to compare against (pre-feature drafts have none). if (draft.parentVersionId != null) { // The draft's base is the app it is stored at, not the name it was addressed by. - const basePath = draftStoragePath + const basePath = storagePath const deployedApp = (await AppService.existsApp({ workspace, path: basePath })) ? await AppService.getAppByPath({ workspace, path: basePath }) : undefined @@ -8215,7 +8285,6 @@ async function deployDraft( // doesn't carry it, so read it from the backend draft. For a chat-created app // (real path, no draft_path) or a draft on a deployed app, the storage path // is the deploy path. Same storage-path resolution as script/flow. - const storagePath = draftStoragePath // `draft_path` is read from the persisted backend draft below, but an // editor rename may still be parked in a debounced/disabled autosave. // Flush first (like script/flow) so we read the latest chosen path. @@ -8311,17 +8380,14 @@ async function deployDraft( // 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, path, triggerKind, { - preserveLiveDraft: true, - storagePath: draftStoragePath - }) + await deleteGlobalDraft(workspace, type, target, 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 // stop matching anything after the draft is gone. const deployedKind = itemKindFor(type, triggerKind) if (deployedKind) { - toolCallbacks.onItemDeployed?.(deployedKind, draftStoragePath, deployedPath) + toolCallbacks.onItemDeployed?.(deployedKind, storagePath, deployedPath) } // Reload the session preview if it's open on the deployed item. Map the @@ -8334,7 +8400,7 @@ async function deployDraft( } const kind = previewKindByType[type] if (kind) { - deployedInSessionHandler?.({ sessionId, kind, path: draftStoragePath, deployedPath }) + deployedInSessionHandler?.({ sessionId, kind, path: storagePath, deployedPath }) } // Named by where the item landed, not by the path the call addressed it at: a draft is @@ -8377,7 +8443,14 @@ async function validateDeleteWorkspaceItemTarget(args: { const { workspace } = args if (await deployedItemExists(workspace, type, path, triggerKind)) return undefined - const draft = await getGlobalDraft(workspace, type, path, triggerKind) + // Only reached on the refusal path, where the call never runs and the dispatch never + // resolves — so this resolution is the operation's only one, not a second. + const draft = await getGlobalDraft( + workspace, + type, + await resolveDraftTarget(workspace, { type, path, triggerKind }), + triggerKind + ) return draft ? `No deployed ${type} at "${path}" — it only exists as a draft, so there is nothing to delete ` + `from the workspace. Call discard_local_draft with the same arguments to remove the draft.` @@ -8463,10 +8536,10 @@ async function deleteWorkspaceItem( throw new Error('trigger_kind is required when deleting a trigger.') } - // Resolved while the deployed item is still there: once it is gone, this path could + // Resolved before the deployed item is deleted: once it is gone, this path could // resolve to another item's draft staged under its name, and the cleanup below would - // delete that instead. - const draftStoragePath = await resolveGlobalDraftStoragePath(workspace, type, path, triggerKind) + // delete that instead. The dispatch resolves it, so that ordering holds by construction. + const target = draftTargetOf(ctx) toolCallbacks.setToolStatus(toolId, { content: `Deleting ${type} "${path}"...` @@ -8500,14 +8573,14 @@ 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, { storagePath: draftStoragePath }) + await deleteGlobalDraft(workspace, type, target, triggerKind) // 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 // by the same (itemKind, storagePath) as writes so it joins the draft/fork lists. const deletedKind = itemKindFor(type, triggerKind) if (deletedKind) { - toolCallbacks.onItemModified?.(deletedKind, draftStoragePath) + toolCallbacks.onItemModified?.(deletedKind, target.storagePath) } toolCallbacks.setToolStatus(toolId, { diff --git a/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts b/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts index ff466d1c6b..8756ac9a52 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 { AppService, DraftService, FlowService, ScriptService } from '$lib/gen' import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte' import { DEFAULT_DATA as DEFAULT_RAW_APP_DATA } from '$lib/components/raw_apps/dataTableRefUtils' import { UserDraft, type UserDraftEntry, type UserDraftItemKind } from '$lib/userDraft.svelte' +import type { ResolvedDraftTarget } from '../shared' import { invalidateWorkspaceDrafts } from '$lib/workspaceDrafts.svelte' import { getWorkspaceItemKey, @@ -417,7 +418,7 @@ function liveEditorStoragePath( } /** Follows the open editor's staged rename only, for callers that already hold a storage - * path. A path that may be a draft's chosen name needs `resolveGlobalDraftStoragePath`. */ + * path. A path that may be a draft's chosen name needs `resolveDraftTarget`. */ export function liveGlobalDraftStoragePath( workspace: string, type: WorkspaceItemType, @@ -428,22 +429,35 @@ export function liveGlobalDraftStoragePath( return itemKind ? liveEditorStoragePath(workspace, itemKind, path) : path } -/** One resolution for a whole operation, so a tool reads its merge base, writes and - * cleans up at the same key. Resolving twice lets a listing that fails in between send - * the two halves to different drafts. `value` is the draft found while resolving, if any; - * `fetched` says the backend was already asked, so no caller asks again. */ -export async function resolveGlobalDraft( +/** The draft a call addresses: the path it named, and whether the call goes on to write. */ +export type DraftTarget = { + type: WorkspaceItemType + path: string + triggerKind?: TriggerKind + /** 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. */ + forWrite?: boolean +} + +/** + * The only way to turn a path into a storage key. Every other function here takes the key, + * so an operation resolves once — at the tool boundary — and its read, its write and its + * cleanup cannot end up on different drafts because a listing failed in between. + * + * A tool body reads its own target off `ctx.target`; it calls this only to address a + * *second* item, such as the script a flow step points at. + */ +export async function resolveDraftTarget( workspace: string, - type: WorkspaceItemType, - path: string, - triggerKind?: TriggerKind, - opts: { forWrite?: boolean } = {} -): Promise<{ storagePath: string; value?: unknown; fetched: boolean }> { + target: DraftTarget +): Promise { + const { type, path, triggerKind, forWrite } = target const itemKind = itemKindFor(type, triggerKind) - if (!itemKind) return { storagePath: path, fetched: false } - const resolved = await resolveDraft(workspace, itemKind, path, opts) + if (!itemKind) return { path, storagePath: path, fetched: false } + const resolved = await resolveDraft(workspace, itemKind, path, { forWrite }) const cell = UserDraft.get(itemKind, resolved.storagePath, { workspace }) return { + path, storagePath: resolved.storagePath, value: cell ?? resolved.value, // Only the path it probed was fetched; a name resolved elsewhere was not. @@ -451,45 +465,6 @@ export async function resolveGlobalDraft( } } -/** Where the draft a model-supplied path names is stored. */ -export async function resolveGlobalDraftStoragePath( - workspace: string, - type: WorkspaceItemType, - path: string, - triggerKind?: TriggerKind -): Promise { - const itemKind = itemKindFor(type, triggerKind) - if (!itemKind) return path - return (await resolveDraft(workspace, itemKind, path)).storagePath -} - -function getGlobalDraftSlot( - workspace: string, - type: WorkspaceItemType, - path: string, - triggerKind?: TriggerKind -) { - const itemKind = itemKindFor(type, triggerKind) - if (!itemKind) return undefined - const storagePath = liveEditorStoragePath(workspace, itemKind, path) - const draft = UserDraft.get(itemKind, storagePath, { workspace }) - if (draft === undefined) return undefined - - const { displayPath, isLiveDraft } = liveDisplayPath(workspace, itemKind, storagePath) - const entry = { - workspace, - itemKind, - path: storagePath, - value: draft, - meta: {}, - persisted: false, - live: false - } - const item = userDraftEntryToWorkspaceItem(entry, displayPath, isLiveDraft) - if (!item) return undefined - return { itemKind, storagePath, displayPath, item } -} - // Current user's persisted draft value (+ records the sync baseline so a later // save detects external conflicts). undefined when no draft exists at that path. // Uses `getOwnDraft` (not `getDraftForUser`): the latter rejects drawer kinds @@ -513,19 +488,21 @@ async function fetchBackendDraftValue( return resp.value ?? undefined } -/** Draft VALUE at a key already resolved: cell-if-present (the user's freshest in-tab - * edits) else the current user's backend draft. */ -export async function readGlobalDraftValueAt( +/** Draft VALUE at a resolved target: cell-if-present (the user's freshest in-tab edits) + * else the current user's backend draft. Answers from what resolving the target already + * read, unless `fresh` — for a caller that has since changed the draft it is reading. */ +export async function readGlobalDraftValue( workspace: string, type: WorkspaceItemType, - storagePath: string, - triggerKind?: TriggerKind + target: ResolvedDraftTarget, + opts: { triggerKind?: TriggerKind; fresh?: boolean } = {} ): Promise { - const itemKind = itemKindFor(type, triggerKind) + if (target.fetched && !opts.fresh) return target.value as V | undefined + const itemKind = itemKindFor(type, opts.triggerKind) if (!itemKind) return undefined - const cell = UserDraft.get(itemKind, storagePath, { workspace }) + const cell = UserDraft.get(itemKind, target.storagePath, { workspace }) if (cell !== undefined) return cell - return (await fetchBackendDraftValue(workspace, itemKind, storagePath)) as V | undefined + return (await fetchBackendDraftValue(workspace, itemKind, target.storagePath)) as V | undefined } // `itemKind` + `storagePath` are the canonical identity of the persisted draft @@ -555,18 +532,13 @@ export type DraftPersistResult = export async function persistGlobalDraft( workspace: string, type: WorkspaceItemType, - path: string, + target: ResolvedDraftTarget, value: unknown, - opts: { triggerKind?: TriggerKind; force?: boolean; storagePath?: string } = {} + opts: { triggerKind?: TriggerKind; force?: boolean } = {} ): Promise { const itemKind = itemKindFor(type, opts.triggerKind) 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. A caller that resolved - // for the whole operation passes its key so both halves land on one draft. - const storagePath = - opts.storagePath ?? - (await resolveDraft(workspace, itemKind, path, { forWrite: true })).storagePath + const { path, storagePath } = target UserDraft.seed(itemKind, storagePath, value, { workspace }) await UserDraftDbSyncer.save({ workspace, @@ -612,39 +584,17 @@ export async function persistGlobalDraft( return { status: 'saved', item, itemKind, storagePath } } +/** The draft at a resolved target, so one operation reads, writes and cleans up at the + * same draft even if the listing changes under it. */ export async function getGlobalDraft( workspace: string, type: WorkspaceItemType, - path: string, + target: ResolvedDraftTarget, triggerKind?: TriggerKind ): Promise { - const slot = getGlobalDraftSlot(workspace, type, path, triggerKind) - if (slot) return slot.item const itemKind = itemKindFor(type, triggerKind) if (!itemKind) return undefined - const resolved = await resolveDraft(workspace, itemKind, path) - return draftItemAt(workspace, itemKind, resolved.storagePath, { - value: resolved.value, - // Resolution already asked for this key and found nothing; asking again returns - // the same nothing. - fetched: resolved.storagePath === path - }) -} - -/** The draft at a key the caller already resolved, so one operation reads, writes and - * cleans up at the same draft even if the listing changes under it. */ -export async function getGlobalDraftAt( - workspace: string, - type: WorkspaceItemType, - storagePath: string, - triggerKind?: TriggerKind, - /** What the caller's own resolution already found at this key, so it is not fetched - * a second time. */ - found: { value?: unknown; fetched?: boolean } = {} -): Promise { - const itemKind = itemKindFor(type, triggerKind) - if (!itemKind) return undefined - return draftItemAt(workspace, itemKind, storagePath, found) + return draftItemAt(workspace, itemKind, target.storagePath, target) } async function draftItemAt( @@ -747,34 +697,28 @@ export async function listGlobalDrafts(workspace: string): Promise { // Return the full result (not just the item) so app write tools surface a // conflict / save failure instead of reporting every stale write as saved. - return persistGlobalDraft(workspace, 'app', path, normalizeAppDraftValue(value), opts) + return persistGlobalDraft(workspace, 'app', target, normalizeAppDraftValue(value)) } type DeleteGlobalDraftOptions = { preserveLiveDraft?: boolean - /** Key resolved by the caller, for a cleanup whose own resolution would now answer - * differently — after a deploy created the item at the draft's chosen name, or after - * the deployed item that outranked that name was deleted. */ - storagePath?: string } export async function deleteGlobalDraft( workspace: string, type: WorkspaceItemType, - path: string, + target: ResolvedDraftTarget, triggerKind?: TriggerKind, options: DeleteGlobalDraftOptions = {} ): Promise { const itemKind = itemKindFor(type, triggerKind) if (!itemKind) return - const storagePath = - options.storagePath ?? (await resolveDraft(workspace, itemKind, path)).storagePath + const { path, storagePath } = target const liveDraft = UserDraft.getLiveEditorDraft(itemKind, { workspace }) if (options.preserveLiveDraft && liveDraft?.storagePath === storagePath) { UserDraft.remove(itemKind, storagePath, { workspace }) diff --git a/frontend/src/lib/components/copilot/chat/shared.ts b/frontend/src/lib/components/copilot/chat/shared.ts index f58ed3b6d3..dc4932a3de 100644 --- a/frontend/src/lib/components/copilot/chat/shared.ts +++ b/frontend/src/lib/components/copilot/chat/shared.ts @@ -815,7 +815,8 @@ async function callTool({ `Unknown tool call: ${functionName}. Probably not in the correct mode, use the change_mode tool to switch to the correct mode.` ) } - const result = await tool.fn({ args, workspace, helpers, toolCallbacks, toolId }) + const target = await tool.draftTarget?.({ args, workspace }) + const result = await tool.fn({ args, workspace, helpers, toolCallbacks, toolId, target }) toolCompletionListener?.(functionName, args, workspace) return result } @@ -1150,6 +1151,19 @@ export function appendPendingToolImages( addedMessages.push(message) } +/** Where a tool's draft is stored, resolved once by the dispatch. A draft's chosen name and + * the key it is stored at differ for a never-deployed item, so every backend call in the tool + * must address `storagePath`. */ +export type ResolvedDraftTarget = { + /** The path the call named. What the model and the user see, so messages quote this. */ + path: string + storagePath: string + /** The draft's value when resolving it already read one, so the body does not refetch. */ + value?: unknown + /** Whether `value` reflects a read: `undefined` then means no draft, not "not looked". */ + fetched: boolean +} + export interface Tool { def: ChatCompletionFunctionTool fn: (p: { @@ -1158,7 +1172,16 @@ export interface Tool { helpers: T toolCallbacks: ToolCallbacks toolId: string + /** Resolved from `draftTarget`; absent when the tool declares none, or when its + * declaration says this call addresses no draft. */ + target?: ResolvedDraftTarget }) => Promise + /** Names the draft this call addresses, and resolves it. The dispatch awaits this once and + * hands the result to `fn` as `target`, so a tool never resolves a path in its own body — + * a read and the write that follows it can then not disagree about which draft they mean. + * Undefined for a call that addresses no draft, such as a read pinned to the deployed + * version. */ + draftTarget?: (p: { args: any; workspace: string }) => Promise preAction?: (p: { toolCallbacks: ToolCallbacks; toolId: string }) => void /** Refuse the call before any confirmation is offered. A bare string is both the row the * user reads and the result the model gets; return the pair when the model needs a steer