From 1f652214df465b86b5ac8cd1d8eaf78215e9d501 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 22 Jul 2026 16:22:36 -0500 Subject: [PATCH] =?UTF-8?q?fix(ui-tui):=20skin=20owns=20BOTH=20terminal=20?= =?UTF-8?q?defaults=20=E2=80=94=20OSC-10=20foreground=20beside=20the=20OSC?= =?UTF-8?q?-11=20background?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The input fix's sibling, hit immediately after: the composer was themed but AGENT text went black-on-black the same way. Root cause is the class, not the call site — markdown body, borders, and every token rendered without an explicit color falls back to the terminal's DEFAULT foreground, which belongs to the HOST profile's polarity, not the skin's. A dark skin on a light terminal repaints the backdrop via OSC-11 while thousands of default-fg cells stay near-black. Chasing every is unwinnable. Instead own the default itself: when a skin authors a background (the existing opt-in), paint the default foreground from the resolved theme's text color via OSC-10. Every unthemed token — present and future — re-bases onto the skin atomically, exactly like the background. terminalModes: the OSC-11 slot generalizes to defaultColorSlot(10|11) — same paint/clear/exit-restore contract, tracked per slot, so a skinless session still never touches the terminal. reapplyTheme repaints the fg too: polarity flips swap paired palettes, moving the text tone while the background stays. Tests: slot contract runs table-driven over both OSC codes; handler test pins the invariant (default fg == theme text; dropping the background releases both defaults). Suite 1344✓, typecheck/lint/prettier clean. --- .../createGatewayEventHandler.test.ts | 38 ++++++++++ ui-tui/src/__tests__/terminalModes.test.ts | 57 ++++++++++---- ui-tui/src/app/createGatewayEventHandler.ts | 31 ++++++-- ui-tui/src/lib/terminalModes.ts | 75 +++++++++++-------- 4 files changed, 146 insertions(+), 55 deletions(-) diff --git a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts index 5a4b75db7cb..fdd12d7b152 100644 --- a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts +++ b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts @@ -743,6 +743,44 @@ describe('createGatewayEventHandler', () => { vi.unstubAllEnvs() }) + it('a skin that owns the background paints BOTH terminal defaults (OSC 11 bg + OSC 10 fg)', () => { + // Default-fg tokens (markdown body, borders) render with the TERMINAL's + // default foreground. A dark skin on a light terminal repaints the + // backdrop black via OSC-11 — without the OSC-10 pair, those tokens stay + // the host's near-black: invisible. The invariant is fg == theme text. + const writes: string[] = [] + + const write = vi.spyOn(process.stdout, 'write').mockImplementation(chunk => { + writes.push(String(chunk)) + + return true + }) + + const tty = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY') + Object.defineProperty(process.stdout, 'isTTY', { configurable: true, value: true }) + + try { + const handle = createGatewayEventHandler(buildCtx([])) + handle({ payload: { colors: { background: '#000000', ui_text: '#ff9f0a' } }, type: 'skin.changed' } as any) + + const joined = writes.join('') + expect(joined).toContain('\x1b]11;#000000\x07') + expect(joined).toContain(`\x1b]10;${getUiState().theme.color.text}\x07`) + + // Dropping the background releases BOTH defaults back to the terminal. + writes.length = 0 + handle({ payload: { colors: { ui_text: '#ff9f0a' } }, type: 'skin.changed' } as any) + expect(writes.join('')).toContain('\x1b]111\x07') + expect(writes.join('')).toContain('\x1b]110\x07') + } finally { + write.mockRestore() + + if (tty) { + Object.defineProperty(process.stdout, 'isTTY', tty) + } + } + }) + it('infers polarity from the OSC-10 foreground only when the answer is decisive', async () => { const { polarityBackgroundFromForeground } = await import('../app/createGatewayEventHandler.js') diff --git a/ui-tui/src/__tests__/terminalModes.test.ts b/ui-tui/src/__tests__/terminalModes.test.ts index 1ff7aac6096..83b5c4edb0d 100644 --- a/ui-tui/src/__tests__/terminalModes.test.ts +++ b/ui-tui/src/__tests__/terminalModes.test.ts @@ -1,6 +1,12 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import { resetTerminalModes, setTerminalBackground, TERMINAL_MODE_RESET } from '../lib/terminalModes.js' +import { + isPaintableHex, + resetTerminalModes, + setTerminalBackground, + setTerminalForeground, + TERMINAL_MODE_RESET +} from '../lib/terminalModes.js' describe('terminal mode reset', () => { it('includes common sticky input modes', () => { @@ -61,7 +67,15 @@ describe('terminal mode reset', () => { }) }) -describe('terminal background (OSC 11)', () => { +// Foreground (OSC 10) and background (OSC 11) are the same slot contract — +// assert it once over both. Painting BOTH is what keeps every default-fg +// token legible when a skin flips the terminal's polarity. +describe.each([ + { name: 'foreground', osc: 10, set: setTerminalForeground }, + { name: 'background', osc: 11, set: setTerminalBackground } +])('terminal default $name (OSC $osc)', ({ osc, set }) => { + const paint = `\x1b]${osc};` + const restore = `\x1b]1${osc}\x07` const tty = (write: ReturnType) => ({ isTTY: true, write }) as unknown as NodeJS.WriteStream const written = (fn: (s: NodeJS.WriteStream) => void): string => { @@ -72,32 +86,43 @@ describe('terminal background (OSC 11)', () => { } // Leave the module's "painted" flag clean so the exact-match reset test above - // (and other files) never see a stray background restore. - afterEach(() => setTerminalBackground('', tty(vi.fn()))) + // (and other files) never see a stray restore. + afterEach(() => set('', tty(vi.fn()))) - it('paints the terminal default background from a valid hex', () => { - expect(written(s => setTerminalBackground('#08201F', s))).toBe('\x1b]11;#08201F\x07') + it('paints the terminal default from a valid hex', () => { + expect(written(s => set('#08201F', s))).toBe(`${paint}#08201F\x07`) }) it('ignores an invalid hex and non-TTY streams', () => { - expect(written(s => setTerminalBackground('teal', s))).toBe('') + expect(written(s => set('teal', s))).toBe('') const write = vi.fn() - setTerminalBackground('#08201f', { isTTY: false, write } as unknown as NodeJS.WriteStream) + set('#08201f', { isTTY: false, write } as unknown as NodeJS.WriteStream) expect(write).not.toHaveBeenCalled() }) - it('appends the background restore to the exit reset once painted, not before', () => { - expect(written(resetTerminalModes)).not.toContain('\x1b]111\x07') + it('appends the restore to the exit reset once painted, not before', () => { + expect(written(resetTerminalModes)).not.toContain(restore) - setTerminalBackground('#101010', tty(vi.fn())) - expect(written(resetTerminalModes)).toContain('\x1b]111\x07') + set('#101010', tty(vi.fn())) + expect(written(resetTerminalModes)).toContain(restore) }) - it('clears back to the terminal default when the next skin has no background', () => { - setTerminalBackground('#123456', tty(vi.fn())) - expect(written(s => setTerminalBackground('', s))).toBe('\x1b]111\x07') + it('clears back to the terminal default when the next skin drops the color', () => { + set('#123456', tty(vi.fn())) + expect(written(s => set('', s))).toBe(restore) // Cleared: a later reset no longer restores. - expect(written(resetTerminalModes)).not.toContain('\x1b]111\x07') + expect(written(resetTerminalModes)).not.toContain(restore) + }) +}) + +describe('isPaintableHex', () => { + it('matches exactly what the slot setters paint', () => { + expect(isPaintableHex('#08201F')).toBe(true) + expect(isPaintableHex('#08201f')).toBe(true) + + for (const junk of ['', 'teal', '#fff', '#12345', '#1234567']) { + expect(isPaintableHex(junk)).toBe(false) + } }) }) diff --git a/ui-tui/src/app/createGatewayEventHandler.ts b/ui-tui/src/app/createGatewayEventHandler.ts index 7ff42d53d26..ec0fb02748b 100644 --- a/ui-tui/src/app/createGatewayEventHandler.ts +++ b/ui-tui/src/app/createGatewayEventHandler.ts @@ -18,7 +18,7 @@ import { isTodoDone } from '../lib/liveProgress.js' import { openExternalUrl } from '../lib/openExternalUrl.js' import { rpcErrorMessage } from '../lib/rpc.js' import { topLevelSubagents } from '../lib/subagentTree.js' -import { setTerminalBackground } from '../lib/terminalModes.js' +import { isPaintableHex, setTerminalBackground, setTerminalForeground } from '../lib/terminalModes.js' import { formatAbandonedClarify, formatToolCall, stripAnsi } from '../lib/text.js' import { bootSeededPin, invalidateBootBackground, writeBootTheme } from '../lib/themeBoot.js' import { defaultThemeForCurrentBackground, detectLightMode, fromSkin, type Theme } from '../theme.js' @@ -117,19 +117,36 @@ const themesEqual = (a: Theme, b: Theme) => { ) } +// A skin that owns the background must own BOTH terminal defaults: OSC-11 +// paints every cell's backdrop, and OSC-10 re-bases every default-fg token — +// markdown body, borders, anything rendered without an explicit color — onto +// the theme's text color. Without the pair, a dark skin on a light terminal +// leaves default-fg text at the HOST's near-black: invisible. Opt-in stays +// intact: no `background` ⇒ both defaults restore to the terminal's own. +const paintTerminalDefaults = (theme: Theme) => { + const background = lastSkin?.colors?.background ?? '' + + setTerminalBackground(background) + setTerminalForeground(isPaintableHex(background) ? theme.color.text : '') +} + const applySkin = (s: GatewaySkin) => { lastSkin = s - commitTheme(themeForSkin(s)) - // Paint the whole terminal from the skin's `background` (empty ⇒ restore the - // terminal default), so Hermes owns its background instead of inheriting it. - // Opt-in: a skin with no `background` leaves the terminal untouched. - setTerminalBackground(s.colors?.background ?? '') + const theme = themeForSkin(s) + + commitTheme(theme) + paintTerminalDefaults(theme) } /** Re-derive the theme from current detection signals (env overrides, cached * OSC-11 answer) — used by /theme, config sync, and the OSC listener. */ export function reapplyTheme(): void { - commitTheme(lastSkin ? themeForSkin(lastSkin) : defaultThemeForCurrentBackground()) + const theme = lastSkin ? themeForSkin(lastSkin) : defaultThemeForCurrentBackground() + + commitTheme(theme) + // Polarity flips swap paired palettes, so the default fg must track the + // re-derived text tone even though the skin's background hasn't moved. + paintTerminalDefaults(theme) } /** diff --git a/ui-tui/src/lib/terminalModes.ts b/ui-tui/src/lib/terminalModes.ts index 32a436ac84f..5a7428fa82f 100644 --- a/ui-tui/src/lib/terminalModes.ts +++ b/ui-tui/src/lib/terminalModes.ts @@ -25,51 +25,62 @@ type ResettableStream = Pick & { fd?: number } -// OSC 11 sets the terminal's DEFAULT background — so the whole TUI, not just -// rendered text, takes the skin color. OSC 111 restores the terminal's own -// default. We only reset when we actually painted, so a user who never uses a -// skin background keeps their terminal untouched. +// OSC 10/11 set the terminal's DEFAULT foreground/background — so every cell, +// including text rendered with no explicit color (markdown body, borders, +// third-party output), takes the skin instead of the host profile's defaults. +// OSC 110/111 restore the terminal's own values. We only reset what we +// actually painted, so a skinless session leaves the terminal untouched. const HEX_RE = /^#[0-9a-f]{6}$/i -const OSC_RESET_BACKGROUND = '\x1b]111\x07' -let _backgroundPainted = false + +/** True when `hex` is a paintable default (the same bar `set` applies). */ +export const isPaintableHex = (hex: string): boolean => HEX_RE.test(hex) /** - * Paint the terminal's default background from a skin (`hex`), or clear it back - * to the terminal default when `hex` is empty/invalid (a skin with no - * `background`, e.g. reverting to `default`). Runtime writes go through the async - * stream so they order cleanly with Ink's frames; the exit-time restore rides - * `resetTerminalModes` (writeSync). No-op off a TTY. + * A paintable terminal default (fg=10, bg=11). `set(hex)` paints from a skin, + * or clears back to the terminal's own default when `hex` is empty/invalid + * (a skin without the key, e.g. reverting to `default`). Runtime writes go + * through the async stream so they order cleanly with Ink's frames; the + * exit-time restore rides `resetTerminalModes` (writeSync). No-op off a TTY. */ -export function setTerminalBackground(hex: string, stream: ResettableStream = process.stdout): void { - if (!stream.isTTY) { - return +const defaultColorSlot = (osc: 10 | 11) => { + const restore = `\x1b]1${osc}\x07` + let painted = false + + const set = (hex: string, stream: ResettableStream = process.stdout): void => { + if (!stream.isTTY) { + return + } + + try { + if (HEX_RE.test(hex)) { + stream.write(`\x1b]${osc};${hex}\x07`) + painted = true + } else if (painted) { + stream.write(restore) + painted = false + } + } catch { + // Terminal that can't take it just keeps its default. + } } - if (HEX_RE.test(hex)) { - try { - stream.write(`\x1b]11;${hex}\x07`) - _backgroundPainted = true - } catch { - // Terminal that can't take it just keeps its background. - } - } else if (_backgroundPainted) { - try { - stream.write(OSC_RESET_BACKGROUND) - _backgroundPainted = false - } catch { - // ignore - } - } + return { restoreSeq: () => (painted ? restore : ''), set } } +const foreground = defaultColorSlot(10) +const background = defaultColorSlot(11) + +export const setTerminalForeground = foreground.set +export const setTerminalBackground = background.set + export function resetTerminalModes(stream: ResettableStream = process.stdout): boolean { if (!stream.isTTY) { return false } - // Append the background restore only if we painted one, so a normal session - // never resets a terminal it didn't touch. - const reset = _backgroundPainted ? TERMINAL_MODE_RESET + OSC_RESET_BACKGROUND : TERMINAL_MODE_RESET + // Append default-color restores only for what we painted, so a normal + // session never resets a terminal it didn't touch. + const reset = TERMINAL_MODE_RESET + foreground.restoreSeq() + background.restoreSeq() const fd = typeof stream.fd === 'number' ? stream.fd : stream === process.stdout ? 1 : undefined if (fd !== undefined) {