From 7ab58d43b2d6649c19e2ade09d2b07438e6b84d9 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 12 Nov 2026 03:24:29 -0800 Subject: [PATCH] Prove effort removal without moving unrelated arguments --- .../runtime-worktree-agent-startup.test.ts | 2 +- .../AgentLaunchSetupPanel.prompt.test.tsx | 8 +- .../pane/agent-launch-through-host.test.ts | 2 +- .../choices/agent-choice-args-analysis.ts | 77 ++------- .../choices/agent-choice-argument-cut.ts | 150 ++++++++++++++++++ .../choices/agent-choice-reconciliation.ts | 3 +- .../startup/terminal-effort-option-edit.ts | 3 +- ...ui-agent-launch-command-pick-flags.test.ts | 53 +++++-- 8 files changed, 215 insertions(+), 83 deletions(-) create mode 100644 src/shared/agent-launch/choices/agent-choice-argument-cut.ts diff --git a/src/main/runtime/workspaces/startup/runtime-worktree-agent-startup.test.ts b/src/main/runtime/workspaces/startup/runtime-worktree-agent-startup.test.ts index 1113bb8c345..08ac06653c1 100644 --- a/src/main/runtime/workspaces/startup/runtime-worktree-agent-startup.test.ts +++ b/src/main/runtime/workspaces/startup/runtime-worktree-agent-startup.test.ts @@ -107,7 +107,7 @@ it.each([false, true])( terminalOptionEdits: inputs?.startupTerminalOptionEdits }) }) - expect(tokenizeStartupCommand(result.effectiveStartup?.command ?? '', 'posix')).toEqual({ + expect(tokenizeStartupCommand(result.effectiveStartup?.command ?? '', 'posix')).toMatchObject({ ok: true, tokens: ['agy', '--dangerously-skip-permissions', '--add-dir', 'current'] }) diff --git a/src/renderer/src/components/agent/AgentLaunchSetupPanel.prompt.test.tsx b/src/renderer/src/components/agent/AgentLaunchSetupPanel.prompt.test.tsx index ad6294084f5..e340eadeccf 100644 --- a/src/renderer/src/components/agent/AgentLaunchSetupPanel.prompt.test.tsx +++ b/src/renderer/src/components/agent/AgentLaunchSetupPanel.prompt.test.tsx @@ -136,11 +136,11 @@ it.each(['dedicated', 'arguments'] as const)( ok: true, tokens: [ 'antigravity', - '--model', - 'gemini-3.8-flash', '--dangerously-skip-permissions', '--add-dir', - 'docs' + 'docs', + '--model', + 'gemini-3.8-flash' ] }) } @@ -408,7 +408,7 @@ it('removes a model-less saved Low chip and uses current destination Arguments a agentDefaultArgs: { antigravity: '--effort=medium --add-dir new' }, agentLaunchChoices: { antigravity: { options: { effort: 'high' } } } }) - ).toEqual({ + ).toMatchObject({ ok: true, tokens: ['antigravity', '--dangerously-skip-permissions', '--add-dir', 'new'] }) diff --git a/src/renderer/src/lib/agent-session/launch/pane/agent-launch-through-host.test.ts b/src/renderer/src/lib/agent-session/launch/pane/agent-launch-through-host.test.ts index 46970db6a1a..0048494a446 100644 --- a/src/renderer/src/lib/agent-session/launch/pane/agent-launch-through-host.test.ts +++ b/src/renderer/src/lib/agent-session/launch/pane/agent-launch-through-host.test.ts @@ -162,7 +162,7 @@ it.each([null, 'ssh-1'])( { worktreeId: WT, tabId, leafId: launchTab(tabId)!.agentLaunchPane!.leafId }, { connectionId, ...(connectionId ? { startupCommandDelivery: 'shell-ready' as const } : {}) } ) - expect(tokenizeStartupCommand(planned.command ?? '', 'posix')).toEqual({ + expect(tokenizeStartupCommand(planned.command ?? '', 'posix')).toMatchObject({ ok: true, tokens: ['agy', '--dangerously-skip-permissions', '--add-dir', 'current'] }) diff --git a/src/shared/agent-launch/choices/agent-choice-args-analysis.ts b/src/shared/agent-launch/choices/agent-choice-args-analysis.ts index 4fa9dd43a21..fe2d2fc7311 100644 --- a/src/shared/agent-launch/choices/agent-choice-args-analysis.ts +++ b/src/shared/agent-launch/choices/agent-choice-args-analysis.ts @@ -1,3 +1,4 @@ +import { cutAgentChoiceArgs } from './agent-choice-argument-cut' import { agentChoiceCliForms, permissionOptionInventory, @@ -229,69 +230,19 @@ export function analyzeChoiceArgs( } } const cut = ( - selected: readonly Pick[] - ): string | null => { - if (!first || !selected.length) { - return selected.length ? null : text - } - const ranges = [...selected].sort((a, b) => a.start - b.start) - if (ranges.some((range, index) => index > 0 && range.start < ranges[index - 1].end)) { - return null - } - const replacements = new Map( - ranges.filter((range) => range.replacement).map((range) => [range.start, range.replacement]) - ) - const chunks: string[] = [] - let cursor = 0 - for (const { start, end, replacement } of ranges) { - chunks.push(text.slice(cursor, start), replacement ?? '') - cursor = end - } - chunks.push(text.slice(cursor)) - const args = chunks.join('').trim() - const proven = originals.every((original, grammarIndex) => { - if (!original) { - return false - } - let rangeIndex = 0 - const removed = original.parsed.spans.map((span) => { - while (rangeIndex < ranges.length && ranges[rangeIndex].end <= span.start) { - rangeIndex++ - } - const range = ranges[rangeIndex] - return !!range && span.start >= range.start && span.end <= range.end - }) - const kept = original.parsed.tokens.filter((_, index) => !removed[index]) - const expected = original.parsed.tokens.flatMap((token, index) => { - if (!removed[index]) { - return [token] - } - const text = replacements.get(original.parsed.spans[index].start) - if (!text) { - return [] - } - const replacement = tokenizeStartupCommand(text, grammars[grammarIndex]) - return replacement.ok ? replacement.tokens : [] - }) - const parsed = tokenizeStartupCommand(args, grammars[grammarIndex]) - if ( - !parsed.ok || - expected.length !== parsed.tokens.length || - !expected.every((token, index) => token === parsed.tokens[index]) - ) { - return false - } - const prefix = original.parsed.tokens.filter((_, index) => removed[index]) - const composed = analyzeAgentChoiceWords(agent, [...prefix, ...kept], inventory) - const roles = original.analysis.roles.filter((_, index) => !removed[index]) - const leading = removed.slice(0, removed.lastIndexOf(true) + 1).every(Boolean) - return ( - leading || - (!original.analysis.roles.includes('uncertain') && - roles.every((role, index) => role === composed.roles[prefix.length + index])) - ) + selected: readonly Pick[], + purpose?: 'delete' + ) => + cutAgentChoiceArgs({ + agent, + text, + grammars, + originals, + occurrences, + inventory, + selected, + purpose }) - return proven ? args : null - } + return { occurrences, cut, opaqueKeys } } diff --git a/src/shared/agent-launch/choices/agent-choice-argument-cut.ts b/src/shared/agent-launch/choices/agent-choice-argument-cut.ts new file mode 100644 index 00000000000..5f8fa895236 --- /dev/null +++ b/src/shared/agent-launch/choices/agent-choice-argument-cut.ts @@ -0,0 +1,150 @@ +import { + analyzeAgentChoiceWords, + optionName, + type permissionOptionInventory +} from '../permissions/agent-permission-args' +import { tokenizeStartupCommand, type AgentStartupShell } from '../startup/tui-agent-startup-shell' +import type { TuiAgent } from '../catalog/tui-agent' +import type { ChoiceOccurrence } from './agent-launch-choices' + +type ParsedArguments = Extract, { ok: true }> + +export function cutAgentChoiceArgs(input: { + agent: TuiAgent + text: string + grammars: readonly AgentStartupShell[] + originals: readonly ({ + parsed: ParsedArguments + analysis: ReturnType + } | null)[] + occurrences: readonly ChoiceOccurrence[] + inventory: ReturnType + selected: readonly Pick[] + purpose?: 'delete' +}): string | null { + const { agent, text, grammars, originals, occurrences, inventory, selected, purpose } = input + const first = originals[0] + + if (!first || !selected.length) { + return selected.length ? null : text + } + const ranges = [...selected].sort((a, b) => a.start - b.start) + if (ranges.some((range, index) => index > 0 && range.start < ranges[index - 1].end)) { + return null + } + const subtraction = + purpose === 'delete' && ranges.every((range) => range.replacement === undefined) + if ( + subtraction && + ranges.some( + (range) => + !occurrences.some( + (entry) => entry.source && entry.start === range.start && entry.end === range.end + ) + ) + ) { + return null + } + const replacements = new Map( + ranges.filter((range) => range.replacement).map((range) => [range.start, range.replacement]) + ) + const chunks: string[] = [] + let cursor = 0 + for (const { start, end, replacement } of ranges) { + chunks.push(text.slice(cursor, start), replacement ?? '') + cursor = end + } + chunks.push(text.slice(cursor)) + const args = subtraction ? chunks.join('') : chunks.join('').trim() + const proven = originals.every((original, grammarIndex) => { + if (!original) { + return false + } + // CLI ownership does not establish shell expansion or command boundaries. + if (subtraction && original.parsed.spans.some((span) => span.divergesFromShell)) { + return false + } + let rangeIndex = 0 + const removed = original.parsed.spans.map((span) => { + while (rangeIndex < ranges.length && ranges[rangeIndex].end <= span.start) { + rangeIndex++ + } + const range = ranges[rangeIndex] + return !!range && span.start >= range.start && span.end <= range.end + }) + const kept = original.parsed.tokens.filter((_, index) => !removed[index]) + const expected = original.parsed.tokens.flatMap((token, index) => { + if (!removed[index]) { + return [token] + } + const text = replacements.get(original.parsed.spans[index].start) + if (!text) { + return [] + } + const replacement = tokenizeStartupCommand(text, grammars[grammarIndex]) + return replacement.ok ? replacement.tokens : [] + }) + const parsed = tokenizeStartupCommand(args, grammars[grammarIndex]) + if ( + !parsed.ok || + expected.length !== parsed.tokens.length || + !expected.every((token, index) => token === parsed.tokens[index]) + ) { + return false + } + if (subtraction) { + return preservesDeletedChoiceArgumentRoles({ + agent, + tokens: original.parsed.tokens, + roles: original.analysis.roles, + optionIndices: original.analysis.optionIndices, + removed, + retained: parsed.tokens, + inventory + }) + } + const prefix = original.parsed.tokens.filter((_, index) => removed[index]) + const composed = analyzeAgentChoiceWords(agent, [...prefix, ...kept], inventory) + const roles = original.analysis.roles.filter((_, index) => !removed[index]) + const leading = removed.slice(0, removed.lastIndexOf(true) + 1).every(Boolean) + return ( + leading || + (!original.analysis.roles.includes('uncertain') && + roles.every((role, index) => role === composed.roles[prefix.length + index])) + ) + }) + return proven ? args : null +} + +/** Subtraction must preserve retained ownership; it never lifts tokens across an opaque suffix. */ +function preservesDeletedChoiceArgumentRoles(args: { + agent: TuiAgent + tokens: readonly string[] + roles: readonly string[] + optionIndices: ReadonlySet + removed: readonly boolean[] + retained: string[] + inventory: ReturnType +}): boolean { + const { agent, tokens, roles, optionIndices, removed, retained, inventory } = args + const uncertain = roles.indexOf('uncertain') + const lastCut = removed.lastIndexOf(true) + if (removed.some((cut, index) => cut && uncertain !== -1 && index >= uncertain)) { + return false + } + if ( + tokens.some((token, index) => { + if (index > lastCut || removed[index] || !optionIndices.has(index)) { + return false + } + const name = optionName(agent, token) + return token === name && inventory.values.has(name) && roles[index + 1] !== `value:${name}` + }) + ) { + return false + } + const next = analyzeAgentChoiceWords(agent, retained, inventory) + return roles + .filter((_, index) => !removed[index]) + .every((role, index) => role === next.roles[index]) +} diff --git a/src/shared/agent-launch/choices/agent-choice-reconciliation.ts b/src/shared/agent-launch/choices/agent-choice-reconciliation.ts index e1c035f8107..4a213be281f 100644 --- a/src/shared/agent-launch/choices/agent-choice-reconciliation.ts +++ b/src/shared/agent-launch/choices/agent-choice-reconciliation.ts @@ -25,6 +25,7 @@ export type AgentChoiceArgsInput = { retainedKeys?: ReadonlySet permissionRanges?: readonly ChoiceOccurrence[] sourceEdit?: boolean + cutPurpose?: 'delete' publication?: AgentChoicePublicationContext } @@ -204,7 +205,7 @@ export function reconcileAgentChoiceArgsDetailed(input: AgentChoiceArgsInput) { const cuts = input.permissionRanges ? [...selected.filter((entry) => entry.key !== 'permissionMode'), ...input.permissionRanges] : selected - const cut = analysis.cut(cuts) + const cut = analysis.cut(cuts, input.cutPurpose) const lifted = cut === null ? [] diff --git a/src/shared/agent-launch/startup/terminal-effort-option-edit.ts b/src/shared/agent-launch/startup/terminal-effort-option-edit.ts index 7ab7360074e..fa3aac77e15 100644 --- a/src/shared/agent-launch/startup/terminal-effort-option-edit.ts +++ b/src/shared/agent-launch/startup/terminal-effort-option-edit.ts @@ -18,6 +18,7 @@ export function clearTerminalEffortArgs(agent: TuiAgent, userArgs: string): stri mode: 'choice-edit', keys, patch: { options: { effort: null } }, - sourceEdit: true + sourceEdit: true, + cutPurpose: 'delete' }).userArgs } diff --git a/src/shared/agent-launch/startup/tui-agent-launch-command-pick-flags.test.ts b/src/shared/agent-launch/startup/tui-agent-launch-command-pick-flags.test.ts index 3ebf0800b84..33ea6c6e65d 100644 --- a/src/shared/agent-launch/startup/tui-agent-launch-command-pick-flags.test.ts +++ b/src/shared/agent-launch/startup/tui-agent-launch-command-pick-flags.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { resolveAgentLaunchCommand, terminalSessionOptionInputs } from './tui-agent-launch-command' import { resolveAgentStartupPlanInputs } from './agent-startup-plan-inputs' +import { clearTerminalEffortArgs } from './terminal-effort-option-edit' import { tokenizeStartupCommand } from './tui-agent-startup-shell' function launch(args: Partial[0]>) { @@ -53,11 +54,11 @@ it('applies effort deletion to current destination defaults, actions and Argumen isRemote: false, sessionOptions: { effort: 'medium' } } - expect(editedLaunch(request).argv).toEqual({ + expect(editedLaunch(request).argv).toMatchObject({ ok: true, - tokens: ['agy', '--effort', 'medium', '--dangerously-skip-permissions', '--add-dir', 'new'] + tokens: ['agy', '--dangerously-skip-permissions', '--add-dir', 'new', '--effort', 'medium'] }) - expect(editedLaunch({ ...request, terminalOptionEdits: { effort: null } }).argv).toEqual({ + expect(editedLaunch({ ...request, terminalOptionEdits: { effort: null } }).argv).toMatchObject({ ok: true, tokens: ['agy', '--dangerously-skip-permissions', '--add-dir', 'new'] }) @@ -67,13 +68,13 @@ it('applies effort deletion to current destination defaults, actions and Argumen terminalOptionEdits: { effort: null }, agentArgs: '--effort low --add-dir authored' }).argv - ).toEqual({ + ).toMatchObject({ ok: true, tokens: ['agy', '--add-dir', 'authored'] }) expect( editedLaunch({ ...request, terminalOptionEdits: { effort: null }, agentArgs: null }).argv - ).toEqual({ ok: true, tokens: ['agy'] }) + ).toMatchObject({ ok: true, tokens: ['agy'] }) expect(JSON.stringify(settings)).toBe(before) }) @@ -82,7 +83,7 @@ it.each([ 'claude', '--effort low --add-dir docs', { effort: 'high' }, - ['claude', '--effort', 'high', '--add-dir', 'docs'], + ['claude', '--add-dir', 'docs', '--effort', 'high'], ['claude', '--add-dir', 'docs'] ], [ @@ -92,11 +93,11 @@ it.each([ [ 'codex', '-c', - 'model_reasoning_effort=high', - '-c', 'disable_paste_burst=true', '-c', - 'personality=friendly' + 'personality=friendly', + '-c', + 'model_reasoning_effort=high' ], ['codex', '-c', 'disable_paste_burst=true', '-c', 'personality=friendly'] ] @@ -111,8 +112,8 @@ it.each([ shell: 'posix' as const, isRemote: false } - expect(editedLaunch(request).argv).toEqual({ ok: true, tokens: ordinary }) - expect(editedLaunch({ ...request, terminalOptionEdits: { effort: null } }).argv).toEqual({ + expect(editedLaunch(request).argv).toMatchObject({ ok: true, tokens: ordinary }) + expect(editedLaunch({ ...request, terminalOptionEdits: { effort: null } }).argv).toMatchObject({ ok: true, tokens: cleared }) @@ -131,7 +132,7 @@ it('preserves literal composed models and opaque Arguments while clearing dedica shell: 'posix' as const, isRemote: false } - expect(editedLaunch(request).argv).toEqual({ + expect(editedLaunch(request).argv).toMatchObject({ ok: true, tokens: ['cursor-agent', '--model', 'sonnet-4.5-thinking', '--add-dir', 'docs'] }) @@ -145,3 +146,31 @@ it('preserves literal composed models and opaque Arguments while clearing dedica }) expect(opaque).toEqual({ agentArgs: '--effort low | tee result' }) }) + +it('deletes only a proven effort before an opaque suffix without rewriting that suffix', () => { + expect( + clearTerminalEffortArgs( + 'antigravity', + '--dangerously-skip-permissions --effort=high --add-dir "current folder" ' + ) + ).toBe('--dangerously-skip-permissions --add-dir "current folder" ') + expect( + clearTerminalEffortArgs( + 'antigravity', + '--dangerously-skip-permissions --effort low --future-option opaque' + ) + ).toBe('--dangerously-skip-permissions --future-option opaque') +}) + +it.each([ + ['antigravity', '--future-option opaque --effort low'], + ['antigravity', '--model --effort low'], + ['claude', '--system-prompt --effort low'], + ['antigravity', '-- --effort low'], + ['antigravity', '--effort low --effort high'], + ['antigravity', "--effort 'low' --add-dir docs"], + ['antigravity', '--effort "low'], + ['antigravity', '--effort low | tee output'] +] as const)('preserves protected effort text for %s: %s', (agent, raw) => { + expect(clearTerminalEffortArgs(agent, raw)).toBe(raw) +})