An automated review of #1189 flagged that BReg's result-poll budget terminalizes reviews that are merely slow. In crates/registry-breg/src/review_store.rs, a ReviewResultResponse::Pending (and ConcealedOrUnknown) response increments result_poll_attempts exactly as a failed lookup does, with the retry backoff capped at 60 seconds. The give-up sweep then marks the accepted submission failed once result_poll_attempts >= 1000 or the recovery deadline passes.
Failure scenario: a human review legitimately stays pending. At a 60-second ceiling the budget saturates after roughly 16.6 hours, which is shorter than many real review turnarounds. The submission flips to failed with result-poll-attempts-exhausted; failed rows are no longer selected for result polling, and reconcile_result only updates rows in state accepted or cancelling, so the eventual approval can never be applied. The same applies to a review still pending at the submission recovery deadline.
Suggested fix:
- Count only failed or uncertain lookups against the give-up budget; a conforming
202 Pending response should reset or not advance it.
- Decide separately whether a pending review should be bounded at all, and if so bound it by a declared review deadline rather than by the submission-recovery window.
- Add a Postgres test where a submission stays pending past the current budget and still reconciles when the result arrives.
Note: this behaviour was introduced by the terminal-budget commit in #1189 itself, which gave accepted rows a give-up path they previously lacked. The give-up path is still needed for a permanently broken result endpoint; only the pending case is wrong.
Filed from the review of #1189; not merge-blocking under the current no-adopter policy, but it is the first follow-up to pick up.
An automated review of #1189 flagged that BReg's result-poll budget terminalizes reviews that are merely slow. In
crates/registry-breg/src/review_store.rs, aReviewResultResponse::Pending(andConcealedOrUnknown) response incrementsresult_poll_attemptsexactly as a failed lookup does, with the retry backoff capped at 60 seconds. The give-up sweep then marks the accepted submissionfailedonceresult_poll_attempts >= 1000or the recovery deadline passes.Failure scenario: a human review legitimately stays pending. At a 60-second ceiling the budget saturates after roughly 16.6 hours, which is shorter than many real review turnarounds. The submission flips to
failedwithresult-poll-attempts-exhausted; failed rows are no longer selected for result polling, andreconcile_resultonly updates rows in stateacceptedorcancelling, so the eventual approval can never be applied. The same applies to a review still pending at the submission recovery deadline.Suggested fix:
202 Pendingresponse should reset or not advance it.Note: this behaviour was introduced by the terminal-budget commit in #1189 itself, which gave accepted rows a give-up path they previously lacked. The give-up path is still needed for a permanently broken result endpoint; only the pending case is wrong.
Filed from the review of #1189; not merge-blocking under the current no-adopter policy, but it is the first follow-up to pick up.