feat(stovepipe): record the build outcome on the request and free its slot

[behinddwalls]

Summary

Why?

Nothing wrote the build outcome states the previous commit introduced, and nothing released a request's build slot on the normal path. The slot was specified as record's to release, but record does not exist — so today every admitted request holds its slot forever and only the DLQ reconciler ever frees one.

Releasing it in record is also the wrong home. The gate process claims is a build slot: it bounds concurrent builds per Queue, and once the build reaches a terminal status the build is over. Waiting for a later stage to notice keeps the gate closed for no reason, and it breaks a useful invariant — a terminal request has already released its slot — which is exactly what makes the DLQ reconciler's early-return on terminal requests safe.

What?

When a build reaches a terminal status, buildsignal now releases the queue's build slot and CAS-transitions the request from processing to the matching outcome, then publishes to record as before. The transition is first-writer-wins, so the duplicate builds that at-least-once delivery can produce cannot flip a request's verdict back and forth.

Two ordering rules, both serving one invariant — the request must not go terminal while still holding a slot, because a terminal request is skipped by redelivery and by the reconciler alike, so nothing would ever decrement it:

• the slot is released before the terminal write, and
• a failed release aborts the write.

Failing this way leaves the request non-terminal, so redelivery re-runs both steps and decrements twice — transiently over-admitting by one until the zero clamp reconverges. That is the same trade the DLQ reconciler already documents and makes, for the same reason.

The early-exit guard also narrows. With outcomes now terminal, the old IsTerminal() check would return before republishing to record, losing the message if the process died between the stamp and the publish. The guard now proceeds when the request is processing or already carries an outcome; falling through in the second case is safe, since the re-poll is redundant but harmless, the status and outcome writes both no-op, and the slot is not released twice.

It is phrased that way rather than as "skip if superseded" because a superseded request cannot reach the poll loop at all — process supersedes solely from accepted, and a build only exists once the request was admitted. Naming an unreachable state would have implied a case that does not exist. The guard is still kept rather than dropped: the slot release is conditioned on not already having an outcome, so a terminal-without-an-outcome request arriving here would decrement a slot that superseding never claimed.

This also fixes a stale RFC sentence in process.md that still described the slot as released by record writing green or not-green.

Test Plan

✅ bazel test //stovepipe/... — outcome stamped for each terminal status; the slot released exactly once and not on a redelivery of an already-stamped request; a failed release leaves the request non-terminal with no record publish; write-once still holds when a later poll disagrees.

✅ bazel test //test/e2e/stovepipe/... — the pipeline runs end to end.

Stack

1. test(stovepipe): add build-slow marker to the fake build runner #464
2. fix(stovepipe): mint a distinct message id for each buildsignal re-poll #465
3. refactor(stovepipe): replace recorded greenness states with build outcomes #466
4. @ feat(stovepipe): record the build outcome on the request and free its slot #467
5. feat(stovepipe)!: key the record stage on the request id #468
6. fix(stovepipe): stop wrapping the buildsignal re-poll publish as retryable #469

[behinddwalls]

unless it was cancelled? there is no reason to get it superseded...at this point build result is the terminal state? other than request just being cancelled?

[behinddwalls]

You're right that it can't be superseded here, and I've reworded the guard.

supersedeRequest is the only writer of superseded and it CAS-guards on state == accepted (process.go:398), while reaching the poll loop at all requires processing — a build only exists once process admitted the request. So naming superseded implied a case that cannot occur.

I didn't delete the check outright, though, because it isn't free: finishRequest conditions the slot release on the request not already having an outcome, so a terminal-without-an-outcome request arriving here would decrement a build slot that superseding never claimed (superseding consumes none). The guard now states the real invariant positively instead:

if request.State != entity.RequestStateProcessing && !request.State.HasBuildOutcome() {
    return nil
}

Same behaviour, but it says "must be processing, or already carry this build's outcome" rather than pointing at a state that can't happen. The comment above it records why it's kept rather than dropped.

On cancellation — worth separating the two senses:

• Build cancellation is already handled and is not what this guard is for: BuildStatusCancelled maps to RequestStateCancelled in outcomeState, which HasBuildOutcome() covers, so it falls through and gets recorded like any other outcome.
• Request cancellation doesn't exist yet. If it lands it would be exactly the terminal-without-an-outcome shape this guard rejects, so this is where it'd be honoured — and the slot-release hazard above is the reason the guard should stay until then.

Also added a test case pinning that a request which never claimed a slot is a no-op.