fix(services): enqueue notify-deliver for the approval-queue staged-action and reminder badges - #10182
Conversation
…ction and reminder badges A notification_deliveries row is only visible in the feed once notify-deliver promotes it out of pending, and every other insert in the codebase pairs the insert with an enqueue. The approval queue's two inserts did not: stageForApproval's staged-action badge and sweepStaleApprovalQueue's JSONbored#9032 reminder badge were both created at pending with no notify-deliver job, so they sat invisible until the stranded-delivery sweep rescued them 10+ minutes later (a rescue for a FAILED enqueue, not the primary path) -- contradicting the staleness module's framing of the staging badge as the notification the maintainer gets. Enqueue one notify-deliver per freshly-created pending delivery at both sites, mirroring evaluateAndEnqueueNotificationDeliveries' created + pending guard: a dedup hit or a suppressed non-pending row enqueues nothing. Best-effort: a rejected send is caught, warns approval_notification_enqueue_failed, and does not abort staging (still returns true) or the sweep loop (still counts the reminder). Extend the notify-deliver requestedBy union with agent-approval. The dedup keys, the insert helper, deliverNotification, buildNotificationFeed, and the stranded sweep are unchanged. Closes JSONbored#10025
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-31 11:29:49 UTC
Review summary Nits — 3 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. 🟩 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.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10182 +/- ##
==========================================
+ Coverage 80.44% 80.60% +0.15%
==========================================
Files 282 285 +3
Lines 58764 59316 +552
Branches 6965 7203 +238
==========================================
+ Hits 47274 47811 +537
- Misses 11199 11205 +6
- Partials 291 300 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
|
What & why
A
notification_deliveriesrow is only visible in the recipient's feed once something promotes it out ofpending— and the only thing that does is anotify-deliverqueue job. Every insert insrc/notifications/service.tsis paired with an enqueue (evaluateAndEnqueueNotificationDeliveries, thenotify-evaluatehandler). The approval queue's two inserts were not:stageForApproval(agent-action-executor.ts) — the badge a maintainer gets when anauto_with_approvalaction is staged.sweepStaleApprovalQueue(agent-approval-queue.ts) — the orb(ux): approval-queue rows never expire and notify only once #9032 reminder badge, the entire point of the escape hatch.Neither passed a
status, so both rows were created at"pending", and neither sent anotify-deliverjob. Both badges therefore sat invisible innotification_deliveries. The only thing that eventually delivered them wassweepStrandedNotificationDeliveries, whose own doc frames it as a rescue for a failed enqueue and which deliberately waitsSTRANDED_NOTIFICATION_GRACE_MS(10 min). So these notifications were structurally dependent on an unrelated self-heal, delayed 10+ minutes, and would vanish entirely if that sweep were ever scoped — directly contradictingagent-approval-staleness.ts's framing of the staging badge as the notification the maintainer "gets."The fix
At both sites, when
insertNotificationDeliveryIfAbsentreportscreated: trueand the returned delivery'sstatusis"pending", send{ type: "notify-deliver", requestedBy: "agent-approval", deliveryId: delivery.id }onenv.JOBS— mirroringevaluateAndEnqueueNotificationDeliveries. A dedup hit (created: false) or a rate-limit-suppressed non-pending row enqueues nothing, preserving the "exactly one badge per (PR, actionClass)" / "one badge per reminder bucket" contracts.Both sends are best-effort: a rejected
env.JOBS.sendis caught, emits one structuredconsole.warn(event: "approval_notification_enqueue_failed"withdeliveryId/repoFullName/pullNumber), and does not abort staging (stageForApprovalstill returnstrue) or the sweep loop (still counts the reminder, continues to the next row). Thenotify-deliverrequestedByunion is extended with"agent-approval".Unchanged: the two dedup keys,
insertNotificationDeliveryIfAbsent,deliverNotification,buildNotificationFeed, andsweepStrandedNotificationDeliveries.Tests (
agent-approval-queue.test.ts,approval-queue-staleness.test.ts)auto_with_approvalaction enqueues exactly onenotify-delivercarrying the inserted delivery's id; a second staging (dedup) enqueues none.APPROVAL_REMINDER_INTERVAL_MSenqueues exactly one remindernotify-deliver; a second sweep in the same bucket enqueues none.env.JOBS.sendon either path warnsapproval_notification_enqueue_failedand does not change the return (queued/reminded: 1).deliverNotificationfor the enqueued id,buildNotificationFeedfor the recipient now contains the staged-action item — against today's code the feed is empty (the row is stillpending).main.Validation
types.tsis a type-only change.npm run typecheckclean;npm run engine-parity:drift-check,ui-derived-types:check, andmanifest:drift-checkall pass (thetypes.tschange touches no generated artifact);dead-exports:checkclean; both suites (117 tests) green.git diff --checkclean; no schema/migration change.Closes #10025