Repository navigation
fix(cli): list the flags each command registers in --usage - #122
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 11 files
Shadow auto-approve: would not auto-approve because issues were found.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 12 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes CLI --usage to list registered flags (whole-body flag name, completion --no-descriptions, runtime-added flags) and aligns help requiredness, with new runtime and snapshot tests pinning the corrected behavior.
View guided diff | Turn on auto-fix | Re-trigger cubic
AshGodfrey
left a comment
There was a problem hiding this comment.
Verified locally: CLI snapshot tests and the review SDK usage tests pass at this head; operationUsesMetadataFlags matches the inline logic it replaced. No blocking issues.
d0464bd to
05be2f7
Compare
`--usage` disagreed with the flags commands accept in three ways: - A body-only operation whose JSON/form body is not expanded into field flags (union, map, array or scalar body) registers one flag named after the request body (e.g. `--request`), but `--usage` listed `--body`, which the command rejects, and omitted the real flag. A multipart body-only operation without flag metadata also lacked its body flag. `--usage` decided the flag with `templateHasBodyFlag`, while `opcmd.go.stmpl` used separate inline logic. - cobra's `completion bash|zsh|fish|powershell` accept `--no-descriptions`, which the `completion` nodes did not list. - Flags added at runtime to a generated command were listed only when the command was marked with `usage.MarkDynamic`. `operationUsesMetadataFlags` (side-effect free) now makes the request strategy decision for both `opcmd.go.stmpl` and `--usage`; `getOperationWholeBodyUsageFlag` lists the whole-body flag the command registers, with its help text and `[required]` marker. The completion shell nodes list `--no-descriptions`. A command rendered from its `usageNodes` entry also lists visible local flags the entry does not have, so `MarkDynamic` is only needed when help, aliases or args change at runtime. Generated command files are unchanged. Tests: `TestUsageSchema_FlagsMatchRegisteredFlags` checks every command's own `--usage` against its registered flags in both directions; `TestUsageSchema_RuntimeFlagsKeepGeneratedLines` replaces `TestUsageSchema_DynamicCommandKeepsGeneratedLines` and runs with and without `MarkDynamic`. README `--usage` section updated; review SDK regenerated.
…only enforced bodies required - Every command rendered from its `usageNodes` entry now takes its flags from cobra (`usageFlagLines`), marked dynamic or not: generated flag lines for the flags it still shows (a flag hidden at runtime is dropped), then visible local flags added at runtime. `usageFlagLines` reads `LocalFlags()` first, which merges persistent flags into `Flags()` before the lookups; without it a root marked dynamic before its first render dropped every generated root flag. - The whole-body flag help says `[required]` only when the body is neither optional nor nullable, matching `templateRequestBodyRequired`. This also corrects `--help` for nullable body-only operations. - `operationUsesMetadataFlags` replaces `intentOperationHasMeta`; the unused `operationHasFlagMetadata` is removed. - `--usage` reads the unescaped `--body` help (`bodyFlagDescription`); the template wrapper still escapes it for Go source. Tests: the empty-body snapshot test checks the generated `usageNodes` entries of body-only operations (registered whole-body flag, no `--body`, `[required]` exactly when the command requires it) and `--no-descriptions` on the completion shells, since the rendered output also lists flags missing from the table. `TestUsageSchema_FlagsMatchRegisteredFlags` covers the root; new `TestUsageSchema_RuntimeHiddenFlagDropped`; `TestUsageSchema_DynamicRoot` marks a fresh root dynamic before its first render. README and review SDK updated.
05be2f7 to
a1103b5
Compare
Why
--usageshould list exactly the flags each command accepts. Three gaps remained:--request,--vehicle, …).--usagelisted--bodyinstead, which the command rejects withunknown flag: --body, and never listed the real, often required, flag. A multipart body-only operation without flag metadata had the same missing flag and no replacement.--usageusedtemplateHasBodyFlag, whileopcmd.go.stmpldecided the registered flag with separate inline logic. In the primary test CLI, 113 commands advertised a--bodythey reject.completion <shell> --no-descriptions. Cobra registers it onbash/zsh/fish/powershell; the hand-writtencompletionnodes inincludes/usage.tsdid not list it.custom.Register) appeared in--usageonly when the command was also marked withusage.MarkDynamic, including on the root.What changed
includes/metadata.ts: new side-effect-freeoperationUsesMetadataFlags, the request-strategy decision (flag metadata plus optional--body, or one whole-body flag). It replaces the inline logic inopcmd.go.stmplandintentOperationHasMetainincludes/intents.ts; the now unusedoperationHasFlagMetadatais removed. Generated command files are byte-identical apart from the[required]change below.includes/usage.ts:getOperationWholeBodyUsageFlagmirrors the flagopcmd.go.stmplregisters:--bodywith flag metadata; otherwise--<request body name>for body-only operations, with the same help text; otherwise--body. The fourcompletionshell nodes list--no-descriptions.includes/templating.ts,includes/metadata.ts:requestBodyDescriptionandbodyFlagDescriptionreturn unescaped help text for--usage; thetemplate*wrappers escape it for Go source.includes/templating.ts: the whole-body flag help says[required]only when the body is neither optional nor nullable, matching the requiredness the command enforces (templateRequestBodyRequired). This also corrects--helpfor nullable body-only operations.usage.go.stmpl: every command rendered from itsusageNodesentry, marked dynamic or not, takes its flags from cobra (usageFlagLines): generated flag lines for the flags it still shows (a flag hidden at runtime is dropped), then visible local flags added at runtime.MarkDynamicis now only needed when help, aliases or args change at runtime.usageFlagLinesreadsLocalFlags()first, which merges persistent flags intoFlags()before the lookups, so a root marked dynamic before its first render keeps its generated flags.usage_test.go.stmpl:TestUsageSchema_FlagsMatchRegisteredFlagswalks every command, root included, and asserts that each flag in its own--usageis registered and each visible registered flag is listed.TestUsageSchema_RuntimeFlagsKeepGeneratedLinesreplacesTestUsageSchema_DynamicCommandKeepsGeneratedLinesand runs with and withoutMarkDynamic, adding a flag on a generated command and a persistent flag on the root.TestUsageSchema_RuntimeHiddenFlagDropped: a generated flag hidden at runtime leaves the command's--usage.TestUsageSchema_DynamicRootmarks a fresh root dynamic before its first render.pkg/generate/snapshots/cli_empty_body_go_test.go: the generatedusageNodesentries of body-only operations list the whole-body flag the command registers (not--body), with[required]exactly when the command requires it, and thecompletionshell entries list--no-descriptions. These check the table itself; the runtime tests above check the rendered output, which also lists flags missing from the table.README.md:--usagesection describes the whole-body flag name,--no-descriptionsand runtime-added flags.zSDKs/sdk-cliregenerated:chatlists--requestinstead of--body;completion <shell>lists--no-descriptions. All other--usageoutput is unchanged.core, cli, fix, patch.Testing
TARGET=primary make test-cli: 2189 tests pass.go test ./pkg/generate/snapshots -run TestSnapCLI: pass.make lint: pass.TARGET=review make build-cli: diff limited tointernal/usage/schema.go,tests/usage_test.goandgen.lock; generated command files are byte-identical.cli_empty_body_go_test.goassertions fail on the base branch templates (standaloneoptionallists--body <body>, not--empty-body) and pass with this change.schema.go,TestUsageSchema_FlagsMatchRegisteredFlagsfails (chat --usage lists unregistered --body) and passes with this change.--usagecompared for every command path against the base branch:chat,completionand its four shells), only by the lines above;--bodyreplaced with the registered whole-body flag and--no-descriptions; generated command files differ only where a nullable whole-body flag's help loses[required](8 commands);completionand its shells differ.Public-safety check
.claude/skills/public-repo-communication/SKILL.md).git diff --check.