uber · prathshenoy · Sep 24, 2026
@@ -54,8 +54,9 @@ For a delivery carrying request id `R`:
 
 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).
+   - the row carries no validation scope; that is recoverable from the Request's immutable
+     fields (see the entity table). When a build wins the terminal-outcome race, buildsignal
+     records its id as `Request.TerminalBuildID`.
    - 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.
@@ -69,7 +70,7 @@ For a delivery carrying request id `R`:
 8. ack.
 ```
 
-`Build.ID` is **minted by the runner at `Trigger`**, exactly as in SubmitQueue's build controller: the runner returns its native id (a Buildkite build number, a CI-gateway job id), `build` adopts it as the `Build`'s key, and that same id travels on every hop that needs a build — `build` → `buildsignal` carries the build id in the message, so the poll loop reaches the `Build` by a direct get on identity it was handed. `buildsignal` → `record` carries the **request id** instead: `record`'s unit of work is a Request, and the build's terminal status is projected onto `Request.State` before the publish, so `record` never reaches a `Build` at all. No reader ever *derives* a build id or needs a reverse index; `Build.RequestID` covers the one navigation the pipeline needs in the other direction (`Build` → `Request`). Another approach is deriving the key from the Request (`buildKey(R)`) and/or passing a caller-supplied idempotency key to `Trigger`; see [Alternatives considered](#alternatives-considered-for-the-build-identity) for what each would buy and cost.
+`Build.ID` is **minted by the runner at `Trigger`**, exactly as in SubmitQueue's build controller: the runner returns its native id (a Buildkite build number, a CI-gateway job id), `build` adopts it as the `Build`'s key, and that same id travels on every hop that needs a build — `build` → `buildsignal` carries the build id in the message, so the poll loop reaches the `Build` by a direct get on identity it was handed. `buildsignal` → `record` carries the **request id** instead: `record`'s unit of work is a Request, and buildsignal records the winning build id on that Request before publishing. A later artifact reader reads `Request.TerminalBuildID`; it does not query builds by RequestID. Another approach is deriving the key from the Request (`buildKey(R)`) and/or passing a caller-supplied idempotency key to `Trigger`; see [Alternatives considered](#alternatives-considered-for-the-build-identity) for what each would buy and cost.
 
 `build` writes only the `Build`, and only at creation; it never mutates `Request.State`. The Request stays `processing` (set by `process`) through `build` until `buildsignal` moves it terminal by recording the build's outcome. `Build.Status` is the fine-grained build lifecycle; `Request.State` is the coarse pipeline lifecycle. This is also what keeps `process.md` step 3 correct: because `build` leaves the Request at `processing`, a redelivered `process` message still matches its "if processing, re-publish to build" guard.
 
@@ -200,7 +201,7 @@ Trigger(ctx context.Context, baseURI, headURI string, projectScope entity.Projec
 
 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.
+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 terminal request-history state entry retains the winning id alongside the terminal Request state; it is selected by buildsignal's outcome CAS, not a derived key or a reverse index. Per the extension rules a runner keeps only transient local state, so that entry and `Build.RequestID` live in **our** store, 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.
 
@@ -264,12 +265,12 @@ Key the `Build` by identity derived from the Request — `buildKey(R) = R.ID` fo
 | Pros | Cons |
 |---|---|
 | Redelivery dedup by direct get: checking `BuildStore.Get(buildKey(R))` before triggering means at-least-once delivery never starts a second build | A second id concept (`Build.ID` beside `Build.RunnerBuildID`) carried by every entity, signature, and reader forever |
-| `Request` → `Build` navigation with no reverse index, per the KV key-derivation rule in [AGENTS.md](../../../../AGENTS.md) | No current reader needs to *derive* a build id — the id travels in every message hop, so each consumer already holds the key it needs |
+| `Request` → `Build` navigation with no reverse index, per the KV key-derivation rule in [AGENTS.md](../../../../AGENTS.md) | `record` needs the winning build's id, which `Request.TerminalBuildID` supplies directly; changing the Build key would still add a second identity |
 | 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.
 
-Note that moving `record`'s input from the build id to the request id does *not* trigger this alternative, even though it removes the last hop that carried a build id to a Request-scoped consumer. The trigger condition is a stage that must **derive a build's key from a Request**, and `record` does not: the build's terminal status is projected onto `Request.State` before the publish, so `record` reads the Request and never reaches a `Build`.
+Moving `record`'s input from the build id to the request id now requires the winning identity to survive that handoff. The trigger condition for this alternative is still a stage that must **derive a build's key from a Request**. `record` does not: buildsignal retains the terminal state and its winning build id before publishing, so `record` can resolve that state entry without a reverse Build index.
 
 #### Alternative B: caller-supplied idempotency key on `Trigger`
 
@@ -318,7 +319,7 @@ The row deliberately carries no scope: `R.URI`, `R.BaseURI`, and `R.BuildStrateg
 
 `IsTerminal()` on `entity.BuildStatus` covers exactly the three terminal rows. Once `buildsignal` persists one of them, that status is **write-once** — a later poll reporting a different terminal value never overwrites it (see [buildsignal.md](buildsignal.md#algorithm), step 6).
 
-Plus the `BuildID{ID string}` wire type in `stovepipe/entity` (same "id only travels" convention as `RequestID`, shaped like SubmitQueue's own `entity.BuildID` but not the same Go type — see the [contract](#stovepipe-buildrunner-contract)), wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, the queue payload, `Status`/`Cancel`'s parameter. `buildsignal` reaches a build by the id carried in its message, and `record` reads the `Request` (whose state carries the build's outcome) rather than a `Build`, so no reverse index from `Request` to its builds is ever needed.
+Plus the `BuildID{ID string}` wire type in `stovepipe/entity` (same "id only travels" convention as `RequestID`, shaped like SubmitQueue's own `entity.BuildID` but not the same Go type — see the [contract](#stovepipe-buildrunner-contract)), wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, the queue payload, `Status`/`Cancel`'s parameter. `buildsignal` reaches a build by the id carried in its message, then records the winning id as `Request.TerminalBuildID`. An artifact reader reads that field instead of querying a Build by request.
 
 **`BuildStore`** (new, added to the `Storage` aggregator via `GetBuildStore()`), matching stovepipe's existing `RequestStore` conventions — **generic `Update` with caller-owned version arithmetic**:
 

@@ -12,7 +12,7 @@ It handles only the poll loop: it does not decide build strategy, write greennes
 
 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.
 
-`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.
+`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, recording the build's outcome and `Request.TerminalBuildID` — the one `Request.State` write outside `process` and the DLQ reconciler.
 
 Its early-exit guard is deliberately narrower than `State.IsTerminal()`: it proceeds when the request is `processing` **or** already carries a build outcome. The second case matters because a redelivery after the outcome was stamped but before the `record` publish landed must re-publish rather than drop the signal; everything it re-runs is a no-op (the status is unchanged, the outcome is already recorded, the slot is not released twice) and the `record` publish is idempotent.
 
@@ -63,12 +63,14 @@ For a delivery carrying build id `B`:
 7. If the stored status is terminal, and R does not already carry an outcome:
    a. Release the queue's build slot: CAS-decrement Queue.in_flight_count, clamped at zero.
       - failure here aborts the step: R must not go terminal while still holding a slot.
-   b. CAS R from processing to the outcome the stored status projects onto it:
+   b. CAS R from processing to the outcome the stored status projects onto it and set
+      R.TerminalBuildID to B:
       succeeded -> succeeded, failed -> failed, cancelled -> cancelled. First writer wins.
    Then publish R.ID to the record topic, partitioned by request id; ack, return.
    No re-publish to buildsignal.
-   - record loads the Request directly by this key and derives greenness from its outcome,
-     so it never reaches a Build and no reverse lookup from Request to its builds is needed.
+   - record loads the Request directly by this key and derives greenness from its outcome.
+     A later artifact reader reads R.TerminalBuildID; no reverse lookup from Request to its
+     builds is needed.
    - the message id is the request id, so a redelivery republishing the same terminal signal
      dedups into the original message; record is idempotent regardless.
    - publish failure -> return raw (non-retryable); the outcome is persisted, operational

@@ -177,7 +177,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
 		if err := c.persistBuildFinishedLog(ctx, store, request, build.ID); err != nil {
 			return err
 		}
-		if err := c.finishRequest(ctx, store, &request, effective); err != nil {
+		if err := c.finishRequest(ctx, store, &request, effective, build.ID); err != nil {
 			return err
 		}
 		if err := c.persistOutcomeLog(ctx, store, request); err != nil {
@@ -234,7 +234,7 @@ func (c *Controller) persistBuildFinishedLog(ctx context.Context, store storage.
 // the request non-terminal, so redelivery re-runs both steps and decrements again
 // — transiently over-admitting by one until releaseBuildSlot's zero clamp
 // reconverges, which is the failure mode this pipeline prefers.
-func (c *Controller) finishRequest(ctx context.Context, store storage.Storage, request *entity.Request, status entity.BuildStatus) error {
+func (c *Controller) finishRequest(ctx context.Context, store storage.Storage, request *entity.Request, status entity.BuildStatus, buildID string) error {
 	if request.State.HasBuildOutcome() {
 		return nil
 	}
@@ -244,7 +244,7 @@ func (c *Controller) finishRequest(ctx context.Context, store storage.Storage, r
 		return err
 	}
 
-	if err := c.markOutcome(ctx, store, request, outcomeState(status)); err != nil {
+	if err := c.markOutcome(ctx, store, request, outcomeState(status), buildID); err != nil {
 		metrics.NamedCounter(c.metricsScope, _opName, "storage_errors", 1, metrics.TagsFromContext(ctx)...)
 		return err
 	}
@@ -290,7 +290,7 @@ func outcomeState(status entity.BuildStatus) entity.RequestState {
 // conflicts. First writer wins: once any outcome is recorded a later caller leaves it
 // alone, so duplicate builds for one request (which build.md accepts) cannot flip the
 // verdict back and forth.
-func (c *Controller) markOutcome(ctx context.Context, store storage.Storage, request *entity.Request, state entity.RequestState) error {
+func (c *Controller) markOutcome(ctx context.Context, store storage.Storage, request *entity.Request, state entity.RequestState, buildID string) error {
 	reqStore := store.GetRequestStore()
 
 	for {
@@ -300,6 +300,7 @@ func (c *Controller) markOutcome(ctx context.Context, store storage.Storage, req
 
 		updated := *request
 		updated.State = state
+		updated.TerminalBuildID = buildID
 		newVersion := request.Version + 1
 		if err := reqStore.Update(ctx, updated, request.Version, newVersion); err != nil {
 			if errors.Is(err, storage.ErrVersionMismatch) {

@@ -178,7 +178,9 @@ func expectFinishWrites(m buildsignalMocks, state entity.RequestState) *gomock.C
 	eventCall := expectBuildFinished(m)
 	m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow(1, 4), nil).After(eventCall)
 	m.queueStore.EXPECT().Update(gomock.Any(), queueRow(0, 4), int32(4), int32(5)).Return(nil)
-	return m.reqStore.EXPECT().Update(gomock.Any(), requestWithState(state), int32(1), int32(2)).Return(nil)
+	request := requestWithState(state)
+	request.TerminalBuildID = testBuildID
+	return m.reqStore.EXPECT().Update(gomock.Any(), request, int32(1), int32(2)).Return(nil)
 }
 
 func expectBuildFinished(m buildsignalMocks) *gomock.Call {
@@ -214,6 +216,22 @@ func expectOutcomeLog(m buildsignalMocks, state entity.RequestState, version int
 	).Return(nil)
 }
 
+func TestMarkOutcomePreservesFirstTerminalBuild(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	c, m := newController(t, ctrl)
+	request := requestWithState(entity.RequestStateProcessing)
+	winner := requestWithState(entity.RequestStateSucceeded)
+	winner.Version = 2
+	winner.TerminalBuildID = "winning-build"
+
+	m.reqStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(storage.ErrVersionMismatch)
+	m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(winner, nil)
+
+	err := c.markOutcome(context.Background(), m.store, &request, entity.RequestStateFailed, "losing-build")
+	require.NoError(t, err)
+	assert.Equal(t, winner, request)
+}
+
 func TestProcess(t *testing.T) {
 	tests := []struct {
 		name    string

@@ -116,6 +116,9 @@ type Request struct {
 
 	// State is the current state of the request in the pipeline.
 	State RequestState `json:"state"`
+	// TerminalBuildID identifies the build that established the terminal state.
+	// It is empty until a build reaches a terminal state.
+	TerminalBuildID string `json:"terminal_build_id"`
 	// Version is the version of the object. It is used for optimistic locking.
 	// Versioning starts at 1 and is incremented for each change to the object.
 	Version int32 `json:"version"`