mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 16:03:17 +00:00
fix(daemon): reconcile shell-ready barrier with the resolved fallback shell
The adapter computes shellReadySupported from the preferred shell before spawn. When the daemon falls back (e.g. to /bin/sh), the session would queue startup commands for the full 15s ready-marker timeout and wrap multiline prompts in bracketed-paste markers the fallback shell cannot parse. Expose the spawned shell on SubprocessHandle and downgrade the barrier and paste wrapping in TerminalHost when the actual shell cannot emit the marker. Also apply the Unix shell fallback after the relay env scrub so a scrubbed SHELL cannot drop the corrected value, and cover the resolve- before-launch-config ordering, the no-shell throw, and candidate dedup with tests. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -221,6 +221,82 @@ describe('createPtySubprocess', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('derives shell-ready launch config from the resolved fallback shell', () => {
|
||||
const proc = mockPtyProcess()
|
||||
spawnMock.mockReturnValue(proc)
|
||||
resolveUnixShellPathMock.mockReturnValue('/bin/sh')
|
||||
const platform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
const previousShell = process.env.SHELL
|
||||
const previousMarker = process.env.ORCA_SHELL_READY_MARKER
|
||||
const previousZdotdir = process.env.ZDOTDIR
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' })
|
||||
delete process.env.SHELL
|
||||
// Why: the test runner itself can execute inside an Orca-wrapped shell
|
||||
// whose exported wrapper vars would leak through the process.env spread.
|
||||
delete process.env.ORCA_SHELL_READY_MARKER
|
||||
delete process.env.ZDOTDIR
|
||||
|
||||
try {
|
||||
const handle = createPtySubprocess({
|
||||
sessionId: 'test',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
env: {},
|
||||
command: 'echo hi'
|
||||
})
|
||||
|
||||
expect(handle.shellPath).toBe('/bin/sh')
|
||||
const [shellPath, shellArgs, spawnOptions] = spawnMock.mock.calls[0]
|
||||
expect(shellPath).toBe('/bin/sh')
|
||||
expect(shellArgs).toEqual(['-l'])
|
||||
// A launch config derived from the missing preferred zsh would inject
|
||||
// ZDOTDIR and ORCA_SHELL_READY_MARKER; /bin/sh must spawn without them.
|
||||
expect(spawnOptions.env.ZDOTDIR).toBeUndefined()
|
||||
expect(spawnOptions.env.ORCA_SHELL_READY_MARKER).toBeUndefined()
|
||||
expect(spawnOptions.env.SHELL).toBe('/bin/sh')
|
||||
} finally {
|
||||
warn.mockRestore()
|
||||
if (platform) {
|
||||
Object.defineProperty(process, 'platform', platform)
|
||||
}
|
||||
if (previousShell === undefined) {
|
||||
delete process.env.SHELL
|
||||
} else {
|
||||
process.env.SHELL = previousShell
|
||||
}
|
||||
if (previousMarker === undefined) {
|
||||
delete process.env.ORCA_SHELL_READY_MARKER
|
||||
} else {
|
||||
process.env.ORCA_SHELL_READY_MARKER = previousMarker
|
||||
}
|
||||
if (previousZdotdir === undefined) {
|
||||
delete process.env.ZDOTDIR
|
||||
} else {
|
||||
process.env.ZDOTDIR = previousZdotdir
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
it('surfaces the no-executable-shell error before node-pty forks', () => {
|
||||
resolveUnixShellPathMock.mockImplementation(() => {
|
||||
throw new Error('No executable Unix shell found (tried: /bin/zsh, /bin/bash, /bin/sh)')
|
||||
})
|
||||
const platform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' })
|
||||
|
||||
try {
|
||||
expect(() => createPtySubprocess({ sessionId: 'test', cols: 80, rows: 24, env: {} })).toThrow(
|
||||
'No executable Unix shell found'
|
||||
)
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
if (platform) {
|
||||
Object.defineProperty(process, 'platform', platform)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
it('uses bundled ConPTY for native Windows daemon terminals', () => {
|
||||
const proc = mockPtyProcess()
|
||||
spawnMock.mockReturnValue(proc)
|
||||
|
||||
@@ -729,14 +729,6 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl
|
||||
addOrcaWslInteropEnv(env)
|
||||
}
|
||||
} else {
|
||||
const preferredShellPath = shellPath
|
||||
shellPath = resolveUnixShellPath(shellPath)
|
||||
if (shellPath !== preferredShellPath) {
|
||||
env.SHELL = shellPath
|
||||
console.warn(
|
||||
`[daemon/pty] Preferred shell "${preferredShellPath}" is unavailable, fell back to "${shellPath}"`
|
||||
)
|
||||
}
|
||||
// Why: relay-side launch modes can ask for host defaults to stay scrubbed
|
||||
// even after environment normalization above.
|
||||
for (const key of opts.envToDelete ?? []) {
|
||||
@@ -745,6 +737,17 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl
|
||||
if (opts.env?.TERM) {
|
||||
env.TERM = opts.env.TERM
|
||||
}
|
||||
// Why after the scrub: SHELL must reflect the shell that actually spawns,
|
||||
// matching LocalPtyProvider; and before the launch-config derivation below
|
||||
// so shell-ready wrappers target the resolved shell, not a missing one.
|
||||
const preferredShellPath = shellPath
|
||||
shellPath = resolveUnixShellPath(shellPath)
|
||||
if (shellPath !== preferredShellPath) {
|
||||
env.SHELL = shellPath
|
||||
console.warn(
|
||||
`[daemon/pty] Preferred shell "${preferredShellPath}" is unavailable, fell back to "${shellPath}"`
|
||||
)
|
||||
}
|
||||
// Why: OpenCode/Codex path restoration and OMP's typed-command status
|
||||
// wrapper need shell-ready code after user startup files run.
|
||||
let shellLaunch: ReturnType<typeof getShellReadyLaunchConfig> | null = null
|
||||
@@ -1010,6 +1013,7 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl
|
||||
|
||||
return {
|
||||
pid: proc.pid,
|
||||
shellPath,
|
||||
...(startupCommandDeliveredInShellArgs ? { startupCommandDeliveredInShellArgs: true } : {}),
|
||||
getForegroundProcess: () => {
|
||||
// Why: node-pty's `.process` getter reports the PTY's live foreground
|
||||
|
||||
@@ -48,6 +48,10 @@ export type SubprocessHandle = {
|
||||
/** True when shell launch args already delivered the startup command, so the
|
||||
* terminal host must skip its stdin fallback write. */
|
||||
startupCommandDeliveredInShellArgs?: boolean
|
||||
/** Shell the subprocess actually spawned, after Unix/Windows fallbacks. The
|
||||
* host reconciles the caller's shell-ready assumption against it so a
|
||||
* fallback shell without a ready marker never gates startup commands. */
|
||||
shellPath?: string
|
||||
write(data: string): void
|
||||
resize(cols: number, rows: number): void
|
||||
/** Stop reading the PTY fd (node-pty pause()) so the kernel/ConPTY buffer
|
||||
|
||||
@@ -348,13 +348,16 @@ export function resolvePtyShellPath(env: Record<string, string>): string {
|
||||
return env.SHELL || process.env.SHELL || '/bin/zsh'
|
||||
}
|
||||
|
||||
export function shellPathSupportsPtyStartupBarrier(shellPath: string): boolean {
|
||||
const shellName = pathWin32.basename(basename(shellPath)).toLowerCase()
|
||||
return shellName === 'zsh' || shellName === 'bash'
|
||||
}
|
||||
|
||||
export function supportsPtyStartupBarrier(env: Record<string, string>): boolean {
|
||||
if (process.platform === 'win32') {
|
||||
return false
|
||||
}
|
||||
const resolvedShell = resolvePtyShellPath(env)
|
||||
const shellName = pathWin32.basename(basename(resolvedShell)).toLowerCase()
|
||||
return shellName === 'zsh' || shellName === 'bash'
|
||||
return shellPathSupportsPtyStartupBarrier(resolvePtyShellPath(env))
|
||||
}
|
||||
|
||||
type ShellLaunchConfig = {
|
||||
|
||||
@@ -3,7 +3,7 @@ import { Session, type SubprocessHandle } from './session'
|
||||
import { TerminalHost } from './terminal-host'
|
||||
|
||||
function createMockSubprocess(
|
||||
options: { startupCommandDeliveredInShellArgs?: boolean } = {}
|
||||
options: { startupCommandDeliveredInShellArgs?: boolean; shellPath?: string } = {}
|
||||
): SubprocessHandle {
|
||||
let onDataCb: ((data: string) => void) | null = null
|
||||
let onExitCb: ((code: number) => void) | null = null
|
||||
@@ -12,6 +12,7 @@ function createMockSubprocess(
|
||||
...(options.startupCommandDeliveredInShellArgs
|
||||
? { startupCommandDeliveredInShellArgs: true }
|
||||
: {}),
|
||||
...(options.shellPath ? { shellPath: options.shellPath } : {}),
|
||||
getForegroundProcess: vi.fn(() => null),
|
||||
write: vi.fn(),
|
||||
resize: vi.fn(),
|
||||
@@ -193,6 +194,88 @@ describe('TerminalHost', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('delivers startup commands immediately when the spawned shell cannot emit the ready marker', async () => {
|
||||
spawnFn = vi.fn(() => {
|
||||
const sub = createMockSubprocess({ shellPath: '/bin/sh' }) as ReturnType<
|
||||
typeof createMockSubprocess
|
||||
> & {
|
||||
_onDataCb: ((data: string) => void) | null
|
||||
_onExitCb: ((code: number) => void) | null
|
||||
}
|
||||
lastSubprocess = sub
|
||||
return sub
|
||||
})
|
||||
host.dispose()
|
||||
host = new TerminalHost({ spawnSubprocess: spawnFn as MockSpawnFn })
|
||||
|
||||
await host.createOrAttach({
|
||||
sessionId: 'session-1',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
command: 'echo hello',
|
||||
shellReadySupported: true,
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
|
||||
expect(lastSubprocess.write).toHaveBeenCalledWith(
|
||||
process.platform === 'win32' ? 'echo hello\r' : 'echo hello\n'
|
||||
)
|
||||
})
|
||||
|
||||
it('does not bracketed-paste-wrap multiline commands for a fallback shell without paste mode', async () => {
|
||||
spawnFn = vi.fn(() => {
|
||||
const sub = createMockSubprocess({ shellPath: '/bin/sh' }) as ReturnType<
|
||||
typeof createMockSubprocess
|
||||
> & {
|
||||
_onDataCb: ((data: string) => void) | null
|
||||
_onExitCb: ((code: number) => void) | null
|
||||
}
|
||||
lastSubprocess = sub
|
||||
return sub
|
||||
})
|
||||
host.dispose()
|
||||
host = new TerminalHost({ spawnSubprocess: spawnFn as MockSpawnFn })
|
||||
|
||||
await host.createOrAttach({
|
||||
sessionId: 'session-1',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
command: 'claude "line one\nline two"',
|
||||
shellReadySupported: true,
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
|
||||
const written = (lastSubprocess.write as ReturnType<typeof vi.fn>).mock.calls[0]?.[0]
|
||||
expect(written).not.toContain('\x1b[200~')
|
||||
expect(written).toContain('line one\nline two')
|
||||
})
|
||||
|
||||
it('keeps the shell-ready barrier when the spawned shell supports the marker', async () => {
|
||||
spawnFn = vi.fn(() => {
|
||||
const sub = createMockSubprocess({ shellPath: '/bin/bash' }) as ReturnType<
|
||||
typeof createMockSubprocess
|
||||
> & {
|
||||
_onDataCb: ((data: string) => void) | null
|
||||
_onExitCb: ((code: number) => void) | null
|
||||
}
|
||||
lastSubprocess = sub
|
||||
return sub
|
||||
})
|
||||
host.dispose()
|
||||
host = new TerminalHost({ spawnSubprocess: spawnFn as MockSpawnFn })
|
||||
|
||||
await host.createOrAttach({
|
||||
sessionId: 'session-1',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
command: 'echo hello',
|
||||
shellReadySupported: true,
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
|
||||
expect(lastSubprocess.write).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not write startup commands already embedded in shell args', async () => {
|
||||
spawnFn = vi.fn(() => {
|
||||
const sub = createMockSubprocess({
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { Session, type SubprocessHandle } from './session'
|
||||
import { normalizePtySize } from './daemon-pty-size'
|
||||
import { shellPathSupportsPtyStartupBarrier } from './shell-ready'
|
||||
import { resolveProcessCwd } from '../providers/process-cwd'
|
||||
import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery'
|
||||
import { buildStartupCommandSubmission } from '../../shared/startup-command-submission'
|
||||
@@ -104,6 +105,16 @@ export class TerminalHost {
|
||||
terminalWindowsPowerShellImplementation: opts.terminalWindowsPowerShellImplementation
|
||||
})
|
||||
|
||||
// Why: the caller computed shellReadySupported from the preferred shell,
|
||||
// before spawn. A Unix fallback (e.g. /bin/sh) never emits the ready
|
||||
// marker, so keeping the stale flag would queue startup commands until the
|
||||
// shell-ready timeout and bracketed-paste-wrap them for a line editor
|
||||
// without paste mode.
|
||||
const shellReadySupported =
|
||||
(opts.shellReadySupported ?? false) &&
|
||||
(subprocess.shellPath === undefined ||
|
||||
shellPathSupportsPtyStartupBarrier(subprocess.shellPath))
|
||||
|
||||
const session = new Session({
|
||||
sessionId: opts.sessionId,
|
||||
cols: size.cols,
|
||||
@@ -111,7 +122,7 @@ export class TerminalHost {
|
||||
terminalHandle: opts.env?.ORCA_TERMINAL_HANDLE,
|
||||
launchAgent: opts.launchAgent,
|
||||
subprocess,
|
||||
shellReadySupported: opts.shellReadySupported ?? false,
|
||||
shellReadySupported,
|
||||
historySeed: opts.historySeed,
|
||||
// Why: reap the dead session (dispose emulator + drop from the map) the
|
||||
// moment its subprocess exits, instead of retaining it for the daemon's
|
||||
@@ -143,7 +154,7 @@ export class TerminalHost {
|
||||
session.write(
|
||||
buildStartupCommandSubmission(opts.command, {
|
||||
submit,
|
||||
bracketedPasteSafe: opts.shellReadySupported ?? false
|
||||
bracketedPasteSafe: shellReadySupported
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
@@ -138,6 +138,14 @@ describe('resolveUnixShellPath', () => {
|
||||
'No executable Unix shell found (tried: /missing/zsh, /bin/zsh, /bin/bash, /bin/sh)'
|
||||
)
|
||||
})
|
||||
|
||||
it('dedupes a preferred shell that is already a fallback candidate', () => {
|
||||
existsSyncMock.mockImplementation((path) => path === '/bin/sh')
|
||||
accessSyncMock.mockReturnValue(undefined)
|
||||
|
||||
expect(resolveUnixShellPath('/bin/sh')).toBe('/bin/sh')
|
||||
expect(existsSyncMock.mock.calls.map(([path]) => path)).toEqual(['/bin/sh'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('spawnShellWithFallback macOS TCC login wrapping', () => {
|
||||
|
||||
Reference in New Issue
Block a user