Merge feat/ai-sre into main — unify the two lineages - #143
Merged
Conversation
…r public-page structure mutations
…tion-confirm docs(skills): status-page card — confirm gate + rollback reporting for public-page structure mutations
…cipline in SKILL.md
The request/response field-tree walker only unwrapped one array level before checking whether the element schema was an object, so a nested array field like an OR-of-AND filter tree (array<array<object>>) never expanded its item fields the way a plain array<object> field did — and its rendered type label collapsed to the uninformative "array<array>". Both --help and the skill-card generator read from this same walker, so every filters/source_filters/target_filters field across silence, escalate, inhibit, and unsubscribe rules (and any other nested-array field) silently dropped its key/oper/vals schema. Generalize the array case to unwrap items through any depth of nested arrays (bounded, not per-command special-cased) before testing for an object leaf, and apply the same fix to the type-label formatter. Add unit tests covering the nested-object, nested-scalar, and single-level regression cases. Also rewrite the channel card's maintenance-silence example, which built a rule with only a time window — servers reject that as having no matching scope — and add a general rule for constructing `filters` from an incident's own labels (severity + distinguishing labels, with the value/key exclusions and key vocabulary needed to avoid silently building a rule that never matches).
…r-schema Fix cligen schema walker to recurse through nested array items
A fence id can now be group[verb-prefix,...]: a subset fence claiming every verb starting with one of the prefixes, while the bare-group fence renders the unclaimed remainder as the catch-all. gen/check discover fences by scanning the cards for markers instead of deriving one filename per group, so a large command group's reference card can be split along task lines. Topology violations (a verb claimed twice, a prefix claiming nothing, unclaimed verbs with no catch-all, a duplicated fence) fail gen and are reported by check as fence-topology issues.
channel.md (367 lines) carried four rule families beside channel CRUD, and incident.md (508) carried the whole post-mortem surface — every task paid for the whole domain, and the guidance competing for attention is exactly how a hot-flow example gets misread. Split along task lines using subset fences: - channel.md → channel CRUD only; escalation.md (escalate-rule-*); noise.md (silence-rule-*, inhibit-rule-*, unsubscribe-rule-*) - incident.md → triage/investigation; postmortem.md (post-mortem-*) - filters.md: new shared reference for filters/source_filters/ target_filters — OR-of-AND shape, IN/NOTIN semantics incl. the missing-key trap, /regex/ values, construction-from-labels rules, and the per-family key vocabulary. Consumer cards and alert.md's pipeline section point here instead of restating it. - Canonical keys only: guidance now teaches severity/status/ data_source_id; the per-surface aliases (event_severity, alert_severity, incident_severity, alert_status, incident_status, integration_id) always carry the same stored value and are called out as never-use. - SKILL.md router: channel row narrowed; new escalation/noise/postmortem rows; shared-reference note for filters.md.
… 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.
…corpus load, FindFence - Prefix claims are now hyphen-boundary-bounded (verb == p or p + "-"): an unbounded match could misroute a near-miss verb (rule2-list under rule-) with a clean single-owner partition no topology check would flag. Fence ids normalized to the boundary-safe form (incident[post-mortem]); a trailing-hyphen prefix now claims nothing and dies loudly as a dead-prefix violation. - runGenAll loads the card corpus once and threads it through every group via genGroup (was one full reload per group); genGroup updates the in-memory docs after each write so later groups see current content. runGen's docOrder/byPath bookkeeping dropped — it iterates the docs slice directly. - New skilldoc.FindFence(body, id) centralizes start/end-marker location for both gen and check (the two ad-hoc copies had already drifted in error wording); hasCatchAll bool and a no-op map insert removed; CheckFences reuses groups(d)'s existing order instead of re-sorting.
Split oversized skill cards along task lines; subset fences; shared filters reference
…lag help The alert list and alert-event list endpoints reject limit > 100 with InvalidParameter. incident list already documents the cap in its --limit flag help; mirror the same wording so the Flags section alone is enough to get the first call right (the long description mentions the cap, but --help readers who grep Flags never see it).
In json/toon mode, incident list, incident similar and alert-event list silently reduce rows to a compact default field set. A reader piping stdout to jq sees an unselected key (labels, description, ...) as null on every row and can reasonably conclude the server never returns it, when it is one --fields away — the incident list card even documents the projection, but nothing at invocation time points back to it. Print a one-line note on stderr whenever the default projection applies, naming the projected fields and the --fields escape hatch. stdout stays byte-identical, so existing jq/toon pipelines are unaffected. alert list is untouched: it has no default projection (bare --json dumps the full record). Also extend the incident card's projection note with the two facts the projection hides: any list-response field (labels included) is selectable via --fields, and wide fields over many rows will hit the 16 KiB structured-output bound — page with a smaller --limit or use insight aggregates for distributions. Tests: new execCommandSplit captures stdout/stderr separately; the projection tests now assert stdout stays pure JSON while the note lands on stderr.
The alerts field appears in list/detail response shapes but no read endpoint ever fills it; an incident's alerts come only from the dedicated incident alerts command. Saying every response field is selectable via --fields would invite --fields alerts and an always-empty column.
Same shared page-query validator as alert/alert-event/incident list; same wording.
…card parity The stderr note embeds the default field names, so the merged-capture toon subtest's positive assertions were satisfied by the note alone. Switch it to the split capture and check the note on stderr, mirroring the json subtest. Also bring alert.md's alert-event list section to parity with the incident card: name the default compact projection and the --fields escape hatch.
fix(cli): state the 100-row server cap in alert/alert-event --limit flag help
feat(cli): announce the default compact projection on stderr
…utate docs(skills): back up critical config before mutating — undo-path discipline
boundProjectedOutput applied one budget strategy to both shapes it receives: list projections (many small rows) and the single-object projection behind `incident detail --fields`. On overflow it shortened every string in place, then halved the per-field limit and retried. On a single object that silently corrupts data. The limit is a global budget divided by the number of string slots, so short fields are punished for long ones sharing the object, and every round re-reads the already-shortened value, compounding it. Once the limit drops below 4, truncateUTF8Bytes stops appending the "..." marker, so a 24-character id can arrive as "6" and "Warning" as "W" -- indistinguishable from a genuinely short value. The loop's fieldLimit == 0 error exit is unreachable: by then every string is empty, so the payload always fits and the command exits 0 with hollowed-out fields. Split the two shapes. Lists keep the existing shorten-and-mark behavior byte for byte. A detail projection is now never modified: if it does not fit, the command fails and names the largest fields with their sizes, so the caller can drop some --fields, or omit --fields for the full, unbounded detail -- which returns more data, not less, so the error is always actionable. Dropping keys instead was rejected: an absent key is indistinguishable from a null one to a jq consumer, which reintroduces the same class of bug this removes. Ties in the largest-field ranking break on name, so the same oversized request never names different fields between runs.
…r-truncates-values fix(cli): never truncate incident detail --fields values
The repository had been carrying two long-lived branches that were never merged: releases were cut from main, while a parallel line of skill-card work accumulated on feat/ai-sre and shipped to nobody. Commits were hand-copied between them, so each branch ended up holding content the other lacked, and the same card existed in two structurally different shapes. This merge makes main the single lineage; feat/ai-sre is retired. Five conflicts, resolved by keeping the correction rather than the newer text, because two of them would have reintroduced defects main had already fixed: - internal/skilldoc/generate.go: keep both orthogonal features — subset fences (renderFence takes a fence id, not a group) and same-card response-shape dedup. Rescoping the dedup map from the group to the rendered fence is required by subset fences: a group split across cards must never emit a back-reference to a command the reader cannot see on the card in front of them. - internal/skilldoc/validate.go: keep main's deletion of the "positional-as-flag" issue kind. That rule encoded the false claim that flags do not work on positional commands; it was removed because the failure mode it guarded is structurally impossible, and no code emits the kind any more. Add "fence-topology", which is real. - reference/channel.md: adopt the post-split structure (escalation and noise rules now have their own cards, which carry every moved item) but restore the group.time_window unit fix — the split copy had lost "in minutes" along with its caps and still used a seconds-era example. Also keep the accurate statement that channel-id works positionally AND as --channel-id, verified against the built binary. - reference/incident.md: adopt the post-mortem card routing, keep the detail/get exception that accepts a 6-char num. - reference/alert.md: keep the rule cap and per-kind settings shapes, and point at the new shared filters card for condition-tree syntax. Generated fences were regenerated from the merged command tree rather than hand-merged, since fence content is derived and only the prose outside them is hand-maintained. make check and make check-cards both pass.
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.
The repository has been carrying two long-lived branches that were never merged. Releases are cut from
main; a parallel line of skill-card work accumulated onfeat/ai-sreand therefore shipped to nobody. Commits were hand-copied between the two, so each branch ended up holding content the other lacked, and the same card existed in two structurally different shapes (incident.mdwas 605 lines here, 420 there).This merge makes
mainthe single lineage.feat/ai-sreis retired afterwards.What comes in
Four new cards that existed only on the other branch —
escalation.md,noise.md,postmortem.md, and the sharedfilters.md— plus the card split that moves escalation/silence/inhibit/unsubscribe rules out ofchannel.mdand post-mortem verbs out ofincident.md, the subset-fence support inskilldocthat makes one command group span several cards, and the cligen nested-array schema-walker fix.Conflicts, and why they resolved this way
Five conflicts. The rule applied was keep the correction, not the newer text — two of them would otherwise have reintroduced defects
mainhad already fixed.internal/skilldoc/generate.gorenderFencetakes a fence id, not a group) and same-card response-shape dedup.internal/skilldoc/validate.gomain's deletion of thepositional-as-flagissue kind; add the realfence-topologykind.reference/channel.mdgroup.time_windowunit fix.reference/incident.mddetail/getexception that accepts a 6-char num.reference/alert.mdsettingsshapes; point at the new shared filters card for condition-tree syntax.Two of those deserve spelling out:
positional-as-flagstays deleted. That issue kind encoded the false claim that flags don't work on positional commands. It was removed because the failure mode it guarded is structurally impossible, and no code emits the kind any more — only the other branch's stale doc comment still named it. Taking the newer line verbatim would have documented a dead false rule back into existence.group.time_windowkeeps its unit. The split copy had lost "in minutes" along with its caps (1440 / 43200) and still carried a seconds-era300example. The corrected text and its30example are restored.Rescoping the dedup map from the group to the rendered fence is required by subset fences: a group split across cards must never emit a back-reference to a command the reader cannot see on the card in front of them.
Generated fences were regenerated from the merged command tree rather than hand-merged, since fence content is derived and only the prose outside them is hand-maintained.
Verification
make check— gofmt/gci, golangci-lint 0 issues,go test -race ./...green across all 9 packages, build OKmake check-cards—skilldoc: cards OK(this cross-checks every card against the real command tree, so it is the gate that would catch a bad fence merge)Content-loss review, checked exhaustively rather than sampled:
channel.mdare present inescalation.md/noise.md, fence content byte-identical; the hand-written prose that moved with them (rule status,--equalssemantics,time_filtervstime_filters,layersshape, the worked example) reappears in equivalent or expanded formpost-mortem-*headings moved out ofincident.mdare present inpostmortem.mdsame shape as X aboveback-reference across all 25 cards was parsed and confirmed to resolve within its own fence — zero cross-card references, including the three-waychannelsplitTwo prose claims were checked against the source rather than trusted from the diff:
resolveIncidentArgis reachable from exactlygetanddetail, and the channel verbs userequireBodyFieldOrExactArg("channel_id", "channel-id"), which accepts either form.