From 61d6ed9116539747d6fbcb3416d196f3f626ae25 Mon Sep 17 00:00:00 2001 From: ysyneu Date: Tue, 28 Jul 2026 06:47:45 -0700 Subject: [PATCH 1/3] docs(skills): correct flag/positional claims in reference cards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- skills/flashduty/reference/calendar.md | 2 +- skills/flashduty/reference/channel.md | 3 +-- skills/flashduty/reference/field.md | 2 +- skills/flashduty/reference/member.md | 8 ++++---- skills/flashduty/reference/role.md | 4 ++-- skills/flashduty/reference/rum.md | 11 +++++++++-- skills/flashduty/reference/schedule.md | 2 +- skills/flashduty/reference/status-page.md | 2 +- skills/flashduty/reference/team.md | 4 ++-- 9 files changed, 22 insertions(+), 16 deletions(-) diff --git a/skills/flashduty/reference/calendar.md b/skills/flashduty/reference/calendar.md index c411d78..066842d 100644 --- a/skills/flashduty/reference/calendar.md +++ b/skills/flashduty/reference/calendar.md @@ -121,7 +121,7 @@ Update calendar ## Key concepts -- **`is-off` (bool, required on event-upsert):** `true` = mark as non-working day (holiday/closure); `false` = override to working day (make-up workday / 補班). This is the only enum-like field — it must be explicit; the server rejects a missing value. +- **`is-off` (bool, required on event-upsert):** `true` = mark as non-working day (holiday/closure); `false` = override to working day (make-up workday / 補班). This is the only enum-like field — it must be explicit; the CLI rejects a missing value before any request is sent. - **`end-at` is exclusive:** a single-day event on 2026-01-17 needs `--start-at 2026-01-17 --end-at 2026-01-18`. - **`workdays` integers:** 0 = Sunday, 1 = Monday … 6 = Saturday. Standard Mon–Fri = `1,2,3,4,5`. - **Calendar kinds:** `personal` (editable, default filter) vs `region.official.holiday` (read-only, browsable). The returned `kind` field can also be `religion.holiday`. diff --git a/skills/flashduty/reference/channel.md b/skills/flashduty/reference/channel.md index 82f98a4..d0f46e1 100644 --- a/skills/flashduty/reference/channel.md +++ b/skills/flashduty/reference/channel.md @@ -304,10 +304,9 @@ Update channel ## Gotchas -- **Positional trap**: `channel-id` is **positional** on `info`, `infos`, `update`, `delete`, `disable`, `enable`, `escalate-rule-list`, `inhibit-rule-create`, `inhibit-rule-list`, `silence-rule-create`, `silence-rule-list`, `unsubscribe-rule-create`, `unsubscribe-rule-list`. It is a **flag** (`--channel-id`) on all `escalate-rule-*`, `inhibit-rule-update/delete/enable/disable`, `silence-rule-update/delete/enable/disable`, `unsubscribe-rule-update/delete/enable/disable`. When in doubt, the fence heading `### verb ` = positional; heading without `<…>` = flag. +- **`channel-id` can be passed positionally or via `--channel-id` — both work.** On `info`, `infos`, `update`, `delete`, `disable`, `enable`, `escalate-rule-list`, `inhibit-rule-create`, `inhibit-rule-list`, `silence-rule-create`, `silence-rule-list`, `unsubscribe-rule-create`, `unsubscribe-rule-list`, the fence heading `### verb ` shows the shorter positional form, but the matching `--channel-id` flag is accepted too. On all `escalate-rule-*` (except `-list`), `inhibit-rule-update/delete/enable/disable`, `silence-rule-update/delete/enable/disable`, `unsubscribe-rule-update/delete/enable/disable` there is no positional — `--channel-id` is the only way in. If both positional and flag are given anywhere, the flag value wins. - **`escalate-rule-create` needs `layers` via `--data`** — it is required and cannot be expressed as a flat flag. Omitting it returns a validation error. - **`rule-id` is a MongoDB ObjectID string**, not an integer. Retrieve it from `escalate-rule-list`, `inhibit-rule-list`, `silence-rule-list`, or `unsubscribe-rule-list` before any update/delete/enable/disable. -- **`channel create` requires `--channel-name` and `--team-id`** even though they are not marked `required` in the flag list — the server rejects the request without them. - **`delete` on a channel is irreversible** — all rules within it are also removed. Confirm the `channel-id` against `list` before proceeding. - **Empty rule list is authoritative** — if `escalate-rule-list` / `silence-rule-list` / etc. returns no rows, no rules exist; do not widen the query. diff --git a/skills/flashduty/reference/field.md b/skills/flashduty/reference/field.md index e42b638..ea333f8 100644 --- a/skills/flashduty/reference/field.md +++ b/skills/flashduty/reference/field.md @@ -91,7 +91,7 @@ Update field ## Gotchas -- **`delete`, `info`, `update` take `` as a POSITIONAL first argument**, not `--field-id`. Example: `fduty field delete `, not `--field-id `. +- **`delete`, `info`, `update` take `` positionally or via `--field-id`** — both work, e.g. `fduty field delete ` or `fduty field delete --field-id `. If both are given, the flag wins. - **`--options` replaces the whole list on `update`** — omitting it leaves options unchanged, but a partial list silently drops the missing values. Always pass the full desired set. - **`--field-name` is the machine key** (`[a-zA-Z0-9_]`, starts with letter/underscore, ≤40 chars). It is the stable identifier for downstream enrichment rules — choose it carefully; it cannot be renamed. - **`delete` is permanent and cascades** — any enrichment rules that reference the field by `field_name` will lose their target. Confirm the name against `field list` before deleting. diff --git a/skills/flashduty/reference/member.md b/skills/flashduty/reference/member.md index 26b590c..df481ef 100644 --- a/skills/flashduty/reference/member.md +++ b/skills/flashduty/reference/member.md @@ -1,6 +1,6 @@ # fduty member — command card -Prereq: `SKILL.md` read. `invite` sends invitation emails immediately (up to 20 per call). `delete` is **irreversible** — it removes the member from the organization; default safety check rejects deletes when the member is referenced by escalation rules or schedules (pass `--is-force` to bypass). `role-update` **replaces** all role assignments atomically; `role-grant`/`role-revoke` are additive/subtractive. +Prereq: `SKILL.md` read. `invite` sends invitation emails immediately (up to 20 per call). `delete` is **irreversible** — it removes the member from the organization. Default safety check rejects deletes when the member is referenced by escalation rules, schedules, team membership, etc. (pass `--is-force` to bypass). A member provisioned via SSO cannot be deleted at all, even with `--is-force` — disable SSO management for them first. `role-update` **replaces** all role assignments atomically; `role-grant`/`role-revoke` are additive/subtractive. ## Route here when @@ -124,10 +124,10 @@ Update member roles - **Resolving a `person_id` → name: use `fduty person infos …`, NOT `member list`.** `schedule`/`oncall`/`incident`/`alert` output returns `person_id`s, a **different namespace from `member_id`**. `fduty person infos` (the sibling `person` group) batch-resolves any number of `person_id`s to `person_name` in one call (rows under `.items[]`). Matching `member list` rows on `member_id == ` is wrong, and paginating the full roster to find them silently misses people on later pages. - **`invite` members array is body-only — use `--data`.** Individual members cannot be passed as flat flags; the `members` array (with nested `role_ids`, `email`, `phone`, etc.) lives only in the JSON body. Up to 20 members per call. -- **`info-reset ` is POSITIONAL.** Pass the member ID as the first bare argument, not `--member-id`: `fduty member info-reset --member-name "New Name"`. The `--member-id` flag exists but the positional form is required per the `use` field. -- **`role-grant / role-revoke / role-update` — role IDs are POSITIONAL.** All three verbs take role IDs as positional args: `fduty member role-grant [...] --member-id `. The `--role-ids` flag also exists but the positional form is authoritative. +- **`info-reset ` can be passed positionally or via `--member-id`** — both work: `fduty member info-reset --member-name "New Name"` or `fduty member info-reset --member-id --member-name "New Name"`. If both are given, the flag wins. +- **`role-grant` / `role-revoke` / `role-update` — role IDs can be passed positionally or via `--role-ids`.** Positional is shorter: `fduty member role-grant [...] --member-id `, or pass `--role-ids ,` instead. If both are given, the flag wins. - **`role-update` is a full replacement.** List current roles with `member list` first; omitting a role removes it. -- **`delete` default is safe** (checks escalation rules / schedules). If it rejects with a reference error, review those references before using `--is-force`. +- **`delete` default is safe** (checks escalation rules / schedules / team membership). If it rejects with a reference error, review those references before using `--is-force`. An SSO-provisioned member rejects unconditionally — `--is-force` does not override that check. - **Empty `member list` result is authoritative** — if `--query` returns nothing the member does not exist; do not widen the query. ## Worked example diff --git a/skills/flashduty/reference/role.md b/skills/flashduty/reference/role.md index c0c6d45..83d10bd 100644 --- a/skills/flashduty/reference/role.md +++ b/skills/flashduty/reference/role.md @@ -121,8 +121,8 @@ Create or update a role ## Gotchas -- **`delete`, `disable`, `enable`, `info` take `` as a POSITIONAL arg**, not `--role-id`: `fduty role delete `. The flag form is silently ignored. -- **`member-grant` / `member-revoke`: `` is POSITIONAL (one or more space-separated); `--role-id` is a flag** — easy to flip. Example: `fduty role member-grant 123 456 --role-id 7`. +- **`delete`, `disable`, `enable`, `info` take `` positionally or via `--role-id`** — both work: `fduty role delete ` or `fduty role delete --role-id `. If both are given, the flag wins. +- **`member-grant` / `member-revoke`: `` is POSITIONAL (one or more space-separated); `--role-id` is a flag** — easy to flip. Example: `fduty role member-grant 123 456 --role-id 7`. Member IDs can also be passed via `--member-ids` instead of the positional (same fold-then-override rule). - **`upsert --permission-ids` replaces the full set** on update — omitting it clears all permissions. Always read `permission-list --role-ids --with-all` first to get the current set before modifying. - **`upsert` with no `--role-id` (or `--role-id 0`) creates; with `--role-id N` updates** — the verb doubles as create and update; check for an existing role with `list` to avoid accidental duplicates. - **`delete` is irreversible** — members who had this role lose its permissions immediately. Prefer `disable` to park a role without destroying it. diff --git a/skills/flashduty/reference/rum.md b/skills/flashduty/reference/rum.md index 80292ce..f066ea3 100644 --- a/skills/flashduty/reference/rum.md +++ b/skills/flashduty/reference/rum.md @@ -19,6 +19,13 @@ Prereq: `SKILL.md` read. Read verbs are free. `application-create` / `applicatio | list front-end error issues (with time window) | `issue-list` | | full detail of one error issue | `issue-info` | | mark issue resolved / label cause | `issue-update` | +| run raw SQL-style queries over RUM data | `data-query` | +| top values for one facet field, by occurrence count | `facet-count` | +| browse facet-enabled RUM fields | `facet-list` | +| browse all RUM field definitions | `field-list` | +| send a test alert to an app's webhook | `application-webhook-test` | +| session replay metadata (app/device/views for a session) | `session-replay-metadata` | +| page through a session's replay segments | `session-replay-segments` | ## Hot flow — triage front-end errors @@ -190,7 +197,7 @@ List session replay segments **`--type` (application-create / update) — closed enum:** `browser` · `ios` · `android` · `react-native` · `flutter` · `kotlin-multiplatform` · `roku` · `unity` -No `miniprogram` / `wechat` — unsupported, do not guess a value. +No `miniprogram` / `wechat` — you cannot create an application with these; do not guess a value. (Session/view `source` on `session-replay-metadata` does include `miniprogram` — that enum describes what recorded the data, not what you can create.) **Issue `--status` (issue-update / issue-list `--statuses`):** `for_review` → `reviewed` → `ignored` | `resolved` @@ -205,7 +212,7 @@ Regression: a `resolved` issue that recurs gets a `regression{}` object on its r - **`issue-list` time flags are MILLISECOND epoch, both required.** Use `--start-time` / `--end-time` (NOT `--since`/`--until`, NOT seconds). Max range 183 days. Example: `$(date +%s)000` converts a seconds epoch to ms. - **`application_id` ≠ `issue_id`.** `issue_id` comes from `issue-list` — never pass an `application_id` where `issue_id` is expected. -- **`application-create` positional:** `use` is `application-create ` — pass the team id as the first bare arg, NOT `--team-id`. Same pattern: `application-delete`, `application-info`, `application-infos`, `application-update`, `issue-info`, `issue-update` all take their primary id as positional. `application-list` and `issue-list` are all-flags. +- **`application-create ` can be passed positionally or via `--team-id`** — both work; positional is shorter. Same pattern on `application-delete`, `application-info`, `application-update`, `issue-info`, `issue-update`: each takes its primary id either as the bare positional shown in the fence heading, or via the matching `--application-id`/`--issue-id` flag. `application-infos` only has the plural `--application-ids` as its flag alternative (comma-separated, vs space-separated positionals). If both positional and flag are given, the flag wins. `application-list` and `issue-list` are all-flags. - **`alerting` and `tracing` are nested objects** — configure them via `--data '{"alerting":{...},"tracing":{...}}'`; there are no flat flags for their sub-fields. Scalar flags (`--application-name`, `--type`, …) override matching `--data` keys. - **Application records hold CONFIG only** — no traffic volume, error-rate, or session-count fields. For trend data, query `monit` RUM series. - **Empty `issue-list` is authoritative** — a filter returning no items means no matching issues, not a missing feature. Do not widen the query or guess. diff --git a/skills/flashduty/reference/schedule.md b/skills/flashduty/reference/schedule.md index c014e04..c315c7d 100644 --- a/skills/flashduty/reference/schedule.md +++ b/skills/flashduty/reference/schedule.md @@ -181,7 +181,7 @@ Update schedule ## Gotchas -- **`info`, `infos`, `delete` take positional `` — NOT `--schedule-id`.** Pass the ID bare: `fduty schedule info 123 --start now --end +7d`. Using `--schedule-id` on these verbs fails. +- **`info` takes `` positionally or via `--schedule-id`** — both work: `fduty schedule info 123 --start now --end +7d` or `fduty schedule info --schedule-id 123 --start now --end +7d`. If both are given, the flag wins. **`infos` and `delete` have no singular `--schedule-id` flag** — only the plural `--schedule-ids` (comma-separated), which folds the same way as the positional list; passing `--schedule-id` there is an unknown-flag error, not a rejected alternative. - **`create` / `update` / `preview` take all inputs as flags** (no positional). `update` requires `--schedule-id` as a flag to identify the target. - **`layers` is body-only.** There is no per-layer typed flag — you must pass the entire `layers` array via `--data`. Scalar top-level flags (`--schedule-name`, `--team-id`) override matching `--data` keys. - **`list` without `--start`/`--end` omits computed shifts** — only schedule metadata is returned. Pass both flags (≤45 day span) to get rotation slots in the list response. diff --git a/skills/flashduty/reference/status-page.md b/skills/flashduty/reference/status-page.md index b83f670..362fa38 100644 --- a/skills/flashduty/reference/status-page.md +++ b/skills/flashduty/reference/status-page.md @@ -247,7 +247,7 @@ Update status page ## Gotchas -- **`page_id` is POSITIONAL on some verbs, a `--page-id` flag on others — follow the fence heading.** Where the heading reads `### ` (change-create, change-active-list, change-list, subscriber-export/import/list, migrate-structure), pass the id as the first bare argument: `change-create …`. Passing `--page-id` there fails with `missing page_id`. Verbs that need *both* `page-id` and `change-id` (change-info, change-delete, change-timeline-*, change-update) take both as flags. The fence heading is authoritative. +- **`page_id` can be passed positionally or via `--page-id` — both work.** Verbs whose fence heading reads `### ` (change-create, change-active-list, change-list, subscriber-export/import/list) accept it either way — positional is shorter: `change-create …` or `change-create --page-id …`; if both are given, the flag wins. Verbs that need *both* `page-id` and `change-id` (change-info, change-delete, change-timeline-*, change-update) take both as flags only — neither has a positional form. `migrate-structure`'s positional/flag is a different field, `source-page-id` (the Atlassian source page ID), not `page-id`. - **`page_id` (int) ≠ `change_id` (int)** — page is the status page; change is one incident/maintenance within it. Don't cross them. - **`updates` is required on `change-create`** and goes via `--data` (it nests `component_changes[]`, which can't be flat flags). `--description` is also required by the server even though it's not flagged required. Typed scalar flags (`--title`, `--status`…) override matching `--data` keys. - **`--notify-subscribers` emails + pushes every subscriber immediately** — set it only once scope is confirmed. diff --git a/skills/flashduty/reference/team.md b/skills/flashduty/reference/team.md index e12dce8..d048311 100644 --- a/skills/flashduty/reference/team.md +++ b/skills/flashduty/reference/team.md @@ -117,7 +117,7 @@ Create or update a team ## Key concepts - **`status`** on `team list` rows: `enabled` | `disabled`. A disabled team still exists but is excluded from most operational contexts. -- **`infos [...]`** — takes team IDs as **positional args** (space-separated), not `--team-ids`. +- **`infos [...]`** — takes team IDs as space-separated positional args, or comma-separated via `--team-ids`; both work. - **`upsert` lookup key** — matched by `--team-id` (if non-zero) or by `--team-name` (name collision). Pass `--reset-if-name-exist` to overwrite membership on a name match; omit it to leave the existing members untouched. ## Gotchas @@ -125,7 +125,7 @@ Create or update a team - **`--person-ids` on `update` / `create` / `upsert` is a full replacement**, not an append. Read the current list with `get --id` first, or you will silently remove members. - **`get` vs `info`** — both fetch a single team; `get` accepts `--id`/`--name`/`--ref-id`; `get []` also allows the ID as a positional arg. `info` uses `--team-id`/`--team-name`/`--ref-id` flags only. Prefer `get` for interactive lookup. - **`delete` is irreversible** and requires confirmation unless `--force` is set. Always confirm the correct `--id` (not `--name`) in scripts to avoid name-collision accidents. -- **`infos` positional trap** — the `use` is `infos [...]`; IDs are space-separated positional args, not a flag. `fduty team infos 101 102 103`, not `--team-ids 101,102,103`. +- **`infos` accepts IDs either way** — space-separated positional args or comma-separated `--team-ids`: `fduty team infos 101 102 103` or `fduty team infos --team-ids 101,102,103`. If both are given, the flag wins. - **`upsert` requires `--team-name`** even when updating by `--team-id`; omitting it returns a validation error. ## Worked example From 45f3c28cfec492a6bf3244f35e5c839eeb7db747 Mon Sep 17 00:00:00 2001 From: ysyneu Date: Tue, 28 Jul 2026 06:49:40 -0700 Subject: [PATCH 2/3] docs(skills): clarify group.time_window unit in channel.md 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. --- skills/flashduty/reference/channel.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/flashduty/reference/channel.md b/skills/flashduty/reference/channel.md index d0f46e1..bc4768b 100644 --- a/skills/flashduty/reference/channel.md +++ b/skills/flashduty/reference/channel.md @@ -296,7 +296,7 @@ Update channel ## Key concepts - **`--auto-resolve-mode`** enum: `trigger` (timer resets on each new alert trigger) | `update` (timer resets on any alert update). -- **Alert grouping `group.method`**: `i` = intelligent (embedding similarity), `p` = pattern (label equality), `n` = none. Set via `--data '{"group":{"method":"p","equals":[["service","env"]],"time_window":300}}'` on `create`/`update`. +- **Alert grouping `group.method`**: `i` = intelligent (embedding similarity), `p` = pattern (label equality), `n` = none. **`group.time_window` is in minutes** (default cap 1440 = 24h; extended accounts may allow up to 43200 = 30 days). Set via `--data '{"group":{"method":"p","equals":[["service","env"]],"time_window":30}}'` on `create`/`update`. - **Rule status**: `enabled` | `disabled` — apply to escalation, inhibit, silence, and drop rules alike. - **Inhibit `--equals`**: label keys that must be **equal** between the source (high-priority) and target (suppressed) alert to form a pair (e.g. `--equals service,env`). - **Silence time windows**: `time_filter` (one-off, unix seconds, mutually exclusive) vs `time_filters` (recurring weekly HH:MM windows). Pass via `--data`. From 50020bfb0b7503108a5171e5860488b629024d40 Mon Sep 17 00:00:00 2001 From: ysyneu Date: Tue, 28 Jul 2026 06:53:14 -0700 Subject: [PATCH 3/3] fix(skilldoc): remove false positional-as-flag validation rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/skilldoc/validate.go | 37 +++++++++++------------------- internal/skilldoc/validate_test.go | 31 ++++++++++++++----------- 2 files changed, 31 insertions(+), 37 deletions(-) diff --git a/internal/skilldoc/validate.go b/internal/skilldoc/validate.go index 41536ec..cfe298c 100644 --- a/internal/skilldoc/validate.go +++ b/internal/skilldoc/validate.go @@ -16,7 +16,7 @@ type Doc struct { type Issue struct { Doc string Line int - Kind string // "unknown-command" | "unknown-flag" | "positional-as-flag" | "stale-fence" + Kind string // "unknown-command" | "unknown-flag" | "stale-fence" Detail string } @@ -104,19 +104,16 @@ func lineOf(body string, off int) int { return strings.Count(body[:off], "\n") + 1 } -// commandIndex maps a command path to its set of declared flag names and to the -// set of flags cligen folded into required positionals, and carries the sorted -// list of paths for longest-prefix resolution. +// commandIndex maps a command path to its set of declared flag names, and +// carries the sorted list of paths for longest-prefix resolution. type commandIndex struct { - flags map[string]map[string]bool - folded map[string]map[string]bool - paths []string + flags map[string]map[string]bool + paths []string } func indexDump(d Dump) commandIndex { idx := commandIndex{ - flags: make(map[string]map[string]bool), - folded: make(map[string]map[string]bool), + flags: make(map[string]map[string]bool), } for _, c := range d.Commands { set := make(map[string]bool, len(c.Flags)) @@ -124,7 +121,6 @@ func indexDump(d Dump) commandIndex { set[f.Name] = true } idx.flags[c.Path] = set - idx.folded[c.Path] = foldedFlagNames(positionalsOf(c.Use)) idx.paths = append(idx.paths, c.Path) } // Longest paths first so resolveCommand prefers the most specific match. @@ -156,26 +152,19 @@ func validateExample(idx commandIndex, docPath string, ex Example) []Issue { }} } - folded := idx.folded[path] var issues []Issue for _, tok := range ex.Tokens { name, isFlag := flagName(tok) if !isFlag || HasPlaceholder(name) { continue } - // cligen folded this field into a required positional: the flag is still - // registered (so it is in flagSet) but passing it as a flag fails the - // binary's Args check. Catch it before the flagSet pass would wave it - // through — this is the exact misuse only a live run surfaced before. - if folded[name] { - issues = append(issues, Issue{ - Doc: docPath, - Line: ex.Line, - Kind: "positional-as-flag", - Detail: "--" + name + " is folded into a required positional of `" + path + "` — pass it as a bare argument, not a flag", - }) - continue - } + // A field cligen folds into a required positional keeps a same-named flag + // registered as a genuine alternative source: every generated command + // with such a fold uses requireBodyFieldOrExactArg/requireBodyFieldOrArgs + // (internal/cli/args.go), which explicitly accepts the flag alone — cligen + // never emits the bare requireExactArg/requireArgs form that would make + // passing the flag fail. So a folded name is just a flag like any other + // here: fall through to the flagSet check below. if globalFlags[name] || flagSet[name] { continue } diff --git a/internal/skilldoc/validate_test.go b/internal/skilldoc/validate_test.go index 4e4a5ed..9d30174 100644 --- a/internal/skilldoc/validate_test.go +++ b/internal/skilldoc/validate_test.go @@ -45,13 +45,14 @@ func TestValidate_UnknownCommandAndFlag(t *testing.T) { } } -// A field cligen folded into a required positional is still a registered flag, -// but passing it as `--flag` fails the binary's Args check. The validator must -// catch this misuse (kind "positional-as-flag") — the exact error that only a -// live run surfaced before Use was threaded into the oracle. Passing the field -// positionally must stay clean, and the same flag name on a command where it is -// NOT folded (two required ids) must remain valid. -func TestValidate_FoldedPositionalAsFlag(t *testing.T) { +// A field cligen folds into a required positional keeps a same-named flag +// registered as a genuine alternative source: every generated command with +// such a fold uses requireBodyFieldOrExactArg/requireBodyFieldOrArgs +// (internal/cli/args.go), which explicitly accepts the flag alone. So passing +// that flag — with or without the positional also present — must validate +// clean; only a flag that is not registered on the command at all is an +// actual defect. +func TestValidate_FoldedFlagIsValidAlternative(t *testing.T) { d := Dump{Commands: []Command{ { // single required id → cligen folds page-id into a positional Path: "status-page change-active-list", @@ -67,23 +68,27 @@ func TestValidate_FoldedPositionalAsFlag(t *testing.T) { }, }} docs := []Doc{ - {Path: "bad", Body: "```bash\nfduty status-page change-active-list --page-id 5\n```\n"}, - {Path: "good", Body: "```bash\nfduty status-page change-active-list 5 --type incident\n```\n"}, + {Path: "flag-alone", Body: "```bash\nfduty status-page change-active-list --page-id 5 --type incident\n```\n"}, + {Path: "positional", Body: "```bash\nfduty status-page change-active-list 5 --type incident\n```\n"}, {Path: "twoid", Body: "```bash\nfduty status-page change-timeline-create --page-id 5 --change-id 9\n```\n"}, + {Path: "unknown-on-folder", Body: "```bash\nfduty status-page change-active-list --page-id 5 --bogus x\n```\n"}, } byDoc := map[string][]Issue{} for _, is := range Validate(d, docs) { byDoc[is.Doc] = append(byDoc[is.Doc], is) } - if n := len(byDoc["bad"]); n != 1 || byDoc["bad"][0].Kind != "positional-as-flag" { - t.Errorf("bad: want 1 positional-as-flag, got %+v", byDoc["bad"]) + if n := len(byDoc["flag-alone"]); n != 0 { + t.Errorf("flag-alone: folded flag used without the positional want 0 issues, got %+v", byDoc["flag-alone"]) } - if n := len(byDoc["good"]); n != 0 { - t.Errorf("good: positional usage want 0 issues, got %+v", byDoc["good"]) + if n := len(byDoc["positional"]); n != 0 { + t.Errorf("positional: positional usage want 0 issues, got %+v", byDoc["positional"]) } if n := len(byDoc["twoid"]); n != 0 { t.Errorf("twoid: --page-id on non-folding command want 0 issues, got %+v", byDoc["twoid"]) } + if n := len(byDoc["unknown-on-folder"]); n != 1 || byDoc["unknown-on-folder"][0].Kind != "unknown-flag" { + t.Errorf("unknown-on-folder: want 1 unknown-flag, got %+v", byDoc["unknown-on-folder"]) + } } func TestValidate_GlobalFlagsAllowed(t *testing.T) {