From 17196d3b10637b5f21337e1d2f8fdef07489cbd4 Mon Sep 17 00:00:00 2001 From: Haider Date: Fri, 21 Aug 2026 04:32:14 +0530 Subject: [PATCH 1/4] fix(tui): resolve startup theme mode from every signal instead of guessing dark MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third attempt at the same defect (#617 → #704 → #736): code rendered in near-white on a light terminal, readable in VS Code but not in the terminal itself. The first two fixes adjusted colour values, which is why neither held. The actual defect is in `app.tsx`, where the mode-resolution chain ended in a hardcoded fallback: const envMode = detectModeFromCOLORFGBG(process.env.COLORFGBG) const mode = envMode === "light" ? "light" : ((await renderer.waitForThemeMode(1000)) ?? "dark") Apple Terminal — the client named in #736's metadata, alongside "macOS Appearance: Light" — sets no `COLORFGBG` and does not reliably answer the OSC 11 background query. Both signals are therefore absent, the chain returns "dark", and a light-background user gets the dark palette no matter how its colours are tuned. That is not a palette bug, so palette fixes could not close it. Changes: - `resolveInitialMode()` encodes the whole chain as one pure function, ordered by how well each signal describes *this terminal window*: COLORFGBG, then the OSC 11 reply, then OS appearance, then dark as a genuine last resort. A dark-profile terminal under a light system theme stays dark. - `detectSystemAppearance()` adds the signal that was missing. macOS sets `AppleInterfaceStyle` to "Dark" in dark mode and leaves it *unset* in light mode, so `defaults` exiting non-zero is the light answer rather than a failure; only ENOENT or a timeout is treated as "unknown". Every report of this bug came from darwin. - The call site now honours a dark `COLORFGBG` too. It previously kept only "light", so a terminal that had already reported a dark background still paid the full one-second OSC timeout before agreeing with it. `detectModeFromCOLORFGBG` carried a comment saying it was "extracted from app.tsx for direct test coverage (#704)" but had no tests at all. It does now. Verified by mutation rather than by the suite going green: restoring the old hardcoded fallback fails the test named for #736, discarding a dark COLORFGBG fails the precedence test, and letting OS appearance outrank the terminal's own background fails two more. Scope note: this closes the colour-mode family. #404 (garbled ASCII logo), #609 (malformed layout) and #737 (unexpected CJK glyphs) were grouped with it during triage, but they are glyph-width and encoding problems rather than colour, and need separate work. #809 (dark text on a dark box) is plausibly the same misdetection, but the report carries no terminal details, so it is referenced rather than closed. Tests: 13 new, 280 tui pass (1 pre-existing failure unrelated, identical on main). Closes #736 Refs #809 Co-Authored-By: Claude Opus 5 (1M context) --- packages/tui/src/app.tsx | 12 ++- packages/tui/src/terminal-detection.ts | 74 ++++++++++++- packages/tui/test/terminal-detection.test.ts | 106 +++++++++++++++++++ 3 files changed, 188 insertions(+), 4 deletions(-) create mode 100644 packages/tui/test/terminal-detection.test.ts diff --git a/packages/tui/src/app.tsx b/packages/tui/src/app.tsx index b7bf10097e..920cb2e224 100644 --- a/packages/tui/src/app.tsx +++ b/packages/tui/src/app.tsx @@ -107,7 +107,7 @@ import { win32DisableProcessedInput, win32FlushInputBuffer } from "./terminal-wi import { destroyRenderer } from "./util/renderer" import { cliErrorMessage, errorFormat } from "./util/error" // altimate_change start — fix: pure helper extracted to terminal-detection for test coverage (#704) -import { detectModeFromCOLORFGBG } from "./terminal-detection" +import { detectModeFromCOLORFGBG, detectSystemAppearance, resolveInitialMode } from "./terminal-detection" // altimate_change end const appGlobalBindingCommands = [ @@ -265,9 +265,15 @@ export const run = Effect.fn("Tui.run")(function* (input: TuiInput) { yield* Effect.tryPromise(async () => { // Prewarm palette before ThemeProvider mounts so `system` theme avoids a first-paint fallback flash. void renderer.getPalette({ size: 16 }).catch(() => undefined) - // altimate_change start — fix: check COLORFGBG eagerly to avoid 1s startup delay on terminals without OSC 11 (#704) + // altimate_change start — fix: resolve the startup mode from every available + // signal instead of falling through to "dark" (#617 → #704 → #736). + // COLORFGBG is free, so it short-circuits the OSC wait in both directions. + // Only when the terminal answers neither do we ask the OS, which is the + // case Apple Terminal users kept hitting. const envMode = detectModeFromCOLORFGBG(process.env.COLORFGBG) - const mode = envMode === "light" ? "light" : ((await renderer.waitForThemeMode(1000)) ?? "dark") + const oscMode = envMode ? null : ((await renderer.waitForThemeMode(1000)) ?? null) + const appearance = envMode || oscMode ? null : await detectSystemAppearance() + const mode = resolveInitialMode({ colorfgbg: process.env.COLORFGBG, osc: oscMode, appearance }) // altimate_change end if (renderer.isDestroyed) return diff --git a/packages/tui/src/terminal-detection.ts b/packages/tui/src/terminal-detection.ts index 9952ae67bf..f21a24b89f 100644 --- a/packages/tui/src/terminal-detection.ts +++ b/packages/tui/src/terminal-detection.ts @@ -1,3 +1,5 @@ +import { execFile } from "node:child_process" + // altimate_change start — fix: pure-TS helper extracted from app.tsx for direct test coverage (#704) /** * Detect terminal background mode from the COLORFGBG env var. @@ -20,4 +22,74 @@ export function detectModeFromCOLORFGBG(value: string | undefined): "dark" | "li if (!Number.isInteger(bg) || bg < 0 || bg > 15) return null return bg === 7 || bg === 15 ? "light" : "dark" } -// altimate_change end + +/** Signals available when choosing a startup theme mode, cheapest first. */ +export interface ModeSignals { + /** `COLORFGBG` env var, set by rxvt/urxvt/konsole and some others. */ + colorfgbg?: string | undefined + /** Answer to the OSC 11 background query, when the terminal replies. */ + osc?: "dark" | "light" | null | undefined + /** OS-level appearance, where the platform exposes one. */ + appearance?: "dark" | "light" | null | undefined +} + +/** + * Choose the startup theme mode from whatever signals are available. + * + * Ordered by how well each signal describes *this terminal window*: an + * explicit background beats an OS-wide preference, because a user may run a + * dark-profile terminal under a light system theme. + * + * The final fallback is the reason #617, #704 and #736 kept recurring. Apple + * Terminal sets no COLORFGBG and does not reliably answer OSC 11, so every + * light-background user on it fell through to `"dark"` and got pale text on a + * pale background. Two earlier fixes adjusted colours; the defect was that the + * chain ended in a guess with no way to be right. + */ +export function resolveInitialMode(signals: ModeSignals): "dark" | "light" { + const fromEnv = detectModeFromCOLORFGBG(signals.colorfgbg) + // Honour both directions. The previous call site kept only "light", so a + // terminal that reported a dark background still paid for the OSC timeout. + if (fromEnv) return fromEnv + if (signals.osc) return signals.osc + if (signals.appearance) return signals.appearance + return "dark" +} + +/** + * OS appearance, for platforms that expose one. Returns null when unknown. + * + * macOS only. `AppleInterfaceStyle` is set to "Dark" in dark mode and is + * *absent* in light mode, so a non-zero exit is the light answer, not an error. + * This is the signal that was missing: every report of this bug came from + * darwin, on a terminal that answers neither of the cheaper probes. + */ +export function detectSystemAppearance( + platform: NodeJS.Platform = process.platform, + timeoutMs = 400, +): Promise<"dark" | "light" | null> { + if (platform !== "darwin") return Promise.resolve(null) + + return new Promise((resolve) => { + try { + execFile( + "defaults", + ["read", "-g", "AppleInterfaceStyle"], + { timeout: timeoutMs, encoding: "utf8" }, + (error, stdout) => { + if (error) { + // `defaults` exits non-zero when the key is unset, which is exactly + // how macOS represents light mode. Distinguish that from a real + // failure: a timeout or a missing binary tells us nothing. + const failed = (error as NodeJS.ErrnoException).code === "ENOENT" || (error as any).killed === true + resolve(failed ? null : "light") + return + } + resolve(stdout.trim().toLowerCase() === "dark" ? "dark" : "light") + }, + ) + } catch { + resolve(null) + } + }) +} diff --git a/packages/tui/test/terminal-detection.test.ts b/packages/tui/test/terminal-detection.test.ts new file mode 100644 index 0000000000..78d7f3bc0d --- /dev/null +++ b/packages/tui/test/terminal-detection.test.ts @@ -0,0 +1,106 @@ +/** + * Startup theme-mode detection. + * + * This is the third attempt at the same defect: #617 → #704 → #736, each + * reporting code rendered in near-white on a light terminal. The first two + * fixes adjusted colour values. They did not hold, because the actual defect is + * that the mode-resolution chain ended in a hardcoded `"dark"` — so on a + * terminal that answers neither COLORFGBG nor OSC 11 (Apple Terminal, which is + * what #736's metadata reports) a light-background user could never be + * detected correctly, whatever the palette said. + * + * These tests pin the chain itself rather than any particular colour. + */ +import { describe, expect, test } from "bun:test" +import { detectModeFromCOLORFGBG, detectSystemAppearance, resolveInitialMode } from "../src/terminal-detection" + +describe("detectModeFromCOLORFGBG", () => { + test("reads the background index from fg;bg", () => { + expect(detectModeFromCOLORFGBG("0;15")).toBe("light") + expect(detectModeFromCOLORFGBG("15;0")).toBe("dark") + }) + + test("reads the rxvt fg;default;bg form", () => { + expect(detectModeFromCOLORFGBG("0;default;15")).toBe("light") + expect(detectModeFromCOLORFGBG("15;default;0")).toBe("dark") + }) + + test("treats only canonically light indices as light", () => { + // 7 (light-gray) and 15 (bright-white) are light; other bright indices are + // dark by luminance and must not be mistaken for light backgrounds. + expect(detectModeFromCOLORFGBG("0;7")).toBe("light") + for (const bg of [9, 12, 13]) { + expect(detectModeFromCOLORFGBG(`0;${bg}`)).toBe("dark") + } + }) + + test("returns null when absent, malformed, or out of range", () => { + expect(detectModeFromCOLORFGBG(undefined)).toBeNull() + expect(detectModeFromCOLORFGBG("")).toBeNull() + expect(detectModeFromCOLORFGBG("0;default")).toBeNull() + expect(detectModeFromCOLORFGBG("0;99")).toBeNull() + expect(detectModeFromCOLORFGBG("0;-1")).toBeNull() + }) +}) + +describe("resolveInitialMode", () => { + test("COLORFGBG wins — it describes this window and costs nothing", () => { + expect(resolveInitialMode({ colorfgbg: "0;15", osc: "dark", appearance: "dark" })).toBe("light") + expect(resolveInitialMode({ colorfgbg: "15;0", osc: "light", appearance: "light" })).toBe("dark") + }) + + test("honours a dark COLORFGBG instead of discarding it", () => { + // The previous call site kept only "light", so a terminal reporting a dark + // background still paid the full OSC timeout before agreeing. + expect(resolveInitialMode({ colorfgbg: "15;0" })).toBe("dark") + }) + + test("falls back to the OSC 11 answer when COLORFGBG is absent", () => { + expect(resolveInitialMode({ osc: "light", appearance: "dark" })).toBe("light") + expect(resolveInitialMode({ osc: "dark", appearance: "light" })).toBe("dark") + }) + + test("prefers the terminal's own background over the OS preference", () => { + // A dark-profile terminal under a light system theme must stay dark. + expect(resolveInitialMode({ osc: "dark", appearance: "light" })).toBe("dark") + }) + + test("uses OS appearance only when the terminal says nothing", () => { + expect(resolveInitialMode({ appearance: "light" })).toBe("light") + expect(resolveInitialMode({ appearance: "dark" })).toBe("dark") + }) + + test("REGRESSION #736: light Apple Terminal with no COLORFGBG and no OSC reply", () => { + // Exactly the reported environment: darwin, macOS appearance Light, + // TERM_PROGRAM=Apple_Terminal. No COLORFGBG is set and OSC 11 goes + // unanswered, so both cheap signals are absent. Before this change the + // chain returned "dark" and rendered pale code on a pale background. + expect(resolveInitialMode({ colorfgbg: undefined, osc: null, appearance: "light" })).toBe("light") + }) + + test("still defaults to dark when nothing is known", () => { + expect(resolveInitialMode({})).toBe("dark") + expect(resolveInitialMode({ colorfgbg: undefined, osc: null, appearance: null })).toBe("dark") + }) +}) + +describe("detectSystemAppearance", () => { + test("returns null off macOS without spawning anything", async () => { + expect(await detectSystemAppearance("linux")).toBeNull() + expect(await detectSystemAppearance("win32")).toBeNull() + }) + + test("on macOS reports a usable answer", async () => { + // `AppleInterfaceStyle` is absent in light mode, so `defaults` exits + // non-zero — that is the light answer, not a failure. The distinction is + // the whole point of the probe, so assert we never turn it into null. + const result = await detectSystemAppearance("darwin") + + if (process.platform === "darwin") { + expect(result === "dark" || result === "light").toBe(true) + } else { + // Off-platform the binary is missing; that genuinely tells us nothing. + expect(result).toBeNull() + } + }) +}) From b01d99a36419e07b525830cd4c3511909f82c2cc Mon Sep 17 00:00:00 2001 From: Haider Date: Fri, 21 Aug 2026 05:48:26 +0530 Subject: [PATCH 2/4] fix(tui): make terminal mode detection sound, and stop the direct-run fallback assuming dark MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reworked after a codex audit of the whole colour-legibility class, which found the first attempt was aimed at the less important of two layers and that several of its tests proved less than their names claimed. ## Direct-run renderer (the layer the audit ranked higher) `resolveRunTheme` returned a hardcoded dark `RUN_THEME_FALLBACK` on both failure exits, so a light terminal whose palette query failed got dark panels. Worse, that fallback's `text` prefers the terminal's *own* default foreground, which on a light terminal resolves to black — a black foreground over a hardcoded `#0f172a` panel is literally dark text in a dark box, the symptom reported in #809. The fallback is now built per mode and memoized, and both exits resolve a mode first. The dark instance is still the same object, so callers comparing it by identity are unaffected. ## Startup detection - Precedence corrected. OSC 11 describes *this* window right now; `COLORFGBG` is inherited and survives ssh, tmux, sudo and profile changes. The previous ordering let a stale env var override a live answer. COLORFGBG now only shortens the OSC wait (250ms instead of 1s), which keeps #704's startup win without trading away correctness. - The appearance probe no longer reports "light" for every failure. macOS leaves `AppleInterfaceStyle` unset in light mode and `defaults` says so explicitly; that diagnostic is the light answer, while EACCES, EMFILE, ENOENT, a signal or a timeout mean unknown. Guessing light on those produces the inverse of the bug being fixed. - It invokes `/usr/bin/defaults`, so a different `defaults` earlier on PATH cannot answer a question about macOS appearance. - It does not run over ssh, where the appearance belongs to the remote host rather than the terminal the user is looking at, nor in CI. ## Tests The audit named six tests that overclaimed. `execFile` is now injectable and the probe tests use a spy, so "does not spawn" is asserted rather than assumed, and every failure branch is driven directly. The regression test is renamed for the shape it actually covers instead of implying end-to-end coverage it does not have. Seven mutants were confirmed to fail: old precedence, missing ssh guard, missing CI guard, relative `defaults`, any-failure-means-light, always-dark direct-run fallback, and a light fallback whose panel is still dark. ## Scope Claims only what the code supports. The audit found #617 was missing Markdown `fg` and code-block background (fixed separately in 5ae5b79fcb), #704 bundled three changes with no way to attribute the fix, and #404/#609/#737/#116 are glyph-width, encoding or layout problems rather than colour. #736 is the best-supported colour-mode case but remains conditional, so it is referenced, not closed. Tests: 17 detection, 9 direct-run theme, 284 tui, 187 cli/run. Typecheck clean. Refs #736 Refs #809 Co-Authored-By: Claude Opus 5 (1M context) --- packages/opencode/src/cli/cmd/run/theme.ts | 68 +++++++++-- packages/opencode/test/cli/run/theme.test.ts | 24 +++- packages/tui/package.json | 31 ++--- packages/tui/src/app.tsx | 8 +- packages/tui/src/terminal-detection.ts | 49 ++++++-- packages/tui/test/terminal-detection.test.ts | 122 +++++++++++++------ 6 files changed, 221 insertions(+), 81 deletions(-) diff --git a/packages/opencode/src/cli/cmd/run/theme.ts b/packages/opencode/src/cli/cmd/run/theme.ts index e4fb315426..aaa51fd707 100644 --- a/packages/opencode/src/cli/cmd/run/theme.ts +++ b/packages/opencode/src/cli/cmd/run/theme.ts @@ -8,6 +8,8 @@ import { RGBA, SyntaxStyle, type CliRenderer, type ColorInput, type TerminalColors } from "@opentui/core" import type { TuiThemeCurrent } from "@opencode-ai/plugin/tui" import type { EntryKind } from "./types" +// Shared with the full-screen TUI so both renderers agree on how a mode is chosen. +import { resolveInitialMode } from "@opencode-ai/tui/terminal-detection" type Tone = { body: ColorInput @@ -581,14 +583,25 @@ function map( } } -const seed = { - highlight: RGBA.fromIndex(6, rgba("#38bdf8")), - muted: RGBA.fromIndex(8, rgba("#64748b")), - text: RGBA.defaultForeground(rgba("#f8fafc")), - panel: rgba("#0f172a"), - success: RGBA.fromIndex(2, rgba("#22c55e")), - warning: RGBA.fromIndex(3, rgba("#f59e0b")), - error: RGBA.fromIndex(1, rgba("#ef4444")), +/** + * Seed colours for the direct-run fallback theme. + * + * `text` deliberately prefers the terminal's own default foreground, which on a + * light terminal resolves to black. The panel therefore has to follow the + * detected mode: a hardcoded dark panel plus a black resolved foreground is + * literally dark text in a dark box, the symptom reported in #809. + */ +function fallbackSeed(mode: "dark" | "light") { + const dark = mode === "dark" + return { + highlight: RGBA.fromIndex(6, rgba("#38bdf8")), + muted: RGBA.fromIndex(8, rgba(dark ? "#64748b" : "#52606d")), + text: RGBA.defaultForeground(rgba(dark ? "#f8fafc" : "#0f172a")), + panel: rgba(dark ? "#0f172a" : "#eef2f7"), + success: RGBA.fromIndex(2, rgba(dark ? "#22c55e" : "#15803d")), + warning: RGBA.fromIndex(3, rgba(dark ? "#f59e0b" : "#b45309")), + error: RGBA.fromIndex(1, rgba(dark ? "#ef4444" : "#b91c1c")), + } } function tone(body: ColorInput, start?: ColorInput): Tone { @@ -602,7 +615,19 @@ const fallbackSplashIndexed = Array.from({ length: 256 }, (_, index) => RGBA.fro const fallbackSplashLeft = RGBA.fromIndex(67) const fallbackSplashRight = RGBA.fromIndex(110) -export const RUN_THEME_FALLBACK: RunTheme = { +const fallbackByMode = new Map<"dark" | "light", RunTheme>() + +/** + * Direct-run fallback theme for a known terminal mode. + * + * Memoized per mode: the theme is large, this sits on a failure path that can + * be hit repeatedly, and callers compare the dark instance by identity. + */ +export function runThemeFallback(mode: "dark" | "light"): RunTheme { + const cached = fallbackByMode.get(mode) + if (cached) return cached + const seed = fallbackSeed(mode) + const theme: RunTheme = { background: RGBA.fromValues(0, 0, 0, 0), footer: { highlight: seed.highlight, @@ -651,6 +676,27 @@ export const RUN_THEME_FALLBACK: RunTheme = { diffAddedLineNumberBg: alpha(seed.success, 0.12), diffRemovedLineNumberBg: alpha(seed.error, 0.12), }, + } + fallbackByMode.set(mode, theme) + return theme +} + +/** Dark instance, kept as the default for callers with no mode to hand. */ +export const RUN_THEME_FALLBACK: RunTheme = runThemeFallback("dark") + +/** + * Best guess at terminal mode when the palette query gives us nothing. + * + * Both exits below used to return the dark fallback unconditionally, so a light + * terminal whose palette query failed got dark panels regardless. This is the + * higher-traffic sibling of the startup detection in packages/tui — it drives + * the direct-run and scrollback renderer. + */ +function fallbackMode(renderer: CliRenderer): "dark" | "light" { + return resolveInitialMode({ + colorfgbg: process.env["COLORFGBG"], + osc: renderer.themeMode ?? null, + }) } export async function resolveRunTheme(renderer: CliRenderer): Promise { @@ -660,7 +706,7 @@ export async function resolveRunTheme(renderer: CliRenderer): Promise }) const bg = colors.defaultBackground ?? colors.palette[0] if (!bg) { - return RUN_THEME_FALLBACK + return runThemeFallback(fallbackMode(renderer)) } // Palette-only terminal reloads can leave renderer.themeMode stale, but @@ -685,6 +731,6 @@ export async function resolveRunTheme(renderer: CliRenderer): Promise shared.generateSubtleSyntax(syntaxTheme), ) } catch { - return RUN_THEME_FALLBACK + return runThemeFallback(fallbackMode(renderer)) } } diff --git a/packages/opencode/test/cli/run/theme.test.ts b/packages/opencode/test/cli/run/theme.test.ts index 4102dea1c9..13867346ee 100644 --- a/packages/opencode/test/cli/run/theme.test.ts +++ b/packages/opencode/test/cli/run/theme.test.ts @@ -1,6 +1,7 @@ import { expect, test } from "bun:test" -import { RGBA, type CliRenderer, type TerminalColors } from "@opentui/core" -import { RUN_THEME_FALLBACK, generateSystem, resolveRunTheme, resolveTheme } from "@/cli/cmd/run/theme" +import { type ColorInput, RGBA, type CliRenderer, type TerminalColors } from "@opentui/core" +import { RUN_THEME_FALLBACK, + runThemeFallback, generateSystem, resolveRunTheme, resolveTheme } from "@/cli/cmd/run/theme" const palette = ["#15161e", "#f7768e", "#9ece6a", "#e0af68", "#7aa2f7", "#bb9af7", "#7dcfff", "#c0caf5"] as const @@ -62,6 +63,25 @@ test("falls back when palette lookup fails", async () => { expect(await resolveRunTheme(renderer({ fail: true }))).toBe(RUN_THEME_FALLBACK) }) +test("the fallback follows a light terminal instead of always going dark", async () => { + // The direct-run fallback used to be unconditionally dark while its `text` + // preferred the terminal's own default foreground. On a light terminal that + // foreground resolves to black, so a dark panel produced black-on-black — + // the class of symptom reported in #809. + const light = await resolveRunTheme(renderer({ fail: true, themeMode: "light" })) + + expect(light).not.toBe(RUN_THEME_FALLBACK) + expect(light).toBe(runThemeFallback("light")) + + // The panel must actually be light, or the fix is cosmetic. + const sum = (color: ColorInput) => (color as RGBA).toInts().slice(0, 3).reduce((a, b) => a + b, 0) + expect(sum(light.block.diffContextBg)).toBeGreaterThan(sum(RUN_THEME_FALLBACK.block.diffContextBg)) +}) + +test("a dark terminal still gets the dark fallback", async () => { + expect(await resolveRunTheme(renderer({ fail: true, themeMode: "dark" }))).toBe(runThemeFallback("dark")) +}) + test("returns syntax styles and indexed splash colors", async () => { const theme = await resolveRunTheme(renderer({ themeMode: "dark" })) diff --git a/packages/tui/package.json b/packages/tui/package.json index ad6cb08877..d3fc71d625 100644 --- a/packages/tui/package.json +++ b/packages/tui/package.json @@ -11,9 +11,14 @@ }, "exports": { ".": "./src/index.tsx", + "./attention": "./src/attention.ts", "./builtins": "./src/feature-plugins/builtins.ts", + "./component/spinner": "./src/component/spinner.tsx", "./config": "./src/config/index.tsx", + "./config/keybind": "./src/config/keybind.ts", "./context/args": "./src/context/args.tsx", + "./context/clipboard": "./src/context/clipboard.tsx", + "./context/editor": "./src/context/editor.ts", "./context/epilogue": "./src/context/epilogue.tsx", "./context/exit": "./src/context/exit.tsx", "./context/kv": "./src/context/kv.tsx", @@ -23,30 +28,26 @@ "./context/sdk": "./src/context/sdk.tsx", "./context/sync": "./src/context/sync.tsx", "./context/theme": "./src/context/theme.tsx", - "./context/editor": "./src/context/editor.ts", - "./context/clipboard": "./src/context/clipboard.tsx", - "./attention": "./src/attention.ts", "./editor": "./src/editor.ts", "./editor-zed": "./src/editor-zed.ts", - "./runtime": "./src/runtime.tsx", - "./terminal-win32": "./src/terminal-win32.ts", - "./config/keybind": "./src/config/keybind.ts", "./keymap": "./src/keymap.tsx", - "./prompt/display": "./src/prompt/display.ts", + "./logo": "./src/logo.ts", + "./parsers-config": "./src/parsers-config.ts", + "./plugin/command-shim": "./src/plugin/command-shim.ts", "./plugin/runtime": "./src/plugin/runtime.tsx", "./plugin/slots": "./src/plugin/slots.tsx", - "./plugin/command-shim": "./src/plugin/command-shim.ts", - "./parsers-config": "./src/parsers-config.ts", + "./prompt/display": "./src/prompt/display.ts", + "./runtime": "./src/runtime.tsx", + "./terminal-detection": "./src/terminal-detection.ts", + "./terminal-win32": "./src/terminal-win32.ts", + "./ui/dialog": "./src/ui/dialog.tsx", + "./ui/spinner": "./src/ui/spinner.ts", + "./ui/toast": "./src/ui/toast.tsx", "./util/error": "./src/util/error.ts", "./util/locale": "./src/util/locale.ts", "./util/persistence": "./src/util/persistence.ts", "./util/record": "./src/util/record.ts", - "./util/transcript": "./src/util/transcript.ts", - "./logo": "./src/logo.ts", - "./ui/dialog": "./src/ui/dialog.tsx", - "./ui/spinner": "./src/ui/spinner.ts", - "./ui/toast": "./src/ui/toast.tsx", - "./component/spinner": "./src/component/spinner.tsx" + "./util/transcript": "./src/util/transcript.ts" }, "dependencies": { "@opencode-ai/core": "workspace:*", diff --git a/packages/tui/src/app.tsx b/packages/tui/src/app.tsx index 920cb2e224..275700825a 100644 --- a/packages/tui/src/app.tsx +++ b/packages/tui/src/app.tsx @@ -271,8 +271,12 @@ export const run = Effect.fn("Tui.run")(function* (input: TuiInput) { // Only when the terminal answers neither do we ask the OS, which is the // case Apple Terminal users kept hitting. const envMode = detectModeFromCOLORFGBG(process.env.COLORFGBG) - const oscMode = envMode ? null : ((await renderer.waitForThemeMode(1000)) ?? null) - const appearance = envMode || oscMode ? null : await detectSystemAppearance() + // Always ask the terminal — it is the only signal describing this window + // now. COLORFGBG only buys a shorter wait: with a usable hint in hand we + // can stop waiting sooner, which keeps #704's startup win without + // letting a stale env var override a live answer. + const oscMode = (await renderer.waitForThemeMode(envMode ? 250 : 1000)) ?? null + const appearance = oscMode || envMode ? null : await detectSystemAppearance() const mode = resolveInitialMode({ colorfgbg: process.env.COLORFGBG, osc: oscMode, appearance }) // altimate_change end if (renderer.isDestroyed) return diff --git a/packages/tui/src/terminal-detection.ts b/packages/tui/src/terminal-detection.ts index f21a24b89f..993f99728c 100644 --- a/packages/tui/src/terminal-detection.ts +++ b/packages/tui/src/terminal-detection.ts @@ -47,11 +47,13 @@ export interface ModeSignals { * chain ended in a guess with no way to be right. */ export function resolveInitialMode(signals: ModeSignals): "dark" | "light" { + // OSC 11 first: it reports the background of *this* window, right now. + // COLORFGBG is inherited, so it survives ssh, tmux, sudo and profile changes + // and can describe a terminal the user is no longer looking at. Letting it + // outrank a live answer trades correctness for a little startup latency. + if (signals.osc) return signals.osc const fromEnv = detectModeFromCOLORFGBG(signals.colorfgbg) - // Honour both directions. The previous call site kept only "light", so a - // terminal that reported a dark background still paid for the OSC timeout. if (fromEnv) return fromEnv - if (signals.osc) return signals.osc if (signals.appearance) return signals.appearance return "dark" } @@ -64,28 +66,51 @@ export function resolveInitialMode(signals: ModeSignals): "dark" | "light" { * This is the signal that was missing: every report of this bug came from * darwin, on a terminal that answers neither of the cheaper probes. */ +/** Minimal shape of `child_process.execFile`, injectable so tests can drive every branch. */ +export type ExecFileLike = ( + file: string, + args: string[], + options: { timeout: number; encoding: "utf8" }, + callback: (error: unknown, stdout: string) => void, +) => unknown + export function detectSystemAppearance( platform: NodeJS.Platform = process.platform, timeoutMs = 400, + env: NodeJS.ProcessEnv = process.env, + exec: ExecFileLike = execFile as unknown as ExecFileLike, ): Promise<"dark" | "light" | null> { if (platform !== "darwin") return Promise.resolve(null) + // Over ssh the OS appearance belongs to the remote machine, not to the + // terminal the user is actually looking at. Answering from it would be + // confidently wrong, which is worse than admitting we do not know. + if (env["SSH_CONNECTION"] || env["SSH_TTY"] || env["SSH_CLIENT"]) return Promise.resolve(null) + // CI has no human looking at a terminal; skip the spawn entirely. + if (env["CI"]) return Promise.resolve(null) + return new Promise((resolve) => { try { - execFile( - "defaults", + exec( + // Absolute path: a different `defaults` earlier on PATH must not get to + // answer a question about macOS appearance. + "/usr/bin/defaults", ["read", "-g", "AppleInterfaceStyle"], { timeout: timeoutMs, encoding: "utf8" }, (error, stdout) => { - if (error) { - // `defaults` exits non-zero when the key is unset, which is exactly - // how macOS represents light mode. Distinguish that from a real - // failure: a timeout or a missing binary tells us nothing. - const failed = (error as NodeJS.ErrnoException).code === "ENOENT" || (error as any).killed === true - resolve(failed ? null : "light") + if (!error) { + resolve(stdout.trim().toLowerCase() === "dark" ? "dark" : "light") return } - resolve(stdout.trim().toLowerCase() === "dark" ? "dark" : "light") + // macOS leaves AppleInterfaceStyle unset in light mode, so `defaults` + // exits 1 with a "does not exist" diagnostic. That is the light + // answer. Everything else — ENOENT, EACCES, sandbox denial, a signal, + // a spawn failure, a timeout — tells us nothing, and must not be + // silently reported as light. + const err = error as NodeJS.ErrnoException & { killed?: boolean; status?: number | null; stderr?: string } + const notFound = /does not exist/i.test(String(err.stderr ?? err.message ?? "")) + const clean = err.code === undefined && err.killed !== true + resolve(notFound || clean ? "light" : null) }, ) } catch { diff --git a/packages/tui/test/terminal-detection.test.ts b/packages/tui/test/terminal-detection.test.ts index 78d7f3bc0d..83a5fbc915 100644 --- a/packages/tui/test/terminal-detection.test.ts +++ b/packages/tui/test/terminal-detection.test.ts @@ -44,37 +44,30 @@ describe("detectModeFromCOLORFGBG", () => { }) describe("resolveInitialMode", () => { - test("COLORFGBG wins — it describes this window and costs nothing", () => { - expect(resolveInitialMode({ colorfgbg: "0;15", osc: "dark", appearance: "dark" })).toBe("light") - expect(resolveInitialMode({ colorfgbg: "15;0", osc: "light", appearance: "light" })).toBe("dark") + test("the terminal's own OSC answer outranks the inherited env var", () => { + // COLORFGBG survives ssh, tmux, sudo and profile switches, so it can + // describe a terminal the user is no longer looking at. A live OSC reply + // cannot. + expect(resolveInitialMode({ colorfgbg: "15;0", osc: "light" })).toBe("light") + expect(resolveInitialMode({ colorfgbg: "0;15", osc: "dark" })).toBe("dark") }) - test("honours a dark COLORFGBG instead of discarding it", () => { - // The previous call site kept only "light", so a terminal reporting a dark - // background still paid the full OSC timeout before agreeing. - expect(resolveInitialMode({ colorfgbg: "15;0" })).toBe("dark") + test("uses COLORFGBG in both directions when the terminal stays silent", () => { + expect(resolveInitialMode({ colorfgbg: "0;15", osc: null })).toBe("light") + // Previously a dark reading here was discarded and the caller waited anyway. + expect(resolveInitialMode({ colorfgbg: "15;0", osc: null })).toBe("dark") }) - test("falls back to the OSC 11 answer when COLORFGBG is absent", () => { - expect(resolveInitialMode({ osc: "light", appearance: "dark" })).toBe("light") + test("OS appearance is consulted only when the terminal says nothing at all", () => { expect(resolveInitialMode({ osc: "dark", appearance: "light" })).toBe("dark") - }) - - test("prefers the terminal's own background over the OS preference", () => { - // A dark-profile terminal under a light system theme must stay dark. - expect(resolveInitialMode({ osc: "dark", appearance: "light" })).toBe("dark") - }) - - test("uses OS appearance only when the terminal says nothing", () => { + expect(resolveInitialMode({ colorfgbg: "15;0", appearance: "light" })).toBe("dark") expect(resolveInitialMode({ appearance: "light" })).toBe("light") - expect(resolveInitialMode({ appearance: "dark" })).toBe("dark") }) - test("REGRESSION #736: light Apple Terminal with no COLORFGBG and no OSC reply", () => { - // Exactly the reported environment: darwin, macOS appearance Light, - // TERM_PROGRAM=Apple_Terminal. No COLORFGBG is set and OSC 11 goes - // unanswered, so both cheap signals are absent. Before this change the - // chain returned "dark" and rendered pale code on a pale background. + test("#736 shape: no COLORFGBG, no OSC reply, light macOS appearance", () => { + // Named for the shape it covers, not for the whole bug: this asserts the + // resolver's contract only. Whether app.tsx supplies these signals is a + // separate question, exercised by the probe tests below. expect(resolveInitialMode({ colorfgbg: undefined, osc: null, appearance: "light" })).toBe("light") }) @@ -85,22 +78,73 @@ describe("resolveInitialMode", () => { }) describe("detectSystemAppearance", () => { - test("returns null off macOS without spawning anything", async () => { - expect(await detectSystemAppearance("linux")).toBeNull() - expect(await detectSystemAppearance("win32")).toBeNull() - }) - - test("on macOS reports a usable answer", async () => { - // `AppleInterfaceStyle` is absent in light mode, so `defaults` exits - // non-zero — that is the light answer, not a failure. The distinction is - // the whole point of the probe, so assert we never turn it into null. - const result = await detectSystemAppearance("darwin") - - if (process.platform === "darwin") { - expect(result === "dark" || result === "light").toBe(true) - } else { - // Off-platform the binary is missing; that genuinely tells us nothing. - expect(result).toBeNull() + /** Records what was spawned so "does not spawn" can be asserted, not assumed. */ + function spy(behaviour: (cb: (err: unknown, stdout: string) => void) => void) { + const spawned: string[] = [] + const exec = (file: string, _a: string[], _o: unknown, cb: (e: unknown, s: string) => void) => { + spawned.push(file) + behaviour(cb) + return undefined } + return { spawned, exec: exec as any } + } + + test("does not spawn anything off macOS", async () => { + const { spawned, exec } = spy((cb) => cb(null, "Dark")) + + expect(await detectSystemAppearance("linux", 400, {}, exec)).toBeNull() + expect(await detectSystemAppearance("win32", 400, {}, exec)).toBeNull() + // The point of the test is the absence of the spawn, so assert it directly. + expect(spawned).toEqual([]) + }) + + test("does not spawn over ssh — that appearance belongs to the remote host", async () => { + const { spawned, exec } = spy((cb) => cb(null, "Dark")) + + expect(await detectSystemAppearance("darwin", 400, { SSH_CONNECTION: "1.2.3.4 22" }, exec)).toBeNull() + expect(spawned).toEqual([]) + }) + + test("does not spawn in CI", async () => { + const { spawned, exec } = spy((cb) => cb(null, "Dark")) + + expect(await detectSystemAppearance("darwin", 400, { CI: "true" }, exec)).toBeNull() + expect(spawned).toEqual([]) + }) + + test("reads Dark from stdout", async () => { + const { exec } = spy((cb) => cb(null, "Dark\n")) + expect(await detectSystemAppearance("darwin", 400, {}, exec)).toBe("dark") + }) + + test("treats the documented missing-key failure as light", async () => { + // macOS leaves the key unset in light mode; `defaults` exits 1 saying so. + const { exec } = spy((cb) => + cb(Object.assign(new Error("x"), { stderr: "The domain/default pair of (kCFPreferencesAnyApplication, AppleInterfaceStyle) does not exist" }), ""), + ) + expect(await detectSystemAppearance("darwin", 400, {}, exec)).toBe("light") + }) + + test("does NOT call a permission or spawn failure light", async () => { + // The distinction codex flagged: only the missing-key diagnostic means + // light. Everything else is unknown, and guessing light on a dark terminal + // produces the inverse of the bug being fixed. + for (const code of ["EACCES", "EMFILE", "ENOENT", "ENOMEM"]) { + const { exec } = spy((cb) => cb(Object.assign(new Error("boom"), { code }), "")) + expect(await detectSystemAppearance("darwin", 400, {}, exec)).toBeNull() + } + }) + + test("does NOT call a timeout light", async () => { + const { exec } = spy((cb) => cb(Object.assign(new Error("timed out"), { killed: true, code: null }), "")) + expect(await detectSystemAppearance("darwin", 400, {}, exec)).toBeNull() + }) + + test("invokes an absolute path, not whatever PATH resolves", async () => { + // A stray executable named `defaults` must not get to answer this. + const { spawned, exec } = spy((cb) => cb(null, "Dark")) + await detectSystemAppearance("darwin", 400, {}, exec) + + expect(spawned).toEqual(["/usr/bin/defaults"]) }) }) From e4521121f7a53e2c4d6737332e23a8f0bba2a7b5 Mon Sep 17 00:00:00 2001 From: Haider Date: Thu, 27 Aug 2026 04:03:03 +0530 Subject: [PATCH 3/4] fix(tui): wrap the direct-run theme changes in altimate_change markers Marker Guard failed on #1152: theme.ts is an upstream-shared file, so custom code there must be fenced to survive an upstream merge overwriting it. The mode-aware fallback added in this branch was unmarked. Six regions are now fenced: the shared-resolver import, the per-mode seed, the memoized per-mode fallback theme, the mode probe, and both failure exits in resolveRunTheme. Verified with the same command CI runs: bun run script/upstream/analyze.ts --markers --base origin/main --strict Co-Authored-By: Claude Opus 5 (1M context) --- packages/opencode/src/cli/cmd/run/theme.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/opencode/src/cli/cmd/run/theme.ts b/packages/opencode/src/cli/cmd/run/theme.ts index aaa51fd707..0a532e6ee4 100644 --- a/packages/opencode/src/cli/cmd/run/theme.ts +++ b/packages/opencode/src/cli/cmd/run/theme.ts @@ -8,8 +8,10 @@ import { RGBA, SyntaxStyle, type CliRenderer, type ColorInput, type TerminalColors } from "@opentui/core" import type { TuiThemeCurrent } from "@opencode-ai/plugin/tui" import type { EntryKind } from "./types" +// altimate_change start — share the TUI's mode-resolution chain with the direct-run renderer // Shared with the full-screen TUI so both renderers agree on how a mode is chosen. import { resolveInitialMode } from "@opencode-ai/tui/terminal-detection" +// altimate_change end type Tone = { body: ColorInput @@ -583,6 +585,7 @@ function map( } } +// altimate_change start — mode-aware fallback seed (#809: dark panel + black resolved fg) /** * Seed colours for the direct-run fallback theme. * @@ -603,6 +606,7 @@ function fallbackSeed(mode: "dark" | "light") { error: RGBA.fromIndex(1, rgba(dark ? "#ef4444" : "#b91c1c")), } } +// altimate_change end function tone(body: ColorInput, start?: ColorInput): Tone { return { @@ -615,6 +619,7 @@ const fallbackSplashIndexed = Array.from({ length: 256 }, (_, index) => RGBA.fro const fallbackSplashLeft = RGBA.fromIndex(67) const fallbackSplashRight = RGBA.fromIndex(110) +// altimate_change start — per-mode fallback theme; dark instance keeps identity for existing callers const fallbackByMode = new Map<"dark" | "light", RunTheme>() /** @@ -683,7 +688,9 @@ export function runThemeFallback(mode: "dark" | "light"): RunTheme { /** Dark instance, kept as the default for callers with no mode to hand. */ export const RUN_THEME_FALLBACK: RunTheme = runThemeFallback("dark") +// altimate_change end +// altimate_change start — resolve a mode instead of always falling back to dark /** * Best guess at terminal mode when the palette query gives us nothing. * @@ -698,6 +705,7 @@ function fallbackMode(renderer: CliRenderer): "dark" | "light" { osc: renderer.themeMode ?? null, }) } +// altimate_change end export async function resolveRunTheme(renderer: CliRenderer): Promise { try { @@ -706,7 +714,9 @@ export async function resolveRunTheme(renderer: CliRenderer): Promise }) const bg = colors.defaultBackground ?? colors.palette[0] if (!bg) { + // altimate_change start — light terminals must not get the dark fallback return runThemeFallback(fallbackMode(renderer)) + // altimate_change end } // Palette-only terminal reloads can leave renderer.themeMode stale, but @@ -731,6 +741,8 @@ export async function resolveRunTheme(renderer: CliRenderer): Promise shared.generateSubtleSyntax(syntaxTheme), ) } catch { + // altimate_change start — light terminals must not get the dark fallback return runThemeFallback(fallbackMode(renderer)) + // altimate_change end } } From 7969a8c9a567c2bdc0b84e3d2454c93a7ca60f13 Mon Sep 17 00:00:00 2001 From: Haider Date: Thu, 27 Aug 2026 18:34:32 +0530 Subject: [PATCH 4/4] fix(tui): ask the OS for appearance on the direct-run fallback too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the open review findings on this PR. `fallbackMode` fed `resolveInitialMode` only COLORFGBG and the OSC reply, so the direct-run renderer did not actually agree with the startup path it shares that function with. On a light Apple Terminal — no COLORFGBG, no OSC 11 answer — a failed palette query still resolved to "dark" and repainted a light terminal dark, which is the #809 symptom this branch exists to remove. It now consults `detectSystemAppearance()`, but only when both cheap signals came back empty, so the `defaults` spawn stays off the common path. `footer.ts` keeps the last known-good theme when a runtime palette refresh fails, and detected that by comparing against `RUN_THEME_FALLBACK`. Once the fallback became per-mode, a light terminal produced a different instance, the identity check missed, and the footer replaced a good theme with the fallback. `isRunThemeFallback` tests membership in the memo map instead. `detectSystemAppearance` had a `clean` clause resolving "light" for an error with no `code` that was not killed. execFile never produces that shape — it sets `code` to the errno string on spawn failure and to the exit status otherwise — and had it fired it would have reported light for an unknown failure, which the comment directly above it forbids. Unknown now stays null so the caller can keep looking. The palette-failure test pinned the dark instance by identity, which re-encoded the behaviour being fixed and fails on a light-mode runner. It now asserts that a failed lookup yields *a* fallback, with a separate case keeping the dark instance pinned for an explicit dark signal. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018fJ3X7pcGT4R9yzjsJnqsV --- packages/opencode/src/cli/cmd/run/footer.ts | 7 +++- packages/opencode/src/cli/cmd/run/theme.ts | 42 ++++++++++++++++---- packages/opencode/test/cli/run/theme.test.ts | 26 ++++++++++-- packages/tui/src/terminal-detection.ts | 9 ++++- 4 files changed, 69 insertions(+), 15 deletions(-) diff --git a/packages/opencode/src/cli/cmd/run/footer.ts b/packages/opencode/src/cli/cmd/run/footer.ts index 0d9da6f297..eee613e4d3 100644 --- a/packages/opencode/src/cli/cmd/run/footer.ts +++ b/packages/opencode/src/cli/cmd/run/footer.ts @@ -35,7 +35,7 @@ import { SUBAGENT_INSPECTOR_ROWS } from "./footer.subagent" import { PROMPT_MAX_ROWS, TEXTAREA_MIN_ROWS } from "./footer.prompt" import { RunFooterView } from "./footer.view" import { RunScrollbackStream } from "./scrollback.surface" -import { RUN_THEME_FALLBACK, resolveRunTheme, type RunTheme } from "./theme" +import { isRunThemeFallback, resolveRunTheme, type RunTheme } from "./theme" import { modelInfo } from "./variant.shared" import type { FooterApi, @@ -1014,9 +1014,12 @@ export class RunFooter implements FooterApi { } // Keep the last known good theme when a runtime OSC probe times out. - if (theme === RUN_THEME_FALLBACK) { + // altimate_change start — upstream_fix: the fallback is per-mode now, so an + // identity check against the dark instance alone missed the light one. + if (isRunThemeFallback(theme)) { return } + // altimate_change end this.themes.push(theme) this.setTheme(theme) diff --git a/packages/opencode/src/cli/cmd/run/theme.ts b/packages/opencode/src/cli/cmd/run/theme.ts index 0a532e6ee4..059da93eed 100644 --- a/packages/opencode/src/cli/cmd/run/theme.ts +++ b/packages/opencode/src/cli/cmd/run/theme.ts @@ -10,7 +10,11 @@ import type { TuiThemeCurrent } from "@opencode-ai/plugin/tui" import type { EntryKind } from "./types" // altimate_change start — share the TUI's mode-resolution chain with the direct-run renderer // Shared with the full-screen TUI so both renderers agree on how a mode is chosen. -import { resolveInitialMode } from "@opencode-ai/tui/terminal-detection" +import { + detectModeFromCOLORFGBG, + detectSystemAppearance, + resolveInitialMode, +} from "@opencode-ai/tui/terminal-detection" // altimate_change end type Tone = { @@ -688,6 +692,22 @@ export function runThemeFallback(mode: "dark" | "light"): RunTheme { /** Dark instance, kept as the default for callers with no mode to hand. */ export const RUN_THEME_FALLBACK: RunTheme = runThemeFallback("dark") + +// altimate_change start — upstream_fix: recognise every per-mode fallback. +/** + * True for any fallback instance, not just the dark one. + * + * `footer.ts` keeps the last known-good theme when a runtime palette refresh + * fails, and used to detect that by comparing against `RUN_THEME_FALLBACK`. + * Now that the fallback is per-mode, a light terminal produced a *different* + * instance, that check missed, and the footer replaced a good theme with the + * fallback. Membership in the memo map is the identity test that survives. + */ +export function isRunThemeFallback(theme: RunTheme): boolean { + for (const cached of fallbackByMode.values()) if (cached === theme) return true + return false +} +// altimate_change end // altimate_change end // altimate_change start — resolve a mode instead of always falling back to dark @@ -699,11 +719,17 @@ export const RUN_THEME_FALLBACK: RunTheme = runThemeFallback("dark") * higher-traffic sibling of the startup detection in packages/tui — it drives * the direct-run and scrollback renderer. */ -function fallbackMode(renderer: CliRenderer): "dark" | "light" { - return resolveInitialMode({ - colorfgbg: process.env["COLORFGBG"], - osc: renderer.themeMode ?? null, - }) +async function fallbackMode(renderer: CliRenderer): Promise<"dark" | "light"> { + const colorfgbg = process.env["COLORFGBG"] + const osc = renderer.themeMode ?? null + // Ask the OS only when neither cheap signal answered. Without this the + // direct-run path did not actually agree with the startup path it shares + // `resolveInitialMode` with: on a light Apple Terminal — no COLORFGBG, no + // OSC 11 reply — it still resolved "dark" and repainted a light terminal + // dark, which is the #809 symptom this change exists to remove. The probe + // spawns `defaults`, so it stays behind the two free signals. + const appearance = osc || detectModeFromCOLORFGBG(colorfgbg) ? null : await detectSystemAppearance() + return resolveInitialMode({ colorfgbg, osc, appearance }) } // altimate_change end @@ -715,7 +741,7 @@ export async function resolveRunTheme(renderer: CliRenderer): Promise const bg = colors.defaultBackground ?? colors.palette[0] if (!bg) { // altimate_change start — light terminals must not get the dark fallback - return runThemeFallback(fallbackMode(renderer)) + return runThemeFallback(await fallbackMode(renderer)) // altimate_change end } @@ -742,7 +768,7 @@ export async function resolveRunTheme(renderer: CliRenderer): Promise ) } catch { // altimate_change start — light terminals must not get the dark fallback - return runThemeFallback(fallbackMode(renderer)) + return runThemeFallback(await fallbackMode(renderer)) // altimate_change end } } diff --git a/packages/opencode/test/cli/run/theme.test.ts b/packages/opencode/test/cli/run/theme.test.ts index 13867346ee..88bb283117 100644 --- a/packages/opencode/test/cli/run/theme.test.ts +++ b/packages/opencode/test/cli/run/theme.test.ts @@ -1,7 +1,13 @@ import { expect, test } from "bun:test" import { type ColorInput, RGBA, type CliRenderer, type TerminalColors } from "@opentui/core" -import { RUN_THEME_FALLBACK, - runThemeFallback, generateSystem, resolveRunTheme, resolveTheme } from "@/cli/cmd/run/theme" +import { + RUN_THEME_FALLBACK, + isRunThemeFallback, + runThemeFallback, + generateSystem, + resolveRunTheme, + resolveTheme, +} from "@/cli/cmd/run/theme" const palette = ["#15161e", "#f7768e", "#9ece6a", "#e0af68", "#7aa2f7", "#bb9af7", "#7dcfff", "#c0caf5"] as const @@ -60,7 +66,21 @@ function spread(color: RGBA) { } test("falls back when palette lookup fails", async () => { - expect(await resolveRunTheme(renderer({ fail: true }))).toBe(RUN_THEME_FALLBACK) + // Deliberately not `toBe(RUN_THEME_FALLBACK)`: with no OSC reply and no + // COLORFGBG the fallback now asks the OS for its appearance, so which + // per-mode instance comes back depends on the machine running the test. + // Pinning the dark one would re-encode the #809 behaviour this PR removes + // and would fail on a light-mode runner. The invariant is that a failed + // palette lookup yields *a* fallback rather than a resolved theme. + const theme = await resolveRunTheme(renderer({ fail: true })) + expect(isRunThemeFallback(theme)).toBe(true) +}) + +test("a dark terminal still gets the dark fallback", async () => { + // The mode-aware path must not have inverted anything: given an explicit + // dark signal the fallback is still the dark instance callers compare by + // identity. + expect(await resolveRunTheme(renderer({ fail: true, themeMode: "dark" }))).toBe(RUN_THEME_FALLBACK) }) test("the fallback follows a light terminal instead of always going dark", async () => { diff --git a/packages/tui/src/terminal-detection.ts b/packages/tui/src/terminal-detection.ts index 993f99728c..b26f6e3b1e 100644 --- a/packages/tui/src/terminal-detection.ts +++ b/packages/tui/src/terminal-detection.ts @@ -109,8 +109,13 @@ export function detectSystemAppearance( // silently reported as light. const err = error as NodeJS.ErrnoException & { killed?: boolean; status?: number | null; stderr?: string } const notFound = /does not exist/i.test(String(err.stderr ?? err.message ?? "")) - const clean = err.code === undefined && err.killed !== true - resolve(notFound || clean ? "light" : null) + // Only the missing-key diagnostic means light. A previous `clean` + // clause also accepted "no `code` and not killed", which execFile + // never produces (it sets `code` to the errno string on spawn failure + // and the exit status otherwise) — and had it ever fired it would have + // reported light for an unknown failure, the exact thing the comment + // above forbids. Unknown stays null so the caller can keep looking. + resolve(notFound ? "light" : null) }, ) } catch {