From 88683188ce5c084f3014b79de56d701162b70c30 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Tue, 13 Oct 2026 01:42:55 -0700 Subject: [PATCH] fix: carry a run proposal on the call, not in the tab url MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The proposal reached the deployed page through the tab url's hash, which the page already reads for "Run again". Two things were wrong with that carrier: - A tab already open on the item does not remount when it is re-pointed, and both detail pages read the hash once at init — so a second proposal for the same item opened its form on the first one's arguments. - Tab urls are persisted with the session, so a draft holding file bytes or an un-minted `password` literal was written to IndexedDB. The transcript avoids both deliberately, through `redactFileArgs` and `MAX_PERSISTED_ARGS_LENGTH`. The call now carries its own proposal: `PendingRun.proposedArgs` reads the card's live draft, the page seeds its form from it at init, and the tab's url names only the call. `ItemViewerView`'s load key includes that name, so a tab re-pointed to another call builds its form again. Two more fixes on the same path: - `adopt` was read off the prop after awaiting job creation, so a call that settled during the request could be answered with another call's job. The press now captures the call it was made under. - An adopted job's arguments and version are the page's, which this tool cannot read, but the model was told `Ran with arguments: {}`. It is now told the user ran it on the item's own page, and that the arguments are the page's. - Adopting hands the carrying tab to the run, so the page no longer navigates there as well — that left two tabs open on the same job. Co-Authored-By: Claude Opus 5 (1M context) --- .../copilot/chat/AIChatManager.svelte.ts | 22 ++----- .../copilot/chat/RunScriptCard.svelte | 1 - .../components/copilot/chat/global/core.ts | 6 ++ .../lib/components/details/FlowDetail.svelte | 62 ++++++++++--------- .../components/details/ScriptDetail.svelte | 62 ++++++++++--------- .../src/lib/components/details/pendingRun.ts | 9 ++- .../components/sessions/ItemViewerView.svelte | 13 ++-- .../components/sessions/PreviewTabHost.svelte | 6 -- .../lib/components/sessions/previewRouter.ts | 7 +-- .../sessions/sessionPreviewTabs.svelte.ts | 6 +- .../sessions/sessionRuntime.svelte.ts | 12 ++-- 11 files changed, 103 insertions(+), 103 deletions(-) diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index 3415b42b29..558e8ce866 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -584,18 +584,16 @@ export class AIChatManager implements ChatViewHost { * so the card hides the control rather than offering a tab that cannot run. */ openRunForm?: (a: { toolCallId: string; label: string }) => void /** Show a DEPLOYED run's item on its own page. With `carries`, that page is handed this - * call: it shows the same form, and Run there answers the call rather than starting a job - * of its own, so the tool still owns the run. Without it the page is only opened to look - * at — a conversational flow's page has no form to confirm on — and the card keeps the - * run. Set only where that page can be shown, a session's preview panel, so the card - * hides the control elsewhere. */ + * call: it opens its own form on what the call proposed, and the job its Run starts is + * the one the tool follows. Without it the page is only opened to look at — a + * conversational flow's page has no form to confirm on — and the card keeps the run. Set + * only where that page can be shown, a session's preview panel, so the card hides the + * control elsewhere. */ openRunOnDeployedPage?: (a: { toolCallId: string kind: 'script' | 'flow' path: string summary: string - /** Prefilled into the page's own run form. */ - args: Record carries: boolean }) => boolean closeRunForm?: (toolCallId: string) => void @@ -2145,18 +2143,10 @@ export class AIChatManager implements ChatViewHost { return true } - /** What a parked form is waiting on, for a surface showing that form outside the card. */ + /** What a parked form is waiting on, for a surface opening its own form on the same call. */ pendingRunFormArgs = (toolId: string): Record | undefined => this.#runForms.get(toolId)?.draft?.args - /** Take over what the form is waiting on, for a surface handing the call back with what - * the reader typed into it. A snapshot for the same reason the draft is one: these are - * `$state` proxies off another component, and the draft must not alias them. */ - setRunFormArgs = (toolId: string, args: Record) => { - const entry = this.#runForms.get(toolId) - if (entry) entry.draft.args = ($state.snapshot(args) ?? {}) as Record - } - handleRunFormCancel = (toolId: string) => { // The card's own copy is settled here rather than only in the tool's fn, which a form // restored from history no longer has: Cancel is that card's one way out, and while it diff --git a/frontend/src/lib/components/copilot/chat/RunScriptCard.svelte b/frontend/src/lib/components/copilot/chat/RunScriptCard.svelte index 258af1879c..2e95a80c58 100644 --- a/frontend/src/lib/components/copilot/chat/RunScriptCard.svelte +++ b/frontend/src/lib/components/copilot/chat/RunScriptCard.svelte @@ -470,7 +470,6 @@ kind: runnableKind === 'flow' ? 'flow' : 'script', path, summary: label, - args: aiChatManager.runFormDraft(message.tool_call_id, runForm!).args ?? {}, carries }) return diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index 4ac3ba0064..68e488abb0 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -6473,6 +6473,12 @@ async function runThroughForm(spec: FormRunSpec, ctx: WriteDraftCtx): Promise pendingRun.proposedArgs) } let canAuthorFlow = $derived(editRights.roleCanAuthor('flow')) @@ -899,31 +908,26 @@ /> {/if} - - {#key pendingRun?.toolCallId} - - {/key} +
diff --git a/frontend/src/lib/components/details/ScriptDetail.svelte b/frontend/src/lib/components/details/ScriptDetail.svelte index 854cf1dc7a..0abede20f7 100644 --- a/frontend/src/lib/components/details/ScriptDetail.svelte +++ b/frontend/src/lib/components/details/ScriptDetail.svelte @@ -385,9 +385,13 @@ invisibleToOwner: boolean | undefined, overrideTag: string | undefined ) { + // Taken before the round trip below: the call this press answers is the one it was + // pressed under, and a call settled meanwhile — declined, then another proposed for + // this same script — must not be answered with a job it never asked for. + const call = pendingRun // The page runs its own job, so nothing downstream gates this: the chat asked for the // run and then promised not to make one. - if (pendingRun?.planModeActive) { + if (call?.planModeActive) { sendUserToast(PLAN_MODE_MESSAGES.runFormRefused, true) return } @@ -416,9 +420,10 @@ skipPreprocessor: true }) } - // The call asked for this run; it follows the job rather than starting one. - pendingRun?.adopt(run) - await onNavigate('/run/' + run + '?workspace=' + workspace) + // The call asked for this run; it follows the job rather than starting one, and + // shows it in place of this page — so navigating here as well would leave two + // surfaces open on the same job. + if (!call?.adopt(run)) await onNavigate('/run/' + run + '?workspace=' + workspace) } catch (err) { sendUserToast(`Could not create job: ${err.body}`, true) } finally { @@ -447,6 +452,10 @@ console.error('Was not able to transform hash as args', e) } } + // A chat call confirmed here opens its form on what the chat proposed. Read at init like + // the hash above, and for the same reason; the host remounts this page when the call + // changes, so a tab already open on this script still builds its form from the new one. + if (pendingRun) args = { ...untrack(() => pendingRun.proposedArgs) } // A dbt retry must name the run it resumes, and a job id is not something to // type from memory: picking `retry` fills the field with the run this caller's @@ -1113,31 +1122,26 @@ onReject={() => pendingRun.decline()} /> {/if} - - {#key pendingRun?.toolCallId} - - {/key} +
diff --git a/frontend/src/lib/components/details/pendingRun.ts b/frontend/src/lib/components/details/pendingRun.ts index e228874d5b..e0d9281650 100644 --- a/frontend/src/lib/components/details/pendingRun.ts +++ b/frontend/src/lib/components/details/pendingRun.ts @@ -11,7 +11,12 @@ export type PendingRun = { * too: a `password` mints as it is typed and the dynamic-select helper runs on mount, * both before Run is ever pressed. */ planModeActive: boolean - /** The job the reader started here answers the call. */ - adopt: (jobId: string) => void + /** What the chat proposed, for the form to open on. Read when the form is built, so this + * is the live draft of the card's own form rather than a copy to keep in step. */ + readonly proposedArgs: Record + /** The job the reader started here answers the call. True when it did: the chat then shows + * that run in place of this page, so the page must not navigate to it as well. False when + * the call settled while the job was starting, and the run is the reader's alone. */ + adopt: (jobId: string) => boolean decline: () => void } diff --git a/frontend/src/lib/components/sessions/ItemViewerView.svelte b/frontend/src/lib/components/sessions/ItemViewerView.svelte index 6c66308a5f..9a66805ba4 100644 --- a/frontend/src/lib/components/sessions/ItemViewerView.svelte +++ b/frontend/src/lib/components/sessions/ItemViewerView.svelte @@ -29,7 +29,6 @@ path, version, pendingRunId, - locationHash = '', workspaceId, tabId, container, @@ -46,8 +45,6 @@ version?: string /** The chat tool call this tab was opened to answer, from its URL's `?pending_run=`. */ pendingRunId?: string - /** The tab url's hash, which the deployed pages read as prefilled run arguments. */ - locationHash?: string workspaceId: string /** Whether this side is the one on screen. Hidden sides stay mounted, so this gates * the window-level keyboard handler — several instances listen on the window. */ @@ -101,8 +98,12 @@ const searchParams = $derived(version ? new URLSearchParams({ version }) : undefined) - // Remounts the detail page below, which loads on mount. - const loadKey = $derived(`${workspaceId}/${kind}/${path}/${version ?? ''}/${reloadKey}`) + // Remounts the detail page below, which loads on mount. The call is part of it because + // the detail pages open their run form on what it proposed as they build it: a tab + // already on this item, re-pointed to a new call, has to build that form again. + const loadKey = $derived( + `${workspaceId}/${kind}/${path}/${version ?? ''}/${pendingRunId ?? ''}/${reloadKey}` + ) // Nothing deployed at this path. Stamped with the load it answers, so a refetch or a // re-pointed tab is never answered from an earlier load's verdict. let notFoundFor: string | undefined = $state(undefined) @@ -234,7 +235,6 @@ {active} embedded {pendingRun} - {locationHash} onLoadState={setLoadState} /> {:else if kind === 'flow'} @@ -246,7 +246,6 @@ {active} embedded {pendingRun} - {locationHash} onLoadState={setLoadState} /> {:else} diff --git a/frontend/src/lib/components/sessions/PreviewTabHost.svelte b/frontend/src/lib/components/sessions/PreviewTabHost.svelte index cf6761088e..795cd67669 100644 --- a/frontend/src/lib/components/sessions/PreviewTabHost.svelte +++ b/frontend/src/lib/components/sessions/PreviewTabHost.svelte @@ -101,11 +101,6 @@ // reason. Re-pointing the tab rebuilds its url without it, so a tab sent anywhere else — // another item, another version — stops carrying the call rather than answering it from a // page it was never confirmed against. - // The arguments a run was proposed with, in the hash the deployed pages already read for - // "Run again" — so the form fills itself as it is built. - const locationHash = $derived( - slot.kind === 'viewer' ? new URL(tab.url, 'http://x').hash : '' - ) const pendingRunId = $derived( slot.kind === 'viewer' ? (new URL(tab.url, 'http://x').searchParams.get('pending_run') ?? undefined) @@ -408,7 +403,6 @@ path={itemPath} version={pinnedVersion} {pendingRunId} - {locationHash} {workspaceId} tabId={tab.id} container={overlayHostEl} diff --git a/frontend/src/lib/components/sessions/previewRouter.ts b/frontend/src/lib/components/sessions/previewRouter.ts index dea4c685a0..89ff945618 100644 --- a/frontend/src/lib/components/sessions/previewRouter.ts +++ b/frontend/src/lib/components/sessions/previewRouter.ts @@ -88,16 +88,15 @@ export type PreviewTarget = // `pendingRunId` hands this tab a chat tool call parked on a run form: the deployed page // shows that form, and Run there answers the call instead of starting a job of its own. // In the query for the same reason as `version` — the tab stays the item's, and the call - // is a property of this tab rather than of the item. + // is a property of this tab rather than of the item. The id alone: what the call proposed + // is read off the chat, since a tab's url is persisted with the session and those + // arguments can hold file bytes and secrets typed into the form. | { type: 'item' item: WorkspaceItem mode?: PreviewItemMode version?: string pendingRunId?: string - /** Prefilled run arguments, written into the url's hash — the form the deployed - * page already builds from "Run again" fills itself from it. */ - runArgs?: Record } | { type: 'artifact'; id: string; name: string; version?: ArtifactVersionTarget } | { type: 'runform'; toolCallId: string; label: string } diff --git a/frontend/src/lib/components/sessions/sessionPreviewTabs.svelte.ts b/frontend/src/lib/components/sessions/sessionPreviewTabs.svelte.ts index 1917ee4d95..ee13729d4d 100644 --- a/frontend/src/lib/components/sessions/sessionPreviewTabs.svelte.ts +++ b/frontend/src/lib/components/sessions/sessionPreviewTabs.svelte.ts @@ -122,11 +122,7 @@ function targetUrl(target: PreviewTarget, onto?: SessionPreviewTab): string { if (target.version) params.set('version', target.version) if (target.pendingRunId) params.set('pending_run', target.pendingRunId) const query = params.size > 0 ? `?${params}` : '' - // Values are JSON, as the deployed pages parse them. - const hashed = new URLSearchParams() - for (const [k, v] of Object.entries(target.runArgs ?? {})) hashed.set(k, JSON.stringify(v)) - const hash = hashed.size > 0 ? `#${hashed}` : '' - return `${viewPathFor(target.item)}${query}${hash}` + return `${viewPathFor(target.item)}${query}` } // Point a tab at a new destination. Clears `friendlyLabel`/`friendlyPath` diff --git a/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts b/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts index 9bb8a9e58d..a3d1d1ebb1 100644 --- a/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts +++ b/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts @@ -607,6 +607,11 @@ function createRuntime(session: Session): SessionRuntime { toolCallId, // Read here, so the page's `$derived` rebuilds when the posture changes under it. planModeActive: manager.planModeActive, + // A getter, not a value: the page reads it once, when it builds the form, and this + // object is rebuilt on every keystroke of the card's draft otherwise. + get proposedArgs() { + return manager.pendingRunFormArgs(toolCallId) ?? {} + }, adopt: (jobId) => manager.handleRunFormJob(toolCallId, jobId), decline: () => manager.handleRunFormCancel(toolCallId) } @@ -630,8 +635,8 @@ function createRuntime(session: Session): SessionRuntime { previewTabs.open({ type: 'runform', toolCallId, label }) } // Show a deployed run on the item's own page. With `carries`, that page answers the call: - // its Run hands the arguments back, so the tool still owns the run. - manager.openRunOnDeployedPage = ({ toolCallId, kind, path, summary, args, carries }) => { + // the job its Run starts is the one the tool follows. + manager.openRunOnDeployedPage = ({ toolCallId, kind, path, summary, carries }) => { // Through the shared adapter: a session target spells a code-based app `raw_app` and a // workspace item spells it `app` with a flag, and one place should know that. const target = previewTargetForSessionTarget(kind, path) @@ -640,8 +645,7 @@ function createRuntime(session: Session): SessionRuntime { ...target, item: { ...target.item, summary }, mode: 'view' as const, - pendingRunId: carries ? toolCallId : undefined, - runArgs: args + pendingRunId: carries ? toolCallId : undefined } // Re-pointed rather than focused: the url is what hands the tab the call, and // rebuilding it also drops whatever version the reader had pinned.