From 150c18d609a302354f9dca3c74804b8dcaaca803 Mon Sep 17 00:00:00 2001 From: AlexRV12 <71396855+AlexRV12@users.noreply.github.com> Date: Wed, 2 Sep 2026 15:03:37 +0200 Subject: [PATCH] feat: give test_run_script the same argument-form card as run_script --- .../copilot/chat/AIChatManager.svelte.ts | 22 +- .../copilot/chat/AIChatManager.test.ts | 19 ++ .../copilot/chat/RunScriptCard.svelte | 14 +- .../copilot/chat/global/core.test.ts | 43 ++++ .../components/copilot/chat/global/core.ts | 235 +++++++++++++----- .../src/lib/components/copilot/chat/shared.ts | 20 +- 6 files changed, 280 insertions(+), 73 deletions(-) diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index b487bac64b..ec6b6c8259 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -1860,20 +1860,28 @@ export class AIChatManager { requestRunArgs = ( toolId: string, - _form: RunFormDisplay + form: RunFormDisplay, + opts?: { autoAcceptable?: boolean } ): Promise | undefined> => { - // The tool reads the deployed schema before it asks, so a stop during that read - // drains the callbacks and settles the card before this runs. Installing one then - // would park the turn on a form the settled card no longer renders, leaving nothing - // able to resolve it. The controller is per-turn, so a later turn still opens. + // The tool reads the schema before it asks, so a stop during that read drains the + // callbacks and settles the card before this runs. Installing one then would park + // the turn on a form the settled card no longer renders, leaving nothing able to + // resolve it. The controller is per-turn, so a later turn still opens. if (this.abortController?.signal.aborted) { // Settle the form the tool attached after the stop. Its card is about to stop // loading without ever having rendered, and settledToolDisplay only reaches a - // loading one — so this is the last point the deployed schema, with the - // script's own password and file defaults, can be dropped. + // loading one — so this is the last point the schema, with the script's own + // password and file defaults, can be dropped. this.#patchRunForm(toolId, { canceled: true }) return Promise.resolve(undefined) } + // Ahead of the wait, not of the stop above: the same skip a yes/no confirmation gets + // under YOLO, for the forms that opted in. A test run is the model's own iteration + // loop, and a form nobody answers would stop it dead. What the form opened with is + // what runs, so the card still records what it ran. A deployed run never opts in. + if (opts?.autoAcceptable && this.autoAcceptToolConfirmationsActive) { + return Promise.resolve(form.args) + } return new Promise((resolve) => { this.runFormCallbacks.set(toolId, resolve) }) diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts index 919b710a45..7a0473679b 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts @@ -699,6 +699,25 @@ describe('AIChatManager autonomy mode', () => { vi.clearAllMocks() }) + // The whole asymmetry between the two run forms: YOLO answers a test's, so the model + // can keep iterating on the code it is writing, and never a deployed run's, whose form + // is the only consent that run has. + it('answers only an auto-acceptable run form under yolo', async () => { + const manager = new AIChatManager() + manager.mode = AIMode.GLOBAL + manager.setAutonomyMode(AIAutonomyMode.YOLO) + const form = { path: 'f/qa/x', args: { name: 'Ada' } } + + await expect(manager.requestRunArgs('t1', form, { autoAcceptable: true })).resolves.toEqual({ + name: 'Ada' + }) + + let deployedSettled = false + void manager.requestRunArgs('t2', form).then(() => (deployedSettled = true)) + await Promise.resolve() + expect(deployedSettled).toBe(false) + }) + it('accepts pending flow edits when auto-accept is enabled from script mode', async () => { const manager = new AIChatManager() const acceptAllModuleActions = vi.fn() diff --git a/frontend/src/lib/components/copilot/chat/RunScriptCard.svelte b/frontend/src/lib/components/copilot/chat/RunScriptCard.svelte index 74240c3291..1b0624c3cb 100644 --- a/frontend/src/lib/components/copilot/chat/RunScriptCard.svelte +++ b/frontend/src/lib/components/copilot/chat/RunScriptCard.svelte @@ -80,13 +80,19 @@ }) // The row is the card's whole heading, in the tense the call is in: a run cancelled - // before it started never ran, so it is still the thing that was going to be run. + // before it started never ran, so it is still the thing that was going to be run. A + // test says so, since what it ran is the draft rather than what is deployed. + const verbs = $derived( + runForm.kind === 'test' + ? { present: 'Testing', past: 'Tested', future: 'Test' } + : { present: 'Running', past: 'Ran', future: 'Run' } + ) const label = $derived( running - ? `Running ${runForm.path}` + ? `${verbs.present} ${runForm.path}` : settled && ran - ? `Ran ${runForm.path}` - : `Run ${runForm.path}` + ? `${verbs.past} ${runForm.path}` + : `${verbs.future} ${runForm.path}` ) // Being cancelled is an outcome like any other, and it is the one the card has to say out 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 ede51e42df..d6f6070103 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -4376,6 +4376,49 @@ describe('global AI tools', () => { }) }) + // A test run meets the same card as a deployed one, so what previews is what the form + // submitted — not what the model proposed. The two tests above cover the other half: + // a schema with no fields to build a form from still runs the proposal as sent. + it('test_run_script previews the arguments the form submitted', async () => { + vi.mocked(ScriptService.getScriptByPath).mockResolvedValueOnce({ + path: 'f/scripts/formed-test', + summary: 'Formed test script', + content: 'export async function main(name: string) {}', + language: 'bun', + schema: { properties: { name: { type: 'string' } } } + } as any) + + let opened: Record | undefined + let kind: string | undefined + await withCompletedTestJob(() => + callGlobalTool( + 'test_run_script', + { path: 'f/scripts/formed-test', args: { name: 'Ada' } }, + { + ...toolCallbacks, + requestRunArgs: async (_toolId, form) => { + opened = form.args + kind = form.kind + return { name: 'Grace' } + } + } + ) + ) + + expect(opened).toEqual({ name: 'Ada' }) + // Drives the card's tense: a test says it tested, not that it ran. + expect(kind).toBe('test') + expect(JobService.runScriptPreview).toHaveBeenCalledWith({ + workspace: WORKSPACE, + requestBody: { + path: 'f/scripts/formed-test', + content: 'export async function main(name: string) {}', + args: { name: 'Grace' }, + language: 'bun' + } + }) + }) + it('test_run_flow previews draft flow content by path', async () => { const modules = [{ id: 'start', value: { type: 'identity' } }] await callGlobalTool('write_flow', { diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index bd84f14023..bb80c7a65c 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -899,7 +899,7 @@ const testRunScriptSchema = z.object({ const testRunScriptToolDef = createToolDef( testRunScriptSchema, 'test_run_script', - 'Execute a preview-style test run of a script by path, preferring draft content when it exists.', + 'Execute a preview-style test run of a script by path, preferring draft content when it exists. The user gets an argument form prefilled with `args` and may edit or dismiss it before it runs, so fill in every argument you can infer.', { strict: false } ) @@ -3665,8 +3665,9 @@ export const globalTools: Tool<{}>[] = [ const parsed = testRunScriptSchema.parse(ctx.args) return testRunScriptByPath(parsed, ctx) }, - requiresConfirmation: true, - confirmationMessage: (args) => `Run a test of ${pathLeaf(args?.path, 'the script')}`, + // No requiresConfirmation: like run_script, the argument form is the confirmation — + // but this one is auto-acceptable, so YOLO answers it and the model keeps iterating. + streamingLabel: 'Preparing the test form...', queuedLabel: (args) => `Test ${args?.path ?? 'the script'}`, showDetails: true, autoCollapseDetails: false @@ -5164,16 +5165,50 @@ function writeVariableDraft(args: WriteVariableArgs, ctx: WriteDraftCtx): Promis async function loadScriptForEdit( path: string, workspace: string -): Promise<{ content: string; language: ScriptLang; summary?: string }> { +): Promise<{ + content: string + language: ScriptLang + summary?: string + schema?: Record +}> { const draft = await getGlobalDraft(workspace, 'script', path) if (draft) { if (typeof draft.value !== 'string' || !draft.language) { throw new Error(`Draft script "${path}" is missing content or language.`) } - return { content: draft.value, language: draft.language, summary: draft.summary } + return { + content: draft.value, + language: draft.language, + summary: draft.summary, + schema: draft.schema as Record | undefined + } } const script = await ScriptService.getScriptByPath({ workspace, path }) - return { content: script.content, language: script.language, summary: script.summary } + return { + content: script.content, + language: script.language, + summary: script.summary, + schema: script.schema as Record | undefined + } +} + +/** The fields a test form offers, for code that may never have been deployed. A draft the + * chat wrote carries the schema it inferred at write time; anything else — a draft written + * elsewhere, a deployed script whose schema predates an edit — is inferred here from the + * content that is about to run, so the form cannot offer a field the code no longer takes. */ +async function schemaForTestRun(script: { + content: string + language: ScriptLang + schema?: Record +}): Promise> { + if (script.schema?.properties) return script.schema + const schema = emptySchema() + try { + await inferArgs(script.language, script.content, schema) + } catch (e) { + console.error('Failed to infer script schema for the test run form', e) + } + return schema as unknown as Record } async function editScript( @@ -5420,54 +5455,113 @@ async function testRunScriptByPath( ): Promise { const { workspace, toolId, toolCallbacks } = ctx const script = await loadScriptForEdit(args.path, workspace) + const schema = await schemaForTestRun(script) const testArgs = normalizeTestRunArgs(args.args) - return executeTestRun({ - jobStarter: () => - JobService.runScriptPreview({ - workspace, - requestBody: { - path: args.path, - content: script.content, - args: testArgs, - language: script.language - } - }), - workspace, - toolCallbacks, - toolId, - startMessage: `Running test for script "${args.path}"...`, - contextName: 'script', - background: args.background, - detachAfterMs: waitSecondsToDetachMs(args.wait_seconds), - label: args.path - }) + // A schema declaring no field at all is indistinguishable from one this could not read, + // and a form built from it would drop every argument the model proposed without either + // of them being able to tell why. Run what was asked for instead — it is what this tool + // did before it grew a form, and a form with no fields was never going to add consent. + if (Object.keys(schema.properties ?? {}).length === 0 && Object.keys(testArgs ?? {}).length > 0) { + return executeTestRun({ + jobStarter: () => + JobService.runScriptPreview({ + workspace, + requestBody: { + path: args.path, + content: script.content, + args: testArgs, + language: script.language + } + }), + workspace, + toolCallbacks, + toolId, + startMessage: `Running test for script "${args.path}"...`, + contextName: 'script', + background: args.background, + detachAfterMs: waitSecondsToDetachMs(args.wait_seconds), + label: args.path + }) + } + + return runThroughForm( + { + path: args.path, + schema, + summary: script.summary, + kind: 'test', + // Never "deployed" here: the code about to run is the draft the model is still + // writing, and a line telling it to re-read the deployed schema would send it + // to the wrong version. + schemaNoun: 'script', + toolName: 'test_run_script', + proposed: args.args, + startMessage: `Running test for script "${args.path}"...`, + contextName: 'script', + // Its own loop: the model is told to test and iterate, so YOLO answers the form + // with what it opened with rather than parking the loop on a card. + autoAcceptable: true, + background: args.background, + detachAfterMs: waitSecondsToDetachMs(args.wait_seconds), + startJob: (submitted) => + JobService.runScriptPreview({ + workspace, + requestBody: { + path: args.path, + content: script.content, + args: submitted, + language: script.language + } + }) + }, + ctx + ) } /** 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 RUN_FORM_CANCELLED = - 'The user cancelled the run form. The script did NOT run. Do not call run_script again unless the user asks for it.' +const runFormCancelled = (toolName: string) => + `The user cancelled the run form. The script did NOT run. Do not call ${toolName} again unless the user asks for it.` /** The model only needs to see what the user changed, and nothing bounds an object or * array argument the form let them paste into. */ const MAX_SUBMITTED_ARGS_LENGTH = 4000 -async function runDeployedScript( - args: z.infer, - ctx: WriteDraftCtx -): Promise { +/** One run through an argument form: conform what the model proposed to the schema of the + * version about to run, open the form on it, then run whatever came back. Both tools that + * run a script are this, differing only in where the schema comes from and how the job + * starts — so the user meets one card whichever they asked for. */ +type FormRunSpec = { + path: string + schema: Record + summary?: string + kind: 'run' | 'test' + /** How the lines the model reads back name the version this ran: telling it to re-read + * the "deployed schema" of a draft would send it to the wrong code. */ + schemaNoun: string + toolName: string + proposed: Record | null | undefined + startMessage: string + contextName: 'script' | 'flow' + /** Whether YOLO may answer this form. See requestRunArgs. */ + autoAcceptable?: boolean + background?: boolean + detachAfterMs?: number + startJob: (submitted: Record) => Promise +} + +async function runThroughForm(spec: FormRunSpec, ctx: WriteDraftCtx): Promise { const { workspace, toolId, toolCallbacks } = ctx - if (!toolCallbacks.requestRunArgs) { + // A host with no form can still run what YOLO would have answered for; a deployed run + // has no such fallback, since the form is the only place its consent comes from. + if (!toolCallbacks.requestRunArgs && !spec.autoAcceptable) { return 'This chat cannot show a run form, so a deployed script cannot be run from here.' } - // No getDraft: this runs the script as it is live, so the form has to offer the - // inputs the live version accepts and not a draft's. - const script = await ScriptService.getScriptByPath({ workspace, path: args.path }) - const schema = (script.schema as Record) ?? {} - const conformed = conformArgsToSchema(normalizeTestRunArgs(args.args), schema) + const schema = spec.schema + const conformed = conformArgsToSchema(normalizeTestRunArgs(spec.proposed), schema) // A secret the model picked is not consent, whatever it holds: a literal is a value // the user never chose, a reference names something the card cannot show them. Files // go the same way — prefilled bytes are bytes the stored transcript then carries. @@ -5478,8 +5572,9 @@ async function runDeployedScript( strippedKeys ) const form: RunFormDisplay = { - path: args.path, - summary: script.summary || undefined, + path: spec.path, + summary: spec.summary || undefined, + kind: spec.kind, schema, args: proposed, droppedKeys: conformed.dropped.undeclared.length ? conformed.dropped.undeclared : undefined, @@ -5489,7 +5584,7 @@ async function runDeployedScript( } toolCallbacks.setToolStatus(toolId, { - content: `Waiting for you to confirm the arguments of "${args.path}"`, + content: `Waiting for you to confirm the arguments of "${spec.path}"`, runForm: form, // Not the raw tool-call arguments: the card settles on what the form opened with. // Only settles it — the raw proposal still renders while the call streams in. @@ -5497,16 +5592,18 @@ async function runDeployedScript( isLoading: true }) - const submitted = await toolCallbacks.requestRunArgs(toolId, form) + const submitted = toolCallbacks.requestRunArgs + ? await toolCallbacks.requestRunArgs(toolId, form, { autoAcceptable: spec.autoAcceptable }) + : proposed if (!submitted) { toolCallbacks.setToolStatus(toolId, { - content: `Run of "${args.path}" cancelled by user`, + content: `Run of "${spec.path}" cancelled by user`, isLoading: false, isStreamingArguments: false, error: 'Cancelled by user', declinedByUser: true }) - return RUN_FORM_CANCELLED + return runFormCancelled(spec.toolName) } // processToolCall re-gates plan mode after a standard confirmation, because it can be @@ -5532,11 +5629,7 @@ async function runDeployedScript( const outcome = await executeTestRun({ jobStarter: async () => { - const jobId = await JobService.runScriptByPath({ - workspace, - path: args.path, - requestBody: submitted - }) + const jobId = await spec.startJob(submitted) // The form's own submitted flag flips a round trip earlier, when the user presses // Run; only from here is there a job for a stopped turn to say it left running. toolCallbacks.markRunFormStarted?.(toolId) @@ -5545,22 +5638,25 @@ async function runDeployedScript( workspace, toolCallbacks, toolId, - startMessage: `Running "${args.path}"...`, - contextName: 'script', - actionNoun: 'run', - label: args.path + startMessage: spec.startMessage, + contextName: spec.contextName, + actionNoun: spec.kind === 'test' ? 'test' : 'run', + background: spec.background, + detachAfterMs: spec.detachAfterMs, + label: spec.path }) + const schemaNoun = `${spec.schemaNoun} schema` const dropped = conformed.dropped.undeclared.length - ? `\nThe deployed schema declares no ${conformed.dropped.undeclared.join(', ')}, so the form never offered ${conformed.dropped.undeclared.length > 1 ? 'them' : 'it'} and the run did not carry ${conformed.dropped.undeclared.length > 1 ? 'them' : 'it'}.` + ? `\nThe ${schemaNoun} declares no ${conformed.dropped.undeclared.join(', ')}, so the form never offered ${conformed.dropped.undeclared.length > 1 ? 'them' : 'it'} and the run did not carry ${conformed.dropped.undeclared.length > 1 ? 'them' : 'it'}.` : '' // Told apart from `dropped`, or the model reads "no such argument" and stops sending // one the script does have, instead of sending it in the shape the schema asks for. const unshowable = conformed.dropped.unshowable.length - ? `\nThe deployed schema does declare ${conformed.dropped.unshowable.join(', ')}, but you sent ${conformed.dropped.unshowable.length > 1 ? 'them in shapes' : 'it in a shape'} the form has no field for, so the run did not carry ${conformed.dropped.unshowable.length > 1 ? 'them' : 'it'}. Re-read the input schema and match ${conformed.dropped.unshowable.length > 1 ? 'their declared types' : 'its declared type'}.` + ? `\nThe ${schemaNoun} does declare ${conformed.dropped.unshowable.join(', ')}, but you sent ${conformed.dropped.unshowable.length > 1 ? 'them in shapes' : 'it in a shape'} the form has no field for, so the run did not carry ${conformed.dropped.unshowable.length > 1 ? 'them' : 'it'}. Re-read the input schema and match ${conformed.dropped.unshowable.length > 1 ? 'their declared types' : 'its declared type'}.` : '' const reset = conformed.resetKeys.length - ? `\nThe deployed schema disables ${conformed.resetKeys.join(', ')}, so the form held ${conformed.resetKeys.length > 1 ? 'their defaults' : 'its default'} rather than the proposed ${conformed.resetKeys.length > 1 ? 'values' : 'value'}. Do not propose ${conformed.resetKeys.length > 1 ? 'them' : 'it'} again.` + ? `\nThe ${schemaNoun} disables ${conformed.resetKeys.join(', ')}, so the form held ${conformed.resetKeys.length > 1 ? 'their defaults' : 'its default'} rather than the proposed ${conformed.resetKeys.length > 1 ? 'values' : 'value'}. Do not propose ${conformed.resetKeys.length > 1 ? 'them' : 'it'} again.` : '' // Otherwise an emptied field reads as the user having deleted it, and the next call // proposes the same secret again. @@ -5584,6 +5680,33 @@ async function runDeployedScript( return `${ran}${dropped}${unshowable}${reset}${stripped}\n${outcome}` } +async function runDeployedScript( + args: z.infer, + ctx: WriteDraftCtx +): Promise { + const { workspace } = ctx + // No getDraft: this runs the script as it is live, so the form has to offer the + // inputs the live version accepts and not a draft's. + const script = await ScriptService.getScriptByPath({ workspace, path: args.path }) + return runThroughForm( + { + path: args.path, + schema: (script.schema as Record) ?? {}, + summary: script.summary, + kind: 'run', + schemaNoun: 'deployed', + toolName: 'run_script', + proposed: args.args, + startMessage: `Running "${args.path}"...`, + contextName: 'script', + // The form is this call's only consent, so nothing may answer it but the user. + startJob: (submitted) => + JobService.runScriptByPath({ workspace, path: args.path, requestBody: submitted }) + }, + ctx + ) +} + async function testRunFlowByPath( args: z.infer, ctx: WriteDraftCtx diff --git a/frontend/src/lib/components/copilot/chat/shared.ts b/frontend/src/lib/components/copilot/chat/shared.ts index 361b3fc4b1..406daf59e0 100644 --- a/frontend/src/lib/components/copilot/chat/shared.ts +++ b/frontend/src/lib/components/copilot/chat/shared.ts @@ -558,10 +558,13 @@ export function answeredChoices(q: UserQuestionDisplay): string[] | undefined { export type RunFormDisplay = { path: string summary?: string - /** Of the DEPLOYED script, not a draft. Only the rendered form reads it, so it is - * dropped once one of the flags below unmounts that form: kept, every settled card - * would carry a copy of the schema — password and file defaults included — in - * history forever. */ + /** What the run is, in the card's own words: a deployed script run, or a preview of the + * draft being written. Only the tense of the row's label turns on it. */ + kind?: 'run' | 'test' + /** Of whatever version is about to run: the deployed script, or the draft a test + * previews. Only the rendered form reads it, so it is dropped once one of the flags + * below unmounts that form: kept, every settled card would carry a copy of the schema + * — password and file defaults included — in history forever. */ schema?: Record /** Prefill only: the card's `parameters` records what the job started with. */ args: Record @@ -1305,10 +1308,15 @@ export interface ToolCallbacks { question: UserQuestionDisplay ) => Promise /** Park the loop on an argument form and resolve with the args the user submitted, - * or undefined if they cancelled. Wired only where the form can be rendered. */ + * or undefined if they cancelled. Wired only where the form can be rendered. + * + * `autoAcceptable` opts the form into the YOLO posture, which answers it with what it + * opened with. Only a run the user can undo by editing the code may set it: a deployed + * run is not one, which is why its form is the confirmation YOLO cannot skip. */ requestRunArgs?: ( toolId: string, - form: RunFormDisplay + form: RunFormDisplay, + opts?: { autoAcceptable?: boolean } ) => Promise | undefined> /** The submitted form's job is queued. Wired alongside requestRunArgs. */ markRunFormStarted?: (toolId: string) => void