docs(skills): correct flag/positional claims in reference cards - #123
Merged
Conversation
Several command cards claimed the flag form of a positional id "fails" or "is silently ignored". In every generated command, the flag and positional fold into the same request field (positional first, flag overwrites if both are given) — both are valid, not just the positional. Reworded the affected Gotchas across channel, field, schedule, status-page, team, role, and member to describe this accurately, and narrowed the schedule/role member-grant cases where only a plural flag (--schedule-ids, --member-ids) exists as the alternative. Also: dropped a channel-create Gotcha whose premise contradicted the fence above it (--channel-name/--team-id are marked required there); restored the member-delete safety check's full scope (team membership, SSO-provisioned members); added the seven RUM verbs missing from the intent-to-verb table; scoped the miniprogram note on rum.md to application creation; and reworded the calendar is-off note to attribute the rejection to the CLI's own validation rather than the server.
The prose example set time_window to a bare 300, which reads as either 5 minutes or 5 hours depending on which unit you assume. State the unit explicitly (minutes, default cap 1440 = 24h) and switch the example to a value that only reads sensibly one way.
The validator's positional-as-flag check asserted that a flag folded into a required positional always fails the binary's Args check when passed alone, and flagged any doc example using such a flag. That premise doesn't hold: every generated command with this kind of fold uses requireBodyFieldOrExactArg or requireBodyFieldOrArgs (internal/cli/args.go), both of which explicitly accept the flag as a standalone alternative to the positional. cligen never emits the bare requireExactArg/requireArgs form that would make a flag-only call fail, so the failure mode this rule targeted cannot occur for any currently generated command — a truthful version of the check would never fire. Removed it rather than keep an inert rule; a folded flag now validates through the same path as any other registered flag, and an actually unregistered flag is still caught as unknown-flag. Adjusted the corresponding test to assert the real behavior: a folded flag used alone or alongside its positional validates clean, while a genuinely unregistered flag on the same command still trips unknown-flag.
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
Several
skills/flashduty/reference/*.mdcommand cards claimed the flag form of a positional id "fails" or "is silently ignored" on generated commands. In fact every such command folds the positional into the request body first, then overwrites it with the matching flag if the flag was also given — so the flag works as a genuine standalone alternative, not just alongside the positional. Verified against--helpoutput and the generated command source (internal/cli/zz_generated_*.go,internal/cli/args.go), plus live invocations against an unreachable--base-urlto confirm each flag-only call clears argument validation before failing on the network hop (no real API endpoint was ever contacted).channel.md,field.md,role.md,team.md,member.md,schedule.md,status-page.md,rum.md: reworded the false "flag fails / positional only" Gotchas to state both forms work, with the flag winning when both are given. Forschedule infos/deleteandrole/membergrant-revoke verbs, narrowed the claim where only a plural flag (--schedule-ids,--member-ids,--role-ids) exists as the alternative (a singular flag there is a genuine unknown-flag error, not a rejected alternative).channel.md: dropped acreateGotcha claiming--channel-name/--team-idaren't marked required — the fence directly above it already marks both(required).member.md: restored thedeletesafety check's full scope — it also blocks deletion while the member belongs to any team, and SSO-provisioned members can't be deleted at all (even with--is-force).rum.md: added the seven fenced-but-unlisted verbs (data-query,facet-count,facet-list,field-list,application-webhook-test,session-replay-metadata,session-replay-segments) to the intent→verb table; scoped the miniprogram note to application creation (session/viewsourcedata can still carryminiprogram).calendar.md: attributed theis-offrequired-field rejection to the CLI's own request validation, not the server.No
GENERATED:*fence content was touched.Note for reviewers:
go run ./internal/cmd/skilldoc checkcurrently flags 5 of these lines underpositional-as-flag. That check assumes a flag folded into a required positional always fails when passed alone — which doesn't hold for commands built onrequireBodyFieldOrExactArg/requireBodyFieldOrArgs(see verification above). Filing that checker fix separately since it touches files outside this PR's scope.Test plan
go build ./...go test ./...go run ./internal/cmd/skilldoc check— 5 pre-existing-pattern findings, see note above; not a regression in fence content--helpoutput, generated command source, and live flag-only invocations