diff --git a/src/renderer/src/components/terminal-pane/pty-connection-cold-restore-resume-command.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-cold-restore-resume-command.test.ts index 51851ead59d..7a6a6153461 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-cold-restore-resume-command.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-cold-restore-resume-command.test.ts @@ -344,6 +344,7 @@ describe('connectPanePty', () => { async function runWindowsColdRestoreResume(args: { terminalWindowsShell?: string tabShellOverride?: string + agentCommand?: string }): Promise { const restoreNavigator = temporarilySetNavigatorUserAgent( 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)' @@ -408,7 +409,8 @@ describe('connectPanePty', () => { paneKey, tabId: 'tab-1', worktreeId: 'wt-1', - agent: 'codex', + agent: args.agentCommand ? 'claude' : 'codex', + ...(args.agentCommand ? { launchConfig: { agentCommand: args.agentCommand } } : {}), providerSession: { key: 'session_id', id: 'codex-session-1' }, prompt: 'finish the task', state: 'done', @@ -461,19 +463,18 @@ describe('connectPanePty', () => { ).resolves.toBe("codex '--dangerously-bypass-approvals-and-sandbox' 'resume' 'codex-session-1'") }) - // Regression (#12320 residual): a cold restore after restart can run before the - // store hydrates `settings`, so terminalWindowsShell is momentarily undefined and - // the pane may be the cmd.exe default. The codex resume argv is cmd-quote-safe, so - // the target resolver's race guess picks cmd — a bare "token" that also parses - // identically in PowerShell, so it fixes the cmd pane with no PowerShell risk. - // Before the guess, PowerShell single quotes reached cmd.exe literally - // ("unrecognized subcommand ''resume''"), exactly the field report. it('quotes for cmd.exe on cold restore when the shell setting has not hydrated', async () => { await expect(runWindowsColdRestoreResume({})).resolves.toBe( 'codex "--dangerously-bypass-approvals-and-sandbox" "resume" "codex-session-1"' ) }) + it('removes a stale Claude selector before cmd-compatible cold-restore quoting', async () => { + await expect( + runWindowsColdRestoreResume({ agentCommand: "claude --resume 'old-session'" }) + ).resolves.toBe('claude "--resume" "codex-session-1"') + }) + it('keeps a contentless reattach when the sleeping record represents a live session', async () => { const { connectPanePty } = await import('./pty-connection') const paneKey = makePaneKey('tab-1', LEAF_2) diff --git a/src/renderer/src/components/terminal-pane/pty-connection/cold-restore-resume-startup.ts b/src/renderer/src/components/terminal-pane/pty-connection/cold-restore-resume-startup.ts index 8087ecda8f3..cfc1c358fa5 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/cold-restore-resume-startup.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/cold-restore-resume-startup.ts @@ -84,7 +84,8 @@ export function bindBuildColdRestoreAgentResumeStartup(session: ConnectPanePtySe ? { ompResumeFilePath: launchConfig.ompResumeFilePath } : {}), platform: resumeTarget.platform, - shell: resumeTarget.shell + shell: resumeTarget.shell, + resumeCommandShell: resumeTarget.resumeCommandShell }) if (!startupPlan) { return null diff --git a/src/renderer/src/lib/agent-resume-launch-target.test.ts b/src/renderer/src/lib/agent-resume-launch-target.test.ts index a1bdebb4cfc..08efdf92db2 100644 --- a/src/renderer/src/lib/agent-resume-launch-target.test.ts +++ b/src/renderer/src/lib/agent-resume-launch-target.test.ts @@ -82,12 +82,9 @@ describe('resolveAgentResumeLaunchTarget on a Windows client', () => { }) it('guesses cmd for an unset shell when every resume token is cmd-quote-safe', async () => { - // Cold-restore race: settings not yet hydrated, so the pane may be the - // %COMSPEC% (cmd.exe) default. A clean codex argv quotes identically in cmd - // and PowerShell, so guessing cmd fixes the cmd pane with no PowerShell risk. await expect( resolveWith({ resumeArgv: ['codex', 'resume', 'a1b2c3d4-0000-4000-8000-000000000000'] }) - ).resolves.toEqual({ platform: 'win32', shell: 'cmd' }) + ).resolves.toEqual({ platform: 'win32', shell: 'cmd', resumeCommandShell: 'powershell' }) }) it('keeps the PowerShell default for an unset shell when a resume token needs cmd escaping', async () => { @@ -131,7 +128,20 @@ describe('resolveAgentResumeLaunchTarget on a Windows client', () => { resumeArgv: ['codex', 'resume', 'a1b2c3d4-0000-4000-8000-000000000000'], resumeAgentArgs: '--dangerously-bypass-approvals-and-sandbox --model gpt' }) - ).resolves.toEqual({ platform: 'win32', shell: 'cmd' }) + ).resolves.toEqual({ platform: 'win32', shell: 'cmd', resumeCommandShell: 'powershell' }) + }) + + it.each([ + '--add-dir C:\\work\\a^b', + '--append-system-prompt "Match ^foo"', + "--append-system-prompt 'it''s a test'" + ])('preserves PowerShell argument parsing for %s', async (resumeAgentArgs) => { + await expect( + resolveWith({ + resumeArgv: ['codex', 'resume', 'session-1'], + resumeAgentArgs + }) + ).resolves.toEqual({ platform: 'win32', shell: 'powershell' }) }) it('never overrides a configured shell with the race guess', async () => { diff --git a/src/renderer/src/lib/agent-resume-launch-target.ts b/src/renderer/src/lib/agent-resume-launch-target.ts index af07619d484..5e5c12b409c 100644 --- a/src/renderer/src/lib/agent-resume-launch-target.ts +++ b/src/renderer/src/lib/agent-resume-launch-target.ts @@ -14,6 +14,8 @@ export type AgentResumeLaunchTarget = { platform: NodeJS.Platform /** undefined keeps the platform default: PowerShell on win32, POSIX elsewhere. */ shell: AgentStartupShell | undefined + /** Preserve persisted-command parsing when only the quoting style changes. */ + resumeCommandShell?: AgentStartupShell } export type AgentResumeLaunchTargetArgs = { @@ -26,15 +28,9 @@ export type AgentResumeLaunchTargetArgs = { terminalWindowsShell: string | null | undefined /** Per-tab Windows shell override, which beats the global setting at spawn time. */ tabShellOverride?: string | null - /** The resume argv this quoting is for. Only consulted for the cold-restore - * race guess below; omit it and the guess never fires. */ + /** Omit resume argv to disable the fallback for an unknown Windows shell. */ resumeArgv?: readonly string[] | null - /** The raw agentArgs suffix the built command will tokenize and `^`-escape - * per token (unless a custom agentCommand supersedes them, in which case pass - * null). The race-guess gate tokenizes it the same way and vets each token, - * so a cmd guess can never `^`-corrupt an agentArg token in a PowerShell race - * pane — including an INTERIOR token ending in `\`, which a whole-string - * check would miss. */ + /** Raw CLI arguments; null when a persisted agentCommand supersedes them. */ resumeAgentArgs?: string | null } @@ -72,32 +68,18 @@ export function resolveAgentResumeLaunchTarget( isRemote, terminalWindowsShell: effectiveWindowsShell }) - // Cold-restore race guess (#12320): a resume typed right after restart can run - // before the renderer store hydrates `settings`, so terminalWindowsShell is - // momentarily empty and we can't read which shell main actually spawned — the - // user's configured shell if set, otherwise the powershell.exe default. We - // still guess cmd here, but ONLY when every piece of free text the command - // `^`-escapes is cmd-quote-safe: `""` then parses identically in cmd - // AND PowerShell, so a configured cmd.exe pane is fixed (single quotes no - // longer reach it literally) with no risk to a PowerShell pane. Codex/Claude - // and every id-based agent qualify (clean flags + UUIDs). A path-carrying - // token (pi/prime-agent/omp transcript path, or a path in agentArgs, e.g. - // under `...\dir (x86)\...`) fails the gate and is left on the PowerShell - // default, because cmd `^`-escaping would corrupt it in a PowerShell race - // pane — never worse than the pre-guard behavior. + // Missing settings must not change argument values or persisted-command parsing. if (shell === 'powershell' && !effectiveWindowsShell?.trim() && args.resumeArgv) { - // Vet agentArgs as the command emits them — tokenized with cmd rules and - // `^`-escaped per token — not as one raw string: the safety of a token - // ending in `\` (arg-merge in cmd) is positional, so an interior token - // would slip a whole-string check. - const agentArgs = args.resumeAgentArgs?.trim() - ? tokenizeStartupCommand(args.resumeAgentArgs, 'cmd') - : null - if (!agentArgs || agentArgs.ok) { - const guardTokens = [...args.resumeArgv, ...(agentArgs?.tokens ?? [])] - if (guardTokens.every((token) => isCmdQuotingPowerShellSafe(token))) { - return { platform, shell: 'cmd' } - } + const cmdArgs = tokenizeStartupCommand(args.resumeAgentArgs?.trim() ?? '', 'cmd') + const powershellArgs = tokenizeStartupCommand(args.resumeAgentArgs?.trim() ?? '', 'powershell') + if ( + cmdArgs.ok && + powershellArgs.ok && + cmdArgs.tokens.length === powershellArgs.tokens.length && + cmdArgs.tokens.every((token, index) => token === powershellArgs.tokens[index]) && + [...args.resumeArgv, ...cmdArgs.tokens].every(isCmdQuotingPowerShellSafe) + ) { + return { platform, shell: 'cmd', resumeCommandShell: shell } } } return { platform, shell } diff --git a/src/renderer/src/lib/sleeping-agent-session-launch-windows-quoting.test.ts b/src/renderer/src/lib/sleeping-agent-session-launch-windows-quoting.test.ts index 59d54fbafb9..73d0b1c753c 100644 --- a/src/renderer/src/lib/sleeping-agent-session-launch-windows-quoting.test.ts +++ b/src/renderer/src/lib/sleeping-agent-session-launch-windows-quoting.test.ts @@ -78,9 +78,9 @@ const record: SleepingAgentSessionRecord = { updatedAt: 1 } -async function launch(): Promise { +async function launch(session = record): Promise { const { launchSleepingAgentSession } = await import('./sleeping-agent-session-launch') - launchSleepingAgentSession(record) + launchSleepingAgentSession(session) const options = mockCreateTab.mock.calls.at(-1)?.[3] as | { pendingStartup?: { command: string } } | undefined @@ -118,6 +118,28 @@ describe('launchSleepingAgentSession Windows shell quoting', () => { ) }) + it('preserves a caret path when the shell setting is unavailable', async () => { + await expect( + launch({ ...record, launchConfig: { agentArgs: '--add-dir C:\\work\\a^b', agentEnv: {} } }) + ).resolves.toBe(`codex '--add-dir' 'C:\\work\\a^b' 'resume' '${SESSION_ID}'`) + }) + + it.each([ + "claude --resume 'old-session'", + "claude '--model' 'sonnet' --continue", + "& claude --resume 'old-session'" + ])('retains selector cleanup during missing-settings fallback: %s', async (agentCommand) => { + const command = await launch({ + ...record, + agent: 'claude', + launchConfig: { agentCommand, agentArgs: '', agentEnv: {} } + }) + expect(command).not.toContain('old-session') + expect(command).not.toContain('--continue') + expect(command?.match(/--resume/g)).toHaveLength(1) + expect(command).toContain(`"--resume" "${SESSION_ID}"`) + }) + it('keeps PowerShell quoting for a powershell tab', async () => { store.settings.terminalWindowsShell = 'powershell.exe' diff --git a/src/renderer/src/lib/sleeping-agent-session-launch.ts b/src/renderer/src/lib/sleeping-agent-session-launch.ts index 141cd1c2a67..65bd1077466 100644 --- a/src/renderer/src/lib/sleeping-agent-session-launch.ts +++ b/src/renderer/src/lib/sleeping-agent-session-launch.ts @@ -101,7 +101,8 @@ export function launchSleepingAgentSession( ? { ompResumeFilePath: launchConfig.ompResumeFilePath } : {}), platform: resumeTarget.platform, - shell: resumeTarget.shell + shell: resumeTarget.shell, + resumeCommandShell: resumeTarget.resumeCommandShell }) if (!startupPlan) { toast.error( diff --git a/src/shared/agent-resume-launch-command.test.ts b/src/shared/agent-resume-launch-command.test.ts index 5bf4797472b..5526f872344 100644 --- a/src/shared/agent-resume-launch-command.test.ts +++ b/src/shared/agent-resume-launch-command.test.ts @@ -567,6 +567,23 @@ describe('buildAgentResumeStartupPlan claude selector guard', () => { expect(restored?.launchCommand).toBe(`gemini '--resume' '--resume' '${SESSION_ID}'`) }) + it('cleans a persisted PowerShell command while quoting the resume argv for cmd', () => { + const agentCommand = "& claude '--model' 'sonnet' --resume 'old-session' -- prompt" + const restored = buildAgentResumeStartupPlan({ + agent: 'claude', + providerSession, + cmdOverrides: {}, + agentCommand, + platform: 'win32', + shell: 'cmd', + resumeCommandShell: 'powershell' + }) + expect(restored?.launchCommand).toBe( + `& claude '--model' 'sonnet' "--resume" "${SESSION_ID}" -- prompt` + ) + expect(restored?.launchConfig.agentCommand).toBe(agentCommand) + }) + it('persists the original base command unchanged', () => { const restored = buildAgentResumeStartupPlan({ agent: 'claude', diff --git a/src/shared/agent-resume-launch-command.ts b/src/shared/agent-resume-launch-command.ts index cda40878c28..a02f5b952cd 100644 --- a/src/shared/agent-resume-launch-command.ts +++ b/src/shared/agent-resume-launch-command.ts @@ -60,11 +60,12 @@ export function buildAgentResumeLaunchCommand( agent: ResumableTuiAgent, baseCommand: string, resumeArgv: readonly string[], - shell: AgentStartupShell + shell: AgentStartupShell, + commandShell: AgentStartupShell = shell ): string { const argv = resumeArgv.slice(1) if (agent === 'claude') { - return buildClaudeResumeLaunchCommand(baseCommand, argv, shell) + return buildClaudeResumeLaunchCommand(baseCommand, argv, shell, commandShell) } const resumeArgs = argv.map((arg) => quoteStartupArg(arg, shell)).join(' ') return resumeArgs ? `${baseCommand} ${resumeArgs}` : baseCommand @@ -84,19 +85,20 @@ export function buildAgentResumeLaunchCommand( export function buildClaudeResumeLaunchCommand( baseCommand: string, resumeArgs: readonly string[], - shell: AgentStartupShell + shell: AgentStartupShell, + commandShell: AgentStartupShell = shell ): string { const quotedResume = resumeArgs.map((arg) => quoteStartupArg(arg, shell)).join(' ') if (!quotedResume) { return baseCommand } const appended = `${baseCommand} ${quotedResume}` - const tokenized = tokenizeStartupCommand(baseCommand, shell) + const tokenized = tokenizeStartupCommand(baseCommand, commandShell) if (!tokenized.ok) { return appended } const { tokens, spans } = tokenized - const claudeIndex = findClaudeExecutableIndex(tokens, shell) + const claudeIndex = findClaudeExecutableIndex(tokens, commandShell) if (claudeIndex === -1) { return appended } @@ -118,11 +120,14 @@ export function buildClaudeResumeLaunchCommand( // 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) === '--%') { + if ( + commandShell === 'powershell' && + baseCommand.slice(spans[i].start, spans[i].end) === '--%' + ) { return appended } if (spans[i].divergesFromShell) { - const isCallOperator = shell === 'powershell' && i === 0 && tokens[i] === '&' + const isCallOperator = commandShell === 'powershell' && i === 0 && tokens[i] === '&' if (!isCallOperator) { return appended } diff --git a/src/shared/agent-resume-quoting.win32.test.ts b/src/shared/agent-resume-quoting.win32.test.ts new file mode 100644 index 00000000000..14a204ca6cc --- /dev/null +++ b/src/shared/agent-resume-quoting.win32.test.ts @@ -0,0 +1,82 @@ +import { mkdtempSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { runProcess } from './child-process/run-process' +import { windowsPowerShellPath, windowsSystem32Binary } from './child-process/windows-system-binary' +import { removeTreeSync } from './windows-transient-lock-removal' +import { isCmdQuotingPowerShellSafe, quoteStartupArg } from './tui-agent-startup-shell' + +describe.skipIf(process.platform !== 'win32')('resume quoting in real Windows shells', () => { + let directory: string + let recorder: string + + beforeAll(() => { + directory = mkdtempSync(join(tmpdir(), 'orca-resume-quoting-')) + recorder = join(directory, 'argv.cjs') + writeFileSync(recorder, 'console.log("ORCA_ARGV:" + JSON.stringify(process.argv.slice(2)))') + }) + + afterAll(() => removeTreeSync(directory)) + + async function runLines(shell: 'cmd' | 'powershell', suffixes: string[]): Promise { + const prefix = + shell === 'cmd' + ? '"%ORCA_ARGV_NODE%" "%ORCA_ARGV_RECORDER%"' + : '& $env:ORCA_ARGV_NODE $env:ORCA_ARGV_RECORDER' + const lines = suffixes.map((suffix) => `${prefix} ${suffix}`).join('\r\n') + const result = await runProcess({ + program: shell === 'cmd' ? windowsSystem32Binary('cmd.exe') : windowsPowerShellPath(), + args: + shell === 'cmd' + ? ['/d', '/q'] + : ['-NoLogo', '-NoProfile', '-NonInteractive', '-Command', lines], + ...(shell === 'cmd' ? { input: `${lines}\r\nexit\r\n` } : {}), + env: { ...process.env, ORCA_ARGV_NODE: process.execPath, ORCA_ARGV_RECORDER: recorder } + }) + expect(result.code, result.stderr).toBe(0) + expect(result.timedOut).toBe(false) + return [...result.stdout.matchAll(/ORCA_ARGV:(\[[^\r\n]*\])/g)].map((match) => + JSON.parse(match[1]) + ) + } + + it.each(['cmd', 'powershell'] as const)( + 'round-trips accepted arguments through %s', + async (shell) => { + const values = [ + 'resume', + '--resume', + 'session-1', + 'C:\\work\\repo', + 'two words', + "it's literal", + 'a;b', + '#tag', + '{text}' + ] + expect(values.every(isCmdQuotingPowerShellSafe)).toBe(true) + expect( + await runLines(shell, [values.map((value) => quoteStartupArg(value, 'cmd')).join(' ')]) + ).toEqual([values]) + } + ) + + it('reproduces literal single quotes in cmd and verifies the double-quote fix', async () => { + expect(await runLines('cmd', ["'resume' 'session-1'", '"resume" "session-1"'])).toEqual([ + ["'resume'", "'session-1'"], + ['resume', 'session-1'] + ]) + }) + + it('keeps smart double quotes literal only with the PowerShell fallback', async () => { + const value = 'a\u201cb\u201dc' + expect(isCmdQuotingPowerShellSafe(value)).toBe(false) + const results = await runLines('powershell', [ + quoteStartupArg(value, 'powershell'), + quoteStartupArg(value, 'cmd') + ]) + expect(results[0]).toEqual([value]) + expect(results[1]).not.toEqual([value]) + }) +}) diff --git a/src/shared/tui-agent-resume-startup.ts b/src/shared/tui-agent-resume-startup.ts index f4924b83c47..39eb13d9857 100644 --- a/src/shared/tui-agent-resume-startup.ts +++ b/src/shared/tui-agent-resume-startup.ts @@ -18,6 +18,8 @@ export function buildAgentResumeStartupPlan(args: { cmdOverrides: Partial> platform: NodeJS.Platform shell?: AgentStartupShell + /** Shell used to interpret the persisted command before appending resume arguments. */ + resumeCommandShell?: AgentStartupShell agentArgs?: string | null agentEnv?: Record | null agentCommand?: string | null @@ -56,7 +58,13 @@ export function buildAgentResumeStartupPlan(args: { ...args, agentCommand: baseCommand.commandWithoutSessionOptions }) - const launchCommand = buildAgentResumeLaunchCommand(args.agent, baseCommand.command, argv, shell) + const launchCommand = buildAgentResumeLaunchCommand( + args.agent, + baseCommand.command, + argv, + shell, + args.resumeCommandShell + ) const applied = baseCommand.appliedSessionOptions return { agent: args.agent, diff --git a/src/shared/tui-agent-startup-shell.test.ts b/src/shared/tui-agent-startup-shell.test.ts index 6580bb3d7e6..b782bbcfe51 100644 --- a/src/shared/tui-agent-startup-shell.test.ts +++ b/src/shared/tui-agent-startup-shell.test.ts @@ -194,3 +194,16 @@ describe('isCmdQuotingPowerShellSafe', () => { expect(isCmdQuotingPowerShellSafe('C:\\dir\\file.jsonl')).toBe(true) }) }) + +describe('ambiguous Windows resume token quoting', () => { + it.each([ + '', + 'line\nbreak', + 'line\rbreak', + 'tab\tvalue', + 'nul\0value', + 'a\u201cb', + 'a\u201db', + 'a\u201eb' + ])('rejects an unsafe token %j', (value) => expect(isCmdQuotingPowerShellSafe(value)).toBe(false)) +}) diff --git a/src/shared/tui-agent-startup-shell.ts b/src/shared/tui-agent-startup-shell.ts index f7f74516f0d..3e771f5e334 100644 --- a/src/shared/tui-agent-startup-shell.ts +++ b/src/shared/tui-agent-startup-shell.ts @@ -215,24 +215,12 @@ function quotePortableUnixArg(value: string): string { return parts.join('') } -/** Characters that stop `quoteStartupArg(value, 'cmd')`'s `"value"` output from - * parsing identically in PowerShell. Two reasons a char lands here: - * - cmd quoting `^`-escapes it (`& | < > ( ) ^ % ! "`), and PowerShell does NOT - * strip the caret, so `"a^&b"` arrives as literal `a^&b` in a PowerShell pane; - * - PowerShell expands it inside a double-quoted string while cmd keeps it - * literal — `$` (variables / `$(...)`) and a backtick (escape char). */ -const CMD_QUOTING_POWERSHELL_UNSAFE = /[\^&|<>()%!"$`]/ +// Reject cmd escapes, PowerShell expansions/quotes, and terminal control characters. +const CMD_QUOTING_POWERSHELL_UNSAFE = /[\p{Cc}^&|<>()%!"$`\u201c-\u201e]/u -/** True when `quoteStartupArg(value, 'cmd')` also parses identically in - * PowerShell — i.e. `"value"` is the literal `value` in BOTH shells. Used to - * decide the resume race guess when the pane's shell is not yet known: cmd - * quoting of such a token is safe whether the pane turns out cmd or PowerShell. - * A token with an unsafe char, or a trailing backslash (which would turn the - * appended closing quote into an escaped quote for the child's - * CommandLineToArgvW parser, e.g. `"C:\dir\"`), fails and keeps the caller on - * its shell default. */ +/** A conservative subset whose cmd quoting preserves literal argv in both Windows shells. */ export function isCmdQuotingPowerShellSafe(value: string): boolean { - return !CMD_QUOTING_POWERSHELL_UNSAFE.test(value) && !value.endsWith('\\') + return value.length > 0 && !CMD_QUOTING_POWERSHELL_UNSAFE.test(value) && !value.endsWith('\\') } export function quoteStartupArg(value: string, shell: AgentStartupShell): string {