feat(stovepipe): record analysis and repair history

doc/rfc/stovepipe/request-log.md

@@ -10,7 +10,12 @@ Stovepipe retains an append-only request log for each validation request. Its in
 - `project_facts_recorded`;
 - `record_abandoned`;
 - `build_analysis_abandoned`;
-- `issue_tracking_abandoned`.
+- `issue_tracking_abandoned`;
+- `build_analysis_completed`;
+- `issues_reconciled`;
+- `fix_submitted`;
+- `fix_submission_skipped`;
+- `fix_submission_abandoned`.
 
 The model deliberately follows SubmitQueue's distinction between statuses describing where a request is and events describing important activity that does not move it. It remains a bounded request-lifecycle log rather than a generic event bus or an audit of every correlated operation.
 
@@ -29,7 +34,7 @@ State changes alone are insufficient. A request remains processing while a build
 An occurrence belongs in the request log only when it is either:
 
 1. a durable `Request.State` transition; or
-2. an asynchronous durable milestone required to explain the next request transition or public verdict.
+2. an asynchronous durable milestone explaining validation, issue reconciliation, or a repair submission for the request.
 
 The following remain with their owning entities, metrics, or structured logs:
 
@@ -130,9 +135,18 @@ Immutable Request context such as URI, build strategy, base URI, and terminal bu
 | `record_abandoned` | Record-stage work stopped after exhausting primary retries. | Event retention time |
 | `build_analysis_abandoned` | Build analysis could not complete; issue observations may be incomplete. This is not a validation verdict. | Build ID metadata and event retention time |
 | `issue_tracking_abandoned` | Issue tracking stopped before completing a pass; existing issue changes remain, but reconciliation may be incomplete. This is not a validation verdict. | Build ID metadata and event retention time |
+| `build_analysis_completed` | Terminal detection completed and its complete issue set was persisted. | Build ID, issue count, and event retention time |
+| `issues_reconciled` | A final issue-tracking pass completed, including any deliberate suppression of absent-issue closure. | Build ID, closure mode, and event retention time |
+| `fix_submitted` | An identified repair attempt was durably associated with a build issue. It does not imply a patch exists or the issue recovered. | Build, task, unit, and fix IDs and event retention time |
+| `fix_submission_skipped` | No eligible open issue existed for the build. | Build ID, bounded reason, and event retention time |
+| `fix_submission_abandoned` | Repair processing stopped without a durable fix ID; external acceptance remains unknown. | Build ID, bounded reason, and event retention time |
 
 Build running and unchanged polls are not retained. Trigger and terminal result explain the request outcome without turning polling into an unbounded log. Individual project facts remain outside the vocabulary; one batch-completion event summarizes their durable recording. Its `project_fact_count` and `broken_project_fact_count` metadata contain the total and the number with degree above zero, counted from persisted facts after their indexes are durable. Project completion projection requires both counts and assigns them together in `RequestSummary` from the retained event.
 
+`build_analysis_completed`, `issues_reconciled`, `fix_submission_skipped`, and `fix_submission_abandoned` use the build ID as their occurrence key. `fix_submitted` uses a digest of the executor fix ID to fit the storage key limit, retaining the verbatim fix ID in metadata. Existing issue-tracking abandonment uses a digest of the failed notification ID to fit the storage key limit. The materializer prefixes each key with its event kind, and request identity scopes all entries. Replays retain the first stored timestamp.
+
+Analysis and repair events do not advance Request state or alter validation facts. An empty terminal analysis records an issue count of zero. `issues_reconciled` records closure mode `reconciled`, `suppressed_cancelled`, or `suppressed_task_wide_issue`; suppression does not mean reconciliation failed. Skipped repair uses reason `no_eligible_issue`; abandoned repair uses `acceptance_unknown`. Raw dependency errors remain in operational logs.
+
 ### Evolution
 
 State and event strings never change meaning or get reused. A new event must satisfy the scope rule and update required-field validation, storage mapping, public projection, and compatibility tests.
