Skip to content

fix(cli): accept --no-color after subcommands#93324

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
ooiuuii:fix/cli-no-color-subcommands
Jun 15, 2026
Merged

fix(cli): accept --no-color after subcommands#93324
vincentkoc merged 1 commit into
openclaw:mainfrom
ooiuuii:fix/cli-no-color-subcommands

Conversation

@ooiuuii

@ooiuuii ooiuuii commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Root --no-color is accepted before a subcommand, but non-help subcommands can reject it when callers append it after the subcommand, for example openclaw doctor --no-color ... or openclaw doctor --lint --json --no-color.
  • This matters for automation/proof runners that append global output flags after the command they are invoking.
  • This PR normalizes unambiguous trailing --no-color tokens back into the root option prefix before Commander parses the final command.
  • Out of scope: changing doctor behavior, changing color rendering, or broadly reordering every root option after subcommands.
  • Success looks like real subcommands accepting trailing --no-color while --no-color still remains a literal value when a required command option would consume it.
  • Reviewer focus: argv normalization around --, required option values such as --message --no-color, and the final Commander-metadata pass that handles boolean command flags without a hard-coded allowlist.

Linked context

No separate issue. This is a small source-proven CLI parser bug found while running Windows upgrade proof commands.

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: root --no-color after a subcommand could be parsed as an unknown command option instead of the root option on non-help command paths.
  • Real environment tested: Windows source checkout on branch fix/cli-no-color-subcommands, commit 361f9c0a57, Node v22.22.0, isolated OpenClaw state under .artifacts/no-color-proof, source rebuilt through scripts/run-node.mjs, then verified through built dist/entry.js.
  • Exact steps or command run after this patch:
node dist/entry.js doctor --lint --json --no-color --severity-min error
  • Evidence after fix:
{"ok":true,"checksRun":22,"checksSkipped":0,"findings":[]}
  • Observed result after fix: the command accepts --no-color after doctor boolean flags and reaches doctor lint JSON output; Commander no longer rejects it as an unknown command option.
  • What was not tested: full CLI suite, packaged npm install, every subcommand/plugin command combination.
  • Proof limitations or environment constraints: source checkout proof only, not a packaged release artifact. The source runner rebuild output was omitted from the evidence block; the copied output above is from a clean dist/entry.js run after rebuild.
  • Before evidence (optional but encouraged): on installed OpenClaw 2026.6.8-beta.1, openclaw doctor --no-color --lint --json --severity-min error exited before doctor ran with OpenClaw does not recognize option "--no-color". Try: openclaw doctor error --help.

Tests and validation

Commands run:

node scripts/run-vitest.mjs src/cli/argv.test.ts
node scripts/run-oxlint.mjs src\cli\argv.ts src\cli\run-main.ts src\cli\argv.test.ts
node_modules\.bin\oxfmt.cmd --check --threads=1 src\cli\argv.ts src\cli\run-main.ts src\cli\argv.test.ts
git diff --check
node scripts/run-node.mjs doctor --lint --json --no-color --severity-min error
node dist/entry.js doctor --lint --json --no-color --severity-min error
node dist/entry.js setup --wizard --no-color --help
python -X utf8 .agents\skills\autoreview\scripts\autoreview --mode local

Results:

argv focused Vitest: 117 passed
oxlint: passed
oxfmt --check: passed
git diff --check: passed
doctor real CLI proof: {"ok":true,"checksRun":22,"checksSkipped":0,"findings":[]}
setup --wizard --no-color --help: rendered setup help without unknown-option failure
autoreview: clean, no accepted/actionable findings

Regression coverage added:

  • Moves --no-color from an unambiguous subcommand position to the root option prefix.
  • Preserves existing root option prefixes such as --profile work.
  • Leaves --no-color after -- positional.
  • Preserves literal required-option values such as agent --message --no-color.
  • Lets final Commander command metadata lift --no-color after boolean command flags without a hand-maintained boolean allowlist.

Risk checklist

Did user-visible behavior change? (Yes/No)

Yes. The CLI now accepts root --no-color after subcommands in unambiguous positions.

Did config, environment, or migration behavior change? (Yes/No)

No.

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No)

No.

What is the highest-risk area?

Accidentally treating a user-provided literal --no-color as the root flag when a command option expects it as a value.

How is that risk mitigated?

The early normalization remains conservative, the final parse uses Commander option metadata, and tests cover required-option values plus the -- terminator.

Current review state

What is the next action?

Wait for CI, ClawSweeper, and maintainer review.

What is still waiting on author, maintainer, CI, or external proof?

CI and ClawSweeper are pending after PR creation.

Which bot or reviewer comments were addressed?

Local autoreview findings were addressed before opening: preserving --no-color as a required option value, and avoiding a hard-coded boolean allowlist by using final Commander command metadata.

