Skip to content

Commit b242e76

Browse files
committed
fix(tui): render light theme with readable roles
1 parent c9a643e commit b242e76

17 files changed

Lines changed: 603 additions & 345 deletions

‎docs/TUI.md‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,10 @@ titlebar row: the model label sits right-aligned in the top rule as
134134
`profile · model · effort · mode` (empty segments omitted), and a
135135
compact `mcp !` sits immediately left of it when any MCP server still needs
136136
authorization (`/mcp` is the surface that names them), painted in
137-
`UI.warning` (sand, `#d1ad7d`) — the same role `plugin !` uses. Orange is
138-
not spent on these standing marks. The brand
137+
`UI.warning` — the same role `plugin !` uses. Warning is caution gold in the
138+
dark theme and muted plum (`#655275`) in the light theme; a bronze warning on
139+
cream is perceptually indistinguishable from machine emphasis, while plum stays
140+
separate without spending action orange or failure red. The brand
139141
lockup sits at the left of the bottom rule with the working directory and git
140142
branch at its right (`AppShell.promptTopRule` / `promptBottomRule`,
141143
`src/tui/shell/internals.ts`). Context occupancy rides that bottom rule as a percent:
@@ -239,6 +241,13 @@ Ongoing, non-decision status uses the bronze/sand/ember chrome ramp and green
239241
The one deliberate exception is diff removals, where orange is content (the
240242
removed line), not a decision marker, and no decision-marker shares that row.
241243

244+
The shell selects its final theme before constructing or mounting renderables.
245+
An explicit `light` or `dark` setting wins; otherwise `COLORFGBG` wins when it
246+
identifies the terminal background, then macOS appearance is read synchronously,
247+
and unknown environments fall back to dark. The macOS read has a 500 ms process
248+
timeout. Production does not query OSC 11 or switch themes after mount, so the
249+
first frame cannot mix palettes across transcript, prompt, borders, or overlays.
250+
242251
## The live task list panel
243252

244253
**Parked pending rebuild.** `formatChromeZones` (`src/tui/chrome-state.ts`)
@@ -418,7 +427,7 @@ framed content in the shell, and their body is shaped rather than merely
418427
listed (`src/tui/overlay-body.ts`): a dithered header (`░▒▓`) carries the
419428
subject in the action color — the only Breakthrough Orange on the card. The
420429
overlay host border and title use calm dim chrome (`UI.textDim`); consequence
421-
impact in the description zone paints `UI.warning` (sand), not orange. A
430+
impact in the description zone paints `UI.warning`, not orange. A
422431
blank row separates the subject from context. Choices are deliberately small:
423432
each one is a bare, single-line action name (`Reject`, `Accept once`, the
424433
scope's label) with no consequence text folded into the row. A scope's hint

‎src/tui/diff-rows.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { toolCallRow } from "./diff";
1111
import { withTestRenderer, type Harness } from "./harness";
1212
import { appendStreamRow } from "./shell/chrome";
1313
import { createAppShell } from "./shell/index";
14-
import { DIFF_FG } from "./stream";
14+
import { UI } from "./theme";
1515
import { toolResultRow } from "./mcp-view";
1616

1717
const WIDE = { width: 100, height: 30 } as const;
@@ -78,8 +78,8 @@ describe("diff transcript rows", () => {
7878
const spans = spansWithHex(h);
7979
const del = spans.find((s) => s.text.includes("-") && s.text.length <= 2);
8080
const add = spans.find((s) => s.text.includes("+") && s.text.length <= 2);
81-
expect(del?.fg).toBe(DIFF_FG.del);
82-
expect(add?.fg).toBe(DIFF_FG.add);
81+
expect(del?.fg).toBe(UI.action);
82+
expect(add?.fg).toBe(UI.done);
8383
}, WIDE);
8484
});
8585

