From 49543d798e490bf777ea367989d26449aabf0e85 Mon Sep 17 00:00:00 2001 From: Kenneth Watson Date: Sun, 16 Aug 2026 18:55:49 +0800 Subject: [PATCH] fix(tui): detect SIXEL outside Windows Terminal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The startup graphics probe only ran on ConPTY hosts with WT_SESSION, so a SIXEL-capable terminal that exposes no identifying environment variable (foot exports TERM=foot and COLORTERM=truecolor only) resolved the trueColor capability row, kept imageProtocol null, and rendered every image as the "[Image: …]" text card. The XTSMGRAPHICS branch also had its status inverted: per xterm ctlseqs a reply of `CSI ? 2 ; Ps ; Pv S` carries Ps = 0 on success, and a terminal without SIXEL reports a zero maximum geometry, so a successful reply was read as unsupported. Drop the dead DA1 half of the probe with it: ProcessTerminal swallows every `CSI ? … c` reply for the whole session so a late one cannot leak into the composer (#8542), which means the attribute list never reached the probe's input listener on any platform. The bare `CSI c` it wrote was also unaccounted for in the DA1 sentinel FIFO, so its reply consumed another probe's sentinel. PI_FORCE_IMAGE_PROTOCOL, including its off/none kill switch, still wins over the probe. --- packages/tui/CHANGELOG.md | 4 + packages/tui/src/terminal-capabilities.ts | 10 ++ packages/tui/src/tui.ts | 66 ++++------ packages/tui/test/sixel-probe.test.ts | 154 +++++++++++++--------- 4 files changed, 127 insertions(+), 107 deletions(-) diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index f4c044438..e885fd2df 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed images rendering as the `[Image: …]` text card on SIXEL terminals that expose no identifying environment variable (foot, xterm, contour): the graphics probe no longer requires Windows Terminal, and no longer reads an XTSMGRAPHICS success reply as a failure. + ## [17.3.5] - 2026-08-16 ### Fixed diff --git a/packages/tui/src/terminal-capabilities.ts b/packages/tui/src/terminal-capabilities.ts index 780f062bc..228d843b0 100644 --- a/packages/tui/src/terminal-capabilities.ts +++ b/packages/tui/src/terminal-capabilities.ts @@ -223,6 +223,16 @@ function getForcedImageProtocol(): ImageProtocol | null | undefined { return null; } +/** + * Whether `PI_FORCE_IMAGE_PROTOCOL` pins the image protocol, including its + * `off`/`none` kill switch. A runtime capability probe must not override an + * explicit user choice: a forced protocol is already applied to {@link TERMINAL}, + * and a forced "off" leaves `imageProtocol` null on purpose. + */ +export function isImageProtocolForced(): boolean { + return getForcedImageProtocol() !== undefined; +} + function parseMajorMinorVersion(versionRaw?: string): { major: number; minor: number } | null { if (!versionRaw) return null; const match = /^(\d+)\.(\d+)/u.exec(versionRaw.trim()); diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index c5f8e8bf8..51342ada5 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -28,6 +28,7 @@ import { encodeKittyDeletePlacement, encodeKittyPlacementLine, ImageProtocol, + isImageProtocolForced, isInsideTerminalMultiplexer, parseKittyDirectPlacementLine, setCellDimensions, @@ -1249,7 +1250,6 @@ export class TUI extends Container { #hardwareCursorState: HardwareCursorState | null = null; #hardwareCursorVisibilityKnown = false; #hardwareCursorVisible = false; - #sixelProbePendingDa = false; #sixelProbePendingGraphics = false; #sixelProbeBuffer = ""; #sixelProbeTimeout?: NodeJS.Timeout; @@ -2200,19 +2200,21 @@ export class TUI extends Container { } #querySixelSupport(): void { + // A statically known protocol (Kitty/iTerm2 terminals) or an explicit + // PI_FORCE_IMAGE_PROTOCOL choice — including its `off` kill switch — wins + // over the probe. if (TERMINAL.imageProtocol) return; - // win32 native or WSL under Windows Terminal — both are ConPTY-hosted and - // reach the same WT graphics negotiation. WSL reports process.platform - // "linux", so a bare win32 check silently skips the probe there (#6009). - if (!isConPTYHosted()) return; - if (!Bun.env.WT_SESSION) return; + if (isImageProtocolForced()) return; if (!process.stdin.isTTY || !process.stdout.isTTY) return; this.#clearSixelProbeState(); - this.#sixelProbePendingDa = true; this.#sixelProbePendingGraphics = true; this.#sixelProbeUnsubscribe = this.addInputListener(data => this.#handleSixelProbeInput(data)); - this.terminal.write("\x1b[c"); + // XTSMGRAPHICS item 2 reports the terminal's maximum SIXEL geometry. DA1 + // attribute 4 advertises SIXEL as well, but ProcessTerminal swallows every + // `CSI ? … c` reply for the whole session so a late one cannot leak into the + // composer (#8542): those bytes never reach an input listener, so this probe + // cannot read them. this.terminal.write("\x1b[?2;1;0S"); this.#sixelProbeTimeout = setTimeout(() => { this.#finishSixelProbe(false); @@ -2220,7 +2222,7 @@ export class TUI extends Container { } #handleSixelProbeInput(data: string): InputListenerResult { - if (!this.#sixelProbePendingDa && !this.#sixelProbePendingGraphics) { + if (!this.#sixelProbePendingGraphics) { return undefined; } @@ -2229,47 +2231,24 @@ export class TUI extends Container { let probeOutcome: boolean | null = null; while (this.#sixelProbeBuffer.length > 0) { - const daMatch = this.#sixelProbeBuffer.match(/\x1b\[\?([0-9;]+)c/u); const graphicsMatch = this.#sixelProbeBuffer.match(/\x1b\[\?2;(\d+);([0-9;]+)S/u); + if (!graphicsMatch || graphicsMatch.index === undefined) break; - if (!daMatch && !graphicsMatch) break; + passthrough += this.#sixelProbeBuffer.slice(0, graphicsMatch.index); + this.#sixelProbeBuffer = this.#sixelProbeBuffer.slice(graphicsMatch.index + graphicsMatch[0].length); - const daIndex = daMatch?.index ?? Number.POSITIVE_INFINITY; - const graphicsIndex = graphicsMatch?.index ?? Number.POSITIVE_INFINITY; - const useDa = daIndex <= graphicsIndex; - const match = useDa ? daMatch : graphicsMatch; - if (!match || match.index === undefined) break; - - passthrough += this.#sixelProbeBuffer.slice(0, match.index); - this.#sixelProbeBuffer = this.#sixelProbeBuffer.slice(match.index + match[0].length); - - if (useDa && this.#sixelProbePendingDa) { - this.#sixelProbePendingDa = false; - const attributes = (match[1] ?? "") - .split(";") - .map(value => Number.parseInt(value, 10)) - .filter(value => Number.isFinite(value)); - const hasSixelAttribute = attributes.includes(4); - if (hasSixelAttribute) { - this.#sixelProbePendingGraphics = false; - probeOutcome = true; - } else if (!this.#sixelProbePendingGraphics) { - probeOutcome = false; - } - } else if (!useDa && this.#sixelProbePendingGraphics) { + if (this.#sixelProbePendingGraphics) { this.#sixelProbePendingGraphics = false; - const status = Number.parseInt(match[1] ?? "", 10); - const supportsSixel = !Number.isNaN(status) && status !== 0; - if (supportsSixel) { - this.#sixelProbePendingDa = false; - probeOutcome = true; - } else if (!this.#sixelProbePendingDa) { - probeOutcome = false; - } + // Reply shape `CSI ? 2 ; Ps ; Pv S`: per xterm ctlseqs Ps is the status + // (0 = success, 1..3 = error/failure) and Pv the maximum SIXEL geometry, + // which a terminal without SIXEL reports as zero. + const status = Number.parseInt(graphicsMatch[1] ?? "", 10); + const hasGeometry = (graphicsMatch[2] ?? "").split(";").some(part => Number.parseInt(part, 10) > 0); + probeOutcome = status === 0 && hasGeometry; } } - if (this.#sixelProbePendingDa || this.#sixelProbePendingGraphics) { + if (this.#sixelProbePendingGraphics) { const partialStart = this.#getSixelProbePartialStart(this.#sixelProbeBuffer); if (partialStart >= 0) { passthrough += this.#sixelProbeBuffer.slice(0, partialStart); @@ -2313,7 +2292,6 @@ export class TUI extends Container { this.#sixelProbeUnsubscribe(); this.#sixelProbeUnsubscribe = undefined; } - this.#sixelProbePendingDa = false; this.#sixelProbePendingGraphics = false; this.#sixelProbeBuffer = ""; } diff --git a/packages/tui/test/sixel-probe.test.ts b/packages/tui/test/sixel-probe.test.ts index 45767aeeb..f48062d56 100644 --- a/packages/tui/test/sixel-probe.test.ts +++ b/packages/tui/test/sixel-probe.test.ts @@ -6,11 +6,16 @@ type MutableTerminalInfo = { imageProtocol: ImageProtocol | null; }; +// XTSMGRAPHICS item 2 reply captured from foot 1.27: status 0 (success) plus the +// terminal's maximum SIXEL geometry in pixels. +const SIXEL_SUPPORTED_REPLY = "\x1b[?2;0;1692;432S"; + const terminalInfo = TERMINAL as unknown as MutableTerminalInfo; const originalProtocol = TERMINAL.imageProtocol; const originalWtSession = Bun.env.WT_SESSION; const originalWslDistro = Bun.env.WSL_DISTRO_NAME; const originalWslInterop = Bun.env.WSL_INTEROP; +const originalForcedProtocol = Bun.env.PI_FORCE_IMAGE_PROTOCOL; const stdinIsTtyDescriptor = Object.getOwnPropertyDescriptor(process.stdin, "isTTY"); const stdoutIsTtyDescriptor = Object.getOwnPropertyDescriptor(process.stdout, "isTTY"); @@ -25,110 +30,133 @@ function restoreIsTty( delete (stream as unknown as { isTTY?: boolean }).isTTY; } +function restoreEnv(name: string, value: string | undefined): void { + if (value === undefined) delete Bun.env[name]; + else Bun.env[name] = value; +} + +function startProbe(terminal: VirtualTerminal): TUI { + setTerminalImageProtocol(null); + terminalInfo.imageProtocol = null; + Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true }); + Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true }); + const tui = new TUI(terminal); + tui.start(); + return tui; +} + describe("TUI SIXEL capability probe", () => { afterEach(() => { setTerminalImageProtocol(originalProtocol); terminalInfo.imageProtocol = originalProtocol; - if (originalWtSession === undefined) delete Bun.env.WT_SESSION; - else Bun.env.WT_SESSION = originalWtSession; - if (originalWslDistro === undefined) delete Bun.env.WSL_DISTRO_NAME; - else Bun.env.WSL_DISTRO_NAME = originalWslDistro; - if (originalWslInterop === undefined) delete Bun.env.WSL_INTEROP; - else Bun.env.WSL_INTEROP = originalWslInterop; + restoreEnv("WT_SESSION", originalWtSession); + restoreEnv("WSL_DISTRO_NAME", originalWslDistro); + restoreEnv("WSL_INTEROP", originalWslInterop); + restoreEnv("PI_FORCE_IMAGE_PROTOCOL", originalForcedProtocol); restoreIsTty(process.stdin, stdinIsTtyDescriptor); restoreIsTty(process.stdout, stdoutIsTtyDescriptor); }); - it("enables SIXEL only after positive terminal capability response", () => { - if (process.platform !== "win32") return; - setTerminalImageProtocol(null); - terminalInfo.imageProtocol = null; - Bun.env.WT_SESSION = "test-wt-session"; - Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true }); - Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true }); - + it("enables SIXEL only after a positive capability reply", () => { const terminal = new VirtualTerminal(80, 24); - const tui = new TUI(terminal); - tui.start(); - terminal.sendInput("\x1b[?1;2;4c"); + const tui = startProbe(terminal); + expect(TERMINAL.imageProtocol).toBeNull(); + + terminal.sendInput(SIXEL_SUPPORTED_REPLY); expect(TERMINAL.imageProtocol).toBe(ImageProtocol.Sixel); tui.stop(); }); - it("enables SIXEL when DA and graphics replies are coalesced in one chunk", () => { - if (process.platform !== "win32") return; - setTerminalImageProtocol(null); - terminalInfo.imageProtocol = null; - Bun.env.WT_SESSION = "test-wt-session"; - Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true }); - Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true }); - + it("enables SIXEL on a terminal identified only by COLORTERM (foot)", () => { + // Regression: the probe used to require isConPTYHosted() && WT_SESSION, so a + // SIXEL-capable terminal that exports no identifying variable (foot sets + // TERM=foot and COLORTERM=truecolor only) resolved the `trueColor` + // capability row, kept imageProtocol null, and rendered every image as the + // `[Image: …]` text card. + delete Bun.env.WT_SESSION; + delete Bun.env.WSL_DISTRO_NAME; + delete Bun.env.WSL_INTEROP; const terminal = new VirtualTerminal(80, 24); - const tui = new TUI(terminal); - tui.start(); - terminal.sendInput("\x1b[?1;2;4c\x1b[?2;1;0S"); + const tui = startProbe(terminal); + + terminal.sendInput(SIXEL_SUPPORTED_REPLY); expect(TERMINAL.imageProtocol).toBe(ImageProtocol.Sixel); tui.stop(); }); - it("enables SIXEL when DA reply arrives split across chunks", () => { - if (process.platform !== "win32") return; - setTerminalImageProtocol(null); - terminalInfo.imageProtocol = null; - Bun.env.WT_SESSION = "test-wt-session"; - Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true }); - Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true }); - + it("enables SIXEL when the reply arrives split across chunks", () => { const terminal = new VirtualTerminal(80, 24); - const tui = new TUI(terminal); - tui.start(); - terminal.sendInput("\x1b[?1;2;"); - terminal.sendInput("4c"); + const tui = startProbe(terminal); + + terminal.sendInput("\x1b[?2;0;1692"); + expect(TERMINAL.imageProtocol).toBeNull(); + terminal.sendInput(";432S"); expect(TERMINAL.imageProtocol).toBe(ImageProtocol.Sixel); tui.stop(); }); - it("keeps SIXEL disabled when capability responses are negative", () => { - if (process.platform !== "win32") return; - setTerminalImageProtocol(null); - terminalInfo.imageProtocol = null; - Bun.env.WT_SESSION = "test-wt-session"; - Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true }); - Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true }); - + it("keeps SIXEL disabled when the terminal reports a zero geometry", () => { const terminal = new VirtualTerminal(80, 24); - const tui = new TUI(terminal); - tui.start(); - terminal.sendInput("\x1b[?1;2c"); + const tui = startProbe(terminal); + terminal.sendInput("\x1b[?2;0;0S"); expect(TERMINAL.imageProtocol).toBeNull(); tui.stop(); }); + it("keeps SIXEL disabled when the terminal reports a failure status", () => { + // `CSI ? 2 ; Ps ; Pv S` carries the status in Ps: 0 is success and 1..3 are + // error/failure per xterm ctlseqs, so only Ps = 0 may enable SIXEL. + const terminal = new VirtualTerminal(80, 24); + const tui = startProbe(terminal); + + terminal.sendInput("\x1b[?2;3;1692;432S"); + + expect(TERMINAL.imageProtocol).toBeNull(); + tui.stop(); + }); + + it("keeps SIXEL disabled when the terminal never answers", () => { + const terminal = new VirtualTerminal(80, 24); + const tui = startProbe(terminal); + + terminal.sendInput("hello"); + + expect(TERMINAL.imageProtocol).toBeNull(); + tui.stop(); + }); + it("enables SIXEL under WSL + Windows Terminal (process.platform is linux)", () => { // Regression for #6009: inside WSL, process.platform reports "linux" even - // though the host is Windows Terminal. The probe used to gate on - // process.platform === "win32", so WSL sessions never negotiated SIXEL and - // fell back to the text image card. It now gates on isConPTYHosted(), which - // treats WSL (WSL_DISTRO_NAME/WSL_INTEROP) as a Windows host. + // though the host is Windows Terminal, so a probe gated on + // process.platform === "win32" never negotiated SIXEL there. The probe no + // longer gates on the host at all; WSL is one covered environment of many. if (process.platform !== "linux") return; - setTerminalImageProtocol(null); - terminalInfo.imageProtocol = null; Bun.env.WT_SESSION = "test-wt-session"; Bun.env.WSL_DISTRO_NAME = "Ubuntu"; - Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true }); - Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true }); - const terminal = new VirtualTerminal(80, 24); - const tui = new TUI(terminal); - tui.start(); - terminal.sendInput("\x1b[?1;2;4c"); + const tui = startProbe(terminal); + + terminal.sendInput(SIXEL_SUPPORTED_REPLY); expect(TERMINAL.imageProtocol).toBe(ImageProtocol.Sixel); tui.stop(); }); + + it("respects the PI_FORCE_IMAGE_PROTOCOL kill switch", () => { + // `off` resolves imageProtocol to null on purpose; the probe must not + // re-enable images behind the user's back. + Bun.env.PI_FORCE_IMAGE_PROTOCOL = "off"; + const terminal = new VirtualTerminal(80, 24); + const tui = startProbe(terminal); + + terminal.sendInput(SIXEL_SUPPORTED_REPLY); + + expect(TERMINAL.imageProtocol).toBeNull(); + tui.stop(); + }); });