diff --git a/apps/backend/internal/github/store.go b/apps/backend/internal/github/store.go index 1b4f3f5e1b..685df6527e 100644 --- a/apps/backend/internal/github/store.go +++ b/apps/backend/internal/github/store.go @@ -3066,7 +3066,6 @@ func (s *Store) RefreshTaskCIFixCheckpoint(ctx context.Context, taskID, reposito last_fix_signature = excluded.last_fix_signature, last_fix_checkpoint_json = excluded.last_fix_checkpoint_json, last_fix_enqueued_at = NULL, - last_fix_session_id = NULL, last_error = NULL, last_error_kind = '', updated_at = excluded.updated_at`), @@ -3230,8 +3229,9 @@ func (s *Store) RecordTaskCIMergeAttemptResult( // RecordTaskCIMergeQueueObservation persists an active queue attempt or a // conservative current-head baseline when a removal is observed first. The // baseline is written only when no queue attempt has been recorded yet, so a -// later poll cannot move the guard to a newer head and accidentally requeue a -// removal that belongs to an older attempt. +// later poll cannot move the automatic requeue guard to a newer head. The +// passive baseline is not auto-fix provenance; queue-removal auto-fix also +// requires the durable merge-attempt signature. func (s *Store) RecordTaskCIMergeQueueObservation(ctx context.Context, observation TaskCIMergeQueueObservation) error { return s.mutateTaskCIPRState(ctx, observation.TaskID, func(ctx context.Context, tx *sqlx.Tx, now time.Time) error { baselineHead := "" diff --git a/apps/backend/internal/github/store_ci_automation_test.go b/apps/backend/internal/github/store_ci_automation_test.go index d7744ed5dc..8348c76cb7 100644 --- a/apps/backend/internal/github/store_ci_automation_test.go +++ b/apps/backend/internal/github/store_ci_automation_test.go @@ -911,7 +911,7 @@ func TestStoreTaskCIPRState_MarkExhaustedAndResetOnReenable(t *testing.T) { } } -func TestStoreTaskCIPRState_RefreshCheckpointClearsPromptDispatchMetadata(t *testing.T) { +func TestStoreTaskCIPRState_RefreshCheckpointPreservesSessionPinning(t *testing.T) { store := newTestStore(t) ctx := context.Background() enqueuedAt := time.Date(2026, 6, 18, 12, 0, 0, 0, time.UTC) @@ -938,8 +938,8 @@ func TestStoreTaskCIPRState_RefreshCheckpointClearsPromptDispatchMetadata(t *tes if state.LastFixSignature != "after" || state.LastFixCheckpointJSON != `{"failed_checks":[]}` { t.Fatalf("checkpoint was not refreshed: %+v", state) } - if state.LastFixSessionID != nil { - t.Fatalf("LastFixSessionID=%v, want nil", state.LastFixSessionID) + if state.LastFixSessionID == nil || *state.LastFixSessionID != "session-1" { + t.Fatalf("LastFixSessionID=%v, want session-1", state.LastFixSessionID) } if state.LastFixEnqueuedAt != nil { t.Fatalf("LastFixEnqueuedAt=%v, want nil", state.LastFixEnqueuedAt) diff --git a/apps/backend/internal/orchestrator/event_handlers_github_ci_automation.go b/apps/backend/internal/orchestrator/event_handlers_github_ci_automation.go index a5e857a03e..dafbcddebf 100644 --- a/apps/backend/internal/orchestrator/event_handlers_github_ci_automation.go +++ b/apps/backend/internal/orchestrator/event_handlers_github_ci_automation.go @@ -21,18 +21,19 @@ import ( ) const ( - ciAutomationOrigin = "ci_automation" - ciAutomationCheckSuccess = "success" - ciAutomationCheckFailure = "failure" - ciAutomationCheckError = "error" - ciAutomationCheckCompleted = "completed" - ciAutomationCheckPending = "pending" - ciAutomationChangesRequested = "changes_requested" - ciAutomationPRFeedbackToken = "{{pr.feedback}}" - ciAutomationFixBlockWindow = time.Hour - ciAutomationMaxFixRounds = github.TaskCIAutoFixMaxRounds - ciAutomationKindAutoFix = "ci_auto_fix" - ciAutomationStateEventSource = "ci_automation_state" + ciAutomationOrigin = "ci_automation" + ciAutomationCheckSuccess = "success" + ciAutomationCheckFailure = "failure" + ciAutomationCheckError = "error" + ciAutomationCheckCompleted = "completed" + ciAutomationCheckPending = "pending" + ciAutomationChangesRequested = "changes_requested" + ciAutomationMergeableStateDirty = "dirty" + ciAutomationPRFeedbackToken = "{{pr.feedback}}" + ciAutomationFixBlockWindow = time.Hour + ciAutomationMaxFixRounds = github.TaskCIAutoFixMaxRounds + ciAutomationKindAutoFix = "ci_auto_fix" + ciAutomationStateEventSource = "ci_automation_state" ) var ciAutomationSnapshotFieldReplacer = strings.NewReplacer("\r", " ", "\n", " ", "<", "", ">", "") @@ -40,6 +41,7 @@ var ciAutomationSnapshotFieldReplacer = strings.NewReplacer("\r", " ", "\n", " " type ciAutomationCheckpoint struct { FailedChecks []ciAutomationCheckSnapshot `json:"failed_checks"` Comments []ciAutomationCommentSnapshot `json:"comments"` + Conflict *ciAutomationConflictSnapshot `json:"conflict,omitempty"` QueueRemovals []ciAutomationQueueRemovalSnapshotData `json:"queue_removals,omitempty"` } @@ -57,6 +59,13 @@ type ciAutomationCommentSnapshot struct { Line int `json:"line,omitempty"` } +type ciAutomationConflictSnapshot struct { + MergeableState string `json:"mergeable_state"` + HeadSHA string `json:"head_sha,omitempty"` + HeadBranch string `json:"head_branch,omitempty"` + BaseBranch string `json:"base_branch,omitempty"` +} + const ( ciAutomationQueueRemovalCauseChecksFailed = "checks_failed" ciAutomationQueueRemovalCauseChecksTimedOut = "checks_timed_out" @@ -300,8 +309,8 @@ func (s *Service) handleTaskPRCIAutoFix(ctx context.Context, pr *github.TaskPR, } feedback = ciAutomationFilterFeedbackForPR(pr, feedback) previous := decodeCIAutomationCheckpoint(state) - delta := ciAutomationBuildDelta(feedback, previous) - checkpoint := ciAutomationCurrentCheckpoint(feedback) + delta := ciAutomationBuildDeltaForPR(pr, feedback, previous) + checkpoint := ciAutomationCurrentCheckpointForPR(pr, feedback, previous) queueRemoval, queueRecovery := ciAutomationNewQueueRemoval(pr, state) if queueRecovery { delta.QueueRemovals = append(delta.QueueRemovals, *queueRemoval) @@ -382,7 +391,7 @@ func (s *Service) handleTaskPRCIAutoFixEmptyDelta(ctx context.Context, pr *githu if state != nil && state.LastFixSignature == signature && ciAutomationDuplicateFixAttemptBlocksMerge(state) { return true } - if state != nil && len(previous.FailedChecks)+len(previous.Comments) > 0 { + if state != nil && !ciAutomationCheckpointEmpty(previous) { if err := s.githubService.RefreshTaskCIFixCheckpoint(context.WithoutCancel(ctx), pr.TaskID, pr.RepositoryID, pr.PRNumber, signature, checkpointJSON); err != nil { s.logger.Debug("record CI auto-fix checkpoint refresh failed", zap.String("task_id", pr.TaskID), zap.Error(err)) } @@ -718,6 +727,15 @@ func ciAutomationBuildDelta(feedback *github.PRFeedback, previous ciAutomationCh return delta } +func ciAutomationBuildDeltaForPR(pr *github.TaskPR, feedback *github.PRFeedback, previous ciAutomationCheckpoint) ciAutomationCheckpoint { + delta := ciAutomationBuildDelta(feedback, previous) + currentConflict := ciAutomationCurrentConflictSnapshot(pr, previous.Conflict) + if !ciAutomationConflictSnapshotEqual(currentConflict, previous.Conflict) && currentConflict != nil { + delta.Conflict = currentConflict + } + return delta +} + func ciAutomationCheckConclusionNeedsFix(conclusion string) bool { return conclusion == ciAutomationCheckFailure || conclusion == "timed_out" || @@ -783,8 +801,41 @@ func ciAutomationCurrentCheckpoint(feedback *github.PRFeedback) ciAutomationChec return ciAutomationBuildDelta(feedback, ciAutomationCheckpoint{}) } +func ciAutomationCurrentCheckpointForPR(pr *github.TaskPR, feedback *github.PRFeedback, previous ciAutomationCheckpoint) ciAutomationCheckpoint { + checkpoint := ciAutomationCurrentCheckpoint(feedback) + checkpoint.Conflict = ciAutomationCurrentConflictSnapshot(pr, previous.Conflict) + return checkpoint +} + func ciAutomationCheckpointEmpty(checkpoint ciAutomationCheckpoint) bool { - return len(checkpoint.FailedChecks) == 0 && len(checkpoint.Comments) == 0 && len(checkpoint.QueueRemovals) == 0 + return len(checkpoint.FailedChecks) == 0 && len(checkpoint.Comments) == 0 && checkpoint.Conflict == nil && len(checkpoint.QueueRemovals) == 0 +} + +func ciAutomationCurrentConflictSnapshot(pr *github.TaskPR, previous *ciAutomationConflictSnapshot) *ciAutomationConflictSnapshot { + if pr == nil { + return previous + } + mergeableState := strings.ToLower(strings.TrimSpace(pr.MergeableState)) + switch mergeableState { + case ciAutomationMergeableStateDirty: + return &ciAutomationConflictSnapshot{ + MergeableState: mergeableState, + HeadSHA: strings.TrimSpace(pr.HeadSHA), + HeadBranch: strings.TrimSpace(pr.HeadBranch), + BaseBranch: strings.TrimSpace(pr.BaseBranch), + } + case "", ciAutomationQueueRemovalCauseUnknown: + return previous + default: + return nil + } +} + +func ciAutomationConflictSnapshotEqual(left, right *ciAutomationConflictSnapshot) bool { + if left == nil || right == nil { + return left == right + } + return *left == *right } func ciAutomationNewQueueRemoval(pr *github.TaskPR, state *github.TaskCIPRAutomationState) (*ciAutomationQueueRemovalSnapshotData, bool) { @@ -803,11 +854,12 @@ func ciAutomationQueueRemovalBelongsToCurrentHead(pr *github.TaskPR, state *gith return false } attemptHead := strings.TrimSpace(state.LastQueueAttemptHeadSHA) - // A removal-only observation may establish a current-head baseline, but it - // does not prove that Kandev ever queued this head. Require the merge - // signature written by an actual merge attempt or active-entry adoption so - // an old removal cannot spend a repair round after automation is enabled. - return attemptHead != "" && attemptHead == strings.TrimSpace(pr.HeadSHA) && strings.TrimSpace(state.LastMergeSignature) != "" + // A removal-only observation can establish a passive same-head baseline, + // but it does not prove which pull-request head produced the removal. Auto- + // fix therefore requires the merge signature written by an attempted or + // adopted queue entry in addition to the matching durable head. + return attemptHead != "" && attemptHead == strings.TrimSpace(pr.HeadSHA) && + strings.TrimSpace(state.LastMergeSignature) != "" } func ciAutomationHasActiveMergeQueueEntry(pr *github.TaskPR) bool { @@ -882,7 +934,7 @@ func ciAutomationQueueRemovalConflictEvidence(pr *github.TaskPR) bool { } mergeable := strings.ToLower(strings.TrimSpace(pr.MergeableState)) queueState := strings.ToLower(strings.TrimSpace(pr.MergeQueueState)) - return mergeable == "dirty" || strings.Contains(mergeable, "conflict") || + return mergeable == ciAutomationMergeableStateDirty || strings.Contains(mergeable, "conflict") || queueState == "unmergeable" || strings.Contains(queueState, "unmergeable") } @@ -940,6 +992,17 @@ func ciAutomationRenderSnapshot(pr *github.TaskPR, delta ciAutomationCheckpoint) b.WriteString(fmt.Sprintf("\n- %s:%d %s", ciAutomationSanitizeSnapshotField(comment.Path), comment.Line, ciAutomationSanitizeSnapshotField(strings.TrimSpace(comment.Body)))) } } + if delta.Conflict != nil { + b.WriteString("\n\nMerge conflict:") + b.WriteString("\n- mergeability: ") + b.WriteString(ciAutomationSanitizeSnapshotField(delta.Conflict.MergeableState)) + b.WriteString("; head branch: ") + b.WriteString(ciAutomationSanitizeSnapshotField(delta.Conflict.HeadBranch)) + b.WriteString("; base branch: ") + b.WriteString(ciAutomationSanitizeSnapshotField(delta.Conflict.BaseBranch)) + b.WriteString("; head commit: ") + b.WriteString(ciAutomationSanitizeSnapshotField(delta.Conflict.HeadSHA)) + } if len(delta.QueueRemovals) > 0 { b.WriteString("\n\nMerge queue removal recovery:") for _, removal := range delta.QueueRemovals { diff --git a/apps/backend/internal/orchestrator/event_handlers_github_ci_automation_test.go b/apps/backend/internal/orchestrator/event_handlers_github_ci_automation_test.go index 0ea4ba425b..6fc5502fac 100644 --- a/apps/backend/internal/orchestrator/event_handlers_github_ci_automation_test.go +++ b/apps/backend/internal/orchestrator/event_handlers_github_ci_automation_test.go @@ -334,6 +334,65 @@ func TestCIAutomationFeedbackDeltaIncludesKnownFailingConclusions(t *testing.T) } } +func TestCIAutomationConflictSignal(t *testing.T) { + pr := &github.TaskPR{ + MergeableState: "DIRTY", + HeadSHA: "head-a", + HeadBranch: "feature-a", + BaseBranch: "main", + } + feedback := &github.PRFeedback{} + + first := ciAutomationBuildDeltaForPR(pr, feedback, ciAutomationCheckpoint{}) + if first.Conflict == nil { + t.Fatal("expected a dirty pull request to produce a conflict delta") + } + if first.Conflict.MergeableState != "dirty" || first.Conflict.HeadSHA != "head-a" || + first.Conflict.HeadBranch != "feature-a" || first.Conflict.BaseBranch != "main" { + t.Fatalf("unexpected conflict snapshot: %+v", first.Conflict) + } + + current := ciAutomationCurrentCheckpointForPR(pr, feedback, ciAutomationCheckpoint{}) + if repeated := ciAutomationBuildDeltaForPR(pr, feedback, current); repeated.Conflict != nil { + t.Fatalf("unchanged conflict produced a duplicate delta: %+v", repeated.Conflict) + } + + clean := *pr + clean.MergeableState = "clean" + if checkpoint := ciAutomationCurrentCheckpointForPR(&clean, feedback, current); checkpoint.Conflict != nil { + t.Fatalf("authoritative clean state kept conflict checkpoint: %+v", checkpoint.Conflict) + } + + unknown := *pr + unknown.MergeableState = "unknown" + if checkpoint := ciAutomationCurrentCheckpointForPR(&unknown, feedback, current); checkpoint.Conflict == nil { + t.Fatal("unknown mergeability state cleared the prior conflict checkpoint") + } + + changed := *pr + changed.HeadSHA = "head-b" + if delta := ciAutomationBuildDeltaForPR(&changed, feedback, current); delta.Conflict == nil { + t.Fatal("changed conflict head did not re-arm conflict repair") + } +} + +func TestCIAutomationRenderSnapshotIncludesConflict(t *testing.T) { + delta := ciAutomationCheckpoint{ + Conflict: &ciAutomationConflictSnapshot{ + MergeableState: "dirty", + HeadSHA: "head-", + HeadBranch: "feature/", + BaseBranch: "main/", + }, + } + + snapshot := ciAutomationRenderSnapshot(&github.TaskPR{Owner: "acme", Repo: "widget", PRNumber: 42}, delta) + if !strings.Contains(snapshot, "Merge conflict") || !strings.Contains(snapshot, "feature/a") || + !strings.Contains(snapshot, "main/b") || !strings.Contains(snapshot, "head-a") { + t.Fatalf("conflict snapshot missing expected details:\n%s", snapshot) + } +} + func TestCIAutomationRenderSnapshotSanitizesUntrustedFields(t *testing.T) { delta := ciAutomationCheckpoint{ FailedChecks: []ciAutomationCheckSnapshot{{Name: "unit\n