Skip to content

Commit 641417b

Browse files
authored
Merge pull request #123 from flashcatcloud/fix/skill-cards-flag-forms
docs(skills): correct flag/positional claims in reference cards
2 parents c4e34ea + 50020bf commit 641417b

11 files changed

Lines changed: 54 additions & 54 deletions

File tree

internal/skilldoc/validate.go

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ type Doc struct {
1616
type Issue struct {
1717
Doc string
1818
Line int
19-
Kind string // "unknown-command" | "unknown-flag" | "positional-as-flag" | "stale-fence"
19+
Kind string // "unknown-command" | "unknown-flag" | "stale-fence"
2020
Detail string
2121
}
2222

@@ -104,27 +104,23 @@ func lineOf(body string, off int) int {
104104
return strings.Count(body[:off], "\n") + 1
105105
}
106106

107-
// commandIndex maps a command path to its set of declared flag names and to the
108-
// set of flags cligen folded into required positionals, and carries the sorted
109-
// list of paths for longest-prefix resolution.
107+
// commandIndex maps a command path to its set of declared flag names, and
108+
// carries the sorted list of paths for longest-prefix resolution.
110109
type commandIndex struct {
111-
flags map[string]map[string]bool
112-
folded map[string]map[string]bool
113-
paths []string
110+
flags map[string]map[string]bool
111+
paths []string
114112
}
115113

116114
func indexDump(d Dump) commandIndex {
117115
idx := commandIndex{
118-
flags: make(map[string]map[string]bool),
119-
folded: make(map[string]map[string]bool),
116+
flags: make(map[string]map[string]bool),
120117
}
121118
for _, c := range d.Commands {
122119
set := make(map[string]bool, len(c.Flags))
123120
for _, f := range c.Flags {
124121
set[f.Name] = true
125122
}
126123
idx.flags[c.Path] = set
127-
idx.folded[c.Path] = foldedFlagNames(positionalsOf(c.Use))
128124
idx.paths = append(idx.paths, c.Path)
129125
}
130126
// Longest paths first so resolveCommand prefers the most specific match.
@@ -156,26 +152,19 @@ func validateExample(idx commandIndex, docPath string, ex Example) []Issue {
156152
}}
157153
}
158154

159-
folded := idx.folded[path]
160155
var issues []Issue
161156
for _, tok := range ex.Tokens {
162157
name, isFlag := flagName(tok)
163158
if !isFlag || HasPlaceholder(name) {
164159
continue
165160
}
166-
// cligen folded this field into a required positional: the flag is still
167-
// registered (so it is in flagSet) but passing it as a flag fails the
168-
// binary's Args check. Catch it before the flagSet pass would wave it
169-
// through — this is the exact misuse only a live run surfaced before.
170-
if folded[name] {
171-
issues = append(issues, Issue{
172-
Doc: docPath,
173-
Line: ex.Line,
174-
Kind: "positional-as-flag",
175-
Detail: "--" + name + " is folded into a required positional of `" + path + "` — pass it as a bare argument, not a flag",
176-
})
177-
continue
178-
}
161+
// A field cligen folds into a required positional keeps a same-named flag
162+
// registered as a genuine alternative source: every generated command
163+
// with such a fold uses requireBodyFieldOrExactArg/requireBodyFieldOrArgs
164+
// (internal/cli/args.go), which explicitly accepts the flag alone — cligen
165+
// never emits the bare requireExactArg/requireArgs form that would make
166+
// passing the flag fail. So a folded name is just a flag like any other
167+
// here: fall through to the flagSet check below.
179168
if globalFlags[name] || flagSet[name] {
180169
continue
181170
}

internal/skilldoc/validate_test.go

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,14 @@ func TestValidate_UnknownCommandAndFlag(t *testing.T) {
4545
}
4646
}
4747

48-
// A field cligen folded into a required positional is still a registered flag,
49-
// but passing it as `--flag` fails the binary's Args check. The validator must
50-
// catch this misuse (kind "positional-as-flag") — the exact error that only a
51-
// live run surfaced before Use was threaded into the oracle. Passing the field
52-
// positionally must stay clean, and the same flag name on a command where it is
53-
// NOT folded (two required ids) must remain valid.
54-
func TestValidate_FoldedPositionalAsFlag(t *testing.T) {
48+
// A field cligen folds into a required positional keeps a same-named flag
49+
// registered as a genuine alternative source: every generated command with
50+
// such a fold uses requireBodyFieldOrExactArg/requireBodyFieldOrArgs
51+
// (internal/cli/args.go), which explicitly accepts the flag alone. So passing
52+
// that flag — with or without the positional also present — must validate
53+
// clean; only a flag that is not registered on the command at all is an
54+
// actual defect.
55+
func TestValidate_FoldedFlagIsValidAlternative(t *testing.T) {
5556
d := Dump{Commands: []Command{
5657
{ // single required id → cligen folds page-id into a positional
5758
Path: "status-page change-active-list",
@@ -67,23 +68,27 @@ func TestValidate_FoldedPositionalAsFlag(t *testing.T) {
6768
},
6869
}}
6970
docs := []Doc{
70-
{Path: "bad", Body: "```bash\nfduty status-page change-active-list --page-id 5\n```\n"},
71-
{Path: "good", Body: "```bash\nfduty status-page change-active-list 5 --type incident\n```\n"},
71+
{Path: "flag-alone", Body: "```bash\nfduty status-page change-active-list --page-id 5 --type incident\n```\n"},
72+
{Path: "positional", Body: "```bash\nfduty status-page change-active-list 5 --type incident\n```\n"},
7273
{Path: "twoid", Body: "```bash\nfduty status-page change-timeline-create --page-id 5 --change-id 9\n```\n"},
74+
{Path: "unknown-on-folder", Body: "```bash\nfduty status-page change-active-list --page-id 5 --bogus x\n```\n"},
7375
}
7476
byDoc := map[string][]Issue{}
7577
for _, is := range Validate(d, docs) {
7678
byDoc[is.Doc] = append(byDoc[is.Doc], is)
7779
}
78-
if n := len(byDoc["bad"]); n != 1 || byDoc["bad"][0].Kind != "positional-as-flag" {
79-
t.Errorf("bad: want 1 positional-as-flag, got %+v", byDoc["bad"])
80+
if n := len(byDoc["flag-alone"]); n != 0 {
81+
t.Errorf("flag-alone: folded flag used without the positional want 0 issues, got %+v", byDoc["flag-alone"])
8082
}
81-
if n := len(byDoc["good"]); n != 0 {
82-
t.Errorf("good: positional usage want 0 issues, got %+v", byDoc["good"])
83+
if n := len(byDoc["positional"]); n != 0 {
84+
t.Errorf("positional: positional usage want 0 issues, got %+v", byDoc["positional"])
8385
}
8486
if n := len(byDoc["twoid"]); n != 0 {
8587
t.Errorf("twoid: --page-id on non-folding command want 0 issues, got %+v", byDoc["twoid"])
8688
}
89+
if n := len(byDoc["unknown-on-folder"]); n != 1 || byDoc["unknown-on-folder"][0].Kind != "unknown-flag" {
90+
t.Errorf("unknown-on-folder: want 1 unknown-flag, got %+v", byDoc["unknown-on-folder"])
91+
}
8792
}
8893

8994
func TestValidate_GlobalFlagsAllowed(t *testing.T) {

skills/flashduty/reference/calendar.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ Update calendar
121121

122122
## Key concepts
123123

124-
- **`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.
124+
- **`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.
125125
- **`end-at` is exclusive:** a single-day event on 2026-01-17 needs `--start-at 2026-01-17 --end-at 2026-01-18`.
126126
- **`workdays` integers:** 0 = Sunday, 1 = Monday … 6 = Saturday. Standard Mon–Fri = `1,2,3,4,5`.
127127
- **Calendar kinds:** `personal` (editable, default filter) vs `region.official.holiday` (read-only, browsable). The returned `kind` field can also be `religion.holiday`.

skills/flashduty/reference/channel.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -296,18 +296,17 @@ Update channel
296296
## Key concepts
297297

298298
- **`--auto-resolve-mode`** enum: `trigger` (timer resets on each new alert trigger) | `update` (timer resets on any alert update).
299-
- **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`.
299+
- **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`.
300300
- **Rule status**: `enabled` | `disabled` — apply to escalation, inhibit, silence, and drop rules alike.
301301
- **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`).
302302
- **Silence time windows**: `time_filter` (one-off, unix seconds, mutually exclusive) vs `time_filters` (recurring weekly HH:MM windows). Pass via `--data`.
303303
- **Escalation `layers`** (required via `--data` on create/update): each layer needs `target` (with `person_ids`/`team_ids`/`schedule_to_role_ids`/`emails` + `by` OR `webhooks`) and optionally `notify_step`, `max_times`, `escalate_window`, `force_escalate`.
304304

305305
## Gotchas
306306

307-
- **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 <channel-id>` = positional; heading without `<…>` = flag.
307+
- **`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 <channel-id>` 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.
308308
- **`escalate-rule-create` needs `layers` via `--data`** — it is required and cannot be expressed as a flat flag. Omitting it returns a validation error.
309309
- **`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.
310-
- **`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.
311310
- **`delete` on a channel is irreversible** — all rules within it are also removed. Confirm the `channel-id` against `list` before proceeding.
312311
- **Empty rule list is authoritative** — if `escalate-rule-list` / `silence-rule-list` / etc. returns no rows, no rules exist; do not widen the query.
313312

skills/flashduty/reference/field.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ Update field
9191

9292
## Gotchas
9393

94-
- **`delete`, `info`, `update` take `<field-id>` as a POSITIONAL first argument**, not `--field-id`. Example: `fduty field delete <field-id>`, not `--field-id <field-id>`.
94+
- **`delete`, `info`, `update` take `<field-id>` positionally or via `--field-id`** — both work, e.g. `fduty field delete <field-id>` or `fduty field delete --field-id <field-id>`. If both are given, the flag wins.
9595
- **`--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.
9696
- **`--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.
9797
- **`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.

skills/flashduty/reference/member.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# fduty member — command card
22

3-
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.
3+
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.
44

55
## Route here when
66

@@ -124,10 +124,10 @@ Update member roles
124124

125125
- **Resolving a `person_id` → name: use `fduty person infos <person_id> …`, 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 == <person_id>` is wrong, and paginating the full roster to find them silently misses people on later pages.
126126
- **`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.
127-
- **`info-reset <member-id>` is POSITIONAL.** Pass the member ID as the first bare argument, not `--member-id`: `fduty member info-reset <member_id> --member-name "New Name"`. The `--member-id` flag exists but the positional form is required per the `use` field.
128-
- **`role-grant / role-revoke / role-update` — role IDs are POSITIONAL.** All three verbs take role IDs as positional args: `fduty member role-grant <role_id> [<role_id2>...] --member-id <member_id>`. The `--role-ids` flag also exists but the positional form is authoritative.
127+
- **`info-reset <member-id>` can be passed positionally or via `--member-id`** — both work: `fduty member info-reset <member_id> --member-name "New Name"` or `fduty member info-reset --member-id <member_id> --member-name "New Name"`. If both are given, the flag wins.
128+
- **`role-grant` / `role-revoke` / `role-update` — role IDs can be passed positionally or via `--role-ids`.** Positional is shorter: `fduty member role-grant <role_id> [<role_id2>...] --member-id <member_id>`, or pass `--role-ids <role_id>,<role_id2>` instead. If both are given, the flag wins.
129129
- **`role-update` is a full replacement.** List current roles with `member list` first; omitting a role removes it.
130-
- **`delete` default is safe** (checks escalation rules / schedules). If it rejects with a reference error, review those references before using `--is-force`.
130+
- **`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.
131131
- **Empty `member list` result is authoritative** — if `--query` returns nothing the member does not exist; do not widen the query.
132132

133133
## Worked example

skills/flashduty/reference/role.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,8 @@ Create or update a role
121121

122122
## Gotchas
123123

124-
- **`delete`, `disable`, `enable`, `info` take `<role-id>` as a POSITIONAL arg**, not `--role-id`: `fduty role delete <role-id>`. The flag form is silently ignored.
125-
- **`member-grant` / `member-revoke`: `<member-id>` 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`.
124+
- **`delete`, `disable`, `enable`, `info` take `<role-id>` positionally or via `--role-id`** — both work: `fduty role delete <role-id>` or `fduty role delete --role-id <role-id>`. If both are given, the flag wins.
125+
- **`member-grant` / `member-revoke`: `<member-id>` 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).
126126
- **`upsert --permission-ids` replaces the full set** on update — omitting it clears all permissions. Always read `permission-list --role-ids <id> --with-all` first to get the current set before modifying.
127127
- **`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.
128128
- **`delete` is irreversible** — members who had this role lose its permissions immediately. Prefer `disable` to park a role without destroying it.

skills/flashduty/reference/rum.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@ Prereq: `SKILL.md` read. Read verbs are free. `application-create` / `applicatio
1919
| list front-end error issues (with time window) | `issue-list` |
2020
| full detail of one error issue | `issue-info` |
2121
| mark issue resolved / label cause | `issue-update` |
22+
| run raw SQL-style queries over RUM data | `data-query` |
23+
| top values for one facet field, by occurrence count | `facet-count` |
24+
| browse facet-enabled RUM fields | `facet-list` |
25+
| browse all RUM field definitions | `field-list` |
26+
| send a test alert to an app's webhook | `application-webhook-test` |
27+
| session replay metadata (app/device/views for a session) | `session-replay-metadata` |
28+
| page through a session's replay segments | `session-replay-segments` |
2229

2330
## Hot flow — triage front-end errors
2431

@@ -190,7 +197,7 @@ List session replay segments
190197

191198
**`--type` (application-create / update) — closed enum:**
192199
`browser` · `ios` · `android` · `react-native` · `flutter` · `kotlin-multiplatform` · `roku` · `unity`
193-
No `miniprogram` / `wechat`unsupported, do not guess a value.
200+
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.)
194201

195202
**Issue `--status` (issue-update / issue-list `--statuses`):**
196203
`for_review``reviewed``ignored` | `resolved`
@@ -205,7 +212,7 @@ Regression: a `resolved` issue that recurs gets a `regression{}` object on its r
205212

206213
- **`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.
207214
- **`application_id``issue_id`.** `issue_id` comes from `issue-list` — never pass an `application_id` where `issue_id` is expected.
208-
- **`application-create` positional:** `use` is `application-create <team-id>` — 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.
215+
- **`application-create <team-id>` 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.
209216
- **`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.
210217
- **Application records hold CONFIG only** — no traffic volume, error-rate, or session-count fields. For trend data, query `monit` RUM series.
211218
- **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.

0 commit comments

Comments
 (0)