diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 1317f2044..43ac4efbe 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -29,6 +29,7 @@ ### Fixed +- Fixed startup status messages (warnings, errors, extension/tool errors, status lines) keeping the dark-mode color after auto-theme detection later switched the active theme to light — e.g. `dark-catppuccin`/`light-catppuccin` warnings rendered in Mocha yellow on the Latte background. Transient status presenters now resolve their color lazily at render time so a theme swap re-shapes them ([#6337](https://github.com/can1357/oh-my-pi/issues/6337)). - Fixed `error.notify` raising a "Stopped with error" toast for provider failures while an auto-retry or async-delivery continuation was pending; the toast now waits for the true terminal settle. - Fixed terminal `yield` results racing post-turn maintenance, which could trigger an unnecessary automatic handoff or compaction. - Fixed credential-shaped tokens (GitHub/GitLab/OpenAI/Anthropic key patterns) being redacted from outbound provider requests even with `secrets.enabled` off; the pattern redaction now follows the `secrets.enabled` ("Hide Secrets") setting like the secret obfuscator. diff --git a/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts b/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts index f38d965f7..117f16c8f 100644 --- a/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts +++ b/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts @@ -532,7 +532,7 @@ export class ExtensionUiController { * Show a tool error in the chat. */ showToolError(toolName: string, error: string): void { - const errorText = new Text(theme.fg("error", `Tool "${toolName}" error: ${error}`), 1, 0); + const errorText = new Text(`Tool "${toolName}" error: ${error}`, 1, 0).setStyleFn(t => theme.fg("error", t)); this.ctx.present(errorText); } @@ -1092,7 +1092,9 @@ export class ExtensionUiController { } showExtensionError(extensionPath: string, error: string): void { - const errorText = new Text(theme.fg("error", `Extension "${extensionPath}" error: ${error}`), 1, 0); + const errorText = new Text(`Extension "${extensionPath}" error: ${error}`, 1, 0).setStyleFn(t => + theme.fg("error", t), + ); this.ctx.present(errorText); } async #handleInteractiveCompact(instructionsOrOptions: string | CompactOptions | undefined): Promise { diff --git a/packages/coding-agent/src/modes/utils/ui-helpers.ts b/packages/coding-agent/src/modes/utils/ui-helpers.ts index 5192834e2..5cb931540 100644 --- a/packages/coding-agent/src/modes/utils/ui-helpers.ts +++ b/packages/coding-agent/src/modes/utils/ui-helpers.ts @@ -114,16 +114,19 @@ export class UiHelpers { const last = children.length > 0 ? children[children.length - 1] : undefined; const secondLast = children.length > 1 ? children[children.length - 2] : undefined; const useDim = options?.dim ?? true; - const rendered = useDim ? theme.fg("dim", message) : message; + // Resolve the dim color lazily so a later theme change re-shapes the line + // instead of leaving the palette that was active when it was presented. + const styleFn = useDim ? (t: string) => theme.fg("dim", t) : undefined; if (last && secondLast && last === this.ctx.lastStatusText && secondLast === this.ctx.lastStatusSpacer) { - this.ctx.lastStatusText.setText(rendered); + this.ctx.lastStatusText.setStyleFn(styleFn); + this.ctx.lastStatusText.setText(message); this.ctx.ui.requestRender(); return; } const spacer = new Spacer(1); - const text = new Text(rendered, 1, 0); + const text = new Text(message, 1, 0).setStyleFn(styleFn); this.ctx.present([spacer, text]); this.ctx.lastStatusSpacer = spacer; this.ctx.lastStatusText = text; @@ -714,11 +717,13 @@ export class UiHelpers { } showError(errorMessage: string): void { - this.ctx.present([new Spacer(1), new Text(theme.fg("error", `Error: ${errorMessage}`), 1, 0)]); + const text = new Text(`Error: ${errorMessage}`, 1, 0).setStyleFn(t => theme.fg("error", t)); + this.ctx.present([new Spacer(1), text]); } showWarning(warningMessage: string): void { - this.ctx.present([new Spacer(1), new Text(theme.fg("warning", `Warning: ${warningMessage}`), 1, 0)]); + const text = new Text(`Warning: ${warningMessage}`, 1, 0).setStyleFn(t => theme.fg("warning", t)); + this.ctx.present([new Spacer(1), text]); } showNewVersionNotification(newVersion: string): void { diff --git a/packages/coding-agent/test/theme-lazy-status-color.test.ts b/packages/coding-agent/test/theme-lazy-status-color.test.ts new file mode 100644 index 000000000..b955f26b6 --- /dev/null +++ b/packages/coding-agent/test/theme-lazy-status-color.test.ts @@ -0,0 +1,66 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import * as themeModule from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import { Text } from "@oh-my-pi/pi-tui"; + +/** + * Regression for issue #6337: a status message presented while the auto-theme + * default guess (dark) was active must re-resolve its color when the terminal's + * appearance reply later switches the active theme to light. The transient + * status presenters supply the color via `Text.setStyleFn` (evaluated at render + * time against the live `theme` binding) instead of baking `theme.fg()` into the + * component, so invalidating on `onThemeChange` re-shapes it. + */ + +/** Opening SGR sequence `theme.fg(color, ...)` emits, independent of color mode. */ +function fgPrefix(color: "warning"): string { + const styled = themeModule.theme.fg(color, "\u0001"); + return styled.slice(0, styled.indexOf("\u0001")); +} + +describe("lazy status color re-resolves on theme switch", () => { + beforeEach(async () => { + themeModule.stopThemeWatcher(); + const dark = await themeModule.getThemeByName("dark"); + if (!dark) throw new Error("Failed to load dark theme for tests"); + themeModule.setThemeInstance(dark); + vi.restoreAllMocks(); + }); + + afterEach(async () => { + themeModule.stopThemeWatcher(); + const dark = await themeModule.getThemeByName("dark"); + if (dark) themeModule.setThemeInstance(dark); + vi.restoreAllMocks(); + }); + + it("swaps a presented warning from dark-catppuccin to light-catppuccin color", async () => { + // Auto-theme resolves dark before the appearance reply arrives. + themeModule.onTerminalAppearanceChange("dark"); + await themeModule.initTheme(false, undefined, undefined, "dark-catppuccin", "light-catppuccin"); + expect(themeModule.getCurrentThemeName()).toBe("dark-catppuccin"); + + const darkPrefix = fgPrefix("warning"); + const warning = new Text("Warning: Failed to load extension", 1, 0).setStyleFn(t => + themeModule.theme.fg("warning", t), + ); + expect(warning.render(80).join("")).toContain(darkPrefix); + + // The OSC 11 reply arrives → auto-theme switches to light-catppuccin. + const switched = Promise.withResolvers(); + const off = themeModule.onThemeChange(() => switched.resolve()); + themeModule.onTerminalAppearanceChange("light"); + await switched.promise; + off(); + expect(themeModule.getCurrentThemeName()).toBe("light-catppuccin"); + + const lightPrefix = fgPrefix("warning"); + expect(lightPrefix).not.toBe(darkPrefix); + + // The onThemeChange handler invalidates + repaints; the warning must now + // render the light-mode color, not the baked dark one. + warning.invalidate(); + const out = warning.render(80).join(""); + expect(out).toContain(lightPrefix); + expect(out).not.toContain(darkPrefix); + }); +}); diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 13b39e774..8ffb14385 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- `Text.setStyleFn()` applies a foreground styler at render time, so a component re-resolves its color after `invalidate()` instead of baking the palette that was active when it was constructed. + ## [17.0.9] - 2026-07-23 ### Added diff --git a/packages/tui/src/components/text.ts b/packages/tui/src/components/text.ts index f18715eb5..753c60d35 100644 --- a/packages/tui/src/components/text.ts +++ b/packages/tui/src/components/text.ts @@ -11,13 +11,20 @@ import { } from "../utils"; /** - * Text component - displays multi-line text with word wrapping + * Text component - displays multi-line text with word wrapping. + * + * Foreground colors may be supplied lazily via {@link setStyleFn} instead of + * baked into `text`: the styler runs at render time, so a caller that + * invalidates the component on a theme change (see the coding-agent's + * `onThemeChange` handler) re-resolves the color against the now-active theme + * rather than replaying the palette active when the component was constructed. */ export class Text implements Component { #text: string; #paddingX: number; // Left/right padding #paddingY: number; // Top/bottom padding #customBgFn?: (text: string) => string; + #styleFn?: (text: string) => string; #ignoreTight = false; @@ -64,6 +71,21 @@ export class Text implements Component { this.#cachedLines = undefined; } + /** + * Supply a foreground styler applied to the text at render time (e.g. a + * theme color resolver). Unlike baking the color into `text`, the styler + * re-runs on every render, so invalidating the component after a theme + * change re-resolves the color against the active theme. + */ + setStyleFn(styleFn?: (text: string) => string): this { + this.#styleFn = styleFn; + this.#cachedText = undefined; + this.#cachedWidth = undefined; + this.#cachedWidthEpoch = undefined; + this.#cachedLines = undefined; + return this; + } + invalidate(): void { this.#cachedText = undefined; this.#cachedWidth = undefined; @@ -93,7 +115,7 @@ export class Text implements Component { } // Replace tabs with 3 spaces - const normalizedText = replaceTabs(this.#text); + const normalizedText = replaceTabs(this.#styleFn ? this.#styleFn(this.#text) : this.#text); // Calculate content width (subtract left/right margins) const paddingX = this.#ignoreTight ? this.#paddingX : getPaddingX(this.#paddingX); diff --git a/packages/tui/test/text.test.ts b/packages/tui/test/text.test.ts index 74e81f0f6..5eae5bfde 100644 --- a/packages/tui/test/text.test.ts +++ b/packages/tui/test/text.test.ts @@ -9,4 +9,28 @@ describe("Text component", () => { expect(text.setText("b")).toBe(true); expect(text.getText()).toBe("b"); }); + + it("applies the style fn at render time, not construction time", () => { + const text = new Text("hello", 0, 0).setStyleFn(t => `${t}`); + expect(text.render(40).join("\n")).toContain("hello"); + }); + + it("re-resolves the style fn after invalidate so a theme change re-shapes", () => { + // The styler reads a mutable `color`, standing in for the active theme. + // The coding-agent invalidates status components on `onThemeChange`, so a + // lazily-styled Text must pick up the new color on the next render — + // something a baked ANSI string can never do (issue #6337). + let color = "RED"; + const text = new Text("hello", 0, 0).setStyleFn(t => `[${color}]${t}`); + expect(text.render(40).join("\n")).toContain("[RED]hello"); + + // Without invalidation the cached render is returned unchanged. + color = "BLUE"; + expect(text.render(40).join("\n")).toContain("[RED]hello"); + + text.invalidate(); + const out = text.render(40).join("\n"); + expect(out).toContain("[BLUE]hello"); + expect(out).not.toContain("[RED]hello"); + }); });