Commit Graph
5 Commits
Author SHA1 Message Date
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
Neil d7a23c84a9 fix(pty): keep a CPR reply from overtaking a deferred colour reply (#15559)
A background-colour probe writes `OSC 11 ;? ST` then `CSI 6n` and reads
exactly one response, using the CPR as its sentinel: a non-OSC first
response means "unsupported" and it stops draining. #13309 routed live
cooked-echo-risk replies through the ECHO-probe deferral while CPR kept
the immediate path, so the CPR overtook the colour reply, the prober gave
up, and the stray `ESC ]` was left in the tty for the next program —
`gh auth login` died on it with an escape-sequence error.

Queue a reply that needs no echo containment behind ones that do, FIFO,
and only while something is actually deferred, so latency-critical
replies stay immediate on every other path. Windows is unaffected: only
posix-pty defers, so the queue is always empty there.

Also: an in-flight echo probe is already the write continuation, so
re-arming the timer for a queued reply would fork a second stty and throw
away the first verdict; and teardown now hands queued uncontained writes
to the pty best-effort instead of dropping bytes the caller was told were
sent.

Known scope limit, pinned by tests and tracked for follow-up: the
guarantee is FIFO among recognised query replies, not over every byte —
a reply coalesced with a keystroke, and ordinary typed input, still
bypass the queue. Both were unordered before this change too.
2026-08-19 20:46:12 -07:00
NeilandOrca 5ea7df1a5b fix(terminal): make DECSET 2031 subscriptions silent (#13904)
fish arms `CSI ?2031h` before painting each prompt and withdraws it when it
hands the tty to a child — a ~1ms window. Orca answered that subscribe with
`CSI ?997;Nn` across a 1-3ms renderer hop, so the reply landed after the
withdrawal and was read as stdin by the next child, corrupting `brew`/`npx`
`[y/N]` prompts.

The reply is not stale by Orca's own view when written (measured
staleReplies: 0), so no suppress-the-stale-reply scheme can close this — the
information needed to suppress does not exist yet. Nothing asked for the reply
either. The Contour spec says a terminal "should only send out the DSR when the
palette has been updated"; Ghostty (Termio.zig:729 — force=true reachable only
from the ?996n DSR), iTerm2 (VT100Terminal.m:995 — flag only) and xterm.js
(InputHandler.ts:2035 — flag only) all emit nothing on the DECSET. So stop
entering the race: record the subscription, answer nothing.

Of 17 real programs measured under a pty, only fish, tmux, claude and opencode
subscribe; none block on a reply, and answering produces one redundant palette
re-query and zero rendering difference. tmux is the only one that sends `?996n`,
which Orca still answers.

- Subscribes are record-only at all four emitters (live scan, hidden-gate fact,
  parked byte watcher, parked responder — the last is deleted, it only replied).
- `?996n` answers, the subscription registry, and the theme-flip push are
  unchanged. `paneLastThemeMode` is still seeded at subscribe so the next
  appearance re-apply is not read as a flip.
- Replay grammar carries `?2031l` alongside `?2031h`, so a late-attaching remote
  client no longer registers a subscription the TUI already retired.

Also closes fish-integration gaps found alongside: `unset` (which fish lacks)
becomes `set -e` on paths parsed by the client's login shell, `config.fish` is
parsed for agent-home detection, and bracketed-paste startup delivery is made
consistent across local/daemon/relay.

Regression test drives real fish 4.7.1 under node-pty and asserts on what the
child process reads; it fails against pre-fix code with the exact payload from
the issue. CI installs fish 4 and fails loudly rather than skipping.

Closes #9993

Co-authored-by: Orca <help@stably.ai>
2026-08-11 21:16:36 -07:00
Neilandbbingz 34f2a62cda fix(pty): stop color-scheme 997 replies from painting cooked prompts (#13309)
Route cooked-echo-risk terminal replies through bounded echo-safe delivery across local, daemon, and SSH relay PTYs. Preserve repeated valid replies, bypass the daemon startup input gate, and keep ordinary input plus latency-critical replies on their existing paths.

Closes #13137

Co-authored-by: bbingz <zzb@gxsmjx.com>
2026-08-09 01:45:15 -07:00
Jinwoo HongandOrca 79d0de9c33 fix(terminal): remote query-reply corruption (#7329) + snapshot grid-width repaint (#7279) (#7736)
Co-authored-by: Orca <help@stably.ai>
2026-07-07 16:42:43 -07:00