From 3075506cdeeeba784916ec0b117eba3d92731749 Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Wed, 12 Aug 2026 15:46:15 -0700 Subject: [PATCH] docs(agents): require persisting state before publishing handoffs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary ### Why? A queue message that hands work to a later stage races the write that backs it. Within a service boundary the payload is only the entity ID and the consumer reloads from storage, so a message published before its write lands lets the consumer read an entity that was never recorded — or a version older than the one the message describes — and then act on an assumption that was never true. The other ordering is self-correcting: a failed publish leaves the state durable and the retry re-publishes, where the reverse leaves a message describing a state nothing wrote. The repo already depends on this ordering but never wrote it down. Two sites deliberately publish *first* — the recovery nudge in `speculate/finalize.go` and the status log in `buildsignal.go` — each with a local comment explaining why. Stated as a blanket rule, it would read as something those two sites violate, and the next reader would "fix" them. ### What? Adds "Persist before you publish" as Key Concept #6, scoped to messages that hand off work. It carves out messages whose consumer depends on nothing the write does — a status log recording a transition, an idempotent nudge whose consumer re-derives from current state — and points at the two sites that document that reasoning in place. Cross-references the rule from the queue-payload guidance, since reloading from storage is what makes the ordering load-bearing there. --- AGENTS.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index f3e8df3e..bfede3c5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,6 +11,7 @@ SubmitQueue is a distributed system for managing code submission workflows. It f 3. **Event sourcing** — store events (what happened) rather than just current state for critical changes. 4. **Optimistic locking** — use version numbers instead of pessimistic locks. Avoid transactions; prefer optimistic concurrency and retries. **Version arithmetic lives in the controller, not the storage layer.** Update methods take both `oldVersion` (the where-clause guard) and `newVersion` (the value to write); the store performs a pure conditional write. Controllers compute `newVersion = oldVersion + 1`, call the store, and only assign `entity.Version = newVersion` after the call succeeds. Pre-incrementing in memory before the call is a bug pattern — on error the in-memory version drifts ahead of the database. See [submitqueue/extension/storage/README.md](submitqueue/extension/storage/README.md). 5. **Idempotency keys** — include unique request IDs, check for duplicates before executing. +6. **Persist before you publish** — when a message hands work to a later stage, write the state first and publish only once the write has succeeded. Publishing first races the consumer against the write: it can load an entity that was never recorded, or a version older than the one the message describes, and then build on an assumption that was never true. Ordered write-then-publish, a failed publish leaves the state durable and the retry re-publishes; the reverse leaves a message describing a state nothing wrote. The exception is a message that depends on nothing the write does — a status log recording a transition, or an idempotent nudge whose consumer re-derives everything from current state. Those publish first on purpose, so that a failure leaves nothing written and the retry redoes the whole step rather than stranding a durable state change with no way to announce it; see `submitqueue/orchestrator/controller/speculate/finalize.go` and `.../buildsignal/buildsignal.go` for the reasoning at those two sites. ```go // Immutable entity pattern @@ -109,7 +110,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er Controllers receive `consumer.Delivery` (subset interface without Ack/Nack) to enforce separation of business logic from infrastructure. -**Queue payloads: IDs within a boundary, full payloads across one.** When producer and consumer share a store (same service — e.g. `build`→`buildsignal`, `validate`→`mergeconflict`), put only the entity **ID** on the queue and reload from storage (the store is the source of truth, messages stay small, redelivery is idempotent). When a queue **crosses a service boundary** (the consumer cannot read the producer's store — e.g. orchestrator→runway), publish the **full payload** the consumer needs, and have the **client own the correlation ID** so it can match the async result back to the work it is tracking. The queue's **owner defines the wire contract and topic keys** (in its own domain package); the other side imports them. +**Queue payloads: IDs within a boundary, full payloads across one.** When producer and consumer share a store (same service — e.g. `build`→`buildsignal`, `validate`→`mergeconflict`), put only the entity **ID** on the queue and reload from storage (the store is the source of truth, messages stay small, redelivery is idempotent). Reloading from storage is what makes the publish ordering load-bearing — see "Persist before you publish" above. When a queue **crosses a service boundary** (the consumer cannot read the producer's store — e.g. orchestrator→runway), publish the **full payload** the consumer needs, and have the **client own the correlation ID** so it can match the async result back to the work it is tracking. The queue's **owner defines the wire contract and topic keys** (in its own domain package); the other side imports them. ### Entities