@@ -97,13 +97,13 @@ describe("diff transcript rows", () => {
9797
const changedAdded = spans.find((s) => s.text.includes("product(a,"));
9898
const shared = spans.filter((s) => s.text.includes("const"));
9999

100-
expect(changedRemoved?.fg).toBe(DIFF_FG.del);
101-
expect(changedAdded?.fg).toBe(DIFF_FG.add);
100+
expect(changedRemoved?.fg).toBe(UI.action);
101+
expect(changedAdded?.fg).toBe(UI.done);
102102
// Bold attribute distinguishes the changed tokens inside the line.
103103
expect(defined(changedRemoved).attributes).toBeGreaterThan(0);
104104
// "const" is shared by both sides, so it stays in the context tone.
105105
expect(shared.length).toBeGreaterThan(0);
106-
expect(shared.every((s) => s.fg === DIFF_FG.context)).toBe(true);
106+
expect(shared.every((s) => s.fg === UI.textDim)).toBe(true);
107107
}, WIDE);
108108
});
109109

‎src/tui/diff.test.ts‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ import {
1111
toolCallRow,
1212
type DiffLine,
1313
} from "./diff.js";
14-
import { DIFF_FG, isDiffRow, isMarkdownRow } from "./stream.js";
14+
import { isDiffRow, isMarkdownRow } from "./stream.js";
15+
import { UI } from "./theme.js";
1516

1617
const textOf = (line: DiffLine): string => line.map((seg) => seg.text).join("");
1718

@@ -91,19 +92,19 @@ describe("renderDiff", () => {
9192
const delBody = defined(lines[0]).slice(2);
9293
const addBody = defined(lines[1]).slice(2);
9394
const delChanged = delBody
94-
.filter((s) => s.fg === DIFF_FG.del)
95+
.filter((s) => s.fg === UI.action)
9596
.map((s) => s.text)
9697
.join("");
9798
const addChanged = addBody
98-
.filter((s) => s.fg === DIFF_FG.add)
99+
.filter((s) => s.fg === UI.done)
99100
.map((s) => s.text)
100101
.join("");
101102
expect(delChanged).toContain("bar");
102103
expect(addChanged).toContain("baz");
103104
expect(delChanged).not.toContain("const");
104105
expect(addChanged).not.toContain("const");
105106
expect(
106-
delBody.some((s) => s.text.includes("const") && s.fg === DIFF_FG.context),
107+
delBody.some((s) => s.text.includes("const") && s.fg === UI.textDim),
107108
).toBe(true);
108109
});
109110

