fix(tui): swallow late DA responses for the whole session
The private-CSI reassembly gate and the DA1 swallow were both guarded by `#da1SentinelOwners.length > 0`, so a Device-Attributes reply that arrived after the startup capability-probe sentinel FIFO drained fell through to the input handler and leaked into the composer as literal text (e.g. `1;22;...;52c`). The extra latency of an SSH/zmx PTY chain makes the race observable. `CSI ? ... c` and split private-CSI responses are terminal->host reports, never keystrokes, so they are now consumed regardless of whether a sentinel is still outstanding. Fixes #8542
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -945,10 +945,11 @@ export class ProcessTerminal implements Terminal {
|
||||
// flush timeout elapses mid-sequence, the prefix `\x1b[?<digits>` arrives as
|
||||
// one event and the tail `;...<terminator>` 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) {
|
||||
|
||||
@@ -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("");
|
||||
});
|
||||
});
|
||||
@@ -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();
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user