diff --git a/src/main/daemon/daemon-pty-startup-delivery.test.ts b/src/main/daemon/daemon-pty-startup-delivery.test.ts index 4b72070bb42..3cffff8dec2 100644 --- a/src/main/daemon/daemon-pty-startup-delivery.test.ts +++ b/src/main/daemon/daemon-pty-startup-delivery.test.ts @@ -52,7 +52,7 @@ describe('DaemonPtyAdapter startup delivery', () => { await vi.advanceTimersByTimeAsync(299) expect(lastSubprocess.write).not.toHaveBeenCalled() await vi.advanceTimersByTimeAsync(1) - expect(lastSubprocess.write).toHaveBeenCalledExactlyOnceWith('codex\n') + expect(lastSubprocess.write).toHaveBeenCalledExactlyOnceWith('codex\r') expect(lastSpawnOpts).not.toEqual( expect.objectContaining({ startupCommandDelivery: 'shell-ready' }) ) @@ -82,7 +82,7 @@ describe('DaemonPtyAdapter startup delivery', () => { ) lastSubprocess._simulateData('\x1b]777;orca-shell-ready\x07\r\nuser@host $ ') await waitFor(() => vi.mocked(lastSubprocess.write).mock.calls.length > 0) - expect(lastSubprocess.write).toHaveBeenCalledExactlyOnceWith('codex\n') + expect(lastSubprocess.write).toHaveBeenCalledExactlyOnceWith('codex\r') } ) @@ -102,6 +102,6 @@ describe('DaemonPtyAdapter startup delivery', () => { lastSubprocess._simulateData('\r\nuser@host $ ') await waitFor(() => vi.mocked(lastSubprocess.write).mock.calls.length > 0) - expect(lastSubprocess.write).toHaveBeenCalledExactlyOnceWith(`${startup.command}\n`) + expect(lastSubprocess.write).toHaveBeenCalledExactlyOnceWith(`${startup.command}\r`) }) }) diff --git a/src/main/daemon/terminal-host-session-create.ts b/src/main/daemon/terminal-host-session-create.ts index 15e86a70a72..f490c5d5722 100644 --- a/src/main/daemon/terminal-host-session-create.ts +++ b/src/main/daemon/terminal-host-session-create.ts @@ -197,11 +197,9 @@ async function spawnAndPublishSession( // Diagnostics must never turn a live PTY into a failed create. } if (startupCommandWritten && opts.command) { - const submit = process.platform === 'win32' ? '\r' : '\n' // Why: only Orca-wrapped shells advertise the paste-safe startup barrier. session.write( buildStartupCommandSubmission(opts.command, { - submit, bracketedPasteSafe: shellReadySupported }) ) diff --git a/src/main/daemon/terminal-host-startup.test.ts b/src/main/daemon/terminal-host-startup.test.ts index c6817b1e758..830bf0c40bd 100644 --- a/src/main/daemon/terminal-host-startup.test.ts +++ b/src/main/daemon/terminal-host-startup.test.ts @@ -19,11 +19,8 @@ function mockSubprocess(): SubprocessHandle { } as SubprocessHandle } -// Why: Windows shells (PowerShell/cmd.exe) submit on CR, not LF. Without CR -// the startup command sits typed at the prompt but unexecuted — forcing the -// user to press Enter after "claude" (or a setup script) is injected. -// POSIX shells (bash/zsh) keep the LF behaviour. A caller-supplied terminator -// must not be doubled. +// Why: without CR (Enter) the startup command sits typed at the prompt but +// unexecuted (#23250). A caller-supplied terminator is replaced, not doubled. describe('TerminalHost startup command terminator', () => { const origPlatform = process.platform afterEach(() => { @@ -39,9 +36,11 @@ describe('TerminalHost startup command terminator', () => { it.each([ ['win32', 'claude', 'claude\r'], - ['darwin', 'claude', 'claude\n'], + ['darwin', 'claude', 'claude\r'], + ['linux', 'claude', 'claude\r'], ['win32', 'claude\r', 'claude\r'], - ['darwin', 'claude\n', 'claude\n'] + ['darwin', 'claude\n', 'claude\r'], + ['win32', 'claude\r\n', 'claude\r'] ])('submits startup with correct terminator on %s', async (platform, cmd, sent) => { Object.defineProperty(process, 'platform', { value: platform }) await host.createOrAttach({ @@ -128,6 +127,6 @@ describe('TerminalHost startup command delivery logging', () => { streamClient: { onData: vi.fn(), onExit: vi.fn() } }) ).resolves.toMatchObject({ isNew: true }) - expect(sub.write).toHaveBeenCalledWith(`codex${process.platform === 'win32' ? '\r' : '\n'}`) + expect(sub.write).toHaveBeenCalledWith('codex\r') }) }) diff --git a/src/main/daemon/terminal-host.test.ts b/src/main/daemon/terminal-host.test.ts index 7ffaadec06d..f7330b3525c 100644 --- a/src/main/daemon/terminal-host.test.ts +++ b/src/main/daemon/terminal-host.test.ts @@ -189,9 +189,7 @@ describe('TerminalHost', () => { lastSubprocess._onDataCb?.('\r\nuser@host $ ') await new Promise((r) => setTimeout(r, 40)) - expect(lastSubprocess.write).toHaveBeenCalledWith( - process.platform === 'win32' ? 'echo hello\r' : 'echo hello\n' - ) + expect(lastSubprocess.write).toHaveBeenCalledWith('echo hello\r') }) it('uses the short daemon settle path when marker and prompt arrive together', async () => { @@ -211,9 +209,7 @@ describe('TerminalHost', () => { expect(lastSubprocess.write).not.toHaveBeenCalled() vi.advanceTimersByTime(1) - expect(lastSubprocess.write).toHaveBeenCalledWith( - process.platform === 'win32' ? 'echo hello\r' : 'echo hello\n' - ) + expect(lastSubprocess.write).toHaveBeenCalledWith('echo hello\r') } finally { vi.useRealTimers() } @@ -242,9 +238,7 @@ describe('TerminalHost', () => { streamClient: { onData: vi.fn(), onExit: vi.fn() } }) - expect(lastSubprocess.write).toHaveBeenCalledWith( - process.platform === 'win32' ? 'echo hello\r' : 'echo hello\n' - ) + expect(lastSubprocess.write).toHaveBeenCalledWith('echo hello\r') }) it('does not bracketed-paste-wrap multiline commands for a fallback shell without paste mode', async () => { @@ -272,7 +266,8 @@ describe('TerminalHost', () => { const written = (lastSubprocess.write as ReturnType).mock.calls[0]?.[0] expect(written).not.toContain('\x1b[200~') - expect(written).toContain('line one\nline two') + // Why CR between the lines: without bracketed paste each break submits its own line. + expect(written).toContain('line one\rline two') }) it('keeps the shell-ready barrier when the spawned shell supports the marker', async () => { diff --git a/src/main/ipc/pty-login-shell-startup-commands.test.ts b/src/main/ipc/pty-login-shell-startup-commands.test.ts index 7ccb0386b9f..a89aca38ea5 100644 --- a/src/main/ipc/pty-login-shell-startup-commands.test.ts +++ b/src/main/ipc/pty-login-shell-startup-commands.test.ts @@ -233,7 +233,7 @@ describe('registerPtyHandlers', () => { mockProc.emitData('\x1b]133;A\x07% ') await Promise.resolve() vi.runAllTimers() - expect(mockProc.proc.write).toHaveBeenCalledWith('claude\n') + expect(mockProc.proc.write).toHaveBeenCalledWith('claude\r') } finally { vi.useRealTimers() } @@ -396,7 +396,7 @@ describe('registerPtyHandlers', () => { vi.advanceTimersByTime(1) await Promise.resolve() vi.runAllTimers() - expect(mockProc.proc.write).toHaveBeenCalledWith('printf "hello"\n') + expect(mockProc.proc.write).toHaveBeenCalledWith('printf "hello"\r') } finally { vi.useRealTimers() } diff --git a/src/main/ipc/pty-spawn-env-codex-resume-provenance.test.ts b/src/main/ipc/pty-spawn-env-codex-resume-provenance.test.ts index a2a6153e99a..f3b30321c55 100644 --- a/src/main/ipc/pty-spawn-env-codex-resume-provenance.test.ts +++ b/src/main/ipc/pty-spawn-env-codex-resume-provenance.test.ts @@ -278,7 +278,7 @@ describe('registerPtyHandlers', () => { await Promise.resolve() vi.runAllTimers() - expect(mockProc.proc.write).toHaveBeenCalledWith(`${command}\n`) + expect(mockProc.proc.write).toHaveBeenCalledWith(`${command}\r`) expect(spawned.agentResumeUnavailable).toBeUndefined() } finally { vi.useRealTimers() diff --git a/src/main/providers/local-pty-provider-shell-readiness.test.ts b/src/main/providers/local-pty-provider-shell-readiness.test.ts index 6a46261e793..e33a681ed8c 100644 --- a/src/main/providers/local-pty-provider-shell-readiness.test.ts +++ b/src/main/providers/local-pty-provider-shell-readiness.test.ts @@ -217,7 +217,7 @@ describe('LocalPtyProvider', () => { expect(mockProc.write).not.toHaveBeenCalled() await vi.advanceTimersByTimeAsync(200) - expect(mockProc.write).toHaveBeenCalledWith(`${command}\n`) + expect(mockProc.write).toHaveBeenCalledWith(`${command}\r`) } finally { vi.useRealTimers() } @@ -269,7 +269,7 @@ describe('LocalPtyProvider', () => { vi.advanceTimersByTime(1) await Promise.resolve() - expect(mockProc.write).toHaveBeenCalledWith("printf 'linked issue context'\n") + expect(mockProc.write).toHaveBeenCalledWith("printf 'linked issue context'\r") } finally { vi.useRealTimers() } @@ -298,7 +298,7 @@ describe('LocalPtyProvider', () => { vi.advanceTimersByTime(200) await Promise.resolve() - expect(mockProc.write).toHaveBeenCalledWith('printf ready\n') + expect(mockProc.write).toHaveBeenCalledWith('printf ready\r') } finally { vi.useRealTimers() } diff --git a/src/main/providers/local-pty-shell-ready-startup-command.test.ts b/src/main/providers/local-pty-shell-ready-startup-command.test.ts index da843c6e42a..9b7c50ef521 100644 --- a/src/main/providers/local-pty-shell-ready-startup-command.test.ts +++ b/src/main/providers/local-pty-shell-ready-startup-command.test.ts @@ -58,8 +58,8 @@ describe('writeStartupCommandWhenShellReady', () => { Object.defineProperty(process, 'platform', { value: origPlatform }) }) - it('appends LF on POSIX so bash/zsh submit the line', async () => { - Object.defineProperty(process, 'platform', { value: 'darwin' }) + it.each(['darwin', 'linux', 'win32'])('submits with CR on %s', async (platform) => { + Object.defineProperty(process, 'platform', { value: platform }) const proc = createMockProc() const ready = Promise.resolve() writeStartupCommandWhenShellReady(ready, proc, 'claude', () => {}) @@ -69,39 +69,23 @@ describe('writeStartupCommandWhenShellReady', () => { vi.advanceTimersByTime(30) await Promise.resolve() - expect(proc._writes).toEqual(['claude\n']) + expect(proc._writes).toEqual(['claude\r']) }) - it('appends CR on Windows so PowerShell/cmd.exe submit the line', async () => { - Object.defineProperty(process, 'platform', { value: 'win32' }) + it('replaces a caller-supplied LF terminator with CR', async () => { const proc = createMockProc() const ready = Promise.resolve() - writeStartupCommandWhenShellReady(ready, proc, 'claude', () => {}) + writeStartupCommandWhenShellReady(ready, proc, 'claude\n', () => {}) await ready - proc._emitData('\r\nPS> ') + proc._emitData('\r\nuser@host % ') vi.advanceTimersByTime(30) await Promise.resolve() expect(proc._writes).toEqual(['claude\r']) }) - it('does not re-append a submit byte if the command already ends in CR or LF', async () => { - Object.defineProperty(process, 'platform', { value: 'win32' }) - const proc = createMockProc() - const ready = Promise.resolve() - writeStartupCommandWhenShellReady(ready, proc, 'claude\n', () => {}) - - await ready - proc._emitData('\r\nPS> ') - vi.advanceTimersByTime(30) - await Promise.resolve() - - expect(proc._writes).toEqual(['claude\n']) - }) - it('keeps the no-prompt fallback conservative to avoid duplicate shell echo', async () => { - Object.defineProperty(process, 'platform', { value: 'darwin' }) const proc = createMockProc() const ready = Promise.resolve() writeStartupCommandWhenShellReady(ready, proc, 'codex', () => {}) @@ -115,11 +99,10 @@ describe('writeStartupCommandWhenShellReady', () => { vi.advanceTimersByTime(150) await Promise.resolve() - expect(proc._writes).toEqual(['codex\n']) + expect(proc._writes).toEqual(['codex\r']) }) it('uses the short settle delay when marker scan already observed post-marker bytes', async () => { - Object.defineProperty(process, 'platform', { value: 'darwin' }) const proc = createMockProc() const ready = Promise.resolve({ postMarkerBytesObserved: true }) writeStartupCommandWhenShellReady(ready, proc, 'codex', () => {}) @@ -131,12 +114,11 @@ describe('writeStartupCommandWhenShellReady', () => { vi.advanceTimersByTime(1) await Promise.resolve() - expect(proc._writes).toEqual(['codex\n']) + expect(proc._writes).toEqual(['codex\r']) }) // Why: multiline startup commands must be bracketed-paste wrapped (ESC[200~ … ESC[201~) so shells insert them literally instead of treating each LF as Enter. it('wraps a multiline startup command in bracketed paste when the shell supports it', async () => { - Object.defineProperty(process, 'platform', { value: 'darwin' }) const proc = createMockProc() const ready = Promise.resolve() const command = "claude '--dangerously-skip-permissions' 'line one\nline two'" @@ -149,11 +131,10 @@ describe('writeStartupCommandWhenShellReady', () => { vi.advanceTimersByTime(30) await Promise.resolve() - expect(proc._writes).toEqual([`\x1b[200~${command}\x1b[201~\n`]) + expect(proc._writes).toEqual([`\x1b[200~${command}\x1b[201~\r`]) }) it('leaves a single-line command on the raw submit path even when bracketed paste is safe', async () => { - Object.defineProperty(process, 'platform', { value: 'darwin' }) const proc = createMockProc() const ready = Promise.resolve() writeStartupCommandWhenShellReady(ready, proc, 'claude', () => {}, { @@ -165,14 +146,14 @@ describe('writeStartupCommandWhenShellReady', () => { vi.advanceTimersByTime(30) await Promise.resolve() - expect(proc._writes).toEqual(['claude\n']) + expect(proc._writes).toEqual(['claude\r']) }) it('does not bracket-wrap a multiline command when the shell lacks bracketed paste', async () => { - Object.defineProperty(process, 'platform', { value: 'darwin' }) const proc = createMockProc() const ready = Promise.resolve() const command = 'echo one\necho two' + // Why CR between the lines: without bracketed paste each break submits its own line. // Why: bracketedPasteSafe defaults false, so keep the raw path to avoid echoing ESC[200~ on shells without bracketed paste. writeStartupCommandWhenShellReady(ready, proc, command, () => {}) @@ -181,6 +162,6 @@ describe('writeStartupCommandWhenShellReady', () => { vi.advanceTimersByTime(30) await Promise.resolve() - expect(proc._writes).toEqual([`${command}\n`]) + expect(proc._writes).toEqual(['echo one\recho two\r']) }) }) diff --git a/src/main/providers/local-pty-shell-ready-startup-command.ts b/src/main/providers/local-pty-shell-ready-startup-command.ts index bf7f73c19d7..5fefb5db5fa 100644 --- a/src/main/providers/local-pty-shell-ready-startup-command.ts +++ b/src/main/providers/local-pty-shell-ready-startup-command.ts @@ -46,12 +46,9 @@ export function writeStartupCommandWhenShellReady( postReadyTimer = null } // Why: run in the same interactive shell (not `shell -c`) so the session survives after the agent exits. - // Why CR on Windows: PSReadLine/cmd.exe submit on `\r`, not LF; POSIX treats either as Enter under ICRNL. - const submit = process.platform === 'win32' ? '\r' : '\n' // Why: single write after the ready barrier avoids incremental-paste char drops; multiline is bracketed-paste wrapped so newlines don't submit early. proc.write( buildStartupCommandSubmission(startupCommand, { - submit, bracketedPasteSafe: options.bracketedPasteSafe === true }) ) diff --git a/src/relay/pty-handler-startup-command-delivery.test.ts b/src/relay/pty-handler-startup-command-delivery.test.ts index 2215eff9d76..a8d812ed89a 100644 --- a/src/relay/pty-handler-startup-command-delivery.test.ts +++ b/src/relay/pty-handler-startup-command-delivery.test.ts @@ -89,8 +89,7 @@ describe('PtyHandler', () => { expect(term.write).not.toHaveBeenCalled() vi.advanceTimersByTime(1) - const submit = process.platform === 'win32' ? '\r' : '\n' - expect(term.write).toHaveBeenCalledWith(`echo provider-owned${submit}`) + expect(term.write).toHaveBeenCalledWith('echo provider-owned\r') expect(handler.retainedStartupCommandCount).toBe(0) }) @@ -364,7 +363,7 @@ describe('PtyHandler', () => { expect(term.write).not.toHaveBeenCalled() vi.advanceTimersByTime(1) - expect(term.write).toHaveBeenCalledWith('echo after-ready\n') + expect(term.write).toHaveBeenCalledWith('echo after-ready\r') expect(handler.retainedStartupCommandCount).toBe(0) vi.advanceTimersByTime(8) expect(dispatcher.notify).toHaveBeenCalledWith('pty.data', { @@ -421,7 +420,7 @@ describe('PtyHandler', () => { promptOptions.onPromptReady() await vi.advanceTimersByTimeAsync(50) - expect(term.write).toHaveBeenCalledWith('echo after-exec\n') + expect(term.write).toHaveBeenCalledWith('echo after-exec\r') expect(handler.retainedStartupCommandCount).toBe(0) } ) @@ -635,7 +634,7 @@ describe('PtyHandler', () => { dataCallback?.('\x1b]777;orca-shell-ready') vi.advanceTimersByTime(1500) - expect(term.write).toHaveBeenCalledWith('echo fallback\n') + expect(term.write).toHaveBeenCalledWith('echo fallback\r') vi.advanceTimersByTime(8) expect(dispatcher.notify).toHaveBeenCalledWith('pty.data', { id: PTY_1, diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index b96781818e7..162553fe613 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -1025,10 +1025,8 @@ export class PtyHandler { if (heldBytes) { managed.startupIngress?.accept(heldBytes) } - const submit = process.platform === 'win32' ? '\r' : '\n' // Why: only the shell-ready wrapper arms bracketed-paste; other shells use raw submit so ESC[200~ markers aren't echoed. const payload = buildStartupCommandSubmission(startup.command, { - submit, bracketedPasteSafe: startup.waitForShellReady }) managed.startupCommand = undefined diff --git a/src/renderer/src/lib/ssh-background-startup-delivery.ts b/src/renderer/src/lib/ssh-background-startup-delivery.ts index 0e1c08c70ff..239bf6326a5 100644 --- a/src/renderer/src/lib/ssh-background-startup-delivery.ts +++ b/src/renderer/src/lib/ssh-background-startup-delivery.ts @@ -139,12 +139,10 @@ export function createSshBackgroundStartupDelivery( // PTYs; hidden automation tabs still submit the command themselves. // Why bracketed paste: multiline prompts are pasted literally only when we // synchronized on the Orca shell-ready marker — that is the bash/zsh overlay - // with bracketed-paste mode armed. Submit with CR since the relay drives a - // remote shell. + // with bracketed-paste mode armed. options.write( ptyId, buildStartupCommandSubmission(command, { - submit: '\r', bracketedPasteSafe: markerObserved }) ) diff --git a/src/shared/powershell-native-argument.test.ts b/src/shared/powershell-native-argument.test.ts index b8e1feb8aa2..33d396dc8b0 100644 --- a/src/shared/powershell-native-argument.test.ts +++ b/src/shared/powershell-native-argument.test.ts @@ -9,10 +9,27 @@ describe('PowerShell native argument quoting', () => { ) }) + it('keeps a value with a line break on one physical line', () => { + expect(quotePowerShellLiteral('line one\nline two')).toBe('"line one`nline two"') + expect(quotePowerShellLiteral('a\r\nb\rc')).toBe('"a`r`nb`rc"') + }) + + it('escapes the double-quoted specials in a multi-line value', () => { + expect( + quotePowerShellLiteral('$(Remove-Item x) `t "q" \u201Cl\u201D \u201Elow \'single\'\nend') + ).toBe('"`$(Remove-Item x) ``t `"q`" `\u201Cl`\u201D `\u201Elow \'single\'`nend"') + }) + + it('leaves single-line values on the single-quoted form', () => { + expect(quotePowerShellLiteral('a $b `c "d"')).toBe('\'a $b `c "d"\'') + }) + it('pre-escapes embedded quotes for Windows native argv parsing', () => { expect(quotePowerShellNativeArgument('eval "decoded"')).toBe(String.raw`'eval \"decoded\"'`) expect(quotePowerShellNativeArgument(String.raw`before\"after`)).toBe( String.raw`'before\\\"after'` ) + // Why: the argv pre-escape survives the multi-line form, so 5.1 still passes `\"` to argv. + expect(quotePowerShellNativeArgument('say "hi"\nbye')).toBe('"say \\`"hi\\`"`nbye"') }) }) diff --git a/src/shared/powershell-native-argument.ts b/src/shared/powershell-native-argument.ts index f357a8e8fb7..23e517505f0 100644 --- a/src/shared/powershell-native-argument.ts +++ b/src/shared/powershell-native-argument.ts @@ -1,8 +1,24 @@ export function quotePowerShellLiteral(value: string): string { + if (/[\r\n]/.test(value)) { + return quotePowerShellMultilineLiteral(value) + } // Why: PowerShell also ends single-quoted strings at typographic single quotes. return `'${value.replace(/['\u2018\u2019\u201A\u201B]/g, '$&$&')}'` } +/** + * Why one physical line: a raw line break typed into PowerShell submits the line, and Windows + * PowerShell 5.1 without PSReadLine then waits at `>>` for an empty line that never comes. + * Backtick escapes keep every byte, so `\r\n` stays `\r\n` exactly as the single-quoted form + * delivered it. + */ +function quotePowerShellMultilineLiteral(value: string): string { + const escaped = value.replace(/[`$"\u201C\u201D\u201E\r\n]/g, (char) => + char === '\r' ? '`r' : char === '\n' ? '`n' : `\`${char}` + ) + return `"${escaped}"` +} + export function quotePowerShellNativeArgument(value: string): string { // Why: Windows PowerShell 5.1 drops unescaped embedded quotes when it // constructs argv for native executables such as wsl.exe. diff --git a/src/shared/powershell-native-argument.windows.test.ts b/src/shared/powershell-native-argument.windows.test.ts new file mode 100644 index 00000000000..1bd81a27721 --- /dev/null +++ b/src/shared/powershell-native-argument.windows.test.ts @@ -0,0 +1,50 @@ +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { expect, it } from 'vitest' +import { runProcess } from './child-process/run-process' +import { quotePowerShellLiteral } from './powershell-native-argument' + +async function captureNativeArgv(root: string, quotedArg: string): Promise { + const capturePath = join(root, 'argv.json') + const scriptPath = join(root, 'capture.js') + await writeFile( + scriptPath, + `require('node:fs').writeFileSync(${JSON.stringify(capturePath)}, JSON.stringify(process.argv.slice(2)))` + ) + const result = await runProcess({ + program: 'powershell.exe', + args: [ + '-NoProfile', + '-NonInteractive', + '-Command', + `& ${quotePowerShellLiteral(process.execPath)} ${quotePowerShellLiteral(scriptPath)} ${quotedArg}` + ], + cwd: root + }) + expect(result.code, JSON.stringify(result)).toBe(0) + const argv: string[] = JSON.parse(await readFile(capturePath, 'utf8')) + return argv +} + +// Why native: argv marshalling, not the generated text, is what a launched agent actually receives. +it.skipIf(process.platform !== 'win32')( + 'passes a multi-line value to a native program exactly as the single-quoted form did', + async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-ps-multiline-')) + try { + const plain = 'line one\r\nline two $HOME `tick\nO\u2019Brien \u201Cdouble\u201D end' + expect(await captureNativeArgv(root, quotePowerShellLiteral(plain))).toEqual([plain]) + + // Why compare with the old raw single-quoted form: 5.1 drops embedded `"` from native argv + // either way, and the one-line form must not change what reaches the program. + const withQuotes = 'say "hi"\nbye' + const legacy = `'${withQuotes.replace(/'/g, "''")}'` + expect(await captureNativeArgv(root, quotePowerShellLiteral(withQuotes))).toEqual( + await captureNativeArgv(root, legacy) + ) + } finally { + await rm(root, { recursive: true, force: true }) + } + } +) diff --git a/src/shared/startup-command-submission.test.ts b/src/shared/startup-command-submission.test.ts index e25ee4c4105..6d85f4d14bc 100644 --- a/src/shared/startup-command-submission.test.ts +++ b/src/shared/startup-command-submission.test.ts @@ -5,46 +5,52 @@ import { } from './startup-command-submission' describe('buildStartupCommandSubmission', () => { - it('appends the submit byte to a single-line command unchanged', () => { - expect( - buildStartupCommandSubmission('claude', { submit: '\n', bracketedPasteSafe: true }) - ).toBe('claude\n') - expect( - buildStartupCommandSubmission('claude', { submit: '\r', bracketedPasteSafe: true }) - ).toBe('claude\r') + it('submits a single-line command with CR', () => { + expect(buildStartupCommandSubmission('claude', { bracketedPasteSafe: true })).toBe('claude\r') + expect(buildStartupCommandSubmission('claude', { bracketedPasteSafe: false })).toBe('claude\r') }) - it('preserves a caller-supplied trailing submit byte on single-line commands', () => { - expect( - buildStartupCommandSubmission('claude\n', { submit: '\r', bracketedPasteSafe: true }) - ).toBe('claude\n') - }) - - it('treats a CRLF-terminated single-line command as single-line', () => { - expect( - buildStartupCommandSubmission('claude\r\n', { submit: '\r', bracketedPasteSafe: true }) - ).toBe('claude\r\n') + it('replaces a caller-supplied trailing terminator with CR', () => { + for (const command of ['claude\n', 'claude\r', 'claude\r\n']) { + expect(buildStartupCommandSubmission(command, { bracketedPasteSafe: true })).toBe('claude\r') + } }) it('wraps a multiline command in bracketed paste with a trailing submit byte', () => { const command = "claude 'first\nsecond'" - expect(buildStartupCommandSubmission(command, { submit: '\n', bracketedPasteSafe: true })).toBe( - `\x1b[200~${command}\x1b[201~\n` + expect(buildStartupCommandSubmission(command, { bracketedPasteSafe: true })).toBe( + `\x1b[200~${command}\x1b[201~\r` ) }) it('strips a trailing submit byte before bracket-wrapping the multiline body', () => { const body = "claude 'first\nsecond'" - expect( - buildStartupCommandSubmission(`${body}\n`, { submit: '\r', bracketedPasteSafe: true }) - ).toBe(`\x1b[200~${body}\x1b[201~\r`) + expect(buildStartupCommandSubmission(`${body}\n`, { bracketedPasteSafe: true })).toBe( + `\x1b[200~${body}\x1b[201~\r` + ) }) it('keeps the raw path for multiline commands when bracketed paste is unsafe', () => { - const command = 'echo one\necho two' - expect( - buildStartupCommandSubmission(command, { submit: '\n', bracketedPasteSafe: false }) - ).toBe(`${command}\n`) + expect(buildStartupCommandSubmission('echo one\necho two', { bracketedPasteSafe: false })).toBe( + 'echo one\recho two\r' + ) + }) + + // Why: on the raw path every line break submits a line, so an interior LF is the same + // remappable Ctrl+J the trailing terminator was (#23250). + it('submits every line of a raw multiline command with CR', () => { + for (const command of ['echo one\necho two', 'echo one\r\necho two', 'echo one\recho two']) { + expect(buildStartupCommandSubmission(command, { bracketedPasteSafe: false })).toBe( + 'echo one\recho two\r' + ) + } + }) + + it('leaves interior line breaks intact inside a bracketed-paste payload', () => { + const command = "claude 'first\nsecond'" + expect(buildStartupCommandSubmission(command, { bracketedPasteSafe: true })).toBe( + `\x1b[200~${command}\x1b[201~\r` + ) }) }) diff --git a/src/shared/startup-command-submission.ts b/src/shared/startup-command-submission.ts index 33d84cd82b2..5b8b225614a 100644 --- a/src/shared/startup-command-submission.ts +++ b/src/shared/startup-command-submission.ts @@ -3,12 +3,12 @@ * submit a startup command (agent launch, setup script, etc.). * * Why bracketed paste: agent launch prompts are single-quoted, but their - * literal embedded newlines survive quoting. bash readline / zsh zle read every - * raw LF as accept-line (Enter), so the first newline inside a multiline prompt + * literal embedded newlines survive quoting. bash readline / zsh zle read a raw + * LF as accept-line by default, so the first newline inside a multiline prompt * submits an unterminated single-quoted command and drops the shell into PS2 * continuation — the prompt is executed piecemeal and mangled. Wrapping the * payload in bracketed-paste markers (ESC[200~ … ESC[201~) tells the line - * editor to insert the whole multiline text literally; only the trailing CR/LF + * editor to insert the whole multiline text literally; only the trailing CR * written after the end marker submits it. Single-line commands keep the proven * raw-write path unchanged so the fast path never regresses. */ @@ -16,12 +16,11 @@ // DEC 2004 bracketed-paste bracket sequences. const BRACKETED_PASTE_START = '\x1b[200~' const BRACKETED_PASTE_END = '\x1b[201~' +// Why CR on every platform: it is the byte the Enter key sends. A line editor reads +// the tty raw, so LF arrives as Ctrl+J — a key users rebind (e.g. vi-mode newline). +const STARTUP_COMMAND_SUBMIT = '\r' export type StartupCommandSubmissionOptions = { - /** Byte that submits the line: CR on Windows (PSReadLine/cmd.exe), LF on - * POSIX; SSH relays remote shells with CR. A caller-supplied trailing submit - * byte on `command` is preserved as-is. */ - submit: string /** Whether the target line editor has bracketed-paste mode active (Orca's * wrapped bash/zsh/fish). Only wrap multiline payloads when true — a shell * without bracketed paste would echo the ESC[200~ markers as literal garbage. */ @@ -52,15 +51,14 @@ export function isBracketedPasteSafeShell(args: { export function buildStartupCommandSubmission( command: string, - { submit, bracketedPasteSafe }: StartupCommandSubmissionOptions + { bracketedPasteSafe }: StartupCommandSubmissionOptions ): string { - // Strip a full CRLF (or lone CR/LF) terminator so a single-line command ending - // in \r\n isn't misread as multiline by the \r/\n body check below. - const trailingTerminator = /\r\n$|\r$|\n$/.exec(command)?.[0] ?? '' - const endsWithSubmit = trailingTerminator.length > 0 - const body = endsWithSubmit ? command.slice(0, -trailingTerminator.length) : command + // Why replace a caller's terminator: a trailing LF is Ctrl+J again, and CRLF submits twice. + const body = command.replace(/\r\n$|\r$|\n$/, '') if (bracketedPasteSafe && (body.includes('\n') || body.includes('\r'))) { - return `${BRACKETED_PASTE_START}${body}${BRACKETED_PASTE_END}${submit}` + return `${BRACKETED_PASTE_START}${body}${BRACKETED_PASTE_END}${STARTUP_COMMAND_SUBMIT}` } - return endsWithSubmit ? command : `${command}${submit}` + // Why normalise interior breaks too: without bracketed paste each line submits itself, and an + // interior LF is the same remappable Ctrl+J as a trailing one. + return `${body.replace(/\r?\n/g, STARTUP_COMMAND_SUBMIT)}${STARTUP_COMMAND_SUBMIT}` } diff --git a/src/shared/tui-agent-startup.test.ts b/src/shared/tui-agent-startup.test.ts index 31690889e9f..c9a62b747b6 100644 --- a/src/shared/tui-agent-startup.test.ts +++ b/src/shared/tui-agent-startup.test.ts @@ -85,6 +85,19 @@ describe('tui agent startup plans', () => { expect(plan?.launchCommand).toBe("claude 'fix Bob''s \"quoted\" branch'") }) + // Why: a typed line break submits early, and PowerShell 5.1 without PSReadLine then hangs at `>>`. + it('keeps a multi-line PowerShell launch on one physical line', () => { + const plan = buildAgentStartupPlan({ + agent: 'claude', + prompt: 'first line\nsecond $line', + cmdOverrides: {}, + platform: 'win32' + }) + + expect(plan?.launchCommand).not.toMatch(/[\r\n]/) + expect(plan?.launchCommand).toBe('claude "first line`nsecond `$line"') + }) + it('invokes fully quoted argv commands in PowerShell', () => { expect(buildShellCommandFromArgv(['codex', 'resume', 's1'], 'powershell')).toBe( "& 'codex' 'resume' 's1'"