fix(stovepipe): persist the terminal outcome reason on Request

doc/rfc/stovepipe/request-log.md

@@ -244,7 +244,7 @@ The `project_facts_recorded` and `record_abandoned` events project completion of
 
 Build running and unchanged polls create no entry. A failed runner trigger that creates no Build creates no event. Cancelled and superseded requests create no validation fact.
 
-No controller infers an outcome reason from `RequestStateFailed` alone or records DLQ as the domain reason. Record DLQ uses that persisted state to recover a missing broken repository fact; it does not infer project verdicts.
+A terminal `Request` persists its outcome reason in the same compare-and-swap as its state, and every terminal state entry copies that reason. Only a Request made terminal before reasons were persisted falls back to the reason derived by the stage repairing its entry. No controller otherwise infers an outcome reason from `RequestStateFailed` alone or records DLQ as the domain reason. Record DLQ uses that persisted state to recover a missing broken repository fact; it does not infer project verdicts.
 
 ### Repair trigger requirement
 

stovepipe/controller/buildsignal/buildsignal.go

@@ -263,20 +263,13 @@ func (c *Controller) finishRequest(ctx context.Context, store storage.Storage, r
 }
 
 func (c *Controller) persistOutcomeLog(ctx context.Context, store storage.Storage, request entity.Request) error {
-	var reason entity.RequestOutcomeReason
-	// The durable request is authoritative when duplicate builds race to record different outcomes.
-	switch request.State {
-	case entity.RequestStateSucceeded:
-		reason = entity.RequestOutcomeReasonBuildSucceeded
-	case entity.RequestStateFailed:
-		reason = entity.RequestOutcomeReasonBuildFailed
-	case entity.RequestStateCancelled:
-		reason = entity.RequestOutcomeReasonBuildCancelled
-	default:
+	if !request.State.HasBuildOutcome() {
 		return fmt.Errorf("request %s has no build outcome to record", request.ID)
 	}
 
-	log := requestlog.NewRequestStateLog(request, reason)
+	// The durable request is authoritative: a DLQ may have forced this outcome, or a duplicate
+	// build won it. The build-derived reason applies only to a request stored without one.
+	log := requestlog.NewRequestStateLog(request, buildOutcomeReason(request.State))
 	if request.TerminalBuildID != "" {
 		log.Metadata[requestlog.MetadataKeyBuildID] = request.TerminalBuildID
 	}
@@ -300,10 +293,21 @@ func outcomeState(status entity.BuildStatus) entity.RequestState {
 	}
 }
 
-// markOutcome CAS-transitions request from processing to state, recording the
-// winning build ID and retrying on version 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 buildOutcomeReason(state entity.RequestState) entity.RequestOutcomeReason {
+	switch state {
+	case entity.RequestStateSucceeded:
+		return entity.RequestOutcomeReasonBuildSucceeded
+	case entity.RequestStateFailed:
+		return entity.RequestOutcomeReasonBuildFailed
+	case entity.RequestStateCancelled:
+		return entity.RequestOutcomeReasonBuildCancelled
+	default:
+		return entity.RequestOutcomeReasonUnknown
+	}
+}
+
+// The first terminal CAS owns the state, build ID and outcome reason; retries
+// reload the winner instead of replacing any of them.
 func (c *Controller) markOutcome(ctx context.Context, store storage.Storage, request *entity.Request, buildID string, state entity.RequestState) error {
 	reqStore := store.GetRequestStore()
 
@@ -315,6 +319,7 @@ func (c *Controller) markOutcome(ctx context.Context, store storage.Storage, req
 		updated := *request
 		updated.State = state
 		updated.TerminalBuildID = buildID
+		updated.OutcomeReason = buildOutcomeReason(state)
 		newVersion := request.Version + 1
 		if err := reqStore.Update(ctx, updated, request.Version, newVersion); err != nil {
 			if errors.Is(err, storage.ErrVersionMismatch) {

stovepipe/controller/buildsignal/buildsignal_test.go

@@ -161,6 +161,11 @@ func requestWithState(state entity.RequestState) entity.Request {
 	}
 	if state.HasBuildOutcome() {
 		request.TerminalBuildID = testBuildID
+		request.OutcomeReason = map[entity.RequestState]entity.RequestOutcomeReason{
+			entity.RequestStateSucceeded: entity.RequestOutcomeReasonBuildSucceeded,
+			entity.RequestStateFailed:    entity.RequestOutcomeReasonBuildFailed,
+			entity.RequestStateCancelled: entity.RequestOutcomeReasonBuildCancelled,
+		}[state]
 	}
 	return request
 }
@@ -225,14 +230,9 @@ func expectFinish(m buildsignalMocks, state entity.RequestState) *gomock.Call {
 }
 
 func expectOutcomeLog(m buildsignalMocks, state entity.RequestState, version int32) *gomock.Call {
-	reason := map[entity.RequestState]entity.RequestOutcomeReason{
-		entity.RequestStateSucceeded: entity.RequestOutcomeReasonBuildSucceeded,
-		entity.RequestStateFailed:    entity.RequestOutcomeReasonBuildFailed,
-		entity.RequestStateCancelled: entity.RequestOutcomeReasonBuildCancelled,
-	}[state]
 	request := requestWithState(state)
 	request.Version = version
-	log := requestlog.NewRequestStateLog(request, reason)
+	log := requestlog.NewRequestStateLog(request, request.OutcomeReason)
 	log.Metadata[requestlog.MetadataKeyBuildID] = request.TerminalBuildID
 	return m.materializer.EXPECT().PersistLog(
 		gomock.Any(),
@@ -403,6 +403,47 @@ func TestProcess(t *testing.T) {
 				m.publisher.EXPECT().Publish(gomock.Any(), "buildanalysis", gomock.Any()).Return(nil).After(logCall)
 			},
 		},
+		{
+			name: "late poll preserves the saved polling failure reason",
+			setup: func(m buildsignalMocks) {
+				request := requestWithState(entity.RequestStateProcessing)
+				request.State, request.OutcomeReason, request.Version = entity.RequestStateFailed, "build_polling_exhausted", 2
+				m.buildStore.EXPECT().Get(gomock.Any(), testBuildID).Return(build(entity.BuildStatusSucceeded, 3), nil)
+				m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(request, nil)
+				m.runnerFactory.EXPECT().For(buildrunner.Config{QueueName: testQueue}).Return(m.runner, nil)
+				m.runner.EXPECT().Status(gomock.Any(), entity.BuildID{ID: testBuildID}).Return(entity.BuildStatusSucceeded, nil, nil)
+				eventCall := expectBuildFinished(m)
+				logCall := m.materializer.EXPECT().PersistLog(gomock.Any(), m.store, entity.RequestLog{
+					ID: publish.IntentID("state", "2"), Queue: testQueue, RequestID: testID,
+					State: "failed", RequestVersion: 2, OutcomeReason: "build_polling_exhausted", Metadata: map[string]string{},
+				}).Return(nil).After(eventCall)
+				m.publisher.EXPECT().Publish(gomock.Any(), "buildanalysis", gomock.Any()).Return(nil).After(logCall)
+			},
+		},
+		{
+			name: "lost terminal CAS preserves the winning polling failure reason",
+			setup: func(m buildsignalMocks) {
+				request := requestWithState(entity.RequestStateProcessing)
+				winner := request
+				winner.State, winner.OutcomeReason, winner.Version = entity.RequestStateFailed, "build_polling_exhausted", 2
+				m.buildStore.EXPECT().Get(gomock.Any(), testBuildID).Return(build(entity.BuildStatusSucceeded, 3), nil)
+				m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(request, nil)
+				m.runnerFactory.EXPECT().For(buildrunner.Config{QueueName: testQueue}).Return(m.runner, nil)
+				m.runner.EXPECT().Status(gomock.Any(), entity.BuildID{ID: testBuildID}).Return(entity.BuildStatusSucceeded, nil, nil)
+				eventCall := expectBuildFinished(m)
+				gomock.InOrder(
+					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),
+					m.reqStore.EXPECT().Update(gomock.Any(), requestWithState(entity.RequestStateSucceeded), int32(1), int32(2)).Return(storage.ErrVersionMismatch),
+					m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(winner, nil),
+				)
+				logCall := m.materializer.EXPECT().PersistLog(gomock.Any(), m.store, entity.RequestLog{
+					ID: publish.IntentID("state", "2"), Queue: testQueue, RequestID: testID,
+					State: "failed", RequestVersion: 2, OutcomeReason: "build_polling_exhausted", Metadata: map[string]string{},
+				}).Return(nil)
+				m.publisher.EXPECT().Publish(gomock.Any(), "buildanalysis", gomock.Any()).Return(nil).After(logCall)
+			},
+		},
 		{
 			name:    "build event failure stops before request outcome",
 			wantErr: true,

stovepipe/controller/dlq/build_test.go

@@ -76,6 +76,7 @@ func TestBuildProcess(t *testing.T) {
 				m.queueStore.EXPECT().Update(gomock.Any(), entity.Queue{Name: testQueue, Version: 5}, int32(5), int32(6)).Return(nil)
 				updated := requestWithState(entity.RequestStateProcessing)
 				updated.State = entity.RequestStateFailed
+				updated.OutcomeReason = entity.RequestOutcomeReasonProcessingFailed
 				updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil)
 				expectFailureLog(m, entity.RequestOutcomeReasonProcessingFailed, 3).After(updateCall)
 			},

stovepipe/controller/dlq/buildsignal_test.go

@@ -123,6 +123,7 @@ func TestBuildSignalProcess(t *testing.T) {
 				}, int32(5), int32(6)).Return(nil)
 				updated := requestWithState(entity.RequestStateProcessing)
 				updated.State = entity.RequestStateFailed
+				updated.OutcomeReason = entity.RequestOutcomeReasonBuildPollingExhausted
 				updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil)
 				request := requestWithState(entity.RequestStateFailed)
 				request.Version = 3

stovepipe/controller/dlq/dlq.go

@@ -128,6 +128,7 @@ func failRequest(
 
 	updated := request
 	updated.State = entity.RequestStateFailed
+	updated.OutcomeReason = reason
 	newVersion := request.Version + 1
 	if err := store.GetRequestStore().Update(ctx, updated, request.Version, newVersion); err != nil {
 		return fmt.Errorf("failed to update request %s state to failed: %w", requestID, err)

stovepipe/controller/dlq/dlq_test.go

@@ -145,6 +145,7 @@ func TestProcess(t *testing.T) {
 				m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(entity.RequestStateAccepted), nil)
 				updated := requestWithState(entity.RequestStateAccepted)
 				updated.State = entity.RequestStateFailed
+				updated.OutcomeReason = entity.RequestOutcomeReasonProcessingFailed
 				updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil)
 				expectFailureLog(m, entity.RequestOutcomeReasonProcessingFailed, 3).After(updateCall)
 			},
@@ -161,6 +162,7 @@ func TestProcess(t *testing.T) {
 				}, int32(5), int32(6)).Return(nil)
 				updated := requestWithState(entity.RequestStateProcessing)
 				updated.State = entity.RequestStateFailed
+				updated.OutcomeReason = entity.RequestOutcomeReasonProcessingFailed
 				updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil)
 				expectFailureLog(m, entity.RequestOutcomeReasonProcessingFailed, 3).After(updateCall)
 			},
@@ -172,12 +174,21 @@ func TestProcess(t *testing.T) {
 			},
 		},
 		{
-			name: "already failed repairs its missing log",
+			name: "already failed without a stored reason repairs its missing log with the stage reason",
 			setup: func(m dlqMocks) {
 				m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(entity.RequestStateFailed), nil)
 				expectFailureLog(m, entity.RequestOutcomeReasonProcessingFailed, 2)
 			},
 		},
+		{
+			name: "already failed repairs its missing log with the stored reason, not the stage reason",
+			setup: func(m dlqMocks) {
+				request := requestWithState(entity.RequestStateFailed)
+				request.OutcomeReason = entity.RequestOutcomeReasonBuildPollingExhausted
+				m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(request, nil)
+				expectFailureLog(m, entity.RequestOutcomeReasonBuildPollingExhausted, 2)
+			},
+		},
 		{
 			name: "request not found is a no-op",
 			setup: func(m dlqMocks) {
@@ -190,6 +201,7 @@ func TestProcess(t *testing.T) {
 				m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(entity.RequestStateAccepted), nil)
 				updated := requestWithState(entity.RequestStateAccepted)
 				updated.State = entity.RequestStateFailed
+				updated.OutcomeReason = entity.RequestOutcomeReasonProcessingFailed
 				m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(storage.ErrVersionMismatch)
 			},
 			wantErr: true,
@@ -212,6 +224,7 @@ func TestProcess(t *testing.T) {
 				}, int32(6), int32(7)).Return(nil)
 				updated := requestWithState(entity.RequestStateProcessing)
 				updated.State = entity.RequestStateFailed
