fix(cli): accept --no-color after subcommands#93324
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 15, 2026, 10:56 AM ET / 14:56 UTC. Summary 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 Review metrics: none identified. Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land the centralized argv-normalization path after normal CI and maintainer review, keeping the 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 Is this the best way to solve the issue? Yes. Central argv normalization after real command registration is narrower than adding command-local AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c1219d161d3e. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +140, Tests +56. Total +196 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
vincentkoc
left a comment
There was a problem hiding this comment.
Reviewed live state, diff, and checks. Bounded CLI argv normalization with focused coverage for subcommand/root --no-color placement; checks are clean.
Summary
--no-coloris accepted before a subcommand, but non-help subcommands can reject it when callers append it after the subcommand, for exampleopenclaw doctor --no-color ...oropenclaw doctor --lint --json --no-color.--no-colortokens back into the root option prefix before Commander parses the final command.--no-colorwhile--no-colorstill remains a literal value when a required command option would consume it.--, 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)
--no-colorafter a subcommand could be parsed as an unknown command option instead of the root option on non-help command paths.fix/cli-no-color-subcommands, commit361f9c0a57, Nodev22.22.0, isolated OpenClaw state under.artifacts/no-color-proof, source rebuilt throughscripts/run-node.mjs, then verified through builtdist/entry.js.{"ok":true,"checksRun":22,"checksSkipped":0,"findings":[]}--no-colorafterdoctorboolean flags and reaches doctor lint JSON output; Commander no longer rejects it as an unknown command option.dist/entry.jsrun after rebuild.OpenClaw 2026.6.8-beta.1,openclaw doctor --no-color --lint --json --severity-min errorexited before doctor ran withOpenClaw does not recognize option "--no-color". Try: openclaw doctor error --help.Tests and validation
Commands run:
Results:
Regression coverage added:
--no-colorfrom an unambiguous subcommand position to the root option prefix.--profile work.--no-colorafter--positional.agent --message --no-color.--no-colorafter 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-colorafter 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-coloras 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-coloras a required option value, and avoiding a hard-coded boolean allowlist by using final Commander command metadata.