diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 0789f1958..05a73229d 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -183,8 +183,11 @@ jobs: with: persist-credentials: false - - name: Install zsh - run: sudo apt-get update && sudo apt-get install -y zsh + # Why fish: shell-ready.test.ts gates its live fish test on the binary being + # present, so without this the fish barrier is only covered by config-shape + # assertions and never actually exercised. + - name: Install zsh and fish + run: sudo apt-get update && sudo apt-get install -y zsh fish - uses: ./.github/actions/install-node-dependencies with: diff --git a/config/scripts/pr-workflow-parallelism.test.mjs b/config/scripts/pr-workflow-parallelism.test.mjs index fe86ab7e7..5b3b66dea 100644 --- a/config/scripts/pr-workflow-parallelism.test.mjs +++ b/config/scripts/pr-workflow-parallelism.test.mjs @@ -55,18 +55,35 @@ describe('PR workflow parallelism', () => { } }) - it('runs real-zsh coverage once outside the general shards', () => { + it('runs real-shell coverage once outside the general shards', () => { const shellStep = workflow.jobs.shell_contracts.steps.find( (step) => step.name === 'Test real shell contracts' ) const shellInstall = workflow.jobs.shell_contracts.steps.find( (step) => step.uses === './.github/actions/install-node-dependencies' ) + // Why parsed rather than substring-matched: the step name changes as shells are + // added, and `includes('fish')` would also match a comment or a longer package. + const aptPackages = (step) => + (step.run?.match(/apt-get install[^\n]*/)?.[0] ?? '') + .split(/\s+/) + .filter((token) => !['apt-get', 'install', 'sudo', ''].includes(token)) + .filter((token) => !token.startsWith('-')) + const jobsInstallingPackages = Object.entries(workflow.jobs) + .filter(([, job]) => (job.steps ?? []).some((step) => aptPackages(step).length > 0)) + .map(([name]) => name) - expect(workflow.jobs.test.steps.some((step) => step.name === 'Install zsh')).toBe(false) - expect(workflow.jobs.shell_contracts.steps.some((step) => step.name === 'Install zsh')).toBe( - true - ) + expect(shellStep).toBeDefined() + expect(shellInstall).toBeDefined() + // Why the whole workflow, not just the general shards: any other lane installing + // these shells would silently start running the real-shell tests twice. + expect(jobsInstallingPackages).toEqual(['shell_contracts']) + // Why each shell is asserted: the live tests skip themselves when the binary is + // missing, so a dropped package silently empties this lane instead of failing it. + const shellPackages = workflow.jobs.shell_contracts.steps.flatMap(aptPackages) + for (const shell of ['zsh', 'fish']) { + expect(shellPackages).toContain(shell) + } expect(shellInstall.with['native-runtime']).toBe('node') for (const testFile of nativeShellContractFiles) { expect(shellStep.run).toContain(testFile) diff --git a/src/main/daemon/headless-emulator.ts b/src/main/daemon/headless-emulator.ts index ca8bb6c33..195b97a30 100644 --- a/src/main/daemon/headless-emulator.ts +++ b/src/main/daemon/headless-emulator.ts @@ -18,6 +18,7 @@ import { installTerminalViewAttributeResponder, type TerminalViewAttributeResponder } from './terminal-view-attribute-responder' +import { installDeviceAttributesResponder } from './startup-device-attributes-responder' import type { TerminalSnapshot, TerminalModes } from './types' import type { TerminalOscLinkRange } from '../../shared/terminal-osc-link-ranges' @@ -100,23 +101,26 @@ export class HeadlessEmulator { } } - /** ConPTY 1.22+ blocks at spawn awaiting a DA1 reply; answers `CSI ?61;4c` and consumes the query so xterm's default `?1;2c` can't double-reply. */ + /** ConPTY 1.22+ blocks at spawn awaiting a DA1 reply. See startup-device-attributes-responder. */ installConptyPrimaryDeviceAttributesOverride(): void { // Why idempotent: installed at creation and again at spawn-mark time (which can land later), so it's never stacked. if (this.conptyDa1OverrideInstalled) { return } this.conptyDa1OverrideInstalled = true - this.terminal.parser.registerCsiHandler({ final: 'c' }, (params) => { - const isPrimaryQuery = params.length === 0 || (params.length === 1 && params[0] === 0) - if (!isPrimaryQuery) { - return false - } - this.emitQueryReply(CONPTY_DA1_RESPONSE) - return true + installDeviceAttributesResponder({ + parser: this.terminal.parser, + response: CONPTY_DA1_RESPONSE, + reply: (data) => this.emitQueryReply(data) }) } + /** Why exposed: responder modules install handlers here (see the view-attribute and + * device-attributes responders); the caller owns disposal. */ + get responderParser(): Terminal['parser'] { + return this.terminal.parser + } + /** Headless core has no theme service, so OSC 4/10/11/12 and DSR ?996n answer from the renderer's pushed attributes; daemon Session must never call this. */ installViewAttributeResponder(getBaseAttributes: () => TerminalViewAttributes | null): void { if (this.viewAttributeResponder) { diff --git a/src/main/daemon/session.test.ts b/src/main/daemon/session.test.ts index 34496fb0e..bc90bf5d9 100644 --- a/src/main/daemon/session.test.ts +++ b/src/main/daemon/session.test.ts @@ -334,6 +334,23 @@ describe('Session', () => { }) describe('shell readiness gating', () => { + // Why: the renderer's DA1 reply would be queued here, and a shell that withholds its + // first prompt until DA1 is answered never emits the marker that would release it. + it('answers DA1 once without forwarding it to a renderer', () => { + createSession({ shellReadySupported: true }) + const onData = vi.fn((d: string) => d === '\x1b[0c' && session.write('\x1b[?1;2c')) + session.attachClient({ onData, onExit: () => {} }) + + subprocess.simulateData('\x1b[0c') + + expect(onData).toHaveBeenCalledWith('', '\x1b[0c'.length, true, '\x1b[0c'.length) + expect(session.takePendingOutput(false)?.records).toEqual([]) + expect(session.getSnapshot()?.outputSequence).toBe('\x1b[0c'.length) + subprocess.simulateData('\x1b]777;orca-shell-ready\x07prompt') + vi.advanceTimersByTime(30) + expect(subprocess.written).toEqual(['\x1b[?1;2c']) + }) + // Why: regression guard for "claude claude" double-echo. The marker fires // from precmd before readline switches the PTY into raw mode; flushing // then lets the kernel re-echo the command under the prompt. Detailed diff --git a/src/main/daemon/session.ts b/src/main/daemon/session.ts index aba49d91c..f24a6a7fc 100644 --- a/src/main/daemon/session.ts +++ b/src/main/daemon/session.ts @@ -1,5 +1,10 @@ /* oxlint-disable max-lines */ import { HeadlessEmulator } from './headless-emulator' +import { + installDeviceAttributesResponder, + STARTUP_DA1_RESPONSE, + StartupDeviceAttributesQueryFilter +} from './startup-device-attributes-responder' import { isValidPtySize, normalizePtySize } from './daemon-pty-size' import { PostReadyFlushGate } from './post-ready-flush-gate' import { @@ -117,6 +122,8 @@ export class Session { private readonly onSessionExit?: (code: number) => void private attachedClients: AttachedClient[] = [] private preReadyStdinQueue: string[] = [] + private releaseStartupDeviceAttributesResponder: (() => void) | null = null + private startupDeviceAttributesQueryFilter: StartupDeviceAttributesQueryFilter | null = null private shellReadyScanState: ShellReadyScanState | null = null private shellReadyTimer: ReturnType | null = null private killTimer: ReturnType | null = null @@ -149,6 +156,8 @@ export class Session { wslDistro: opts.wslDistro // No onData: the daemon emulator must never reply to query sequences — the renderer's xterm is // the authoritative responder and a daemon reply would race ahead and clobber it. See HeadlessEmulator. + // The one exception is DA1 while the shell-ready barrier holds (below): the renderer's reply + // would be queued behind the marker it is needed to produce, so it cannot be authoritative there. }) // Why: seed recovery must precede listener registration; shells can emit their prompt synchronously once onData subscribes. // Why the every() short-circuit is safe: writeSync only fails emulator-wide (disposed / no sync write API), so later @@ -161,6 +170,15 @@ export class Session { if (opts.shellReadySupported) { this._shellState = 'pending' this.shellReadyScanState = createShellReadyScanState() + // Why: `write` queues everything until the ready marker, including the renderer's DA1 + // reply — and a shell that withholds its first prompt until DA1 is answered (fish) then + // never emits the marker that would release it. Answer from the daemon, past the queue. + this.releaseStartupDeviceAttributesResponder = installDeviceAttributesResponder({ + parser: this.emulator.responderParser, + response: STARTUP_DA1_RESPONSE, + reply: (data) => this.subprocess.write(data) + }) + this.startupDeviceAttributesQueryFilter = new StartupDeviceAttributesQueryFilter() this.shellReadyTimer = setTimeout(() => { this.onShellReadyTimeout() }, opts.shellReadyTimeoutMs ?? SHELL_READY_TIMEOUT_MS) @@ -523,6 +541,7 @@ export class Session { // Why: `wasTerminating` below must be read BEFORE the `_state = 'exited'` flip — it guards the // "dispose while kill() in flight" case and the invariant needs the pre-flip `_state`; do NOT move it down. + this.releaseStartupDeviceAttributes() this.releaseHeldShellReadyBytes() this.startupIngress.drainAndClose() const wasTerminating = this._isTerminating && this._state !== 'exited' @@ -629,26 +648,34 @@ export class Session { return } + let releaseStartupDeviceAttributes = false if (this._shellState === 'pending' && this.shellReadyScanState) { const scanned = scanForShellReady(this.shellReadyScanState, data) data = scanned.output if (scanned.matched) { this.transitionToReady(scanned.postMarkerBytesObserved) + releaseStartupDeviceAttributes = true } } else { this.postReadyFlushGate.notifyData() } this.startupIngress.accept(data) + if (releaseStartupDeviceAttributes) { + this.releaseStartupDeviceAttributes() + } } private emitSubprocessOutput(emission: PtyIngressEmission): void { - const { data } = emission + let { data } = emission const rawLength = emission.rawEndSeq - emission.rawStartSeq // Why: absolute raw count (daemon stream thinning can drop bytes) lets a snapshot cover the gaps while the renderer dedups the tail. this.outputSequence += rawLength if (data.length > 0) { this.emulator.write(data) + data = this.startupDeviceAttributesQueryFilter?.accept(data) ?? data + } + if (data.length > 0) { this.recordPendingOutput({ kind: 'output', data }) } @@ -668,6 +695,7 @@ export class Session { return } + this.releaseStartupDeviceAttributes() this.releaseHeldShellReadyBytes() this.startupIngress.drainAndClose() this._exitCode = code @@ -713,6 +741,21 @@ export class Session { return this.startupIngress.closeQueryAuthority() } + /** Hands DA1 back to the renderer once the barrier is done, however it ended. */ + private releaseStartupDeviceAttributes(): void { + this.releaseStartupDeviceAttributesResponder?.() + this.releaseStartupDeviceAttributesResponder = null + const pending = this.startupDeviceAttributesQueryFilter?.release() ?? '' + this.startupDeviceAttributesQueryFilter = null + if (pending.length === 0) { + return + } + this.recordPendingOutput({ kind: 'output', data: pending }) + for (const client of this.attachedClients) { + client.onData(pending, 0, true, this.outputSequence) + } + } + private transitionToReady(postMarkerBytesObserved = false): void { this._shellState = 'ready' this.shellReadyScanState = null @@ -732,6 +775,7 @@ export class Session { return } this._shellState = 'timed_out' + this.releaseStartupDeviceAttributes() this.releaseHeldShellReadyBytes() this.flushPreReadyQueue() } diff --git a/src/main/daemon/shell-ready.test.ts b/src/main/daemon/shell-ready.test.ts index 3b62b6d3f..42e6ccc15 100644 --- a/src/main/daemon/shell-ready.test.ts +++ b/src/main/daemon/shell-ready.test.ts @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { spawnSync } from 'node:child_process' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import type * as ShellReadyModule from './shell-ready' import { getZshShellReadyMarkerRegistrationBlock } from '../shell-templates' @@ -16,9 +16,25 @@ const hasBash = process.platform !== 'win32' && spawnSync('bash', ['--version']) const itWithBash = hasBash ? it : it.skip const hasZsh = process.platform !== 'win32' && spawnSync('zsh', ['--version']).status === 0 const itWithZsh = hasZsh ? it : it.skip +const hasFish = process.platform !== 'win32' && spawnSync('fish', ['--version']).status === 0 +const itWithFish = hasFish ? it : it.skip const SHELL_READY_MARKER_OUTPUT = '\x1b]777;orca-shell-ready\x07' +/** Minimal xterm.js-shaped answers to the capability queries fish emits at startup + * and again around every prompt. */ +const TERMINAL_QUERY_REPLIES: readonly (readonly [string, string])[] = [ + ['\x1b[0c', '\x1b[?6c'], // primary device attributes + ['\x1b[?u', '\x1b[?0u'], // kitty keyboard flags + ['\x1b[6n', '\x1b[1;1R'], // cursor position report + ['\x1b]11;?', '\x1b]11;rgb:0000/0000/0000\x1b\\'], // background colour + ['\x1bP+q', '\x1bP0+r\x1b\\'] // XTGETTCAP (unsupported) +] + +/** Derived, not hardcoded: a shorter carry than the longest query would silently + * stop matching sequences split across two PTY chunks. */ +const QUERY_CARRY_LEN = Math.max(...TERMINAL_QUERY_REPLIES.map(([query]) => query.length)) + // Why: the shell-ready marker fires from zle-line-init only on a real TTY, so spawn through node-pty not spawnSync. async function runInteractiveZshLogin(args: { tempHome: string @@ -209,6 +225,140 @@ describePosix('daemon shell-ready launch config', () => { expect(existsSync(join(userDataPath, 'shell-ready', 'zsh', '.zshenv'))).toBe(true) }) + it('extends the startup barrier to fish so launch commands queue until the prompt', async () => { + const { shellPathSupportsPtyStartupBarrier, supportsPtyStartupBarrier } = + await importFreshShellReady() + + expect(shellPathSupportsPtyStartupBarrier('/opt/homebrew/bin/fish')).toBe(true) + expect(supportsPtyStartupBarrier({ SHELL: '/usr/local/bin/fish' })).toBe(true) + // Why: unwrapped shells must stay off the barrier or their first command queues forever. + expect(shellPathSupportsPtyStartupBarrier('/usr/bin/tcsh')).toBe(false) + }) + + it('wraps fish launches with a fish_prompt shell-ready marker init command', async () => { + const { getShellReadyLaunchConfig } = await importFreshShellReady() + + const config = getShellReadyLaunchConfig('/opt/homebrew/bin/fish') + + expect(config.supportsReadyMarker).toBe(true) + expect(config.env).toEqual({ ORCA_SHELL_READY_MARKER: '1' }) + expect(config.args?.slice(0, 2)).toEqual(['-l', '-C']) + const init = config.args?.[2] ?? '' + expect(init).toContain('--on-event fish_prompt') + // Why `builtin`: a user-defined printf function would swallow the marker and + // stall every launch on the ready timeout. + expect(init).toContain('builtin printf "\\033]777;orca-shell-ready\\007"') + // Why: the marker must fire once; a repeating marker would corrupt later output scans. + expect(init).toContain('functions -e __orca_shell_ready_marker') + }) + + it('keeps attribution-only fish spawns unwrapped', async () => { + const { getAttributionShellLaunchConfig } = await importFreshShellReady() + + const config = getAttributionShellLaunchConfig('/opt/homebrew/bin/fish') + + expect(config).toEqual({ args: null, env: {}, supportsReadyMarker: false }) + }) + + itWithFish( + 'emits the marker at the first real fish prompt and executes a post-marker command', + async () => { + const { getShellReadyLaunchConfig } = await importFreshShellReady() + const config = getShellReadyLaunchConfig('fish') + const tempHome = mkdtempSync(join(tmpdir(), 'fish-shell-ready-')) + const sentinel = join(tempHome, 'launched') + const erased = join(tempHome, 'marker-erased') + const stillRegistered = join(tempHome, 'marker-still-registered') + try { + mkdirSync(join(tempHome, '.config', 'fish'), { recursive: true }) + // Why: mimic a slow prompt integration (Starship) — init work before the first prompt. + writeFileSync( + join(tempHome, '.config', 'fish', 'config.fish'), + 'command sleep 0.2\nfunction fish_prompt\n printf "> "\nend\n' + ) + const pty = await import('node-pty') + const proc = pty.spawn('fish', config.args ?? [], { + name: 'xterm-256color', + cols: 80, + rows: 24, + cwd: tempHome, + env: { + PATH: process.env.PATH ?? '/usr/bin:/bin', + HOME: tempHome, + TERM: 'xterm-256color', + ...config.env + } + }) + let output = '' + let commandWritten = false + let erasureProbeWritten = false + let queryCarry = '' + let settle = (): void => {} + const done = new Promise((resolve) => { + settle = resolve + }) + const deadline = setTimeout(settle, 10_000) + // Why: settling on the first sentinel observes only one post-marker prompt, + // so a marker that never erased itself still looks single. Drive a second + // command and settle on its result, which also probes the erase directly. + const sentinelPoll = setInterval(() => { + if (commandWritten && !erasureProbeWritten && existsSync(sentinel)) { + erasureProbeWritten = true + proc.write( + `functions -q __orca_shell_ready_marker; and touch ${stillRegistered}; or touch ${erased}\n` + ) + return + } + if (erasureProbeWritten && (existsSync(erased) || existsSync(stillRegistered))) { + settle() + } + }, 50) + proc.onData((chunk) => { + output += chunk + // Why: fish stalls its first prompt 10s waiting on these and re-queries + // each prompt, so answer every occurrence — an unanswered query makes + // fish swallow the post-marker command as its reply. + const carriedLength = queryCarry.length + const scan = queryCarry + chunk + queryCarry = scan.slice(-QUERY_CARRY_LEN) + for (const [query, reply] of TERMINAL_QUERY_REPLIES) { + for ( + let at = scan.indexOf(query); + at !== -1; + at = scan.indexOf(query, at + query.length) + ) { + // Why: a query wholly inside the carry was answered on the previous + // chunk; replying again would land in fish's stdin as typed input. + if (at + query.length > carriedLength) { + proc.write(reply) + } + } + } + if (!commandWritten && output.includes(SHELL_READY_MARKER_OUTPUT)) { + commandWritten = true + // Why: mirror PostReadyFlushGate — flush shortly after the post-marker prompt draw. + setTimeout(() => proc.write(`touch ${sentinel}\n`), 50) + } + }) + await done + clearTimeout(deadline) + clearInterval(sentinelPoll) + proc.kill() + + expect(output).toContain(SHELL_READY_MARKER_OUTPUT) + expect(output.split(SHELL_READY_MARKER_OUTPUT)).toHaveLength(2) + expect(existsSync(sentinel)).toBe(true) + // Why: asserts the erase directly rather than inferring it from the marker + // count, which only holds once enough prompts have been drawn to expose it. + expect(existsSync(erased)).toBe(true) + expect(existsSync(stillRegistered)).toBe(false) + } finally { + rmSync(tempHome, { recursive: true, force: true }) + } + }, + 15_000 + ) + it('falls back to HOME for ORCA_ORIG_ZDOTDIR when inherited ZDOTDIR points at a wrapper dir', async () => { // Why: an Orca-PTY parent has ZDOTDIR=.../shell-ready/zsh; propagating it makes the wrapper source itself (recursion loop). const previousZdotdir = process.env.ZDOTDIR diff --git a/src/main/daemon/shell-ready.ts b/src/main/daemon/shell-ready.ts index d602f5f7f..38a26be8b 100644 --- a/src/main/daemon/shell-ready.ts +++ b/src/main/daemon/shell-ready.ts @@ -12,6 +12,7 @@ import { } from '../powershell-osc133-bootstrap' import { getPosixOmpShellWrapper } from '../pty/omp-shell-wrapper' import { + getFishShellReadyInitCommand, getZshEnvTemplate, getZshFinalZdotdirRestoreBlock, getZshShellReadyMarkerRegistrationBlock, @@ -350,7 +351,9 @@ export function resolvePtyShellPath(env: Record): string { export function shellPathSupportsPtyStartupBarrier(shellPath: string): boolean { const shellName = pathWin32.basename(basename(shellPath)).toLowerCase() - return shellName === 'zsh' || shellName === 'bash' + // Why fish: markerless, its startup command is written before fish's reader owns + // the PTY and the launch is lost under slow prompts like Starship (STA-3417). + return shellName === 'zsh' || shellName === 'bash' || shellName === 'fish' } export function supportsPtyStartupBarrier(env: Record): boolean { @@ -412,6 +415,15 @@ function getWrappedShellLaunchConfig( } } + // Why: mirrors local-pty-shell-ready.ts; attribution-only fish stays unwrapped. + if (shellName === 'fish' && options.emitReadyMarker) { + return { + args: ['-l', '-C', getFishShellReadyInitCommand(SHELL_READY_MARKER)], + env: { ORCA_SHELL_READY_MARKER: '1' }, + supportsReadyMarker: true + } + } + return { args: null, env: {}, diff --git a/src/main/daemon/startup-device-attributes-responder.test.ts b/src/main/daemon/startup-device-attributes-responder.test.ts new file mode 100644 index 000000000..f0b907592 --- /dev/null +++ b/src/main/daemon/startup-device-attributes-responder.test.ts @@ -0,0 +1,138 @@ +import { describe, expect, it } from 'vitest' +import { + installDeviceAttributesResponder, + STARTUP_DA1_RESPONSE, + StartupDeviceAttributesQueryFilter +} from './startup-device-attributes-responder' + +type CsiHandler = (params: number[]) => boolean + +function createFakeParser() { + const handlers: CsiHandler[] = [] + return { + registered: handlers, + parser: { + registerCsiHandler(_id: { final: string }, handler: CsiHandler) { + handlers.push(handler) + return { + dispose() { + const at = handlers.indexOf(handler) + if (at !== -1) { + handlers.splice(at, 1) + } + } + } + } + } as never + } +} + +describe('startup device attributes responder', () => { + it('answers a bare DA1 query and consumes it so the renderer cannot double-reply', () => { + const { parser, registered } = createFakeParser() + const replies: string[] = [] + + installDeviceAttributesResponder({ + parser, + response: STARTUP_DA1_RESPONSE, + reply: (d) => replies.push(d) + }) + + expect(registered[0]?.([])).toBe(true) + expect(replies).toEqual([STARTUP_DA1_RESPONSE]) + }) + + it('answers the explicit `CSI 0 c` form', () => { + const { parser, registered } = createFakeParser() + const replies: string[] = [] + + installDeviceAttributesResponder({ + parser, + response: STARTUP_DA1_RESPONSE, + reply: (d) => replies.push(d) + }) + + expect(registered[0]?.([0])).toBe(true) + expect(replies).toEqual([STARTUP_DA1_RESPONSE]) + }) + + it('answers every occurrence, because shells re-query around each prompt', () => { + const { parser, registered } = createFakeParser() + const replies: string[] = [] + + installDeviceAttributesResponder({ + parser, + response: STARTUP_DA1_RESPONSE, + reply: (d) => replies.push(d) + }) + + registered[0]?.([]) + registered[0]?.([]) + + expect(replies).toEqual([STARTUP_DA1_RESPONSE, STARTUP_DA1_RESPONSE]) + }) + + it('declines non-primary variants so they fall through to the renderer', () => { + const { parser, registered } = createFakeParser() + const replies: string[] = [] + + installDeviceAttributesResponder({ + parser, + response: STARTUP_DA1_RESPONSE, + reply: (d) => replies.push(d) + }) + + // Why: a non-zero parameter is a secondary/tertiary DA request, not DA1. + expect(registered[0]?.([1])).toBe(false) + expect(registered[0]?.([0, 1])).toBe(false) + expect(replies).toEqual([]) + }) + + it('stops answering once disposed, handing the query back to the renderer', () => { + const { parser, registered } = createFakeParser() + const replies: string[] = [] + + const release = installDeviceAttributesResponder({ + parser, + response: STARTUP_DA1_RESPONSE, + reply: (d) => replies.push(d) + }) + release() + + expect(registered).toHaveLength(0) + expect(replies).toEqual([]) + }) + + it('reports the same primary attributes the renderer would, so consuming the query is transparent', () => { + // Why pinned: the renderer's xterm answers `?1;2c` for xterm-* TERMs. Diverging here + // would silently change the capabilities a TUI sees on barrier-gated panes only. + expect(STARTUP_DA1_RESPONSE).toBe('\x1b[?1;2c') + }) +}) + +describe('startup device attributes query filter', () => { + it('removes both DA1 forms while preserving surrounding output', () => { + const filter = new StartupDeviceAttributesQueryFilter() + + expect(filter.accept(`before\x1b[c middle\x1b[0c after`)).toBe('before middle after') + }) + + it('removes a DA1 query split at every chunk boundary', () => { + for (const query of ['\x1b[c', '\x1b[0c']) { + for (let split = 1; split < query.length; split++) { + const filter = new StartupDeviceAttributesQueryFilter() + expect(filter.accept(`before${query.slice(0, split)}`)).toBe('before') + expect(filter.accept(`${query.slice(split)}after`)).toBe('after') + expect(filter.release()).toBe('') + } + } + }) + + it('releases incomplete and non-DA1 sequences unchanged', () => { + const filter = new StartupDeviceAttributesQueryFilter() + + expect(filter.accept('before\x1b[')).toBe('before') + expect(filter.release()).toBe('\x1b[') + expect(filter.accept('\x1b[>c')).toBe('\x1b[>c') + }) +}) diff --git a/src/main/daemon/startup-device-attributes-responder.ts b/src/main/daemon/startup-device-attributes-responder.ts new file mode 100644 index 000000000..2887dfb05 --- /dev/null +++ b/src/main/daemon/startup-device-attributes-responder.ts @@ -0,0 +1,85 @@ +/** + * Primary Device Attributes (DA1) responders for the daemon's headless emulator. + * + * Both callers answer DA1 on behalf of a renderer that cannot, and both consume + * the query so the renderer's xterm never sees it and cannot double-reply: + * + * - ConPTY 1.22+ blocks at spawn awaiting a DA1 reply. + * - The shell-ready barrier queues all inbound input until the ready marker, + * including the renderer's DA1 reply — and a shell that withholds its first + * prompt until DA1 is answered (fish waits 10s) never emits the marker that + * would release it. That caller's `reply` must write straight to the + * subprocess, past the queue, or the deadlock remains. + */ +import type { Terminal } from '@xterm/headless' + +type DeviceAttributesParser = Pick + +const PRIMARY_DEVICE_ATTRIBUTES_QUERIES = ['\x1b[c', '\x1b[0c'] as const + +/** Matches what the renderer's xterm answers for xterm-* TERMs, so consuming the + * query upstream cannot change the capabilities a TUI sees. */ +export const STARTUP_DA1_RESPONSE = '\x1b[?1;2c' + +/** Removes startup DA1 queries from renderer-bound output after the daemon answers them. */ +export class StartupDeviceAttributesQueryFilter { + private pending = '' + + accept(data: string): string { + const input = this.pending + data + this.pending = '' + let output = '' + let offset = 0 + + while (offset < input.length) { + const candidate = input.indexOf('\x1b', offset) + if (candidate === -1) { + output += input.slice(offset) + break + } + output += input.slice(offset, candidate) + const query = PRIMARY_DEVICE_ATTRIBUTES_QUERIES.find((value) => + input.startsWith(value, candidate) + ) + if (query) { + offset = candidate + query.length + continue + } + const tail = input.slice(candidate) + if (PRIMARY_DEVICE_ATTRIBUTES_QUERIES.some((value) => value.startsWith(tail))) { + this.pending = tail + break + } + output += '\x1b' + offset = candidate + 1 + } + + return output + } + + release(): string { + const pending = this.pending + this.pending = '' + return pending + } +} + +/** Returns a disposer so a caller scoped to startup can hand DA1 back to the + * renderer once its window closes. */ +export function installDeviceAttributesResponder(deps: { + parser: DeviceAttributesParser + response: string + reply: (data: string) => void +}): () => void { + const handler = deps.parser.registerCsiHandler({ final: 'c' }, (params) => { + // Why the param check: only DA1 is answered here. Secondary/tertiary variants + // carry a prefix and must fall through to the renderer. + const isPrimaryQuery = params.length === 0 || (params.length === 1 && params[0] === 0) + if (!isPrimaryQuery) { + return false + } + deps.reply(deps.response) + return true + }) + return () => handler.dispose() +} diff --git a/src/main/providers/local-pty-shell-ready.test.ts b/src/main/providers/local-pty-shell-ready.test.ts index 88a7debd9..1cb10ad7f 100644 --- a/src/main/providers/local-pty-shell-ready.test.ts +++ b/src/main/providers/local-pty-shell-ready.test.ts @@ -363,6 +363,31 @@ describePosix('local PTY shell-ready launch config', () => { vi.restoreAllMocks() }) + it('wraps fish launches with a fish_prompt shell-ready marker init command', async () => { + // Why: markerless fish resolved the ready barrier instantly and blind-wrote agent + // launch commands while fish/Starship still initialized (STA-3417). + const { getShellReadyLaunchConfig } = await importFreshLocalPtyShellReady() + + const config = getShellReadyLaunchConfig('/opt/homebrew/bin/fish') + + expect(config.supportsReadyMarker).toBe(true) + expect(config.env).toEqual({ ORCA_SHELL_READY_MARKER: '1' }) + expect(config.args?.slice(0, 2)).toEqual(['-l', '-C']) + const init = config.args?.[2] ?? '' + expect(init).toContain('--on-event fish_prompt') + // Why `builtin`: a user-defined printf function would swallow the marker. + expect(init).toContain('builtin printf "\\033]777;orca-shell-ready\\007"') + expect(init).toContain('functions -e __orca_shell_ready_marker') + }) + + it('keeps attribution-only fish spawns unwrapped', async () => { + const { getAttributionShellLaunchConfig } = await importFreshLocalPtyShellReady() + + const config = getAttributionShellLaunchConfig('/opt/homebrew/bin/fish') + + expect(config).toEqual({ args: null, env: {}, supportsReadyMarker: false }) + }) + it('falls back to HOME for ORCA_ORIG_ZDOTDIR when inherited ZDOTDIR points at a wrapper dir', async () => { // Why: mirrors the daemon path — guards the same zsh recursion loop for renderer/local PTYs spawned inside an Orca terminal. const previousZdotdir = process.env.ZDOTDIR diff --git a/src/main/providers/local-pty-shell-ready.ts b/src/main/providers/local-pty-shell-ready.ts index e3f06c563..18052dfa6 100644 --- a/src/main/providers/local-pty-shell-ready.ts +++ b/src/main/providers/local-pty-shell-ready.ts @@ -17,6 +17,7 @@ import { import { getPosixOmpShellWrapper } from '../pty/omp-shell-wrapper' import { buildStartupCommandSubmission } from '../../shared/startup-command-submission' import { + getFishShellReadyInitCommand, getZshEnvTemplate, getZshFinalZdotdirRestoreBlock, getZshShellReadyMarkerRegistrationBlock, @@ -397,6 +398,15 @@ function getWrappedShellLaunchConfig( } } + // Why: mirrors daemon/shell-ready.ts; attribution-only fish stays unwrapped. + if (shellName === 'fish' && options.emitReadyMarker) { + return { + args: ['-l', '-C', getFishShellReadyInitCommand(SHELL_READY_MARKER_ESCAPED)], + env: { ORCA_SHELL_READY_MARKER: '1' }, + supportsReadyMarker: true + } + } + return { args: null, env: {}, diff --git a/src/main/shell-templates.ts b/src/main/shell-templates.ts index da520112d..e63729790 100644 --- a/src/main/shell-templates.ts +++ b/src/main/shell-templates.ts @@ -160,6 +160,20 @@ fi ` } +// Why: fish has no ZDOTDIR-style wrapper dir, so the marker rides `--init-command` +// and fires on fish_prompt — the earliest event fish exposes (STA-3417). Unlike zsh's +// zle-line-init this lands just *before* fish arms `?2004h`, which PostReadyFlushGate +// absorbs. `builtin printf` so a user-defined printf can't silently swallow the marker +// and send every launch to the ready timeout. +export function getFishShellReadyInitCommand(escapedMarker: string): string { + return `if test "$ORCA_SHELL_READY_MARKER" = 1 + function __orca_shell_ready_marker --on-event fish_prompt + builtin printf "${escapedMarker}" + functions -e __orca_shell_ready_marker + end +end` +} + export function getZshFinalZdotdirRestoreBlock(homeExpression = '"${ORCA_ORIG_ZDOTDIR:-$HOME}"') { return `_orca_home=${homeExpression} case "\${_orca_home%/}" in