Store events and create deliveries in the delivery engine (closes #544)

The receiver, resubmit and replay handlers now parse the request and
make one call. Engine.StoreEvent stores an event with a pending
delivery for each active target and the totals in one transaction,
then queues them; Engine.CreateReplay does the same for one replay
delivery. The handlers take *delivery.Engine, so the Notifier
interface and its bridge are gone, and the engine's notify is no
longer exported. Archives and CircuitBreakers stay because the handler
tests fake them. The tests that read a queued task moved to the
engine's tests; the handler tests use an engine that is never started.

Model: opus-5-5

Dockerfile

@@ -111,16 +111,11 @@ RUN --network=none golangci-lint run --config .golangci.yml --build-tags browser
 # Nothing is wanted from js-lint; the copy is what makes this phase run it.
 COPY --from=js-lint /src/yarn.lock /dev/null
 
-# Test phase. The tests run with cgo off and so without Go's race detector,
+# Test phase. -race needs cgo and so a C compiler, which the Debian Go image
-# which needs cgo on Linux; sneak ruled the race detector out in
+# ships and the alpine one does not. `make test` (script/test) builds this
-# https://git.eeqj.de/sneak/webhooker/issues/538#issuecomment-153273.
+# stage alone; the build stage below depends on it.
-# `make test` (script/test) builds this stage alone; the build stage below
-# depends on it.
 #
-# golang:1.26.1-bookworm (Debian-based), 2026-03-17: the builder stage's
+# golang:1.26.1-bookworm (Debian-based), 2026-03-17
-# image, which that stage needs for its git and make. Its test step runs
-# under the image's bash, and the tests in internal/versionscript use its
-# git, make and sh, and skip when they are missing.
 FROM golang:1.26.1-bookworm@sha256:4465644228bc2857a954b092167e12aa59c006a3492282a6c820bf4755fd64a4 AS test
 
 WORKDIR /src
@@ -136,8 +131,8 @@ RUN script/assets
 # -timeout applies to each package on its own, so 90s has only to clear the
 # slowest one. -p 4 -parallel 8 keep the run under 2 GB of memory: at most
 # four test binaries build or run at once, each with at most eight parallel
-# tests. The defaults are one per core, so without these the memory the run
+# tests. Under -race every test binary and every link costs a few hundred MB,
-# needs grows with the host's core count.
+# so the defaults (one per core) add up to several GB on a many-core host.
 #
 # The first run has no -v: go test then prints one result line per package,
 # with its coverage, and for a package that fails, everything its tests
@@ -159,12 +154,12 @@ RUN script/assets
 SHELL ["/bin/bash", "-o", "pipefail", "-c"]
 RUN --mount=type=tmpfs,target=/tmp/tests,size=512m \
     export TMPDIR=/tmp/tests GOTMPDIR=/tmp; \
-    CGO_ENABLED=0 go test -cover -p 4 -parallel 8 -timeout 90s ./... 2>&1 | tee /tmp/go-test.log && exit 0; \
+    go test -race -cover -p 4 -parallel 8 -timeout 90s ./... 2>&1 | tee /tmp/go-test.log && exit 0; \
     tests="$(awk '/^--- FAIL: / { print $3 }' /tmp/go-test.log | paste -s -d '|' -)"; \
     packages="$(awk '/^FAIL\t/ { print $2 }' /tmp/go-test.log)"; \
     if [ -n "$tests" ]; then \
         echo "--- Rerunning the failed tests with -v for details ---"; \
-        CGO_ENABLED=0 go test -v -p 4 -parallel 8 -timeout 90s -run "^($tests)\$" $packages; \
+        go test -race -v -p 4 -parallel 8 -timeout 90s -run "^($tests)\$" $packages; \
     fi; \
     exit 1
 

Dockerfile.browser

@@ -15,9 +15,9 @@ RUN go mod download
 COPY . .
 
 # The test binary embeds the templates and static files, so the browser
-# stage needs nothing else. It is built with cgo off and -p 4 keeps the
+# stage needs nothing else. -p 4 keeps the compile's memory down, as in
-# compile's memory down, both as in the test phase of Dockerfile.
+# the test phase of Dockerfile.
-RUN make assets && CGO_ENABLED=0 go test -c -p 4 -tags browser -o /browser.test ./internal/server
+RUN make assets && go test -c -p 4 -tags browser -o /browser.test ./internal/server
 
 # chromedp/headless-shell:151.0.7922.109 (Debian trixie), 2026-08-11. The
 # browser is on PATH as headless-shell, where the test's browser library

README.md

@@ -60,7 +60,7 @@ make assets     # Extract Alpine.js from 3p/ (build and dev run it)
 make fmt        # Format Go (gofmt + goimports) and Markdown (prettier, in Docker)
 make fmt-check  # Fail if gofmt or prettier would change anything (writes nothing)
 make lint       # Run golangci-lint and ESLint in Docker
-make test       # Run tests with cgo off, in Docker
+make test       # Run tests with race detection, in Docker
 make test-browser # Run the browser test in Docker (Dockerfile.browser)
 make check      # test + lint + fmt-check + css-check (CI gate)
 make build      # Build binary to bin/webhooker (version-stamped)
@@ -75,11 +75,6 @@ make css-check  # Fail if static/css/tailwind.css is stale (writes nothing)
 make clean      # Remove bin/
 ```
 
-The tests run with cgo off and so without Go's race detector, which needs cgo on
-Linux: an exception to the `-race` in the test phase of the vendored
-`REPO_POLICIES.md`, ruled by @sneak in
-https://git.eeqj.de/sneak/webhooker/issues/538#issuecomment-153273.
-
 ### Configuration
 
 All configuration is via environment variables. For local development, you can
@@ -2192,7 +2187,7 @@ A **Resubmit** stores its copy of an event through the same `Engine.StoreEvent`,
 and a **Replay** adds one delivery of a stored event through
 `Engine.CreateReplay`, which writes the delivery and its target's totals in one
 transaction and queues it. Those two methods are the only code that creates a
-delivery; the handlers read and check the request, then make the one call.
+delivery; the handlers parse the request and make the one call.
 
 ### Bounded Worker Pool
 
@@ -2203,9 +2198,8 @@ preventing goroutine explosions regardless of queue depth.
 **Architecture:**
 
 - **Channels as queues:** Two buffered channels serve as bounded queues: a
-  delivery channel (the tasks of newly stored deliveries and of the `pending`
+  delivery channel (the tasks of deliveries the engine has just stored) and a
-  deliveries the recovery routine hands back) and a retry channel (tasks from
+  retry channel (tasks from backoff timers). Both are buffered to 10,000.
-  backoff timers). Both are buffered to 10,000.
 - **Fan-out via channel, not goroutines:** When an event arrives with multiple
   targets, each `delivery.Task` is sent to the delivery channel. Workers pick
   them up and process them — no goroutine-per-target.
@@ -3167,7 +3161,7 @@ Components are wired via Uber fx in this order:
 11. `metrics.New` — The delivery collectors, registered on that registry
 12. `middleware.New` — HTTP middleware
 13. `delivery.New` — Event-driven delivery engine, which also stores each event
-    with its deliveries and creates replay deliveries
+    with its deliveries
 14. `archive.NewWriters` — The database targets' archive writers, and the lock
     every rename of their files runs under
 15. `archive.NewSweeper` — Periodic pruning of idle archives
@@ -3485,8 +3479,8 @@ fixed independently of the compiler's:
    `golangci-lint config verify` and `golangci-lint run`, both with
    `--network=none`, and depends on `js-lint` (it copies a file from it).
    `make lint` builds this stage alone.
-5. **Test phase** (`test`, `golang:1.26.1-bookworm`, the builder stage's image)
+5. **Test phase** (`test`, `golang:1.26.1-bookworm`, whose C compiler `-race`
-   — extracts Alpine.js, then runs `go test -cover` with cgo off, at most four
+   needs) — extracts Alpine.js, then runs `go test -race -cover` at most four
    packages and eight tests at a time, with a 90-second timeout per package. On
    a failure it runs only the failed tests again with `-v`, since verbose output
    from the whole suite would pass the 2 MiB at which the Docker build cuts off

internal/archive/evict_test.go

@@ -303,7 +303,7 @@ func (r *racingWrites) wait() (bool, error) {
 // contending for the writer's mutex when the eviction takes it.
 // The write that wins the mutex after the eviction must abandon
 // its work rather than reopen the archive, leaving the writer
-// permanently handle-free.
+// permanently handle-free. Run under -race.
 func TestEvictWebhook_RacingWriteDoesNotReopenHandle(
 	t *testing.T,
 ) {

internal/archive/sweeper_test.go

@@ -959,11 +959,9 @@ func TestArchiveSweep_SkipsDeletedWebhookTargets(t *testing.T) {
 	)
 }
 
-// TestArchiveSweep_ConcurrentWrites runs the sweep while writes to
+// TestArchiveSweep_ConcurrentWrites proves the sweep serialises
-// the same target are under way. The sweep serialises against them
+// against writes through the target's writer mutex. Run under
-// through the target's writer mutex; the suite runs without Go's race
+// -race, an unsynchronised sweep would be caught here.
-// detector, so a sweep that did not would fail this test only by
-// panicking, hanging or leaving no archive file.
 func TestArchiveSweep_ConcurrentWrites(t *testing.T) {
 	t.Parallel()
 

internal/database/password.go

@@ -67,10 +67,9 @@ func DefaultPasswordConfig() *PasswordConfig {
 // testArgon2Memory is the Argon2id memory cost, in KiB, that a test
 // binary hashes with: 1 MB instead of the shipped 64 MB. Every test
 // that starts a database hashes the bootstrap admin password, dozens
-// of them run in parallel, and each 64 MB hash holds at least 64 MB
+// of them run in parallel, and under the race detector each 64 MB hash
-// while it runs (about 150 MB when the suite ran under the race
+// holds about 150 MB. VerifyPassword reads the cost from the hash it
-// detector). VerifyPassword reads the cost from the hash it checks, so
+// checks, so verification follows.
-// verification follows.
 const testArgon2Memory = 1024
 
 // hashAtShippedCostInTest makes a test binary hash at the shipped

internal/delivery/engine.go

@@ -23,11 +23,10 @@ import (
 
 const (
 	// deliveryChannelSize is the buffer size for the delivery
-	// channel. The Tasks of newly stored deliveries, and of the
+	// channel. The Tasks of newly stored deliveries are sent
-	// pending deliveries the recovery routine hands back, are
+	// here. Workers drain this channel. Sized large enough
-	// sent here. Workers drain this channel. Sized large enough
+	// that storing an event should never block under normal
-	// that no task should be dropped under normal load: one that
+	// load.
-	// does not fit is left for the sweep.
 	deliveryChannelSize = 10000
 
 	// retryChannelSize is the buffer size for the retry
@@ -152,13 +151,12 @@ type EngineParams struct {
 }
 
 // Engine processes queued deliveries in the background
-// using a bounded worker pool architecture. It also stores each
+// using a bounded worker pool architecture. It owns only
-// event with its deliveries and creates replay deliveries (see
+// the cross-target machinery: the worker pool, the queue
-// store.go). Beyond that it owns only the cross-target machinery:
+// and retry channels, the recovery routine, and the persistence
-// the worker pool, the delivery and retry channels, the recovery
+// helpers and Scheduler that individual targets rely on.
-// routine, and the persistence helpers and Scheduler that
+// Each target type owns its own delivery, including retries,
-// individual targets rely on. Each target type owns its own
+// backoff, and circuit breaking.
-// delivery, including retries, backoff, and circuit breaking.
 type Engine struct {
 	database   *database.Database
 	dbManager  *database.WebhookDBManager

internal/delivery/inflight_test.go

@@ -84,7 +84,7 @@ func TestSweepLeavesAQueuedDeliveryAlone(t *testing.T) {
 	)
 	rAgePending(t, s.WebhookDB, d.ID)
 
-	// Queued exactly as StoreEvent queues it, and never dequeued:
+	// Queued exactly as the receiver queues it, and never dequeued:
 	// no workers are running in this engine.
 	s.Engine.ExportNotify([]delivery.Task{{
 		DeliveryID: d.ID,
@@ -248,9 +248,9 @@ func TestOwnershipIsReleasedAfterDelivery(t *testing.T) {
 }
 
 // TestNotifyAfterRecoveryDoesNotSendAgain is the startup race of
-// https://git.eeqj.de/sneak/webhooker/issues/299. StoreEvent has
+// https://git.eeqj.de/sneak/webhooker/issues/299. The receiver has
 // written a delivery, the recovery routine's run at start finds it
-// pending, sends it and releases it, and only then does StoreEvent's
+// pending, sends it and releases it, and only then does the receiver's
 // notify for it arrive. Nothing owns the delivery by then, so notify
 // takes it.
 func TestNotifyAfterRecoveryDoesNotSendAgain(t *testing.T) {

internal/handlers/event_resubmit.go

@@ -172,10 +172,8 @@ func loadResubmitSource(
 }
 
 // queueResubmit has the delivery engine store the copy, with a delivery
-// for each of the webhook's active targets as they stand now, through
+// for each of the webhook's active targets as they stand now, exactly
-// the same StoreEvent the receiver uses. The one difference is that the
+// as it stores a received event.
-// copy leaves its entrypoint's last event as it is, since it did not
-// arrive on that URL.
 func (h *Handlers) queueResubmit(
 	w http.ResponseWriter,
 	r *http.Request,

internal/handlers/handlers_test.go

@@ -239,8 +239,8 @@ func newTestAppWithConfig(
 			session.New,
 			archive.NewWriters,
 			// The engine is built on a lifecycle that is never started,
-			// so no worker runs: every delivery the engine creates for a
+			// so no worker runs: every delivery a handler creates stays
-			// handler stays pending, where the test can read it.
+			// pending, where the test can read it.
 			func(p delivery.EngineParams) *delivery.Engine {
 				return delivery.New(fxtest.NewLifecycle(t), p)
 			},

internal/logfield/logfield_test.go

@@ -21,9 +21,8 @@ const budget = 64
 
 // batchRunes is how many consecutive code points the charge test logs
 // in one value from U+1000 up. Logging each of those on its own line
-// was too slow for the suite when it ran under the race detector;
+// is too slow for the suite under the race detector; 4,096 at a time
-// 4,096 at a time is 271 batches, each logged on two lines, so 542
+// is 271 batches, each logged on two lines, so 542 lines per handler.
-// lines per handler.
 const batchRunes = 4096
 
 // newHandlers are the two handlers internal/logger can install. Time
@@ -153,10 +152,10 @@ func TestEncodedBytes_ChargesAtLeastWhatTheHandlersEmit(t *testing.T) {
 		batches = append(batches, batch.String())
 	}
 
-	// What EncodedBytes charges for each batch. When the suite ran
+	// What EncodedBytes charges for each batch. Under -race -cover this
-	// under -race -cover this took longer than logging the batches, so
+	// takes longer than logging the batches, so it is worked out once,
-	// it is worked out once, by whichever handler finishes logging
+	// by whichever handler finishes logging first, while the other is
-	// first, while the other is still logging.
+	// still logging.
 	charged := sync.OnceValue(func() []int {
 		costs := make([]int, len(batches))
 

internal/server/early_shutdown_test.go

@@ -10,12 +10,14 @@ import (
 	"sneak.berlin/go/webhooker/internal/server"
 )
 
-// earlyStopIterations is how many start/stop cycles the test below
+// earlyStopIterations is how many start/stop cycles the race test
 // runs. The window it aims at is the gap between the OnStart hook
 // returning and the serving goroutine reaching its first field
-// access, which is microseconds wide, so a single cycle can miss it
+// access, which is microseconds wide. The race detector reports an
-// purely on scheduling. Repetition makes hitting it reliable; the
+// unsynchronised pair whenever it observes one, but it has to observe
-// collaborators are built once, so the cycles themselves are cheap.
+// one, so a single cycle can miss purely on scheduling. Repetition
+// makes the observation reliable; the collaborators are built once,
+// so the cycles themselves are cheap.
 const earlyStopIterations = 25
 
 // TestEarlyShutdown_NoPanicAndNoRace stops the application
@@ -36,9 +38,8 @@ const earlyStopIterations = 25
 //
 // httpServer is now built in New, on the constructing goroutine, so
 // it is written before any hook exists and can never be nil.
-// sentryEnabled is atomic. This test catches the nil dereference
+// sentryEnabled is atomic. This test is what catches either one
-// coming back, as a panic. The suite runs without Go's race
+// coming back — under -race, which is how the suite runs.
-// detector, so the data race coming back would go unreported here.
 func TestEarlyShutdown_NoPanicAndNoRace(t *testing.T) {
 	t.Parallel()