From 3ba63f9ea4c7a88386d05a78fc2d9a67a9c50dd3 Mon Sep 17 00:00:00 2001 From: mnoah1 Date: Thu, 16 Jul 2026 17:50:16 +0000 Subject: [PATCH] Record step design --- doc/rfc/index.md | 3 + doc/rfc/stovepipe/steps/build.md | 38 ++-- doc/rfc/stovepipe/steps/record.md | 302 ++++++++++++++++++++++++++++++ doc/rfc/stovepipe/workflow.md | 15 +- 4 files changed, 335 insertions(+), 23 deletions(-) create mode 100644 doc/rfc/stovepipe/steps/record.md diff --git a/doc/rfc/index.md b/doc/rfc/index.md index b7cd7db5..aa4f4131 100644 --- a/doc/rfc/index.md +++ b/doc/rfc/index.md @@ -26,6 +26,9 @@ Design documents and technical proposals, grouped by scope. Shared/cross-cutting - [Stovepipe Workflow](stovepipe/workflow.md) - Post-merge validation pipeline overview: ingest, process, build, record greenness, analyze projects, notify downstream - [Process stage](stovepipe/steps/process.md) - Build-strategy decision, per-queue concurrency gate, backlog coalescing, entity model, platform prerequisites +- [Build stage](stovepipe/steps/build.md) - Trigger-only stage and Stovepipe's URI-based BuildRunner contract +- [Buildsignal stage](stovepipe/steps/buildsignal.md) - Build polling, terminal status persistence, and the handoff to record +- [Record stage](stovepipe/steps/record.md) - Immutable validation facts keyed by `(queue, uri, project)`, monotonic last-green bookmark advancement and ref promotion, and the deferred hook-event and analyze handoffs ## Runway diff --git a/doc/rfc/stovepipe/steps/build.md b/doc/rfc/stovepipe/steps/build.md index 54e56905..0078ecd2 100644 --- a/doc/rfc/stovepipe/steps/build.md +++ b/doc/rfc/stovepipe/steps/build.md @@ -39,7 +39,7 @@ For a delivery carrying request id `R`: - baseURI = R.BaseURI if R.BuildStrategy == incremental_since_green, else "" (full build). - (headURI = R.URI, baseURI) identify the scope; both are opaque SourceControl tokens. -5. Trigger: buildID, err := buildRunner.Trigger(ctx, R.URI, baseURI, metadata) +5. Trigger: buildID, err := buildRunner.Trigger(ctx, baseURI, R.URI, metadata) - Trigger takes no caller-supplied id; the runner mints the build's identity, and buildID becomes Build.ID — SubmitQueue's exact convention (see "Alternatives considered" under the contract sketch). @@ -52,8 +52,10 @@ For a delivery carrying request id `R`: either domain — the shape is deferred until then, not decided here. - failure -> return raw; classifier decides (transient runner blip retryable, bad URI not). -6. Persist Build{ID: buildID.ID, RequestID: R.ID, URI: R.URI, BaseURI: baseURI, - Status: accepted, Version: 1} via BuildStore.Create. +6. Persist Build{ID: buildID.ID, RequestID: R.ID, Status: accepted, Version: 1} + via BuildStore.Create. + - the row carries no scope; it is recoverable from the Request's immutable fields + (see the entity table). - a crash between step 5 and this write orphans the triggered build (see Idempotency). - ErrAlreadyExists -> benign (reachable only with a backend that returns deterministic ids for retried triggers); continue to step 7. @@ -78,8 +80,8 @@ Every branch is safe under at-least-once redelivery — with SubmitQueue's postu - **Request not found** — non-retryable; storage's read-after-write guarantee means a miss here is a storage defect, not a lag condition to retry through. - **Strategy not yet visible** — retryable; the producing stage's write is not visible on this reader yet. - **Request already terminal** (step 2) — ack, no build. A redelivery after `record` finished, or after `process` superseded the head, never starts a stale build. -- **Redelivery while the Request is still in flight** (crash or failure anywhere in steps 5–8) — the redelivery re-runs from step 1, `Trigger` mints a fresh id, `Create` persists a second `Build` row, and a second poll loop starts. Harmless, in three layers: both builds target the identical `(headURI, baseURI)` scope; each `Build` polls in its own partition and `buildsignal` short-circuits the moment the Request goes terminal (its step 3); and `record`'s terminal transition is CAS-guarded, so the second verdict is a no-op. A build triggered but never persisted (crash between steps 5 and 6) is the same story minus the row: an orphan the runner finishes and nobody ever reads. Wasted CI compute, not a correctness risk — the same accepted trade as SubmitQueue. -- **Trigger / publish / other store failure** — nothing durable is left half-written that a redelivery can't reconcile; the error rejects to DLQ, and the fail-closed reconciler drives the Request terminal (see [workflow.md](doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work)). +- **Redelivery while the Request is still in flight** (crash or failure anywhere in steps 5–8) — the redelivery re-runs from step 1, `Trigger` mints a fresh id, `Create` persists a second `Build` row, and a second poll loop starts. Harmless, in three layers: both builds target the identical `(headURI, baseURI)` scope; each `Build` polls in its own partition and `buildsignal` short-circuits the moment the Request goes terminal (its step 3); and `buildsignal`'s outcome write is first-writer-wins, so the second verdict cannot flip the Request's state or overwrite the create-only validation fact. A build triggered but never persisted (crash between steps 5 and 6) is the same story minus the row: an orphan the runner finishes and nobody ever reads. Wasted CI compute, not a correctness risk — the same accepted trade as SubmitQueue. +- **Trigger / publish / other store failure** — nothing durable is left half-written that a redelivery can't reconcile; the error rejects to DLQ, where the fail-closed posture is meant to drive the Request terminal (see [workflow.md](doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work)). No reconciler consumes `build_dlq` yet, so that last step does not happen today — see [Fail-closed interaction](#fail-closed-interaction). ## Edge cases @@ -90,6 +92,10 @@ Every branch is safe under at-least-once redelivery — with SubmitQueue's postu 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. +**That chain is not closed at `build` yet.** The `build` subscription enables dead-lettering, but no controller consumes `build_dlq` — the wiring registers only `process_dlq` and `buildsignal_dlq` — so nothing forces the Request terminal and nothing frees the slot. How much that costs depends on how far the delivery got. If a `Build` row was persisted and its signal published, a poll chain survives the dead-letter and `buildsignal` still releases the slot when the build goes terminal. If the message dead-letters before that — `Trigger` failing every attempt, `BuildStore.Create` down, the publish never landing — the Request stays `processing` and its Queue loses a slot for good, which is exactly the failure [buildsignal.md](doc/rfc/stovepipe/steps/buildsignal.md#fail-closed-interaction) describes for a deployment missing its own reconciler. + +Whoever wires that reconciler has to decide what it records, not just what it releases: forcing `failed` on a Request whose build may still be running is what produces the permanently-wrong-fact path in [record.md](record.md#what-fail-closed-actually-guarantees), so this gap and that open question belong to the same piece of work. + 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`. ## Cancellation: defined, not yet called @@ -128,7 +134,7 @@ The batches are **identity** — thin references carrying ids, not change conten Stovepipe validates **one commit** against a baseline (or in full). Its `build` controller reads two opaque URIs off the `Request` and triggers: ```go -buildID, err := buildRunner.Trigger(ctx, headURI, baseURI, metadata) +buildID, err := buildRunner.Trigger(ctx, baseURI, headURI, metadata) ``` There is no batch, no dependency list, and nothing to resolve — the URIs *are* the identity, owned by `SourceControl`. `process` already decided incremental-vs-full; `build` just reads `R.BuildStrategy`/`R.BaseURI` and acts. @@ -153,16 +159,16 @@ type BuildRunner interface { // Trigger starts a new build every call and mints the build's identity — // there is no caller-supplied dedup input, matching SubmitQueue's contract // exactly (see "Alternatives considered for the build identity" below - // for other shapes this doc considered). headURI is the commit - // under validation; baseURI is the incremental baseline (empty for a full - // build). metadata is caller annotations the runner may echo but must not + // for other shapes this doc considered). baseURI is the incremental + // baseline (empty for a full build); headURI is the commit under + // validation. metadata is caller annotations the runner may echo but must not // depend on — empty today, but expected to carry real data eventually (e.g. // conflict-graph info, or other upstream decisions relevant to the build) // once a concrete need lands in either domain; the shape is deferred until // then, not decided here. Runner-side work is async; callers learn progress // via Status. // Returns the runner-assigned build id, which the caller adopts as Build.ID. - Trigger(ctx context.Context, headURI, baseURI string, metadata entity.BuildMetadata) (entity.BuildID, error) + Trigger(ctx context.Context, baseURI, headURI string, metadata entity.BuildMetadata) (entity.BuildID, error) // Status returns the current status. Takes the id Trigger returned // (Build.ID). May round-trip to the backend. BuildMetadata is @@ -189,7 +195,7 @@ type Factory interface{ For(cfg Config) (BuildRunner, error) } The shape isn't decided here because project semantics belong to `analyze`, not `build`: how a project maps to a buildable scope (a Bazel target pattern, a directory, a service name) is implementer-specific per [workflow.md](doc/rfc/stovepipe/workflow.md#project---greenness-at-a-finer-grain). The expectation is that this stays an opaque token — following the same "identity in, resolve internally" shape already used for `headURI`/`baseURI` (owned and interpreted by `SourceControl`) — that `build` reads off the `Request`/message and hands to the runner uninterpreted, rather than a structured type `build` would have to understand: ```go -Trigger(ctx context.Context, headURI, baseURI string, projectScope entity.ProjectScope, metadata entity.BuildMetadata) (entity.BuildID, error) +Trigger(ctx context.Context, baseURI, headURI string, projectScope entity.ProjectScope, metadata entity.BuildMetadata) (entity.BuildID, error) ``` `ProjectScope` lives in `stovepipe/entity` alongside `BuildID`/`BuildStatus`/`BuildMetadata` — projects have no SubmitQueue equivalent at all, not even a shape to mirror. Its zero value covers Phase 1 (no project — whole-repo/incremental scope only, exactly today's sketch); `analyze` is what would populate a non-zero value for Phase 2. This mirrors the additive optional field already reserved on `BuildRequest` for the same purpose (see [Queue contract additions](#queue-contract-additions)) — the wire message and the extension contract need the same new dimension, and both are deferred to the same design. @@ -198,7 +204,7 @@ Both `Trigger` and `Status`/`Cancel` differ *in contract* between domains, even There is exactly one build id: the runner mints it at `Trigger`, `build` adopts it as `Build.ID`, and every later call and message carries it verbatim — `Status`/`Cancel` take the same value `Trigger` returned, the queue payload is the same value, the store key is the same value. This is SubmitQueue's convention end to end. The id is opaque: no stovepipe reader parses it, derives it, or equates it with another entity's id — the trap SubmitQueue's speculate/cancel path falls into. And per the extension rules a runner keeps only transient local state, so the durable `Request` ↔ `Build` linkage lives in **our** store as `Build.RequestID`, never in the runner. -Supporting entity types: `BuildStatus`, `BuildMetadata`, and `BuildID` live in `stovepipe/entity`, shaped the same as SubmitQueue's `submitqueue/entity` equivalents but defined and duplicated locally rather than shared — `BuildStatus` is the narrow lowercase enum `"" (unknown) / accepted / running / succeeded / failed / cancelled` with an `IsTerminal()` predicate covering the last three, `BuildMetadata` is the free-form `map[string]string`, and `BuildID` is a `{ID string}` wire struct wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, `Status`/`Cancel`'s parameter, the queue payload. `stovepipe/entity/build.go` keeps what's stovepipe-specific: the `Build` entity itself (`RequestID`/`URI`/`BaseURI` alongside `ID`/`Status`/`Version`). How a target graph reaches `analyze` is out of scope for this doc — left to the `analyze` design. +Supporting entity types: `BuildStatus`, `BuildMetadata`, and `BuildID` live in `stovepipe/entity`, shaped the same as SubmitQueue's `submitqueue/entity` equivalents but defined and duplicated locally rather than shared — `BuildStatus` is the narrow lowercase enum `"" (unknown) / accepted / running / succeeded / failed / cancelled` with an `IsTerminal()` predicate covering the last three, `BuildMetadata` is the free-form `map[string]string`, and `BuildID` is a `{ID string}` wire struct wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, `Status`/`Cancel`'s parameter, the queue payload. `stovepipe/entity/build.go` keeps what's stovepipe-specific: the `Build` entity itself (`RequestID` alongside `ID`/`Status`/`Version`). How a target graph reaches `analyze` is out of scope for this doc — left to the `analyze` design. ### Alternatives considered for sharing the contract @@ -210,7 +216,7 @@ Several shapes for sharing the `BuildRunner` contract across domains were raised // package platform/extension/buildrunner type BuildRunner interface { Trigger(ctx context.Context, base []entity.Batch, head entity.Batch, metadata entity.BuildMetadata) (entity.BuildID, error) - TriggerChanges(ctx context.Context, headURI, baseURI string, metadata entity.BuildMetadata) (entity.BuildID, error) + TriggerChanges(ctx context.Context, baseURI, headURI string, metadata entity.BuildMetadata) (entity.BuildID, error) Status(ctx context.Context, buildID entity.BuildID) (entity.BuildStatus, entity.BuildMetadata, error) Cancel(ctx context.Context, buildID entity.BuildID) error } @@ -289,18 +295,18 @@ Either could be adopted independently: the idempotency token, if a backend that ## Entity and storage additions needed -**`Build` entity** (`stovepipe/entity/build.go`), following the immutable-except-`Status`/`Version` shape of `entity.Request`; `ID` and `Status` use the stovepipe-local `BuildID`/`BuildStatus` types (see the [contract sketch](#stovepipe-buildrunner-contract-design-sketch)), while `RequestID`/`URI`/`BaseURI` stay stovepipe-specific: +**`Build` entity** (`stovepipe/entity/build.go`), following the immutable-except-`Status`/`Version` shape of `entity.Request`; `ID` and `Status` use the stovepipe-local `BuildID`/`BuildStatus` types (see the [contract sketch](#stovepipe-buildrunner-contract-design-sketch)), while `RequestID` stays stovepipe-specific: | Field | Role | Mutable? | |---|---|---| | `ID` | The build's own key — the runner-assigned id returned by `Trigger` (a Buildkite build number, a CI-gateway job id); opaque, never parsed or derived | no | | `RequestID` | The `Request` this build validates (`Build`→`Request` navigation) | no | -| `URI` | Head URI being built (`== Request.URI`) | no | -| `BaseURI` | Incremental baseline; empty for full builds | no | | `Status` | `accepted / running / succeeded / failed / cancelled` | **yes** — `buildsignal` | | `Version` | `int32` optimistic-locking version | **yes** — with `Status` | +The row deliberately carries no scope: `R.URI`, `R.BaseURI`, and `R.BuildStrategy` — immutable and reachable through `RequestID` — fully determine what a build ran against. + **States** (`Build.Status`): | Status | Meaning | Terminal? | diff --git a/doc/rfc/stovepipe/steps/record.md b/doc/rfc/stovepipe/steps/record.md new file mode 100644 index 00000000..e822c9ac --- /dev/null +++ b/doc/rfc/stovepipe/steps/record.md @@ -0,0 +1,302 @@ +# Record stage + +`record` turns a terminal build outcome into a durable validation fact. + +- Phase 1 records whole-repository greenness. On green it also advances the Queue's last-green bookmark and promotes the commit onto the Queue's promotion ref. This is implemented: see [stovepipe/controller/record/record.go](../../../../stovepipe/controller/record/record.go). +- Phase 2 records greenness per project instead of per repository. Sketched here, to be expanded before implementation. + +Notifying downstream systems is **not** implemented in either phase. It will ride the cross-domain hook framework instead of a Stovepipe-specific extension; see [Hooks](#hooks). + +See [workflow.md](../workflow.md) for the whole pipeline, [build.md](build.md) for how builds are created, and [buildsignal.md](buildsignal.md) for the terminal-only handoff into this stage. + +`record` owns the validation facts and the two caches derived from them, the last-green bookmark and the promotion ref. It does not decide build scope, poll a build, release the Queue's build slot, interpret a target graph, or map targets to projects. Those belong to `process`, `buildsignal`, and `analyze`. + +## Phase 1 algorithm + +For a delivery carrying a `Record` payload: + +``` +1. Deserialize the payload into (request id R, queue name Q). + - malformed -> return raw; non-retryable. + +2. Resolve the queue's storage from Q. + - unresolvable -> return raw; non-retryable. A queue with no storage is a + malformed message or a config error, not a transient condition. + +3. Load Request R. + - ErrNotFound -> return raw; non-retryable. + - other store error -> return raw; the classifier decides. + +4. If Q is non-empty and disagrees with R.Queue, return a non-retryable error. + The Request is the authoritative binding; Q is a routing hint, empty on + payloads written before the field existed. + +5. Inspect R.State. + - succeeded / failed -> continue. buildsignal stamps the outcome before it + publishes here, and both values are verdicts about the code. + - cancelled -> ack, no fact: the build decided nothing about the commit (see + "When to record an outcome"). + - superseded -> ack, no fact. Unreachable in practice. + - accepted / processing / anything else -> return a non-retryable invariant error. + +6. Map R.State to a whole-repository degree and create the ValidationFact keyed by + (R.URI, empty project) in the queue-bound fact store. + - ErrAlreadyExists -> load and reconcile the existing immutable fact. + - other store error -> return raw. + +7. If the persisted fact is not green, ack. Report how long the break went undetected + first, but only if step 6 is the write that created the fact (see "Observability"). + +8. Advance the bookmark to (R.URI, R.ID) in a CAS retry loop, which also reports + whether R holds the bookmark afterwards: + - stored id empty or older than R.ID -> write under the version guard; R holds it. + - stored id equals R.ID -> R set it on an earlier delivery; no write, + and R still holds it. + - stored id newer -> no write, and R does not hold it. + - ErrVersionMismatch -> reload and re-evaluate. + +9. If R holds the bookmark, ask SourceControl to point the promotion ref at R.URI. + Otherwise ack: whichever commit holds the bookmark owns the ref. + - ErrNotFound -> count and ack. A rewritten history dropped the commit from the + ref, and no retry can promote it. + - other error -> return raw. + +10. ack. +``` + +Every decision after step 6 uses the persisted fact, not the outcome read from this delivery's Request. The first immutable fact controls the bookmark and the ref, and will control the hook event. Once hooks land, the publish becomes a new step between 9 and 10. + +## Validation facts + +A fact answers "how broken was this scope at this Queue URI?" Its identity is `(queue, uri, project)`, where `project` is empty for whole-repository greenness and a stable project id in Phase 2. + +`queue` is the binding of the store the fact lives in, not a field on the entity. Storage is resolved per queue through `storage.Factory.For`, so a fact only ever exists inside one queue's store and does not need to name it. The column still leads the primary key, `PRIMARY KEY (queue, uri, project)`, because every domain table here has to be shardable by queue: the primary key leads with the queue column and no secondary index leads with anything else. That keeps one queue's rows unreachable through another queue's binding and makes every read a primary-key-prefix scan inside a single queue. It is an invariant rather than a per-table choice, and `make lint-queue-shard` enforces it. It is not MySQL table partitioning; no schema here uses `PARTITION BY`. + +So `entity.ValidationFact` carries only the rest of the identity: + + +| Field | Meaning | +| ----------- | ----------------------------------------------------------------------------------------------------------------- | +| `URI` | Commit URI under validation | +| `Project` | Empty for the whole repository; stable project id for Phase 2 | +| `Degree` | Health degree in the closed interval `[entity.DegreeGreen, entity.DegreeBroken]` — `0` is green, `1` fully broken | +| `RequestID` | Request that established the fact | +| `CreatedAt` | Millisecond timestamp at which the fact was first recorded | + + +Greenness is read through `ValidationFact.IsGreen()` rather than compared against a bare literal, and the endpoints are the named constants `entity.DegreeGreen` and `entity.DegreeBroken`. + +Facts are create-only, so `ErrAlreadyExists` on create means one of exactly two things: + +- Same Request: a redelivery. The stored fact is this delivery's own earlier write, and it carries the same degree, since the Request's outcome is immutable once stamped. Load it and continue. +- Different Request: the `(Queue, URI)` ingest dedup invariant is broken. Return an error rather than overwrite history. + +Competing verdicts never reach this point. Duplicate builds for one Request are resolved a stage earlier, where `buildsignal`'s first-writer-wins outcome write discards the losing build's verdict. + +Absence is still distinct from degree `0`. Callers gating deployments must treat absence as not green. + +### When to record an outcome + +A fact is written only when the Request reaches `succeeded` or `failed`. A `cancelled` build is acked with no fact. + +The fail-closed path is subtler than "no fact". A DLQ reconciler forces the Request to `failed` and does not itself publish here, so reconciliation on its own records nothing. That is not the same as nothing being recorded: a `buildsignal` delivery still in flight can reach this stage afterwards, and it will write `DegreeBroken` from the forced state. See [What fail-closed actually guarantees](#what-fail-closed-actually-guarantees). + +### Phase 1 degree mapping + +Whole-repository builds use only the endpoints, mapped from the outcome `buildsignal` stamped on the Request: + + +| Request outcome | Result | +| --------------- | ----------------------------- | +| `succeeded` | fact at `entity.DegreeGreen` | +| `failed` | fact at `entity.DegreeBroken` | +| `cancelled` | no fact | + + +Intermediate degrees are reserved for project analysis and deferred with the project mapping contract. Phase 1 does not manufacture fractional values. + +## Last-green advancement (Queue bookmark) + +The bookmark only moves forward. `entity.Queue` carries `LastGreenURI` alongside `LastGreenRequestID`, the request id that owns the current URI, plus `InFlightCount` and `LatestRequestID`. The guard compares ids with `entity.CompareRequestID(R.Queue, …)`, the same ingest-order comparison `ingest` and `process` use for coalescing. It errors on an id that does not match the queue's format, which is non-retryable: re-parsing the same ids cannot start succeeding. + +The write goes through `QueueStore.Update(ctx, queue, oldVersion, newVersion)`, so version arithmetic stays in the controller: it computes `newVersion = oldVersion + 1` and the store does a pure conditional write. `ErrVersionMismatch` is absorbed by the loop rather than returned, since a concurrent writer moved the row and reloading converges. + +**Why the bookmark moves only after the fact is durable.** The bookmark is a cache of "newest green URI" derived from the facts, so it must never lead them. Losing the advance to a crash is recoverable, since the redelivery reloads the same fact and retries. A bookmark with no fact behind it would point at greenness nothing recorded, and `process` would pick a baseline URI that no validation supports. + +A failed or cancelled build never moves the bookmark, and so never promotes either. + +## Promotion ref + +A green commit is also promoted: `record` asks `SourceControl` to point the Queue's promotion ref, a stable branch name like `verified-main`, at that commit. This is the pull-shaped counterpart to the hook event's push, so a deploy gate or cache warmer can resolve a name and know nothing about Stovepipe, URIs, or degrees. See [Promotion ref](../workflow.md#promotion-ref--the-last-green-commit-by-name) in the pipeline overview. The stage names only the commit; which ref a Queue promotes to, and whether it has one at all, is integrator configuration injected into the `SourceControl` implementation alongside the endpoint and credentials, so a backend with no promotion target makes `Promote` a no-op. + +**Promotion is gated on holding the bookmark**, and only the holder promotes. Without that gate an older green commit recording late would drag the ref backwards: the ref has no version guard of its own, and `Promote` lands the URI it is given rather than refusing a non-descendant, so the caller has to enforce monotonicity. Deciding it from the bookmark instead of a fresh comparison means both caches move on one already-serialized decision. + +The equal case matters as much as the newer one. A redelivery that finds its own id on the bookmark reports that it *holds* it rather than skipping, so the promotion behind it is retried. That is what makes a crash between the bookmark write and the promotion recoverable, and it is safe because `Promote` is idempotent. Promotion follows the durable fact for the same reason the bookmark does: the ref is a cache of the facts and must not lead them. + +`ErrNotFound` from `Promote` means a rewritten history dropped the commit from the ref. It is counted and acked rather than retried, since no retry will put that commit back and the next green commit corrects the ref. + +## Observability + +Two things are measured here, because this is where greenness becomes known. Both need a `SourceControl` lookup of a commit's creation time. + +- **Last-green age.** A gauge carrying the creation timestamp of the commit the bookmark now points at, emitted once the bookmark write is durable. It is a timestamp rather than an elapsed time so that subtracting it at scrape time gives the current age, and a queue that stops going green keeps reporting a staler value without the stage re-emitting anything. +- **Failure-detection latency.** A histogram of how long the break a build failed on went undetected, measured from the creation time of the base URI it validated against. A distribution rather than a gauge, because what matters is how long breaks typically survive, not how long the last one did. There is no later moment to sample it from, since an elapsed time only means something against the failure that just became known, so this lookup cannot move off the delivery path onto a clock. + +Only the delivery that *wrote* the fact reports the latency. That is why `recordFact` returns whether it created the fact and not just the fact itself: a redelivery adopts the stored fact, and a second sample would count one break twice. A full build pins no base URI, so its failures are counted as unmeasurable instead of timed; absent is the ordinary case there, not a fault. + +Both are best-effort. Every way the lookup can fail is counted, logged, and swallowed, because an observability fault must not turn an already-recorded outcome into a retry. A non-positive creation timestamp is treated as a broken extension contract and dropped, since emitting it would put a 1970 timestamp in a gauge that reads as an infinitely stale queue, or a decades-long sample in the latency distribution. + +## Build slot release + +`Queue.in_flight_count` is released by `buildsignal` before it stamps the outcome and publishes to the record step. The DLQ reconciler releases the slot on the fail-closed path for the same reason. `record`'s only Queue write is the bookmark. + +## Hooks + +Recording a fact is when the rest of the company can learn "this URI is now green (or not green)". `record` publishes that as a `HookEvent` on Stovepipe's durable `hook` topic; it does not call a notification extension inline. + +The mechanics are already settled in [hook-framework.md](../../hook-framework.md) — the envelope, the delivery promise, the per-domain dispatcher stage, the `hook_dlq`, and the reasoning behind each. This section covers only what Stovepipe has to decide for validation facts. The earlier design here, a Stovepipe `Hooks` extension called with `Notify(ctx, ValidationFactRef{…})` as the last algorithm step, is rejected there on both halves: inline calls couple pipeline latency to third-party integrations and drop the notification on a crash between the state write and the call, and a per-domain contract multiplies schemas and sinks for no gain. + +None of it is built. It needs the shared `HookEvent` contract at `api/base/hook/` and the hook extension at `platform/extension/hook/`, plus Stovepipe's own share: a `hook` topic key, a dispatcher stage, a `hook_dlq` reconciler, and the wiring for all three. + +### Where the publish belongs + +Last thing before the ack, after the fact write and after both caches derived from it have moved: + +``` +create fact → advance bookmark (green only) → promote (bookmark holder only) → publish HookEvent → [Phase 2: publish to analyze] → ack +``` + +Last because the payload carries no entity snapshot and hooks resolve entities from stores: a hook reacting to "URI is green" by reading `LastGreenURI`, or by fetching the promotion ref, must not find either still pointing at the previous commit. Inside the delivery rather than after it, because that is what makes the event lossless without an outbox — the state writes are recognize-and-skip on redelivery, so a crash before the ack replays the whole chain. + +### Event shape + + +| Envelope field | Value for a validation fact | +| -------------- | --------------------------------------------------------------- | +| `source` | `stovepipe` | +| `type` | `validation.repository.recorded` (see below) | +| `version` | `0` — a fact is create-only and has no version to report | +| `timestamp_ms` | Publish time; the fact's own `CreatedAt` travels in the payload | +| `id` | `source` / `type` / request id (see below) | + + +The **subject** is the Request — a payload fact rather than an envelope field, but it is what `id` is minted from and what the event partitions on. The Request over the URI keeps partitioning identical to the `record` topic's own, so per-request ordering carries through the seam, and it hands a consumer a way back into the pipeline. The two are near-interchangeable anyway: ingest dedups on `(queue, uri)`, so one Request means one URI. + +The payload carries the fact's identity and value: `queue`, `uri`, `project`, `degree`, `request_id`. `queue` has to be there because neither the envelope nor the fact entity carries one, so the event is the only place a cross-queue consumer sees it. `degree` is there even though a hook could read it from the store: the fact is immutable, so the staleness objection behind the no-snapshots rule does not apply, and it is what a consumer branches on to tell green from broken. Build failure detail stays off, since the payload is reserved for facts persisted nowhere else and a failed build's detail is durable on the `Build` row. + +### What the `type` carries + +Data belongs in `type` when consumers need to avoid receiving the event, and in the payload when they need to interpret it. The type names the scope, `validation.repository.recorded`, with `validation.project.recorded` beside it in Phase 2. + +### What a consumer can and cannot assume + +Ordering is per-subject only and the subject is the Request, so events for *different* Requests can arrive out of order. A consumer must not infer "the newest green commit" from arrival order; it should compare request ids by ingest order (`entity.CompareRequestID`) or read the bookmark, which is monotonic by construction. + +Absence of an event is not a signal: a cancelled build records nothing, and a Request abandoned before any build went terminal never reaches this stage, so a consumer waiting for one event per ingested commit waits forever on those. Gating keeps treating "no recorded fact" as not green. The converse holds too — an event is not proof the code was tested, since a fail-closed Request can produce a broken fact without a build having failed. + +Hooks here must be idempotent on `id`, as everywhere. "Fire-and-forget" describes downstream consumption, not the publish: `record` never waits for a hook, but a failed *publish* fails the delivery. Per `[platform/errs](../../../../platform/errs/README.md)` rule 4 it is not wrapped retryable just because replaying it is convenient, so it dead-letters, which is where the missing `record_dlq` reconciler stops being theoretical: the fact is durable and only the notification is lost. + +## Request lifecycle + +Phase 1 uses the states in [stovepipe/entity/request.go](../../../../stovepipe/entity/request.go). `record` runs *after* the Request is terminal: `buildsignal` projects the build's terminal status onto it as `succeeded`, `failed`, or `cancelled` (`RequestState.HasBuildOutcome()`), and only then publishes. So `record` reads an outcome and writes no state. `superseded` is terminal without an outcome. + +Phase 2 broadens "complete" to "all planned facts recorded", which needs a marker this stage does not own; see [Completion marker: open](#completion-marker-open). + +## Storage and queue contract + +`ValidationFactStore` is key/value-shaped, and queue-bound rather than queue-parameterized: + +- `Create(ctx, fact)` writes one immutable fact for the bound queue. It returns `ErrAlreadyExists` when the composite identity is already taken, leaving the stored fact untouched. +- `Get(ctx, uri, project)` reads one fact by the rest of its identity, and returns `ErrNotFound` when none exists. The `project` argument is reserved for Phase 2. + +There is no `Update`. The first fact written for an identity is the permanent answer, and a caller that needs to know whether it won the race reads `ErrAlreadyExists` and then loads the winner. + +The topic key, the message, and the consumer all exist. The DLQ consumer does not (see [DLQ and fail-closed behavior](#dlq-and-fail-closed-behavior)). + + +| Topic key | Message | Producer | Consumer | Partition key | Message id | +| --------- | ------------------------ | ------------- | -------- | ------------- | ---------- | +| `record` | `Record{id, queue_name}` | `buildsignal` | `record` | Request id | Request id | + + +The payload carries the request id plus the queue name, so the consumer can resolve per-queue storage before it loads any state. `queue_name` is empty on payloads written before the field existed, which is why step 4 only enforces the match when it is set. + +Partitioning by request id keeps completion bookkeeping single-writer per Request, and reusing the request id as the message id dedups a redelivered signal into the original message instead of enqueuing a second one. + +## Error classification + + +| Failure | Disposition | Reason | +| -------------------------------------------------- | -------------------- | --------------------------------------------------------------------------------- | +| Malformed `Record` payload | non-retryable | a malformed message will never succeed regardless of retries | +| Queue name that resolves to no storage | non-retryable | malformed message or missing config, not a transient condition | +| Payload queue disagreeing with the Request's queue | non-retryable | malformed message; the Request is the authoritative binding | +| Request not found | non-retryable | the publish follows the committed outcome write, so a miss is a storage defect | +| Request carrying no build outcome | non-retryable | producer/state-machine invariant violation | +| Existing fact owned by a different Request | non-retryable | ingest dedup invariant violated; the stored fact is immutable | +| Malformed request id at bookmark comparison | non-retryable | re-parsing the same ids cannot succeed | +| Queue CAS version mismatch | absorbed, not raised | `storage.ErrVersionMismatch` is handled by the bookmark loop: reload and re-apply | +| Promotion target unknown to the ref | absorbed, not raised | `sourcecontrol.ErrNotFound` means a rewritten history dropped the commit | +| SourceControl resolution or `Promote` failure | raw error | backend classifier has the required failure knowledge | +| Commit-timestamp lookup for a metric | swallowed | counted and logged; observability must not retry a recorded outcome | +| ValidationFactStore, QueueStore, RequestStore | raw error | backend classifier has the required failure knowledge | + + + + +## Edge cases and idempotency + +Every effect is recognize-and-skip, so a redelivery after a complete run re-runs each step as a no-op. Once hooks land the publish re-fires, and the framework's dedupe on `id` absorbs it. + +- **Request not visible.** A storage defect rather than lag, since the publish follows the committed outcome write. Non-retryable. +- **Fact already created.** Load it and continue from the stored fact. A fact from a *different* Request, or a Request carrying no build outcome, is an invariant violation rather than an expected outcome. +- **Duplicate builds for one Request.** Absorbed a stage earlier: `buildsignal`'s outcome write is first-writer-wins, so the Request carries one immutable verdict and `record` never sees a competing one. +- **Head equals last-green.** The build still produces a terminal verdict. A green result may share the bookmark's URI, but the guard compares request ids rather than URIs, so an equal-or-older candidate skips and nothing regresses. +- **Green fact recorded out of order across Requests.** An older green commit can reach this stage after a newer one. Its fact is written as usual, since facts are per-URI and independent, but the bookmark guard skips it, and because it does not hold the bookmark it does not promote either. Neither cache moves backwards. +- **Cancelled build.** Stovepipe never initiates cancellation, but a backend may still report it. No fact is written; the slot was already released and the Request already stamped `cancelled`. The fact identity stays unclaimed, and nothing can claim it today, since `cancelled` is terminal and re-validation does not exist. Recovery in practice is the next commit; the unclaimed identity only matters to a future re-run mechanism (see [Supporting re-run of the same URI](#supporting-re-run-of-the-same-uri)). +- **History rewrite while a build runs.** The Request keeps the strategy and URI pinned at admission, and record stores the fact about that immutable URI. A later head is handled independently by `process`. If the rewrite dropped the commit from the ref, the fact and the bookmark still stand, since they describe a commit and not a ref, and only the promotion is skipped. +- **A fail-closed terminal outranks a build that passed.** The degree derives from `R.State`, so a Request forced to `failed` by DLQ reconciliation records `DegreeBroken` even when one of its builds reports success afterwards. Reachable today, and permanent once written; see [What fail-closed actually guarantees](#what-fail-closed-actually-guarantees). +- **Crash between the fact write and the bookmark advance.** Redelivery reloads the existing fact and re-applies the idempotent guard. +- **Crash between the bookmark advance and the promotion.** Redelivery finds its own id on the bookmark, reports that it holds it, and retries the idempotent promotion. + + + +## DLQ and fail-closed behavior + +**Neither** `record_dlq` **nor** `build_dlq` **has a consumer today, and both topics are already receiving messages.** Every primary subscription comes from `DefaultSubscriptionConfig`, which enables dead-lettering with the `_dlq` suffix, so a message that is rejected outright *or* runs out of retries moves to its stage's dead-letter topic. The wiring registers only `process_dlq` and `buildsignal_dlq`, so messages pile up unread on the other two. + +Two different things put a message there, and only one is a poison payload. A delivery that fails with its retry budget spent is dead-lettered by the nack itself, carrying the reason it actually failed. A delivery that never reaches a nack, because it crashed or because its **ack failed** and the visibility timeout redelivered it, is dead-lettered by the poll loop once `retry_count` reaches `MaxAttempts` (3 by default), without the controller running on that final attempt and with only a generic reason recorded. So a missing reconciler exposes more than malformed messages: a fact can be lost to a storage failure that would have succeeded on a later retry, or to an ack that never landed even though the write did. + +Gating stays safe, because everything this stage can lose reads as not-green: a Request with no fact is indistinguishable from one not yet validated. What is lost is the *fact*. A green build whose fact write permanently failed leaves the URI looking unvalidated, which costs the queue an incremental baseline and forces a full build at the next head. Once hooks land, a lost notification joins that list, and unlike the fact it gets no second chance from a later commit. + +This is the same failure shape [buildsignal.md](buildsignal.md#what-it-costs-when-a-backend-does-not-classify-status-errors) describes for a deployment that registers primary consumers without their reconciler. When the reconciler is built it should re-run this same idempotent algorithm from the request id, under `errs.AlwaysRetryableProcessor`: write and publish the immutable fact as usual if the Request carries a build outcome, keep retrying if Request storage is temporarily unavailable, and treat a malformed payload or a permanently missing Request as poison, which needs an operational alert rather than more retries. + +## Future Items + + + +### Coverage of intermediate commits + +Coalescing means most commits never become a validated Request: a verdict on head `H` with base `B` is implicitly a verdict on every commit in the range `(B, H]`. Downstream tooling still needs prior and next green for any commit, including commits never ingested directly and commits that were superseded. + +The rough idea is to track this with additional stores written during this stage, to be expanded in a separate doc: + +- `CoverageStore` — on every verdict, green or failed, one row per commit in the covered range: `(queue, uri)` → the covering request and the commit's position within its range. Gives commits with no Request of their own a place in the queue's history. +- `GreenLogStore` — on green verdicts only, one row keyed by that position. The key is ordered, so "previous green" and "next green" become two seeks: the nearest entry below or above a commit's position. + +Neither fits `ValidationFactStore`. Facts are looked up by exact identity and URIs do not sort, while previous/next-green needs an ordered seek over positions, a different key shape. A genuinely needed reverse lookup getting its own first-class store is the established pattern here; `RequestURIStore` is the existing example (see [storage README](../../../../stovepipe/extension/storage/README.md#key-value-contract)). + +### Completion marker: open + +`record` makes no `Request` write in Phase 1, so the stage has no durable marker saying it finished. `buildsignal` drives the Request terminal before `record` ever runs, leaving no non-terminal window to occupy. + +### Supporting re-run of the same URI + +Widen the key with the `RequestID` the facts already record, so each attempt is its own immutable row, and add a pointer store from `(queue, uri, project)` to the authoritative attempt. Advance to a newer attempt unless the current one is green, since a green build proved the code passed and a later failure only proves the build is non-deterministic. + +### Phase 2 + +- **Pipeline**: `record` publishes the request id onward to `analyze`, for green and not-green facts alike, because a failed build is when project attribution matters most. An earlier draft had `record` retarget the Request from `processing` to `analyzing`, with `analyze` owning the terminal transition. That no longer fits, because the Request is already terminal before `record` runs, so tracking "all facts recorded" belongs to the `analyze` design (see [Completion marker: open](#completion-marker-open)). The message onward carries the request id and queue name, exactly as `record`'s own does, and the consumer stays idempotent. +- **Project facts**: `record` runs the same create-fact-then-notify flow. The fact is keyed by the stable project id carried on the per-project signal, and one `validation.project.recorded` event is published per project identity (see [What the](#what-the-type-carries) `type` [carries](#what-the-type-carries)). Each per-project signal needs its own message id (see [Storage and queue contract](#storage-and-queue-contract)). The Queue's `last_green_uri` describes the whole repository and stays untouched. + +Deciding project identity, retrieving the target graph, tracking completion, and defining intermediate degrees belong to `analyze.md`. diff --git a/doc/rfc/stovepipe/workflow.md b/doc/rfc/stovepipe/workflow.md index 28bc3662..c7b62442 100644 --- a/doc/rfc/stovepipe/workflow.md +++ b/doc/rfc/stovepipe/workflow.md @@ -55,10 +55,10 @@ The ref is a *cache* of the last-green URI, not a second record of greenness. It |---|---| | **SourceControl** | Resolve a Queue name to its current head URI; answer ancestry/comparison questions between two URIs (is the new head a fast-forward descendant of the last green, or was history rewritten?); enumerate commits in a range; advance the Queue's **promotion ref** to a commit. The sole owner of URI semantics, including which refs a Queue name resolves to. | | **build-runner** | Build a scope at a URI (optionally relative to a baseline URI), returning pass/fail and the target graph. See [build-runner.md](../submitqueue/build-runner.md). | -| **Hooks** | Publish Stovepipe's greenness events to downstream systems — "this URI / this project is now green (or not green)". Fire-and-forget notification, decoupled so Stovepipe does not know or care who consumes the event. | +| **Hooks** | Deliver Stovepipe's greenness events to downstream systems — "this URI / this project is now green (or not green)". Fire-and-forget notification, decoupled so Stovepipe does not know or care who consumes the event. Not implemented yet; it will be the shared cross-domain hook seam rather than a Stovepipe-specific extension. See [hook-framework.md](../hook-framework.md). | | **Storage** | Persist Queues (incl. last-green URI), Requests, build records, and per-URI / per-project greenness. Key/value-shaped per the extension-design rules in [CLAUDE.md](../../../CLAUDE.md). | -The **Hooks** extension is the notification boundary. Whenever a greenness fact is recorded — whole-repo green/not-green, or later a project green/not-green — `record` fires the relevant hook so deployment systems, dashboards, and developer tooling learn about it without polling Stovepipe's store. Hooks are pluggable so each environment can route events to its own downstream (a deploy gate, a Slack notifier, an event bus) without changing the pipeline. +Hooks are the notification boundary. When a validation fact is recorded — whole-repo green/not-green, or later a project green/not-green — the event reaches deployment systems, dashboards, and developer tooling without any of them polling Stovepipe's store, and each environment can route it to its own downstream (a deploy gate, a Slack notifier, an event bus) without changing the pipeline. The mechanism is the cross-domain hook framework rather than a call out of the recording stage: `record` publishes a `HookEvent` to Stovepipe's `hook` topic, and a dispatcher stage consumes it and invokes the wired hooks, so a slow or failing downstream cannot add latency to the pipeline. Neither half exists yet; see [record.md](steps/record.md#hooks) for the fact-to-event mapping and its open questions. ## Workflow @@ -125,7 +125,7 @@ The pipeline runs in two phases against the same Request. **Phase 1** establishe │ ┌──────────────────────────────┐ Hooks └───────────────────►│ record │┄┄┄┄┄► "project P │ Capture per-project greenness │ green / not - │ for the URI; fire Hooks │ green at URI" + │ for the URI; hook event │ green at URI" └──────────────────────────────┘ ``` @@ -135,13 +135,13 @@ The pipeline runs in two phases against the same Request. **Phase 1** establishe 2. **process** — decides build strategy (incremental since last-green vs full monorepo), gates concurrent work per Queue, coalesces backlog to the latest head, and publishes to `build`. See [process.md](steps/process.md). 3. **build** — runs the build-runner for the chosen scope. A flag derived from `process` decides whether to build relative to the last-green **baseline URI** (incremental) or from scratch (full). It records a build and publishes the BuildID. 4. **buildsignal** — records the build's status and target graph when the build completes, then releases the Queue's `in_flight_count` slot, projects the terminal status onto the Request (`succeeded` / `failed` / `cancelled`), and publishes the RequestID to `record`. -5. **record** — writes the whole-repo greenness for the head URI (`0` green / `1` broken to start), derived from the Request's build outcome. On green it advances the Queue's **last-green URI** so the next `process` can build incrementally from here, and asks `SourceControl` to advance the Queue's **promotion ref** to the same commit (see [Promotion ref](#promotion-ref)). It fires the **Hooks** extension with the green/not-green event, then fans out into Phase 2. The Queue's `in_flight_count` was already released by `buildsignal` when the build went terminal. +5. **record** — writes the whole-repo greenness for the head URI (`0` green / `1` broken to start), derived from the Request's build outcome. On green it advances the Queue's **last-green URI** so the next `process` can build incrementally from here, and asks `SourceControl` to advance the Queue's **promotion ref** to the same commit (see [Promotion ref](#promotion-ref--the-last-green-commit-by-name)). It publishes a **hook event** for the green/not-green transition, then fans out into Phase 2. The Queue's `in_flight_count` was already released by `buildsignal` when the build went terminal. ### Phase 2 — project greenness 6. **analyze** (project-analysis) — takes the build's target graph and maps the relevant targets to **projects**, using whatever implementer-specific mapping is configured. It decides which project-scoped builds / CI jobs are needed to attribute breakage to specific projects, and publishes those builds. 7. **build → buildsignal** — the project-scoped CI job runs; its artifacts are stored in a blob store (e.g. TerraBlob), and `buildsignal` reads back the status. This is the same machinery as Phase 1, reused at project granularity. -8. **record** — captures **per-project greenness for the URI** — for each project, green or not at this commit — and fires **Hooks** per project. This is what lets a caller ask "is project P green at URI U?" and "what is the latest URI where project P is green?". +8. **record** — captures **per-project greenness for the URI** — for each project, green or not at this commit — and publishes one hook event per project. This is what lets a caller ask "is project P green at URI U?" and "what is the latest URI where project P is green?". `record` appearing twice is intentional: it is one re-entrant stage that records greenness at whatever granularity the current phase produced and notifies downstream. The Request is *complete* when every planned granularity has been recorded, not at a single terminal hop. @@ -153,7 +153,7 @@ The pipeline runs in two phases against the same Request. **Phase 1** establishe | **process** | RequestID | build | Build strategy, concurrency gate, backlog coalescing → [process.md](steps/process.md) | | **build** | RequestID | buildsignal | Run the build-runner for the chosen scope; baseline = last-green URI iff incremental | | **buildsignal** | BuildID | record (P1), record (P2) | Record build status + target graph; release `in_flight_count`; project the outcome onto the Request; signal completion | -| **record** | RequestID | analyze (P1→P2), Hooks | Write greenness; on whole-repo green advance last-green URI and the promotion ref; fire Hooks | +| **record** | RequestID | analyze (P1→P2), hook topic | Write greenness; on whole-repo green advance last-green URI and the promotion ref; publish the hook event | | **analyze** | RequestID | build | Map broken/at-risk targets → projects; decide project-scoped builds | ## Step RFCs @@ -163,6 +163,7 @@ Per-stage design detail lives under `steps/` so this doc stays a pipeline overvi - [process.md](steps/process.md) — build-strategy decision, concurrency gate, backlog coalescing, [concurrency lifecycle](steps/process.md#concurrency-lifecycle), entity changes, [waiting for a slot](steps/process.md#waiting-for-a-slot) - [build.md](steps/build.md) — trigger-only stage: reads the decided scope off the Request, triggers the build-runner, hands off to buildsignal; the stovepipe `BuildRunner` contract and why it differs from SubmitQueue's - [buildsignal.md](steps/buildsignal.md) — the poll loop: hold-based re-poll cadence, target-graph return, per-build partitioning, and the fail-closed handoff to record +- [record.md](steps/record.md) — turning a terminal build outcome into an immutable validation fact, monotonic last-green advancement and ref promotion, and the deferred hook and analyze handoffs ## Dedup, idempotency, and history rewrites @@ -170,7 +171,7 @@ Ingestion is idempotent on `(Queue, head URI)`, so duplicate poller reports — ## Fail-closed on unprocessable work -Callers gate deployments on greenness, so the dangerous failure is a Request that can never finish and silently leaves a URI with no recorded greenness — indistinguishable, to a naive caller, from "not yet validated". Following SQ's DLQ-reconciliation posture, a Request whose validation can never complete must be driven to a **conservative terminal `failed` outcome** — which whatever records greenness treats as not-green — rather than left non-terminal: gating stays safe (never falsely green), and the pipeline moves on. State writes use optimistic-locking CAS, so a late successful update wins cleanly over the conservative one. See [submitqueue/orchestrator/controller/dlq/README.md](../../../submitqueue/orchestrator/controller/dlq/README.md) for the shared reconcile-only design. +Callers gate deployments on greenness, so the dangerous failure is a Request that can never finish and silently leaves a URI with no recorded greenness — indistinguishable, to a naive caller, from "not yet validated". Following SQ's DLQ-reconciliation posture, a Request whose validation can never complete must be driven to a **conservative terminal `failed` outcome** — which whatever records greenness treats as not-green — rather than left non-terminal: gating stays safe (never falsely green), and the pipeline moves on. That conservative outcome is **final**, not provisional: validation facts are immutable and first-fact-wins, so a late successful result for a fail-closed URI is dropped rather than overwriting recorded history. The cost is bounded — the branch keeps moving, and the next head re-establishes greenness on its own Request. Note that finality cuts deeper than "the outcome is never revised": a forced `failed` can itself be recorded as a broken fact by a delivery still in flight, so a commit whose build actually passed can end up permanently marked broken. Whether that is correct or a category error is open — see [record.md](steps/record.md#what-fail-closed-actually-guarantees). See [submitqueue/orchestrator/controller/dlq/README.md](../../../submitqueue/orchestrator/controller/dlq/README.md) for the shared reconcile-only design. ## Open questions