@openclaw-barnacle openclaw-barnacle Bot added cli CLI command changes size: M proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 15, 2026
@clawsweeper

clawsweeper Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 15, 2026, 10:56 AM ET / 14:56 UTC.

Summary
The branch adds CLI argv normalization and tests so trailing --no-color after subcommands is lifted to the root option when it is not a command option value.

PR surface: Source +140, Tests +56. Total +196 across 3 files.

Reproducibility: yes. I did not run the CLI locally, but current source plus the Commander 14 contract show why doctor --lint --json --no-color is rejected before the patch, and the PR includes before/after command evidence.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: migration/backfill/repair: src/cli/argv.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Next step before merge

  • [P2] No repair job is needed; the branch is focused and reviewable, with normal CI and maintainer review remaining.

Security
Cleared: The diff is limited to CLI argv parsing and tests, with no dependency, workflow, secret, auth, network, or package-resolution changes.

Review details

Best possible solution:

Land the centralized argv-normalization path after normal CI and maintainer review, keeping the -- and required-option-value protections covered by tests.

Do we have a high-confidence way to reproduce the issue?

Yes. I did not run the CLI locally, but current source plus the Commander 14 contract show why doctor --lint --json --no-color is rejected before the patch, and the PR includes before/after command evidence.

Is this the best way to solve the issue?

Yes. Central argv normalization after real command registration is narrower than adding command-local --no-color flags or relaxing Commander positional option behavior globally.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against c1219d161d3e.

Label changes

Label changes:

  • add P2: This fixes a real but bounded CLI automation ergonomics bug around a documented global output flag.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-fix real CLI output from a Windows source checkout and built dist/entry.js showing the changed doctor command reaches JSON output.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides after-fix real CLI output from a Windows source checkout and built dist/entry.js showing the changed doctor command reaches JSON output.

Label justifications:

  • P2: This fixes a real but bounded CLI automation ergonomics bug around a documented global output flag.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides after-fix real CLI output from a Windows source checkout and built dist/entry.js showing the changed doctor command reaches JSON output.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-fix real CLI output from a Windows source checkout and built dist/entry.js showing the changed doctor command reaches JSON output.
Evidence reviewed

PR surface:

Source +140, Tests +56. Total +196 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 2 143 3 +140
Tests 1 56 0 +56
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 199 3 +196

What I checked:

  • Current global flag contract: --no-color is documented as a global CLI flag and registered on the root Commander program. Public docs: docs/cli/index.md. (docs/cli/index.md:47, c1219d161d3e)
  • Current root-option parser: Current main already treats --no-color as a root boolean option in the shared root-option scanner. (src/infra/cli-root-options.ts:4, c1219d161d3e)
  • Patch normalization: The PR adds normalizeRootNoColorArgv, which moves unambiguous --no-color tokens into the root prefix while preserving -- positionals and possible option values. (src/cli/argv.ts:274, 361f9c0a5714)
  • Commander-metadata pass: The final parse path normalizes again after the real Commander tree is registered, using command option metadata before calling program.parseAsync. (src/cli/run-main.ts:967, 361f9c0a5714)
  • Regression coverage: The added tests cover trailing subcommand --no-color, existing root options, --, required option values, assigned values, and final metadata lifting after boolean flags. (src/cli/argv.test.ts:175, 361f9c0a5714)
  • Dependency contract: Commander 14 marks required and optional option values on Option, consumes the next token for required options, and stops root option processing at subcommands when positional options are enabled.

Likely related people:

  • steipete: Recent live history shows repeated work on CLI argv/help/startup files, including root help routing and CLI helper documentation. (role: recent area contributor; confidence: high; commits: 20c4e9475a7f, dd0dd662a108, 5f6293a902aa; files: src/cli/run-main.ts, src/cli/argv.ts, docs/cli/index.md)
  • vincentkoc: Recent commits touched shared CLI help argv scanning and runtime/root-help startup imports around the same parsing and dispatch path. (role: adjacent CLI parsing contributor; confidence: medium; commits: 39c5de484d25, b01290cf646b, c363816fea41; files: src/cli/argv.ts, src/cli/run-main.ts)
  • alexzhu0: Recent run-main work kept machine-readable CLI startup output parseable, which shares the same pre-Commander parse flow affected here. (role: recent CLI startup contributor; confidence: medium; commits: 9ac30b587ebd; files: src/cli/run-main.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jun 15, 2026
@vincentkoc vincentkoc self-assigned this Jun 15, 2026

@vincentkoc vincentkoc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed live state, diff, and checks. Bounded CLI argv normalization with focused coverage for subcommand/root --no-color placement; checks are clean.

@vincentkoc
vincentkoc merged commit 212fef8 into openclaw:main Jun 15, 2026
216 of 222 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli CLI command changes P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants