Skip to content

Commit 0ba6179

Browse files
committed
docs(skill),fix(skilldoc): review fixes — integration_id equivalence; guard splice index; single-file two-fence test
- filters.md: integration_id is not a legacy alias — the server accepts it and data_source_id interchangeably (the API's response docs in fact deprecate data_source_id), so present the pair as equivalent and keep the never-use list to the severity/status aliases, scoped to rule-evaluation time. - runGen: check the start-marker index before slicing the body with it. - New TestRunGen_TwoFencesInOneFile pins the sequential splice loop for a subset fence and catch-all living in one card.
1 parent 10f9df5 commit 0ba6179

3 files changed

Lines changed: 54 additions & 9 deletions

File tree

internal/cmd/skilldoc/main.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,11 @@ func runGen(d skilldoc.Dump, base, group string) error {
130130
for _, id := range perDoc[p] {
131131
start, end := skilldoc.FenceStart(id), skilldoc.FenceEnd(id)
132132
si := strings.Index(body, start)
133+
if si < 0 {
134+
return fmt.Errorf("%s: unterminated GENERATED:%s fence", p, id)
135+
}
133136
ei := strings.Index(body[si:], end)
134-
if si < 0 || ei < 0 {
137+
if ei < 0 {
135138
return fmt.Errorf("%s: unterminated GENERATED:%s fence", p, id)
136139
}
137140
body = body[:si] + rendered[id] + body[si+ei+len(end):]

internal/cmd/skilldoc/main_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,3 +399,43 @@ func TestRunGen_TopologyViolationFails(t *testing.T) {
399399
t.Fatalf("want topology error mentioning the missing catch-all, got %v", err)
400400
}
401401
}
402+
403+
// TestRunGen_TwoFencesInOneFile pins the sequential splice loop: a single card
404+
// carrying both a subset fence and the catch-all fence of the same group must
405+
// have both rewritten in one pass (offsets are re-resolved by marker text
406+
// after each splice, so the first replacement must not derail the second).
407+
func TestRunGen_TwoFencesInOneFile(t *testing.T) {
408+
dir := t.TempDir()
409+
mk := func(verb string) skilldoc.Command {
410+
return skilldoc.Command{Path: "svc " + verb, Group: "svc", Short: "S " + verb, Use: verb}
411+
}
412+
d := skilldoc.Dump{Commands: []skilldoc.Command{mk("list"), mk("rule-create"), mk("rule-delete")}}
413+
414+
card := filepath.Join(dir, "reference", "svc.md")
415+
writeFile(t, card, "# svc\n\nrules first\n\n"+
416+
skilldoc.FenceStart("svc[rule-]")+"\n"+skilldoc.FenceEnd("svc[rule-]")+"\n\nthen the rest\n\n"+
417+
skilldoc.FenceStart("svc")+"\n"+skilldoc.FenceEnd("svc")+"\n")
418+
419+
if err := runGen(d, dir, "svc"); err != nil {
420+
t.Fatalf("runGen: %v", err)
421+
}
422+
423+
body, err := os.ReadFile(card)
424+
if err != nil {
425+
t.Fatal(err)
426+
}
427+
got := string(body)
428+
ruleAt := strings.Index(got, "### rule-create")
429+
listAt := strings.Index(got, "### list")
430+
if ruleAt < 0 || listAt < 0 || ruleAt > listAt {
431+
t.Fatalf("both fences must be filled, subset before catch-all:\n%s", got)
432+
}
433+
if !strings.Contains(got, "rules first") || !strings.Contains(got, "then the rest") {
434+
t.Errorf("gen clobbered hand-written content between fences:\n%s", got)
435+
}
436+
437+
var out bytes.Buffer
438+
if n, _ := runCheck(d, dir, &out); n != 0 {
439+
t.Errorf("after gen, check should be clean; got %d:\n%s", n, out.String())
440+
}
441+
}

skills/flashduty/reference/filters.md

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ the rule fires when ANY group matches; each inner array holds
3636
## Keys — use the canonical names only
3737

3838
Common to every rule family: `severity`, `status`, `title`, `description`,
39-
`data_source_id`, and `labels.<name>` for any custom label. Per family:
39+
`data_source_id` / `integration_id` (interchangeable — the server accepts
40+
both and they always carry the same value), and `labels.<name>` for any
41+
custom label. Per family:
4042

4143
| rule family | matched against | extra keys | keys that DO NOT exist here |
4244
|---|---|---|---|
@@ -47,13 +49,13 @@ A key outside the family's vocabulary (e.g. `dedup_key` in a silence rule)
4749
produces a rule that never matches while looking configured — check the table
4850
before creating, and prefer server-rejected over silently-dead if unsure.
4951

50-
**Legacy aliases — never use them.** `event_severity`, `alert_severity`,
51-
`incident_severity`, `alert_status`, `incident_status`, and `integration_id`
52-
are stored aliases that always carry the SAME value as the canonical
53-
`severity` / `status` / `data_source_id` on their surface. Spelling them adds
54-
no precision and invites wrong reads — on escalation rules `alert_severity`
55-
is an alias of the *incident's* severity, not of any alert's. Always write
56-
the canonical key.
52+
**Legacy severity/status aliases — never use them.** `event_severity`,
53+
`alert_severity`, `incident_severity`, `alert_status`, and `incident_status`
54+
are stored aliases that, at rule-evaluation time, always carry the SAME
55+
value as the canonical `severity` / `status` on their surface. Spelling them
56+
adds no precision and invites wrong reads — on escalation rules
57+
`alert_severity` is an alias of the *incident's* severity, not of any
58+
alert's. Always write the canonical key.
5759

5860
## Building filters from incident labels (scoping a rule to one incident)
5961

0 commit comments

Comments
 (0)