fix(terminal): agent pane auto-launch lost under fish + Starship (STA-3417) (#12840)
* fix(terminal): extend the shell-ready startup barrier to fish (STA-3417) Fish never emitted the OSC 777 shell-ready marker, so agent launch commands were written into the PTY while fish/Starship were still initializing: the daemon path wrote them synchronously at session create and the local path blind-wrote ~30ms after the first output byte. The command was echoed by the kernel but never executed. - shell-templates: shared fish --init-command that emits the marker once on the first fish_prompt event (the earliest point fish's own reader owns the PTY, mirroring zsh's zle-line-init marker) - daemon shell-ready: fish joins the startup barrier so the launch command queues until the marker (timeout fallback unchanged) - local-pty-shell-ready: fish launch config gains the marker wrapper - codex-startup-delivery/tui-agent-startup: omp/pi/opencode plans now request shell-ready delivery (codex parity) so the SSH renderer path also waits for the prompt; plain payload-free codex stays on the markerless fast path * fix(terminal): answer DA1 past the shell-ready barrier The barrier queues all inbound input until the ready marker, including the renderer's DA1 reply. A shell that withholds its first prompt until DA1 is answered — fish waits 10s — therefore never emits the marker that would release the reply it is waiting for. Measured: 10.37s to launch an agent, versus 0.35s once the reply lands. Answer DA1 from the daemon while the barrier holds, writing straight to the subprocess so the reply bypasses the queue, and consume the query so the renderer's xterm cannot also reply. Released on ready, timeout, or dispose, handing DA1 back to the renderer for steady state. Consolidates the identical DA1 handler the ConPTY override already used. * fix(terminal): prevent duplicate startup DA1 replies
This commit is contained in:
Brennan Benson
GitHub
parent
b09f7bd431
commit
6858e072cf
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<typeof setTimeout> | null = null
|
||||
private killTimer: ReturnType<typeof setTimeout> | 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()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<void>((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
|
||||
|
|
|
|||
|
|
@ -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, string>): 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<string, string>): 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: {},
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
})
|
||||
})
|
||||
|
|
@ -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<Terminal['parser'], 'registerCsiHandler'>
|
||||
|
||||
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()
|
||||
}
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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: {},
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue