diff --git a/packages/ai/test/remote-auth-store.test.ts b/packages/ai/test/remote-auth-store.test.ts index 56b203664..d730efce7 100644 --- a/packages/ai/test/remote-auth-store.test.ts +++ b/packages/ai/test/remote-auth-store.test.ts @@ -1110,7 +1110,7 @@ describe("RemoteAuthCredentialStore + AuthStorage integration", () => { test("broker invalidation drops server-side last-good usage reports", async () => { const credential = serverStore!.listAuthCredentials("anthropic")[0]; - if (!credential || credential.credential.type !== "oauth") throw new Error("expected OAuth credential"); + if (credential?.credential.type !== "oauth") throw new Error("expected OAuth credential"); serverStore!.updateAuthCredential(credential.id, { ...credential.credential, expires: Date.now() + 3_600_000, diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 67107570e..0414a5349 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed a terminal Device-Attributes reply leaking into the composer as literal text (e.g. `1;22;…;52c`) when it arrived after the startup capability-probe sentinel FIFO drained, a race made observable by the added latency of an SSH/zmx PTY chain. DA1 replies (`CSI ? … c`) and split private-CSI responses are now consumed for the whole session lifetime, not only while a probe sentinel is outstanding ([#8542](https://github.com/can1357/oh-my-pi/issues/8542)). + ## [17.3.3] - 2026-08-14 ### Fixed diff --git a/packages/tui/src/terminal.ts b/packages/tui/src/terminal.ts index 537123db2..36d0f1170 100644 --- a/packages/tui/src/terminal.ts +++ b/packages/tui/src/terminal.ts @@ -945,10 +945,11 @@ export class ProcessTerminal implements Terminal { // flush timeout elapses mid-sequence, the prefix `\x1b[?` arrives as // one event and the tail `;...` arrives as individual character // events that would otherwise leak into the prompt as keystrokes. See #1238. - if ( - this.#privateCsiResponseBuffer || - (privateCsiPartialPattern.test(sequence) && this.#da1SentinelOwners.length > 0) - ) { + // A private CSI (`\x1b[?…`) is a terminal->host report, never a keystroke, so + // reassembly stays armed for the whole session — not just while a probe + // sentinel is outstanding — otherwise a reply that lands after the sentinel + // FIFO drains (slow SSH/PTY links) leaks its tail into the composer (#8542). + if (this.#privateCsiResponseBuffer || privateCsiPartialPattern.test(sequence)) { if (this.#privateCsiResponseBuffer && sequence.startsWith("\x1b")) { // New escape arrived mid-reassembly — abandon partial and re-process the new sequence. this.#privateCsiResponseBuffer = ""; @@ -1038,10 +1039,16 @@ export class ProcessTerminal implements Terminal { } // DA1 response: swallow our sentinel reply regardless of whether an - // earlier capability-specific response already succeeded. Other terminal - // probes should never see these replies. - if (da1ResponsePattern.test(sequence) && this.#da1SentinelOwners.length > 0) { - const owner = this.#da1SentinelOwners.shift()!; + // earlier capability-specific response already succeeded. `CSI ? … c` is + // exclusively a terminal->host report, so it is swallowed even with no + // outstanding sentinel — a reply that arrives after the FIFO drains (slow + // SSH/PTY links) must never reach the composer as literal text (#8542). + if (da1ResponsePattern.test(sequence)) { + const owner = this.#da1SentinelOwners.shift(); + if (!owner) { + // Late/unowned reply: nothing to resolve, just drop the bytes. + return; + } switch (owner.kind) { case "osc11": { if (this.#osc11Pending) { diff --git a/packages/tui/test/issue-8542-repro.test.ts b/packages/tui/test/issue-8542-repro.test.ts new file mode 100644 index 000000000..f0db7ed64 --- /dev/null +++ b/packages/tui/test/issue-8542-repro.test.ts @@ -0,0 +1,88 @@ +import { afterEach, describe, expect, it, vi } from "bun:test"; +import { ProcessTerminal } from "@oh-my-pi/pi-tui/terminal"; +import { setTerminalHeadless } from "@oh-my-pi/pi-utils"; + +// #8542: a terminal Device-Attributes reply to omp's startup capability probe +// leaks into the composer as literal text (`1;22;...;52c`) when it arrives +// after the DA1 sentinel FIFO has already drained. The extra SSH+zmx PTY hops +// slow the query->response round-trip enough to make the race observable. +// +// Contract: `CSI ? … c` is exclusively a terminal->host report, never a +// keystroke, so it MUST be consumed for the whole session lifetime and never +// forwarded to the input handler that feeds the composer. + +// A meaty multi-parameter DA1 reply, exactly as the reporter observed it. +const DA1_REPLY = "\x1b[?1;22;23;24;28;32;42;52c"; + +describe("issue #8542: late DA response must not leak into the composer", () => { + let terminal: ProcessTerminal | undefined; + let previousHeadless = false; + let spies: Array<{ mockRestore(): void }> = []; + const captured: string[] = []; + + function setup(): void { + previousHeadless = setTerminalHeadless(false); + Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true }); + Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true }); + Object.defineProperty(process.stdin, "setRawMode", { value: vi.fn(), configurable: true }); + Object.defineProperty(process.stdout, "columns", { value: 100, configurable: true }); + Object.defineProperty(process.stdout, "rows", { value: 30, configurable: true }); + spies = [ + vi.spyOn(process.stdin, "resume").mockImplementation(() => process.stdin), + vi.spyOn(process.stdin, "pause").mockImplementation(() => process.stdin), + vi.spyOn(process.stdin, "setEncoding").mockImplementation(() => process.stdin), + vi.spyOn(process.stdout, "write").mockImplementation(() => true), + vi.spyOn(process, "kill").mockImplementation(() => true), + ]; + captured.length = 0; + terminal = new ProcessTerminal(); + terminal.start( + data => captured.push(data), + () => {}, + ); + } + + afterEach(() => { + terminal?.stop(); + terminal = undefined; + for (const spy of spies) spy.mockRestore(); + spies = []; + Reflect.deleteProperty(process.stdin, "isTTY"); + Reflect.deleteProperty(process.stdout, "isTTY"); + Reflect.deleteProperty(process.stdin, "setRawMode"); + Reflect.deleteProperty(process.stdout, "columns"); + Reflect.deleteProperty(process.stdout, "rows"); + setTerminalHeadless(previousHeadless); + }); + + it("swallows a single-event DA reply that arrives after the sentinel FIFO drains", () => { + setup(); + // Complete `CSI ? … c` sequences flow through the StdinBuffer synchronously. + // Over-supply them: the first few resolve the startup probe sentinels, the + // rest model the slow SSH/PTY reply that lands with an empty FIFO. None may + // reach the composer. + for (let i = 0; i < 32; i++) process.stdin.emit("data", DA1_REPLY); + + expect(captured.join("")).toBe(""); + }); + + it("reassembles and swallows a split DA reply arriving with an empty FIFO", async () => { + setup(); + // Drain the sentinel FIFO first (complete replies, processed synchronously). + for (let i = 0; i < 32; i++) process.stdin.emit("data", "\x1b[?62c"); + captured.length = 0; + + // The prefix of a slow reply arrives alone; the StdinBuffer holds it as an + // unambiguous private-CSI partial, then flushes it once its real timeout + // (<= PARTIAL_HOLD_MAX_MS = 150ms) elapses mid-sequence. This exercises the + // terminal-level reassembly path that only fires against the wall clock — + // deterministic fake timers cannot drive the StdinBuffer's internal flush + // here, so a genuine delay past the hold bound is required. + process.stdin.emit("data", "\x1b[?1;22;23"); + await Bun.sleep(200); + // Tail bytes arrive as ordinary input after the flush. + process.stdin.emit("data", ";24;28;32;42;52c"); + + expect(captured.join("")).toBe(""); + }); +}); diff --git a/packages/tui/test/terminal-appearance.test.ts b/packages/tui/test/terminal-appearance.test.ts index 1cb6169fd..dcf828b7a 100644 --- a/packages/tui/test/terminal-appearance.test.ts +++ b/packages/tui/test/terminal-appearance.test.ts @@ -625,9 +625,12 @@ describe("ProcessTerminal OSC 11 appearance detection", () => { process.stdin.emit("data", "\x1b[?1;2c"); expect(received).toEqual([]); - // An eighth stray DA1 has no owner and must reach the input handler — it is + // An eighth stray DA1 has no owner, yet is still swallowed: `CSI ? … c` is + // exclusively a terminal->host report, never a keystroke, so a reply that + // lands after the sentinel FIFO drains (slow SSH/PTY links) must not leak + // into the composer as literal text (#8542). process.stdin.emit("data", "\x1b[?1;2c"); - expect(received).toEqual(["\x1b[?1;2c"]); + expect(received).toEqual([]); terminal.stop(); });