Skip to content

feat: offer shared alert starters for newly connected services - #77

Merged
dobleuber merged 2 commits into
nsolid-console-7from
feat/82-alert-starters
Oct 21, 2026
Merged

dobleuber merged 2 commits into
nsolid-console-7from
feat/82-alert-starters

Conversation

@dobleuber

@dobleuber dobleuber commented Oct 20, 2026 •

Copy link
Copy Markdown

A newly connected service can now start from the Console 7 alert catalog instead of copied rule definitions. ns-manage-alerts calls list_alert_starters, checks existing rule names and coverage, and offers useful missing starters before saving through set_alert_rule.

The workflow reviews service scope, existing notification channels, thresholds and diagnostics with the user. It keeps explicit approval before writes and handles an older server without the catalog tool.

Validation

  • node scripts/sync-plugin-assets.mjs --check
  • node scripts/materialize-github-marketplace.mjs --check
  • Companion API/MCP tests cover all 14 starters and listing-to-writer compatibility.
  • No end-to-end baseline setup with a real connected agent was run locally.

Companion changes

@ns-control-tower

ns-control-tower commented Oct 20, 2026 •

Copy link
Copy Markdown

Walkthrough

Docs-only: the ns-manage-alerts skill gains a Start a Newly Connected Service's Baseline section (skills/ns-manage-alerts/SKILL.md:9-36) plus the multi-metric documentation requested in review (:44-48, :62). The workflow routes a newly connected service through list_alert_starters: check existing rules for names and coverage first, offer only the useful missing starters with their why, adapt the chosen spec to one service (an app expression in scope; combined into where with parentheses for finding/behavior rules), preview with step 3, and save through set_alert_rule only after explicit approval. Fallbacks cover a missing set_alert_rule (review/draft only) and an older server without the catalog tool. Re-review delta at 02c7fc6 ("docs: explain multi-metric alert starters and previews"): step 1 now explains metric.values — letters A–E, up to five metrics, agg/field/windowMinutes/per inside each value, every letter required, a missing value makes that letter's comparisons false, letters compare with numbers only — and step 3 explains previewing a values rule per letter.

overview
starter-baseline-view

Assessment

  • ✔ Values-documentation gap raised in review is implemented at this head: :44-48 now explain letters and metric.values, so :31 ("Keep each metric's letters, windows, conditions and hold times") and the two starters that use values now have the explanation they need in this copy. Wording verified against the companion product docs (nodesource/nsdocs#129, docs/console/alerts.md § "Several metrics at once"): up to five metrics A–E, each read its own way; missing value ⇒ that letter's comparisons false; every letter used; no letter-to-letter arithmetic (a ratio uses per).
  • Internal consistency re-verified: "step 3 below" (:28) resolves to ### 3. Check What It Would Have Caught (:60); strict-argument semantics (:45) match §5 (:74-77); the permission fallback matches the intro (:7).
  • CI: test workflow run #157 on 02c7fc6 — ubuntu, macOS and windows all green (lint, build, tests, marketplace install).
  • Not actionable in this repository: the review note that the VS Code extension's separate copy needs the same material — skills/ is the only canonical copy here and the root manifests are generated from bundle.json, so the carry-over stays manual, as the review notes.

Verdict: APPROVE — docs-only change; the review feedback is implemented and verified against the companion docs; CI green; no correctness, security or performance concerns.

@dobleuber
dobleuber marked this pull request as ready for review October 20, 2026 18:45

@brycebaril brycebaril left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The workflow is right, and the names match the server:

  • list_alert_starters takes no arguments, and its spec goes to set_alert_rule with tenant. The skill says not to pass the id, title or why, which set_alert_rule would refuse as unknown keys.
  • It checks existing names and coverage first, and saves only once the user says so. It leaves enabled alone, so an existing rule someone switched off stays off.
  • CI is green.

Before merge: the other copy of ns-manage-alerts, the one the VS Code extension installs its Console 7 skills from, needs this section too. The two copies have diverged:

  • That copy documents rules on several metrics (metric.values, letters A to E). This one doesn't.
  • Two of the 14 starters, clock-skew and busy-and-saturated, use values.
  • A change made only here misses the extension now, and has to be carried over by hand when the skills are published here at launch.

- Preview what it would have caught using step 3 below, and show the final
spec. Once the user authorizes saving, pass that spec with the selected
`tenant` to `set_alert_rule`; do not pass the starter's id, title or why.
Keep each metric's letters, windows, conditions and hold times unless the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In this copy of the skill, nothing explains letters or metric.values. The other copy does, in step 1's "Several metrics". So this line, and the two starters that use letters, read better there.

@dobleuber
dobleuber merged commit 29aad46 into nsolid-console-7 Oct 21, 2026
5 checks passed
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.

3 participants