diff --git a/ui-tui/src/__tests__/theme.test.ts b/ui-tui/src/__tests__/theme.test.ts index db2b1eac38..b73251188f 100644 --- a/ui-tui/src/__tests__/theme.test.ts +++ b/ui-tui/src/__tests__/theme.test.ts @@ -1,46 +1,90 @@ -import { describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' -import { DARK_THEME, DEFAULT_THEME, detectLightMode, fromSkin, LIGHT_THEME } from '../theme.js' +// `theme.js` reads `process.env` at module-load to compute DEFAULT_THEME, +// and `fromSkin` closes over DEFAULT_THEME. A developer shell with +// HERMES_TUI_THEME=light (or HERMES_TUI_BACKGROUND set to something +// bright) would flip the base and turn these assertions into a local- +// only failure. We sterilize the relevant env vars + dynamically +// import the module fresh so EVERY symbol that closes over the env +// (DEFAULT_THEME, DARK_THEME, LIGHT_THEME, fromSkin) is loaded against +// a known-empty environment. +// +// `detectLightMode` takes env as an explicit arg, so it's safe to import +// statically — but we stay consistent and dynamic-import it too. +const RELEVANT_ENV = [ + 'HERMES_TUI_LIGHT', + 'HERMES_TUI_THEME', + 'HERMES_TUI_BACKGROUND', + 'COLORFGBG', + 'TERM_PROGRAM', +] as const + +async function importThemeWithCleanEnv() { + for (const key of RELEVANT_ENV) { + vi.stubEnv(key, '') + } + vi.resetModules() + return import('../theme.js') +} + +afterEach(() => { + vi.unstubAllEnvs() + vi.resetModules() +}) describe('DEFAULT_THEME', () => { - it('has brand defaults', () => { + it('has brand defaults', async () => { + const { DEFAULT_THEME } = await importThemeWithCleanEnv() + expect(DEFAULT_THEME.brand.name).toBe('Hermes Agent') expect(DEFAULT_THEME.brand.prompt).toBe('❯') expect(DEFAULT_THEME.brand.tool).toBe('┊') }) - it('has color palette', () => { + it('has color palette', async () => { + const { DEFAULT_THEME } = await importThemeWithCleanEnv() + expect(DEFAULT_THEME.color.gold).toBe('#FFD700') expect(DEFAULT_THEME.color.error).toBe('#ef5350') }) }) describe('LIGHT_THEME', () => { - it('avoids bright-yellow accents unreadable on white backgrounds (#11300)', () => { + it('avoids bright-yellow accents unreadable on white backgrounds (#11300)', async () => { + const { LIGHT_THEME } = await importThemeWithCleanEnv() + expect(LIGHT_THEME.color.gold).not.toBe('#FFD700') expect(LIGHT_THEME.color.amber).not.toBe('#FFBF00') expect(LIGHT_THEME.color.dim).not.toBe('#B8860B') expect(LIGHT_THEME.color.statusWarn).not.toBe('#FFD700') }) - it('keeps the same shape as DARK_THEME', () => { + it('keeps the same shape as DARK_THEME', async () => { + const { DARK_THEME, LIGHT_THEME } = await importThemeWithCleanEnv() + expect(Object.keys(LIGHT_THEME.color).sort()).toEqual(Object.keys(DARK_THEME.color).sort()) expect(LIGHT_THEME.brand).toEqual(DARK_THEME.brand) }) }) describe('DEFAULT_THEME aliasing', () => { - it('defaults to DARK_THEME when nothing signals light', () => { - expect(DEFAULT_THEME).toBe(DARK_THEME) + it('defaults to DARK_THEME when nothing signals light', async () => { + const { DEFAULT_THEME, DARK_THEME: DARK } = await importThemeWithCleanEnv() + + expect(DEFAULT_THEME).toBe(DARK) }) }) describe('detectLightMode', () => { - it('returns false on empty env', () => { + it('returns false on empty env', async () => { + const { detectLightMode } = await importThemeWithCleanEnv() + expect(detectLightMode({})).toBe(false) }) - it('honors HERMES_TUI_LIGHT on/off', () => { + it('honors HERMES_TUI_LIGHT on/off', async () => { + const { detectLightMode } = await importThemeWithCleanEnv() + expect(detectLightMode({ HERMES_TUI_LIGHT: '1' })).toBe(true) expect(detectLightMode({ HERMES_TUI_LIGHT: 'true' })).toBe(true) expect(detectLightMode({ HERMES_TUI_LIGHT: 'on' })).toBe(true) @@ -48,7 +92,9 @@ describe('detectLightMode', () => { expect(detectLightMode({ HERMES_TUI_LIGHT: 'off' })).toBe(false) }) - it('sniffs COLORFGBG bg slots 7 and 15 as light (#11300)', () => { + it('sniffs COLORFGBG bg slots 7 and 15 as light (#11300)', async () => { + const { detectLightMode } = await importThemeWithCleanEnv() + expect(detectLightMode({ COLORFGBG: '0;15' })).toBe(true) expect(detectLightMode({ COLORFGBG: '0;default;15' })).toBe(true) expect(detectLightMode({ COLORFGBG: '0;7' })).toBe(true) @@ -56,38 +102,119 @@ describe('detectLightMode', () => { expect(detectLightMode({ COLORFGBG: '7;default;0' })).toBe(false) }) - it('lets HERMES_TUI_LIGHT=0 override a light COLORFGBG', () => { + it('falls through on malformed COLORFGBG with empty/non-numeric trailing field', async () => { + const { detectLightMode } = await importThemeWithCleanEnv() + // `Number('')` is 0, so `'15;'` would have been read as bg=0 + // (authoritative dark) and incorrectly blocked TERM_PROGRAM. + // The strict /^\d+$/ guard makes these fall through instead. + const allowList = new Set(['Apple_Terminal']) + + expect(detectLightMode({ COLORFGBG: '15;', TERM_PROGRAM: 'Apple_Terminal' }, allowList)).toBe(true) + expect(detectLightMode({ COLORFGBG: 'default;default', TERM_PROGRAM: 'Apple_Terminal' }, allowList)).toBe(true) + // Without an allow-list match, fall-through still defaults to dark. + expect(detectLightMode({ COLORFGBG: '15;' })).toBe(false) + }) + + it('lets HERMES_TUI_LIGHT=0 override a light COLORFGBG', async () => { + const { detectLightMode } = await importThemeWithCleanEnv() + expect(detectLightMode({ COLORFGBG: '0;15', HERMES_TUI_LIGHT: '0' })).toBe(false) }) + + it('honors HERMES_TUI_THEME=light/dark as a symmetric explicit override', async () => { + const { detectLightMode } = await importThemeWithCleanEnv() + + expect(detectLightMode({ HERMES_TUI_THEME: 'light' })).toBe(true) + expect(detectLightMode({ HERMES_TUI_THEME: 'dark' })).toBe(false) + expect(detectLightMode({ COLORFGBG: '0;15', HERMES_TUI_THEME: 'dark' })).toBe(false) + expect(detectLightMode({ COLORFGBG: '15;0', HERMES_TUI_THEME: 'light' })).toBe(true) + }) + + it('uses HERMES_TUI_BACKGROUND luminance when COLORFGBG is missing', async () => { + const { detectLightMode } = await importThemeWithCleanEnv() + + expect(detectLightMode({ HERMES_TUI_BACKGROUND: '#ffffff' })).toBe(true) + expect(detectLightMode({ HERMES_TUI_BACKGROUND: '#000000' })).toBe(false) + expect(detectLightMode({ HERMES_TUI_BACKGROUND: '#1e1e1e' })).toBe(false) + // Three-char hex normalises like CSS. + expect(detectLightMode({ HERMES_TUI_BACKGROUND: '#fff' })).toBe(true) + // Garbage falls through to the default-dark path. + expect(detectLightMode({ HERMES_TUI_BACKGROUND: 'not-a-colour' })).toBe(false) + }) + + it('rejects partially-invalid hex instead of silently truncating', async () => { + const { detectLightMode } = await importThemeWithCleanEnv() + // `parseInt('fffgff'.slice(2,4), 16)` would return 15 — the strict + // regex must reject these inputs so they fall through to default- + // dark instead of producing a false-positive light reading. + expect(detectLightMode({ HERMES_TUI_BACKGROUND: '#fffgff' })).toBe(false) + expect(detectLightMode({ HERMES_TUI_BACKGROUND: 'ffggff' })).toBe(false) + expect(detectLightMode({ HERMES_TUI_BACKGROUND: '#xyz' })).toBe(false) + // Wrong length also rejected (no implicit padding/truncation). + expect(detectLightMode({ HERMES_TUI_BACKGROUND: '#fffff' })).toBe(false) + expect(detectLightMode({ HERMES_TUI_BACKGROUND: '#fffffff' })).toBe(false) + }) + + it('treats COLORFGBG as authoritative when present so it dominates the TERM_PROGRAM allow-list', async () => { + const { detectLightMode } = await importThemeWithCleanEnv() + // Inject a light-default allow-list so the precedence test is + // meaningful even though the production allow-list is empty. + const allowList = new Set(['Apple_Terminal']) + + // Sanity: the allow-list alone WOULD turn this terminal light. + expect(detectLightMode({ TERM_PROGRAM: 'Apple_Terminal' }, allowList)).toBe(true) + + // Dark COLORFGBG must beat the allow-list. + expect( + detectLightMode({ COLORFGBG: '15;0', TERM_PROGRAM: 'Apple_Terminal' }, allowList), + ).toBe(false) + }) }) describe('fromSkin', () => { - it('overrides banner colors', () => { + // `fromSkin` closes over DEFAULT_THEME (which is env-derived), so we + // must dynamic-import it after sterilizing env — otherwise an ambient + // HERMES_TUI_THEME=light would flip the base palette and make these + // assertions order-dependent on the developer's shell. + + it('overrides banner colors', async () => { + const { fromSkin } = await importThemeWithCleanEnv() + expect(fromSkin({ banner_title: '#FF0000' }, {}).color.gold).toBe('#FF0000') }) - it('preserves unset colors', () => { + it('preserves unset colors', async () => { + const { DEFAULT_THEME, fromSkin } = await importThemeWithCleanEnv() + expect(fromSkin({ banner_title: '#FF0000' }, {}).color.amber).toBe(DEFAULT_THEME.color.amber) }) - it('overrides branding', () => { + it('overrides branding', async () => { + const { fromSkin } = await importThemeWithCleanEnv() const { brand } = fromSkin({}, { agent_name: 'TestBot', prompt_symbol: '$' }) + expect(brand.name).toBe('TestBot') expect(brand.prompt).toBe('$') }) - it('defaults for empty skin', () => { + it('defaults for empty skin', async () => { + const { DEFAULT_THEME, fromSkin } = await importThemeWithCleanEnv() + expect(fromSkin({}, {}).color).toEqual(DEFAULT_THEME.color) expect(fromSkin({}, {}).brand.icon).toBe(DEFAULT_THEME.brand.icon) }) - it('passes banner logo/hero', () => { + it('passes banner logo/hero', async () => { + const { fromSkin } = await importThemeWithCleanEnv() + expect(fromSkin({}, {}, 'LOGO', 'HERO').bannerLogo).toBe('LOGO') expect(fromSkin({}, {}, 'LOGO', 'HERO').bannerHero).toBe('HERO') }) - it('maps ui_ color keys + cascades to status', () => { + it('maps ui_ color keys + cascades to status', async () => { + const { fromSkin } = await importThemeWithCleanEnv() const { color } = fromSkin({ ui_ok: '#008000' }, {}) + expect(color.ok).toBe('#008000') expect(color.statusGood).toBe('#008000') }) diff --git a/ui-tui/src/theme.ts b/ui-tui/src/theme.ts index daeedb3377..baff80abf1 100644 --- a/ui-tui/src/theme.ts +++ b/ui-tui/src/theme.ts @@ -179,23 +179,129 @@ export const LIGHT_THEME: Theme = { bannerHero: '' } -// Pick light vs dark. Explicit `HERMES_TUI_LIGHT` wins; otherwise sniff -// `COLORFGBG` (set by XFCE Terminal, rxvt, Terminal.app, etc.) — last field is the -// background ANSI index; 7/15 are the "white" slots most light themes emit (#11300). -export function detectLightMode(env: NodeJS.ProcessEnv = process.env): boolean { - const explicit = (env.HERMES_TUI_LIGHT ?? '').trim().toLowerCase() +const TRUE_RE = /^(?:1|true|yes|on)$/ +const FALSE_RE = /^(?:0|false|no|off)$/ - if (/^(?:1|true|yes|on)$/.test(explicit)) { +// Reserved for future TERM_PROGRAM-based heuristics. Empty by default: +// most modern terminals (Ghostty, Warp, iTerm2, Apple_Terminal) ship a +// dark profile out of the box, so guessing wrong here is more annoying +// than missing a light user — light users can always set +// `HERMES_TUI_LIGHT=1` or `HERMES_TUI_THEME=light`. +const LIGHT_DEFAULT_TERM_PROGRAMS = new Set() + +// Best-effort RGB → luminance check. Currently only accepts a 3- or +// 6-digit hex value (with or without a leading `#`); the env var name +// `HERMES_TUI_BACKGROUND` is intentionally generic so a future OSC11 +// query helper can cache its answer there too, but additional formats +// (rgb()/hsl()/named colours) would need explicit parsing here first. +const LUMA_LIGHT_THRESHOLD = 0.6 + +// Strict allow-list: parseInt(..., 16) silently truncates at the first +// non-hex character (e.g. `fffgff` would parse as `fff` and yield a +// false-positive "white" reading), so reject anything that doesn't match +// the canonical 3- or 6-digit shape up front. +const HEX_3_RE = /^[0-9a-f]{3}$/ +const HEX_6_RE = /^[0-9a-f]{6}$/ + +function backgroundLuminance(raw: string): null | number { + const v = raw.trim().toLowerCase() + + if (!v) { + return null + } + + const hex = v.startsWith('#') ? v.slice(1) : v + const rgb = HEX_6_RE.test(hex) + ? [parseInt(hex.slice(0, 2), 16), parseInt(hex.slice(2, 4), 16), parseInt(hex.slice(4, 6), 16)] + : HEX_3_RE.test(hex) + ? [parseInt(hex[0]! + hex[0]!, 16), parseInt(hex[1]! + hex[1]!, 16), parseInt(hex[2]! + hex[2]!, 16)] + : null + + if (!rgb) { + return null + } + + // Rec. 709 luma — close enough for "is this background bright". + return (0.2126 * rgb[0]! + 0.7152 * rgb[1]! + 0.0722 * rgb[2]!) / 255 +} + +// Pick light vs dark with ordered, explainable signals (#11300): +// +// 1. `HERMES_TUI_LIGHT` boolean — `1`/`true`/`yes`/`on` → light; +// `0`/`false`/`no`/`off` → dark. Either explicit value wins +// regardless of any later signal. +// 2. `HERMES_TUI_THEME` named override — `light` / `dark` win over +// every signal below. +// 3. `HERMES_TUI_BACKGROUND` hex hint (3- or 6-digit) — luminance +// ≥ LUMA_LIGHT_THRESHOLD → light. +// 4. `COLORFGBG` last field — XFCE / rxvt / Terminal.app emit +// slot 7 or 15 on light profiles; 0–15 ranges are otherwise +// treated as authoritatively dark so the TERM_PROGRAM +// allow-list below cannot override an explicit dark profile. +// 5. `TERM_PROGRAM` light-default allow-list (currently empty). +// +// Anything we can't decide stays dark — the default Hermes palette +// is the dark one. +export function detectLightMode( + env: NodeJS.ProcessEnv = process.env, + // Injectable so tests can prove the COLORFGBG-over-TERM_PROGRAM + // precedence rule even though the production allow-list is empty. + lightDefaultTermPrograms: ReadonlySet = LIGHT_DEFAULT_TERM_PROGRAMS, +): boolean { + const lightFlag = (env.HERMES_TUI_LIGHT ?? '').trim().toLowerCase() + + if (TRUE_RE.test(lightFlag)) { return true } - if (/^(?:0|false|no|off)$/.test(explicit)) { + if (FALSE_RE.test(lightFlag)) { return false } - const bg = Number((env.COLORFGBG ?? '').trim().split(';').at(-1)) + const themeFlag = (env.HERMES_TUI_THEME ?? '').trim().toLowerCase() - return bg === 7 || bg === 15 + if (themeFlag === 'light') { + return true + } + + if (themeFlag === 'dark') { + return false + } + + const bgHint = backgroundLuminance(env.HERMES_TUI_BACKGROUND ?? '') + + if (bgHint !== null) { + return bgHint >= LUMA_LIGHT_THRESHOLD + } + + const colorfgbg = (env.COLORFGBG ?? '').trim() + + if (colorfgbg) { + // Validate as a decimal integer before coercing — `Number('')` is 0, + // so a malformed `COLORFGBG='15;'` would otherwise look like an + // authoritative dark slot and incorrectly block the TERM_PROGRAM + // allow-list. Anything that isn't pure digits falls through. + const lastField = colorfgbg.split(';').at(-1) ?? '' + + if (/^\d+$/.test(lastField)) { + const bg = Number(lastField) + + if (bg === 7 || bg === 15) { + return true + } + + // Slots 0–6 and 8–14 are the dark half of the 0–15 ANSI range. + // When COLORFGBG is set we trust it as authoritative — a non-light + // value here shouldn't get overridden by the TERM_PROGRAM allow-list. + if (bg >= 0 && bg < 16) { + return false + } + } + } + + const termProgram = (env.TERM_PROGRAM ?? '').trim() + + return lightDefaultTermPrograms.has(termProgram) } export const DEFAULT_THEME: Theme = detectLightMode() ? LIGHT_THEME : DARK_THEME