diff --git a/doc/rfc/index.md b/doc/rfc/index.md index 3f2a9af2..4db40849 100644 --- a/doc/rfc/index.md +++ b/doc/rfc/index.md @@ -23,6 +23,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, Queue coordination, Hooks notification, and the Phase 1 handoff to analyze ## Runway diff --git a/doc/rfc/stovepipe/steps/build.md b/doc/rfc/stovepipe/steps/build.md index 91d1bdf4..4f70cdd0 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,7 +80,7 @@ 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. +- **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, and the fail-closed reconciler drives the Request terminal (see [workflow.md](doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work)). ## Edge cases @@ -128,7 +130,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 +155,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 +191,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 +200,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 +212,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 +291,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..60984456 --- /dev/null +++ b/doc/rfc/stovepipe/steps/record.md @@ -0,0 +1,215 @@ +# Record stage + +`record` turns a terminal build outcome into a durable validation fact. + +- In Phase 1 it records whole-repository greenness, advances the Queue's last-green bookmark when the result is green, and notifies downstream systems. +- In Phase 2 the same stage records project greenness and notifies downstream systems at project granularity. Mentioned in this doc, but to be expanded on before future implementation. + +See [workflow.md](../workflow.md) for the complete 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 persistence and publication of validation facts. 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 responsibilities belong to `process`, `buildsignal`, and `analyze`. + +## Phase 1 algorithm + +For a delivery carrying request id `R`: + +``` +1. Load Request R. + - ErrNotFound -> return raw; non-retryable. + - other store error -> return raw; the classifier decides. + +2. Inspect R.State. + - succeeded / failed -> continue. This is the entry condition: buildsignal stamps the + outcome before it publishes here, and both values are verdicts about the code. + - cancelled -> ack. No fact and no notification: the build decided nothing about the + commit (see "When to record an outcome"). + - superseded -> ack; no fact or notification is written. Unreachable in practice. + - accepted / processing / anything else -> return a non-retryable invariant error. + +3. Map R.State to a whole-repository degree and create the ValidationFact keyed by + (R.Queue, R.URI, empty project). + - ErrAlreadyExists -> load and reconcile the existing immutable fact. + - other store error -> return raw. + +4. If the persisted fact is green, advance the Queue bookmark in one CAS retry loop: + a. If LastGreenRequestID is empty or older than R.ID per entity.CompareRequestID, set + LastGreenURI = R.URI and LastGreenRequestID = R.ID. + b. If no field changes, skip the write. + +5. Notify the Hooks extension with the fact identity. + - return errors raw; the hook backend's classifier decides retryability. + +6. ack. +``` + +Every decision after step 3 uses the persisted fact (not the outcome read from this delivery's Request). The first immutable fact controls the Queue bookmark and the Hooks event. + +## Validation Fact Recording + +A validation fact answers "how broken was this scope at this Queue URI?" Its identity is: + +``` +(queue, uri, project) +``` + +`project` is empty for whole-repository greenness and is a stable project id in Phase 2. + +The fact contains: + + +| Field | Meaning | +| ----------- | ----------------------------------------------------------------------------------- | +| `Queue` | Stable Queue name that namespaces the validation | +| `URI` | Commit URI under validation | +| `Project` | Empty for the whole repository; stable project id for Phase 2 | +| `Degree` | Health degree in the closed interval `[0, 1]`; `0` is green and `1` is fully broken | +| `RequestID` | Request that established the fact | +| `CreatedAt` | Millisecond timestamp at which the fact was first recorded | + + +Facts are create-only. `ErrAlreadyExists` on create means one of exactly two things: + +- Same Request → a redelivery. The existing fact is this delivery's own prior write and necessarily 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 has been violated. Return an error rather than overwrite history. + +Competing verdicts cannot 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, so `record` only ever sees one. + +Absence remains 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 a `succeeded` or `failed` verdict. A `cancelled` build is acked with no fact recorded. Callers gating deployments treat an absent fact as not green. + +The fail-closed path also produces no fact, as the DLQ reconciler forces `failed` and never publishes to `record`. + +### Phase 1 degree mapping + +MVP whole-repository builds use only the endpoints, mapped from the outcome `buildsignal` stamped on the Request: + + +| Request outcome | Result | +| --------------- | ------------------ | +| `succeeded` | fact at degree `0` | +| `failed` | fact at degree `1` | +| `cancelled` | no fact | + + +Intermediate degrees are reserved for project analysis and deferred with the project mapping contract; Phase 1 does not manufacture fractional values. + +### Supporting re-run of same URI (future use case) + +Widen the key with the `RequestID` they 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. + +### 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 to be able to retrieve prior and next green for any commit, including those never ingested directly, or that have been superseded. + +To support that, the rough idea is that we can track this via some additional stores that are written during this record step, which we can expand upon 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. Because the key is ordered, "previous green" and "next green" become two seeks: the nearest entry below or above a commit's position. + +Neither fits the `ValidationFactStore` proposed above: 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. + +## Build slot release + +`Queue.in_flight_count` is released by `buildsignal` before it stamps the outcome and publishes 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 last-green bookmark. + +## Completion marker: open + +`record` makes no `Request` write in Phase 1, which leaves the stage with no durable marker saying it finished. The consequence is contained today, because every effect is idempotent and a redelivery simply re-runs them, but two things depend on a marker and stay unresolved: + +- **Redelivery cost.** With no marker to check, a redelivery always re-fires the hook. That is safe under at-least-once delivery with the fact identity as the idempotency key, but it is a real duplicate rather than a skipped no-op. +- **Phase 2 completion.** "All planned facts recorded" needs somewhere to live. An earlier draft inserted a non-terminal `analyzing` state between `processing` and the recorded states; that no longer fits, because `buildsignal` drives the Request terminal before `record` ever runs, leaving no non-terminal window to occupy. Phase 2 must either move the outcome write back behind analysis, add a separate completion entity, or track fan-out on the facts themselves. Deferred to `analyze.md`. + +## Last-green advancement (Queue bookmark) + +The bookmark only moves forward. The Queue gains one field, `LastGreenRequestID` — the request id that owns the current `LastGreenURI`. It is a proposed addition; `entity.Queue` today carries `LastGreenURI`, `InFlightCount`, and `LatestRequestID` only. On a green fact, step 4 adopts the pair `(R.URI, R.ID)` only when the stored id is empty or older than `R.ID` per `entity.CompareRequestID(R.Queue, …)` — the same ingest-order comparison `ingest` and `process` already use for coalescing. That comparison returns an error on an id that does not match the queue's format; treat it as non-retryable, since re-parsing the same ids cannot succeed. + +A failed or cancelled build never moves the bookmark. + +## Hooks + +After a validation fact is recorded, hooks will be fired to notify downstream consumers of the greenness change. + +The Hooks contract takes a thin fact identity, and implementations may resolve additional details as needed: + +``` +Notify(ctx, ValidationFactRef{Queue, URI, Project}) error +``` + +Delivery is at-least-once with the fact identity as the idempotency key; hook implementations or their downstreams must absorb duplicates. "Fire-and-forget" refers to downstream consumption, not the publish itself: `record` never waits for consumers to act on an event, but a failed `Notify` fails the delivery and is retried. The Request is already terminal and its slot already released by then, so a stuck hook delays the notification without holding up the pipeline. + +## 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 mean "all planned facts recorded", which needs a marker this stage does not own — see [Completion marker: open](#completion-marker-open). + +## Message-queue additions + +The topic key and message already exist in `stovepipe/core/messagequeue`; only the consumer is outstanding. + + +| Topic key | Message | Producer | Consumer | Partition key | Message id | +| --------- | ------------------------- | ------------- | -------- | ------------- | ---------- | +| `record` | `Record{id}` (request id) | `buildsignal` | `record` | Request id | Request id | + + +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. + +## Idempotency and competing outcomes + +- **Request not visible** — a storage defect rather than a lag, since the publish follows the committed outcome write. Non-retryable. +- **Fact already created** — load it and continue from the authoritative fact. +- **Bookmark already advanced** — the guard skips equal-or-older candidates and the write is skipped. +- **Hook notified, then crash** — retry re-notifies; the fact identity dedups. +- **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. +- **Redelivery after a complete run** — every effect is recognize-and-skip except the hook, which re-fires (see [Completion marker: open](#completion-marker-open)). + +An existing fact from a different Request, or a Request carrying no build outcome, is an invariant violation rather than an expected control-flow outcome. + +## Error classification + + +| Failure | Disposition | Reason | +| ----------------------------------------------- | ------------------------ | ----------------------------------------------------------------------------------------------------------------- | +| 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 | +| Malformed request id at bookmark comparison | non-retryable | re-parsing the same ids cannot succeed | +| Queue CAS version mismatch | retryable at declaration | `storage.ErrVersionMismatch` is declared retryable; return raw, reload, and reapply the idempotent bookmark guard | +| Hooks, ValidationFactStore, QueueStore, RequestStore | raw error | backend classifier has the required failure knowledge | + + +## DLQ and fail-closed behavior + +`record_dlq` re-runs the same idempotent record algorithm from the request id, under `errs.AlwaysRetryableProcessor`: + +- If the Request carries a build outcome, write and publish its immutable fact as usual. +- If Request storage is temporarily unavailable, keep retrying. +- If the payload is malformed or the Request is permanently missing, the message is poison: there is no trustworthy identity to act on, so it needs an operational alert rather than more retries. + +If no build verdict is available, a validation fact is not written. Gating stays safe because an absent fact reads as not green (see [When to record an outcome](#when-to-record-an-outcome)). + +## Edge cases + +- **Head equals last-green.** The build still produces a terminal verdict. A green result may share the bookmark's URI; the guard compares request ids, not URIs, so an equal-or-older candidate skips and nothing regresses. +- **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, but nothing can claim it today — `cancelled` is a terminal state and re-validation does not exist — so in practice recovery is the next commit; the unclaimed identity only matters to a future re-run mechanism (see [Supporting re-run of same URI](#supporting-re-run-of-same-uri-future-use-case)). +- **History rewrite while a build runs.** The Request keeps the strategy and URI pinned at admission; record stores the fact about that immutable URI. A later head is handled independently by `process`. +- **Late successful result after a fail-closed terminal.** The terminal `failed` Request wins: the fact records degree `1` even though the build passed, because the degree derives from `R.State`. Nothing rewrites greenness or reopens the pipeline. +- **Crash between the fact write and the hook.** Retry reloads the existing fact and re-notifies; the fact identity makes the duplicate safe. +- **Ack fails after a complete run.** Redelivery re-runs every step as a no-op except the hook, which re-fires. + +## Phase 2 plans + +Phase 2 can expand upon the record phase: + +- **Pipeline**: `record` publishes the request id onward to `analyze`. It does this for green and not-green facts alike, because a failed build is exactly 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 stays id-only and the consumer stays idempotent. +- **Project builds**: `record` runs the same load-fact-notify flow. The fact is keyed by the stable project id carried on the per-project signal, and Hooks is notified with that identity. Each per-project signal needs its own message id (see [Message-queue additions](#message-queue-additions)). The Queue's `last_green_uri` describes the whole repository and stays untouched. + +Other functionality such as deciding project identity, retrieving the target graph, tracking completion, defining intermediate degrees — belongs to `analyze.md`. + +`ValidationFactStore` is key/value-shaped: + +- `Create(ctx, fact)` writes one immutable fact. It returns `ErrAlreadyExists` when the composite identity is already taken. +- `Get(ctx, queue, uri, project)` reads one fact by its full identity. It returns `ErrNotFound` when no fact exists. The `project` field is reserved for Phase 2. diff --git a/doc/rfc/stovepipe/workflow.md b/doc/rfc/stovepipe/workflow.md index 93e3c132..f111190c 100644 --- a/doc/rfc/stovepipe/workflow.md +++ b/doc/rfc/stovepipe/workflow.md @@ -52,7 +52,7 @@ A **project** is a caller-defined slice of the repository. Whole-repo greenness | **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. | | **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. +The **Hooks** extension is the notification boundary. Whenever a validation 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. ## Workflow @@ -157,6 +157,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: `PublishAfter` re-poll cadence, target-graph return, per-build partitioning, and the fail-closed handoff to record +- [record.md](steps/record.md) — immutable validation facts, idempotent Queue-slot release, monotonic last-green advancement, Hooks notification, and the Phase 1 handoff to analyze ## Dedup, idempotency, and history rewrites @@ -164,7 +165,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. See [record.md](steps/record.md#dlq-and-fail-closed-behavior). See [submitqueue/orchestrator/controller/dlq/README.md](../../../submitqueue/orchestrator/controller/dlq/README.md) for the shared reconcile-only design. ## Open questions