[Fix] PR review feedback notifications disappear during long-running tasks

[roomote-roomote]

​Opened on behalf of @daniel-lxs. View the task or mention @roomote-roomote for follow-up asks.

What changed

PR review notifications now stop deferring and use the normal delivery path when a task remains active through the deferral cap. Pending feedback is drained once, triaged, routed to the originating conversation, and posted with any applicable resolve offer instead of being discarded.

The regression coverage exercises both lifecycle states that previously dropped feedback (Running and an idle run whose taskPhase is still running) and runs a duplicate capped job to verify that Redis destructive-drain semantics still produce one user-visible notification and one history record.

Why this change was made

The end-to-end path is GitHub review or terminal review-summary classification, linked-task lookup, Redis pending-event append and schedule-marker claim, BullMQ debounce and lifecycle deferral, live-state triage, originating-conversation routing, action-record creation, and either interactive delivery or active-run/snapshot follow-up dispatch.

The concrete failure was the BullMQ terminal deferral transition: after 288 five-minute deferrals, matching the pending event's roughly 24-hour lifetime, a still-active task called consumePendingPrReviewActivity() and returned before triage or delivery. This deterministically deleted review feedback and therefore also suppressed the resolve offer. Immediate self-review summaries and ordinary delayed review events both reach this same lifecycle gate.

Impact

Review feedback can no longer disappear solely because its linked task remains active for the full deferral window. Before the cap, notifications remain debounced and deferred as before. At the cap, the existing delivery and failure-requeue path is used. Duplicate or late jobs remain harmless because only the first job receives events from the atomic Redis drain; later jobs observe an empty batch.

To reproduce the old state-machine failure, check out the parent commit and run:

pnpm --filter @roomote/bullmq exec vitest run src/jobs/pr-review-notification.test.ts

The old assertions named drops at the deferral cap... and drops pending activity... pass only because the worker consumes the batch without calling delivery preparation or posting. On this branch, the replacement assertions prove capped delivery and exactly-once behavior.

Focused verification:

pnpm --filter @roomote/bullmq exec vitest run src/jobs/pr-review-notification.test.ts
pnpm exec dotenvx run -f .env.test -- pnpm --filter @roomote/api exec vitest run src/handlers/github/__tests__/notifyPrReviewActivity.test.ts src/handlers/github/__tests__/index.test.ts
pnpm exec dotenvx run -f .env.test -- pnpm --filter @roomote/sdk exec vitest run src/server/lib/task-runs/__tests__/pr-review-notification.test.ts src/server/lib/task-runs/__tests__/pr-review-notification-delivery.test.ts
pnpm exec dotenvx run -f .env.test -- pnpm --filter @roomote/bullmq exec vitest run src/jobs/active-pr-review-follow-up.test.ts
pnpm --filter @roomote/bullmq check-types
pnpm --filter @roomote/bullmq format:check
pnpm --filter @roomote/bullmq exec eslint src/jobs/pr-review-notification.ts src/jobs/pr-review-notification.test.ts --max-warnings=0

These cover webhook classification and terminal summary promotion, linked-task and Redis schedule/cycle behavior, triage and conversation routing, active-run versus snapshot-resume dispatch, and the corrected capped-delivery transition. The repository pre-push hook also passed oxlint, fast typechecks, knip, and residual web/worker lint.

[roomote-community]

1 issue outstanding. See task

• apps/bullmq/src/jobs/pr-review-notification.ts:268 - The maximum deferral schedule exceeds the 24-hour pending-event TTL, so the terminal delivery path usually drains an expired batch.

_{Reviewed a9b2cbd}

[roomote-community]

The cap is reached after the pending-list TTL for ordinary activity: enqueue waits one minute before the first run, then 288 five-minute deferrals put this branch at about 24h01m, while appendPendingEventAndClaimSchedule() expires the list after exactly 24h. By then consumePendingPrReviewActivity() returns an empty batch, so this still drops the feedback in the long-running case. Lower the cap with a buffer or refresh/extend the pending-event TTL while deferring.

[roomote-roomote]

Closing this draft after re-validating the support timeline against current develop.

The observed missing-feedback symptom predates #1253, which moved PR comment, submitted-review, inline-review, and terminal review-summary notification scheduling ahead of skipped-repository early returns. That directly fixes the linked-task delivery gap in excluded repositories. #1161 and #1206 cover the adjacent summary identity and duplicate batching paths, while #1170 intentionally preserves the fresh-heartbeat deferral cap and releases stale workers earlier.

The draft's regression test also mocked away an important production constraint: pending events expire after 24 hours, but an isolated human event reaches the 288-deferral cap only after the initial debounce plus 24 hours. The forced-delivery branch therefore normally consumes an already-expired list and does not prove a residual fix on latest develop. No non-redundant production failure was reproduced, so this change should not merge.