From 53a5670380226282f48347dcefe13078b9f60adf Mon Sep 17 00:00:00 2001 From: mjq2020 <74635395+mjq2020@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:24:12 +0800 Subject: [PATCH] fix(cli): honor no-color for policy output --- CHANGELOG.md | 2 ++ __tests__/e2e/cli/cli-args.e2e.test.ts | 14 +++++++++++ __tests__/hooks/manager.test.ts | 21 ++++++++++++++++ bin/failproofai.mjs | 14 +++++++++++ src/hooks/manager.ts | 35 ++++++++++++++------------ 5 files changed, 70 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fbc6790a6..5255600ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ ### Fixes +- Honor `NO_COLOR` and `--no-color` when listing policies (#695). + - Stop the local audit report claiming it would have blocked things it only detected. `totalHits` folds in `detectorRows` (source `audit-detector`) — replay-only findings with no builtin behind them, whose own fix column renders `audit-only` and which the slipping-through section already labels "caught by audit, not blocked" — yet the TL;DR line users paste publicly said the agent did N things "`failproofai` would have stopped". "Caught" is true of both halves; "would have stopped" was true of one. The same footer also named six integrations when `runAuditInner` defaults to all twelve. (#731) - Stop the localized navigation referencing pages that were never translated, which is what still discarded a partial run. `--allow-partial` published what succeeded — and then `--update-nav` regenerated the nav from the ENGLISH tree, emitting an entry for the failed page in the language that failed it, so `mintlify validate` rejected the missing file and the job died before its push anyway. The 784 pages that HAD translated went with it, which is precisely the loss `--allow-partial` exists to prevent. Nav generation now omits any localized page whose file is not on disk, prunes a group left with no pages and a tab left with no groups, and keeps an `openapi` group that never had pages to begin with. The check is injected rather than hardcoded, so the pure transform stays testable and the two paths that actually write `docs.json` get the real one. This also closes the same hazard from every other direction it can arrive from — a pruned page, or a translation that only exists on an unmerged branch — because the nav is now derived from what is present rather than from what English says should be. (#725) diff --git a/__tests__/e2e/cli/cli-args.e2e.test.ts b/__tests__/e2e/cli/cli-args.e2e.test.ts index 615f607e4..805be4862 100644 --- a/__tests__/e2e/cli/cli-args.e2e.test.ts +++ b/__tests__/e2e/cli/cli-args.e2e.test.ts @@ -147,6 +147,20 @@ describe("policies: list (default)", () => { expect(result.stdout).toContain("block-sudo"); }); + it("accepts --no-color and emits no ANSI escapes", () => { + const result = runCli("policies", "--no-color"); + assertSuccess(result); + expect(result.stdout).toContain("block-sudo"); + expect(result.stdout).not.toMatch(/\x1B\[/); + }); + + it("--no-color works before the subcommand", () => { + const result = runCli("--no-color", "policies"); + assertSuccess(result); + expect(result.stdout).toContain("block-sudo"); + expect(result.stdout).not.toMatch(/\x1B\[/); + }); + it("rejects unexpected positional argument", () => { const result = runCli("policies", "hi"); assertCleanError(result, "Unexpected argument: hi"); diff --git a/__tests__/hooks/manager.test.ts b/__tests__/hooks/manager.test.ts index 4b80874b2..9a3db5846 100644 --- a/__tests__/hooks/manager.test.ts +++ b/__tests__/hooks/manager.test.ts @@ -68,6 +68,7 @@ describe("hooks/manager", () => { }); afterEach(() => { + vi.unstubAllEnvs(); vi.restoreAllMocks(); }); @@ -1053,6 +1054,26 @@ describe("hooks/manager", () => { }); describe("listHooks", () => { + it("honors NO_COLOR for every policy-list status", async () => { + vi.stubEnv("NO_COLOR", "1"); + const { readMergedHooksConfig } = await import("../../src/hooks/hooks-config"); + vi.mocked(readMergedHooksConfig).mockReturnValue({ + enabledPolicies: ["block-sudo"], + policyParams: { "unknown-policy": { enabled: true } }, + customPoliciesPath: "/tmp/missing-policy.js", + }); + vi.mocked(existsSync).mockReturnValue(false); + + const { listHooks } = await import("../../src/hooks/manager"); + await listHooks(); + + const output = vi.mocked(console.log).mock.calls.map((call) => call[0]).join("\n"); + expect(output).toContain("\u2713"); + expect(output).toContain("unknown policyParams key"); + expect(output).toContain("File not found"); + expect(output).not.toMatch(/\x1B\[/); + }); + it("compact output when no hooks installed", async () => { const { readMergedHooksConfig } = await import("../../src/hooks/hooks-config"); vi.mocked(readMergedHooksConfig).mockReturnValue({ enabledPolicies: [] }); diff --git a/bin/failproofai.mjs b/bin/failproofai.mjs index 6be9c4393..2777911ad 100755 --- a/bin/failproofai.mjs +++ b/bin/failproofai.mjs @@ -6,6 +6,7 @@ * --hook Hook event from Claude Code (minimal startup latency) * --version / -v Print version and exit * --help / -h Show usage and exit + * --no-color Suppress ANSI color output * policies Manage policies (list / install / uninstall) * (default) Launch production dashboard */ @@ -34,6 +35,15 @@ if (!process.env.FAILPROOFAI_DIST_PATH) { const args = process.argv.slice(2); +// Global presentation flag: consume it before subcommand validation so it can +// appear before or after the command without becoming an unknown argument. +if (args.includes("--no-color")) { + process.env.NO_COLOR = "1"; + for (let i = args.length - 1; i >= 0; i--) { + if (args[i] === "--no-color") args.splice(i, 1); + } +} + // Normalize 'p' → 'policies' (shorthand alias) if (args[0] === "p") args[0] = "policies"; // Normalize 'configure' / 'setup' → 'config' (aliases), so every later check @@ -373,6 +383,7 @@ COMMANDS --version, -v Print version and exit --help, -h Show this help message + --no-color Suppress ANSI color output CONVENTION POLICIES Drop *policies.{js,mjs,ts} files into .failproofai/policies/ for auto-loading. @@ -1146,6 +1157,9 @@ OPTIONS (install) --custom, -c Custom policy file (repeat for multiple files) (skips interactive prompt; validates file first) +OPTIONS (output) + --no-color Suppress ANSI color output + OPTIONS (uninstall) [names...] Specific policy names to disable (omit to remove hooks) --cli claude|codex|copilot|cursor|opencode|pi|hermes|openclaw|factory|devin|antigravity|goose diff --git a/src/hooks/manager.ts b/src/hooks/manager.ts index baef48d8c..32e92b544 100644 --- a/src/hooks/manager.ts +++ b/src/hooks/manager.ts @@ -26,6 +26,7 @@ import { CliError } from "../cli-error"; import { hookLogWarn } from "./hook-logger"; import { customPoliciesDir, globalPolicyConfigFile } from "./fp-home"; import { readActiveCloudManagedPolicies } from "./cloud-managed-policies"; +import { colorsEnabled, paint } from "./tui"; const VALID_POLICY_NAMES = new Set(BUILTIN_POLICIES.map((p) => p.name)); @@ -390,9 +391,10 @@ async function installHooksImpl( const otherScopes = deduplicateScopes(HOOK_SCOPES, cwd).filter((s) => s !== scope); const duplicates = otherScopes.filter((s) => hooksInstalledInSettings(s, cwd)); if (duplicates.length > 0) { + const color = paint(colorsEnabled(process.stdout)); 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`); + console.log(color.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.`); @@ -592,6 +594,7 @@ export async function removeHooks(policyNames?: string[], scope: HookScope | "al * - Custom Hooks section if customPoliciesPath is set */ export async function listHooks(cwd?: string): Promise { + const color = paint(colorsEnabled(process.stdout)); const config = readMergedHooksConfig(cwd); const enabledSet = new Set(config.enabledPolicies); const disabledCustomSet = new Set(config.disabledCustomPolicies ?? []); @@ -621,13 +624,13 @@ export async function listHooks(cwd?: string): Promise { const statusCol = 8; const printSimpleRow = (policy: { name: string; description: string }) => { - const mark = enabledSet.has(policy.name) ? `\x1B[32m\u2713\x1B[0m` : " "; + const mark = enabledSet.has(policy.name) ? color.guide("\u2713") : " "; console.log(` ${mark}${" ".repeat(statusCol - 1)}${policy.name.padEnd(nameColWidth)}${policy.description}`); printParamsSummary(policy.name, ` ${" ".repeat(statusCol)}`); }; const printBetaSection = (printRow: (p: { name: string; description: string }) => void) => { if (betaPolicies.length > 0) { - console.log(`\n \x1B[2m\u2500\u2500 Beta \u2500\u2500\x1B[0m`); + console.log(`\n ${color.dim("\u2500\u2500 Beta \u2500\u2500")}`); for (const policy of betaPolicies) printRow(policy); } }; @@ -686,7 +689,7 @@ export async function listHooks(cwd?: string): Promise { let row = " "; for (const _scope of installedScopes) { if (enabled) { - row += `\x1B[32m\u2713 ON\x1B[0m` + " ".repeat(COL - 4); + row += color.guide("\u2713 ON") + " ".repeat(COL - 4); } else { row += " OFF" + " ".repeat(COL - 5); } @@ -699,7 +702,7 @@ export async function listHooks(cwd?: string): Promise { for (const policy of regularPolicies) printMultiScopeRow(policy); if (betaPolicies.length > 0) { - console.log(`\n \x1B[2m\u2500\u2500 Beta \u2500\u2500\x1B[0m`); + console.log(`\n ${color.dim("\u2500\u2500 Beta \u2500\u2500")}`); for (const policy of betaPolicies) printMultiScopeRow(policy); } @@ -708,7 +711,7 @@ export async function listHooks(cwd?: string): Promise { // Multi-scope warning const scopeNames = installedScopes.join(", "); console.log(); - console.log(`\x1B[33m\u26A0 Hooks in multiple scopes (${scopeNames}).\x1B[0m`); + console.log(color.warn(`\u26A0 Hooks in multiple scopes (${scopeNames}).`)); console.log(" Consider keeping one. Remove with: failproofai policies --uninstall --scope \n"); } @@ -717,7 +720,7 @@ export async function listHooks(cwd?: string): Promise { const unknownKeys: string[] = []; for (const key of Object.keys(config.policyParams)) { if (!builtinPolicyNames.has(key)) { - console.log(` \x1B[33mWarning: unknown policyParams key "${key}" — possible typo\x1B[0m`); + console.log(` ${color.warn(`Warning: unknown policyParams key "${key}" — possible typo`)}`); unknownKeys.push(key); } } @@ -742,17 +745,17 @@ export async function listHooks(cwd?: string): Promise { const absPath = resolve(findProjectConfigDir(cwd ?? process.cwd()), path); console.log(` ${absPath}`); if (!existsSync(absPath)) { - console.log(` \x1B[31m\u2717 File not found: ${absPath}\x1B[0m`); + console.log(` ${color.pink(`\u2717 File not found: ${absPath}`)}`); continue; } const hooks = await loadCustomHooks(absPath); if (hooks.length === 0) { - console.log(` \x1B[31m\u2717 ERR failed to load (check ~/.failproofai/logs/hooks.log)\x1B[0m`); + console.log(` ${color.pink("\u2717 ERR failed to load (check ~/.failproofai/logs/hooks.log)")}`); } else { const descColWidth = nameColWidth; for (const hook of hooks) { const disabled = disabledCustomSet.has(`custom:${absPath}:${hook.name}`); - const status = disabled ? "\x1B[2m OFF\x1B[0m" : "\x1B[32m\u2713 ON\x1B[0m"; + const status = disabled ? color.dim(" OFF") : color.guide("\u2713 ON"); console.log(` ${status} ${hook.name.padEnd(descColWidth)}${hook.description ?? ""}`); } } @@ -814,7 +817,7 @@ export async function listHooks(cwd?: string): Promise { const filename = basename(file); record(filename, hooks.map((h) => h.name)); if (hooks.length === 0) { - console.log(` \x1B[31m\u2717\x1B[0m ${filename.padEnd(colWidth)}\x1B[31mfailed to load\x1B[0m`); + console.log(` ${color.pink("\u2717")} ${filename.padEnd(colWidth)}${color.pink("failed to load")}`); } else { const hookStates = hooks.map((hook) => ({ hook, @@ -822,10 +825,10 @@ export async function listHooks(cwd?: string): Promise { })); const disabledCount = hookStates.filter((entry) => entry.disabled).length; const status = disabledCount === 0 - ? "\x1B[32m\u2713 ON\x1B[0m" + ? color.guide("\u2713 ON") : disabledCount === hooks.length - ? "\x1B[2m OFF\x1B[0m" - : "\x1B[33m\u25D0 MIXED\x1B[0m"; + ? color.dim(" OFF") + : color.warn("\u25D0 MIXED"); const hookSummary = hookStates .map(({ hook, disabled }) => `${hook.name}${disabled ? " (OFF)" : ""}`) .join(", "); @@ -834,7 +837,7 @@ export async function listHooks(cwd?: string): Promise { } catch { const filename = basename(file); record(filename, []); - console.log(` \x1B[31m\u2717\x1B[0m ${filename.padEnd(colWidth)}\x1B[31merror\x1B[0m`); + console.log(` ${color.pink("\u2717")} ${filename.padEnd(colWidth)}${color.pink("error")}`); } } console.log(); @@ -862,7 +865,7 @@ export async function listHooks(cwd?: string): Promise { // that read "ON" would claim enforcement this policy deliberately is // not doing. const status = - artifact.effect === "observe" ? "\x1B[33m\u25D0 OBS\x1B[0m" : "\x1B[32m\u2713 ON\x1B[0m"; + artifact.effect === "observe" ? color.warn("\u25D0 OBS") : color.guide("\u2713 ON"); console.log(` ${status} ${artifact.id.padEnd(colWidth)}v${artifact.version}`); } console.log("\n Managed from the dashboard \u2014 not switchable with `failproofai policies`.");