fix(terminal): let the daemon own what a program has negotiated about the keyboard (#290) - #346
Merged
Merged
Conversation
10 tasks
… the keyboard (#290) Terminal scrolling dies after a project switch and only a window resize brings it back. Underneath the symptom is a belief rather than a pixel. `use-terminal.ts` decides on every keydown: const programOwnsKeyboard = kittyKeyboardActive(kitty) || altBuffer; and while that is wrongly true on the normal buffer, Ctrl+Home and Ctrl+End stop being reserved for scrollback and plain PageUp/PageDown skip the `term.scrollPages(...)` branch that is gated on it. Two of the three routes in the report die together, from one boolean. THE BELIEF DID NOT GO STALE BY BEING MISSED. IT WAS COUNTED TWICE. Rebuilding a view restored the saved state from `keyboard-mode-store` and then replayed the daemon's scrollback tail — which still contains the very sequences that produced that state, verbatim, since `appendScrollback` preserves control bytes. The kitty protocol is a STACK: `CSI > flags u` pushes and `CSI < n u` pops. Two pushes against one pop leaves it enabled, so when the program finally turned the protocol off, its pop only cancelled the duplicate. That also accounts for the recovery behaviour the report describes and nothing else explained. A repaint cannot help, because the belief is not in the pixels. A resize helps every time because — as the reset comment in this same file already records — programs re-assert their setup after one, and a mounted view parses the re-negotiation live. THE FIX IS ONE AUTHORITY INSTEAD OF TWO DERIVATIONS - core gains `scanKeyboardNegotiation`, which reads the negotiation out of the raw output stream. It is the sibling of `trackAltScreen` and exists for the same reason: the daemon sees every byte whether or not a view is mounted, and a panel in a background tab is unmounted. Unlike `trackAltScreen` it carries an unterminated trailing sequence across chunk boundaries, because a lost `1049` self-corrects on the next screen switch while a lost pop is permanent. - the daemon tracks it per session and returns it on attach, beside `altScreen`. - the view ADOPTS that answer instead of reconstructing it, and mutes the negotiation handlers for the span of the replayed tail so the replay cannot re-apply what the daemon already accounted for. The replay is paint; the daemon is truth. `keyboard-mode-store` keeps a narrower job — the value a view starts from before the attach resolves — and its comment now says so rather than claiming to be the authority. BOTH HALVES ARE LOAD-BEARING, MEASURED Each was disabled in turn, rebuilt, and the regression test re-run: adoption of res.keyboard disabled RED replay suppression disabled RED The first pass of that A/B is worth recording, because it found a hole in the test rather than in the code: with suppression disabled the test still PASSED, since a tail holding both a push and its pop replays balanced and the flag lands right by luck. What it gets wrong is the stack DEPTH, and nothing observable goes wrong until the program pops again — one program exit later than anyone would think to look. The test grew a second phase that negotiates, rebuilds, and then pops live; that phase is what pins the suppression. No user-visible behaviour changes except the defect: a terminal whose program has finished with the keyboard now gives its scrollback keys back.
…the flag behind them (#290) The reporter, on the frozen state: "it was CTRL+Home / End, PGUP / PGDN etc, I did not exhaustively test control keys, but typing in to the prompt worked OK." The regression test asserted `Ctrl+Home` and the `programOwnsKeyboard` flag underneath it. Every other route dies from that same flag, so the test went red for the right reason — but that is an argument, not coverage, and the argument is exactly the kind that turns out to have an exception in it. So each route is now asserted as the user would see it: - Ctrl+Home reaches the top of the scrollback - Ctrl+End returns to the live bottom - plain PageUp moves off the live bottom, plain PageDown brings it back (plain, not Shift+ — that is the pair `use-terminal.ts` gates on `!programOwnsKeyboard`) - typing still reaches the shell That last one is the reporter's own control and it earns its place: a terminal frozen this way still accepts input, so if typing were broken too, the failure would be a different defect and this test would be pointing at the wrong one. The observable assertions now come BEFORE the diagnostics read, which changes what a failure says. Disabling each half of the fix in turn to confirm it is load-bearing: adoption of res.keyboard disabled Error: Ctrl+Home did not reach the top of the scrollback replay suppression disabled Error: one pop did not undo one push - the rebuilt view was carrying a duplicate on its negotiation stack The first of those is the sentence the person who filed the issue would recognise. Previously the same run reported that an internal boolean had the wrong value, which is true, less useful, and one inference away from the thing that actually happened to them. No production change; the fix is unaltered.
Bidthedog
force-pushed
the
feature/I290-keyboard-negotiation-authority
branch
from
August 27, 2026 14:49
f3bbe93 to
2751506
Compare
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked issue
Related to #290. Deliberately not a closing reference — see What is still open below, which is
the part worth reading before this is merged and the issue ticked off.
for [Observation] Terminal scrolling dies after a project switch until the window is resized #290 to be worked, and chose this approach over the two alternatives offered.
bug, plusarea:terminalandarea:projects.Intent (what & why, in human terms)
#290: after throng has been running a while, switching projects leaves a terminal that accepts input
and paints new output but will not scroll by any route — wheel,
PageUp/PageDown,Ctrl+Home/Ctrl+End. A window resize fixes it every time;Ctrl+F5never does.Two sessions had looked at this and could not reproduce it. What follows is a reproduction, a root
cause, and a fix — but not, honestly, a complete account of every symptom in the report.
Outcomes (what changed, in human terms)
A terminal whose program has finished with the keyboard gives the scrollback keys back.
The root cause: the negotiation was applied twice, not missed
The symptom is a belief, not a pixel.
use-terminal.tsdecides on every keydown:While that is wrongly
trueon the normal buffer,Ctrl+Home/Ctrl+Endare no longer reserved forscrollback and plain
PageUp/PageDownskip theterm.scrollPages(...)branch, which is gated on!programOwnsKeyboard. Two routes die from one boolean.The interesting part is why the boolean is stale, because it is not the obvious reason. Rebuilding
a view restored the saved state from
keyboard-mode-storeand then replayed the daemon'sscrollback tail — which still contains the very sequences that produced that state, verbatim,
because
appendScrollbackpreserves control bytes. The kitty protocol is a stack:CSI > flags upushes,CSI < n upops. Two pushes against one pop leaves it enabled, so when theprogram finally turned the protocol off, its pop only cancelled the duplicate.
That also explains the recovery behaviour nothing else did. A repaint cannot help, because the belief
is not in the pixels. A resize helps every time because — as this file's own reset comment already
records — programs re-assert their setup after one, and a mounted view parses the re-negotiation
live.
How it was pinned down
Four E2E probes, each differing from the last by one variable, reading
window.__throngTerminalDiagnostics()after a realCtrl+Home. A fixture Node program negotiates oncue, so nothing depends on catching a real Claude Code session in the act.
kittyprogramOwnsKeyboardreservedfalsefalsetruetruetruefalsetruetruefalsefalsefalsetruethe pop. So the pop is well-formed and the unmount is what breaks it.
MAX_SCROLLBACK-ageing theory I had posted on the issue earlier the same day; I was wrong, and Dis what showed it.
__throngLastReplayByteswas 12,460, so the tail was replayed and didcontain the pop.
because the first is eaten by the duplicate push.
The fix: one authority instead of two derivations
scanKeyboardNegotiation, reading the negotiation out of the raw output stream. Itis the sibling of
trackAltScreenand exists for the same reason — the daemon sees every bytewhether or not a view is mounted. Unlike
trackAltScreenit carries an unterminated trailingsequence across chunk boundaries: a lost
1049self-corrects on the next screen switch, whereas alost pop is permanent and would silently re-create this very defect.
altScreen.for the span of the replayed tail. The replay is paint; the daemon is truth.
keyboard-mode-storekeeps a narrower job — the value a view starts from before the attach resolves— and its comment now says so instead of claiming to be the authority.
Both halves are load-bearing, and that was measured rather than assumed
Each was disabled in turn, rebuilt, and the regression test re-run:
The observable assertions run before the diagnostics read, deliberately, and that is what those
messages are. A red used to say an internal boolean had the wrong value — true, less useful, and one
inference away from what actually happened to the person who filed the issue.
The first run of that A/B is the part worth keeping, because it found a hole in the test rather
than in the code: with suppression disabled the test still passed. A tail holding both a push and
its pop replays balanced, so the flag lands right by luck — what it gets wrong is the stack depth,
and nothing observable goes wrong until the program pops again, one program exit later than anyone
would think to look. The test grew a second phase that negotiates, rebuilds, then pops live. That
phase is what pins the suppression, and without the A/B it would have shipped as dead code with a
confident comment on it.
Every route the report names is asserted, as the user would see it
The reporter, on the frozen state:
All of those die from the one boolean, so asserting the boolean would have been enough to make the
test go red. That is an argument, not coverage, so each is asserted directly:
Ctrl+HomeTOP_OF_HISTORYCtrl+EndPageUp/PageDownShift+, because that is the pairuse-terminal.tsgates on!programOwnsKeyboardecho STILL_ALIVEstill reaches the shell"Typing worked OK" is not a throwaway detail — it is the signature. A stale
programOwnsKeyboardaffects the reserved chords and the pager keys and nothing else; ordinary characters never consult
it. Scroll dead, typing fine is exactly what this defect looks like and is what distinguishes it from
a wedged or disconnected terminal. It is asserted for that reason, as the reporter's own control.
What is still open — please read before closing #290
The routes above are fixed and proven. The WHEEL is not addressed, and I will not claim it is.
decideWheelkeys offaltBufferandmouseReporting, not offprogramOwnsKeyboard, so nothing inthis change touches the wheel route. Either it dies by a second mechanism —
mouseReportingisrebuilt from the replayed tail and is not persisted at all, which is the same family of bug from a
different direction — or those sessions also had the alternate screen or DEC mouse modes in play.
So this PR is
Related to, notCloses. The check worth doing with a real session in front of you:when it next happens, confirm whether the wheel alone is still dead once the keyboard chords
work. If it is, that is a follow-up issue and I would take it.
Testing (observed passing — paste/attach evidence)
npm run test:unit— greennpm run test:integration— greennpm run test:contract— greennpm run test:e2e— greenfile explains why nothing cheaper can hold it: the double-count needs a real daemon with a
scrollback tail, a view genuinely torn down and rebuilt, and a program emitting negotiation while
no view exists to parse it. A component test would pass with the defect present.
scanKeyboardNegotiationis pure and unit-tested in core (12 cases, including every split offset of a push/pop pair).
@admin/ elevation-dependent behaviour verified under an elevated run — n/a.Test output
Full
npm run gate— all eight stages, no flaky retries (205 parallel + 354 serial E2E tests):The core parser on its own — 12 cases, including every split offset of a push/pop pair, so a PTY
chunk boundary cannot swallow a pop:
Red before, green after, on the same test and the same command:
Documentation
README.md— n/a: no user-facing behaviour, setup, architecture or capability changedbeyond the defect itself.
docs/— n/a, and checked rather than assumed: no guide documents the keyboardnegotiation (
grep -rln "kitty\|negotiat" docs/returns nothing). The explanation lives where thenext reader will be standing —
negotiation-scan.ts,keyboard-mode-store.ts, and the spec — andinventing a docs page for it would be inventing a surface the repo does not have.
CONTRIBUTING.md— n/a, no process, toolchain, testing bar or setup change.Constitution compliance
configuration (X) are respected. Principle II is the one this change is really about: the parsing
is a pure function in
@throng/corewith no OS or DOM in it, the daemon owns the stream, and therenderer is told rather than inferring. Principle V is respected in the direction that costs
something — the parser is unit-tested and only the wiring is E2E.
Toolchain & review
Claude Opus 4.8 or a more capable model. Model used:
claude-opus-5AI Code -> Spec Kit artifacts
Spec directory: none — a defect fix with a reproduction, not spec work, so the branch carries no
Ssegment and creates nospecs/NNN-*/. The checklist below is n/a for that reason rather thanskipped.
spec.mdis fully specified — n/a/speckit-clarifywas run — n/a/speckit-planproducedplan.md,research.md,data-model.md,contracts/— n/a/speckit-tasksproduced a dependency-orderedtasks.md— n/a/speckit-analyzewas run — n/a/speckit-superpowers-bridge— n/a/speckit-convergewas run — n/aprobes, the red/green pair, the two-way A/B, and the full gate.
Licensing