fix(github): restore PR sync context (#2916)

apps/backend/internal/github/service_comparison_target_sync_test.go

@@ -0,0 +1,87 @@
+package github
+
+import (
+	"context"
+	"testing"
+
+	taskmodels "github.com/kandev/kandev/internal/task/models"
+)
+
+type syncComparisonTargetObserver struct {
+	store          *Store
+	calls          int
+	taskID         string
+	candidate      taskmodels.ComparisonTargetCandidate
+	persistedTitle string
+}
+
+func (o *syncComparisonTargetObserver) ReconcileComparisonTarget(
+	context.Context, string, taskmodels.ComparisonTargetCandidate,
+) (*taskmodels.ComparisonTargetReconciliation, error) {
+	return nil, nil
+}
+
+func (o *syncComparisonTargetObserver) ReconcileComparisonTargetFromSync(
+	ctx context.Context, taskID string, candidate taskmodels.ComparisonTargetCandidate,
+) (*taskmodels.ComparisonTargetReconciliation, error) {
+	persisted, err := o.store.GetTaskPR(ctx, taskID)
+	if err != nil {
+		return nil, err
+	}
+	o.calls++
+	o.taskID = taskID
+	o.candidate = candidate
+	o.persistedTitle = persisted.PRTitle
+	return &taskmodels.ComparisonTargetReconciliation{
+		Status: taskmodels.ComparisonTargetNoMatch,
+	}, nil
+}
+
+func (o *syncComparisonTargetObserver) RemoveComparisonTargetForChange(
+	context.Context, string, string, string, string, int,
+) error {
+	return nil
+}
+
+func TestSyncTaskPR_PersistsBeforeComparisonTargetReconciliation(t *testing.T) {
+	svc, store, _ := setupSyncTest(t)
+	ctx := context.Background()
+	createOutcomeSyncTestTaskPR(t, store, &TaskPR{
+		TaskID: "t1", Owner: "owner", Repo: "repo", PRNumber: 1,
+		PRURL: "https://github.com/owner/repo/pull/1", PRTitle: "Initial",
+		HeadBranch: "feat", BaseBranch: "main", State: "open",
+	})
+
+	observer := &syncComparisonTargetObserver{store: store}
+	svc.SetComparisonTargetObserver(observer)
+	status := &PRStatus{PR: &PR{
+		Number:        1,
+		Title:         "Updated title",
+		State:         "open",
+		RepoOwner:     "owner",
+		RepoName:      "repo",
+		HeadRepoOwner: "contributor",
+		HeadRepoName:  "repo",
+		BaseRepoOwner: "upstream",
+		BaseRepoName:  "repo",
+		HeadBranch:    "feat",
+		BaseBranch:    "main",
+	}}
+
+	if err := svc.SyncTaskPR(ctx, "t1", status); err != nil {
+		t.Fatalf("SyncTaskPR: %v", err)
+	}
+
+	if observer.calls != 1 {
+		t.Fatalf("comparison reconciliation calls = %d, want 1", observer.calls)
+	}
+	if observer.taskID != "t1" {
+		t.Fatalf("comparison reconciliation task ID = %q, want t1", observer.taskID)
+	}
+	if observer.persistedTitle != "Updated title" {
+		t.Fatalf("persisted title during reconciliation = %q, want Updated title", observer.persistedTitle)
+	}
+	if observer.candidate.Number != 1 || observer.candidate.HeadRepository.Path != "contributor/repo" {
+		t.Fatalf("comparison candidate = %#v, want PR 1 from contributor/repo", observer.candidate)
+	}
+}

apps/backend/internal/github/service_pr_outcome_sync_test.go

@@ -338,7 +338,7 @@ func TestPersistAndPublishTaskPRSync_PublishesReReadValueNotStaleInMemoryOne(t *
 	staleObservedAt := persistedAt.Add(-time.Minute)
 	staleTP.AutoMergeObservedAt = &staleObservedAt
 
-	if err := svc.persistAndPublishTaskPRSync(ctx, &staleTP, true, true); err != nil {
+	if err := svc.persistAndPublishTaskPRSync(ctx, staleTP.TaskID, nil, &staleTP, true, true); err != nil {
 		t.Fatalf("persistAndPublishTaskPRSync: %v", err)
 	}
 

apps/backend/internal/github/service_pr_unwatched.go

@@ -158,7 +158,7 @@ func (s *Service) reconcileTaskPRLifecycle(ctx context.Context, tp *TaskPR, stat
 	// this call's own resolved value. persistAndPublishTaskPRSync already
 	// re-reads before publishing for exactly this reason (codex [P2] on the
 	// SyncTaskPR path); reuse it here instead of duplicating the write.
-	return s.persistAndPublishTaskPRSync(ctx, tp, changed, status.OutcomeFieldsPopulated)
+	return s.persistAndPublishTaskPRSync(ctx, tp.TaskID, status.PR, tp, changed, status.OutcomeFieldsPopulated)
 }
 
 // fetchUnwatchedTaskPRs prefers the batched GraphQL query — one call for the

apps/backend/internal/github/service_pr_watch.go

@@ -994,7 +994,7 @@ func (s *Service) SyncTaskPR(ctx context.Context, taskID string, status *PRStatu
 		)
 	}
 
-	return s.persistAndPublishTaskPRSync(ctx, tp, changed, status.OutcomeFieldsPopulated)
+	return s.persistAndPublishTaskPRSync(ctx, taskID, status.PR, tp, changed, status.OutcomeFieldsPopulated)
 }
 
 // persistAndPublishTaskPRSync writes the reconciled sync state and, on
@@ -1009,7 +1009,9 @@ func (s *Service) SyncTaskPR(ctx context.Context, taskID string, status *PRStatu
 // what a subsequent read of the row returns (codex [P2]). Split out of
 // SyncTaskPR to keep that function within the repo's complexity limits and
 // to make the re-read-before-publish behavior directly testable.
-func (s *Service) persistAndPublishTaskPRSync(ctx context.Context, tp *TaskPR, changed, outcomeFieldsPopulated bool) error {
+func (s *Service) persistAndPublishTaskPRSync(
+	ctx context.Context, taskID string, pr *PR, tp *TaskPR, changed, outcomeFieldsPopulated bool,
+) error {
 	// AC-38/AC-18c: the counter fires at the populated-ness decision point,
 	// before the write is attempted, and survives write failure — it
 	// measures what the sync observed, not whether the store call happened
@@ -1021,7 +1023,7 @@ func (s *Service) persistAndPublishTaskPRSync(ctx context.Context, tp *TaskPR, c
 	// Provider payloads carry the authoritative head/base repository identity
 	// and branch. Reconcile after the TaskPR write so a malformed or
 	// unmatchable payload never prevents the review association from persisting.
-	s.reconcileComparisonTargetFromSync(ctx, taskID, status.PR)
+	s.reconcileComparisonTargetFromSync(ctx, taskID, pr)
 
 	if changed && s.eventBus != nil {
 		published, err := s.store.GetTaskPRByID(ctx, tp.ID)

docs/plans/github-pr-sync-context-regression/plan.md

@@ -0,0 +1,110 @@
+---
+spec: docs/specs/pr-outcome-attribution/spec.md
+created: 2026-08-22
+status: implemented
+---
+
+# Implementation Plan: Restore GitHub PR Sync Context
+
+## Overview
+
+The pull-request outcome change extracted the final sync write into a helper.
+The helper still uses `taskID` and `status.PR`, but its signature does not
+receive either value. As a result, the GitHub package does not compile.
+
+Pass the task and pull-request context into the helper. Keep the existing order:
+persist the task PR, reconcile the comparison target, then publish the committed
+row when semantic data changed.
+
+## Confirmed Root Cause
+
+Commit `9178780ba` extracted `persistAndPublishTaskPRSync` from `SyncTaskPR`.
+The extracted body kept this call:
+
+```go
+s.reconcileComparisonTargetFromSync(ctx, taskID, status.PR)
+```
+
+The new helper signature does not receive `taskID` or `status`. The focused
+backend build therefore stops with these compiler errors:
+
+```text
+internal/github/service_pr_watch.go:1024:43: undefined: taskID
+internal/github/service_pr_watch.go:1024:51: undefined: status
+```
+
+Use this reproduction command when the default Go cache is read-only:
+
+```bash
+cd apps/backend && GOCACHE=/tmp/kandev-go-build-cache make build-kandev
+```
+
+## Backend
+
+### Sync writer context
+
+Update `apps/backend/internal/github/service_pr_watch.go`:
+
+- Add the task ID and pull-request payload to
+  `persistAndPublishTaskPRSync`.
+- Pass `taskID` and `status.PR` from `SyncTaskPR`.
+- Pass `tp.TaskID` and `status.PR` from unwatched lifecycle reconciliation.
+- Keep comparison-target reconciliation after `UpdateTaskPR` succeeds.
+- Keep event publication after comparison-target reconciliation.
+- Return immediately when `UpdateTaskPR` fails.
+
+### Regression coverage
+
+Add `apps/backend/internal/github/service_comparison_target_sync_test.go`.
+Use a comparison-target observer that reads the stored row during its callback.
+The test proves that the write occurs first and that the observer receives the
+correct task and pull-request identity.
+
+Update the direct helper call in
+`apps/backend/internal/github/service_pr_outcome_sync_test.go`. Pass explicit
+task and pull-request context through the helper contract.
+
+Update the unwatched lifecycle caller in
+`apps/backend/internal/github/service_pr_unwatched.go` with the same context.
+
+## Tests
+
+- **What:** the GitHub package compiles after the helper receives its required
+  context.
+  **File:** `apps/backend/internal/github/service_pr_watch.go`.
+  **How:** compile the package through the focused Go test command.
+- **What:** a normal sync persists the task PR before comparison reconciliation.
+  **File:** `apps/backend/internal/github/service_comparison_target_sync_test.go`.
+  **How:** use a fake observer that reads the stored row in its callback.
+- **What:** the helper still publishes the committed row after a stale write.
+  **File:** `apps/backend/internal/github/service_pr_outcome_sync_test.go`.
+  **How:** run the existing deterministic stale-writer regression test.
+
+## Verification Results
+
+- RED: `GOCACHE=/tmp/kandev-go-build-cache go test ./internal/github -run
+  TestSyncTaskPR_PersistsBeforeComparisonTargetReconciliation` failed at the
+  expected undefined `taskID` and `status` compiler errors.
+- GREEN: the focused regression test passed.
+- Targeted tests: the regression test and stale-writer test passed.
+- Backend build: `GOCACHE=/tmp/kandev-go-build-cache make build` passed.
+- Formatting: `gofmt -w` completed for all changed Go files.
+- `git diff --check` passed.
+- Build output was written to ignored files under `apps/backend/bin`.
+- No temporary source files or E2E assets were created.
+
+## Implementation Waves And Parallel Candidates
+
+Wave 1:
+
+- [x] [task-01-restore-sync-context](task-01-restore-sync-context.md) (done)
+
+The task is sequential because the source and tests share one package contract.
+
+## Risks And Out Of Scope
+
+- The fix does not change pull-request field sourcing or storage schemas.
+- The fix does not change comparison-target matching rules.
+- The fix does not change event payloads or event ordering.
+- The fix does not change the root Makefile output suppression.
+- The read-only default Go cache remains an environment concern.

docs/plans/github-pr-sync-context-regression/task-01-restore-sync-context.md

@@ -0,0 +1,100 @@
+---
+id: "01-restore-sync-context"
+title: "Restore GitHub PR sync context"
+status: done
+wave: 1
+depends_on: []
+plan: "plan.md"
+spec: "../../specs/pr-outcome-attribution/spec.md"
+---
+
+# Task 01: Restore GitHub PR Sync Context
+
+## Intent
+
+Restore the missing task and pull-request context in the final sync helper.
+Preserve the existing persistence, reconciliation, and publication order.
+
+## Acceptance
+
+- `apps/backend/internal/github` compiles without undefined identifiers.
+- `SyncTaskPR` persists the task PR before comparison-target reconciliation.
+- The comparison observer receives the same task ID and PR identity as the sync.
+- Existing stale-writer event behavior remains unchanged.
+
+## Red Test First
+
+Add `TestSyncTaskPR_PersistsBeforeComparisonTargetReconciliation` in
+`service_comparison_target_sync_test.go`. The current package must fail to
+compile because the helper does not receive `taskID` or `status.PR`.
+
+## Implementation
+
+- Extend `persistAndPublishTaskPRSync` with `taskID string` and `pr *PR`.
+- Pass `taskID` and `status.PR` from `SyncTaskPR`.
+- Pass `tp.TaskID` and `status.PR` from unwatched lifecycle reconciliation.
+- Use those parameters for `reconcileComparisonTargetFromSync`.
+- Update direct helper calls in existing tests.
+- Do not change storage, reconciliation, or event behavior.
+
+## Files Likely Touched
+
+- `apps/backend/internal/github/service_pr_watch.go`
+- `apps/backend/internal/github/service_pr_unwatched.go`
+- `apps/backend/internal/github/service_comparison_target_sync_test.go`
+- `apps/backend/internal/github/service_pr_outcome_sync_test.go`
+- `docs/plans/github-pr-sync-context-regression/plan.md`
+- `docs/plans/github-pr-sync-context-regression/task-01-restore-sync-context.md`
+
+## Dependencies
+
+None.
+
+## Parallelism
+
+`sequential`. The source and tests share one helper contract.
+
+## Inputs
+
+- `docs/specs/pr-outcome-attribution/spec.md`, section `Sync writer`
+- `docs/specs/platform/workspace-git-status.md`, section
+  `Repository-qualified comparison targets`
+- `docs/decisions/2026-08-19-repository-qualified-comparison-targets.md`
+
+## Verification
+
+Run these commands from the repository root:
+
+```bash
+cd apps/backend && \
+GOCACHE=/tmp/kandev-go-build-cache go test ./internal/github -run 'TestSyncTaskPR_PersistsBeforeComparisonTargetReconciliation|TestPersistAndPublishTaskPRSync_PublishesReReadValueNotStaleInMemoryOne' && \
+GOCACHE=/tmp/kandev-go-build-cache make build
+```
+
+Then run this command from the repository root:
+
+```bash
+git diff --check
+```
+
+## Output Contract
+
+Report the changed files, exact command results, blockers, and remaining risks.
+Update this task and `plan.md` in the same conversation.
+
+## Results
+
+- RED: `GOCACHE=/tmp/kandev-go-build-cache go test ./internal/github -run
+  TestSyncTaskPR_PersistsBeforeComparisonTargetReconciliation` failed with the
+  expected undefined `taskID` and `status` compiler errors.
+- GREEN: `GOCACHE=/tmp/kandev-go-build-cache go test ./internal/github -run
+  TestSyncTaskPR_PersistsBeforeComparisonTargetReconciliation` passed.
+- Targeted regression command passed:
+  `GOCACHE=/tmp/kandev-go-build-cache go test ./internal/github -run
+  "TestSyncTaskPR_PersistsBeforeComparisonTargetReconciliation|TestPersistAndPublishTaskPRSync_PublishesReReadValueNotStaleInMemoryOne"`.
+- Backend build passed:
+  `GOCACHE=/tmp/kandev-go-build-cache make build`.
+- `gofmt -w` completed for all changed Go files.
+- `git diff --check` passed.
+- Build output was written to ignored files under `apps/backend/bin`.
+- No temporary source files or E2E assets were created.

docs/specs/pr-outcome-attribution/spec.md

@@ -2,6 +2,7 @@
 title: Pull request outcome attribution
 status: shipped
 created: 2026-08-12
+updated: 2026-08-22
 owner: kandev
 ---
 
@@ -80,6 +81,11 @@ transaction while reading the outgoing row. This preserves values when an
 operation has no new upstream observation. The stored row is re-read before a
 sync event is published, so the event matches the committed state.
 
+After the task-PR write succeeds, the normal sync reconciles the
+repository-qualified comparison target. The reconciliation receives the same
+task ID and pull-request payload as the sync. A reconciliation error does not
+reverse the task-PR write.
+
 ## Acceptance criteria
 
 - **AC-01 to AC-03:** schema creation and replay add nullable columns without
@@ -99,6 +105,8 @@ sync event is published, so the event matches the committed state.
 - **AC-16 and AC-17:** the auto-merge timestamp is write-once.
 - **AC-18 to AC-18c:** unchanged syncs publish no update, changed syncs publish
   the committed row, and failed writes publish nothing.
+- **AC-18d:** a successful normal sync keeps its task and pull-request context
+  through persistence, comparison-target reconciliation, and event publication.
 - **AC-19:** draft pull requests remain non-mergeable in the stored state.
 - **AC-30 and AC-30b:** the backend emits explicit nullable JSON keys and the
   frontend exposes types without adding a core screen.
@@ -109,6 +117,14 @@ sync event is published, so the event matches the committed state.
 - **AC-43, AC-43a, and AC-43p:** replace and restore resolve and preserve
   outcome fields inside their own transaction using explicit observation flags.
 
+## Regression scenarios
+
+- **GIVEN** a stored task PR and a comparison-target observer, **WHEN** a normal
+  sync applies an authoritative pull-request payload, **THEN** the write succeeds
+  before the observer receives the same task and pull-request identity.
+- **GIVEN** comparison-target reconciliation returns an error, **WHEN** the
+  task-PR write succeeds, **THEN** the stored task-PR update remains committed.
+
 ## Verification
 
 Backend unit and persistence tests cover migration replay, source decoding,