fix(services): enqueue notify-deliver for the approval-queue staged-action and reminder badges - #10173
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) -- directly 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' guard: a dedup hit or a rate-limit-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. |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (81.81%) is below the target coverage (99.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #10173 +/- ##
===========================================
- Coverage 92.21% 80.60% -11.62%
===========================================
Files 932 285 -647
Lines 114104 59322 -54782
Branches 27559 8923 -18636
===========================================
- Hits 105220 47815 -57405
- Misses 7583 11205 +3622
+ Partials 1301 302 -999
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Caution 🛑 LoopOver review result - fixes requiredReview updated: 2026-07-31 11:10:10 UTC
Review summary Nits — 3 non-blocking
CI checks failing
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.
|
|
LoopOver is closing this pull request on the maintainer's behalf (CI is failing (codecov/patch)). 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. |
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