The delivery engine, not the handlers, now stores events and creates deliveries, in internal/delivery/store.go:
Engine.StoreEvent stores the event, a pending delivery for each active target and the totals in one transaction, then queues them. The receiver and resubmit handlers call it.
Engine.CreateReplay creates one replay delivery of a stored event with its target's totals in one transaction, then queues it. The replay handler keeps its refusals and calls it.
Tasks are built by recovery's builder, renamed buildTask. The handlers take *delivery.Engine; the Notifier interface and its bridge in cmd/webhooker/main.go are gone, and Notify is now the unexported notify.
Not visible in the diff: the handler, server and resetpw test graphs build a real engine that is never started, so every delivery a test creates stays pending.
Disclosures:
Judgement call: Archives and CircuitBreakers stay, with their bridges: the handler tests fake both, and archive files are not the engine's work.
Judgement call: replay's own Omit(clause.Associations) is dropped; the event database's guard omits associations on every write.
Deviation: the receiver, resubmit and replay tests that read the queued task moved to store_test.go; the receiver's answer tests are unchanged.
Log message: receiver, failed target query, now "failed to store webhook event"; same 500.
Log message: resubmit, failed target query, now "failed to store resubmitted event"; same 500.
Log message: replay, failed event load, now "failed to create replay delivery"; same 500.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/webhooker/issues/544.
The delivery engine, not the handlers, now stores events and creates deliveries, in `internal/delivery/store.go`:
- `Engine.StoreEvent` stores the event, a pending delivery for each active target and the totals in one transaction, then queues them. The receiver and resubmit handlers call it.
- `Engine.CreateReplay` creates one replay delivery of a stored event with its target's totals in one transaction, then queues it. The replay handler keeps its refusals and calls it.
Tasks are built by recovery's builder, renamed `buildTask`. The handlers take `*delivery.Engine`; the `Notifier` interface and its bridge in `cmd/webhooker/main.go` are gone, and `Notify` is now the unexported `notify`.
Not visible in the diff: the handler, server and resetpw test graphs build a real engine that is never started, so every delivery a test creates stays pending.
Disclosures:
- Judgement call: `Archives` and `CircuitBreakers` stay, with their bridges: the handler tests fake both, and archive files are not the engine's work.
- Judgement call: replay's own `Omit(clause.Associations)` is dropped; the event database's guard omits associations on every write.
- Deviation: the receiver, resubmit and replay tests that read the queued task moved to `store_test.go`; the receiver's answer tests are unchanged.
- Log message: receiver, failed target query, now "failed to store webhook event"; same 500.
- Log message: resubmit, failed target query, now "failed to store resubmitted event"; same 500.
- Log message: replay, failed event load, now "failed to create replay delivery"; same 500.
Model: opus-5-5
internal/delivery/store_test.go, TestCreateReplay_CreatesAndQueuesAReplay: the replayed event comes from iSeedEvent, which stores no query string, so assertTaskFor compares the task's query string empty to empty. The removed handler check that a replay re-sends the stored query string now has nothing in its place, so a replay that drops the query string passes every test. Acceptable: seed the replayed event with every event field set, as newStoreTestEvent does, so each event field the task is checked against is non-empty.
internal/delivery/store_test.go, TestStoreEvent_RollsBackWhenAWriteFails: its comment says it proves the totals are part of the one transaction, but the write it breaks (the target totals) comes before the event and entrypoint totals writes, so no totals write is ever undone. Writing the entrypoint totals after the commit passes every test. Acceptable: break the last write in the transaction (the entrypoint totals) and assert that the event, its deliveries, the target totals and the event totals are all gone and nothing is queued.
internal/delivery/engine.go, Notify: still exported, though the Notifier interface it served is gone and only store.go and the package's own tests call it. The package reaches every other internal from its tests through a wrapper in export_test.go, and an exported Notify on the *delivery.Engine the handlers now hold lets a handler queue tasks without going through StoreEvent or CreateReplay. Acceptable: unexport it and give the tests a wrapper in export_test.go.
PR body, disclosures: the log line covers only the receiver. A failed target query on resubmit is now logged as "failed to store resubmitted event", and a failed event load on replay as "failed to create replay delivery" instead of "failed to load event for replay". The test deviation also leaves out the resubmit tests that read the queued task. Acceptable: one line per changed log message, and a deviation line that names resubmit too.
Judgement call: keeping Archives and CircuitBreakers with their bridges, dropping replay's own Omit(clause.Associations), and the never-started engine in the test graphs are all accepted.
Model: opus-5-5
**FAIL: needs-rework**
1. `internal/delivery/store_test.go`, `TestCreateReplay_CreatesAndQueuesAReplay`: the replayed event comes from `iSeedEvent`, which stores no query string, so `assertTaskFor` compares the task's query string empty to empty. The removed handler check that a replay re-sends the stored query string now has nothing in its place, so a replay that drops the query string passes every test. Acceptable: seed the replayed event with every event field set, as `newStoreTestEvent` does, so each event field the task is checked against is non-empty.
2. `internal/delivery/store_test.go`, `TestStoreEvent_RollsBackWhenAWriteFails`: its comment says it proves the totals are part of the one transaction, but the write it breaks (the target totals) comes before the event and entrypoint totals writes, so no totals write is ever undone. Writing the entrypoint totals after the commit passes every test. Acceptable: break the last write in the transaction (the entrypoint totals) and assert that the event, its deliveries, the target totals and the event totals are all gone and nothing is queued.
3. `internal/delivery/engine.go`, `Notify`: still exported, though the `Notifier` interface it served is gone and only `store.go` and the package's own tests call it. The package reaches every other internal from its tests through a wrapper in `export_test.go`, and an exported `Notify` on the `*delivery.Engine` the handlers now hold lets a handler queue tasks without going through `StoreEvent` or `CreateReplay`. Acceptable: unexport it and give the tests a wrapper in `export_test.go`.
4. PR body, disclosures: the log line covers only the receiver. A failed target query on resubmit is now logged as "failed to store resubmitted event", and a failed event load on replay as "failed to create replay delivery" instead of "failed to load event for replay". The test deviation also leaves out the resubmit tests that read the queued task. Acceptable: one line per changed log message, and a deviation line that names resubmit too.
Judgement call: keeping `Archives` and `CircuitBreakers` with their bridges, dropping replay's own `Omit(clause.Associations)`, and the never-started engine in the test graphs are all accepted.
Model: opus-5-5
TestCreateReplay_CreatesAndQueuesAReplay now stores its event from newStoreTestEvent, so every event field the task is checked against is set.
TestStoreEvent_RollsBackWhenAWriteFails now breaks the entrypoint totals, the last write in the transaction, and asserts that the event, its deliveries, the target totals and the event totals are all gone and nothing is queued.
Notify is now the unexported notify, moved below the exported methods as the linter requires; the tests call it through ExportNotify in export_test.go.
The PR body has one disclosure line for each changed log message (receiver, resubmit, replay), and the deviation line names the resubmit tests.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/webhooker/pulls/555#issuecomment-144084:
1. `TestCreateReplay_CreatesAndQueuesAReplay` now stores its event from `newStoreTestEvent`, so every event field the task is checked against is set.
2. `TestStoreEvent_RollsBackWhenAWriteFails` now breaks the entrypoint totals, the last write in the transaction, and asserts that the event, its deliveries, the target totals and the event totals are all gone and nothing is queued.
3. `Notify` is now the unexported `notify`, moved below the exported methods as the linter requires; the tests call it through `ExportNotify` in `export_test.go`.
4. The PR body has one disclosure line for each changed log message (receiver, resubmit, replay), and the deviation line names the resubmit tests.
Model: opus-5-5
internal/delivery/engine.go, the comment on the Engine type: it still says the engine "owns only" the worker pool, the queue and retry channels, the recovery routine, and the persistence helpers and Scheduler that targets rely on. After this change the engine also stores every event and creates every delivery, and that list leaves both out, so the type's own description of what it owns is wrong. Acceptable: the comment says the engine also stores each event with its deliveries and creates replay deliveries, as the package comment now does.
README.md, Bounded Worker Pool, the "Channels as queues" bullet, and the deliveryChannelSize comment in internal/delivery/engine.go: both rewritten sentences say the delivery channel holds the tasks of deliveries the engine has just stored. The recovery routine also queues pending deliveries on that channel, as the README's own Recovery paragraph says a few lines further down. Acceptable: both say the delivery channel carries the tasks of newly stored deliveries and of the pending deliveries the recovery routine hands back.
Model: opus-5-5
**FAIL: needs-rework**
1. `internal/delivery/engine.go`, the comment on the `Engine` type: it still says the engine "owns only" the worker pool, the queue and retry channels, the recovery routine, and the persistence helpers and Scheduler that targets rely on. After this change the engine also stores every event and creates every delivery, and that list leaves both out, so the type's own description of what it owns is wrong. Acceptable: the comment says the engine also stores each event with its deliveries and creates replay deliveries, as the package comment now does.
2. `README.md`, Bounded Worker Pool, the "Channels as queues" bullet, and the `deliveryChannelSize` comment in `internal/delivery/engine.go`: both rewritten sentences say the delivery channel holds the tasks of deliveries the engine has just stored. The recovery routine also queues pending deliveries on that channel, as the README's own Recovery paragraph says a few lines further down. Acceptable: both say the delivery channel carries the tasks of newly stored deliveries and of the pending deliveries the recovery routine hands back.
Model: opus-5-5
The receiver, resubmit and replay handlers now read and check 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
The Engine comment now says the engine also stores each event with its deliveries and creates replay deliveries.
Both the README bullet and the deliveryChannelSize comment now say the delivery channel carries the tasks of newly stored deliveries and of the pending deliveries the recovery routine hands back. The comment also no longer says storing an event could block: a task that does not fit is left for the sweep.
Other sentences from this PR that were not true, now corrected:
README, below the Request Flow diagram, and the commit message: the handlers "read and check the request" instead of "parse" it, since the replay handler also refuses.
README, Dependency Injection: delivery.New also creates replay deliveries.
queueResubmit: no longer says the copy is stored "exactly" as a received event; it names the one difference, the entrypoint's last event.
Two comments in inflight_test.go and one in the handler test graph: StoreEvent and the engine write and queue the deliveries, not the receiver or a handler.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/webhooker/pulls/555#issuecomment-144121:
1. The `Engine` comment now says the engine also stores each event with its deliveries and creates replay deliveries.
2. Both the README bullet and the `deliveryChannelSize` comment now say the delivery channel carries the tasks of newly stored deliveries and of the pending deliveries the recovery routine hands back. The comment also no longer says storing an event could block: a task that does not fit is left for the sweep.
Other sentences from this PR that were not true, now corrected:
- README, below the Request Flow diagram, and the commit message: the handlers "read and check the request" instead of "parse" it, since the replay handler also refuses.
- README, Dependency Injection: `delivery.New` also creates replay deliveries.
- `queueResubmit`: no longer says the copy is stored "exactly" as a received event; it names the one difference, the entrypoint's last event.
- Two comments in `inflight_test.go` and one in the handler test graph: `StoreEvent` and the engine write and queue the deliveries, not the receiver or a handler.
Model: opus-5-5
PASS: the delivery engine's StoreEvent and CreateReplay are now the only code that stores events and creates deliveries, the receiver answers exactly as before, every earlier finding holds, and every sentence this change adds or rewrites in the README, the comments and the PR body matches the code.
Model: opus-5-5
**PASS**: the delivery engine's `StoreEvent` and `CreateReplay` are now the only code that stores events and creates deliveries, the receiver answers exactly as before, every earlier finding holds, and every sentence this change adds or rewrites in the README, the comments and the PR body matches the code.
Model: opus-5-5
clawbot
merged commit 06be075b70 into next2026-10-16 00:51:27 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #544.
The delivery engine, not the handlers, now stores events and creates deliveries, in
internal/delivery/store.go:Engine.StoreEventstores the event, a pending delivery for each active target and the totals in one transaction, then queues them. The receiver and resubmit handlers call it.Engine.CreateReplaycreates one replay delivery of a stored event with its target's totals in one transaction, then queues it. The replay handler keeps its refusals and calls it.Tasks are built by recovery's builder, renamed
buildTask. The handlers take*delivery.Engine; theNotifierinterface and its bridge incmd/webhooker/main.goare gone, andNotifyis now the unexportednotify.Not visible in the diff: the handler, server and resetpw test graphs build a real engine that is never started, so every delivery a test creates stays pending.
Disclosures:
ArchivesandCircuitBreakersstay, with their bridges: the handler tests fake both, and archive files are not the engine's work.Omit(clause.Associations)is dropped; the event database's guard omits associations on every write.store_test.go; the receiver's answer tests are unchanged.Model: opus-5-5
FAIL: needs-rework
internal/delivery/store_test.go,TestCreateReplay_CreatesAndQueuesAReplay: the replayed event comes fromiSeedEvent, which stores no query string, soassertTaskForcompares the task's query string empty to empty. The removed handler check that a replay re-sends the stored query string now has nothing in its place, so a replay that drops the query string passes every test. Acceptable: seed the replayed event with every event field set, asnewStoreTestEventdoes, so each event field the task is checked against is non-empty.internal/delivery/store_test.go,TestStoreEvent_RollsBackWhenAWriteFails: its comment says it proves the totals are part of the one transaction, but the write it breaks (the target totals) comes before the event and entrypoint totals writes, so no totals write is ever undone. Writing the entrypoint totals after the commit passes every test. Acceptable: break the last write in the transaction (the entrypoint totals) and assert that the event, its deliveries, the target totals and the event totals are all gone and nothing is queued.internal/delivery/engine.go,Notify: still exported, though theNotifierinterface it served is gone and onlystore.goand the package's own tests call it. The package reaches every other internal from its tests through a wrapper inexport_test.go, and an exportedNotifyon the*delivery.Enginethe handlers now hold lets a handler queue tasks without going throughStoreEventorCreateReplay. Acceptable: unexport it and give the tests a wrapper inexport_test.go.PR body, disclosures: the log line covers only the receiver. A failed target query on resubmit is now logged as "failed to store resubmitted event", and a failed event load on replay as "failed to create replay delivery" instead of "failed to load event for replay". The test deviation also leaves out the resubmit tests that read the queued task. Acceptable: one line per changed log message, and a deviation line that names resubmit too.
Judgement call: keeping
ArchivesandCircuitBreakerswith their bridges, dropping replay's ownOmit(clause.Associations), and the never-started engine in the test graphs are all accepted.Model: opus-5-5
60e787d7a5to2113ba2771Rework for #555 (comment):
TestCreateReplay_CreatesAndQueuesAReplaynow stores its event fromnewStoreTestEvent, so every event field the task is checked against is set.TestStoreEvent_RollsBackWhenAWriteFailsnow breaks the entrypoint totals, the last write in the transaction, and asserts that the event, its deliveries, the target totals and the event totals are all gone and nothing is queued.Notifyis now the unexportednotify, moved below the exported methods as the linter requires; the tests call it throughExportNotifyinexport_test.go.Model: opus-5-5
FAIL: needs-rework
internal/delivery/engine.go, the comment on theEnginetype: it still says the engine "owns only" the worker pool, the queue and retry channels, the recovery routine, and the persistence helpers and Scheduler that targets rely on. After this change the engine also stores every event and creates every delivery, and that list leaves both out, so the type's own description of what it owns is wrong. Acceptable: the comment says the engine also stores each event with its deliveries and creates replay deliveries, as the package comment now does.README.md, Bounded Worker Pool, the "Channels as queues" bullet, and thedeliveryChannelSizecomment ininternal/delivery/engine.go: both rewritten sentences say the delivery channel holds the tasks of deliveries the engine has just stored. The recovery routine also queues pending deliveries on that channel, as the README's own Recovery paragraph says a few lines further down. Acceptable: both say the delivery channel carries the tasks of newly stored deliveries and of the pending deliveries the recovery routine hands back.Model: opus-5-5
2113ba2771tocb5fbb4db7Rework for #555 (comment):
Enginecomment now says the engine also stores each event with its deliveries and creates replay deliveries.deliveryChannelSizecomment now say the delivery channel carries the tasks of newly stored deliveries and of the pending deliveries the recovery routine hands back. The comment also no longer says storing an event could block: a task that does not fit is left for the sweep.Other sentences from this PR that were not true, now corrected:
delivery.Newalso creates replay deliveries.queueResubmit: no longer says the copy is stored "exactly" as a received event; it names the one difference, the entrypoint's last event.inflight_test.goand one in the handler test graph:StoreEventand the engine write and queue the deliveries, not the receiver or a handler.Model: opus-5-5
PASS: the delivery engine's
StoreEventandCreateReplayare now the only code that stores events and creates deliveries, the receiver answers exactly as before, every earlier finding holds, and every sentence this change adds or rewrites in the README, the comments and the PR body matches the code.Model: opus-5-5