feat(cli): add --no-color flag and fix color propagation to workers#1163
feat(cli): add --no-color flag and fix color propagation to workers#1163hztBUAA wants to merge 2 commits intoyamadashy:mainfrom
Conversation
Register --no-color as a proper Commander CLI option so it appears in --help output and is not rejected as unknown. The underlying picocolors library already reads process.argv for --no-color and NO_COLOR env var, but Commander needs to know about the flag. Additionally: - Fix FORCE_COLOR propagation in worker processes: previously workers always set FORCE_COLOR which overrides NO_COLOR. Now respects NO_COLOR env var and --no-color flag by propagating NO_COLOR instead. - Fix hardcoded colors:true in logger formatArgs: now uses pc.isColorSupported so util.inspect respects color settings. - Add semantic suggestions (plain, monochrome) for --no-color. - Add color property to CliOptions type. Fixes yamadashy#1057
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request adds support for disabling colored output through a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello @hztBUAA, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the command-line interface by introducing a "--no-color" flag and improving how color settings are managed across the application, especially within worker processes. The changes ensure that color preferences, whether set via the CLI flag or the NO_COLOR environment variable, are consistently applied throughout the system, including in logging and child processes, aligning with the NO_COLOR standard. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
src/shared/processConcurrency.ts
Outdated
| ...(process.env.NO_COLOR || process.argv.includes('--no-color') | ||
| ? { NO_COLOR: '1' } | ||
| : { FORCE_COLOR: process.env.FORCE_COLOR || (process.stdout.isTTY ? '1' : '0') }), |
There was a problem hiding this comment.
🟡 Truthy check on process.env.NO_COLOR fails for empty-string values per NO_COLOR standard
The condition at src/shared/processConcurrency.ts:99 uses process.env.NO_COLOR which is a truthy check. Per the NO_COLOR standard, setting NO_COLOR= (empty string) is a valid way to disable colors. However, in JavaScript, '' is falsy, so process.env.NO_COLOR evaluates to false when NO_COLOR is set to an empty string.
Root Cause and Impact
Picocolors itself correctly uses "NO_COLOR" in env (an existence/in check) to detect the variable, which handles empty strings. But the new propagation logic in processConcurrency.ts uses process.env.NO_COLOR (a truthy check):
...(process.env.NO_COLOR || process.argv.includes('--no-color')
? { NO_COLOR: '1' }
: { FORCE_COLOR: process.env.FORCE_COLOR || (process.stdout.isTTY ? '1' : '0') }),When a user sets NO_COLOR= (empty string):
- Parent process: picocolors correctly disables colors (uses
"NO_COLOR" in env) - Worker child process: the condition evaluates to falsy, so
FORCE_COLORis set instead ofNO_COLOR, and colors remain enabled in workers
This creates an inconsistency where the parent process respects NO_COLOR= but worker processes do not, violating the NO_COLOR standard that this PR explicitly aims to support.
| ...(process.env.NO_COLOR || process.argv.includes('--no-color') | |
| ? { NO_COLOR: '1' } | |
| : { FORCE_COLOR: process.env.FORCE_COLOR || (process.stdout.isTTY ? '1' : '0') }), | |
| ...('NO_COLOR' in process.env || process.argv.includes('--no-color') | |
| ? { NO_COLOR: '1' } | |
| : { FORCE_COLOR: process.env.FORCE_COLOR || (process.stdout.isTTY ? '1' : '0') }), |
Was this helpful? React with 👍 or 👎 to provide feedback.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request does a great job of adding the --no-color flag and fixing color propagation to worker processes. The changes are logical and the inclusion of new tests to cover these changes is excellent. I have a few suggestions to improve the design by reducing coupling and to enhance the robustness of the tests. Overall, this is a solid contribution.
src/shared/processConcurrency.ts
Outdated
| ...(process.env.NO_COLOR || process.argv.includes('--no-color') | ||
| ? { NO_COLOR: '1' } | ||
| : { FORCE_COLOR: process.env.FORCE_COLOR || (process.stdout.isTTY ? '1' : '0') }), |
There was a problem hiding this comment.
While this works, checking process.argv.includes('--no-color') directly creates a coupling between this shared module and a specific CLI flag string. This can be brittle if the flag name changes. A more robust approach would be to pass the color preference down from the CLI command handler. The color property was already added to CliOptions and could be used for this. You could extend WorkerOptions to include the color setting and pass it through initTaskRunner to createWorkerPool. This would decouple this module from the specific CLI implementation details and make the data flow more explicit.
| describe('color support in formatArgs', () => { | ||
| it('should pass pc.isColorSupported to util.inspect for object formatting', () => { | ||
| const obj = { key: 'value' }; | ||
| logger.info('Test:', obj); | ||
| // When isColorSupported is true (mock), util.inspect should use colors | ||
| expect(console.log).toHaveBeenCalledWith(expect.stringContaining('CYAN:Test: ')); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
This test is a good start, but it only indirectly verifies the behavior by checking the final console.log output. To make the test more robust and specific, you could spy on util.inspect and assert that it's called with the correct colors option. This would directly test the logic in formatArgs. You could also test the case where colors are disabled to ensure util.inspect is called with colors: false.
| describe('color propagation to worker processes', () => { | ||
| beforeEach(() => { | ||
| vi.mocked(os).availableParallelism = vi.fn().mockReturnValue(4); | ||
| vi.mocked(Tinypool).mockImplementation(function (this: unknown) { | ||
| (this as Record<string, unknown>).run = vi.fn(); | ||
| (this as Record<string, unknown>).destroy = vi.fn(); | ||
| return this as Tinypool; | ||
| }); | ||
| }); | ||
|
|
||
| it('should propagate NO_COLOR to worker when NO_COLOR env is set', () => { | ||
| const originalNoColor = process.env.NO_COLOR; | ||
| process.env.NO_COLOR = '1'; | ||
| try { | ||
| createWorkerPool({ numOfTasks: 100, workerType: 'fileProcess', runtime: 'child_process' }); | ||
|
|
||
| expect(Tinypool).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| env: expect.objectContaining({ | ||
| NO_COLOR: '1', | ||
| }), | ||
| }), | ||
| ); | ||
| } finally { | ||
| if (originalNoColor === undefined) { | ||
| delete process.env.NO_COLOR; | ||
| } else { | ||
| process.env.NO_COLOR = originalNoColor; | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| it('should propagate NO_COLOR to worker when --no-color is in argv', () => { | ||
| const originalArgv = process.argv; | ||
| process.argv = [...originalArgv, '--no-color']; | ||
| try { | ||
| createWorkerPool({ numOfTasks: 100, workerType: 'fileProcess', runtime: 'child_process' }); | ||
|
|
||
| expect(Tinypool).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| env: expect.objectContaining({ | ||
| NO_COLOR: '1', | ||
| }), | ||
| }), | ||
| ); | ||
| } finally { | ||
| process.argv = originalArgv; | ||
| } | ||
| }); | ||
|
|
||
| it('should set FORCE_COLOR when colors are not disabled', () => { | ||
| const originalNoColor = process.env.NO_COLOR; | ||
| delete process.env.NO_COLOR; | ||
| const originalArgv = process.argv; | ||
| process.argv = originalArgv.filter((arg) => arg !== '--no-color'); | ||
| try { | ||
| createWorkerPool({ numOfTasks: 100, workerType: 'fileProcess', runtime: 'child_process' }); | ||
|
|
||
| expect(Tinypool).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| env: expect.objectContaining({ | ||
| FORCE_COLOR: expect.any(String), | ||
| }), | ||
| }), | ||
| ); | ||
| } finally { | ||
| if (originalNoColor !== undefined) { | ||
| process.env.NO_COLOR = originalNoColor; | ||
| } | ||
| process.argv = originalArgv; | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
To improve test isolation and reduce repetitive code, consider managing the global state (process.env.NO_COLOR and process.argv) using beforeEach and afterEach hooks for this describe block. You can save the original values in beforeEach and restore them in afterEach. This would make the tests cleaner and less prone to state leakage between tests, as you wouldn't need the try...finally blocks in each test.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shared/processConcurrency.ts (1)
89-105:⚠️ Potential issue | 🟡 MinorEnvironment variables may not be in optimal order when propagating color settings to child processes.
When
--no-coloris passed and the parent environment containsFORCE_COLOR, both variables end up in the child process env:{ FORCE_COLOR: '1', NO_COLOR: '1', ... }.Currently, picocolors v1.1.1 checks
NO_COLORfirst in its detection logic (!(!!env.NO_COLOR || ...)), soNO_COLORtakes absolute precedence and colors are correctly disabled. However, this relies on picocolors' internal evaluation order. Other color libraries may checkFORCE_COLORfirst, or picocolors may change its detection algorithm in future versions.To make the behavior unambiguous and library-agnostic, explicitly exclude
FORCE_COLORfrom the child env whenNO_COLORis set:Suggested approach
- env: { - ...process.env, - REPOMIX_WORKER_TYPE: workerType, - REPOMIX_LOG_LEVEL: logger.getLogLevel().toString(), - ...(process.env.NO_COLOR || process.argv.includes('--no-color') - ? { NO_COLOR: '1' } - : { FORCE_COLOR: process.env.FORCE_COLOR || (process.stdout.isTTY ? '1' : '0') }), - TERM: process.env.TERM || 'xterm-256color', - }, + env: (() => { + const noColor = !!(process.env.NO_COLOR || process.argv.includes('--no-color')); + const { FORCE_COLOR: _fc, NO_COLOR: _nc, ...restEnv } = process.env; + return { + ...restEnv, + REPOMIX_WORKER_TYPE: workerType, + REPOMIX_LOG_LEVEL: logger.getLogLevel().toString(), + ...(noColor ? { NO_COLOR: '1' } : { FORCE_COLOR: process.env.FORCE_COLOR || (process.stdout.isTTY ? '1' : '0') }), + TERM: process.env.TERM || 'xterm-256color', + }; + })(),This destructures the parent env to exclude
FORCE_COLORandNO_COLORupfront, then sets only the intended value, ensuring no ambiguity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/processConcurrency.ts` around lines 89 - 105, The child_process env construction can end up containing both NO_COLOR and FORCE_COLOR which creates ambiguity; update the runtime === 'child_process' env logic so that when NO_COLOR is present (process.env.NO_COLOR or process.argv.includes('--no-color')) you explicitly remove/omit FORCE_COLOR from the child env and only set NO_COLOR='1', otherwise restore FORCE_COLOR based on process.env.FORCE_COLOR or process.stdout.isTTY; implement this by destructuring process.env to exclude FORCE_COLOR (and NO_COLOR when setting it) before spreading into env inside the env object used in the runtime === 'child_process' block so that functions like logger.getLogLevel(), and the checks process.env.NO_COLOR / process.argv.includes('--no-color') remain intact but FORCE_COLOR is never propagated when NO_COLOR is set.
🧹 Nitpick comments (2)
tests/shared/processConcurrency.test.ts (1)
171-178:beforeEachsetup is duplicated acrossinitWorker,initTaskRunner, andcolor propagationblocks.The three-line
os.availableParallelism+Tinypoolmock setup is repeated identically in lines 83–91, 135–143, and 171–178. Consider hoisting it to a sharedbeforeEachat the top-levelprocessConcurrencydescribe (or a helper function) to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/shared/processConcurrency.test.ts` around lines 171 - 178, The three-line mock setup for os.availableParallelism and Tinypool that appears in beforeEach blocks (mocking os.availableParallelism and mocking Tinypool implementation with run/destroy) is duplicated; hoist this setup into a single top-level beforeEach inside the processConcurrency describe (or extract into a helper function called from each beforeEach), replacing the repeated blocks in the initWorker, initTaskRunner, and color propagation describe blocks so tests reuse the same mocked os.availableParallelism and Tinypool implementation (refer to the mocked os.availableParallelism and Tinypool mockImplementation/run/destroy symbols to locate and remove the duplicates).tests/shared/logger.test.ts (1)
115-122: Test is a near-duplicate and doesn't validate its stated behavior.The assertion
expect.stringContaining('CYAN:Test: ')is functionally equivalent to the already-existing test at lines 104–108 ('Object:'→'Test:'). Neither test actually verifies thatutil.inspectreceives{ colors: true }— only that the outerpc.cyan()wrapper is applied, which was true before this PR.To genuinely verify the new
colors: useColorspath, spy onutil.inspectdirectly:🧪 Suggested stronger test
+import util from 'node:util'; describe('color support in formatArgs', () => { it('should pass pc.isColorSupported to util.inspect for object formatting', () => { + const inspectSpy = vi.spyOn(util, 'inspect'); const obj = { key: 'value' }; logger.info('Test:', obj); - // When isColorSupported is true (mock), util.inspect should use colors - expect(console.log).toHaveBeenCalledWith(expect.stringContaining('CYAN:Test: ')); + expect(inspectSpy).toHaveBeenCalledWith(obj, expect.objectContaining({ colors: true })); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/shared/logger.test.ts` around lines 115 - 122, The test is a near-duplicate and doesn't actually assert that util.inspect receives colors: true; replace or extend the spec that references formatArgs/logger.info to spyOn (or jest.spyOn) util.inspect, call logger.info('Test:', obj) with pc.isColorSupported mocked true, and assert util.inspect was called with the object and an options object containing { colors: true } (and similarly add a case for pc.isColorSupported false asserting colors: false); ensure the spy is restored after the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/shared/processConcurrency.test.ts`:
- Around line 170-242: Add a test in processConcurrency.test.ts that ensures
when NO_COLOR is active (either via process.env.NO_COLOR = '1' or via
'--no-color' in process.argv) any pre-existing FORCE_COLOR in the parent env is
not passed to workers: set process.env.FORCE_COLOR = '1' before calling
createWorkerPool, set NO_COLOR (or argv) so the code should drop FORCE_COLOR,
call createWorkerPool, and assert Tinypool was called with env not containing
FORCE_COLOR (or containing NO_COLOR and not FORCE_COLOR). Use the existing test
patterns and restore process.env and process.argv afterward; reference
createWorkerPool and Tinypool to locate where to add the assertion.
---
Outside diff comments:
In `@src/shared/processConcurrency.ts`:
- Around line 89-105: The child_process env construction can end up containing
both NO_COLOR and FORCE_COLOR which creates ambiguity; update the runtime ===
'child_process' env logic so that when NO_COLOR is present (process.env.NO_COLOR
or process.argv.includes('--no-color')) you explicitly remove/omit FORCE_COLOR
from the child env and only set NO_COLOR='1', otherwise restore FORCE_COLOR
based on process.env.FORCE_COLOR or process.stdout.isTTY; implement this by
destructuring process.env to exclude FORCE_COLOR (and NO_COLOR when setting it)
before spreading into env inside the env object used in the runtime ===
'child_process' block so that functions like logger.getLogLevel(), and the
checks process.env.NO_COLOR / process.argv.includes('--no-color') remain intact
but FORCE_COLOR is never propagated when NO_COLOR is set.
---
Nitpick comments:
In `@tests/shared/logger.test.ts`:
- Around line 115-122: The test is a near-duplicate and doesn't actually assert
that util.inspect receives colors: true; replace or extend the spec that
references formatArgs/logger.info to spyOn (or jest.spyOn) util.inspect, call
logger.info('Test:', obj) with pc.isColorSupported mocked true, and assert
util.inspect was called with the object and an options object containing {
colors: true } (and similarly add a case for pc.isColorSupported false asserting
colors: false); ensure the spy is restored after the test.
In `@tests/shared/processConcurrency.test.ts`:
- Around line 171-178: The three-line mock setup for os.availableParallelism and
Tinypool that appears in beforeEach blocks (mocking os.availableParallelism and
mocking Tinypool implementation with run/destroy) is duplicated; hoist this
setup into a single top-level beforeEach inside the processConcurrency describe
(or extract into a helper function called from each beforeEach), replacing the
repeated blocks in the initWorker, initTaskRunner, and color propagation
describe blocks so tests reuse the same mocked os.availableParallelism and
Tinypool implementation (refer to the mocked os.availableParallelism and
Tinypool mockImplementation/run/destroy symbols to locate and remove the
duplicates).
| describe('color propagation to worker processes', () => { | ||
| beforeEach(() => { | ||
| vi.mocked(os).availableParallelism = vi.fn().mockReturnValue(4); | ||
| vi.mocked(Tinypool).mockImplementation(function (this: unknown) { | ||
| (this as Record<string, unknown>).run = vi.fn(); | ||
| (this as Record<string, unknown>).destroy = vi.fn(); | ||
| return this as Tinypool; | ||
| }); | ||
| }); | ||
|
|
||
| it('should propagate NO_COLOR to worker when NO_COLOR env is set', () => { | ||
| const originalNoColor = process.env.NO_COLOR; | ||
| process.env.NO_COLOR = '1'; | ||
| try { | ||
| createWorkerPool({ numOfTasks: 100, workerType: 'fileProcess', runtime: 'child_process' }); | ||
|
|
||
| expect(Tinypool).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| env: expect.objectContaining({ | ||
| NO_COLOR: '1', | ||
| }), | ||
| }), | ||
| ); | ||
| } finally { | ||
| if (originalNoColor === undefined) { | ||
| delete process.env.NO_COLOR; | ||
| } else { | ||
| process.env.NO_COLOR = originalNoColor; | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| it('should propagate NO_COLOR to worker when --no-color is in argv', () => { | ||
| const originalArgv = process.argv; | ||
| process.argv = [...originalArgv, '--no-color']; | ||
| try { | ||
| createWorkerPool({ numOfTasks: 100, workerType: 'fileProcess', runtime: 'child_process' }); | ||
|
|
||
| expect(Tinypool).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| env: expect.objectContaining({ | ||
| NO_COLOR: '1', | ||
| }), | ||
| }), | ||
| ); | ||
| } finally { | ||
| process.argv = originalArgv; | ||
| } | ||
| }); | ||
|
|
||
| it('should set FORCE_COLOR when colors are not disabled', () => { | ||
| const originalNoColor = process.env.NO_COLOR; | ||
| delete process.env.NO_COLOR; | ||
| const originalArgv = process.argv; | ||
| process.argv = originalArgv.filter((arg) => arg !== '--no-color'); | ||
| try { | ||
| createWorkerPool({ numOfTasks: 100, workerType: 'fileProcess', runtime: 'child_process' }); | ||
|
|
||
| expect(Tinypool).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| env: expect.objectContaining({ | ||
| FORCE_COLOR: expect.any(String), | ||
| }), | ||
| }), | ||
| ); | ||
| } finally { | ||
| if (originalNoColor !== undefined) { | ||
| process.env.NO_COLOR = originalNoColor; | ||
| } | ||
| process.argv = originalArgv; | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing test coverage for FORCE_COLOR leak when NO_COLOR is active.
The three new tests cover the positive propagation cases, but there is no test verifying that a pre-existing FORCE_COLOR in the parent environment is absent from the child env when NO_COLOR should take effect. This is precisely the defect described in the processConcurrency.ts comment above — the test suite would not catch it.
Add a fourth test:
🧪 Suggested additional test
+ it('should not carry FORCE_COLOR into worker when NO_COLOR is active', () => {
+ const originalNoColor = process.env.NO_COLOR;
+ const originalForceColor = process.env.FORCE_COLOR;
+ process.env.NO_COLOR = '1';
+ process.env.FORCE_COLOR = '1'; // simulate CI/parent setting FORCE_COLOR
+ try {
+ createWorkerPool({ numOfTasks: 100, workerType: 'fileProcess', runtime: 'child_process' });
+
+ const callArgs = vi.mocked(Tinypool).mock.calls[0][0] as { env: Record<string, string> };
+ expect(callArgs.env.NO_COLOR).toBe('1');
+ expect(callArgs.env.FORCE_COLOR).toBeUndefined();
+ } finally {
+ if (originalNoColor === undefined) delete process.env.NO_COLOR;
+ else process.env.NO_COLOR = originalNoColor;
+ if (originalForceColor === undefined) delete process.env.FORCE_COLOR;
+ else process.env.FORCE_COLOR = originalForceColor;
+ }
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/shared/processConcurrency.test.ts` around lines 170 - 242, Add a test
in processConcurrency.test.ts that ensures when NO_COLOR is active (either via
process.env.NO_COLOR = '1' or via '--no-color' in process.argv) any pre-existing
FORCE_COLOR in the parent env is not passed to workers: set
process.env.FORCE_COLOR = '1' before calling createWorkerPool, set NO_COLOR (or
argv) so the code should drop FORCE_COLOR, call createWorkerPool, and assert
Tinypool was called with env not containing FORCE_COLOR (or containing NO_COLOR
and not FORCE_COLOR). Use the existing test patterns and restore process.env and
process.argv afterward; reference createWorkerPool and Tinypool to locate where
to add the assertion.
|
Addressed the actionable review in commit Changes:
|
|
Follow-up after running full local verification in a project-specific environment. Updated branch head: Validation run (from repo root):
Results:
This PR is fully validated locally and includes regression coverage for the |
2 participants
Summary
Fixes #1057
--no-coloras a proper Commander CLI option so it appears in--helpand is not rejected as an unknown optionFORCE_COLORpropagation in worker processes: previously workers always setFORCE_COLORwhich overridesNO_COLOR. Now respectsNO_COLORenv var and--no-colorflag by propagatingNO_COLOR=1to child processes insteadcolors: truein logger'sformatArgs: now usespc.isColorSupportedsoutil.inspectrespects the user's color settingsplain,monochrome) mapping to--no-colorfor discoverabilitycolorproperty toCliOptionstype for Commander compatibilityThe underlying color library (
picocolors) already natively disables coloring when--no-coloris inprocess.argvorNO_COLORis set — this PR makes the flag discoverable in--help, fixes bugs where the color-disable setting was not properly propagated, and follows the NO_COLOR standard.Test plan
--no-colorflag accepted without error (cliRun.test.ts)NO_COLORenv propagated to worker processes (processConcurrency.test.ts)--no-colorargv propagated to worker processes (processConcurrency.test.ts)FORCE_COLORset when colors are not disabled (processConcurrency.test.ts)pc.isColorSupportedused in logger formatArgs (logger.test.ts)