From 35c4aff8a51d0d33fbab626393c650b22365b088 Mon Sep 17 00:00:00 2001 From: olafura Date: Mon, 22 Jun 2026 18:58:30 +0200 Subject: [PATCH 01/18] fix(server): stop terminal escape sequences leaking as garbled text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sanitize terminal output into scrollback (queries + responses stripped so a replay can never re-trigger the echo loop) and the live stream (responses only), strip the browser emulator's auto-replies from client input at the idle prompt, and derive every matching layer from one canonical sequence grammar with cross-layer invariant tests. Also cap retained history by characters (default 2 MiB), not just lines: a full-screen program repainting with synchronized-output frames (no newlines) defeated the line cap — observed 21 MB of redraw frames at 4,999 lines, bloating server memory, the persisted log, and the reattach snapshot. Fixes #1238. Co-Authored-By: Claude Opus 4.8 --- apps/server/src/terminal/Manager.test.ts | 756 +++++++++++++++++++++- apps/server/src/terminal/Manager.ts | 783 +++++++++++++++++++++-- 2 files changed, 1480 insertions(+), 59 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index 1cf7e8dffec..d8f02397d97 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -1,5 +1,5 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; -import { assert, it } from "@effect/vitest"; +import { assert, describe, it } from "@effect/vitest"; import { DEFAULT_TERMINAL_ID, type TerminalAttachStreamEvent, @@ -28,6 +28,12 @@ import { expect } from "vite-plus/test"; import * as ProcessRunner from "../processRunner.ts"; import * as TerminalManager from "./Manager.ts"; +import { + sanitizePersistedTerminalHistory, + sanitizeTerminalHistoryChunk, + stripTerminalResponsesFromInput, + TERMINAL_SEQUENCE_GRAMMAR, +} from "./Manager.ts"; import * as PtyAdapter from "./PtyAdapter.ts"; class WaitForConditionError extends Data.TaggedError("WaitForConditionError")<{ @@ -203,11 +209,13 @@ const multiTerminalHistoryLogPath = ( interface CreateManagerOptions { shellResolver?: () => string; + historyCharLimit?: number; env?: NodeJS.ProcessEnv; subprocessInspector?: (terminalPid: number) => Effect.Effect<{ readonly hasRunningSubprocess: boolean; readonly childCommand: string | null; readonly processIds: ReadonlyArray; + readonly shellForeground?: boolean; }>; subprocessPollIntervalMs?: number; processKillGraceMs?: number; @@ -241,6 +249,9 @@ const createManager = ( const manager = yield* TerminalManager.makeWithOptions({ logsDir, historyLineLimit, + ...(options.historyCharLimit !== undefined + ? { historyCharLimit: options.historyCharLimit } + : {}), ptyAdapter, ...(options.shellResolver !== undefined ? { shellResolver: options.shellResolver } : {}), ...(options.env !== undefined ? { env: options.env } : {}), @@ -927,6 +938,152 @@ it.layer( }), ); + it.effect( + "strips capability replies at an idle prompt but relays them to a foreground program", + () => + Effect.gen(function* () { + let inspect: { + readonly hasRunningSubprocess: boolean; + readonly childCommand: string | null; + readonly processIds: ReadonlyArray; + } = { hasRunningSubprocess: false, childCommand: null, processIds: [] }; + const { manager, ptyAdapter, getEvents } = yield* createManager(5, { + subprocessInspector: () => Effect.succeed(inspect), + subprocessPollIntervalMs: 20, + }); + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + expect(process).toBeDefined(); + if (!process) return; + + // No relayed cursor query yet: `CSI 1;2R` is xterm's modified-F3 + // keystroke, not a CPR reply — it must reach the PTY. + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[1;2R", + }); + expect(process.writes).toEqual(["\x1b[1;2R"]); + process.writes.length = 0; + + // The prompt queries the cursor position — relaying it arms the CPR strip. + process.emitData("\x1b[6n"); + yield* waitFor( + Effect.map(getEvents, (events) => + events.some((event) => event.type === "output" && event.data.includes("\x1b[6n")), + ), + "1200 millis", + ); + + // Idle prompt (no subprocess) with an outstanding query: the emulator's + // CPR auto-reply is dropped — the shell would only echo it back (the + // `;1RR` flood). + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[1;1R", + }); + expect(process.writes).toEqual([]); + + // Foreground program running (vim): it issued the query and is blocked + // reading the answer — input must pass through verbatim. + inspect = { hasRunningSubprocess: true, childCommand: "vim", processIds: [100] }; + yield* waitFor( + Effect.map(getEvents, (events) => + events.some((event) => event.type === "activity" && event.hasRunningSubprocess), + ), + "1200 millis", + ); + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[1;1R", + }); + expect(process.writes).toEqual(["\x1b[1;1R"]); + }), + ); + + it.effect("keeps stripping replies while only a BACKGROUND job runs (shell owns the PTY)", () => + Effect.gen(function* () { + // `sleep 100 &` puts a child under the shell, but the shell still owns + // the PTY's foreground group (tpgid == shell pgid) and is at the prompt — + // the echo loop is live, so replies must still be stripped. The inspector + // reports the foreground signal explicitly. + let inspect: { + readonly hasRunningSubprocess: boolean; + readonly childCommand: string | null; + readonly processIds: ReadonlyArray; + readonly shellForeground?: boolean; + } = { + hasRunningSubprocess: false, + childCommand: null, + processIds: [], + shellForeground: true, + }; + let inspections = 0; + const { manager, ptyAdapter, getEvents } = yield* createManager(5, { + subprocessInspector: () => { + inspections += 1; + return Effect.succeed(inspect); + }, + subprocessPollIntervalMs: 20, + }); + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + expect(process).toBeDefined(); + if (!process) return; + + inspect = { + hasRunningSubprocess: true, + childCommand: "sleep", + processIds: [100], + shellForeground: true, // background job — the shell keeps the prompt + }; + yield* waitFor( + Effect.map(getEvents, (events) => + events.some((event) => event.type === "activity" && event.hasRunningSubprocess), + ), + "1200 millis", + ); + // The prompt's cursor query arms the CPR strip (CPR is query-gated). + process.emitData("\x1b[6n"); + yield* waitFor( + Effect.map(getEvents, (events) => + events.some((event) => event.type === "output" && event.data.includes("\x1b[6n")), + ), + "1200 millis", + ); + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[1;1R", + }); + expect(process.writes).toEqual([]); // still stripped — echo loop stays broken + + // The job moves to the foreground (`fg`): tpgid flips to the job's group + // even though the wire label doesn't change — replies must now pass. + inspect = { + hasRunningSubprocess: true, + childCommand: "sleep", + processIds: [100], + shellForeground: false, + }; + // No activity event fires for a pure fg/bg flip; wait until the poller has + // demonstrably run with the flipped fixture instead of sleeping blind. + const inspectionsAtFlip = inspections; + yield* waitFor( + Effect.sync(() => inspections >= inspectionsAtFlip + 2), + "1200 millis", + ); + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[1;1R", + }); + expect(process.writes).toEqual(["\x1b[1;1R"]); + }), + ); + it.effect("does not invoke subprocess polling until a terminal session is running", () => Effect.gen(function* () { let checks = 0; @@ -953,6 +1110,32 @@ it.layer( }), ); + it.effect("caps history by characters when a redraw stream has no newlines", () => + Effect.gen(function* () { + // A full-screen program repainting with synchronized-output frames emits + // megabytes with almost no newlines — the line cap alone retains all of + // it (observed: 21 MB at 4,999 lines). The character cap bounds it. + const { manager, ptyAdapter } = yield* createManager(5_000, { historyCharLimit: 400 }); + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + expect(process).toBeDefined(); + if (!process) return; + + const frame = "\u001b[?2026h\u001b[18;2H\u001b[0m\u001b[49m\u001b[Kframe\u001b[?2026l"; + for (let i = 0; i < 40; i += 1) { + process.emitData(frame); + } + process.emitData("tail-marker"); + yield* manager.close({ threadId: "thread-1" }); + + const reopened = yield* manager.open(openInput()); + expect(reopened.history.length).toBeLessThanOrEqual(400); + expect(reopened.history.includes("tail-marker")).toBe(true); + // The cut lands on an escape boundary, not mid-sequence. + expect(reopened.history.startsWith("\u001b")).toBe(true); + }), + ); + it.effect("caps persisted history to configured line limit", () => Effect.gen(function* () { const { manager, ptyAdapter } = yield* createManager(3); @@ -980,7 +1163,9 @@ it.layer( process.emitData("prompt "); process.emitData("\u001b[32mok\u001b[0m "); - process.emitData("\u001b]11;rgb:ffff/ffff/ffff\u0007"); + // A colour QUERY is replay-unsafe (the emulator would re-answer it); the + // rgb SET form is legitimate output and covered by the grammar invariants. + process.emitData("\u001b]11;?\u0007"); process.emitData("\u001b[1;1R"); process.emitData("done\n"); @@ -991,6 +1176,22 @@ it.layer( }), ); + it.effect("sanitizes a pre-existing raw history log on load (older builds wrote it dirty)", () => + Effect.gen(function* () { + const { manager, logsDir } = yield* createManager(); + const logPath = yield* historyLogPath(logsDir); + // A log an older build persisted without sanitizing: the exact repeating + // DECRPM residue from #1238, ESC introducers intact. On load it must be + // stripped so it cannot replay (and re-trigger) at the prompt. + const garble = "[?69;0$y[?2026;2$y[?2027;0$y[?2031;0$y[?2048;0$y"; + yield* writeFileString(logPath, `prompt$ ${garble.repeat(15)}done\n`); + + const opened = yield* manager.open(openInput()); + assert.equal(opened.history, "prompt$ done\n"); + // The cleaned history is persisted back, so it stays clean on re-read. + assert.equal(yield* readFileString(logPath), "prompt$ done\n"); + }), + ); it.effect( "preserves clear and style control sequences while dropping chunk-split query traffic", () => @@ -1005,7 +1206,7 @@ it.layer( process.emitData("\u001b[H\u001b[2J"); process.emitData("prompt "); process.emitData("\u001b]11;"); - process.emitData("rgb:ffff/ffff/ffff\u0007\u001b[1;1"); + process.emitData("?\u0007\u001b[1;1"); process.emitData("R\u001b[36mdone\u001b[0m\n"); yield* manager.close({ threadId: "thread-1" }); @@ -1702,3 +1903,552 @@ it.layer( }).pipe(Effect.provide(TestClock.layer())), ); }); + +describe("sanitizeTerminalHistoryChunk", () => { + const sanitize = (data: string, pending = "") => sanitizeTerminalHistoryChunk(pending, data); + + it("strips DECRPM mode reports (CSI ? Pm ; Ps $ y) from history", () => { + const reports = "\x1b[?69;0$y\x1b[?2026;2$y\x1b[?2048;0$y"; + const { visibleText } = sanitize(`before${reports}after`); + assert.equal(visibleText, "beforeafter"); + // The residue users were seeing must not survive. + assert.ok(!visibleText.includes("$y")); + assert.ok(!visibleText.includes("2026")); + }); + + it("strips DECRQM mode queries (CSI ? Pm $ p) so replay can't re-trigger them", () => { + const { visibleText } = sanitize("x\x1b[?2026$p\x1b[?2048$py"); + assert.equal(visibleText, "xy"); + }); + + it("keeps ordinary text and non-report CSI sequences", () => { + // SGR colour (m) and cursor moves stay; a plain 'p'/'y' without the `$` + // intermediate is not a mode sequence and must be preserved. + const { visibleText } = sanitize("\x1b[31mred\x1b[0m \x1b[2Aup happy"); + assert.equal(visibleText, "\x1b[31mred\x1b[0m \x1b[2Aup happy"); + }); + + it("drops the flattened mode-reply residue a shell echoes at the prompt", () => { + // The ESC introducer is already gone (the shell flattened the reply), so the + // escape-aware strip can't see it. A run of flattened DECRPM / DA / OSC-colour + // replies is dropped (DSR "n"/BEL/CR may separate them). + assert.equal( + sanitize("prompt$ 69;0$y2026;2$y2027;0$y2031;0$y2048;0$y").visibleText, + "prompt$ ", + ); + assert.equal(sanitize("a 1;2c11;rgb:1616/1616/1616n1;2c b").visibleText, "a b"); + // Lone DECRPM / OSC-colour / DECRPSS tokens are distinctive enough on their own. + assert.equal(sanitize("x 2026;2$y y").visibleText, "x y"); + assert.equal(sanitize("c 4;0;rgb:1818/1e1e/2626 d").visibleText, "c d"); + assert.equal(sanitize("tail 1$r0m end").visibleText, "tail end"); // flattened DECRPSS (#1238) + // Ambiguous lone tokens and ordinary words are preserved. + assert.equal(sanitize("see commit 1;2c now").visibleText, "see commit 1;2c now"); + assert.equal(sanitize("running a connection").visibleText, "running a connection"); + }); + + it("drops a flattened cursor-position-report (CPR) run, keeps a lone one", () => { + // The `;1RR`/`;R` flood from a prompt's CSI 6n re-query echoing at + // an idle prompt. Stripped as a run; a lone `;R` is ambiguous and kept. + assert.equal(sanitize(`prompt$ ${";1RR".repeat(40)}`).visibleText, "prompt$ "); + assert.equal(sanitize(`x ${"1;1R".repeat(20)} y`).visibleText, "x y"); + assert.equal(sanitize("at 12;5R done").visibleText, "at 12;5R done"); // lone, kept + }); + + it("drops a flattened secondary-DA (three-parameter) run", () => { + // `CSI > Pp;Pv;Pc c` flattens to ">0;276;0c" (the ">" is sometimes kept by + // the echo); stripped in a run like the two-parameter primary form. + assert.equal(sanitize("prompt$ >0;276;0c>0;276;0c").visibleText, "prompt$ "); + assert.equal(sanitize("a 0;276;0c1;2c b").visibleText, "a b"); + // A lone three-parameter token is ambiguous and kept. + assert.equal(sanitize("ver 0;276;0c here").visibleText, "ver 0;276;0c here"); + }); + + it("does not over-match ordinary text that merely looks reply-shaped", () => { + // The colour alternative is pinned to OSC 10/11/12 and OSC 4 (`4;;`), so + // an arbitrary ";rgb:…" in program output survives. + assert.equal(sanitize("set 1;rgb:ff/00/00 now").visibleText, "set 1;rgb:ff/00/00 now"); + assert.equal(sanitize("hsl 7;rgb:aabbcc done").visibleText, "hsl 7;rgb:aabbcc done"); + // A DECRPM/DA token immediately followed by a word must not swallow its first + // letter (regression: a trailing "n?" used to eat the "n" of "next"). + assert.equal(sanitize("v 1;2$ynext").visibleText, "v next"); + // The DECRPSS payload is length-bounded so it can't eat a following number run. + assert.equal( + sanitize("tail 1$r0;120;340;Hello there").visibleText, + "tail 1$r0;120;340;Hello there", + ); + // A space is ordinary text, not a run separator: an ambiguous lone token next + // to a genuine one must not be bridged into a deletable run — only the + // unambiguous DECRPM token goes. + assert.equal(sanitize("see 1;2c 2026;2$y now").visibleText, "see 1;2c now"); + assert.equal( + sanitize("coords 5;10c 6;11c 7;12c here").visibleText, + "coords 5;10c 6;11c 7;12c here", + ); + }); + + it("strips an OSC 4 palette query from scrollback but relays it live", () => { + // A replayed OSC 4 query (`OSC 4;;? ST`) makes the emulator re-answer, + // and the echoed answer garbles the prompt — so scrollback drops it, while + // the live stream relays it for the client to answer (the answer is then + // stripped by the input filter, breaking the loop). + const query = "\x1b]4;1;?\x07"; + assert.equal(sanitize(`a${query}b`).visibleText, "ab"); + assert.equal( + sanitizeTerminalHistoryChunk("", `a${query}b`, { responsesOnly: true }).visibleText, + `a${query}b`, + ); + }); + + it("preserves a framed OSC 4 palette report instead of mangling its inner rgb", () => { + // The escape walk keeps a framed OSC 4 report (only OSC 10/11/12 are stripped), + // so the flattened pass must not delete the inner `4;;rgb:…` and leave a + // broken `ESC ] … ST` shell — in either view. The flattened (unframed) form is + // still dropped. + const framed = "\x1b]4;1;rgb:ff/00/00\x07"; + assert.equal(sanitize(`a ${framed} b`).visibleText, `a ${framed} b`); + assert.equal( + sanitizeTerminalHistoryChunk("", `a ${framed} b`, { responsesOnly: true }).visibleText, + `a ${framed} b`, + ); + assert.equal(sanitize("echo 4;1;rgb:ff/00/00 here").visibleText, "echo here"); + }); + + it("strips a huge adversarial ';'-run in linear time (no ReDoS)", () => { + // A program-controlled buffer of many ";" groups that never reaches + // "rgb:" used to drive catastrophic backtracking (tens of seconds). The + // pinned colour alternative makes this fail fast. + const evil = "1".repeat(20) + ";"; + const start = process.hrtime.bigint(); + sanitize(`${evil.repeat(16000)}rgb`); + const ms = Number(process.hrtime.bigint() - start) / 1e6; + assert.ok(ms < 1000, `flattened strip took ${ms}ms — possible ReDoS`); + }); + + it("handles a report split across chunks via the pending buffer", () => { + const first = sanitize("tail\x1b[?69;0"); + assert.equal(first.visibleText, "tail"); + assert.notEqual(first.pendingControlSequence, ""); + const second = sanitize("$ydone", first.pendingControlSequence); + assert.equal(second.visibleText, "done"); + }); + + it("flushes an over-long unterminated introducer instead of freezing the stream", () => { + // A stray OSC/DCS introducer with no terminator (binary output, a program + // killed mid-escape-write) must not swallow all subsequent output into the + // pending buffer forever — past the cap the remainder flushes verbatim and + // the stream recovers. + let pending = ""; + let emitted = ""; + const first = sanitize("before\x1b]0;never-terminated-", pending); + emitted += first.visibleText; + pending = first.pendingControlSequence; + assert.equal(first.visibleText, "before"); + assert.notEqual(pending, ""); + const chunk = "x".repeat(8 * 1024); + for (let i = 0; i < 12; i += 1) { + const step = sanitize(chunk, pending); + emitted += step.visibleText; + pending = step.pendingControlSequence; + } + assert.ok( + emitted.includes("x".repeat(1024)), + "stream stayed frozen after an unterminated introducer", + ); + assert.equal(pending, ""); + // Later output flows normally again. + assert.equal(sanitize("after", pending).visibleText, "after"); + }); + + it("still strips framed replies inside an overflow-recovered tail", () => { + // The overflow recovery re-sanitizes the remainder after the stuck + // introducer instead of flushing it raw, so a framed capability reply + // buried past an unterminated introducer can't land in history and replay. + const junk = "j".repeat(70 * 1024); + const result = sanitize(`\x9d${junk}\x1b[?2026;2$yvisible`); + assert.equal(result.visibleText.includes("2026;2$y"), false); + assert.equal(result.visibleText.includes("visible"), true); + assert.equal(result.visibleText.includes(junk.slice(0, 1024)), true); // junk kept (raw text) + }); + + it("preserves scrollback after an unterminated introducer on whole-buffer load", () => { + // A log written by an older build can contain an introducer with no + // terminator partway through; everything after it must survive the + // load-time sanitize verbatim, since the result is persisted back over the + // log file (dropping the pending tail would be permanent truncation). + const raw = `early history\n\x1b]0;unterminated${"later content\n".repeat(50)}`; + assert.equal(sanitizePersistedTerminalHistory(raw), raw); + // Residue in the terminated portion is still stripped; the tail survives. + const dirty = "prompt$ \x1b[?2026;2$y rest\x1b]0;tail-without-terminator"; + const cleaned = sanitizePersistedTerminalHistory(dirty); + assert.equal(cleaned.includes("$y"), false); + assert.equal(cleaned.includes("tail-without-terminator"), true); + }); + + it("self-heals a flattened token split across PTY chunks on the next reload", () => { + // A *flattened* reply (ESC introducer already gone) has no escape framing for + // the pending buffer to hold, so if a PTY read splits it mid-token both halves + // are written to history live (transient garble). But they land contiguously, + // so readHistory()'s whole-buffer sanitize rejoins and strips them on restore — + // the residue does not return after a restart (the persistence concern in the + // cross-chunk #1238 follow-up). + const live = sanitize("prompt$ 2026;2").visibleText + sanitize("$y done").visibleText; + assert.equal(live, "prompt$ 2026;2$y done"); // contiguous in the persisted log + assert.equal(sanitize(live).visibleText, "prompt$ done"); // stripped on reload + }); + + it("strips the real-world restore residue reported in issue #1238", () => { + // The exact escape-reply fragments a user saw flood the prompt on terminal + // restore: "2026;2$y2027;0$y2031;0$y2048;0$y1$r0m" — DECRPM mode reports + // (CSI ? Pm ; Ps $ y) plus a DECRPSS status reply (DCS Ps $ r D…D ST), + // reconstructed as the raw sequences the replayed history carried. + const residue = "\x1b[?2026;2$y\x1b[?2027;0$y\x1b[?2031;0$y\x1b[?2048;0$y\x1bP1$r0m\x1b\\"; + assert.equal(sanitize(`prompt$ ${residue}`).visibleText, "prompt$ "); + }); + + describe("responsesOnly (live stream)", () => { + const live = (data: string, pending = "") => + sanitizeTerminalHistoryChunk(pending, data, { responsesOnly: true }); + + it("strips terminal responses (DA, DECRPM, cursor, DSR) that leak as garbage", () => { + const responses = "\x1b[?1;2c\x1b[?2026;2$y\x1b[2;5R\x1b[0n"; + assert.equal(live(`a${responses}b`).visibleText, "ab"); + }); + + it("keeps framed OSC 10/11/12 rgb output — the legitimate set-colour command", () => { + // In OUTPUT that shape is a host→terminal set (themes), not a response; + // the reply form only travels as client input, where it is stripped. + const set = "\x1b]11;rgb:1616/1616/1616\x07"; + assert.equal(live(`a${set}b`).visibleText, `a${set}b`); + assert.equal(sanitize(`a${set}b`).visibleText, `a${set}b`); + }); + + it("keeps queries the client must still answer (DECRQM, DA, DSR, OSC colour)", () => { + const queries = "\x1b[?2026$p\x1b[c\x1b[6n\x1b]11;?\x07"; + assert.equal(live(`x${queries}y`).visibleText, `x${queries}y`); + }); + + it("keeps ordinary display sequences", () => { + assert.equal(live("\x1b[31mred\x1b[0m up").visibleText, "\x1b[31mred\x1b[0m up"); + }); + + it("relays a query split across chunks while history strips it", () => { + // The query (DECRQM `$p`) arrives in two pieces. The live view must relay + // it across the pending boundary; the scrollback view strips it. + const liveFirst = live("x\x1b[?2026"); + assert.equal(liveFirst.visibleText, "x"); + assert.notEqual(liveFirst.pendingControlSequence, ""); + assert.equal(live("$py", liveFirst.pendingControlSequence).visibleText, "\x1b[?2026$py"); + + const histFirst = sanitize("x\x1b[?2026"); + assert.equal(histFirst.visibleText, "x"); + assert.equal(sanitize("$py", histFirst.pendingControlSequence).visibleText, "y"); + }); + + it("diverges within one chunk: strips the response, relays the query", () => { + // `\x1b[0n` is a DSR *response* (stripped by both views); `\x1b[6n` is the + // cursor-position *query* the client must answer (relayed live, stripped + // from scrollback). Same input, two outputs from one parse. + const data = "A\x1b[0n B\x1b[6n C"; + assert.equal(live(data).visibleText, "A B\x1b[6n C"); + assert.equal(sanitize(data).visibleText, "A B C"); + }); + }); + + describe("8-bit C1 introducers", () => { + it("strips an 8-bit CSI DECRPM report (0x9b … $ y) like its ESC[ form", () => { + assert.equal(sanitize("a\x9b?2026;2$yb").visibleText, "ab"); + // Live view strips the report too (it is a response, not a query). + assert.equal( + sanitizeTerminalHistoryChunk("", "a\x9b?2026;2$yb", { responsesOnly: true }).visibleText, + "ab", + ); + }); + + it("keeps an 8-bit OSC colour set (0x9d … BEL); relays the query live only", () => { + // The rgb form in output is the legitimate set-colour command — kept in + // both views, 8-bit framing included. + assert.equal( + sanitize("a\x9d11;rgb:1616/1616/1616\x07b").visibleText, + "a\x9d11;rgb:1616/1616/1616\x07b", + ); + // The `?` colour query is relayed live (the client must answer it) but + // stripped from scrollback so a replay cannot re-trigger it. + assert.equal( + sanitizeTerminalHistoryChunk("", "a\x9d11;?\x07b", { responsesOnly: true }).visibleText, + "a\x9d11;?\x07b", + ); + assert.equal(sanitize("a\x9d11;?\x07b").visibleText, "ab"); + }); + + it("buffers an incomplete 8-bit CSI across chunks", () => { + const first = sanitize("tail\x9b?69;0"); + assert.equal(first.visibleText, "tail"); + assert.notEqual(first.pendingControlSequence, ""); + assert.equal(sanitize("$ydone", first.pendingControlSequence).visibleText, "done"); + }); + }); + + describe("DCS status strings (DECRQSS / DECRPSS)", () => { + const live = (data: string) => sanitizeTerminalHistoryChunk("", data, { responsesOnly: true }); + + it("strips a DECRPSS status reply (DCS Ps $ r D…D ST) from both views", () => { + assert.equal(sanitize("a\x1bP1$r0m\x1b\\b").visibleText, "ab"); + assert.equal(live("a\x1bP1$r0m\x1b\\b").visibleText, "ab"); + }); + + it("relays a DECRQSS query (DCS $ q D…D ST) live but strips it from scrollback", () => { + assert.equal(live("a\x1bP$qm\x1b\\b").visibleText, "a\x1bP$qm\x1b\\b"); + assert.equal(sanitize("a\x1bP$qm\x1b\\b").visibleText, "ab"); + }); + + it("leaves other DCS strings (sixel, DECUDK) untouched", () => { + const sixel = "\x1bPq#0;2;0;0;0#0~~\x1b\\"; + assert.equal(sanitize(`a${sixel}b`).visibleText, `a${sixel}b`); + assert.equal(live(`a${sixel}b`).visibleText, `a${sixel}b`); + }); + }); +}); + +describe("stripTerminalResponsesFromInput", () => { + it("drops the browser's auto-replies that drive the echo loop", () => { + const flood = + "\x1b[?69;0$y\x1b[?2026;2$y\x1b[?1;2c\x1b]11;rgb:1616/1616/1616\x1b\\\x1b[0n\x1bP1$r0m\x1b\\\x1b[>0;276;0c"; + assert.equal(stripTerminalResponsesFromInput(flood), ""); + }); + + it("accepts the 8-bit ST (0x9c) terminator for OSC/DCS replies", () => { + assert.equal(stripTerminalResponsesFromInput("\x1b]11;rgb:1616/1616/1616\x9c"), ""); + assert.equal(stripTerminalResponsesFromInput("\x1bP1$r0m\x9c"), ""); + }); + + it("strips OSC 4 palette colour replies so they can't re-arm the echo loop", () => { + assert.equal(stripTerminalResponsesFromInput("\x1b]4;1;rgb:1616/1616/1616\x07"), ""); + assert.equal(stripTerminalResponsesFromInput("\x1b]4;255;rgb:ffff/0000/0000\x1b\\"), ""); + }); + + it("strips replies that use 8-bit C1 introducers (0x9b CSI, 0x9d OSC, 0x90 DCS)", () => { + assert.equal(stripTerminalResponsesFromInput("\x9b?69;0$y"), ""); // C1 CSI DECRPM + assert.equal(stripTerminalResponsesFromInput("\x9b>0;276;0c"), ""); // C1 CSI secondary DA + assert.equal(stripTerminalResponsesFromInput("\x9d4;1;rgb:1616/1616/1616\x9c"), ""); // C1 OSC 4 + C1 ST + assert.equal(stripTerminalResponsesFromInput("\x901$r0m\x9c"), ""); // C1 DCS DECRPSS + }); + + it("keeps focus events so DECSET ?1004 programs (vim/tmux) still receive them", () => { + assert.equal(stripTerminalResponsesFromInput("\x1b[I"), "\x1b[I"); // focus in + assert.equal(stripTerminalResponsesFromInput("\x1b[O"), "\x1b[O"); // focus out + }); + + it("strips cursor-position report (CPR) replies that drive the prompt redraw flood", () => { + assert.equal(stripTerminalResponsesFromInput("\x1b[1;1R"), ""); // CPR reply + assert.equal(stripTerminalResponsesFromInput("\x1b[;1R"), ""); // empty-row CPR + assert.equal(stripTerminalResponsesFromInput("\x1b[1;1R\x1b[1;1R\x1b[1;1R"), ""); // flood + assert.equal(stripTerminalResponsesFromInput("\x9b5;10R"), ""); // 8-bit C1 CPR + // DEC-private CPR (`CSI ? r;c R`, the DECXCPR answer to `CSI ? 6 n`) — + // matches the output strip so neither form can feed the echo loop. + assert.equal(stripTerminalResponsesFromInput("\x1b[?1;1R"), ""); + assert.equal(stripTerminalResponsesFromInput("\x9b?5;10R"), ""); + }); + + it("keeps CPR-shaped bytes when no cursor query is outstanding (modified F3)", () => { + // `CSI 1;2R` is byte-identical to xterm's Shift+F3 — without a recently + // relayed `CSI 6 n` the write path passes it through; the ungated reply + // shapes still strip. + assert.equal( + stripTerminalResponsesFromInput("\x1b[1;2R", { includeQueryGated: false }), + "\x1b[1;2R", + ); + assert.equal( + stripTerminalResponsesFromInput("\x1b[?2026;2$y\x1b[1;2R", { includeQueryGated: false }), + "\x1b[1;2R", + ); + assert.equal(stripTerminalResponsesFromInput("\x1b[1;2R", { includeQueryGated: true }), ""); + }); + + it("keeps real user input, cursor moves, and bare query forms", () => { + assert.equal(stripTerminalResponsesFromInput("ls -la\r"), "ls -la\r"); // keystrokes + assert.equal( + stripTerminalResponsesFromInput("\x1b[A\x1b[B\x1b[C\x1b[D"), + "\x1b[A\x1b[B\x1b[C\x1b[D", + ); // arrows + assert.equal(stripTerminalResponsesFromInput("\x03"), "\x03"); // Ctrl-C + assert.equal(stripTerminalResponsesFromInput("\x1b[1;5H"), "\x1b[1;5H"); // cursor-move (H, not CPR) + assert.equal(stripTerminalResponsesFromInput("\x1b[c"), "\x1b[c"); // bare DA query kept + assert.equal(stripTerminalResponsesFromInput("\x1b[>c"), "\x1b[>c"); // bare secondary DA query kept + assert.equal(stripTerminalResponsesFromInput("\x1b[6n"), "\x1b[6n"); // DSR query kept + }); +}); + +// ─── Cross-layer grammar invariants ────────────────────────────────────────── +// Every sample in TERMINAL_SEQUENCE_GRAMMAR is exercised against every layer in +// every framing. These encode the consistency laws the individual layers must +// obey — the class of defect earlier review rounds kept finding ("handled in +// layer X, missed in layer Y") fails here instead of shipping. +describe("terminal sequence grammar invariants", () => { + const historyView = (data: string) => sanitizeTerminalHistoryChunk("", data).visibleText; + const liveView = (data: string) => + sanitizeTerminalHistoryChunk("", data, { responsesOnly: true }).visibleText; + + type Kind = (typeof TERMINAL_SEQUENCE_GRAMMAR)[number]["kind"]; + // 7-bit, 8-bit-C1, and (for string sequences) alternate-terminator framings. + const framings = (kind: Kind, body: string): ReadonlyArray<[label: string, framed: string]> => { + switch (kind) { + case "csi": + return [ + ["7-bit", `\x1b[${body}`], + ["8-bit", `\x9b${body}`], + ]; + case "osc": + return [ + ["7-bit BEL", `\x1b]${body}\x07`], + ["7-bit ST", `\x1b]${body}\x1b\\`], + ["8-bit", `\x9d${body}\x9c`], + ]; + case "dcs": + return [ + ["7-bit ST", `\x1bP${body}\x1b\\`], + ["7-bit BEL", `\x1bP${body}\x07`], + ["8-bit", `\x90${body}\x9c`], + ]; + } + }; + const strippedBy = (view: (data: string) => string, framed: string) => + view(`a${framed}b`) === "ab"; + const keptBy = (view: (data: string) => string, framed: string) => + view(`a${framed}b`) === `a${framed}b`; + + for (const descriptor of TERMINAL_SEQUENCE_GRAMMAR) { + describe(descriptor.name, () => { + const response = descriptor.response; + if (response) { + it("response: output views obey stripFromOutput in every framing", () => { + for (const sample of response.samples) { + for (const [label, framed] of framings(descriptor.kind, sample)) { + if (response.stripFromOutput) { + assert.equal( + strippedBy(historyView, framed), + true, + `history keeps ${label} ${sample}`, + ); + assert.equal(strippedBy(liveView, framed), true, `live keeps ${label} ${sample}`); + } else { + assert.equal( + keptBy(historyView, framed), + true, + `history strips ${label} ${sample}`, + ); + assert.equal(keptBy(liveView, framed), true, `live strips ${label} ${sample}`); + } + } + } + }); + + it("response: input filter obeys `input` in every framing", () => { + for (const sample of response.samples) { + for (const [label, framed] of framings(descriptor.kind, sample)) { + const filtered = stripTerminalResponsesFromInput(`a${framed}b`); + if (response.input !== null) { + assert.equal(filtered, "ab", `input relays ${label} ${sample}`); + } else { + assert.equal(filtered, `a${framed}b`, `input strips ${label} ${sample}`); + } + } + } + }); + + it("law: a response stripped from input is stripped from scrollback (OSC 4 rgb is the one documented exception)", () => { + // Replay safety: if the input filter drops the emulator reply, the + // persisted scrollback must never carry the framed reply either — + // except OSC 4 rgb, whose output shape is the legitimate set-palette + // command and MUST survive output. + const exception = !response.stripFromOutput; + for (const sample of response.samples) { + for (const [label, framed] of framings(descriptor.kind, sample)) { + if (response.input !== null && !exception) { + assert.equal( + strippedBy(historyView, framed), + true, + `input-stripped ${label} ${sample} survives scrollback`, + ); + } + } + } + }); + } + + const query = descriptor.query; + if (query) { + it("query: stripped from scrollback, relayed live, untouched in input", () => { + for (const sample of query.samples) { + for (const [label, framed] of framings(descriptor.kind, sample)) { + assert.equal( + strippedBy(historyView, framed), + true, + `history keeps query ${label} ${sample}`, + ); + assert.equal(keptBy(liveView, framed), true, `live strips query ${label} ${sample}`); + assert.equal( + stripTerminalResponsesFromInput(`a${framed}b`), + `a${framed}b`, + `input strips query ${label} ${sample}`, + ); + } + } + }); + } + + const flattened = descriptor.flattened; + if (flattened) { + it("flattened: a run always strips; a lone token strips iff loneStrippable", () => { + for (const sample of flattened.samples) { + const run = `${sample}${sample}`; + assert.equal(historyView(`a ${run} b`), "a b", `run survives history: ${sample}`); + assert.equal(liveView(`a ${run} b`), "a b", `run survives live: ${sample}`); + if (flattened.loneStrippable) { + assert.equal(historyView(`a ${sample} b`), "a b", `lone token survives: ${sample}`); + } else { + assert.equal( + historyView(`a ${sample} b`), + `a ${sample} b`, + `ambiguous lone token stripped: ${sample}`, + ); + } + } + }); + } + + it("law: 7-bit and 8-bit framings behave identically in every layer", () => { + const bodies = [...(response?.samples ?? []), ...(query?.samples ?? [])]; + for (const sample of bodies) { + const framed = framings(descriptor.kind, sample); + const reference = framed[0]; + if (!reference) continue; + for (const [label, form] of framed.slice(1)) { + for (const [layer, view] of [ + ["history", historyView], + ["live", liveView], + ["input", (data: string) => stripTerminalResponsesFromInput(data)], + ] as const) { + assert.equal( + view(`a${form}b`) === "ab", + view(`a${reference[1]}b`) === "ab", + `${layer} disagrees between ${reference[0]} and ${label} for ${sample}`, + ); + } + } + } + }); + + it("law: sanitizing is idempotent over every framed sample", () => { + const bodies = [...(response?.samples ?? []), ...(query?.samples ?? [])]; + for (const sample of bodies) { + for (const [label, framed] of framings(descriptor.kind, sample)) { + const once = historyView(`a${framed}b`); + assert.equal(historyView(once), once, `history not idempotent for ${label} ${sample}`); + const live = liveView(`a${framed}b`); + assert.equal(liveView(live), live, `live not idempotent for ${label} ${sample}`); + } + } + }); + }); + } +}); diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index caa5106bb9f..90bce62b413 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -1,3 +1,7 @@ +// @effect-diagnostics globalDateInEffect:off -- the CPR-gate grace window +// (lastCursorQueryRelayedAt) is wall-clock session state shared between the +// sync PTY drain and the write path; threading Clock through both is a +// follow-up. /** * TerminalManager - Terminal session orchestration service interface. * @@ -192,6 +196,14 @@ interface TerminalSubprocessInspectResult { readonly hasRunningSubprocess: boolean; readonly childCommand: string | null; readonly processIds: ReadonlyArray; + /** + * Whether the shell itself owns the PTY's foreground process group + * (`tpgid === pgid` of the terminal process) — i.e. the terminal is at an + * interactive prompt, even if background jobs (`… &`) exist. `undefined` + * when the platform/inspector can't tell (Windows, custom fixtures); the + * caller then falls back to `!hasRunningSubprocess`. + */ + readonly shellForeground?: boolean; } interface TerminalSubprocessInspector { @@ -251,6 +263,21 @@ export interface TerminalSessionState { unsubscribeData: (() => void) | null; unsubscribeExit: (() => void) | null; hasRunningSubprocess: boolean; + /** + * Whether the shell owns the PTY's foreground process group (idle prompt, + * possibly with background jobs). Drives the input reply-strip: strip only + * while true — a foreground job (vim, a CPR-based UI) is reading the replies + * to its own queries. Defaults true at spawn; refreshed by the subprocess + * poll (`tpgid === pgid`), falling back to `!hasRunningSubprocess` when the + * platform can't tell. + */ + shellForeground: boolean; + /** + * When a cursor-position query (`CSI 6 n`) was last relayed in the live + * stream (epoch ms; 0 = never). Arms the query-gated CPR strip in `write` + * for {@link CURSOR_QUERY_REPLY_GRACE_MS}. + */ + lastCursorQueryRelayedAt: number; /** Normalized child command name when `hasRunningSubprocess`; cleared when idle. */ childCommandLabel: string | null; runtimeEnv: Record | null; @@ -703,6 +730,32 @@ const posixInspectSubprocess = Effect.fn("terminal.posixInspectSubprocess")(func ProcessRunner.ProcessRunner > { const processRunner = yield* ProcessRunner.ProcessRunner; + + // Foreground-ownership probe: the PTY's foreground process group (`tpgid`) + // equals the shell's own group (`pgid`) exactly when the shell is at an + // interactive prompt — background jobs (`… &`) keep tpgid on the shell while + // a foreground vim/fzf moves it to the job's group. `undefined` when `ps` + // fails or the fields don't parse (callers fall back to the child check). + let shellForeground: boolean | undefined; + const tpgidResult = yield* Effect.exit( + processRunner.run({ + command: "ps", + args: ["-p", String(terminalPid), "-o", "tpgid=,pgid="], + timeout: "1 second", + maxOutputBytes: 8_192, + outputMode: "truncate", + timeoutBehavior: "timedOutResult", + }), + ); + if (tpgidResult._tag === "Success" && tpgidResult.value.code === 0) { + const [tpgidRaw, pgidRaw] = tpgidResult.value.stdout.trim().split(/\s+/g); + const tpgid = Number(tpgidRaw); + const pgid = Number(pgidRaw); + if (Number.isInteger(tpgid) && Number.isInteger(pgid) && tpgid > 0 && pgid > 0) { + shellForeground = tpgid === pgid; + } + } + const runPgrep = processRunner .run({ command: "pgrep", @@ -750,14 +803,24 @@ const posixInspectSubprocess = Effect.fn("terminal.posixInspectSubprocess")(func if (pgrepResult.value.code === 0) { childPid = parseFirstChildPidFromPgrep(pgrepResult.value.stdout); } else if (pgrepResult.value.code === 1) { - return { hasRunningSubprocess: false, childCommand: null, processIds: [] }; + return { + hasRunningSubprocess: false, + childCommand: null, + processIds: [], + ...(shellForeground !== undefined ? { shellForeground } : {}), + }; } } if (childPid === null) { const psResult = yield* Effect.exit(runPs); if (psResult._tag === "Failure" || psResult.value.code !== 0) { - return { hasRunningSubprocess: false, childCommand: null, processIds: [] }; + return { + hasRunningSubprocess: false, + childCommand: null, + processIds: [], + ...(shellForeground !== undefined ? { shellForeground } : {}), + }; } for (const line of psResult.value.stdout.split(/\r?\n/g)) { const [pidRaw, ppidRaw] = line.trim().split(/\s+/g); @@ -772,7 +835,12 @@ const posixInspectSubprocess = Effect.fn("terminal.posixInspectSubprocess")(func } if (childPid === null) { - return { hasRunningSubprocess: false, childCommand: null, processIds: [] }; + return { + hasRunningSubprocess: false, + childCommand: null, + processIds: [], + ...(shellForeground !== undefined ? { shellForeground } : {}), + }; } const runComm = processRunner.run({ @@ -837,6 +905,7 @@ const posixInspectSubprocess = Effect.fn("terminal.posixInspectSubprocess")(func hasRunningSubprocess: true, childCommand: normalized ? truncateTerminalWireLabel(normalized) : null, processIds: [...processIds], + ...(shellForeground !== undefined ? { shellForeground } : {}), }; }); @@ -852,37 +921,300 @@ function defaultSubprocessInspectorForPlatform(platform: NodeJS.Platform) { }); } -function capHistory(history: string, maxLines: number): string { +/** + * Upper bound on retained scrollback CHARACTERS, complementing the line limit. + * + * The line cap alone cannot bound a full-screen program: synchronized-output + * redraw frames (cursor addressing + `CSI K` clears, no newlines) make a + * single "line" arbitrarily large — observed 21 MB of history at only 4,999 + * lines from a TUI repainting on a spinner tick. That bloats server memory + * (`session.history`), the persisted log, and the reattach snapshot shipped + * to clients. 2 MiB comfortably holds 5,000 lines of ordinary shell output. + */ +const DEFAULT_HISTORY_CHAR_LIMIT = 2 * 1024 * 1024; + +function capHistory(history: string, maxLines: number, maxChars: number): string { if (history.length === 0) return history; const hasTrailingNewline = history.endsWith("\n"); const lines = history.split("\n"); if (hasTrailingNewline) { lines.pop(); } - if (lines.length <= maxLines) return history; - const capped = lines.slice(lines.length - maxLines).join("\n"); - return hasTrailingNewline ? `${capped}\n` : capped; + let capped = history; + if (lines.length > maxLines) { + const joined = lines.slice(lines.length - maxLines).join("\n"); + capped = hasTrailingNewline ? `${joined}\n` : joined; + } + if (capped.length > maxChars) { + // Cut from the head at the friendliest boundary inside the excess: the + // next line start if one exists, else the next escape introducer (a + // frame-ish boundary in redraw streams), else a hard cut. A torn leading + // sequence degrades exactly like the line cap tearing SGR state does. + const cut = capped.length - maxChars; + const newline = capped.indexOf("\n", cut); + if (newline !== -1 && newline < cut + 64 * 1024) { + capped = capped.slice(newline + 1); + } else { + const escape = capped.indexOf("\u001b", cut); + capped = capped.slice(escape !== -1 && escape < cut + 64 * 1024 ? escape : cut); + } + } + return capped; } function isCsiFinalByte(codePoint: number): boolean { return codePoint >= 0x40 && codePoint <= 0x7e; } -function shouldStripCsiSequence(body: string, finalByte: string): boolean { - if (finalByte === "n") { - return true; - } - if (finalByte === "R" && /^[0-9;?]*$/.test(body)) { - return true; - } - if (finalByte === "c" && /^[>0-9;?]*$/.test(body)) { - return true; - } +// ─── Canonical terminal capability-sequence grammar ───────────────────────── +// +// Single source of truth for every capability sequence the sanitizer knows. +// ALL matching layers derive from this table — the framed strip rules for the +// scrollback and live views, the client-input reply filter, and the flattened +// residue patterns — so a sequence type added here is wired into every layer +// at once and the layers cannot drift out of sync (the recurring review +// finding class: "handled in layer X but missed in layer Y"). +// +// Semantics per entry: +// response (terminal→host): spurious echo in output — stripped from history +// AND live when `stripFromOutput` (OSC 4 rgb is the exception: in output +// that shape is the legitimate set-palette command); stripped from client +// input via `input` (the emulator auto-reply that feeds the idle-prompt +// echo loop; null relays it). +// query (host→terminal): stripped from scrollback (a replay would re-trigger +// the emulator's answer) but relayed live so the client answers it. +// flattened: the reply's parameters echoed as visible text once the shell +// flattened away the introducer; stripped in runs, and alone only when +// `loneStrippable` (unambiguous shapes). +// +// `samples` hold concrete bodies for the cross-layer invariant tests: every +// layer is exercised against every sample in 7-bit and 8-bit framings, so a +// half-wired entry fails the suite instead of shipping. +export interface TerminalSequenceDescriptor { + readonly name: string; + readonly kind: "csi" | "osc" | "dcs"; + readonly response?: { + /** Framed body (CSI: full body incl. final byte; OSC/DCS: content prefix). */ + readonly body: string; + readonly stripFromOutput: boolean; + /** Input-filter body (null = relayed in input). */ + readonly input: string | null; + /** + * Only strip this reply from input while a matching query was recently + * relayed in the live stream. Needed when the reply's byte shape collides + * with real keyboard input (CPR `CSI 1;2R` == xterm's modified F3): the + * keystroke passes at a quiet prompt, while the query-driven echo flood is + * still broken because the querying prompt re-arms the gate every redraw. + */ + readonly inputRequiresRecentQuery?: boolean; + readonly samples: ReadonlyArray; + }; + readonly query?: { + readonly body: string; + readonly samples: ReadonlyArray; + }; + readonly flattened?: { + readonly source: string; + readonly loneStrippable: boolean; + readonly samples: ReadonlyArray; + }; +} + +export const TERMINAL_SEQUENCE_GRAMMAR: ReadonlyArray = [ + { + name: "DECRPM mode report / DECRQM query", + kind: "csi", + response: { + body: "[?0-9;]*\\$y", + stripFromOutput: true, + input: "\\?[0-9;]*\\$y", + samples: ["?2026;2$y", "?69;0$y"], + }, + query: { body: "[?0-9;]*\\$p", samples: ["?2026$p"] }, + flattened: { + source: "[0-9]+;[0-9]+\\$y", + loneStrippable: true, + samples: ["2026;2$y", "69;0$y"], + }, + }, + { + name: "device attributes (primary/secondary DA)", + kind: "csi", + response: { + // Responses carry parameters; the bare `CSI c` / `CSI ? c` / `CSI > c` + // query forms deliberately do NOT match (covered by `query` below). + body: "(?:\\?[0-9;]+|>[0-9]+;[0-9;]+)c", + stripFromOutput: true, + input: "[?>][0-9;]+c", + samples: ["?1;2c", ">0;276;0c"], + }, + query: { body: "[>0-9;?]*c", samples: ["c", ">c", "?c"] }, + flattened: { + // Two or three params, the `>` sometimes preserved by the echo. Run-only: + // a lone "1;2c" is indistinguishable from ordinary text. + source: ">?[0-9]+;[0-9]+(?:;[0-9]+)?c", + loneStrippable: false, + samples: ["1;2c", ">0;276;0c", "0;276;0c"], + }, + }, + { + name: "device status report (DSR)", + kind: "csi", + response: { + body: "\\??[03]n", + stripFromOutput: true, + input: "\\??[03]n", + samples: ["0n", "3n", "?0n"], + }, + // Any other `n`-final CSI (`CSI 5 n`, `CSI 6 n`, DEC forms) is a query — + // `[^]*` keeps scrollback stripping every n-final CSI, as it always has. + query: { body: "[^]*n", samples: ["5n", "6n"] }, + }, + { + name: "cursor position report (CPR / DECXCPR)", + kind: "csi", + response: { + body: "[0-9;?]*R", + stripFromOutput: true, + // Input requires the two-parameter reply shape so a bare `CSI R` or + // anything keystroke-like is never eaten — and is additionally gated on a + // recently-relayed `CSI 6 n` query, because `CSI 1;R` is also + // xterm's encoding for modified F3. + input: "\\??[0-9]*;[0-9]+R", + inputRequiresRecentQuery: true, + samples: ["1;1R", "?5;10R", ";1R"], + }, + flattened: { + // The `;1RR` prompt-flood shape; the echoed "R" can double. Run-only. + source: "[0-9]*;[0-9]+R+", + loneStrippable: false, + samples: [";1RR", "1;1R"], + }, + }, + { + name: "OSC 10/11/12 colour", + kind: "osc", + response: { + // In OUTPUT `OSC 1[012];rgb:` is the legitimate set-default-colour + // command (themes) — like OSC 4, never stripped there; the reply shape + // only travels as client input, where it is dropped. A framed reply + // replayed from an old log re-applies the colour the terminal already + // has (a no-op set), never re-triggering the echo loop. + body: "(?:10|11|12);rgb:", + stripFromOutput: false, + input: "1[012];rgb:[0-9a-fA-F/]*", + samples: ["11;rgb:1616/1616/1616", "10;rgb:ffff/ffff/ffff"], + }, + query: { body: "(?:10|11|12);\\?", samples: ["11;?"] }, + flattened: { + // Pinned to the real OSC numbers (never an unbounded `(?:[0-9]+;)+` run — + // that both over-matched ordinary ";rgb:…" text and was a ReDoS). The + // lookbehind skips a colour run still inside an intact OSC frame the + // escape walk chose to keep. + source: "(?;rgb:` is the legitimate set-palette command + // (themes) — never stripped there. The reply shape only travels as + // client input, where it is dropped. + body: "4;[0-9]+;rgb:", + stripFromOutput: false, + input: "4;[0-9]+;rgb:[0-9a-fA-F/]*", + samples: ["4;1;rgb:1616/1616/1616"], + }, + query: { body: "4;[0-9]+;\\?", samples: ["4;1;?"] }, + flattened: { + // Same framed-lookbehind guard as OSC 10/11/12: a kept framed OSC 4 + // set-palette command must not have its inner payload deleted. + source: "(?, +): RegExp | null { + if (sources.length === 0) return null; + const alternation = sources.map((source) => `(?:${source})`).join("|"); + return kind === "csi" ? new RegExp(`^(?:${alternation})$`) : new RegExp(`^(?:${alternation})`); +} + +function framedMatchers(kind: TerminalSequenceDescriptor["kind"]) { + const entries = TERMINAL_SEQUENCE_GRAMMAR.filter((descriptor) => descriptor.kind === kind); + return { + response: composeFramedMatcher( + kind, + entries.flatMap((d) => (d.response?.stripFromOutput ? [d.response.body] : [])), + ), + query: composeFramedMatcher( + kind, + entries.flatMap((d) => (d.query ? [d.query.body] : [])), + ), + }; +} + +const CSI_MATCHERS = framedMatchers("csi"); +const OSC_MATCHERS = framedMatchers("osc"); +const DCS_MATCHERS = framedMatchers("dcs"); + +/** + * Whether a CSI sequence should be dropped from the sanitized terminal stream. + * `responsesOnly` (the live view) strips only terminal→host responses; the + * default (scrollback) also strips host→terminal queries so a replay can't + * re-trigger one. Derived from {@link TERMINAL_SEQUENCE_GRAMMAR}. + */ +function shouldStripCsiSequence(body: string, finalByte: string, responsesOnly = false): boolean { + const full = `${body}${finalByte}`; + if (CSI_MATCHERS.response?.test(full)) return true; + if (!responsesOnly && CSI_MATCHERS.query?.test(full)) return true; return false; } -function shouldStripOscSequence(content: string): boolean { - return /^(10|11|12);(?:\?|rgb:)/.test(content); +/** OSC counterpart of {@link shouldStripCsiSequence} (tests content, not body+final). */ +function shouldStripOscSequence(content: string, responsesOnly = false): boolean { + if (OSC_MATCHERS.response?.test(content)) return true; + if (!responsesOnly && OSC_MATCHERS.query?.test(content)) return true; + return false; +} + +/** + * DCS counterpart. Only the `$`-intermediate capability-negotiation forms are + * classified; other DCS (sixel, DECUDK, tmux passthrough) is left untouched. + */ +function shouldStripDcsSequence(content: string, responsesOnly = false): boolean { + if (DCS_MATCHERS.response?.test(content)) return true; + if (!responsesOnly && DCS_MATCHERS.query?.test(content)) return true; + return false; } function stripStringTerminator(value: string): string { @@ -928,17 +1260,199 @@ function findEscapeSequenceEndIndex(input: string, start: number): number | null return isEscapeFinalByte(input.charCodeAt(cursor)) ? cursor + 1 : start + 1; } -function sanitizeTerminalHistoryChunk( - pendingControlSequence: string, +// Flattened-residue patterns, derived from the grammar. RUN strips 2+ +// fragments in sequence; TOKEN additionally strips the unambiguous shapes even +// in isolation. Fragments in an echoed run are adjacent or separated by BEL/CR/ +// a flattened DSR "n" tail — never by a space (verified against the captured +// logs): a space is ordinary text, and including it would let a run bridge +// across real words and delete an ambiguous lone token next to a genuine one +// ("see 1;2c 2026;2$y"). +const FLATTENED_SOURCES = TERMINAL_SEQUENCE_GRAMMAR.flatMap((descriptor) => + descriptor.flattened ? [descriptor.flattened] : [], +); +const FLATTENED_FRAGMENT = `(?:${FLATTENED_SOURCES.map((f) => `(?:${f.source})`).join("|")})`; +const FLATTENED_REPLY_RUN = new RegExp( + `${FLATTENED_FRAGMENT}(?:[\\x07\\rn]{0,8}${FLATTENED_FRAGMENT})+`, + "g", +); +const FLATTENED_REPLY_TOKEN = new RegExp( + `(?:${FLATTENED_SOURCES.filter((f) => f.loneStrippable) + .map((f) => `(?:${f.source})`) + .join("|")})`, + "g", +); +/** + * Drop the flattened terminal-reply residue a shell echoes at the prompt. + * + * When a capability reply lands at an idle prompt the shell echoes its + * *flattened* parameters as visible text (the ESC introducer is already gone, so + * the escape-aware strip can't see it). Two passes: drop a run of 2+ flattened + * fragments (DSR "n"/BEL/CR may separate them), then drop the unambiguous + * OSC-colour / DECRPM / DECRPSS tokens even when isolated. Ambiguous lone + * ";c" / "n" forms and ordinary words (e.g. "running", "1;2c") are kept. + */ +function stripFlattenedModeReplyResidue(text: string): string { + // Every fragment contains either ";" (DECRPM/DA/OSC) or "$r" (DECRPSS), so text + // with neither can't hold residue — skip the regexes. + if (!text.includes(";") && !text.includes("$r")) { + return text; + } + return text.replace(FLATTENED_REPLY_RUN, "").replace(FLATTENED_REPLY_TOKEN, ""); +} + +// Matches the terminal→host response sequences the browser emulator +// auto-generates in answer to a program's capability queries: DECRPM "$y", +// device-attributes "c", device-status "0n"/"3n", cursor-position report +// ";R" (CPR), OSC 10/11/12 + OSC 4 palette colour, and DECRPSS "$r". +// Each introducer accepts both the 7-bit ESC form and the 8-bit C1 byte (CSI +// 0x9b, OSC 0x9d, DCS 0x90), and each terminator the BEL, ESC\, or 8-bit ST +// (0x9c) — matching the output sanitizer so a C1-encoded reply can't slip past. +// +// CPR (`CSI ; R`) IS stripped: like the other capability replies it is +// an emulator auto-answer (to `CSI 6 n`), and a prompt that re-queries on redraw +// makes the echoed reply the worst runaway-flood source (issue: a prompt's +// `;1RR` flood). The `;`-separated two-parameter form is required, so it never +// matches a single keystroke or a bare `CSI R`. The bare DSR query forms are +// kept — the DA alternation requires a parameter so `CSI ? c` / `CSI > c` and +// `CSI 6 n` queries pass through. +// +// Focus in/out (CSI I / CSI O) are NOT stripped: a program that enabled focus +// reporting (DECSET ?1004 — vim, tmux) legitimately expects them, and they are +// user-action-driven so they never feed the runaway redraw-requery loop the +// capability responses do. +const INPUT_CSI = "(?:\\x1b\\[|\\x9b)"; +const INPUT_OSC = "(?:\\x1b\\]|\\x9d)"; +const INPUT_DCS = "(?:\\x1bP|\\x90)"; +const INPUT_ST = "(?:\\x07|\\x1b\\\\|\\x9c)"; +function composeInputMatcher(gated: boolean): RegExp | null { + const sources = TERMINAL_SEQUENCE_GRAMMAR.flatMap((descriptor) => { + const response = descriptor.response; + if (!response?.input) return []; + if ((response.inputRequiresRecentQuery ?? false) !== gated) return []; + switch (descriptor.kind) { + case "csi": + return [`${INPUT_CSI}(?:${response.input})`]; + case "osc": + return [`${INPUT_OSC}(?:${response.input})${INPUT_ST}`]; + case "dcs": + return [`${INPUT_DCS}(?:${response.input})${INPUT_ST}`]; + } + }); + return sources.length === 0 ? null : new RegExp(sources.join("|"), "g"); +} + +const INPUT_TERMINAL_RESPONSE = composeInputMatcher(false); +const INPUT_QUERY_GATED_RESPONSE = composeInputMatcher(true); +// A relayed cursor-position query in the LIVE stream (`CSI 6 n` / `CSI ? 6 n`, +// 7-bit or 8-bit) — arms the query-gated input strip for a grace window. +const RELAYED_CURSOR_QUERY = new RegExp(`${INPUT_CSI}\\??6n`); +/** + * How long after relaying a cursor-position query the CPR reply strip stays + * armed. A prompt that re-queries on every redraw (the flood scenario) + * constantly re-arms it; a quiet prompt lets modified-F3 keystrokes through. + */ +export const CURSOR_QUERY_REPLY_GRACE_MS = 5_000; +/** + * Strip the browser emulator's auto-generated terminal responses from client + * input before it reaches the PTY. + * + * The emulator answers the program's capability queries (DECRPM, device + * attributes, device status, cursor position, OSC colour) and emits focus + * events, sending them all as input. At an idle prompt the shell has no reader + * for them, so it echoes them — and a prompt that re-queries on redraw turns + * that into a runaway feedback loop. A user never types these, so dropping them + * at the source breaks the loop. The cursor-position report (CPR) is the most + * aggressive offender (a prompt's `;1RR` flood), so its two-parameter + * `CSI ; R` reply is stripped too; only the bare query forms + * (`CSI 6 n`, `CSI ? c`) are kept. + * + * The write path only applies this while NO foreground subprocess is running + * (the idle-prompt scenario above): a running program (vim, a CPR-based UI) is + * presumed to be reading the replies to its own queries, and stripping them + * would stall its capability negotiation. Exported for unit testing. + */ +export function stripTerminalResponsesFromInput( data: string, -): { visibleText: string; pendingControlSequence: string } { - const input = `${pendingControlSequence}${data}`; - let visibleText = ""; + options: { + /** + * Include the query-gated reply shapes (CPR). Defaults true (the full + * strip); the write path passes whether a cursor query was recently + * relayed, so an idle prompt with no outstanding query lets the + * byte-identical modified-F3 keystrokes through. + */ + readonly includeQueryGated?: boolean; + } = {}, +): string { + // Skip the regexes unless the data carries a 7-bit ESC or one of the 8-bit C1 + // introducers (CSI 0x9b, OSC 0x9d, DCS 0x90) a response could start with. + const hasIntroducer = + data.includes("\x1b") || + data.includes("\x9b") || + data.includes("\x9d") || + data.includes("\x90"); + if (!hasIntroducer) return data; + let stripped = INPUT_TERMINAL_RESPONSE ? data.replace(INPUT_TERMINAL_RESPONSE, "") : data; + if ((options.includeQueryGated ?? true) && INPUT_QUERY_GATED_RESPONSE) { + stripped = stripped.replace(INPUT_QUERY_GATED_RESPONSE, ""); + } + return stripped; +} + +// Upper bound on the buffered incomplete-sequence remainder. An unterminated +// OSC/DCS introducer (a program killed mid-escape-write, or binary output +// containing a stray 0x9d/0x90 byte) would otherwise swallow ALL subsequent +// output into `pendingControlSequence` forever — freezing the live stream and +// growing server memory unboundedly. Past this cap the stuck introducer is +// emitted verbatim and the rest re-sanitized (see sanitizeTerminalChunkDual), +// so the stream recovers and complete sequences after the introducer still get +// stripped. Sized to hold any realistic legitimate cross-chunk sequence (OSC 52 +// clipboard payloads, tmux-passthrough wrappers); a larger-than-cap sixel +// merely degrades to raw passthrough of bytes the sanitizer keeps anyway. +const MAX_PENDING_CONTROL_SEQUENCE_LENGTH = 64 * 1024; + +/** + * Single parse of a chunk that produces BOTH sanitized views at once: + * - `historyText`: the scrollback strip — drops terminal queries AND responses + * so a replay can never re-trigger a query whose answer would echo. + * - `liveText`: the live-stream strip — drops only terminal→host responses + * (spurious echo) while relaying host→terminal queries the client answers. + * + * Both share one walk and one pending-sequence boundary: the boundary depends + * only on byte structure (where an incomplete escape sequence ends), never on + * which complete sequences are stripped, so it is identical for both views. + */ +function sanitizeTerminalChunkOnce(input: string): { + historyText: string; + liveText: string; + pendingControlSequence: string; +} { + let historyText = ""; + let liveText = ""; let index = 0; - const append = (value: string) => { - visibleText += value; + // Ordinary text and sequences neither view strips go to both buffers. + const appendBoth = (value: string) => { + historyText += value; + liveText += value; + }; + // A CSI sequence: each view keeps it unless its own strip rule removes it. + const appendCsi = (sequence: string, body: string, finalByte: string) => { + if (!shouldStripCsiSequence(body, finalByte, false)) historyText += sequence; + if (!shouldStripCsiSequence(body, finalByte, true)) liveText += sequence; + }; + const appendOsc = (sequence: string, content: string) => { + if (!shouldStripOscSequence(content, false)) historyText += sequence; + if (!shouldStripOscSequence(content, true)) liveText += sequence; }; + const appendDcs = (sequence: string, content: string) => { + if (!shouldStripDcsSequence(content, false)) historyText += sequence; + if (!shouldStripDcsSequence(content, true)) liveText += sequence; + }; + const pending = () => ({ + historyText: stripFlattenedModeReplyResidue(historyText), + liveText: stripFlattenedModeReplyResidue(liveText), + pendingControlSequence: input.slice(index), + }); while (index < input.length) { const codePoint = input.charCodeAt(index); @@ -946,7 +1460,7 @@ function sanitizeTerminalHistoryChunk( if (codePoint === 0x1b) { const nextCodePoint = input.charCodeAt(index + 1); if (Number.isNaN(nextCodePoint)) { - return { visibleText, pendingControlSequence: input.slice(index) }; + return pending(); } if (nextCodePoint === 0x5b) { @@ -955,16 +1469,14 @@ function sanitizeTerminalHistoryChunk( if (isCsiFinalByte(input.charCodeAt(cursor))) { const sequence = input.slice(index, cursor + 1); const body = input.slice(index + 2, cursor); - if (!shouldStripCsiSequence(body, input[cursor] ?? "")) { - append(sequence); - } + appendCsi(sequence, body, input[cursor] ?? ""); index = cursor + 1; break; } cursor += 1; } if (cursor >= input.length) { - return { visibleText, pendingControlSequence: input.slice(index) }; + return pending(); } continue; } @@ -977,12 +1489,16 @@ function sanitizeTerminalHistoryChunk( ) { const terminatorIndex = findStringTerminatorIndex(input, index + 2); if (terminatorIndex === null) { - return { visibleText, pendingControlSequence: input.slice(index) }; + return pending(); } const sequence = input.slice(index, terminatorIndex); const content = stripStringTerminator(input.slice(index + 2, terminatorIndex)); - if (nextCodePoint !== 0x5d || !shouldStripOscSequence(content)) { - append(sequence); + if (nextCodePoint === 0x5d) { + appendOsc(sequence, content); + } else if (nextCodePoint === 0x50) { + appendDcs(sequence, content); + } else { + appendBoth(sequence); } index = terminatorIndex; continue; @@ -990,9 +1506,9 @@ function sanitizeTerminalHistoryChunk( const escapeSequenceEndIndex = findEscapeSequenceEndIndex(input, index + 1); if (escapeSequenceEndIndex === null) { - return { visibleText, pendingControlSequence: input.slice(index) }; + return pending(); } - append(input.slice(index, escapeSequenceEndIndex)); + appendBoth(input.slice(index, escapeSequenceEndIndex)); index = escapeSequenceEndIndex; continue; } @@ -1003,16 +1519,14 @@ function sanitizeTerminalHistoryChunk( if (isCsiFinalByte(input.charCodeAt(cursor))) { const sequence = input.slice(index, cursor + 1); const body = input.slice(index + 1, cursor); - if (!shouldStripCsiSequence(body, input[cursor] ?? "")) { - append(sequence); - } + appendCsi(sequence, body, input[cursor] ?? ""); index = cursor + 1; break; } cursor += 1; } if (cursor >= input.length) { - return { visibleText, pendingControlSequence: input.slice(index) }; + return pending(); } continue; } @@ -1020,22 +1534,119 @@ function sanitizeTerminalHistoryChunk( if (codePoint === 0x9d || codePoint === 0x90 || codePoint === 0x9e || codePoint === 0x9f) { const terminatorIndex = findStringTerminatorIndex(input, index + 1); if (terminatorIndex === null) { - return { visibleText, pendingControlSequence: input.slice(index) }; + return pending(); } const sequence = input.slice(index, terminatorIndex); const content = stripStringTerminator(input.slice(index + 1, terminatorIndex)); - if (codePoint !== 0x9d || !shouldStripOscSequence(content)) { - append(sequence); + if (codePoint === 0x9d) { + appendOsc(sequence, content); + } else if (codePoint === 0x90) { + appendDcs(sequence, content); + } else { + appendBoth(sequence); } index = terminatorIndex; continue; } - append(input[index] ?? ""); + appendBoth(input[index] ?? ""); index += 1; } - return { visibleText, pendingControlSequence: "" }; + return { + historyText: stripFlattenedModeReplyResidue(historyText), + liveText: stripFlattenedModeReplyResidue(liveText), + pendingControlSequence: "", + }; +} + +// Bounded number of overflow recoveries per chunk. Each recovery emits the +// stuck introducer verbatim and RE-SANITIZES the rest of the remainder, so +// complete sequences after an unterminated introducer still get stripped +// instead of being flushed raw into history. The bound keeps an adversarial +// introducer-flood from turning the walk quadratic; past it the remainder is +// flushed verbatim (raw passthrough). +const MAX_PENDING_OVERFLOW_RECOVERIES = 4; + +/** + * {@link sanitizeTerminalChunkOnce} plus overflow recovery: when the buffered + * incomplete-sequence remainder exceeds {@link MAX_PENDING_CONTROL_SEQUENCE_LENGTH} + * (an unterminated OSC/DCS introducer would otherwise swallow all subsequent + * output forever), the stuck introducer is emitted verbatim and the rest is + * re-walked so it is still properly sanitized. + */ +function sanitizeTerminalChunkDual( + pendingControlSequence: string, + data: string, +): { historyText: string; liveText: string; pendingControlSequence: string } { + let historyText = ""; + let liveText = ""; + let input = `${pendingControlSequence}${data}`; + for (let recoveries = 0; recoveries < MAX_PENDING_OVERFLOW_RECOVERIES; recoveries += 1) { + const walk = sanitizeTerminalChunkOnce(input); + historyText += walk.historyText; + liveText += walk.liveText; + if (walk.pendingControlSequence.length <= MAX_PENDING_CONTROL_SEQUENCE_LENGTH) { + return { + historyText, + liveText, + pendingControlSequence: walk.pendingControlSequence, + }; + } + // Overflowed: the remainder starts at an introducer that never terminated. + // Emit the introducer bytes verbatim and re-sanitize everything after them. + const stuck = walk.pendingControlSequence; + const introducerLength = stuck.charCodeAt(0) === 0x1b ? 2 : 1; + const introducer = stuck.slice(0, introducerLength); + historyText += introducer; + liveText += introducer; + input = stuck.slice(introducerLength); + } + // Recovery budget exhausted (adversarial introducer flood): flush raw. + return { + historyText: historyText + input, + liveText: liveText + input, + pendingControlSequence: "", + }; +} + +/** + * Sanitize one chunk of terminal output. `responsesOnly` selects the live-stream + * view (strips only terminal responses, relaying queries the client answers); + * the default selects the scrollback view (also strips queries). Both are + * computed in one pass — see {@link sanitizeTerminalChunkDual}. Exported for unit + * testing. + */ +export function sanitizeTerminalHistoryChunk( + pendingControlSequence: string, + data: string, + options: { readonly responsesOnly?: boolean } = {}, +): { visibleText: string; pendingControlSequence: string } { + const dual = sanitizeTerminalChunkDual(pendingControlSequence, data); + return { + visibleText: (options.responsesOnly ?? false) ? dual.liveText : dual.historyText, + pendingControlSequence: dual.pendingControlSequence, + }; +} + +/** + * Sanitize a WHOLE persisted scrollback buffer for load/migration, losslessly. + * + * Unlike the per-chunk API, there is no next chunk: a trailing incomplete + * sequence (an unterminated OSC/DCS introducer a program left mid-write) must + * be appended back VERBATIM rather than held in `pendingControlSequence` — + * discarding it would silently truncate everything after the introducer, and + * the caller persists the result over the log file, making the loss permanent. + * Exported for unit testing. + */ +export function sanitizePersistedTerminalHistory(raw: string): string { + // Single walk with NO overflow recovery: the streaming path's 64 KiB cap + // exists to keep a LIVE stream from freezing, but here the whole buffer is + // already in hand and the result is persisted back over the log file — the + // trailing incomplete sequence must be preserved byte-for-byte however large + // it is, never re-sanitized as if it were ordinary text. + const result = sanitizeTerminalChunkOnce(raw); + return `${result.historyText}${result.pendingControlSequence}`; } function legacySafeThreadId(threadId: string): string { @@ -1142,6 +1753,7 @@ function normalizedRuntimeEnv( interface TerminalManagerOptions { logsDir: string; historyLineLimit?: number; + historyCharLimit?: number; ptyAdapter: PtyAdapter.PtyAdapter["Service"]; shellResolver?: () => string; env?: NodeJS.ProcessEnv; @@ -1182,6 +1794,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func const logsDir = options.logsDir; const historyLineLimit = options.historyLineLimit ?? DEFAULT_HISTORY_LINE_LIMIT; + const historyCharLimit = options.historyCharLimit ?? DEFAULT_HISTORY_CHAR_LIMIT; const platform = yield* HostProcessPlatform; // Terminals must inherit the user's full environment (minus the blocklist // applied in createTerminalSpawnEnv) — an allowlist here silently strips @@ -1451,7 +2064,15 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func (cause) => new TerminalHistoryError({ operation: "read", threadId, terminalId, cause }), ), ); - const capped = capHistory(raw, historyLineLimit); + // Sanitize on load so terminal query/response residue persisted by older + // builds (the "…$y" / colour-report garble) is stripped from replayed + // scrollback — not just from newly-written output. Idempotent for clean + // logs; the rewrite below persists the cleanup. + const capped = capHistory( + sanitizePersistedTerminalHistory(raw), + historyLineLimit, + historyCharLimit, + ); if (capped !== raw) { yield* fileSystem .writeFileString(nextPath, capped) @@ -1491,7 +2112,12 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func new TerminalHistoryError({ operation: "migrate", threadId, terminalId, cause }), ), ); - const capped = capHistory(raw, historyLineLimit); + // Sanitize while migrating so the new-path log starts clean (see above). + const capped = capHistory( + sanitizePersistedTerminalHistory(raw), + historyLineLimit, + historyCharLimit, + ); yield* fileSystem .writeFileString(nextPath, capped) .pipe( @@ -1671,15 +2297,24 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func } if (nextEvent.type === "output") { - const sanitized = sanitizeTerminalHistoryChunk( + // One parse yields both views: the scrollback strip (drops queries and + // responses) feeds history; the live strip (drops only responses, + // relaying queries the client answers) feeds the streamed data. + const sanitized = sanitizeTerminalChunkDual( session.pendingHistoryControlSequence, nextEvent.data, ); session.pendingHistoryControlSequence = sanitized.pendingControlSequence; - if (sanitized.visibleText.length > 0) { + // A relayed cursor query means a CPR reply is genuinely expected — + // arm the query-gated input strip for the grace window. + if (RELAYED_CURSOR_QUERY.test(sanitized.liveText)) { + session.lastCursorQueryRelayedAt = Date.now(); + } + if (sanitized.historyText.length > 0) { session.history = capHistory( - `${session.history}${sanitized.visibleText}`, + `${session.history}${sanitized.historyText}`, historyLineLimit, + historyCharLimit, ); } const eventStamp = advanceEventSequence(session); @@ -1689,8 +2324,8 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func threadId: session.threadId, terminalId: session.terminalId, sequence: eventStamp.sequence, - history: sanitized.visibleText.length > 0 ? session.history : null, - data: nextEvent.data, + history: sanitized.historyText.length > 0 ? session.history : null, + data: sanitized.liveText, } as const; } @@ -1699,6 +2334,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.process = null; session.pid = null; session.hasRunningSubprocess = false; + session.shellForeground = true; session.childCommandLabel = null; session.status = "exited"; session.pendingHistoryControlSequence = ""; @@ -1771,6 +2407,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.process = null; session.pid = null; session.hasRunningSubprocess = false; + session.shellForeground = true; session.childCommandLabel = null; session.status = "exited"; session.pendingHistoryControlSequence = ""; @@ -1868,6 +2505,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.exitCode = null; session.exitSignal = null; session.hasRunningSubprocess = false; + session.shellForeground = true; session.childCommandLabel = null; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; @@ -1945,6 +2583,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.pid = null; session.process = null; session.hasRunningSubprocess = false; + session.shellForeground = true; session.childCommandLabel = null; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; @@ -2061,9 +2700,17 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func if ( Option.isNone(liveSession) || liveSession.value.status !== "running" || - liveSession.value.pid !== terminalPid || - (liveSession.value.hasRunningSubprocess === next.hasRunningSubprocess && - liveSession.value.childCommandLabel === nextChildLabel) + liveSession.value.pid !== terminalPid + ) { + return [Option.none(), state] as const; + } + // Refresh the foreground-ownership signal even when nothing wire-label- + // worthy changed (a `fg`/`bg` flip of the same child alters what the + // input reply-strip must do without changing the activity event). + liveSession.value.shellForeground = next.shellForeground ?? !next.hasRunningSubprocess; + if ( + liveSession.value.hasRunningSubprocess === next.hasRunningSubprocess && + liveSession.value.childCommandLabel === nextChildLabel ) { return [Option.none(), state] as const; } @@ -2176,6 +2823,8 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func unsubscribeData: null, unsubscribeExit: null, hasRunningSubprocess: false, + shellForeground: true, + lastCursorQueryRelayedAt: 0, childCommandLabel: null, runtimeEnv: normalizedRuntimeEnv(input.env), }; @@ -2499,8 +3148,28 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func terminalId, }); } + // The reply-strip exists to break the IDLE-PROMPT echo loop (a shell with + // no reader echoes the emulator's auto-replies, and a prompt that re-queries + // on redraw turns that into a flood). The gate is PTY foreground ownership, + // not mere child existence: a background job (`sleep 100 &`) leaves the + // shell at the prompt (tpgid still the shell's — keep stripping), while a + // foreground vim/fzf/CPR-based UI owns the terminal and is reading the + // replies to its own queries — relay input verbatim; stripping would starve + // its capability negotiation. The ~1s subprocess-poll latency means a + // program's very first queries can still lose a reply, and a program + // `exec`'d over the shell keeps its pgid so it still looks like the shell — + // both accepted next to the runaway flood the strip prevents. + const data = session.shellForeground + ? stripTerminalResponsesFromInput(input.data, { + // CPR shares its byte shape with modified function keys; only strip + // it while a relayed cursor query makes a reply genuinely expected. + includeQueryGated: + Date.now() - session.lastCursorQueryRelayedAt < CURSOR_QUERY_REPLY_GRACE_MS, + }) + : input.data; + if (data.length === 0) return; yield* Effect.try({ - try: () => process.write(input.data), + try: () => process.write(data), catch: (cause) => new TerminalWriteError({ threadId: input.threadId, @@ -2588,6 +3257,8 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func unsubscribeData: null, unsubscribeExit: null, hasRunningSubprocess: false, + shellForeground: true, + lastCursorQueryRelayedAt: 0, childCommandLabel: null, runtimeEnv: normalizedRuntimeEnv(input.env), }; From 567c3319b1f527c529c0d1da17c11101497b056a Mon Sep 17 00:00:00 2001 From: olafura Date: Wed, 15 Jul 2026 20:22:38 +0200 Subject: [PATCH 02/18] fix(terminal): filter fragmented escape replies --- apps/server/src/terminal/Manager.test.ts | 63 +++++++++++++++++ apps/server/src/terminal/Manager.ts | 87 +++++++++++++++++++++--- 2 files changed, 142 insertions(+), 8 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index d8f02397d97..a40d2c7f23d 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -31,6 +31,7 @@ import * as TerminalManager from "./Manager.ts"; import { sanitizePersistedTerminalHistory, sanitizeTerminalHistoryChunk, + sanitizeTerminalInputChunk, stripTerminalResponsesFromInput, TERMINAL_SEQUENCE_GRAMMAR, } from "./Manager.ts"; @@ -985,6 +986,21 @@ it.layer( }); expect(process.writes).toEqual([]); + // Transport chunking must not bypass the same filter. Previously the + // first half reached the shell before the stateless matcher could see + // the final `R`, leaving `;1R`/`R` residue in the persisted log. + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[16", + }); + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: ";1R", + }); + expect(process.writes).toEqual([]); + // Foreground program running (vim): it issued the query and is blocked // reading the answer — input must pass through verbatim. inspect = { hasRunningSubprocess: true, childCommand: "vim", processIds: [100] }; @@ -2278,6 +2294,53 @@ describe("stripTerminalResponsesFromInput", () => { }); }); +describe("sanitizeTerminalInputChunk", () => { + it("reassembles and strips replies split across client writes", () => { + const cprPrefix = sanitizeTerminalInputChunk("", "\x1b[16"); + assert.equal(cprPrefix.data, ""); + assert.equal(cprPrefix.pendingControlSequence, "\x1b[16"); + assert.deepEqual(sanitizeTerminalInputChunk(cprPrefix.pendingControlSequence, ";1R"), { + data: "", + pendingControlSequence: "", + }); + + const oscPrefix = sanitizeTerminalInputChunk("", "\x1b]11;rgb:1616"); + assert.equal(oscPrefix.data, ""); + assert.deepEqual( + sanitizeTerminalInputChunk(oscPrefix.pendingControlSequence, "/1616/1616\x07"), + { data: "", pendingControlSequence: "" }, + ); + }); + + it("keeps query-gated CPR and real keys when no query is outstanding", () => { + const prefix = sanitizeTerminalInputChunk("", "\x1b[1", { + includeQueryGated: false, + }); + assert.deepEqual( + sanitizeTerminalInputChunk(prefix.pendingControlSequence, ";2R", { + includeQueryGated: false, + }), + { data: "\x1b[1;2R", pendingControlSequence: "" }, + ); + assert.deepEqual(sanitizeTerminalInputChunk("", "\x1b"), { + data: "\x1b", + pendingControlSequence: "", + }); + assert.deepEqual(sanitizeTerminalInputChunk("", "\x1b[A"), { + data: "\x1b[A", + pendingControlSequence: "", + }); + }); + + it("bounds an unterminated string instead of growing session state forever", () => { + const malformed = `\x1b]11;rgb:${"a".repeat(64 * 1024)}`; + assert.deepEqual(sanitizeTerminalInputChunk("", malformed), { + data: malformed, + pendingControlSequence: "", + }); + }); +}); + // ─── Cross-layer grammar invariants ────────────────────────────────────────── // Every sample in TERMINAL_SEQUENCE_GRAMMAR is exercised against every layer in // every framing. These encode the consistency laws the individual layers must diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index 90bce62b413..9252fc27603 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -250,6 +250,8 @@ export interface TerminalSessionState { pid: number | null; history: string; pendingHistoryControlSequence: string; + /** Incomplete client-input control sequence held across terminal.write calls. */ + pendingInputControlSequence: string; pendingProcessEvents: Array; pendingProcessEventIndex: number; processEventDrainRunning: boolean; @@ -1421,36 +1423,46 @@ const MAX_PENDING_CONTROL_SEQUENCE_LENGTH = 64 * 1024; * only on byte structure (where an incomplete escape sequence ends), never on * which complete sequences are stripped, so it is identical for both views. */ -function sanitizeTerminalChunkOnce(input: string): { +function sanitizeTerminalChunkOnce( + input: string, + inputOptions: { readonly includeQueryGated: boolean } = { includeQueryGated: true }, +): { historyText: string; liveText: string; + inputText: string; pendingControlSequence: string; } { let historyText = ""; let liveText = ""; + let inputText = ""; let index = 0; // Ordinary text and sequences neither view strips go to both buffers. const appendBoth = (value: string) => { historyText += value; liveText += value; + inputText += value; }; // A CSI sequence: each view keeps it unless its own strip rule removes it. const appendCsi = (sequence: string, body: string, finalByte: string) => { if (!shouldStripCsiSequence(body, finalByte, false)) historyText += sequence; if (!shouldStripCsiSequence(body, finalByte, true)) liveText += sequence; + inputText += stripTerminalResponsesFromInput(sequence, inputOptions); }; const appendOsc = (sequence: string, content: string) => { if (!shouldStripOscSequence(content, false)) historyText += sequence; if (!shouldStripOscSequence(content, true)) liveText += sequence; + inputText += stripTerminalResponsesFromInput(sequence, inputOptions); }; const appendDcs = (sequence: string, content: string) => { if (!shouldStripDcsSequence(content, false)) historyText += sequence; if (!shouldStripDcsSequence(content, true)) liveText += sequence; + inputText += stripTerminalResponsesFromInput(sequence, inputOptions); }; const pending = () => ({ historyText: stripFlattenedModeReplyResidue(historyText), liveText: stripFlattenedModeReplyResidue(liveText), + inputText, pendingControlSequence: input.slice(index), }); @@ -1556,6 +1568,7 @@ function sanitizeTerminalChunkOnce(input: string): { return { historyText: stripFlattenedModeReplyResidue(historyText), liveText: stripFlattenedModeReplyResidue(liveText), + inputText, pendingControlSequence: "", }; } @@ -1629,6 +1642,46 @@ export function sanitizeTerminalHistoryChunk( }; } +/** + * Stateful counterpart to {@link stripTerminalResponsesFromInput}. Client + * transports are allowed to split a terminal reply across multiple writes; the + * old stateless filter passed the first half to the PTY and could no longer + * recognize the suffix. This uses the same structural walk and declarative + * grammar as output sanitization, holding only an incomplete control sequence. + */ +export function sanitizeTerminalInputChunk( + pendingControlSequence: string, + data: string, + options: { readonly includeQueryGated?: boolean } = {}, +): { data: string; pendingControlSequence: string } { + const input = `${pendingControlSequence}${data}`; + const parsed = sanitizeTerminalChunkOnce(input, { + includeQueryGated: options.includeQueryGated ?? true, + }); + + // Escape is both a complete user keystroke and a possible introducer. Never + // hold a lone Esc waiting for another write; complete CSI/OSC/DCS prefixes are + // still buffered and reassembled across transport chunks. + if (parsed.pendingControlSequence === "\x1b") { + return { data: `${parsed.inputText}\x1b`, pendingControlSequence: "" }; + } + + // Match the live-output parser's memory bound. A malformed client must not + // grow session state forever by opening an OSC/DCS string without a + // terminator; beyond the cap, degrade to verbatim input and recover. + if (parsed.pendingControlSequence.length > MAX_PENDING_CONTROL_SEQUENCE_LENGTH) { + return { + data: `${parsed.inputText}${parsed.pendingControlSequence}`, + pendingControlSequence: "", + }; + } + + return { + data: parsed.inputText, + pendingControlSequence: parsed.pendingControlSequence, + }; +} + /** * Sanitize a WHOLE persisted scrollback buffer for load/migration, losslessly. * @@ -2338,6 +2391,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.childCommandLabel = null; session.status = "exited"; session.pendingHistoryControlSequence = ""; + session.pendingInputControlSequence = ""; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; session.processEventDrainRunning = false; @@ -2411,6 +2465,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.childCommandLabel = null; session.status = "exited"; session.pendingHistoryControlSequence = ""; + session.pendingInputControlSequence = ""; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; session.processEventDrainRunning = false; @@ -2810,6 +2865,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func pid: null, history, pendingHistoryControlSequence: "", + pendingInputControlSequence: "", pendingProcessEvents: [], pendingProcessEventIndex: 0, processEventDrainRunning: false, @@ -2873,6 +2929,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func liveSession.runtimeEnv = nextRuntimeEnv; liveSession.history = ""; liveSession.pendingHistoryControlSequence = ""; + liveSession.pendingInputControlSequence = ""; liveSession.pendingProcessEvents = []; liveSession.pendingProcessEventIndex = 0; liveSession.processEventDrainRunning = false; @@ -2882,6 +2939,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func liveSession.worktreePath = nextWorktreePath; liveSession.history = ""; liveSession.pendingHistoryControlSequence = ""; + liveSession.pendingInputControlSequence = ""; liveSession.pendingProcessEvents = []; liveSession.pendingProcessEventIndex = 0; liveSession.processEventDrainRunning = false; @@ -3160,13 +3218,23 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // `exec`'d over the shell keeps its pgid so it still looks like the shell — // both accepted next to the runaway flood the strip prevents. const data = session.shellForeground - ? stripTerminalResponsesFromInput(input.data, { - // CPR shares its byte shape with modified function keys; only strip - // it while a relayed cursor query makes a reply genuinely expected. - includeQueryGated: - Date.now() - session.lastCursorQueryRelayedAt < CURSOR_QUERY_REPLY_GRACE_MS, - }) - : input.data; + ? (() => { + const sanitized = sanitizeTerminalInputChunk( + session.pendingInputControlSequence, + input.data, + { + // CPR shares its byte shape with modified function keys; only + // strip it while a relayed cursor query makes a reply expected. + includeQueryGated: + DateTime.nowUnsafe().epochMilliseconds - session.lastCursorQueryRelayedAt < + CURSOR_QUERY_REPLY_GRACE_MS, + }, + ); + session.pendingInputControlSequence = sanitized.pendingControlSequence; + return sanitized.data; + })() + : `${session.pendingInputControlSequence}${input.data}`; + if (!session.shellForeground) session.pendingInputControlSequence = ""; if (data.length === 0) return; yield* Effect.try({ try: () => process.write(data), @@ -3207,6 +3275,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func const session = yield* requireSession(input.threadId, terminalId); session.history = ""; session.pendingHistoryControlSequence = ""; + session.pendingInputControlSequence = ""; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; session.processEventDrainRunning = false; @@ -3244,6 +3313,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func pid: null, history: "", pendingHistoryControlSequence: "", + pendingInputControlSequence: "", pendingProcessEvents: [], pendingProcessEventIndex: 0, processEventDrainRunning: false, @@ -3282,6 +3352,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.history = ""; session.pendingHistoryControlSequence = ""; + session.pendingInputControlSequence = ""; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; session.processEventDrainRunning = false; From cb76c4fd63beaa81c8020a5b24f06ae299d84b53 Mon Sep 17 00:00:00 2001 From: olafura Date: Thu, 16 Jul 2026 10:18:51 +0200 Subject: [PATCH 03/18] fix(terminal): stop focus report escape floods --- apps/server/src/terminal/Manager.test.ts | 37 ++++++++++-- apps/server/src/terminal/Manager.ts | 77 ++++++++++++++++++++---- 2 files changed, 99 insertions(+), 15 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index a40d2c7f23d..ab974dae082 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -1015,7 +1015,12 @@ it.layer( terminalId: DEFAULT_TERMINAL_ID, data: "\x1b[1;1R", }); - expect(process.writes).toEqual(["\x1b[1;1R"]); + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[I", + }); + expect(process.writes).toEqual(["\x1b[1;1R", "\x1b[I"]); }), ); @@ -1956,6 +1961,8 @@ describe("sanitizeTerminalHistoryChunk", () => { // Lone DECRPM / OSC-colour / DECRPSS tokens are distinctive enough on their own. assert.equal(sanitize("x 2026;2$y y").visibleText, "x y"); assert.equal(sanitize("c 4;0;rgb:1818/1e1e/2626 d").visibleText, "c d"); + assert.equal(sanitize("c ;0;rgb:1818/1e1e/2626 d").visibleText, "c d"); + assert.equal(sanitize("c ;rgb:1616/1616/1616 d").visibleText, "c d"); assert.equal(sanitize("tail 1$r0m end").visibleText, "tail end"); // flattened DECRPSS (#1238) // Ambiguous lone tokens and ordinary words are preserved. assert.equal(sanitize("see commit 1;2c now").visibleText, "see commit 1;2c now"); @@ -1970,6 +1977,11 @@ describe("sanitizeTerminalHistoryChunk", () => { assert.equal(sanitize("at 12;5R done").visibleText, "at 12;5R done"); // lone, kept }); + it("drops shell caret notation for the captured reply flood", () => { + const captured = "^[[I^[[I^[[?^[[?^[[?1;2c^[]^[\\^[[0n^[]^[\\^[[0n^[[16;1R^[[1;1R"; + assert.equal(sanitize(`before${captured}after`).visibleText, "beforeafter"); + }); + it("drops a flattened secondary-DA (three-parameter) run", () => { // `CSI > Pp;Pv;Pc c` flattens to ">0;276;0c" (the ">" is sometimes kept by // the echo); stripped in a run like the two-parameter primary form. @@ -2249,9 +2261,14 @@ describe("stripTerminalResponsesFromInput", () => { assert.equal(stripTerminalResponsesFromInput("\x901$r0m\x9c"), ""); // C1 DCS DECRPSS }); - it("keeps focus events so DECSET ?1004 programs (vim/tmux) still receive them", () => { - assert.equal(stripTerminalResponsesFromInput("\x1b[I"), "\x1b[I"); // focus in - assert.equal(stripTerminalResponsesFromInput("\x1b[O"), "\x1b[O"); // focus out + it("strips focus events before they redraw an idle prompt", () => { + assert.equal(stripTerminalResponsesFromInput("\x1b[I"), ""); // focus in + assert.equal(stripTerminalResponsesFromInput("\x1b[O"), ""); // focus out + }); + + it("strips empty OSC/DCS frames left by a fragmented response", () => { + assert.equal(stripTerminalResponsesFromInput("\x1b]\x1b\\"), ""); + assert.equal(stripTerminalResponsesFromInput("\x1bP\x1b\\"), ""); }); it("strips cursor-position report (CPR) replies that drive the prompt redraw flood", () => { @@ -2339,6 +2356,18 @@ describe("sanitizeTerminalInputChunk", () => { pendingControlSequence: "", }); }); + + it("drops the captured focus and abandoned private-CSI flood", () => { + const captured = + "\x1b[I\x1b[I" + + "\x1b[?\x1b[?\x1b[?\x1b[?\x1b[?\x1b[?1;2c" + + "\x1b]\x1b\\\x1b[0n\x1b]\x1b\\\x1b[0n\x1b[I\x1b[?1;2c"; + + assert.deepEqual(sanitizeTerminalInputChunk("", captured), { + data: "", + pendingControlSequence: "", + }); + }); }); // ─── Cross-layer grammar invariants ────────────────────────────────────────── diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index 9252fc27603..dc0d9a024c7 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -1093,6 +1093,41 @@ export const TERMINAL_SEQUENCE_GRAMMAR: ReadonlyArray;rgb:…" text and was a ReDoS). The // lookbehind skips a colour run still inside an intact OSC frame the // escape walk chose to keep. - source: "(?][0-9;]+c|\\??[03]n|\\?[0-9;]*\\$y|[IO]|\\?)", + "\\^\\[\\]\\^\\[\\\\", + ].join("|"), + "g", +); /** * Drop the flattened terminal-reply residue a shell echoes at the prompt. * @@ -1294,12 +1339,13 @@ const FLATTENED_REPLY_TOKEN = new RegExp( * ";c" / "n" forms and ordinary words (e.g. "running", "1;2c") are kept. */ function stripFlattenedModeReplyResidue(text: string): string { + const withoutCaretNotation = text.replace(CARET_NOTATION_TERMINAL_REPLY, ""); // Every fragment contains either ";" (DECRPM/DA/OSC) or "$r" (DECRPSS), so text // with neither can't hold residue — skip the regexes. - if (!text.includes(";") && !text.includes("$r")) { - return text; + if (!withoutCaretNotation.includes(";") && !withoutCaretNotation.includes("$r")) { + return withoutCaretNotation; } - return text.replace(FLATTENED_REPLY_RUN, "").replace(FLATTENED_REPLY_TOKEN, ""); + return withoutCaretNotation.replace(FLATTENED_REPLY_RUN, "").replace(FLATTENED_REPLY_TOKEN, ""); } // Matches the terminal→host response sequences the browser emulator @@ -1318,18 +1364,23 @@ function stripFlattenedModeReplyResidue(text: string): string { // kept — the DA alternation requires a parameter so `CSI ? c` / `CSI > c` and // `CSI 6 n` queries pass through. // -// Focus in/out (CSI I / CSI O) are NOT stripped: a program that enabled focus -// reporting (DECSET ?1004 — vim, tmux) legitimately expects them, and they are -// user-action-driven so they never feed the runaway redraw-requery loop the -// capability responses do. +// Focus in/out (CSI I / CSI O) are stripped at an idle shell too: the captured +// flood showed repeated focus-in reports redrawing the prompt and re-triggering +// its capability queries. The manager bypasses this entire filter for a +// foreground program, so vim/tmux still receive legitimate focus events. const INPUT_CSI = "(?:\\x1b\\[|\\x9b)"; const INPUT_OSC = "(?:\\x1b\\]|\\x9d)"; const INPUT_DCS = "(?:\\x1bP|\\x90)"; const INPUT_ST = "(?:\\x07|\\x1b\\\\|\\x9c)"; +const INPUT_CONTROL_INTRODUCER = "(?:\\x1b[\\[\\]P]|[\\x90\\x9b\\x9d])"; +const ABANDONED_PRIVATE_CSI_INPUT_PREFIX = new RegExp( + `${INPUT_CSI}\\?[0-9;]*(?=${INPUT_CONTROL_INTRODUCER})`, + "g", +); function composeInputMatcher(gated: boolean): RegExp | null { const sources = TERMINAL_SEQUENCE_GRAMMAR.flatMap((descriptor) => { const response = descriptor.response; - if (!response?.input) return []; + if (!response || response.input === null) return []; if ((response.inputRequiresRecentQuery ?? false) !== gated) return []; switch (descriptor.kind) { case "csi": @@ -1654,7 +1705,11 @@ export function sanitizeTerminalInputChunk( data: string, options: { readonly includeQueryGated?: boolean } = {}, ): { data: string; pendingControlSequence: string } { - const input = `${pendingControlSequence}${data}`; + // A client parser can time out after `CSI ?`, then begin the next response + // with a fresh introducer. Treat the abandoned private-CSI prefix as residue; + // otherwise the second `[` is mistaken for the first sequence's final byte + // and both prefixes leak to the shell as visible `^[[?` text. + const input = `${pendingControlSequence}${data}`.replace(ABANDONED_PRIVATE_CSI_INPUT_PREFIX, ""); const parsed = sanitizeTerminalChunkOnce(input, { includeQueryGated: options.includeQueryGated ?? true, }); From b9e76e72c7328c11ad85d0d4da6ae4ca1c76bd97 Mon Sep 17 00:00:00 2001 From: olafura Date: Thu, 16 Jul 2026 14:12:08 +0200 Subject: [PATCH 04/18] fix(terminal): filter nested renderer replies --- apps/server/src/terminal/Manager.test.ts | 68 ++++----- apps/server/src/terminal/Manager.ts | 176 +++++++++++------------ 2 files changed, 117 insertions(+), 127 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index ab974dae082..94f841b3b40 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -957,28 +957,18 @@ it.layer( expect(process).toBeDefined(); if (!process) return; - // No relayed cursor query yet: `CSI 1;2R` is xterm's modified-F3 - // keystroke, not a CPR reply — it must reach the PTY. + // Nested renderers issue cursor queries outside this PTY stream, so an + // idle shell must drop a CPR-shaped sequence even when the server did + // not observe the matching query. Otherwise it becomes the `;1RR` loop. yield* manager.write({ threadId: "thread-1", terminalId: DEFAULT_TERMINAL_ID, data: "\x1b[1;2R", }); - expect(process.writes).toEqual(["\x1b[1;2R"]); - process.writes.length = 0; - - // The prompt queries the cursor position — relaying it arms the CPR strip. - process.emitData("\x1b[6n"); - yield* waitFor( - Effect.map(getEvents, (events) => - events.some((event) => event.type === "output" && event.data.includes("\x1b[6n")), - ), - "1200 millis", - ); + expect(process.writes).toEqual([]); - // Idle prompt (no subprocess) with an outstanding query: the emulator's - // CPR auto-reply is dropped — the shell would only echo it back (the - // `;1RR` flood). + // Idle prompt (no subprocess): the emulator's CPR auto-reply is dropped + // because the shell would only echo it back. yield* manager.write({ threadId: "thread-1", terminalId: DEFAULT_TERMINAL_ID, @@ -1977,6 +1967,19 @@ describe("sanitizeTerminalHistoryChunk", () => { assert.equal(sanitize("at 12;5R done").visibleText, "at 12;5R done"); // lone, kept }); + it("drops the BEL-fragmented CPR flood captured from the nested renderer", () => { + const captured = ";1R\x07R\x07R\x07;1R\x07R\x07R\x07;1R\x07R\x07R\x07"; + assert.equal(sanitize(`prompt$ ${captured}`).visibleText, "prompt$ "); + }); + + it("drops the collapsed palette-reply run captured from the terminal log", () => { + const captured = + "6e6e/7878/8888;RR;;;rgb:1616/1616/1616;rgb:f5f5/f5f5/f5f5;" + + "2;rgb:8686/e7e7/9595;5;rgb:d0d0/b0b0/ffff;RR;;;rgb:1616/1616/1616;" + + "rgb:f5f5/f5f5/f5f5;2;rgb:8686/e7e7/9595;5;rgb:d0d0/b0b0/ffff"; + assert.equal(sanitize(`prompt$ ${captured}`).visibleText, "prompt$ "); + }); + it("drops shell caret notation for the captured reply flood", () => { const captured = "^[[I^[[I^[[?^[[?^[[?1;2c^[]^[\\^[[0n^[]^[\\^[[0n^[[16;1R^[[1;1R"; assert.equal(sanitize(`before${captured}after`).visibleText, "beforeafter"); @@ -2282,19 +2285,12 @@ describe("stripTerminalResponsesFromInput", () => { assert.equal(stripTerminalResponsesFromInput("\x9b?5;10R"), ""); }); - it("keeps CPR-shaped bytes when no cursor query is outstanding (modified F3)", () => { - // `CSI 1;2R` is byte-identical to xterm's Shift+F3 — without a recently - // relayed `CSI 6 n` the write path passes it through; the ungated reply - // shapes still strip. - assert.equal( - stripTerminalResponsesFromInput("\x1b[1;2R", { includeQueryGated: false }), - "\x1b[1;2R", - ); - assert.equal( - stripTerminalResponsesFromInput("\x1b[?2026;2$y\x1b[1;2R", { includeQueryGated: false }), - "\x1b[1;2R", - ); - assert.equal(stripTerminalResponsesFromInput("\x1b[1;2R", { includeQueryGated: true }), ""); + it("strips CPR-shaped bytes without requiring an observable query", () => { + // `CSI 1;2R` also encodes Shift+F3, but the idle-shell filter cannot know + // about queries issued by an outer renderer. Foreground programs bypass the + // filter and still receive the same bytes verbatim. + assert.equal(stripTerminalResponsesFromInput("\x1b[1;2R"), ""); + assert.equal(stripTerminalResponsesFromInput("\x1b[?2026;2$y\x1b[1;2R"), ""); }); it("keeps real user input, cursor moves, and bare query forms", () => { @@ -2329,16 +2325,12 @@ describe("sanitizeTerminalInputChunk", () => { ); }); - it("keeps query-gated CPR and real keys when no query is outstanding", () => { - const prefix = sanitizeTerminalInputChunk("", "\x1b[1", { - includeQueryGated: false, + it("strips split CPR while keeping real keys", () => { + const prefix = sanitizeTerminalInputChunk("", "\x1b[1"); + assert.deepEqual(sanitizeTerminalInputChunk(prefix.pendingControlSequence, ";2R"), { + data: "", + pendingControlSequence: "", }); - assert.deepEqual( - sanitizeTerminalInputChunk(prefix.pendingControlSequence, ";2R", { - includeQueryGated: false, - }), - { data: "\x1b[1;2R", pendingControlSequence: "" }, - ); assert.deepEqual(sanitizeTerminalInputChunk("", "\x1b"), { data: "\x1b", pendingControlSequence: "", diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index dc0d9a024c7..8c1ba97cca2 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -1,7 +1,3 @@ -// @effect-diagnostics globalDateInEffect:off -- the CPR-gate grace window -// (lastCursorQueryRelayedAt) is wall-clock session state shared between the -// sync PTY drain and the write path; threading Clock through both is a -// follow-up. /** * TerminalManager - Terminal session orchestration service interface. * @@ -274,12 +270,6 @@ export interface TerminalSessionState { * platform can't tell. */ shellForeground: boolean; - /** - * When a cursor-position query (`CSI 6 n`) was last relayed in the live - * stream (epoch ms; 0 = never). Arms the query-gated CPR strip in `write` - * for {@link CURSOR_QUERY_REPLY_GRACE_MS}. - */ - lastCursorQueryRelayedAt: number; /** Normalized child command name when `hasRunningSubprocess`; cleared when idle. */ childCommandLabel: string | null; runtimeEnv: Record | null; @@ -806,11 +796,11 @@ const posixInspectSubprocess = Effect.fn("terminal.posixInspectSubprocess")(func childPid = parseFirstChildPidFromPgrep(pgrepResult.value.stdout); } else if (pgrepResult.value.code === 1) { return { - hasRunningSubprocess: false, - childCommand: null, - processIds: [], - ...(shellForeground !== undefined ? { shellForeground } : {}), - }; + hasRunningSubprocess: false, + childCommand: null, + processIds: [], + ...(shellForeground !== undefined ? { shellForeground } : {}), + }; } } @@ -818,11 +808,11 @@ const posixInspectSubprocess = Effect.fn("terminal.posixInspectSubprocess")(func const psResult = yield* Effect.exit(runPs); if (psResult._tag === "Failure" || psResult.value.code !== 0) { return { - hasRunningSubprocess: false, - childCommand: null, - processIds: [], - ...(shellForeground !== undefined ? { shellForeground } : {}), - }; + hasRunningSubprocess: false, + childCommand: null, + processIds: [], + ...(shellForeground !== undefined ? { shellForeground } : {}), + }; } for (const line of psResult.value.stdout.split(/\r?\n/g)) { const [pidRaw, ppidRaw] = line.trim().split(/\s+/g); @@ -1001,14 +991,6 @@ export interface TerminalSequenceDescriptor { readonly stripFromOutput: boolean; /** Input-filter body (null = relayed in input). */ readonly input: string | null; - /** - * Only strip this reply from input while a matching query was recently - * relayed in the live stream. Needed when the reply's byte shape collides - * with real keyboard input (CPR `CSI 1;2R` == xterm's modified F3): the - * keystroke passes at a quiet prompt, while the query-driven echo flood is - * still broken because the querying prompt re-arms the gate every redraw. - */ - readonly inputRequiresRecentQuery?: boolean; readonly samples: ReadonlyArray; }; readonly query?: { @@ -1079,11 +1061,11 @@ export const TERMINAL_SEQUENCE_GRAMMAR: ReadonlyArrayR` is also - // xterm's encoding for modified F3. + // anything keystroke-like is never eaten. `CSI 1;R` also encodes a + // modified F3 key, but this filter runs only while the shell owns the PTY; + // nested renderers issue CPR queries the server cannot observe, so leaving + // that collision unfiltered lets their replies feed a runaway prompt loop. input: "\\??[0-9]*;[0-9]+R", - inputRequiresRecentQuery: true, samples: ["1;1R", "?5;10R", ";1R"], }, flattened: { @@ -1309,7 +1291,7 @@ const FLATTENED_SOURCES = TERMINAL_SEQUENCE_GRAMMAR.flatMap((descriptor) => ); const FLATTENED_FRAGMENT = `(?:${FLATTENED_SOURCES.map((f) => `(?:${f.source})`).join("|")})`; const FLATTENED_REPLY_RUN = new RegExp( - `${FLATTENED_FRAGMENT}(?:[\\x07\\rn]{0,8}${FLATTENED_FRAGMENT})+`, + `${FLATTENED_FRAGMENT}(?:[\\x07\\rnR]{0,8}${FLATTENED_FRAGMENT})+[\\x07R]{0,8}`, "g", ); const FLATTENED_REPLY_TOKEN = new RegExp( @@ -1328,6 +1310,66 @@ const CARET_NOTATION_TERMINAL_REPLY = new RegExp( ].join("|"), "g", ); +function isPaletteReplyResidueCharacter(codePoint: number): boolean { + return ( + (codePoint >= 0x30 && codePoint <= 0x39) || + (codePoint >= 0x41 && codePoint <= 0x46) || + (codePoint >= 0x61 && codePoint <= 0x66) || + codePoint === 0x2f || + codePoint === 0x3a || + codePoint === 0x3b || + codePoint === 0x24 || + codePoint === 0x52 || + codePoint === 0x62 || + codePoint === 0x67 || + codePoint === 0x72 || + codePoint === 0x79 + ); +} + +/** + * Drop a multiplexer-flattened palette run in linear time. + * + * OpenTUI's palette probe can be fragmented so aggressively that several OSC + * replies collapse into one introducer-less token. A maximal token made only + * from reply characters is corruption when it contains at least three `rgb:` + * bodies. The threshold avoids deleting ordinary output with one CSS colour, + * and the explicit scan avoids a nested-regex ReDoS on long `;` input. + */ +function stripCorruptedPaletteReplyRuns(text: string): string { + if (!text.includes("rgb:")) return text; + + let output = ""; + let copiedThrough = 0; + let cursor = 0; + while (cursor < text.length) { + if (!isPaletteReplyResidueCharacter(text.charCodeAt(cursor))) { + cursor += 1; + continue; + } + + const tokenStart = cursor; + let rgbCount = 0; + let removalStart = -1; + while (cursor < text.length && isPaletteReplyResidueCharacter(text.charCodeAt(cursor))) { + if (removalStart === -1) { + const codePoint = text.charCodeAt(cursor); + if ((codePoint >= 0x30 && codePoint <= 0x39) || codePoint === 0x3b) { + removalStart = cursor; + } + } + if (text.startsWith("rgb:", cursor)) rgbCount += 1; + cursor += 1; + } + + if (rgbCount >= 3) { + const start = removalStart === -1 ? tokenStart : removalStart; + output += text.slice(copiedThrough, start); + copiedThrough = cursor; + } + } + return copiedThrough === 0 ? text : output + text.slice(copiedThrough); +} /** * Drop the flattened terminal-reply residue a shell echoes at the prompt. * @@ -1339,7 +1381,9 @@ const CARET_NOTATION_TERMINAL_REPLY = new RegExp( * ";c" / "n" forms and ordinary words (e.g. "running", "1;2c") are kept. */ function stripFlattenedModeReplyResidue(text: string): string { - const withoutCaretNotation = text.replace(CARET_NOTATION_TERMINAL_REPLY, ""); + const withoutCaretNotation = stripCorruptedPaletteReplyRuns( + text.replace(CARET_NOTATION_TERMINAL_REPLY, ""), + ); // Every fragment contains either ";" (DECRPM/DA/OSC) or "$r" (DECRPSS), so text // with neither can't hold residue — skip the regexes. if (!withoutCaretNotation.includes(";") && !withoutCaretNotation.includes("$r")) { @@ -1377,11 +1421,10 @@ const ABANDONED_PRIVATE_CSI_INPUT_PREFIX = new RegExp( `${INPUT_CSI}\\?[0-9;]*(?=${INPUT_CONTROL_INTRODUCER})`, "g", ); -function composeInputMatcher(gated: boolean): RegExp | null { +function composeInputMatcher(): RegExp | null { const sources = TERMINAL_SEQUENCE_GRAMMAR.flatMap((descriptor) => { const response = descriptor.response; if (!response || response.input === null) return []; - if ((response.inputRequiresRecentQuery ?? false) !== gated) return []; switch (descriptor.kind) { case "csi": return [`${INPUT_CSI}(?:${response.input})`]; @@ -1394,17 +1437,7 @@ function composeInputMatcher(gated: boolean): RegExp | null { return sources.length === 0 ? null : new RegExp(sources.join("|"), "g"); } -const INPUT_TERMINAL_RESPONSE = composeInputMatcher(false); -const INPUT_QUERY_GATED_RESPONSE = composeInputMatcher(true); -// A relayed cursor-position query in the LIVE stream (`CSI 6 n` / `CSI ? 6 n`, -// 7-bit or 8-bit) — arms the query-gated input strip for a grace window. -const RELAYED_CURSOR_QUERY = new RegExp(`${INPUT_CSI}\\??6n`); -/** - * How long after relaying a cursor-position query the CPR reply strip stays - * armed. A prompt that re-queries on every redraw (the flood scenario) - * constantly re-arms it; a quiet prompt lets modified-F3 keystrokes through. - */ -export const CURSOR_QUERY_REPLY_GRACE_MS = 5_000; +const INPUT_TERMINAL_RESPONSE = composeInputMatcher(); /** * Strip the browser emulator's auto-generated terminal responses from client * input before it reaches the PTY. @@ -1424,18 +1457,7 @@ export const CURSOR_QUERY_REPLY_GRACE_MS = 5_000; * presumed to be reading the replies to its own queries, and stripping them * would stall its capability negotiation. Exported for unit testing. */ -export function stripTerminalResponsesFromInput( - data: string, - options: { - /** - * Include the query-gated reply shapes (CPR). Defaults true (the full - * strip); the write path passes whether a cursor query was recently - * relayed, so an idle prompt with no outstanding query lets the - * byte-identical modified-F3 keystrokes through. - */ - readonly includeQueryGated?: boolean; - } = {}, -): string { +export function stripTerminalResponsesFromInput(data: string): string { // Skip the regexes unless the data carries a 7-bit ESC or one of the 8-bit C1 // introducers (CSI 0x9b, OSC 0x9d, DCS 0x90) a response could start with. const hasIntroducer = @@ -1444,11 +1466,7 @@ export function stripTerminalResponsesFromInput( data.includes("\x9d") || data.includes("\x90"); if (!hasIntroducer) return data; - let stripped = INPUT_TERMINAL_RESPONSE ? data.replace(INPUT_TERMINAL_RESPONSE, "") : data; - if ((options.includeQueryGated ?? true) && INPUT_QUERY_GATED_RESPONSE) { - stripped = stripped.replace(INPUT_QUERY_GATED_RESPONSE, ""); - } - return stripped; + return INPUT_TERMINAL_RESPONSE ? data.replace(INPUT_TERMINAL_RESPONSE, "") : data; } // Upper bound on the buffered incomplete-sequence remainder. An unterminated @@ -1474,10 +1492,7 @@ const MAX_PENDING_CONTROL_SEQUENCE_LENGTH = 64 * 1024; * only on byte structure (where an incomplete escape sequence ends), never on * which complete sequences are stripped, so it is identical for both views. */ -function sanitizeTerminalChunkOnce( - input: string, - inputOptions: { readonly includeQueryGated: boolean } = { includeQueryGated: true }, -): { +function sanitizeTerminalChunkOnce(input: string): { historyText: string; liveText: string; inputText: string; @@ -1498,17 +1513,17 @@ function sanitizeTerminalChunkOnce( const appendCsi = (sequence: string, body: string, finalByte: string) => { if (!shouldStripCsiSequence(body, finalByte, false)) historyText += sequence; if (!shouldStripCsiSequence(body, finalByte, true)) liveText += sequence; - inputText += stripTerminalResponsesFromInput(sequence, inputOptions); + inputText += stripTerminalResponsesFromInput(sequence); }; const appendOsc = (sequence: string, content: string) => { if (!shouldStripOscSequence(content, false)) historyText += sequence; if (!shouldStripOscSequence(content, true)) liveText += sequence; - inputText += stripTerminalResponsesFromInput(sequence, inputOptions); + inputText += stripTerminalResponsesFromInput(sequence); }; const appendDcs = (sequence: string, content: string) => { if (!shouldStripDcsSequence(content, false)) historyText += sequence; if (!shouldStripDcsSequence(content, true)) liveText += sequence; - inputText += stripTerminalResponsesFromInput(sequence, inputOptions); + inputText += stripTerminalResponsesFromInput(sequence); }; const pending = () => ({ historyText: stripFlattenedModeReplyResidue(historyText), @@ -1703,16 +1718,13 @@ export function sanitizeTerminalHistoryChunk( export function sanitizeTerminalInputChunk( pendingControlSequence: string, data: string, - options: { readonly includeQueryGated?: boolean } = {}, ): { data: string; pendingControlSequence: string } { // A client parser can time out after `CSI ?`, then begin the next response // with a fresh introducer. Treat the abandoned private-CSI prefix as residue; // otherwise the second `[` is mistaken for the first sequence's final byte // and both prefixes leak to the shell as visible `^[[?` text. const input = `${pendingControlSequence}${data}`.replace(ABANDONED_PRIVATE_CSI_INPUT_PREFIX, ""); - const parsed = sanitizeTerminalChunkOnce(input, { - includeQueryGated: options.includeQueryGated ?? true, - }); + const parsed = sanitizeTerminalChunkOnce(input); // Escape is both a complete user keystroke and a possible introducer. Never // hold a lone Esc waiting for another write; complete CSI/OSC/DCS prefixes are @@ -2413,11 +2425,6 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func nextEvent.data, ); session.pendingHistoryControlSequence = sanitized.pendingControlSequence; - // A relayed cursor query means a CPR reply is genuinely expected — - // arm the query-gated input strip for the grace window. - if (RELAYED_CURSOR_QUERY.test(sanitized.liveText)) { - session.lastCursorQueryRelayedAt = Date.now(); - } if (sanitized.historyText.length > 0) { session.history = capHistory( `${session.history}${sanitized.historyText}`, @@ -2935,7 +2942,6 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func unsubscribeExit: null, hasRunningSubprocess: false, shellForeground: true, - lastCursorQueryRelayedAt: 0, childCommandLabel: null, runtimeEnv: normalizedRuntimeEnv(input.env), }; @@ -3277,13 +3283,6 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func const sanitized = sanitizeTerminalInputChunk( session.pendingInputControlSequence, input.data, - { - // CPR shares its byte shape with modified function keys; only - // strip it while a relayed cursor query makes a reply expected. - includeQueryGated: - DateTime.nowUnsafe().epochMilliseconds - session.lastCursorQueryRelayedAt < - CURSOR_QUERY_REPLY_GRACE_MS, - }, ); session.pendingInputControlSequence = sanitized.pendingControlSequence; return sanitized.data; @@ -3383,7 +3382,6 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func unsubscribeExit: null, hasRunningSubprocess: false, shellForeground: true, - lastCursorQueryRelayedAt: 0, childCommandLabel: null, runtimeEnv: normalizedRuntimeEnv(input.env), }; From 85dc7fb763ac64180ad4abf8f9759d93e9a23893 Mon Sep 17 00:00:00 2001 From: olafura Date: Fri, 17 Jul 2026 15:54:55 +0200 Subject: [PATCH 05/18] fix(terminal): close capability reply exit race --- apps/server/src/terminal/Manager.test.ts | 66 ++++++++++++++++++++++++ apps/server/src/terminal/Manager.ts | 36 +++++++++++++ 2 files changed, 102 insertions(+) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index 94f841b3b40..e11698de84a 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -1014,6 +1014,72 @@ it.layer( }), ); + it.effect("refreshes foreground ownership when a program exits between subprocess polls", () => + Effect.gen(function* () { + let inspect: { + readonly hasRunningSubprocess: boolean; + readonly childCommand: string | null; + readonly processIds: ReadonlyArray; + readonly shellForeground?: boolean; + } = { + hasRunningSubprocess: true, + childCommand: "claude", + processIds: [100], + shellForeground: false, + }; + let inspections = 0; + const { manager, ptyAdapter, getEvents } = yield* createManager(5, { + subprocessInspector: () => { + inspections += 1; + return Effect.succeed(inspect); + }, + subprocessPollIntervalMs: 200, + }); + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + expect(process).toBeDefined(); + if (!process) return; + + yield* waitFor( + Effect.map(getEvents, (events) => + events.some((event) => event.type === "activity" && event.hasRunningSubprocess), + ), + "1200 millis", + ); + const inspectionsBeforeExit = inspections; + + // Claude has exited and the shell owns the PTY again, but the periodic + // snapshot still says the foreground program is active. + inspect = { + hasRunningSubprocess: false, + childCommand: null, + processIds: [], + shellForeground: true, + }; + + // The captured leak commonly splits DA replies across client writes. + // Neither half may reach the shell during the stale-cache window. + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[?1", + }); + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: ";2c", + }); + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b]11;rgb:1616/1616/1616\x1b\\", + }); + + expect(inspections).toBeGreaterThan(inspectionsBeforeExit); + expect(process.writes).toEqual([]); + }), + ); + it.effect("keeps stripping replies while only a BACKGROUND job runs (shell owns the PTY)", () => Effect.gen(function* () { // `sleep 100 &` puts a child under the shell, but the shell still owns diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index 8c1ba97cca2..849be58b43e 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -1749,6 +1749,26 @@ export function sanitizeTerminalInputChunk( }; } +/** + * Whether a client write is (or starts) a terminal-generated capability reply. + * + * This is intentionally structural rather than a second signature list: the + * canonical input sanitizer above owns reply recognition. An incomplete + * sequence also counts so a transport split cannot send its prefix to the PTY + * before foreground ownership is refreshed. + */ +function mayContainTerminalResponse(pendingControlSequence: string, data: string): boolean { + if (pendingControlSequence.length > 0) return true; + const hasIntroducer = + data.includes("\x1b") || + data.includes("\x9b") || + data.includes("\x9d") || + data.includes("\x90"); + if (!hasIntroducer) return false; + const sanitized = sanitizeTerminalInputChunk("", data); + return sanitized.data !== data || sanitized.pendingControlSequence.length > 0; +} + /** * Sanitize a WHOLE persisted scrollback buffer for load/migration, losslessly. * @@ -3267,6 +3287,22 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func terminalId, }); } + // The periodic subprocess snapshot is deliberately cheap and eventually + // consistent, but a foreground program can exit between polls. Capability + // replies arriving in that gap must refresh foreground ownership before we + // decide whether to relay them; otherwise the now-idle shell receives a + // whole response burst and prompt redraws amplify it into a feedback loop. + if ( + !session.shellForeground && + mayContainTerminalResponse(session.pendingInputControlSequence, input.data) + ) { + const refreshed = yield* Effect.exit(subprocessInspector(process.pid)); + if (refreshed._tag === "Success") { + session.shellForeground = + refreshed.value.shellForeground ?? !refreshed.value.hasRunningSubprocess; + } + } + // The reply-strip exists to break the IDLE-PROMPT echo loop (a shell with // no reader echoes the emulator's auto-replies, and a prompt that re-queries // on redraw turns that into a flood). The gate is PTY foreground ownership, From 4bc007d8b8a4a5d78462aac7ac727c1417a5415e Mon Sep 17 00:00:00 2001 From: olafura Date: Fri, 17 Jul 2026 16:03:22 +0200 Subject: [PATCH 06/18] fix(terminal): harden reply routing transitions --- apps/server/src/terminal/Manager.test.ts | 113 ++++++++++++++++++++++- apps/server/src/terminal/Manager.ts | 77 +++++++++------ 2 files changed, 157 insertions(+), 33 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index e11698de84a..8e289798bd7 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -947,7 +947,13 @@ it.layer( readonly hasRunningSubprocess: boolean; readonly childCommand: string | null; readonly processIds: ReadonlyArray; - } = { hasRunningSubprocess: false, childCommand: null, processIds: [] }; + readonly shellForeground?: boolean; + } = { + hasRunningSubprocess: false, + childCommand: null, + processIds: [], + shellForeground: true, + }; const { manager, ptyAdapter, getEvents } = yield* createManager(5, { subprocessInspector: () => Effect.succeed(inspect), subprocessPollIntervalMs: 20, @@ -993,7 +999,12 @@ it.layer( // Foreground program running (vim): it issued the query and is blocked // reading the answer — input must pass through verbatim. - inspect = { hasRunningSubprocess: true, childCommand: "vim", processIds: [100] }; + inspect = { + hasRunningSubprocess: true, + childCommand: "vim", + processIds: [100], + shellForeground: false, + }; yield* waitFor( Effect.map(getEvents, (events) => events.some((event) => event.type === "activity" && event.hasRunningSubprocess), @@ -1161,6 +1172,96 @@ it.layer( }), ); + it.effect("treats missing foreground ownership as unknown instead of foreground", () => + Effect.gen(function* () { + let inspect: { + readonly hasRunningSubprocess: boolean; + readonly childCommand: string | null; + readonly processIds: ReadonlyArray; + } = { + hasRunningSubprocess: false, + childCommand: null, + processIds: [], + }; + const { manager, ptyAdapter, getEvents } = yield* createManager(5, { + subprocessInspector: () => Effect.succeed(inspect), + subprocessPollIntervalMs: 20, + }); + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + expect(process).toBeDefined(); + if (!process) return; + + inspect = { + hasRunningSubprocess: true, + childCommand: "sleep", + processIds: [100], + }; + yield* waitFor( + Effect.map(getEvents, (events) => + events.some((event) => event.type === "activity" && event.hasRunningSubprocess), + ), + "1200 millis", + ); + + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[?1;2c", + }); + expect(process.writes).toEqual([]); + }), + ); + + it.effect("drops an idle-shell reply prefix before relaying foreground input", () => + Effect.gen(function* () { + let inspect: { + readonly hasRunningSubprocess: boolean; + readonly childCommand: string | null; + readonly processIds: ReadonlyArray; + readonly shellForeground?: boolean; + } = { + hasRunningSubprocess: false, + childCommand: null, + processIds: [], + shellForeground: true, + }; + const { manager, ptyAdapter, getEvents } = yield* createManager(5, { + subprocessInspector: () => Effect.succeed(inspect), + subprocessPollIntervalMs: 20, + }); + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + expect(process).toBeDefined(); + if (!process) return; + + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b", + }); + inspect = { + hasRunningSubprocess: true, + childCommand: "vim", + processIds: [100], + shellForeground: false, + }; + yield* waitFor( + Effect.map(getEvents, (events) => + events.some((event) => event.type === "activity" && event.hasRunningSubprocess), + ), + "1200 millis", + ); + + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "x", + }); + expect(process.writes).toEqual(["x"]); + }), + ); + it.effect("does not invoke subprocess polling until a terminal session is running", () => Effect.gen(function* () { let checks = 0; @@ -2391,14 +2492,18 @@ describe("sanitizeTerminalInputChunk", () => { ); }); - it("strips split CPR while keeping real keys", () => { + it("strips CPR split immediately after ESC while keeping complete real keys", () => { const prefix = sanitizeTerminalInputChunk("", "\x1b[1"); assert.deepEqual(sanitizeTerminalInputChunk(prefix.pendingControlSequence, ";2R"), { data: "", pendingControlSequence: "", }); assert.deepEqual(sanitizeTerminalInputChunk("", "\x1b"), { - data: "\x1b", + data: "", + pendingControlSequence: "\x1b", + }); + assert.deepEqual(sanitizeTerminalInputChunk("\x1b", "[1;2R"), { + data: "", pendingControlSequence: "", }); assert.deepEqual(sanitizeTerminalInputChunk("", "\x1b[A"), { diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index 849be58b43e..75acf8d04e9 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -196,8 +196,9 @@ interface TerminalSubprocessInspectResult { * Whether the shell itself owns the PTY's foreground process group * (`tpgid === pgid` of the terminal process) — i.e. the terminal is at an * interactive prompt, even if background jobs (`… &`) exist. `undefined` - * when the platform/inspector can't tell (Windows, custom fixtures); the - * caller then falls back to `!hasRunningSubprocess`. + * when the platform/inspector can't tell (Windows, custom fixtures); callers + * must treat that as unknown rather than equating any child with foreground + * ownership. */ readonly shellForeground?: boolean; } @@ -265,11 +266,11 @@ export interface TerminalSessionState { * Whether the shell owns the PTY's foreground process group (idle prompt, * possibly with background jobs). Drives the input reply-strip: strip only * while true — a foreground job (vim, a CPR-based UI) is reading the replies - * to its own queries. Defaults true at spawn; refreshed by the subprocess - * poll (`tpgid === pgid`), falling back to `!hasRunningSubprocess` when the - * platform can't tell. + * to its own queries. `null` means the inspector failed or could not observe + * foreground ownership; that state uses the safe idle-shell filtering policy + * until a later probe provides a definitive answer. */ - shellForeground: boolean; + shellForeground: boolean | null; /** Normalized child command name when `hasRunningSubprocess`; cleared when idle. */ childCommandLabel: string | null; runtimeEnv: Record | null; @@ -1121,8 +1122,12 @@ export const TERMINAL_SEQUENCE_GRAMMAR: ReadonlyArray;rgb:…" text and was a ReDoS). The // lookbehind skips a colour run still inside an intact OSC frame the // escape walk chose to keep. - source: "(? { + const liveSession = state.sessions.get( + toSessionKey(session.threadId, session.terminalId), + ); + if (liveSession?.status === "running" && liveSession.pid === terminalPid) { + // Do not keep routing input from a stale foreground observation. + // Unknown ownership strips capability replies until a later probe + // succeeds, while ordinary keyboard input still passes unchanged. + liveSession.shellForeground = null; + } + return [undefined, state] as const; + }); return; } @@ -2844,7 +2854,13 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // Refresh the foreground-ownership signal even when nothing wire-label- // worthy changed (a `fg`/`bg` flip of the same child alters what the // input reply-strip must do without changing the activity event). - liveSession.value.shellForeground = next.shellForeground ?? !next.hasRunningSubprocess; + liveSession.value.shellForeground = + next.shellForeground ?? (next.hasRunningSubprocess ? null : true); + if (liveSession.value.shellForeground === false) { + // A prefix buffered while the shell owned the PTY is unsolicited + // reply residue, not input for the newly foreground application. + liveSession.value.pendingInputControlSequence = ""; + } if ( liveSession.value.hasRunningSubprocess === next.hasRunningSubprocess && liveSession.value.childCommandLabel === nextChildLabel @@ -3299,7 +3315,9 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func const refreshed = yield* Effect.exit(subprocessInspector(process.pid)); if (refreshed._tag === "Success") { session.shellForeground = - refreshed.value.shellForeground ?? !refreshed.value.hasRunningSubprocess; + refreshed.value.shellForeground ?? (refreshed.value.hasRunningSubprocess ? null : true); + } else { + session.shellForeground = null; } } @@ -3314,17 +3332,18 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // program's very first queries can still lose a reply, and a program // `exec`'d over the shell keeps its pgid so it still looks like the shell — // both accepted next to the runaway flood the strip prevents. - const data = session.shellForeground - ? (() => { - const sanitized = sanitizeTerminalInputChunk( - session.pendingInputControlSequence, - input.data, - ); - session.pendingInputControlSequence = sanitized.pendingControlSequence; - return sanitized.data; - })() - : `${session.pendingInputControlSequence}${input.data}`; - if (!session.shellForeground) session.pendingInputControlSequence = ""; + const data = + session.shellForeground !== false + ? (() => { + const sanitized = sanitizeTerminalInputChunk( + session.pendingInputControlSequence, + input.data, + ); + session.pendingInputControlSequence = sanitized.pendingControlSequence; + return sanitized.data; + })() + : input.data; + if (session.shellForeground === false) session.pendingInputControlSequence = ""; if (data.length === 0) return; yield* Effect.try({ try: () => process.write(data), From 1091462715f971c11b101f41d2c763d022718ab2 Mon Sep 17 00:00:00 2001 From: olafura Date: Fri, 17 Jul 2026 17:25:30 +0200 Subject: [PATCH 07/18] fix(terminal): address sanitizer review feedback --- apps/server/src/terminal/Manager.test.ts | 122 +++++++++++++++++++++++ apps/server/src/terminal/Manager.ts | 75 ++++++++++---- 2 files changed, 176 insertions(+), 21 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index 8e289798bd7..a31a71fe140 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -1025,6 +1025,66 @@ it.layer( }), ); + it.effect("serializes overlapping reply writes through the per-thread lock", () => + Effect.gen(function* () { + const inspect = { + hasRunningSubprocess: true, + childCommand: "vim", + processIds: [100], + shellForeground: false, + }; + let delayInspections = false; + let activeInspections = 0; + let maxActiveInspections = 0; + const { manager, ptyAdapter, getEvents } = yield* createManager(5, { + subprocessInspector: () => + delayInspections + ? Effect.gen(function* () { + activeInspections += 1; + maxActiveInspections = Math.max(maxActiveInspections, activeInspections); + yield* Effect.sleep("25 millis"); + activeInspections -= 1; + return inspect; + }) + : Effect.succeed(inspect), + // Leave enough room after the initial ownership poll for the two + // overlapping writes to exercise only their on-demand refreshes. + subprocessPollIntervalMs: 200, + }); + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + expect(process).toBeDefined(); + if (!process) return; + + yield* waitFor( + Effect.map(getEvents, (events) => + events.some((event) => event.type === "activity" && event.hasRunningSubprocess), + ), + "1200 millis", + ); + delayInspections = true; + + yield* Effect.all( + [ + manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[1;1R", + }), + manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[2;1R", + }), + ], + { concurrency: "unbounded" }, + ); + + expect(maxActiveInspections).toBe(1); + expect(process.writes).toEqual(["\x1b[1;1R", "\x1b[2;1R"]); + }), + ); + it.effect("refreshes foreground ownership when a program exits between subprocess polls", () => Effect.gen(function* () { let inspect: { @@ -1213,6 +1273,41 @@ it.layer( }), ); + it.effect( + "relays replies to a running Windows child when foreground ownership is unavailable", + () => + Effect.gen(function* () { + const inspect = { + hasRunningSubprocess: true, + childCommand: "vim.exe", + processIds: [100], + }; + const fixture = yield* createManager(5, { + subprocessInspector: () => Effect.succeed(inspect), + subprocessPollIntervalMs: 20, + }).pipe(Effect.provide(withHostPlatform("win32"))); + const { manager, ptyAdapter, getEvents } = fixture; + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + expect(process).toBeDefined(); + if (!process) return; + + yield* waitFor( + Effect.map(getEvents, (events) => + events.some((event) => event.type === "activity" && event.hasRunningSubprocess), + ), + "1200 millis", + ); + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[1;2R", + }); + + expect(process.writes).toEqual(["\x1b[1;2R"]); + }), + ); + it.effect("drops an idle-shell reply prefix before relaying foreground input", () => Effect.gen(function* () { let inspect: { @@ -1314,6 +1409,24 @@ it.layer( }), ); + it.effect("does not split a surrogate pair at the history character boundary", () => + Effect.gen(function* () { + const { manager, ptyAdapter } = yield* createManager(5_000, { historyCharLimit: 2 }); + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + expect(process).toBeDefined(); + if (!process) return; + + // UTF-16 indices: A=0, emoji high=1, emoji low=2, B=3. A hard cut at + // length-maxChars (=2) would retain a lone low surrogate followed by B. + process.emitData("A😀B"); + yield* manager.close({ threadId: "thread-1" }); + + const reopened = yield* manager.open(openInput()); + expect(reopened.history).toBe("😀B"); + }), + ); + it.effect("caps persisted history to configured line limit", () => Effect.gen(function* () { const { manager, ptyAdapter } = yield* createManager(3); @@ -2531,6 +2644,15 @@ describe("sanitizeTerminalInputChunk", () => { pendingControlSequence: "", }); }); + + it("drops abandoned CPR and DA prefixes before a fresh response", () => { + for (const abandoned of ["\x1b[1;", "\x1b[>0;", "\x9b12;", "\x1b[?2026;"]) { + assert.deepEqual(sanitizeTerminalInputChunk("", `${abandoned}\x1b[1;2R`), { + data: "", + pendingControlSequence: "", + }); + } + }); }); // ─── Cross-layer grammar invariants ────────────────────────────────────────── diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index 75acf8d04e9..0e8f2d99f39 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -914,6 +914,24 @@ function defaultSubprocessInspectorForPlatform(platform: NodeJS.Platform) { }); } +/** + * Resolve whether the shell owns the PTY from the best signal available. + * + * POSIX inspectors report foreground process-group ownership directly. When + * that probe is unavailable, unknown is the conservative choice: ordinary keys + * still pass, but capability replies are filtered. Windows has no equivalent + * ownership probe, so preserve its previous behavior and treat a running child + * as foreground; otherwise full-screen programs lose their terminal replies. + */ +function resolveShellForeground( + platform: NodeJS.Platform, + inspection: TerminalSubprocessInspectResult, +): boolean | null { + if (inspection.shellForeground !== undefined) return inspection.shellForeground; + if (!inspection.hasRunningSubprocess) return true; + return platform === "win32" ? false : null; +} + /** * Upper bound on retained scrollback CHARACTERS, complementing the line limit. * @@ -949,7 +967,11 @@ function capHistory(history: string, maxLines: number, maxChars: number): string capped = capped.slice(newline + 1); } else { const escape = capped.indexOf("\u001b", cut); - capped = capped.slice(escape !== -1 && escape < cut + 64 * 1024 ? escape : cut); + const sliceAt = escape !== -1 && escape < cut + 64 * 1024 ? escape : cut; + const codePoint = capped.charCodeAt(sliceAt); + const unicodeSafeSliceAt = + sliceAt > 0 && codePoint >= 0xdc00 && codePoint <= 0xdfff ? sliceAt - 1 : sliceAt; + capped = capped.slice(unicodeSafeSliceAt); } } return capped; @@ -1422,8 +1444,8 @@ const INPUT_OSC = "(?:\\x1b\\]|\\x9d)"; const INPUT_DCS = "(?:\\x1bP|\\x90)"; const INPUT_ST = "(?:\\x07|\\x1b\\\\|\\x9c)"; const INPUT_CONTROL_INTRODUCER = "(?:\\x1b[\\[\\]P]|[\\x90\\x9b\\x9d])"; -const ABANDONED_PRIVATE_CSI_INPUT_PREFIX = new RegExp( - `${INPUT_CSI}\\?[0-9;]*(?=${INPUT_CONTROL_INTRODUCER})`, +const ABANDONED_CSI_INPUT_PREFIX = new RegExp( + `${INPUT_CSI}[\\x20-\\x3f]*(?=${INPUT_CONTROL_INTRODUCER})`, "g", ); function composeInputMatcher(): RegExp | null { @@ -1724,11 +1746,11 @@ export function sanitizeTerminalInputChunk( pendingControlSequence: string, data: string, ): { data: string; pendingControlSequence: string } { - // A client parser can time out after `CSI ?`, then begin the next response - // with a fresh introducer. Treat the abandoned private-CSI prefix as residue; - // otherwise the second `[` is mistaken for the first sequence's final byte - // and both prefixes leak to the shell as visible `^[[?` text. - const input = `${pendingControlSequence}${data}`.replace(ABANDONED_PRIVATE_CSI_INPUT_PREFIX, ""); + // A client parser can time out after a partial CSI parameter sequence, then + // begin the next response with a fresh introducer. Treat the abandoned prefix + // as residue; otherwise the second `[` is mistaken for the first sequence's + // final byte and both prefixes leak to the shell as visible text. + const input = `${pendingControlSequence}${data}`.replace(ABANDONED_CSI_INPUT_PREFIX, ""); const parsed = sanitizeTerminalChunkOnce(input); // Match the live-output parser's memory bound. A malformed client must not @@ -2830,7 +2852,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func } return [undefined, state] as const; }); - return; + return Option.none(); } const next = inspectResult.value; @@ -2854,8 +2876,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // Refresh the foreground-ownership signal even when nothing wire-label- // worthy changed (a `fg`/`bg` flip of the same child alters what the // input reply-strip must do without changing the activity event). - liveSession.value.shellForeground = - next.shellForeground ?? (next.hasRunningSubprocess ? null : true); + liveSession.value.shellForeground = resolveShellForeground(platform, next); if (liveSession.value.shellForeground === false) { // A prefix buffered while the shell owned the PTY is unsolicited // reply residue, not input for the newly foreground application. @@ -2885,15 +2906,25 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func ] as const; }); - if (Option.isSome(event)) { - yield* publishEvent(event.value); - } + return event; }); - yield* Effect.forEach(runningSessions, checkSubprocessActivity, { - concurrency: "unbounded", - discard: true, - }); + yield* Effect.forEach( + runningSessions, + (session) => + Effect.gen(function* () { + const event = yield* withThreadLock(session.threadId, checkSubprocessActivity(session)); + // Listener callbacks stay outside the session lock so a subscriber + // can safely trigger another terminal operation for this thread. + if (Option.isSome(event)) { + yield* publishEvent(event.value); + } + }), + { + concurrency: "unbounded", + discard: true, + }, + ); }); const hasRunningSessions = readManagerState.pipe( @@ -3292,7 +3323,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func ); }; - const write: TerminalManager["Service"]["write"] = Effect.fn("terminal.write")(function* (input) { + const writeLocked = Effect.fn("terminal.writeLocked")(function* (input: TerminalWriteInput) { const terminalId = input.terminalId; const session = yield* requireSession(input.threadId, terminalId); const process = session.process; @@ -3314,8 +3345,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func ) { const refreshed = yield* Effect.exit(subprocessInspector(process.pid)); if (refreshed._tag === "Success") { - session.shellForeground = - refreshed.value.shellForeground ?? (refreshed.value.hasRunningSubprocess ? null : true); + session.shellForeground = resolveShellForeground(platform, refreshed.value); } else { session.shellForeground = null; } @@ -3357,6 +3387,9 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func }); }); + const write: TerminalManager["Service"]["write"] = (input) => + withThreadLock(input.threadId, writeLocked(input)); + const resizeLocked = Effect.fn("terminal.resize")(function* (input: TerminalResizeInput) { const session = yield* getSession(input.threadId, input.terminalId); // ResizeObserver traffic can already be in flight when the UI closes the session. From 41312b233e17b69c0002346969be0c63a43e4c87 Mon Sep 17 00:00:00 2001 From: olafura Date: Fri, 24 Jul 2026 11:28:23 +0200 Subject: [PATCH 08/18] fix(terminal): separate keyboard input from emulator replies --- apps/server/src/terminal/Manager.test.ts | 71 ++++++++++++++++++++++++ apps/server/src/terminal/Manager.ts | 11 +++- packages/contracts/src/terminal.test.ts | 22 ++++++++ packages/contracts/src/terminal.ts | 11 ++++ 4 files changed, 112 insertions(+), 3 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index a31a71fe140..5b7186aafda 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -1025,6 +1025,77 @@ it.layer( }), ); + it.effect( + "strips outer-terminal replies from keyboard input while a git diff pager owns the PTY", + () => + Effect.gen(function* () { + const inspect = { + hasRunningSubprocess: true, + childCommand: "less", + processIds: [100], + shellForeground: false, + }; + const { manager, ptyAdapter, getEvents } = yield* createManager(5, { + subprocessInspector: () => Effect.succeed(inspect), + subprocessPollIntervalMs: 20, + }); + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + expect(process).toBeDefined(); + if (!process) return; + + yield* waitFor( + Effect.map(getEvents, (events) => + events.some((event) => event.type === "activity" && event.hasRunningSubprocess), + ), + "1200 millis", + ); + + // Reproduced by feeding the queries emitted around `git diff`/less + // through xterm and by inspecting the persisted failing PTY log. These + // replies belong to the OUTER renderer, not to less, even though less + // currently owns the embedded PTY. Relaying them makes less display + // `ESC...` and starts the feedback flood. + for (const data of [ + "\x1b[?69;0$y", + "\x1b[?2026;2$y", + "\x1b[?2027;0$y", + "\x1b[?2031;0$y", + "\x1b[?2048;0$y", + "\x1b[?1;2c", + "\x1b]11;rgb:1616/1616/1616\x1b\\", + "\x1b[0n", + "\x1b[I", + ]) { + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data, + inputSource: "keyboard", + }); + } + + // Ordinary pager input still passes through the same keyboard stream. + // In particular, a bare Escape must not be held as an incomplete reply + // prefix: keyboard events are already complete units when they arrive + // in one terminal.write RPC. + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b", + inputSource: "keyboard", + }); + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "q", + inputSource: "keyboard", + }); + + expect(process.writes).toEqual(["\x1b", "q"]); + }), + ); + it.effect("serializes overlapping reply writes through the per-thread lock", () => Effect.gen(function* () { const inspect = { diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index 0e8f2d99f39..1d45605a717 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -3339,7 +3339,9 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // replies arriving in that gap must refresh foreground ownership before we // decide whether to relay them; otherwise the now-idle shell receives a // whole response burst and prompt redraws amplify it into a feedback loop. + const alwaysFilterTerminalResponses = input.inputSource === "keyboard"; if ( + !alwaysFilterTerminalResponses && !session.shellForeground && mayContainTerminalResponse(session.pendingInputControlSequence, input.data) ) { @@ -3362,8 +3364,9 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // program's very first queries can still lose a reply, and a program // `exec`'d over the shell keeps its pgid so it still looks like the shell — // both accepted next to the runaway flood the strip prevents. - const data = - session.shellForeground !== false + const data = alwaysFilterTerminalResponses + ? stripTerminalResponsesFromInput(input.data) + : session.shellForeground !== false ? (() => { const sanitized = sanitizeTerminalInputChunk( session.pendingInputControlSequence, @@ -3373,7 +3376,9 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func return sanitized.data; })() : input.data; - if (session.shellForeground === false) session.pendingInputControlSequence = ""; + if (session.shellForeground === false) { + session.pendingInputControlSequence = ""; + } if (data.length === 0) return; yield* Effect.try({ try: () => process.write(data), diff --git a/packages/contracts/src/terminal.test.ts b/packages/contracts/src/terminal.test.ts index a08ed492388..8adc27124a3 100644 --- a/packages/contracts/src/terminal.test.ts +++ b/packages/contracts/src/terminal.test.ts @@ -134,6 +134,28 @@ describe("TerminalWriteInput", () => { ).toBe(true); }); + it("accepts an explicit keyboard input source", () => { + expect( + decodes(TerminalWriteInput, { + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\u001b[?1;2c", + inputSource: "keyboard", + }), + ).toBe(true); + }); + + it("rejects an unknown input source", () => { + expect( + decodes(TerminalWriteInput, { + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "q", + inputSource: "renderer", + }), + ).toBe(false); + }); + it("rejects empty data", () => { expect( decodes(TerminalWriteInput, { diff --git a/packages/contracts/src/terminal.ts b/packages/contracts/src/terminal.ts index fa5f1821169..123c14081b4 100644 --- a/packages/contracts/src/terminal.ts +++ b/packages/contracts/src/terminal.ts @@ -60,6 +60,17 @@ export type TerminalAttachInput = Schema.Codec.Encoded; From 181807bb45a288b8083401cea096ea4670ee41c3 Mon Sep 17 00:00:00 2001 From: olafura Date: Fri, 24 Jul 2026 11:51:29 +0200 Subject: [PATCH 09/18] fix(terminal): keep subprocess polling outside session locks --- apps/server/src/terminal/Manager.test.ts | 46 ++++++++++++++++++++++++ apps/server/src/terminal/Manager.ts | 20 +++++++++-- 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index 5b7186aafda..c6bfed4c5fb 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -10,6 +10,7 @@ import { } from "@t3tools/contracts"; import { HostProcessPlatform } from "@t3tools/shared/hostProcess"; import * as Data from "effect/Data"; +import * as Deferred from "effect/Deferred"; import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as Encoding from "effect/Encoding"; @@ -939,6 +940,51 @@ it.layer( }), ); + it.effect("does not hold the terminal lock while inspecting subprocesses", () => + Effect.gen(function* () { + const inspectionStarted = yield* Deferred.make(); + const releaseInspection = yield* Deferred.make(); + let blockInspection = false; + const idleShell = { + hasRunningSubprocess: false, + childCommand: null, + processIds: [] as ReadonlyArray, + shellForeground: true, + }; + const { manager, ptyAdapter } = yield* createManager(5, { + subprocessInspector: () => + blockInspection + ? Effect.gen(function* () { + yield* Deferred.succeed(inspectionStarted, undefined); + yield* Deferred.await(releaseInspection); + return idleShell; + }) + : Effect.succeed(idleShell), + subprocessPollIntervalMs: 20, + }); + + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + expect(process).toBeDefined(); + if (!process) return; + + blockInspection = true; + yield* Deferred.await(inspectionStarted).pipe(Effect.timeout("1200 millis")); + + const writeExit = yield* manager + .write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "x", + }) + .pipe(Effect.timeout("100 millis"), Effect.exit); + yield* Deferred.succeed(releaseInspection, undefined); + + expect(Exit.isSuccess(writeExit)).toBe(true); + expect(process.writes).toEqual(["x"]); + }), + ); + it.effect( "strips capability replies at an idle prompt but relays them to a foreground program", () => diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index 1d45605a717..29ba5ee35cd 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -2823,11 +2823,11 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func return; } - const checkSubprocessActivity = Effect.fn("terminal.checkSubprocessActivity")(function* ( + const inspectSubprocessActivity = Effect.fn("terminal.inspectSubprocessActivity")(function* ( session: TerminalSessionState & { pid: number }, ) { const terminalPid = session.pid; - const inspectResult = yield* subprocessInspector(terminalPid).pipe( + return yield* subprocessInspector(terminalPid).pipe( Effect.map(Option.some), Effect.catch((reason) => Effect.logWarning("failed to check terminal subprocess activity", { @@ -2838,7 +2838,13 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func }).pipe(Effect.as(Option.none())), ), ); + }); + const applySubprocessActivity = Effect.fn("terminal.applySubprocessActivity")(function* ( + session: TerminalSessionState & { pid: number }, + inspectResult: Option.Option, + ) { + const terminalPid = session.pid; if (Option.isNone(inspectResult)) { yield* modifyManagerState((state) => { const liveSession = state.sessions.get( @@ -2913,7 +2919,15 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func runningSessions, (session) => Effect.gen(function* () { - const event = yield* withThreadLock(session.threadId, checkSubprocessActivity(session)); + // Process inspection invokes external `ps`/`pgrep` commands and can + // take seconds to time out. Keep that I/O outside the per-thread lock + // so writes and resizes remain responsive, then revalidate the + // captured PID while applying the result under the lock. + const inspectResult = yield* inspectSubprocessActivity(session); + const event = yield* withThreadLock( + session.threadId, + applySubprocessActivity(session, inspectResult), + ); // Listener callbacks stay outside the session lock so a subscriber // can safely trigger another terminal operation for this thread. if (Option.isSome(event)) { From 1db2d6ae19eafbc8e860f8c9ca53e217fbe7eab8 Mon Sep 17 00:00:00 2001 From: olafura Date: Fri, 24 Jul 2026 11:52:29 +0200 Subject: [PATCH 10/18] fix(terminal): forward standalone xterm escape keys --- apps/server/src/terminal/Manager.test.ts | 19 +++++++++++++++++-- apps/server/src/terminal/Manager.ts | 22 ++++++++++++++-------- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index c6bfed4c5fb..7c60bbfdaa3 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -1043,6 +1043,21 @@ it.layer( }); expect(process.writes).toEqual([]); + // xterm's default onData path also carries physical keys. A standalone + // Escape callback is a complete key event, not the first chunk of a + // capability reply, so it must reach the shell before the next key. + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b", + }); + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "q", + }); + expect(process.writes).toEqual(["\x1b", "q"]); + // Foreground program running (vim): it issued the query and is blocked // reading the answer — input must pass through verbatim. inspect = { @@ -1067,7 +1082,7 @@ it.layer( terminalId: DEFAULT_TERMINAL_ID, data: "\x1b[I", }); - expect(process.writes).toEqual(["\x1b[1;1R", "\x1b[I"]); + expect(process.writes).toEqual(["\x1b", "q", "\x1b[1;1R", "\x1b[I"]); }), ); @@ -1450,7 +1465,7 @@ it.layer( yield* manager.write({ threadId: "thread-1", terminalId: DEFAULT_TERMINAL_ID, - data: "\x1b", + data: "\x1b[1", }); inspect = { hasRunningSubprocess: true, diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index 29ba5ee35cd..1af1fb5da9f 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -3378,17 +3378,23 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // program's very first queries can still lose a reply, and a program // `exec`'d over the shell keeps its pgid so it still looks like the shell — // both accepted next to the runaway flood the strip prevents. + const isStandaloneTerminalEscape = + (input.inputSource ?? "terminal") === "terminal" && + session.pendingInputControlSequence.length === 0 && + input.data === "\x1b"; const data = alwaysFilterTerminalResponses ? stripTerminalResponsesFromInput(input.data) : session.shellForeground !== false - ? (() => { - const sanitized = sanitizeTerminalInputChunk( - session.pendingInputControlSequence, - input.data, - ); - session.pendingInputControlSequence = sanitized.pendingControlSequence; - return sanitized.data; - })() + ? isStandaloneTerminalEscape + ? input.data + : (() => { + const sanitized = sanitizeTerminalInputChunk( + session.pendingInputControlSequence, + input.data, + ); + session.pendingInputControlSequence = sanitized.pendingControlSequence; + return sanitized.data; + })() : input.data; if (session.shellForeground === false) { session.pendingInputControlSequence = ""; From a9da52b1798e2381ccdeef7adb8f94c86aa3743b Mon Sep 17 00:00:00 2001 From: olafura Date: Fri, 24 Jul 2026 12:02:32 +0200 Subject: [PATCH 11/18] fix(terminal): discard superseded subprocess polls --- apps/server/src/terminal/Manager.test.ts | 79 ++++++++++++++++++++++ apps/server/src/terminal/Manager.ts | 86 +++++++++++++++++------- 2 files changed, 140 insertions(+), 25 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index 7c60bbfdaa3..21f8e88c60c 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -985,6 +985,85 @@ it.layer( }), ); + it.effect("discards a periodic inspection superseded by a write refresh", () => + Effect.gen(function* () { + const initialInspectionCompleted = yield* Deferred.make(); + const staleInspectionStarted = yield* Deferred.make(); + const releaseStaleInspection = yield* Deferred.make(); + let initialInspection = true; + let blockNextInspection = false; + let freshWriteInspection = true; + let inspections = 0; + const foregroundProgram = { + hasRunningSubprocess: true, + childCommand: "vim", + processIds: [100] as ReadonlyArray, + shellForeground: false, + }; + const idleShell = { + hasRunningSubprocess: false, + childCommand: null, + processIds: [] as ReadonlyArray, + shellForeground: true, + }; + const { manager, ptyAdapter } = yield* createManager(5, { + subprocessInspector: () => { + inspections += 1; + if (initialInspection) { + initialInspection = false; + return Deferred.succeed(initialInspectionCompleted, undefined).pipe( + Effect.as(foregroundProgram), + ); + } + if (blockNextInspection) { + blockNextInspection = false; + return Deferred.succeed(staleInspectionStarted, undefined).pipe( + Effect.andThen(Deferred.await(releaseStaleInspection)), + Effect.as(foregroundProgram), + ); + } + if (freshWriteInspection) { + freshWriteInspection = false; + return Effect.succeed(idleShell); + } + return Effect.succeed(foregroundProgram); + }, + subprocessPollIntervalMs: 100, + }); + + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + expect(process).toBeDefined(); + if (!process) return; + + yield* Deferred.await(initialInspectionCompleted).pipe(Effect.timeout("1200 millis")); + blockNextInspection = true; + yield* Deferred.await(staleInspectionStarted).pipe(Effect.timeout("1200 millis")); + + // This on-demand inspection is newer than the blocked periodic poll and + // observes that the shell owns the PTY again. + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[1;1R", + }); + yield* Deferred.succeed(releaseStaleInspection, undefined); + yield* Effect.sleep("25 millis"); + + // If the stale poll clobbered the refreshed foreground state, this write + // performs another inspection and relays the reply to the foreground + // program. Keeping the fresh state strips it at the idle prompt. + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[2;1R", + }); + + expect(inspections).toBe(3); + expect(process.writes).toEqual([]); + }), + ); + it.effect( "strips capability replies at an idle prompt but relays them to a foreground program", () => diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index 1af1fb5da9f..4c22249c220 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -271,6 +271,12 @@ export interface TerminalSessionState { * until a later probe provides a definitive answer. */ shellForeground: boolean | null; + /** + * Advances whenever foreground ownership is refreshed. Periodic inspections + * capture this revision before running outside the thread lock and may only + * apply their result if it is still current. + */ + subprocessInspectionRevision: number; /** Normalized child command name when `hasRunningSubprocess`; cleared when idle. */ childCommandLabel: string | null; runtimeEnv: Record | null; @@ -2490,6 +2496,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.pid = null; session.hasRunningSubprocess = false; session.shellForeground = true; + session.subprocessInspectionRevision += 1; session.childCommandLabel = null; session.status = "exited"; session.pendingHistoryControlSequence = ""; @@ -2564,6 +2571,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.pid = null; session.hasRunningSubprocess = false; session.shellForeground = true; + session.subprocessInspectionRevision += 1; session.childCommandLabel = null; session.status = "exited"; session.pendingHistoryControlSequence = ""; @@ -2663,6 +2671,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.exitSignal = null; session.hasRunningSubprocess = false; session.shellForeground = true; + session.subprocessInspectionRevision += 1; session.childCommandLabel = null; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; @@ -2814,17 +2823,24 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func const pollSubprocessActivity = Effect.fn("terminal.pollSubprocessActivity")(function* () { const state = yield* readManagerState; - const runningSessions = [...state.sessions.values()].filter( - (session): session is TerminalSessionState & { pid: number } => - session.status === "running" && Number.isInteger(session.pid), - ); + const runningSessions = [...state.sessions.values()] + .filter( + (session): session is TerminalSessionState & { pid: number } => + session.status === "running" && Number.isInteger(session.pid), + ) + .map((session) => ({ + threadId: session.threadId, + terminalId: session.terminalId, + pid: session.pid, + subprocessInspectionRevision: session.subprocessInspectionRevision, + })); if (runningSessions.length === 0) { return; } const inspectSubprocessActivity = Effect.fn("terminal.inspectSubprocessActivity")(function* ( - session: TerminalSessionState & { pid: number }, + session: (typeof runningSessions)[number], ) { const terminalPid = session.pid; return yield* subprocessInspector(terminalPid).pipe( @@ -2841,20 +2857,26 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func }); const applySubprocessActivity = Effect.fn("terminal.applySubprocessActivity")(function* ( - session: TerminalSessionState & { pid: number }, + session: (typeof runningSessions)[number], inspectResult: Option.Option, ) { const terminalPid = session.pid; + const expectedRevision = session.subprocessInspectionRevision; if (Option.isNone(inspectResult)) { yield* modifyManagerState((state) => { const liveSession = state.sessions.get( toSessionKey(session.threadId, session.terminalId), ); - if (liveSession?.status === "running" && liveSession.pid === terminalPid) { + if ( + liveSession?.status === "running" && + liveSession.pid === terminalPid && + liveSession.subprocessInspectionRevision === expectedRevision + ) { // Do not keep routing input from a stale foreground observation. // Unknown ownership strips capability replies until a later probe // succeeds, while ordinary keyboard input still passes unchanged. liveSession.shellForeground = null; + liveSession.subprocessInspectionRevision += 1; } return [undefined, state] as const; }); @@ -2862,27 +2884,27 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func } const next = inspectResult.value; - yield* registerTerminalProcesses({ - threadId: session.threadId, - terminalId: session.terminalId, - processIds: next.processIds, - }); const nextChildLabel = next.hasRunningSubprocess ? next.childCommand : null; - const event = yield* modifyManagerState((state) => { + const appliedResult = yield* modifyManagerState<{ + readonly applied: boolean; + readonly event: Option.Option; + }>((state) => { const liveSession: Option.Option = Option.fromNullishOr( state.sessions.get(toSessionKey(session.threadId, session.terminalId)), ); if ( Option.isNone(liveSession) || liveSession.value.status !== "running" || - liveSession.value.pid !== terminalPid + liveSession.value.pid !== terminalPid || + liveSession.value.subprocessInspectionRevision !== expectedRevision ) { - return [Option.none(), state] as const; + return [{ applied: false, event: Option.none() }, state] as const; } // Refresh the foreground-ownership signal even when nothing wire-label- // worthy changed (a `fg`/`bg` flip of the same child alters what the // input reply-strip must do without changing the activity event). liveSession.value.shellForeground = resolveShellForeground(platform, next); + liveSession.value.subprocessInspectionRevision += 1; if (liveSession.value.shellForeground === false) { // A prefix buffered while the shell owned the PTY is unsolicited // reply residue, not input for the newly foreground application. @@ -2892,7 +2914,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func liveSession.value.hasRunningSubprocess === next.hasRunningSubprocess && liveSession.value.childCommandLabel === nextChildLabel ) { - return [Option.none(), state] as const; + return [{ applied: true, event: Option.none() }, state] as const; } liveSession.value.hasRunningSubprocess = next.hasRunningSubprocess; @@ -2900,19 +2922,30 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func const eventStamp = advanceEventSequence(liveSession.value); return [ - Option.some({ - type: "activity" as const, - threadId: liveSession.value.threadId, - terminalId: liveSession.value.terminalId, - sequence: eventStamp.sequence, - hasRunningSubprocess: next.hasRunningSubprocess, - label: terminalWireLabel(liveSession.value), - }), + { + applied: true, + event: Option.some({ + type: "activity" as const, + threadId: liveSession.value.threadId, + terminalId: liveSession.value.terminalId, + sequence: eventStamp.sequence, + hasRunningSubprocess: next.hasRunningSubprocess, + label: terminalWireLabel(liveSession.value), + }), + }, state, ] as const; }); - return event; + if (appliedResult.applied) { + yield* registerTerminalProcesses({ + threadId: session.threadId, + terminalId: session.terminalId, + processIds: next.processIds, + }); + } + + return appliedResult.event; }); yield* Effect.forEach( @@ -3023,6 +3056,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func unsubscribeExit: null, hasRunningSubprocess: false, shellForeground: true, + subprocessInspectionRevision: 0, childCommandLabel: null, runtimeEnv: normalizedRuntimeEnv(input.env), }; @@ -3365,6 +3399,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func } else { session.shellForeground = null; } + session.subprocessInspectionRevision += 1; } // The reply-strip exists to break the IDLE-PROMPT echo loop (a shell with @@ -3495,6 +3530,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func unsubscribeExit: null, hasRunningSubprocess: false, shellForeground: true, + subprocessInspectionRevision: 0, childCommandLabel: null, runtimeEnv: normalizedRuntimeEnv(input.env), }; From 8b3d8c19c360945eab20922db0cd7515308d5096 Mon Sep 17 00:00:00 2001 From: olafura Date: Fri, 24 Jul 2026 12:09:00 +0200 Subject: [PATCH 12/18] fix(terminal): distinguish xterm renderer replies --- apps/server/src/terminal/Manager.test.ts | 122 +++++++++--------- apps/server/src/terminal/Manager.ts | 3 +- .../components/ThreadTerminalDrawer.test.ts | 25 ++++ .../src/components/ThreadTerminalDrawer.tsx | 36 +++++- packages/contracts/src/terminal.test.ts | 13 +- packages/contracts/src/terminal.ts | 14 +- 6 files changed, 136 insertions(+), 77 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index 21f8e88c60c..27eb445651c 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -1165,75 +1165,73 @@ it.layer( }), ); - it.effect( - "strips outer-terminal replies from keyboard input while a git diff pager owns the PTY", - () => - Effect.gen(function* () { - const inspect = { - hasRunningSubprocess: true, - childCommand: "less", - processIds: [100], - shellForeground: false, - }; - const { manager, ptyAdapter, getEvents } = yield* createManager(5, { - subprocessInspector: () => Effect.succeed(inspect), - subprocessPollIntervalMs: 20, - }); - yield* manager.open(openInput()); - const process = ptyAdapter.processes[0]; - expect(process).toBeDefined(); - if (!process) return; - - yield* waitFor( - Effect.map(getEvents, (events) => - events.some((event) => event.type === "activity" && event.hasRunningSubprocess), - ), - "1200 millis", - ); + it.effect("strips xterm renderer replies while a git diff pager owns the PTY", () => + Effect.gen(function* () { + const inspect = { + hasRunningSubprocess: true, + childCommand: "less", + processIds: [100], + shellForeground: false, + }; + const { manager, ptyAdapter, getEvents } = yield* createManager(5, { + subprocessInspector: () => Effect.succeed(inspect), + subprocessPollIntervalMs: 20, + }); + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + expect(process).toBeDefined(); + if (!process) return; - // Reproduced by feeding the queries emitted around `git diff`/less - // through xterm and by inspecting the persisted failing PTY log. These - // replies belong to the OUTER renderer, not to less, even though less - // currently owns the embedded PTY. Relaying them makes less display - // `ESC...` and starts the feedback flood. - for (const data of [ - "\x1b[?69;0$y", - "\x1b[?2026;2$y", - "\x1b[?2027;0$y", - "\x1b[?2031;0$y", - "\x1b[?2048;0$y", - "\x1b[?1;2c", - "\x1b]11;rgb:1616/1616/1616\x1b\\", - "\x1b[0n", - "\x1b[I", - ]) { - yield* manager.write({ - threadId: "thread-1", - terminalId: DEFAULT_TERMINAL_ID, - data, - inputSource: "keyboard", - }); - } + yield* waitFor( + Effect.map(getEvents, (events) => + events.some((event) => event.type === "activity" && event.hasRunningSubprocess), + ), + "1200 millis", + ); - // Ordinary pager input still passes through the same keyboard stream. - // In particular, a bare Escape must not be held as an incomplete reply - // prefix: keyboard events are already complete units when they arrive - // in one terminal.write RPC. + // Reproduced by feeding the queries emitted around `git diff`/less + // through xterm and by inspecting the persisted failing PTY log. These + // replies belong to the OUTER renderer, not to less, even though less + // currently owns the embedded PTY. Relaying them makes less display + // `ESC...` and starts the feedback flood. + for (const data of [ + "\x1b[?69;0$y", + "\x1b[?2026;2$y", + "\x1b[?2027;0$y", + "\x1b[?2031;0$y", + "\x1b[?2048;0$y", + "\x1b[?1;2c", + "\x1b]11;rgb:1616/1616/1616\x1b\\", + "\x1b[0n", + "\x1b[I", + ]) { yield* manager.write({ threadId: "thread-1", terminalId: DEFAULT_TERMINAL_ID, - data: "\x1b", - inputSource: "keyboard", - }); - yield* manager.write({ - threadId: "thread-1", - terminalId: DEFAULT_TERMINAL_ID, - data: "q", - inputSource: "keyboard", + data, + inputSource: "renderer", }); + } - expect(process.writes).toEqual(["\x1b", "q"]); - }), + // Ordinary pager input still passes through xterm's physical-key path. + // In particular, a bare Escape must not be held as an incomplete reply + // prefix: keyboard events are already complete units when they arrive + // in one terminal.write RPC. + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b", + inputSource: "terminal", + }); + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "q", + inputSource: "terminal", + }); + + expect(process.writes).toEqual(["\x1b", "q"]); + }), ); it.effect("serializes overlapping reply writes through the per-thread lock", () => diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index 4c22249c220..89ed1af00e8 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -3387,7 +3387,8 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // replies arriving in that gap must refresh foreground ownership before we // decide whether to relay them; otherwise the now-idle shell receives a // whole response burst and prompt redraws amplify it into a feedback loop. - const alwaysFilterTerminalResponses = input.inputSource === "keyboard"; + const alwaysFilterTerminalResponses = + input.inputSource === "keyboard" || input.inputSource === "renderer"; if ( !alwaysFilterTerminalResponses && !session.shellForeground && diff --git a/apps/web/src/components/ThreadTerminalDrawer.test.ts b/apps/web/src/components/ThreadTerminalDrawer.test.ts index 9f483af3c3b..af8c708011c 100644 --- a/apps/web/src/components/ThreadTerminalDrawer.test.ts +++ b/apps/web/src/components/ThreadTerminalDrawer.test.ts @@ -1,11 +1,36 @@ import { describe, expect, it } from "vite-plus/test"; import { + createTerminalInputSourceClassifier, resolveTerminalSelectionActionPosition, shouldHandleTerminalSelectionMouseUp, terminalSelectionActionDelayForClickCount, } from "./ThreadTerminalDrawer"; +describe("createTerminalInputSourceClassifier", () => { + it("classifies xterm key data as physical terminal input", () => { + const classifier = createTerminalInputSourceClassifier(); + classifier.recordKey("\u001b"); + + expect(classifier.classifyData("\u001b")).toBe("terminal"); + }); + + it("classifies xterm capability replies as renderer input", () => { + const classifier = createTerminalInputSourceClassifier(); + + expect(classifier.classifyData("\u001b[?1;2c")).toBe("renderer"); + expect(classifier.classifyData("\u001b]11;rgb:1616/1616/1616\u001b\\")).toBe("renderer"); + }); + + it("does not reuse a stale physical-key marker for later renderer data", () => { + const classifier = createTerminalInputSourceClassifier(); + classifier.recordKey("q"); + + expect(classifier.classifyData("\u001b[?2026;2$y")).toBe("renderer"); + expect(classifier.classifyData("q")).toBe("renderer"); + }); +}); + describe("resolveTerminalSelectionActionPosition", () => { it("prefers the selection rect over the last pointer position", () => { expect( diff --git a/apps/web/src/components/ThreadTerminalDrawer.tsx b/apps/web/src/components/ThreadTerminalDrawer.tsx index 8591c24c71a..2d4c46ca673 100644 --- a/apps/web/src/components/ThreadTerminalDrawer.tsx +++ b/apps/web/src/components/ThreadTerminalDrawer.tsx @@ -71,6 +71,23 @@ const MIN_DRAWER_HEIGHT = 180; const MAX_DRAWER_HEIGHT_RATIO = 0.75; const MULTI_CLICK_SELECTION_ACTION_DELAY_MS = 260; +export function createTerminalInputSourceClassifier(): { + readonly recordKey: (data: string) => void; + readonly classifyData: (data: string) => "terminal" | "renderer"; +} { + let pendingKeyData: string | null = null; + return { + recordKey: (data) => { + pendingKeyData = data; + }, + classifyData: (data) => { + const source = pendingKeyData === data ? "terminal" : "renderer"; + pendingKeyData = null; + return source; + }, + }; +} + function maxDrawerHeight(): number { if (typeof window === "undefined") return DEFAULT_THREAD_TERMINAL_HEIGHT; return Math.max(MIN_DRAWER_HEIGHT, Math.floor(window.innerHeight * MAX_DRAWER_HEIGHT_RATIO)); @@ -352,11 +369,12 @@ export function TerminalViewport({ ...(runtimeEnv ? { env: runtimeEnv } : {}), }, }); - const writeTerminal = useEffectEvent((data: string) => - runTerminalWrite({ - environmentId, - input: { threadId, terminalId, data }, - }), + const writeTerminal = useEffectEvent( + (data: string, inputSource: "terminal" | "renderer" = "terminal") => + runTerminalWrite({ + environmentId, + input: { threadId, terminalId, data, inputSource }, + }), ); const resizeTerminal = useEffectEvent((cols: number, rows: number) => runTerminalResize({ @@ -562,6 +580,11 @@ export function TerminalViewport({ return false; }); + const inputSourceClassifier = createTerminalInputSourceClassifier(); + const keyDisposable = terminal.onKey(({ key }) => { + inputSourceClassifier.recordKey(key); + }); + const terminalLinksDisposable = terminal.registerLinkProvider({ provideLinks: (bufferLineNumber, callback) => { const activeTerminal = terminalRef.current; @@ -648,7 +671,7 @@ export function TerminalViewport({ const inputDisposable = terminal.onData((data) => { void (async () => { - const result = await writeTerminal(data); + const result = await writeTerminal(data, inputSourceClassifier.classifyData(data)); if (result._tag === "Success" || isAtomCommandInterrupted(result)) { return; } @@ -718,6 +741,7 @@ export function TerminalViewport({ return () => { window.clearTimeout(fitTimer); + keyDisposable.dispose(); inputDisposable.dispose(); selectionDisposable.dispose(); terminalLinksDisposable.dispose(); diff --git a/packages/contracts/src/terminal.test.ts b/packages/contracts/src/terminal.test.ts index 8adc27124a3..278e1fccd53 100644 --- a/packages/contracts/src/terminal.test.ts +++ b/packages/contracts/src/terminal.test.ts @@ -145,13 +145,24 @@ describe("TerminalWriteInput", () => { ).toBe(true); }); + it("accepts an explicit renderer input source", () => { + expect( + decodes(TerminalWriteInput, { + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\u001b[?1;2c", + inputSource: "renderer", + }), + ).toBe(true); + }); + it("rejects an unknown input source", () => { expect( decodes(TerminalWriteInput, { threadId: "thread-1", terminalId: DEFAULT_TERMINAL_ID, data: "q", - inputSource: "renderer", + inputSource: "automation", }), ).toBe(false); }); diff --git a/packages/contracts/src/terminal.ts b/packages/contracts/src/terminal.ts index 123c14081b4..7fa2ea3ab2e 100644 --- a/packages/contracts/src/terminal.ts +++ b/packages/contracts/src/terminal.ts @@ -63,14 +63,14 @@ export const TerminalWriteInput = Schema.Struct({ /** * Where the bytes came from. * - * Browser xterm's `onData` stream mixes physical keys with legitimate - * emulator-generated capability replies, so the default remains `terminal`. - * TUI keyboard input is different: it is decoded by the outer terminal first, - * which can accidentally surface that outer terminal's own replies as key - * sequences. The server always filters reply-shaped bytes from `keyboard` - * writes so they cannot be injected into an embedded pager or shell. + * Browser xterm classifies physical keys as `terminal` and capability replies + * generated by its emulator as `renderer`. TUI keyboard input is decoded by + * the outer terminal first, which can accidentally surface that outer + * terminal's replies as key sequences. The server always filters reply-shaped + * bytes from `renderer` and `keyboard` writes so they cannot be injected into + * an embedded pager or shell. */ - inputSource: Schema.optional(Schema.Literals(["terminal", "keyboard", "paste"])), + inputSource: Schema.optional(Schema.Literals(["terminal", "keyboard", "renderer", "paste"])), }); export type TerminalWriteInput = Schema.Codec.Encoded; From 89c28897d5287aa56217af8eb355e0eaab768a72 Mon Sep 17 00:00:00 2001 From: olafura Date: Fri, 24 Jul 2026 12:17:59 +0200 Subject: [PATCH 13/18] fix(terminal): refine renderer reply ownership --- apps/server/src/terminal/Manager.test.ts | 24 ++++++++++-- apps/server/src/terminal/Manager.ts | 48 ++++++++++++++++-------- 2 files changed, 54 insertions(+), 18 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index 27eb445651c..8417df547d6 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -1000,13 +1000,19 @@ it.layer( processIds: [100] as ReadonlyArray, shellForeground: false, }; + const staleActivity = { + hasRunningSubprocess: true, + childCommand: "less", + processIds: [100, 101] as ReadonlyArray, + shellForeground: false, + }; const idleShell = { hasRunningSubprocess: false, childCommand: null, processIds: [] as ReadonlyArray, shellForeground: true, }; - const { manager, ptyAdapter } = yield* createManager(5, { + const { manager, ptyAdapter, getEvents } = yield* createManager(5, { subprocessInspector: () => { inspections += 1; if (initialInspection) { @@ -1019,7 +1025,7 @@ it.layer( blockNextInspection = false; return Deferred.succeed(staleInspectionStarted, undefined).pipe( Effect.andThen(Deferred.await(releaseStaleInspection)), - Effect.as(foregroundProgram), + Effect.as(staleActivity), ); } if (freshWriteInspection) { @@ -1048,7 +1054,15 @@ it.layer( data: "\x1b[1;1R", }); yield* Deferred.succeed(releaseStaleInspection, undefined); - yield* Effect.sleep("25 millis"); + yield* waitFor( + Effect.map(getEvents, (events) => + events.some( + (event) => + event.type === "activity" && event.hasRunningSubprocess && event.label === "less", + ), + ), + "1200 millis", + ); // If the stale poll clobbered the refreshed foreground state, this write // performs another inspection and relays the reply to the foreground @@ -1155,11 +1169,13 @@ it.layer( threadId: "thread-1", terminalId: DEFAULT_TERMINAL_ID, data: "\x1b[1;1R", + inputSource: "renderer", }); yield* manager.write({ threadId: "thread-1", terminalId: DEFAULT_TERMINAL_ID, data: "\x1b[I", + inputSource: "renderer", }); expect(process.writes).toEqual(["\x1b", "q", "\x1b[1;1R", "\x1b[I"]); }), @@ -1195,6 +1211,8 @@ it.layer( // currently owns the embedded PTY. Relaying them makes less display // `ESC...` and starts the feedback flood. for (const data of [ + "\x1b[?", + "\x1b[?1;2c", "\x1b[?69;0$y", "\x1b[?2026;2$y", "\x1b[?2027;0$y", diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index 89ed1af00e8..981b588c32f 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -349,6 +349,13 @@ function terminalWireLabel(session: TerminalSessionState): string { return truncateTerminalWireLabel(getTerminalLabel(session.terminalId)); } +const TERMINAL_REPLY_UNAWARE_PAGERS = new Set(["less", "more", "most", "lv"]); + +function isTerminalReplyUnawarePager(session: TerminalSessionState): boolean { + const command = session.childCommandLabel?.trim().toLowerCase(); + return command !== undefined && TERMINAL_REPLY_UNAWARE_PAGERS.has(command); +} + function snapshot(session: TerminalSessionState): TerminalSessionSnapshot { return { threadId: session.threadId, @@ -2895,21 +2902,24 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func if ( Option.isNone(liveSession) || liveSession.value.status !== "running" || - liveSession.value.pid !== terminalPid || - liveSession.value.subprocessInspectionRevision !== expectedRevision + liveSession.value.pid !== terminalPid ) { return [{ applied: false, event: Option.none() }, state] as const; } - // Refresh the foreground-ownership signal even when nothing wire-label- - // worthy changed (a `fg`/`bg` flip of the same child alters what the - // input reply-strip must do without changing the activity event). - liveSession.value.shellForeground = resolveShellForeground(platform, next); - liveSession.value.subprocessInspectionRevision += 1; - if (liveSession.value.shellForeground === false) { - // A prefix buffered while the shell owned the PTY is unsolicited - // reply residue, not input for the newly foreground application. - liveSession.value.pendingInputControlSequence = ""; + if (liveSession.value.subprocessInspectionRevision === expectedRevision) { + // Refresh the foreground-ownership signal only when no newer + // on-demand inspection has made this routing observation stale. + liveSession.value.shellForeground = resolveShellForeground(platform, next); + liveSession.value.subprocessInspectionRevision += 1; + if (liveSession.value.shellForeground === false) { + // A prefix buffered while the shell owned the PTY is unsolicited + // reply residue, not input for the newly foreground application. + liveSession.value.pendingInputControlSequence = ""; + } } + // Process metadata is still useful when a newer write inspection won + // the foreground-routing race: it drives the activity label and + // process registry without overwriting that newer routing decision. if ( liveSession.value.hasRunningSubprocess === next.hasRunningSubprocess && liveSession.value.childCommandLabel === nextChildLabel @@ -3387,10 +3397,13 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // replies arriving in that gap must refresh foreground ownership before we // decide whether to relay them; otherwise the now-idle shell receives a // whole response burst and prompt redraws amplify it into a feedback loop. - const alwaysFilterTerminalResponses = - input.inputSource === "keyboard" || input.inputSource === "renderer"; + const alwaysFilterTerminalResponses = input.inputSource === "keyboard"; + const initiallyFilterRendererResponses = + input.inputSource === "renderer" && + (session.shellForeground !== false || isTerminalReplyUnawarePager(session)); if ( !alwaysFilterTerminalResponses && + !initiallyFilterRendererResponses && !session.shellForeground && mayContainTerminalResponse(session.pendingInputControlSequence, input.data) ) { @@ -3402,6 +3415,9 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func } session.subprocessInspectionRevision += 1; } + const filterRendererResponses = + input.inputSource === "renderer" && + (session.shellForeground !== false || isTerminalReplyUnawarePager(session)); // The reply-strip exists to break the IDLE-PROMPT echo loop (a shell with // no reader echoes the emulator's auto-replies, and a prompt that re-queries @@ -3418,9 +3434,11 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func (input.inputSource ?? "terminal") === "terminal" && session.pendingInputControlSequence.length === 0 && input.data === "\x1b"; + const statefullyFilterTerminalResponses = + filterRendererResponses || session.shellForeground !== false; const data = alwaysFilterTerminalResponses ? stripTerminalResponsesFromInput(input.data) - : session.shellForeground !== false + : statefullyFilterTerminalResponses ? isStandaloneTerminalEscape ? input.data : (() => { @@ -3432,7 +3450,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func return sanitized.data; })() : input.data; - if (session.shellForeground === false) { + if (session.shellForeground === false && !filterRendererResponses) { session.pendingInputControlSequence = ""; } if (data.length === 0) return; From 97cd8dbb7caa41bdb4174a842d80017111052705 Mon Sep 17 00:00:00 2001 From: olafura Date: Fri, 24 Jul 2026 12:23:47 +0200 Subject: [PATCH 14/18] refactor(terminal): centralize reply filtering in server --- apps/server/src/terminal/Manager.test.ts | 10 +++--- apps/server/src/terminal/Manager.ts | 14 +++----- .../components/ThreadTerminalDrawer.test.ts | 25 ------------- .../src/components/ThreadTerminalDrawer.tsx | 36 ++++--------------- packages/contracts/src/terminal.test.ts | 11 ------ packages/contracts/src/terminal.ts | 12 +++---- 6 files changed, 20 insertions(+), 88 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index 8417df547d6..82c823caaeb 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -1169,19 +1169,17 @@ it.layer( threadId: "thread-1", terminalId: DEFAULT_TERMINAL_ID, data: "\x1b[1;1R", - inputSource: "renderer", }); yield* manager.write({ threadId: "thread-1", terminalId: DEFAULT_TERMINAL_ID, data: "\x1b[I", - inputSource: "renderer", }); expect(process.writes).toEqual(["\x1b", "q", "\x1b[1;1R", "\x1b[I"]); }), ); - it.effect("strips xterm renderer replies while a git diff pager owns the PTY", () => + it.effect("strips terminal replies while a git diff pager owns the PTY", () => Effect.gen(function* () { const inspect = { hasRunningSubprocess: true, @@ -1207,9 +1205,10 @@ it.layer( // Reproduced by feeding the queries emitted around `git diff`/less // through xterm and by inspecting the persisted failing PTY log. These - // replies belong to the OUTER renderer, not to less, even though less + // These reply-shaped bytes do not belong to less, even though less // currently owns the embedded PTY. Relaying them makes less display - // `ESC...` and starts the feedback flood. + // `ESC...` and starts the feedback flood. This policy is enforced here in + // the backend regardless of which terminal client supplied the bytes. for (const data of [ "\x1b[?", "\x1b[?1;2c", @@ -1227,7 +1226,6 @@ it.layer( threadId: "thread-1", terminalId: DEFAULT_TERMINAL_ID, data, - inputSource: "renderer", }); } diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index 981b588c32f..ab9d3a35a19 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -3398,12 +3398,10 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // decide whether to relay them; otherwise the now-idle shell receives a // whole response burst and prompt redraws amplify it into a feedback loop. const alwaysFilterTerminalResponses = input.inputSource === "keyboard"; - const initiallyFilterRendererResponses = - input.inputSource === "renderer" && - (session.shellForeground !== false || isTerminalReplyUnawarePager(session)); + const filterTerminalResponsesForForegroundProcess = isTerminalReplyUnawarePager(session); if ( !alwaysFilterTerminalResponses && - !initiallyFilterRendererResponses && + !filterTerminalResponsesForForegroundProcess && !session.shellForeground && mayContainTerminalResponse(session.pendingInputControlSequence, input.data) ) { @@ -3415,9 +3413,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func } session.subprocessInspectionRevision += 1; } - const filterRendererResponses = - input.inputSource === "renderer" && - (session.shellForeground !== false || isTerminalReplyUnawarePager(session)); + const filterTerminalResponsesForCurrentProcess = isTerminalReplyUnawarePager(session); // The reply-strip exists to break the IDLE-PROMPT echo loop (a shell with // no reader echoes the emulator's auto-replies, and a prompt that re-queries @@ -3435,7 +3431,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.pendingInputControlSequence.length === 0 && input.data === "\x1b"; const statefullyFilterTerminalResponses = - filterRendererResponses || session.shellForeground !== false; + filterTerminalResponsesForCurrentProcess || session.shellForeground !== false; const data = alwaysFilterTerminalResponses ? stripTerminalResponsesFromInput(input.data) : statefullyFilterTerminalResponses @@ -3450,7 +3446,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func return sanitized.data; })() : input.data; - if (session.shellForeground === false && !filterRendererResponses) { + if (session.shellForeground === false && !filterTerminalResponsesForCurrentProcess) { session.pendingInputControlSequence = ""; } if (data.length === 0) return; diff --git a/apps/web/src/components/ThreadTerminalDrawer.test.ts b/apps/web/src/components/ThreadTerminalDrawer.test.ts index af8c708011c..9f483af3c3b 100644 --- a/apps/web/src/components/ThreadTerminalDrawer.test.ts +++ b/apps/web/src/components/ThreadTerminalDrawer.test.ts @@ -1,36 +1,11 @@ import { describe, expect, it } from "vite-plus/test"; import { - createTerminalInputSourceClassifier, resolveTerminalSelectionActionPosition, shouldHandleTerminalSelectionMouseUp, terminalSelectionActionDelayForClickCount, } from "./ThreadTerminalDrawer"; -describe("createTerminalInputSourceClassifier", () => { - it("classifies xterm key data as physical terminal input", () => { - const classifier = createTerminalInputSourceClassifier(); - classifier.recordKey("\u001b"); - - expect(classifier.classifyData("\u001b")).toBe("terminal"); - }); - - it("classifies xterm capability replies as renderer input", () => { - const classifier = createTerminalInputSourceClassifier(); - - expect(classifier.classifyData("\u001b[?1;2c")).toBe("renderer"); - expect(classifier.classifyData("\u001b]11;rgb:1616/1616/1616\u001b\\")).toBe("renderer"); - }); - - it("does not reuse a stale physical-key marker for later renderer data", () => { - const classifier = createTerminalInputSourceClassifier(); - classifier.recordKey("q"); - - expect(classifier.classifyData("\u001b[?2026;2$y")).toBe("renderer"); - expect(classifier.classifyData("q")).toBe("renderer"); - }); -}); - describe("resolveTerminalSelectionActionPosition", () => { it("prefers the selection rect over the last pointer position", () => { expect( diff --git a/apps/web/src/components/ThreadTerminalDrawer.tsx b/apps/web/src/components/ThreadTerminalDrawer.tsx index 2d4c46ca673..8591c24c71a 100644 --- a/apps/web/src/components/ThreadTerminalDrawer.tsx +++ b/apps/web/src/components/ThreadTerminalDrawer.tsx @@ -71,23 +71,6 @@ const MIN_DRAWER_HEIGHT = 180; const MAX_DRAWER_HEIGHT_RATIO = 0.75; const MULTI_CLICK_SELECTION_ACTION_DELAY_MS = 260; -export function createTerminalInputSourceClassifier(): { - readonly recordKey: (data: string) => void; - readonly classifyData: (data: string) => "terminal" | "renderer"; -} { - let pendingKeyData: string | null = null; - return { - recordKey: (data) => { - pendingKeyData = data; - }, - classifyData: (data) => { - const source = pendingKeyData === data ? "terminal" : "renderer"; - pendingKeyData = null; - return source; - }, - }; -} - function maxDrawerHeight(): number { if (typeof window === "undefined") return DEFAULT_THREAD_TERMINAL_HEIGHT; return Math.max(MIN_DRAWER_HEIGHT, Math.floor(window.innerHeight * MAX_DRAWER_HEIGHT_RATIO)); @@ -369,12 +352,11 @@ export function TerminalViewport({ ...(runtimeEnv ? { env: runtimeEnv } : {}), }, }); - const writeTerminal = useEffectEvent( - (data: string, inputSource: "terminal" | "renderer" = "terminal") => - runTerminalWrite({ - environmentId, - input: { threadId, terminalId, data, inputSource }, - }), + const writeTerminal = useEffectEvent((data: string) => + runTerminalWrite({ + environmentId, + input: { threadId, terminalId, data }, + }), ); const resizeTerminal = useEffectEvent((cols: number, rows: number) => runTerminalResize({ @@ -580,11 +562,6 @@ export function TerminalViewport({ return false; }); - const inputSourceClassifier = createTerminalInputSourceClassifier(); - const keyDisposable = terminal.onKey(({ key }) => { - inputSourceClassifier.recordKey(key); - }); - const terminalLinksDisposable = terminal.registerLinkProvider({ provideLinks: (bufferLineNumber, callback) => { const activeTerminal = terminalRef.current; @@ -671,7 +648,7 @@ export function TerminalViewport({ const inputDisposable = terminal.onData((data) => { void (async () => { - const result = await writeTerminal(data, inputSourceClassifier.classifyData(data)); + const result = await writeTerminal(data); if (result._tag === "Success" || isAtomCommandInterrupted(result)) { return; } @@ -741,7 +718,6 @@ export function TerminalViewport({ return () => { window.clearTimeout(fitTimer); - keyDisposable.dispose(); inputDisposable.dispose(); selectionDisposable.dispose(); terminalLinksDisposable.dispose(); diff --git a/packages/contracts/src/terminal.test.ts b/packages/contracts/src/terminal.test.ts index 278e1fccd53..11dc3f50167 100644 --- a/packages/contracts/src/terminal.test.ts +++ b/packages/contracts/src/terminal.test.ts @@ -145,17 +145,6 @@ describe("TerminalWriteInput", () => { ).toBe(true); }); - it("accepts an explicit renderer input source", () => { - expect( - decodes(TerminalWriteInput, { - threadId: "thread-1", - terminalId: DEFAULT_TERMINAL_ID, - data: "\u001b[?1;2c", - inputSource: "renderer", - }), - ).toBe(true); - }); - it("rejects an unknown input source", () => { expect( decodes(TerminalWriteInput, { diff --git a/packages/contracts/src/terminal.ts b/packages/contracts/src/terminal.ts index 7fa2ea3ab2e..def7da5d012 100644 --- a/packages/contracts/src/terminal.ts +++ b/packages/contracts/src/terminal.ts @@ -63,14 +63,12 @@ export const TerminalWriteInput = Schema.Struct({ /** * Where the bytes came from. * - * Browser xterm classifies physical keys as `terminal` and capability replies - * generated by its emulator as `renderer`. TUI keyboard input is decoded by - * the outer terminal first, which can accidentally surface that outer - * terminal's replies as key sequences. The server always filters reply-shaped - * bytes from `renderer` and `keyboard` writes so they cannot be injected into - * an embedded pager or shell. + * TUI keyboard input is decoded by the outer terminal first, which can + * accidentally surface that outer terminal's replies as key sequences. The + * server uses this transport-level hint while retaining all terminal-response + * parsing and process-ownership policy in the terminal manager. */ - inputSource: Schema.optional(Schema.Literals(["terminal", "keyboard", "renderer", "paste"])), + inputSource: Schema.optional(Schema.Literals(["terminal", "keyboard", "paste"])), }); export type TerminalWriteInput = Schema.Codec.Encoded; From 638968ff706a25ca4818d23af07c67cb0d079e1e Mon Sep 17 00:00:00 2001 From: olafura Date: Fri, 24 Jul 2026 12:31:16 +0200 Subject: [PATCH 15/18] fix(terminal): filter replies for nested pagers --- apps/server/src/terminal/Manager.test.ts | 28 +++++++++---- apps/server/src/terminal/Manager.ts | 52 ++++++++++++++++++++++-- 2 files changed, 69 insertions(+), 11 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index 82c823caaeb..ecfdf365c1e 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -1179,12 +1179,13 @@ it.layer( }), ); - it.effect("strips terminal replies while a git diff pager owns the PTY", () => + it.effect("strips terminal replies while a nested git diff pager owns the PTY", () => Effect.gen(function* () { const inspect = { hasRunningSubprocess: true, - childCommand: "less", - processIds: [100], + childCommand: "git", + processIds: [100, 101], + hasTerminalReplyUnawareSubprocess: true, shellForeground: false, }; const { manager, ptyAdapter, getEvents } = yield* createManager(5, { @@ -1205,10 +1206,21 @@ it.layer( // Reproduced by feeding the queries emitted around `git diff`/less // through xterm and by inspecting the persisted failing PTY log. These - // These reply-shaped bytes do not belong to less, even though less - // currently owns the embedded PTY. Relaying them makes less display - // `ESC...` and starts the feedback flood. This policy is enforced here in - // the backend regardless of which terminal client supplied the bytes. + // These reply-shaped bytes do not belong to the nested less process. + // Relaying them makes less display `ESC...` and starts the feedback + // flood. This policy is enforced here in the backend regardless of which + // terminal client supplied the bytes. + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[?", + }); + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "q", + }); + for (const data of [ "\x1b[?", "\x1b[?1;2c", @@ -1246,7 +1258,7 @@ it.layer( inputSource: "terminal", }); - expect(process.writes).toEqual(["\x1b", "q"]); + expect(process.writes).toEqual(["q", "\x1b", "q"]); }), ); diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index ab9d3a35a19..fab4c6f0d18 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -192,6 +192,8 @@ interface TerminalSubprocessInspectResult { readonly hasRunningSubprocess: boolean; readonly childCommand: string | null; readonly processIds: ReadonlyArray; + /** Whether any process in the terminal's descendant tree cannot consume terminal replies. */ + readonly hasTerminalReplyUnawareSubprocess?: boolean; /** * Whether the shell itself owns the PTY's foreground process group * (`tpgid === pgid` of the terminal process) — i.e. the terminal is at an @@ -279,6 +281,8 @@ export interface TerminalSessionState { subprocessInspectionRevision: number; /** Normalized child command name when `hasRunningSubprocess`; cleared when idle. */ childCommandLabel: string | null; + /** Whether any current descendant is known not to consume terminal replies. */ + hasTerminalReplyUnawareSubprocess: boolean; runtimeEnv: Record | null; } @@ -351,9 +355,17 @@ function terminalWireLabel(session: TerminalSessionState): string { const TERMINAL_REPLY_UNAWARE_PAGERS = new Set(["less", "more", "most", "lv"]); +function isTerminalReplyUnawareCommand(raw: string, platform: NodeJS.Platform): boolean { + const command = normalizeChildCommandName(raw, platform)?.toLowerCase(); + return command !== undefined && TERMINAL_REPLY_UNAWARE_PAGERS.has(command); +} + function isTerminalReplyUnawarePager(session: TerminalSessionState): boolean { const command = session.childCommandLabel?.trim().toLowerCase(); - return command !== undefined && TERMINAL_REPLY_UNAWARE_PAGERS.has(command); + return ( + session.hasTerminalReplyUnawareSubprocess || + (command !== undefined && TERMINAL_REPLY_UNAWARE_PAGERS.has(command)) + ); } function snapshot(session: TerminalSessionState): TerminalSessionSnapshot { @@ -714,6 +726,11 @@ function windowsInspectSubprocess( hasRunningSubprocess: true, childCommand: normalized ? truncateTerminalWireLabel(normalized) : null, processIds: [...processIds], + hasTerminalReplyUnawareSubprocess: [...processIds].some( + (pid) => + pid !== terminalPid && + isTerminalReplyUnawareCommand(processNameById.get(pid) ?? "", platform), + ), } as const; }), Effect.mapError( @@ -785,7 +802,7 @@ const posixInspectSubprocess = Effect.fn("terminal.posixInspectSubprocess")(func const runPs = processRunner .run({ command: "ps", - args: ["-eo", "pid=,ppid="], + args: ["-eo", "pid=,ppid=,comm="], timeout: "1 second", maxOutputBytes: 262_144, outputMode: "truncate", @@ -882,14 +899,16 @@ const posixInspectSubprocess = Effect.fn("terminal.posixInspectSubprocess")(func const normalized = rawComm ? normalizeChildCommandName(rawComm, platform) : null; const processIds = new Set([terminalPid]); + const processCommandById = new Map(); const psResult = yield* Effect.exit(runPs); if (psResult._tag === "Success" && psResult.value.code === 0) { const childrenByParent = new Map(); for (const line of psResult.value.stdout.split(/\r?\n/g)) { - const [pidRaw, ppidRaw] = line.trim().split(/\s+/g); + const [pidRaw, ppidRaw, command = ""] = line.trim().split(/\s+/g); const pid = Number(pidRaw); const ppid = Number(ppidRaw); if (!Number.isInteger(pid) || !Number.isInteger(ppid)) continue; + processCommandById.set(pid, command); const children = childrenByParent.get(ppid) ?? []; children.push(pid); childrenByParent.set(ppid, children); @@ -911,6 +930,11 @@ const posixInspectSubprocess = Effect.fn("terminal.posixInspectSubprocess")(func hasRunningSubprocess: true, childCommand: normalized ? truncateTerminalWireLabel(normalized) : null, processIds: [...processIds], + hasTerminalReplyUnawareSubprocess: [...processIds].some( + (pid) => + pid !== terminalPid && + isTerminalReplyUnawareCommand(processCommandById.get(pid) ?? "", platform), + ), ...(shellForeground !== undefined ? { shellForeground } : {}), }; }); @@ -2505,6 +2529,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.shellForeground = true; session.subprocessInspectionRevision += 1; session.childCommandLabel = null; + session.hasTerminalReplyUnawareSubprocess = false; session.status = "exited"; session.pendingHistoryControlSequence = ""; session.pendingInputControlSequence = ""; @@ -2580,6 +2605,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.shellForeground = true; session.subprocessInspectionRevision += 1; session.childCommandLabel = null; + session.hasTerminalReplyUnawareSubprocess = false; session.status = "exited"; session.pendingHistoryControlSequence = ""; session.pendingInputControlSequence = ""; @@ -2680,6 +2706,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.shellForeground = true; session.subprocessInspectionRevision += 1; session.childCommandLabel = null; + session.hasTerminalReplyUnawareSubprocess = false; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; session.processEventDrainRunning = false; @@ -2758,6 +2785,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func session.hasRunningSubprocess = false; session.shellForeground = true; session.childCommandLabel = null; + session.hasTerminalReplyUnawareSubprocess = false; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; session.processEventDrainRunning = false; @@ -2920,6 +2948,8 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // Process metadata is still useful when a newer write inspection won // the foreground-routing race: it drives the activity label and // process registry without overwriting that newer routing decision. + liveSession.value.hasTerminalReplyUnawareSubprocess = + next.hasTerminalReplyUnawareSubprocess ?? false; if ( liveSession.value.hasRunningSubprocess === next.hasRunningSubprocess && liveSession.value.childCommandLabel === nextChildLabel @@ -3068,6 +3098,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func shellForeground: true, subprocessInspectionRevision: 0, childCommandLabel: null, + hasTerminalReplyUnawareSubprocess: false, runtimeEnv: normalizedRuntimeEnv(input.env), }; @@ -3408,6 +3439,8 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func const refreshed = yield* Effect.exit(subprocessInspector(process.pid)); if (refreshed._tag === "Success") { session.shellForeground = resolveShellForeground(platform, refreshed.value); + session.hasTerminalReplyUnawareSubprocess = + refreshed.value.hasTerminalReplyUnawareSubprocess ?? false; } else { session.shellForeground = null; } @@ -3430,6 +3463,18 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func (input.inputSource ?? "terminal") === "terminal" && session.pendingInputControlSequence.length === 0 && input.data === "\x1b"; + if ( + filterTerminalResponsesForCurrentProcess && + session.pendingInputControlSequence.length > 0 && + input.data !== "\x1b" && + [...input.data].length === 1 + ) { + // Client transports multiplex generated replies and physical keys. A + // reply-unaware foreground process must never lose a complete one-key + // command (`q`, Enter, Ctrl-C) merely because an abandoned reply prefix + // was buffered by an earlier write. + session.pendingInputControlSequence = ""; + } const statefullyFilterTerminalResponses = filterTerminalResponsesForCurrentProcess || session.shellForeground !== false; const data = alwaysFilterTerminalResponses @@ -3547,6 +3592,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func shellForeground: true, subprocessInspectionRevision: 0, childCommandLabel: null, + hasTerminalReplyUnawareSubprocess: false, runtimeEnv: normalizedRuntimeEnv(input.env), }; const createdSession = session; From f28baa0b9256d7081d886c67448d04114972071f Mon Sep 17 00:00:00 2001 From: olafura Date: Fri, 24 Jul 2026 12:33:24 +0200 Subject: [PATCH 16/18] fix(terminal): keep pager routing observations current --- apps/server/src/terminal/Manager.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index fab4c6f0d18..d88c7452370 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -361,11 +361,7 @@ function isTerminalReplyUnawareCommand(raw: string, platform: NodeJS.Platform): } function isTerminalReplyUnawarePager(session: TerminalSessionState): boolean { - const command = session.childCommandLabel?.trim().toLowerCase(); - return ( - session.hasTerminalReplyUnawareSubprocess || - (command !== undefined && TERMINAL_REPLY_UNAWARE_PAGERS.has(command)) - ); + return session.hasTerminalReplyUnawareSubprocess; } function snapshot(session: TerminalSessionState): TerminalSessionSnapshot { @@ -2938,6 +2934,8 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // Refresh the foreground-ownership signal only when no newer // on-demand inspection has made this routing observation stale. liveSession.value.shellForeground = resolveShellForeground(platform, next); + liveSession.value.hasTerminalReplyUnawareSubprocess = + next.hasTerminalReplyUnawareSubprocess ?? false; liveSession.value.subprocessInspectionRevision += 1; if (liveSession.value.shellForeground === false) { // A prefix buffered while the shell owned the PTY is unsolicited @@ -2948,8 +2946,6 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // Process metadata is still useful when a newer write inspection won // the foreground-routing race: it drives the activity label and // process registry without overwriting that newer routing decision. - liveSession.value.hasTerminalReplyUnawareSubprocess = - next.hasTerminalReplyUnawareSubprocess ?? false; if ( liveSession.value.hasRunningSubprocess === next.hasRunningSubprocess && liveSession.value.childCommandLabel === nextChildLabel From 2369d9f4c427897251f33e3859cd4ce004d7d4d6 Mon Sep 17 00:00:00 2001 From: olafura Date: Fri, 24 Jul 2026 13:49:47 +0200 Subject: [PATCH 17/18] fix(terminal): harden pager ownership filtering --- apps/server/src/terminal/Manager.test.ts | 116 +++++++++++++++++++++++ apps/server/src/terminal/Manager.ts | 114 ++++++++++++++++------ 2 files changed, 203 insertions(+), 27 deletions(-) diff --git a/apps/server/src/terminal/Manager.test.ts b/apps/server/src/terminal/Manager.test.ts index ecfdf365c1e..c203e475206 100644 --- a/apps/server/src/terminal/Manager.test.ts +++ b/apps/server/src/terminal/Manager.test.ts @@ -30,6 +30,7 @@ import { expect } from "vite-plus/test"; import * as ProcessRunner from "../processRunner.ts"; import * as TerminalManager from "./Manager.ts"; import { + hasReplyUnawareForegroundProcess, sanitizePersistedTerminalHistory, sanitizeTerminalHistoryChunk, sanitizeTerminalInputChunk, @@ -1220,6 +1221,16 @@ it.layer( terminalId: DEFAULT_TERMINAL_ID, data: "q", }); + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[1;2", + }); + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "R", + }); for (const data of [ "\x1b[?", @@ -1262,6 +1273,48 @@ it.layer( }), ); + it.effect("refreshes stale pager ownership before routing a terminal reply", () => + Effect.gen(function* () { + let inspect = { + hasRunningSubprocess: true, + childCommand: "git", + processIds: [100, 101], + hasTerminalReplyUnawareSubprocess: true, + shellForeground: false, + }; + const { manager, ptyAdapter, getEvents } = yield* createManager(5, { + subprocessInspector: () => Effect.succeed(inspect), + subprocessPollIntervalMs: 20, + }); + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + expect(process).toBeDefined(); + if (!process) return; + + yield* waitFor( + Effect.map(getEvents, (events) => + events.some((event) => event.type === "activity" && event.hasRunningSubprocess), + ), + "1200 millis", + ); + inspect = { + hasRunningSubprocess: true, + childCommand: "vim", + processIds: [100, 102], + hasTerminalReplyUnawareSubprocess: false, + shellForeground: false, + }; + + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "\x1b[1;2R", + }); + + expect(process.writes).toEqual(["\x1b[1;2R"]); + }), + ); + it.effect("serializes overlapping reply writes through the per-thread lock", () => Effect.gen(function* () { const inspect = { @@ -2432,6 +2485,69 @@ it.layer( ); }); +describe("hasReplyUnawareForegroundProcess", () => { + it("detects a nested pager in the foreground process group", () => { + expect( + hasReplyUnawareForegroundProcess({ + platform: "linux", + foregroundProcessGroupId: 200, + shellForeground: false, + childPid: 101, + childCommand: "git", + processes: [ + { pid: 101, processGroupId: 200, command: "git" }, + { pid: 102, processGroupId: 200, command: "less" }, + ], + }), + ).toBe(true); + }); + + it("ignores a background pager when an interactive app owns the foreground group", () => { + expect( + hasReplyUnawareForegroundProcess({ + platform: "linux", + foregroundProcessGroupId: 300, + shellForeground: false, + childPid: 101, + childCommand: "vim", + processes: [ + { pid: 101, processGroupId: 300, command: "vim" }, + { pid: 102, processGroupId: 200, command: "less" }, + ], + }), + ).toBe(false); + }); + + it("does not use the direct-pager fallback when another foreground group is observed", () => { + expect( + hasReplyUnawareForegroundProcess({ + platform: "linux", + foregroundProcessGroupId: 300, + shellForeground: false, + childPid: 101, + childCommand: "less", + processes: [ + { pid: 101, processGroupId: undefined, command: "" }, + { pid: 102, processGroupId: 300, command: "vim" }, + ], + }), + ).toBe(false); + }); + + it("falls back to a known direct pager when tree metadata is unavailable", () => { + expect( + hasReplyUnawareForegroundProcess({ + platform: "linux", + foregroundProcessGroupId: 200, + shellForeground: false, + childPid: 101, + childCommand: "less", + processes: [{ pid: 101, processGroupId: undefined, command: "" }], + }), + ).toBe(true); + }); +}); + describe("sanitizeTerminalHistoryChunk", () => { const sanitize = (data: string, pending = "") => sanitizeTerminalHistoryChunk(pending, data); diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index d88c7452370..4747fee9c02 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -360,6 +360,49 @@ function isTerminalReplyUnawareCommand(raw: string, platform: NodeJS.Platform): return command !== undefined && TERMINAL_REPLY_UNAWARE_PAGERS.has(command); } +export function hasReplyUnawareForegroundProcess(input: { + readonly platform: NodeJS.Platform; + readonly foregroundProcessGroupId: number | undefined; + readonly shellForeground: boolean | undefined; + readonly childPid: number; + readonly childCommand: string | null; + readonly processes: ReadonlyArray<{ + readonly pid: number; + readonly processGroupId: number | undefined; + readonly command: string; + }>; +}): boolean { + const directChildIsReplyUnaware = isTerminalReplyUnawareCommand( + input.childCommand ?? "", + input.platform, + ); + const directChild = input.processes.find((process) => process.pid === input.childPid); + if (input.foregroundProcessGroupId === undefined) { + return input.shellForeground === false && directChildIsReplyUnaware; + } + if ( + input.processes.some( + (process) => + process.processGroupId === input.foregroundProcessGroupId && + isTerminalReplyUnawareCommand(process.command, input.platform), + ) + ) { + return true; + } + const observedForegroundProcess = input.processes.some( + (process) => process.processGroupId === input.foregroundProcessGroupId, + ); + // The tree-wide `ps` probe may omit command or group data while the focused + // direct-child probe still succeeds. Preserve that known `less` result only + // when no contradictory process-group observation exists. + return ( + !observedForegroundProcess && + directChild?.processGroupId === undefined && + input.shellForeground === false && + directChildIsReplyUnaware + ); +} + function isTerminalReplyUnawarePager(session: TerminalSessionState): boolean { return session.hasTerminalReplyUnawareSubprocess; } @@ -722,10 +765,12 @@ function windowsInspectSubprocess( hasRunningSubprocess: true, childCommand: normalized ? truncateTerminalWireLabel(normalized) : null, processIds: [...processIds], - hasTerminalReplyUnawareSubprocess: [...processIds].some( - (pid) => - pid !== terminalPid && - isTerminalReplyUnawareCommand(processNameById.get(pid) ?? "", platform), + // Windows does not expose POSIX foreground process groups. Restrict + // the fallback to the direct child instead of letting a background + // pager anywhere in the tree suppress replies for an interactive app. + hasTerminalReplyUnawareSubprocess: isTerminalReplyUnawareCommand( + processNameById.get(childPid) ?? "", + platform, ), } as const; }), @@ -756,6 +801,7 @@ const posixInspectSubprocess = Effect.fn("terminal.posixInspectSubprocess")(func // a foreground vim/fzf moves it to the job's group. `undefined` when `ps` // fails or the fields don't parse (callers fall back to the child check). let shellForeground: boolean | undefined; + let foregroundProcessGroupId: number | undefined; const tpgidResult = yield* Effect.exit( processRunner.run({ command: "ps", @@ -772,6 +818,7 @@ const posixInspectSubprocess = Effect.fn("terminal.posixInspectSubprocess")(func const pgid = Number(pgidRaw); if (Number.isInteger(tpgid) && Number.isInteger(pgid) && tpgid > 0 && pgid > 0) { shellForeground = tpgid === pgid; + foregroundProcessGroupId = tpgid; } } @@ -798,7 +845,7 @@ const posixInspectSubprocess = Effect.fn("terminal.posixInspectSubprocess")(func const runPs = processRunner .run({ command: "ps", - args: ["-eo", "pid=,ppid=,comm="], + args: ["-eo", "pid=,ppid=,pgid=,comm="], timeout: "1 second", maxOutputBytes: 262_144, outputMode: "truncate", @@ -896,15 +943,18 @@ const posixInspectSubprocess = Effect.fn("terminal.posixInspectSubprocess")(func const normalized = rawComm ? normalizeChildCommandName(rawComm, platform) : null; const processIds = new Set([terminalPid]); const processCommandById = new Map(); + const processGroupById = new Map(); const psResult = yield* Effect.exit(runPs); if (psResult._tag === "Success" && psResult.value.code === 0) { const childrenByParent = new Map(); for (const line of psResult.value.stdout.split(/\r?\n/g)) { - const [pidRaw, ppidRaw, command = ""] = line.trim().split(/\s+/g); + const [pidRaw, ppidRaw, pgidRaw, command = ""] = line.trim().split(/\s+/g); const pid = Number(pidRaw); const ppid = Number(ppidRaw); - if (!Number.isInteger(pid) || !Number.isInteger(ppid)) continue; + const pgid = Number(pgidRaw); + if (!Number.isInteger(pid) || !Number.isInteger(ppid) || !Number.isInteger(pgid)) continue; processCommandById.set(pid, command); + processGroupById.set(pid, pgid); const children = childrenByParent.get(ppid) ?? []; children.push(pid); childrenByParent.set(ppid, children); @@ -922,15 +972,23 @@ const posixInspectSubprocess = Effect.fn("terminal.posixInspectSubprocess")(func } else { processIds.add(childPid); } + const hasReplyUnawareProcess = hasReplyUnawareForegroundProcess({ + platform, + foregroundProcessGroupId, + shellForeground, + childPid, + childCommand: normalized, + processes: [...processIds].map((pid) => ({ + pid, + processGroupId: processGroupById.get(pid), + command: processCommandById.get(pid) ?? (pid === childPid ? (normalized ?? "") : ""), + })), + }); return { hasRunningSubprocess: true, childCommand: normalized ? truncateTerminalWireLabel(normalized) : null, processIds: [...processIds], - hasTerminalReplyUnawareSubprocess: [...processIds].some( - (pid) => - pid !== terminalPid && - isTerminalReplyUnawareCommand(processCommandById.get(pid) ?? "", platform), - ), + hasTerminalReplyUnawareSubprocess: hasReplyUnawareProcess, ...(shellForeground !== undefined ? { shellForeground } : {}), }; }); @@ -2907,6 +2965,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // Unknown ownership strips capability replies until a later probe // succeeds, while ordinary keyboard input still passes unchanged. liveSession.shellForeground = null; + liveSession.hasTerminalReplyUnawareSubprocess = false; liveSession.subprocessInspectionRevision += 1; } return [undefined, state] as const; @@ -3425,10 +3484,8 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func // decide whether to relay them; otherwise the now-idle shell receives a // whole response burst and prompt redraws amplify it into a feedback loop. const alwaysFilterTerminalResponses = input.inputSource === "keyboard"; - const filterTerminalResponsesForForegroundProcess = isTerminalReplyUnawarePager(session); if ( !alwaysFilterTerminalResponses && - !filterTerminalResponsesForForegroundProcess && !session.shellForeground && mayContainTerminalResponse(session.pendingInputControlSequence, input.data) ) { @@ -3439,6 +3496,7 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func refreshed.value.hasTerminalReplyUnawareSubprocess ?? false; } else { session.shellForeground = null; + session.hasTerminalReplyUnawareSubprocess = false; } session.subprocessInspectionRevision += 1; } @@ -3459,18 +3517,6 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func (input.inputSource ?? "terminal") === "terminal" && session.pendingInputControlSequence.length === 0 && input.data === "\x1b"; - if ( - filterTerminalResponsesForCurrentProcess && - session.pendingInputControlSequence.length > 0 && - input.data !== "\x1b" && - [...input.data].length === 1 - ) { - // Client transports multiplex generated replies and physical keys. A - // reply-unaware foreground process must never lose a complete one-key - // command (`q`, Enter, Ctrl-C) merely because an abandoned reply prefix - // was buffered by an earlier write. - session.pendingInputControlSequence = ""; - } const statefullyFilterTerminalResponses = filterTerminalResponsesForCurrentProcess || session.shellForeground !== false; const data = alwaysFilterTerminalResponses @@ -3479,11 +3525,25 @@ export const makeWithOptions = Effect.fn("TerminalManager.makeWithOptions")(func ? isStandaloneTerminalEscape ? input.data : (() => { + const previousPendingControlSequence = session.pendingInputControlSequence; const sanitized = sanitizeTerminalInputChunk( - session.pendingInputControlSequence, + previousPendingControlSequence, input.data, ); session.pendingInputControlSequence = sanitized.pendingControlSequence; + if ( + filterTerminalResponsesForCurrentProcess && + previousPendingControlSequence.length > 0 && + sanitized.pendingControlSequence.length === 0 && + sanitized.data.length > 0 && + input.data !== "\x1b" && + [...input.data].length === 1 + ) { + // The buffered prefix plus this byte was not a recognized + // terminal reply. Drop only the abandoned prefix and preserve + // the complete one-key pager command (`q`, Enter, Ctrl-C). + return input.data; + } return sanitized.data; })() : input.data; From 3bf7e82b171076b49d1ba05fb456d842650750bf Mon Sep 17 00:00:00 2001 From: olafura Date: Fri, 24 Jul 2026 14:33:12 +0200 Subject: [PATCH 18/18] fix(terminal): preserve live output past buffer cap --- .../features/terminal/terminalMenu.test.ts | 2 + .../src/components/ThreadTerminalDrawer.tsx | 29 +++++++--- .../src/state/terminalSession.test.ts | 50 ++++++++++++++++ .../src/state/terminalSession.ts | 57 ++++++++++++++++++- 4 files changed, 129 insertions(+), 9 deletions(-) diff --git a/apps/mobile/src/features/terminal/terminalMenu.test.ts b/apps/mobile/src/features/terminal/terminalMenu.test.ts index 1f176263ca5..823f2bf9b76 100644 --- a/apps/mobile/src/features/terminal/terminalMenu.test.ts +++ b/apps/mobile/src/features/terminal/terminalMenu.test.ts @@ -57,6 +57,8 @@ function makeKnownSession(input: { } : null, buffer: "", + bufferEpoch: 1, + appendedLength: 0, status: input.status, error: null, hasRunningSubprocess: false, diff --git a/apps/web/src/components/ThreadTerminalDrawer.tsx b/apps/web/src/components/ThreadTerminalDrawer.tsx index 8591c24c71a..ee1af3d0350 100644 --- a/apps/web/src/components/ThreadTerminalDrawer.tsx +++ b/apps/web/src/components/ThreadTerminalDrawer.tsx @@ -4,6 +4,7 @@ import { isAtomCommandInterrupted, squashAtomCommandFailure, } from "@t3tools/client-runtime/state/runtime"; +import { resolveTerminalBufferRenderUpdate } from "@t3tools/client-runtime/state/terminal"; import { Plus, SquareSplitHorizontal, @@ -370,6 +371,8 @@ export function TerminalViewport({ const terminalVersion = terminalSession.version; const previousSessionRef = useRef({ buffer: terminalBuffer, + bufferEpoch: terminalSession.bufferEpoch, + appendedLength: terminalSession.appendedLength, error: terminalError, status: terminalStatus, version: terminalVersion, @@ -403,6 +406,8 @@ export function TerminalViewport({ fitAddonRef.current = fitAddon; previousSessionRef.current = { buffer: "", + bufferEpoch: 0, + appendedLength: 0, status: "closed", error: null, version: 0, @@ -740,6 +745,8 @@ export function TerminalViewport({ const terminal = terminalRef.current; const current = { buffer: terminalBuffer, + bufferEpoch: terminalSession.bufferEpoch, + appendedLength: terminalSession.appendedLength, error: terminalError, status: terminalStatus, version: terminalVersion, @@ -754,13 +761,11 @@ export function TerminalViewport({ return; } - if ( - current.buffer.length >= previous.buffer.length && - current.buffer.startsWith(previous.buffer) - ) { - terminal.write(current.buffer.slice(previous.buffer.length)); - } else { - writeTerminalBuffer(terminal, current.buffer); + const bufferUpdate = resolveTerminalBufferRenderUpdate(previous, current); + if (bufferUpdate.type === "append") { + terminal.write(bufferUpdate.data); + } else if (bufferUpdate.type === "replace") { + writeTerminalBuffer(terminal, bufferUpdate.data); } terminal.clearSelection(); @@ -793,7 +798,15 @@ export function TerminalViewport({ }); } previousSessionRef.current = current; - }, [autoFocus, terminalBuffer, terminalError, terminalStatus, terminalVersion]); + }, [ + autoFocus, + terminalBuffer, + terminalError, + terminalSession.appendedLength, + terminalSession.bufferEpoch, + terminalStatus, + terminalVersion, + ]); useEffect(() => { if (!autoFocus) return; diff --git a/packages/client-runtime/src/state/terminalSession.test.ts b/packages/client-runtime/src/state/terminalSession.test.ts index 85c57592d11..aa7bff9fcc9 100644 --- a/packages/client-runtime/src/state/terminalSession.test.ts +++ b/packages/client-runtime/src/state/terminalSession.test.ts @@ -7,6 +7,7 @@ import { applyTerminalMetadataStreamEvent, combineTerminalSessionState, EMPTY_TERMINAL_BUFFER_STATE, + resolveTerminalBufferRenderUpdate, selectRunningSubprocessTerminalIds, } from "./terminalSession.ts"; @@ -127,6 +128,8 @@ describe("terminal session reducers", () => { expect(output).toMatchObject({ buffer: "lo world", + bufferEpoch: 1, + appendedLength: 6, status: "running", error: null, version: 2, @@ -184,4 +187,51 @@ describe("terminal session reducers", () => { expect(state.buffer).toBe("🙂"); }); + + it("keeps rolling history as an append instead of resetting the renderer", () => { + const first = applyTerminalAttachStreamEvent( + EMPTY_TERMINAL_BUFFER_STATE, + { + type: "output", + threadId: TARGET.threadId, + terminalId: TARGET.terminalId, + data: "abcde", + }, + 5, + ); + const rolled = applyTerminalAttachStreamEvent( + first, + { + type: "output", + threadId: TARGET.threadId, + terminalId: TARGET.terminalId, + data: "fg", + }, + 5, + ); + + expect(rolled.buffer).toBe("cdefg"); + expect(resolveTerminalBufferRenderUpdate(first, rolled)).toEqual({ + type: "append", + data: "fg", + }); + }); + + it("replaces the renderer when more than the retained tail was missed", () => { + const rolled = applyTerminalAttachStreamEvent( + EMPTY_TERMINAL_BUFFER_STATE, + { + type: "output", + threadId: TARGET.threadId, + terminalId: TARGET.terminalId, + data: "abcdefg", + }, + 5, + ); + + expect(resolveTerminalBufferRenderUpdate(EMPTY_TERMINAL_BUFFER_STATE, rolled)).toEqual({ + type: "replace", + data: "cdefg", + }); + }); }); diff --git a/packages/client-runtime/src/state/terminalSession.ts b/packages/client-runtime/src/state/terminalSession.ts index ee444e36db4..13d58b71f6f 100644 --- a/packages/client-runtime/src/state/terminalSession.ts +++ b/packages/client-runtime/src/state/terminalSession.ts @@ -10,6 +10,8 @@ import type { export interface TerminalSessionState { readonly summary: TerminalSummary | null; readonly buffer: string; + readonly bufferEpoch: number; + readonly appendedLength: number; readonly status: TerminalSessionSnapshot["status"] | "closed"; readonly error: string | null; readonly hasRunningSubprocess: boolean; @@ -19,6 +21,8 @@ export interface TerminalSessionState { export interface TerminalBufferState { readonly buffer: string; + readonly bufferEpoch: number; + readonly appendedLength: number; readonly status: TerminalSessionSnapshot["status"] | "closed"; readonly error: string | null; readonly updatedAt: string | null; @@ -46,6 +50,8 @@ export function selectRunningSubprocessTerminalIds( export const EMPTY_TERMINAL_BUFFER_STATE = Object.freeze({ buffer: "", + bufferEpoch: 0, + appendedLength: 0, status: "closed", error: null, updatedAt: null, @@ -55,6 +61,8 @@ export const EMPTY_TERMINAL_BUFFER_STATE = Object.freeze({ export const EMPTY_TERMINAL_SESSION_STATE = Object.freeze({ summary: null, buffer: "", + bufferEpoch: 0, + appendedLength: 0, status: "closed", error: null, hasRunningSubprocess: false, @@ -91,9 +99,12 @@ function trimBufferToBytes(buffer: string, maxBufferBytes: number): string { export function terminalBufferStateFromSnapshot( snapshot: TerminalSessionSnapshot, maxBufferBytes: number, + bufferEpoch = 1, ): TerminalBufferState { return { buffer: trimBufferToBytes(snapshot.history, maxBufferBytes), + bufferEpoch, + appendedLength: 0, status: snapshot.status, error: null, updatedAt: snapshot.updatedAt, @@ -114,6 +125,8 @@ export function combineTerminalSessionState( return { summary, buffer: buffer.buffer, + bufferEpoch: buffer.bufferEpoch, + appendedLength: buffer.appendedLength, status: buffer.version > 0 ? buffer.status : (summary?.status ?? buffer.status), error: buffer.error, hasRunningSubprocess: summary?.hasRunningSubprocess ?? false, @@ -130,11 +143,16 @@ export function applyTerminalAttachStreamEvent( switch (event.type) { case "snapshot": case "restarted": - return terminalBufferStateFromSnapshot(event.snapshot, maxBufferBytes); + return terminalBufferStateFromSnapshot( + event.snapshot, + maxBufferBytes, + current.bufferEpoch + 1, + ); case "output": return { ...current, buffer: trimBufferToBytes(`${current.buffer}${event.data}`, maxBufferBytes), + appendedLength: current.appendedLength + event.data.length, status: current.status === "closed" ? "running" : current.status, error: null, version: current.version + 1, @@ -143,6 +161,8 @@ export function applyTerminalAttachStreamEvent( return { ...current, buffer: "", + bufferEpoch: current.bufferEpoch + 1, + appendedLength: 0, error: null, version: current.version + 1, }; @@ -172,6 +192,41 @@ export function applyTerminalAttachStreamEvent( } } +export type TerminalBufferRenderUpdate = + | { readonly type: "none" } + | { readonly type: "append"; readonly data: string } + | { readonly type: "replace"; readonly data: string }; + +/** + * Resolve how a terminal renderer should consume a reduced attach-stream state. + * + * The retained buffer is a bounded tail. Once it reaches the cap, appending + * output shifts its prefix, so prefix comparison alone incorrectly looks like + * a full replacement on every event. `appendedLength` is monotonic within a + * buffer epoch and lets renderers append only the unseen tail while history + * rolls underneath them. + */ +export function resolveTerminalBufferRenderUpdate( + previous: Pick, + current: Pick, +): TerminalBufferRenderUpdate { + if (current.bufferEpoch !== previous.bufferEpoch) { + return { type: "replace", data: current.buffer }; + } + + const appendedLength = current.appendedLength - previous.appendedLength; + if (appendedLength === 0) { + return { type: "none" }; + } + if (appendedLength < 0 || appendedLength > current.buffer.length) { + return { type: "replace", data: current.buffer }; + } + return { + type: "append", + data: current.buffer.slice(current.buffer.length - appendedLength), + }; +} + export function applyTerminalMetadataStreamEvent( current: ReadonlyArray, event: TerminalMetadataStreamEvent,