fix: filter run-form arguments at every level, and keep the form's own keys out of the turn

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte

@@ -224,12 +224,13 @@
 			const focusOnChat =
 				!active || active === document.body || (panelEl?.contains(active) ?? false)
 			if (!focusOnChat) return
-			// The run form parks the loop on the user, so an Escape aimed at one of its
-			// fields must not discard what they typed. Only the fields: from its buttons
-			// Escape still stops the turn, which is the way out while a submit is in flight.
+			// The run form parks the loop on the user, so an Escape aimed at it must not
+			// discard what they typed — its widgets are buttons as often as fields (a oneOf
+			// branch toggle, add-item, the pickers). Only the action row still stops the
+			// turn, which is the way out while a submit is in flight.
 			if (
 				active?.closest('[data-chat-keyboard-scope="run-args-form"]') &&
-				active.matches('input, textarea, select, [contenteditable]')
+				!active.closest('[data-run-form-actions]')
 			) {
 				return
 			}

frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts

@@ -1798,6 +1798,8 @@ export class AIChatManager {
 		})
 	}
 
+	markRunFormStarted = (toolId: string) => this.#patchRunForm(toolId, { started: true })
+
 	// A form restored from history has no callback: the loop that opened it is gone.
 	isRunFormPending = (toolId: string): boolean => this.runFormCallbacks.has(toolId)
 
