feat(github): add PR outcome attribution (merge, close, draft, changed files) (#2614)

Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>

apps/backend/internal/github/client_helpers.go

@@ -160,6 +160,12 @@ func newPRStatus(pr *PR, reviews []PRReview, checks []CheckRun) *PRStatus {
 		// from the batched-GraphQL path that only carries rollup state.
 		ChecksPopulated:       true,
 		ReviewCountsPopulated: true,
+		// This path fetched a full single pull request (REST or gh CLI), so
+		// is_draft/changed_files/merged_by_login are real observations, not
+		// "I didn't look" (AC-10). ClosureAttributionPopulated stays false:
+		// neither REST caller can see the closing actor — that's GraphQL-only
+		// (AC-15).
+		OutcomeFieldsPopulated: true,
 	}
 }
 

apps/backend/internal/github/client_helpers_test.go

@@ -7,6 +7,24 @@ import (
 	"time"
 )
 
+// TestNewPRStatus_MarksOutcomeFieldsPopulatedNotClosureAttribution covers
+// AC-10: newPRStatus (the single convergence point for REST and gh CLI
+// single-PR paths) always marks the outcome-field group populated, since it
+// fetched a full pull request, but leaves closure attribution unpopulated —
+// neither REST caller can see the closing actor (AC-15).
+func TestNewPRStatus_MarksOutcomeFieldsPopulatedNotClosureAttribution(t *testing.T) {
+	status := newPRStatus(&PR{Number: 1}, nil, nil)
+	if !status.OutcomeFieldsPopulated {
+		t.Error("OutcomeFieldsPopulated = false, want true")
+	}
+	if status.ClosureAttributionPopulated {
+		t.Error("ClosureAttributionPopulated = true, want false")
+	}
+	if status.ClosedByLogin != "" {
+		t.Errorf("ClosedByLogin = %q, want empty", status.ClosedByLogin)
+	}
+}
+
 func TestGetPRFeedbackStartsIndependentRequestsConcurrently(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
 	defer cancel()

apps/backend/internal/github/gh_client.go

@@ -23,6 +23,8 @@ import (
 // sync.
 const ghSearchReposPath = "search/repositories"
 
+const ghMergeableState = "MERGEABLE"
+
 // ghAccessibleReposPath is the GET /user/repos endpoint (with affiliation +
 // sort + per_page baked in) that backs ListAccessibleRepos. It returns a flat
 // JSON array on the core REST quota, replacing the per-org search fan-out.
@@ -183,19 +185,24 @@ type ghRequestedReviewer struct {
 
 // ghPR is the JSON shape returned by gh pr list/view.
 type ghPR struct {
-	Number              int                   `json:"number"`
-	Title               string                `json:"title"`
-	URL                 string                `json:"url"`
-	State               string                `json:"state"`
-	Body                string                `json:"body"`
-	HeadRefName         string                `json:"headRefName"`
-	HeadRefOid          string                `json:"headRefOid"`
-	BaseRefName         string                `json:"baseRefName"`
-	IsDraft             bool                  `json:"isDraft"`
-	Mergeable           string                `json:"mergeable"`
-	MergeStateStatus    string                `json:"mergeStateStatus"`
-	Additions           int                   `json:"additions"`
-	Deletions           int                   `json:"deletions"`
+	Number      int    `json:"number"`
+	Title       string `json:"title"`
+	URL         string `json:"url"`
+	State       string `json:"state"`
+	Body        string `json:"body"`
+	HeadRefName string `json:"headRefName"`
+	HeadRefOid  string `json:"headRefOid"`
+	BaseRefName string `json:"baseRefName"`
+	// IsDraft is a pointer: AC-12a requires distinguishing an omitted or
+	// null isDraft from a genuine false, which a plain bool can't after decode.
+	IsDraft          *bool  `json:"isDraft"`
+	Mergeable        string `json:"mergeable"`
+	MergeStateStatus string `json:"mergeStateStatus"`
+	Additions        int    `json:"additions"`
+	Deletions        int    `json:"deletions"`
+	// ChangedFiles is a pointer for the same reason as IsDraft (AC-12a): 0 is
+	// a legitimate observation and must stay distinguishable from absent/null.
+	ChangedFiles        *int                  `json:"changedFiles"`
 	CreatedAt           time.Time             `json:"createdAt"`
 	UpdatedAt           time.Time             `json:"updatedAt"`
 	MergedAt            string                `json:"mergedAt"`
@@ -207,6 +214,17 @@ type ghPR struct {
 	Author              struct {
 		Login string `json:"login"`
 	} `json:"author"`
+	// MergedBy decodes to a zero-value Login on gh's `null` for an unmerged
+	// PR — never a placeholder value.
+	MergedBy struct {
+		Login string `json:"login"`
+	} `json:"mergedBy"`
+	// AutoMergeRequest is a pointer: gh returns null when auto-merge was
+	// never armed. Any non-nil value means "armed at fetch time" — never
+	// "merged by auto-merge" (auto_merge is cleared once it fires).
+	AutoMergeRequest *struct {
+		EnabledAt string `json:"enabledAt"`
+	} `json:"autoMergeRequest"`
 }
 
 type ghRepository struct {
@@ -247,7 +265,7 @@ type ghIssue struct {
 func (c *GHClient) GetPR(ctx context.Context, owner, repo string, number int) (*PR, error) {
 	out, err := c.run(ctx, "pr", "view", fmt.Sprintf("%d", number),
 		"--repo", fmt.Sprintf("%s/%s", owner, repo),
-		"--json", "number,title,url,state,body,headRefName,headRefOid,baseRefName,author,isDraft,mergeable,mergeStateStatus,additions,deletions,createdAt,updatedAt,mergedAt,closedAt,reviewRequests,maintainerCanModify,headRepository,headRepositoryOwner")
+		"--json", "number,title,url,state,body,headRefName,headRefOid,baseRefName,author,isDraft,mergeable,mergeStateStatus,additions,deletions,changedFiles,mergedBy,autoMergeRequest,createdAt,updatedAt,mergedAt,closedAt,reviewRequests,maintainerCanModify,headRepository,headRepositoryOwner")
 	if err != nil {
 		if isNotFoundErr(err) {
 			return nil, &GitHubAPIError{
@@ -1306,30 +1324,42 @@ func convertGHPR(raw *ghPR, owner, repo string) *PR {
 	if raw.MergedAt != "" {
 		state = prStateMerged
 	}
+	draft, changedFiles := false, 0
+	if raw.IsDraft != nil {
+		draft = *raw.IsDraft
+	}
+	if raw.ChangedFiles != nil {
+		changedFiles = *raw.ChangedFiles
+	}
 	pr := &PR{
-		Number:              raw.Number,
-		Title:               raw.Title,
-		URL:                 raw.URL,
-		HTMLURL:             raw.URL,
-		State:               state,
-		Body:                raw.Body,
-		HeadBranch:          raw.HeadRefName,
-		HeadSHA:             raw.HeadRefOid,
-		BaseBranch:          raw.BaseRefName,
-		AuthorLogin:         raw.Author.Login,
-		RepoOwner:           owner,
-		RepoName:            repo,
-		MaintainerCanModify: raw.MaintainerCanModify,
-		Draft:               raw.IsDraft,
-		Mergeable:           raw.Mergeable == "MERGEABLE",
-		MergeableState:      strings.ToLower(raw.MergeStateStatus),
-		Additions:           raw.Additions,
-		Deletions:           raw.Deletions,
-		RequestedReviewers:  convertGHRequestedReviewers(raw.ReviewRequests),
-		CreatedAt:           raw.CreatedAt,
-		UpdatedAt:           raw.UpdatedAt,
-		MergedAt:            parseTimePtr(raw.MergedAt),
-		ClosedAt:            parseTimePtr(raw.ClosedAt),
+		Number:               raw.Number,
+		Title:                raw.Title,
+		URL:                  raw.URL,
+		HTMLURL:              raw.URL,
+		State:                state,
+		Body:                 raw.Body,
+		HeadBranch:           raw.HeadRefName,
+		HeadSHA:              raw.HeadRefOid,
+		BaseBranch:           raw.BaseRefName,
+		AuthorLogin:          raw.Author.Login,
+		RepoOwner:            owner,
+		RepoName:             repo,
+		MaintainerCanModify:  raw.MaintainerCanModify,
+		Draft:                draft,
+		IsDraftObserved:      raw.IsDraft != nil,
+		Mergeable:            raw.Mergeable == ghMergeableState,
+		MergeableState:       strings.ToLower(raw.MergeStateStatus),
+		Additions:            raw.Additions,
+		Deletions:            raw.Deletions,
+		ChangedFiles:         changedFiles,
+		ChangedFilesObserved: raw.ChangedFiles != nil,
+		MergedByLogin:        raw.MergedBy.Login,
+		AutoMergeEnabled:     raw.AutoMergeRequest != nil,
+		RequestedReviewers:   convertGHRequestedReviewers(raw.ReviewRequests),
+		CreatedAt:            raw.CreatedAt,
+		UpdatedAt:            raw.UpdatedAt,
+		MergedAt:             parseTimePtr(raw.MergedAt),
+		ClosedAt:             parseTimePtr(raw.ClosedAt),
 	}
 	pr.HeadRepoNodeID = raw.HeadRepository.ID
 	pr.HeadRepoOwner = raw.HeadRepositoryOwner.Login

apps/backend/internal/github/gh_client_reads_test.go

@@ -147,6 +147,88 @@ func TestGHClient_GetPR_NotFoundBecomesTypedError(t *testing.T) {
 	}
 }
 
+// TestGHClient_GetPR_RequestsOutcomeFields covers AC-09: the --json field
+// list gains changedFiles, mergedBy, autoMergeRequest, and never requests
+// closedBy (absent from the gh CLI's PR field set). The decoded PR carries
+// the three new values.
+func TestGHClient_GetPR_RequestsOutcomeFields(t *testing.T) {
+	calls := newFakeGH(t, ghResponse{
+		Prefix: "pr view",
+		Stdout: `{"number":2554,"title":"fix","url":"https://github.com/kdlbs/kandev/pull/2554",
+			"state":"MERGED","headRefName":"h","headRefOid":"abc","baseRefName":"main",
+			"isDraft":false,"mergeable":"MERGEABLE","mergeStateStatus":"CLEAN",
+			"additions":10,"deletions":2,"changedFiles":8,
+			"author":{"login":"alice"},"mergedBy":{"login":"carlosflorencio"},
+			"autoMergeRequest":{"enabledAt":"2026-08-12T00:00:00Z"},
+			"createdAt":"2026-08-01T00:00:00Z","updatedAt":"2026-08-02T00:00:00Z",
+			"mergedAt":"2026-08-12T00:00:00Z","closedAt":"2026-08-12T00:00:00Z"}`,
+	})
+
+	pr, err := NewGHClient().GetPR(context.Background(), "kdlbs", "kandev", 2554)
+	if err != nil {
+		t.Fatalf("GetPR: %v", err)
+	}
+
+	argv := calls(t)[0]
+	jsonArgIdx := -1
+	for i, a := range argv {
+		if a == "--json" {
+			jsonArgIdx = i + 1
+			break
+		}
+	}
+	if jsonArgIdx < 0 || jsonArgIdx >= len(argv) {
+		t.Fatalf("no --json argument found in %v", argv)
+	}
+	fieldList := argv[jsonArgIdx]
+	for _, want := range []string{"changedFiles", "mergedBy", "autoMergeRequest"} {
+		if !strings.Contains(fieldList, want) {
+			t.Errorf("--json field list missing %q: %s", want, fieldList)
+		}
+	}
+	if strings.Contains(fieldList, "closedBy") {
+		t.Errorf("--json field list must not request closedBy (not in gh CLI's PR field set): %s", fieldList)
+	}
+
+	if pr.ChangedFiles != 8 {
+		t.Errorf("ChangedFiles = %d, want 8", pr.ChangedFiles)
+	}
+	if pr.MergedByLogin != "carlosflorencio" {
+		t.Errorf("MergedByLogin = %q, want carlosflorencio", pr.MergedByLogin)
+	}
+	if !pr.AutoMergeEnabled {
+		t.Error("AutoMergeEnabled = false, want true (non-null autoMergeRequest)")
+	}
+}
+
+// TestGHClient_GetPR_NoMergerOrAutoMergeDecodesToZeroValues covers the
+// "never observed / not armed" side: a null mergedBy and a null
+// autoMergeRequest must decode to an empty login and AutoMergeEnabled=false,
+// never a placeholder value.
+func TestGHClient_GetPR_NoMergerOrAutoMergeDecodesToZeroValues(t *testing.T) {
+	newFakeGH(t, ghResponse{
+		Prefix: "pr view",
+		Stdout: `{"number":2476,"title":"wip","url":"https://github.com/kdlbs/kandev/pull/2476",
+			"state":"CLOSED","isDraft":true,"changedFiles":114,
+			"author":{"login":"alice"},"mergedBy":null,"autoMergeRequest":null,
+			"createdAt":"2026-08-01T00:00:00Z","updatedAt":"2026-08-02T00:00:00Z","closedAt":"2026-08-05T00:00:00Z"}`,
+	})
+
+	pr, err := NewGHClient().GetPR(context.Background(), "kdlbs", "kandev", 2476)
+	if err != nil {
+		t.Fatalf("GetPR: %v", err)
+	}
+	if pr.MergedByLogin != "" {
+		t.Errorf("MergedByLogin = %q, want empty", pr.MergedByLogin)
+	}
+	if pr.AutoMergeEnabled {
+		t.Error("AutoMergeEnabled = true, want false (null autoMergeRequest)")
+	}
+	if pr.ChangedFiles != 114 {
+		t.Errorf("ChangedFiles = %d, want 114", pr.ChangedFiles)
+	}
+}
+
 func TestGHClient_GetPR_UnparseableOutput(t *testing.T) {
 	newFakeGH(t, ghResponse{Prefix: "pr view", Stdout: "not json"})
 	if _, err := NewGHClient().GetPR(context.Background(), "acme", "widget", 7); err == nil ||

apps/backend/internal/github/gh_client_test.go

@@ -396,7 +396,7 @@ func TestConvertGHPR(t *testing.T) {
 		HeadRefName: "feature-branch",
 		HeadRefOid:  "abc123def456",
 		BaseRefName: "main",
-		IsDraft:     true,
+		IsDraft:     boolPtr(true),
 		Mergeable:   "MERGEABLE",
 		Additions:   100,
 		Deletions:   50,
@@ -428,6 +428,9 @@ func TestConvertGHPR(t *testing.T) {
 	if !pr.Draft {
 		t.Error("expected draft = true")
 	}
+	if !pr.IsDraftObserved {
+		t.Error("expected IsDraftObserved = true when isDraft is present in the response")
+	}
 	if !pr.Mergeable {
 		t.Error("expected mergeable = true")
 	}
@@ -448,6 +451,41 @@ func TestConvertGHPR(t *testing.T) {
 	}
 }
 
+// TestConvertGHPR_MissingIsDraftAndChangedFilesLeavesUnobserved covers
+// AC-12a on the gh CLI decode path: a response that omits isDraft and
+// changedFiles must decode to unobserved (Observed=false), not a fabricated
+// false/0 masquerading as a real observation.
+func TestConvertGHPR_MissingIsDraftAndChangedFilesLeavesUnobserved(t *testing.T) {
+	raw := &ghPR{Number: 7, State: "OPEN"}
+	pr := convertGHPR(raw, "owner", "repo")
+	if pr.IsDraftObserved {
+		t.Error("IsDraftObserved = true, want false when isDraft is absent from the response")
+	}
+	if pr.ChangedFilesObserved {
+		t.Error("ChangedFilesObserved = true, want false when changedFiles is absent from the response")
+	}
+}
+
+// TestConvertGHPR_ExplicitFalseAndZeroAreObserved covers AC-12a's other
+// half: a response that genuinely reports isDraft=false and changedFiles=0
+// must be distinguishable from one that omits them — both mark Observed=true.
+func TestConvertGHPR_ExplicitFalseAndZeroAreObserved(t *testing.T) {
+	raw := &ghPR{Number: 7, State: "OPEN", IsDraft: boolPtr(false), ChangedFiles: intPtr(0)}
+	pr := convertGHPR(raw, "owner", "repo")
+	if !pr.IsDraftObserved {
+		t.Error("IsDraftObserved = false, want true for an explicit isDraft=false")
+	}
+	if pr.Draft {
+		t.Error("Draft = true, want false")
+	}
+	if !pr.ChangedFilesObserved {
+		t.Error("ChangedFilesObserved = false, want true for an explicit changedFiles=0")
+	}
+	if pr.ChangedFiles != 0 {
+		t.Errorf("ChangedFiles = %d, want 0", pr.ChangedFiles)
+	}
+}
+
 func TestConvertGHPR_Merged(t *testing.T) {
 	raw := &ghPR{
 		Number:   1,

apps/backend/internal/github/graphql.go

@@ -161,12 +161,29 @@ func chunkRefs[T any](refs []T, chunkSize int) [][]T {
 	return out
 }
 
+// timelineActor is a GraphQL Actor's login, used by the closed-event
+// timeline selection to attribute who closed a PR.
+type timelineActor struct {
+	Login string `json:"login"`
+}
+
+// timelineClosedEventNode is one node of the `timelineItems(itemTypes:
+// CLOSED_EVENT)` selection. Actor is a pointer because GitHub can return a
+// null actor (e.g. a bot-deleted account), which must leave closure
+// attribution unpopulated rather than writing an empty login.
+type timelineClosedEventNode struct {
+	Actor *timelineActor `json:"actor"`
+}
+
 // batchedPRResult is the decoded shape of one aliased pullRequest block.
 type batchedPRResult struct {
-	State       string `json:"state"`
-	Title       string `json:"title"`
-	URL         string `json:"url"`
-	IsDraft     bool   `json:"isDraft"`
+	State string `json:"state"`
+	Title string `json:"title"`
+	URL   string `json:"url"`
+	// IsDraft is a pointer: AC-12a requires distinguishing an upstream
+	// response that omits or nulls isDraft from one that genuinely reports
+	// false, and a plain bool can't tell the two apart after decode.
+	IsDraft     *bool  `json:"isDraft"`
 	Mergeable   string `json:"mergeable"`
 	MergeStatus string `json:"mergeStateStatus"`
 	HeadRefName string `json:"headRefName"`
@@ -174,9 +191,25 @@ type batchedPRResult struct {
 	HeadRefOid  string `json:"headRefOid"`
 	Additions   int    `json:"additions"`
 	Deletions   int    `json:"deletions"`
-	Author      struct {
+	// ChangedFiles is a pointer for the same reason as IsDraft (AC-12a): 0 is
+	// a legitimate observation and must stay distinguishable from absent/null.
+	ChangedFiles *int `json:"changedFiles"`
+	Author       struct {
 		Login string `json:"login"`
 	} `json:"author"`
+	// MergedBy is a value (not a pointer) because GitHub always serializes
+	// the field for a PullRequest, using a zero-value login when there is no
+	// merger; the empty-login case is handled explicitly in
+	// convertBatchedPRResult rather than relying on a nil check.
+	MergedBy struct {
+		Login string `json:"login"`
+	} `json:"mergedBy"`
+	// AutoMergeRequest is a pointer: GitHub returns null when auto-merge was
+	// never armed. Any non-nil value means "armed at fetch time" — never
+	// "merged by auto-merge" (auto_merge is cleared once it fires).
+	AutoMergeRequest *struct {
+		EnabledAt string `json:"enabledAt"`
+	} `json:"autoMergeRequest"`
 	CreatedAt time.Time `json:"createdAt"`
 	UpdatedAt time.Time `json:"updatedAt"`
 	MergedAt  string    `json:"mergedAt"`
@@ -197,6 +230,14 @@ type batchedPRResult struct {
 			} `json:"commit"`
 		} `json:"nodes"`
 	} `json:"commits"`
+	// TimelineItems carries at most the single most-recent CLOSED_EVENT, so
+	// closed-by attribution reflects the latest closure even across a
+	// reopen-then-close cycle. closedBy has no equivalent on the REST pulls
+	// endpoint or the gh CLI's PR field set (AC-09, AC-15) — this GraphQL
+	// selection is the only source.
+	TimelineItems struct {
+		Nodes []timelineClosedEventNode `json:"nodes"`
+	} `json:"timelineItems"`
 }
 
 type batchedBranchPRNode struct {
@@ -370,13 +411,15 @@ func buildBatchedPRQuery(refs []graphQLPRRef) (string, map[string]any) {
 // the batched and single-PR paths returning the same data.
 func prFieldsBlock() string {
 	return `state title url isDraft mergeable mergeStateStatus ` +
-		`headRefName baseRefName headRefOid additions deletions ` +
-		`author { login } createdAt updatedAt mergedAt closedAt ` +
+		`headRefName baseRefName headRefOid additions deletions changedFiles ` +
+		`author { login } mergedBy { login } autoMergeRequest { enabledAt } ` +
+		`createdAt updatedAt mergedAt closedAt ` +
 		`reviews(last: 100) { nodes { state author { login } submittedAt } } ` +
 		`reviewRequests(first: 0) { totalCount } ` +
 		fmt.Sprintf(`reviewThreads(first: %d) { totalCount nodes { isResolved } pageInfo { hasNextPage endCursor } } `,
 			graphQLReviewThreadPageSize) +
-		`commits(last: 1) { nodes { commit { statusCheckRollup { state } } } }`
+		`commits(last: 1) { nodes { commit { statusCheckRollup { state } } } } ` +
+		`timelineItems(last: 1, itemTypes: CLOSED_EVENT) { nodes { ... on ClosedEvent { actor { login } } } }`
 }
 
 // buildBatchedBranchQuery emits one aliased pullRequests(headRefName:) block
@@ -400,27 +443,39 @@ func convertBatchedPRResult(raw *batchedPRResult, owner, repo string, number int
 	if raw.MergedAt != "" {
 		state = prStateMerged
 	}
+	draft, changedFiles := false, 0
+	if raw.IsDraft != nil {
+		draft = *raw.IsDraft
+	}
+	if raw.ChangedFiles != nil {
+		changedFiles = *raw.ChangedFiles
+	}
 	pr := &PR{
-		Number:         number,
-		Title:          raw.Title,
-		URL:            raw.URL,
-		HTMLURL:        raw.URL,
-		State:          state,
-		HeadBranch:     raw.HeadRefName,
-		HeadSHA:        raw.HeadRefOid,
-		BaseBranch:     raw.BaseRefName,
-		AuthorLogin:    raw.Author.Login,
-		RepoOwner:      owner,
-		RepoName:       repo,
-		Draft:          raw.IsDraft,
-		Mergeable:      raw.Mergeable == "MERGEABLE",
-		MergeableState: strings.ToLower(raw.MergeStatus),
-		Additions:      raw.Additions,
-		Deletions:      raw.Deletions,
-		CreatedAt:      raw.CreatedAt,
-		UpdatedAt:      raw.UpdatedAt,
-		MergedAt:       parseTimePtr(raw.MergedAt),
-		ClosedAt:       parseTimePtr(raw.ClosedAt),
+		Number:               number,
+		Title:                raw.Title,
+		URL:                  raw.URL,
+		HTMLURL:              raw.URL,
+		State:                state,
+		HeadBranch:           raw.HeadRefName,
+		HeadSHA:              raw.HeadRefOid,
+		BaseBranch:           raw.BaseRefName,
+		AuthorLogin:          raw.Author.Login,
+		RepoOwner:            owner,
+		RepoName:             repo,
+		Draft:                draft,
+		IsDraftObserved:      raw.IsDraft != nil,
+		Mergeable:            raw.Mergeable == ghMergeableState,
+		MergeableState:       strings.ToLower(raw.MergeStatus),
+		Additions:            raw.Additions,
+		Deletions:            raw.Deletions,
+		ChangedFiles:         changedFiles,
+		ChangedFilesObserved: raw.ChangedFiles != nil,
+		MergedByLogin:        raw.MergedBy.Login,
+		AutoMergeEnabled:     raw.AutoMergeRequest != nil,
+		CreatedAt:            raw.CreatedAt,
+		UpdatedAt:            raw.UpdatedAt,
+		MergedAt:             parseTimePtr(raw.MergedAt),
+		ClosedAt:             parseTimePtr(raw.ClosedAt),
 	}
 
 	reviewState := summarizeReviewState(raw.Reviews.Nodes)
@@ -434,6 +489,7 @@ func convertBatchedPRResult(raw *batchedPRResult, owner, repo string, number int
 			unresolved++
 		}
 	}
+	closedByLogin, closureAttributionPopulated := closedEventActor(raw.TimelineItems.Nodes)
 	return &PRStatus{
 		PR:                               pr,
 		ReviewState:                      reviewState,
@@ -444,9 +500,24 @@ func convertBatchedPRResult(raw *batchedPRResult, owner, repo string, number int
 		ReviewCountsPopulated:            true,
 		UnresolvedReviewThreads:          unresolved,
 		UnresolvedReviewThreadsPopulated: true,
+		OutcomeFieldsPopulated:           true,
+		ClosedByLogin:                    closedByLogin,
+		ClosureAttributionPopulated:      closureAttributionPopulated,
 	}
 }
 
+// closedEventActor extracts the closed-event actor's login from a
+// `timelineItems(itemTypes: CLOSED_EVENT)` node list. Returns
+// populated=false when there is no node or the node's actor is null (e.g. a
+// bot-deleted account) — the caller must not write an empty login as if it
+// were an observation (AC-15).
+func closedEventActor(nodes []timelineClosedEventNode) (login string, populated bool) {
+	if len(nodes) == 0 || nodes[0].Actor == nil || nodes[0].Actor.Login == "" {
+		return "", false
+	}
+	return nodes[0].Actor.Login, true
+}
+
 // normalizeGraphQLCheckRollupState converts GitHub's GraphQL status-rollup
 // enum to the smaller contract shared by REST and TaskPR persistence.
 func normalizeGraphQLCheckRollupState(raw string) string {

apps/backend/internal/github/graphql_test.go

@@ -164,6 +164,49 @@ func TestBuildBatchedPRQuery_GroupsByRepo(t *testing.T) {
 	}
 }
 
+// TestPRFieldsBlock_RequestsOutcomeFields covers AC-08: the shared field
+// block requests changedFiles, mergedBy, autoMergeRequest, and a
+// CLOSED_EVENT timeline selection, so both the batched PR query and the
+// batched branch query return the same fields.
+func TestPRFieldsBlock_RequestsOutcomeFields(t *testing.T) {
+	block := prFieldsBlock()
+	for _, want := range []string{"changedFiles", "mergedBy { login }", "autoMergeRequest", "CLOSED_EVENT"} {
+		if !strings.Contains(block, want) {
+			t.Errorf("prFieldsBlock() missing %q: %s", want, block)
+		}
+	}
+	if strings.Contains(block, "closedBy") {
+		t.Errorf("prFieldsBlock() must not request closedBy (absent from PullRequest type): %s", block)
+	}
+
+	prQuery, _ := buildBatchedPRQuery([]graphQLPRRef{{Owner: "o", Repo: "r", Number: 1}})
+	if !strings.Contains(prQuery, "changedFiles") || !strings.Contains(prQuery, "CLOSED_EVENT") {
+		t.Errorf("buildBatchedPRQuery must include the outcome fields: %s", prQuery)
+	}
+	branchQuery, _ := buildBatchedBranchQuery([]graphQLBranchRef{{Owner: "o", Repo: "r", Branch: "feat"}})
+	if !strings.Contains(branchQuery, "changedFiles") || !strings.Contains(branchQuery, "CLOSED_EVENT") {
+		t.Errorf("buildBatchedBranchQuery must include the outcome fields: %s", branchQuery)
+	}
+}
+
+// TestPRFieldsBlock_PinsLastOneForClosedEventOrdering is the AC-08a
+// regression: closedEventActor (below) trusts the query to hand it at most
+// one timelineItems node and unconditionally reads nodes[0], so "most
+// recent closure" is guaranteed entirely by the GraphQL connection argument,
+// not by any Go-side ordering logic. TestPRFieldsBlock_RequestsOutcomeFields
+// only substring-matches "CLOSED_EVENT", which stays green if `last: 1` were
+// changed to `first: N` — GitHub connection ordering is oldest-first, so
+// `first: N` would silently attribute closure to the PR's FIRST close
+// instead of its latest one across a reopen cycle, with every existing
+// single-node test still passing. Pin the exact argument.
+func TestPRFieldsBlock_PinsLastOneForClosedEventOrdering(t *testing.T) {
+	block := prFieldsBlock()
+	const want = "timelineItems(last: 1, itemTypes: CLOSED_EVENT)"
+	if !strings.Contains(block, want) {
+		t.Errorf("prFieldsBlock() = %s, want it to contain %q", block, want)
+	}
+}
+
 func TestBuildBatchedBranchQuery_AliasesAllBranches(t *testing.T) {
 	q, _ := buildBatchedBranchQuery([]graphQLBranchRef{
 		{Owner: "o", Repo: "r", Branch: "feat-1"},
@@ -180,6 +223,144 @@ func TestBuildBatchedBranchQuery_AliasesAllBranches(t *testing.T) {
 	}
 }
 
+// TestConvertBatchedPRResult_SetsOutcomeFieldsPopulatedUnconditionally
+// covers AC-10: the batched GraphQL path always marks the outcome-field
+// group populated, since it always fetches a full pull request.
+func TestConvertBatchedPRResult_SetsOutcomeFieldsPopulatedUnconditionally(t *testing.T) {
+	status := convertBatchedPRResult(&batchedPRResult{State: "OPEN"}, "o", "r", 1)
+	if !status.OutcomeFieldsPopulated {
+		t.Error("OutcomeFieldsPopulated = false, want true")
+	}
+}
+
+// TestConvertBatchedPRResult_MergedByAndChangedFiles covers AC-12's upstream
+// half: a merged PR with a non-null mergedBy and a real changedFiles count
+// converts onto PR, not just PRStatus.
+func TestConvertBatchedPRResult_MergedByAndChangedFiles(t *testing.T) {
+	raw := &batchedPRResult{
+		State:        "MERGED",
+		MergedAt:     "2026-08-12T00:00:00Z",
+		ChangedFiles: intPtr(8),
+	}
+	raw.MergedBy.Login = "carlosflorencio"
+	status := convertBatchedPRResult(raw, "kdlbs", "kandev", 2554)
+	if status.PR.ChangedFiles != 8 {
+		t.Errorf("ChangedFiles = %d, want 8", status.PR.ChangedFiles)
+	}
+	if !status.PR.ChangedFilesObserved {
+		t.Error("ChangedFilesObserved = false, want true for an explicit changedFiles=8")
+	}
+	if status.PR.MergedByLogin != "carlosflorencio" {
+		t.Errorf("MergedByLogin = %q, want carlosflorencio", status.PR.MergedByLogin)
+	}
+}
+
+// TestConvertBatchedPRResult_MissingIsDraftAndChangedFilesLeavesUnobserved
+// covers AC-12a: a batched GraphQL result that omits isDraft and
+// changedFiles must decode to unobserved (Observed=false), so
+// resolveTaskPROutcomeFields writes NULL for those columns instead of a
+// fabricated false/0 that would masquerade as a real observation.
+// TestConvertBatchedPRResult_SetsOutcomeFieldsPopulatedUnconditionally
+// above proves the *group* flag stays true for this exact input — this test
+// proves the two field-level flags do NOT, which is what AC-12a actually
+// requires ("each field is judged on its own presence").
+func TestConvertBatchedPRResult_MissingIsDraftAndChangedFilesLeavesUnobserved(t *testing.T) {
+	status := convertBatchedPRResult(&batchedPRResult{State: "OPEN"}, "o", "r", 1)
+	if status.PR.IsDraftObserved {
+		t.Error("IsDraftObserved = true, want false when isDraft is absent from the response")
+	}
+	if status.PR.ChangedFilesObserved {
+		t.Error("ChangedFilesObserved = true, want false when changedFiles is absent from the response")
+	}
+}
+
+// TestConvertBatchedPRResult_ExplicitFalseAndZeroAreObserved covers AC-12a's
+// other half: a response that genuinely reports isDraft=false and
+// changedFiles=0 must be distinguishable from one that omits them — both
+// mark Observed=true.
+func TestConvertBatchedPRResult_ExplicitFalseAndZeroAreObserved(t *testing.T) {
+	status := convertBatchedPRResult(
+		&batchedPRResult{State: "OPEN", IsDraft: boolPtr(false), ChangedFiles: intPtr(0)}, "o", "r", 1,
+	)
+	if !status.PR.IsDraftObserved {
+		t.Error("IsDraftObserved = false, want true for an explicit isDraft=false")
+	}
+	if status.PR.Draft {
+		t.Error("Draft = true, want false")
+	}
+	if !status.PR.ChangedFilesObserved {
+		t.Error("ChangedFilesObserved = false, want true for an explicit changedFiles=0")
+	}
+	if status.PR.ChangedFiles != 0 {
+		t.Errorf("ChangedFiles = %d, want 0", status.PR.ChangedFiles)
+	}
+}
+
+// TestConvertBatchedPRResult_ClosedEventActorPopulatesAttribution covers
+// AC-14: a closed PR carrying a ClosedEvent timeline node with a non-empty
+// actor login marks closure attribution populated and copies the login.
+func TestConvertBatchedPRResult_ClosedEventActorPopulatesAttribution(t *testing.T) {
+	raw := &batchedPRResult{State: "CLOSED"}
+	raw.TimelineItems.Nodes = []timelineClosedEventNode{{}}
+	raw.TimelineItems.Nodes[0].Actor = &timelineActor{Login: "nova28"}
+	status := convertBatchedPRResult(raw, "kdlbs", "kandev", 2476)
+	if !status.ClosureAttributionPopulated {
+		t.Fatal("ClosureAttributionPopulated = false, want true")
+	}
+	if status.ClosedByLogin != "nova28" {
+		t.Errorf("ClosedByLogin = %q, want nova28", status.ClosedByLogin)
+	}
+}
+
+// TestConvertBatchedPRResult_NullActorLeavesAttributionUnpopulated covers
+// AC-15's decode half: no timeline node, or a node with a null actor, must
+// not mark closure attribution populated.
+func TestConvertBatchedPRResult_NullActorLeavesAttributionUnpopulated(t *testing.T) {
+	t.Run("no timeline nodes", func(t *testing.T) {
+		status := convertBatchedPRResult(&batchedPRResult{State: "CLOSED"}, "o", "r", 1)
+		if status.ClosureAttributionPopulated {
+			t.Error("ClosureAttributionPopulated = true, want false")
+		}
+		if status.ClosedByLogin != "" {
+			t.Errorf("ClosedByLogin = %q, want empty", status.ClosedByLogin)
+		}
+	})
+	t.Run("null actor", func(t *testing.T) {
+		raw := &batchedPRResult{State: "CLOSED"}
+		raw.TimelineItems.Nodes = []timelineClosedEventNode{{}}
+		status := convertBatchedPRResult(raw, "o", "r", 1)
+		if status.ClosureAttributionPopulated {
+			t.Error("ClosureAttributionPopulated = true, want false")
+		}
+	})
+}
+
+// TestConvertBatchedPRResult_ClosedEventActorReflectsLatestClosureAfterReopen
+// is the AC-08a close-reopen-close regression. A PR closed by "alice",
+// reopened, then closed again by "bob" must attribute closure to "bob" — the
+// PR's CURRENT closure, not its first one ever. `timelineItems(last: 1, ...)`
+// is what makes this true: GitHub only ever sends the single most-recent
+// CLOSED_EVENT node in the slice this test constructs, so decoding it and
+// reading nodes[0] (closedEventActor's actual logic) yields "bob". This
+// documents the semantic the pinned `last: 1` argument
+// (TestPRFieldsBlock_PinsLastOneForClosedEventOrdering) exists to guarantee.
+func TestConvertBatchedPRResult_ClosedEventActorReflectsLatestClosureAfterReopen(t *testing.T) {
+	raw := &batchedPRResult{State: "CLOSED"}
+	// What GitHub returns for timelineItems(last: 1, itemTypes: CLOSED_EVENT)
+	// after alice's close, a reopen, then bob's close: only bob's event.
+	raw.TimelineItems.Nodes = []timelineClosedEventNode{
+		{Actor: &timelineActor{Login: "bob"}},
+	}
+	status := convertBatchedPRResult(raw, "kdlbs", "kandev", 3001)
+	if !status.ClosureAttributionPopulated {
+		t.Fatal("ClosureAttributionPopulated = false, want true")
+	}
+	if status.ClosedByLogin != "bob" {
+		t.Errorf("ClosedByLogin = %q, want %q (the reopen cycle's later closer, not alice's earlier close)",
+			status.ClosedByLogin, "bob")
+	}
+}
+
 func TestRunBatchedPRQuery_DecodesAliasesBackToRefs(t *testing.T) {
 	exec := &stubGraphQLExecutor{
 		response: `{

apps/backend/internal/github/metrics_vars.go

@@ -0,0 +1,40 @@
+package github
+
+import (
+	"expvar"
+	"strings"
+)
+
+// expvar maps published at package init, exposed via stdlib's /debug/vars
+// handler in dev mode (AC-38). Process-local and dev-mode-visible only; the
+// durable, snapshot-checkable signals are the writer-health invariants
+// AC-36/AC-37, not these counters. Mirrors the label idiom in
+// internal/office/scheduler/metrics_vars.go.
+var taskPROutcomeSyncsTotal = expvar.NewMap("github_task_pr_outcome_syncs_total")
+
+// outcomeMetricLabel builds a "k1=v1;k2=v2;..." label string for an expvar
+// map key, matching the idiom in internal/office/scheduler/metrics_vars.go.
+func outcomeMetricLabel(pairs ...string) string {
+	if len(pairs)%2 != 0 {
+		return ""
+	}
+	parts := make([]string, 0, len(pairs)/2)
+	for i := 0; i < len(pairs); i += 2 {
+		parts = append(parts, pairs[i]+"="+pairs[i+1])
+	}
+	return strings.Join(parts, ";")
+}
+
+// incTaskPROutcomeSync records whether a sync populated the outcome-field
+// group (AC-38). populated=false is the "the writer stopped" canary's raw
+// material; AC-36/AC-37 remain the durable signal.
+func incTaskPROutcomeSync(populated bool) {
+	taskPROutcomeSyncsTotal.Add(outcomeMetricLabel("populated", boolLabel(populated)), 1)
+}
+
+func boolLabel(b bool) string {
+	if b {
+		return "true"
+	}
+	return "false"
+}

apps/backend/internal/github/metrics_vars_test.go

@@ -0,0 +1,92 @@
+package github
+
+import (
+	"expvar"
+	"strconv"
+	"strings"
+	"testing"
+)
+
+// readOutcomeCounter walks the expvar map looking for a key that matches the
+// supplied prefix. Returns 0 when no key matches. The prefix match keeps the
+// assertion robust against process-wide test pollution (other tests in the
+// package may push entries with the same label under -count=1 reruns).
+func readOutcomeCounter(t *testing.T, m *expvar.Map, prefix string) int64 {
+	t.Helper()
+	var total int64
+	m.Do(func(kv expvar.KeyValue) {
+		if !strings.HasPrefix(kv.Key, prefix) {
+			return
+		}
+		n, err := strconv.ParseInt(kv.Value.String(), 10, 64)
+		if err != nil {
+			t.Fatalf("counter %q value not int: %s", kv.Key, kv.Value.String())
+		}
+		total += n
+	})
+	return total
+}
+
+func TestOutcomeMetricLabel(t *testing.T) {
+	cases := []struct {
+		name  string
+		pairs []string
+		want  string
+	}{
+		{"single_pair", []string{"populated", "true"}, "populated=true"},
+		{"two_pairs", []string{"action", "set", "outcome", "unknown"}, "action=set;outcome=unknown"},
+		{"odd_args_returns_empty", []string{"populated"}, ""},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := outcomeMetricLabel(tc.pairs...); got != tc.want {
+				t.Errorf("outcomeMetricLabel(%v) = %q, want %q", tc.pairs, got, tc.want)
+			}
+		})
+	}
+}
+
+func TestBoolLabel(t *testing.T) {
+	if got := boolLabel(true); got != "true" {
+		t.Errorf("boolLabel(true) = %q, want true", got)
+	}
+	if got := boolLabel(false); got != "false" {
+		t.Errorf("boolLabel(false) = %q, want false", got)
+	}
+}
+
+// TestIncTaskPROutcomeSyncRecordsOnExpvarMap covers AC-38: a sync's populated
+// state is recorded on the outcome-syncs expvar map under the label
+// incTaskPROutcomeSync builds, so the "did the writer stop populating
+// outcome fields" canary has raw material to read.
+func TestIncTaskPROutcomeSyncRecordsOnExpvarMap(t *testing.T) {
+	labelTrue := outcomeMetricLabel("populated", boolLabel(true))
+	beforeTrue := readOutcomeCounter(t, taskPROutcomeSyncsTotal, labelTrue)
+	incTaskPROutcomeSync(true)
+	afterTrue := readOutcomeCounter(t, taskPROutcomeSyncsTotal, labelTrue)
+	if afterTrue-beforeTrue != 1 {
+		t.Errorf("populated=true counter delta = %d, want 1", afterTrue-beforeTrue)
+	}
+
+	labelFalse := outcomeMetricLabel("populated", boolLabel(false))
+	beforeFalse := readOutcomeCounter(t, taskPROutcomeSyncsTotal, labelFalse)
+	incTaskPROutcomeSync(false)
+	afterFalse := readOutcomeCounter(t, taskPROutcomeSyncsTotal, labelFalse)
+	if afterFalse-beforeFalse != 1 {
+		t.Errorf("populated=false counter delta = %d, want 1", afterFalse-beforeFalse)
+	}
+}
+
+// TestOutcomeExpvarMapsPublishedAtKnownNames guards the /debug/vars names
+// AC-38 promises: a rename here silently breaks any external dashboard
+// scraping the names directly.
+func TestOutcomeExpvarMapsPublishedAtKnownNames(t *testing.T) {
+	expected := []string{
+		"github_task_pr_outcome_syncs_total",
+	}
+	for _, name := range expected {
+		if expvar.Get(name) == nil {
+			t.Errorf("expvar %q not published — /debug/vars consumers will miss it", name)
+		}
+	}
+}

apps/backend/internal/github/mock_controller.go

@@ -644,10 +644,15 @@ func (c *MockController) associateTaskPR(ctx *gin.Context) {
 	}
 	now := time.Now().UTC()
 	tp := buildTaskPRFromRequest(&req, now)
-	if err := c.store.ReplaceTaskPR(ctx.Request.Context(), tp); err != nil {
+	// This endpoint never simulates a populating GitHub fetch, so it passes
+	// a zero-value *PRStatus: ReplaceTaskPR resolves the five outcome
+	// columns as "not observed" rather than trusting tp's own fields.
+	replaced, err := c.store.ReplaceTaskPR(ctx.Request.Context(), tp, &PRStatus{})
+	if err != nil {
 		ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 		return
 	}
+	tp = replaced
 	c.ensureMockPRForRequest(ctx.Request.Context(), &req, now)
 	// Publish the event so the frontend Zustand store picks up the new PR
 	// without requiring a page reload — mirrors real AssociatePRWithTask.

apps/backend/internal/github/models.go

@@ -280,6 +280,29 @@ type PR struct {
 	UpdatedAt           time.Time           `json:"updated_at"`
 	MergedAt            *time.Time          `json:"merged_at,omitempty"`
 	ClosedAt            *time.Time          `json:"closed_at,omitempty"`
+	// ChangedFiles is the number of files touched by the PR. 0 is a real
+	// observation, distinct from "never observed" — see ChangedFilesObserved,
+	// which is what actually carries that distinction (AC-12a).
+	ChangedFiles int `json:"changed_files,omitempty"`
+	// MergedByLogin is "" when the PR was never merged, or upstream reported
+	// no merger. Callers that persist this must write NULL for "", never "".
+	MergedByLogin string `json:"merged_by_login,omitempty"`
+	// AutoMergeEnabled reports whether upstream observed auto-merge armed at
+	// fetch time. GitHub clears auto_merge once it fires, so this can only
+	// ever mean "armed at this instant" — never "merged by auto-merge".
+	AutoMergeEnabled bool `json:"auto_merge_enabled,omitempty"`
+	// IsDraftObserved reports whether Draft was decoded from an explicit,
+	// non-null upstream isDraft/draft value, as opposed to json.Unmarshal
+	// defaulting an absent or null field to false. Internal bookkeeping only
+	// (json:"-"): AC-12a requires resolveTaskPROutcomeFields to write NULL
+	// for is_draft when this is false, rather than persisting a fabricated
+	// "false".
+	IsDraftObserved bool `json:"-"`
+	// ChangedFilesObserved mirrors IsDraftObserved for ChangedFiles. It is
+	// what actually distinguishes a genuine "0 files changed" observation
+	// from "changedFiles was absent or null" (AC-12a) — ChangedFiles alone
+	// cannot, since both cases decode to the Go zero value.
+	ChangedFilesObserved bool `json:"-"`
 }
 
 // RequestedReviewer represents a pending reviewer request on a PR.
@@ -367,6 +390,19 @@ type PRStatus struct {
 	// the REST value back to 0 (the popover's "Approved (1)" turning
 	// into "Approved (0)" until a new REST call landed).
 	ReviewCountsPopulated bool `json:"review_counts_populated,omitempty"`
+	// OutcomeFieldsPopulated mirrors ChecksPopulated for is_draft,
+	// changed_files, and merged_by_login: set only by sync paths that fetched
+	// a full single pull request or a batched GraphQL result, so a zero/empty
+	// value is a real observation rather than "I didn't look."
+	OutcomeFieldsPopulated bool `json:"outcome_fields_populated,omitempty"`
+	// ClosedByLogin is the closed-event actor's login, populated only by the
+	// GraphQL path (closed_by is absent from the REST pulls endpoint and the
+	// gh CLI's PR field set).
+	ClosedByLogin string `json:"closed_by_login,omitempty"`
+	// ClosureAttributionPopulated is set only when a GraphQL sync observed a
+	// closed-event actor with a non-empty login. REST and gh CLI single-PR
+	// syncs can never set this — they have no closing-actor field to read.
+	ClosureAttributionPopulated bool `json:"closure_attribution_populated,omitempty"`
 }
 
 // PRSearchPage is a paginated slice of PR search results, with the total
@@ -444,6 +480,58 @@ type TaskPR struct {
 	LastSyncedAt            *time.Time `json:"last_synced_at,omitempty" db:"last_synced_at"`
 	DetachedAt              *time.Time `json:"-" db:"detached_at"`
 	UpdatedAt               time.Time  `json:"updated_at" db:"updated_at"`
+
+	// --- PR outcome attribution (five nullable columns, never backfilled) ---
+	//
+	// Every field below is NULL on any row that predates this feature's
+	// activation instant (kandev_meta key taskPROutcomeActivatedAtMetaKey)
+	// and stays NULL until a post-activation observation writes it. None has a
+	// non-NULL default, and none is ever
+	// inferred, backfilled, or defaulted to a zero value. No `omitempty` on
+	// any of these json tags: AC-30 requires the keys to always be present,
+	// because `null` vs. absent is exactly the distinction this feature
+	// exists to preserve.
+	//
+	// Writer-health invariants (auditable, not a dashboard):
+	//   - AC-36: for any row where merged_at >= activation, MergedByLogin
+	//     must be non-NULL. merged_at is only ever written by the sync
+	//     writer, so a row that merged after activation was necessarily
+	//     observed by a post-activation writer; a NULL there is a writer
+	//     fault, not a data gap.
+	//   - AC-37: for any row where last_synced_at >= activation, IsDraft
+	//     must be non-NULL whenever the row's most recent sync was a
+	//     populating one. IsDraft is supplied by every populating sync path,
+	//     so it is the primary canary for "the writer stopped."
+	//   - AC-39: neither invariant applies to rows whose merged_at /
+	//     closed_at predates the activation instant — those rows are
+	//     legitimately and permanently NULL.
+
+	// IsDraft is never observed by a populating sync when NULL.
+	IsDraft *bool `json:"is_draft" db:"is_draft"`
+	// ChangedFiles is never observed when NULL, distinct from 0 (a real
+	// "no files changed" observation).
+	ChangedFiles *int `json:"changed_files" db:"changed_files"`
+	// MergedByLogin is NULL when the PR was never merged, or merged but
+	// never observed by a populating sync.
+	MergedByLogin *string `json:"merged_by_login" db:"merged_by_login"`
+	// ClosedByLogin is NULL when the PR was never closed, or closure was
+	// never observed by the GraphQL path specifically. GitHub's closed_by
+	// is absent from the REST pulls endpoint and from the gh CLI's PR field
+	// set (only the issues endpoint carries it), so this column is sourced
+	// from a GraphQL closed-event actor selection only. A PR whose only
+	// post-closure sync came through REST or gh CLI keeps this NULL
+	// permanently, because terminal rows are excluded from the orphan sweep
+	// (service_pr_unwatched.go). This gap is accepted (AC-15) and must be
+	// stated wherever this column is consumed.
+	ClosedByLogin *string `json:"closed_by_login" db:"closed_by_login"`
+	// AutoMergeObservedAt is a latched observation, never a merge cause.
+	// GitHub clears `auto_merge` once it fires, so a poller can only ever
+	// learn "auto-merge was armed at some instant while we were looking."
+	// It is set once (the first time armed auto-merge is observed) and is
+	// never cleared or overwritten afterwards, including when a later sync
+	// observes auto-merge disarmed or absent. It must not be read, named,
+	// or charted as "merged by auto-merge."
+	AutoMergeObservedAt *time.Time `json:"auto_merge_observed_at" db:"auto_merge_observed_at"`
 }
 
 // TaskCIOptions stores task-level PR automation preferences.

apps/backend/internal/github/models_test.go

@@ -0,0 +1,42 @@
+package github
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+// TestTaskPR_JSONMarshalsExplicitNullsForOutcomeFields is the AC-30
+// regression: the frontend distinguishes "not observed" (explicit JSON
+// null) from "observed absent" only if these five fields are always present
+// in the payload. Adding `omitempty` to any of them would make an
+// unobserved field disappear from the JSON entirely instead of serializing
+// as null, and every other TaskPR test would stay green because none of
+// them inspect the raw JSON — only this test would catch it.
+func TestTaskPR_JSONMarshalsExplicitNullsForOutcomeFields(t *testing.T) {
+	tp := &TaskPR{
+		TaskID: "task-1", Owner: "owner", Repo: "repo", PRNumber: 1,
+	}
+
+	raw, err := json.Marshal(tp)
+	if err != nil {
+		t.Fatalf("json.Marshal: %v", err)
+	}
+
+	var decoded map[string]json.RawMessage
+	if err := json.Unmarshal(raw, &decoded); err != nil {
+		t.Fatalf("json.Unmarshal: %v", err)
+	}
+
+	for _, key := range []string{
+		"is_draft", "changed_files", "merged_by_login", "closed_by_login", "auto_merge_observed_at",
+	} {
+		value, present := decoded[key]
+		if !present {
+			t.Errorf("key %q missing from JSON payload entirely — want present with value null (an omitempty tag would do this)", key)
+			continue
+		}
+		if string(value) != "null" {
+			t.Errorf("key %q = %s, want null", key, value)
+		}
+	}
+}

apps/backend/internal/github/noop_client_test.go

@@ -48,4 +48,14 @@ func TestNoopClient_ReturnsErrNoClient(t *testing.T) {
 	if !errors.Is(err, ErrNoClient) {
 		t.Errorf("RequestReviewers error = %v, want ErrNoClient", err)
 	}
+
+	// AC-11: the noop path never returns a PRStatus at all, so it can never
+	// mark the outcome-field or closure-attribution groups populated.
+	status, err := c.GetPRStatus(ctx, "o", "r", 1)
+	if !errors.Is(err, ErrNoClient) {
+		t.Errorf("GetPRStatus error = %v, want ErrNoClient", err)
+	}
+	if status != nil {
+		t.Errorf("GetPRStatus status = %#v, want nil", status)
+	}
 }

apps/backend/internal/github/pat_client.go

@@ -1044,20 +1044,34 @@ func (c *PATClient) FetchBranchProtection(ctx context.Context, owner, repo, bran
 
 // patPR is the JSON shape from the GitHub REST API for PRs.
 type patPR struct {
-	Number             int       `json:"number"`
-	Title              string    `json:"title"`
-	HTMLURL            string    `json:"html_url"`
-	Body               string    `json:"body"`
-	State              string    `json:"state"`
-	Draft              bool      `json:"draft"`
-	Mergeable          *bool     `json:"mergeable"`
-	MergeableState     string    `json:"mergeable_state"`
-	Additions          int       `json:"additions"`
-	Deletions          int       `json:"deletions"`
-	CreatedAt          time.Time `json:"created_at"`
-	UpdatedAt          time.Time `json:"updated_at"`
-	MergedAt           *string   `json:"merged_at"`
-	ClosedAt           *string   `json:"closed_at"`
+	Number  int    `json:"number"`
+	Title   string `json:"title"`
+	HTMLURL string `json:"html_url"`
+	Body    string `json:"body"`
+	State   string `json:"state"`
+	// Draft is a pointer: AC-12a requires distinguishing an omitted or null
+	// draft from a genuine false, which a plain bool can't after decode.
+	Draft          *bool  `json:"draft"`
+	Mergeable      *bool  `json:"mergeable"`
+	MergeableState string `json:"mergeable_state"`
+	Additions      int    `json:"additions"`
+	Deletions      int    `json:"deletions"`
+	// ChangedFiles is a pointer for the same reason as Draft (AC-12a): 0 is
+	// a legitimate observation and must stay distinguishable from absent/null.
+	ChangedFiles *int      `json:"changed_files"`
+	CreatedAt    time.Time `json:"created_at"`
+	UpdatedAt    time.Time `json:"updated_at"`
+	MergedAt     *string   `json:"merged_at"`
+	ClosedAt     *string   `json:"closed_at"`
+	// MergedBy is absent from the REST pulls response for an unmerged PR;
+	// its zero-value Login must never be written as a placeholder.
+	MergedBy *struct {
+		Login string `json:"login"`
+	} `json:"merged_by"`
+	// AutoMerge is present as a non-null object only while auto-merge is
+	// armed; GitHub clears it to null once it fires, so this can only ever
+	// mean "armed at fetch time" — never "merged by auto-merge".
+	AutoMerge          *struct{} `json:"auto_merge"`
 	RequestedReviewers []struct {
 		Login string `json:"login"`
 	} `json:"requested_reviewers"`
@@ -1162,27 +1176,43 @@ func convertPatPR(raw *patPR, owner, repo string) *PR {
 	if raw.Mergeable != nil {
 		mergeable = *raw.Mergeable
 	}
+	mergedByLogin := ""
+	if raw.MergedBy != nil {
+		mergedByLogin = raw.MergedBy.Login
+	}
+	draft, changedFiles := false, 0
+	if raw.Draft != nil {
+		draft = *raw.Draft
+	}
+	if raw.ChangedFiles != nil {
+		changedFiles = *raw.ChangedFiles
+	}
 	pr := &PR{
-		Number:              raw.Number,
-		Title:               raw.Title,
-		HTMLURL:             raw.HTMLURL,
-		Body:                raw.Body,
-		State:               state,
-		HeadBranch:          raw.Head.Ref,
-		HeadSHA:             raw.Head.SHA,
-		BaseBranch:          raw.Base.Ref,
-		AuthorLogin:         raw.User.Login,
-		RepoOwner:           owner,
-		RepoName:            repo,
-		MaintainerCanModify: raw.MaintainerCanModify,
-		Draft:               raw.Draft,
-		Mergeable:           mergeable,
-		MergeableState:      strings.ToLower(raw.MergeableState),
-		Additions:           raw.Additions,
-		Deletions:           raw.Deletions,
-		RequestedReviewers:  convertPatRequestedReviewers(raw),
-		CreatedAt:           raw.CreatedAt,
-		UpdatedAt:           raw.UpdatedAt,
+		Number:               raw.Number,
+		Title:                raw.Title,
+		HTMLURL:              raw.HTMLURL,
+		Body:                 raw.Body,
+		State:                state,
+		HeadBranch:           raw.Head.Ref,
+		HeadSHA:              raw.Head.SHA,
+		BaseBranch:           raw.Base.Ref,
+		AuthorLogin:          raw.User.Login,
+		RepoOwner:            owner,
+		RepoName:             repo,
+		MaintainerCanModify:  raw.MaintainerCanModify,
+		Draft:                draft,
+		IsDraftObserved:      raw.Draft != nil,
+		Mergeable:            mergeable,
+		MergeableState:       strings.ToLower(raw.MergeableState),
+		Additions:            raw.Additions,
+		Deletions:            raw.Deletions,
+		ChangedFiles:         changedFiles,
+		ChangedFilesObserved: raw.ChangedFiles != nil,
+		MergedByLogin:        mergedByLogin,
+		AutoMergeEnabled:     raw.AutoMerge != nil,
+		RequestedReviewers:   convertPatRequestedReviewers(raw),
+		CreatedAt:            raw.CreatedAt,
+		UpdatedAt:            raw.UpdatedAt,
 	}
 	applyPatHeadRepository(pr, raw.HeadRepository)
 	applyPatBaseRepository(pr, raw.BaseRepository)

apps/backend/internal/github/pat_client_reads_test.go

@@ -211,6 +211,61 @@ func assertPRRepositories(t *testing.T, pr *PR) {
 	}
 }
 
+// TestPATClient_GetPR_DecodesOutcomeFields covers AC-09's REST half:
+// changed_files, merged_by, and a non-null auto_merge decode onto PR.
+func TestPATClient_GetPR_DecodesOutcomeFields(t *testing.T) {
+	c, _ := newRecordingPATServer(t, map[string]string{
+		"/repos/kdlbs/kandev/pulls/2554": `{
+			"number":2554,"state":"closed","merged_at":"2026-08-12T00:00:00Z",
+			"changed_files":8,"merged_by":{"login":"carlosflorencio"},
+			"auto_merge":{"enabled_by":{"login":"carlosflorencio"},"merge_method":"squash"},
+			"user":{"login":"carlosflorencio"},"head":{"ref":"f"},"base":{"ref":"main"}
+		}`,
+	})
+
+	pr, err := c.GetPR(context.Background(), "kdlbs", "kandev", 2554)
+	if err != nil {
+		t.Fatalf("GetPR: %v", err)
+	}
+	if pr.ChangedFiles != 8 {
+		t.Errorf("ChangedFiles = %d, want 8", pr.ChangedFiles)
+	}
+	if pr.MergedByLogin != "carlosflorencio" {
+		t.Errorf("MergedByLogin = %q, want carlosflorencio", pr.MergedByLogin)
+	}
+	if !pr.AutoMergeEnabled {
+		t.Error("AutoMergeEnabled = false, want true (non-null auto_merge)")
+	}
+}
+
+// TestPATClient_GetPR_NullMergedByAndAutoMergeDecodeToZeroValues covers the
+// "never observed / not armed" side: GitHub reports auto_merge: null once it
+// fires (merged PR 2554 in the spec's live sample) or when never armed, and
+// merged_by: null on an unmerged PR.
+func TestPATClient_GetPR_NullMergedByAndAutoMergeDecodeToZeroValues(t *testing.T) {
+	c, _ := newRecordingPATServer(t, map[string]string{
+		"/repos/kdlbs/kandev/pulls/2476": `{
+			"number":2476,"state":"closed","draft":true,"changed_files":114,
+			"merged_by":null,"auto_merge":null,
+			"user":{"login":"nova28"},"head":{"ref":"f"},"base":{"ref":"main"}
+		}`,
+	})
+
+	pr, err := c.GetPR(context.Background(), "kdlbs", "kandev", 2476)
+	if err != nil {
+		t.Fatalf("GetPR: %v", err)
+	}
+	if pr.MergedByLogin != "" {
+		t.Errorf("MergedByLogin = %q, want empty", pr.MergedByLogin)
+	}
+	if pr.AutoMergeEnabled {
+		t.Error("AutoMergeEnabled = true, want false (null auto_merge)")
+	}
+	if pr.ChangedFiles != 114 {
+		t.Errorf("ChangedFiles = %d, want 114", pr.ChangedFiles)
+	}
+}
+
 func TestPATClient_GetPR_MergedStateAndTimestamps(t *testing.T) {
 	c, _ := newRecordingPATServer(t, map[string]string{
 		"/repos/acme/widget/pulls/9": `{"number":9,"state":"closed",

apps/backend/internal/github/pat_client_test.go

@@ -18,7 +18,7 @@ func TestConvertPatPR(t *testing.T) {
 		Title:     "Feature Y",
 		HTMLURL:   "https://github.com/org/repo/pull/10",
 		State:     "open",
-		Draft:     false,
+		Draft:     boolPtr(false),
 		Additions: 200,
 		Deletions: 30,
 		CreatedAt: time.Date(2025, 3, 1, 0, 0, 0, 0, time.UTC),
@@ -66,6 +66,9 @@ func TestConvertPatPR(t *testing.T) {
 	if pr.Mergeable {
 		t.Error("expected mergeable = false when nil")
 	}
+	if !pr.IsDraftObserved {
+		t.Error("expected IsDraftObserved = true when draft is present in the response")
+	}
 	if len(pr.RequestedReviewers) != 2 {
 		t.Fatalf("requested reviewers = %d, want 2", len(pr.RequestedReviewers))
 	}
@@ -80,6 +83,42 @@ func TestConvertPatPR(t *testing.T) {
 	}
 }
 
+// TestConvertPatPR_MissingDraftAndChangedFilesLeavesUnobserved covers
+// AC-12a on the REST decode path: a response that omits draft and
+// changed_files must decode to unobserved (Observed=false), not a
+// fabricated false/0 masquerading as a real observation.
+func TestConvertPatPR_MissingDraftAndChangedFilesLeavesUnobserved(t *testing.T) {
+	raw := &patPR{Number: 11, State: "open"}
+	pr := convertPatPR(raw, "org", "repo")
+	if pr.IsDraftObserved {
+		t.Error("IsDraftObserved = true, want false when draft is absent from the response")
+	}
+	if pr.ChangedFilesObserved {
+		t.Error("ChangedFilesObserved = true, want false when changed_files is absent from the response")
+	}
+}
+
+// TestConvertPatPR_ExplicitFalseAndZeroAreObserved covers AC-12a's other
+// half: a response that genuinely reports draft=false and changed_files=0
+// must be distinguishable from one that omits them — both mark
+// Observed=true.
+func TestConvertPatPR_ExplicitFalseAndZeroAreObserved(t *testing.T) {
+	raw := &patPR{Number: 11, State: "open", Draft: boolPtr(false), ChangedFiles: intPtr(0)}
+	pr := convertPatPR(raw, "org", "repo")
+	if !pr.IsDraftObserved {
+		t.Error("IsDraftObserved = false, want true for an explicit draft=false")
+	}
+	if pr.Draft {
+		t.Error("Draft = true, want false")
+	}
+	if !pr.ChangedFilesObserved {
+		t.Error("ChangedFilesObserved = false, want true for an explicit changed_files=0")
+	}
+	if pr.ChangedFiles != 0 {
+		t.Errorf("ChangedFiles = %d, want 0", pr.ChangedFiles)
+	}
+}
+
 func TestConvertPatPR_Merged(t *testing.T) {
 	mergedAt := "2025-03-05T10:00:00Z"
 	raw := &patPR{

apps/backend/internal/github/service_associate_existing_test.go

@@ -3,6 +3,7 @@ package github
 import (
 	"context"
 	"testing"
+	"time"
 )
 
 // TestAssociateExistingPRByURL_CreatesTwoRowsForDifferentTasks regression-tests
@@ -55,3 +56,72 @@ func TestAssociateExistingPRByURL_RejectsBadURL(t *testing.T) {
 		t.Fatal("expected error for malformed PR URL")
 	}
 }
+
+// TestAssociateExistingPRByURL_LinkingAlreadyMergedPRPopulatesOutcomeFields
+// is the regression for AC-36 on the "+Task" URL link path: this flow
+// fetches via GetPR, a full single-PR fetch (AC-10), so a PR that is already
+// merged at the moment of linking must not create a row with merged_at set
+// and merged_by_login left NULL — that combination is exactly the
+// writer-fault state AC-36 forbids.
+func TestAssociateExistingPRByURL_LinkingAlreadyMergedPRPopulatesOutcomeFields(t *testing.T) {
+	_, svc, mockClient, _ := setupPollerTest(t)
+	ctx := context.Background()
+
+	mergedAt := time.Now().UTC()
+	mockClient.AddPR(&PR{
+		Number: 43, Title: "Already merged", State: prStateMerged,
+		HeadSHA: "abc", HeadBranch: "feat/y", RepoOwner: "org", RepoName: "repo",
+		HTMLURL:  "https://github.com/org/repo/pull/43",
+		MergedAt: &mergedAt, Draft: false, IsDraftObserved: true,
+		ChangedFiles: 7, ChangedFilesObserved: true,
+		MergedByLogin: "carlosflorencio",
+	})
+
+	tp, err := svc.AssociateExistingPRByURL(ctx, "task-C", "repo-1", "https://github.com/org/repo/pull/43")
+	if err != nil {
+		t.Fatalf("associate: %v", err)
+	}
+	if tp == nil || tp.MergedAt == nil {
+		t.Fatalf("unexpected TaskPR: %+v", tp)
+	}
+	if tp.MergedByLogin == nil || *tp.MergedByLogin != "carlosflorencio" {
+		t.Errorf("merged_by_login = %v, want %q (AC-36 requires non-NULL once merged_at is set)",
+			tp.MergedByLogin, "carlosflorencio")
+	}
+	if tp.IsDraft == nil || *tp.IsDraft != false {
+		t.Errorf("is_draft = %v, want false observed, not NULL", tp.IsDraft)
+	}
+	if tp.ChangedFiles == nil || *tp.ChangedFiles != 7 {
+		t.Errorf("changed_files = %v, want 7 observed, not NULL", tp.ChangedFiles)
+	}
+}
+
+// TestAssociateExistingPRByURL_LinkingAutoMergeArmedPRSetsLatch is the
+// regression for COR-001 / codex[P1]: associatePRWithTask's fresh-insert
+// branch only captured resolveTaskPROutcomeFields' first three return
+// values (is_draft, changed_files, merged_by_login) via `_, _` for the rest,
+// discarding closed_by_login and auto_merge_observed_at. Linking a PR that
+// already has auto-merge armed at the moment of the "+Task" URL link (a
+// populating fetch, AC-10) must set the auto_merge_observed_at latch
+// immediately (AC-16) rather than leaving it NULL until some later sync
+// happens to observe the same state again.
+func TestAssociateExistingPRByURL_LinkingAutoMergeArmedPRSetsLatch(t *testing.T) {
+	_, svc, mockClient, _ := setupPollerTest(t)
+	ctx := context.Background()
+
+	mockClient.AddPR(&PR{
+		Number: 44, Title: "Auto-merge armed", State: "open",
+		HeadSHA: "abc", HeadBranch: "feat/z", RepoOwner: "org", RepoName: "repo",
+		HTMLURL: "https://github.com/org/repo/pull/44",
+		Draft:   false, ChangedFiles: 3, AutoMergeEnabled: true,
+	})
+
+	tp, err := svc.AssociateExistingPRByURL(ctx, "task-D", "repo-1", "https://github.com/org/repo/pull/44")
+	if err != nil {
+		t.Fatalf("associate: %v", err)
+	}
+	if tp == nil || tp.AutoMergeObservedAt == nil {
+		t.Fatalf("AutoMergeObservedAt = %v, want set on a fresh insert observing auto-merge armed (AC-16)",
+			tp.AutoMergeObservedAt)
+	}
+}

apps/backend/internal/github/service_pr.go

@@ -632,3 +632,25 @@ func intPtrEqual(a, b *int) bool {
 	}
 	return *a == *b
 }
+
+// boolPtrEqual compares two nullable bool pointers for equality.
+func boolPtrEqual(a, b *bool) bool {
+	if a == nil && b == nil {
+		return true
+	}
+	if a == nil || b == nil {
+		return false
+	}
+	return *a == *b
+}
+
+// stringPtrEqual compares two nullable string pointers for equality.
+func stringPtrEqual(a, b *string) bool {
+	if a == nil && b == nil {
+		return true
+	}
+	if a == nil || b == nil {
+		return false
+	}
+	return *a == *b
+}

apps/backend/internal/github/service_pr_outcome_sync_test.go

@@ -0,0 +1,417 @@
+package github
+
+import (
+	"context"
+	"testing"
+	"time"
+)
+
+func createOutcomeSyncTestTaskPR(t *testing.T, store *Store, tp *TaskPR) {
+	t.Helper()
+	if err := store.CreateTaskPR(context.Background(), tp); err != nil {
+		t.Fatalf("create task PR: %v", err)
+	}
+}
+
+// TestSyncTaskPR_PopulatedSyncWritesOutcomeFields covers AC-12: a populating
+// sync writes is_draft, changed_files, and merged_by_login from the observed
+// values.
+func TestSyncTaskPR_PopulatedSyncWritesOutcomeFields(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",
+	})
+
+	status := &PRStatus{
+		PR: &PR{
+			Number: 1, State: "merged", RepoOwner: "owner", RepoName: "repo",
+			Draft: false, IsDraftObserved: true,
+			ChangedFiles: 8, ChangedFilesObserved: true,
+			MergedByLogin: "carlosflorencio",
+		},
+		OutcomeFieldsPopulated: true,
+	}
+	if err := svc.SyncTaskPR(ctx, "t1", status); err != nil {
+		t.Fatalf("SyncTaskPR: %v", err)
+	}
+
+	got, err := store.GetTaskPR(ctx, "t1")
+	if err != nil {
+		t.Fatalf("GetTaskPR: %v", err)
+	}
+	if got.IsDraft == nil || *got.IsDraft != false {
+		t.Errorf("IsDraft = %v, want false", got.IsDraft)
+	}
+	if got.ChangedFiles == nil || *got.ChangedFiles != 8 {
+		t.Errorf("ChangedFiles = %v, want 8", got.ChangedFiles)
+	}
+	if got.MergedByLogin == nil || *got.MergedByLogin != "carlosflorencio" {
+		t.Errorf("MergedByLogin = %v, want carlosflorencio", got.MergedByLogin)
+	}
+}
+
+// TestSyncTaskPR_NoMergerWritesNilNotEmptyString covers the nil-vs-empty
+// rule: an upstream merged_by=null (empty MergedByLogin on PR) must persist
+// as NULL, never "".
+func TestSyncTaskPR_NoMergerWritesNilNotEmptyString(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",
+	})
+
+	status := &PRStatus{
+		PR: &PR{
+			Number: 1, State: "open", RepoOwner: "owner", RepoName: "repo",
+			Draft: true, IsDraftObserved: true,
+			ChangedFiles: 0, ChangedFilesObserved: true,
+			MergedByLogin: "",
+		},
+		OutcomeFieldsPopulated: true,
+	}
+	if err := svc.SyncTaskPR(ctx, "t1", status); err != nil {
+		t.Fatalf("SyncTaskPR: %v", err)
+	}
+
+	got, err := store.GetTaskPR(ctx, "t1")
+	if err != nil {
+		t.Fatalf("GetTaskPR: %v", err)
+	}
+	if got.MergedByLogin != nil {
+		t.Errorf("MergedByLogin = %v, want nil (NULL), not empty string", *got.MergedByLogin)
+	}
+	if got.ChangedFiles == nil || *got.ChangedFiles != 0 {
+		t.Errorf("ChangedFiles = %v, want 0 (real observation, not NULL)", got.ChangedFiles)
+	}
+}
+
+// TestSyncTaskPR_UnpopulatedSyncPreservesStoredOutcomeValues covers AC-13: a
+// non-populating sync leaves is_draft/changed_files/merged_by_login at their
+// stored values, including a stored NULL and a previously-written non-NULL.
+func TestSyncTaskPR_UnpopulatedSyncPreservesStoredOutcomeValues(t *testing.T) {
+	svc, store, _ := setupSyncTest(t)
+	ctx := context.Background()
+	isDraft := true
+	changedFiles := 3
+	mergedBy := "alice"
+	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",
+		IsDraft: &isDraft, ChangedFiles: &changedFiles, MergedByLogin: &mergedBy,
+	})
+
+	// Unpopulated sync (e.g. the batched GraphQL branch-search path or list
+	// results) must not touch the stored values, even though status.PR
+	// carries different (zero) values.
+	status := &PRStatus{
+		PR: &PR{
+			Number: 1, State: "open", RepoOwner: "owner", RepoName: "repo",
+			Draft: false, ChangedFiles: 0, MergedByLogin: "",
+		},
+		OutcomeFieldsPopulated: false,
+	}
+	if err := svc.SyncTaskPR(ctx, "t1", status); err != nil {
+		t.Fatalf("SyncTaskPR: %v", err)
+	}
+
+	got, err := store.GetTaskPR(ctx, "t1")
+	if err != nil {
+		t.Fatalf("GetTaskPR: %v", err)
+	}
+	if got.IsDraft == nil || *got.IsDraft != true {
+		t.Errorf("IsDraft = %v, want preserved true", got.IsDraft)
+	}
+	if got.ChangedFiles == nil || *got.ChangedFiles != 3 {
+		t.Errorf("ChangedFiles = %v, want preserved 3", got.ChangedFiles)
+	}
+	if got.MergedByLogin == nil || *got.MergedByLogin != "alice" {
+		t.Errorf("MergedByLogin = %v, want preserved alice", got.MergedByLogin)
+	}
+}
+
+// TestSyncTaskPR_ClosedByLoginFollowsClosureAttributionPopulated covers
+// AC-14/AC-15: closed_by_login is written only when closure attribution was
+// populated (the GraphQL path), and preserved otherwise.
+func TestSyncTaskPR_ClosedByLoginFollowsClosureAttributionPopulated(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",
+	})
+
+	populated := &PRStatus{
+		PR:                          &PR{Number: 1, State: "closed", RepoOwner: "owner", RepoName: "repo"},
+		ClosureAttributionPopulated: true,
+		ClosedByLogin:               "nova28",
+	}
+	if err := svc.SyncTaskPR(ctx, "t1", populated); err != nil {
+		t.Fatalf("SyncTaskPR (populated): %v", err)
+	}
+	got, err := store.GetTaskPR(ctx, "t1")
+	if err != nil {
+		t.Fatalf("GetTaskPR: %v", err)
+	}
+	if got.ClosedByLogin == nil || *got.ClosedByLogin != "nova28" {
+		t.Fatalf("ClosedByLogin = %v, want nova28", got.ClosedByLogin)
+	}
+
+	// A later REST/gh CLI sync (unpopulated attribution) must not clear it.
+	unpopulated := &PRStatus{
+		PR:                          &PR{Number: 1, State: "closed", RepoOwner: "owner", RepoName: "repo"},
+		ClosureAttributionPopulated: false,
+	}
+	if err := svc.SyncTaskPR(ctx, "t1", unpopulated); err != nil {
+		t.Fatalf("SyncTaskPR (unpopulated): %v", err)
+	}
+	got2, err := store.GetTaskPR(ctx, "t1")
+	if err != nil {
+		t.Fatalf("GetTaskPR after unpopulated sync: %v", err)
+	}
+	if got2.ClosedByLogin == nil || *got2.ClosedByLogin != "nova28" {
+		t.Fatalf("ClosedByLogin after unpopulated sync = %v, want preserved nova28", got2.ClosedByLogin)
+	}
+}
+
+// TestSyncTaskPR_AutoMergeObservedAtLatchesOnceAndNeverClears covers
+// AC-16/AC-17: the latch sets once on the first armed observation, is not
+// overwritten by a second armed observation, and survives a later sync that
+// observes auto-merge disarmed or absent.
+func TestSyncTaskPR_AutoMergeObservedAtLatchesOnceAndNeverClears(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",
+	})
+
+	armed := &PRStatus{
+		PR:                     &PR{Number: 1, State: "open", RepoOwner: "owner", RepoName: "repo", AutoMergeEnabled: true},
+		OutcomeFieldsPopulated: true,
+	}
+	if err := svc.SyncTaskPR(ctx, "t1", armed); err != nil {
+		t.Fatalf("SyncTaskPR (armed): %v", err)
+	}
+	first, err := store.GetTaskPR(ctx, "t1")
+	if err != nil {
+		t.Fatalf("GetTaskPR: %v", err)
+	}
+	if first.AutoMergeObservedAt == nil {
+		t.Fatal("AutoMergeObservedAt = nil, want set after first armed observation")
+	}
+	firstObservedAt := *first.AutoMergeObservedAt
+
+	// A later sync observing auto-merge disarmed must not clear the latch.
+	disarmed := &PRStatus{
+		PR:                     &PR{Number: 1, State: "open", RepoOwner: "owner", RepoName: "repo", AutoMergeEnabled: false},
+		OutcomeFieldsPopulated: true,
+	}
+	if err := svc.SyncTaskPR(ctx, "t1", disarmed); err != nil {
+		t.Fatalf("SyncTaskPR (disarmed): %v", err)
+	}
+	second, err := store.GetTaskPR(ctx, "t1")
+	if err != nil {
+		t.Fatalf("GetTaskPR after disarmed sync: %v", err)
+	}
+	if second.AutoMergeObservedAt == nil || !second.AutoMergeObservedAt.Equal(firstObservedAt) {
+		t.Fatalf("AutoMergeObservedAt after disarmed sync = %v, want unchanged %v", second.AutoMergeObservedAt, firstObservedAt)
+	}
+
+	// A later sync observing auto-merge armed again must not overwrite the
+	// latch with a new timestamp.
+	time.Sleep(2 * time.Millisecond)
+	armedAgain := &PRStatus{
+		PR:                     &PR{Number: 1, State: "open", RepoOwner: "owner", RepoName: "repo", AutoMergeEnabled: true},
+		OutcomeFieldsPopulated: true,
+	}
+	if err := svc.SyncTaskPR(ctx, "t1", armedAgain); err != nil {
+		t.Fatalf("SyncTaskPR (armed again): %v", err)
+	}
+	third, err := store.GetTaskPR(ctx, "t1")
+	if err != nil {
+		t.Fatalf("GetTaskPR after second armed sync: %v", err)
+	}
+	if third.AutoMergeObservedAt == nil || !third.AutoMergeObservedAt.Equal(firstObservedAt) {
+		t.Fatalf("AutoMergeObservedAt after second armed sync = %v, want still %v", third.AutoMergeObservedAt, firstObservedAt)
+	}
+}
+
+// TestUpdateTaskPR_AutoMergeLatchIsAtomicUnderConcurrentStaleWrites is the
+// deterministic regression for the race SyncTaskPR's own read-then-write
+// shape can't rule out: two syncs can both read AutoMergeObservedAt as NULL
+// (neither has observed the other's write yet) and independently compute
+// their own "now" timestamp before either writes. This drives
+// store.UpdateTaskPR directly with two such stale in-memory TaskPR copies —
+// SyncTaskPR always re-reads fresh at the top of each call, so it cannot
+// itself reproduce two readers racing off the same NULL snapshot — and
+// asserts the SQL-level COALESCE(auto_merge_observed_at, ?) means whichever
+// write actually lands first wins, with the second writer's differing
+// timestamp silently discarded rather than overwriting it.
+func TestUpdateTaskPR_AutoMergeLatchIsAtomicUnderConcurrentStaleWrites(t *testing.T) {
+	store := newTestStore(t)
+	ctx := context.Background()
+	base := &TaskPR{
+		TaskID: "task-latch-race", Owner: "owner", Repo: "repo", PRNumber: 1,
+		PRURL: "https://github.com/owner/repo/pull/1", PRTitle: "Initial",
+		HeadBranch: "feat", BaseBranch: "main", State: "open", CreatedAt: time.Now().UTC(),
+	}
+	if err := store.CreateTaskPR(ctx, base); err != nil {
+		t.Fatalf("create task PR: %v", err)
+	}
+
+	// Two independent in-memory copies of the same row, both taken while
+	// AutoMergeObservedAt was still NULL — simulating two syncs that raced
+	// off the same stale snapshot and each computed their own timestamp.
+	writerA := *base
+	firstObservedAt := time.Now().UTC()
+	writerA.AutoMergeObservedAt = &firstObservedAt
+
+	writerB := *base
+	secondObservedAt := firstObservedAt.Add(time.Minute)
+	writerB.AutoMergeObservedAt = &secondObservedAt
+
+	if err := store.UpdateTaskPR(ctx, &writerA); err != nil {
+		t.Fatalf("UpdateTaskPR (writer A, lands first): %v", err)
+	}
+	if err := store.UpdateTaskPR(ctx, &writerB); err != nil {
+		t.Fatalf("UpdateTaskPR (writer B, lands second, stale nil-based timestamp): %v", err)
+	}
+
+	got, err := store.GetTaskPRByID(ctx, base.ID)
+	if err != nil {
+		t.Fatalf("GetTaskPRByID: %v", err)
+	}
+	if got.AutoMergeObservedAt == nil || !got.AutoMergeObservedAt.Equal(firstObservedAt) {
+		t.Fatalf("AutoMergeObservedAt = %v, want the first writer's timestamp %v preserved, not the second writer's %v",
+			got.AutoMergeObservedAt, firstObservedAt, secondObservedAt)
+	}
+}
+
+// TestPersistAndPublishTaskPRSync_PublishesReReadValueNotStaleInMemoryOne is
+// the deterministic regression for codex [P2]. SyncTaskPR computes
+// tp.AutoMergeObservedAt from its OWN top-of-call read, but UpdateTaskPR
+// persists that field through COALESCE(auto_merge_observed_at, ?), so a
+// concurrent sync landing its own write in the gap between this call's read
+// and its write can leave the column holding a different, earlier timestamp
+// than tp carries in memory. Reproducing the exact interleaving would need
+// real goroutines racing a synchronous function with no injectable pause
+// point, which is nondeterministic; this instead drives the extracted
+// persistAndPublishTaskPRSync helper directly with a manufactured "stale"
+// tp (as if this call's read happened before a concurrent write it never
+// observed) after seeding the DB with a different, already-persisted value
+// (the concurrent write). The bug: publishing tp unmodified would broadcast
+// the stale value, not what a fresh read returns.
+func TestPersistAndPublishTaskPRSync_PublishesReReadValueNotStaleInMemoryOne(t *testing.T) {
+	svc, store, eb := setupSyncTest(t)
+	ctx := context.Background()
+
+	tp := &TaskPR{
+		TaskID: "task-p2-race", Owner: "owner", Repo: "repo", PRNumber: 1,
+		PRURL: "https://github.com/owner/repo/pull/1", PRTitle: "p2 race",
+		HeadBranch: "feat", BaseBranch: "main", State: "open", CreatedAt: time.Now().UTC(),
+	}
+	if err := store.CreateTaskPR(ctx, tp); err != nil {
+		t.Fatalf("create task PR: %v", err)
+	}
+
+	// Simulate a concurrent sync that already landed its own write: the DB
+	// now holds persistedAt, set while our own in-memory tp (below) still
+	// reflects the pre-race NULL it read moments earlier.
+	concurrentWriter := *tp
+	persistedAt := time.Now().UTC()
+	concurrentWriter.AutoMergeObservedAt = &persistedAt
+	if err := store.UpdateTaskPR(ctx, &concurrentWriter); err != nil {
+		t.Fatalf("simulate concurrent writer: %v", err)
+	}
+
+	// Our own call's stale view: computed from a read taken before the
+	// concurrent writer's timestamp above landed.
+	staleTP := *tp
+	staleObservedAt := persistedAt.Add(-time.Minute)
+	staleTP.AutoMergeObservedAt = &staleObservedAt
+
+	if err := svc.persistAndPublishTaskPRSync(ctx, &staleTP, true, true); err != nil {
+		t.Fatalf("persistAndPublishTaskPRSync: %v", err)
+	}
+
+	got, err := store.GetTaskPRByID(ctx, tp.ID)
+	if err != nil {
+		t.Fatalf("GetTaskPRByID: %v", err)
+	}
+	if got.AutoMergeObservedAt == nil || !got.AutoMergeObservedAt.Equal(persistedAt) {
+		t.Fatalf("persisted AutoMergeObservedAt = %v, want the concurrent writer's %v preserved by COALESCE",
+			got.AutoMergeObservedAt, persistedAt)
+	}
+
+	if eb.publishedCount() != 1 {
+		t.Fatalf("published events = %d, want 1", eb.publishedCount())
+	}
+	published, ok := eb.events[0].Data.(*TaskPR)
+	if !ok {
+		t.Fatalf("published event data = %T, want *TaskPR", eb.events[0].Data)
+	}
+	if published.AutoMergeObservedAt == nil || !published.AutoMergeObservedAt.Equal(persistedAt) {
+		t.Fatalf("published AutoMergeObservedAt = %v, want the re-read persisted value %v, not the stale in-memory %v",
+			published.AutoMergeObservedAt, persistedAt, staleObservedAt)
+	}
+}
+
+// TestSyncTaskPR_OutcomeFieldChangePublishesEvent covers AC-18: a sync that
+// changes only an outcome field (no legacy field change) still publishes
+// github.task_pr.updated, and an unchanged sync publishes nothing.
+//
+// The seed and status deliberately match on every non-outcome comparison
+// SyncTaskPR's `changed` expression checks, including mergeable_state: a
+// draft PR forces prepareTaskPRSyncState to override mergeableState to
+// "draft" regardless of status.MergeableState, so an earlier version of this
+// test (Draft: true, tp.MergeableState left at its zero value "") had TWO
+// independent causes of changed == true — the mergeableState override and
+// the outcome-field change — and would have stayed green even if
+// taskPROutcomeFieldsChanged were deleted from that expression entirely.
+// Using a non-draft PR with a pre-matched, non-empty MergeableState removes
+// that second cause, so changedFiles is the only thing that can make this
+// sync report changed.
+func TestSyncTaskPR_OutcomeFieldChangePublishesEvent(t *testing.T) {
+	svc, store, eb := setupSyncTest(t)
+	ctx := context.Background()
+	seededIsDraft := false
+	createOutcomeSyncTestTaskPR(t, store, &TaskPR{
+		TaskID: "t1", Owner: "owner", Repo: "repo", PRNumber: 1,
+		PRURL: "https://github.com/owner/repo/pull/1", PRTitle: "Same title",
+		HeadBranch: "feat", BaseBranch: "main", State: "open",
+		MergeableState: "clean", IsDraft: &seededIsDraft,
+	})
+
+	status := &PRStatus{
+		PR: &PR{
+			Number: 1, Title: "Same title", State: "open", RepoOwner: "owner", RepoName: "repo",
+			Draft: false, IsDraftObserved: true,
+			ChangedFiles: 5, ChangedFilesObserved: true,
+			MergedByLogin: "",
+		},
+		MergeableState:         "clean",
+		OutcomeFieldsPopulated: true,
+	}
+	if err := svc.SyncTaskPR(ctx, "t1", status); err != nil {
+		t.Fatalf("SyncTaskPR (change): %v", err)
+	}
+	if eb.publishedCount() != 1 {
+		t.Fatalf("published events = %d, want 1 after an outcome-only change", eb.publishedCount())
+	}
+
+	// Re-syncing identical values must not publish again.
+	if err := svc.SyncTaskPR(ctx, "t1", status); err != nil {
+		t.Fatalf("SyncTaskPR (no change): %v", err)
+	}
+	if eb.publishedCount() != 1 {
+		t.Fatalf("published events = %d, want still 1 after an unchanged sync", eb.publishedCount())
+	}
+}

apps/backend/internal/github/service_pr_unwatched.go

@@ -7,8 +7,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/kandev/kandev/internal/events"
-	"github.com/kandev/kandev/internal/events/bus"
 	"go.uber.org/zap"
 )
 
@@ -100,60 +98,79 @@ func (s *Service) syncUnwatchedTaskPRGroup(ctx context.Context, workspaceID stri
 	}
 	live := s.fetchUnwatchedTaskPRs(ctx, resolved, group)
 	for _, tp := range group {
-		pr := live[prStatusCacheKey(tp.Owner, tp.Repo, tp.PRNumber)]
-		if pr == nil {
+		status := live[prStatusCacheKey(tp.Owner, tp.Repo, tp.PRNumber)]
+		if status == nil || status.PR == nil {
 			continue
 		}
-		if syncErr := s.reconcileTaskPRLifecycle(ctx, tp, pr); syncErr != nil {
+		if syncErr := s.reconcileTaskPRLifecycle(ctx, tp, status); syncErr != nil {
 			s.logger.Debug("unwatched task PR reconcile failed",
 				zap.String("task_id", tp.TaskID), zap.Int("pr_number", tp.PRNumber), zap.Error(syncErr))
 		}
 	}
 }
 
-// reconcileTaskPRLifecycle writes only the fields that describe where the PR
-// sits in its lifecycle, then publishes the same update event SyncTaskPR does
-// so every client converges.
+// reconcileTaskPRLifecycle writes the fields that describe where the PR sits
+// in its lifecycle, plus the five outcome-attribution fields (AC-36/AC-37
+// require merged_by_login / is_draft to be non-NULL on a row this path can
+// take terminal, so they cannot be left for a populating sync that may never
+// come — a PR nobody watches gets no other writer). It reuses
+// resolveTaskPROutcomeFields, the same populated/preserve logic SyncTaskPR
+// uses, on the *PRStatus fetchUnwatchedTaskPRs already builds from data this
+// path was fetching anyway. It then publishes the same update event
+// SyncTaskPR does so every client converges.
 //
-// Everything else on the row is left alone on purpose. The aggregates
-// (checks_state, checks_total/passing, review_state, review counts, unresolved
-// threads, mergeable_state) are owned by the watch-driven sync, which fetches
-// the reviews and check runs needed to compute them. Writing them from here
-// would mean either re-fetching all of that for a PR nobody is working on, or
-// — worse — persisting a less-informed answer over a better one: the per-PR
-// REST status sets ChecksPopulated/ReviewCountsPopulated with zeroed counts
-// when it has nothing to count, which SyncTaskPR then faithfully stores.
-func (s *Service) reconcileTaskPRLifecycle(ctx context.Context, tp *TaskPR, pr *PR) error {
+// Aggregates (checks_state, checks_total/passing, review_state, review
+// counts, unresolved threads, mergeable_state) stay untouched on purpose:
+// they are owned by the watch-driven sync, which fetches the reviews and
+// check runs needed to compute them. Writing them from here would mean
+// either re-fetching all of that for a PR nobody is working on, or — worse —
+// persisting a less-informed answer over a better one: the per-PR REST
+// status sets ChecksPopulated/ReviewCountsPopulated with zeroed counts when
+// it has nothing to count, which SyncTaskPR then faithfully stores.
+func (s *Service) reconcileTaskPRLifecycle(ctx context.Context, tp *TaskPR, status *PRStatus) error {
+	pr := status.PR
+	isDraft, changedFiles, mergedByLogin, closedByLogin, autoMergeObservedAt :=
+		resolveTaskPROutcomeFields(tp, status)
+
 	changed := tp.State != pr.State ||
 		!timeEqual(tp.MergedAt, pr.MergedAt) ||
-		!timeEqual(tp.ClosedAt, pr.ClosedAt)
+		!timeEqual(tp.ClosedAt, pr.ClosedAt) ||
+		!boolPtrEqual(tp.IsDraft, isDraft) ||
+		!intPtrEqual(tp.ChangedFiles, changedFiles) ||
+		!stringPtrEqual(tp.MergedByLogin, mergedByLogin) ||
+		!stringPtrEqual(tp.ClosedByLogin, closedByLogin) ||
+		!timeEqual(tp.AutoMergeObservedAt, autoMergeObservedAt)
 
 	tp.State = pr.State
 	tp.MergedAt = pr.MergedAt
 	tp.ClosedAt = pr.ClosedAt
+	tp.IsDraft = isDraft
+	tp.ChangedFiles = changedFiles
+	tp.MergedByLogin = mergedByLogin
+	tp.ClosedByLogin = closedByLogin
+	tp.AutoMergeObservedAt = autoMergeObservedAt
 	now := time.Now().UTC()
 	tp.LastSyncedAt = &now
 
-	if err := s.store.UpdateTaskPR(ctx, tp); err != nil {
-		return fmt.Errorf("update task PR: %w", err)
-	}
-	if changed && s.eventBus != nil {
-		event := bus.NewEvent(events.GitHubTaskPRUpdated, "github", tp)
-		if err := s.eventBus.Publish(ctx, events.GitHubTaskPRUpdated, event); err != nil {
-			s.logger.Debug("failed to publish task PR updated event", zap.Error(err))
-		}
-	}
-	return nil
+	// AC-18: publish must reflect the row as stored, not this call's
+	// in-memory tp — UpdateTaskPR latches AutoMergeObservedAt through
+	// COALESCE, so a concurrent writer can persist an earlier timestamp than
+	// 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)
 }
 
 // fetchUnwatchedTaskPRs prefers the batched GraphQL query — one call for the
 // whole group — and falls back to per-PR reads when the client can't speak
 // GraphQL (NoopClient) or the batch itself fails. Same batched-then-per-item
-// shape as runBatchedOrPerWatchSync. Returns the live PRs keyed by
-// prStatusCacheKey.
+// shape as runBatchedOrPerWatchSync. Returns the live statuses keyed by
+// prStatusCacheKey, carrying the outcome-field population flags each path
+// already computed rather than the bare PR (AC-08/AC-10/AC-14 apply here
+// exactly as they do to the watch-driven batched query).
 func (s *Service) fetchUnwatchedTaskPRs(
 	ctx context.Context, resolved *resolvedServiceClient, group []*TaskPR,
-) map[string]*PR {
+) map[string]*PRStatus {
 	refs := make([]graphQLPRRef, 0, len(group))
 	for _, tp := range group {
 		// The repo is in the 10-min negative cache; probing it would only
@@ -169,7 +186,7 @@ func (s *Service) fetchUnwatchedTaskPRs(
 	if exec, execErr := graphQLExecutorFor(resolved.Client); execErr == nil {
 		out, err := s.batchedUnwatchedFetch(ctx, exec, resolved.CacheScope, refs)
 		if err == nil {
-			return prsFromStatuses(out)
+			return out
 		}
 		s.logger.Debug("batched unwatched task PR query failed; falling back per PR", zap.Error(err))
 	}
@@ -222,23 +239,16 @@ func batchedRefsKey(refs []graphQLPRRef) string {
 	return strings.Join(parts, "|")
 }
 
-func prsFromStatuses(statuses map[string]*PRStatus) map[string]*PR {
-	out := make(map[string]*PR, len(statuses))
-	for key, status := range statuses {
-		if status != nil && status.PR != nil {
-			out[key] = status.PR
-		}
-	}
-	return out
-}
-
 // fetchUnwatchedTaskPRsPerPR reads one PR at a time. GetPR is deliberate: the
 // full status helper also lists reviews and check runs (three calls per PR) to
-// compute aggregates this path does not write.
+// compute aggregates this path does not write. GetPR is still a full
+// single-pull-request fetch, so is_draft/changed_files/merged_by_login are
+// real observations here too (AC-10); ClosureAttributionPopulated stays
+// false since neither REST nor the gh CLI can see the closing actor (AC-15).
 func (s *Service) fetchUnwatchedTaskPRsPerPR(
 	ctx context.Context, resolved *resolvedServiceClient, refs []graphQLPRRef,
-) map[string]*PR {
-	out := make(map[string]*PR, len(refs))
+) map[string]*PRStatus {
+	out := make(map[string]*PRStatus, len(refs))
 	for _, ref := range refs {
 		pr, err := resolved.Client.GetPR(ctx, ref.Owner, ref.Repo, ref.Number)
 		if err != nil {
@@ -248,7 +258,10 @@ func (s *Service) fetchUnwatchedTaskPRsPerPR(
 			continue
 		}
 		if pr != nil {
-			out[prStatusCacheKey(ref.Owner, ref.Repo, ref.Number)] = pr
+			out[prStatusCacheKey(ref.Owner, ref.Repo, ref.Number)] = &PRStatus{
+				PR:                     pr,
+				OutcomeFieldsPopulated: true,
+			}
 		}
 	}
 	return out

apps/backend/internal/github/service_pr_unwatched_test.go

@@ -412,6 +412,54 @@ func TestTriggerPRSyncAll_UnwatchedReconcilePreservesAggregates(t *testing.T) {
 	}
 }
 
+// TestTriggerPRSyncAll_UnwatchedReconcileWritesOutcomeAttribution is the
+// regression for AC-36: a PR that merges with no active watch is reconciled
+// only through syncUnwatchedTaskPRGroup, never through SyncTaskPR. Before
+// this fix, reconcileTaskPRLifecycle discarded the already-fetched draft
+// status and merger identity, so the row went terminal with
+// merged_by_login/is_draft still NULL — merged_at >= activation with a NULL
+// merged_by_login is exactly the writer-fault state AC-36 forbids.
+func TestTriggerPRSyncAll_UnwatchedReconcileWritesOutcomeAttribution(t *testing.T) {
+	_, svc, mockClient, store := setupPollerTest(t)
+	ctx := context.Background()
+	seedTask(t, store, "task-1", false)
+	seedUnwatchedTaskPR(t, store, "task-1", 1293, "feature/first")
+
+	mergedAt := time.Now().UTC().Add(-10 * time.Minute)
+	mockClient.AddPR(&PR{
+		Number: 1293, Title: "Left behind", State: prStateMerged,
+		HTMLURL:    "https://github.com/owner/repo/pull/1293",
+		HeadBranch: "feature/first", BaseBranch: "main",
+		RepoOwner: "owner", RepoName: "repo",
+		CreatedAt: mergedAt.Add(-2 * time.Hour), UpdatedAt: mergedAt, MergedAt: &mergedAt,
+		Draft: false, IsDraftObserved: true,
+		ChangedFiles: 4, ChangedFilesObserved: true,
+		MergedByLogin: "carlosflorencio",
+	})
+
+	if _, err := svc.TriggerPRSyncAll(ctx, "task-1"); err != nil {
+		t.Fatalf("TriggerPRSyncAll: %v", err)
+	}
+
+	got, err := store.GetTaskPRByRepoAndNumber(ctx, "task-1", "repo-1", 1293)
+	if err != nil || got == nil {
+		t.Fatalf("GetTaskPRByRepoAndNumber: err=%v row=%v", err, got)
+	}
+	if got.State != prStateMerged || got.MergedAt == nil {
+		t.Fatalf("state=%q merged_at=%v, want merged with a timestamp", got.State, got.MergedAt)
+	}
+	if got.MergedByLogin == nil || *got.MergedByLogin != "carlosflorencio" {
+		t.Errorf("merged_by_login = %v, want %q (AC-36 requires non-NULL once merged_at is set)",
+			got.MergedByLogin, "carlosflorencio")
+	}
+	if got.IsDraft == nil || *got.IsDraft != false {
+		t.Errorf("is_draft = %v, want false observed, not NULL", got.IsDraft)
+	}
+	if got.ChangedFiles == nil || *got.ChangedFiles != 4 {
+		t.Errorf("changed_files = %v, want 4 observed, not NULL", got.ChangedFiles)
+	}
+}
+
 func TestUnwatchedTaskPRs_Selection(t *testing.T) {
 	now := time.Now().UTC()
 	stale := now.Add(-1 * time.Hour)
@@ -450,3 +498,70 @@ func TestUnwatchedTaskPRs_Selection(t *testing.T) {
 		t.Errorf("selected PRs = %d, %d; want 1, 6", got[0].PRNumber, got[1].PRNumber)
 	}
 }
+
+// TestReconcileTaskPRLifecycle_PublishesReReadValueNotStaleInMemoryOne is the
+// AC-18 regression on the unwatched-sync path, mirroring
+// TestPersistAndPublishTaskPRSync_PublishesReReadValueNotStaleInMemoryOne in
+// service_pr_outcome_sync_test.go for the codex [P2] race on the other
+// writer. reconcileTaskPRLifecycle used to publish its own in-memory tp
+// directly after UpdateTaskPR; because UpdateTaskPR persists
+// AutoMergeObservedAt through COALESCE(auto_merge_observed_at, ?), a
+// concurrent writer that lands first leaves the stored column holding an
+// earlier timestamp than this call's own resolved value, and the old code
+// would broadcast the wrong one. It now delegates the write+publish to
+// persistAndPublishTaskPRSync, which re-reads before publishing.
+func TestReconcileTaskPRLifecycle_PublishesReReadValueNotStaleInMemoryOne(t *testing.T) {
+	_, svc, _, store := setupPollerTest(t)
+	ctx := context.Background()
+	seedUnwatchedTaskPR(t, store, "task-unwatched-p2-race", 1293, "feature/first")
+
+	tp, err := store.GetTaskPRByRepoAndNumber(ctx, "task-unwatched-p2-race", "repo-1", 1293)
+	if err != nil || tp == nil {
+		t.Fatalf("GetTaskPRByRepoAndNumber: err=%v row=%v", err, tp)
+	}
+
+	// Simulate a concurrent sync that already landed its own write: the DB
+	// now holds persistedAt, set while this call's own tp (below) still
+	// reflects the pre-race value it read moments earlier.
+	concurrentWriter := *tp
+	persistedAt := time.Now().UTC()
+	concurrentWriter.AutoMergeObservedAt = &persistedAt
+	if err := store.UpdateTaskPR(ctx, &concurrentWriter); err != nil {
+		t.Fatalf("simulate concurrent writer: %v", err)
+	}
+
+	// This call's stale in-memory view, computed before the concurrent
+	// writer's timestamp above landed.
+	staleObservedAt := persistedAt.Add(-time.Minute)
+	tp.AutoMergeObservedAt = &staleObservedAt
+
+	updated := subscribeTaskPRUpdated(t, svc)
+
+	mergedAt := time.Now().UTC()
+	status := &PRStatus{
+		PR: &PR{
+			Number: 1293, State: prStateMerged, MergedAt: &mergedAt,
+			HeadBranch: "feature/first", BaseBranch: "main",
+			RepoOwner: "owner", RepoName: "repo",
+		},
+	}
+
+	if err := svc.reconcileTaskPRLifecycle(ctx, tp, status); err != nil {
+		t.Fatalf("reconcileTaskPRLifecycle: %v", err)
+	}
+
+	published := awaitTaskPRUpdated(t, updated)
+	if published.AutoMergeObservedAt == nil || !published.AutoMergeObservedAt.Equal(persistedAt) {
+		t.Fatalf("published AutoMergeObservedAt = %v, want the re-read persisted value %v, not the stale in-memory %v",
+			published.AutoMergeObservedAt, persistedAt, staleObservedAt)
+	}
+
+	got, err := store.GetTaskPRByID(ctx, tp.ID)
+	if err != nil {
+		t.Fatalf("GetTaskPRByID: %v", err)
+	}
+	if got.AutoMergeObservedAt == nil || !got.AutoMergeObservedAt.Equal(persistedAt) {
+		t.Fatalf("persisted AutoMergeObservedAt = %v, want the concurrent writer's %v preserved by COALESCE",
+			got.AutoMergeObservedAt, persistedAt)
+	}
+}

apps/backend/internal/github/service_pr_watch.go

@@ -299,7 +299,10 @@ func (s *Service) ensurePRWatch(
 // callers MUST pass it — empty causes ReplaceTaskPR to wipe the entire task's
 // PR rows (legacy "delete all" branch), which is what older code relied on.
 func (s *Service) AssociatePRWithTask(ctx context.Context, taskID, repositoryID string, pr *PR) (*TaskPR, error) {
-	return s.associatePRWithTask(ctx, "", taskID, repositoryID, pr, false)
+	// pr here comes from branch-search discovery (poller.detectPRForWatch) or
+	// a batched status result of unknown populated-ness — never assert
+	// outcomeFieldsPopulated for this wrapper's callers (AC-11).
+	return s.associatePRWithTask(ctx, "", taskID, repositoryID, pr, false, false)
 }
 
 func (s *Service) AssociatePRWithTaskForWorkspace(
@@ -308,11 +311,17 @@ func (s *Service) AssociatePRWithTaskForWorkspace(
 	if strings.TrimSpace(workspaceID) == "" {
 		return nil, ErrGitHubWorkspaceRequired
 	}
-	return s.associatePRWithTask(ctx, workspaceID, taskID, repositoryID, pr, false)
+	return s.associatePRWithTask(ctx, workspaceID, taskID, repositoryID, pr, false, false)
 }
 
+// associatePRWithTask creates the task-PR association. outcomeFieldsPopulated
+// tells it whether pr came from a full single-PR fetch (AC-10) — only the
+// direct GetPR-based callers in this file set it true; the two public
+// wrappers above always pass false, preserving their existing callers'
+// behavior exactly, because a branch-search or batched-status pr may not
+// carry real observations for these fields (AC-11).
 func (s *Service) associatePRWithTask(
-	ctx context.Context, workspaceID, taskID, repositoryID string, pr *PR, restoreDetached bool,
+	ctx context.Context, workspaceID, taskID, repositoryID string, pr *PR, restoreDetached, outcomeFieldsPopulated bool,
 ) (*TaskPR, error) {
 	// Multi-branch: scope the "already-current" short-circuit by exact
 	// pr_number too. A task can hold multiple PR rows per (task, repo) on
@@ -327,7 +336,15 @@ func (s *Service) associatePRWithTask(
 	}
 	if existing != nil {
 		if existing.DetachedAt != nil && restoreDetached {
-			restored, restoreErr := s.store.RestoreTaskPR(ctx, taskID, repositoryID, pr)
+			// RestoreTaskPR resolves the five outcome-attribution columns
+			// itself, inside its own transaction, against the row it is
+			// about to overwrite (AC-43, AC-43a) — this caller must not
+			// pre-resolve them via resolveTaskPROutcomeFields, since a
+			// value resolved here would be stale by the time the store's
+			// statement executes.
+			restored, restoreErr := s.store.RestoreTaskPR(
+				ctx, taskID, repositoryID, &PRStatus{PR: pr, OutcomeFieldsPopulated: outcomeFieldsPopulated},
+			)
 			if restoreErr != nil || restored == nil {
 				return restored, restoreErr
 			}
@@ -362,16 +379,21 @@ func (s *Service) associatePRWithTask(
 		MergedAt:     pr.MergedAt,
 		ClosedAt:     pr.ClosedAt,
 	}
-	// ReplaceTaskPR upserts the row matching (task, repository, pr_number).
-	// Multi-branch tasks may already hold sibling rows for the SAME
-	// (task, repository) on different PR numbers — ReplaceTaskPR no longer
-	// touches them. The early-return above guarantees we only reach this
-	// line when no row for the exact pr_number exists yet, so this is a
-	// straight insert in steady state; the delete-then-insert form is
-	// retained so a retry that races a partial write resolves cleanly.
-	if err := s.store.ReplaceTaskPR(ctx, tp); err != nil {
+	// ReplaceTaskPR upserts the row matching (task, repository, pr_number)
+	// and resolves the five outcome-attribution columns itself, inside its
+	// own transaction, against the row it is about to overwrite (AC-43,
+	// AC-43a) — this caller must not pre-resolve them. Multi-branch tasks
+	// may already hold sibling rows for the SAME (task, repository) on
+	// different PR numbers — ReplaceTaskPR no longer touches them. The
+	// early-return above guarantees we only reach this line when no row for
+	// the exact pr_number exists yet, so this is a straight insert in
+	// steady state; the delete-then-insert form is retained so a retry that
+	// races a partial write resolves cleanly.
+	replaced, err := s.store.ReplaceTaskPR(ctx, tp, &PRStatus{PR: pr, OutcomeFieldsPopulated: outcomeFieldsPopulated})
+	if err != nil {
 		return nil, fmt.Errorf("replace task PR: %w", err)
 	}
+	tp = replaced
 
 	// Publish event for UI
 	if s.eventBus != nil {
@@ -411,7 +433,7 @@ func (s *Service) AssociateExistingPRByURL(ctx context.Context, taskID, reposito
 	if err != nil {
 		return nil, fmt.Errorf("fetch PR: %w", err)
 	}
-	tp, err := s.associatePRWithTask(ctx, "", taskID, repositoryID, pr, true)
+	tp, err := s.associatePRWithTask(ctx, "", taskID, repositoryID, pr, true, true)
 	if err != nil {
 		return nil, fmt.Errorf("associate PR with task: %w", err)
 	}
@@ -436,7 +458,7 @@ func (s *Service) AssociateExistingPRByURLForWorkspace(
 	if err != nil {
 		return nil, fmt.Errorf("fetch PR: %w", err)
 	}
-	tp, err := s.associatePRWithTask(ctx, workspaceID, taskID, repositoryID, pr, true)
+	tp, err := s.associatePRWithTask(ctx, workspaceID, taskID, repositoryID, pr, true, true)
 	if err != nil {
 		return nil, fmt.Errorf("associate PR with task: %w", err)
 	}
@@ -476,7 +498,7 @@ func (s *Service) AssociatePRByURL(ctx context.Context, sessionID, taskID, repos
 	}
 
 	// Associate PR with task (persists + publishes WS event)
-	if _, assocErr := s.associatePRWithTask(ctx, "", taskID, repositoryID, pr, true); assocErr != nil {
+	if _, assocErr := s.associatePRWithTask(ctx, "", taskID, repositoryID, pr, true, true); assocErr != nil {
 		s.logger.Error("failed to associate PR with task after creation",
 			zap.String("task_id", taskID), zap.Error(assocErr))
 	}
@@ -508,7 +530,7 @@ func (s *Service) AssociatePRByURLForWorkspace(
 	); err != nil {
 		return fmt.Errorf("create PR watch: %w", err)
 	}
-	if _, err := s.associatePRWithTask(ctx, workspaceID, taskID, repositoryID, pr, true); err != nil {
+	if _, err := s.associatePRWithTask(ctx, workspaceID, taskID, repositoryID, pr, true, true); err != nil {
 		return fmt.Errorf("associate PR with task: %w", err)
 	}
 	return nil
@@ -720,6 +742,66 @@ type taskPRSyncState struct {
 	requiredReviews                             *int
 	baseBranch, mergeableState, state           string
 	mergedAt, closedAt                          *time.Time
+	isDraft                                     *bool
+	changedFiles                                *int
+	mergedByLogin, closedByLogin                *string
+	autoMergeObservedAt                         *time.Time
+}
+
+// resolveTaskPROutcomeFields applies the populated/preserve dance for the
+// five outcome-attribution columns, split out of prepareTaskPRSyncState to
+// keep that function's complexity within the repo's lint limits (see the
+// cyclomatic-complexity suppression on SyncTaskPR below).
+//
+//   - is_draft / changed_files each follow their own presence flag
+//     (status.PR.IsDraftObserved / status.PR.ChangedFilesObserved) rather
+//     than the whole-group status.OutcomeFieldsPopulated: a group marked
+//     populated is not a promise that every field in it arrived, so a
+//     populated sync whose upstream response omitted or nulled one of these
+//     two writes NULL for that column specifically, not the decoder's zero
+//     value (AC-12a). merged_by_login stays keyed off the group flag alone
+//     (nil merged_by_login when upstream reports no merger, never "").
+//     An unpopulated group preserves all three stored values verbatim,
+//     including NULL (AC-13).
+//   - closed_by_login follows status.ClosureAttributionPopulated the same
+//     way (AC-14, AC-15) — only the GraphQL path ever sets it.
+//   - auto_merge_observed_at is a latch: set once, on the first populating
+//     sync that observes auto-merge armed while the stored value is NULL,
+//     and never cleared or overwritten afterwards (AC-16, AC-17).
+func resolveTaskPROutcomeFields(tp *TaskPR, status *PRStatus) (
+	isDraft *bool, changedFiles *int, mergedByLogin, closedByLogin *string, autoMergeObservedAt *time.Time,
+) {
+	isDraft, changedFiles, mergedByLogin = tp.IsDraft, tp.ChangedFiles, tp.MergedByLogin
+	if status.OutcomeFieldsPopulated {
+		isDraft = nil
+		if status.PR.IsDraftObserved {
+			draft := status.PR.Draft
+			isDraft = &draft
+		}
+		changedFiles = nil
+		if status.PR.ChangedFilesObserved {
+			files := status.PR.ChangedFiles
+			changedFiles = &files
+		}
+		mergedByLogin = nil
+		if status.PR.MergedByLogin != "" {
+			login := status.PR.MergedByLogin
+			mergedByLogin = &login
+		}
+	}
+
+	closedByLogin = tp.ClosedByLogin
+	if status.ClosureAttributionPopulated {
+		login := status.ClosedByLogin
+		closedByLogin = &login
+	}
+
+	autoMergeObservedAt = tp.AutoMergeObservedAt
+	if status.OutcomeFieldsPopulated && status.PR.AutoMergeEnabled && tp.AutoMergeObservedAt == nil {
+		now := time.Now().UTC()
+		autoMergeObservedAt = &now
+	}
+	return isDraft, changedFiles, mergedByLogin, closedByLogin, autoMergeObservedAt
 }
 
 // resolveTerminalMergeState keeps a row that already reached "merged" there.
@@ -792,11 +874,16 @@ func (s *Service) prepareTaskPRSyncState(ctx context.Context, tp *TaskPR, status
 		nextMergeableState = "draft"
 	}
 	nextState, nextMergedAt, nextClosedAt := resolveTerminalMergeState(tp, status.PR)
+	nextIsDraft, nextChangedFiles, nextMergedByLogin, nextClosedByLogin, nextAutoMergeObservedAt :=
+		resolveTaskPROutcomeFields(tp, status)
 	return taskPRSyncState{
 		checksTotal: nextChecksTotal, checksPassing: nextChecksPassing,
 		unresolved: nextUnresolved, reviewCount: nextReviewCount, pendingReviewCount: nextPendingReviewCount,
 		requiredReviews: nextRequiredReviews, baseBranch: nextBaseBranch, mergeableState: nextMergeableState,
 		state: nextState, mergedAt: nextMergedAt, closedAt: nextClosedAt,
+		isDraft: nextIsDraft, changedFiles: nextChangedFiles,
+		mergedByLogin: nextMergedByLogin, closedByLogin: nextClosedByLogin,
+		autoMergeObservedAt: nextAutoMergeObservedAt,
 	}
 }
 
@@ -833,6 +920,15 @@ func taskPRChangedFields(tp *TaskPR, status *PRStatus, next taskPRSyncState) []s
 	changed = appendChangedField(changed, "base_branch", tp.BaseBranch != next.baseBranch)
 	changed = appendChangedField(changed, "merged_at", !timeEqual(tp.MergedAt, next.mergedAt))
 	changed = appendChangedField(changed, "closed_at", !timeEqual(tp.ClosedAt, next.closedAt))
+	changed = appendChangedField(changed, "is_draft", !boolPtrEqual(tp.IsDraft, next.isDraft))
+	changed = appendChangedField(changed, "changed_files", !intPtrEqual(tp.ChangedFiles, next.changedFiles))
+	changed = appendChangedField(changed, "merged_by_login", !stringPtrEqual(tp.MergedByLogin, next.mergedByLogin))
+	changed = appendChangedField(changed, "closed_by_login", !stringPtrEqual(tp.ClosedByLogin, next.closedByLogin))
+	changed = appendChangedField(
+		changed,
+		"auto_merge_observed_at",
+		!timeEqual(tp.AutoMergeObservedAt, next.autoMergeObservedAt),
+	)
 	return changed
 }
 
@@ -862,6 +958,7 @@ func (s *Service) SyncTaskPR(ctx context.Context, taskID string, status *PRStatu
 	next := s.prepareTaskPRSyncState(ctx, tp, status)
 
 	changedFields := taskPRChangedFields(tp, status, next)
+	changed := len(changedFields) > 0
 
 	tp.State = next.state
 	tp.PRTitle = status.PR.Title
@@ -879,10 +976,45 @@ func (s *Service) SyncTaskPR(ctx context.Context, taskID string, status *PRStatu
 	tp.ChecksPassing = next.checksPassing
 	tp.UnresolvedReviewThreads = next.unresolved
 	tp.BaseBranch = next.baseBranch
+	tp.IsDraft = next.isDraft
+	tp.ChangedFiles = next.changedFiles
+	tp.MergedByLogin = next.mergedByLogin
+	tp.ClosedByLogin = next.closedByLogin
+	tp.AutoMergeObservedAt = next.autoMergeObservedAt
 	// CommentCount is no longer updated from polling -- only refreshed on-demand
 	now := time.Now().UTC()
 	tp.LastSyncedAt = &now
+	if len(changedFields) > 0 && s.logger != nil {
+		s.logger.Debug("task PR semantic state changed",
+			zap.String("task_id", taskID),
+			zap.String("pr_owner", tp.Owner),
+			zap.String("pr_repo", tp.Repo),
+			zap.Int("pr_number", tp.PRNumber),
+			zap.Strings("changed_fields", changedFields),
+		)
+	}
 
+	return s.persistAndPublishTaskPRSync(ctx, tp, changed, status.OutcomeFieldsPopulated)
+}
+
+// persistAndPublishTaskPRSync writes the reconciled sync state and, on
+// change, publishes github.task_pr.updated. It re-reads the row after the
+// write rather than publishing tp directly: tp.AutoMergeObservedAt is
+// computed from THIS call's own (possibly stale-by-the-time-of-write) read
+// at the top of SyncTaskPR, but UpdateTaskPR persists it through
+// COALESCE(auto_merge_observed_at, ?), so a concurrent sync that lands its
+// own write in the gap between this call's read and its write can leave the
+// column holding a DIFFERENT, earlier timestamp than tp carries in memory.
+// Publishing tp unmodified would then broadcast a value that never matches
+// 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 {
+	// 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
+	// to succeed.
+	incTaskPROutcomeSync(outcomeFieldsPopulated)
 	if err := s.store.UpdateTaskPR(ctx, tp); err != nil {
 		return fmt.Errorf("update task PR: %w", err)
 	}
@@ -891,19 +1023,13 @@ func (s *Service) SyncTaskPR(ctx context.Context, taskID string, status *PRStatu
 	// unmatchable payload never prevents the review association from persisting.
 	s.reconcileComparisonTargetFromSync(ctx, taskID, status.PR)
 
-	if len(changedFields) > 0 {
-		if s.logger != nil {
-			s.logger.Debug("task PR semantic state changed",
-				zap.String("task_id", taskID),
-				zap.String("pr_owner", tp.Owner),
-				zap.String("pr_repo", tp.Repo),
-				zap.Int("pr_number", tp.PRNumber),
-				zap.Strings("changed_fields", changedFields),
-			)
+	if changed && s.eventBus != nil {
+		published, err := s.store.GetTaskPRByID(ctx, tp.ID)
+		if err != nil {
+			s.logger.Debug("failed to re-read task PR before publishing", zap.Error(err))
+			published = tp
 		}
-	}
-	if len(changedFields) > 0 && s.eventBus != nil {
-		event := bus.NewEvent(events.GitHubTaskPRUpdated, "github", tp)
+		event := bus.NewEvent(events.GitHubTaskPRUpdated, "github", published)
 		if err := s.eventBus.Publish(ctx, events.GitHubTaskPRUpdated, event); err != nil {
 			s.logger.Debug("failed to publish task PR updated event", zap.Error(err))
 		}

apps/backend/internal/github/service_pr_writer_health_test.go

@@ -0,0 +1,205 @@
+package github
+
+import (
+	"context"
+	"testing"
+	"time"
+)
+
+// TestWriterHealthInvariants_AC36_AC37_AC39 is the regression for the Writer
+// health verification surface: over one seeded database with a known
+// activation instant, AC-36 and AC-37 must hold for rows a populating sync
+// legitimately observed, AC-39 must exclude rows predating activation from
+// either check entirely, and the three legitimate NULL classes named in the
+// Writer health note (deleted-merger-account, merge-boundary poll-inversion,
+// and the AC-12a isDraft-omission case) must be treated as exemptions, not
+// invariant violations. A checker that asserts either invariant
+// unconditionally would fail every exemption row below against a correct
+// implementation, which is exactly what this test guards against.
+func TestWriterHealthInvariants_AC36_AC37_AC39(t *testing.T) {
+	svc, store, _ := setupSyncTest(t)
+	ctx := context.Background()
+
+	activation, err := time.Parse(time.RFC3339, requireOutcomeActivationOnce(t, store.db))
+	if err != nil {
+		t.Fatalf("parse activation instant: %v", err)
+	}
+	after := activation.Add(time.Hour)
+	before := activation.Add(-time.Hour)
+
+	seedOpenPR := func(taskID string, prNumber int) {
+		t.Helper()
+		createOutcomeSyncTestTaskPR(t, store, &TaskPR{
+			TaskID: taskID, Owner: "owner", Repo: "repo", PRNumber: prNumber,
+			PRURL: "https://github.com/owner/repo/pull/1", PRTitle: "Writer health fixture",
+			HeadBranch: "feat", BaseBranch: "main", State: "open",
+		})
+	}
+
+	// AC-36, normal case: a row merged after activation by a populating sync
+	// must carry a non-NULL merged_by_login.
+	seedOpenPR("t-merge-normal", 1)
+	mergedAtNormal := after
+	if err := svc.SyncTaskPR(ctx, "t-merge-normal", &PRStatus{
+		PR: &PR{
+			Number: 1, State: prStateMerged, RepoOwner: "owner", RepoName: "repo",
+			MergedAt: &mergedAtNormal, MergedByLogin: "carlosflorencio",
+			IsDraftObserved: true, ChangedFilesObserved: true,
+		},
+		OutcomeFieldsPopulated: true,
+	}); err != nil {
+		t.Fatalf("SyncTaskPR (normal merge): %v", err)
+	}
+	mergeNormal, err := store.GetTaskPR(ctx, "t-merge-normal")
+	if err != nil {
+		t.Fatalf("GetTaskPR (normal merge): %v", err)
+	}
+	if mergeNormal.MergedAt == nil || mergeNormal.MergedAt.Before(activation) {
+		t.Fatalf("test setup: t-merge-normal merged_at = %v, want set and >= activation %v", mergeNormal.MergedAt, activation)
+	}
+	if mergeNormal.MergedByLogin == nil || *mergeNormal.MergedByLogin != "carlosflorencio" {
+		t.Errorf("AC-36: merged_by_login = %v, want non-NULL for a row merged after activation by a populating sync", mergeNormal.MergedByLogin)
+	}
+
+	// AC-37, normal case: a row synced after activation by a populating sync
+	// must carry a non-NULL is_draft.
+	seedOpenPR("t-draft-normal", 2)
+	if err := svc.SyncTaskPR(ctx, "t-draft-normal", &PRStatus{
+		PR: &PR{
+			Number: 2, State: "open", RepoOwner: "owner", RepoName: "repo",
+			Draft: false, IsDraftObserved: true, ChangedFilesObserved: true,
+		},
+		OutcomeFieldsPopulated: true,
+	}); err != nil {
+		t.Fatalf("SyncTaskPR (normal draft sync): %v", err)
+	}
+	draftNormal, err := store.GetTaskPR(ctx, "t-draft-normal")
+	if err != nil {
+		t.Fatalf("GetTaskPR (normal draft sync): %v", err)
+	}
+	if draftNormal.LastSyncedAt == nil || draftNormal.LastSyncedAt.Before(activation) {
+		t.Fatalf("test setup: t-draft-normal last_synced_at = %v, want set and >= activation %v", draftNormal.LastSyncedAt, activation)
+	}
+	if draftNormal.IsDraft == nil {
+		t.Errorf("AC-37: is_draft = nil, want non-NULL for a row synced after activation by a populating sync")
+	}
+
+	// Exemption 1 (deleted-merger-account): merged after activation, but
+	// upstream reports no merger. merged_by_login staying NULL here is not a
+	// writer fault.
+	seedOpenPR("t-deleted-merger", 3)
+	mergedAtDeletedMerger := after
+	if err := svc.SyncTaskPR(ctx, "t-deleted-merger", &PRStatus{
+		PR: &PR{
+			Number: 3, State: prStateMerged, RepoOwner: "owner", RepoName: "repo",
+			MergedAt: &mergedAtDeletedMerger, MergedByLogin: "",
+			IsDraftObserved: true, ChangedFilesObserved: true,
+		},
+		OutcomeFieldsPopulated: true,
+	}); err != nil {
+		t.Fatalf("SyncTaskPR (deleted merger): %v", err)
+	}
+	deletedMerger, err := store.GetTaskPR(ctx, "t-deleted-merger")
+	if err != nil {
+		t.Fatalf("GetTaskPR (deleted merger): %v", err)
+	}
+	if deletedMerger.MergedAt == nil || deletedMerger.MergedAt.Before(activation) {
+		t.Fatalf("test setup: t-deleted-merger merged_at = %v, want set and >= activation %v", deletedMerger.MergedAt, activation)
+	}
+	if deletedMerger.MergedByLogin != nil {
+		t.Errorf("exemption (deleted-merger-account): merged_by_login = %q, want NULL: upstream reported no merger for a real merge, not a writer fault",
+			*deletedMerger.MergedByLogin)
+	}
+
+	// Exemption 2 (merge-boundary poll-inversion): two polls straddle the
+	// merge; the earlier-reading poll's write lands last and clobbers
+	// merged_by_login back to NULL. The row is terminal, so no later poll
+	// repairs it. Simulated directly at the store layer since reproducing
+	// the actual race nondeterministically would not make this assertion any
+	// stronger — the accepted residual is the stored state itself, not the
+	// interleaving that produced it.
+	seedOpenPR("t-poll-inversion", 4)
+	mergedAtInversion := after
+	if err := svc.SyncTaskPR(ctx, "t-poll-inversion", &PRStatus{
+		PR: &PR{
+			Number: 4, State: prStateMerged, RepoOwner: "owner", RepoName: "repo",
+			MergedAt: &mergedAtInversion, MergedByLogin: "alice",
+			IsDraftObserved: true, ChangedFilesObserved: true,
+		},
+		OutcomeFieldsPopulated: true,
+	}); err != nil {
+		t.Fatalf("SyncTaskPR (poll-inversion, first poll): %v", err)
+	}
+	straddled, err := store.GetTaskPR(ctx, "t-poll-inversion")
+	if err != nil {
+		t.Fatalf("GetTaskPR (poll-inversion, before second poll): %v", err)
+	}
+	if straddled.MergedByLogin == nil {
+		t.Fatalf("test setup: t-poll-inversion merged_by_login not set by first poll")
+	}
+	straddled.MergedByLogin = nil
+	if err := store.UpdateTaskPR(ctx, straddled); err != nil {
+		t.Fatalf("UpdateTaskPR (poll-inversion, second poll landing last): %v", err)
+	}
+	pollInversion, err := store.GetTaskPR(ctx, "t-poll-inversion")
+	if err != nil {
+		t.Fatalf("GetTaskPR (poll-inversion, after second poll): %v", err)
+	}
+	if pollInversion.MergedAt == nil || pollInversion.MergedAt.Before(activation) {
+		t.Fatalf("test setup: t-poll-inversion merged_at = %v, want set and >= activation %v", pollInversion.MergedAt, activation)
+	}
+	if pollInversion.MergedByLogin != nil {
+		t.Errorf("exemption (merge-boundary poll-inversion): merged_by_login = %q, want NULL: the earlier-straddling poll's write landed last and the row is terminal",
+			*pollInversion.MergedByLogin)
+	}
+
+	// Exemption 3 (AC-12a isDraft omission): synced after activation by a
+	// populating sync, but upstream omitted isDraft on that response.
+	// is_draft staying NULL here is not a writer fault.
+	seedOpenPR("t-missing-isdraft", 5)
+	if err := svc.SyncTaskPR(ctx, "t-missing-isdraft", &PRStatus{
+		PR: &PR{
+			Number: 5, State: "open", RepoOwner: "owner", RepoName: "repo",
+			IsDraftObserved: false, ChangedFilesObserved: true,
+		},
+		OutcomeFieldsPopulated: true,
+	}); err != nil {
+		t.Fatalf("SyncTaskPR (missing isDraft): %v", err)
+	}
+	missingIsDraft, err := store.GetTaskPR(ctx, "t-missing-isdraft")
+	if err != nil {
+		t.Fatalf("GetTaskPR (missing isDraft): %v", err)
+	}
+	if missingIsDraft.LastSyncedAt == nil || missingIsDraft.LastSyncedAt.Before(activation) {
+		t.Fatalf("test setup: t-missing-isdraft last_synced_at = %v, want set and >= activation %v", missingIsDraft.LastSyncedAt, activation)
+	}
+	if missingIsDraft.IsDraft != nil {
+		t.Errorf("exemption (AC-12a isDraft omission): is_draft = %v, want NULL: the populating response omitted isDraft, not a writer fault",
+			*missingIsDraft.IsDraft)
+	}
+
+	// AC-39: a row whose merged_at predates activation is excluded from
+	// AC-36 entirely — it is legitimately and permanently NULL, and no
+	// assertion is made on merged_by_login for it below. Seeded directly
+	// (not through SyncTaskPR, whose LastSyncedAt is always "now" and so
+	// could never itself land before activation).
+	preActivationMergedAt := before
+	if err := store.CreateTaskPR(ctx, &TaskPR{
+		TaskID: "t-pre-activation-merge", Owner: "owner", Repo: "repo", PRNumber: 6,
+		PRURL: "https://github.com/owner/repo/pull/6", PRTitle: "Merged before activation",
+		HeadBranch: "feat", BaseBranch: "main", State: prStateMerged,
+		MergedAt: &preActivationMergedAt,
+	}); err != nil {
+		t.Fatalf("CreateTaskPR (pre-activation merge): %v", err)
+	}
+	preActivation, err := store.GetTaskPR(ctx, "t-pre-activation-merge")
+	if err != nil {
+		t.Fatalf("GetTaskPR (pre-activation merge): %v", err)
+	}
+	if preActivation.MergedAt == nil || !preActivation.MergedAt.Before(activation) {
+		t.Fatalf("test setup: t-pre-activation-merge merged_at = %v, want set and before activation %v", preActivation.MergedAt, activation)
+	}
+	// Deliberately no assertion on preActivation.MergedByLogin (AC-39): this
+	// row predates activation, so AC-36 does not apply to it regardless of
+	// what merged_by_login holds.
+}

apps/backend/internal/github/service_task_pr_detach_test.go

@@ -193,7 +193,7 @@ func TestAssociatePRWithTaskExplicitLinkRestoresDetachedAssociation(t *testing.T
 		Title: "fresh title", HeadBranch: "fresh-branch", BaseBranch: "develop", AuthorLogin: "bob",
 		State: "closed", MergeableState: "dirty", Additions: 8, Deletions: 3,
 	}
-	associated, err := svc.associatePRWithTask(ctx, row.WorkspaceID, row.TaskID, row.RepositoryID, freshPR, true)
+	associated, err := svc.associatePRWithTask(ctx, row.WorkspaceID, row.TaskID, row.RepositoryID, freshPR, true, true)
 	if err != nil {
 		t.Fatalf("explicitly associate PR: %v", err)
 	}
@@ -218,3 +218,53 @@ func TestAssociatePRWithTaskExplicitLinkRestoresDetachedAssociation(t *testing.T
 		t.Fatalf("published events = %d, want one restoration update", eventBus.publishedCount())
 	}
 }
+
+// TestAssociatePRWithTaskExplicitLinkOnAlreadyMergedDetachedPRPopulatesOutcomeFields
+// is the detached -> merged -> relink regression: a PR that was detached
+// while open, then merged upstream while nobody was watching, then relinked
+// via the explicit URL flow (a GetPR-based, populating fetch) must not
+// surface with merged_at set and merged_by_login left NULL from before the
+// detach (AC-36). RestoreTaskPR previously updated only lifecycle/display
+// columns and left the five outcome fields untouched.
+func TestAssociatePRWithTaskExplicitLinkOnAlreadyMergedDetachedPRPopulatesOutcomeFields(t *testing.T) {
+	svc, store, _ := setupSyncTest(t)
+	ctx := context.Background()
+	row := &TaskPR{
+		WorkspaceID: "ws-1", TaskID: "task-1", RepositoryID: "repo-1", Owner: "acme", Repo: "demo",
+		PRNumber: 5, PRURL: "https://github.com/acme/demo/pull/5", PRTitle: "old", HeadBranch: "old",
+		BaseBranch: "main", State: "open", CreatedAt: time.Now().UTC(),
+	}
+	if err := store.CreateTaskPR(ctx, row); err != nil {
+		t.Fatalf("create PR: %v", err)
+	}
+	if _, _, err := store.DetachTaskPR(ctx, row.ID); err != nil {
+		t.Fatalf("detach PR: %v", err)
+	}
+
+	mergedAt := time.Now().UTC()
+	mergedPR := &PR{
+		Number: 5, RepoOwner: "acme", RepoName: "demo", HTMLURL: "https://github.com/acme/demo/pull/5",
+		Title: "merged while detached", HeadBranch: "old", BaseBranch: "main", AuthorLogin: "bob",
+		State: prStateMerged, MergedAt: &mergedAt,
+		Draft: false, IsDraftObserved: true,
+		ChangedFiles: 12, ChangedFilesObserved: true,
+		MergedByLogin: "carlosflorencio",
+	}
+	associated, err := svc.associatePRWithTask(ctx, row.WorkspaceID, row.TaskID, row.RepositoryID, mergedPR, true, true)
+	if err != nil {
+		t.Fatalf("explicitly associate PR: %v", err)
+	}
+	if associated == nil || associated.MergedAt == nil {
+		t.Fatalf("associated = %+v, want merged with a timestamp", associated)
+	}
+	if associated.MergedByLogin == nil || *associated.MergedByLogin != "carlosflorencio" {
+		t.Errorf("merged_by_login = %v, want %q (AC-36 requires non-NULL once merged_at is set)",
+			associated.MergedByLogin, "carlosflorencio")
+	}
+	if associated.IsDraft == nil || *associated.IsDraft != false {
+		t.Errorf("is_draft = %v, want false observed, not NULL", associated.IsDraft)
+	}
+	if associated.ChangedFiles == nil || *associated.ChangedFiles != 12 {
+		t.Errorf("changed_files = %v, want 12 observed, not NULL", associated.ChangedFiles)
+	}
+}

apps/backend/internal/github/store.go

@@ -15,6 +15,7 @@ import (
 	"github.com/jmoiron/sqlx"
 
 	dbutil "github.com/kandev/kandev/internal/db"
+	"github.com/kandev/kandev/internal/persistence"
 )
 
 // Store provides SQLite persistence for GitHub integration data.
@@ -136,6 +137,11 @@ const createTablesSQL = `
 		last_synced_at DATETIME,
 		detached_at DATETIME,
 		updated_at DATETIME NOT NULL,
+		is_draft BOOLEAN,
+		changed_files INTEGER,
+		merged_by_login TEXT,
+		closed_by_login TEXT,
+		auto_merge_observed_at DATETIME,
 		UNIQUE(task_id, repository_id, pr_number)
 	);
 
@@ -477,6 +483,9 @@ func (s *Store) initSchema(legacyUpgrade bool) error {
 	if err := s.initSchemaUpgrades(); err != nil {
 		return err
 	}
+	if err := s.activateTaskPROutcomeTracking(); err != nil {
+		return err
+	}
 	if err := s.initSchemaData(legacyUpgrade); err != nil {
 		return err
 	}
@@ -484,6 +493,33 @@ func (s *Store) initSchema(legacyUpgrade bool) error {
 	return s.ensureWorkspaceOwnershipIndexes()
 }
 
+// taskPROutcomeActivatedAtMetaKey is the kandev_meta key under which the
+// PR-outcome-attribution feature's one-time activation instant is recorded
+// (AC-05). Any point-in-time extract over the five outcome columns must
+// read this key to scope its window and distinguish "not yet activated"
+// from "writer broke" (spec: Persistence guarantees).
+const taskPROutcomeActivatedAtMetaKey = "github_task_pr_outcome_activated_at"
+
+// activateTaskPROutcomeTracking stamps the activation instant exactly once
+// per database (AC-05, AC-06). WriteMetaKeyIfAbsent's ON CONFLICT DO NOTHING
+// makes this safe to call on every boot rather than gate it behind whether
+// this boot's migration literally ran an ALTER TABLE: a fresh install
+// receives the five columns inline via createTablesSQL, not an ALTER, but
+// still needs the instant stamped on its very first boot. A write failure
+// aborts startup — a database with the columns but no activation instant is
+// unreportable and must not be shipped (spec: Failure modes).
+func (s *Store) activateTaskPROutcomeTracking() error {
+	if err := persistence.EnsureMetaTable(s.db); err != nil {
+		return fmt.Errorf("ensure kandev_meta table: %w", err)
+	}
+	if _, err := persistence.WriteMetaKeyIfAbsent(
+		s.db, taskPROutcomeActivatedAtMetaKey, time.Now().UTC().Format(time.RFC3339),
+	); err != nil {
+		return fmt.Errorf("activate task PR outcome tracking: %w", err)
+	}
+	return nil
+}
+
 func (s *Store) initSchemaFoundations() error {
 	if err := s.resetUnpublishedGitHubAuthSchema(); err != nil {
 		return err
@@ -554,6 +590,49 @@ func (s *Store) initSchemaUpgrades() error {
 	if err := s.addAppRegistrationReferenceColumns(); err != nil {
 		return err
 	}
+	if err := s.addTaskPROutcomeColumns(); err != nil {
+		return err
+	}
+	return nil
+}
+
+// taskPROutcomeColumnDDL lists the five nullable PR-outcome-attribution
+// columns and their type fragments. Every column is nullable with no
+// DEFAULT (AC-01): NULL means "never observed" and must never be confused
+// with a zero value or an empty string.
+var taskPROutcomeColumnDDL = []struct {
+	name string
+	ddl  string
+}{
+	{"is_draft", "BOOLEAN"},
+	{"changed_files", "INTEGER"},
+	{"merged_by_login", "TEXT"},
+	{"closed_by_login", "TEXT"},
+	{"auto_merge_observed_at", "DATETIME"},
+}
+
+// addTaskPROutcomeColumns adds the five PR-outcome-attribution columns to
+// github_task_prs. These columns join taskPRColumns, so every existing read
+// scans them unconditionally; unlike applyIdempotentSchemaColumns, a
+// driver-level ALTER failure here must abort startup rather than silently
+// turn into a scan error on the next read (AC-03). Only
+// dbutil.IsDuplicateColumnError is tolerated, per ADR 0027 — no local
+// error-string classifier, and no UPDATE/backfill runs against
+// github_task_prs anywhere in this path (AC-04).
+func (s *Store) addTaskPROutcomeColumns() error {
+	cols, err := s.tableColumns("github_task_prs")
+	if err != nil {
+		return fmt.Errorf("read github_task_prs columns: %w", err)
+	}
+	for _, col := range taskPROutcomeColumnDDL {
+		if _, ok := cols[col.name]; ok {
+			continue
+		}
+		stmt := "ALTER TABLE github_task_prs ADD COLUMN " + col.name + " " + col.ddl
+		if _, err := s.db.Exec(stmt); err != nil && !dbutil.IsDuplicateColumnError(err) {
+			return fmt.Errorf("add github_task_prs.%s: %w", col.name, err)
+		}
+	}
 	return nil
 }
 
@@ -1267,18 +1346,30 @@ func (s *Store) migratePRTablesForMultiRepo() error {
 			last_synced_at DATETIME,
 			detached_at DATETIME,
 			updated_at DATETIME NOT NULL,
+			is_draft BOOLEAN,
+			changed_files INTEGER,
+			merged_by_login TEXT,
+			closed_by_login TEXT,
+			auto_merge_observed_at DATETIME,
 			UNIQUE(task_id, repository_id, pr_number)
 		)`,
+		// The five outcome-attribution columns are selected directly, not
+		// via COALESCE: addTaskPROutcomeColumns runs earlier in
+		// initSchemaUpgrades, before this data-migration step, so the old
+		// table being rebuilt here already has them (fail-loud — startup
+		// would have aborted otherwise).
 		`INSERT INTO github_task_prs_new (
 			id, workspace_id, task_id, repository_id, owner, repo, pr_number, pr_url, pr_title,
 			head_branch, base_branch, author_login, state, review_state, checks_state,
 			mergeable_state, review_count, pending_review_count, comment_count,
-			additions, deletions, created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at
+			additions, deletions, created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at,
+			is_draft, changed_files, merged_by_login, closed_by_login, auto_merge_observed_at
 		) SELECT
 			id, COALESCE(workspace_id, ''), task_id, COALESCE(repository_id, ''), owner, repo, pr_number, pr_url, pr_title,
 			head_branch, base_branch, author_login, state, review_state, checks_state,
 			mergeable_state, review_count, pending_review_count, comment_count,
-			additions, deletions, created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at
+			additions, deletions, created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at,
+			is_draft, changed_files, merged_by_login, closed_by_login, auto_merge_observed_at
 		FROM github_task_prs`,
 	)
 }
@@ -1596,12 +1687,14 @@ func (s *Store) CreateTaskPR(ctx context.Context, tp *TaskPR) error {
 		INSERT INTO github_task_prs (id, workspace_id, task_id, repository_id, owner, repo, pr_number, pr_url, pr_title, head_branch, base_branch, author_login,
 			state, review_state, checks_state, mergeable_state, review_count, pending_review_count, required_reviews, comment_count,
 			unresolved_review_threads, checks_total, checks_passing, additions, deletions,
-			created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at)
-		VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
+			created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at,
+			is_draft, changed_files, merged_by_login, closed_by_login, auto_merge_observed_at)
+		VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
 		tp.ID, tp.WorkspaceID, tp.TaskID, tp.RepositoryID, tp.Owner, tp.Repo, tp.PRNumber, tp.PRURL, tp.PRTitle, tp.HeadBranch, tp.BaseBranch, tp.AuthorLogin,
 		tp.State, tp.ReviewState, tp.ChecksState, tp.MergeableState, tp.ReviewCount, tp.PendingReviewCount, tp.RequiredReviews, tp.CommentCount,
 		tp.UnresolvedReviewThreads, tp.ChecksTotal, tp.ChecksPassing, tp.Additions, tp.Deletions,
-		tp.CreatedAt, tp.MergedAt, tp.ClosedAt, tp.LastSyncedAt, tp.DetachedAt, tp.UpdatedAt)
+		tp.CreatedAt, tp.MergedAt, tp.ClosedAt, tp.LastSyncedAt, tp.DetachedAt, tp.UpdatedAt,
+		tp.IsDraft, tp.ChangedFiles, tp.MergedByLogin, tp.ClosedByLogin, tp.AutoMergeObservedAt)
 	return err
 }
 
@@ -1618,7 +1711,8 @@ const taskPRColumns = `id, workspace_id, task_id, repository_id, owner, repo, pr
 	pr_title, head_branch, base_branch, author_login, state, review_state, checks_state,
 	mergeable_state, review_count, pending_review_count, required_reviews, comment_count,
 	unresolved_review_threads, checks_total, checks_passing, additions, deletions,
-	created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at`
+	created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at,
+	is_draft, changed_files, merged_by_login, closed_by_login, auto_merge_observed_at`
 
 // taskPRColumnsQualified is taskPRColumns with each column qualified by the
 // `gtp` alias, for queries that join github_task_prs against another table.
@@ -1627,7 +1721,8 @@ const taskPRColumnsQualified = `gtp.id, gtp.workspace_id, gtp.task_id, gtp.repos
 	gtp.state, gtp.review_state, gtp.checks_state, gtp.mergeable_state, gtp.review_count,
 	gtp.pending_review_count, gtp.required_reviews, gtp.comment_count, gtp.unresolved_review_threads,
 	gtp.checks_total, gtp.checks_passing, gtp.additions, gtp.deletions,
-	gtp.created_at, gtp.merged_at, gtp.closed_at, gtp.last_synced_at, gtp.detached_at, gtp.updated_at`
+	gtp.created_at, gtp.merged_at, gtp.closed_at, gtp.last_synced_at, gtp.detached_at, gtp.updated_at,
+	gtp.is_draft, gtp.changed_files, gtp.merged_by_login, gtp.closed_by_login, gtp.auto_merge_observed_at`
 
 // GetTaskPR returns the first PR association for a task. For multi-repo tasks
 // the result is non-deterministic across repos — use ListTaskPRsByTask instead.
@@ -1757,20 +1852,54 @@ func (s *Store) DetachTaskPR(ctx context.Context, associationID string) (*TaskPR
 
 // RestoreTaskPR clears a detached tombstone for an explicit link action and
 // refreshes the persisted fields available from the fetched GitHub PR.
-func (s *Store) RestoreTaskPR(ctx context.Context, taskID, repositoryID string, pr *PR) (*TaskPR, error) {
-	if pr == nil {
+// status carries the caller's raw PR observation and its populated-ness
+// flags (AC-43p) — RestoreTaskPR resolves the five outcome-attribution
+// columns itself, inside this call's own transaction, against the outgoing
+// row it is about to overwrite (AC-43, AC-43a). The caller must not
+// pre-resolve them via resolveTaskPROutcomeFields: a value resolved against
+// an earlier read is stale by the time this statement executes, and on a
+// row that has just reached a terminal state there is no later poll to
+// repair a value clobbered that way (AC-36).
+func (s *Store) RestoreTaskPR(ctx context.Context, taskID, repositoryID string, status *PRStatus) (*TaskPR, error) {
+	if status == nil || status.PR == nil {
 		return nil, errors.New("restore task PR: missing PR data")
 	}
-	if _, err := s.db.ExecContext(ctx,
+	pr := status.PR
+
+	tx, err := s.db.BeginTxx(ctx, nil)
+	if err != nil {
+		return nil, err
+	}
+	defer func() { _ = tx.Rollback() }()
+
+	var outgoing TaskPR
+	err = tx.GetContext(ctx, &outgoing,
+		`SELECT `+taskPRColumns+` FROM github_task_prs
+		 WHERE task_id = ? AND repository_id = ? AND pr_number = ?`,
+		taskID, repositoryID, pr.Number)
+	if err != nil && !errors.Is(err, sql.ErrNoRows) {
+		return nil, err
+	}
+
+	isDraft, changedFiles, mergedByLogin, closedByLogin, autoMergeObservedAt :=
+		resolveTaskPROutcomeFields(&outgoing, status)
+
+	if _, err := tx.ExecContext(ctx,
 		`UPDATE github_task_prs SET owner = ?, repo = ?, pr_url = ?, pr_title = ?,
 			head_branch = ?, base_branch = ?, author_login = ?, state = ?, mergeable_state = ?,
-			additions = ?, deletions = ?, merged_at = ?, closed_at = ?, detached_at = NULL, updated_at = ?
+			additions = ?, deletions = ?, merged_at = ?, closed_at = ?, detached_at = NULL, updated_at = ?,
+			is_draft = ?, changed_files = ?, merged_by_login = ?, closed_by_login = ?,
+			auto_merge_observed_at = COALESCE(auto_merge_observed_at, ?)
 		 WHERE task_id = ? AND repository_id = ? AND pr_number = ?`,
 		pr.RepoOwner, pr.RepoName, pr.HTMLURL, pr.Title, pr.HeadBranch, pr.BaseBranch, pr.AuthorLogin,
 		pr.State, pr.MergeableState, pr.Additions, pr.Deletions, pr.MergedAt, pr.ClosedAt, time.Now().UTC(),
+		isDraft, changedFiles, mergedByLogin, closedByLogin, autoMergeObservedAt,
 		taskID, repositoryID, pr.Number); err != nil {
 		return nil, err
 	}
+	if err := tx.Commit(); err != nil {
+		return nil, err
+	}
 	return s.GetTaskPRByRepoAndNumber(ctx, taskID, repositoryID, pr.Number)
 }
 
@@ -1881,11 +2010,23 @@ func groupTaskPRsByTask(prs []TaskPR) map[string][]*TaskPR {
 // callers (RepositoryID == "") only delete legacy untagged rows for the
 // same PR number.
 //
+// status carries the caller's raw PR observation and its populated-ness
+// flags (AC-43p): ReplaceTaskPR resolves the five outcome-attribution
+// columns itself, inside this transaction, against the outgoing row it is
+// about to replace (AC-43, AC-43a) — callers must not pre-resolve them.
+// When no outgoing row exists (this method's ordinary production path,
+// since its caller only reaches here after confirming no row exists for
+// this PR number), resolution runs against a zero-value outgoing row: a
+// populating observation writes what it observed, a non-populating one
+// writes NULL in all five (AC-43).
+//
 // The DELETE+INSERT pair inside one transaction is the upsert form; an
 // ON CONFLICT would also work but the per-row delete pattern matches the
 // existing migration layout (rebuilds are easier to reason about) and
-// avoids leaking SQLite-specific syntax into the service layer.
-func (s *Store) ReplaceTaskPR(ctx context.Context, tp *TaskPR) error {
+// avoids leaking SQLite-specific syntax into the service layer. Returns
+// the row actually written, since the five resolved outcome fields may
+// differ from what the caller's tp carried in.
+func (s *Store) ReplaceTaskPR(ctx context.Context, tp *TaskPR, status *PRStatus) (*TaskPR, error) {
 	if tp.ID == "" {
 		tp.ID = uuid.New().String()
 	}
@@ -1894,41 +2035,91 @@ func (s *Store) ReplaceTaskPR(ctx context.Context, tp *TaskPR) error {
 
 	tx, err := s.db.BeginTxx(ctx, nil)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	defer func() { _ = tx.Rollback() }()
 
+	outgoing, err := replaceTaskPROutgoingRow(ctx, tx, tp)
+	if err != nil {
+		return nil, err
+	}
+	tp.IsDraft, tp.ChangedFiles, tp.MergedByLogin, tp.ClosedByLogin, tp.AutoMergeObservedAt =
+		resolveTaskPROutcomeFields(outgoing, status)
+
 	if tp.RepositoryID != "" {
 		if _, err := tx.ExecContext(ctx,
 			`DELETE FROM github_task_prs
 			 WHERE task_id = ? AND repository_id = ? AND pr_number = ?`,
 			tp.TaskID, tp.RepositoryID, tp.PRNumber); err != nil {
-			return err
+			return nil, err
 		}
 	} else {
 		if _, err := tx.ExecContext(ctx,
 			`DELETE FROM github_task_prs
 			 WHERE task_id = ? AND repository_id = '' AND pr_number = ?`,
 			tp.TaskID, tp.PRNumber); err != nil {
-			return err
+			return nil, err
 		}
 	}
 	if _, err := tx.ExecContext(ctx, `
 		INSERT INTO github_task_prs (id, workspace_id, task_id, repository_id, owner, repo, pr_number, pr_url, pr_title, head_branch, base_branch, author_login,
 			state, review_state, checks_state, mergeable_state, review_count, pending_review_count, required_reviews, comment_count,
 			unresolved_review_threads, checks_total, checks_passing, additions, deletions,
-			created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at)
-		VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
+			created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at,
+			is_draft, changed_files, merged_by_login, closed_by_login, auto_merge_observed_at)
+		VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
 		tp.ID, tp.WorkspaceID, tp.TaskID, tp.RepositoryID, tp.Owner, tp.Repo, tp.PRNumber, tp.PRURL, tp.PRTitle, tp.HeadBranch, tp.BaseBranch, tp.AuthorLogin,
 		tp.State, tp.ReviewState, tp.ChecksState, tp.MergeableState, tp.ReviewCount, tp.PendingReviewCount, tp.RequiredReviews, tp.CommentCount,
 		tp.UnresolvedReviewThreads, tp.ChecksTotal, tp.ChecksPassing, tp.Additions, tp.Deletions,
-		tp.CreatedAt, tp.MergedAt, tp.ClosedAt, tp.LastSyncedAt, tp.DetachedAt, tp.UpdatedAt); err != nil {
-		return err
+		tp.CreatedAt, tp.MergedAt, tp.ClosedAt, tp.LastSyncedAt, tp.DetachedAt, tp.UpdatedAt,
+		tp.IsDraft, tp.ChangedFiles, tp.MergedByLogin, tp.ClosedByLogin, tp.AutoMergeObservedAt); err != nil {
+		return nil, err
 	}
-	return tx.Commit()
+	if err := tx.Commit(); err != nil {
+		return nil, err
+	}
+	return tp, nil
 }
 
-// UpdateTaskPR updates a task-PR association.
+// replaceTaskPROutgoingRow reads, inside tx, the row ReplaceTaskPR is about
+// to delete-and-replace for tp's (task_id, repository_id, pr_number), or a
+// zero-value TaskPR when none exists (AC-43's "no outgoing row" case, the
+// ordinary production path). Reading via tx means this observes the same
+// snapshot the subsequent DELETE/INSERT commits against (AC-43a).
+func replaceTaskPROutgoingRow(ctx context.Context, tx *sqlx.Tx, tp *TaskPR) (*TaskPR, error) {
+	var outgoing TaskPR
+	var err error
+	if tp.RepositoryID != "" {
+		err = tx.GetContext(ctx, &outgoing,
+			`SELECT `+taskPRColumns+` FROM github_task_prs
+			 WHERE task_id = ? AND repository_id = ? AND pr_number = ?`,
+			tp.TaskID, tp.RepositoryID, tp.PRNumber)
+	} else {
+		err = tx.GetContext(ctx, &outgoing,
+			`SELECT `+taskPRColumns+` FROM github_task_prs
+			 WHERE task_id = ? AND repository_id = '' AND pr_number = ?`,
+			tp.TaskID, tp.PRNumber)
+	}
+	if errors.Is(err, sql.ErrNoRows) {
+		return &TaskPR{}, nil
+	}
+	if err != nil {
+		return nil, err
+	}
+	return &outgoing, nil
+}
+
+// UpdateTaskPR updates a task-PR association. This is the sync writer's
+// exclusive write path.
+// UpdateTaskPR writes auto_merge_observed_at through a SQL-level
+// COALESCE(auto_merge_observed_at, ?) rather than a direct SET. The Go-side
+// latch check in resolveTaskPROutcomeFields (tp.AutoMergeObservedAt == nil)
+// reads a snapshot that can be stale by the time this statement executes —
+// two concurrent syncs can both observe NULL and each compute their own
+// "now" timestamp. COALESCE evaluates the row's *current* value at
+// UPDATE-execution time, inside this single statement, so whichever write
+// actually lands first wins atomically and the second's differing timestamp
+// is silently discarded instead of overwriting it (AC-16/AC-17).
 func (s *Store) UpdateTaskPR(ctx context.Context, tp *TaskPR) error {
 	tp.UpdatedAt = time.Now().UTC()
 	_, err := s.db.ExecContext(ctx, `
@@ -1936,13 +2127,17 @@ func (s *Store) UpdateTaskPR(ctx context.Context, tp *TaskPR) error {
 			review_count = ?, pending_review_count = ?, required_reviews = ?, comment_count = ?,
 			unresolved_review_threads = ?, checks_total = ?, checks_passing = ?,
 			additions = ?, deletions = ?, pr_title = ?, base_branch = ?,
-			merged_at = ?, closed_at = ?, last_synced_at = ?, updated_at = ?
+			merged_at = ?, closed_at = ?, last_synced_at = ?, updated_at = ?,
+			is_draft = ?, changed_files = ?, merged_by_login = ?, closed_by_login = ?,
+			auto_merge_observed_at = COALESCE(auto_merge_observed_at, ?)
 		WHERE id = ?`,
 		tp.State, tp.ReviewState, tp.ChecksState, tp.MergeableState,
 		tp.ReviewCount, tp.PendingReviewCount, tp.RequiredReviews, tp.CommentCount,
 		tp.UnresolvedReviewThreads, tp.ChecksTotal, tp.ChecksPassing,
 		tp.Additions, tp.Deletions, tp.PRTitle, tp.BaseBranch,
-		tp.MergedAt, tp.ClosedAt, tp.LastSyncedAt, tp.UpdatedAt, tp.ID)
+		tp.MergedAt, tp.ClosedAt, tp.LastSyncedAt, tp.UpdatedAt,
+		tp.IsDraft, tp.ChangedFiles, tp.MergedByLogin, tp.ClosedByLogin,
+		tp.AutoMergeObservedAt, tp.ID)
 	return err
 }
 

apps/backend/internal/github/store_ci_automation_test.go

@@ -766,3 +766,5 @@ func TestStoreMigrateTaskCIOptionsToPRScope_NewlyLinkedPRStartsAllOff(t *testing
 }
 
 func boolPtr(v bool) *bool { return &v }
+
+func intPtr(v int) *int { return &v }

apps/backend/internal/github/store_multi_repo_test.go

@@ -116,10 +116,10 @@ func TestTaskPR_ReplaceTaskPR_PreservesSiblingsAndUpserts(t *testing.T) {
 
 	// Add a new PR #99 on repo-a — must NOT delete repo-a's PR #1
 	// (multi-branch) nor repo-b's #2 (multi-repo).
-	if err := store.ReplaceTaskPR(ctx, &TaskPR{
+	if _, err := store.ReplaceTaskPR(ctx, &TaskPR{
 		TaskID: "task-2", RepositoryID: "repo-a",
 		Owner: "o", Repo: "r", PRNumber: 99, CreatedAt: now,
-	}); err != nil {
+	}, &PRStatus{}); err != nil {
 		t.Fatalf("replace A#99: %v", err)
 	}
 
@@ -130,10 +130,10 @@ func TestTaskPR_ReplaceTaskPR_PreservesSiblingsAndUpserts(t *testing.T) {
 
 	// Now overwrite repo-a #99 by calling ReplaceTaskPR with the same
 	// (task, repo, pr_number) and a different field — upsert path.
-	if err := store.ReplaceTaskPR(ctx, &TaskPR{
+	if _, err := store.ReplaceTaskPR(ctx, &TaskPR{
 		TaskID: "task-2", RepositoryID: "repo-a",
 		Owner: "o", Repo: "r", PRNumber: 99, PRTitle: "updated", CreatedAt: now,
-	}); err != nil {
+	}, &PRStatus{}); err != nil {
 		t.Fatalf("upsert A#99: %v", err)
 	}
 	all, _ = store.ListTaskPRsByTask(ctx, "task-2")

apps/backend/internal/github/store_task_pr_detach_test.go

@@ -152,3 +152,120 @@ func TestLegacyTaskPRRebuildPreservesDetachedAt(t *testing.T) {
 		t.Fatalf("migrated active PRs = %+v, want detached row filtered", active)
 	}
 }
+
+// TestLegacyTaskPRRebuildPreservesOutcomeColumns is the regression for
+// AC-07a: the legacy-constraint rebuild's CREATE TABLE and
+// INSERT...SELECT copy list must both carry all five outcome-attribution
+// columns, or a real user database silently loses merged_by_login,
+// closed_by_login, is_draft, changed_files, and auto_merge_observed_at the
+// moment the rebuild fires. TestLegacyTaskPRRebuildPreservesDetachedAt above
+// never populates the outcome columns on its seeded row, so it cannot catch
+// a rebuild that drops them from the copy list: without a seeded legacy
+// constraint AND non-NULL outcome values, the rebuild either no-ops or
+// degenerately "succeeds" while silently losing data. This test seeds all
+// five with non-NULL values and reopens the store twice
+// (task_external_id_migration_test.go's replay precedent) to also prove
+// idempotence once the legacy constraint is gone.
+func TestLegacyTaskPRRebuildPreservesOutcomeColumns(t *testing.T) {
+	store := newTestStore(t)
+	ctx := context.Background()
+	now := time.Now().UTC()
+	mergedAt := now.Add(-time.Hour)
+	autoMergeAt := now.Add(-2 * time.Hour)
+	if _, err := store.db.ExecContext(ctx, `
+		INSERT INTO workspaces (id) VALUES ('ws-legacy-outcome');
+		INSERT INTO tasks (id, workspace_id) VALUES ('task-legacy-outcome', 'ws-legacy-outcome');
+		DROP TABLE github_task_prs;
+		CREATE TABLE github_task_prs (
+			id TEXT PRIMARY KEY,
+			workspace_id TEXT NOT NULL DEFAULT '',
+			task_id TEXT NOT NULL,
+			owner TEXT NOT NULL,
+			repo TEXT NOT NULL,
+			pr_number INTEGER NOT NULL,
+			pr_url TEXT NOT NULL,
+			pr_title TEXT NOT NULL,
+			head_branch TEXT NOT NULL,
+			base_branch TEXT NOT NULL,
+			author_login TEXT NOT NULL,
+			state TEXT NOT NULL DEFAULT 'open',
+			review_state TEXT NOT NULL DEFAULT '',
+			checks_state TEXT NOT NULL DEFAULT '',
+			mergeable_state TEXT NOT NULL DEFAULT '',
+			review_count INTEGER DEFAULT 0,
+			pending_review_count INTEGER DEFAULT 0,
+			required_reviews INTEGER,
+			comment_count INTEGER DEFAULT 0,
+			unresolved_review_threads INTEGER DEFAULT 0,
+			checks_total INTEGER DEFAULT 0,
+			checks_passing INTEGER DEFAULT 0,
+			additions INTEGER DEFAULT 0,
+			deletions INTEGER DEFAULT 0,
+			created_at DATETIME NOT NULL,
+			merged_at DATETIME,
+			closed_at DATETIME,
+			last_synced_at DATETIME,
+			detached_at DATETIME,
+			updated_at DATETIME NOT NULL,
+			is_draft BOOLEAN,
+			changed_files INTEGER,
+			merged_by_login TEXT,
+			closed_by_login TEXT,
+			auto_merge_observed_at DATETIME,
+			UNIQUE(task_id, pr_number)
+		);
+		INSERT INTO github_task_prs (
+			id, workspace_id, task_id, owner, repo, pr_number, pr_url, pr_title,
+			head_branch, base_branch, author_login, state, created_at, merged_at, updated_at,
+			is_draft, changed_files, merged_by_login, closed_by_login, auto_merge_observed_at
+		) VALUES (
+			'legacy-outcome', 'ws-legacy-outcome', 'task-legacy-outcome', 'acme', 'demo', 77,
+			'https://github.com/acme/demo/pull/77', 'Legacy outcome PR', 'feat/legacy-outcome',
+			'main', 'octocat', 'merged', ?, ?, ?,
+			0, 12, 'carlosflorencio', 'octocat', ?
+		)
+	`, now, mergedAt, now, autoMergeAt); err != nil {
+		t.Fatalf("seed legacy task PR schema with outcome columns: %v", err)
+	}
+
+	assertOutcomeColumnsSurvived := func(t *testing.T, s *Store) {
+		t.Helper()
+		got, err := s.GetTaskPRByID(ctx, "legacy-outcome")
+		if err != nil {
+			t.Fatalf("get migrated outcome PR: %v", err)
+		}
+		if got == nil {
+			t.Fatal("migrated outcome PR not found")
+		}
+		if got.IsDraft == nil || *got.IsDraft != false {
+			t.Errorf("is_draft = %v, want false (observed, not NULL)", got.IsDraft)
+		}
+		if got.ChangedFiles == nil || *got.ChangedFiles != 12 {
+			t.Errorf("changed_files = %v, want 12", got.ChangedFiles)
+		}
+		if got.MergedByLogin == nil || *got.MergedByLogin != "carlosflorencio" {
+			t.Errorf("merged_by_login = %v, want carlosflorencio", got.MergedByLogin)
+		}
+		if got.ClosedByLogin == nil || *got.ClosedByLogin != "octocat" {
+			t.Errorf("closed_by_login = %v, want octocat", got.ClosedByLogin)
+		}
+		if got.AutoMergeObservedAt == nil || !got.AutoMergeObservedAt.Equal(autoMergeAt) {
+			t.Errorf("auto_merge_observed_at = %v, want %s", got.AutoMergeObservedAt, autoMergeAt)
+		}
+	}
+
+	reopened, err := NewStore(store.db, store.ro)
+	if err != nil {
+		t.Fatalf("reopen store with legacy outcome schema: %v", err)
+	}
+	assertOutcomeColumnsSurvived(t, reopened)
+
+	// Reopen a second time: the legacy UNIQUE(task_id, pr_number) constraint
+	// is gone after the first rebuild, so this run must be a no-op that
+	// leaves the already-migrated values untouched (idempotence).
+	reopenedAgain, err := NewStore(reopened.db, reopened.ro)
+	if err != nil {
+		t.Fatalf("reopen store a second time: %v", err)
+	}
+	assertOutcomeColumnsSurvived(t, reopenedAgain)
+}

apps/backend/internal/github/store_task_pr_outcome_migration_test.go

@@ -0,0 +1,236 @@
+package github
+
+import (
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/jmoiron/sqlx"
+
+	dbutil "github.com/kandev/kandev/internal/db"
+	"github.com/kandev/kandev/internal/persistence"
+)
+
+// preOutcomeGithubTaskPRsDDL is a snapshot of the github_task_prs DDL as it
+// existed before the five outcome-attribution columns were added. Used to
+// simulate a pre-migration database.
+const preOutcomeGithubTaskPRsDDL = `
+	CREATE TABLE github_task_prs (
+		id TEXT PRIMARY KEY,
+		workspace_id TEXT NOT NULL DEFAULT '',
+		task_id TEXT NOT NULL,
+		repository_id TEXT NOT NULL DEFAULT '',
+		owner TEXT NOT NULL,
+		repo TEXT NOT NULL,
+		pr_number INTEGER NOT NULL,
+		pr_url TEXT NOT NULL,
+		pr_title TEXT NOT NULL,
+		head_branch TEXT NOT NULL,
+		base_branch TEXT NOT NULL,
+		author_login TEXT NOT NULL,
+		state TEXT NOT NULL DEFAULT 'open',
+		review_state TEXT NOT NULL DEFAULT '',
+		checks_state TEXT NOT NULL DEFAULT '',
+		mergeable_state TEXT NOT NULL DEFAULT '',
+		review_count INTEGER DEFAULT 0,
+		pending_review_count INTEGER DEFAULT 0,
+		required_reviews INTEGER,
+		comment_count INTEGER DEFAULT 0,
+		unresolved_review_threads INTEGER DEFAULT 0,
+		checks_total INTEGER DEFAULT 0,
+		checks_passing INTEGER DEFAULT 0,
+		additions INTEGER DEFAULT 0,
+		deletions INTEGER DEFAULT 0,
+		created_at DATETIME NOT NULL,
+		merged_at DATETIME,
+		closed_at DATETIME,
+		last_synced_at DATETIME,
+		detached_at DATETIME,
+		updated_at DATETIME NOT NULL,
+		UNIQUE(task_id, repository_id, pr_number)
+	)`
+
+const outcomeMetaKey = "github_task_pr_outcome_activated_at"
+
+// newPreOutcomeMigrationStore opens a fresh SQLite DB seeded with the
+// pre-migration shape of github_task_prs (missing all five outcome columns)
+// plus the tasks/workspaces tables NewStore expects, and returns the raw
+// handle for pre-migration seeding before the caller drives schema init via
+// NewStore.
+func newPreOutcomeMigrationStore(t *testing.T) (*sqlx.DB, string) {
+	t.Helper()
+	dbPath := filepath.Join(t.TempDir(), "github-outcome-migration.db")
+	dbConn, err := dbutil.OpenSQLite(dbPath)
+	if err != nil {
+		t.Fatalf("open sqlite: %v", err)
+	}
+	db := sqlx.NewDb(dbConn, "sqlite3")
+	t.Cleanup(func() { _ = db.Close() })
+	if _, err := db.Exec(`CREATE TABLE tasks (id TEXT PRIMARY KEY, workspace_id TEXT, archived_at DATETIME)`); err != nil {
+		t.Fatalf("create tasks table: %v", err)
+	}
+	if _, err := db.Exec(`CREATE TABLE workspaces (id TEXT PRIMARY KEY)`); err != nil {
+		t.Fatalf("create workspaces table: %v", err)
+	}
+	if _, err := db.Exec(preOutcomeGithubTaskPRsDDL); err != nil {
+		t.Fatalf("create pre-migration github_task_prs: %v", err)
+	}
+	return db, dbPath
+}
+
+// TestTaskPROutcomeMigration_AddsColumnsNoBackfillActivatesOnce covers AC-01
+// through AC-06: an existing database lacking the five outcome columns gets
+// them via ADD COLUMN with no NOT NULL/DEFAULT, a pre-existing terminal row
+// stays NULL in all five after two startups, the activation instant is
+// written exactly once in RFC 3339 form, and a second startup does not
+// advance it.
+func TestTaskPROutcomeMigration_AddsColumnsNoBackfillActivatesOnce(t *testing.T) {
+	db, _ := newPreOutcomeMigrationStore(t)
+
+	now := time.Now().UTC()
+	if _, err := db.Exec(db.Rebind(`
+		INSERT INTO tasks (id, workspace_id) VALUES (?, ?)
+	`), "task-pre-existing", "ws-outcome-migration"); err != nil {
+		t.Fatalf("seed task: %v", err)
+	}
+	if _, err := db.Exec(db.Rebind(`
+		INSERT INTO github_task_prs (
+			id, workspace_id, task_id, owner, repo, pr_number, pr_url, pr_title,
+			head_branch, base_branch, author_login, state, created_at, closed_at, updated_at
+		) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+	`), "pr-pre-existing", "ws-outcome-migration", "task-pre-existing", "kdlbs", "kandev",
+		2476, "https://github.com/kdlbs/kandev/pull/2476", "pre-existing PR",
+		"feature/x", "main", "nova28", "closed", now, now, now); err != nil {
+		t.Fatalf("seed pre-existing terminal github_task_prs row: %v", err)
+	}
+
+	if _, err := NewStore(db, db); err != nil {
+		t.Fatalf("first NewStore (migration): %v", err)
+	}
+
+	assertOutcomeColumnsNull(t, db, "pr-pre-existing")
+	firstActivation := requireOutcomeActivationOnce(t, db)
+
+	if _, err := NewStore(db, db); err != nil {
+		t.Fatalf("second NewStore (replay): %v", err)
+	}
+
+	assertOutcomeColumnsNull(t, db, "pr-pre-existing")
+	secondActivation := requireOutcomeActivationOnce(t, db)
+	if secondActivation != firstActivation {
+		t.Fatalf("activation instant changed across replay: first=%q second=%q", firstActivation, secondActivation)
+	}
+}
+
+// TestTaskPROutcomeMigration_FreshInstallReplaysClean covers the ADR-0027
+// fresh-plus-replay requirement: a brand-new database (columns already in
+// createTablesSQL) initializes once and replays cleanly a second time,
+// still stamping the activation instant exactly once.
+func TestTaskPROutcomeMigration_FreshInstallReplaysClean(t *testing.T) {
+	store := newTestStore(t)
+
+	first := requireOutcomeActivationOnce(t, store.db)
+
+	if _, err := NewStore(store.db, store.db); err != nil {
+		t.Fatalf("replay NewStore on fresh install: %v", err)
+	}
+	second := requireOutcomeActivationOnce(t, store.db)
+	if second != first {
+		t.Fatalf("activation instant changed across fresh-install replay: first=%q second=%q", first, second)
+	}
+}
+
+func assertOutcomeColumnsNull(t *testing.T, db *sqlx.DB, id string) {
+	t.Helper()
+	var isDraft, changedFiles, mergedByLogin, closedByLogin, autoMergeObservedAt any
+	err := db.QueryRow(db.Rebind(`
+		SELECT is_draft, changed_files, merged_by_login, closed_by_login, auto_merge_observed_at
+		FROM github_task_prs WHERE id = ?
+	`), id).Scan(
+		&isDraft, &changedFiles, &mergedByLogin, &closedByLogin, &autoMergeObservedAt,
+	)
+	if err != nil {
+		t.Fatalf("scan outcome columns for %q: %v", id, err)
+	}
+	for name, val := range map[string]any{
+		"is_draft":               isDraft,
+		"changed_files":          changedFiles,
+		"merged_by_login":        mergedByLogin,
+		"closed_by_login":        closedByLogin,
+		"auto_merge_observed_at": autoMergeObservedAt,
+	} {
+		if val != nil {
+			t.Errorf("%s on %q = %v, want NULL", name, id, val)
+		}
+	}
+}
+
+func requireOutcomeActivationOnce(t *testing.T, db *sqlx.DB) string {
+	t.Helper()
+	val, err := persistence.ReadMetaKey(db, outcomeMetaKey)
+	if err != nil {
+		t.Fatalf("ReadMetaKey(%s): %v", outcomeMetaKey, err)
+	}
+	if val == "" {
+		t.Fatalf("%s not set after migration", outcomeMetaKey)
+	}
+	if _, err := time.Parse(time.RFC3339, val); err != nil {
+		t.Fatalf("%s = %q is not RFC 3339: %v", outcomeMetaKey, val, err)
+	}
+	return val
+}
+
+// TestAddTaskPROutcomeColumns_NonDuplicateErrorAbortsRatherThanSwallows is
+// the regression for AC-03's fail-loud branch, which the migration replay
+// tests above never exercise (they only ever see the tolerated
+// duplicate-column case). Deliberately omitting github_task_prs entirely
+// makes tableColumns' PRAGMA table_info read return zero rows without an
+// error (SQLite does not error on a missing table there), so
+// addTaskPROutcomeColumns proceeds to `ALTER TABLE github_task_prs ADD
+// COLUMN ...`, which fails with "no such table" — a genuine non-duplicate
+// error distinct from the one dbutil.IsDuplicateColumnError tolerates.
+func TestAddTaskPROutcomeColumns_NonDuplicateErrorAbortsRatherThanSwallows(t *testing.T) {
+	dbPath := filepath.Join(t.TempDir(), "no-github-task-prs-table.db")
+	dbConn, err := dbutil.OpenSQLite(dbPath)
+	if err != nil {
+		t.Fatalf("open sqlite: %v", err)
+	}
+	t.Cleanup(func() { _ = dbConn.Close() })
+	db := sqlx.NewDb(dbConn, "sqlite3")
+
+	store := &Store{db: db, ro: db}
+	err = store.addTaskPROutcomeColumns()
+	if err == nil {
+		t.Fatal("addTaskPROutcomeColumns: want error against a database missing github_task_prs, got nil")
+	}
+	if dbutil.IsDuplicateColumnError(err) {
+		t.Fatalf("addTaskPROutcomeColumns misclassified a missing-table error as duplicate-column: %v", err)
+	}
+}
+
+// TestActivateTaskPROutcomeTracking_WriteFailureAbortsRatherThanSwallows is
+// the companion regression for the activation-instant write (spec: Failure
+// modes — "kandev_meta write fails during activation: Startup aborts").
+// Pre-creating kandev_meta with a key column that carries no UNIQUE/PRIMARY
+// KEY constraint leaves EnsureMetaTable's CREATE TABLE IF NOT EXISTS a
+// no-op (the table already exists), so WriteMetaKeyIfAbsent's `INSERT ...
+// ON CONFLICT(key) DO NOTHING` fails outright — SQLite requires the
+// conflict target to match a real constraint — which is a genuine,
+// non-swallowed write failure.
+func TestActivateTaskPROutcomeTracking_WriteFailureAbortsRatherThanSwallows(t *testing.T) {
+	dbPath := filepath.Join(t.TempDir(), "malformed-kandev-meta.db")
+	dbConn, err := dbutil.OpenSQLite(dbPath)
+	if err != nil {
+		t.Fatalf("open sqlite: %v", err)
+	}
+	t.Cleanup(func() { _ = dbConn.Close() })
+	db := sqlx.NewDb(dbConn, "sqlite3")
+	if _, err := db.Exec(`CREATE TABLE kandev_meta (key TEXT)`); err != nil {
+		t.Fatalf("seed malformed kandev_meta: %v", err)
+	}
+
+	store := &Store{db: db, ro: db}
+	if err := store.activateTaskPROutcomeTracking(); err == nil {
+		t.Fatal("activateTaskPROutcomeTracking: want error against a malformed kandev_meta table, got nil")
+	}
+}

apps/backend/internal/github/store_taskpr_schema_drift_test.go

@@ -2,6 +2,8 @@ package github
 
 import (
 	"context"
+	"strings"
+	"sync"
 	"testing"
 	"time"
 )
@@ -46,7 +48,7 @@ func TestTaskPRReads_ToleratesExtraColumn(t *testing.T) {
 		State:       "open",
 		CreatedAt:   time.Now().UTC(),
 	}
-	if err := store.ReplaceTaskPR(ctx, tp); err != nil {
+	if _, err := store.ReplaceTaskPR(ctx, tp, &PRStatus{}); err != nil {
 		t.Fatalf("ReplaceTaskPR: %v", err)
 	}
 
@@ -94,3 +96,409 @@ func TestTaskPRReads_ToleratesExtraColumn(t *testing.T) {
 		t.Fatalf("ListTaskPRsByWorkspaceID: expected 1 PR (#1978) for task-drift, got %+v", byWorkspace)
 	}
 }
+
+// taskPROutcomeColumnNames is the AC-07 checklist: every one of the five
+// outcome-attribution columns must appear in taskPRColumns,
+// taskPRColumnsQualified, and the CreateTaskPR/ReplaceTaskPR INSERT column
+// lists, or a read/write path silently regresses to losing the column.
+var taskPROutcomeColumnNames = []string{
+	"is_draft", "changed_files", "merged_by_login", "closed_by_login",
+	"auto_merge_observed_at",
+}
+
+// TestTaskPRColumnLists_IncludeAllOutcomeColumns covers AC-07: every outcome
+// column is present in both the qualified and unqualified read projections.
+func TestTaskPRColumnLists_IncludeAllOutcomeColumns(t *testing.T) {
+	for _, name := range taskPROutcomeColumnNames {
+		if !strings.Contains(taskPRColumns, name) {
+			t.Errorf("taskPRColumns missing %q", name)
+		}
+		if !strings.Contains(taskPRColumnsQualified, "gtp."+name) {
+			t.Errorf("taskPRColumnsQualified missing qualified %q", name)
+		}
+	}
+}
+
+// TestCreateAndReplaceTaskPR_RoundTripOutcomeColumns covers the write half
+// of AC-07: CreateTaskPR and ReplaceTaskPR must persist all five outcome
+// columns, not just the pre-existing ones, or a linked PR loses any
+// upstream-observed outcome data the moment it is (re)written.
+//
+// ReplaceTaskPR resolves the five columns itself from a *PRStatus
+// observation (AC-43), rather than trusting a caller-supplied TaskPR's own
+// field values, so the "replaced" case below drives it with an explicit
+// populating status instead of presetting replaced's outcome fields.
+func TestCreateAndReplaceTaskPR_RoundTripOutcomeColumns(t *testing.T) {
+	store := newTestStore(t)
+	ctx := context.Background()
+	if _, err := store.db.Exec(`INSERT INTO tasks (id, workspace_id) VALUES ('task-roundtrip', 'ws-roundtrip')`); err != nil {
+		t.Fatalf("seed task: %v", err)
+	}
+
+	isDraft := true
+	changedFiles := 7
+	mergedBy := "carlosflorencio"
+	closedBy := "nova28"
+	autoMergeAt := time.Now().UTC().Truncate(time.Second)
+
+	created := &TaskPR{
+		WorkspaceID:         "ws-roundtrip",
+		TaskID:              "task-roundtrip",
+		Owner:               "kdlbs",
+		Repo:                "kandev",
+		PRNumber:            5001,
+		PRURL:               "https://github.com/kdlbs/kandev/pull/5001",
+		PRTitle:             "outcome column round trip",
+		State:               "closed",
+		CreatedAt:           time.Now().UTC(),
+		IsDraft:             &isDraft,
+		ChangedFiles:        &changedFiles,
+		MergedByLogin:       &mergedBy,
+		ClosedByLogin:       &closedBy,
+		AutoMergeObservedAt: &autoMergeAt,
+	}
+	if err := store.CreateTaskPR(ctx, created); err != nil {
+		t.Fatalf("CreateTaskPR: %v", err)
+	}
+	gotCreated, err := store.GetTaskPRByID(ctx, created.ID)
+	if err != nil {
+		t.Fatalf("GetTaskPRByID after CreateTaskPR: %v", err)
+	}
+	assertOutcomeColumnsRoundTrip(t, "CreateTaskPR", gotCreated, isDraft, changedFiles, mergedBy, closedBy)
+
+	replaced := &TaskPR{
+		WorkspaceID: "ws-roundtrip",
+		TaskID:      "task-roundtrip",
+		Owner:       "kdlbs",
+		Repo:        "kandev",
+		PRNumber:    5001,
+		PRURL:       "https://github.com/kdlbs/kandev/pull/5001",
+		PRTitle:     "outcome column round trip (replaced)",
+		State:       "closed",
+		CreatedAt:   time.Now().UTC(),
+	}
+	status := &PRStatus{
+		PR: &PR{
+			Draft:                isDraft,
+			IsDraftObserved:      true,
+			ChangedFiles:         changedFiles,
+			ChangedFilesObserved: true,
+			MergedByLogin:        mergedBy,
+			AutoMergeEnabled:     true,
+		},
+		OutcomeFieldsPopulated:      true,
+		ClosedByLogin:               closedBy,
+		ClosureAttributionPopulated: true,
+	}
+	gotReplaceResult, err := store.ReplaceTaskPR(ctx, replaced, status)
+	if err != nil {
+		t.Fatalf("ReplaceTaskPR: %v", err)
+	}
+	assertOutcomeColumnsRoundTrip(t, "ReplaceTaskPR (return value)", gotReplaceResult, isDraft, changedFiles, mergedBy, closedBy)
+
+	gotReplaced, err := store.GetTaskPRByRepoAndNumber(ctx, "task-roundtrip", "", 5001)
+	if err != nil {
+		t.Fatalf("GetTaskPRByRepoAndNumber after ReplaceTaskPR: %v", err)
+	}
+	assertOutcomeColumnsRoundTrip(t, "ReplaceTaskPR", gotReplaced, isDraft, changedFiles, mergedBy, closedBy)
+}
+
+func assertOutcomeColumnsRoundTrip(
+	t *testing.T, writer string, got *TaskPR,
+	wantIsDraft bool, wantChangedFiles int, wantMergedBy, wantClosedBy string,
+) {
+	t.Helper()
+	if got == nil {
+		t.Fatalf("%s: row not found", writer)
+	}
+	if got.IsDraft == nil || *got.IsDraft != wantIsDraft {
+		t.Errorf("%s: IsDraft = %v, want %v", writer, got.IsDraft, wantIsDraft)
+	}
+	if got.ChangedFiles == nil || *got.ChangedFiles != wantChangedFiles {
+		t.Errorf("%s: ChangedFiles = %v, want %v", writer, got.ChangedFiles, wantChangedFiles)
+	}
+	if got.MergedByLogin == nil || *got.MergedByLogin != wantMergedBy {
+		t.Errorf("%s: MergedByLogin = %v, want %v", writer, got.MergedByLogin, wantMergedBy)
+	}
+	if got.ClosedByLogin == nil || *got.ClosedByLogin != wantClosedBy {
+		t.Errorf("%s: ClosedByLogin = %v, want %v", writer, got.ClosedByLogin, wantClosedBy)
+	}
+	if got.AutoMergeObservedAt == nil {
+		t.Errorf("%s: AutoMergeObservedAt = nil, want set", writer)
+	}
+}
+
+// TestReplaceTaskPR_PreservesAutoMergeLatch covers AC-43: a non-populating
+// observation must not clobber any of the five outcome columns just because
+// ReplaceTaskPR's DELETE+INSERT upsert touches the row. Seeding and
+// asserting all five, not just the latch, matters here: the spec's
+// verification-surfaces section calls out that a latch-only implementation
+// can pass a latch-only test while silently zeroing is_draft, changed_files,
+// merged_by_login and closed_by_login.
+func TestReplaceTaskPR_PreservesAutoMergeLatch(t *testing.T) {
+	store := newTestStore(t)
+	ctx := context.Background()
+	if _, err := store.db.Exec(`INSERT INTO tasks (id, workspace_id) VALUES ('task-latch-replace', 'ws-latch')`); err != nil {
+		t.Fatalf("seed task: %v", err)
+	}
+
+	latchedAt := time.Now().UTC().Add(-time.Hour).Truncate(time.Second)
+	seedIsDraft := true
+	seedChangedFiles := 9
+	seedMergedBy := "carlosflorencio"
+	seedClosedBy := "nova28"
+	seed := &TaskPR{
+		WorkspaceID:         "ws-latch",
+		TaskID:              "task-latch-replace",
+		Owner:               "kdlbs",
+		Repo:                "kandev",
+		PRNumber:            6001,
+		PRURL:               "https://github.com/kdlbs/kandev/pull/6001",
+		PRTitle:             "latch seed",
+		State:               "open",
+		CreatedAt:           time.Now().UTC(),
+		IsDraft:             &seedIsDraft,
+		ChangedFiles:        &seedChangedFiles,
+		MergedByLogin:       &seedMergedBy,
+		ClosedByLogin:       &seedClosedBy,
+		AutoMergeObservedAt: &latchedAt,
+	}
+	if err := store.CreateTaskPR(ctx, seed); err != nil {
+		t.Fatalf("seed CreateTaskPR: %v", err)
+	}
+
+	replace := &TaskPR{
+		WorkspaceID: "ws-latch",
+		TaskID:      "task-latch-replace",
+		Owner:       "kdlbs",
+		Repo:        "kandev",
+		PRNumber:    6001,
+		PRURL:       "https://github.com/kdlbs/kandev/pull/6001",
+		PRTitle:     "latch seed (replaced)",
+		State:       "open",
+		CreatedAt:   time.Now().UTC(),
+	}
+	replaced, err := store.ReplaceTaskPR(ctx, replace, &PRStatus{})
+	if err != nil {
+		t.Fatalf("ReplaceTaskPR: %v", err)
+	}
+	assertOutcomeColumnsRoundTrip(t, "ReplaceTaskPR return value (preserved)",
+		replaced, seedIsDraft, seedChangedFiles, seedMergedBy, seedClosedBy)
+	if !replaced.AutoMergeObservedAt.Equal(latchedAt) {
+		t.Fatalf("ReplaceTaskPR return value: AutoMergeObservedAt = %v, want preserved %v", replaced.AutoMergeObservedAt, latchedAt)
+	}
+
+	got, err := store.GetTaskPRByRepoAndNumber(ctx, "task-latch-replace", "", 6001)
+	if err != nil {
+		t.Fatalf("GetTaskPRByRepoAndNumber: %v", err)
+	}
+	assertOutcomeColumnsRoundTrip(t, "ReplaceTaskPR stored row (preserved)",
+		got, seedIsDraft, seedChangedFiles, seedMergedBy, seedClosedBy)
+	if got == nil || got.AutoMergeObservedAt == nil || !got.AutoMergeObservedAt.Equal(latchedAt) {
+		t.Fatalf("stored row: AutoMergeObservedAt = %v, want preserved %v", got, latchedAt)
+	}
+}
+
+// TestRestoreTaskPR_PreservesAutoMergeLatch mirrors
+// TestReplaceTaskPR_PreservesAutoMergeLatch for the relink-after-detach path
+// (AC-43): RestoreTaskPR must carry the deleted row's outcome columns
+// forward — all five, not just auto_merge_observed_at — when the relink's
+// observation doesn't populate outcome fields.
+func TestRestoreTaskPR_PreservesAutoMergeLatch(t *testing.T) {
+	store := newTestStore(t)
+	ctx := context.Background()
+	if _, err := store.db.Exec(`INSERT INTO tasks (id, workspace_id) VALUES ('task-latch-restore', 'ws-latch')`); err != nil {
+		t.Fatalf("seed task: %v", err)
+	}
+
+	latchedAt := time.Now().UTC().Add(-time.Hour).Truncate(time.Second)
+	seedIsDraft := true
+	seedChangedFiles := 3
+	seedMergedBy := "carlosflorencio"
+	seedClosedBy := "nova28"
+	seed := &TaskPR{
+		WorkspaceID:         "ws-latch",
+		TaskID:              "task-latch-restore",
+		Owner:               "kdlbs",
+		Repo:                "kandev",
+		PRNumber:            6002,
+		PRURL:               "https://github.com/kdlbs/kandev/pull/6002",
+		PRTitle:             "latch seed",
+		State:               "open",
+		CreatedAt:           time.Now().UTC(),
+		IsDraft:             &seedIsDraft,
+		ChangedFiles:        &seedChangedFiles,
+		MergedByLogin:       &seedMergedBy,
+		ClosedByLogin:       &seedClosedBy,
+		AutoMergeObservedAt: &latchedAt,
+	}
+	if err := store.CreateTaskPR(ctx, seed); err != nil {
+		t.Fatalf("seed CreateTaskPR: %v", err)
+	}
+
+	pr := &PR{
+		Number:    6002,
+		RepoOwner: "kdlbs",
+		RepoName:  "kandev",
+		HTMLURL:   "https://github.com/kdlbs/kandev/pull/6002",
+		Title:     "latch seed (restored)",
+		State:     "open",
+	}
+	restored, err := store.RestoreTaskPR(ctx, "task-latch-restore", "", &PRStatus{PR: pr})
+	if err != nil {
+		t.Fatalf("RestoreTaskPR: %v", err)
+	}
+	if restored == nil {
+		t.Fatalf("RestoreTaskPR: row not found")
+	}
+	assertOutcomeColumnsRoundTrip(t, "RestoreTaskPR (preserved)",
+		restored, seedIsDraft, seedChangedFiles, seedMergedBy, seedClosedBy)
+	if restored.AutoMergeObservedAt == nil || !restored.AutoMergeObservedAt.Equal(latchedAt) {
+		t.Fatalf("RestoreTaskPR: AutoMergeObservedAt = %v, want preserved %v", restored, latchedAt)
+	}
+}
+
+// TestReplaceTaskPR_ConcurrentWritersBothLand is the AC-43a regression:
+// unlike TestUpdateTaskPR_AutoMergeLatchIsAtomicUnderConcurrentStaleWrites
+// (which must manufacture a stale-read scenario because UpdateTaskPR has no
+// injectable pause point), ReplaceTaskPR's read-resolve-write lives entirely
+// inside one BeginTxx/Commit and the store's writer connection pool is
+// SetMaxOpenConns(1) (see newTestStore), so two real concurrent goroutines
+// calling ReplaceTaskPR are genuinely, deterministically serialized by the
+// pool rather than interleaving — which is exactly the guarantee AC-43a
+// requires. This launches two goroutines that each populate a DIFFERENT
+// outcome field (one resolves merged_by_login, the other resolves
+// closed_by_login, each preserving the field it doesn't touch) and asserts
+// the final row carries BOTH contributions. If the outgoing read were taken
+// outside the transaction (or against a stale snapshot) — the defect class
+// the spec calls "invisible in a serial test" — whichever goroutine
+// committed last would silently revert the other's write, and this would be
+// flaky/order-dependent instead of reliably showing both fields set.
+func TestReplaceTaskPR_ConcurrentWritersBothLand(t *testing.T) {
+	store := newTestStore(t)
+	ctx := context.Background()
+	if _, err := store.db.Exec(`INSERT INTO tasks (id, workspace_id) VALUES ('task-race-replace', 'ws-race')`); err != nil {
+		t.Fatalf("seed task: %v", err)
+	}
+	seed := &TaskPR{
+		WorkspaceID: "ws-race", TaskID: "task-race-replace", Owner: "kdlbs", Repo: "kandev",
+		PRNumber: 6003, PRURL: "https://github.com/kdlbs/kandev/pull/6003", PRTitle: "race seed",
+		State: "open", CreatedAt: time.Now().UTC(),
+	}
+	if err := store.CreateTaskPR(ctx, seed); err != nil {
+		t.Fatalf("seed CreateTaskPR: %v", err)
+	}
+
+	mergerStatus := &PRStatus{
+		PR:                     &PR{MergedByLogin: "carlosflorencio"},
+		OutcomeFieldsPopulated: true,
+	}
+	closerStatus := &PRStatus{
+		ClosedByLogin:               "nova28",
+		ClosureAttributionPopulated: true,
+	}
+	replaceWith := func(title string) *TaskPR {
+		return &TaskPR{
+			WorkspaceID: "ws-race", TaskID: "task-race-replace", Owner: "kdlbs", Repo: "kandev",
+			PRNumber: 6003, PRURL: "https://github.com/kdlbs/kandev/pull/6003", PRTitle: title,
+			State: "closed", CreatedAt: time.Now().UTC(),
+		}
+	}
+
+	var wg sync.WaitGroup
+	errs := make(chan error, 2)
+	wg.Add(2)
+	go func() {
+		defer wg.Done()
+		if _, err := store.ReplaceTaskPR(ctx, replaceWith("replaced by merger"), mergerStatus); err != nil {
+			errs <- err
+		}
+	}()
+	go func() {
+		defer wg.Done()
+		if _, err := store.ReplaceTaskPR(ctx, replaceWith("replaced by closer"), closerStatus); err != nil {
+			errs <- err
+		}
+	}()
+	wg.Wait()
+	close(errs)
+	for err := range errs {
+		t.Fatalf("ReplaceTaskPR: %v", err)
+	}
+
+	got, err := store.GetTaskPRByRepoAndNumber(ctx, "task-race-replace", "", 6003)
+	if err != nil || got == nil {
+		t.Fatalf("GetTaskPRByRepoAndNumber: err=%v row=%v", err, got)
+	}
+	if got.MergedByLogin == nil || *got.MergedByLogin != "carlosflorencio" {
+		t.Errorf("MergedByLogin = %v, want %q — the merger goroutine's write was lost to a non-serialized read",
+			got.MergedByLogin, "carlosflorencio")
+	}
+	if got.ClosedByLogin == nil || *got.ClosedByLogin != "nova28" {
+		t.Errorf("ClosedByLogin = %v, want %q — the closer goroutine's write was lost to a non-serialized read",
+			got.ClosedByLogin, "nova28")
+	}
+}
+
+// TestRestoreTaskPR_ConcurrentWritersBothLand mirrors
+// TestReplaceTaskPR_ConcurrentWritersBothLand for RestoreTaskPR's own
+// in-transaction read-resolve-write (AC-43a).
+func TestRestoreTaskPR_ConcurrentWritersBothLand(t *testing.T) {
+	store := newTestStore(t)
+	ctx := context.Background()
+	if _, err := store.db.Exec(`INSERT INTO tasks (id, workspace_id) VALUES ('task-race-restore', 'ws-race')`); err != nil {
+		t.Fatalf("seed task: %v", err)
+	}
+	seed := &TaskPR{
+		WorkspaceID: "ws-race", TaskID: "task-race-restore", Owner: "kdlbs", Repo: "kandev",
+		PRNumber: 6004, PRURL: "https://github.com/kdlbs/kandev/pull/6004", PRTitle: "race seed",
+		State: "open", CreatedAt: time.Now().UTC(),
+	}
+	if err := store.CreateTaskPR(ctx, seed); err != nil {
+		t.Fatalf("seed CreateTaskPR: %v", err)
+	}
+
+	basePR := PR{
+		Number: 6004, RepoOwner: "kdlbs", RepoName: "kandev",
+		HTMLURL: "https://github.com/kdlbs/kandev/pull/6004", Title: "restored", State: "open",
+	}
+	mergerPR := basePR
+	mergerPR.MergedByLogin = "carlosflorencio"
+	mergerStatus := &PRStatus{PR: &mergerPR, OutcomeFieldsPopulated: true}
+	closerPR := basePR
+	closerStatus := &PRStatus{PR: &closerPR, ClosedByLogin: "nova28", ClosureAttributionPopulated: true}
+
+	var wg sync.WaitGroup
+	errs := make(chan error, 2)
+	wg.Add(2)
+	go func() {
+		defer wg.Done()
+		if _, err := store.RestoreTaskPR(ctx, "task-race-restore", "", mergerStatus); err != nil {
+			errs <- err
+		}
+	}()
+	go func() {
+		defer wg.Done()
+		if _, err := store.RestoreTaskPR(ctx, "task-race-restore", "", closerStatus); err != nil {
+			errs <- err
+		}
+	}()
+	wg.Wait()
+	close(errs)
+	for err := range errs {
+		t.Fatalf("RestoreTaskPR: %v", err)
+	}
+
+	got, err := store.GetTaskPRByRepoAndNumber(ctx, "task-race-restore", "", 6004)
+	if err != nil || got == nil {
+		t.Fatalf("GetTaskPRByRepoAndNumber: err=%v row=%v", err, got)
+	}
+	if got.MergedByLogin == nil || *got.MergedByLogin != "carlosflorencio" {
+		t.Errorf("MergedByLogin = %v, want %q — the merger goroutine's write was lost to a non-serialized read",
+			got.MergedByLogin, "carlosflorencio")
+	}
+	if got.ClosedByLogin == nil || *got.ClosedByLogin != "nova28" {
+		t.Errorf("ClosedByLogin = %v, want %q — the closer goroutine's write was lost to a non-serialized read",
+			got.ClosedByLogin, "nova28")
+	}
+}

apps/backend/internal/persistence/meta.go

@@ -58,6 +58,16 @@ func ensureMetaTable(db *sqlx.DB) error {
 	return nil
 }
 
+// EnsureMetaTable is the exported form of ensureMetaTable, for repository
+// packages outside internal/persistence that write to kandev_meta before
+// persistence.Provide is guaranteed to have run (e.g. isolated unit tests
+// that construct a repository store directly). In production,
+// persistence.Provide already creates the table before any repository
+// schema init runs, so this is a cheap, idempotent no-op there.
+func EnsureMetaTable(db *sqlx.DB) error {
+	return ensureMetaTable(db)
+}
+
 // readKey returns the value for key, or "" when the key is absent.
 func readKey(db *sqlx.DB, key string) (string, error) {
 	var value string
@@ -76,6 +86,33 @@ func writeKey(db *sqlx.DB, key, value string) error {
 	return writeMetaKey(db, db.Rebind(metaKeyUpsert), key, value)
 }
 
+// ReadMetaKey returns the value for key, or "" when the key is absent. It is
+// the exported form of readKey, for repository packages outside
+// internal/persistence (e.g. internal/github) that need to read a
+// kandev_meta value such as an activation instant.
+func ReadMetaKey(db *sqlx.DB, key string) (string, error) {
+	return readKey(db, key)
+}
+
+// WriteMetaKeyIfAbsent inserts key=value only when key is not already
+// present in kandev_meta, and reports whether this call performed the
+// insert. Unlike writeKey (an upsert), this never overwrites an existing
+// value, and the INSERT ... ON CONFLICT DO NOTHING form makes the
+// write-once guarantee race-free without a read-modify-write.
+func WriteMetaKeyIfAbsent(db *sqlx.DB, key, value string) (bool, error) {
+	result, err := db.Exec(db.Rebind(
+		`INSERT INTO kandev_meta (key, value) VALUES (?, ?) ON CONFLICT(key) DO NOTHING`,
+	), key, value)
+	if err != nil {
+		return false, fmt.Errorf("write meta key %q if absent: %w", key, err)
+	}
+	affected, err := result.RowsAffected()
+	if err != nil {
+		return false, fmt.Errorf("write meta key %q if absent: rows affected: %w", key, err)
+	}
+	return affected > 0, nil
+}
+
 // WriteVersion records currentVersion as the binary version that last
 // successfully completed boot against this DB. Call this only after every
 // repository initSchema has succeeded.

apps/backend/internal/persistence/meta_test.go

@@ -240,6 +240,66 @@ func TestStableAndNightlyLatestVersionsAreIsolated(t *testing.T) {
 	}
 }
 
+// TestReadMetaKey_EmptyOnAbsentKey verifies the exported reader returns ""
+// and no error for a key that was never written.
+func TestReadMetaKey_EmptyOnAbsentKey(t *testing.T) {
+	db := memSQLiteDB(t)
+	if err := ensureMetaTable(db); err != nil {
+		t.Fatalf("ensureMetaTable: %v", err)
+	}
+
+	val, err := ReadMetaKey(db, "github_task_pr_outcome_activated_at")
+	if err != nil {
+		t.Fatalf("ReadMetaKey: %v", err)
+	}
+	if val != "" {
+		t.Errorf("expected empty value for absent key, got %q", val)
+	}
+}
+
+// TestWriteMetaKeyIfAbsent_WriteOnceSemantics verifies that the first call
+// inserts and reports true, and a second call with a different value leaves
+// the original value in place and reports false. This is the race-free
+// write-once primitive the outcome-attribution activation instant depends on.
+func TestWriteMetaKeyIfAbsent_WriteOnceSemantics(t *testing.T) {
+	db := memSQLiteDB(t)
+	if err := ensureMetaTable(db); err != nil {
+		t.Fatalf("ensureMetaTable: %v", err)
+	}
+
+	inserted, err := WriteMetaKeyIfAbsent(db, "github_task_pr_outcome_activated_at", "2026-08-13T00:00:00Z")
+	if err != nil {
+		t.Fatalf("WriteMetaKeyIfAbsent first call: %v", err)
+	}
+	if !inserted {
+		t.Fatal("expected first WriteMetaKeyIfAbsent call to report inserted=true")
+	}
+
+	val, err := ReadMetaKey(db, "github_task_pr_outcome_activated_at")
+	if err != nil {
+		t.Fatalf("ReadMetaKey: %v", err)
+	}
+	if val != "2026-08-13T00:00:00Z" {
+		t.Fatalf("got %q, want the first-written value", val)
+	}
+
+	inserted2, err := WriteMetaKeyIfAbsent(db, "github_task_pr_outcome_activated_at", "2027-01-01T00:00:00Z")
+	if err != nil {
+		t.Fatalf("WriteMetaKeyIfAbsent second call: %v", err)
+	}
+	if inserted2 {
+		t.Fatal("expected second WriteMetaKeyIfAbsent call to report inserted=false")
+	}
+
+	val2, err := ReadMetaKey(db, "github_task_pr_outcome_activated_at")
+	if err != nil {
+		t.Fatalf("ReadMetaKey after second call: %v", err)
+	}
+	if val2 != "2026-08-13T00:00:00Z" {
+		t.Fatalf("value changed on second call: got %q, want the first-written value unchanged", val2)
+	}
+}
+
 // TestShouldBackup verifies the backup decision logic.
 func TestShouldBackup(t *testing.T) {
 	tests := []struct {

apps/web/lib/types/github.ts

@@ -254,6 +254,28 @@ export type TaskPR = {
   closed_at: string | null;
   last_synced_at: string | null;
   updated_at: string;
+  // The five PR-outcome-attribution fields below are always present on a
+  // real API/WS payload (the backend sends every key, never omits one) — the
+  // `?:` here follows this file's existing convention for nullable fields
+  // added after the type's original shape (e.g. required_reviews?) so
+  // hand-written test fixtures aren't forced to enumerate all five. Treat
+  // `undefined` the same as `null` ("nobody looked" / "never observed");
+  // never treat `null` as "unknown" or vice versa.
+  /** Never observed by a populating sync when null. */
+  is_draft?: boolean | null;
+  /** Never observed when null, distinct from 0 (a real "no files changed" observation). */
+  changed_files?: number | null;
+  /** Not merged, or merged but never observed by a populating sync, when null. */
+  merged_by_login?: string | null;
+  /** Not closed, or closure never observed by the GraphQL path specifically
+   *  (closed_by is absent from the REST pulls endpoint and the gh CLI's PR
+   *  field set), when null. A PR closed only through those paths keeps this
+   *  null permanently. */
+  closed_by_login?: string | null;
+  /** A latched observation, never a merge cause: GitHub clears auto_merge
+   *  once it fires, so this can only mean "armed at some instant while
+   *  Kandev was watching." */
+  auto_merge_observed_at?: string | null;
 };
 
 /** Workspace-scoped websocket payload emitted when a task PR association is detached. */

docs/plans/pr-outcome-attribution/plan.md

@@ -0,0 +1,72 @@
+---
+spec: docs/specs/pr-outcome-attribution/spec.md
+created: 2026-08-13
+status: implemented
+---
+
+# Implementation Plan: Pull request outcome attribution
+
+## Overview
+
+Persist five facts that GitHub already provides during the existing pull
+request syncs. The change is backend-first: add nullable storage and activation
+metadata, source the fields from each client, reconcile them in the existing
+sync writer, and expose the values in the shared frontend type. No core UI or
+mutation endpoint is part of this feature.
+
+## Backend
+
+### Schema and activation
+
+`apps/backend/internal/persistence/meta.go` provides exported read and
+write-once helpers for `kandev_meta`.
+
+`apps/backend/internal/github/store.go`:
+
+- Declares `is_draft`, `changed_files`, `merged_by_login`,
+  `closed_by_login`, and `auto_merge_observed_at` as nullable columns.
+- Adds missing columns with the fail-loud migration path.
+- Records `github_task_pr_outcome_activated_at` once after all columns exist.
+- Includes the columns in every explicit projection and row writer.
+- Carries the columns through the legacy multi-repository table rebuild.
+
+`apps/backend/internal/github/models.go` keeps nullable pointers on `TaskPR`
+and presence flags on the upstream `PR` and `PRStatus` models.
+
+### Upstream field sourcing
+
+- `graphql.go` requests the shared fields and the latest closed-event actor.
+- `gh_client.go` requests the fields available from the gh CLI.
+- `pat_client.go` decodes the fields available from the REST pull-request API.
+- `client_helpers.go` marks full single-PR reads as populated.
+- No-op and partial reads leave populated flags false.
+
+### Sync writer
+
+`service_pr_watch.go` applies populated observations, preserves stored values
+for unpopulated reads, and compares nullable values before publishing an event.
+The auto-merge timestamp is latched with a SQL `COALESCE` guard.
+
+`ReplaceTaskPR` and `RestoreTaskPR` resolve outcome fields inside their write
+transactions. The sync path re-reads the committed row before publishing.
+Unwatched terminal reconciliation uses the same rules.
+
+## Frontend
+
+`apps/web/lib/types/github.ts` exposes the five nullable fields on `TaskPR`.
+No component, store slice, API mutation, or translation is added because the
+core product has no screen for these observations.
+
+## Tests
+
+- Migration replay and activation: `store_task_pr_outcome_migration_test.go`.
+- Column-list and rebuild coverage: `store_taskpr_schema_drift_test.go` and
+  `store_task_pr_detach_test.go`.
+- GraphQL, gh, REST, and presence flags: the corresponding client tests.
+- Sync, latch, unwatched reconciliation, and writer health:
+  `service_pr_outcome_sync_test.go`, `service_pr_unwatched_test.go`,
+  `service_pr_writer_health_test.go`, and detach tests.
+- Meta helper semantics: `internal/persistence/meta_test.go`.
+
+The implementation is complete. CI provides backend and frontend verification;
+there is no browser or mobile surface to exercise.

docs/plans/pr-outcome-attribution/task-01-schema-and-activation.md

@@ -0,0 +1,31 @@
+---
+id: "01-schema-and-activation"
+title: "Add outcome storage and activation"
+status: done
+wave: 1
+plan: "plan.md"
+spec: "../../specs/pr-outcome-attribution/spec.md"
+---
+
+# Task 01: Schema and activation
+
+Add the five nullable outcome columns to `github_task_prs`, expose them on
+`TaskPR`, and add the one-time activation marker in `kandev_meta`.
+
+## Implementation
+
+- Add `ReadMetaKey` and `WriteMetaKeyIfAbsent` in
+  `apps/backend/internal/persistence/meta.go`.
+- Add fail-loud, idempotent column migration and activation handling in
+  `apps/backend/internal/github/store.go`.
+- Include all five columns in the table DDL, explicit projections, create,
+  replace, restore, update, and legacy rebuild statements.
+- Keep all fields nullable with no default and perform no row backfill.
+- Add nullable pointers and writer-health documentation to `models.go`.
+
+## Verification
+
+`store_task_pr_outcome_migration_test.go`, `store_taskpr_schema_drift_test.go`,
+`store_task_pr_detach_test.go`, and `internal/persistence/meta_test.go` cover
+fresh installs, replay, activation write-once behavior, fail-loud migration,
+column-list drift, and legacy rebuild preservation.

docs/plans/pr-outcome-attribution/task-02-upstream-field-sourcing.md

@@ -0,0 +1,30 @@
+---
+id: "02-upstream-field-sourcing"
+title: "Source outcome fields from GitHub"
+status: done
+wave: 2
+depends_on: ["01-schema-and-activation"]
+plan: "plan.md"
+spec: "../../specs/pr-outcome-attribution/spec.md"
+---
+
+# Task 02: Upstream field sourcing
+
+Read the five upstream facts from the existing GraphQL, gh CLI, and REST
+pull-request paths. Mark a result as populated only when the path actually
+observed the relevant fields.
+
+## Implementation
+
+- Extend the shared GraphQL field block with draft, changed files, merger,
+  auto-merge, and the latest closed-event actor.
+- Extend gh CLI and REST decoders with the fields those APIs expose.
+- Keep closure attribution GraphQL-only because the other pull-request APIs do
+  not expose a closing actor.
+- Add field-level presence flags for optional draft and changed-file values.
+- Keep no-op and partial paths unpopulated.
+
+## Verification
+
+Client and helper tests cover field selection, nullable decoding, explicit
+`false` and `0`, closed-event ordering, and populated flags.

docs/plans/pr-outcome-attribution/task-03-sync-writer.md

@@ -0,0 +1,32 @@
+---
+id: "03-sync-writer"
+title: "Reconcile outcome fields in syncs"
+status: done
+wave: 3
+depends_on: ["02-upstream-field-sourcing"]
+plan: "plan.md"
+spec: "../../specs/pr-outcome-attribution/spec.md"
+---
+
+# Task 03: Sync writer
+
+Persist populated observations through the existing sync service and preserve
+stored values when a read did not observe the outcome fields.
+
+## Implementation
+
+- Reconcile all five fields in `service_pr_watch.go` with nil-safe comparisons.
+- Preserve values for unpopulated reads and write `NULL` for observed absence
+  where the upstream contract defines absence as a cleared value.
+- Latch `auto_merge_observed_at` with a SQL-level `COALESCE` guard.
+- Resolve replace and restore values inside their own write transactions.
+- Re-read the committed row before publishing a task-PR update event.
+- Apply the same logic to unwatched terminal reconciliation.
+- Increment the populated and unpopulated expvar counters at the observation
+  decision point.
+
+## Verification
+
+Sync, unwatched reconciliation, writer-health, detach, and metrics tests cover
+presence flags, null preservation, latch behavior, event publication, and
+concurrent replace/restore writes.

docs/specs/INDEX.md

@@ -157,6 +157,7 @@ Per-workspace credentials and triage triggers for external services.
 | [gitlab-mr-task-list-badges](gitlab-mr-task-list-badges/spec.md) | draft |
 | [gitlab-workflow-sync](gitlab-workflow-sync/spec.md) | shipped |
 | [jira-status-filter](jira-status-filter/spec.md) | shipped |
+| [pr-outcome-attribution](pr-outcome-attribution/spec.md) | shipped |
 | [enable-disable-toggle](integrations/enable-disable-toggle.md) | shipped |
 
 ## workspaces/ — workspace lifecycle

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

@@ -0,0 +1,117 @@
+---
+title: Pull request outcome attribution
+status: shipped
+created: 2026-08-12
+owner: kandev
+---
+
+# Pull request outcome attribution
+
+## Why
+
+Kandev already synchronizes pull request state. The sync contains useful facts
+that were not persisted: draft state, changed file count, the merger, the
+latest closing actor, and whether auto-merge was observed. Persisting these
+facts lets integrations and reports use the upstream record without guessing.
+
+## Scope and boundary
+
+The feature persists exactly five nullable fields on `github_task_prs`:
+
+| Field | Meaning |
+| --- | --- |
+| `is_draft` | The upstream draft value. `NULL` means it was not observed. |
+| `changed_files` | The upstream changed-file count. `0` is a real value. |
+| `merged_by_login` | The upstream merger login, when present. |
+| `closed_by_login` | The latest GraphQL closed-event actor, when present. |
+| `auto_merge_observed_at` | The first time Kandev observed auto-merge armed. |
+
+These are observations, not inferred causes. `auto_merge_observed_at` does not
+mean that auto-merge caused the merge. A missing closing actor remains `NULL`.
+
+Core does not store human-authored closure reasons or replacement links. A
+plugin can own that workflow, including its taxonomy, storage, and UI.
+
+The feature has no core UI, mutation endpoint, or translation keys. The web
+package receives the five fields in its `TaskPR` type for future integrations,
+but no core component consumes them.
+
+## Persistence
+
+Fresh databases declare the five columns in the GitHub task-PR table. Existing
+databases add missing columns with fail-loud, idempotent `ALTER TABLE` steps.
+No migration updates or backfills a task-PR row. All five columns are nullable,
+have no default, and are added to every explicit read and write column list.
+
+Schema initialization writes the current UTC instant once to
+`kandev_meta.github_task_pr_outcome_activated_at`. The write uses a set-if-absent
+operation, so concurrent startup cannot replace the first value. A migration
+or activation failure stops startup.
+
+The legacy multi-repository table rebuild copies all five columns in both its
+new-table DDL and its `INSERT ... SELECT` list.
+
+## Upstream acquisition
+
+- The shared GraphQL field block requests draft state, changed files, merger,
+  auto-merge, and the most recent `CLOSED_EVENT` actor.
+- The gh CLI and REST pull-request clients request the fields they expose.
+  Neither path claims a closing actor because that field is not available from
+  those pull-request responses.
+- A full REST, gh, or GraphQL read marks the outcome group as populated. A
+  partial or no-op read leaves it unpopulated.
+- Draft and changed-file values have field-level presence flags. This preserves
+  the difference between an observed `false` or `0` and an absent value.
+
+## Sync writer
+
+The normal sync writer applies populated observations. An unpopulated sync
+preserves stored values, including `NULL`. A populated response with no merger
+clears `merged_by_login` to `NULL`; it does not store an empty string. Closure
+attribution follows its own populated flag.
+
+`auto_merge_observed_at` is a latch. The first populated observation with
+auto-merge armed sets it. Later observations cannot clear or replace it. The
+SQL update also uses `COALESCE` so concurrent syncs cannot overwrite the first
+timestamp.
+
+Replace and restore operations resolve the five fields inside their write
+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.
+
+## Acceptance criteria
+
+- **AC-01 to AC-03:** schema creation and replay add nullable columns without
+  defaults, do not silently ignore migration errors, and remain idempotent.
+- **AC-04 to AC-06:** migration does not backfill rows; activation is recorded
+  once, including on fresh installs, and later boots keep the original instant.
+- **AC-07 and AC-07a:** every explicit projection, row writer, and legacy table
+  rebuild carries all five columns.
+- **AC-08 and AC-08a:** GraphQL requests the shared fields and selects the most
+  recent closed event.
+- **AC-09 to AC-11:** REST, gh, GraphQL, and no-op paths report only the fields
+  they actually observed through populated flags.
+- **AC-12 and AC-12a:** populated values preserve real zero and false values,
+  and absent optional fields remain `NULL`.
+- **AC-13 to AC-15a:** unpopulated values are preserved; closure attribution is
+  written only from a valid GraphQL actor and follows the latest event.
+- **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-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.
+- **AC-36 to AC-39:** activation-scoped writer-health checks can distinguish a
+  missing observation from a legitimate pre-activation or partial-read `NULL`.
+- **AC-38:** the populated and unpopulated sync counters remain available in
+  dev-mode expvar output.
+- **AC-43, AC-43a, and AC-43p:** replace and restore resolve and preserve
+  outcome fields inside their own transaction using explicit observation flags.
+
+## Verification
+
+Backend unit and persistence tests cover migration replay, source decoding,
+presence flags, writer behavior, concurrency, event publication, and table
+rebuilds. The frontend change is type-only. No browser or mobile test is needed
+because this feature has no user-visible core surface.