Repository navigation
fix(terminal): stop answering mode-2031 toggles that the same chunk withdrew - #10817
Conversation
…withdrew fish enables and disables mode 2031 around every prompt (tty_handoff.rs), so a single PTY chunk routinely carries `?2031h ... ?2031l`. All three responders answered the sticky "an h appeared anywhere" flag, so each prompt cycle wrote `?997;1n` into a shell that had already handed the tty to a child — it lands as literal text, or as stdin for whatever is reading. pty-connection.ts's hidden-pane responder already had the right shape (`finalState !== 'subscribed'`); this brings the other three in line: - shared tracker: gate the '2031-subscribe' fact on the chunk-final state - parked-tab byte sidecar: same guard - visible-pane xterm CSI handler: xterm dispatches mid-parse, so there is no chunk-final state to read. Defer the reply to a microtask and re-check the subscription, letting a same-chunk `?2031l` cancel it. Refs #9993 Co-authored-by: Orca <help@stably.ai>
The previous commit deferred the visible-pane reply to a microtask so a same-chunk `?2031l` could cancel it. That cannot work: xterm's WriteBuffer parses every queued `terminal.write()` synchronously in one batch before any microtask runs, so the microtask sees the net state of N PTY chunks, not of the one that carried the subscribe. A TUI that subscribes in chunk N gets no reply when chunk N+1 happens to withdraw, and a fish prompt straddling two writes still gets answered. Move the decision to where chunk boundaries actually exist — pty-connection's dataCallback, which receives one PTY chunk per call. It scans raw bytes with `scanMode2031Sequences`, carrying a tail across chunks so a CSI split mid- sequence still resolves, and replies only when that chunk *ends* subscribed. Ownership stays single: gate-managed PTYs are answered by main's '2031-subscribe' fact, so the chunk scanner returns early for them, and the xterm CSI handler now observes only panes the scanner does not own. The tail is dropped on PTY replacement — a partial prefix belongs to the stream that produced it. Removes the microtask responder and the seed-reply retry path it needed. Mutation-tested: 6 mutations applied, 6 killed.
The previous commit moved 2031 reply decisions to the PTY chunk boundary and gave gate-managed panes a single owner: main's '2031-subscribe' fact. But the fact union is subscribe-only, and that left the withdrawal unobserved. For a gate-managed pane, main drops renderer-bound bytes after model ingestion, the chunk scanner early-returns, and xterm's CSI handler is disabled. So when a TUI emits `?2031l` while hidden, nothing retires the subscription: paneMode2031 stays set, and the next theme flip has maybePushMode2031Flip push `CSI ?997;2n` into the shell that replaced the TUI — #9993 again, through the theme-change door. Before this branch, skipHiddenRendererOutput observed those withheld bytes; consolidating ownership removed that observer without replacing it. No renderer-side observer can close this: the bytes are gone before the renderer sees them. The state protocol has to carry the withdrawal, so add a '2031-unsubscribe' fact alongside the subscribe across the three fact unions (shared, provider, daemon). It fires only on a real chunk-final withdrawal — a chunk with no 2031 bytes scans to null and stays silent. The renderer handler clears both maps and sends nothing: a withdrawal is not a query. Also closes two gaps an adversarial review found by mutation, both previously resting on comments rather than tests: the lifecycle parser-ownership predicate (extracted as isPaneParserOwnedMode2031Observer so it is directly testable) and the scan-before-reconciliation ordering that lets a chunk the snapshot drops as a duplicate still answer its query. Mutation-tested: 12 mutations applied, 12 killed (6 from the prior round re-run, 6 new covering this fix and the two survivors).
Round-2 review found a wire-compatibility hole in the original #9993 fix. Daemons survive app updates, so a new desktop can drive a daemon that was started by the previous build. Pre-v29 daemons emit '2031-subscribe' but have no '2031-unsubscribe' fact at all. For a gate-managed pane, main drops the renderer-bound bytes before the renderer sees them, so main's transient facts are the ONLY thing that can retire a subscription. Against such a daemon a TUI exiting while its pane is hidden leaves the subscription registered forever, and the next theme flip injects CSI 997 into whatever shell replaced it -- #9993 all over again, reached through the upgrade path. Gate it: bump PROTOCOL_VERSION 28 -> 29, add MODE_2031_UNSUBSCRIBE_FACT_PROTOCOL_VERSION with supportsMode2031UnsubscribeFact(), and drop '2031-subscribe' from any daemon below that floor. Trade-off: a gate-managed pane on a preserved v28 daemon keeps renderer-scanner authority instead of daemon-fact authority. That is exactly the pre-fact behaviour -- correct for visible panes, no worse than today for hidden ones -- and it resolves on the daemon's next restart. Non-2031 transient facts (bell, etc.) are unaffected at every version. Tests: two adapter regression tests (v28 drops subscribe, v29 forwards it), plus a version-pin test asserting the floor sits above every entry in PREVIOUS_DAEMON_PROTOCOL_VERSIONS -- so adding a new preserved version cannot silently re-open the hole. Mutation-verified in both directions: `false &&` (under-block) and `true` (over-block) each fail the new tests.
A pre-v29 daemon can announce a 2031 subscribe but never retract it. Filtering that fact is not enough: while a pane is visible main's own scanner registers the subscription, and scan authority only moves to the daemon when the session is backgrounded. So the gate belongs on setPtyBackgrounded — decline to hand a non-retracting daemon authority at all, and main stays authoritative over the whole stream. Co-authored-by: Orca <help@stably.ai>
… just at background Co-authored-by: Orca <help@stably.ai>
…es a chunk Review found the chunk-final-state fix left one hole open. When the kernel cuts fish's toggle pair mid-withdrawal — chunk 1 ends "...?2031h prompt ESC[?20", chunk 2 is "31l" — chunk 1 genuinely ends subscribed, so it answers, and the reply lands as literal text at the prompt. Chunk 2 then recognizes the withdrawal but cannot recall bytes already written. The same byte stream is safe or corrupting purely by where the kernel split it. The scanner already retains an incomplete private-mode tail; it just didn't tell the caller whether that tail could still resolve to 2031. It now does, and a subscribe is held one chunk while the answer is still in doubt. Only subscribes defer — retiring a subscription writes nothing to the pty, so withdrawals stay eager. Deferral is narrow: a trailing "ESC[?25" (cursor hide) can never become 2031, so a subscribe already seen in that chunk is still answered immediately. This case predates the branch — the old sticky-flag policy replied here too — so it is a residual this fix now closes rather than a regression it introduced. Tests: three cases pinned (split withdrawal, non-2031 partial must not defer, split re-subscribe answers once). Removing the deferral fails only the first.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change introduces chunk-final Mode 2031 reply decisions, including split-sequence handling and unsubscribe facts. Daemon protocol version 29 adds capability gating for unsubscribe support and carries provisional scan state across background handoffs. PTY dropped-output handling preserves and reconstructs Mode 2031 data. Renderer processing scans live chunks, applies unsubscribe facts, removes retry-based seed replies, and resets state across PTY replacement. Tests cover daemon compatibility, buffering, handoffs, parked terminals, hidden panes, restoration, and chunk boundaries. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/main/daemon/daemon-protocol-version.ts (1)
22-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider deriving
PREVIOUS_DAEMON_PROTOCOL_VERSIONSinstead of hand-maintaining it.Each protocol bump requires manually appending the new value to this literal array; forgetting it silently breaks any
.includes()/.toContain()checks. This PR's own test file already computes the equivalent sequence generically (Array.from({ length: 28 }, (_, index) => index + 1)), which could replace the literal here too.♻️ Proposed refactor
-export const PREVIOUS_DAEMON_PROTOCOL_VERSIONS = [ - 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, - 28 -] as const +export const PREVIOUS_DAEMON_PROTOCOL_VERSIONS = Array.from( + { length: PROTOCOL_VERSION - 1 }, + (_, index) => index + 1 +)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7fc49a60-363d-4813-b5ef-258dd05e6a26
📒 Files selected for processing (26)
src/main/daemon/daemon-background-transient-facts.tssrc/main/daemon/daemon-foreground-confirmation-protocol.test.tssrc/main/daemon/daemon-protocol-version.test.tssrc/main/daemon/daemon-protocol-version.tssrc/main/daemon/daemon-pty-adapter.test.tssrc/main/daemon/daemon-pty-adapter.tssrc/main/daemon/daemon-stream-events.tssrc/main/daemon/types.tssrc/main/providers/pty-provider-events.tssrc/main/runtime/orca-runtime.tssrc/renderer/src/components/terminal-pane/parked-terminal-mode2031-responder.test.tssrc/renderer/src/components/terminal-pane/parked-terminal-mode2031-responder.tssrc/renderer/src/components/terminal-pane/pty-connection.test.tssrc/renderer/src/components/terminal-pane/pty-connection.tssrc/renderer/src/components/terminal-pane/terminal-appearance.test.tssrc/renderer/src/components/terminal-pane/terminal-appearance.tssrc/renderer/src/components/terminal-pane/terminal-mode-2031-replies.test.tssrc/renderer/src/components/terminal-pane/terminal-mode-2031-replies.tssrc/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.tssrc/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.tssrc/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.tssrc/shared/terminal-color-scheme-protocol.tssrc/shared/terminal-mode-2031-final-state.test.tssrc/shared/terminal-output-side-effects.test.tssrc/shared/terminal-output-side-effects.tssrc/shared/terminal-side-effect-facts.ts
💤 Files with no reviewable changes (2)
- src/renderer/src/components/terminal-pane/terminal-mode-2031-replies.test.ts
- src/renderer/src/components/terminal-pane/terminal-mode-2031-replies.ts
The red
|
Independent verification of the four open bot findings (Opus sub-agents + A/B against
|
| chunks | origin/main / HEAD~1 |
this PR |
|---|---|---|
ESC[?2031h hello ESC[ + 31m world |
['subscribe'] |
[] ← lost |
origin/main emits it and this PR does not, so this is a genuine regression introduced by the deferral mechanism itself.
Severity, stated honestly: it is self-healing in the common case. fish re-toggles 2031 around every prompt, so the very next prompt re-subscribes:
['?2031h', 'ESC[', '31m']→ lost['?2031h', 'ESC[', '31m', '?2031h']→ recovered
The unbounded case is a chunk whose CSI never terminates, which strands the subscribe indefinitely. So: real, but a missed color-scheme update for one prompt cycle, not a permanent wedge.
🟡 CLAIM: leading-zero normalization — CONFIRMED REAL, but PRE-EXISTING (not a regression)
The asymmetry is exactly as described: hasMode2031() uses Number(param) === 2031 so it accepts 02031, while tailCouldStillBeMode2031() uses '2031'.startsWith(segment) so it rejects 0203. A padded split withdrawal therefore emits a reply that the next bytes retract.
But HEAD~1 and HEAD behave identically on this input (['subscribe','unsubscribe'] both). This PR neither introduced nor worsened it. Worth fixing — one-line normalization on the trailing segment — but it is not a merge blocker for this diff.
🟡 CLAIM: observeLiveMode2031Chunk bypassed by the droppedOutput early return — CONFIRMED REAL, PRE-EXISTING
The mechanism checks out end-to-end. Driving a real pane, a droppedOutput sentinel carrying ESC[?2031h produces no reply — the call at pty-connection.ts:7067 sits after the return at 7066. And the salvage path genuinely cannot rescue it: extractDroppedPtyQueryBytes strips 2031 entirely (verified — a bare ?2031h salvages to '', while a control ESC[6n correctly survives, so the carver does work; 2031 just isn't in its grammar).
But origin/main has the same ordering — its respondToSkippedMode2031Subscribe(data) call at line 7085 is likewise below the droppedOutput return at 7066. Pre-existing, already disclosed on this PR, correctly left out of scope.
🟠 NEW — found during this pass, not reported by the bot: the fix doesn't reach the parked responder
The headline scenario this PR fixes (?2031h … ESC[?20 | 31l) still reproduces through startParkedTerminalMode2031Responder — it writes ?997;1n into the pty after the shell already withdrew, which is issue #9993's exact symptom. Control case in the same probe confirms a same-chunk toggle pair is suppressed there, so this is specifically the cross-chunk deferral that path lacks.
Reachability is limited: that responder only starts when hiddenDeliveryGateActive is false, i.e. a remote/SSH runtime (runtimeEnvironmentId !== null forces main authority off) or an explicitly flipped kill switch. Both gates default on. So this is a degraded-path gap, not the default desktop path — but given #9993 was itself reported against real shell usage, and SSH is a first-class Orca use case, it's worth a follow-up.
On the two Minor daemon-pty-adapter.ts threads
Both are the bot re-running failed shell probes (Length of output: 151) rather than substantive findings. No action.
Verdict
Not mergeable as-is — one confirmed regression against origin/main (the lost subscribe). Everything else is either pre-existing or out of scope.
Notably, all 25 committed tests pass on this head, including the ones added by this PR. The regression lives in a coverage hole: the suite exercises split-withdrawal and same-chunk cases but never "deferred subscribe followed by an unrelated CSI." Green CI did not catch this, which is the main reason I'd hold the merge.
Heads-up: a fix for exactly this is already in flight in a working copy on this branch — a scanMode2031ReplyDecision that carries a pendingSubscribe bit across chunks. I tested that in-flight version against all four cases plus edges (unterminated-CSI hold/release, no duplicate subscribes, semicolon-list withdrawal, split subscribe across the boundary) and it passes all of them, and takes the committed suite 25 → 28. It is uncommitted and unpushed, so it isn't reflected in this PR's head yet.
Verification was read-only against a pinned detached worktree; nothing here was pushed, and no PR was merged.
Re-verified against the new head
|
| scenario | origin/main |
55423d6017 |
d0f74a1b28 |
|---|---|---|---|
deferred subscribe + unrelated CSI (?2031h … ESC[ | 31m) |
subscribe |
[] ← lost |
subscribe ✅ |
leading-zero split withdrawal (?2031h … ?0203 | 1l) |
sub,unsub |
sub,unsub |
unsub ✅ |
headline #9993 split withdrawal (?2031h … ?20 | 31l) |
sub,unsub |
unsub ✅ |
unsub ✅ |
| plain subscribe | subscribe |
subscribe ✅ |
subscribe ✅ |
So the new commit closes both the regression I confirmed and the pre-existing leading-zero asymmetry — the latter wasn't strictly required to merge, since HEAD~1 shared it, but it's genuinely fixed rather than papered over.
The parked-responder gap is also closed
I reported a fourth issue the bot didn't catch: the same split withdrawal still leaked ?997;1n into the pty through startParkedTerminalMode2031Responder — the path used on remote/SSH runtimes and when the hidden-delivery kill switch is off. That path now correctly emits nothing on a split withdrawal, with the control case (plain subscribe still answered) confirming the responder wasn't simply disabled.
Test state
521 tests green across terminal-mode-2031-final-state, terminal-output-side-effects, and pty-connection at this head. The committed 2031 suite went 25 → 28, and the new cases cover exactly the hole that let the regression through: previously every test exercised same-chunk or split-withdrawal sequences, none exercised "deferred subscribe followed by an unrelated CSI."
The full CI matrix is currently running on this head (16-way test shard, both native-smoke jobs, crash-survival, Windows terminal restart, wayland input) — worth waiting for green before merging, since my verification is unit-level.
What remains open (unchanged, and correctly out of scope)
The droppedOutput early return at pty-connection.ts:7066 still bypasses the 2031 observation, and extractDroppedPtyQueryBytes doesn't carry 2031 bytes through the salvage path. I re-confirmed this is pre-existing — origin/main's equivalent call sits below the same return — so it belongs in its own issue, not this diff.
Verdict: mergeable on the merits, pending the CI matrix. No regressions against origin/main remain. As instructed, I have not merged anything.
CI on
|
|
Post-
A fresh CI run is now attached to the updated head. |
Re-verified at
|
| scenario | origin/main |
94afa841fd |
|---|---|---|
deferred subscribe + unrelated CSI (?2031h … ESC[ | 31m) |
sub |
sub ✅ |
leading-zero split withdrawal (?2031h … ?0203 | 1l) |
— | unsub ✅ |
headline #9993 split withdrawal (?2031h … ?20 | 31l) |
— | unsub ✅ |
| plain subscribe | sub |
sub ✅ |
Parked/SSH responder — also re-checked at this head: a split withdrawal writes nothing into the pty, and the control case confirms a plain subscribe is still answered (i.e. the path wasn't just switched off).
Verdict
Mergeable once the remaining matrix jobs report green. No regressions against origin/main at the current head; the one real CI failure was this PR's own contract drift and it is fixed and verified.
The droppedOutput bypass at pty-connection.ts:7066 remains open and remains correctly out of scope — origin/main's equivalent call sits below the same return, so it is pre-existing and belongs in its own issue.
All verification was read-only. I have merged nothing and pushed nothing.
…ithdrew (stablyai#10817) * fix(terminal): stop answering DECSET 2031 subscriptions fish already withdrew fish enables and disables mode 2031 around every prompt (tty_handoff.rs), so a single PTY chunk routinely carries `?2031h ... ?2031l`. All three responders answered the sticky "an h appeared anywhere" flag, so each prompt cycle wrote `?997;1n` into a shell that had already handed the tty to a child — it lands as literal text, or as stdin for whatever is reading. pty-connection.ts's hidden-pane responder already had the right shape (`finalState !== 'subscribed'`); this brings the other three in line: - shared tracker: gate the '2031-subscribe' fact on the chunk-final state - parked-tab byte sidecar: same guard - visible-pane xterm CSI handler: xterm dispatches mid-parse, so there is no chunk-final state to read. Defer the reply to a microtask and re-check the subscription, letting a same-chunk `?2031l` cancel it. Refs stablyai#9993 Co-authored-by: Orca <help@stably.ai> * fix(terminal): decide 2031 replies per PTY chunk, not per xterm parse The previous commit deferred the visible-pane reply to a microtask so a same-chunk `?2031l` could cancel it. That cannot work: xterm's WriteBuffer parses every queued `terminal.write()` synchronously in one batch before any microtask runs, so the microtask sees the net state of N PTY chunks, not of the one that carried the subscribe. A TUI that subscribes in chunk N gets no reply when chunk N+1 happens to withdraw, and a fish prompt straddling two writes still gets answered. Move the decision to where chunk boundaries actually exist — pty-connection's dataCallback, which receives one PTY chunk per call. It scans raw bytes with `scanMode2031Sequences`, carrying a tail across chunks so a CSI split mid- sequence still resolves, and replies only when that chunk *ends* subscribed. Ownership stays single: gate-managed PTYs are answered by main's '2031-subscribe' fact, so the chunk scanner returns early for them, and the xterm CSI handler now observes only panes the scanner does not own. The tail is dropped on PTY replacement — a partial prefix belongs to the stream that produced it. Removes the microtask responder and the seed-reply retry path it needed. Mutation-tested: 6 mutations applied, 6 killed. * fix(terminal): carry DECSET 2031 withdrawals as a side-effect fact The previous commit moved 2031 reply decisions to the PTY chunk boundary and gave gate-managed panes a single owner: main's '2031-subscribe' fact. But the fact union is subscribe-only, and that left the withdrawal unobserved. For a gate-managed pane, main drops renderer-bound bytes after model ingestion, the chunk scanner early-returns, and xterm's CSI handler is disabled. So when a TUI emits `?2031l` while hidden, nothing retires the subscription: paneMode2031 stays set, and the next theme flip has maybePushMode2031Flip push `CSI ?997;2n` into the shell that replaced the TUI — stablyai#9993 again, through the theme-change door. Before this branch, skipHiddenRendererOutput observed those withheld bytes; consolidating ownership removed that observer without replacing it. No renderer-side observer can close this: the bytes are gone before the renderer sees them. The state protocol has to carry the withdrawal, so add a '2031-unsubscribe' fact alongside the subscribe across the three fact unions (shared, provider, daemon). It fires only on a real chunk-final withdrawal — a chunk with no 2031 bytes scans to null and stays silent. The renderer handler clears both maps and sends nothing: a withdrawal is not a query. Also closes two gaps an adversarial review found by mutation, both previously resting on comments rather than tests: the lifecycle parser-ownership predicate (extracted as isPaneParserOwnedMode2031Observer so it is directly testable) and the scan-before-reconciliation ordering that lets a chunk the snapshot drops as a duplicate still answer its query. Mutation-tested: 12 mutations applied, 12 killed (6 from the prior round re-run, 6 new covering this fix and the two survivors). * fix(daemon): refuse 2031 authority from a daemon that cannot retract it Round-2 review found a wire-compatibility hole in the original stablyai#9993 fix. Daemons survive app updates, so a new desktop can drive a daemon that was started by the previous build. Pre-v29 daemons emit '2031-subscribe' but have no '2031-unsubscribe' fact at all. For a gate-managed pane, main drops the renderer-bound bytes before the renderer sees them, so main's transient facts are the ONLY thing that can retire a subscription. Against such a daemon a TUI exiting while its pane is hidden leaves the subscription registered forever, and the next theme flip injects CSI 997 into whatever shell replaced it -- stablyai#9993 all over again, reached through the upgrade path. Gate it: bump PROTOCOL_VERSION 28 -> 29, add MODE_2031_UNSUBSCRIBE_FACT_PROTOCOL_VERSION with supportsMode2031UnsubscribeFact(), and drop '2031-subscribe' from any daemon below that floor. Trade-off: a gate-managed pane on a preserved v28 daemon keeps renderer-scanner authority instead of daemon-fact authority. That is exactly the pre-fact behaviour -- correct for visible panes, no worse than today for hidden ones -- and it resolves on the daemon's next restart. Non-2031 transient facts (bell, etc.) are unaffected at every version. Tests: two adapter regression tests (v28 drops subscribe, v29 forwards it), plus a version-pin test asserting the floor sits above every entry in PREVIOUS_DAEMON_PROTOCOL_VERSIONS -- so adding a new preserved version cannot silently re-open the hole. Mutation-verified in both directions: `false &&` (under-block) and `true` (over-block) each fail the new tests. * fix(daemon): gate background delegation, not just the fact stream A pre-v29 daemon can announce a 2031 subscribe but never retract it. Filtering that fact is not enough: while a pane is visible main's own scanner registers the subscription, and scan authority only moves to the daemon when the session is backgrounded. So the gate belongs on setPtyBackgrounded — decline to hand a non-retracting daemon authority at all, and main stays authoritative over the whole stream. Co-authored-by: Orca <help@stably.ai> * fix(daemon): clear a preserved pre-v29 background hint at attach, not just at background Co-authored-by: Orca <help@stably.ai> * fix(terminal): don't answer a 2031 subscribe whose withdrawal straddles a chunk Review found the chunk-final-state fix left one hole open. When the kernel cuts fish's toggle pair mid-withdrawal — chunk 1 ends "...?2031h prompt ESC[?20", chunk 2 is "31l" — chunk 1 genuinely ends subscribed, so it answers, and the reply lands as literal text at the prompt. Chunk 2 then recognizes the withdrawal but cannot recall bytes already written. The same byte stream is safe or corrupting purely by where the kernel split it. The scanner already retains an incomplete private-mode tail; it just didn't tell the caller whether that tail could still resolve to 2031. It now does, and a subscribe is held one chunk while the answer is still in doubt. Only subscribes defer — retiring a subscription writes nothing to the pty, so withdrawals stay eager. Deferral is narrow: a trailing "ESC[?25" (cursor hide) can never become 2031, so a subscribe already seen in that chunk is still answered immediately. This case predates the branch — the old sticky-flag policy replied here too — so it is a residual this fix now closes rather than a regression it introduced. Tests: three cases pinned (split withdrawal, non-2031 partial must not defer, split re-subscribe answers once). Removing the deferral fails only the first. * fix(terminal): preserve mode 2031 reply decisions * fix(build): record daemon protocol v29 compatibility --------- Co-authored-by: Orca <help@stably.ai>
Closes #9993
Summary
Fish enables and disables DEC private mode 2031 around each prompt. Orca replied as soon as it observed a subscribe, even when the same logical output withdrew it before the reply was safe. The late
CSI ?997;1nthen became literal shell input or stdin for the next interactive program.This PR makes the reply decision stateful and final-state based across visible panes, parked panes, main-side facts, pending-output drops, and daemon scan-authority handoffs.
Reproduction
The bug is reproducible deterministically at the raw PTY chunk boundary:
Before the fix, both the visible and parked responders sent a color-scheme reply after chunk 1. The shared fact scanner also lost a deferred subscribe when an ambiguous tail resolved to a different mode, and
CSI ?25was incorrectly treated as unable to append;2031l.Red phase:
Fix
;2031; apply withdrawals eagerly.Related work
Testing
pnpm typecheck:nodepnpm typecheck:weboxlintoxfmt --checkpnpm check:max-lines-ratchetpnpm check:reliability-gatesgit diff --checkAI review report
Review findings addressed:
?25;2031lwere misclassified;Screenshots
Not applicable; this is terminal protocol behavior covered by byte-level regressions.
Security audit
No new command execution, filesystem access, network surface, or user-controlled parsing beyond the existing bounded terminal escape scanner.
Notes
The only added latency is while an incomplete private-mode sequence remains ambiguous. The retained tail is capped at 128 characters. Ordinary output stays on an allocation-free fast path.
The implementation is platform-neutral and uses the same shared scanner for local, SSH, worktree, and folder-workspace terminal flows.
Made with Orca 🐋