[ferpart]

Refs #9993 (still reproducible on v1.4.168 after #8206 / #8214 / #10817)

Summary

^[[?997;1n^[[?997;1n still lands in the stdin of whatever runs next after a fish prompt-accept, corrupting interactive prompts (brew/npx [y/N] confirmations, git add -p, any keystroke reader). Reproduced deterministically on v1.4.168.

Root cause (byte-level ground truth)

Captured from fish 4.7.1 + Tide behind a nested PTY inside an Orca 1.4.168 terminal. At command accept, fish toggles mode 2031 once per repaint, each toggle its own PTY chunk, five chunks in ~5 ms:

T+5.056  fish  ?2004l ?2031l          (withdraw)
T+5.056  fish  ?2004h ?2031h          (re-subscribe — chunk ends subscribed)
T+5.058  fish  ?2004l ?2031l          (withdraw)
T+5.058  orca  CSI ?997;1n            (reply to the 5.056 h — stale on arrival)
T+5.060  fish  OSC 0 + ?2004h ?2031h  (re-subscribe)
T+5.060  fish  ?2004l ?2031l + 133;C  (final withdraw, command starts)
T+5.061  orca  CSI ?997;1n            (reply to the 5.060 h — stale on arrival)
T+5.061  fish  ^[[?997;1n^[[?997;1n   (both replies echoed / fed to the child)

Every responder decision here is correct for the chunk it observed — the #10817 chunk-final contract holds. The replies lose to the next chunk: each one crosses a scheduling boundary (renderer IPC, remote protocol, fact relay) before reaching the PTY, and by arrival fish has withdrawn in a later chunk and handed stdin to the command. In-process terminals answer in microseconds and never lose this race; a multi-process pipeline loses it on every prompt accept, doubled because two repaint subscribes get answered late. No amount of per-chunk decision hardening at the responder sites can close this — it is a race between reply latency and the next chunk.

Fix

Main ingests PTY output ahead of every reply route, so at the moment a reply write reaches the PTY, main always holds fresher subscription state than the responder that produced it. This PR adds a last-moment gate at the write boundary (pty-color-scheme-reply-write-gate.ts), checked at both write funnels:

• the renderer pty:write provider path (writePtyProviderInput)
• the runtime controller write (remote clients, CLI terminal.send, main-emulator query replies)

A write that is exactly a standalone CSI ?997;1n / CSI ?997;2n report is dropped iff main's ingestion-fresh state says the subscription was withdrawn.

Design points:

• The gate keeps its own bounded raw-byte scan (reusing scanMode2031ReplyDecision) fed from onPtyData, so its state does not depend on fact consumers or per-pane reply ownership.
• While a PTY's scan authority is delegated to the daemon, delivered bytes may be gapped: the raw scan stands down and the daemon's relayed 2031-subscribe/2031-unsubscribe facts feed the state instead. Any signal that can trigger a reply also feeds the gate first, so the gate can only ever be stricter than the responder that fired.
• A ?996n DSR answer reuses the CSI 997 bytes and must never be dropped — observed queries bank one pass each (bounded at 4).
• Fail-open everywhere main has no state: never-observed PTYs, data gaps, teardown/reuse, killed switches. Only exact standalone report writes are ever considered, so keystrokes and bracketed paste can never match.

Reproduction

On v1.4.168, fish 4.7.1 + Tide, in a regular pane or the floating terminal:

python3 -c 'import sys; d=sys.stdin.readline(); print("GOT:", repr(d))'
# type: hello<Enter>
GOT: '\x1b[?997;1n\x1b[?997;1nhello\n'     # before this PR
GOT: 'hello\n'                             # after this PR (verified on a dev build)

The repro test replays the captured chunk stream verbatim through OrcaRuntimeService.onPtyData and pins both drops, the pass-while-subscribed case, the daemon fact path, and the ?996n pass.

Also verified live on a dev build: a program that subscribes and stays subscribed still receives exactly one CSI ?997;1n (gate pass), and the doubled leak is gone.

Screenshots

No visual change.

Testing

• Changed-file oxlint (also runs in the pre-commit hook)
• pnpm typecheck:node
• pnpm test (focused): new gate unit suite (15) + repro suite (4), plus orca-runtime (1023), pty (327), terminal-query-responder (66), daemon-background-transient-facts, terminal-output-side-effects, terminal-mode-2031-final-state, terminal-color-scheme-protocol — all passing
• pnpm check:max-lines-ratchet, pnpm check:reliability-gates, git diff --check
• Added high-quality regression tests: pty-color-scheme-reply-write-gate.test.ts (unit) and issue-9993-fish-prompt-accept-stale-mode2031-reply.repro.test.ts (end-to-end through the runtime ingest path, chunk stream pinned from a real byte trace)

AI Review Report

Reviewed with an AI coding agent. Main risks checked and addressed:

• Over-dropping legitimate replies: ?996n answers share the CSI 997 bytes — covered by pending-query slots; subscribe-and-hold seeds and theme-flip pushes pass because the gate state is subscribed; never-observed PTYs fail open.
• Feed staleness/divergence: the raw scan runs at ingestion in onPtyData, strictly before renderer delivery and any reply route; delegated (gapped) streams switch to the daemon fact feed, and both boundaries reset the scan carry (mirroring setTransientFactScanningSuppressed semantics).
• State leakage across PTY reuse: cleared in clearProviderPtyState alongside the Phase-5 ConPTY spawn record.
• Cross-platform: the gate is byte-level and platform-neutral (local, daemon, SSH, folder workspaces, remote clients all cross the same two funnels); no shortcuts, labels, paths, or shell-specific behavior touched. ConPTY DA1 and other query replies are unaffected (exact-match on CSI 997 only).
• Hot path cost: ordinary output takes an allocation-free early return (no ESC/CSI byte and no tracked state).

Security Audit

No new command execution, filesystem access, network surface, or IPC endpoints. The change only inspects bytes already flowing through existing write/ingest paths and can only drop two exact 9-byte sequences, never inject or transform input. Bracketed paste and batched user input cannot match the exact-write predicate.

Notes

• The residual race window shrinks from a renderer/remote round-trip (milliseconds, lost on every fish prompt-accept) to main's ingest-to-write interval (sub-millisecond, same order as in-process terminals).
• [Bug]: Main-window terminal tabs never live-refresh color scheme (mode 2031) on system appearance change — floating terminal unaffected #10913 (visibility-resume theme refresh) is unaffected: flips to a still-subscribed PTY pass the gate.
• If a future responder legitimately needs to answer while a PTY reads as withdrawn, it should feed observePtyMode2031Decision — see the module doc.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: acae7286-42ed-4e3c-bf68-19b551145d45

📥 Commits

Reviewing files that changed from the base of the PR and between bb545c2 and 70e9a43.

📒 Files selected for processing (2)

• src/main/runtime/pty-color-scheme-reply-write-gate.test.ts
• src/main/runtime/pty-color-scheme-reply-write-gate.ts

---

📝 Walkthrough

Walkthrough

Added a per-PTY gate for CSI 997 color-scheme replies. The gate tracks mode-2031 subscriptions, delegated daemon decisions, scan gaps, and bounded ?996n queries. Runtime PTY output and terminal facts update the gate. PTY write paths discard stale replies before forwarding input. PTY teardown clears gate state. Unit and regression tests cover fish prompt acceptance, split output, delegation, query responses, PTY isolation, and state reset.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: dropping stale CSI 997 color-scheme replies at the PTY write boundary.
Description check	✅ Passed	The description covers the required sections, explains the fix, documents testing and security review, and states that there is no visual change.
Docstring Coverage	✅ Passed	Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

src/main/runtime/pty-color-scheme-reply-write-gate.ts (1)

1-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Condense the module comment.

Lines 1-23 repeat the boundary rationale and timing detail. Keep the protocol contract and issue reference. Move the detailed scenario to a design note or issue.

As per coding guidelines, comments must be concise, non-obvious, and preferably one line.

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f0ecf80a-492d-4f07-b267-7019f40a59f8

📥 Commits

Reviewing files that changed from the base of the PR and between b3a4a4f and bb545c2.

📒 Files selected for processing (5)

• src/main/ipc/pty.ts
• src/main/runtime/issue-9993-fish-prompt-accept-stale-mode2031-reply.repro.test.ts
• src/main/runtime/orca-runtime.ts
• src/main/runtime/pty-color-scheme-reply-write-gate.test.ts
• src/main/runtime/pty-color-scheme-reply-write-gate.ts

[ferpart]

@coderabbitai Fixing. Will be in the next commit

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@ferpart Understood. I will review the update after you push the next commit.

_{You are interacting with an AI system.}

[nwparker]

@ferpart Thanks for staying on this, and sorry for the long wait. I picked #9993 up today and used your byte-level trace as the starting point — it's accurate, and the timing capture is what made it possible to test the competing hypotheses instead of arguing about them. Two of your conclusions turned out to be right and load-bearing:

• It is not a dual-responder double. Your correction of the earlier triage was correct; I measured 3 subscribes → 3 replies, one each.
• Per-chunk decision hardening cannot close it. Also correct, and for the reason you gave.

I've opened #13904 with a different fix, and I want to be straight about why rather than just superseding this quietly.

Where the write-boundary gate doesn't hold

I built a harness around real fish 4.7.1 + node-pty and instrumented whether each reply was stale by main's own view at the moment it was written. It usually isn't:

RESULT replies {"total":3,"duringPromptAccept":1,"decisionsDuringPromptAccept":2,"staleReplies":0}

At a 1 ms reply delay the leak is deterministic (5/5) while staleReplies stays 0 — the ?2031l withdrawing the subscription has not been ingested yet when the reply goes out. The gate can only drop what it already knows is stale, so it under-drops in exactly the failure case. The information needed to suppress doesn't exist yet at the moment it would have to act.

Two smaller things, for completeness: the predicate is an exact whole-payload match against a 2-element set, so a coalesced ?997;1n?997;1n — the literal payload in the issue title — passes through unfiltered; and #13309 has since moved the real last write boundary below the gate into PtyStartupReplyDelivery, which on POSIX queues behind an ECHO probe.

What #13904 does instead

Stops emitting the reply at all. Replying to CSI ?2031h turns out to be an Orca extension, not protocol: upstream xterm.js sets colorSchemeUpdates = true and emits nothing (InputHandler.ts:2035), the protocol's actual query is CSI ?996n, and fish never sends ?996n — 0 occurrences across a full session. The ?996n answer and the theme-flip-on-actual-change push both stay. You can't lose a race you don't enter.

Your repro is now a test: fish-color-scheme-child-stdin.node-pty.test.ts drives real fish through the real renderer module and asserts on what the child process reads. Against pre-fix code it fails with your exact payload.

The part of your report I under-weighted

Your original comment said the bytes are "read as stdin by whatever program runs next" — framed around 997, but the mechanism you described is more general than that, and you were right. With 997 replies fully disabled, fish still leaks:

GOT: '\x1b[1;1R\x1b]11;rgb:1e1e/1e1e/1e1e\x1b\\hello\n'

CPR and OSC 11, into the same child stdin. Different cause — #13309's echo-safe path defers OSC and private-DSR replies 21–200 ms while CPR and DA1 go out immediately, and fish uses DA1 as its read sentinel, so anything landing after it is stranded. Deterministic, not racy, on every prompt paint on every POSIX host. Filed as #13892 with a validated fix sketch. So #13904 will not fully close the class of failure you reported — #13892 is the other half, and I'd genuinely value your eyes on it given how well you characterised the first one.

Closing this in favor of #13904 — but the diagnosis work here was good and it shortened the path considerably.

[nwparker]

Superseded by #13904, now merged as 5ea7df1a5b8 — issue #9993 is closed.

Closing this properly; my earlier comment said I was closing it and I only left the comment, sorry for the noise.

Reasoning is in that comment, but the short version: the reply usually isn't stale by main's own view at write time (staleReplies: 0), so a stale-reply gate under-drops in exactly the failure case. #13904 stops emitting the reply at all instead. Your byte-level trace is what made the competing hypotheses testable rather than arguable, and your repro is now a real-fish regression test in the suite.

The other half of what you originally reported — replies landing in the next child's stdin generally, not just 997 — is #13892, if you want to follow it.