From feeb6c2aa6661731a8f871e9d3f7d81b2205bb94 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 26 Apr 2026 12:47:50 +0700 Subject: [PATCH] fix: answer terminal protocol queries on the daemon, never the browser Browser xterm.js auto-replies to DA1/DA2/DA3 traveled back over the websocket and arrived at the PTY after the foreground app had exited, so the response bytes (\x1b[?62;4;9;22c) landed on the shell prompt as visible garbage. Move all replies to the daemon's headless xterm where they reach the PTY synchronously, and suppress every query class on the browser side. The image addon also registers a {final:"c"} handler inside the post-mount raf, so re-register suppression after addons load to stay last in xterm's LIFO dispatch. Replaces the previous printf-injection test with a real foreground helper that queries DA1 and asserts the reply lands on stdin, plus a browser table covering DA1/DA2/DA3 + DSR/CPR/DECRQM. --- .../terminal-emulator-runtime.browser.test.ts | 19 +++-- .../runtime/terminal-emulator-runtime.ts | 30 +++++--- packages/server/src/terminal/terminal.test.ts | 74 +++++++++++++++++-- packages/server/src/terminal/terminal.ts | 8 ++ 4 files changed, 111 insertions(+), 20 deletions(-) diff --git a/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts b/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts index c5abe9acd..623342c3a 100644 --- a/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts +++ b/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts @@ -162,17 +162,24 @@ describe("terminal emulator runtime in a real browser", () => { expect(refreshCalls.at(-1)).toEqual([0, terminal.rows - 1]); }); - it("suppresses non-identity terminal query responses as PTY input", async () => { + it.each([ + { name: "DA1", bytes: "\x1b[c" }, + { name: "DA1-zero", bytes: "\x1b[0c" }, + { name: "DA2", bytes: "\x1b[>c" }, + { name: "DA3", bytes: "\x1b[=c" }, + { name: "DSR-5", bytes: "\x1b[5n" }, + { name: "DSR-6", bytes: "\x1b[6n" }, + { name: "DSR-?6", bytes: "\x1b[?6n" }, + { name: "DECRQM", bytes: "\x1b[1$p" }, + { name: "DECRQM-?", bytes: "\x1b[?1$p" }, + ])("does not emit a PTY input reply for $name", async ({ bytes }) => { await page.viewport(900, 600); const mounted = createTerminalHost({ width: 720, height: 360 }); await waitFor({ predicate: () => mounted.sizes.length > 0 }); - const querySequences = ["\x1b[5n", "\x1b[6n", "\x1b[?6n", "\x1b[1$p", "\x1b[?1$p"]; - - for (const sequence of querySequences) { - mounted.runtime.write({ text: sequence }); - } + mounted.runtime.write({ text: bytes }); + await nextFrame(); await nextFrame(); expect(mounted.inputs).toEqual([]); diff --git a/packages/app/src/terminal/runtime/terminal-emulator-runtime.ts b/packages/app/src/terminal/runtime/terminal-emulator-runtime.ts index c1c64a497..ae0fa6b2c 100644 --- a/packages/app/src/terminal/runtime/terminal-emulator-runtime.ts +++ b/packages/app/src/terminal/runtime/terminal-emulator-runtime.ts @@ -220,6 +220,26 @@ export class TerminalEmulatorRuntime { this.fitAndEmitResize?.(true); }; + // Browser xterm is a renderer only; it never replies to terminal protocol queries. + // Replies live on the daemon (one process boundary from the PTY) so they arrive + // before the foreground app exits, instead of racing back over the websocket. + // Re-registered after the image addon loads so our handlers stay last in the + // LIFO dispatch (the image addon registers its own {final:"c"} for sixel DA1). + const registerProtocolQuerySuppression = (): void => { + terminal.parser.registerCsiHandler({ final: "c" }, () => true); + terminal.parser.registerCsiHandler({ prefix: ">", final: "c" }, () => true); + terminal.parser.registerCsiHandler({ prefix: "=", final: "c" }, () => true); + terminal.parser.registerCsiHandler({ final: "n" }, () => true); + terminal.parser.registerCsiHandler({ prefix: "?", final: "n" }, () => true); + terminal.parser.registerCsiHandler({ final: "R" }, () => true); + terminal.parser.registerCsiHandler({ intermediates: "$", final: "p" }, () => true); + terminal.parser.registerCsiHandler( + { prefix: "?", intermediates: "$", final: "p" }, + () => true, + ); + }; + registerProtocolQuerySuppression(); + let webglAddonRaf: number | null = requestAnimationFrame(() => { webglAddonRaf = null; try { @@ -231,21 +251,13 @@ export class TerminalEmulatorRuntime { terminal.loadAddon(webglAddon); imageAddon = new ImageAddon(); terminal.loadAddon(imageAddon); + registerProtocolQuerySuppression(); this.fitAndEmitResize?.(true); } catch { disposeWebglRenderer(); } }); - // Suppress cursor/status query responses whose async browser replies can race - // with foreground app exit and feed back to the shell as visible text. DA is - // intentionally left to xterm so attached TUIs can identify the terminal. - terminal.parser.registerCsiHandler({ final: "n" }, () => true); - terminal.parser.registerCsiHandler({ prefix: "?", final: "n" }, () => true); - terminal.parser.registerCsiHandler({ final: "R" }, () => true); - terminal.parser.registerCsiHandler({ intermediates: "$", final: "p" }, () => true); - terminal.parser.registerCsiHandler({ prefix: "?", intermediates: "$", final: "p" }, () => true); - const restoreDocumentStyles = this.applyDocumentBoundsStyles({ root: input.root, }); diff --git a/packages/server/src/terminal/terminal.test.ts b/packages/server/src/terminal/terminal.test.ts index ef77e6c69..5d32f6cbb 100644 --- a/packages/server/src/terminal/terminal.test.ts +++ b/packages/server/src/terminal/terminal.test.ts @@ -978,8 +978,55 @@ describe("subscribe", () => { expect(messages.length).toBe(0); }); +}); + +const DA_HELPER_SCRIPT = `process.stdin.setRawMode(true); +process.stdin.resume(); +let buf = ""; +const timer = setTimeout(() => { + process.stdout.write("DA_TIMEOUT\\n"); + process.exit(2); +}, 2500); +process.stdin.on("data", (chunk) => { + buf += chunk.toString("binary"); + const m = buf.match(/\\x1b\\[\\?[\\d;]+c/); + if (m) { + clearTimeout(timer); + process.stdout.write("DA_OK:" + m[0].slice(1) + "\\n"); + process.exit(0); + } +}); +process.stdout.write("\\x1b[c"); +`; + +function writeDaHelper(prefix: string): string { + const dir = mkdtempSync(join(tmpdir(), prefix)); + temporaryDirs.push(dir); + const path = join(dir, "helper.cjs"); + writeFileSync(path, DA_HELPER_SCRIPT); + return path; +} + +function isDaOkLine(line: string): boolean { + return line.startsWith("DA_OK:"); +} + +function hasDaOkLine(state: ReturnType): boolean { + return getLines(state).some(isDaOkLine); +} + +function lastNonEmptyLineIsPrompt(state: ReturnType): boolean { + const last = + getLines(state) + .toReversed() + .find((line) => line.length > 0) ?? ""; + return last === "$"; +} + +describe("terminal protocol queries", () => { + it("delivers a DA1 reply to a foreground app on stdin", async () => { + const helperPath = writeDaHelper("terminal-da-helper-"); - it("does not answer terminal identity queries from the headless snapshot parser", async () => { const session = trackSession( await createTerminal({ cwd: "/tmp", @@ -987,13 +1034,30 @@ describe("subscribe", () => { env: { PS1: "$ " }, }), ); - await waitForLines(session, ["$"]); - session.send({ type: "input", data: "printf '\\033[c'; echo after-da\r" }); - await waitForState(session, (state) => getLines(state).join("\n").includes("after-da")); + session.send({ type: "input", data: `${process.execPath} ${helperPath}\r` }); + await waitForState(session, hasDaOkLine); - expect(getLines(session.getState()).join("\n")).not.toContain("62;4;22c"); + const ack = getLines(session.getState()).find(isDaOkLine) ?? ""; + expect(ack).toMatch(/^DA_OK:\[\?[\d;]+c$/); + }); + + it("does not echo DA1 replies onto the prompt after the foreground app exits", async () => { + const helperPath = writeDaHelper("terminal-da-cleanup-"); + + const session = trackSession( + await createTerminal({ + cwd: "/tmp", + shell: "/bin/sh", + env: { PS1: "$ " }, + }), + ); + await waitForLines(session, ["$"]); + + session.send({ type: "input", data: `${process.execPath} ${helperPath}\r` }); + await waitForState(session, hasDaOkLine); + await waitForState(session, lastNonEmptyLineIsPrompt); }); }); diff --git a/packages/server/src/terminal/terminal.ts b/packages/server/src/terminal/terminal.ts index 94e0c8905..bba62a367 100644 --- a/packages/server/src/terminal/terminal.ts +++ b/packages/server/src/terminal/terminal.ts @@ -657,6 +657,14 @@ export async function createTerminal(options: CreateTerminalOptions): Promise { + if (params.length === 0 || (params.length === 1 && params[0] === 0)) { + ptyProcess.write("\x1b[?62;4;22c"); + return true; + } + return false; + }); + let disposeTitleChangeSubscription: { dispose(): void } | null = null; if (!lockedTitle) { disposeTitleChangeSubscription = terminal.onTitleChange((nextTitle) => {