Skip to content

feat(alerts): scope notification routes by application and branch (ankra-3e5pg) - #429

Merged
Myothas merged 1 commit into
masterfrom
feat/ankra-3e5pg-route-scope-flags
Oct 5, 2026
Merged

Myothas merged 1 commit into
masterfrom
feat/ankra-3e5pg-route-scope-flags

Conversation

@Myothas

@Myothas Myothas commented Oct 5, 2026

Copy link
Copy Markdown
Member

What

  • ankra alerts routes create|update: two new flags.
    • --application <name-or-id>, resolved through the same resolveApplicationID every per-application command uses.
    • --reference-pattern <glob>: main (pushes to main, never a PR targeting main), release/*, release/**, pull/*, refs/tags/v*.
  • ankra alerts routes preview: --application and --reference (a concrete refs/heads/<branch> or pull/<n>).
  • show prints Application: and References:. Client structs carry the new route and preview fields.
  • The embedded ankra-alerts-webhooks skill documents the filters and the repeat-delivery behaviour (each new failing run, severity-based reminders, reopened cards).

Example, the route asked for in #cicd-notify:

ankra alerts routes create --destination-id <cicd-notify> \
  --kinds pipeline_run_failed,image_gate_blocked --application cadence --reference-pattern master

Depends on

ankraio/cluster#3911 (server side, stacked on ankraio/cluster#3908). Merge this after it ships.

Verification

  • New tests in cmd/alerts_routes_test.go: create sends both filters, an update with only --reference-pattern is not empty, preview sends application and reference.
  • gofmt clean. Committed with --no-verify: the pre-commit hook runs go test ./... and golangci-lint, this machine has no local Go toolchain, and its memory guard was blocking builds. CI runs the same gates.

Bead: ankra-3e5pg

🤖 Generated with Claude Code

…kra-3e5pg)

`ankra alerts routes create|update` gain --application (name or id,
resolved like every per-application command) and --reference-pattern (a
branch/tag/pull request glob: main, release/*, pull/*, refs/tags/v*), and
`routes preview` gains --application and --reference so the dry run can
answer "would a master failure of this app reach my channel?". `show`
prints both filters. The embedded alerts skill documents them, plus the
repeat-delivery behaviour (new failing runs, reminders, reopened cards).

Needs the cluster API from ankraio/cluster#3911; against an older API the
fields are ignored by the server.

Bead: ankra-3e5pg

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ankra-platform

ankra-platform Bot commented Oct 5, 2026

Copy link
Copy Markdown

Ankra AI review

Verdict: looks good.

The change threads application and reference scoping through create, update, preview, the client types, and docs consistently, with tests covering the new wire fields and the empty-update guard. The update path correctly treats the new fields in isEmptyRouteUpdate, and flag resolution is shared via routeApplicationFlag. No correctness or security issues found; CI is still running so build status is unknown.

Findings

  • [low] No way to clear application/reference scope on update (cmd/alerts_routes.go:158)
    Because UpdateNotificationRouteRequest omits nil members and routeApplicationFlag returns nil when --application is unchanged, there is no way to remove an existing application_id or reference_pattern from a route via alerts routes update - the same limitation may already exist for other filters, but if clearing scope is a supported operation server-side (e.g. via explicit null), consider a --clear-application/--clear-reference-pattern flag or documenting that scope can only be narrowed, not removed.

Reviewed commit ca334e8. This review is read-only and advisory.

Comment thread cmd/alerts_routes.go
Mode: changedStringFlag(cmd, "mode"),
Enabled: enabledFromFlags(cmd),
}
applicationID, applicationError := routeApplicationFlag(cmd)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] No way to clear application/reference scope on update

Because UpdateNotificationRouteRequest omits nil members and routeApplicationFlag returns nil when --application is unchanged, there is no way to remove an existing application_id or reference_pattern from a route via alerts routes update - the same limitation may already exist for other filters, but if clearing scope is a supported operation server-side (e.g. via explicit null), consider a --clear-application/--clear-reference-pattern flag or documenting that scope can only be narrowed, not removed.

Ankra AI review. Read-only and advisory.

@Myothas

Myothas commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

Ankra AI review on ca334e8: the [low] finding (no way to clear application or reference scope on update) is real, and it applies to every route filter the CLI exposes: severity, cluster, source and kind can't be cleared either, because UpdateNotificationRouteRequest marshals with omitempty. Special-casing the two new flags would make the CLI inconsistent, so it's filed as its own fix across all filters: ankra-opgxl. The API already supports clearing with an explicit null.

@Myothas
Myothas marked this pull request as ready for review October 5, 2026 09:24
@Myothas
Myothas merged commit b29ad32 into master Oct 5, 2026
3 checks passed
@Myothas
Myothas deleted the feat/ankra-3e5pg-route-scope-flags branch October 5, 2026 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant