From 7402e7292118ea12475961f7679aca9e43a21d28 Mon Sep 17 00:00:00 2001 From: Ed Zynda Date: Thu, 30 Jul 2026 13:55:14 +0300 Subject: [PATCH 1/2] feat(ui): resolve terminal capabilities lazily and per-client (#101) - Replace the package-init probes in internal/ui/style (isDarkBg, termColorProfile) with a mutex-guarded, lazily-resolved capability holder so the blocking OSC background query no longer fires at import time against arbitrary process fds - Add SetTerminalCapabilities for frontends to supply the terminal they actually serve, and ResolveTerminalCapabilities to probe eagerly - Defer DefaultTheme derivation until first use; rebuild a derived theme on a capability change while preserving an explicitly set theme - Resolve capabilities in cmd/root.go before the TUI takes over stdin to keep the query off the render path - Re-export the new entry points from internal/ui and cover the override, derived-theme rebuild, and explicit-theme preservation with tests First step of decoupling session/terminal state from the TUI layer. --- cmd/root.go | 6 + internal/ui/exports.go | 6 + internal/ui/style/enhanced.go | 144 +++++++++++++++++++++--- internal/ui/style/logo_test.go | 18 +-- internal/ui/style/terminal_caps_test.go | 93 +++++++++++++++ internal/ui/style/themes.go | 4 +- 6 files changed, 246 insertions(+), 25 deletions(-) create mode 100644 internal/ui/style/terminal_caps_test.go diff --git a/cmd/root.go b/cmd/root.go index 8383de61..775ee5b5 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -1760,6 +1760,12 @@ func runInteractiveModeBubbleTea(_ context.Context, deps runModeDeps) error { MCPResourceReader: mcpResourceReader, }) + // Resolve terminal capabilities (background, colour profile) now, before + // the TUI takes over stdin. The detection runs a synchronous OSC query, so + // it must happen here rather than lazily mid-render where it would race the + // event loop. No-op if a theme in config already resolved them. + ui.ResolveTerminalCapabilities() + program := tea.NewProgram(appModel) // Register the program with the app layer so agent events are sent to the TUI. diff --git a/internal/ui/exports.go b/internal/ui/exports.go index dcf949e6..75d48313 100644 --- a/internal/ui/exports.go +++ b/internal/ui/exports.go @@ -67,4 +67,10 @@ var ( AdaptiveColor = style.AdaptiveColor IsDarkBackground = style.IsDarkBackground SyntaxStyle = style.SyntaxStyle + + // SetTerminalCapabilities and ResolveTerminalCapabilities let a frontend + // control per-terminal styling inputs instead of relying on a process-global + // probe. See internal/ui/style. + SetTerminalCapabilities = style.SetTerminalCapabilities + ResolveTerminalCapabilities = style.ResolveTerminalCapabilities ) diff --git a/internal/ui/style/enhanced.go b/internal/ui/style/enhanced.go index aa604cec..f1c17b9c 100644 --- a/internal/ui/style/enhanced.go +++ b/internal/ui/style/enhanced.go @@ -6,6 +6,7 @@ import ( "math" "os" "strings" + "sync" "charm.land/lipgloss/v2" "github.com/alecthomas/chroma/v2" @@ -16,11 +17,102 @@ import ( // Enhanced styling utilities and theme definitions -// isDarkBg caches the terminal background detection result at package init. -var isDarkBg = lipgloss.HasDarkBackground(os.Stdin, os.Stdout) +// terminalCapabilities captures the per-terminal styling inputs: whether the +// background is dark (drives adaptive colour selection) and the terminal's +// colour depth (drives whether smooth animation is worthwhile). +type terminalCapabilities struct { + darkBackground bool + colorProfile colorprofile.Profile +} + +// termCaps holds the resolved terminal capabilities. It is nil until first use +// or until SetTerminalCapabilities is called, so the (blocking, fd-touching) +// probe never runs at package-init time. Guarded by termCapsMu. +var ( + termCapsMu sync.RWMutex + termCaps *terminalCapabilities +) + +// resolveTerminalCapabilities probes the current process's terminal. It is the +// default source of capabilities when SetTerminalCapabilities has not been +// called, and is kept separate so it never fires as an import-time side effect. +func resolveTerminalCapabilities() terminalCapabilities { + return terminalCapabilities{ + darkBackground: lipgloss.HasDarkBackground(os.Stdin, os.Stdout), + colorProfile: colorprofile.Env(os.Environ()), + } +} + +// currentTerminalCapabilities returns the active capabilities, resolving them +// lazily against the process terminal the first time they are needed. Callers +// that serve a specific client should call SetTerminalCapabilities first so +// this never falls back to whatever fds the process happens to hold. +func currentTerminalCapabilities() terminalCapabilities { + termCapsMu.RLock() + c := termCaps + termCapsMu.RUnlock() + if c != nil { + return *c + } -// termColorProfile caches the terminal's colour depth at package init. -var termColorProfile = colorprofile.Env(os.Environ()) + termCapsMu.Lock() + defer termCapsMu.Unlock() + if termCaps == nil { + resolved := resolveTerminalCapabilities() + termCaps = &resolved + } + return *termCaps +} + +// SetTerminalCapabilities overrides the terminal background and colour profile +// used by every subsequent styling decision in this package. +// +// It exists so a frontend can supply the capabilities of the terminal it is +// actually serving instead of inheriting whatever the process's own fds report. +// This replaces the former package-init probes, which fired once against the +// process stdin/stdout and could not represent more than one terminal. Safe to +// call from any goroutine. +func SetTerminalCapabilities(darkBackground bool, colorProfile colorprofile.Profile) { + termCapsMu.Lock() + termCaps = &terminalCapabilities{ + darkBackground: darkBackground, + colorProfile: colorProfile, + } + termCapsMu.Unlock() + + // A lazily-derived DefaultTheme baked in the previously-detected + // background, so drop it and let GetTheme rebuild against the new + // capabilities. A theme the caller chose explicitly is left as-is. + if !themeExplicitlySet { + currentTheme = nil + } + invalidateThemeCaches() +} + +// isDarkBackground reports whether the active terminal has a dark background, +// resolving capabilities lazily on first use. +func isDarkBackground() bool { + return currentTerminalCapabilities().darkBackground +} + +// ResolveTerminalCapabilities eagerly resolves the terminal background and +// colour profile against the current process terminal, unless they have +// already been resolved or supplied via SetTerminalCapabilities. +// +// The interactive frontend calls this once during startup, before the TUI +// takes over the terminal, so the background-detection OSC query runs at a +// point where it cannot race the event loop's reads of stdin. Headless +// frontends should call SetTerminalCapabilities instead and skip this, to avoid +// probing fds that do not describe their client. +func ResolveTerminalCapabilities() { + _ = currentTerminalCapabilities() +} + +// terminalColorProfile returns the active terminal's colour depth, resolving +// capabilities lazily on first use. +func terminalColorProfile() colorprofile.Profile { + return currentTerminalCapabilities().colorProfile +} // SupportsSmoothAnimation reports whether the terminal has the colour depth // for a gradient animation to read as motion. @@ -30,21 +122,30 @@ var termColorProfile = colorprofile.Env(os.Environ()) // effect. Honouring the profile also means NO_COLOR and non-TTY output opt out // for free. func SupportsSmoothAnimation() bool { - return termColorProfile >= colorprofile.ANSI256 + return terminalColorProfile() >= colorprofile.ANSI256 } // AdaptiveColor picks between a light-mode and dark-mode hex color string // based on the detected terminal background. This replaces the old // lipgloss.AdaptiveColor{Light: ..., Dark: ...} pattern from v1. func AdaptiveColor(light, dark string) color.Color { - if isDarkBg { + if isDarkBackground() { return lipgloss.Color(dark) } return lipgloss.Color(light) } -// Global theme instance -var currentTheme = DefaultTheme() +// currentTheme holds the active UI theme. It is nil until first use so that +// DefaultTheme — which calls AdaptiveColor and would therefore probe the +// terminal — is not evaluated at package-init time. Resolved lazily by +// GetTheme, or set explicitly by SetTheme. +var currentTheme *Theme + +// themeExplicitlySet records whether the active theme came from SetTheme (true) +// or was lazily derived from DefaultTheme (false). A derived theme bakes in the +// terminal background detected when it was built, so SetTerminalCapabilities +// must discard and rebuild it; an explicitly chosen theme is left untouched. +var themeExplicitlySet bool // themeGeneration counts theme changes. It starts at 1 so that a zero-valued // generation stamp — the state of any freshly allocated cache — never compares @@ -55,9 +156,14 @@ var currentTheme = DefaultTheme() var themeGeneration uint64 = 1 // GetTheme returns the currently active UI theme. The theme controls all color -// and styling decisions throughout the application's interface. +// and styling decisions throughout the application's interface. On first use it +// lazily derives DefaultTheme against the active terminal capabilities. func GetTheme() Theme { - return currentTheme + if currentTheme == nil { + derived := DefaultTheme() + currentTheme = &derived + } + return *currentTheme } // ThemeGeneration returns a counter that increments on every theme change. @@ -76,7 +182,15 @@ func ThemeGeneration() uint64 { // It also invalidates the markdownTypographyCache so the next call to // GetMarkdownTypography picks up the new theme. func SetTheme(theme Theme) { - currentTheme = theme + currentTheme = &theme + themeExplicitlySet = true + invalidateThemeCaches() +} + +// invalidateThemeCaches bumps the theme generation and drops every cache whose +// contents depend on the active theme colours, so the next access rebuilds +// them. Shared by SetTheme and SetTerminalCapabilities. +func invalidateThemeCaches() { themeGeneration++ // invalidate generation-stamped render caches markdownTypographyCache = nil // invalidate cached renderer; colors may have changed uiTypographyCache = nil // invalidate cached block typography; colors may have changed @@ -266,9 +380,11 @@ func DefaultTheme() Theme { } } -// IsDarkBackground returns the cached terminal background detection result. +// IsDarkBackground returns whether the active terminal has a dark background. +// Capabilities are resolved lazily on first use, or supplied ahead of time via +// SetTerminalCapabilities. func IsDarkBackground() bool { - return isDarkBg + return isDarkBackground() } // CreateBadge generates a styled badge or label with inverted colors (text on @@ -408,7 +524,7 @@ const ( // subtler there. That is a smaller cost than inverting the gesture. func logoShineColor() color.Color { amount := logoShineLightenLight - if isDarkBg { + if isDarkBackground() { amount = logoShineLightenDark } return blendColors(GetTheme().Primary, color.RGBA{R: 0xFF, G: 0xFF, B: 0xFF, A: 0xFF}, amount) diff --git a/internal/ui/style/logo_test.go b/internal/ui/style/logo_test.go index a48a22cb..c894a8d8 100644 --- a/internal/ui/style/logo_test.go +++ b/internal/ui/style/logo_test.go @@ -213,11 +213,11 @@ func relativeLuminance(c color.Color) float64 { // named for: an earlier contrast-pole rule inverted on pale palettes and swept // something *darker* across the wordmark, which reads as a smudge. func TestLogoShineIsAlwaysLighterThanWordmark(t *testing.T) { - origTheme, origDark := GetTheme(), isDarkBg - t.Cleanup(func() { isDarkBg = origDark; SetTheme(origTheme) }) + origTheme, origDark, origProfile := GetTheme(), isDarkBackground(), terminalColorProfile() + t.Cleanup(func() { SetTerminalCapabilities(origDark, origProfile); SetTheme(origTheme) }) for _, dark := range []bool{true, false} { - isDarkBg = dark + SetTerminalCapabilities(dark, origProfile) for name, th := range builtinThemes() { SetTheme(th) shine := relativeLuminance(logoShineColor()) @@ -235,8 +235,8 @@ func TestLogoShineIsAlwaysLighterThanWordmark(t *testing.T) { // Whatever theme is active, the shine has to be far enough from the wordmark's // own colours to register as a highlight crossing it. func TestLogoShineContrastsWithWordmarkInEveryTheme(t *testing.T) { - origTheme, origDark := GetTheme(), isDarkBg - t.Cleanup(func() { isDarkBg = origDark; SetTheme(origTheme) }) + origTheme, origDark, origProfile := GetTheme(), isDarkBackground(), terminalColorProfile() + t.Cleanup(func() { SetTerminalCapabilities(origDark, origProfile); SetTheme(origTheme) }) // Floor sits just below the measured worst case (~46, catppuccin on dark // and vesper on light). Those two palettes have an unusually pale Primary, @@ -244,7 +244,7 @@ func TestLogoShineContrastsWithWordmarkInEveryTheme(t *testing.T) { const minDistance = 40 for _, dark := range []bool{true, false} { - isDarkBg = dark + SetTerminalCapabilities(dark, origProfile) for name, th := range builtinThemes() { SetTheme(th) shine := logoShineColor() @@ -266,14 +266,14 @@ func TestLogoShineContrastsWithWordmarkInEveryTheme(t *testing.T) { // lift toward white from simply being turned up: on a light background it // closes on the page fast. func TestLogoShineDoesNotCollideWithBackground(t *testing.T) { - origTheme, origDark := GetTheme(), isDarkBg - t.Cleanup(func() { isDarkBg = origDark; SetTheme(origTheme) }) + origTheme, origDark, origProfile := GetTheme(), isDarkBackground(), terminalColorProfile() + t.Cleanup(func() { SetTerminalCapabilities(origDark, origProfile); SetTheme(origTheme) }) // Worst measured case is ~53 (vesper on a light background). const minDistance = 45 for _, dark := range []bool{true, false} { - isDarkBg = dark + SetTerminalCapabilities(dark, origProfile) for name, th := range builtinThemes() { SetTheme(th) if d := distance(logoShineColor(), th.Background); d < minDistance { diff --git a/internal/ui/style/terminal_caps_test.go b/internal/ui/style/terminal_caps_test.go new file mode 100644 index 00000000..9bc9d2c8 --- /dev/null +++ b/internal/ui/style/terminal_caps_test.go @@ -0,0 +1,93 @@ +package style + +import ( + "testing" + + "charm.land/lipgloss/v2" + "github.com/charmbracelet/colorprofile" +) + +// saveStyleState snapshots the package-global styling state these tests mutate +// and returns a restore function, so a test never leaks capabilities or a theme +// into the ones that follow (this package's tests run non-parallel). +func saveStyleState(t *testing.T) { + t.Helper() + termCapsMu.Lock() + caps := termCaps + termCapsMu.Unlock() + theme := currentTheme + explicit := themeExplicitlySet + gen := themeGeneration + + t.Cleanup(func() { + termCapsMu.Lock() + termCaps = caps + termCapsMu.Unlock() + currentTheme = theme + themeExplicitlySet = explicit + themeGeneration = gen + styleCache = nil + markdownTypographyCache = nil + uiTypographyCache = nil + syntaxStyleCache = nil + }) +} + +func TestSetTerminalCapabilitiesOverridesDetection(t *testing.T) { + saveStyleState(t) + + SetTerminalCapabilities(true, colorprofile.TrueColor) + if !IsDarkBackground() { + t.Fatal("IsDarkBackground() = false after setting dark background") + } + if !SupportsSmoothAnimation() { + t.Fatal("SupportsSmoothAnimation() = false for a TrueColor profile") + } + + SetTerminalCapabilities(false, colorprofile.Ascii) + if IsDarkBackground() { + t.Fatal("IsDarkBackground() = true after setting light background") + } + if SupportsSmoothAnimation() { + t.Fatal("SupportsSmoothAnimation() = true for an Ascii profile") + } +} + +func TestSetTerminalCapabilitiesRebuildsDerivedTheme(t *testing.T) { + saveStyleState(t) + + // Force a fresh, lazily-derived theme for a light terminal. + currentTheme = nil + themeExplicitlySet = false + SetTerminalCapabilities(false, colorprofile.TrueColor) + lightText := GetTheme().Text + + // Switching the terminal background must rebuild the derived theme so the + // adaptive colours flip to the dark-mode branch. + SetTerminalCapabilities(true, colorprofile.TrueColor) + darkText := GetTheme().Text + + if colorsEqual(lightText, darkText) { + t.Fatalf("derived theme Text did not change with background: %v", lightText) + } +} + +func TestSetTerminalCapabilitiesPreservesExplicitTheme(t *testing.T) { + saveStyleState(t) + + custom := DefaultTheme() + custom.Text = lipgloss.Color("#123456") + SetTheme(custom) + + // An explicitly chosen theme must survive a capability change untouched. + SetTerminalCapabilities(true, colorprofile.TrueColor) + if got := GetTheme().Text; !colorsEqual(got, lipgloss.Color("#123456")) { + t.Fatalf("explicit theme Text changed after SetTerminalCapabilities: got %v", got) + } +} + +func colorsEqual(a, b interface{ RGBA() (r, g, b, a uint32) }) bool { + ar, ag, ab, aa := a.RGBA() + br, bg, bb, ba := b.RGBA() + return ar == br && ag == bg && ab == bb && aa == ba +} diff --git a/internal/ui/style/themes.go b/internal/ui/style/themes.go index f67f47de..e518760e 100644 --- a/internal/ui/style/themes.go +++ b/internal/ui/style/themes.go @@ -61,7 +61,7 @@ func blendHex(base, tint string, amount float64) string { func deriveInputBg(bgPair [2]string) color.Color { idx := 0 contrast := "#000000" - if isDarkBg { + if isDarkBackground() { idx = 1 contrast = "#ffffff" } @@ -89,7 +89,7 @@ func deriveDiffBg(bgPair, successPair, errorPair [2]string) (diffInsert, diffDel // Pick the correct index based on detected background. idx := 0 - if isDarkBg { + if isDarkBackground() { idx = 1 } insL, delL, eqL, missL := derive(idx) From 36262fe4ba9ee403166cb1842205b07ac17eb94a Mon Sep 17 00:00:00 2001 From: Ed Zynda Date: Thu, 30 Jul 2026 14:07:53 +0300 Subject: [PATCH 2/2] fix(ui): correct SetTerminalCapabilities goroutine-safety contract (#101) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address CodeRabbit review on PR #102. - enhanced.go: SetTerminalCapabilities claimed "safe to call from any goroutine" but mutated currentTheme/themeExplicitlySet and rebuilt the theme-derived caches (generation counter, typography/style/syntax) with no lock, while the render loop reads them unsynchronized. Narrow the contract to match SetTheme (call on the UI goroutine); the capability swap itself stays mutex-guarded. Documented the shared UI-goroutine discipline on currentTheme/themeExplicitlySet (🔴 critical, valid). - Skipped: cmd/root.go nitpick to wire detection through tea.RequestBackgroundColor. Out of scope for this step — the PR preserves the pre-existing os.Stdin/os.Stdout probe for behaviour parity, and SetTerminalCapabilities is the seam a future Init/Update-based (Wish/SSH-aware) wiring will use (🔵 trivial). --- internal/ui/style/enhanced.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/internal/ui/style/enhanced.go b/internal/ui/style/enhanced.go index f1c17b9c..f9a22c38 100644 --- a/internal/ui/style/enhanced.go +++ b/internal/ui/style/enhanced.go @@ -70,8 +70,13 @@ func currentTerminalCapabilities() terminalCapabilities { // It exists so a frontend can supply the capabilities of the terminal it is // actually serving instead of inheriting whatever the process's own fds report. // This replaces the former package-init probes, which fired once against the -// process stdin/stdout and could not represent more than one terminal. Safe to -// call from any goroutine. +// process stdin/stdout and could not represent more than one terminal. +// +// Call this on the UI goroutine, the same as SetTheme: besides the +// mutex-guarded capability swap, it rebuilds theme-derived caches +// (currentTheme, the generation counter, the typography/style/syntax caches) +// that the render loop reads unsynchronized. It is intended to be called once +// during setup, before the render loop starts, or from within Update. func SetTerminalCapabilities(darkBackground bool, colorProfile colorprofile.Profile) { termCapsMu.Lock() termCaps = &terminalCapabilities{ @@ -139,6 +144,10 @@ func AdaptiveColor(light, dark string) color.Color { // DefaultTheme — which calls AdaptiveColor and would therefore probe the // terminal — is not evaluated at package-init time. Resolved lazily by // GetTheme, or set explicitly by SetTheme. +// +// Like themeGeneration below, this and themeExplicitlySet are mutated only on +// the UI goroutine (SetTheme / SetTerminalCapabilities) and read from the same +// Update/View cycle, so they carry no lock of their own. var currentTheme *Theme // themeExplicitlySet records whether the active theme came from SetTheme (true)