Files
orca/src/shared/terminal-query-reply.ts
NeilandBrennan c92f394cde fix(pty): delete the reply-withholding scheduler (#15578)
* 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>
2026-08-20 02:15:42 -07:00

165 lines
7.3 KiB
TypeScript

// Why this module exists: xterm's public onData stream mixes real keystrokes
// with the parser's synthetic replies to terminal queries a program embedded in
// its output (CPR/DSR cursor + device-status reports, DA device attributes,
// DECRPM mode reports, window/cell pixel-size reports, OSC 10/11 color reports,
// kitty keyboard flag reports, DCS-framed DECRQSS/XTVERSION reports).
// A querying program (e.g. starship/orb) reads these replies synchronously in
// raw mode with a short timeout, so on the remote path they must NOT sit behind
// the input debounce — a late reply lands on the shell prompt in cooked mode,
// which echoes it literally and splices it into the next typed line (#7329).
// This classifier lets the transport send replies immediately while keeping
// ordinary typed input (including bursty arrow-key auto-repeat) coalesced.
const ESC = String.fromCharCode(0x1b)
// Built via new RegExp from \u-escaped strings so no literal control
// characters appear in the source. // Final bytes of xterm's own query-reply grammars:
// R — CPR / DECXCPR cursor position report (answer to CSI 6n / CSI ? 6n)
// n — DSR device status report (answer to CSI 5n → CSI 0n)
// c — DA1/DA2/DA3 device attributes (answer to CSI c / CSI > c / CSI = c)
// t — window/cell pixel-size + text-area-size reports (CSI 14t/16t/18t)
// y — DECRPM mode report (answer to CSI ? Ps $ p), body ends "$y"
// u — kitty keyboard flags report (answer to CSI ? u), carries "?"
/* oxlint-disable no-control-regex -- grammars match terminal ESC/BEL sequences by definition */
// Known accepted collision: xterm.js encodes MODIFIED F3 (Shift/Ctrl/Alt+F3) as
// `CSI 1 ; <mod> R`, which is indistinguishable from a CPR report (a classic
// VT ambiguity). Such a keystroke is sent immediately and shares the bounded reply
// budget, so a pathological reply flood can shed it. It is also held behind a deferred
// reply like any CPR, so within that window (bounded by the host's echo budget) a
// keystroke typed after it can reach the pty first. Accepted: it needs the chord and a
// following keypress inside a live colour-query deferral, and we keep the reply grammar
// complete rather than special-casing an unresolvable ambiguity.
const CPR_OR_DSR_PREFIX_RE = new RegExp('^\\u001b\\[\\??[0-9;]*[Rn]')
const DEVICE_ATTRIBUTES_PREFIX_RE = new RegExp('^\\u001b\\[[?>=]?[0-9;]*c')
// 4/6 = pixel-size reports, 8 = text-area size in characters (answer to CSI 18t).
const WINDOW_SIZE_REPORT_PREFIX_RE = new RegExp('^\\u001b\\[[468];[0-9]+;[0-9]+t')
// `?` optional: private-mode reports carry it (DECRPM), ANSI-mode reports don't.
const DECRPM_PREFIX_RE = new RegExp('^\\u001b\\[\\??[0-9;]*\\$y')
// Kitty keyboard protocol flags report: CSI ? flags u. The `?` distinguishes it
// from kitty-protocol *keystrokes* (CSI code;mods u), which must stay batched.
const KITTY_FLAGS_PREFIX_RE = new RegExp('^\\u001b\\[\\?[0-9]+u')
// OSC color/title responses: ESC ] Ps ; body ST (ST = BEL or ESC backslash).
const OSC_RESPONSE_PREFIX_RE = new RegExp(
'^\\u001b\\][0-9]+;[^\\u0007\\u001b]*(?:\\u0007|\\u001b\\\\)'
)
// DCS-framed reports xterm emits: DECRQSS "ESC P 1 $ r Pt ST" / "ESC P 0 $ r ST"
// (vim queries cursor style this way) and XTVERSION "ESC P > | text ST".
const DCS_RESPONSE_PREFIX_RE = new RegExp(
'^\\u001bP(?:[01]\\$r[^\\u001b]*|>\\|[^\\u001b]*)\\u001b\\\\'
)
// Private-mode DSR (CSI ? … n) — e.g. color-scheme `?997;1n` — often lands cooked.
// Prefix form peels consecutive replies out of one coalesced payload.
const COOKED_ECHO_RISK_PRIVATE_DSR_PREFIX_RE = new RegExp('^\\u001b\\[\\?[0-9;]*n')
const COOKED_ECHO_RISK_OSC_PREFIX_RE = new RegExp(
'^\\u001b\\][0-9]+;[^\\u0007\\u001b]*(?:\\u0007|\\u001b\\\\)'
)
const QUERY_REPLY_PREFIX_RES = [
CPR_OR_DSR_PREFIX_RE,
DEVICE_ATTRIBUTES_PREFIX_RE,
WINDOW_SIZE_REPORT_PREFIX_RE,
DECRPM_PREFIX_RE,
KITTY_FLAGS_PREFIX_RE,
OSC_RESPONSE_PREFIX_RE,
DCS_RESPONSE_PREFIX_RE
] as const
/* oxlint-enable no-control-regex */
function terminalQueryReplyEnd(data: string, start: number): number {
if (start >= data.length || data[start] !== ESC) {
return -1
}
const slice = data.slice(start)
for (const re of QUERY_REPLY_PREFIX_RES) {
const match = re.exec(slice)
if (match?.[0]) {
return start + match[0].length
}
}
return -1
}
/**
* True when `data` (from xterm.onData) is a synthetic reply the emulator
* generated in response to a query — not something the user typed. These are
* latency-critical and must bypass input coalescing on the remote transport.
*
* Conservative by design: matches only complete, well-formed reply grammars so
* ordinary keystrokes and navigation sequences (arrows CSI A/B/C/D, Home/End,
* function keys ending in ~, kitty CSI-u keystrokes) are never misclassified
* as replies — with the single documented modified-F3/CPR collision above.
*/
export function isTerminalQueryReply(data: string): boolean {
return data.length >= 3 && terminalQueryReplyEnd(data, 0) === data.length
}
/** End index (exclusive) of one cooked-echo-risk reply at `start`, else -1. */
function cookedEchoSafeReplyEnd(data: string, start: number): number {
if (start >= data.length || data[start] !== ESC) {
return -1
}
const slice = data.slice(start)
const dsr = COOKED_ECHO_RISK_PRIVATE_DSR_PREFIX_RE.exec(slice)
if (dsr?.[0]) {
return start + dsr[0].length
}
const osc = COOKED_ECHO_RISK_OSC_PREFIX_RE.exec(slice)
if (osc?.[0]) {
return start + osc[0].length
}
return -1
}
/**
* If `data` is entirely one or more consecutive cooked-echo-risk replies, return each
* reply. Repeats still arrive coalesced from a rapid theme flip or an older client that
* answers 2031 subscribes (this host stopped — #9993), so peel them individually.
* Mixed payloads (reply + keystroke) return null so hosts fall through to raw write.
*/
export function extractOnlyCookedEchoSafeQueryReplies(data: string): string[] | null {
if (data.length < 4 || data[0] !== ESC) {
return null
}
const replies: string[] = []
let offset = 0
while (offset < data.length) {
const end = cookedEchoSafeReplyEnd(data, offset)
if (end === -1) {
return null
}
replies.push(data.slice(offset, end))
offset = end
}
return replies.length > 0 ? replies : null
}
/** If `data` is entirely one or more consecutive query replies, return each reply. */
export function extractOnlyTerminalQueryReplies(data: string): string[] | null {
if (data.length < 3 || data[0] !== ESC) {
return null
}
const replies: string[] = []
let offset = 0
while (offset < data.length) {
const end = terminalQueryReplyEnd(data, offset)
if (end === -1) {
return null
}
replies.push(data.slice(offset, end))
offset = end
}
return replies.length > 0 ? replies : null
}
/**
* Query replies that must use the ECHO-safe write path on POSIX PTYs so cooked
* prompts do not paint reply bytes (e.g. `997;1n` on `npx` confirm, #13137).
* Latency-critical CPR/DSR without `?` stay on the immediate write path.
*
* Whole-string only: a single complete reply. For repeated / write-queue
* coalesced payloads use {@link extractOnlyCookedEchoSafeQueryReplies}.
*/
export function needsCookedEchoSafeQueryReply(data: string): boolean {
const replies = extractOnlyCookedEchoSafeQueryReplies(data)
return replies !== null && replies.length === 1
}