From 474b75c8dac7bd5e99bf1d9dee117bc628528cad Mon Sep 17 00:00:00 2001 From: m4air Date: Mon, 7 Sep 2026 03:24:46 -0700 Subject: [PATCH] fix(terminal): quote agent resume for cmd.exe when the Windows shell setting is unset A cold restore right after restart can run before the renderer store hydrates `settings`, so terminalWindowsShell is momentarily undefined. The renderer predicted PowerShell quoting for that empty case, but the spawn side launches %COMSPEC% (cmd.exe) when no shell is configured (local-pty-launch-plan.ts / local-pty-session-operations.ts). The single-quoted resume argv then reached the cmd.exe pane literally: codex rejected `'resume'` ("unrecognized subcommand ''resume''") and claude read `'--dangerously-skip-permissions'` as a prompt, starting a new session instead of resuming (#12320 residual). resolveLocalWindowsAgentStartupShell now mirrors that %COMSPEC% fallback, quoting an unset local Windows shell for cmd. Explicit powershell.exe/cmd.exe/git-bash/ wsl.exe settings, SSH/remote targets, and WSL panes are unchanged. --- ...ection-cold-restore-resume-command.test.ts | 19 ++++++- .../lib/agent-resume-launch-target.test.ts | 8 ++- ...h-agent-in-new-tab-windows-quoting.test.ts | 3 ++ src/shared/windows-terminal-shell.test.ts | 51 ++++++++++++++++++- src/shared/windows-terminal-shell.ts | 11 ++++ 5 files changed, 87 insertions(+), 5 deletions(-) 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 89bcc7254d1..1281f12b119 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 @@ -342,7 +342,7 @@ describe('connectPanePty', () => { // Regression (#12320): a cold restore after reboot typed PowerShell single quotes into // cmd.exe tabs, so the agent CLI rejected the resume argv ("unexpected argument"). async function runWindowsColdRestoreResume(args: { - terminalWindowsShell: string + terminalWindowsShell?: string tabShellOverride?: string }): Promise { const restoreNavigator = temporarilySetNavigatorUserAgent( @@ -397,7 +397,11 @@ describe('connectPanePty', () => { settings: { ...mockStoreState.settings, agentCmdOverrides: {}, - terminalWindowsShell: args.terminalWindowsShell + // Why: omitting the shell models the post-restart window where the store + // has not hydrated `settings` yet, so terminalWindowsShell is undefined. + ...(args.terminalWindowsShell !== undefined + ? { terminalWindowsShell: args.terminalWindowsShell } + : {}) }, sleepingAgentSessionsByPaneKey: { [paneKey]: { @@ -457,6 +461,17 @@ 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. 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. + 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('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/lib/agent-resume-launch-target.test.ts b/src/renderer/src/lib/agent-resume-launch-target.test.ts index c26541331f2..064bcf8c699 100644 --- a/src/renderer/src/lib/agent-resume-launch-target.test.ts +++ b/src/renderer/src/lib/agent-resume-launch-target.test.ts @@ -72,10 +72,14 @@ describe('resolveAgentResumeLaunchTarget on a Windows client', () => { }) }) - it('keeps PowerShell quoting when no Windows shell is configured', async () => { + 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). await expect(resolveWith({})).resolves.toEqual({ platform: 'win32', - shell: 'powershell' + shell: 'cmd' }) }) diff --git a/src/renderer/src/lib/launch-agent-in-new-tab-windows-quoting.test.ts b/src/renderer/src/lib/launch-agent-in-new-tab-windows-quoting.test.ts index 4d91f86a632..185623974ab 100644 --- a/src/renderer/src/lib/launch-agent-in-new-tab-windows-quoting.test.ts +++ b/src/renderer/src/lib/launch-agent-in-new-tab-windows-quoting.test.ts @@ -150,6 +150,9 @@ describe('launchAgentInNewTab Windows shell quoting', () => { it('uses the explicit startup shell platform when building draft launch commands', async () => { const { launchAgentInNewTab } = await import('./launch-agent-in-new-tab') + // Pin the shell so this asserts launchPlatform resolution, not the unset-shell + // fallback (which mirrors %COMSPEC% = cmd.exe; see windows-terminal-shell.test.ts). + store.settings.terminalWindowsShell = 'powershell.exe' launchAgentInNewTab({ agent: 'claude', worktreeId: 'wt-1', diff --git a/src/shared/windows-terminal-shell.test.ts b/src/shared/windows-terminal-shell.test.ts index e7d86a07cc9..a746a1a7f3c 100644 --- a/src/shared/windows-terminal-shell.test.ts +++ b/src/shared/windows-terminal-shell.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from 'vitest' -import { resolveWindowsShellStartupFamily } from './windows-terminal-shell' +import { + resolveLocalWindowsAgentStartupShell, + resolveWindowsShellStartupFamily +} from './windows-terminal-shell' describe('resolveWindowsShellStartupFamily', () => { it('defaults to PowerShell when unset', () => { @@ -33,3 +36,49 @@ describe('resolveWindowsShellStartupFamily', () => { expect(resolveWindowsShellStartupFamily('C:\\Program Files\\Git\\bin\\bash')).toBe('posix') }) }) + +describe('resolveLocalWindowsAgentStartupShell', () => { + it('yields no quoting override off Windows or for remote targets', () => { + expect( + resolveLocalWindowsAgentStartupShell({ platform: 'linux', isRemote: false }) + ).toBeUndefined() + expect( + resolveLocalWindowsAgentStartupShell({ + platform: 'win32', + isRemote: true, + terminalWindowsShell: 'cmd.exe' + }) + ).toBeUndefined() + }) + + it('classifies a configured local Windows shell', () => { + expect( + resolveLocalWindowsAgentStartupShell({ + platform: 'win32', + isRemote: false, + terminalWindowsShell: 'powershell.exe' + }) + ).toBe('powershell') + expect( + resolveLocalWindowsAgentStartupShell({ + platform: 'win32', + isRemote: false, + terminalWindowsShell: 'git-bash' + }) + ).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. + for (const shell of [undefined, null, '', ' ']) { + expect( + resolveLocalWindowsAgentStartupShell({ + platform: 'win32', + isRemote: false, + terminalWindowsShell: shell + }) + ).toBe('cmd') + } + }) +}) diff --git a/src/shared/windows-terminal-shell.ts b/src/shared/windows-terminal-shell.ts index 1f55bf89d35..57474b1d113 100644 --- a/src/shared/windows-terminal-shell.ts +++ b/src/shared/windows-terminal-shell.ts @@ -52,5 +52,16 @@ 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) }