From 287915d3d3ad0c3e3197466b5b285f98a2a9e6d1 Mon Sep 17 00:00:00 2001 From: HikariLan Date: Sun, 23 Aug 2026 04:49:04 +0800 Subject: [PATCH] fix(cli): ask the terminal for color support instead of inferring it from TERM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PowerShell and cmd.exe set no TERM at all, and the detector returned 0 for an unset TERM before considering anything else — so the TUI emitted no ANSI at all on native Windows shells while WSL and Git Bash stayed coloured. The consoles do support truecolor; only the detection was wrong. The same early return also swallowed an explicit COLORTERM whenever TERM was unset, on every platform. The function's own benchmark, `supports-color`, avoids this by returning on process.platform === 'win32' before it reads TERM. Node's getColorDepth() already implements that check, so ask it rather than keeping a second copy of the ladder. NO_COLOR, TERM=dumb and the explicit truecolor upgrade still win, and piped output — which has no stream to ask — keeps the old TERM ladder. Generated-by: Claude Code Co-Authored-By: Claude Opus 5 (1M context) --- packages/cli/src/__tests__/tui-ansi.test.ts | 70 +++++++++++++++++++++ packages/cli/src/tui-ansi.ts | 64 ++++++++++++++----- 2 files changed, 117 insertions(+), 17 deletions(-) create mode 100644 packages/cli/src/__tests__/tui-ansi.test.ts diff --git a/packages/cli/src/__tests__/tui-ansi.test.ts b/packages/cli/src/__tests__/tui-ansi.test.ts new file mode 100644 index 0000000000..d77575ca59 --- /dev/null +++ b/packages/cli/src/__tests__/tui-ansi.test.ts @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import { describe, test } from 'node:test'; +import { detectColorLevelFromEnv } from '../tui-ansi.js'; + +/** Node's answer on a Windows 10+ console: truecolor, whatever TERM says. */ +const WINDOWS_CONSOLE_DEPTH = 24; +/** Node's answer for a 256-color xterm. */ +const XTERM_256_DEPTH = 8; +/** Node's answer when it finds no evidence of color support. */ +const NO_COLOR_DEPTH = 1; + +describe('detectColorLevelFromEnv', () => { + test('uses the terminal capability when TERM is unset — native Windows shells', () => { + // PowerShell and cmd.exe set no TERM at all. Reading that as "colorless" + // turns the whole TUI monochrome on a console that supports truecolor. + assert.equal(detectColorLevelFromEnv({}, WINDOWS_CONSOLE_DEPTH), 3); + }); + + test('honours an explicit COLORTERM even when TERM is unset', () => { + assert.equal(detectColorLevelFromEnv({ COLORTERM: 'truecolor' }, NO_COLOR_DEPTH), 3); + assert.equal(detectColorLevelFromEnv({ COLORTERM: '24bit' }, NO_COLOR_DEPTH), 3); + }); + + test('NO_COLOR and TERM=dumb still win over any capability', () => { + assert.equal( + detectColorLevelFromEnv({ NO_COLOR: '1', TERM: 'xterm-256color' }, WINDOWS_CONSOLE_DEPTH), + 0, + ); + assert.equal(detectColorLevelFromEnv({ TERM: 'dumb' }, WINDOWS_CONSOLE_DEPTH), 0); + // The NO_COLOR spec: an empty value does NOT disable color. + assert.equal( + detectColorLevelFromEnv({ NO_COLOR: '', TERM: 'xterm-256color' }, XTERM_256_DEPTH), + 2, + ); + }); + + test('maps the reported depth onto the three color levels', () => { + assert.equal(detectColorLevelFromEnv({ TERM: 'xterm-256color' }, XTERM_256_DEPTH), 2); + assert.equal(detectColorLevelFromEnv({ TERM: 'xterm' }, 4), 1); + assert.equal(detectColorLevelFromEnv({ TERM: 'xterm' }, NO_COLOR_DEPTH), 0); + }); + + test('falls back to the TERM ladder when there is no terminal to ask', () => { + // Piped output has no tty.WriteStream, so no depth is available. The + // pre-existing env ladder still answers, unchanged. + assert.equal(detectColorLevelFromEnv({ TERM: 'xterm-256color' }, undefined), 2); + assert.equal(detectColorLevelFromEnv({ TERM: 'xterm' }, undefined), 1); + assert.equal(detectColorLevelFromEnv({ TERM: 'xterm-truecolor' }, undefined), 3); + assert.equal(detectColorLevelFromEnv({}, undefined), 0); + }); +}); diff --git a/packages/cli/src/tui-ansi.ts b/packages/cli/src/tui-ansi.ts index 93e67d6492..8895d70071 100644 --- a/packages/cli/src/tui-ansi.ts +++ b/packages/cli/src/tui-ansi.ts @@ -91,38 +91,68 @@ function buildAnsi() { } function detectColorLevel(): 0 | 1 | 2 | 3 { - return detectColorLevelFromEnv({ - NO_COLOR: process.env.NO_COLOR, - TERM: process.env.TERM, - COLORTERM: process.env.COLORTERM, - }); + return detectColorLevelFromEnv( + { + NO_COLOR: process.env.NO_COLOR, + TERM: process.env.TERM, + COLORTERM: process.env.COLORTERM, + }, + // Only a tty.WriteStream can answer; piped output leaves this undefined. + process.stdout.getColorDepth?.(), + ); } /** - * Pure color level detection from an env snapshot. - * - 0: no color (NO_COLOR non-empty, or TERM is dumb/empty) + * Pure color level detection from an env snapshot and the terminal's own + * reported depth. + * - 0: no color (NO_COLOR non-empty, TERM is dumb, or nothing reports color) * - 1: 16-color (basic ANSI) - * - 2: 256-color (TERM contains 256color) - * - 3: 24-bit truecolor (COLORTERM=truecolor/24bit or TERM ends with -truecolor) + * - 2: 256-color + * - 3: 24-bit truecolor (COLORTERM=truecolor/24bit, TERM ending -truecolor, or + * a terminal reporting 24-bit depth) * * Benchmark: codex `supports-color` 3-level; pi `theme.ts` 256 fallback. + * + * `TERM` alone cannot answer this. Native Windows shells (PowerShell, cmd) set + * no `TERM` at all on consoles that do support truecolor, so treating an unset + * `TERM` as colorless made the whole TUI monochrome there while WSL and Git + * Bash stayed coloured (#3536). `supports-color` avoids that by returning on + * `process.platform === 'win32'` before it ever reads `TERM`; Node's + * `getColorDepth()` already implements that same Windows build check, so this + * asks the terminal rather than maintaining a second copy of the ladder. + * + * `depth` is the value from `tty.WriteStream.getColorDepth()`: 1 (none), 4 + * (16), 8 (256) or 24 (16m). It is undefined when stdout is not a terminal, + * which leaves the pre-existing `TERM` ladder as the fallback. */ -function detectColorLevelFromEnv(env: { - NO_COLOR?: string; - TERM?: string; - COLORTERM?: string; -}): 0 | 1 | 2 | 3 { +export function detectColorLevelFromEnv( + env: { + NO_COLOR?: string; + TERM?: string; + COLORTERM?: string; + }, + depth: number | undefined, +): 0 | 1 | 2 | 3 { // NO_COLOR spec — a non-empty value disables all color. // (NO_COLOR= with an empty string does NOT disable color per the spec.) if (env.NO_COLOR && env.NO_COLOR.length > 0) return 0; // TERM=dumb is explicitly colorless. const term = env.TERM ?? ''; - if (term === 'dumb' || term === '') return 0; - // COLORTERM=truecolor → 24-bit. + if (term === 'dumb') return 0; + // An explicit declaration outranks the reported depth: the terminal is + // telling us something it knows and the capability probe may not. const colorterm = env.COLORTERM ?? ''; if (colorterm === 'truecolor' || colorterm === '24bit') return 3; // Known truecolor terminals by TERM name. - if (/\-(truecolor|24bit)$/.test(term)) return 3; + if (/-(truecolor|24bit)$/.test(term)) return 3; + if (depth !== undefined) { + if (depth >= 24) return 3; + if (depth >= 8) return 2; + if (depth >= 4) return 1; + return 0; + } + // No terminal to ask (piped output). Fall back to the env ladder. + if (term === '') return 0; // 256-color: most modern terminals set this explicitly. if (/256color|256-color/.test(term)) { return 2;