Stop automatic retries of failed queued messages - #2830
Conversation
|
🚨 SLOP COP 🚨 · I am the Slop Cop. I am reviewing this pull request now. I will check security, code quality, performance, architecture, duplication, and the user flow. |
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Plain English summary: This change stops bb from automatically trying a failed queued message again. The user can still send it manually.
I found one high-severity correctness defect.
A clean member of a grouped message can claim a failed member during a scheduled or plugin drain. This action retries the failed message automatically.
The new selector filter checks each reference row. The later atomic claim rebuilds the complete group and checks only its deadline or wait holder.
The fix must check failureReason for every group member inside the claim transaction. The explicit Send now path must stay unchanged.
Please add grouped regression tests for the scheduled drain and the requested-plugin drain.
The architecture search found no other raw automatic selectors outside the database data module. The shared predicate removes useful duplication.
The performance review found no regression. The existing partial indexes remain active.
As an optional follow-up, the partial due and wait-holder indexes can exclude failed rows. This change can reduce stale index entries.
Verification passed:
- 29 focused server tests.
- 66 focused database and query-plan tests.
- Type checks for
@bb/dband@bb/server. - A browser smoke test of the local app, with no console errors.
I used a comment review. I did not approve this pull request or request changes.
0c064cf to
61d7d4e
Compare
Human comments
What was wrong
The durable queue introduced in #2779 records a background dispatch failure on the queued row. Automatic selectors did not exclude that failed state, so later sweeps could keep running plugin hooks and dispatch preparation without an explicit retry.
There was also a grouped-message race: even after filtering failed rows from the selectors, a clean member could enter the atomic claim and pull a failed member from the same group back into an automatic attempt.
What changed
The shared database selectors exclude terminally failed rows from automatic drains. The atomic grouped-claim boundary now also requires every group member to have no failure whenever an automatic eligibility callback is present. This closes both the clean-member bypass and a failure recorded after selection.
Explicit Send now claims without that automatic callback, so users can still retry failed messages deliberately. Failed rows remain visible through the API and UI.
Normal temporary conditions such as a busy thread, future schedule, provisioning, pending interaction, plugin wait, disconnected host, and host-command timeout do not enter the failed state and still recover automatically.
There are no schema, migration, host-daemon protocol, CLI, configuration, SDK, or documentation changes.
How you verified
pnpm exec turbo run test typecheck build --filter=@bb/db --filter=@bb/server --forceon the rebased tree — DB 442 tests passed, server 2124 tests passed, and typechecks/builds passed.Fixes #2779