Skip to content

fix(cli): honour NO_COLOR in manager warning, add --no-color flag - #848

Open
lakshya-dhariwal wants to merge 2 commits into
FailproofAI:mainfrom
lakshya-dhariwal:fix/no-color-manager
Open

lakshya-dhariwal wants to merge 2 commits into
FailproofAI:mainfrom
lakshya-dhariwal:fix/no-color-manager

Conversation

@lakshya-dhariwal

@lakshya-dhariwal lakshya-dhariwal commented Sep 27, 2026 •

Copy link
Copy Markdown

Description

Fixes #688. One note up front: by the time I got here, the policies listing itself was already routed through paint()/chip() in tui.ts, so the 16 hardcoded sites this issue describes had shrunk to exactly one - the duplicate-scope warning in src/hooks/manager.ts, which still printed \x1B[33m…\x1B[0m regardless of NO_COLOR. This PR finishes the job rather than pretending the listing was still broken:

  • src/hooks/manager.ts: the warning now uses paint(!process.env.NO_COLOR).warn(...) (the install-prompt.ts pattern), so no hardcoded escapes remain in the file.
  • src/hooks/tui.ts: new applyNoColorFlag(args) helper - sets NO_COLOR=1 and strips --no-color from argv, so no subcommand's flag parser ever sees it.
  • bin/failproofai.mjs: calls it once, right after argv is read; documents --no-color in the top-level help footer and in policies --help.
  • __tests__/hooks/no-color.test.ts: covers the TTY gate (colorsEnabled false under NO_COLOR), the painter emitting zero ESC bytes under NO_COLOR, the flag helper's strip-and-set behaviour, and a regression lock asserting manager.ts never reintroduces a hardcoded escape.

I verified the help screens stay within the layout budget the help-index test enforces: index goes 25 to 26 lines (cap 30), max column stays 78 (cap 80). I measured by rendering helpScreen with the real specs, since this environment has no bun to spawn the binary.

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation

Checklist

  • npm run lint passes (eslint on the touched files - clean)
  • npx tsc --noEmit passes
  • npm run test:run passes - ran the related suites instead: no-color.test.ts (new, 5 tests), tui.test.ts, tui-kit.test.ts, manager.test.ts, manager-cloud-listing.test.ts - 152/152 green. Full test:run + build need bun, which I don't have here - flagging rather than ticking a box I didn't run. The bun-driven suites (help-index, unified-policies-surface) should exercise the help layout and the new flag in CI.
  • npm run build succeeds - same bun constraint.

Summary by CodeRabbit

  • New Features
    • Added a --no-color option to disable colored output across commands. It can also be enabled with NO_COLOR=1, and is documented in the command-line help.
  • Bug Fixes
    • Duplicate-scope warnings now respect the no-color setting instead of always displaying colored text.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks @lakshya-dhariwal for your contribution to Failproof AI! 🙌

We'd love to discuss your PR and welcome you to our community.

Discord: https://discord.befailproof.ai/
Reddit: https://www.reddit.com/r/failproofai/

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: dc55de6c-7511-4fd6-994d-3db70b90309a

📥 Commits

Reviewing files that changed from the base of the PR and between e40de6c and d3c2e33.

📒 Files selected for processing (4)
  • __tests__/hooks/no-color.test.ts
  • bin/failproofai.mjs
  • src/hooks/manager.ts
  • src/hooks/tui.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The CLI now removes --no-color before command dispatch and sets NO_COLOR=1 when the flag is present. Help text documents the option. The duplicate-scope warning uses the shared color helper and respects NO_COLOR.

Changes

Color output controls

Layer / File(s) Summary
Global no-color flag
src/hooks/tui.ts, bin/failproofai.mjs, __tests__/hooks/no-color.test.ts
The CLI removes --no-color before dispatch and sets NO_COLOR=1 when the flag is present. Help text documents the option. Tests cover flag handling and color output.
Multi-scope warning color
src/hooks/manager.ts
The duplicate-scope warning uses paint and disables color when NO_COLOR is set.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: chhhee10

Merge Risk: ⚪ Minimal · up to d3c2e

The CLI color-control change appears ready to merge after normal checks.

Architecture Summary

Architecture risk: 🔵 Low · up to d3c2e

The change affects 3 systems.

Changed systems: src, bin, __tests__

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 2 changed files map to changed impact.
  • observed — bin (service) was modified; 1 changed file maps to changed impact.
  • observed — tests (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in tests/hooks/no-color.test.ts: Adds a Node-environment Vitest suite that checks the stated NO_COLOR and flag-handling expectations, restores the environment variable after each test, and scans manager.ts for hardcoded ANSI escape sequences.
  • observed — Modified behavior in bin/failproofai.mjs: Before command normalization and dispatch, the entry point now lazily loads applyNoColorFlag and applies it to the argument list, allowing the flag to affect every command before subcommand parsing.
  • observed — Modified behavior in bin/failproofai.mjs: The top-level help footer now documents --no-color as plain output equivalent to NO_COLOR=1.
  • observed — Modified behavior in bin/failproofai.mjs: The policies options now document --no-color and state that it works across commands.
🚥 Pre-merge checks | ✅ 6 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the two main changes: honoring NO_COLOR in the manager warning and adding the --no-color flag.
Description check ✅ Passed The description follows the repository template, explains the changes, identifies the issue, and reports completed and unavailable validation steps.
Linked Issues check ✅ Passed The description references and addresses issue #688. The stated implementation matches the issue objectives.
Out of Scope Changes check ✅ Passed The changes remain within scope: CLI color handling, flag processing, documentation, and related tests.
Linked Issues check ✅ Passed Issue #688 requires NO_COLOR=1 and --no-color to disable ANSI output for failproofai policies, with tests and use of the existing paint() path. The reviewed head routes the duplicate-scope war…
Out of Scope Changes check ✅ Passed The changes stay within issue #688. The manager update fixes the hardcoded warning color. The CLI helper, help text, and tests support the requested --no-color behavior. No unrelated production beha…
  • Fix all pre-merge checks with AI

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

__tests__/hooks/no-color.test.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

bin/failproofai.mjs

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).

src/hooks/manager.ts

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).

  • 1 others

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit hops through terminal light,
Then dims the colors, clean and bright.
The flag is caught before commands run,
The warning prints without its hue.
Tests keep watch on every cue.
One quiet burrow, plain output too.

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI: failproofai policies ignores NO_COLOR, and --no-color does not exist

1 participant