Fix PR-link reconcile wrong-PR attribution (#520)

[dhilgaertner]

Closes #520

Problem

Reconcile attached the wrong PR to sessions whose worktree branch carries a repo-name prefix that the PR head branch drops — e.g. worktree feature/max-monorepo-maxx-7035-citations-chain-of-custody vs PR head feature/maxx-7035-citations-chain-of-custody. On the max-monorepo/MAXX sessions, 5 of 12 showed the wrong PR, including MAXX-6854 which has no PR yet was shown #171.

The key-matching + persistence machinery the ticket "proposes" already existed (added by #463/#464); derived links are persisted. The bug persisted because of three remaining gaps, all fixed here.

Fix

1. Body-match phantom PRs (GitHubCodeBackend.parseKeyPRMatches) — a PR was matched when the ticket key appeared in its body, so a PR merely mentioning a related ticket (e.g. "related to MAXX-6854") attached to that unrelated session. Matching now requires the key in the PR title or head branch only.
2. Branch-derived ticket key — Validation.ticketKey(fromBranch:) extracts MAXX-7035 from the worktree branch. Used as a fallback for task-only (Jira/Corveil) sessions, gated so a lowercased GitHub issue branch (feature/acme-api-197-fix) can't masquerade as a key.
3. Cross-session de-dup — dedupeContestedPRs drops a PR claimed by sessions with differing work-item identities from all of them. Never guess; a no-PR session gets none.

Tests

• ValidationTicketKeyTests — branch-key extraction (incl. prefix-drop and keyless branches).
• BackendsTests — body-only match rejected; head-only match kept.
• IssueTrackerReconcileTests — the exact PR-link reconcile attaches the wrong PR when worktree branch ≠ PR head branch (and can't be corrected) #520 max-monorepo scenario end-to-end (MAXX-7035 → TicketCard: make issue and PR links clickable using LinkChip (match session header) #171, MAXX-6854 → none, no PR on two sessions) + focused de-dup cases.

All suites green: CrowCore 277, CrowProvider 52 + XCTest, root 234.

🤖 Generated with Claude Code

[dgershman]

Code & Security Review

Critical Issues

None.

Security Review

Strengths:

• No new attack surface — all changes are internal logic / post-filtering on already-fetched data.
• parseKeyPRMatches post-filter tightens (not loosens) matching; rejects body-only mentions that previously cross-attributed.
• Branch-derived key extraction (Validation.ticketKey(fromBranch:)) validates through parseJiraKey's ^[A-Z][A-Z0-9]+$ + numeric checks; output is [A-Z0-9-] only, safe to embed in gh argv (and is, via KeyCandidate.key → --search "<key> in:title,body").
• dedupeContestedPRs is a conservative "drop on conflict" rather than "guess" — exactly the right call for a data-attribution bug.

Concerns: None.

Code Quality

Strengths:

• Three discrete fixes, each minimal, each with focused test coverage.
• End-to-end regression test (branchDerivedKeyResolvesCorrectPRAndNoPRSessionGetsNone) exercises the precise #520 scenario (MAXX-7035 → #171, MAXX-6854 → none).
• The branch-fallback is correctly gated to task-only providers (Jira/Corveil) with a clear comment explaining why (feature/acme-api-197-fix would otherwise yield bogus API-197).
• identityBySession overwriting (key > branch) makes key the canonical identity when both are present — right precedence.
• Excellent docstrings on the new pure helpers (ticketKey(fromBranch:), dedupeContestedPRs, parseKeyPRMatches) that explain why, not just what.

Consider (green — non-blocking):

• parseKeyPRMatches (GitHubCodeBackend.swift:245) uses title.contains(needle) || head.contains(needle) — substring, not word-bounded. For prefix-related ticket numbers (e.g., MAXX-6859 vs MAXX-68591), a search for MAXX-6859 could keep a PR whose only references are to MAXX-68591. The dedupeContestedPRs safety net catches the cross-attribution case (the contested PR is dropped from both), but the legitimate MAXX-68591 session would lose its real link as collateral. Practical impact is low because gh's search tokenization typically word-bounds, and the prefix-sharing pattern is rare — a word-boundary regex ((?<![A-Z0-9])<key>(?![0-9])) would close it fully. Not a blocker.
• Legacy wrong-PR links from before this fix won't be proactively repaired (reconcile only attaches to sessions without a .pr link). Out of scope for this PR; worth noting for a follow-up cleanup if users report stale wrong links.

Tests

• ValidationTicketKeyTests (4 tests) — branch-key extraction including prefix-drop, no-prefix, ambiguous-shape, and keyless cases. Verified passing locally.
• BackendsTests.testGitHubCodeBackendFindPRsMatchingKeysParsesAndFilters — covers all four buckets (title+head ✓, head only ✓, body only ✗, no match ✗). Verified passing.
• IssueTrackerReconcileKeyScenarioTests — end-to-end #520 reproduction. Asserts no PR attaches to two distinct sessions.
• IssueTrackerReconcileDedupTests (3 tests) — contested-drop, same-identity-keep, distinct-keep.

Summary Table

Color	Meaning	Verdict effect
Red	Must fix	Request changes
Yellow	Should fix	Request changes
Green	Consider	Approve allowed

Recommendation: Approve — driven by [0 Red, 0 Yellow, 2 Green] findings. The fix is precisely scoped to #520, defensively layered (three independent gates), and well-tested. The substring-matching consideration is theoretical and bounded by the dedupe safety net.

---

🐦‍⬛ Reviewed by Crow via Claude Code