From cb97638ba888ce5c5e3b6fbc61644f8f4403513f Mon Sep 17 00:00:00 2001 From: mnoah1 Date: Tue, 20 Oct 2026 15:56:54 +0000 Subject: [PATCH] docs(stovepipe): outline build analysis and repair workflow --- doc/rfc/stovepipe/steps/build.md | 35 ++---- doc/rfc/stovepipe/steps/buildsignal.md | 8 +- doc/rfc/stovepipe/steps/record.md | 42 ++++--- doc/rfc/stovepipe/workflow.md | 161 ++++++++++++++++--------- 4 files changed, 144 insertions(+), 102 deletions(-) diff --git a/doc/rfc/stovepipe/steps/build.md b/doc/rfc/stovepipe/steps/build.md index 7fb415d74..f7666678e 100644 --- a/doc/rfc/stovepipe/steps/build.md +++ b/doc/rfc/stovepipe/steps/build.md @@ -8,13 +8,13 @@ It handles only the trigger: it does not poll for completion, record greenness, ## Input, partitioning, and the single-writer property -`build` consumes a request id, published by `process` in Phase 1 (see [workflow.md](../workflow.md#workflow)). Both phases drive the same `build` → `buildsignal` machinery against the same `Request` row. The `process`/`analyze` → `build` topic is partitioned by **request id**; see [Partitioning](#partitioning) for the full rationale, including why `build` → `buildsignal` partitions finer (by build id) than SubmitQueue's equivalent topic. +`build` consumes a request id published by `process` (see [workflow.md](../workflow.md#workflow)). The `process` → `build` topic is partitioned by **request id**; see [Partitioning](#partitioning) for the full rationale, including why `build` → `buildsignal` partitions finer (by build id) than SubmitQueue's equivalent topic. **`build` is not the sole writer of the rows it touches, and its own writes are narrow.** On `Request`, `build` never writes at all — it only reads the `BuildStrategy`/`BaseURI`/`URI` fields `process` set at admit, and it leaves `Request.State` untouched at `processing` throughout (`process`, `buildsignal`, and the DLQ reconciler are `Request.State`'s only writers). On `Build`, `build` is the sole creator — it calls `BuildStore.Create` exactly once, at step 6 — and never mutates the row again; `buildsignal` is the sole writer of `Build.Status`/`Build.Version` afterward (see [buildsignal.md](buildsignal.md#input-and-re-entrancy)). This division is why `build` never needs a CAS/version write of its own: `Create` is the only storage mutation in its algorithm. -`build` is phase-agnostic: it never asks "which phase is this?" It reads whatever scope is already persisted and immutable on the `Request` and acts on it. Phase 2's project-scoped invocation is expected to read project-scoped equivalents of the scope fields off the same `Request`; the exact shape of a project-scoped trigger is left to the `analyze` design, consistent with `workflow.md`'s "project mapping contract" open question — see [Project-scoped `Trigger`: reserved, not yet designed](#project-scoped-trigger-reserved-not-yet-designed) for the resulting gap in the `BuildRunner` contract itself. +Project analysis already runs in `record` through `projectstatus.Resolver`, using the terminal build from this validation. Artifact extraction and project-result calculation may later move into [build analysis](../workflow.md#build-analysis). -In both phases `build` is strictly the trigger — `buildsignal` owns polling — so a crash between `Trigger` and the publish leaves a build no poller ever picks up; the redelivery triggers a replacement and the orphan is accepted waste, mirroring SubmitQueue (see [Idempotency](#idempotency)). +`build` triggers and `buildsignal` polls. A crash between `Trigger` and the publish leaves a build no poller picks up; redelivery triggers a replacement and the orphan is accepted waste, mirroring SubmitQueue (see [Idempotency](#idempotency)). ## Algorithm @@ -86,7 +86,6 @@ Every branch is safe under at-least-once redelivery — with SubmitQueue's postu ## Edge cases - **Head equals last-green** (`R.URI == R.BaseURI`). `process.md` calls this degenerate and leaves it to `build`. Resolution: run a normal incremental build with an empty delta — pass `baseURI = R.BaseURI` unchanged and let the runner apply zero changes on top of it. This needs no branch in `build`: the runner must already handle "no delta" (two adjacent commits) as valid input. Rejected: skipping the build and copying last-green's result forward, because that would force `build` to reason about whether last-green's *recorded greenness* (not just its URI) is still valid — a `record` concern `build` has no business making. -- **Phase 2 triggers another build for the same Request.** No collision by construction: every `Trigger` mints a fresh runner id, so the Phase-1 whole-repo build and each Phase-2 project-scoped build get distinct `Build` rows, all carrying the same `RequestID`. Each build's id travels in its own messages, so downstream navigation stays a direct get; how `analyze` obtains and forwards the ids of the project builds it triggers is part of the deferred project-scoped trigger design. ## Fail-closed interaction @@ -186,23 +185,15 @@ type Config struct{ QueueName string } // the only identity the system hands a F type Factory interface{ For(cfg Config) (BuildRunner, error) } ``` -#### Project-scoped `Trigger`: reserved, not yet designed +#### Project results from the validation build -**TODO**, tracked pending the `analyze` design (see [workflow.md](../workflow.md#open-questions)'s "Project mapping contract" open question). The interface above has only a whole-repo/incremental dimension (`headURI`, `baseURI`); it has no parameter for "build only this project," so as written it cannot express a Phase 2 invocation. `build` itself stays phase-agnostic (see [Input, partitioning, and the single-writer property](#input-partitioning-and-the-single-writer-property)) — it reads whatever scope is already decided and passes it through — but `Trigger` still needs a slot to read that scope from and forward to the runner. - -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](../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, 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 contract); `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. +[`projectstatus.Resolver`](../../../../stovepipe/extension/projectstatus/projectstatus.go) derives project results from the terminal validation build when called by `record`. A project-scoped build would be a separate capability requiring an explicit scope contract; it is independent of the project analysis already implemented. Both `Trigger` and `Status`/`Cancel` differ *in contract* between domains, even though `Status`/`Cancel` happen to be identical in shape: both domains poll and cancel by the same opaque, runner-minted id with the same async semantics. Rather than promoting that shape parity into a shared `platform/base`/`platform/extension/buildrunner` type and interface — which would force a one-time migration of SubmitQueue's already-shipped controllers, storage, and protobuf mappings onto the shared type — each domain keeps its own `BuildRunner` interface and its own local `BuildID`/`BuildStatus`/`BuildMetadata`, and real code reuse happens one layer down, in a shared backend implementation (e.g. a Buildkite client) that both domains' concrete runners wrap. [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract) below records the shapes weighed against this one, including the shared-interface alternative that was set aside. 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` 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`). Artifact interpretation is delegated to `projectstatus.Resolver` and may later move into build analysis. ### Alternatives considered for sharing the contract @@ -259,7 +250,7 @@ Two alternatives to the runner-minted id are worth recording. They are independe #### Alternative A: caller-derived build key -Key the `Build` by identity derived from the Request — `buildKey(R) = R.ID` for the Phase-1 whole-repo build, a `{R.ID}/{hash(project)}` composite for Phase 2 — and store the runner's id in a separate `Build.RunnerBuildID` field (with a distinct wire type, so the two ids can never be passed for each other). +Key the `Build` by identity derived from the Request — `buildKey(R) = R.ID` for the validation build — and store the runner's id in a separate `Build.RunnerBuildID` field (with a distinct wire type, so the two ids can never be passed for each other). | Pros | Cons | |---|---| @@ -267,13 +258,13 @@ Key the `Build` by identity derived from the Request — `buildKey(R) = R.ID` fo | `Request` → `Build` navigation with no reverse index, per the KV key-derivation rule in [AGENTS.md](../../../../AGENTS.md) | The terminal build id is persisted on the Request so project-result resolution can use it without deriving a build id | | Enforces (rather than assumes) the direct-navigation property SubmitQueue's speculate takes on faith | Diverges entity shape and controller flow from SubmitQueue, weakening the "structurally the same controller" claim and dual-implementing-backend symmetry | -Trade-offs: the dedup guards a rare event at a permanent modeling cost. The duplicate it prevents arises only from a redelivery inside the trigger window — rare, and already harmless (identical scope; `buildsignal`'s superseded short-circuit and its first-writer-wins outcome CAS make the loser a no-op — see [Idempotency](#idempotency)). The prospective key-derivers — a future canceller, or `analyze` reaching back to the Phase-1 target graph — would need to be handed the id by their producing stage instead, if those designs land. +Trade-offs: the dedup guards a rare event at a permanent modeling cost. The duplicate it prevents arises only from a redelivery inside the trigger window — rare, and already harmless (identical scope; `buildsignal`'s superseded short-circuit and its first-writer-wins outcome CAS make the loser a no-op — see [Idempotency](#idempotency)). A future consumer needing the build identity can receive it from the producing stage or load `Request.TerminalBuildID` after completion. Moving `record`'s input from the build id to the request id does not require a caller-derived key. `buildsignal` persists the terminal build id with the Request's terminal state, and `record` reads that id directly from the Request. #### Alternative B: caller-supplied idempotency key on `Trigger` -Orthogonal to how the `Build` is keyed: give `Trigger` an extra parameter — a stable per-build token the caller already holds (`R.ID` in Phase 1) — that a runner supporting deduplication uses to **re-attach** a retried `Trigger` to the build it already started instead of spawning a second. The runner still returns its own native id; `Build.ID` stays runner-minted. Backends that cannot dedup ignore the token and degrade to today's behavior. +Orthogonal to how the `Build` is keyed: give `Trigger` an extra parameter — a stable per-build token the caller already holds (`R.ID` for the validation) — that a runner supporting deduplication uses to **re-attach** a retried `Trigger` to the build it already started instead of spawning a second. The runner still returns its own native id; `Build.ID` stays runner-minted. Backends that cannot dedup ignore the token and degrade to today's behavior. | Pros | Cons | |---|---| @@ -289,7 +280,7 @@ Either could be adopted independently: the idempotency token, if a backend that - **Shaped the same, not shared code**: the `BuildStatus` enum and `IsTerminal()` (nothing batch-specific — [build-runner.md](../../submitqueue/build-runner.md#buildstatus)); `BuildMetadata` (caller-supplied, provider-echoed, controller-uninterpreted — [#buildmetadata](../../submitqueue/build-runner.md#buildmetadata)); the async contract — `Trigger` returns a handle not an outcome, `Status` may round-trip, `Cancel` reaches the provider not the engine ([#async-vs-sync-contract](../../submitqueue/build-runner.md#async-vs-sync-contract)); and the id model — no caller-supplied id, the runner mints the build's identity, and that one `entity.BuildID` is the store key, queue payload, and `Status`/`Cancel` parameter (see [Alternatives considered](#alternatives-considered-for-the-build-identity)). These are duplicated locally in `stovepipe/entity`/`stovepipe/extension/buildrunner` rather than promoted to `platform/base`/`platform/extension/buildrunner` — see the contract above and [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract). - **Shared as implementation, not contract**: a concrete backend (e.g. Buildkite) can satisfy both domains' `BuildRunner` interfaces off one client, sharing HTTP/auth/poll-loop plumbing internally even though the two `BuildRunner` interfaces it implements are separate types — see the "Shared backend under `platform`, thin per-domain contracts" option above. -- **New in stovepipe**: URI-based scope in `Trigger` instead of batch lists — the one part of the contract that was always domain-specific by necessity, per [Why separate contracts](#why-separate-contracts). Mapping targets to projects is still stovepipe-only; how `analyze` obtains a target graph is left to its own design, out of scope for this doc. +- **New in stovepipe**: URI-based scope in `Trigger` instead of batch lists — the one part of the contract that was always domain-specific by necessity, per [Why separate contracts](#why-separate-contracts). Project analysis is Stovepipe-specific and delegated to `projectstatus.Resolver` in `record`. ## Entity and storage additions needed @@ -340,11 +331,11 @@ Single-key reads/writes only — no list-by-request, no query-by-attribute — p ## Queue contract additions -Two topic keys in `stovepipe/core/messagequeue/topics.go` — `TopicKeyBuild` (`process`/`analyze` → `build`) and `TopicKeyBuildSignal` (`build` → `buildsignal`) — and one proto message per key, since the contract test binds **exactly one message to each topic key** (see [messagequeue-contract.md](../../messagequeue-contract.md)). `ProcessRequest` is bound to `process` and cannot be reused; the new messages mirror its shape (one id field, its own `topic_keys` option): +Two topic keys in `stovepipe/core/messagequeue/topics.go` — `TopicKeyBuild` (`process` → `build`) and `TopicKeyBuildSignal` (`build` → `buildsignal`) — and one proto message per key, since the contract test binds **exactly one message to each topic key** (see [messagequeue-contract.md](../../messagequeue-contract.md)). `ProcessRequest` is bound to `process` and cannot be reused; the new messages mirror its shape (one id field, its own `topic_keys` option): -- `BuildRequest{ id }` (request id) → `topic_keys "build"`, produced by `process`/`analyze`, consumed by `build`. Phase 2's per-project trigger must also identify its project; because each topic key binds exactly one message, that lands as an **additive optional field on this same message** (protojson discards unknown fields, so the evolution is backward-compatible), not a second message type — the field's shape is deferred to the `analyze` design with the rest of the project-scoped trigger. +- `BuildRequest{ id }` (request id) → `topic_keys "build"`, produced by `process`, consumed by `build`. - `BuildSignal{ id }` (build id) → `topic_keys "buildsignal"`, produced by `build` and consumed by `buildsignal`. A non-terminal poll postpones that same delivery with `delivery.Hold`; `buildsignal` does not publish another `BuildSignal` (see [buildsignal.md](buildsignal.md)). ### Partitioning -`process`/`analyze` → `build` is partitioned by **request id**: per-request build work is independent (the per-Queue concurrency gate already ran in `process`), and a single Request's (rare) duplicate deliveries stay ordered — though with no pre-trigger dedup check that ordering is a tidiness property, not a correctness dependency; duplicates are absorbed downstream (see [Idempotency](#idempotency)). `build` → `buildsignal` is partitioned by **build id** (the runner-assigned `Build.ID`), so each build's poll loop is an independent partition. The id is unique per build, so one `Request`'s several Phase-2 project builds land in distinct partitions — the point: a slow poll on one must not block the others. The flip side of request-id partitioning on the `build` topic is that all of one Request's Phase-2 *triggers* serialize through a single partition; that is fine because `build` is trigger-only and cheap, and the concurrency Phase 2 needs comes from the per-build poll partitions, not from parallel triggering. This is a deliberate divergence from SubmitQueue, which partitions its build poll loop by *batch id* (its unit of work); stovepipe goes finer. +`process` → `build` is partitioned by **request id**: per-request build work is independent, and duplicate deliveries for one Request stay ordered. Duplicate builds are absorbed downstream (see [Idempotency](#idempotency)). `build` → `buildsignal` is partitioned by **build id**, the runner-assigned `Build.ID`, so a slow poll for one build does not block another. SubmitQueue instead partitions its build poll loop by batch id, its unit of work. diff --git a/doc/rfc/stovepipe/steps/buildsignal.md b/doc/rfc/stovepipe/steps/buildsignal.md index e1095acf5..3b8a79a20 100644 --- a/doc/rfc/stovepipe/steps/buildsignal.md +++ b/doc/rfc/stovepipe/steps/buildsignal.md @@ -2,15 +2,15 @@ `buildsignal` polls the build-runner until a build reaches a terminal state, records the status, and — once the build is terminal — releases the queue's build slot, projects the outcome onto the `Request`, and publishes the **request id** to `record`. See [workflow.md](../workflow.md) for where it sits in the pipeline and [build.md](build.md) for how the build it polls was triggered. -It handles only the poll loop: it does not decide build strategy, write greenness, or map targets to projects — those are `process`'s, `record`'s, and `analyze`'s jobs. It reports the *fact* (this request's build ended succeeded, failed, or cancelled); `record` derives what that fact *means* for greenness. +Build strategy belongs to `process`. Repository and project facts belong to `record`, which already invokes `projectstatus.Resolver` for project analysis. `buildsignal` reports the *fact* (this request's build ended succeeded, failed, or cancelled); `record` derives what that fact *means* for greenness. `buildsignal` is structurally the same controller as `submitqueue/orchestrator/controller/buildsignal/buildsignal.go`, and the "Flow", "Status delivery", and "Polling primitive" sections of [doc/rfc/submitqueue/build-runner.md](../../submitqueue/build-runner.md) are the reference rationale it reuses directly. The entity and storage shapes it depends on are defined in [build.md](build.md#entity-and-storage-additions-needed); this doc introduces none of its own. ## Input and re-entrancy -`buildsignal` consumes a build id, published by `build` — once per phase, since Phase 1 (whole-repo) and Phase 2 (per-project) each run their own `build` → `buildsignal` cycle against builds tied to the same `Request` (see [workflow.md](../workflow.md#workflow)). +`buildsignal` consumes a build id published by `build` (see [workflow.md](../workflow.md#workflow)). Project analysis uses the terminal build from this validation through `projectstatus.Resolver` in `record`. -Its logic does not branch on phase: it loads the `Build`, polls it toward terminal, persists the result, and publishes the request id onward to `record`. What differs between phases is what `record` does with that publish (whole-repo vs. per-project greenness) — not anything `buildsignal` decides. +It loads the `Build`, polls it toward terminal, persists the result, and publishes the request id onward to `record`, which records repository and project greenness. `buildsignal` is the sole writer of `Build.Status`/`Build.Version` after `build` creates the row (see [build.md](build.md#input-partitioning-and-the-single-writer-property)). It reads `Request` via `RequestStore.Get` (for `R.Queue`, to resolve the build-runner) and writes it exactly once, at the terminal transition, to record the build's outcome — the one `Request.State` write outside `process` and the DLQ reconciler. @@ -94,7 +94,7 @@ For a delivery carrying build id `B`: `Status` is not a point of conceptual divergence from SubmitQueue: both domains' `BuildRunner`s poll by the same opaque, runner-minted id with the same `Status(ctx, buildID) (BuildStatus, BuildMetadata, error)` signature. That similarity stays a shape, not a shared `platform/extension/buildrunner` interface — each domain keeps its own `BuildRunner` (including its own local `Status`), with reuse pushed to a shared backend implementation instead (see [build.md](build.md#alternatives-considered-for-sharing-the-contract)). `BuildMetadata` is caller-supplied and provider-echoed — the runner must not depend on it, but nothing stops a consumer from reading it. `buildsignal`'s own poll loop (steps 6-8) doesn't need to interpret it to decide when to stop polling, same as SubmitQueue's, but that's a statement about what the poll loop happens to need, not a rule that the value is unused: `build-runner.md` describes its purpose as round-tripping to users ([#buildmetadata](../../submitqueue/build-runner.md#buildmetadata)), and a future check in either domain's buildsignal is free to read it — e.g. to short-circuit some behavior — without changing the contract. -Returning `TargetGraph` from `Status` in place of `BuildMetadata`, with `buildsignal` persisting it for `analyze` to read later, was considered and set aside — how `analyze` obtains the target graph is left to its own design, not `buildsignal`'s poll loop. +Artifact extraction belongs to `projectstatus.Resolver` today and may move into the planned [build-analysis step](../workflow.md#build-analysis). `Status` remains the build-status contract; `buildsignal` persists that status and the terminal build identity for downstream work. ## Polling primitive: hold, not `Nack` diff --git a/doc/rfc/stovepipe/steps/record.md b/doc/rfc/stovepipe/steps/record.md index 5d0140027..1b0374f64 100644 --- a/doc/rfc/stovepipe/steps/record.md +++ b/doc/rfc/stovepipe/steps/record.md @@ -2,16 +2,15 @@ `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, then announces the outcome as a hook event. 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. +It records whole-repository greenness and already performs project analysis through [`projectstatus.Resolver`](../../../../stovepipe/extension/projectstatus/projectstatus.go), persisting one validation fact per returned project. On repository green it also advances the Queue's last-green bookmark and promotion ref, then announces the outcome as a repository hook event. See [`record.Controller`](../../../../stovepipe/controller/record/record.go). Notifying downstream systems rides the cross-domain hook framework rather than 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`. +`record` owns validation facts, the last-green bookmark, and the promotion ref. It delegates project analysis to the Queue's `projectstatus.Resolver`; that implementation determines project identities and degrees. Build scope belongs to `process`, while status polling and slot release belong to `buildsignal`. -## Phase 1 algorithm +## Algorithm For a delivery carrying a `Record` payload: @@ -76,9 +75,17 @@ Every decision after step 6 uses the persisted fact, not the outcome read from t Project-status processing does not delay baseline advancement or promotion. It still completes before the recorded notification. Bookmark advancement and ref promotion remain separate operations; this ordering does not make them atomic. +## Project analysis + +[`recordProjectFacts`](../../../../stovepipe/controller/record/record.go) obtains a resolver through `projectstatus.ResolverFactory.For(ResolverConfig{QueueName: request.Queue})`, then calls `Resolve(ctx, request, request.TerminalBuildID)`. The resolver performs repository-specific analysis and returns project identities and health degrees; an empty result is valid. The example server wires a noop resolver. + +Before writing project facts, `record` validates that project identities are nonempty and unique, and that degrees are finite and within `[0, 1]`. It creates each fact under `(queue, request.URI, project)` using the same immutable, idempotent write path as the repository fact. For nonempty results it also retains a `project_facts_recorded` request-history event. Resolver or persistence failures return an error; the repository fact and cache updates already made remain durable. + +As the planned [build-analysis step](../workflow.md#build-analysis) is implemented, artifact extraction and project-result calculation may move there to share observations with failure-episode tracking. In that arrangement, `record` would wait for final analysis results to be persisted for the relevant scope before writing immutable project facts. The runner guarantees artifact availability at build completion; this wait coordinates analysis persistence with recording. `record` would retain fact persistence, cache updates, and the repository hook. + ## 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. +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 for a project result. `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`. @@ -88,7 +95,7 @@ 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 | +| `Project` | Empty for the whole repository; stable project id returned by the resolver | | `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 | @@ -111,7 +118,7 @@ A fact is written only when the Request reaches `succeeded` or `failed`. A `canc 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 [DLQ and fail-closed behavior](#dlq-and-fail-closed-behavior). -### Phase 1 degree mapping +### Whole-repository degree mapping Whole-repository builds use only the endpoints, mapped from the outcome `buildsignal` stamped on the Request: @@ -123,7 +130,7 @@ Whole-repository builds use only the endpoints, mapped from the outcome `buildsi | `cancelled` | no fact | -Intermediate degrees are reserved for project analysis and deferred with the project mapping contract. Phase 1 does not manufacture fractional values. +Whole-repository facts use these endpoints. Project facts use the degree returned by `projectstatus.Resolver`, validated to be finite and within `[0, 1]`. ## Last-green advancement (Queue bookmark) @@ -173,7 +180,7 @@ The shared `HookEvent` contract at `api/base/hook/`, the hook extension at `plat 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 +create repository fact → advance bookmark (green only) → promote (bookmark holder only) → resolve and record project facts → publish repository HookEvent → ack ``` Last because the payload names the Request rather than snapshotting it, 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. @@ -198,7 +205,7 @@ So a hook reads state as it is when the hook runs, not as it was when the event ### 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. +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`. Project facts are already stored; publishing a per-project hook event is a separate future addition. `validation.repository.cancelled` is a separate type rather than a `state` on the recorded one, because the two differ in what a consumer can do with them: a recorded event carries a verdict to act on, a cancelled event only says to stop waiting. A sink that gates on greenness subscribes to one and ignores the other, and naming a cancellation "recorded" would claim a fact that was never written. ### What a consumer can and cannot assume @@ -211,16 +218,16 @@ Hooks here must be idempotent on `id`, as everywhere. "Fire-and-forget" describe ## 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. +The stage 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). +Request terminal state describes the build outcome; recording completion is tracked separately. 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. +- `Get(ctx, uri, project)` reads one fact by the rest of its identity, and returns `ErrNotFound` when none exists. The `project` argument is empty for the repository fact or identifies a project returned by the resolver. 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. @@ -301,15 +308,12 @@ Neither fits `ValidationFactStore`. Facts are looked up by exact identity and UR ### 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. +`record` retains a `project_facts_recorded` request-history event when project results are nonempty. That event covers project facts; the stage has no single durable marker saying all recording and notification work 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. +### Per-project hooks -Deciding project identity, retrieving the target graph, tracking completion, and defining intermediate degrees belong to `analyze.md`. +Project facts are already persisted. A future `validation.project.recorded` hook would announce each stable project identity with its own event ID. The Queue's last-green bookmark would continue to describe whole-repository greenness. diff --git a/doc/rfc/stovepipe/workflow.md b/doc/rfc/stovepipe/workflow.md index 2a746e7c0..a6fcbc518 100644 --- a/doc/rfc/stovepipe/workflow.md +++ b/doc/rfc/stovepipe/workflow.md @@ -6,14 +6,16 @@ The pipeline is a queue-driven chain of small, single-purpose controllers, in th > **poll for a new head → ingest → process the build strategy → build → buildsignal → record whole-repo greenness (and any project results the resolver returns) → hook.** -There is no `analyze` controller and no `analyze` topic. A later project-analysis stage that would map a target graph onto another `build` pass is design only, described under [Designed, not built](#designed-not-built-project-analysis). `record` resolves project results inline on the same delivery that writes the repository fact. +The planned `buildanalysis` step runs alongside that chain: it extracts observations, updates failure episodes and recovery, and requests repairs. It runs periodically while a build is active and performs a final pass after completion. Health tracking is a responsibility of `buildanalysis`; failure episodes are the durable records it maintains. The [workflow diagram](#workflow) shows the intended flow through repair decisions, execution, and recovery; [Build analysis](#build-analysis) describes the step. + +Project analysis is already implemented in `record` through [`projectstatus.Resolver`](../../../stovepipe/extension/projectstatus/projectstatus.go). It resolves project results and records their validation facts on the same delivery as the repository fact. As build analysis is added, project-result extraction and calculation may move there so project status and breakage tracking can share the same observations; `record` would continue to persist validation facts. See [Project analysis](#project-analysis). ## What Stovepipe is agnostic about Two deliberate abstractions keep Stovepipe from being a git tool or a Bazel tool: - **The VCS is behind a `SourceControl` extension.** Stovepipe never shells out to git. Every commit, ref, and branch head is an opaque **URI** that a `SourceControl` implementation produces and interprets. A ref is `git://remote/repo/ref/…`; a specific commit is `git://remote/repo/ref/…/`. The `git://` scheme is just the reference implementation — a Mercurial or Perforce backend would mint its own scheme behind the same contract. Nothing downstream of `SourceControl` parses a URI; it is a token you hand back to `SourceControl` to ask questions ("is A an ancestor of B?", "what is the head of this ref?"). -- **The build system is behind a build-runner extension** (see [build-runner.md](../submitqueue/build-runner.md)). `Trigger` starts a build at a head URI, optionally against a baseline URI, and `Status` returns pass/fail plus metadata. A target graph that a separate analyze stage would map to projects is part of the unbuilt design, not this contract. +- **The build system is behind a build-runner extension** (see [build-runner.md](../submitqueue/build-runner.md)). `Trigger` starts a build at a head URI, optionally against a baseline URI, and `Status` returns pass/fail plus metadata. Project results are resolved separately through `projectstatus.Resolver`. Designing to these contracts — not to git and Bazel specifically — is the whole point: the same pipeline should validate any branch in any VCS built by any build system. @@ -39,11 +41,11 @@ Identity for the *head* is the `(Queue, head URI)` pair, and that pair is the ** ### Greenness — a degree, not a boolean -Greenness is recorded as a **health degree** where **`0` means green** and **higher means more broken**, with **`1` meaning fully broken**. Stovepipe starts with only the two endpoints — `0` (green) and `1` (broken) — at the whole-repo level. The space between exists so that, once project-level analysis lands, "broken to some extent" (a fraction of projects failing) has somewhere to live without changing the contract or the state machine. A commit that has been ingested but not yet validated has *no* recorded greenness for the relevant scope — absence is distinct from `0`, and callers must treat "not yet recorded" as not-green for gating. +Greenness is recorded as a **health degree** where **`0` means green** and **higher means more broken**, with **`1` meaning fully broken**. Stovepipe starts with only the two endpoints — `0` (green) and `1` (broken) — at the whole-repo level. Project resolvers can already return intermediate degrees for partial breakage; each implementation defines how it calculates them. A commit that has been ingested but not yet validated has *no* recorded greenness for the relevant scope — absence is distinct from `0`, and callers must treat "not yet recorded" as not-green for gating. ### Project — greenness at a finer grain -A **project** is a caller-defined slice of the repository. Whole-repo greenness answers "is the branch green at this URI"; project greenness answers the question deployments actually need — **"is *this project* green at this URI"**, and its dual, "what is the latest URI at which this project is green". The running pipeline does not derive projects from a target graph. On a succeeded or failed request, `record` gives the request's terminal build id to `projectstatus.Resolver` and writes one validation fact per result it returns. The example server wires the noop resolver, which returns no results. How a resolver chooses projects is implementer-specific. The separate analyze stage that would map a target graph to project-scoped builds is not built; see [Designed, not built](#designed-not-built-project-analysis). +A **project** is a caller-defined slice of the repository. Whole-repo greenness answers "is the branch green at this URI"; project greenness answers the question deployments actually need — **"is *this project* green at this URI"**, and its dual, "what is the latest URI at which this project is green". Project analysis runs today in [`record`](steps/record.md#project-analysis). On a succeeded or failed request, `record` gives the request and its terminal build id to `projectstatus.Resolver` and writes one validation fact per result it returns. The example server wires the noop resolver, which returns no results. How a resolver chooses projects is implementer-specific. This derives project status from the existing validation build. ### Promotion ref — the last green commit, by name @@ -57,6 +59,8 @@ 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. `Trigger` returns a build id; `Status` returns pass/fail and the caller-supplied metadata it echoed. It does not return a target graph. See [build-runner.md](../submitqueue/build-runner.md). | +| **projectstatus.Resolver** | Resolve project identities and health degrees from a terminal validation. Called by `record` today; extraction and calculation may move into build analysis. | +| **buildanalysis.FailedTargetReader** (planned) | Read available failure reports for a referenced Build, including check and attempt identities, report availability, completeness, and automatic retry disposition. | | **Hooks** | Deliver Stovepipe's validation events to downstream systems. What is published today is repository-scoped: validation of this URI has begun, this URI is green or not green, and a cancelled validation ended without a fact. Fire-and-forget notification, decoupled so Stovepipe does not know or care who consumes the event. 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 [AGENTS.md](../../../AGENTS.md). | @@ -66,50 +70,75 @@ Hooks are the notification boundary. When validation of a commit begins, when a What runs is one pass per Request. It establishes whole-repository greenness, and on a succeeded or failed outcome `record` also gives the Request's terminal build id to `projectstatus.Resolver` before writing whatever project facts it returns. That call is inline on the record delivery. It does not publish to another stage, and it does not start another build. -``` - external poller ──(Queue name)──► ┌──────────────────────────────┐ - "Queue moved" (outside OSS) │ ingest │ - │ Resolve head URI via │ - │ SourceControl; mint Request; │ - │ persist (greenness: none); │ - │ dedup on (Queue, head URI) │ - └───────────────┬──────────────┘ - │ RequestID - ▼ - ┌──────────────────────────────┐ Hooks - │ process │┄┄┄┄┄► "validation - │ Ask SourceControl: is head a │ started" - │ descendant of last-green? │ - │ → incremental since green │ - │ else (history rewrite) │ - │ → full monorepo │ - └───────────────┬──────────────┘ - │ RequestID (+ strategy, baseline URI) - ▼ - ┌──────────────────────────────┐ - │ build │ - │ Run build-runner for the │ - │ chosen scope; baseline = │ - │ last-green URI iff incremental│ - └───────────────┬──────────────┘ - │ BuildID - ▼ - ┌──────────────────────────────┐ - │ buildsignal │ - │ Poll until terminal; record │ - │ status; release the slot; │ - │ project the outcome │ - └───────────────┬──────────────┘ - │ RequestID - ▼ - ┌──────────────────────────────┐ Hooks - │ record │┄┄┄┄┄► "URI green, - │ Write whole-repo greenness; │ not green, or - │ on green advance last-green │ cancelled" - │ and the promotion ref; │ - │ resolve project results │ - │ inline │ - └──────────────────────────────┘ +The diagram shows the intended workflow. Build analysis, including failure-episode tracking, and repair are planned additions to the validation pipeline. Their runtime implementations follow this design. + +```mermaid +flowchart TD + poller["External poller: Queue moved"] + + subgraph validation["Validation pipeline"] + ingest["ingest
Resolve head; persist and deduplicate Request"] + process["process
Choose scope; admit under Queue concurrency limit"] + build["build
Trigger build-runner; persist Build"] + buildsignal["buildsignal
Poll build status; on completion release slot
and project outcome onto Request"] + record["record
Analyze projects via projectstatus.Resolver;
write repository and project facts;
advance last-green and promotion ref on green"] + hook["hook
Deliver validation events"] + + ingest -->|RequestID| process + process -->|RequestID, scope, baseline| build + build -->|BuildID| buildsignal + buildsignal -->|RequestID| record + process -. validation started .-> hook + record -. recorded or cancelled .-> hook + end + + subgraph analysis["Build analysis: planned queue step"] + buildanalysis["buildanalysis.Controller.Process
Reload Build; extract available details
through buildanalysis.FailedTargetReader"] + observations["Persist build observations
Failed checks, evidence references,
report completeness and coverage"] + episodes["Update failure episodes and recovery;
preserve current FixID assignments"] + + buildanalysis --> observations + observations --> episodes + buildanalysis -. "Hold: periodic poll while build is active" .-> buildanalysis + end + + subgraph repair["Repair workflow: design; runtime not wired"] + requestFix["fix.Controller.Process
Reload episode and current assignment"] + evaluate["fixer.RepairEvaluator.Evaluate
Compare evidence with active repairs"] + decision{"Start, Attach, or Defer?"} + startFix["Persist new repair;
update active IDs and episode assignment"] + attach["Validate existing repair;
persist episode association"] + deferFix["Keep pending;
Hold until next evaluation"] + submit["fix.Controller.Process
Recheck ownership;
persist submitting state;
RepairExecutor.Submit"] + reconcile["fixsignal.Controller.Process
Reload repair and failure episodes;
observe execution and check recovery"] + executor["fixer.RepairExecutor.Observe"] + pendingRecovered["Resolve episode recovered
before dispatch"] + recovered["All episodes recovered
and known execution settled"] + + requestFix -->|Unassigned failure| evaluate + requestFix -->|Existing execution: FixSignal| reconcile + requestFix -. Resume assigned repair .-> submit + requestFix -->|Recovered before assignment| pendingRecovered + evaluate --> decision + decision -->|Start| startFix + decision -->|Attach| attach + decision -->|Defer| deferFix + attach -. Candidate changed .-> evaluate + deferFix --> evaluate + startFix -->|New repair| submit + submit -->|Execution ID persisted: FixSignal| reconcile + submit -->|Recovered before submission| pendingRecovered + attach -->|FixSignal: existing fix ID| reconcile + reconcile --> executor + executor -->|Execution and change status| reconcile + reconcile -. Hold between observations .-> reconcile + reconcile -->|Recovery and known execution settled| recovered + end + + poller -->|Queue name| ingest + build -. "After Build is persisted: BuildID" .-> buildanalysis + buildsignal -. "After terminal outcome is persisted: BuildID" .-> buildanalysis + episodes -->|FixRequest after persisted episode update| requestFix ``` 1. **ingest** — invoked by the external poller with a **Queue name**. It asks `SourceControl` for that Queue's current head URI, mints a Request namespaced by the Queue, persists it with no recorded greenness yet, and dedups on `(Queue, head URI)` so a re-reported head is processed once. It publishes the RequestID onward. @@ -118,9 +147,24 @@ What runs is one pass per Request. It establishes whole-repository greenness, an 4. **buildsignal** — polls until the build is terminal, records that status, releases the Queue's `in_flight_count` slot, projects the terminal status and build id onto the Request (`succeeded` / `failed` / `cancelled`), and publishes the RequestID to `record`. 5. **record** — for a succeeded or failed Request, 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 then gives the Request's terminal build id to `projectstatus.Resolver` and writes one validation fact per returned result. The example server uses the noop resolver, so that list is empty unless a deployment supplies another. It publishes `validation.repository.recorded` for that fact. The Queue's `in_flight_count` was already released by `buildsignal` when the build went terminal. A cancelled Request writes no fact and publishes `validation.repository.cancelled`, so a consumer can stop waiting on the commit. -### Designed, not built: project analysis +### Build analysis + +Add `stovepipe/controller/buildanalysis.Controller` implementing [`consumer.Controller.Process`](../../../platform/consumer/controller.go), with a `BuildAnalysis` message and `TopicKeyBuildAnalysis` in [`stovepipe/core/messagequeue`](../../../stovepipe/core/messagequeue/messagequeue.go). Messages carry the Queue name and BuildID; processing reloads the Build and its Request from storage. + +1. **Start analysis after build creation.** Extend [`build.Controller.Process`](../../../stovepipe/controller/build/build.go) to publish the BuildID to `buildanalysis` after persisting the Build. Keep the existing handoff to `buildsignal` and make both publications safe to replay. +2. **Extract available details on each poll.** Resolve the planned `buildanalysis.FailedTargetReader` extension for the Queue and pass it the BuildID. Its implementation reads available reports through injected dependencies; the reader contract is being drafted in [#898](https://github.com/uber/submitqueue/pull/898). Reuse the extraction infrastructure underlying [`projectstatus.Resolver`](../../../stovepipe/extension/projectstatus/projectstatus.go). While the Build is active, use `delivery.Hold(...)` for the next analysis pass, partitioned by BuildID so each build progresses independently. +3. **Persist observations.** Retain the BuildID, each report's CheckID and AttemptID, failed item identities, report availability, completeness, retry disposition, and configured coverage. Map failed items to stable queue-scoped keys including the validation predicate and relevant configuration. Deduplicate repeated reads and reject superseded observations; opaque attempt IDs identify executions but do not order them. Project analysis can reuse these observations. +4. **Update failure episodes and request repair.** In the same `buildanalysis` delivery, use persisted observations to create or update episodes under `(queue, check key, generation)`, preserving the first failure build and current `FixID` assignment. Verified recovery closes an episode; a later failure creates a new generation. Build retries are observations within an episode, rather than new episode generations. Persist episode updates before publishing a `FixRequest` for a new failure, recovery, or evidence that warrants reevaluation. Replay completes interrupted writes and publications. An available failed report can trigger repair while other checks are still running. Apply queue policy to the report's automatic retry disposition when deciding when to request repair; diagnosis can emerge during evaluation or repair. +5. **Run a final pass at completion.** Extend [`buildsignal.Controller.Process`](../../../stovepipe/controller/buildsignal/buildsignal.go) to publish a completion analysis signal after persisting the terminal outcome. The runner guarantees that all artifacts are available once `buildsignal` detects completion, so this pass reads the final artifacts immediately. Initial and completion signals use distinct message identities; retries reuse each signal's identity. A periodic delivery that observes completion uses the same idempotent final-analysis path. Artifact-read or analysis failures follow normal error handling. A cancelled build may still supply useful failure evidence; record the coverage actually obtained. +6. **Share the results with downstream work.** Keep build status polling and slot release in `buildsignal`; keep validation facts in [`record`](../../../stovepipe/controller/record/record.go). Project analysis already runs through `projectstatus.Resolver` in `record`. Its extraction and project-result calculation may move into build analysis to share observations with failure-episode tracking. In that arrangement, `record` waits for final analysis results to be persisted for the relevant scope before writing immutable project facts. This coordinates analysis completion with recording; the runner makes final artifacts available at build completion. For failure-only reports, infer recovery only from a complete, comparable report covering the check. The job configuration supplies coverage; the CI artifact can continue to list only failures. + +The initial and completion handoffs follow the existing persist-then-publish retry pattern. Analysis holds only its own per-build partition. Versioned writes make repeated polls and completion signals converge while preserving final results against stale updates. Episode updates preserve independent repair assignments: `buildanalysis` writes failure and recovery evidence, while `fix` conditionally updates the authoritative `FixID`. `fixsignal` reads these records to determine recovery. Repair membership and episode assignment converge through replay; concurrent evaluations can occasionally start overlapping repairs. + +### Project analysis + +Project analysis is implemented by [`record.Controller.recordProjectFacts`](../../../stovepipe/controller/record/record.go). For a succeeded or failed Request, it resolves the Queue's `projectstatus.Resolver`, calls `Resolve(ctx, request, request.TerminalBuildID)`, validates the returned project identities and degrees, and creates a `ValidationFact` per project. This runs after the repository fact and derived caches are updated, and before the repository hook is published. The example server supplies a noop resolver; deployments supply their own project analysis. -An `analyze` stage is not part of the pipeline. `stovepipe/core/messagequeue` has no analyze topic, and `stovepipe/controller` has no analyze package. The design, still only a design, is a stage that would take a build's target graph, map broken or at-risk targets to projects, and publish project-scoped builds back through `build` → `buildsignal` → `record`. That second pass, a per-project hook event, and intermediate greenness degrees are not what the controllers do. Open questions for that design stay at the bottom of this doc. +Build analysis offers a possible home for artifact extraction and project-result calculation when it is implemented. Moving that work would let project status and failure-episode tracking share evidence gathered during the build and at completion. `record` would read the stored results and remain responsible for immutable validation facts, bookmark advancement, promotion, and the repository hook. It would wait for the final analysis results to be persisted for the relevant scope before recording project facts. ## Per-controller summary @@ -128,9 +172,12 @@ An `analyze` stage is not part of the pipeline. `stovepipe/core/messagequeue` ha |---|---|---|---| | **ingest** | Queue name (from poller) | process | Resolve head URI via SourceControl, mint Request, persist (no greenness), dedup on `(Queue, head URI)` | | **process** | RequestID | build, hook topic | Build strategy, concurrency gate, backlog coalescing; announce validation start on admit → [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 | Record terminal build status; release `in_flight_count`; project the outcome onto the Request; publish the request id | -| **record** | RequestID | hook topic | Write whole-repo greenness; resolve project results inline; on green advance last-green URI and the promotion ref; publish the recorded or cancelled repository hook event | +| **build** | RequestID | buildsignal; planned buildanalysis | Run the build-runner for the chosen scope; baseline = last-green URI iff incremental; start analysis after persisting the Build | +| **buildsignal** | BuildID | record; planned buildanalysis | Record terminal build status; release `in_flight_count`; project the outcome onto the Request; request final analysis | +| **buildanalysis** (planned) | BuildID after creation or completion; periodic redelivery | Persisted observations and failure episodes; fix | Extract evidence, update failure and recovery, preserve repair assignments, and publish `FixRequest` after episode updates | +| **fix** (planned) | FixRequest with episode identity | fixsignal | Evaluate Start, Attach, or Defer; persist repair membership and episode assignment; submit new work through `RepairExecutor.Submit` | +| **fixsignal** (planned) | FixSignal with fix ID | Periodic redelivery while pending | Call `RepairExecutor.Observe`; check episode recovery; resolve repairs when health and execution are settled | +| **record** | RequestID | hook topic | Write whole-repo greenness; analyze projects through `projectstatus.Resolver` and persist their facts; on green advance last-green URI and the promotion ref; publish the recorded or cancelled repository hook event | | **hook** | HookEvent | — | Invoke the hooks the resolver returns. The example server resolves every event to noop | ## Step RFCs @@ -140,7 +187,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, 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 hook event announcing the outcome. Its Phase 2 / analyze handoff is the unbuilt design, not a topic `record` publishes +- [record.md](steps/record.md) — turning a terminal build outcome into an immutable validation fact, monotonic last-green advancement and ref promotion, and the hook event announcing the outcome. Project analysis already runs inline through `projectstatus.Resolver` and may later use results calculated by build analysis ## Dedup, idempotency, and history rewrites @@ -148,10 +195,10 @@ 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. 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. +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#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 -- **Greenness degree semantics.** The endpoints (`0` green, `1` fully broken) are fixed; the meaning of intermediate values once projects exist (fraction of projects broken? weighted severity?) is deferred until project analysis is concrete. +- **Greenness degree semantics.** Whole-repository facts use `0` (green) or `1` (broken). Project resolvers can already return degrees in `[0, 1]`; a shared interpretation of intermediate values, such as a weighted failure fraction, remains open. - **Poller vs. webhook ingestion.** Only the external poller is in scope now. The dedup key is designed so a webhook producer can be added later without changing identity, but that producer is out of scope for this RFC. -- **Project mapping contract.** Not built. There is no analyze controller or topic. `record` already persists facts from `projectstatus.Resolver` on the same delivery as the repository fact. The unbuilt analyze design — a target-graph mapping, project-scoped builds, and whether that mapping is an extension or an external service — is still open. +- **Project analysis placement.** `record` invokes `projectstatus.Resolver` today. When build analysis is implemented, decide whether to move extraction and project-result calculation there, with a completion handoff that lets `record` consume complete results before persisting project facts.