diff --git a/__tests__/hooks/no-color.test.ts b/__tests__/hooks/no-color.test.ts new file mode 100644 index 000000000..9f91c7114 --- /dev/null +++ b/__tests__/hooks/no-color.test.ts @@ -0,0 +1,60 @@ +// @vitest-environment node +// +// NO_COLOR=1 `failproofai policies` still emitted ANSI (issue #688): the +// duplicate-scope warning in manager.ts hardcoded `\x1B[33m`, and there was +// no --no-color flag at all. The listing itself already routes through +// paint(), so the fix is three small things: the warning uses the shared +// painter, the bin spells the env var as a flag, and these lock both. +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { applyNoColorFlag, colorsEnabled, paint } from "../../src/hooks/tui"; + +describe("NO_COLOR support", () => { + const saved = process.env.NO_COLOR; + + beforeEach(() => { + delete process.env.NO_COLOR; + }); + + afterEach(() => { + if (saved === undefined) delete process.env.NO_COLOR; + else process.env.NO_COLOR = saved; + }); + + it("colorsEnabled reports false for a TTY when NO_COLOR is set", () => { + const tty = { isTTY: true } as typeof process.stdout; + expect(colorsEnabled(tty)).toBe(true); + process.env.NO_COLOR = "1"; + expect(colorsEnabled(tty)).toBe(false); + }); + + it("the painter the warning now uses emits no ESC bytes under NO_COLOR", () => { + const text = "Warning: Failproof AI hooks are also installed at user (user)."; + expect(paint(true).warn(text)).toContain("\x1B["); + process.env.NO_COLOR = "1"; + expect(paint(!process.env.NO_COLOR).warn(text)).toBe(text); + }); + + it("--no-color sets NO_COLOR and strips the flag so subcommand parsers never see it", () => { + const args = ["policies", "--no-color"]; + expect(applyNoColorFlag(args)).toBe(true); + expect(process.env.NO_COLOR).toBe("1"); + expect(args).toEqual(["policies"]); + }); + + it("applyNoColorFlag is a no-op when the flag is absent", () => { + const args = ["policies", "--list"]; + expect(applyNoColorFlag(args)).toBe(false); + expect(process.env.NO_COLOR).toBeUndefined(); + expect(args).toEqual(["policies", "--list"]); + }); + + it("manager.ts holds no hardcoded ANSI escapes - the 16-site bug stays dead", () => { + const src = readFileSync( + resolve(__dirname, "..", "..", "src", "hooks", "manager.ts"), + "utf-8", + ); + expect(src.match(/\\x1[Bb]\[|\\u001[bB]\[|\\033\[/)).toBeNull(); + }); +}); diff --git a/bin/failproofai.mjs b/bin/failproofai.mjs index 49cfc2e65..7a5f91bc4 100755 --- a/bin/failproofai.mjs +++ b/bin/failproofai.mjs @@ -34,6 +34,15 @@ if (!process.env.FAILPROOFAI_DIST_PATH) { const args = process.argv.slice(2); +// --no-color: the flag spelling of NO_COLOR=1. Applied HERE, above SUBCOMMANDS +// and every dispatch, because it works by setting the env var every painter +// already gates on - and because stripping it now means no subcommand's own +// flag parser ever has to know it exists. +{ + const { applyNoColorFlag } = await import("../src/hooks/tui"); + applyNoColorFlag(args); +} + // ── one noun for policies ────────────────────────────────────────────────── // `policies`, `policy` and `pack` were three commands for one idea, two of them // a single letter apart and doing unrelated things. They are now three @@ -497,6 +506,7 @@ async function runCli() { footer: [ "failproofai [options] failproofai help for detail", "docs.befailproof.ai discord.befailproof.ai -h this screen -v version", + "--no-color for plain output (same as NO_COLOR=1)", ], }); process.exit(0); @@ -1942,6 +1952,7 @@ async function runCli() { ["--scope ", "user, project or local. Default: user. --uninstall also takes all."], ["--beta", "Include beta policies. On --uninstall, only those."], ["--custom, -c ", "Custom policy file; repeat for several. Bare on --uninstall, clears every explicit path."], + ["--no-color", "Plain output, no colour. Same as NO_COLOR=1; works on every command."], ], }, { diff --git a/src/hooks/manager.ts b/src/hooks/manager.ts index dd833e3e8..9f70319b8 100644 --- a/src/hooks/manager.ts +++ b/src/hooks/manager.ts @@ -24,6 +24,7 @@ import { trackHookEvent } from "./hook-telemetry"; import { getInstanceId, hashToId } from "../../lib/telemetry-id"; import { CliError } from "../cli-error"; import { hookLogWarn } from "./hook-logger"; +import { paint } from "./tui"; import { customPoliciesDir, globalPolicyConfigFile } from "./fp-home"; import { readActiveCloudManagedPolicies } from "./cloud-managed-policies"; import { CORE_SOURCE, addPack, setPackPolicyEnabled } from "./pack-store"; @@ -795,7 +796,10 @@ async function installHooksImpl( if (duplicates.length > 0) { const scopeList = duplicates.map((s) => `${s} (${scopeLabel(s)})`).join(", "); console.log(); - console.log(`\x1B[33mWarning: Failproof AI hooks are also installed at ${scopeList}.\x1B[0m`); + // Brand painter, NO_COLOR-gated like every other screen - the one place + // this warning used to hardcode the escape instead. + const { warn } = paint(!process.env.NO_COLOR); + console.log(warn(`Warning: Failproof AI hooks are also installed at ${scopeList}.`)); console.log(`Having hooks in multiple scopes may cause duplicate policy evaluation.`); console.log(`Use \`failproofai policies --uninstall --scope ${duplicates[0]}\` to remove the other installation,`); console.log(`or \`failproofai policies\` to see all scopes.`); diff --git a/src/hooks/tui.ts b/src/hooks/tui.ts index bbe7fa103..c09d854ee 100644 --- a/src/hooks/tui.ts +++ b/src/hooks/tui.ts @@ -155,6 +155,25 @@ export function colorsEnabled(out: TTYOut): boolean { return !!out.isTTY && !process.env.NO_COLOR; } +/** + * `--no-color`, the flag spelling of NO_COLOR=1 (https://no-color.org). Sets + * the env var - which every paint() call above already gates on - and strips + * the flag from argv so no subcommand's own flag parser ever sees it. Called + * ONCE by the bin, above SUBCOMMANDS and every dispatch. Returns whether the + * flag was present, for tests and for any caller that wants to know. + */ +export function applyNoColorFlag(args: string[]): boolean { + let found = false; + for (let i = args.length - 1; i >= 0; i--) { + if (args[i] === "--no-color") { + args.splice(i, 1); + found = true; + } + } + if (found) process.env.NO_COLOR = "1"; + return found; +} + /** * The raw ANSI opening sequence for a brand role, for callers that assemble * their own strings instead of using `paint()`'s wrappers (`src/audit/cli.ts`).