@@ -220,8 +234,12 @@ The `project_facts_recorded` and `record_abandoned` events project completion of
 | Buildsignal | Persist terminal Build and retain `build_finished`; CAS the Request outcome and retain its terminal state. | Existing terminal Build and Request outcome each ensure their own entry before record publication. |
 | Record | Create or verify the whole-repository fact, then retain `validation_fact_recorded`; after recording project facts, retain one `project_facts_recorded` event. | An identical whole-repository fact and project-fact batch ensure their events before bookmark or promotion work. |
 | Record DLQ | Retain `record_abandoned` and project the end of project-result production; for failed Requests, create or verify the repository fact and retain `validation_fact_recorded` before publishing its hook. | Existing facts remain authoritative; stable log and hook IDs make replay idempotent without resolving projects or replaying promotion. |
+| Build analysis | Persist complete terminal detection results, then retain `build_analysis_completed` before issue-tracking publication. | Repeat detection and issue creation, then reconcile the stable per-build event before publication. Nonterminal polls add no event. |
 | Build analysis DLQ | Retain `build_analysis_abandoned` without rerunning detection or changing validation state. | One stable event per build deduplicates recovery deliveries; only state reads and history retention retry. |
 | Issue tracking DLQ | Protect the current cursor from delayed writes, retain its issue history and `issue_tracking_abandoned`, then mark the pass abandoned. | A stable hash of the notification ID deduplicates the event. Recovery retains accepted issue changes and never processes the remaining range. |
+| Issue tracking | Finalize the durable checkpoint, then retain `issues_reconciled`. | Reconstruct history from the finalized checkpoint before it can be replaced by another pass. |
+| Fix | Associate the returned fix ID, then retain `fix_submitted`; retain `fix_submission_skipped` when no issue is eligible. | Ensure submitted history from any existing association before skipping executor work. |
+| Fix DLQ | Repair submitted or skipped history from current durable state, or retain `fix_submission_abandoned` when acceptance is unknown. | Only read sources and retain history; never call the executor or change issue state. |
 | Reconciler | CAS an unrecoverable non-terminal Request to failed, then retain failed. | An existing terminal Request is repaired from its persisted outcome without relabeling it. |
 
 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.
@@ -280,7 +298,7 @@ Rejected because bookmarks, promotion, project facts, hooks, and operational boo
 
 ### Use a generic versioned envelope
 
-Rejected for six request states and three explanatory events. Explicit fields are simpler to inspect, constrain, test, and expose. A future migration should be justified by actual vocabulary growth.
+Rejected because explicit request states and event fields are simpler to inspect than a generic envelope. A future migration should be justified by actual vocabulary growth.
 
 ### Copy SubmitQueue's log topic
 

doc/rfc/stovepipe/steps/buildanalysis.md

@@ -12,7 +12,7 @@ Publication errors are returned through the producing stage's retry/DLQ policy.
 
 ## Build issues and polling
 
-The [analysis controller](../../../../stovepipe/controller/buildanalysis/README.md) owns detection, per-build issue persistence, polling, and the stop condition for Requests that end without a recorded build result. It accumulates per-build issues and publishes queue-partitioned issue-tracking notifications after persistence; it does not maintain open issues itself. Successful terminal analysis requests final reconciliation only after the Request records that Build as its winning outcome. Terminal detection must return the complete cumulative issue set, including issues returned by earlier calls, and analysis persists that set before publishing final reconciliation. Issue tracking applies all accumulated build issues before permitting any absence-based closure; a task-wide failure preserves existing issues across all tasks. See the controller documentation for handoff and deduplication rules.
+The [analysis controller](../../../../stovepipe/controller/buildanalysis/README.md) owns detection, per-build issue persistence, polling, and the stop condition for Requests that end without a recorded build result. It accumulates per-build issues and publishes queue-partitioned issue-tracking notifications after persistence; it does not maintain open issues itself. Successful terminal analysis requests final reconciliation only after the Request records that Build as its winning outcome. Terminal detection must return the complete cumulative issue set, including issues returned by earlier calls. Analysis persists that set and retains `build_analysis_completed` with the issue count before publishing final reconciliation. Nonterminal polls add no completion event. Issue tracking applies all accumulated build issues before permitting any absence-based closure; a task-wide failure preserves existing issues across all tasks. See the controller documentation for handoff and deduplication rules.
 
 Status-polling DLQ recovery does not publish a build-completion notice. The analysis polling loop observes the ended Request through storage instead.
 

