fix(terminal): preserve parsing during Windows resume quoting fallback

This commit is contained in:
Orca Worker
2026-09-07 13:51:17 -07:00
parent 00b3d0b81d
commit 8ff270ff57
12 changed files with 204 additions and 74 deletions
@@ -344,6 +344,7 @@ describe('connectPanePty', () => {
async function runWindowsColdRestoreResume(args: {
terminalWindowsShell?: string
tabShellOverride?: string
agentCommand?: string
}): Promise<string | undefined> {
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)
@@ -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
@@ -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 () => {
@@ -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: `"<token>"` 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 }
@@ -78,9 +78,9 @@ const record: SleepingAgentSessionRecord = {
updatedAt: 1
}
async function launch(): Promise<string | undefined> {
async function launch(session = record): Promise<string | undefined> {
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'
@@ -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(
@@ -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',
+12 -7
View File
@@ -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
}
@@ -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<string[][]> {
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])
})
})
+9 -1
View File
@@ -18,6 +18,8 @@ export function buildAgentResumeStartupPlan(args: {
cmdOverrides: Partial<Record<TuiAgent, string>>
platform: NodeJS.Platform
shell?: AgentStartupShell
/** Shell used to interpret the persisted command before appending resume arguments. */
resumeCommandShell?: AgentStartupShell
agentArgs?: string | null
agentEnv?: Record<string, string> | 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,
@@ -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))
})
+4 -16
View File
@@ -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 {