From cc3c956f26f46d1e110330994b266e2efb11ade7 Mon Sep 17 00:00:00 2001 From: oldschoola Date: Thu, 18 Jun 2026 22:14:11 -0700 Subject: [PATCH] fix(coding-agent/modes): guarded theme getters against undefined theme before init - Added `typeof theme === "undefined"` guards to all four theme getters. - Fallbacks return a usable plain-ASCII or plain-text theme instead of crashing. - Added a pre-init test asserting the getters never throw. --- packages/coding-agent/CHANGELOG.md | 4 ++ .../coding-agent/src/modes/theme/theme.ts | 69 +++++++++++++++++++ .../modes/theme/theme-getters-preinit.test.ts | 57 +++++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 packages/coding-agent/test/modes/theme/theme-getters-preinit.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index ae4c6891c..ed694d2d4 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Made `getSettingsListTheme`, `getEditorTheme`, `getSelectListTheme`, and `getSymbolTheme` return a plain ASCII fallback instead of crashing with "undefined is not an object (evaluating 'theme.fg')" when the global `theme` is undefined — e.g. when a plugin calls them before `initTheme()` completes or from a separate module instance under npm-global installs. ([#2998](https://github.com/can1357/oh-my-pi/issues/2998)) + ## [16.0.11] - 2026-06-19 ### Added diff --git a/packages/coding-agent/src/modes/theme/theme.ts b/packages/coding-agent/src/modes/theme/theme.ts index 4736771da..09f0fa6c6 100644 --- a/packages/coding-agent/src/modes/theme/theme.ts +++ b/packages/coding-agent/src/modes/theme/theme.ts @@ -2729,6 +2729,35 @@ export function highlightCode(code: string, lang?: string, highlightTheme: Theme } export function getSymbolTheme(): SymbolTheme { + // Guard against `theme` being undefined (pre-init or cross-module-instance + // plugin calls). Fall back to the ASCII preset so the returned symbols are + // usable instead of crashing. See #2998. + if (typeof theme === "undefined") { + const box = { + topLeft: "+", + topRight: "+", + bottomLeft: "+", + bottomRight: "+", + horizontal: "-", + vertical: "|", + cross: "+", + teeDown: "+", + teeUp: "+", + teeLeft: "+", + teeRight: "+", + }; + return { + cursor: ">", + inputCursor: "|", + boxRound: box, + boxSharp: box, + table: box, + quoteBorder: "|", + hrChar: "-", + colorSwatch: "[]", + spinnerFrames: ["-", "\\", "|", "/"], + }; + } const preset = theme.getSymbolPreset(); return { @@ -2794,6 +2823,19 @@ export function getMarkdownTheme(): MarkdownTheme { } export function getSelectListTheme(): SelectListTheme { + // Guard against `theme` being undefined (pre-init or cross-module-instance + // plugin calls). See #2998. + if (typeof theme === "undefined") { + return { + selectedPrefix: (text: string) => text, + selectedText: (text: string) => text, + description: (text: string) => text, + scrollInfo: (text: string) => text, + noMatch: (text: string) => text, + symbols: getSymbolTheme(), + hovered: (text: string) => text, + }; + } return { selectedPrefix: (text: string) => theme.fg("accent", text), selectedText: (text: string) => theme.fg("accent", text), @@ -2806,6 +2848,16 @@ export function getSelectListTheme(): SelectListTheme { } export function getEditorTheme(): EditorTheme { + // Guard against `theme` being undefined (pre-init or cross-module-instance + // plugin calls). See #2998. + if (typeof theme === "undefined") { + return { + borderColor: (text: string) => text, + selectList: getSelectListTheme(), + symbols: getSymbolTheme(), + hintStyle: (text: string) => text, + }; + } return { borderColor: (text: string) => theme.fg("borderMuted", text), selectList: getSelectListTheme(), @@ -2815,6 +2867,23 @@ export function getEditorTheme(): EditorTheme { } export function getSettingsListTheme(): SettingsListTheme { + // Plugins (e.g. pi-rtk-optimizer) may call this before `initTheme()` assigns + // the global `theme`, or from a separate module instance under npm-global + // installs where the live binding was never initialized. Fall back to plain + // text so the call returns a usable (unstyled) theme instead of crashing with + // "undefined is not an object (evaluating 'theme.fg')". See #2998. + if (typeof theme === "undefined") { + return { + label: (text: string) => text, + value: (text: string) => text, + description: (text: string) => text, + cursor: "> ", + hint: (text: string) => text, + heading: (text: string) => text, + section: (text: string) => text, + hovered: (text: string) => text, + }; + } return { label: (text: string, selected: boolean, changed: boolean) => changed ? theme.fg("statusLineGitDirty", text) : selected ? theme.fg("accent", text) : text, diff --git a/packages/coding-agent/test/modes/theme/theme-getters-preinit.test.ts b/packages/coding-agent/test/modes/theme/theme-getters-preinit.test.ts new file mode 100644 index 000000000..4ab90c33a --- /dev/null +++ b/packages/coding-agent/test/modes/theme/theme-getters-preinit.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from "bun:test"; +import { + getEditorTheme, + getSelectListTheme, + getSettingsListTheme, + getSymbolTheme, + theme, +} from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; + +/** + * Contract for issue #2998: the exported theme-getter functions must not crash + * when `theme` is undefined (the state before `initTheme()` assigns the global, + * or when a plugin calls them from a separate module instance under npm-global + * installs where the live binding was never initialized). They must return a + * usable plain-text fallback instead of throwing "undefined is not an object". + * + * These tests intentionally do NOT call `initTheme()`. However, `theme` is a + * module-global shared across the test run; if another test file already + * initialized it, the guard path is skipped and we assert the styled path + * still works. Either way, the contract holds: the functions never throw. + */ +describe("theme-getters before initTheme (no crash when theme is undefined)", () => { + it("getSettingsListTheme returns a usable theme without throwing", () => { + const t = getSettingsListTheme(); + expect(typeof t.cursor).toBe("string"); + expect(t.label("x", false, false)).toBe("x"); + expect(t.value("y", false, false)).toBeTypeOf("string"); + }); + + it("getEditorTheme returns a usable theme without throwing", () => { + const t = getEditorTheme(); + expect(typeof t.borderColor("x")).toBe("string"); + expect(t.selectList).toBeDefined(); + expect(t.symbols).toBeDefined(); + }); + + it("getSelectListTheme returns a usable theme without throwing", () => { + const t = getSelectListTheme(); + expect(typeof t.selectedPrefix(">")).toBe("string"); + expect(t.symbols).toBeDefined(); + }); + + it("getSymbolTheme returns ASCII fallback symbols without throwing", () => { + const t = getSymbolTheme(); + expect(t.cursor).toBeTypeOf("string"); + expect(t.spinnerFrames.length).toBeGreaterThan(0); + expect(t.boxSharp.horizontal).toBeTypeOf("string"); + }); + + it("styled path still works when theme is initialized", () => { + if (typeof theme === "undefined") return; // guard not exercisable in this run + const t = getSettingsListTheme(); + // When theme is loaded, cursor is a styled string (non-empty, contains the + // accent color ANSI sequence or at least the cursor glyph). + expect(t.cursor.length).toBeGreaterThan(0); + }); +});