service/stovepipe/server/main.go

@@ -417,19 +417,19 @@ func registerPrimaryControllers(
 	}
 	count++
 
-	analysisController := buildanalysis.NewController(logger, scope, store, detectorFactory, registry, stovepipemq.TopicKeyBuildAnalysis, "stovepipe-buildanalysis")
+	analysisController := buildanalysis.NewController(logger, scope, store, materializer, detectorFactory, registry, stovepipemq.TopicKeyBuildAnalysis, "stovepipe-buildanalysis")
 	if err := c.Register(analysisController); err != nil {
 		return count, fmt.Errorf("failed to register buildanalysis controller: %w", err)
 	}
 	count++
 
-	issueTrackController := issuetrack.NewController(logger, scope, store, registry, stovepipemq.TopicKeyIssueTrack, "stovepipe-issuetrack")
+	issueTrackController := issuetrack.NewController(logger, scope, store, materializer, registry, stovepipemq.TopicKeyIssueTrack, "stovepipe-issuetrack")
 	if err := c.Register(issueTrackController); err != nil {
 		return count, fmt.Errorf("failed to register issuetrack controller: %w", err)
 	}
 	count++
 
-	fixController := fix.NewController(logger, scope, store, repairExecutors, stovepipemq.TopicKeyFix, "stovepipe-fix")
+	fixController := fix.NewController(logger, scope, store, materializer, repairExecutors, stovepipemq.TopicKeyFix, "stovepipe-fix")
 	if err := c.Register(fixController); err != nil {
 		return count, fmt.Errorf("failed to register fix controller: %w", err)
 	}
@@ -492,6 +492,12 @@ func registerDLQControllers(
 	}
 	count++
 
+	fixDLQController := fix.NewDLQController(logger, scope, store, materializer, dlq.TopicKey(stovepipemq.TopicKeyFix), "stovepipe-fix-dlq")
+	if err := c.Register(fixDLQController); err != nil {
+		return count, fmt.Errorf("failed to register fix dlq controller: %w", err)
+	}
+	count++
+
 	recordDLQController := record.NewDLQController(logger, scope, store, materializer, registry, dlq.TopicKey(stovepipemq.TopicKeyRecord), "stovepipe-record-dlq")
 	if err := c.Register(recordDLQController); err != nil {
 		return count, fmt.Errorf("failed to register record dlq controller: %w", err)
@@ -606,6 +612,12 @@ func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRe
 			Queue:        q,
 			Subscription: extqueue.ExtendedSubscriptionConfig(subscriberName, "stovepipe-fix"),
 		},
+		{
+			Key:          dlq.TopicKey(stovepipemq.TopicKeyFix),
+			Name:         "fix_dlq",
+			Queue:        q,
+			Subscription: extqueue.DLQSubscriptionConfig(subscriberName, "stovepipe-fix-dlq"),
+		},
 		{
 			Key:          stovepipemq.TopicKeyBuildAnalysis,
 			Name:         "buildanalysis",

service/stovepipe/server/main_test.go

@@ -120,9 +120,6 @@ func TestValidationStagesHaveRecovery(t *testing.T) {
 	_, primary, deadLetter := registeredControllers(t)
 	recoveryKeys := topicKeys(deadLetter)
 	for _, controller := range primary {
-		if controller.TopicKey() == stovepipemq.TopicKeyFix {
-			continue
-		}
 		t.Run(controller.Name(), func(t *testing.T) {
 			assert.Contains(t, recoveryKeys, dlq.TopicKey(controller.TopicKey()))
 		})
@@ -135,7 +132,7 @@ func TestFixRegistration(t *testing.T) {
 	require.True(t, ok)
 	assert.Equal(t, "fix", name)
 	assert.Contains(t, topicKeys(primary), stovepipemq.TopicKeyFix)
-	assert.NotContains(t, topicKeys(deadLetter), dlq.TopicKey(stovepipemq.TopicKeyFix))
+	assert.Contains(t, topicKeys(deadLetter), dlq.TopicKey(stovepipemq.TopicKeyFix))
 }
 
 func TestRegisteredFixControllerRecordsNoopRepairOncePerBuild(t *testing.T) {
@@ -150,6 +147,27 @@ func TestRegisteredFixControllerRecordsNoopRepairOncePerBuild(t *testing.T) {
 	store.EXPECT().GetBuildIssueStore().Return(observations).AnyTimes()
 	store.EXPECT().GetIssueStateStore().Return(issues).AnyTimes()
 
+	builds := storagemock.NewMockBuildStore(ctrl)
+	requests := storagemock.NewMockRequestStore(ctrl)
+	history := storagemock.NewMockRequestLogStore(ctrl)
+	store.EXPECT().GetBuildStore().Return(builds).AnyTimes()
+	store.EXPECT().GetRequestStore().Return(requests).AnyTimes()
+	store.EXPECT().GetRequestLogStore().Return(history).AnyTimes()
+	builds.EXPECT().Get(gomock.Any(), buildID).Return(entity.Build{ID: buildID, RequestID: "1"}, nil).AnyTimes()
+	requests.EXPECT().Get(gomock.Any(), "1").Return(entity.Request{ID: "1", Queue: queueName}, nil).AnyTimes()
+	var retained entity.RequestLog
+	history.EXPECT().Create(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, log entity.RequestLog) error {
+		if retained.ID != "" {
+			return storage.ErrAlreadyExists
+		}
+		retained = log
+		return nil
+	}).Times(2)
+	history.EXPECT().Get(gomock.Any(), "1", gomock.Any()).DoAndReturn(func(_ context.Context, _, id string) (entity.RequestLog, error) {
+		require.Equal(t, retained.ID, id)
+		return retained, nil
+	})
+
 	observation := entity.BuildIssue{TaskID: "test", UnitID: "unit", Version: 1}
 	observations.EXPECT().List(gomock.Any(), buildID, gomock.Any()).DoAndReturn(
 		func(_ context.Context, _ string, bounds storage.BuildIssueRange) ([]entity.BuildIssue, error) {
@@ -196,6 +214,8 @@ func TestRegisteredFixControllerRecordsNoopRepairOncePerBuild(t *testing.T) {
 		delivery.EXPECT().Message().Return(message)
 		require.NoError(t, fixController.Process(context.Background(), delivery))
 	}
+	require.Equal(t, entity.RequestEventFixSubmitted, retained.Event)
+	require.Equal(t, "noop/"+buildID, retained.Metadata[requestlog.MetadataKeyFixID])
 }
 
 func TestHookStage(t *testing.T) {

stovepipe/controller/build/build_analysis_test.go

@@ -119,7 +119,7 @@ func TestBuildAnalysisPipeline(t *testing.T) {
 	)
 
 	analysisController := buildanalysis.NewController(zap.NewNop().Sugar(), tally.NoopScope,
-		staticStorageFactory{store: m.store}, detectorFactory, buildController.registry, stovepipemq.TopicKeyBuildAnalysis, "analysis")
+		staticStorageFactory{store: m.store}, m.materializer, detectorFactory, buildController.registry, stovepipemq.TopicKeyBuildAnalysis, "analysis")
 	statusController := buildsignal.NewController(zap.NewNop().Sugar(), tally.NoopScope,
 		staticStorageFactory{store: m.store}, m.materializer, m.runnerFactory, buildController.registry,
 		stovepipemq.TopicKeyBuildSignal, "signal")

stovepipe/controller/buildanalysis/BUILD.bazel

@@ -29,6 +29,7 @@ go_test(
     srcs = [
         "buildanalysis_test.go",
         "dlq_test.go",
+        "history_test.go",
     ],
     embed = [":go_default_library"],
     deps = [

stovepipe/controller/buildanalysis/README.md

@@ -4,6 +4,8 @@
 
 The controller reloads the queue-scoped Build, invokes the queue's issue detector, and persists each returned build issue individually. Build issues already stored for the same build, task, and unit within a queue are not persisted again, so repeated polls and retries after partial writes are safe. Detector results accompanying an error are discarded.
 
+After successful terminal detection and persistence, the controller retains `build_analysis_completed` with the build ID and complete issue count, including zero. History retention must succeed before the issue-tracking handoff; retries reconcile the same per-build entry. Nonterminal polls add no completion entry.
+
 After persistence, the controller publishes an issue-tracking notification partitioned by queue. Issue tracking maintains open issues and lifecycle history; analysis never writes them directly. An empty detection result does not delete earlier build issues.
 
 ## Issue-tracking handoff

stovepipe/controller/buildanalysis/buildanalysis.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strconv"
 
 	"github.com/uber-go/tally"
 	entityqueue "github.com/uber/submitqueue/platform/base/messagequeue"
@@ -28,6 +29,7 @@ import (
 	"github.com/uber/submitqueue/platform/publish"
 	"github.com/uber/submitqueue/stovepipe/core/loader"
 	stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue"
+	"github.com/uber/submitqueue/stovepipe/core/requestlog"
 	"github.com/uber/submitqueue/stovepipe/entity"
 	"github.com/uber/submitqueue/stovepipe/extension/buildissue"
 	"github.com/uber/submitqueue/stovepipe/extension/storage"
@@ -44,6 +46,7 @@ type Controller struct {
 	logger          *zap.SugaredLogger
 	metricsScope    tally.Scope
 	stores          storage.Factory
+	materializer    requestlog.Materializer
 	detectorFactory buildissue.DetectorFactory
 	registry        consumer.TopicRegistry
 	topicKey        consumer.TopicKey
@@ -54,6 +57,7 @@ func NewController(
 	logger *zap.SugaredLogger,
 	scope tally.Scope,
 	stores storage.Factory,
+	materializer requestlog.Materializer,
 	detectorFactory buildissue.DetectorFactory,
 	registry consumer.TopicRegistry,
 	topicKey consumer.TopicKey,
@@ -63,6 +67,7 @@ func NewController(
 		logger:          logger.Named("buildanalysis_controller"),
 		metricsScope:    scope.SubScope("buildanalysis_controller"),
 		stores:          stores,
+		materializer:    materializer,
 		detectorFactory: detectorFactory,
 		registry:        registry,
 		topicKey:        topicKey,
@@ -165,6 +170,17 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
 		hold = !finalize && !request.State.IsTerminal()
 	}
 
+	if build.Status.IsTerminal() {
+		log := requestlog.NewRequestEventLog(request, entity.RequestEventBuildAnalysisCompleted, build.ID,
+			map[string]string{
+				requestlog.MetadataKeyBuildID:    build.ID,
+				requestlog.MetadataKeyIssueCount: strconv.Itoa(len(issues)),
+			})
+		if err := c.materializer.PersistLog(ctx, store, log); err != nil {
+			return fmt.Errorf("retain completed analysis for build %q: %w", build.ID, err)
+		}
+	}
+
 	if err := c.publishIssueTracking(ctx, request.Queue, build.ID, finalize); err != nil {
 		return err
 	}