fix: Show complete option help for every uloop command - #2016
Merged
hatayama merged 8 commits intoJul 26, 2026
Merged
Conversation
enable-pause-point accepts six orchestration flags that exist only in the CLI, so both option listings had to add them by hand and they drifted: `uloop list` documented all six while `--help` documented none of them. An agent reading `enable-pause-point --help` could not discover --await, --trigger, or --resume-play at all. Define the flags once in tooldocs and have both listings read that table, following the --code-file precedent. The dispatcher self-updates while the project runner stays pinned, so a new contract test asserts every flag the help table advertises is actually consumed by the runner's parsers — a drifted table would otherwise advertise options that fail on use.
Unity's schema cache serializes a C# enum default as its ordinal while listing the members by name, so `simulate-keyboard --help` reported "default: 0" for a parameter that only accepts "Press". `uloop list` already converted the ordinal back to a name; --help did not. Move that conversion into tooldocs so both listings share it. Indexing the name list by ordinal only works while the enum is zero-based and contiguous, so a Unity-side guard test now asserts that of every enum a first-party schema exposes — an explicit member value or a [Flags] enum would otherwise make --help name the wrong default.
`--help` can only list option names and one-line summaries, so an agent that discovered a command there had no way to learn that a skill with the workflow rules, response shapes, and failure diagnoses exists. Close each command's help with an instruction to load its skill. The mapping is a static table rather than a name derived from the command: the four pause-point commands are all documented by uloop-pause-point, and a uloop-enable-pause-point skill does not exist. Commands absent from the table (custom commands) get no line at all. A guard test asserts every skill named in the table is declared by a real SKILL.md.
Unity's schema generator emits "Parameter: <Name>" for every property with no [Description] attribute, and the generated cache has no tool-level description field at all. Inside a synced project that made `--help` and `uloop list` strictly worse than having no cache: 99 of 102 cached options carried no information, and the tool summary line disappeared entirely. Fall back to the descriptions embedded in this binary when the cache's text is a placeholder. Only description text is taken; type, enum, default, and required always stay as the cache reported them, since Unity is the authority on what the running Editor accepts. Authored descriptions and tools absent from the embedded catalog (custom commands) are left untouched. Both readers are wired up, because `list` formats the raw response and never passes through the project-cache loader. Also document that simulate-keyboard rejects bare digit keys, and that enable-pause-point's --max-preview-elements caps later pause-point-status previews too.
cli/common carries non-test source changes in this branch, so the dispatcher and project-runner release triggers have to be restamped in the same PR or check-release-triggers fails.
Contributor
📝 WalkthroughWalkthroughThe changes centralize tool descriptions, enum default rendering, pause-point CLI option definitions, and command-to-skill guidance. Catalog, list, help, schema generation, and pause-point parsing paths now consume these shared contracts with expanded validation coverage. ChangesTool metadata and catalog output
Pause-point CLI contract
Command skill guidance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CommandHelp
participant SkillGuidanceLine
participant ToolHelp
participant User
CommandHelp->>SkillGuidanceLine: resolve command skill
SkillGuidanceLine-->>CommandHelp: guidance line or no mapping
CommandHelp->>ToolHelp: append guidance after options
ToolHelp-->>User: rendered command help
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Review follow-ups on the skill guidance line and option help formatting: - The watch commands and `launch` now get the line too. Watch expressions are documented by the pause-point skill, and launch renders its own help, so it was the one command with a real skill that never mentioned it. - The wording no longer refers to "options not listed above": commands such as focus-window have no options for that phrasing to point at. It now names what the skill adds instead. - An empty-string default is no longer rendered, since Unity reports one for every unset string parameter and it printed a bare "default: " with nothing after it. `uloop list` already omitted it. - Widen the option column so --captured-variable-names does not push its own description out of alignment. - Route list's fallback call through the clicore facade like every other tool catalog access in that package. - Build Unity's placeholder description from the JSON property name. The CLI recognizes the placeholder by comparing against the JSON name it received, so a [JsonProperty] rename would otherwise leave the placeholder in help output as if it were a real description.
The six CLI-only flags were parsed by six hand-written same-shaped branches, pushing extractPausePointEnableAwaitFlags to 33 branches over 157 lines — past the repository's complexity limit — right as this branch introduced a shared table for the very same flags. Keying the handlers by that table's flag names makes the two sets comparable, so a flag can no longer be documented without being parsed or parsed without being documented; a contract test now asserts the two key sets are equal. That is the direction the previous subset check could not see, and it is how all six flags came to be missing from --help. Behavior-preserving: no existing test changed, and new tests pin the two edges that the branch order encoded implicitly — --await has no =value form and unrelated arguments pass through to schema parsing.
Unity reports an empty string as the default of every unset string parameter. `--help` already suppresses that value, but list still emitted "Default": "" for 20 options because encoding/json's omitempty cannot drop an empty string held in an interface field. Reporting nil restores the help/list symmetry this change set exists to guarantee. Re-stamps the shared release inputs for the cli/common-adjacent change.
hatayama
merged commit Jul 26, 2026
a0896f2
into
feature/cli-discoverability-integration
1 of 2 checks passed
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cli/common/tooldocs/tool_option_help.go (1)
96-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize the "omit empty-string default" rule in
tooldocs.Both
--helpandlistindependently re-implement "Unity's empty string means no default, so don't render it" — exactly the kind of contract this PR otherwise centralizes viatooldocs.EnumValueForNumericDefault. A future change to the omission rule (e.g. extending it to another sentinel) would need identical edits in both places to stay in sync.
cli/common/tooldocs/tool_option_help.go#L96-L100: extract thepropertyDefault != nil && propertyDefault != ""check into a sharedtooldocshelper (e.g.IsOmittedDefault(value any) bool) and use it here.cli/project-runner/internal/projectrunner/list_output.go#L112-L118: use the same shared helper instead of the localdefaultValue == ""check.♻️ Proposed shared helper
// in cli/common/tooldocs/enum_defaults.go or a new file func IsOmittedDefault(value any) bool { return value == nil || value == "" }🤖 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 `@cli/common/tooldocs/tool_option_help.go` around lines 96 - 100, The empty-string default omission rule is duplicated across help and list output; centralize it in a shared tooldocs helper. Add an IsOmittedDefault helper returning true for nil or empty-string values, use it in tool_option_help.go around property.EffectiveDefault(), and replace the local defaultValue == "" check in cli/project-runner/internal/projectrunner/list_output.go lines 112-118 with the helper.
🤖 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 `@cli/common/tooldocs/tool_option_help.go`:
- Around line 96-100: The empty-string default omission rule is duplicated
across help and list output; centralize it in a shared tooldocs helper. Add an
IsOmittedDefault helper returning true for nil or empty-string values, use it in
tool_option_help.go around property.EffectiveDefault(), and replace the local
defaultValue == "" check in
cli/project-runner/internal/projectrunner/list_output.go lines 112-118 with the
helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 86de72f8-ea1c-4f6f-b90e-0aa3fde22ada
📒 Files selected for processing (29)
Assets/Tests/Editor/DynamicCodeToolTests/FirstPartyToolSchemaMetadataTests.csPackages/src/Editor/ToolContracts/UnityCliLoopToolParameterSchemaGenerator.cscli/common/clicore/tool_catalog.gocli/common/tooldocs/enum_defaults.gocli/common/tooldocs/enum_defaults_test.gocli/common/tooldocs/pause_point_cli_options.gocli/common/tooldocs/pause_point_cli_options_test.gocli/common/tooldocs/skill_guidance.gocli/common/tooldocs/skill_guidance_test.gocli/common/tooldocs/tool_option_help.gocli/common/tools/catalog.gocli/common/tools/default-tools.jsoncli/common/tools/description_fallback.gocli/common/tools/description_fallback_test.gocli/dispatcher/internal/dispatcher/command_help.gocli/dispatcher/internal/dispatcher/help_test.gocli/dispatcher/internal/dispatcher/launch.gocli/dispatcher/shared-inputs-stamp.jsoncli/project-runner/internal/projectrunner/list_output.gocli/project-runner/internal/projectrunner/list_output_test.gocli/project-runner/internal/projectrunner/native_command_help.gocli/project-runner/internal/projectrunner/native_command_help_test.gocli/project-runner/internal/projectrunner/pause_point_captured_variables_mode.gocli/project-runner/internal/projectrunner/pause_point_cli_options_contract_test.gocli/project-runner/internal/projectrunner/pause_point_enable.gocli/project-runner/internal/projectrunner/pause_point_expect.gocli/project-runner/internal/projectrunner/pause_point_trigger.gocli/project-runner/internal/projectrunner/pause_point_wait.gocli/project-runner/shared-inputs-stamp.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
uloop <command> --helpnow shows the real description of every option, the default value as it must be typed, and every flag the command actually accepts.User Impact
Inside a synced Unity project,
--helpwas close to useless:Parameter: <Name>and the tool summary line was missing entirely — 99 of the 102 cached options carried no information. The same command run outside a project showed real help, so the cache made things strictly worse.simulate-keyboard --helpreporteddefault: 0for--action, a value that has to be passed asPress.enable-pause-point --helplisted none of its six orchestration flags (--await,--captured-variables,--captured-variable-names,--expect,--trigger,--resume-play), whileuloop listlisted all six. An agent reading the help could not discover them at all.After this change the same command reports its description, real per-option help,
default: Press, all six pause-point flags, and closes withLoad the uloop-simulate-keyboard skill before using options not listed above.Changes
--helprenderer anduloop list, so the two listings cannot drift. A contract test asserts every flag the help table advertises is consumed by the runner's parsers — the dispatcher self-updates while the runner stays pinned, so a drifted table would advertise options that fail on use.listformats the raw response and never passes through the project-cache loader.uloop-pause-point, and a command with no skill gets no line. A guard test asserts every skill named in the table is declared by a realSKILL.md.simulate-keyboard --keynow documents that digit keys areDigit0-Digit9/Numpad0-Numpad9and bare0-9is rejected;enable-pause-point --max-preview-elementsdocuments that the value also caps laterpause-point-statuspreviews.Verification
scripts/check-go-cli.sh: all four modules format/vet/lint clean (0 issues.x4) and all tests pass.uloop compile:ErrorCount: 0.uloop run-tests --filter-type regex --filter-value FirstPartyToolSchemaMetadataTests: 3/3 passed (2 existing + the new enum guard).uloop enable-pause-point --help: all six CLI-only flags listed.uloop list: the enable-pause-point entry is byte-identical to the previous output (descriptions, types, andfull|namesvalues compared against the base revision).uloop simulate-keyboard --help: tool description present,--actionshowsdefault: Press,--keyshows the digit rule, skill line present — verified both with a synced.uloop/tools.jsonand from a directory with no Unity project (embedded path).uloop list --project-path <repo>: placeholder descriptions dropped from 99 to 0 for first-party tools; the three custom sample commands correctly pass through untouched.pull_request.branchesis limited tomain/v3-beta), so the checks above were run locally; the integration PR will exercise CI.