@@ -118,7 +119,7 @@ describe("renderDiff", () => {
118119
const lines = renderDiff("a b c", "a x c", 40);
119120
const changed = defined(lines[0])
120121
.slice(2)
121-
.filter((s) => s.fg === DIFF_FG.del)
122+
.filter((s) => s.fg === UI.action)
122123
.map((s) => s.text.trim())
123124
.filter(Boolean);
124125
expect(changed).toEqual(["b"]);
@@ -130,26 +131,26 @@ describe("renderDiff", () => {
130131
expect(
131132
defined(removed)
132133
.slice(1)
133-
.every((s) => s.fg === DIFF_FG.del),
134+
.every((s) => s.fg === UI.action),
134135
).toBe(true);
135136
expect(
136137
defined(added)
137138
.slice(1)
138-
.every((s) => s.fg === DIFF_FG.add),
139+
.every((s) => s.fg === UI.done),
139140
).toBe(true);
140141
});
141142

142143
test("context rows take the muted context tone", () => {
143144
const lines = renderDiff("a\nb", "a\nB", 40);
144-
expect(defined(lines[0]).every((s) => s.fg === DIFF_FG.context)).toBe(true);
145+
expect(defined(lines[0]).every((s) => s.fg === UI.textDim)).toBe(true);
145146
});
146147

147148
test("line-number column always uses the muted context tone", () => {
148149
const lines = renderDiff("a\nb", "a\nB", 40);
149150
expect(lines.map((line) => defined(line[0]).fg)).toEqual([
150-
DIFF_FG.context,
151-
DIFF_FG.context,
152-
DIFF_FG.context,
151+
UI.textDim,
152+
UI.textDim,
153+
UI.textDim,
153154
]);
154155
});
155156

‎src/tui/diff.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ import { describeToolCall } from "./tool-formatter.js";
1616
// The one wrap implementation: a diff row soft-wraps by the same column rules
1717
// as every other row, so a wide glyph cannot overflow the gutter here alone.
1818
import { wrapRanges } from "./view/height.js";
19-
import { DIFF_FG, type StreamRow } from "./stream.js";
19+
import type { StreamRow } from "./stream.js";
20+
import { UI } from "./theme.js";
2021
import { toolArgsView } from "./tool-args.js";
2122

2223
export type DiffRowKind = "add" | "del" | "context";
@@ -139,9 +140,9 @@ const GUTTER: Record<DiffRowKind, string> = {
139140
};
140141

141142
function rowColor(kind: DiffRowKind): string {
142-
if (kind === "add") return DIFF_FG.add;
143-
if (kind === "del") return DIFF_FG.del;
144-
return DIFF_FG.context;
143+
if (kind === "add") return UI.done;
144+
if (kind === "del") return UI.action;
145+
return UI.textDim;
145146
}
146147

147148
// Attach each row's position in the old/new file before any collapsing, so a
@@ -227,7 +228,7 @@ export function wordDiffSegments(
227228
const m = other.length;
228229
while (i < n && j < m) {
229230
if (self[i] === other[j]) {
230-
out.push({ text: requireDiffLine(self, i), fg: DIFF_FG.context });
231+
out.push({ text: requireDiffLine(self, i), fg: UI.textDim });
231232
i++;
232233
j++;
233234
} else if (lcsCell(lcs, i + 1, j) >= lcsCell(lcs, i, j + 1)) {
@@ -351,7 +352,7 @@ export function renderDiff(
351352
? [
352353
{
353354
text: idx === 0 ? numCol : " ".repeat(numColWidth),
354-
fg: DIFF_FG.context,
355+
fg: UI.textDim,
355356
},
356357
]
357358
: []),
@@ -453,7 +454,7 @@ export function editDiffView(
453454
[
454455
{
455456
text: `… ${all.length - MAX_DIFF_LINES} more diff lines`,
456-
fg: DIFF_FG.context,
457+
fg: UI.textDim,
457458
},
458459
],
459460
]

‎src/tui/runner/index.ts‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88

99
import { EventEmitter } from "node:events";
10+
import { spawnSync } from "node:child_process";
1011
import type { Config } from "../../config/index.js";
1112
import { listFavoriteModels, listRecentModels } from "../../config/settings.js";
1213
import { isCodexProviderName } from "../../config/codex-providers.js";
@@ -39,8 +40,10 @@ import { createRunnerState, liveAgent } from "./state.js";
3940
import { applyCredentialRecoverySelection } from "./credential-recovery.js";
4041
import { setTheme } from "../theme.js";
4142
import {
43+
detectOsAppearance,
4244
resolveDetectedTheme,
4345
resolveThemeSetting,
46+
sniffSyncTheme,
4447
syncEnvFromRecord,
4548
} from "../theme-detect.js";
4649
import { getLogger } from "@intx/log";
@@ -52,25 +55,36 @@ export function createTUIEventEmitter(): EventEmitter {
5255

5356
export { getTUIRunSummaryStatus } from "../../session/run-sink.js";
5457

58+
function detectStartupOsTheme() {
59+
return detectOsAppearance(process.platform, (command, args) => {
60+
try {
61+
const out = spawnSync(command, [...args], {
62+
encoding: "utf8",
63+
timeout: 500,
64+
});
65+
if (out.error !== undefined) return undefined;
66+
if (out.status !== 0) return null;
67+
return typeof out.stdout === "string" ? out.stdout : undefined;
68+
} catch {
69+
return undefined;
70+
}
71+
});
72+
}
73+
5574
export async function runTUI(initialConfig: Config): Promise<number> {
5675
const tuiLogger = getLogger([LOG_NAMESPACE_ROOT, "tui"]);
76+
const setting = resolveThemeSetting(initialConfig.settings?.theme);
77+
const syncEnv = syncEnvFromRecord(process.env);
78+
const os =
79+
setting === "auto" && sniffSyncTheme(syncEnv) === null
80+
? detectStartupOsTheme()
81+
: null;
82+
setTheme(resolveDetectedTheme({ setting, syncEnv, os }));
83+
5784
const start = await prepareTUISession(initialConfig, liveTelemetry);
5885
if (start === null) return 0;
5986
const state = createRunnerState(start);
6087

61-
// Sync theme answer (CL-8993): the explicit setting wins, else the
62-
// COLORFGBG/TERM_PROGRAM sniff, else dark. Paints correctly on first frame;
63-
// the async OS-appearance upgrade in wirePostStartup repaints if it lands
64-
// lighter. Unknown terminals stay dark.
65-
setTheme(
66-
resolveDetectedTheme({
67-
setting: resolveThemeSetting(state.config.settings?.theme),
68-
syncEnv: syncEnvFromRecord(process.env),
69-
osc: null,
70-
os: null,
71-
}),
72-
);
73-
7488
const { pluginModules } = start.trust;
7589
// /plugins UI backend state: discovered modules plus live, persisted config
7690
// (enabled flag, credentials, web override, extra paths). Trust grants swap

‎src/tui/runner/wiring.ts‎

Lines changed: 1 addition & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
*/
88

99
import { getLogger } from "@intx/log";
10-
import { spawnSync } from "node:child_process";
1110
import {
1211
loadSettings,
1312
listFavoriteModels,
@@ -48,19 +47,7 @@ import {
4847
RESUME_TRANSCRIPT_BLOCK_LIMIT,
4948
turnsToContentBlocks,
5049
} from "../turns-to-blocks.js";
51-
import {
52-
setPluginNeedsAttention,
53-
setStatusFlash,
54-
repaintTranscriptWindow,
55-
} from "../shell/chrome.js";
56-
import { UI, setTheme } from "../theme.js";
57-
import {
58-
detectOsAppearance,
59-
resolveDetectedTheme,
60-
resolveThemeSetting,
61-
sniffSyncTheme,
62-
syncEnvFromRecord,
63-
} from "../theme-detect.js";
50+
import { setPluginNeedsAttention, setStatusFlash } from "../shell/chrome.js";
6451
import {
6552
setEffortCycleHandler,
6653
setMentionSuggestionSource,
@@ -503,39 +490,6 @@ export function wirePostStartup(
503490
});
504491
});
505492

506-
// Async theme upgrade (CL-8993): the sync answer already painted, so a
507-
// slower OS-appearance read that disagrees swaps the live binding and
508-
// repaints once. Fire-and-forget and spawn-capped — a missing `defaults`
509-
// binary abstains to dark rather than stalling startup.
510-
void (async () => {
511-
const setting = resolveThemeSetting(state.config.settings?.theme);
512-
if (setting !== "auto") return;
513-
const syncEnv = syncEnvFromRecord(process.env);
514-
if (sniffSyncTheme(syncEnv) !== null) return;
515-
const os = detectOsAppearance(process.platform, (command, args) => {
516-
try {
517-
const out = spawnSync(command, [...args], {
518-
encoding: "utf8",
519-
timeout: 500,
520-
});
521-
if (out.error !== undefined) return undefined;
522-
if (out.status !== 0) return null;
523-
return typeof out.stdout === "string" ? out.stdout : undefined;
524-
} catch {
525-
return undefined;
526-
}
527-
});
528-
const next = resolveDetectedTheme({ setting, syncEnv, osc: null, os });
529-
if (next === UI.name) return;
530-
setTheme(next);
531-
const host = services.hostHolder.instance;
532-
if (host !== undefined) repaintTranscriptWindow(host.shell);
533-
})().catch((err: unknown) => {
534-
tuiLogger.debug("async theme detect failed: {error}", {
535-
error: err instanceof Error ? err.message : String(err),
536-
});
537-
});
538-
539493
if (!state.resumeSkipInitialTask && state.config.task.trim().length > 0) {
540494
// The operator's initial task, typed as a CLI argument before launch —
541495
// same provenance as a prompt submit.

0 commit comments

Comments
 (0)