Fix duplicate review sessions on in-flight crow:auto kickoff

[dgershman]

Summary

• Stop trusting the lagging ReviewRequest.reviewSessionID cross-ref and consult appState.sessions directly via a new AppState.existingReviewSession(forPRURL:) helper.
• Layer the check at three sites: auto-review watcher guard, createReviewSession early-return backstop, and both Start Review buttons in ReviewBoardView.
• Extract the PR-URL parser into Session.parseReviewPR(url:) so all sites share one parser.

Why

After #405 moved createReviewSession's git work into Task.detached, the main actor stays responsive during the ~10s clone window. The IssueTracker watcher can now tick during that window — and because request.reviewSessionID is populated lazily on the next refresh, the watcher's existing guard sees stale nil, enqueues a second kickoff, and createReviewSession (with no dedup of its own) creates a duplicate session.

The same lag was also why the per-row Start Review button stayed clickable for several seconds after a session had been created.

Test plan

• arch -arm64 swift test --package-path Packages/CrowCore — 205 tests pass (includes the new existingReviewSession and parseReviewPR test files)
• arch -arm64 swift test (root + every package) — all suites pass except a pre-existing flaky retryReadinessEmitsTimedOutWhenSentinelMissing tmux timing test in CrowTerminal that's unrelated to this change
• swift build --arch arm64 clean
• Manual smoke (reviewer): with autoReviewRepos configured, open a crow:auto PR, confirm exactly one review-<repo>-<num> session appears even when the IssueTracker ticks multiple times during the clone; confirm the Start Review button flips to "Go to Session" on the same tick the session appears; force-push the PR to confirm the SHA-advanced re-kick path still works.

Closes #406

🤖 Generated with Claude Code

[dhilgaertner]

Code & Security Review

Critical Issues

None.

Security Review

Strengths:

• Session.parseReviewPR(url:) is pure string parsing with no injection surface; it preserves the exact behavior of the inlined parser it replaces (split(separator: "/"), count >= 5, trailing-segment Int cast).
• No new untrusted input paths — PR URLs originate from the GitHub-backed ReviewRequest/SessionLink data that already flows through the app.
• The new existingReviewSession(forPRURL:) is correctly @MainActor-isolated (matches AppState), and every caller (watcher, review-board buttons, SessionService) is already on the main actor, so there's no data-race introduced.

Concerns:

• None.

Code Quality

• Dedup is genuinely authoritative, not racy. The serial reviewKickoffTail queue (AppDelegate.swift:173-187) awaits the previous tail, so the second createReviewSession only runs after the first has fully returned — and the session + .pr link are appended synchronously on the main actor at SessionService.swift:1337-1340 after the detached clone await. The backstop at SessionService.swift:1255 therefore always sees the prior session. Sound.
• Watcher guard change is the right shape. guard (request.reviewSessionID == nil && existingByPR == nil) || shaAdvanced is strictly more restrictive only on the fresh-kickoff path and leaves the shaAdvanced re-kick path (force-push / round-2) untouched, including the stale-session teardown at AppDelegate.swift:637. The A-path completion case still kicks off correctly since existingReviewSession excludes completed/archived sessions (reviewSessions filter at AppState.swift:380-382).
• Refactor removes real duplication by sharing one parser across SessionService and the dedup helpers, and the .pr-linkType + exact-URL match is consistent across all three call sites (the link is stored from the same prURL the watcher/buttons pass in).
• Test coverage is thorough — the new suites cover nil/match/completed/archived/non-PR-linkType/duplicate cases for the lookup and owner/repo/number/malformed cases for the parser. All 10 new tests pass locally (swift test --package-path Packages/CrowCore).

Minor, non-blocking observations (no action needed): existingReviewSession is O(sessions × links) and recomputed each watcher iteration, but both sets are tiny; and exact-string URL matching is fine here because the same URL source feeds both the stored link and every lookup.

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, 0 Green] findings. Correct, well-isolated, well-tested fix for the in-flight kickoff race.

🐦‍⬛ Reviewed by Crow via Claude Code