From 1ba5771ed98f60b95bc7844b5f13239ccc672c36 Mon Sep 17 00:00:00 2001 From: Max Hsu Date: Thu, 16 Jul 2026 22:17:57 +0800 Subject: [PATCH] fix(cli): honor NO_COLOR, add --no-color, disable colors when stdout is not a TTY (#1281) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Human-format output (query, status, callers, callees, impact, files, …) embedded ANSI color codes unconditionally: piping to a file or another tool captured the escapes, and the no-color.org escape hatch was ignored (NO_COLOR=1 produced byte-identical output). All CLI coloring already flows through the colors palette in src/bin/codegraph.ts (and a mini-palette in src/upgrade/index.ts), so the fix is a single shared gate (src/ui/colors.ts): - --no-color anywhere on the command line disables color (stripped before commander parses, same pre-parse intercept pattern as -v, so it also works after subcommands) - a non-empty NO_COLOR environment variable disables color - otherwise color is enabled only when stdout is a TTY With color disabled the palette collapses to empty strings, so every existing colors.X interpolation and chalk.X() helper becomes a plain-text no-op — no call sites change. Exercised end-to-end against the built binary: piped query/status/ callers output must be escape-free, NO_COLOR is honored, --no-color is accepted after a subcommand, and an empty NO_COLOR does not disable color (per spec). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01XWwajmLbgJbK6JBCY8nKfk --- CHANGELOG.md | 1 + __tests__/cli-no-color.test.ts | 88 ++++++++++++++++++++++++++++++++++ src/bin/codegraph.ts | 52 +++++++++++++++----- src/ui/colors.ts | 22 +++++++++ src/upgrade/index.ts | 16 +++++-- 5 files changed, 161 insertions(+), 18 deletions(-) create mode 100644 __tests__/cli-no-color.test.ts create mode 100644 src/ui/colors.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d3ba15487..98e49e46f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- Command output is now clean when piped or captured: colors only render when stdout is a terminal, so redirecting `codegraph query`, `status`, `callers`, and friends to a file or another tool no longer embeds ANSI escape codes. The standard `NO_COLOR` environment variable (no-color.org) is honored, and a `--no-color` flag is available to force plain output in a terminal too. (#1281) - Callers and impact analysis no longer silently under-count a function that calls the same callee many times. When one caller contained several call sites to the same callee and an internal resolution batch boundary happened to split them, cleanup after the first batch removed the later sites' pending rows before they were ever attempted — their edges were never created, deterministically, and which edges went missing shifted with unrelated changes to the project's total reference count. Post-pass cleanup now targets the exact database row each processed reference came from. Found while validating the operator-call fix on nlohmann/json, where `write_cbor`'s 11 calls to `to_char_type` indexed as 10. (#1269) - C++ explicit operator calls — `a.operator+(b)`, `p->operator+(b)`, `a.operator[](3)`, and the other symbolic forms — now produce a `calls` edge to the operator method, so an operator invoked only through the explicit syntax no longer looks uncalled in callers and impact analysis. tree-sitter parses these call sites with the operator name stranded in an error node (never as a normal member access), so the call's target was silently read as just the receiver variable; the operator name is now recovered from the error node and resolved through receiver-type inference like any other member call — a same-named operator on an unrelated class can never capture the edge. Infix uses (`a + b`, `a[i]`) need real type inference and are tracked separately. (#1247) - `codegraph init` and `codegraph index` no longer look hung after "Resolving refs" reaches 100%. The dynamic-dispatch linking that runs after resolution (callbacks, React re-renders, C function pointers, and the rest) had no progress display, so on repos where it takes a while — large C codebases especially — the bar just sat frozen at 100% until it finished. That work now shows as its own "Linking dynamic dispatch" progress phase, and the heaviest pass — C function-pointer linking — additionally reports progress within the pass, so a large C codebase advances the bar smoothly instead of parking it on one number for the bulk of the phase. diff --git a/__tests__/cli-no-color.test.ts b/__tests__/cli-no-color.test.ts new file mode 100644 index 000000000..9fd22f61f --- /dev/null +++ b/__tests__/cli-no-color.test.ts @@ -0,0 +1,88 @@ +/** + * ANSI color suppression for piped / agent consumption (#1281). + * + * Human-format output (`query`, `status`, …) used to embed ANSI color codes + * unconditionally: piping to a file or another tool captured the escapes, and + * the standard `NO_COLOR` escape hatch (https://no-color.org) was ignored. + * + * The CLI now emits colors only when stdout is a TTY, and honors both a + * non-empty `NO_COLOR` environment variable and a `--no-color` flag. These + * tests run the built binary with stdout piped (never a TTY), so every mode + * below must produce escape-free output. + * + * Exercised end-to-end against the built binary. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFileSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); +const ANSI_ESCAPE = /\x1b\[/; + +function run(cwd: string, args: string[], extraEnv: Record = {}): string { + return execFileSync(process.execPath, [BIN, ...args], { + cwd, // `status` resolves the project from cwd (it has no `-p` option) + encoding: 'utf-8', + env: { + ...process.env, + CODEGRAPH_NO_DAEMON: '1', + CODEGRAPH_WASM_RELAUNCHED: '1', + NO_COLOR: '', // an empty NO_COLOR must NOT disable color per no-color.org — the pipe gate below is what keeps these runs clean + ...extraEnv, + }, + stdio: ['ignore', 'pipe', 'ignore'], // drop stderr (SQLite experimental warning) + }); +} + +describe('CLI ANSI color suppression (#1281)', () => { + let tempDir: string; + + beforeAll(async () => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-no-color-')); + fs.mkdirSync(path.join(tempDir, 'src')); + fs.writeFileSync( + path.join(tempDir, 'src/auth.ts'), + 'export function parseToken(t: string){ return t.trim(); }\n' + + 'export function checkToken(t: string){ return parseToken(t).length > 0; }\n', + ); + const cg = CodeGraph.initSync(tempDir); + await cg.indexAll(); + cg.close(); + }); + + afterAll(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + it('query human output is escape-free when stdout is piped (non-TTY default)', () => { + const out = run(tempDir, ['query', 'parseToken', '-l', '5']); + expect(out).toContain('parseToken'); + expect(out).not.toMatch(ANSI_ESCAPE); + }); + + it('status human output is escape-free when stdout is piped', () => { + const out = run(tempDir, ['status']); + expect(out).not.toMatch(ANSI_ESCAPE); + }); + + it('callers human output is escape-free when stdout is piped', () => { + const out = run(tempDir, ['callers', 'parseToken']); + expect(out).not.toMatch(ANSI_ESCAPE); + }); + + it('a non-empty NO_COLOR is honored', () => { + const out = run(tempDir, ['query', 'parseToken', '-l', '5'], { NO_COLOR: '1' }); + expect(out).toContain('parseToken'); + expect(out).not.toMatch(ANSI_ESCAPE); + }); + + it('--no-color is accepted after a subcommand and produces clean output', () => { + const out = run(tempDir, ['query', 'parseToken', '-l', '5', '--no-color']); + expect(out).toContain('parseToken'); + expect(out).not.toMatch(ANSI_ESCAPE); + }); +}); diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 69d98566a..0e1c4fe2c 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -36,6 +36,7 @@ import { extractProseCandidates } from '../search/identifier-segments'; import { detectWorktreeIndexMismatch, worktreeMismatchWarning } from '../sync/worktree'; import { createShimmerProgress } from '../ui/shimmer-progress'; import { getGlyphs } from '../ui/glyphs'; +import { colorEnabled } from '../ui/colors'; import { buildNode25BlockBanner, buildNodeTooOldBanner, MIN_NODE_MAJOR } from './node-version-check'; import { installFatalHandlers } from './fatal-handler'; @@ -143,18 +144,40 @@ if (firstArg === '-v' || firstArg === '-version') { // ANSI Color Helpers (avoid chalk ESM issues) // ============================================================================= -const colors = { - reset: '\x1b[0m', - bold: '\x1b[1m', - dim: '\x1b[2m', - red: '\x1b[31m', - green: '\x1b[32m', - yellow: '\x1b[33m', - blue: '\x1b[34m', - cyan: '\x1b[36m', - white: '\x1b[37m', - gray: '\x1b[90m', -}; +// Decide color once, then strip `--no-color` before commander parses: it's a +// global affordance that must also work after a subcommand +// (`codegraph query … --no-color`), where commander would reject it as an +// unknown option. Same pre-parse intercept pattern as `-v` above. (#1281) +const useColor = colorEnabled(); +process.argv = process.argv.filter((arg) => arg !== '--no-color'); + +// With color disabled every code is an empty string, so both the `colors.X` +// interpolations and the `chalk.X()` helpers below become plain-text no-ops. +const colors = useColor + ? { + reset: '\x1b[0m', + bold: '\x1b[1m', + dim: '\x1b[2m', + red: '\x1b[31m', + green: '\x1b[32m', + yellow: '\x1b[33m', + blue: '\x1b[34m', + cyan: '\x1b[36m', + white: '\x1b[37m', + gray: '\x1b[90m', + } + : { + reset: '', + bold: '', + dim: '', + red: '', + green: '', + yellow: '', + blue: '', + cyan: '', + white: '', + gray: '', + }; const chalk = { bold: (s: string) => `${colors.bold}${s}${colors.reset}`, @@ -171,7 +194,10 @@ const chalk = { program .name('codegraph') .description('Code intelligence and knowledge graph for any codebase') - .version(packageJson.version); + .version(packageJson.version) + // Parsed by the pre-parse intercept above (and stripped from argv, so it + // works after subcommands too); registered here so it shows in --help. + .option('--no-color', 'disable ANSI colors (also honors NO_COLOR; color is off by default when stdout is not a TTY)'); // Anonymous usage telemetry (see TELEMETRY.md): record the invoked subcommand // NAME only — never arguments or paths. Counts buffer locally; network sends diff --git a/src/ui/colors.ts b/src/ui/colors.ts new file mode 100644 index 000000000..e03f73cfc --- /dev/null +++ b/src/ui/colors.ts @@ -0,0 +1,22 @@ +/** + * Whether human-format CLI output should include ANSI color codes (#1281). + * + * Follows https://no-color.org and common CLI convention: + * - `--no-color` anywhere on the command line disables color + * - a non-empty `NO_COLOR` environment variable disables color + * - otherwise color is only enabled when stdout is a TTY, so piped / + * redirected / agent-captured output is always escape-free + * + * `--json` output was already escape-free; this gate covers the human format, + * which is the most token-efficient shape for agent consumers. + */ +export function colorEnabled( + argv: readonly string[] = process.argv, + env: NodeJS.ProcessEnv = process.env, + stdoutIsTTY: boolean = process.stdout.isTTY === true, +): boolean { + if (argv.includes('--no-color')) return false; + // Per no-color.org, NO_COLOR disables color when present and non-empty. + if (env.NO_COLOR !== undefined && env.NO_COLOR !== '') return false; + return stdoutIsTTY; +} diff --git a/src/upgrade/index.ts b/src/upgrade/index.ts index 7e68d16c1..8c785f3a6 100644 --- a/src/upgrade/index.ts +++ b/src/upgrade/index.ts @@ -28,6 +28,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as https from 'https'; import { spawnSync } from 'child_process'; +import { colorEnabled } from '../ui/colors'; export const REPO = 'colbymchenry/codegraph'; export const NPM_PACKAGE = '@colbymchenry/codegraph'; @@ -305,12 +306,17 @@ export interface UpgradeDeps { offerBetaSignup?: () => Promise; } +// Evaluated per call so the NO_COLOR / non-TTY gate (#1281) applies at the +// moment of printing, and unit tests (piped stdout) stay escape-free. +const paint = (code: string) => (s: string) => + colorEnabled() ? `\x1b[${code}m${s}\x1b[0m` : s; + const c = { - bold: (s: string) => `\x1b[1m${s}\x1b[0m`, - dim: (s: string) => `\x1b[2m${s}\x1b[0m`, - green: (s: string) => `\x1b[32m${s}\x1b[0m`, - yellow: (s: string) => `\x1b[33m${s}\x1b[0m`, - cyan: (s: string) => `\x1b[36m${s}\x1b[0m`, + bold: paint('1'), + dim: paint('2'), + green: paint('32'), + yellow: paint('33'), + cyan: paint('36'), }; /** The honest, additive re-index reminder shown after a successful upgrade. */