mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
* fix(pty): answer a terminal colour query in its own turn Root-cause follow-up to #15559, which stopped a CPR overtaking a deferred colour reply but left the deferral itself in place. Orca answers terminal queries by writing to the PTY master, which a line discipline in ECHO copies straight back out as junk on a cooked prompt (#12112). The guard was to withhold the write until an `stty` subprocess proved ECHO clear — and forking is what forced the decision to be async. Any deferral, however short, lets a reply written later in the same turn overtake this one, so the async probe was the bug's root cause. Read the bit synchronously instead. Linux and the BSDs redirect a master's mode ioctls to the slave, so a `tcgetattr` on the master fd node-pty already owns answers for the slave with no fork: measured 0.26us against 2403us for the subprocess. With a verdict available inline, a querying program that already cleared ECHO — every raw-mode prober, including the colour probe behind the `gh auth login` report — is answered in its own turn and can never be reordered. The deferral stays for the genuinely cooked case, and the ordering guarantee stays underneath it: hosts whose node-pty predates this patch get no sync probe and fall back to the deferred path, which mixed client/host versions make a live production path. Reply routing is all-or-nothing: a payload needing neither containment nor ordering stays on the host's own path, so a CPR answered during shell startup cannot pass the daemon's post-ready flush gate and splice into the buffered startup command. Native side is fail-safe: a kernel that did not redirect would answer from the master's own termios, whose ECHO defaults set, so the degraded verdict is "echoing" — never a false "quiet". The JS half ships in the pnpm patch while the binding needs a source build, so ORCA_REQUIRE_NODE_PTY_ECHO_STATE=1 makes CI fail rather than silently skip when it is handed an upstream prebuild. Co-authored-by: Brennan <brennanb2025@users.noreply.github.com> * fix(pty): keep the flush ordered under synchronous re-entry Three defects found in external review of the reply-ordering work. node-pty delivers onData inside the master write, so a query can be answered while the queue is mid-flush. `flushPendingWrites` spliced the array off before writing, so that reply saw an empty queue, took the same-turn path, and landed ahead of entries the loop had not written yet — reproduced as 01, 99, 02, 03. It now shifts one entry at a time so a re-entrant reply queues behind the rest, bounded by the length at entry so a re-entrant push cannot spin the loop. An overflow flush can re-enter as far as teardown. `answer` did not re-check `closed` afterwards, so it queued behind a closed delivery, returned true, and the reply was never written and never reported. The payload router's ownership comment overstated its guarantee. The `any` semantics are deliberate — returning false after a constituent was already written would have the caller re-write the whole payload and duplicate it into the child's stdin — so the residual mixed-failure drop is now documented rather than implied away. * fix(pty): delete the reply-withholding scheduler Orca answered a terminal query by withholding the write until a probe proved the slave's ECHO bit was clear. That was the wrong mechanism, and it is now gone: replies are written in the caller's turn and their echo is contained on the output side, where it always was. Withholding never removed an echo. The wait was bounded and always ended in a write, so the output-side projections were doing the work the whole time — including the readline rewrite, which happens with the tty already raw and which therefore no reading of the ECHO bit can predict. What withholding did add was an asynchronous write path, and that is what let one reply overtake another and land in the next program's stdin (#15559), what produced a re-entrancy inversion inside its own flush, and what four rounds of regressions have lived in. The last thing it covered was the verbatim echo of a `stty -echoctl` tty. That shape is now projected directly. It starts with ESC, so it is matched only when complete and never held as a partial: holding it would take a bare trailing ESC from the query parser and an expired hold would release it raw, so a query torn at its own ESC would never be answered. Complete-match-only is what makes the shape safe to project at all. Measured on a real pty: a cooked-mode master write is both echoed AND delivered — ECHO copies the bytes without consuming them from the slave's input queue, so a program arming raw mode with TCSANOW/TCSADRAIN (libuv's setRawMode, hence every Node agent) still reads them. Only a TCSAFLUSH switcher discards it, which it does on every terminal, none of which gates a reply on termios state. Deletes the pending-write queue, the async stty probe, the poll budget and probe rate limit, the deadline-driven flush, and the answer/ answerInOrder split. Replies now leave in call order by construction. No packaging, native or CI surface is touched. * test(pty): restore stty-probe coverage and pin the duplicate-query retry Archaeology on how withholding got here, and what its tests were really protecting. Deleting the ECHO probe took four tests with it that were not about the probe at all: they cover createSttyProbe, which the shell-readiness line-editor probe still uses — in-flight sharing, the per-platform stty flag, and transient-versus-permanent failure latching. Restored against the line-editor probe, which is now their only caller. Also pins the property that answers the one case an immediate write cannot serve. A program that queries while cooked and then arms raw mode with TCSAFLUSH discards the reply with the rest of its input queue. Nothing can prevent that from the terminal side, and no terminal tries. What matters is that such a program re-queries after its own timeout: the ingress declines to answer an already-answered slot but forwards the duplicate downstream, so the renderer's emulator answers the retry, by which point the program is raw. The retry path is the recovery, not withholding. * ci(pty): keep the fish real-PTY test in the shell-contracts lane only Reverting pr.yml to main dropped the exclusion for the fish query-reply test, which this branch keeps, so it would have run in the sharded lane as well. Restores it to the shell-contracts include list and the shard exclude list, and drops the parallelism expectations for the deleted cooked-querier suite and the echo-state env guard. --------- Co-authored-by: Brennan <brennanb2025@users.noreply.github.com>
187 lines
7.5 KiB
TypeScript
187 lines
7.5 KiB
TypeScript
import { mkdirSync } from 'node:fs'
|
|
import path from 'node:path'
|
|
import { expect, test } from './helpers/orca-app'
|
|
import { connectDockerSshRelayTarget } from './helpers/docker-ssh-relay-connection'
|
|
import {
|
|
cleanupDockerSshRelayTarget,
|
|
execDockerSshRelayTargetCommand,
|
|
shellQuote as dockerShellQuote,
|
|
startDockerSshRelayTarget,
|
|
type DockerSshRelayTarget,
|
|
writeDockerSshRelayTargetFile
|
|
} from './helpers/docker-ssh-relay-target'
|
|
import {
|
|
execInTerminal,
|
|
sendToTerminal,
|
|
waitForActivePanePtyId,
|
|
waitForActiveTerminalManager,
|
|
waitForTerminalOutput
|
|
} from './helpers/terminal'
|
|
import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store'
|
|
|
|
const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1'
|
|
const FISH_VERSION = '4.8.1'
|
|
const FISH_ASSETS = {
|
|
aarch64: {
|
|
asset: `fish-${FISH_VERSION}-linux-aarch64.tar.xz`,
|
|
sha256: 'a03c8a445570a2a37e114cb13cebe41842cf17c4dd67a6530a57f742db04eee4'
|
|
},
|
|
x86_64: {
|
|
asset: `fish-${FISH_VERSION}-linux-x86_64.tar.xz`,
|
|
sha256: '39cab35242ab77bfdbce73b473000c3b045aaf2fe0951b042199bb7fdba3df78'
|
|
}
|
|
} as const
|
|
|
|
function shellQuote(value: string): string {
|
|
return `'${value.replaceAll("'", "'\\''")}'`
|
|
}
|
|
|
|
function remoteOscQueryScript(runId: string): string {
|
|
return [
|
|
"process.stdin.setEncoding('utf8')",
|
|
'if (process.stdin.isTTY) process.stdin.setRawMode(true)',
|
|
'process.stdin.resume()',
|
|
"let received = ''",
|
|
`process.stdout.write('REMOTE_OSC_READY_${runId}\\n')`,
|
|
"process.stdin.on('data', (chunk) => {",
|
|
' received += chunk',
|
|
" if (received.includes('\\x1b]10;rgb:')) {",
|
|
` process.stdout.write('REMOTE_OSC_REPLY_${runId}\\n')`,
|
|
' process.exit(0)',
|
|
' }',
|
|
'})',
|
|
"setTimeout(() => process.stdout.write('\\x1b]10;?\\x1b\\\\'), 100)"
|
|
].join(';')
|
|
}
|
|
|
|
function installRemoteFish(target: DockerSshRelayTarget): void {
|
|
const arch = execDockerSshRelayTargetCommand(target, 'uname -m') as keyof typeof FISH_ASSETS
|
|
const release = FISH_ASSETS[arch]
|
|
if (!release) {
|
|
throw new Error(`No pinned fish binary for Docker architecture ${arch}`)
|
|
}
|
|
const url = `https://github.com/fish-shell/fish-shell/releases/download/${FISH_VERSION}/${release.asset}`
|
|
execDockerSshRelayTargetCommand(
|
|
target,
|
|
[
|
|
`node -e ${dockerShellQuote("fetch(process.argv[1]).then(r => { if (!r.ok) throw new Error(String(r.status)); return r.arrayBuffer() }).then(b => require('node:fs').writeFileSync('/tmp/fish.tar.xz', Buffer.from(b)))")} ${dockerShellQuote(url)}`,
|
|
`echo ${dockerShellQuote(`${release.sha256} /tmp/fish.tar.xz`)} | sha256sum -c -`,
|
|
'tar -xJf /tmp/fish.tar.xz -C /usr/local/bin',
|
|
'chmod 0755 /usr/local/bin/fish',
|
|
'/usr/local/bin/fish --version'
|
|
].join(' && ')
|
|
)
|
|
}
|
|
|
|
test.describe('PTY input write queue over SSH', () => {
|
|
test.skip(!RUN_DOCKER_SSH, 'Set ORCA_E2E_SSH_DOCKER=1 to run Docker-backed SSH E2E.')
|
|
test.skip(process.platform === 'win32', 'Docker SSH E2E uses POSIX ssh tooling.')
|
|
|
|
test('returns an xterm OSC query reply through the live SSH PTY', async ({
|
|
orcaPage
|
|
}, testInfo) => {
|
|
test.slow()
|
|
let target: DockerSshRelayTarget | null = null
|
|
try {
|
|
target = startDockerSshRelayTarget(testInfo)
|
|
await waitForSessionReady(orcaPage)
|
|
await waitForActiveWorktree(orcaPage)
|
|
await connectDockerSshRelayTarget(orcaPage, target)
|
|
await ensureTerminalVisible(orcaPage, 45_000)
|
|
await waitForActiveTerminalManager(orcaPage, 60_000)
|
|
const ptyId = await waitForActivePanePtyId(orcaPage, 60_000)
|
|
const runId = String(Date.now())
|
|
|
|
await execInTerminal(orcaPage, ptyId, `node -e ${shellQuote(remoteOscQueryScript(runId))}`)
|
|
await waitForTerminalOutput(orcaPage, `REMOTE_OSC_READY_${runId}`, 30_000, 80_000)
|
|
await waitForTerminalOutput(orcaPage, `REMOTE_OSC_REPLY_${runId}`, 30_000, 80_000)
|
|
} finally {
|
|
cleanupDockerSshRelayTarget(target)
|
|
}
|
|
})
|
|
|
|
test('keeps fish query replies out of the next child stdin on an upstream relay pty', async ({
|
|
orcaPage
|
|
}, testInfo) => {
|
|
test.slow()
|
|
let target: DockerSshRelayTarget | null = null
|
|
try {
|
|
target = startDockerSshRelayTarget(testInfo)
|
|
installRemoteFish(target)
|
|
const runId = String(Date.now())
|
|
const home = `/tmp/orca-fish-${runId}`
|
|
const prompt = `ORCA_SSH_FISH_${runId}> `
|
|
const childReady = `CHILD_READY_${runId}`
|
|
const childRead = `CHILD_READ_${runId}`
|
|
execDockerSshRelayTargetCommand(
|
|
target,
|
|
`mkdir -p ${dockerShellQuote(`${home}/.config/fish`)} ${dockerShellQuote(`${home}/.local/share`)}`
|
|
)
|
|
writeDockerSshRelayTargetFile(
|
|
target,
|
|
`${home}/.config/fish/config.fish`,
|
|
[
|
|
'set -g fish_greeting ""',
|
|
`function fish_prompt; printf ${dockerShellQuote(prompt)}; end`,
|
|
'function fish_right_prompt; end',
|
|
''
|
|
].join('\n')
|
|
)
|
|
const childScript = `${home}/read-stdin.mjs`
|
|
writeDockerSshRelayTargetFile(
|
|
target,
|
|
childScript,
|
|
[
|
|
`process.stdout.write(${JSON.stringify(`${childReady}\\n`)})`,
|
|
"let buffered = ''",
|
|
"process.stdin.on('data', chunk => {",
|
|
" buffered += chunk.toString('utf8')",
|
|
" if (!buffered.includes('\\n')) return",
|
|
` process.stdout.write(${JSON.stringify(`${childRead}:`)} + JSON.stringify(buffered) + '\\n')`,
|
|
' process.exit(0)',
|
|
'})',
|
|
''
|
|
].join('\n')
|
|
)
|
|
|
|
await waitForSessionReady(orcaPage)
|
|
await waitForActiveWorktree(orcaPage)
|
|
await connectDockerSshRelayTarget(orcaPage, target)
|
|
const relayExports = execDockerSshRelayTargetCommand(
|
|
target,
|
|
'module=$(find /root/.orca-remote -type d -path \'*/node_modules/node-pty\' | head -n 1); node -e "const p=require(process.argv[1]); console.log(Object.keys(p.native || {}).join(\',\'))" "$module"'
|
|
)
|
|
testInfo.annotations.push({ type: 'relay-node-pty-exports', description: relayExports })
|
|
expect(relayExports).not.toContain('echoState')
|
|
|
|
await ensureTerminalVisible(orcaPage, 45_000)
|
|
await waitForActiveTerminalManager(orcaPage, 60_000)
|
|
const ptyId = await waitForActivePanePtyId(orcaPage, 60_000)
|
|
await execInTerminal(
|
|
orcaPage,
|
|
ptyId,
|
|
`env HOME=${shellQuote(home)} XDG_CONFIG_HOME=${shellQuote(`${home}/.config`)} XDG_DATA_HOME=${shellQuote(`${home}/.local/share`)} TERM=xterm-256color /usr/local/bin/fish -l -i`
|
|
)
|
|
await waitForTerminalOutput(orcaPage, prompt, 30_000, 80_000)
|
|
|
|
const blocker = `node -e ${shellQuote(`console.log('BLOCKER_STARTED_${runId}'); setTimeout(() => process.exit(0), 5000)`)}`
|
|
await execInTerminal(orcaPage, ptyId, blocker)
|
|
await waitForTerminalOutput(orcaPage, `BLOCKER_STARTED_${runId}`, 30_000, 80_000)
|
|
await execInTerminal(orcaPage, ptyId, `node ${shellQuote(childScript)}`)
|
|
await waitForTerminalOutput(orcaPage, childReady, 30_000, 80_000)
|
|
await sendToTerminal(orcaPage, ptyId, 'hello\r')
|
|
await waitForTerminalOutput(orcaPage, `${childRead}:"hello\\n"`, 30_000, 80_000)
|
|
const screenshotDir = path.join(process.cwd(), 'validation-screenshots', 'sta-3948')
|
|
const screenshotPath = path.join(screenshotDir, 'linux-ssh-fish-child-stdin-pass.png')
|
|
mkdirSync(screenshotDir, { recursive: true })
|
|
await orcaPage.screenshot({ path: screenshotPath, fullPage: true })
|
|
await testInfo.attach('linux-ssh-fish-child-stdin-pass', {
|
|
path: screenshotPath,
|
|
contentType: 'image/png'
|
|
})
|
|
} finally {
|
|
cleanupDockerSshRelayTarget(target)
|
|
}
|
|
})
|
|
})
|