Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Backfill of merged PRs since the 0.1.0 release, grouped by theme.
- #220 — Filtering for the tickets list.
- #226 — Per-section select all and icon-only cancel button in selection mode.
- #231 — Quick action buttons on the session detail header.
- #520 — PR-link reconcile no longer attaches the wrong PR when the worktree branch carries a repo-name prefix the PR head drops (`feature/max-monorepo-maxx-7035-…` vs `feature/maxx-7035-…`). The ticket key is derived from the worktree branch, key matching ignores body-only mentions, and a PR can attach to at most one session — a session whose ticket has no PR now gets none.

### Terminal Runtime

Expand Down
14 changes: 14 additions & 0 deletions Packages/CrowCore/Sources/CrowCore/Validation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,18 @@ public enum Validation {
public static func jiraKey(from spec: String) -> String? {
parseJiraKey(spec)?.key
}

/// Extract a ticket key (e.g. `MAXX-7035`) embedded in a git branch name such
/// as `feature/max-monorepo-maxx-7035-citations`. Returns the first
/// LETTERS-DIGITS token that validates as a Jira-style key, uppercased.
///
/// The character class excludes `-`, so on `…-max-monorepo-maxx-7035-…` the
/// first complete LETTERS-DIGITS match is `maxx-7035` → `MAXX-7035`. Numeric
/// GitHub-issue branches like `feature/acme-api-197-fix` yield `api-197`,
/// which fails `parseJiraKey`'s uppercase-project rule → `nil`.
public static func ticketKey(fromBranch branch: String) -> String? {
guard let r = branch.range(of: "[A-Za-z][A-Za-z0-9]+-[0-9]+", options: .regularExpression)
else { return nil }
return parseJiraKey(String(branch[r]).uppercased())?.key
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import Foundation
import Testing
@testable import CrowCore

// MARK: - Validation.ticketKey(fromBranch:)
//
// Regression coverage for #520: worktree branches embed a repo-name prefix
// (`feature/max-monorepo-maxx-7035-…`) that the PR head branch drops. Reconcile
// derives the ticket key from the branch to recover the right PR.

@Test func ticketKeyExtractsJiraKeyFromPrefixedBranch() {
#expect(
Validation.ticketKey(fromBranch: "feature/max-monorepo-maxx-7035-citations-chain-of-custody")
== "MAXX-7035"
)
}

@Test func ticketKeyHandlesBranchWithoutRepoPrefix() {
#expect(Validation.ticketKey(fromBranch: "feature/maxx-7035-citations") == "MAXX-7035")
}

@Test func ticketKeyExtractsAnyJiraShapedTokenAfterUppercasing() {
// A lowercased branch can't tell a real Jira project ("maxx") from an
// ordinary segment ("api"), so this pure extractor returns the first
// Jira-*shaped* token — `api-197` → `API-197`. Guarding against false keys
// on GitHub/GitLab issue branches is the caller's job (it gates the branch
// fallback to task-only trackers), not this function's.
#expect(Validation.ticketKey(fromBranch: "feature/acme-api-197-fix-tab-url-hash") == "API-197")
}

@Test func ticketKeyIsNilWhenNoJiraShapedToken() {
// No LETTERS-DIGITS token at all.
#expect(Validation.ticketKey(fromBranch: "feature/cleanup-readme") == nil)
#expect(Validation.ticketKey(fromBranch: "feature/197-only-number") == nil)
#expect(Validation.ticketKey(fromBranch: "") == nil)
}
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,11 @@ public struct GitHubCodeBackend: CodeBackend {
}

/// Parse `gh pr list --json …` output into `KeyPRMatch`es. Post-filters to
/// PRs where the key actually appears (case-insensitively) in the title,
/// body, or head branch — `gh`'s search can be fuzzy, and we only want PRs
/// that genuinely reference the ticket.
/// PRs where the key actually appears (case-insensitively) in the **title or
/// head branch** — `gh`'s search can be fuzzy, and a key mentioned only in a
/// PR's *body* (e.g. "related to MAXX-6854") belongs to a different ticket.
/// Matching on body attached phantom PRs to sessions whose ticket had none
/// (#520), so it is deliberately excluded.
static func parseKeyPRMatches(_ output: String, candidate: KeyCandidate) -> [KeyPRMatch] {
guard let data = output.data(using: .utf8),
let arr = try? JSONSerialization.jsonObject(with: data) as? [[String: Any]] else { return [] }
Expand All @@ -239,9 +241,8 @@ public struct GitHubCodeBackend: CodeBackend {
let url = node["url"] as? String,
let state = node["state"] as? String else { continue }
let title = (node["title"] as? String ?? "").lowercased()
let body = (node["body"] as? String ?? "").lowercased()
let head = (node["headRefName"] as? String ?? "").lowercased()
guard title.contains(needle) || body.contains(needle) || head.contains(needle) else { continue }
guard title.contains(needle) || head.contains(needle) else { continue }
let updatedAt = (node["updatedAt"] as? String).flatMap { dateFmt.date(from: $0) }
matches.append(KeyPRMatch(
candidate: candidate, number: number, url: url, state: state, updatedAt: updatedAt
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -442,11 +442,17 @@ final class BackendsTests: XCTestCase {

func testGitHubCodeBackendFindPRsMatchingKeysParsesAndFilters() async throws {
let fake = FakeShellRunner()
// Two PRs returned by gh search: one references MAXX-6859 in its title,
// one is a fuzzy match that mentions it nowhere — only the first counts.
// Four PRs returned by gh search:
// #52 — key in title AND head → kept
// #53 — key in head only (title/body unrelated) → kept
// #41 — key in body ONLY → rejected (#520: a body mention belongs to a
// different ticket; matching it attached phantom PRs)
// #40 — key nowhere → rejected
let json = """
[
{"number":52,"url":"https://github.com/a/b/pull/52","state":"OPEN","updatedAt":"2026-01-02T00:00:00Z","title":"feat: thing. MAXX-6859","headRefName":"feature/maxx-6859-thing","body":"closes MAXX-6859"},
{"number":53,"url":"https://github.com/a/b/pull/53","state":"OPEN","updatedAt":"2026-01-03T00:00:00Z","title":"feat: unrelated title","headRefName":"feature/maxx-6859-other","body":"no mention"},
{"number":41,"url":"https://github.com/a/b/pull/41","state":"MERGED","updatedAt":"2026-01-01T00:00:00Z","title":"different ticket","headRefName":"feature/other-work","body":"related to MAXX-6859"},
{"number":40,"url":"https://github.com/a/b/pull/40","state":"MERGED","updatedAt":"2026-01-01T00:00:00Z","title":"unrelated","headRefName":"feature/other","body":"no key here"}
]
"""
Expand All @@ -455,11 +461,10 @@ final class BackendsTests: XCTestCase {
let matches = try await backend.findPRsMatchingKeys([
KeyCandidate(repoSlug: "a/b", key: "MAXX-6859")
])
XCTAssertEqual(matches.count, 1)
XCTAssertEqual(matches[0].number, 52)
XCTAssertEqual(matches[0].state, "OPEN")
XCTAssertEqual(matches[0].candidate.key, "MAXX-6859")
// Command shape: gh pr list --search "<key> in:title,body".
XCTAssertEqual(Set(matches.map(\.number)), [52, 53])
XCTAssertTrue(matches.allSatisfy { $0.candidate.key == "MAXX-6859" })
// Command shape: gh pr list --search "<key> in:title,body" (broad recall;
// results are post-filtered to title/head only).
let args = fake.calls[0].args
XCTAssertEqual(Array(args.prefix(3)), ["gh", "pr", "list"])
XCTAssertTrue(args.contains("--search"))
Expand Down
52 changes: 46 additions & 6 deletions Sources/Crow/App/IssueTracker.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,26 @@ final class IssueTracker {
return picks
}

/// Enforce that a single PR attaches to at most one work item. Groups the
/// final per-session picks by PR URL; if a URL is claimed by sessions with
/// more than one distinct work-item identity (ticket key, else branch), the
/// PR can't be attributed to one of them with confidence, so it is dropped
/// from all of them — never guess (#520). Duplicate sessions sharing one
/// identity (same key/branch) keep the link. Pure — no appState, no I/O.
nonisolated static func dedupeContestedPRs(
_ picks: [ReconcileBranchMatch],
identityBySession: [UUID: String]
) -> [ReconcileBranchMatch] {
let byURL = Dictionary(grouping: picks, by: { $0.url })
var out: [ReconcileBranchMatch] = []
for (_, group) in byURL {
let identities = Set(group.compactMap { identityBySession[$0.sessionID] })
if identities.count > 1 { continue } // contested across tickets → none
out.append(contentsOf: group)
}
return out
}

/// Route a reconcile candidate to a *code* backend. A task-only provider
/// (`.jira`/`.corveil`) has no code surface, so a session tracked by one
/// resolves PRs through its `codeProvider` — mirroring the
Expand Down Expand Up @@ -1130,7 +1150,15 @@ final class IssueTracker {
// session resolve to a single best pick.
matches.append(contentsOf: await fetchPRsByKeyForReconcile(candidates: keyCandidates))

applyReconciledPRLinks(Self.decideReconcileLinks(matches: matches))
// Each session's work-item identity (key preferred, else branch) so the
// de-dup pass can tell a legitimate duplicate-session match from one PR
// being claimed by two different tickets.
var identityBySession: [UUID: String] = [:]
for c in candidates { identityBySession[c.sessionID] = c.branch }
for c in keyCandidates { identityBySession[c.sessionID] = c.key }

let decided = Self.decideReconcileLinks(matches: matches)
applyReconciledPRLinks(Self.dedupeContestedPRs(decided, identityBySession: identityBySession))
}

/// Walk appState and build the set of sessions needing a reconcile pass.
Expand Down Expand Up @@ -1191,14 +1219,26 @@ final class IssueTracker {
let links = appState.links(for: session.id)
guard !links.contains(where: { $0.linkType == .pr }) else { continue }

// Only task-only trackers whose PR branch won't match the worktree.
guard let ticketURL = session.ticketURL,
Validation.isJiraSpec(ticketURL),
let key = Validation.jiraKey(from: ticketURL) else { continue }

let wts = appState.worktrees(for: session.id)
guard let primaryWt = wts.first(where: { $0.isPrimary }) ?? wts.first else { continue }

// Resolve the ticket key: prefer a Jira ticket URL, else derive it
// from the worktree branch (e.g. `feature/max-monorepo-maxx-7035-…`
// → `MAXX-7035`). The branch fallback covers the prefix-drop case
// where the PR head loses the repo prefix the worktree carries (#520).
//
// The branch fallback is gated to task-only trackers (Jira/Corveil):
// a lowercased branch can't distinguish a real Jira project ("maxx")
// from an ordinary word/repo segment ("api"), so a GitHub/GitLab
// issue branch like `feature/acme-api-197-fix` would yield a bogus
// "API-197" key. Those sessions resolve via the branch path instead.
let urlKey = session.ticketURL.flatMap {
Validation.isJiraSpec($0) ? Validation.jiraKey(from: $0) : nil
}
let branchKey = (session.provider?.isTaskOnly == true)
? Validation.ticketKey(fromBranch: primaryWt.branch) : nil
guard let key = urlKey ?? branchKey else { continue }

let info = resolveRepoInfo(worktree: primaryWt)
guard !info.slug.isEmpty else { continue }

Expand Down
108 changes: 108 additions & 0 deletions Tests/CrowTests/IssueTrackerReconcileTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -378,3 +378,111 @@ struct IssueTrackerReconcileKeyFanOutTests {
#expect(fanned[0].number == 1)
}
}

@Suite("IssueTracker reconcile cross-session de-dup")
struct IssueTrackerReconcileDedupTests {

private func match(_ sid: UUID, number: Int, url: String, state: String = "OPEN")
-> IssueTracker.ReconcileBranchMatch {
IssueTracker.ReconcileBranchMatch(
sessionID: sid, number: number, url: url, state: state, updatedAt: nil
)
}

@Test func dropsPRContestedAcrossDistinctTickets() {
// One PR claimed by two sessions with different ticket keys can't be
// attributed to one of them → attach to neither (#520: MAXX-6854 has no
// PR yet was shown #171).
let s7035 = UUID(); let s6854 = UUID()
let url = "https://github.com/rm/max-monorepo/pull/171"
let out = IssueTracker.dedupeContestedPRs(
[match(s7035, number: 171, url: url), match(s6854, number: 171, url: url)],
identityBySession: [s7035: "MAXX-7035", s6854: "MAXX-6854"]
)
#expect(out.isEmpty)
}

@Test func keepsPRSharedByDuplicateSessionsWithSameIdentity() {
// Two sessions on the *same* ticket (a duplicated Jira session) both
// legitimately link the same PR.
let a = UUID(); let b = UUID()
let url = "https://github.com/rm/max-monorepo/pull/52"
let out = IssueTracker.dedupeContestedPRs(
[match(a, number: 52, url: url), match(b, number: 52, url: url)],
identityBySession: [a: "MAXX-6859", b: "MAXX-6859"]
)
#expect(out.count == 2)
#expect(Set(out.map(\.sessionID)) == [a, b])
}

@Test func keepsDistinctPRsForDistinctSessions() {
let s1 = UUID(); let s2 = UUID()
let out = IssueTracker.dedupeContestedPRs(
[
match(s1, number: 131, url: "https://github.com/rm/max-monorepo/pull/131"),
match(s2, number: 132, url: "https://github.com/rm/max-monorepo/pull/132"),
],
identityBySession: [s1: "MAXX-6971", s2: "MAXX-6972"]
)
#expect(out.count == 2)
}
}

// Regression for the exact #520 max-monorepo scenario, exercised end-to-end at
// the pure-function level: worktree branch `feature/{repo}-{ticket}-{slug}` vs
// PR head `feature/{ticket}-{slug}`. The key (e.g. MAXX-7035) is derived from
// the branch; the backend post-filters matches to head/title (no body), so a
// no-PR session resolves to nothing rather than a phantom PR.
@Suite("IssueTracker reconcile key scenario (#520)")
struct IssueTrackerReconcileKeyScenarioTests {

private func keyCandidate(_ sid: UUID, _ key: String) -> IssueTracker.ReconcileKeyCandidate {
IssueTracker.ReconcileKeyCandidate(
sessionID: sid, provider: .github, repoSlug: "rm/max-monorepo", key: key, gitlabHost: nil
)
}

@Test func branchDerivedKeyResolvesCorrectPRAndNoPRSessionGetsNone() {
let s7035 = UUID() // citations-chain-of-custody → PR #171
let s7031 = UUID() // documents-observability-ui → PR #168
let s6854 = UUID() // ut-cascade-dual-write → NO PR exists

// The branch ticket key MAXX-7035 is what `Validation.ticketKey` pulls
// from `feature/max-monorepo-maxx-7035-citations-chain-of-custody`.
#expect(
Validation.ticketKey(fromBranch: "feature/max-monorepo-maxx-7035-citations-chain-of-custody")
== "MAXX-7035"
)

let candidates = [
keyCandidate(s7035, "MAXX-7035"),
keyCandidate(s7031, "MAXX-7031"),
keyCandidate(s6854, "MAXX-6854"),
]

// Backend matches are already post-filtered to head/title. #171's title
// carries MAXX-7035; #168's carries MAXX-7031. MAXX-6854 has no PR — and
// #171 only *mentions* it in its body, which the backend no longer
// matches, so MAXX-6854 produces no match.
let kc7035 = KeyCandidate(repoSlug: "rm/max-monorepo", key: "MAXX-7035")
let kc7031 = KeyCandidate(repoSlug: "rm/max-monorepo", key: "MAXX-7031")
let backendMatches = [
KeyPRMatch(candidate: kc7035, number: 171,
url: "https://github.com/rm/max-monorepo/pull/171", state: "OPEN", updatedAt: nil),
KeyPRMatch(candidate: kc7031, number: 168,
url: "https://github.com/rm/max-monorepo/pull/168", state: "OPEN", updatedAt: nil),
]

let fanned = IssueTracker.fanOutKeyMatches(backendMatches, across: candidates)
let decided = IssueTracker.decideReconcileLinks(matches: fanned)
let identity = [s7035: "MAXX-7035", s7031: "MAXX-7031", s6854: "MAXX-6854"]
let final = IssueTracker.dedupeContestedPRs(decided, identityBySession: identity)

let bySession = Dictionary(grouping: final, by: { $0.sessionID })
#expect(bySession[s7035]?.map(\.number) == [171])
#expect(bySession[s7031]?.map(\.number) == [168])
#expect(bySession[s6854] == nil) // no PR — never a phantom one
// No PR attaches to two distinct sessions.
#expect(Set(final.map(\.url)).count == final.count)
}
}
Loading