Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,9 @@ export {
type KeyChord,
appendScrollback,
trackAltScreen,
createNegotiationScan,
scanKeyboardNegotiation,
type NegotiationScan,
MOUSE_REPORTING_MODES,
createMouseReportingState,
decideWheel,
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/terminal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ export {
type WheelRoute,
} from './wheel-decision.js';
export { trackAltScreen } from './alt-screen.js';
export {
createNegotiationScan,
scanKeyboardNegotiation,
type NegotiationScan,
} from './negotiation-scan.js';
export { encodeModifiedKey, kittyReportsAllKeys, KITTY_REPORT_ALL_KEYS } from './kitty-keyboard.js';
export {
terminalReloadAction,
Expand Down
129 changes: 129 additions & 0 deletions packages/core/src/terminal/negotiation-scan.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import {
applyDecPrivateMode,
applyKittyCsi,
createKittyKeyboardState,
type KittyCsiPrefix,
type KittyKeyboardState,
} from './kitty-keyboard.js';

/**
* Track a terminal program's KEYBOARD NEGOTIATION from the raw output stream (#290).
*
* The renderer has always derived this by parsing the program's output through xterm's CSI handlers,
* which works exactly as long as a view exists. It does not while a panel is unmounted — a
* background tab, or a project being switched away from — and the program has no reason to wait: it
* negotiates when it starts and un-negotiates when it exits, whether or not anyone is watching.
*
* The daemon sees every byte regardless, which is the property this needs. It already tracks the
* ALTERNATE SCREEN from the same stream for the same reason ({@link trackAltScreen}); this is the
* other half of what a rebuilt view has to be told rather than left to infer.
*
* ══ WHY INFERRING IT WAS WORSE THAN MISSING IT ══
*
* The failure this fixes is not a missed negotiation. It is a DOUBLE-COUNTED one. Rebuilding a view
* restored the saved state from the renderer's panel store and then replayed the daemon's scrollback
* tail — which still contains the very sequences that produced that state, verbatim, because
* `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, and throng went on believing a
* program wanted enhanced key reporting after it had said otherwise.
*
* Downstream that boolean is `programOwnsKeyboard`, and while it is wrongly true on the normal
* buffer the scrollback chords are handed to the program: Ctrl+Home and Ctrl+End stop being
* reserved, and plain PageUp/PageDown skip the viewport branch that is gated on it. That is #290's
* "every scroll route dies at once", from one boolean.
*
* So the answer is a single authority rather than two derivations. The daemon tracks it here, hands
* it over on attach, and the view adopts it instead of reconstructing it.
*
* ══ SEQUENCES SPLIT ACROSS CHUNKS ══
*
* A PTY chunk boundary can fall inside an escape sequence, and a `CSI < u` that arrives as `CSI <`
* then `u` would simply not be seen — which would silently re-create the very defect this exists to
* fix, in a form nothing would ever reproduce. So an incomplete trailing sequence is carried to the
* next call rather than dropped. {@link trackAltScreen} does not do this and is not being changed
* here: a lost `1049` self-corrects on the program's next screen switch, whereas a lost pop is
* permanent until the program re-negotiates.
*/

/** How much unterminated trailing escape text is worth carrying. A CSI parameter list is short; */
/** anything longer is malformed and holding it would grow without bound. */
const MAX_PENDING = 64;

/** ESC, and the CSI introducer that follows it. */
const ESC = '\x1b';

/**
* One CSI sequence: the introducer, an optional private-marker prefix, numeric parameters, and the
* final byte. Only `u` (kitty) and `h`/`l` (DEC private modes) are acted on; every other final is
* matched so that the scan steps over it rather than mistaking its bytes for a later sequence.
*/
// This matches a CONTROL sequence, so it necessarily contains a control character — the same
// exemption `alt-screen.ts` takes for the same reason.
// eslint-disable-next-line no-control-regex
const CSI = /\u001b\[([?=><])?([0-9;]*)([A-Za-z])/g;

/** The kitty CSI-u private markers throng honours — the same set the renderer registers. */
const KITTY_PREFIXES = new Set<string>(['?', '=', '>', '<']);

export interface NegotiationScan {
/** What the program has negotiated, after this chunk. */
readonly state: KittyKeyboardState;
/** An unterminated trailing escape sequence, to be prepended to the next chunk. */
readonly pending: string;
}

/** A fresh scan: nothing negotiated, nothing half-read. */
export function createNegotiationScan(): NegotiationScan {
return { state: createKittyKeyboardState(), pending: '' };
}

/** `"1;2"` → `[1, 2]`; `""` → `[]`. Empty positions read as 0, as a terminal's parser does. */
function params(raw: string): number[] {
if (raw === '') return [];
return raw.split(';').map((p) => (p === '' ? 0 : Number.parseInt(p, 10)));
}

/**
* Apply every complete negotiation sequence in `chunk`, in the order the program emitted them.
*
* Order is load-bearing and is why this is a scan rather than a last-match-wins search like
* {@link trackAltScreen}: pushes and pops only mean anything in sequence.
*/
export function scanKeyboardNegotiation(previous: NegotiationScan, chunk: string): NegotiationScan {
const text = previous.pending + chunk;
let state = previous.state;
let consumedTo = 0;

CSI.lastIndex = 0;
for (let m = CSI.exec(text); m !== null; m = CSI.exec(text)) {
consumedTo = m.index + m[0].length;
const prefix = m[1] ?? '';
const final = m[3] ?? '';
if (final === 'u') {
if (!KITTY_PREFIXES.has(prefix)) continue; // a bare `CSI u` is not ours
// The `?` query's REPLY belongs to the view, which answers the program itself. Here the
// sequence is only evidence about state, and `applyKittyCsi` leaves state untouched for it.
state = applyKittyCsi(state, prefix as KittyCsiPrefix, params(m[2] ?? '')).state;
} else if ((final === 'h' || final === 'l') && prefix === '?') {
state = applyDecPrivateMode(state, params(m[2] ?? ''), final === 'h');
}
}

return { state, pending: trailingPartial(text, consumedTo) };
}

/**
* The unterminated escape sequence at the end, if any.
*
* Searching from the last COMPLETE sequence rather than the end of the string: an ESC that has
* already been consumed as part of a match is not pending, and re-finding it would replay it.
*/
function trailingPartial(text: string, consumedTo: number): string {
const esc = text.lastIndexOf(ESC);
if (esc < consumedTo) return '';
const tail = text.slice(esc);
// A complete sequence would have matched above, so anything here is genuinely partial — unless it
// is too long to be a CSI parameter list, in which case it is malformed and not worth carrying.
return tail.length > MAX_PENDING ? '' : tail;
}
146 changes: 146 additions & 0 deletions packages/core/tests/unit/negotiation-scan.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import { describe, expect, it } from 'vitest';
import {
createNegotiationScan,
scanKeyboardNegotiation,
} from '../../src/terminal/negotiation-scan.js';
import {
kittyKeyboardActive,
win32InputActive,
applicationReadingInput,
} from '../../src/terminal/kitty-keyboard.js';

/**
* #290 — reading a program's keyboard negotiation out of the raw output stream.
*
* The daemon needs this because the renderer cannot always be listening: a panel in a background
* tab is unmounted, and a program that un-negotiates while nobody is watching is simply not heard.
* The daemon reads every byte regardless, so it holds the answer and hands it over on attach.
*
* The tests that matter most here are the two the defect was actually made of — order sensitivity,
* and what happens when the same bytes are applied twice.
*/

const PUSH = '\x1b[>1u'; // CSI > 1 u — push the disambiguate flag
const POP = '\x1b[<u'; // CSI < u — pop it back off
const WIN32_ON = '\x1b[?9001h';
const WIN32_OFF = '\x1b[?9001l';
const PASTE_ON = '\x1b[?2004h';

/** Feed a series of chunks through the scan, as the daemon does. */
const feed = (...chunks: readonly string[]) =>
chunks.reduce((scan, chunk) => scanKeyboardNegotiation(scan, chunk), createNegotiationScan());

describe('scanKeyboardNegotiation', () => {
it('sees a program enable enhanced key reporting', () => {
expect(kittyKeyboardActive(feed(PUSH).state)).toBe(true);
});

it('sees it turned back off again', () => {
expect(kittyKeyboardActive(feed(PUSH, POP).state)).toBe(false);
});

it('applies pushes and pops in the order they were emitted, not by last-match-wins', () => {
// Both chunks contain both sequences; only the ORDER differs, and it decides the answer.
expect(kittyKeyboardActive(feed(`${PUSH}${POP}`).state)).toBe(false);
expect(kittyKeyboardActive(feed(`${POP}${PUSH}`).state)).toBe(true);
});

it('sees a sequence SPLIT across two chunks — a PTY boundary must not swallow a pop', () => {
/*
* The failure this guards is silent and permanent. A missed `1049` self-corrects the next time
* the program switches screens; a missed POP leaves the protocol enabled until the program
* negotiates again, which for a program that has just exited is never.
*/
const armed = feed(PUSH);
expect(kittyKeyboardActive(armed.state)).toBe(true);

const half = scanKeyboardNegotiation(armed, '\x1b[<');
expect(half.pending, 'the incomplete sequence must be carried, not dropped').toBe('\x1b[<');
expect(kittyKeyboardActive(half.state), 'a half-read sequence must not be acted on yet').toBe(
true,
);

expect(kittyKeyboardActive(scanKeyboardNegotiation(half, 'u').state)).toBe(false);
});

it('splits at every offset of a push/pop pair and still ends up off', () => {
const stream = `${PUSH}${POP}`;
for (let cut = 0; cut <= stream.length; cut++) {
const scan = feed(stream.slice(0, cut), stream.slice(cut));
expect(kittyKeyboardActive(scan.state), `split after ${String(cut)} byte(s)`).toBe(false);
}
});

it('carries nothing when a chunk ends on a complete sequence', () => {
expect(feed(PUSH).pending).toBe('');
});

it('drops an unterminated run too long to be a CSI parameter list', () => {
// Otherwise a stream containing a bare ESC would accumulate forever.
expect(feed(`\x1b[${'9'.repeat(200)}`).pending).toBe('');
});

it('tracks win32-input-mode and bracketed paste, which travel with the same state', () => {
expect(win32InputActive(feed(WIN32_ON).state)).toBe(true);
expect(win32InputActive(feed(WIN32_ON, WIN32_OFF).state)).toBe(false);
expect(applicationReadingInput(feed(PASTE_ON).state)).toBe(true);
});

it('ignores sequences that are not a keyboard negotiation', () => {
// Alt-screen, a clear, a colour — none of these say anything about the keyboard, and stepping
// over them wrongly is how a scan starts inventing state.
const scan = feed(`\x1b[?1049h${PUSH}\x1b[2J\x1b[31m`);
expect(kittyKeyboardActive(scan.state)).toBe(true);
expect(win32InputActive(scan.state)).toBe(false);
});

it('requires the ESC — the same bytes as plain text negotiate nothing', () => {
/*
* The introducer is a control character, so it can only be written as an escape in source, and
* an editing accident that drops it leaves a regex which still compiles and still matches — just
* against ordinary output. A program printing the literal text `[>1u` would then silently switch
* enhanced key reporting on for the whole session.
*/
expect(kittyKeyboardActive(feed('[>1u').state)).toBe(false);
expect(win32InputActive(feed('[?9001h').state)).toBe(false);
});

it('treats a `CSI ? u` support query as evidence of nothing', () => {
// The program is ASKING. Only the view answers it, and the answer is not a negotiation.
expect(kittyKeyboardActive(feed('\x1b[?u').state)).toBe(false);
expect(kittyKeyboardActive(feed(PUSH, '\x1b[?u').state)).toBe(true);
});

it('THE DEFECT: applying one tail twice leaves the protocol stuck on (#290)', () => {
/*
* This is the whole reason the daemon is now the authority, expressed at the layer where it can
* be seen. A rebuilt view used to start from the state its panel store had saved AND then parse
* the replayed scrollback tail — which still contains the sequences that produced that state.
*
* The protocol is a stack, so the second push is not a no-op: it buries the first. The program's
* single pop then only cancels the duplicate, and enhanced key reporting stays on after the
* program has said it wants no such thing. Downstream that is `programOwnsKeyboard` stuck true,
* the scrollback chords surrendered, and Ctrl+Home no longer scrolling.
*/
// What the renderer's panel store held: the push was seen live, the pop never was, because by
// then the panel had been unmounted and no view existed to parse it.
const storeHeld = feed(PUSH);
expect(kittyKeyboardActive(storeHeld.state), 'the store is mid-sequence, by construction').toBe(
true,
);

// The daemon's tail, which spans the whole program: both the push AND the pop it never delivered.
const tail = `${PUSH}${POP}`;

// Read on its own, the tail says exactly the right thing — which is why this was so easy to miss.
expect(kittyKeyboardActive(feed(tail).state), 'the tail alone is honest').toBe(false);

// Applied ON TOP of the restored state, it is not. The replayed push buries the restored one, so
// the program's single pop cancels only the duplicate and the protocol survives its own exit.
const restoredThenReplayed = scanKeyboardNegotiation(storeHeld, tail);
expect(
kittyKeyboardActive(restoredThenReplayed.state),
'restore + replay leaves enhanced key reporting on after the program turned it off (#290)',
).toBe(true);
});
});
18 changes: 18 additions & 0 deletions packages/daemon/src/terminal-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import {
type PtyHandle,
appendScrollback,
trackAltScreen,
createNegotiationScan,
scanKeyboardNegotiation,
type NegotiationScan,
classifyFailure,
type FailureCause,
} from '@throng/core';
Expand Down Expand Up @@ -117,6 +120,17 @@ interface Session {
* for nothing before the program's own redraw overwrites it.
*/
altScreen: boolean;
/**
* What the program has negotiated about the KEYBOARD (#290) — kitty CSI-u flags and the DEC
* private modes, plus any half-read escape sequence carried across a chunk boundary.
*
* Tracked here for the same reason as `altScreen` directly above: the daemon reads every byte
* whether or not a view exists, and a panel in a background tab is unmounted, so a program that
* un-negotiates while nobody is looking would otherwise never be heard. A rebuilt view used to
* work this out for itself from the store PLUS the replayed tail, which double-counted every
* push and left the protocol stuck on. See `negotiation-scan.ts`.
*/
negotiation: NegotiationScan;
/**
* The grid is stale because every view has gone (028 follow-up). The next attach MUST push a real
* resize even when the recomputed grid equals the stored one, because the program needs a window
Expand Down Expand Up @@ -437,6 +451,7 @@ export class TerminalService {
grid: existing.grid,
redrawn,
altScreen: existing.altScreen,
keyboard: existing.negotiation.state,
};
}
this.terminate(existing);
Expand Down Expand Up @@ -521,6 +536,7 @@ export class TerminalService {
grid: { cols: startCols, rows: startRows },
scrollback: '',
altScreen: false,
negotiation: createNegotiationScan(),
gridStale: false,
status: 'running',
userKilled: false,
Expand All @@ -543,6 +559,8 @@ export class TerminalService {
host.onData(handle, (chunk) => {
session.scrollback = appendScrollback(session.scrollback, chunk, MAX_SCROLLBACK);
session.altScreen = trackAltScreen(session.altScreen, chunk);
// #290 — the other half a rebuilt view must be TOLD rather than left to infer.
session.negotiation = scanKeyboardNegotiation(session.negotiation, chunk);
this.events.publishOutput(panelId, chunk);
if (startupCommandPending) {
startupCommandPending = false;
Expand Down
17 changes: 17 additions & 0 deletions packages/ipc-contract/src/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// contracts/terminal-rpc.md). Commands are request/response; output/exit are
// JSON-RPC *notifications* (no id) pushed over a long-lived subscribed socket.

import type { KittyKeyboardState } from '@throng/core';

// --- Command methods (request → response) ---
export const TERMINAL_ATTACH_METHOD = 'terminal.attach';
export const TERMINAL_WRITE_METHOD = 'terminal.write';
Expand Down Expand Up @@ -187,6 +189,21 @@ export interface TerminalAttachResult {
* buffer and reclaims keys the program owns: measured as Ctrl+End dying after a tab switch.
*/
altScreen?: boolean;
/**
* What the program has negotiated about the KEYBOARD (#290) — the same fact as `altScreen` above,
* for the other half of what a rebuilt view must be told rather than left to work out.
*
* The view used to reconstruct this from two sources at once: the renderer's own panel store, and
* the replayed scrollback tail, which still contains the sequences that produced what the store
* holds. Applying both counts every `CSI > flags u` push twice, and since the protocol is a stack
* the program's matching pop then only cancels the duplicate — leaving throng convinced a program
* wants enhanced key reporting after it has said it does not. `programOwnsKeyboard` sticks true,
* the scrollback chords are surrendered, and Ctrl+Home / PageUp stop scrolling.
*
* The daemon reads every byte whether or not a view exists, so it is the one place that can hold
* this honestly. Absent only when there is no running session to ask.
*/
keyboard?: KittyKeyboardState;
exit?: { code: number | null; signal?: string };
}

Expand Down
6 changes: 6 additions & 0 deletions packages/ui/src/renderer/global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,12 @@ export type TerminalAttachEnvelope =
/** The program owns the alternate screen (028 follow-up) — the view must match, or it
* reclaims keys the program owns. */
altScreen?: boolean;
/**
* What the program has negotiated about the keyboard (#290) — the companion to `altScreen`
* above. A rebuilt view adopts this instead of re-deriving it from the replayed tail, which
* still contains the sequences that produced it and would count every push a second time.
*/
keyboard?: import('@throng/core').KittyKeyboardState;
exit?: { code: number | null };
/** A remembered directory that no longer exists; the terminal started at the root (FR-005b). */
cwdFallback?: string;
Expand Down
Loading
Loading