Skip to content

services(approval-queue): enqueue notify-deliver for the staged-action and reminder badges so they reach the feed #10025

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

A notification_deliveries row is only visible to its recipient once something moves it out of pending. The
feed builder skips everything else — src/notifications/service.ts:260-262:

  for (const delivery of deliveries) {
    if (delivery.status !== "delivered" && delivery.status !== "read") continue;

The only thing that promotes a row is deliverNotification (src/notifications/service.ts:280-287), which is
reached exclusively via a notify-deliver queue job (src/queue/job-dispatch.ts:400-402). Every insert in
src/notifications/service.ts is therefore paired with an enqueue — see
evaluateAndEnqueueNotificationDeliveries (src/notifications/service.ts:217-232) and the notify-evaluate
handler (src/queue/job-dispatch.ts:386-397).

The approval queue's two inserts are not paired with anything.

src/services/agent-action-executor.ts:1468-1478 — the badge a maintainer gets when an
auto_with_approval action is staged:

  await insertNotificationDeliveryIfAbsent(env, {
    dedupKey: `agent.pending_action:${ctx.repoFullName}#${ctx.pullNumber}:${action.actionClass}`,
    channel: "badge",
    ...
  });
  return true;

src/services/agent-approval-queue.ts:617-631 — the #9032 reminder badge, the entire point of the
approval-queue escape hatch:

      const { created } = await insertNotificationDeliveryIfAbsent(env, {
        dedupKey: `agent.pending_action.reminder:${row.repoFullName}#${row.pullNumber}:${row.actionClass}:${plan.bucket}`,
        ...
      }).catch(() => ({ created: false }));
      if (created) reminded += 1;
      continue;

Neither passes a status, so both rows are created at "pending"
(src/db/repositories.ts:2378: status: input.status ?? "pending"), and neither sends a notify-deliver
job. Both badges therefore sit invisible in notification_deliveries.

The only thing that eventually delivers them is sweepStrandedNotificationDeliveries, whose own doc frames it
as a rescue for a failed enqueue ("a failed/partial enqueue", src/notifications/stranded-delivery-sweep.ts:5)
and which deliberately waits STRANDED_NOTIFICATION_GRACE_MS = 10 * 60 * 1000
(src/notifications/stranded-delivery-sweep.ts:26) so "a delivery merely waiting its turn behind a busy queue
is never mistaken for a lost one". So the approval-queue notifications are structurally dependent on an
unrelated self-heal, delayed by at least ten minutes, and would disappear entirely if that sweep were ever
scoped or disabled. This directly contradicts src/services/agent-approval-staleness.ts:3-8, which describes
the staging badge as the notification the maintainer "gets" and the reminder as the fix for a maintainer who
"misses that single badge".

Requirements

  • stageForApproval (src/services/agent-action-executor.ts:1455-1480) must, when
    insertNotificationDeliveryIfAbsent reports created: true AND the returned delivery's status is
    "pending", send { type: "notify-deliver", requestedBy: "agent-approval", deliveryId: delivery.id } on
    env.JOBS.
  • sweepStaleApprovalQueue (src/services/agent-approval-queue.ts:606-649) must do the same for the reminder
    badge, using the delivery object the same call already returns.
  • Both sends must be best-effort: a rejected env.JOBS.send must be caught and must not abort staging or the
    sweep loop, and must emit one structured console.warn with
    event: "approval_notification_enqueue_failed" carrying deliveryId, repoFullName and pullNumber.
    Staging must still return true; the sweep must still count the reminder and continue to the next row.
  • A created: false result (the dedup already fired for this key) must send NOTHING, so the "exactly one badge
    per (PR, actionClass)" and "exactly one badge per reminder bucket" contracts
    (src/services/agent-approval-staleness.ts:11-14) are unchanged.
  • A delivery inserted at a non-pending status (rate-limit suppression, src/notifications/service.ts:193)
    must send nothing — mirroring evaluateNotificationEvent's own
    if (created && delivery.status === "pending") guard at src/notifications/service.ts:207.
  • insertNotificationDeliveryIfAbsent, deliverNotification, buildNotificationFeed, and
    sweepStrandedNotificationDeliveries must NOT change.
  • The dedup keys at src/services/agent-action-executor.ts:1469 and src/services/agent-approval-queue.ts:620
    must NOT change — the reminder key's bucket index is what makes the reminder fire once per interval.

⚠️ Required pattern: src/notifications/service.ts:217-232
(evaluateAndEnqueueNotificationDeliveries) — insert, keep only the freshly-created pending rows, enqueue
one notify-deliver per row. What does NOT satisfy this issue: (a) inserting the row with
status: "delivered" to bypass the queue, which skips deliverNotification and leaves delivered_at
unset; (b) changing buildNotificationFeed to also surface pending rows, which would expose every
genuinely-undelivered and rate-limit-suppressed row on every surface; (c) leaning on
sweepStrandedNotificationDeliveries and documenting the ten-minute delay as intended; (d) a test-only PR.

Deliverables

  • stageForApproval enqueues one notify-deliver job for a freshly-created pending badge delivery, and
    nothing on created: false or a non-pending status.
  • sweepStaleApprovalQueue enqueues one notify-deliver job for a freshly-created pending reminder
    delivery, and nothing on created: false or a non-pending status.
  • A rejected env.JOBS.send on either path is caught, emits
    event: "approval_notification_enqueue_failed", and does not change the function's return value
    (stageForApproval still returns true; the sweep still counts the reminder).
  • A test in test/unit/agent-approval-queue.test.ts asserting that staging an auto_with_approval action
    sends exactly one notify-deliver message carrying the inserted delivery's id, and that staging the SAME
    action a second time sends none.
  • A test in test/unit/approval-queue-staleness.test.ts asserting that a row aged past
    APPROVAL_REMINDER_INTERVAL_MS produces exactly one notify-deliver message, and that a second sweep
    inside the same reminder bucket produces none.
  • A regression test at test/unit/agent-approval-queue.test.ts named for this bug that stages an action,
    runs deliverNotification for the enqueued id, and asserts buildNotificationFeed for the recipient now
    contains the staged-action item — against today's code the feed is empty because the row is still
    pending.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example fixing
stageForApproval and leaving sweepStaleApprovalQueue's reminder (the #9032 escape hatch whose whole purpose
is reaching a maintainer who missed the first badge) still undelivered — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts, so src/services/agent-action-executor.ts and src/services/agent-approval-queue.ts are
both measured and gated. The change introduces, per call site, a two-condition guard
(created && delivery.status === "pending") plus a send-failure catch. Every arm needs a test: created+pending
(send), created+suppressed (no send), not-created (no send), and send-rejects (warn, no throw) — four cases per
call site, eight in total.

Expected Outcome

After this ships, a maintainer whose repo stages an auto_with_approval action sees the badge in their
notification feed as soon as the notify-deliver job runs, and the daily reminder for a still-unapproved row
arrives on its own schedule — instead of both sitting at pending and depending on an unrelated
stranded-delivery sweep that deliberately waits ten minutes before touching them.

Links & Resources

  • src/services/agent-action-executor.ts:1455-1480stageForApproval, insert with no enqueue
  • src/services/agent-approval-queue.ts:606-649sweepStaleApprovalQueue, insert with no enqueue
  • src/notifications/service.ts:191-232 — the insert-then-enqueue pattern every other caller follows
  • src/notifications/service.ts:257-287buildNotificationFeed's delivered/read filter and deliverNotification
  • src/db/repositories.ts:2360-2382status: input.status ?? "pending"
  • src/notifications/stranded-delivery-sweep.ts:1-40 — the 10-minute-grace rescue currently doing this job
  • src/services/agent-approval-staleness.ts:1-20 — the orb(ux): approval-queue rows never expire and notify only once #9032 contract these badges are supposed to satisfy

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions