Merge branch 'main' into preetam/rebase

Author: behinddwalls

doc/rfc/runway/workflow.md

@@ -61,6 +61,20 @@ The merge-conflict-check controller always publishes a result — even when all
 
 The merge controller publishes a conflict result (and acks) when the merge detects a conflict; SubmitQueue handles rebatching. On infrastructure error it nacks for retry. On success it publishes per-step outcomes (output IDs of the revisions produced) so SubmitQueue can update its request state.
 
+## Terminal failures and dead-lettering
+
+Runway is stateless and the sole responder on the client's correlation id: SubmitQueue records the in-flight work before publishing and waits for exactly one `MergeResult` echoing that id. Every request must therefore resolve to a result — success or failure — or the client waits forever.
+
+Failures split into two classes:
+
+- **Named terminal outcomes** — a merge conflict or an invalid request (an unknown/unsupported strategy, a malformed change URI, or an invalid `PROMOTE` composition). These can never succeed on retry, so the controller publishes a `FAILED` `MergeResult` (with a reason) and acks, rather than nacking. The merger surfaces them as the `ErrConflict` / `ErrInvalidRequest` sentinels; `IsTerminal` is the single classification point.
+
+- **Infrastructure faults** — fetch/network/auth failures, a push rejected for a reason other than a moved tip, and so on. These are nacked for retry.
+
+An infrastructure fault that never recovers would exhaust retries and dead-letter. Because nothing consumed those dead-letter topics, such a request produced no signal and left the client's correlation id unresolved. Runway closes that gap with a **DLQ reconciler**: a dedicated consumer subscribes to the inbound topics' `_dlq` queues and, for each dead-lettered request, republishes a `FAILED` `MergeResult` (echoing the correlation id) to the corresponding signal topic. Unlike the SubmitQueue/Stovepipe DLQ reconcilers it writes no entity state — the signal is the resolution. It runs under an always-retryable error policy so a transient publish failure retries indefinitely rather than dead-lettering again. A payload that cannot even be decoded carries no correlation id and is dropped.
+
+Together these guarantee the client's correlation id always resolves: the primary controllers handle what they can name, and the reconciler is the backstop for everything else.
+
 ## Idempotency
 
 Runway has no persistent state — no request store, no job store, no database. Idempotency is achieved through the VCS contract: merge detects already-pushed changes (revisions reachable from HEAD) and treats them as already-landed. Merge-conflict check is read-only and naturally idempotent.

doc/rfc/stovepipe/steps/build.md

