From a077db20570339155bb32b89d25baee42f3cdc2d Mon Sep 17 00:00:00 2001 From: liruifengv Date: Tue, 30 Jun 2026 19:08:46 +0800 Subject: [PATCH] refactor(pi-tui): tighten capability probe wiring and remove dead code --- packages/pi-tui/src/terminal-capabilities.ts | 19 ----- packages/pi-tui/src/terminal.ts | 30 +++----- packages/pi-tui/src/tui.ts | 7 +- .../pi-tui/test/terminal-capabilities.test.ts | 37 --------- packages/pi-tui/test/terminal.test.ts | 76 ++++++++++++++----- 5 files changed, 70 insertions(+), 99 deletions(-) diff --git a/packages/pi-tui/src/terminal-capabilities.ts b/packages/pi-tui/src/terminal-capabilities.ts index 47d7ced8e..c0fd957fb 100644 --- a/packages/pi-tui/src/terminal-capabilities.ts +++ b/packages/pi-tui/src/terminal-capabilities.ts @@ -65,25 +65,6 @@ export function createStaticCapabilities(env: NodeJS.ProcessEnv = process.env): }; } -/** - * Assemble an immutable {@link TerminalCapabilities} snapshot from a completed - * probe. The only probed field consumed today is `syncOutput` (DECRQM ?2026); - * the remaining fields are conservative defaults until later tasks wire them in. - */ -export function createProbedCapabilities( - result: ProbeResult, - env: NodeJS.ProcessEnv = process.env, -): TerminalCapabilities { - return { - syncEnabled: shouldEnableSyncOutput(env, result.syncOutput), - supportsScreenToScrollback: false, - deccara: false, - hyperlinks: shouldEnableHyperlinks(env), - imageProtocol: "none", - isImageLine: detectImageLine, - }; -} - /** * Mutable, terminal-owned capabilities. Starts with the static env-derived * defaults and is updated in place when the startup probe resolves, so an diff --git a/packages/pi-tui/src/terminal.ts b/packages/pi-tui/src/terminal.ts index aa6d3a81a..51a2e124d 100644 --- a/packages/pi-tui/src/terminal.ts +++ b/packages/pi-tui/src/terminal.ts @@ -5,7 +5,7 @@ import { fileURLToPath } from "node:url"; import { setKittyProtocolActive } from "./keys.ts"; import { isNativeModifierPressed } from "./native-modifiers.ts"; import { StdinBuffer } from "./stdin-buffer.ts"; -import { ProcessTerminalCapabilities } from "./terminal-capabilities.ts"; +import { ProcessTerminalCapabilities, type TerminalCapabilities } from "./terminal-capabilities.ts"; import { type ProbeIO, type ProbeResult, probeCapabilities } from "./terminal-probe.ts"; const cjsRequire = createRequire(import.meta.url); @@ -82,6 +82,12 @@ export interface Terminal { // Whether Kitty keyboard protocol is active get kittyProtocolActive(): boolean; + /** Resolves with the startup probe result; undefined when probing is skipped (non-TTY). */ + probeReady?: Promise; + + /** Mutable, probe-backed terminal capabilities shared with the ledger engine. */ + readonly terminalCapabilities?: TerminalCapabilities; + // Cursor positioning (relative to current position) moveBy(lines: number): void; // Move cursor up (negative) or down (positive) by N lines @@ -236,8 +242,8 @@ export class ProcessTerminal implements Terminal { * keyboard-protocol negotiation buffer, without writing the kitty query. In * TTY mode the kitty query is emitted by the capability probe (its first probe * is `CSI ? u` + DA1), so this must not write a second one — a duplicate would - * desync the probe's DA1 FIFO. Split out from {@link queryAndEnableKittyProtocol} - * so start() can order the flag push, the probe, and the headless fallback. + * desync the probe's DA1 FIFO. Kept separate so start() can order the flag push, + * the probe, and the headless fallback. */ private setupKeyboardProtocolPipeline(): void { this.setupStdinBuffer(); @@ -246,24 +252,6 @@ export class ProcessTerminal implements Terminal { this.clearKeyboardProtocolNegotiationBuffer(); } - /** - * Query terminal for Kitty keyboard protocol support and enable it if available. - * - * Kitty's progressive enhancement detection requires requesting the desired - * flags before querying them. The trailing DA query is a sentinel supported by - * terminals that do not know Kitty keyboard protocol; receiving DA before a - * Kitty response enables modifyOtherKeys fallback without a startup timeout. - * - * The requested flags are: - * - 1 = disambiguate escape codes - * - 2 = report event types (press/repeat/release) - * - 4 = report alternate keys (shifted key, base layout key) - */ - private queryAndEnableKittyProtocol(): void { - this.setupKeyboardProtocolPipeline(); - process.stdout.write(KITTY_KEYBOARD_PROTOCOL_QUERY); - } - /** * Run the startup capability probe (kitty / OSC 11 / DECRQM ?2026 ?2048 ?2031). * diff --git a/packages/pi-tui/src/tui.ts b/packages/pi-tui/src/tui.ts index 69ddc64d6..0d2097beb 100644 --- a/packages/pi-tui/src/tui.ts +++ b/packages/pi-tui/src/tui.ts @@ -17,7 +17,6 @@ import { } from "./terminal-colors.ts"; import { deleteKittyImage, getCapabilities, isImageLine, setCellDimensions } from "./terminal-image.ts"; import { createStaticCapabilities, type TerminalCapabilities } from "./terminal-capabilities.ts"; -import type { ProbeResult } from "./terminal-probe.ts"; import { extractSegments, normalizeTerminalOutput, sliceByColumn, sliceWithWidth, visibleWidth } from "./utils.ts"; import { LedgerTuiEngine } from "./ledger/engine.ts"; @@ -667,13 +666,13 @@ export class TUI extends Container { * object is already mutated in place by the terminal; this just re-reads it. */ private attachProbeRefresh(): void { - const probeReady = (this.terminal as { probeReady?: Promise }).probeReady; + const probeReady = this.terminal.probeReady; if (!probeReady) return; void probeReady.then(() => { if (this.stopped) return; this.ledgerEngine?.refreshSyncFraming(); this.requestRender(); - }); + }).catch(() => {}); } addInputListener(listener: InputListener): () => void { @@ -1669,7 +1668,7 @@ export class TUI extends Container { * defaults. */ private resolveTerminalCapabilities(): TerminalCapabilities { - const caps = (this.terminal as { terminalCapabilities?: TerminalCapabilities }).terminalCapabilities; + const caps = this.terminal.terminalCapabilities; return caps ?? createStaticCapabilities(); } diff --git a/packages/pi-tui/test/terminal-capabilities.test.ts b/packages/pi-tui/test/terminal-capabilities.test.ts index 5e5294c32..7c57566f6 100644 --- a/packages/pi-tui/test/terminal-capabilities.test.ts +++ b/packages/pi-tui/test/terminal-capabilities.test.ts @@ -1,14 +1,12 @@ import assert from "node:assert"; import { describe, it } from "node:test"; import { - createProbedCapabilities, createStaticCapabilities, isMultiplexerSession, shouldEnableHyperlinks, shouldEnableSyncOutput, } from "../src/terminal-capabilities.ts"; import { isImageLine as detectImageLine } from "../src/terminal-image.ts"; -import type { ProbeResult } from "../src/terminal-probe.ts"; describe("terminal-capabilities", () => { it("detects mux via env and TERM fallback", () => { @@ -65,38 +63,3 @@ describe("createStaticCapabilities", () => { assert.strictEqual(caps.isImageLine(plainLine), detectImageLine(plainLine)); }); }); - -function probeResult(syncOutput: boolean | undefined): ProbeResult { - return { - kittyKeyboard: false, - syncOutput, - inBandResize: undefined, - appearancePush: undefined, - background: undefined, - }; -} - -describe("createProbedCapabilities", () => { - it("enables sync when DECRQM reports supported", () => { - // Even a terminal the static table would reject is forced on by the probe. - assert.strictEqual(createProbedCapabilities(probeResult(true), { TERM: "dumb" }).syncEnabled, true); - }); - - it("disables sync when DECRQM reports unsupported, even for a known-sync terminal", () => { - // The runtime DECRQM result beats the static "xterm-kitty" heuristic. - assert.strictEqual(createProbedCapabilities(probeResult(false), { TERM: "xterm-kitty" }).syncEnabled, false); - }); - - it("falls back to the env heuristic when DECRQM is inconclusive", () => { - assert.strictEqual(createProbedCapabilities(probeResult(undefined), { TERM: "xterm-kitty" }).syncEnabled, true); - assert.strictEqual(createProbedCapabilities(probeResult(undefined), { TERM: "dumb" }).syncEnabled, false); - }); - - it("keeps the conservative defaults for the non-sync fields", () => { - const caps = createProbedCapabilities(probeResult(true), { TERM: "xterm-kitty" }); - assert.strictEqual(caps.supportsScreenToScrollback, false); - assert.strictEqual(caps.deccara, false); - assert.strictEqual(caps.imageProtocol, "none"); - assert.strictEqual(caps.hyperlinks, shouldEnableHyperlinks({ TERM: "xterm-kitty" })); - }); -}); diff --git a/packages/pi-tui/test/terminal.test.ts b/packages/pi-tui/test/terminal.test.ts index a356bff1d..9aaf341a7 100644 --- a/packages/pi-tui/test/terminal.test.ts +++ b/packages/pi-tui/test/terminal.test.ts @@ -49,15 +49,10 @@ describe("ProcessTerminal Kitty keyboard protocol negotiation", () => { return process.stdin; }) as typeof process.stdin.on; - ( - terminal as unknown as { - inputHandler?: (data: string) => void; - queryAndEnableKittyProtocol(): void; - } - ).inputHandler = (data) => { + (terminal as unknown as { inputHandler?: (data: string) => void }).inputHandler = (data) => { input = data; }; - (terminal as unknown as { queryAndEnableKittyProtocol(): void }).queryAndEnableKittyProtocol(); + (terminal as unknown as { setupKeyboardProtocolPipeline(): void }).setupKeyboardProtocolPipeline(); return { terminal, @@ -82,17 +77,6 @@ describe("ProcessTerminal Kitty keyboard protocol negotiation", () => { }; } - it("queries Kitty mode before enabling modifyOtherKeys fallback", () => { - const harness = setupNegotiation(); - try { - assert.equal(harness.writes[0], "\x1b[>7u\x1b[?u\x1b[c"); - assert.equal(harness.writes.includes("\x1b[>4;2m"), false); - assert.equal(harness.terminal.kittyProtocolActive, false); - } finally { - harness.cleanup(); - } - }); - it("activates Kitty mode for non-zero negotiated flags", () => { const harness = setupNegotiation(); try { @@ -190,6 +174,62 @@ describe("ProcessTerminal Kitty keyboard protocol negotiation", () => { }); }); +describe("ProcessTerminal start", () => { + it("pushes Kitty keyboard protocol flags on the TTY path", () => { + const terminal = new ProcessTerminal(); + const writes: string[] = []; + const previousWrite = process.stdout.write; + const previousStdoutOn = process.stdout.on; + const previousStdinOn = process.stdin.on; + const previousResume = process.stdin.resume; + const previousSetRawMode = process.stdin.setRawMode; + const previousKill = process.kill; + const previousIsTTYDescriptor = Object.getOwnPropertyDescriptor(process.stdout, "isTTY"); + + const restore = (): void => { + process.stdout.write = previousWrite; + process.stdout.on = previousStdoutOn; + process.stdin.on = previousStdinOn; + process.stdin.resume = previousResume; + process.stdin.setRawMode = previousSetRawMode; + process.kill = previousKill; + if (previousIsTTYDescriptor) { + Object.defineProperty(process.stdout, "isTTY", previousIsTTYDescriptor); + } else { + Reflect.deleteProperty(process.stdout, "isTTY"); + } + setKittyProtocolActive(false); + }; + + try { + process.stdout.write = ((chunk: string | Uint8Array) => { + writes.push(String(chunk)); + return true; + }) as typeof process.stdout.write; + process.stdout.on = (() => process.stdout) as typeof process.stdout.on; + process.stdin.on = (() => process.stdin) as typeof process.stdin.on; + process.stdin.resume = (() => process.stdin) as typeof process.stdin.resume; + process.stdin.setRawMode = (() => process.stdin) as typeof process.stdin.setRawMode; + process.kill = (() => true) as typeof process.kill; + Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true }); + // Stub the probe so start() performs only the synchronous Kitty push. + (terminal as unknown as { runCapabilityProbe(): void }).runCapabilityProbe = () => {}; + + terminal.start( + () => {}, + () => {}, + ); + assert.equal(writes.includes("\x1b[>7u"), true); + } finally { + try { + terminal.stop(); + } finally { + restore(); + } + } + }); +}); + describe("ProcessTerminal dimensions", () => { it("falls back to COLUMNS and LINES before default dimensions", () => { const previousColumnsDescriptor = Object.getOwnPropertyDescriptor(process.stdout, "columns");