Fix cligen schema walker to recurse through nested array items - #135
Merged
Conversation
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).
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
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.
What
The request/response field-tree walker in
internal/cmd/cligen/main.goonly unwrapped one array level before checking whether the element schema was an object. A nested array field — the OR-of-AND filter tree shape used byfilters/source_filters/target_filtersacross silence, escalate, inhibit, and unsubscribe rules (array<array<object>>) — never expanded itskey/oper/valsitem fields the way a plainarray<object>field (e.g.layers) already did. The type label had the same bug, collapsing to the uninformativearray<array>instead ofarray<array<object>>.Both
--helpand the skill-card generator (internal/skilldoc) read from this same walker's output, so the gap showed up in both places.Fix
tree()'s array case now unwrapsitemsthrough any depth of nested arrays (via a newarrayLeafSchemahelper, bounded defensively) before testing for an object leaf — generalized, not special-cased per command.schemaType()is now recursive for the same reason, so nested array type labels show their real depth (array<array<object>>,array<array<string>>, …).zz_generated_*.gocommand files and the skill-card fences (go run ./internal/cmd/cligen+go run ./internal/cmd/skilldoc gen). Affected commands:escalate-rule-create/update(filters),inhibit-rule-create/update(source_filters/target_filters),silence-rule-create/update(filters),unsubscribe-rule-create/update(filters), plus the alert-pipeline read paths that document the same filter shape (if, nestedsource_filters).array<object>behavior.Card content
skills/flashduty/reference/channel.md's hand-written "add a silence rule during maintenance" flow previously built a rule from only atime_filter, which the server rejects (no matching scope). Replaced it with a flow that pulls the target incident's labels and buildsfiltersfrom them, and added a general "Buildingfiltersfrom incident labels" rule: base severity condition, per-label conditions with exclusions for id-shaped/long/date-shaped/noise-key values, the validkeyvocabulary andoperenum, a note that an out-of-vocabularykeysilently never matches instead of erroring, and verifying via the matching*-rule-listcommand.Verification
make check(fmt, lint,go test -race ./..., build): all green, 0 lint issues, no test failures.go run ./internal/cmd/skilldoc check: cards OK../bin/flashduty channel silence-rule-create --helpconfirmsfiltersnow renderskey/oper/valschildren with descriptions and theIN/NOTINenum.Follow-up (not in this PR)
fc-safariholds its own copy of this fence atlogic/runtime/bootstrap/skills/flashduty/reference/channel.mdand needs a resync after this merges. No other repo under the org was found holding a copy.