fix(terminal): guess cmd quoting for agent resume during the pre-hydration shell race

On Windows, a cold-restore agent resume can run before the renderer store
hydrates `settings`, so `terminalWindowsShell` is momentarily empty and the
base resolver falls back to the win32 PowerShell default. If the pane is
actually cmd.exe, PowerShell single quotes reach cmd literally and the agent
CLI rejects the resume argv (codex "unrecognized subcommand ''resume''";
claude treats '--dangerously-skip-permissions' as a prompt) — #12320 residual.

Fix: a resume-scoped, gated race guess. When the shell setting is empty (the
race) and the base is the PowerShell default, prefer cmd quoting ONLY when
every free-text token the command `^`-escapes is cmd-quote-safe — i.e. its
`"token"` form parses to the same literal in cmd AND PowerShell. Such a guess
fixes a cmd pane with zero risk to a PowerShell pane. Path-carrying tokens
(pi/prime-agent/omp transcript paths, or a path in agentArgs) fail the gate and
stay on the PowerShell default, so cmd `^`-escaping never corrupts them.

- New `isCmdQuotingPowerShellSafe` rejects chars cmd `^`-escapes (^&|<>()%!")
  AND chars PowerShell expands in double quotes ($, backtick) AND a trailing
  backslash (would escape the appended closing quote for CommandLineToArgvW).
- The generic `resolveLocalWindowsAgentStartupShell` is unchanged; the guess
  lives only in `resolveAgentResumeLaunchTarget`, gated on the resume argv plus
  agentArgs (unless a custom agentCommand supersedes them).
This commit is contained in:
m4air
2026-09-07 04:13:01 -07:00
parent 474b75c8da
commit 00b3d0b81d
9 changed files with 233 additions and 49 deletions
@@ -462,10 +462,12 @@ describe('connectPanePty', () => {
})
// Regression (#12320 residual): a cold restore after restart can run before the
// store hydrates `settings`, so terminalWindowsShell is momentarily undefined. The
// spawn side then launches %COMSPEC% (cmd.exe), so the resume must be cmd-quoted —
// PowerShell single quotes reach cmd.exe literally ("unrecognized subcommand
// ''resume''"), which is exactly the field report on a released build.
// 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"'
@@ -8,6 +8,7 @@ import {
} from '../../../../../shared/tui-agent-launch-defaults'
import {
agentProviderSessionsEqual,
getAgentResumeArgv,
isResumableTuiAgent,
normalizeAgentProviderSession
} from '../../../../../shared/agent-session-resume'
@@ -49,6 +50,10 @@ export function bindBuildColdRestoreAgentResumeStartup(session: ConnectPanePtySe
const launchConfig =
(useLiveEntry && entry ? state.getAgentLaunchConfigForStatusEntry(entry) : undefined) ??
matchingSleepingLaunchConfig
const effectiveAgentArgs =
launchConfig !== undefined
? launchConfig.agentArgs
: resolveTuiAgentLaunchArgs(agent, state.settings?.agentDefaultArgs)
// Why: the resume line is typed into this pane's live shell, so its quoting must
// follow the tab's effective Windows shell, not the win32 PowerShell default.
const resumeTarget = resolveAgentResumeLaunchTarget({
@@ -57,16 +62,19 @@ export function bindBuildColdRestoreAgentResumeStartup(session: ConnectPanePtySe
executionHostId: session.executionHostId,
worktreePath: session.worktree?.path,
terminalWindowsShell: state.settings?.terminalWindowsShell,
tabShellOverride: session.shellOverride
tabShellOverride: session.shellOverride,
// The cold-restore path can run before the store hydrates the shell setting;
// pass the resume argv (and agentArgs, unless a custom command supersedes
// them) so the target resolver's race guess can prove cmd-quoting is safe.
resumeArgv:
getAgentResumeArgv(agent, providerSession, launchConfig?.ompResumeFilePath) ?? undefined,
resumeAgentArgs: launchConfig?.agentCommand?.trim() ? null : effectiveAgentArgs
})
const startupPlan = buildAgentResumeStartupPlan({
agent,
providerSession,
cmdOverrides: state.settings?.agentCmdOverrides ?? {},
agentArgs:
launchConfig !== undefined
? launchConfig.agentArgs
: resolveTuiAgentLaunchArgs(agent, state.settings?.agentDefaultArgs),
agentArgs: effectiveAgentArgs,
agentEnv:
launchConfig !== undefined
? launchConfig.agentEnv
@@ -72,17 +72,78 @@ describe('resolveAgentResumeLaunchTarget on a Windows client', () => {
})
})
it('quotes for cmd.exe when no Windows shell is configured (mirrors the %COMSPEC% spawn fallback)', async () => {
// Why cmd, not PowerShell: with no configured shell the spawn side launches
// %COMSPEC% (cmd.exe), and this state is reachable when a cold restore runs
// before the store hydrates `settings`. PowerShell quoting here would put
// single quotes into a cmd.exe pane and break the resume (#12320 residual).
it('keeps the PowerShell default when no Windows shell is configured and no resume argv is given', async () => {
// Without the resume argv the race guess cannot prove cmd-quoting is safe, so
// it must not fire — an unset shell falls back to the win32 PowerShell default.
await expect(resolveWith({})).resolves.toEqual({
platform: 'win32',
shell: 'cmd'
shell: 'powershell'
})
})
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' })
})
it('keeps the PowerShell default for an unset shell when a resume token needs cmd escaping', async () => {
// An omp/pi/prime-agent transcript path can carry cmd-special chars (parens,
// e.g. `(x86)`). cmd `^`-escaping would corrupt that path in a PowerShell
// race pane, so the guess must stay off and leave the win32 default.
await expect(
resolveWith({
resumeArgv: ['omp', '--resume', 'C:\\Users\\neil\\AppData (x86)\\omp\\session.jsonl']
})
).resolves.toEqual({ platform: 'win32', shell: 'powershell' })
})
it('keeps the PowerShell default for an unset shell when agentArgs need cmd escaping', async () => {
// The resume argv is clean, but agentArgs the command will ^-escape carry a
// path with parens; a cmd guess would corrupt them in a PowerShell race pane.
await expect(
resolveWith({
resumeArgv: ['codex', 'resume', 'a1b2c3d4-0000-4000-8000-000000000000'],
resumeAgentArgs: '--add-dir C:\\Program Files (x86)\\proj'
})
).resolves.toEqual({ platform: 'win32', shell: 'powershell' })
})
it('keeps the PowerShell default when an INTERIOR agentArgs token ends in a backslash', async () => {
// The raw string does not end in `\`, but the command tokenizes agentArgs
// and cmd-quotes each token, so the interior `C:\projects\` becomes
// `"C:\projects\"` — an arg-merge under CommandLineToArgvW. The gate must
// tokenize the same way and reject it, not scan the raw string.
await expect(
resolveWith({
resumeArgv: ['codex', 'resume', 'a1b2c3d4-0000-4000-8000-000000000000'],
resumeAgentArgs: '--add-dir C:\\projects\\ --model gpt'
})
).resolves.toEqual({ platform: 'win32', shell: 'powershell' })
})
it('still guesses cmd for an unset shell when clean agentArgs accompany a clean resume argv', async () => {
await expect(
resolveWith({
resumeArgv: ['codex', 'resume', 'a1b2c3d4-0000-4000-8000-000000000000'],
resumeAgentArgs: '--dangerously-bypass-approvals-and-sandbox --model gpt'
})
).resolves.toEqual({ platform: 'win32', shell: 'cmd' })
})
it('never overrides a configured shell with the race guess', async () => {
// A hydrated powershell.exe must win even when the argv is cmd-quote-safe.
await expect(
resolveWith({
terminalWindowsShell: 'powershell.exe',
resumeArgv: ['codex', 'resume', 'a1b2c3d4-0000-4000-8000-000000000000']
})
).resolves.toEqual({ platform: 'win32', shell: 'powershell' })
})
it('leaves an SSH workspace on its own default quoting', async () => {
await expect(
resolveWith({
@@ -4,7 +4,11 @@ import { parseExecutionHostId } from '../../../shared/execution-host'
import { isWslUncPath } from '../../../shared/wsl-paths'
import { resolveLocalWindowsAgentStartupShell } from '../../../shared/windows-terminal-shell'
import type { ProjectExecutionRuntimeResolution } from '../../../shared/project-execution-runtime'
import type { AgentStartupShell } from '../../../shared/tui-agent-startup-shell'
import {
isCmdQuotingPowerShellSafe,
tokenizeStartupCommand,
type AgentStartupShell
} from '../../../shared/tui-agent-startup-shell'
export type AgentResumeLaunchTarget = {
platform: NodeJS.Platform
@@ -22,6 +26,16 @@ 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. */
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. */
resumeAgentArgs?: string | null
}
function resolveResumeLaunchPlatform(args: AgentResumeLaunchTargetArgs): NodeJS.Platform {
@@ -47,16 +61,44 @@ export function resolveAgentResumeLaunchTarget(
args: AgentResumeLaunchTargetArgs
): AgentResumeLaunchTarget {
const platform = resolveResumeLaunchPlatform(args)
return {
const isRemote =
Boolean(args.connectionId) || parseExecutionHostId(args.executionHostId)?.kind !== 'local'
const effectiveWindowsShell = resolveWindowsShellOverride(
args.tabShellOverride,
args.terminalWindowsShell
)
const shell = resolveLocalWindowsAgentStartupShell({
platform,
shell: resolveLocalWindowsAgentStartupShell({
platform,
isRemote:
Boolean(args.connectionId) || parseExecutionHostId(args.executionHostId)?.kind !== 'local',
terminalWindowsShell: resolveWindowsShellOverride(
args.tabShellOverride,
args.terminalWindowsShell
)
})
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.
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' }
}
}
}
return { platform, shell }
}
@@ -13,7 +13,10 @@ import {
resolveTuiAgentLaunchArgs,
resolveTuiAgentLaunchEnv
} from '../../../shared/tui-agent-launch-defaults'
import type { SleepingAgentSessionRecord } from '../../../shared/agent-session-resume'
import {
getAgentResumeArgv,
type SleepingAgentSessionRecord
} from '../../../shared/agent-session-resume'
import { translate } from '@/i18n/i18n'
export type ResumeSleepingAgentSessionsOptions = {
@@ -28,7 +31,11 @@ export type ResumeSleepingAgentSessionsOptions = {
onSessionLaunched?: (tabId: string) => void
}
function getResumeLaunchTarget(worktreeId: string): AgentResumeLaunchTarget {
function getResumeLaunchTarget(
record: SleepingAgentSessionRecord,
effectiveAgentArgs: string | null | undefined
): AgentResumeLaunchTarget {
const worktreeId = record.worktreeId
const state = useAppStore.getState()
const worktree = state.getKnownWorktreeById(worktreeId)
const repo = worktree ? state.repos.find((entry) => entry.id === worktree.repoId) : null
@@ -38,7 +45,14 @@ function getResumeLaunchTarget(worktreeId: string): AgentResumeLaunchTarget {
connectionId: repo?.connectionId,
executionHostId: getExecutionHostIdForWorktree(state, worktreeId),
worktreePath: worktree?.path,
terminalWindowsShell: state.settings?.terminalWindowsShell
terminalWindowsShell: state.settings?.terminalWindowsShell,
resumeArgv:
getAgentResumeArgv(
record.agent,
record.providerSession,
record.launchConfig?.ompResumeFilePath
) ?? undefined,
resumeAgentArgs: record.launchConfig?.agentCommand?.trim() ? null : effectiveAgentArgs
})
}
@@ -68,15 +82,16 @@ export function launchSleepingAgentSession(
): boolean {
const state = useAppStore.getState()
const launchConfig = record.launchConfig
const resumeTarget = getResumeLaunchTarget(record.worktreeId)
const effectiveAgentArgs =
launchConfig !== undefined
? launchConfig.agentArgs
: resolveTuiAgentLaunchArgs(record.agent, state.settings?.agentDefaultArgs)
const resumeTarget = getResumeLaunchTarget(record, effectiveAgentArgs)
const startupPlan = buildAgentResumeStartupPlan({
agent: record.agent,
providerSession: record.providerSession,
cmdOverrides: state.settings?.agentCmdOverrides ?? {},
agentArgs:
launchConfig !== undefined
? launchConfig.agentArgs
: resolveTuiAgentLaunchArgs(record.agent, state.settings?.agentDefaultArgs),
agentArgs: effectiveAgentArgs,
agentEnv:
launchConfig !== undefined
? launchConfig.agentEnv
@@ -3,6 +3,7 @@ import {
buildShellCommandFromArgv,
clearEnvCommand,
commandSeparator,
isCmdQuotingPowerShellSafe,
isPosixStartupShell,
quoteStartupArg,
tokenizeStartupCommand
@@ -148,3 +149,48 @@ describe('one Unix startup dialect', () => {
expect(plan?.env?.ORCA_PI_PREFILL).toBe('hello')
})
})
describe('isCmdQuotingPowerShellSafe', () => {
it('accepts tokens cmd quotes as a bare "token" (safe in cmd AND PowerShell)', () => {
// Every id-based resume argv token: binary names, flags and UUIDs.
for (const token of [
'codex',
'resume',
'a1b2c3d4-0000-4000-8000-000000000000',
'--dangerously-bypass-approvals-and-sandbox',
'--resume=abc',
'C:\\Users\\neil\\repo'
]) {
expect(isCmdQuotingPowerShellSafe(token)).toBe(true)
// Contract: a safe token quotes to the same bare "token" in both shells.
expect(quoteStartupArg(token, 'cmd')).toBe(`"${token}"`)
}
})
it('rejects tokens that cmd would ^-escape, which corrupts them in a PowerShell pane', () => {
for (const token of [
'C:\\Users\\neil\\AppData (x86)\\omp\\session.jsonl',
'a&b',
'pct%VALUE%',
'a|b',
'a<b>c',
'has"quote'
]) {
expect(isCmdQuotingPowerShellSafe(token)).toBe(false)
}
})
it('rejects tokens PowerShell expands inside double quotes ($ and backtick) even though cmd keeps them literal', () => {
// `"$env"` / "a`b" parse literally in cmd but expand/escape in PowerShell.
for (const token of ['$env', 'a$b', 'has`backtick', '`n']) {
expect(isCmdQuotingPowerShellSafe(token)).toBe(false)
}
})
it('rejects a trailing backslash, which would escape the appended closing quote', () => {
// `"C:\dir\"` → the child CommandLineToArgvW sees \" as a literal quote.
expect(isCmdQuotingPowerShellSafe('C:\\dir\\')).toBe(false)
// A backslash NOT at the end (a normal Windows path) stays safe.
expect(isCmdQuotingPowerShellSafe('C:\\dir\\file.jsonl')).toBe(true)
})
})
+20
View File
@@ -215,6 +215,26 @@ 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 = /[\^&|<>()%!"$`]/
/** 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. */
export function isCmdQuotingPowerShellSafe(value: string): boolean {
return !CMD_QUOTING_POWERSHELL_UNSAFE.test(value) && !value.endsWith('\\')
}
export function quoteStartupArg(value: string, shell: AgentStartupShell): string {
if (shell === 'powershell') {
return `'${value.replace(/'/g, "''")}'`
+5 -4
View File
@@ -68,9 +68,10 @@ describe('resolveLocalWindowsAgentStartupShell', () => {
).toBe('posix')
})
it('defaults an unset local Windows shell to cmd, mirroring the %COMSPEC% spawn fallback', () => {
// Why not PowerShell: the spawn side launches %COMSPEC% (cmd.exe) when no shell
// is configured, so quoting must match or single quotes reach cmd.exe literally.
it('defaults an unset local Windows shell to PowerShell, matching the win32 default', () => {
// The resume race guess that may prefer cmd for an unset shell lives in
// resolveAgentResumeLaunchTarget, not here; this resolver reflects only the
// configured shell and its win32 default.
for (const shell of [undefined, null, '', ' ']) {
expect(
resolveLocalWindowsAgentStartupShell({
@@ -78,7 +79,7 @@ describe('resolveLocalWindowsAgentStartupShell', () => {
isRemote: false,
terminalWindowsShell: shell
})
).toBe('cmd')
).toBe('powershell')
}
})
})
-11
View File
@@ -52,16 +52,5 @@ export function resolveLocalWindowsAgentStartupShell(args: {
if (args.platform !== 'win32' || args.isRemote) {
return undefined
}
// Why cmd, not the PowerShell default: with no configured shell the spawn side
// falls back to %COMSPEC% (cmd.exe) — local-pty-launch-plan.ts and
// local-pty-session-operations.ts both `... || process.env.COMSPEC || 'powershell.exe'`.
// A cold restore right after restart can run before the renderer store hydrates
// `settings`, so terminalWindowsShell is momentarily empty; predicting PowerShell
// then quotes the resume argv in single quotes that cmd.exe passes through
// literally, breaking the resume (#12320 residual: codex "unrecognized subcommand
// ''resume''", claude reads '--dangerously-skip-permissions' as a prompt).
if (!args.terminalWindowsShell?.trim()) {
return 'cmd'
}
return resolveWindowsShellStartupFamily(args.terminalWindowsShell)
}