@@ -10,7 +10,7 @@ It handles only the trigger: it does not poll for completion, record greenness,
 
 `build` consumes a request id, published by `process` in Phase 1 (see [workflow.md](doc/rfc/stovepipe/workflow.md#workflow)). Both phases drive the same `build` → `buildsignal` machinery against the same `Request` row. The `process`/`analyze` → `build` topic is partitioned by **request id**; see [Partitioning](#partitioning) for the full rationale, including why `build` → `buildsignal` partitions finer (by build id) than SubmitQueue's equivalent topic.
 
-**`build` is not the sole writer of the rows it touches, and its own writes are narrow.** On `Request`, `build` never writes at all — it only reads the `BuildStrategy`/`BaseURI`/`URI` fields `process` set at admit, and it leaves `Request.State` untouched at `processing` throughout (`process` and `record` are `Request.State`'s only writers). On `Build`, `build` is the sole creator — it calls `BuildStore.Create` exactly once, at step 6 — and never mutates the row again; `buildsignal` is the sole writer of `Build.Status`/`Build.Version` afterward (see [buildsignal.md](doc/rfc/stovepipe/steps/buildsignal.md#input-and-re-entrancy)). This division is why `build` never needs a CAS/version write of its own: `Create` is the only storage mutation in its algorithm.
+**`build` is not the sole writer of the rows it touches, and its own writes are narrow.** On `Request`, `build` never writes at all — it only reads the `BuildStrategy`/`BaseURI`/`URI` fields `process` set at admit, and it leaves `Request.State` untouched at `processing` throughout (`process`, `buildsignal`, and the DLQ reconciler are `Request.State`'s only writers). On `Build`, `build` is the sole creator — it calls `BuildStore.Create` exactly once, at step 6 — and never mutates the row again; `buildsignal` is the sole writer of `Build.Status`/`Build.Version` afterward (see [buildsignal.md](doc/rfc/stovepipe/steps/buildsignal.md#input-and-re-entrancy)). This division is why `build` never needs a CAS/version write of its own: `Create` is the only storage mutation in its algorithm.
 
 `build` is phase-agnostic: it never asks "which phase is this?" It reads whatever scope is already persisted and immutable on the `Request` and acts on it. Phase 2's project-scoped invocation is expected to read project-scoped equivalents of the scope fields off the same `Request`; the exact shape of a project-scoped trigger is left to the `analyze` design, consistent with `workflow.md`'s "project mapping contract" open question — see [Project-scoped `Trigger`: reserved, not yet designed](#project-scoped-trigger-reserved-not-yet-designed) for the resulting gap in the `BuildRunner` contract itself.
 
@@ -25,9 +25,9 @@ For a delivery carrying request id `R`:
    - ErrNotFound       -> return raw; non-retryable (storage is read-after-write consistent; see [storage README](stovepipe/extension/storage/README.md)).
    - other store error -> return raw; classifier decides.
 
-2. If R.State is terminal (superseded / recorded-green / recorded-not-green): ack and return.
-   - a redelivery after record already finished, or after process superseded R, must not start a
-     fresh build.
+2. If R.State is terminal (superseded / succeeded / failed / cancelled): ack and return.
+   - a redelivery after the build outcome was already recorded, or after process superseded R,
+     must not start a fresh build.
 
 3. Resolve the build-runner for R's Queue: buildRunner = Factory.For(Config{QueueName: R.Queue}).
    - lookup failure -> non-retryable (a queue with no builder is a config error).
@@ -67,9 +67,9 @@ For a delivery carrying request id `R`:
 8. ack.
 ```
 
-`Build.ID` is **minted by the runner at `Trigger`**, exactly as in SubmitQueue's build controller: the runner returns its native id (a Buildkite build number, a CI-gateway job id), `build` adopts it as the `Build`'s key, and that same id travels on every downstream hop — `build` → `buildsignal` → `record` each carry the build id in the message, so every reader reaches the `Build` by a direct get on identity it was handed. No reader ever *derives* a build id or needs a reverse index; `Build.RequestID` covers the one navigation the pipeline needs in the other direction (`Build` → `Request`). Another approach is deriving the key from the Request (`buildKey(R)`) and/or passed a caller-supplied idempotency key to `Trigger`; see [Alternatives considered](#alternatives-considered-for-the-build-identity) for what each would buy and cost.
+`Build.ID` is **minted by the runner at `Trigger`**, exactly as in SubmitQueue's build controller: the runner returns its native id (a Buildkite build number, a CI-gateway job id), `build` adopts it as the `Build`'s key, and that same id travels on every hop that needs a build — `build` → `buildsignal` carries the build id in the message, so the poll loop reaches the `Build` by a direct get on identity it was handed. `buildsignal` → `record` carries the **request id** instead: `record`'s unit of work is a Request, and the build's terminal status is projected onto `Request.State` before the publish, so `record` never reaches a `Build` at all. No reader ever *derives* a build id or needs a reverse index; `Build.RequestID` covers the one navigation the pipeline needs in the other direction (`Build` → `Request`). Another approach is deriving the key from the Request (`buildKey(R)`) and/or passing a caller-supplied idempotency key to `Trigger`; see [Alternatives considered](#alternatives-considered-for-the-build-identity) for what each would buy and cost.
 
-`build` writes only the `Build`, and only at creation; it never mutates `Request.State`. The Request stays `processing` (set by `process`) through `build` and `buildsignal` until `record` moves it terminal. `Build.Status` is the fine-grained build lifecycle; `Request.State` is the coarse pipeline lifecycle. This is also what keeps `process.md` step 3 correct: because `build` leaves the Request at `processing`, a redelivered `process` message still matches its "if processing, re-publish to build" guard.
+`build` writes only the `Build`, and only at creation; it never mutates `Request.State`. The Request stays `processing` (set by `process`) through `build` until `buildsignal` moves it terminal by recording the build's outcome. `Build.Status` is the fine-grained build lifecycle; `Request.State` is the coarse pipeline lifecycle. This is also what keeps `process.md` step 3 correct: because `build` leaves the Request at `processing`, a redelivered `process` message still matches its "if processing, re-publish to build" guard.
 
 ## Idempotency
 
@@ -88,7 +88,7 @@ Every branch is safe under at-least-once redelivery — with SubmitQueue's postu
 
 ## Fail-closed interaction
 
-A build that never reaches step 8 — `Trigger` failing repeatedly, the publish to `buildsignal` never landing, `BuildStore.Create` down — must not wedge its `Request`'s Queue slot forever: `process`'s per-Queue concurrency gate holds `in_flight_count` open until the Request reaches a terminal state (see [process.md](doc/rfc/stovepipe/steps/process.md#concurrency-lifecycle)). `build` does not implement the forcing function itself. Per [workflow.md](doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work), every non-retryable failure in the algorithm rejects to DLQ (see [Error classification](#error-classification)), and a Request stuck past `MaxAttempts` is driven to a conservative terminal not-green by the DLQ reconciler, which decrements `in_flight_count` and frees the slot. This is the same posture `buildsignal` relies on for its own poll loop (see [buildsignal.md](doc/rfc/stovepipe/steps/buildsignal.md#fail-closed-interaction)) — `build` and `buildsignal` are two links in the same fail-closed chain that keeps one bad Request from wedging its Queue.
+A build that never reaches step 8 — `Trigger` failing repeatedly, the publish to `buildsignal` never landing, `BuildStore.Create` down — must not wedge its `Request`'s Queue slot forever: `process`'s per-Queue concurrency gate holds `in_flight_count` open until the Request reaches a terminal state (see [process.md](doc/rfc/stovepipe/steps/process.md#concurrency-lifecycle)). `build` does not implement the forcing function itself. Per [workflow.md](doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work), every non-retryable failure in the algorithm rejects to DLQ (see [Error classification](#error-classification)), and a Request stuck past `MaxAttempts` is driven to a conservative terminal `failed` by the DLQ reconciler, which decrements `in_flight_count` and frees the slot. This is the same posture `buildsignal` relies on for its own poll loop (see [buildsignal.md](doc/rfc/stovepipe/steps/buildsignal.md#fail-closed-interaction)) — `build` and `buildsignal` are two links in the same fail-closed chain that keeps one bad Request from wedging its Queue.
 
 One boundary is worth stating explicitly: this path fires only when `build` (or a downstream stage) *errors*. A `Trigger` call that returns successfully but the backend never actually runs — or a `Build` row created for a build the runner silently drops — has no protocol-level failure to escalate at the `build` stage; nothing here retries or dead-letters, because nothing failed. That gap surfaces one hop later, when `buildsignal` polls: either the runner reports an error (handled by `buildsignal`'s own classification) or it reports a non-terminal status forever, which is `buildsignal`'s fail-closed boundary to close, not `build`'s (see [buildsignal.md](doc/rfc/stovepipe/steps/buildsignal.md#fail-closed-interaction)). `build`'s liveness responsibility ends at a successful publish to `buildsignal`.
 
@@ -98,7 +98,7 @@ One boundary is worth stating explicitly: this path fires only when `build` (or
 
 ## Error classification
 
-Per `platform/errs`'s non-retryable-by-default rule (see [platform/errs/README.md](platform/errs/README.md)), a plain returned error is already non-retryable and rejects straight to DLQ, where the fail-closed path forces a conservative not-green so a Request never wedges its Queue's slot ([workflow.md](doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work)). So this section documents only the departures from that default, not every failure the algorithm can hit:
+Per `platform/errs`'s non-retryable-by-default rule (see [platform/errs/README.md](platform/errs/README.md)), a plain returned error is already non-retryable and rejects straight to DLQ, where the fail-closed path forces a conservative terminal `failed` so a Request never wedges its Queue's slot ([workflow.md](doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work)). So this section documents only the departures from that default, not every failure the algorithm can hit:
 
 | Failure | Disposition | Why |
 |---|---|---|
@@ -263,7 +263,9 @@ Key the `Build` by identity derived from the Request — `buildKey(R) = R.ID` fo
 | `Request` → `Build` navigation with no reverse index, per the KV key-derivation rule in CLAUDE.md | No current reader needs to *derive* a build id — the id travels in every message hop, so each consumer already holds the key it needs |
 | Enforces (rather than assumes) the direct-navigation property SubmitQueue's speculate takes on faith | Diverges entity shape and controller flow from SubmitQueue, weakening the "structurally the same controller" claim and dual-implementing-backend symmetry |
 
-Trade-offs: the dedup guards a rare event at a permanent modeling cost. The duplicate it prevents arises only from a redelivery inside the trigger window — rare, and already harmless (identical scope; `buildsignal`'s terminal-Request short-circuit and `record`'s CAS make the loser a no-op — see [Idempotency](#idempotency)). The prospective key-derivers — a future canceller, or `analyze` reaching back to the Phase-1 target graph — would need to be handed the id by their producing stage instead, if those designs land.
+Trade-offs: the dedup guards a rare event at a permanent modeling cost. The duplicate it prevents arises only from a redelivery inside the trigger window — rare, and already harmless (identical scope; `buildsignal`'s superseded short-circuit and its first-writer-wins outcome CAS make the loser a no-op — see [Idempotency](#idempotency)). The prospective key-derivers — a future canceller, or `analyze` reaching back to the Phase-1 target graph — would need to be handed the id by their producing stage instead, if those designs land.
+
+Note that moving `record`'s input from the build id to the request id does *not* trigger this alternative, even though it removes the last hop that carried a build id to a Request-scoped consumer. The trigger condition is a stage that must **derive a build's key from a Request**, and `record` does not: the build's terminal status is projected onto `Request.State` before the publish, so `record` reads the Request and never reaches a `Build`.
 
 #### Alternative B: caller-supplied idempotency key on `Trigger`
 
@@ -312,7 +314,7 @@ Either could be adopted independently: the idempotency token, if a backend that
 
 `IsTerminal()` on `entity.BuildStatus` covers exactly the three terminal rows. Once `buildsignal` persists one of them, that status is **write-once** — a later poll reporting a different terminal value never overwrites it (see [buildsignal.md](doc/rfc/stovepipe/steps/buildsignal.md#algorithm), step 6).
 
-Plus the `BuildID{ID string}` wire type in `stovepipe/entity` (same "id only travels" convention as `RequestID`, shaped like SubmitQueue's own `entity.BuildID` but not the same Go type — see the [contract sketch](#stovepipe-buildrunner-contract-design-sketch)), wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, the queue payload, `Status`/`Cancel`'s parameter. `buildsignal` and `record` reach a build by the id carried in their messages, so no reverse index from `Request` to its builds is ever needed.
+Plus the `BuildID{ID string}` wire type in `stovepipe/entity` (same "id only travels" convention as `RequestID`, shaped like SubmitQueue's own `entity.BuildID` but not the same Go type — see the [contract sketch](#stovepipe-buildrunner-contract-design-sketch)), wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, the queue payload, `Status`/`Cancel`'s parameter. `buildsignal` reaches a build by the id carried in its message, and `record` reads the `Request` (whose state carries the build's outcome) rather than a `Build`, so no reverse index from `Request` to its builds is ever needed.
 
 **`BuildStore`** (new, added to the `Storage` aggregator via `GetBuildStore()`), matching stovepipe's existing `RequestStore` conventions — **generic `Update` with caller-owned version arithmetic, not SubmitQueue's field-specific `UpdateStatus`**: