Skip to content

fix: Make CLI help consistent for native and Unity commands#1146

Merged
hatayama merged 3 commits into
v3-betafrom
feature/hatayama/improve-cli-help-consistency
May 17, 2026
Merged

fix: Make CLI help consistent for native and Unity commands#1146
hatayama merged 3 commits into
v3-betafrom
feature/hatayama/improve-cli-help-consistency

Conversation

@hatayama
Copy link
Copy Markdown
Owner

Summary

  • Command-specific help now works consistently for native commands and Unity tool commands.
  • CLI option discovery no longer re-exposes removed options from stale project caches, and README examples now use the current flag names.

User Impact

  • Users can run <command> --help for both built-in CLI commands and Unity tools without needing Unity project resolution first.
  • Deprecated or removed options such as --wait-for-domain-reload no longer appear through stale cache data, reducing confusing guidance.
  • Completion helper commands are documented in the completion help instead of crowding the main help output.

Changes

  • Added a shared command help renderer for Unity tool schemas and native command global options.
  • Prefer the embedded first-party tool catalog for help and option completion when the command is bundled with the CLI.
  • Updated help tests, completion tests, tool parsing tests, README examples, and regenerated checked-in native CLI binaries.

Verification

  • scripts/check-go-cli.sh
  • ~/.codex/skills/codex-review/scripts/codex-review v3-beta --parallel-tests "scripts/check-go-cli.sh"

hatayama added 3 commits May 17, 2026 10:19
Expose command help for first-party tools and native subcommands before Unity project resolution, so users can rely on `uloop <command> --help` for the current CLI surface.

Keep first-party option discovery anchored to the embedded schema so stale project caches cannot re-expose removed flags such as `--wait-for-domain-reload`, and update docs to describe the public `--no-wait-for-domain-reload` form.
The main help now matches the CLI behavior added for first-party Unity tools, so users are no longer pointed only at native command help when `uloop <command> --help` works for both command families.
Move completion-only probes out of the main help, keep command list summaries concise, and make project-scoped command help expose the shared project path option. Also let --help win after other command options and report unknown leading flags as global option errors.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 17, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR refactors the CLI help system by introducing unified command help routing, generalizing help-request detection across commands, integrating tool help resolution, adding global options to help outputs, validating unknown global options, and providing comprehensive test coverage for the new behavior.

Changes

CLI Help System Refactor

Layer / File(s) Summary
Core help routing and formatting infrastructure
Packages/src/Cli~/internal/cli/command_help.go, Packages/src/Cli~/internal/cli/run_help.go
New command_help.go implements tryHandleCommandHelp router for native commands or default tool help resolution; adds formatting helpers for option descriptions, defaults, and PascalCase-to-spaced conversion. run_help.go introduces containsHelpRequest helper and commandListDescription formatter for truncated tool descriptions.
Help request detection generalization
Packages/src/Cli~/internal/cli/launch.go, Packages/src/Cli~/internal/cli/completion.go, Packages/src/Cli~/internal/cli/skills.go, Packages/src/Cli~/internal/cli/uninstall.go, Packages/src/Cli~/internal/cli/update.go, Packages/src/Cli~/internal/cli/run.go
Commands now use containsHelpRequest to detect --help or -h anywhere in argument lists, replacing strict argument-count checks; improves help discoverability when flags are intermixed with other options.
Tool discovery and description formatting
Packages/src/Cli~/internal/cli/tools.go, Packages/src/Cli~/internal/cli/completion.go
New findDefaultTool helper loads the default tool catalog and resolves tools by name; findToolForCommandWithInternalToolNames now prefers the default tool cache; completion help updated to document --list-commands and --list-options <command> helpers.
Global options integration in help outputs
Packages/src/Cli~/internal/cli/launch.go, Packages/src/Cli~/internal/cli/skills_display.go
Launch help now includes global options (--project-path); skills main help and new printSkillsSubcommandHelp both append global options display after their primary usage text.
Argument validation and error messaging
Packages/src/Cli~/internal/cli/run.go, Packages/src/Cli~/internal/cli/tools.go, Packages/src/Cli~/internal/cli/error_envelope.go
RunProjectLocal detects unknown leading options via isUnknownLeadingOption and returns exit code 1; updated error messages for invalid --project-path usage; parseGlobalProjectPath filters both --project-path and --project-path=... forms.
Comprehensive help system testing
Packages/src/Cli~/internal/cli/help_test.go, Packages/src/Cli~/internal/cli/completion_test.go, Packages/src/Cli~/internal/cli/tools_test.go
Updated existing help tests to reflect new output; added tests verifying tool description truncation, global options display, help precedence over execution, unknown option rejection, compile flag validation, and completion helper documentation.
Documentation updates
README.md, README_ja.md
Compile tool documentation updated to specify --no-wait-for-domain-reload flag (fire-and-forget behavior) replacing WaitForDomainReload=false.

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • hatayama/unity-cli-loop#1026: Introduces structured error envelopes; this PR refines error envelope messaging for PROJECT_NOT_FOUND and command validation errors in the new help/argument validation system.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.54% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately captures the main objective: improving CLI help consistency between native and Unity commands, which is the primary theme across all file changes.
Description check ✅ Passed The description clearly relates to the changeset, explaining the user impact, high-level changes, and verification steps for making CLI help consistent across command types.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/hatayama/improve-cli-help-consistency

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

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
Packages/src/Cli~/internal/cli/skills_display.go (1)

