Skip to content

Fix NO_COLOR on TUI detail screens and add ROBOREV_COLOR_MODE env var#566

Open
strofimovsky wants to merge 2 commits intoroborev-dev:mainfrom
strofimovsky:fix/tui-color-mode
Open

Fix NO_COLOR on TUI detail screens and add ROBOREV_COLOR_MODE env var#566
strofimovsky wants to merge 2 commits intoroborev-dev:mainfrom
strofimovsky:fix/tui-color-mode

Conversation

@strofimovsky
Copy link
Contributor

Summary

  • Fix NO_COLOR=1 not working on TUI review/prompt detail screens (glamour was ignoring it)
  • Add ROBOREV_COLOR_MODE env var (auto|dark|light|none) for explicit TUI color theme control
  • Apply color mode to both glamour (markdown) and lipgloss (AdaptiveColor) layers
  • NO_COLOR takes precedence over ROBOREV_COLOR_MODE at all layers

Problem

glamour's NewTermRenderer defaults to TrueColor profile regardless of NO_COLOR. The main queue screen (lipgloss only) correctly respected NO_COLOR, but review/prompt detail screens (glamour-rendered markdown) always emitted ANSI color sequences.

Additionally, there was no way to force a dark/light theme when terminal background auto-detection failed.

Changes

Core fix

  • Pass glamour.WithColorProfile() to all glamour.NewTermRenderer calls in both cmd/roborev/tui/helpers.go and internal/streamfmt/render.go
  • Fix stripTrailingPadding/StripTrailingPadding to skip the \x1b[0m reset suffix in no-color mode

ROBOREV_COLOR_MODE env var

  • auto (default): auto-detect terminal background
  • dark: force dark color palette
  • light: force light color palette
  • none: strip all colors (same as NO_COLOR=1)

Lipgloss integration

  • Call lipgloss.SetHasDarkBackground() / lipgloss.SetColorProfile() in TUI startup so AdaptiveColor styles on the queue screen also respect the env var
  • Check NO_COLOR first to ensure it always takes precedence

Consolidation

  • Remove duplicate resolveColorMode() from TUI helpers; delegate to streamfmt.GlamourStyle() + streamfmt.ResolveColorProfile()
  • Update GlamourStyle() to check NO_COLOR (previously skipped, causing unnecessary terminal background queries)

Documentation

  • Add ROBOREV_COLOR_MODE and NO_COLOR to README.md, AGENTS.md, and CLAUDE.md

Testing

  • All existing tests pass (go test ./...)
  • New tests: TestResolveColorProfile, TestRenderMarkdownLinesNoColor, TestGlamourStyleRespectsColorMode with proper dark/light style differentiation
  • Manual testing confirmed: NO_COLOR=1, ROBOREV_COLOR_MODE=none, =dark, =light all work on both queue and detail screens

@roborev-ci
Copy link

roborev-ci bot commented Mar 24, 2026

roborev: Combined Review (46c262d)

Verdict: The PR introduces a high-severity security vulnerability allowing arbitrary local code execution
and a medium-severity TUI formatting bug.

High

  • Location: internal/agent/acp.go#L1536, internal/config/config.go#L145
    Problem: opencode_cmd is a repo-loadable
    config value that directly overrides the executable path for the OpenCode agent. Because .roborev.toml is read from the repository and takes precedence over global config, a malicious/shared repo can point this field at a repo-controlled binary or script (e.g., ./tools/opencode), which will
    then be executed on the operator's machine when opencode is selected. This allows arbitrary local code execution via repository metadata.
    Fix: Do not honor executable-path overrides from repo config; restrict them to global config or CLI flags only. If repo-level support is strictly required, mandate explicit opt-in
    and validate that the path is absolute and rooted in a trusted user-controlled location.

Medium

  • Location: cmd/roborev/tui/helpers.go:476, internal/streamfmt/render.go:91
    Problem: In NO _COLOR/ROBOREV_COLOR_MODE=none mode, these helpers stop re-appending \x1b[0m after trimming trailing SGR sequences. Glamour can still emit non-color formatting codes in termenv.Ascii mode (such as bold). Removing
    the trailing reset leaves styles open and can cause subsequent lines to render with leaked formatting.
    Fix: Always restore a reset after trimming, or fully strip remaining non-color SGR sequences when color is disabled.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

glamour's NewTermRenderer defaults to TrueColor profile, ignoring the
NO_COLOR convention. This caused review/prompt detail screens to emit
ANSI color sequences even with NO_COLOR=1, while the main queue screen
(using lipgloss only) correctly suppressed colors.

Pass glamour.WithColorProfile() when creating term renderers in both
the TUI markdown cache and the streamfmt package. When NO_COLOR is set
or ROBOREV_COLOR_MODE=none, the Ascii profile is used, which strips
all color output from glamour.

Also fix stripTrailingPadding/StripTrailingPadding to skip the
unconditional \x1b[0m reset suffix in no-color mode.

Add ROBOREV_COLOR_MODE env var (auto|dark|light|none) for explicit
control over the TUI color theme without relying on terminal detection.
GlamourStyle() and resolveColorMode() both respect this env var.
glamour's NewTermRenderer defaults to TrueColor profile, ignoring the
NO_COLOR convention. Pass glamour.WithColorProfile() to all term
renderers so NO_COLOR=1 strips colors from review/prompt detail screens.

Add ROBOREV_COLOR_MODE env var (auto|dark|light|none) for explicit
control over the TUI color theme. Apply the setting to both the glamour
markdown layer and the lipgloss AdaptiveColor layer via
SetHasDarkBackground/SetColorProfile on the default renderer.

NO_COLOR takes precedence over ROBOREV_COLOR_MODE at all layers.

Consolidate color resolution: remove duplicate resolveColorMode() from
TUI helpers, delegate to streamfmt.GlamourStyle() and
streamfmt.ResolveColorProfile(). GlamourStyle() now checks NO_COLOR to
avoid unnecessary terminal background queries.
@roborev-ci
Copy link

roborev-ci bot commented Mar 24, 2026

roborev: Combined Review (6251457)

Verdict: One medium severity finding regarding unclosed text formatting sequences when colors
are disabled.

Medium

  • Location: cmd/roborev/tui/helpers.go:476, internal/streamfmt/render.go:91
  • Problem: stripTrailingPadding/StripTrailingPadding omit the trailing reset when noColor is
    enabled, but Glamour still emits non-color SGR sequences (e.g., bold/underline) in termenv.Ascii mode. Removing the reset at the end of the line can leave attributes open, causing subsequent lines to inherit formatting unexpectedly under NO_COLOR / ROBO REV_COLOR_MODE=none.
  • Fix: Keep the final reset even in no-color mode, or strip all remaining SGR sequences when noColor is enabled to prevent open formatting state from bleeding across lines.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants