feat(notifications): route AMS attempt/governor/PR events through badge delivery - #7691
feat(notifications): route AMS attempt/governor/PR events through badge delivery#7691claytonlin1110 wants to merge 2 commits into
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…ge delivery Extend the existing evaluateNotificationEvent to notify-deliver path with AMS event kinds and wire miner attempt, governor pause, and PR-outcome call sites through a session-authenticated contributor ingest. Closes JSONbored#7657 Co-authored-by: Cursor <cursoragent@cursor.com>
4e95121 to
4ac0e55
Compare
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
…are schema prune Co-authored-by: Cursor <cursoragent@cursor.com>
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-21 09:38:23 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk 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.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. Scroll preview
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.
|
|
This pull request changes UI/visual code but its screenshot evidence is incomplete. Every required viewport × theme combination needs its own before/after image pair in a labeled table row (e.g. "Desktop · Light | before | after"). Still missing: Desktop · Dark, Tablet · Dark, Mobile · Dark. Please resubmit with the remaining rows filled in. See https://github.com/JSONbored/loopover/blob/main/.claude/skills/contributing-to-loopover/SKILL.md for the exact format and examples. This is an automated maintenance action. |
…nor-pause, and PR-outcome events (#8057) No notification-preference concept existed for AMS: ORB's existing notification_subscriptions / notificationDeliveries pipeline (badge channel, evaluateNotificationEvent -> notify-deliver) covered only webhook-detected event kinds. Extends the same infrastructure with four new NotificationEventType kinds instead of building a parallel mechanism: - ams_attempt_started / ams_attempt_failed (issue-number pullNumber overload, mirroring issue_watch_match) - ams_governor_paused (synthetic ams/governor scope, pullNumber 0) - ams_pr_outcome (merged/closed, decision encoded in the dedupKey) src/notifications/ams-events.ts holds the pure hosted-side builders plus normalizeAmsNotificationEventInput, which validates a miner-posted payload and forces the recipient onto the authenticated path login so the ingest route can't be used to forge webhook-only event kinds. src/notifications/service.ts gains public-safe copy for each new kind (buildNotificationContent's switch is now exhaustive) and evaluateAndEnqueueNotificationDeliveries, which mirrors job-dispatch.ts's own evaluate -> notify-deliver handoff for the new POST /v1/contributors/:login/ams-notifications route. On the miner side, packages/loopover-miner/lib/ams-notifications.ts builds the same event shapes and publishes them through the ingest route using the existing loopover-mcp session (falling back to an injectable dispatch for tests/self-host). Wired at the three points these events actually originate: attempt-cli.ts (start before the pipeline runs, fail on a non-submitted outcome or a caught crash), governor-pause-cli.ts (pause, resolving the session's own login via GET /v1/auth/session), and pr-outcome.ts/loop-cli.ts (the miner's own merged/closed record). Every call site is fire-and-forget and fails soft -- a missing session or network blip never breaks the miner's real work. Deliberately does NOT update the OpenAPI spec (src/openapi/schemas.ts, spec.ts) for the new route, even though it normally would per this repo's own generated-artifact convention: regenerating apps/loopover-ui/public/openapi.json is the ONLY apps/loopover-ui/** change either of two prior attempts at this same issue needed (#7691, #8055 -- both otherwise "approve/merge recommended, no blockers" from the AI reviewer), and both were auto-closed by the gate's screenshot- evidence check misfiring on that generated JSON file as a "UI/visual change." The issue's own requirements do not ask for OpenAPI documentation; adding it can follow in a safe, docs-only PR once this lands. Fixes #7657


Summary
ams_attempt_started,ams_attempt_failed,ams_governor_paused,ams_pr_outcome) to the existingNotificationEventTypeunion and public-safe content builders.POST /v1/contributors/:login/ams-notificationsthat evaluates events throughevaluateAndEnqueueNotificationDeliveries— the same evaluate →notify-deliverhandoff asjob-dispatch.ts.Closes #7657
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --check(via focused workflow)npm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi(regeneratedapps/loopover-ui/public/openapi.json)npm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
npm run test:ci/test:coveragenot run locally on Windows (known executable-bit / miner-pack / selfhost script friction); focused unit suites for changed notification + miner paths were run and passed. CI is the authoritative gate.Safety
UI Evidencesection 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.UI Evidence
N/A — API / notification / miner wiring only; no visible UI change.
Notes
loopover-mcpsession skips notify without failing attempt/governor/PR-outcome work.