@@ -3762,6 +3764,7 @@ export class AIChatManager {
 					onToolBlockedByPlanMode: this.planMode.noteBlockedTool,
 					requestUserQuestion: this.requestUserQuestion,
 					requestRunArgs: this.requestRunArgs,
+					markRunFormStarted: this.markRunFormStarted,
 					onItemModified: (kind, path) => this.recordModifiedItem(kind, path),
 					onItemDeployed: (kind, from, to) => void this.renameModifiedItem(kind, from, to),
 					onItemDiscarded: (kind, path) => void this.removeModifiedItem(kind, path),
@@ -4591,9 +4594,9 @@ export class AIChatManager {
 	): DisplayMessage[] =>
 		messages.map((message) => {
 			if (message.role === 'tool' && (message.isLoading || message.isQueued)) {
-				// Stopping the turn does not stop the job: once the form was submitted the
-				// script is running for real, so the card must not claim it was canceled.
-				const ranAlready = message.runForm?.submitted === true
+				// Stopping the turn does not stop the job: once the job is queued the script
+				// is running for real, so the card must not claim it was canceled.
+				const ranAlready = message.runForm?.started === true
 				return {
 					...message,
 					isLoading: false,

frontend/src/lib/components/copilot/chat/AIChatManager.test.ts

@@ -282,7 +282,29 @@ describe('AIChatManager run form', () => {
 
 	// Stop ends the turn, not the job: the deployed script is already running with all
 	// its side effects, so the transcript must not record it as cancelled.
-	it('does not mark a submitted run cancelled when the turn is stopped', () => {
+	it('does not mark a started run cancelled when the turn is stopped', () => {
+		const manager = new AIChatManager()
+		manager.displayMessages = [
+			{
+				role: 'tool',
+				tool_call_id: 'call_s',
+				content: 'Running "f/a/b"...',
+				isLoading: true,
+				runForm: { path: 'f/a/b', schema: {}, args: {}, submitted: true, started: true }
+			}
+		]
+
+		manager.cancelLoadingTools()
+
+		const settled = manager.displayMessages[0]
+		expect(settled.runForm?.canceled).toBe(false)
+		expect(settled.error).toBe(undefined)
+		expect(settled.isLoading).toBe(false)
+	})
+
+	// Run flips `submitted` a round trip before the job exists. Stop in that window
+	// cancelled nothing that ran, so the card must not claim the script was left running.
+	it('marks a submitted run cancelled while its job has not started', () => {
 		const manager = new AIChatManager()
 		manager.displayMessages = [
 			{
@@ -297,9 +319,8 @@ describe('AIChatManager run form', () => {
 		manager.cancelLoadingTools()
 
 		const settled = manager.displayMessages[0]
-		expect(settled.runForm?.canceled).toBe(false)
-		expect(settled.error).toBe(undefined)
-		expect(settled.isLoading).toBe(false)
+		expect(settled.runForm?.canceled).toBe(true)
+		expect(settled.content).toBe('Run f/a/b — Canceled')
 	})
 })
 

frontend/src/lib/components/copilot/chat/RunArgsFormDisplay.svelte

@@ -123,8 +123,8 @@
 
 	<!-- Both buttons rest while a submit is in flight: the ephemeral variables exist by
 	     then, so cancelling would settle the call as declined on a run that is already
-	     starting. Escape from a field still stops the turn. -->
-	<div class="mt-3 flex items-center gap-2">
+	     starting. Marked as the one part of the form Escape still stops the turn from. -->
+	<div class="mt-3 flex items-center gap-2" data-run-form-actions>
 		<Button
 			variant="accent"
 			unifiedSize="sm"

frontend/src/lib/components/copilot/chat/global/core.test.ts

@@ -4709,6 +4709,7 @@ describe('global AI tools', () => {
 		} as any)
 
 		let shown: Record<string, any> | undefined
+		const statuses: any[] = []
 		const result = await withCompletedTestJob(() =>
 			callGlobalTool(
 				'run_script',
@@ -4722,6 +4723,7 @@ describe('global AI tools', () => {
 				},
 				{
 					...toolCallbacks,
+					setToolStatus: (_toolId: string, status: any) => statuses.push(status),
 					requestRunArgs: async (_toolId, form) => {
 						shown = form.args
 						return { ...form.args, token: '$var:u/ada/secret_arg/typed' }
@@ -4735,6 +4737,9 @@ describe('global AI tools', () => {
 		expect(result).not.toContain('secret_arg')
 		expect(result).not.toContain('prod_api_key')
 		expect(result).toContain('ada')
+		// The card's parameters are persisted too: a variable path is enough to run a job
+		// on a value whoever reads the transcript cannot see.
+		expect(JSON.stringify(statuses)).not.toContain('secret_arg')
 	})
 
 	// The form is its own confirmation, so it never reaches processToolCall's second gate.
@@ -4815,18 +4820,25 @@ describe('global AI tools', () => {
 
 		const bytes = 'QUJD'.repeat(1024)
 		const statuses: any[] = []
+		let shown: Record<string, any> | undefined
 		const result = await withCompletedTestJob(() =>
 			callGlobalTool(
 				'run_script',
-				{ path: 'f/scripts/upload', args: {} },
+				// Proposed, not just user-attached: prefilled bytes are bytes the stored
+				// transcript carries, for a value no model can produce anyway.
+				{ path: 'f/scripts/upload', args: { doc: bytes } },
 				{
 					...toolCallbacks,
 					setToolStatus: (_toolId: string, status: any) => statuses.push(status),
-					requestRunArgs: async () => ({ doc: bytes })
+					requestRunArgs: async (_toolId, form) => {
+						shown = form.args
+						return { doc: bytes }
+					}
 				}
 			)
 		)
 
+		expect(shown).toEqual({})
 		expect(JobService.runScriptByPath).toHaveBeenCalledWith({
 			workspace: WORKSPACE,
 			path: 'f/scripts/upload',

frontend/src/lib/components/copilot/chat/global/core.ts

@@ -51,6 +51,7 @@ import {
 	conformArgsToSchema,
 	redactFileArgs,
 	redactSecretArgs,
+	stripFileArgs,
 	stripSecretArgs
 } from '$lib/components/job_args'
 import { PLAN_MODE_MESSAGES } from '../planModeMessages'
@@ -5466,8 +5467,9 @@ async function runDeployedScript(
 	const schema = (script.schema as Record<string, any>) ?? {}
 	const conformed = conformArgsToSchema(normalizeTestRunArgs(args.args), 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.
-	const proposed = stripSecretArgs(conformed.args, schema as any)
+	// 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.
+	const proposed = stripFileArgs(stripSecretArgs(conformed.args, schema as any), schema as any)
 	const form: RunFormDisplay = {
 		path: args.path,
 		summary: script.summary || undefined,
@@ -5513,12 +5515,24 @@ async function runDeployedScript(
 		return PLAN_MODE_MESSAGES.blockedResult
 	}
 
-	// The card's details pane must show what ran, not what was proposed.
-	toolCallbacks.setToolStatus(toolId, { parameters: redactFileArgs(submitted, schema as any) })
+	// The card's details pane must show what ran, not what was proposed — and it is
+	// persisted, so it carries no more of a secret or a file than the model's copy does.
+	toolCallbacks.setToolStatus(toolId, {
+		parameters: redactFileArgs(redactSecretArgs(submitted, schema as any), schema as any)
+	})
 
 	const outcome = await executeTestRun({
-		jobStarter: () =>
-			JobService.runScriptByPath({ workspace, path: args.path, requestBody: submitted }),
+		jobStarter: async () => {
+			const jobId = await JobService.runScriptByPath({
+				workspace,
+				path: args.path,
+				requestBody: 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)
+			return jobId
+		},
 		workspace,
 		toolCallbacks,
 		toolId,

frontend/src/lib/components/copilot/chat/shared.ts

@@ -572,6 +572,9 @@ export type RunFormDisplay = {
 	 * stopped waiting on this card. */
 	submitted?: boolean
 	canceled?: boolean
+	/** The job exists. Distinct from `submitted`, which flips a round trip earlier: a turn
+	 * stopped in between must not record a run that never started. */
+	started?: boolean
 }
 
 /** One page hit from a provider-side web search (OpenAI sources carry no title). */
@@ -1285,6 +1288,8 @@ export interface ToolCallbacks {
 		toolId: string,
 		form: RunFormDisplay
 	) => Promise<Record<string, any> | undefined>
+	/** The submitted form's job is queued. Wired alongside requestRunArgs. */
+	markRunFormStarted?: (toolId: string) => void
 	/** Records a workspace item the tool call created/edited/deleted, by its
 	 * canonical (itemKind, storagePath). Session chats wire this to accumulate the
 	 * chat's modified-items mask; the global side-panel chat omits it (no-op). */

frontend/src/lib/components/job_args.test.ts

@@ -34,6 +34,42 @@ describe('conformArgsToSchema', () => {
 		const { args } = conformArgsToSchema(JSON.parse('{"a":1}'), undefined)
 		expect(Object.getPrototypeOf(args)).toBe(Object.prototype)
 	})
+
+	// The mounted nested form prunes its own extras, but ArgInput renders only the first
+	// 50 array items, so past that nothing else would ever drop them.
+	it('drops undeclared arguments nested inside declared ones', () => {
+		const { args, droppedKeys } = conformArgsToSchema(
+			{ cfg: { batch: 10, evil: true }, rows: [{ mode: 'safe' }, { mode: 'safe', evil: true }] },
+			{
+				properties: {
+					cfg: { properties: { batch: { type: 'number' } } },
+					rows: { items: { properties: { mode: { type: 'string' } } } }
+				}
+			}
+		)
+		expect(args).toEqual({ cfg: { batch: 10 }, rows: [{ mode: 'safe' }, { mode: 'safe' }] })
+		expect(droppedKeys).toEqual(['cfg.evil', 'rows[1].evil'])
+	})
+
+	// ArgInput writes the tag itself and reads it back to pick the branch that opens, so
+	// dropping it would reopen the form on the wrong variant.
+	it('keeps the oneOf tag and every branch key', () => {
+		const { args, droppedKeys } = conformArgsToSchema(
+			{ either: { kind: 'b', level: 2, evil: true } },
+			{
+				properties: {
+					either: {
+						oneOf: [
+							{ title: 'a', properties: { name: { type: 'string' } } },
+							{ title: 'b', properties: { level: { type: 'number' } } }
+						]
+					}
+				}
+			}
+		)
+		expect(args).toEqual({ either: { kind: 'b', level: 2 } })
+		expect(droppedKeys).toEqual(['either.evil'])
+	})
 })
 
 describe('enforceDisabledDefaults', () => {
@@ -46,7 +82,8 @@ describe('enforceDisabledDefaults', () => {
 			},
 			either: {
 				oneOf: [
-					{ title: 'a', properties: { level: { type: 'number', default: 1, disabled: true } } }
+					{ title: 'a', properties: { level: { type: 'number', default: 1, disabled: true } } },
+					{ title: 'b', properties: { rate: { type: 'number', default: 5, disabled: true } } }
 				]
 			},
 			free: { type: 'string' }
@@ -81,6 +118,13 @@ describe('enforceDisabledDefaults', () => {
 		expect(args.top).toBe('fixed')
 		expect(resetKeys).toEqual([])
 	})
+
+	// Every branch is visited because the tag is runtime state, so writing an absent
+	// default would hand the run an argument from the variant nobody selected.
+	it('leaves the unselected oneOf branch out of the run', () => {
+		const { args } = enforceDisabledDefaults({ either: { kind: 'a', level: 99 } }, schema)
+		expect(args.either).toEqual({ kind: 'a', level: 1 })
+	})
 })
 
 describe('secret args at every level the form nests', () => {

frontend/src/lib/components/job_args.ts

@@ -4,8 +4,9 @@ const isLockedProp = (prop: any) => !!prop?.disabled && 'default' in prop
 
 /**
  * A field the schema disables is not the caller's to set: whatever it holds, the run
- * sends the schema's default. Returns the paths it actually overwrote so the caller can
- * say so — notifying is the caller's job, this stays pure.
+ * sends the schema's default. Only a field's own `disabled` counts — `SchemaForm`
+ * propagates a parent's downward, so one locked by inheritance alone keeps its value.
+ * Returns the paths it overwrote so the caller can say so; notifying is the caller's job.
  */
 export function enforceDisabledDefaults(
 	args: Record<string, any>,
@@ -26,32 +27,78 @@ export function enforceDisabledDefaults(
 
 /**
  * Conform caller-supplied arguments to what a run form can actually show: drop what the
- * schema does not declare, then apply {@link enforceDisabledDefaults}. An argument with
- * no field — including every argument of a script whose schema declares none — would
- * otherwise be approved without ever being seen.
+ * schema does not declare at any level, then apply {@link enforceDisabledDefaults}. An
+ * argument with no field — including every argument of a script whose schema declares
+ * none — would otherwise be approved without ever being seen. A mounted nested form
+ * prunes its own extras, but `ArgInput` renders only the first 50 items of an array.
  */
 export function conformArgsToSchema(
 	args: Record<string, any>,
 	schema: { properties?: Record<string, any> } | undefined
 ): { args: Record<string, any>; resetKeys: string[]; droppedKeys: string[] } {
-	const properties = schema?.properties ?? {}
-	// hasOwn, not `in`: every object inherits `constructor` and `toString`, so `in` would
-	// wave through arguments no schema declares. Null prototype for the same reason from
-	// the other side: assigning a declared `__proto__` into a plain `{}` reaches the
-	// inherited setter instead, and the argument vanishes unreported.
-	const known: Record<string, any> = Object.create(null)
 	const droppedKeys: string[] = []
-	for (const [key, value] of Object.entries(args ?? {})) {
-		if (Object.hasOwn(properties, key)) {
-			known[key] = value
-		} else {
-			droppedKeys.push(key)
-		}
-	}
+	const known = dropUndeclaredArgs(args ?? {}, schema?.properties ?? {}, droppedKeys)
 	const { args: result, resetKeys } = enforceDisabledDefaults(known, schema)
 	return { args: result, resetKeys, droppedKeys }
 }
 
+/** The tag naming the selected `oneOf` branch. No branch has to declare it, but
+ * `ArgInput` writes it into the value and reads it back to pick the branch that opens. */
+const ONE_OF_TAG_KEYS = ['kind', 'label']
+
+/**
+ * Rebuild `holder` with only the arguments `properties` declares, appending the dotted
+ * path of each one removed. Recurses down the levels the form nests, so an undeclared
+ * argument cannot ride along inside a declared one.
+ */
+function dropUndeclaredArgs(
+	holder: Record<string, any>,
+	properties: Record<string, any>,
+	droppedKeys: string[],
+	path = '',
+	alsoAllowed?: string[]
+): Record<string, any> {
+	// hasOwn, not `in`: every object inherits `constructor` and `toString`, so `in` would
+	// wave through arguments no schema declares. Accumulated on a null prototype for the
+	// same reason from the other side: assigning a declared `__proto__` into a plain `{}`
+	// reaches the inherited setter instead, and the argument vanishes unreported.
+	const kept: Record<string, any> = Object.create(null)
+	for (const [key, value] of Object.entries(holder)) {
+		const keyPath = path ? `${path}.${key}` : key
+		if (!Object.hasOwn(properties, key)) {
+			if (alsoAllowed?.includes(key)) kept[key] = value
+			else droppedKeys.push(keyPath)
+			continue
+		}
+		kept[key] = dropUndeclaredNested(value, properties[key], droppedKeys, keyPath)
+	}
+	// Spread rather than the accumulator itself: $state.snapshot returns a null-prototype
+	// object by identity, and a form editing it in place would write into the stored copy.
+	return { ...kept }
+}
+
+function dropUndeclaredNested(value: any, prop: any, droppedKeys: string[], path: string): any {
+	if (value == null || typeof value !== 'object') return value
+	if (prop?.properties && !Array.isArray(value)) {
+		return dropUndeclaredArgs(value, prop.properties, droppedKeys, path)
+	}
+	if (prop?.items?.properties && Array.isArray(value)) {
+		return value.map((item: any, i: number) =>
+			item != null && typeof item === 'object' && !Array.isArray(item)
+				? dropUndeclaredArgs(item, prop.items.properties, droppedKeys, `${path}[${i}]`)
+				: item
+		)
+	}
+	if (Array.isArray(prop?.oneOf) && !Array.isArray(value)) {
+		// The union of every branch, not the one the tag names: which branch is selected is
+		// runtime state, and pruning by a stale tag would delete what the user typed.
+		const union: Record<string, any> = {}
+		for (const branch of prop.oneOf) Object.assign(union, branch?.properties ?? {})
+		return dropUndeclaredArgs(value, union, droppedKeys, path, ONE_OF_TAG_KEYS)
+	}
+	return value
+}
+
 /**
  * Rebuild `holder` with `visit` applied to every argument whose schema property matches
  * `isLeaf`, at any depth; returning `undefined` removes that argument.
@@ -66,7 +113,8 @@ function mapMatchingArgs(
 	properties: Record<string, any>,
 	isLeaf: (prop: any) => boolean,
 	visit: (value: unknown, prop: any, path: string) => unknown,
-	path = ''
+	path = '',
+	inOneOf = false
 ): any {
 	if (holder == null || typeof holder !== 'object' || Array.isArray(holder)) return holder
 	const result = { ...holder }
@@ -75,6 +123,9 @@ function mapMatchingArgs(
 		// A matching object is a leaf, not a level: a password object is stored whole as a
 		// single $jsonvar: reference, and a file is one opaque base64 string.
 		if (isLeaf(prop)) {
+			// Every oneOf branch is visited, so an absent argument under one belongs to a
+			// variant that was not selected: a visitor that writes would add it to the run.
+			if (inOneOf && !Object.hasOwn(result, key)) continue
 			const mapped = visit(result[key], prop, keyPath)
 			if (mapped === undefined) delete result[key]
 			else result[key] = mapped
@@ -84,10 +135,10 @@ function mapMatchingArgs(
 		// leave the key behind holding undefined.
 		if (!Object.hasOwn(result, key)) continue
 		if (prop?.properties) {
-			result[key] = mapMatchingArgs(result[key], prop.properties, isLeaf, visit, keyPath)
+			result[key] = mapMatchingArgs(result[key], prop.properties, isLeaf, visit, keyPath, inOneOf)
 		} else if (prop?.items?.properties && Array.isArray(result[key])) {
 			result[key] = result[key].map((item: any, i: number) =>
-				mapMatchingArgs(item, prop.items.properties, isLeaf, visit, `${keyPath}[${i}]`)
+				mapMatchingArgs(item, prop.items.properties, isLeaf, visit, `${keyPath}[${i}]`, inOneOf)
 			)
 		} else if (Array.isArray(prop?.oneOf)) {
 			// Every branch, not the one the value's tag names: which branch is selected is
@@ -95,7 +146,14 @@ function mapMatchingArgs(
 			// is visited.
 			for (const branch of prop.oneOf) {
 				if (branch?.properties) {
-					result[key] = mapMatchingArgs(result[key], branch.properties, isLeaf, visit, keyPath)
+					result[key] = mapMatchingArgs(
+						result[key],
+						branch.properties,
+						isLeaf,
+						visit,
+						keyPath,
+						true
+					)
 				}
 			}
 		}
@@ -128,6 +186,20 @@ export function stripSecretArgs(
 	return mapMatchingArgs(args, properties, isSecretProp, () => undefined)
 }
 
+/**
+ * Drop every file argument, so a caller cannot propose file bytes on the user's behalf:
+ * the field opens empty and the user attaches the file. Bytes a form is prefilled with
+ * are bytes the stored transcript carries, unbounded, for a value no caller can produce.
+ */
+export function stripFileArgs(
+	args: Record<string, any>,
+	schema: { properties?: Record<string, any> } | undefined
+): Record<string, any> {
+	const properties = schema?.properties
+	if (!properties) return args
+	return mapMatchingArgs(args, properties, isFileProp, () => undefined)
+}
+
 /**
  * Replace every password-typed argument with a fixed marker, for text that leaves the
  * form. A reference is enough to run a job on something the reader cannot see.