Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions __tests__/hooks/no-color.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
11 changes: 11 additions & 0 deletions bin/failproofai.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -497,6 +506,7 @@ async function runCli() {
footer: [
"failproofai <command> [options] failproofai help <command> 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);
Expand Down Expand Up @@ -1942,6 +1952,7 @@ async function runCli() {
["--scope <scope>", "user, project or local. Default: user. --uninstall also takes all."],
["--beta", "Include beta policies. On --uninstall, only those."],
["--custom, -c <path>", "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."],
],
},
{
Expand Down
6 changes: 5 additions & 1 deletion src/hooks/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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.`);
Expand Down
19 changes: 19 additions & 0 deletions src/hooks/tui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
Expand Down