+				updated.OutcomeReason = entity.RequestOutcomeReasonProcessingFailed
 				updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil)
 				expectFailureLog(m, entity.RequestOutcomeReasonProcessingFailed, 3).After(updateCall)
 			},
@@ -225,6 +238,7 @@ func TestProcess(t *testing.T) {
 				}, nil)
 				updated := requestWithState(entity.RequestStateProcessing)
 				updated.State = entity.RequestStateFailed
+				updated.OutcomeReason = entity.RequestOutcomeReasonProcessingFailed
 				updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil)
 				expectFailureLog(m, entity.RequestOutcomeReasonProcessingFailed, 3).After(updateCall)
 			},
@@ -236,6 +250,7 @@ func TestProcess(t *testing.T) {
 				m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(entity.RequestStateAccepted), nil)
 				updated := requestWithState(entity.RequestStateAccepted)
 				updated.State = entity.RequestStateFailed
+				updated.OutcomeReason = entity.RequestOutcomeReasonProcessingFailed
 				updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil)
 				request := requestWithState(entity.RequestStateFailed)
 				request.Version = 3

stovepipe/controller/process/process.go

@@ -409,6 +409,7 @@ func (c *Controller) supersedeRequest(ctx context.Context, store storage.Storage
 
 		updated := request
 		updated.State = entity.RequestStateSuperseded
+		updated.OutcomeReason = entity.RequestOutcomeReasonSupersededByNewerHead
 		newVersion := request.Version + 1
 		if err := reqStore.Update(ctx, updated, request.Version, newVersion); err != nil {
 			if errors.Is(err, storage.ErrVersionMismatch) {

stovepipe/controller/process/process_test.go

@@ -772,6 +772,7 @@ func TestProcess(t *testing.T) {
 				}, nil)
 				updated := acceptedRequest(testOlderID)
 				updated.State = entity.RequestStateSuperseded
+				updated.OutcomeReason = entity.RequestOutcomeReasonSupersededByNewerHead
 				updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(nil)
 				loggedRequest := updated
 				loggedRequest.Version = 2
@@ -791,6 +792,7 @@ func TestProcess(t *testing.T) {
 				}, nil)
 				updated := acceptedRequest(testOlderID)
 				updated.State = entity.RequestStateSuperseded
+				updated.OutcomeReason = entity.RequestOutcomeReasonSupersededByNewerHead
 				updateCall := m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(nil)
 				request := entity.Request{
 					ID: testOlderID, Queue: testQueue, State: entity.RequestStateSuperseded, Version: 2,
@@ -814,6 +816,7 @@ func TestProcess(t *testing.T) {
 				}, nil)
 				updated := acceptedRequest(testOlderID)
 				updated.State = entity.RequestStateSuperseded
+				updated.OutcomeReason = entity.RequestOutcomeReasonSupersededByNewerHead
 				m.reqStore.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(storage.ErrVersionMismatch)
 				reloaded := entity.Request{
 					ID: testOlderID, Queue: testQueue, State: entity.RequestStateSuperseded, Version: 2,
@@ -938,6 +941,7 @@ func TestDelayedRegistrationDoesNotAdmitAnOlderRequest(t *testing.T) {
 	m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(entity.Queue{Name: testQueue, LatestRequestID: "8", Version: 2}, nil)
 	request := acceptedRequest(testID)
 	request.State = entity.RequestStateSuperseded
+	request.OutcomeReason = entity.RequestOutcomeReasonSupersededByNewerHead
 	m.reqStore.EXPECT().Update(gomock.Any(), request, int32(1), int32(2)).Return(nil)
 	request.Version = 2
 	expectStateLog(t, m, request, entity.RequestOutcomeReasonSupersededByNewerHead)

stovepipe/core/requestlog/materializer.go

@@ -84,7 +84,13 @@ func NewMaterializer(scope tally.Scope) Materializer {
 }
 
 // NewRequestStateLog constructs the occurrence representing the request's current durable state.
-func NewRequestStateLog(request entity.Request, outcomeReason entity.RequestOutcomeReason) entity.RequestLog {
+// It records the request's persisted outcome reason. fallbackReason applies only when none is
+// persisted: on a nonterminal request, or on one made terminal before reasons were persisted.
+func NewRequestStateLog(request entity.Request, fallbackReason entity.RequestOutcomeReason) entity.RequestLog {
+	outcomeReason := request.OutcomeReason
+	if outcomeReason == entity.RequestOutcomeReasonUnknown {
+		outcomeReason = fallbackReason
+	}
 	metadata := make(map[string]string)
 	switch request.BuildStrategy {
 	case entity.BuildStrategyFull:

stovepipe/entity/request.go

@@ -122,6 +122,9 @@ type Request struct {
 	// TerminalBuildID is the build that established this request's terminal
 	// state. It is empty when no build established the terminal state.
 	TerminalBuildID string `json:"terminal_build_id"`
+	// OutcomeReason is the durable reason for the current terminal state. It is empty while
+	// the request is nonterminal and on requests made terminal before reasons were persisted.
+	OutcomeReason RequestOutcomeReason `json:"outcome_reason"`
 	// 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"`

stovepipe/extension/storage/README.md

@@ -40,6 +40,10 @@ Listing fixes the queue/request prefix and returns bounded pages in ascending by
 
 Create returns `ErrAlreadyExists` for a repeated entry. Listing fixes the queue/request/project prefix and returns bounded pages in ascending bytewise task/unit order. The zero cursor starts the range; other cursors must match the project and exclude their task/unit key. An empty page ends the range, not evidence that attribution recording is complete. There are no update or delete operations.
 
+## Requests
+
+Existing MySQL deployments must add `outcome_reason VARCHAR(64) NOT NULL DEFAULT ''` to `request` before deploying its readers and writers. Empty reasons retain the legacy fallback; existing immutable logs are not rewritten.
+
 ## Build issue observations
 
 `BuildIssueStore` accumulates observations identified by build, task, and unit within the bound queue. The build ID is supplied separately from the detected issue. Repeated observations return `ErrAlreadyExists`, which callers may treat as successful persistence. Build and task IDs must be nonempty; empty unit IDs represent task-wide observations.