67-76: ⚡ Quick win

Generate the skill target flag list from shared config.

This hardcodes the same target set that printSkillsTargetGuidance already duplicates below, so the next target change can easily leave help output inconsistent again. Prefer rendering these flags from defaultSkillTargetIDs/targetConfigs via a shared helper.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Packages/src/Cli`~/internal/cli/skills_display.go around lines 67 - 76, The
help output currently hardcodes the skill target flags (the writeLine calls)
causing duplication with printSkillsTargetGuidance; replace the hardcoded flag
lines by rendering them from the shared config
(defaultSkillTargetIDs/targetConfigs) via the same helper used by
printSkillsTargetGuidance (or introduce a small helper like
renderSkillTargetFlags) so both places derive flags from the same source; update
the section that writes "Options:" to call that helper and remove the duplicated
literal writeLine entries so changes to defaultSkillTargetIDs/targetConfigs
automatically update the help output.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@Packages/src/Cli`~/internal/cli/skills_display.go:
- Around line 67-76: The help output currently hardcodes the skill target flags
(the writeLine calls) causing duplication with printSkillsTargetGuidance;
replace the hardcoded flag lines by rendering them from the shared config
(defaultSkillTargetIDs/targetConfigs) via the same helper used by
printSkillsTargetGuidance (or introduce a small helper like
renderSkillTargetFlags) so both places derive flags from the same source; update
the section that writes "Options:" to call that helper and remove the duplicated
literal writeLine entries so changes to defaultSkillTargetIDs/targetConfigs
automatically update the help output.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d43ae29b-125a-4544-ac5f-dbb94c1c15a1

📥 Commits

Reviewing files that changed from the base of the PR and between 4e61787 and 536a6be.

⛔ Files ignored due to path filters (3)
  • Packages/src/Cli~/dist/darwin-amd64/uloop is excluded by !**/dist/** and included by none
  • Packages/src/Cli~/dist/darwin-arm64/uloop is excluded by !**/dist/** and included by none
  • Packages/src/Cli~/dist/windows-amd64/uloop.exe is excluded by !**/dist/**, !**/*.exe and included by none
📒 Files selected for processing (16)
  • Packages/src/Cli~/internal/cli/command_help.go
  • Packages/src/Cli~/internal/cli/completion.go
  • Packages/src/Cli~/internal/cli/completion_test.go
  • Packages/src/Cli~/internal/cli/error_envelope.go
  • Packages/src/Cli~/internal/cli/help_test.go
  • Packages/src/Cli~/internal/cli/launch.go
  • Packages/src/Cli~/internal/cli/run.go
  • Packages/src/Cli~/internal/cli/run_help.go
  • Packages/src/Cli~/internal/cli/skills.go
  • Packages/src/Cli~/internal/cli/skills_display.go
  • Packages/src/Cli~/internal/cli/tools.go
  • Packages/src/Cli~/internal/cli/tools_test.go
  • Packages/src/Cli~/internal/cli/uninstall.go
  • Packages/src/Cli~/internal/cli/update.go
  • README.md
  • README_ja.md

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 19 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Packages/src/Cli~/internal/cli/help_test.go">

<violation number="1" location="Packages/src/Cli~/internal/cli/help_test.go:251">
P1: The removed-flag assertion uses a substring that also matches `--no-wait-for-domain-reload`, making the test produce false failures.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic

Comment thread Packages/src/Cli~/internal/cli/help_test.go
@hatayama hatayama merged commit 802afa3 into v3-beta May 17, 2026
16 checks passed
@hatayama hatayama deleted the feature/hatayama/improve-cli-help-consistency branch May 17, 2026 02:05
@github-actions github-actions Bot mentioned this pull request May 17, 2026
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.

1 participant