diff --git a/frontend/src/lib/components/copilot/chat/RunScriptCard.svelte b/frontend/src/lib/components/copilot/chat/RunScriptCard.svelte index 114a49fe6b..fa45c49a2a 100644 --- a/frontend/src/lib/components/copilot/chat/RunScriptCard.svelte +++ b/frontend/src/lib/components/copilot/chat/RunScriptCard.svelte @@ -393,7 +393,10 @@ // has no time to give, so its outcome takes the slot — as a word, never "Not run", which // stutters against the "Run " label beside it. const outcome = $derived(failed ? 'Failed' : canceled ? 'Cancelled' : 'Done') - const statusTime = $derived(running ? elapsed : duration || outcome) + // A call that only opened the page has no outcome and no duration, and "Done" there would + // claim the completion the rest of this card is careful not to imply. The label already + // says `Opened`, which is the whole status. + const statusTime = $derived(openedDeployedPage ? '' : running ? elapsed : duration || outcome) // What the preview button opens changes with the card: the form while the call is still // waiting on one, the run once a job exists. Neither, and there is nothing to open, so 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 63bfa6e1b2..c9334da5f6 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -5126,6 +5126,75 @@ describe('global AI tools', () => { }) }) + // A DEPLOYED run inside a session is handed to the item's own page and the call ends + // there: the user runs it from the page, with the scheduling, tag and version controls + // this tool could never carry. The three things that must hold are that no job is + // started, that the model is told so rather than left to infer a run, and that a TEST + // run — which executes the draft, not what the page shows — never takes the branch. + it('hands a deployed run to the item’s page and starts no job', async () => { + const script = { + path: 'f/scripts/handed-over', + summary: 'Handed over', + content: 'export async function main(name: string) {}', + language: 'bun', + schema: { properties: { name: { type: 'string' } } } + } as any + vi.mocked(ScriptService.getScriptByPath).mockResolvedValueOnce(script) + + const opened: any[] = [] + const requestRunArgs = vi.fn(async (_toolId: string, form: any) => form.args) + const result = await callGlobalTool( + 'run_script', + { path: 'f/scripts/handed-over', args: { name: 'Ada' } }, + { + ...toolCallbacks, + requestRunArgs, + openDeployedRunPage: (a: any) => { + opened.push(a) + return true + } + } + ) + + expect(opened).toEqual([ + { + kind: 'script', + path: 'f/scripts/handed-over', + summary: 'Handed over', + args: { name: 'Ada' } + } + ]) + // Never parks on a form: there is nothing to confirm when the page is the confirmation. + expect(requestRunArgs).not.toHaveBeenCalled() + expect(JobService.runScriptByPath).not.toHaveBeenCalled() + // The model reports back from this string, so it has to deny the run outright — an + // "opened" it read as "started" would have it tell the user the job is away. + expect(result).toContain('has NOT run') + }) + + // The draft is what a test executes, and the deployed page does not show it. Handing one + // over would run the deployed version instead — a different script. + it('keeps a test run on its own form even where a page could be opened', async () => { + vi.mocked(ScriptService.getScriptByPath).mockResolvedValueOnce({ + path: 'f/scripts/still-tested', + content: 'export async function main(name: string) {}', + language: 'bun', + schema: { properties: { name: { type: 'string' } } } + } as any) + + const openDeployedRunPage = vi.fn(() => true) + await withCompletedTestJob(() => + callGlobalTool( + 'test_run_script', + { path: 'f/scripts/still-tested', args: { name: 'Ada' } }, + { ...toolCallbacks, openDeployedRunPage } + ) + ) + + expect(openDeployedRunPage).not.toHaveBeenCalled() + expect(JobService.runScriptPreview).toHaveBeenCalled() + }) + // The bypass posture answers a run form as it answers any other confirmation, for a // deployed run as much as a test. The card must never render one first: a form nobody // will fill in is attached already settled, so no field is ever mounted, and the schema @@ -5142,11 +5211,16 @@ describe('global AI tools', () => { const statuses: any[] = [] const requestRunArgs = vi.fn(async (_toolId: string, form: any) => form.args) + // Wired on purpose: the posture answers for a reader who already consented, so a + // deployed run must still run here rather than being handed to a page with nobody + // left to press Run. + const openDeployedRunPage = vi.fn(() => true) const yolo = { ...toolCallbacks, setToolStatus: (_toolId: string, status: any) => statuses.push(status), shouldAutoAcceptToolConfirmations: () => true, - requestRunArgs + requestRunArgs, + openDeployedRunPage } await withCompletedTestJob(() => @@ -5176,6 +5250,7 @@ describe('global AI tools', () => { expect(JobService.runScriptByPath).toHaveBeenCalledWith( expect.objectContaining({ requestBody: { name: 'Ada' }, skipPreprocessor: true }) ) + expect(openDeployedRunPage).not.toHaveBeenCalled() }) // The posture is the user's standing answer to whether to ask, so a run it answers starts on diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index 00378dbb26..107970186b 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -907,14 +907,14 @@ const backgroundArgSchema = z .boolean() .optional() .describe( - 'Run in the background without waiting. Set true for jobs you expect to be long (deploys, backfills, big queries) — you will be notified when it finishes. Leave unset for normal runs, which wait briefly and only background automatically if slow.' + "Run in the background without waiting. Set true for jobs you expect to be long (deploys, backfills, big queries) — you will be notified when it finishes. Leave unset for normal runs, which wait briefly and only background automatically if slow. Has no effect where the call hands a deployed run to the item's own page: nothing is waited on there, because the user starts the job themselves." ) const waitSecondsArgSchema = z .number() .optional() .describe( - 'How many seconds to wait for the job in-turn before it detaches into the background jobs tray. Defaults to 15. Raise it (capped at 120) for a job you expect to finish in, say, 30–60s and want the result in this same turn. Ignored when background is true. Do not use this to poll — larger values just hold the turn longer.' + "How many seconds to wait for the job in-turn before it detaches into the background jobs tray. Defaults to 15. Raise it (capped at 120) for a job you expect to finish in, say, 30–60s and want the result in this same turn. Ignored when background is true, and where the call hands a deployed run to the item's own page — there is no job to wait for until the user starts one. Do not use this to poll — larger values just hold the turn longer." ) /** Translate the model's `wait_seconds` into executeTestRun's `detachAfterMs` @@ -948,7 +948,7 @@ const runScriptSchema = z.object({ const runScriptToolDef = createToolDef( runScriptSchema, 'run_script', - 'Run a DEPLOYED script for real, under the user\'s own permissions. Fill in every argument you can infer: the user gets an argument form prefilled with `args` and decides what runs. For a secret argument prefer `$var:` naming an existing workspace variable; a literal is minted into a short-lived secret before the run, but stays in this call. A required file is the user\'s to attach, so call this even when you cannot supply one rather than asking in chat. Use only when the user names the deployed version ("the deployed X", "in production", "for real").', + 'Run a DEPLOYED script for real, under the user\'s own permissions. Fill in every argument you can infer: the user gets an argument form prefilled with `args` and decides what runs. Inside an AI session this opens the script\'s own deployed page with that form filled in and the call ENDS THERE — the user runs it from the page, so do not report it as started, and read the call\'s result for which happened. For a secret argument prefer `$var:` naming an existing workspace variable; a literal is minted into a short-lived secret before the run, but stays in this call. A required file is the user\'s to attach, so call this even when you cannot supply one rather than asking in chat. Use only when the user names the deployed version ("the deployed X", "in production", "for real").', { strict: false } ) @@ -987,7 +987,7 @@ const runFlowSchema = z.object({ const runFlowToolDef = createToolDef( runFlowSchema, 'run_flow', - 'Run a DEPLOYED flow for real, under the user\'s own permissions. Fill in every argument you can infer: the user gets an argument form prefilled with `args` and decides what runs. For a secret argument prefer `$var:` naming an existing workspace variable; a literal is minted into a short-lived secret before the run, but stays in this call. A required file is the user\'s to attach, so call this even when you cannot supply one rather than asking in chat. Use only when the user names the deployed version ("the deployed X", "in production", "for real").', + 'Run a DEPLOYED flow for real, under the user\'s own permissions. Fill in every argument you can infer: the user gets an argument form prefilled with `args` and decides what runs. Inside an AI session this opens the flow\'s own deployed page with that form filled in and the call ENDS THERE — the user runs it from the page, so do not report it as started, and read the call\'s result for which happened. For a secret argument prefer `$var:` naming an existing workspace variable; a literal is minted into a short-lived secret before the run, but stays in this call. A required file is the user\'s to attach, so call this even when you cannot supply one rather than asking in chat. Use only when the user names the deployed version ("the deployed X", "in production", "for real").', { strict: false } ) @@ -3917,7 +3917,9 @@ export const globalTools: SessionTool<{}>[] = [ // No requiresConfirmation, for the reason test_run_script carries. bypassedByAutoAccept: true, confirmationMessage: 'Run a deployed script', - streamingLabel: 'Preparing the run form...', + // Both endings start the same way: the arguments are read, then either a form opens + // here or the script's own deployed page does. + streamingLabel: 'Preparing the arguments...', queuedLabel: (args) => `Run ${args?.path ?? 'a script'}`, showDetails: true, autoCollapseDetails: false @@ -3945,7 +3947,7 @@ export const globalTools: SessionTool<{}>[] = [ return runDeployedFlow(parsed, ctx) }, bypassedByAutoAccept: true, - streamingLabel: 'Preparing the run form...', + streamingLabel: 'Preparing the arguments...', confirmationMessage: 'Run a deployed flow', queuedLabel: (args) => `Run ${args?.path ?? 'a flow'}`, showDetails: true, @@ -6038,16 +6040,16 @@ async function testRunScriptByPath( ) } -/** The "do not call again" half is load-bearing: without it the model re-proposes the - * call, which re-opens the form the user just dismissed, and Stop becomes their only - * way out. */ /** Nothing ran, and saying so is the whole point: the model is about to report back, and a * "started" it inferred would be wrong. The "do not call again" half is load-bearing for the - * same reason as the cancelled one — a re-proposal would reopen the page the reader is + * same reason as the cancelled one below — a re-proposal would reopen the page the reader is * already looking at. */ const deployedPageOpened = (path: string, noun: string, toolName: string) => `Opened the deployed page for "${path}" in the preview panel, with these arguments filled into its run form. The ${noun} has NOT run: the user starts it from that page, where they can also edit the arguments, schedule it, override the tag or pick a version. Do not call ${toolName} again for this. Tell the user it is ready to run; if you need the result afterwards, ask them or call get_run once they say it has run.` +/** The "do not call again" half is load-bearing: without it the model re-proposes the + * call, which re-opens the form the user just dismissed, and Stop becomes their only + * way out. */ const runFormCancelled = (toolName: string, noun: string) => `The user cancelled the run form. The ${noun} did NOT run. Do not call ${toolName} again unless the user asks for it.` diff --git a/frontend/src/lib/components/details/FlowDetail.svelte b/frontend/src/lib/components/details/FlowDetail.svelte index ab475450c7..4d3e834086 100644 --- a/frontend/src/lib/components/details/FlowDetail.svelte +++ b/frontend/src/lib/components/details/FlowDetail.svelte @@ -372,16 +372,28 @@ let flowChat: FlowChat | undefined = $state() let seededSeq: number | undefined = undefined $effect(() => { - if (!seededRun) return - const surface = chatInputEnabled ? flowChat : runForm - if (!surface) return - if (seededSeq === seededRun.seq) return - seededSeq = seededRun.seq + if (!seededRun || seededSeq === seededRun.seq) return if (chatInputEnabled) { const message = seededRun.args?.user_message - if (typeof message === 'string' && message) flowChat?.offerMessage(message) - } else { - runForm?.setArgs(seededRun.args) + // Nothing to offer is settled, not pending: latch it so a flow whose schema has no + // `user_message` does not re-ask on every flush. + if (typeof message !== 'string' || !message) { + seededSeq = seededRun.seq + return + } + // The rest of the proposal goes with it: a chat flow's schema can declare more than + // `user_message`, and sending the message alone would run on saved values or schema + // defaults — a different run from the one being offered. + const { user_message: _m, ...inputs } = seededRun.args ?? {} + // Only on success. The composer mounts a flush or two after the panel, and a + // declined offer here means it is not there yet — this effect tracks the host's + // input through `offerMessage` and runs again when it registers. A composer that + // already holds a draft also declines, and re-offering on its next change is right: + // the reader may clear it, and then the message lands. + if (flowChat?.offerMessage(message, inputs)) seededSeq = seededRun.seq + } else if (runForm) { + seededSeq = seededRun.seq + runForm.setArgs(seededRun.args) } }) diff --git a/frontend/src/lib/components/flows/conversations/FlowChat.svelte b/frontend/src/lib/components/flows/conversations/FlowChat.svelte index cf214715d5..e73c9fa94d 100644 --- a/frontend/src/lib/components/flows/conversations/FlowChat.svelte +++ b/frontend/src/lib/components/flows/conversations/FlowChat.svelte @@ -178,11 +178,19 @@ const shownKey = $derived(poolState?.shownKey) /** Put a message in the composer of the conversation on screen, for the reader to edit and - * send. Returns false when that composer already holds a draft — it is never overwritten, - * and the caller leaves the message where the reader can still see it. */ - export function offerMessage(text: string): boolean { + * send, and set the flow's other inputs to go with it. Returns false when that composer + * already holds a draft — it is never overwritten, and the caller leaves the message where + * the reader can still see it. + * + * `inputs` are the run's non-message arguments: a chat flow's schema can declare more than + * `user_message`, and a message sent without them would run on saved values or schema + * defaults — a different run from the one being offered. Applied only alongside a message + * the composer took, so a declined offer leaves the reader's settings alone. */ + export function offerMessage(text: string, inputs?: Record): boolean { const panel = shownKey ? pool?.get(shownKey) : undefined - return panel?.host.offerMessage(text) ?? false + if (!panel?.host.offerMessage(text)) return false + if (inputs) inputValues = { ...inputValues, ...inputs } + return true } // Derive additional inputs schema (excluding user_message) for chat mode diff --git a/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts b/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts index 9b28019796..ac62039769 100644 --- a/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts +++ b/frontend/src/lib/components/flows/conversations/flowChatViewHost.svelte.ts @@ -570,7 +570,10 @@ export class FlowChatViewHost implements ChatViewHost, DraftSender[0] = null + // Reactive, because a caller offering a message has to wait for it: the composer mounts a + // flush or two after the panel does, and a reader of this field inside an effect must be + // woken when it lands rather than deciding once that there was nowhere to put the message. + #aiChatInput: Parameters[0] = $state(null) setAiChatInput: ChatViewHost['setAiChatInput'] = (aiChatInput) => // Called from the composer's mount effect, and taking what a turn handed back reads the // draft it writes: tracked, that would rerun the effect and hand it back again. diff --git a/frontend/src/lib/components/sessions/ItemViewerView.svelte b/frontend/src/lib/components/sessions/ItemViewerView.svelte index 58deabe52b..1ee2e847af 100644 --- a/frontend/src/lib/components/sessions/ItemViewerView.svelte +++ b/frontend/src/lib/components/sessions/ItemViewerView.svelte @@ -69,7 +69,18 @@ // Arguments a chat tool filled this item's run form with when it opened the page. Nothing // is waiting on them: the reader runs the page as they would with no chat open, and the // note above the form only says where the values came from. - const seededRun = $derived(runtime.seededRunArgsFor(kind, path)) + // Taken from the runtime once and held here: the request belongs to the turn that made it, + // so leaving it in the map would re-apply a stale proposal — and reannounce it — to every + // later open of this item's deployed page. + let seededRun = $state<{ args: Record; seq: number } | undefined>(undefined) + $effect(() => { + const pending = runtime.seededRunArgsFor(kind, path) + if (!pending) return + untrack(() => { + seededRun = pending + runtime.clearSeededRunArgs(kind, path) + }) + }) /** Refetch the deployed item. The host calls this for the reload signals that reach a * tab from outside it; nothing about becoming visible triggers it, so a form the reader @@ -210,7 +221,7 @@ {active} embedded {seededRun} - onClearSeededRun={() => runtime.clearSeededRunArgs(kind, path)} + onClearSeededRun={() => (seededRun = undefined)} onLoadState={setLoadState} /> {:else if kind === 'flow'} @@ -222,7 +233,7 @@ {active} embedded {seededRun} - onClearSeededRun={() => runtime.clearSeededRunArgs(kind, path)} + onClearSeededRun={() => (seededRun = undefined)} onLoadState={setLoadState} /> {:else}