From e77e1fe850108aaa2d03674745f8a1b7d3895637 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 11 Aug 2026 16:16:24 -0700 Subject: [PATCH] fix(claude): guard cold-restore resume selectors (#13868) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(claude): guard cold-restore resume selectors Persisted Claude default args or a custom command can carry their own --resume/-r/--continue/-c selectors (a bare picker default or a stale id). Cold restore appended the authoritative --resume after them, typing a command with competing selectors into the restored pane (#12982). buildAgentResumeStartupPlan now routes Claude through a selector guard that tokenizes the base with the existing startup tokenizer, strips selectors in option position only (value-taking options keep dash-leading values), and appends exactly one authoritative selector, inserting before Claude's own -- terminator when present. Splicing is span-based so untouched bytes stay verbatim, wrapper commands are left alone, and any tokenization failure falls back to the previous append-only behavior. Launch paths, other agents, persistence, and the wire are unchanged. * fix(claude): harden resume selector guard against false matches Round-1 review findings: locate the claude executable by command position (index 0, after a wrapper --, or behind NAME=value assignments) so an argument merely ending in /claude can never be mistaken for it; stop matching the joined -r form, which was ambiguous with dash-leading option values and forced an unmaintainable arity table (now deleted). Ambiguous shapes degrade to the pre-guard append-only behavior. * fix(claude): fail resume guard open on chained shell syntax Round-2 review findings: an unquoted operator or newline after the claude token means the base chains other commands, and splicing across that boundary handed the selector to the wrong command — detect it and fall back to plain appending. Also recognize claude behind PowerShell's & call operator, decouple the test oracle from the implementation's selector predicate, add Windows tokenizer span tests, and rename the module after its public API. * fix(claude): flag bare shell operators inside the tokenizers Round-3 review findings: the guard's operator scan compared raw source to token value, so one quote or escape anywhere in a token hid a shell-active operator outside the quotes and the splice crossed a live command boundary, losing the resume entirely. Both tokenizers now flag tokens carrying an unquoted, unescaped operator byte (or a word-leading # comment on posix/powershell) on their spans, where quote state actually lives, and the guard fails open on that flag. Also strengthens the redirect fail-open test to carry a stale selector, re-tokenizes each raw span in the shell span tests, and documents agent-resume-argv-drop as codex-only. * fix(claude): flag expansions and clamp separator backoff Round-4 review findings: unquoted multi-token expansions (backtick, $(, ${) split across whitespace, so removing only the recognized selector token left a broken construct tail — both tokenizers now raise the span flag (renamed bareShellSyntax) for those openers, on cmd also for operators between single quotes, which cmd does not treat as quoting. The separator backoff is clamped to the previous token's span end so a token ending in an escaped space can no longer donate its escape to the appended selector. * fix(claude): treat cmd single-quoted regions as unmodelable Round-5 review finding: cmd.exe has no single-quote syntax, so the Windows tokenizer's grouping of a single-quoted region diverges from what cmd parses — literal argv like 'claude ...--resume... old' was being read as a real selector and stripped, and a literal '--' as claude's terminator. Flag any cmd single-quoted token as bareShellSyntax so the guard fails open. * fix(claude): flag quoted expansions and scope assignment prefixes Round-6 review findings: the span flag was only evaluated in the unquoted branch, so an expansion opener inside double quotes went unflagged — and inside $(…)/backticks a nested quote re-opens a context this tokenizer does not model, so the splice could cut mid-construct (syntax error, or a silently mutated substitution body). Both tokenizers now flag those, and the flag is renamed divergesFromShell to say what it means. Restrict the NAME=value command-position prefix to posix, where that syntax exists. Drops two branches proven dead. * fix(claude): model shell-literal escapes and scan the whole base Round-7 review findings: (1) the divergence scan started after the claude token, so an expansion opened in a prefix — $(x; npx -- claude --resume s) — had its closer spliced away, producing a base bash cannot parse; it now covers every token including the executable, exempting only PowerShell's leading call operator. (2) posix drops a double-quoted backslash the shell keeps literal, and the Windows escape branch ran inside quoted regions where cmd/PowerShell keep the escape byte literal — both now flagged, so a literal can never be misread as a selector. (3) an unquoted line continuation hid a selector inside a token and skipped the newline gap check. Also removes a third provably dead branch and collapses the cut floor into the cut itself. * fix(claude): flag escapes the tokenizer models but the shell removes Round-8 review findings, all one family — escapes whose token value hides a selector the shell would see: a double-quoted line continuation (bash deletes both bytes), posix $'…'/$"…" quoting, a windows escaped newline, and a trailing unpaired escape. The last one was previously written off as pre-fix-identical, but once stripping happens the dangling escape swallows the separator and no exact --resume reaches claude at all — strictly worse than appending, so it must fail open. Also folds the three gap predicates into one scan. * fix(claude): stop over-flagging a literal dollar sign Round-9 review findings from both lanes: inside double quotes only $( and ${ open an expansion — $' and $" are literal there — and a trailing $ was flagged unconditionally because JS ''.includes('') is true. Both made the guard fail open on modelable bases, leaving the stale selector to compete, so #12982 went unfixed for them. Separately, cmd strips ^ before the child re-splits on the bare whitespace, so an escaped separator hides two real arguments and must fail open rather than drop one. * fix(claude): fail open on cmd caret-quotes and bare PowerShell syntax Round-10 review findings, both Windows-only (a bash oracle cannot reach them): cmd strips a caret before a quote and the child's parser then reads a bare quote delimiter, so the tokenizer's word boundaries stop matching argv — one case turned a working resume into no resume at all, another let a stale selector survive the splice. And bare (…)/{…} are live PowerShell syntax in argument position, so splicing through them emitted unbalanced output that PowerShell cannot parse. * fix(claude): fail open on the PowerShell stop-parsing token Round-11 review finding: after a bare --%, PowerShell passes the rest of the line to the child literally, so the guard stripped a real selector and then appended quoting that arrives as literal bytes — claude ends up with no exact --resume at all, worse than leaving the stale one. Quoted "--%" and cmd, where the token is ordinary, still splice. * fix(claude): model cmd backslash-escaped quotes Round-11 review finding: an odd run of backslashes before a quote makes it a literal byte to the child's CommandLineToArgvW parser, not a delimiter, so the tokenizer's word boundaries stopped matching argv. Orca manufactures that pattern itself — quoteStartupArg wraps every token in quotes without escaping a trailing backslash — so a pasted Windows path was enough to move the selector into a desynced region and leave claude with no resume flag. Also replaces a caret test case that was byte-identical before and after its own fix, and merges two stacked comment blocks. * fix(claude): fail open on PowerShell double-quoted escape sequences Round-12 finding: PowerShell expands backtick escapes only inside double quotes, so a sequence there produces a token value argv never sees — the guard could strip "-`r" plus the argument after it. Also narrows the stop-parsing comment: a quoted --% can engage stop-parsing before a parameter token, where the base is already mangled either way. * fix(claude): flag PowerShell escape sequences in bare arguments too Round-13 finding: the previous commit gated on quote === '"', but PowerShell's tokenizer calls Backtick() from ScanGenericToken, so it expands these sequences in unquoted arguments as well — bare -`r really is a control character, not -r. The guard read it as a selector and dropped it plus the argument after it. Widening to all PowerShell contexts measures 0 under-flag and 0 over-flag across the full printable matrix; the backtick-escaped-space idiom still splices. Also swaps a test case that was byte-identical with and without its own fix. * fix(claude): drop a token-leading PowerShell backtick before whitespace Round-14 observations, all pre-existing and measured: PowerShell drops a token-leading backtick together with the whitespace after it, emitting no token, so the tokenizer's extra token shifted the locator; and a backtick before a bare CR is a line continuation too. Flagging both takes the lane's 329k-base sweep from 87 bad to 0 with no new failures and the must-splice list byte-unchanged. Also corrects a comment that no longer listed every PowerShell divergence. * docs(claude): correct the bare-CR rationale in the tokenizer comment Round-15 verified against a real PowerShell 7.6.4 engine: a backtick before a bare CR is not a line continuation there — pwsh keeps the CR in the token. The flag stays because 5.1 is unverified and failing open costs nothing, but the comment now says that rather than claiming continuation. --- src/shared/agent-resume-argv-drop.ts | 3 + .../agent-resume-launch-command.test.ts | 580 ++++++++++++++++++ src/shared/agent-resume-launch-command.ts | 168 +++++ src/shared/commit-message-prompt.test.ts | 52 +- src/shared/commit-message-prompt.ts | 44 +- src/shared/tui-agent-launch-command.ts | 2 +- src/shared/tui-agent-startup-shell.test.ts | 58 ++ src/shared/tui-agent-startup-shell.ts | 77 ++- src/shared/tui-agent-startup.ts | 8 +- 9 files changed, 974 insertions(+), 18 deletions(-) create mode 100644 src/shared/agent-resume-launch-command.test.ts create mode 100644 src/shared/agent-resume-launch-command.ts create mode 100644 src/shared/tui-agent-startup-shell.test.ts diff --git a/src/shared/agent-resume-argv-drop.ts b/src/shared/agent-resume-argv-drop.ts index ae393d0897e..df8c16aae4f 100644 --- a/src/shared/agent-resume-argv-drop.ts +++ b/src/shared/agent-resume-argv-drop.ts @@ -37,6 +37,9 @@ function stripSuffix(command: string, suffix: string): string | null { * Remove the resume argv `buildAgentResumeStartupPlan` appended, leaving the plain * agent launch. Main uses this when it cannot verify which account owns the session, * so the pane starts fresh instead of resuming under whichever account is selected. + * Codex-only in practice: claude resume commands may carry the selector before a + * `--` terminator (agent-resume-launch-command.ts), which this trailing-suffix + * strip would report as `unrecognized`. */ export function dropAgentResumeArgvFromCommand(args: { command: string diff --git a/src/shared/agent-resume-launch-command.test.ts b/src/shared/agent-resume-launch-command.test.ts new file mode 100644 index 00000000000..5bf4797472b --- /dev/null +++ b/src/shared/agent-resume-launch-command.test.ts @@ -0,0 +1,580 @@ +import { describe, expect, it } from 'vitest' +import { buildClaudeResumeLaunchCommand } from './agent-resume-launch-command' +import { buildAgentResumeStartupPlan, buildAgentStartupPlan } from './tui-agent-startup' +import { tokenizeStartupCommand, type AgentStartupShell } from './tui-agent-startup-shell' + +const SESSION_ID = 'claude-session-1' +const RESUME = ['--resume', SESSION_ID] as const +const providerSession = { key: 'session_id', id: SESSION_ID } as const + +const SHELLS: { platform: NodeJS.Platform; shell: AgentStartupShell }[] = [ + { platform: 'linux', shell: 'posix' }, + { platform: 'darwin', shell: 'posix' }, + { platform: 'win32', shell: 'powershell' }, + { platform: 'win32', shell: 'cmd' } +] + +/** Independent selector oracle — deliberately NOT the implementation's own + * predicate, so a regression that shrinks the stripped set cannot also blind + * this assertion. */ +function isSelectorShapedToken(token: string): boolean { + return ( + ['--resume', '--continue', '-r', '-c'].includes(token) || + ['--resume=', '--continue=', '-r=', '-c='].some((prefix) => token.startsWith(prefix)) + ) +} + +/** Tokenizes a launch command and asserts exactly one identity-bearing resume. */ +function expectSingleAuthoritativeResume(command: string, shell: AgentStartupShell): void { + const tokenized = tokenizeStartupCommand(command, shell) + expect(tokenized.ok).toBe(true) + if (!tokenized.ok) { + return + } + const selectors = tokenized.tokens.filter(isSelectorShapedToken) + expect(selectors).toEqual(['--resume']) + const index = tokenized.tokens.indexOf('--resume') + expect(tokenized.tokens[index + 1]).toBe(SESSION_ID) +} + +describe('buildClaudeResumeLaunchCommand', () => { + it.each(SHELLS)('appends the authoritative selector to a plain base ($shell)', ({ shell }) => { + const command = buildClaudeResumeLaunchCommand('claude', RESUME, shell) + expectSingleAuthoritativeResume(command, shell) + expect(command.startsWith('claude ')).toBe(true) + }) + + it.each(SHELLS)('strips a bare persisted --resume picker default ($shell)', ({ shell }) => { + const base = shell === 'cmd' ? 'claude "--resume"' : "claude '--resume'" + const command = buildClaudeResumeLaunchCommand(base, RESUME, shell) + expectSingleAuthoritativeResume(command, shell) + }) + + it.each([ + 'claude --resume stale-session', + 'claude --resume=stale-session', + 'claude -r stale-session', + 'claude -r=stale-session', + 'claude --resume= --model sonnet', + 'claude --continue', + 'claude -c', + 'claude --continue=1', + 'claude -c=1', + 'claude --resume stale -r older --continue -c' + ])('replaces stale selectors in %s', (base) => { + const command = buildClaudeResumeLaunchCommand(base, RESUME, 'posix') + expectSingleAuthoritativeResume(command, 'posix') + }) + + it('keeps surviving options when stripping selectors', () => { + expect( + buildClaudeResumeLaunchCommand('claude --resume stale --model sonnet', RESUME, 'posix') + ).toBe(`claude --model sonnet '--resume' '${SESSION_ID}'`) + }) + + it('never mistakes a dash-leading option value for a selector', () => { + expect(buildClaudeResumeLaunchCommand('claude --model -recent', RESUME, 'posix')).toBe( + `claude --model -recent '--resume' '${SESSION_ID}'` + ) + expect( + buildClaudeResumeLaunchCommand('claude --append-system-prompt -rules-here', RESUME, 'posix') + ).toBe(`claude --append-system-prompt -rules-here '--resume' '${SESSION_ID}'`) + expect(buildClaudeResumeLaunchCommand('claude --agent -reviewer', RESUME, 'posix')).toBe( + `claude --agent -reviewer '--resume' '${SESSION_ID}'` + ) + expect(buildClaudeResumeLaunchCommand('claude --plugin-url -remote.zip', RESUME, 'posix')).toBe( + `claude --plugin-url -remote.zip '--resume' '${SESSION_ID}'` + ) + }) + + it('leaves the ambiguous joined -r form alone (degrades to pre-guard behavior)', () => { + expect(buildClaudeResumeLaunchCommand('claude -rstale-session', RESUME, 'posix')).toBe( + `claude -rstale-session '--resume' '${SESSION_ID}'` + ) + }) + + it('leaves wrapper commands untouched and appends at the end', () => { + expect(buildClaudeResumeLaunchCommand('bash -c claude', RESUME, 'posix')).toBe( + `bash -c claude '--resume' '${SESSION_ID}'` + ) + expect(buildClaudeResumeLaunchCommand('mise exec -- claude', RESUME, 'posix')).toBe( + `mise exec -- claude '--resume' '${SESSION_ID}'` + ) + expect(buildClaudeResumeLaunchCommand('sudo -u dev -- claude', RESUME, 'posix')).toBe( + `sudo -u dev -- claude '--resume' '${SESSION_ID}'` + ) + }) + + it('strips selectors that follow claude after a wrapper terminator', () => { + expect( + buildClaudeResumeLaunchCommand("mise exec -- claude '--resume' stale", RESUME, 'posix') + ).toBe(`mise exec -- claude '--resume' '${SESSION_ID}'`) + }) + + it('fails open for claude outside command position (wrapper without --)', () => { + // Why: npx/bunx-style passthrough cannot be told apart from an argument + // that merely names claude, so the guard defers to append-only behavior. + expect(buildClaudeResumeLaunchCommand("npx claude '--resume'", RESUME, 'posix')).toBe( + `npx claude '--resume' '--resume' '${SESSION_ID}'` + ) + }) + + it('never mistakes a claude-suffixed argument for the executable', () => { + expect( + buildClaudeResumeLaunchCommand( + 'ssh -i ~/.ssh/claude devbox -- claude --resume OLD', + RESUME, + 'posix' + ) + ).toBe(`ssh -i ~/.ssh/claude devbox -- claude '--resume' '${SESSION_ID}'`) + expect( + buildClaudeResumeLaunchCommand( + 'mise exec --cd /Users/me/src/claude -- claude --resume OLD', + RESUME, + 'posix' + ) + ).toBe(`mise exec --cd /Users/me/src/claude -- claude '--resume' '${SESSION_ID}'`) + // No claude in command position at all: wrapper flags stay untouched. + expect( + buildClaudeResumeLaunchCommand( + 'nix develop /Users/me/src/claude -c claude --resume OLD', + RESUME, + 'posix' + ) + ).toBe(`nix develop /Users/me/src/claude -c claude --resume OLD '--resume' '${SESSION_ID}'`) + }) + + it.each(['claude "\\--resume" old', 'claude "\\-r" old', 'claude --model "\\--resume"'])( + 'fails open on a double-quoted backslash the shell keeps literal: %s', + (base) => { + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'posix')).toBe( + `${base} '--resume' '${SESSION_ID}'` + ) + } + ) + + it('still strips when a double-quoted backslash is shell-consumed', () => { + expect( + buildClaudeResumeLaunchCommand('claude --model "a\\"b" --resume old', RESUME, 'posix') + ).toBe(`claude --model "a\\"b" '--resume' '${SESSION_ID}'`) + }) + + it.each([ + 'claude "--resu\\\nme" --resume stale', + "claude $'-c' --resume stale", + "claude $'--resu\\x6de' --resume stale" + ])('fails open when an escape hides a selector from the tokenizer: %s', (base) => { + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'posix')).toBe( + `${base} '--resume' '${SESSION_ID}'` + ) + }) + + it.each([ + 'claude --resume old --append-system-prompt "spend under 5$"', + 'claude --resume old --note x$', + 'claude --resume old --note "a$"' + ])('still strips when a $ is not an expansion opener: %s', (base) => { + const command = buildClaudeResumeLaunchCommand(base, RESUME, 'posix') + expect(command).not.toContain('--resume old') + expectSingleAuthoritativeResume(command, 'posix') + }) + + it.each(['claude a^" --resume ^"b', 'claude ^"x --resume^" --resume old'])( + 'fails open when a cmd caret escapes a quote: %s', + (base) => { + // cmd strips the caret and the child's parser reads a bare quote + // delimiter, so the tokenizer's word boundaries stop matching argv. + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'cmd')).toBe( + `${base} "--resume" "${SESSION_ID}"` + ) + } + ) + + it.each(['claude --resume (Get-Content id.txt)', 'claude --hook { npm -c run } --resume old'])( + 'fails open on bare powershell evaluation syntax: %s', + (base) => { + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'powershell')).toBe( + `${base} '--resume' '${SESSION_ID}'` + ) + } + ) + + it.each([ + 'claude "-`r" foo --model x', + 'claude "-`u{72}" oldid --resume x', + 'claude -`r foo --model x', + 'claude --resum`e stale --model x' + ])('fails open on powershell escape sequences, quoted or bare: %s', (base) => { + // PowerShell expands these in bare arguments too, so the tokenizer's + // token value is not what argv receives. + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'powershell')).toBe( + `${base} '--resume' '${SESSION_ID}'` + ) + }) + + it('fails open on a token-leading powershell backtick before whitespace', () => { + // PowerShell drops the backtick AND the whitespace, emitting no token, + // so the tokenizer's extra token would shift the locator. + const base = 'claude --resume ` \t"q"' + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'powershell')).toBe( + `${base} '--resume' '${SESSION_ID}'` + ) + }) + + it('fails open on the powershell stop-parsing token', () => { + // After a bare --%, PowerShell hands the rest of the line to the child + // literally, so an appended quoted selector would arrive as literal bytes. + const base = 'claude --% --resume stale --model sonnet' + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'powershell')).toBe( + `${base} '--resume' '${SESSION_ID}'` + ) + }) + + it('still strips when --% is quoted or on cmd, where it is an ordinary token', () => { + expect( + buildClaudeResumeLaunchCommand('claude "--%" --resume stale', RESUME, 'powershell') + ).toBe(`claude "--%" '--resume' '${SESSION_ID}'`) + expect(buildClaudeResumeLaunchCommand('claude --% --resume stale', RESUME, 'cmd')).toBe( + `claude --% "--resume" "${SESSION_ID}"` + ) + }) + + it('still strips with parens safely inside quotes on powershell', () => { + expect( + buildClaudeResumeLaunchCommand( + 'claude --allowedTools "Bash(git:*)" --resume old', + RESUME, + 'powershell' + ) + ).toBe(`claude --allowedTools "Bash(git:*)" '--resume' '${SESSION_ID}'`) + }) + + it.each([ + 'claude "--add-dir" "C:\\a\\" "--" "--out" "D:\\x\\"', + 'claude --add-dir "C:\\repo\\" --resume old' + ])('fails open when a cmd backslash run makes a quote literal: %s', (base) => { + // An odd run of backslashes makes the quote a literal byte to the child's + // CommandLineToArgvW parser, so tokenizer boundaries stop matching argv. + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'cmd')).toBe( + `${base} "--resume" "${SESSION_ID}"` + ) + }) + + it.each([ + 'claude --add-dir "C:\\Users\\me\\repo" --resume old', + 'claude --add-dir "C:\\Program Files (x86)\\x" --resume old' + ])('still strips for ordinary quoted windows paths: %s', (base) => { + const command = buildClaudeResumeLaunchCommand(base, RESUME, 'cmd') + expect(command).not.toContain('--resume old') + expectSingleAuthoritativeResume(command, 'cmd') + }) + + it('fails open when a cmd caret escapes a real argument separator', () => { + // cmd strips ^ before the child re-splits on the bare space, so the + // tokenizer's merged token would drop the user's second argument. + const base = 'claude --resume a^ b --model x' + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'cmd')).toBe( + `${base} "--resume" "${SESSION_ID}"` + ) + }) + + it.each([ + ['claude --resume old \\', 'posix' as const], + ['claude --resume old `', 'powershell' as const], + ['claude --resume old ^', 'cmd' as const] + ])('fails open on a trailing unpaired escape: %s', (base, shell) => { + // A dangling escape would swallow the separator before the appended + // selector, so claude would receive no exact --resume at all. + const quoted = shell === 'cmd' ? `"--resume" "${SESSION_ID}"` : `'--resume' '${SESSION_ID}'` + expect(buildClaudeResumeLaunchCommand(base, RESUME, shell)).toBe(`${base} ${quoted}`) + }) + + it('fails open on a windows escaped line continuation', () => { + const powershellBase = 'claude --resume stale `\n--resume hidden' + expect(buildClaudeResumeLaunchCommand(powershellBase, RESUME, 'powershell')).toBe( + `${powershellBase} '--resume' '${SESSION_ID}'` + ) + const cmdBase = 'claude --resume stale ^\n--resume hidden' + expect(buildClaudeResumeLaunchCommand(cmdBase, RESUME, 'cmd')).toBe( + `${cmdBase} "--resume" "${SESSION_ID}"` + ) + }) + + it('fails open on a posix line continuation hiding a selector', () => { + const base = 'claude --model x \\\n--resume old' + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'posix')).toBe( + `${base} '--resume' '${SESSION_ID}'` + ) + }) + + it('fails open on escape characters the windows shells keep literal', () => { + // cmd keeps ^ literal inside double quotes; PowerShell keeps ` literal + // inside single quotes, so these tokens are not really selectors. + expect(buildClaudeResumeLaunchCommand('claude "-^-resume" old', RESUME, 'cmd')).toBe( + `claude "-^-resume" old "--resume" "${SESSION_ID}"` + ) + expect(buildClaudeResumeLaunchCommand("claude '-`-resume' old", RESUME, 'powershell')).toBe( + `claude '-\`-resume' old '--resume' '${SESSION_ID}'` + ) + }) + + it('fails open on an unclosed expansion opened before claude', () => { + const base = '$(x; npx -- claude --resume stale)' + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'posix')).toBe( + `${base} '--resume' '${SESSION_ID}'` + ) + }) + + it('fails open when the executable token itself is unmodelable', () => { + // cmd has no single-quote syntax, so 'claude' is not the claude executable. + expect(buildClaudeResumeLaunchCommand("'claude' --resume old", RESUME, 'cmd')).toBe( + `'claude' --resume old "--resume" "${SESSION_ID}"` + ) + }) + + it.each(['cmd', 'powershell'] as const)( + 'fails open on a posix-style assignment prefix under %s', + (shell) => { + // NAME=value prefixes are posix-only syntax; on Windows shells that + // token is a bogus executable, so nothing may be spliced. + const base = "FOO='bar' claude --resume old" + const quoted = shell === 'cmd' ? `"--resume" "${SESSION_ID}"` : `'--resume' '${SESSION_ID}'` + expect(buildClaudeResumeLaunchCommand(base, RESUME, shell)).toBe(`${base} ${quoted}`) + } + ) + + it('preserves an env-assignment or path prefix byte for byte', () => { + expect( + buildClaudeResumeLaunchCommand('FOO="$HOME/x" ~/bin/claude \'--resume\'', RESUME, 'posix') + ).toBe(`FOO="$HOME/x" ~/bin/claude '--resume' '${SESSION_ID}'`) + }) + + it('recognizes Windows claude executable spellings', () => { + expect(buildClaudeResumeLaunchCommand('C:\\tools\\claude.CMD "--resume"', RESUME, 'cmd')).toBe( + `C:\\tools\\claude.CMD "--resume" "${SESSION_ID}"` + ) + }) + + it("inserts the selector before claude's own -- terminator", () => { + expect( + buildClaudeResumeLaunchCommand('claude --resume stale -- positional', RESUME, 'posix') + ).toBe(`claude '--resume' '${SESSION_ID}' -- positional`) + }) + + it('fails open when the base cannot be tokenized', () => { + expect(buildClaudeResumeLaunchCommand('claude "unterminated', RESUME, 'posix')).toBe( + `claude "unterminated '--resume' '${SESSION_ID}'` + ) + }) + + it('fails open when no claude executable token exists', () => { + expect(buildClaudeResumeLaunchCommand('my-agent-wrapper --resume', RESUME, 'posix')).toBe( + `my-agent-wrapper --resume '--resume' '${SESSION_ID}'` + ) + }) + + it.each([ + 'claude -c && echo done', + 'claude --resume stale; echo hi', + 'claude --resume stale;echo hi', + 'claude -c | tee /tmp/log', + 'claude --resume stale 2>/tmp/x.log', + 'claude --resume stale\n--verbose', + 'claude --resume stale # note' + ])('fails open when the base chains shell syntax after claude: %s', (base) => { + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'posix')).toBe( + `${base} '--resume' '${SESSION_ID}'` + ) + }) + + it.each([ + 'claude --resume old "y"; echo hi', + "claude --resume old 'y'; echo hi", + 'claude --resume old --model "sonnet"&&echo hi', + 'claude --resume old "y"| tee /tmp/x', + 'claude --resume old \\"; echo hi' + ])('fails open when shell syntax hides behind partial quoting: %s', (base) => { + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'posix')).toBe( + `${base} '--resume' '${SESSION_ID}'` + ) + }) + + it('fails open on escape-adjacent operators in windows shells', () => { + const powershellBase = 'claude --resume old `x; echo hi' + expect(buildClaudeResumeLaunchCommand(powershellBase, RESUME, 'powershell')).toBe( + `${powershellBase} '--resume' '${SESSION_ID}'` + ) + const cmdBase = 'claude --resume old ^x& echo hi' + expect(buildClaudeResumeLaunchCommand(cmdBase, RESUME, 'cmd')).toBe( + `${cmdBase} "--resume" "${SESSION_ID}"` + ) + }) + + it.each([ + 'claude --resume $(cat sid.txt)', + 'claude --resume=$(cat sid.txt)', + 'claude --resume `cat sid.txt`', + 'claude --resume ${SID:-a b}' + ])('fails open on unquoted multi-token shell expansions: %s', (base) => { + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'posix')).toBe( + `${base} '--resume' '${SESSION_ID}'` + ) + }) + + it.each([ + 'claude --resume "`cat "a b"`"', + 'claude --model "$(pick "x -c y")"', + 'claude --model "$(f "x --resume y")"', + 'claude --resume "$(cat "$HOME/My Sessions/id")"' + ])('fails open on expansions nested inside double quotes: %s', (base) => { + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'posix')).toBe( + `${base} '--resume' '${SESSION_ID}'` + ) + }) + + it('fails open on a powershell double-quoted subexpression', () => { + const base = 'claude --model "$(pick "x -c y")"' + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'powershell')).toBe( + `${base} '--resume' '${SESSION_ID}'` + ) + }) + + it('fails open on a powershell subexpression locator', () => { + const base = 'claude --resume $(Get-Content sid.txt)' + expect(buildClaudeResumeLaunchCommand(base, RESUME, 'powershell')).toBe( + `${base} '--resume' '${SESSION_ID}'` + ) + }) + + it('never walks the separator backoff into an escaped-space token', () => { + expect( + buildClaudeResumeLaunchCommand( + 'claude --append-system-prompt Be\\ nice\\ --resume OLD', + RESUME, + 'posix' + ) + ).toBe(`claude --append-system-prompt Be\\ nice\\ '--resume' '${SESSION_ID}'`) + }) + + it('fails open when cmd operators hide in single quotes', () => { + // cmd.exe has no single-quote syntax, so '&' is a live command separator. + expect(buildClaudeResumeLaunchCommand("claude '&' --resume=old-session", RESUME, 'cmd')).toBe( + `claude '&' --resume=old-session "--resume" "${SESSION_ID}"` + ) + }) + + it('fails open on cmd single-quoted literals that merely look like selectors', () => { + // cmd passes the quotes through, so these are junk positionals for + // claude, not real selectors or terminators. + expect(buildClaudeResumeLaunchCommand("claude '--resume' old", RESUME, 'cmd')).toBe( + `claude '--resume' old "--resume" "${SESSION_ID}"` + ) + expect(buildClaudeResumeLaunchCommand("claude '--' --resume old", RESUME, 'cmd')).toBe( + `claude '--' --resume old "--resume" "${SESSION_ID}"` + ) + }) + + it('still strips next to a caret-escaped literal ampersand on cmd', () => { + // ^& is an inactive, literal & in cmd, so the guard may keep working. + expect(buildClaudeResumeLaunchCommand('claude --resume old ^&', RESUME, 'cmd')).toBe( + `claude ^& "--resume" "${SESSION_ID}"` + ) + }) + + it('still strips when operators are safely quoted', () => { + expect( + buildClaudeResumeLaunchCommand( + "claude --append-system-prompt 'use && wisely' --resume stale", + RESUME, + 'posix' + ) + ).toBe(`claude --append-system-prompt 'use && wisely' '--resume' '${SESSION_ID}'`) + }) + + it('recognizes claude behind the PowerShell call operator', () => { + expect(buildClaudeResumeLaunchCommand("& claude '--resume'", RESUME, 'powershell')).toBe( + `& claude '--resume' '${SESSION_ID}'` + ) + expect( + buildClaudeResumeLaunchCommand( + "& 'C:\\Program Files\\claude\\claude.exe' --resume old", + RESUME, + 'powershell' + ) + ).toBe(`& 'C:\\Program Files\\claude\\claude.exe' '--resume' '${SESSION_ID}'`) + }) +}) + +describe('buildAgentResumeStartupPlan claude selector guard', () => { + it.each(SHELLS)( + 'emits one identity-bearing resume for a persisted bare selector ($platform/$shell)', + ({ platform, shell }) => { + const initial = buildAgentStartupPlan({ + agent: 'claude', + prompt: '', + cmdOverrides: {}, + agentArgs: '--resume', + platform, + shell, + allowEmptyPromptLaunch: true + }) + expect(initial).not.toBeNull() + const restored = buildAgentResumeStartupPlan({ + agent: 'claude', + providerSession, + cmdOverrides: {}, + agentArgs: initial?.launchConfig.agentArgs, + agentCommand: initial?.launchConfig.agentCommand, + platform, + shell + }) + expect(restored).not.toBeNull() + expectSingleAuthoritativeResume(restored?.launchCommand ?? '', shell) + } + ) + + it('emits one identity-bearing resume when only default args carry a stale id', () => { + const restored = buildAgentResumeStartupPlan({ + agent: 'claude', + providerSession, + cmdOverrides: {}, + agentArgs: '--resume stale-session --model sonnet', + platform: 'linux' + }) + expect(restored?.launchCommand).toBe(`claude '--model' 'sonnet' '--resume' '${SESSION_ID}'`) + }) + + it('still launches exotic custom commands that the tokenizer rejects', () => { + const restored = buildAgentResumeStartupPlan({ + agent: 'claude', + providerSession, + cmdOverrides: {}, + agentCommand: 'claude --model $(cat ~/.claude-model) "unterminated', + platform: 'darwin' + }) + expect(restored).not.toBeNull() + expect(restored?.launchCommand.endsWith(`'--resume' '${SESSION_ID}'`)).toBe(true) + }) + + it('does not change other agents', () => { + const restored = buildAgentResumeStartupPlan({ + agent: 'gemini', + providerSession, + cmdOverrides: {}, + agentArgs: '--resume', + platform: 'linux' + }) + expect(restored?.launchCommand).toBe(`gemini '--resume' '--resume' '${SESSION_ID}'`) + }) + + it('persists the original base command unchanged', () => { + const restored = buildAgentResumeStartupPlan({ + agent: 'claude', + providerSession, + cmdOverrides: {}, + agentCommand: "claude '--resume'", + platform: 'linux' + }) + expect(restored?.launchConfig.agentCommand).toBe("claude '--resume'") + }) +}) diff --git a/src/shared/agent-resume-launch-command.ts b/src/shared/agent-resume-launch-command.ts new file mode 100644 index 00000000000..1093735d266 --- /dev/null +++ b/src/shared/agent-resume-launch-command.ts @@ -0,0 +1,168 @@ +import type { ResumableTuiAgent } from './agent-session-resume' +import { + quoteStartupArg, + tokenizeStartupCommand, + type AgentStartupShell +} from './tui-agent-startup-shell' + +function isClaudeResumeSelector(token: string): boolean { + if (token === '--resume' || token.startsWith('--resume=')) { + return true + } + if (token === '--continue' || token.startsWith('--continue=')) { + return true + } + // Why: the joined -r form is deliberately NOT matched — any `-r…` token + // is ambiguous with another option's dash-leading value (`--agent -review`), + // and no arity table can keep up with the CLI. Only exact selector shapes + // are stripped; a persisted joined form degrades to pre-guard behavior. + return token === '-r' || token.startsWith('-r=') || token === '-c' || token.startsWith('-c=') +} + +function isClaudeExecutableToken(token: string): boolean { + const base = token.split(/[\\/]/).pop() ?? '' + return /^claude(\.(exe|cmd|bat|ps1))?$/i.test(base) +} + +/** Accepts a claude token only in command position — index 0, right after a + * wrapper's `--`, behind PowerShell's `&` call operator, or preceded solely by + * NAME=value assignments — so an argument that merely ends in /claude (an ssh + * key, a project dir) can never be mistaken for the executable. */ +function findClaudeExecutableIndex(tokens: readonly string[], shell: AgentStartupShell): number { + let commandPosition = true + for (let i = 0; i < tokens.length; i += 1) { + const token = tokens[i] + if (commandPosition) { + if (isClaudeExecutableToken(token)) { + return i + } + if ( + // Why: `NAME=value cmd` is posix-only syntax; on cmd/PowerShell such a + // token is just a bogus executable name, not a prefix to skip. + (shell === 'posix' && /^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) || + (shell === 'powershell' && token === '&' && i === 0) + ) { + continue + } + commandPosition = false + } + if (token === '--') { + commandPosition = true + } + } + return -1 +} + +/** Joins the resolved base command with the agent's resume argv. Claude goes + * through the selector guard below; other agents keep plain appending. */ +export function buildAgentResumeLaunchCommand( + agent: ResumableTuiAgent, + baseCommand: string, + resumeArgv: readonly string[], + shell: AgentStartupShell +): string { + const argv = resumeArgv.slice(1) + if (agent === 'claude') { + return buildClaudeResumeLaunchCommand(baseCommand, argv, shell) + } + const resumeArgs = argv.map((arg) => quoteStartupArg(arg, shell)).join(' ') + return resumeArgs ? `${baseCommand} ${resumeArgs}` : baseCommand +} + +/** Builds the Claude cold-restore launch command: strips any resume/continue + * selector the user's persisted command carries and appends exactly one + * authoritative selector, so a stale or bare selector can never compete with + * the provider session id (#12982). + * + * Fails open by design: when the base command cannot be tokenized, or no + * claude executable token can be located (wrapper commands like + * `bash -c claude`), the base is left byte-for-byte untouched and the + * selector is appended, which is the pre-guard behavior. Bytes outside + * removed selector tokens are always preserved verbatim — the base is + * spliced by source span, never re-quoted. */ +export function buildClaudeResumeLaunchCommand( + baseCommand: string, + resumeArgs: readonly string[], + shell: AgentStartupShell +): string { + const quotedResume = resumeArgs.map((arg) => quoteStartupArg(arg, shell)).join(' ') + if (!quotedResume) { + return baseCommand + } + const appended = `${baseCommand} ${quotedResume}` + const tokenized = tokenizeStartupCommand(baseCommand, shell) + if (!tokenized.ok) { + return appended + } + const { tokens, spans } = tokenized + const claudeIndex = findClaudeExecutableIndex(tokens, shell) + if (claudeIndex === -1) { + return appended + } + // Why: any token the tokenizer cannot model for this shell — an operator, + // comment, expansion, or cmd single-quoted region — means the splice could + // cut live syntax or misread a literal as a selector. The whole base must + // be modelable, including the executable itself; only PowerShell's leading + // call operator is a known-safe divergent token. + for (let i = 0; i <= tokens.length; i += 1) { + const gapStart = i === 0 ? 0 : spans[i - 1].end + const gapEnd = i === tokens.length ? baseCommand.length : spans[i].start + if (!/^[ \t]*$/.test(baseCommand.slice(gapStart, gapEnd))) { + return appended + } + if (i === tokens.length) { + break + } + // Why: a bare `--%` makes PowerShell pass the rest of the line to the + // child literally, so appended quoting would arrive as literal bytes. A + // quoted `--%` can also stop parsing, but only before a parameter token, + // where the base is already mangled with or without the guard. + if (shell === 'powershell' && baseCommand.slice(spans[i].start, spans[i].end) === '--%') { + return appended + } + if (spans[i].divergesFromShell) { + const isCallOperator = shell === 'powershell' && i === 0 && tokens[i] === '&' + if (!isCallOperator) { + return appended + } + } + } + const cuts: { start: number; end: number }[] = [] + let terminatorStart: number | null = null + for (let i = claudeIndex + 1; i < tokens.length; i += 1) { + const token = tokens[i] + if (token === '--') { + // Why: claude is the executable here, so `--` is claude's own + // terminator; the selector must stay in option position before it. + // Span-splice equivalent of insertBeforeTerminator in + // tui-agent-launch-command.ts, which re-quotes and cannot be reused. + terminatorStart = spans[i].start + break + } + if (!isClaudeResumeSelector(token)) { + continue + } + // Why: absorb the separator before the selector, but never cross into the + // previous token, whose span can end with an escaped-space byte. + let start = spans[i].start + while (start > spans[i - 1].end && ' \t'.includes(baseCommand[start - 1])) { + start -= 1 + } + let end = spans[i].end + const next = tokens[i + 1] + if ((token === '--resume' || token === '-r') && next !== undefined && !next.startsWith('-')) { + // A stale session locator rides along with its selector. + end = spans[i + 1].end + i += 1 + } + cuts.push({ start, end }) + } + let result = baseCommand + if (terminatorStart !== null) { + result = `${result.slice(0, terminatorStart)}${quotedResume} ${result.slice(terminatorStart)}` + } + for (let i = cuts.length - 1; i >= 0; i -= 1) { + result = `${result.slice(0, cuts[i].start)}${result.slice(cuts[i].end)}` + } + return terminatorStart !== null ? result : `${result} ${quotedResume}` +} diff --git a/src/shared/commit-message-prompt.test.ts b/src/shared/commit-message-prompt.test.ts index 407b604f76d..ba2a975190f 100644 --- a/src/shared/commit-message-prompt.test.ts +++ b/src/shared/commit-message-prompt.test.ts @@ -250,27 +250,67 @@ describe('excerptAgentFailureOutput', () => { describe('tokenizeCustomCommandTemplate', () => { it('splits on whitespace', () => { const r = tokenizeCustomCommandTemplate('claude -p') - expect(r).toEqual({ ok: true, tokens: ['claude', '-p'] }) + expect(r).toEqual({ ok: true, tokens: ['claude', '-p'], spans: expect.any(Array) }) }) it('groups double-quoted segments with spaces', () => { const r = tokenizeCustomCommandTemplate('claude --msg "hello world"') - expect(r).toEqual({ ok: true, tokens: ['claude', '--msg', 'hello world'] }) + expect(r).toEqual({ + ok: true, + tokens: ['claude', '--msg', 'hello world'], + spans: expect.any(Array) + }) }) it('groups single-quoted segments verbatim', () => { const r = tokenizeCustomCommandTemplate(`agent --json '{"k":"v"}'`) - expect(r).toEqual({ ok: true, tokens: ['agent', '--json', '{"k":"v"}'] }) + expect(r).toEqual({ + ok: true, + tokens: ['agent', '--json', '{"k":"v"}'], + spans: expect.any(Array) + }) }) it('honors backslash escapes inside double quotes', () => { const r = tokenizeCustomCommandTemplate('claude --msg "she said \\"hi\\""') - expect(r).toEqual({ ok: true, tokens: ['claude', '--msg', 'she said "hi"'] }) + expect(r).toEqual({ + ok: true, + tokens: ['claude', '--msg', 'she said "hi"'], + spans: expect.any(Array) + }) }) it('keeps adjacent quoted/unquoted regions in one token (a"b"c → abc)', () => { const r = tokenizeCustomCommandTemplate('foo a"b"c') - expect(r).toEqual({ ok: true, tokens: ['foo', 'abc'] }) + expect(r).toEqual({ ok: true, tokens: ['foo', 'abc'], spans: expect.any(Array) }) + }) + + it('always reports one span per token', () => { + for (const source of ['claude -p', 'claude --msg "hello world"', 'foo a"b"c', ' \t ']) { + const r = tokenizeCustomCommandTemplate(source) + expect(r.ok && r.spans.length).toBe(r.ok && r.tokens.length) + } + }) + + it('reports source spans covering each raw token including quotes', () => { + const source = 'claude --msg "hello world"' + const r = tokenizeCustomCommandTemplate(source) + expect(r).toEqual({ + ok: true, + tokens: ['claude', '--msg', 'hello world'], + spans: [ + { start: 0, end: 6, divergesFromShell: false }, + { start: 7, end: 12, divergesFromShell: false }, + { start: 13, end: 26, divergesFromShell: false } + ] + }) + if (r.ok) { + expect(r.spans.map(({ start, end }) => source.slice(start, end))).toEqual([ + 'claude', + '--msg', + '"hello world"' + ]) + } }) it('returns an error for an unclosed quote', () => { @@ -283,7 +323,7 @@ describe('tokenizeCustomCommandTemplate', () => { it('returns an empty token list for whitespace-only input', () => { const r = tokenizeCustomCommandTemplate(' \t ') - expect(r).toEqual({ ok: true, tokens: [] }) + expect(r).toEqual({ ok: true, tokens: [], spans: [] }) }) }) diff --git a/src/shared/commit-message-prompt.ts b/src/shared/commit-message-prompt.ts index 4a034d2d53f..82dc830fd59 100644 --- a/src/shared/commit-message-prompt.ts +++ b/src/shared/commit-message-prompt.ts @@ -131,8 +131,16 @@ export function truncateDiffForPrompt( export const CUSTOM_PROMPT_PLACEHOLDER = '{prompt}' +/** Source range of a token: [start, end) offsets into the original string. + * `divergesFromShell` marks a token this tokenizer cannot model faithfully for + * the target shell: an unquoted operator (`;&|<>`), a word-leading `#` + * comment, an expansion opener whose body can span tokens (backtick, `$(`, + * `${`, quoted or not), or a cmd single-quoted region (cmd has no + * single-quote syntax). Not recoverable from the token value alone. */ +export type CommandTokenSpan = { start: number; end: number; divergesFromShell: boolean } + export type TokenizeCustomCommandResult = - | { ok: true; tokens: string[] } + | { ok: true; tokens: string[]; spans: CommandTokenSpan[] } | { ok: false; error: string } // Why: deliberately POSIX-shell-style only for *grouping* (single + double @@ -143,8 +151,11 @@ export type TokenizeCustomCommandResult = // surface we don't need. export function tokenizeCustomCommandTemplate(template: string): TokenizeCustomCommandResult { const tokens: string[] = [] + const spans: CommandTokenSpan[] = [] let current = '' let inToken = false + let tokenStart = 0 + let divergesFromShell = false let quote: '"' | "'" | null = null let i = 0 @@ -152,10 +163,17 @@ export function tokenizeCustomCommandTemplate(template: string): TokenizeCustomC const ch = template[i] if (quote) { if (ch === '\\' && quote === '"' && i + 1 < template.length) { + // Why: inside double quotes the shell only consumes the backslash + // before these; elsewhere it stays a literal byte this tokenizer drops. + divergesFromShell ||= !'$`"\\'.includes(template[i + 1]) current += template[i + 1] i += 2 continue } + // Why: a `"` inside $(…) or `…` re-opens a nested quoting context in the + // real shell, so this tokenizer's word boundaries stop matching it. + divergesFromShell ||= + quote === '"' && (ch === '`' || (ch === '$' && '({'.includes(template[i + 1] ?? '\0'))) if (ch === quote) { quote = null i++ @@ -171,13 +189,22 @@ export function tokenizeCustomCommandTemplate(template: string): TokenizeCustomC if (ch === '"' || ch === "'") { quote = ch + if (!inToken) { + tokenStart = i + } inToken = true i++ continue } if (ch === '\\' && i + 1 < template.length) { + // Why: an unquoted line continuation joins words the shell splits, so a + // selector can hide inside the joined token and skip the gap check. + divergesFromShell ||= template[i + 1] === '\n' current += template[i + 1] + if (!inToken) { + tokenStart = i + } inToken = true i += 2 continue @@ -186,13 +213,25 @@ export function tokenizeCustomCommandTemplate(template: string): TokenizeCustomC if (/\s/.test(ch)) { if (inToken) { tokens.push(current) + spans.push({ start: tokenStart, end: i, divergesFromShell }) current = '' inToken = false + divergesFromShell = false } i++ continue } + if (!inToken) { + tokenStart = i + } + // Why: a trailing unpaired escape swallows whatever a consumer appends + // after the base, so the base is not safe to build on. + divergesFromShell ||= ch === '\\' && i + 1 >= template.length + divergesFromShell ||= + ';&|<>`'.includes(ch) || + (ch === '#' && !inToken) || + (ch === '$' && '({\'"'.includes(template[i + 1] ?? '\0')) current += ch inToken = true i++ @@ -203,8 +242,9 @@ export function tokenizeCustomCommandTemplate(template: string): TokenizeCustomC } if (inToken) { tokens.push(current) + spans.push({ start: tokenStart, end: template.length, divergesFromShell }) } - return { ok: true, tokens } + return { ok: true, tokens, spans } } export type CustomCommandPlan = diff --git a/src/shared/tui-agent-launch-command.ts b/src/shared/tui-agent-launch-command.ts index 3ce20571f4e..c45ac67bd19 100644 --- a/src/shared/tui-agent-launch-command.ts +++ b/src/shared/tui-agent-launch-command.ts @@ -43,7 +43,7 @@ export function resolveAgentLaunchCommand(args: { } const trailingTokens = args.agentArgs?.trim() ? tokenizeStartupCommand(args.agentArgs.trim(), args.shell) - : { ok: true as const, tokens: [] } + : { ok: true as const, tokens: [], spans: [] } if (!trailingTokens.ok) { return { ok: false, error: `CLI arguments are invalid: ${trailingTokens.error}` } } diff --git a/src/shared/tui-agent-startup-shell.test.ts b/src/shared/tui-agent-startup-shell.test.ts new file mode 100644 index 00000000000..40c8f4ac8ee --- /dev/null +++ b/src/shared/tui-agent-startup-shell.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from 'vitest' +import { tokenizeStartupCommand } from './tui-agent-startup-shell' + +function expectSpansCoverTokens(source: string, shell: 'powershell' | 'cmd'): string[] { + const result = tokenizeStartupCommand(source, shell) + expect(result.ok).toBe(true) + if (!result.ok) { + return [] + } + expect(result.spans).toHaveLength(result.tokens.length) + let previousEnd = 0 + for (const [index, { start, end }] of result.spans.entries()) { + expect(start).toBeGreaterThanOrEqual(previousEnd) + expect(end).toBeGreaterThan(start) + // Every raw span must re-tokenize to exactly its own token. + const slice = tokenizeStartupCommand(source.slice(start, end), shell) + expect(slice.ok && slice.tokens).toEqual([result.tokens[index]]) + previousEnd = end + } + return result.spans.map(({ start, end }) => source.slice(start, end)) +} + +describe('tokenizeStartupCommand spans (windows shells)', () => { + it('covers plain and quoted tokens on powershell', () => { + const source = "claude --msg 'hello world'" + const result = tokenizeStartupCommand(source, 'powershell') + expect(result).toEqual({ + ok: true, + tokens: ['claude', '--msg', 'hello world'], + spans: [ + { start: 0, end: 6, divergesFromShell: false }, + { start: 7, end: 12, divergesFromShell: false }, + { start: 13, end: 26, divergesFromShell: false } + ] + }) + }) + + it('starts a span at a token-leading escape character', () => { + expect(expectSpansCoverTokens('claude ^&literal next', 'cmd')).toEqual([ + 'claude', + '^&literal', + 'next' + ]) + expect(expectSpansCoverTokens('claude `x tail', 'powershell')).toEqual(['claude', '`x', 'tail']) + }) + + it('spans a powershell doubled-quote token as one raw range', () => { + expect(expectSpansCoverTokens("claude 'a''b' end", 'powershell')).toEqual([ + 'claude', + "'a''b'", + 'end' + ]) + }) + + it('spans a token opened by a quote at end of input', () => { + expect(expectSpansCoverTokens('claude ""', 'cmd')).toEqual(['claude', '""']) + }) +}) diff --git a/src/shared/tui-agent-startup-shell.ts b/src/shared/tui-agent-startup-shell.ts index 8bdb5465845..744e5a1a27f 100644 --- a/src/shared/tui-agent-startup-shell.ts +++ b/src/shared/tui-agent-startup-shell.ts @@ -1,28 +1,74 @@ -import { tokenizeCustomCommandTemplate } from './commit-message-prompt' +import { tokenizeCustomCommandTemplate, type CommandTokenSpan } from './commit-message-prompt' export type AgentStartupShell = 'posix' | 'powershell' | 'cmd' -export type StartupCommandTokens = { ok: true; tokens: string[] } | { ok: false; error: string } +export type StartupCommandTokens = + | { ok: true; tokens: string[]; spans: CommandTokenSpan[] } + | { ok: false; error: string } + +/** True when an odd run of backslashes precedes this quote, which makes it a + * literal byte to the child's CommandLineToArgvW parser rather than a + * delimiter — so cmd's word boundaries stop matching this tokenizer's. */ +function hasOddBackslashRun(value: string, quoteIndex: number): boolean { + let backslashes = 0 + while (value[quoteIndex - 1 - backslashes] === '\\') { + backslashes += 1 + } + return backslashes % 2 === 1 +} function tokenizeWindowsStartupCommand( value: string, shell: Exclude ): StartupCommandTokens { const tokens: string[] = [] + const spans: CommandTokenSpan[] = [] let token = '' + let tokenStart = 0 + let divergesFromShell = false let quote: "'" | '"' | null = null let tokenStarted = false for (let index = 0; index < value.length; index += 1) { const char = value[index] const escape = shell === 'cmd' ? '^' : '`' if (char === escape && index + 1 < value.length) { + // Why: cmd strips `^` and hands the bare byte to the child's parser, + // which re-splits on whitespace and reopens a quote, and keeps the caret + // literal inside double quotes — either way the token stops matching + // argv. PowerShell folds the backtick into the token except for LF line + // continuations, verbatim single quotes, escape sequences, and a + // token-leading backtick before whitespace, which it drops entirely. + // A bare CR is treated as unmodelable rather than folded: pwsh 7 keeps + // it in the token, Windows PowerShell 5.1 is unverified, and failing + // open there costs nothing. + divergesFromShell ||= + (shell === 'cmd' ? /[\s"]/.test(value[index + 1]) : /[\n\r]/.test(value[index + 1])) || + (shell === 'cmd' && quote === '"') || + (shell === 'powershell' && quote === "'") || + // A token-leading backtick before whitespace is dropped with the + // whitespace, emitting no token at all rather than the one built here. + (shell === 'powershell' && !tokenStarted && /\s/.test(value[index + 1])) || + // PowerShell expands these escape sequences in quoted AND bare + // arguments, so the token value this branch builds is not argv's. + (shell === 'powershell' && '0abefnrtuv'.includes(value[index + 1])) token += value[index + 1] + if (!tokenStarted) { + tokenStart = index + } tokenStarted = true index += 1 continue } if (quote) { + // Why: see the posix tokenizer — a `"` inside $(…) re-opens a nested + // quoting context that this tokenizer does not model. + divergesFromShell ||= + shell === 'powershell' && + quote === '"' && + char === '$' && + (value[index + 1] === '(' || value[index + 1] === '{') if (char === quote) { + divergesFromShell ||= shell === 'cmd' && char === '"' && hasOddBackslashRun(value, index) if (shell === 'powershell' && quote === "'" && value[index + 1] === "'") { token += "'" index += 1 @@ -36,15 +82,39 @@ function tokenizeWindowsStartupCommand( continue } if (char === "'" || char === '"') { + divergesFromShell ||= shell === 'cmd' && char === '"' && hasOddBackslashRun(value, index) quote = char + // Why: cmd.exe has no single-quote syntax, so this tokenizer's grouping + // of a single-quoted region diverges from what cmd actually parses; + // flag the token so consumers treat it as unmodelable. + divergesFromShell ||= shell === 'cmd' && char === "'" + if (!tokenStarted) { + tokenStart = index + } tokenStarted = true } else if (/\s/.test(char)) { if (tokenStarted) { tokens.push(token) + spans.push({ start: tokenStart, end: index, divergesFromShell }) token = '' tokenStarted = false + divergesFromShell = false } } else { + if (!tokenStarted) { + tokenStart = index + } + // Why: see the posix tokenizer — a trailing unpaired escape would + // swallow the separator before anything appended to the base. + divergesFromShell ||= char === escape && index + 1 >= value.length + divergesFromShell ||= + ';&|<>'.includes(char) || + (shell === 'powershell' && + // Why: bare (…) is evaluated and {…} is a script block in argument + // position, so both are live syntax the span splice cannot model. + ('(){}'.includes(char) || + (char === '#' && !tokenStarted) || + (char === '$' && (value[index + 1] === '(' || value[index + 1] === '{')))) token += char tokenStarted = true } @@ -54,8 +124,9 @@ function tokenizeWindowsStartupCommand( } if (tokenStarted) { tokens.push(token) + spans.push({ start: tokenStart, end: value.length, divergesFromShell }) } - return { ok: true, tokens } + return { ok: true, tokens, spans } } export function tokenizeStartupCommand( diff --git a/src/shared/tui-agent-startup.ts b/src/shared/tui-agent-startup.ts index 27d4b40ac8d..4cef246ccdb 100644 --- a/src/shared/tui-agent-startup.ts +++ b/src/shared/tui-agent-startup.ts @@ -20,6 +20,7 @@ import { inlineAgentDraftFitsPlatform } from './agent-draft-platform-limit' import type { TuiAgent } from './types' import type { SessionOptionValue } from './native-chat-session-options' import { resolveAgentLaunchCommand } from './tui-agent-launch-command' +import { buildAgentResumeLaunchCommand } from './agent-resume-launch-command' export type AgentStartupPlan = { agent: TuiAgent @@ -222,14 +223,9 @@ export function buildAgentResumeStartupPlan(args: { ...args, agentCommand: baseCommand.command }) - const resumeArgs = argv - .slice(1) - .map((arg) => quoteStartupArg(arg, shell)) - .join(' ') - const launchCommand = resumeArgs ? `${baseCommand.command} ${resumeArgs}` : baseCommand.command return { agent: args.agent, - launchCommand, + launchCommand: buildAgentResumeLaunchCommand(args.agent, baseCommand.command, argv, shell), expectedProcess: config.expectedProcess, followupPrompt: null, launchConfig,