Skip to content

feat(ams): page PagerDuty on miner kill-switch trips - #7699

Closed
jsdevninja wants to merge 6 commits into
JSONbored:mainfrom
jsdevninja:feat/ams-kill-switch-pagerduty-v2
Closed

feat(ams): page PagerDuty on miner kill-switch trips#7699
jsdevninja wants to merge 6 commits into
JSONbored:mainfrom
jsdevninja:feat/ams-kill-switch-pagerduty-v2

Conversation

@jsdevninja

Copy link
Copy Markdown
Contributor

Summary

  • Add pure buildMinerKillSwitchPagerDutyAlert in the engine kill-switch detector (null unless the transition is a real trip into an active scope).
  • On ledger trip, miner recordMinerKillSwitchTransition fires PagerDuty Events API v2 using the same LOOPOVER_ENABLE_PAGERDUTY / PAGERDUTY_ROUTING_KEY / enqueue / dedup_key contract as ORB's notify-pagerduty module. Resumes stay silent; paging is fire-and-forget so it cannot block the ledger write.
  • No hosted wrapper: AMS kill-switch trips only exist in the miner process (no Worker Env/D1 call site). Prior PR feat(ams): page PagerDuty on miner kill-switch trips #7685 was closed for adding an unused hosted export — this revision drops that dead path and documents the miner-side contract in the runbook.

Closes #7666

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (Closes #7666).

Validation

  • git diff --check
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • Focused validation: notify-pagerduty, miner-governor-kill-switch, and kill-switch-incident-runbook (all green). Full test:ci left to CI.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

UI Evidence

N/A — docs runbook text + miner alerting only; no visible UI chrome.

Notes

jsdevninja and others added 6 commits July 21, 2026 03:04
Wire kill-switch engage through the existing Events API v2 path so trips are not ledger-only; resumes stay silent.

Co-authored-by: Cursor <cursoragent@cursor.com>
Exercise every notify/env/error branch under vitest so patch coverage clears the 99% gate.

Co-authored-by: Cursor <cursoragent@cursor.com>
Collapse sync/async failure handling into shared helpers and Promise.resolve so Codecov patch does not miss branches under sharded merges.

Co-authored-by: Cursor <cursoragent@cursor.com>
AMS kill-switch trips only run in the miner process, so the hosted notifyMinerKillSwitchPagerDuty export had no call site. Keep paging on the miner trip path and document the Events API contract mirror.

Co-authored-by: Cursor <cursoragent@cursor.com>
@jsdevninja
jsdevninja requested a review from JSONbored as a code owner July 21, 2026 09:11
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.56%. Comparing base (ce48673) to head (4664893).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7699      +/-   ##
==========================================
- Coverage   88.56%   88.56%   -0.01%     
==========================================
  Files          83      725     +642     
  Lines       20664    76297   +55633     
  Branches     3648    22716   +19068     
==========================================
+ Hits        18302    67575   +49273     
- Misses       2184     7680    +5496     
- Partials      178     1042     +864     
Flag Coverage Δ
shard-1 52.83% <27.27%> (?)
shard-2 51.97% <96.96%> (?)
shard-3 48.44% <9.09%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ckages/loopover-engine/src/governor/kill-switch.ts 100.00% <100.00%> (ø)
...ackages/loopover-miner/lib/governor-kill-switch.ts 100.00% <100.00%> (ø)
src/services/notify-pagerduty.ts 100.00% <ø> (ø)

... and 639 files with indirect coverage changes

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 21, 2026
@loopover-orb

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-21 09:32:56 UTC

9 files · 1 AI reviewer · no blockers · CI failing · blocked

🛑 Suggested Action - Fix Blockers

Review summary
This PR wires a real PagerDuty page into a kill-switch TRIP: a pure `buildMinerKillSwitchPagerDutyAlert` in the engine (null unless the transition is a genuine trip into an active scope), consumed by `recordMinerKillSwitchTransition` via an injectable, fire-and-forget `notify` hook that mirrors ORB's `triggerPagerDutyIncident` Events API v2 contract (same flag/routing-key/dedup_key shape) without blocking the synchronous ledger append. Trace confirms the guard logic is sound — `previousScope !== scope && isMinerKillSwitchActive(scope)` correctly fires only on engage, not on resume or no-op same-scope calls — and the async/sync throw/reject paths are all funneled through one `warnKillSwitchPagerDutyFailed` so a paging failure can never surface to the caller. The PR correctly scopes itself to the miner-only call site (no hosted wrapper, per the #7685 rationale) and the test suite drives the real default-notify/process.env path in addition to the injected-mock path, not just a fabricated scenario.

Nits — 5 non-blocking
  • packages/loopover-miner/lib/governor-kill-switch.js and .ts hand-duplicate the entire PagerDuty implementation (regexes, URL, try/catch structure) with no build step enforcing parity between the compiled .js and its .ts source — a future edit to one and not the other will silently drift; consider generating the .js from the .ts in CI instead of hand-authoring both.
  • packages/loopover-miner/lib/governor-kill-switch.ts:67/70/99/107 — the 5000ms timeout, 1024/200-char slice caps, and events.pagerduty.com URL are inline magic values; ORB's notify-pagerduty.ts has the same pattern already so this is consistent with existing convention, but a shared constant (or re-export from notify-pagerduty.ts) would remove the duplication across both call sites.
  • The miner-side page has no severity floor or cooldown (unlike ORB's `triggerPagerDutyIncident`, which gates on `resolvePagerDutyMinSeverity`/`resolvePagerDutyCooldownMinutes`) — documented as intentional given no Worker Env/D1 in the miner process, but worth confirming an operator understands every trip pages regardless of frequency (PagerDuty's own `dedup_key` coalescing is the only backstop).
  • Extract PAGERDUTY_EVENTS_URL / ROUTING_KEY_RE / TRUTHY_ENV into a small shared module importable by both src/services/notify-pagerduty.ts and packages/loopover-miner/lib/governor-kill-switch.ts to remove the copy-paste and guarantee the two paging paths can't drift on routing-key validation.
  • Consider a lightweight script-based check (or a pretest hook) that recompiles governor-kill-switch.ts and diffs against the checked-in .js, so a hand-edit skew between the two is caught in CI rather than relying on reviewer diligence.

CI checks failing

  • validate
  • validate-code

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7666, #7685
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (2 linked issues).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 203 registered-repo PR(s), 127 merged, 37 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jsdevninja; Gittensor profile; 203 PR(s), 37 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: significant
Linked issue satisfaction

Addressed
The PR wires the miner kill-switch trip path to fire a PagerDuty page mirroring notify-pagerduty.ts's flag/routing-key/enqueue/dedup_key contract, adds a pure alert-builder in the engine detector, keeps resumes silent, includes mocked tests across both packages, and updates the runbook to reflect the new paging behavior. It deliberately reuses the same contract rather than calling notify-pagerduty

Review context
  • Author: jsdevninja
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript, Swift, C, CSS, MDX, Python, TypeScript, Vue
  • Official Gittensor activity: 203 PR(s), 37 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/ desktop before /
before /
after /
after /
/ mobile before / (mobile)
before / (mobile)
after / (mobile)
after / (mobile)

Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy.

Scroll preview
Route Before (production) After (this PR's preview)
/ before / (scroll)
before / (scroll)
after / (scroll)
after / (scroll)

A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-code)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wire real PagerDuty alerting into AMS kill-switch trips (currently ledger-only, no paging)

1 participant