mirror of
https://github.com/stablyai/orca.git
synced 2026-09-30 08:03:12 +00:00
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.
This commit is contained in:
+17
-2
@@ -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<string | undefined> {
|
||||
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)
|
||||
|
||||
@@ -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'
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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')
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user