fix(cli): ask the terminal for color support instead of inferring it from TERM

[shaokeyibb]

Summary

detectColorLevelFromEnv returned 0 for an unset TERM before considering any other signal. PowerShell and cmd.exe set no TERM at all, so the TUI emitted no ANSI whatsoever on native Windows shells while WSL and Git Bash stayed coloured. The same early return also swallowed an explicit COLORTERM=truecolor whenever TERM was unset — on every platform, not just Windows.

The function's stated benchmark already handles this. supports-color returns on process.platform === 'win32' (by OS build) before it reads TERM; the transcription kept the TERM ladder and dropped the platform branch. Node's tty.WriteStream.getColorDepth() implements the same Windows build check, so this asks the terminal instead of maintaining a second copy of the ladder.

Order is now: NO_COLOR → TERM=dumb → explicit truecolor declaration → reported depth → (no terminal to ask) the pre-existing TERM ladder. Linux and macOS behaviour is unchanged; the fallback keeps piped output exactly as it was.

Fixes #3536

Verification

End-to-end in a real pty with TERM unset — the case the bug is about:

before   REAL TTY, TERM unset => "XY●"
after    REAL TTY, TERM unset => "�[32mX�[39m�[38;2;87;163;239mY�[39m…"

24-bit output where there was previously not a single escape sequence.

Measured capability on this machine (Windows 11 10.0.26200, Windows Terminal), showing the detector disagreeing with the terminal:

shell	TERM	getColorDepth	before	after
PowerShell 5.1	unset	24	0	3
cmd.exe	unset	24	0	3
Git Bash	xterm-256color	24	2	3
WSL2	xterm-256color	8	2	2

New packages/cli/src/__tests__/tui-ansi.test.ts (5 tests) pins the matrix — there was no test for this function before. Checked against a plausible partial fix that adds the depth probe but keeps the empty-TERM early return: 2 of the 5 go red (expected: 3, actual: 0), including the COLORTERM case.

Ran: the new suite plus every existing consumer of tui-ansi (pi-goal, pi-tui-transcript-viewer, pi-transcript) — 84 green; biome check on both files; npm run check:asf-headers; tsc -p packages/cli.

Not run: the rest of the CLI suite, Desktop, Playwright. PowerShell 7 was not available on this machine, so only PowerShell 5.1 and cmd.exe were measured natively.

Review focus

getColorDepth exists only on tty.WriteStream, so process.stdout.getColorDepth?.() is undefined for piped output. That path deliberately falls through to the original TERM ladder rather than to 0, which keeps redirected output and the existing test environment byte-identical. Making non-TTY output colourless would be defensible, but it is a separate behaviour change and nothing in packages/cli reaches tui-ansi from a non-TUI path today.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code — investigation, the measurements above, the test, and the fix.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Copilot]

Pull request overview

Fixes CLI TUI color detection by using terminal-reported color depth while preserving existing environment fallbacks.

Changes:

• Adds stdout.getColorDepth() detection.
• Preserves NO_COLOR, TERM=dumb, truecolor, and legacy fallback behavior.
• Adds focused detection tests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
packages/cli/src/tui-ansi.ts	Uses terminal-reported color depth and retains fallback logic.
packages/cli/src/__tests__/tui-ansi.test.ts	Tests detection precedence, depth mapping, and fallback behavior.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Astro-Han]

Reviewed at 287915d3. No blocking findings. One naming nit and a note about CI.

The problem is real and the diagnosis is precise: PowerShell and cmd.exe set no TERM at all, and the old ladder's if (term === 'dumb' || term === '') return 0; turned that into "colorless" before anything else got a chance — so the TUI went monochrome on a console that supports 24-bit, while WSL and Git Bash stayed coloured. That is an ordinary path for every native-Windows user, not a constructed one.

What I want to call out, because it is the part most likely to be undervalued in a diff this size: this change removes a duplicated capability ladder rather than adding a special case. The obvious fix here is if (process.platform === 'win32') return 3; — a second Windows-specific branch that then has to be kept in sync with Node's own build checks forever. Delegating to getColorDepth() deletes that future maintenance instead of creating it, and the comment says so explicitly. Net effect on the ladder is one fewer thing to keep correct.

The precedence order also survives scrutiny. NO_COLOR (non-empty, per spec — the empty-string case is correctly excluded) and TERM=dumb still outrank everything; an explicit COLORTERM and a -truecolor / -24bit suffix outrank the probe, on the stated reasoning that a declaration is something the terminal knows and a probe may not; the depth mapping only runs when a depth exists; and term === '' still returns 0 on the no-tty path, which keeps piped output unchanged. process.stdout.getColorDepth?.() correctly yields undefined for a non-tty stdout rather than throwing.

The tests pin each of those branches, including the two that are easy to get backwards — NO_COLOR= empty not disabling color, and the piped-output fallback still using the old ladder.

[P3] The function name no longer describes what it does.

detectColorLevelFromEnv(env, depth) now decides from an env snapshot and a terminal capability probe, and in the common case the probe is what actually decides. Something like detectColorLevel(env, depth) would read truer. Purely cosmetic, and the doc comment does explain the second parameter — flagging it only because the FromEnv suffix is exactly the kind of stale name that later leads someone to pass undefined for depth thinking it is optional detail.

On CI: no checks have been reported on this head, so there is no CI evidence for it yet. Nothing looks wrong with the branch; it just has not run. Worth a maintainer kicking it off.

One thing I could not verify from here: whether any terminal that sets TERM=xterm-256color reports a getColorDepth() below 8, which would now downgrade it from 256 to 16. Node derives its depth partly from TERM itself, so I do not expect it, but I do not have a Windows or exotic-terminal environment to confirm against — noting it as a boundary rather than a finding.

[Astro-Han]

CI is now terminal green on 287915d3d3ad0c3e3197466b5b285f98a2a9e6d1 (test: completed / success), which matches the exact head this review is bound to.

Re-confirming the earlier review conclusion on this head: querying the terminal for color support instead of inferring it from TERM removes a heuristic rather than adding a layer on top of one — the TERM string stops being a second authority on capability. Scope is small and the change is contained.

Approving.