Startup recovery races the receiver and can deliver a fresh event twice #299

New Issue

clawbot · 2026-08-24T06:44:52+02:00

clawbot commented

2026-08-24 06:44:52 +02:00

Found by the review of #297. Pre-existing — a deterministic probe returns posts=2 identically on next and on that PR's head, so it predates the branch and is not caused by it.

Startup recoverPending runs in a goroutine concurrently with the receiver accepting traffic. A delivery row written just before Notify can be selected by recovery, claimed, delivered, and RELEASED — after which Notify's retainIdle succeeds on the now-free id and dispatches the same delivery again. The target receives two POSTs.

The window is the startup interval during which recovery is still scanning and the receiver is already serving. It is narrow, and the reviewer found it only by building a deterministic probe after a sloppy harness surfaced it as an intermittent 901-for-900.

Why this is not tag-blocking

Delivery is at-least-once by design and now says so in the README (see the Rationale section, and #256 for why a bookkeeping-write failure deliberately leaves a delivery recoverable rather than lying about it). A receiver is already required to be idempotent. This race produces a duplicate that falls inside that documented guarantee rather than violating it.

It is still worth fixing: every other duplicate in the system is the consequence of a failure, and this one happens on a healthy database during ordinary startup. "At-least-once when something went wrong" and "at-least-once on every restart with concurrent traffic" are different operational characters even though the contract covers both.

Definition of done

A fresh event arriving during startup recovery is delivered exactly once. The likely shape is ordering — do not accept receiver traffic until recovery's scan has claimed what it is going to claim — but confirm that does not simply move the race, and do not solve it by extending shutdown or startup past its documented budget (#134, #102).
The ownership mechanism from #256 (internal/delivery/inflight.go) is the right place to reason about this; the bug is that release-then-reacquire is legal for a delivery that was never actually finished by the second claimant's standard.
A deterministic test, not a probabilistic one. The reviewer built a deterministic probe; reproduce that shape rather than looping until a duplicate appears.
Re-run the at-scale no-duplicate check that #256 established — several hundred events across multiple targets, both sweep arms firing, sink POSTs equal to delivery count exactly.

Not milestoned. Top of the next milestone.

Found by the review of https://git.eeqj.de/sneak/webhooker/pulls/297. **Pre-existing** — a deterministic probe returns `posts=2` identically on `next` and on that PR's head, so it predates the branch and is not caused by it. Startup `recoverPending` runs in a goroutine concurrently with the receiver accepting traffic. A delivery row written just before `Notify` can be selected by recovery, claimed, delivered, and RELEASED — after which `Notify`'s `retainIdle` succeeds on the now-free id and dispatches the same delivery again. The target receives two POSTs. The window is the startup interval during which recovery is still scanning and the receiver is already serving. It is narrow, and the reviewer found it only by building a deterministic probe after a sloppy harness surfaced it as an intermittent 901-for-900. ## Why this is not tag-blocking Delivery is at-least-once by design and now says so in the README (see the Rationale section, and https://git.eeqj.de/sneak/webhooker/issues/256 for why a bookkeeping-write failure deliberately leaves a delivery recoverable rather than lying about it). A receiver is already required to be idempotent. This race produces a duplicate that falls inside that documented guarantee rather than violating it. It is still worth fixing: every other duplicate in the system is the consequence of a failure, and this one happens on a healthy database during ordinary startup. "At-least-once when something went wrong" and "at-least-once on every restart with concurrent traffic" are different operational characters even though the contract covers both. ## Definition of done - A fresh event arriving during startup recovery is delivered exactly once. The likely shape is ordering — do not accept receiver traffic until recovery's scan has claimed what it is going to claim — but confirm that does not simply move the race, and do not solve it by extending shutdown or startup past its documented budget (https://git.eeqj.de/sneak/webhooker/issues/134, https://git.eeqj.de/sneak/webhooker/issues/102). - The ownership mechanism from https://git.eeqj.de/sneak/webhooker/issues/256 (`internal/delivery/inflight.go`) is the right place to reason about this; the bug is that release-then-reacquire is legal for a delivery that was never actually finished by the second claimant's standard. - A deterministic test, not a probabilistic one. The reviewer built a deterministic probe; reproduce that shape rather than looping until a duplicate appears. - Re-run the at-scale no-duplicate check that https://git.eeqj.de/sneak/webhooker/issues/256 established — several hundred events across multiple targets, both sweep arms firing, sink POSTs equal to delivery count exactly. Not milestoned. Top of the next milestone.

clawbot referenced this issue

2026-08-24 17:59:07 +02:00

Milestone 1.0.0: internet-facing readiness #111

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/webhooker#299