Fix IssueTracker duplicate-key crash on PR status refresh

[dhilgaertner]

Summary

• Crash: Swift/NativeDictionary.swift:792: Fatal error: Duplicate values for key: '…/pull/201' at Dictionary(uniqueKeysWithValues:) in applyPRStatuses / autoCompleteFinishedSessions.
• allKnownPRs = viewerPRs + stalePRs could surface the same URL twice (stale-PR follow-up race, or a GitHub response anomaly). The dictionary builders assumed uniqueness and trapped.
• Replace with a state-ranked merge (MERGED > CLOSED > OPEN; empty winner fields backfilled from the loser) applied at allKnownPRs assembly, and switch the two Dictionary call sites from uniqueKeysWithValues: to uniquingKeysWith: Self.mergePRRecords as a belt-and-suspenders safety net.
• Adds a CrowTests target (Tests/CrowTests/) with 6 regression cases and wires root-level swift test into make test and CI.

Test plan

• make test — 265 tests across 9 suites pass, including 6 new IssueTracker PR dedup cases
• make build — clean build
• Manual: launch app with a session whose PR link matches a viewer-returned PR; confirm badge renders and no crash on first refresh tick

Follow-ups (not in this PR)

• [GhosttyApp] Unhandled action: tag=35 log noise originates from vendor/ghostty and is unrelated to the crash — flag for a separate issue if wanted.
• Opt-in crash telemetry to surface the next trap without relying on user logs is a separate design decision.

🤖 Generated with Claude Code

[dgershman]

Code & Security Review

Critical Issues

None found.

Security Review

Strengths:

• No user-supplied input reaches the dedup logic — all data originates from GitHub API responses
• No secrets, credentials, or injection surfaces introduced
• nonisolated static functions are pure and side-effect-free

Concerns:

• None identified

Code Quality

Strengths:

• Root cause is correctly identified: ghResult.viewerPRs + stalePRs can produce duplicate URLs (race between stale-PR follow-up and viewer query). The Dictionary(uniqueKeysWithValues:) call then traps on the duplicate key.
• Fix applies dedup at the assembly site (dedupedByURL) and retains uniquingKeysWith: at the two downstream dictionary call sites as a belt-and-suspenders safety net — good defensive layering.
• mergePRRecords state-rank merge (MERGED > CLOSED > OPEN) with empty-field backfill is well-reasoned: the stale-PR query returns minimal fields, so the richer OPEN record's checks/reviews are preserved when the winner is MERGED.
• dedupedByURL preserves insertion order via a separate order array — deterministic downstream iteration.
• Test suite covers: exact duplicates, state precedence regardless of order, backfill of empty fields from loser, three-way merge, order preservation, and the dictionary uniquingKeysWith path directly.
• ViewerPR and LinkedIssue correctly marked Sendable to support nonisolated static usage.

Minor observations (non-blocking):

• stateRank returns 0 for unknown states — the >= comparison in mergePRRecords means the left-hand side wins ties, which is fine since both records are equivalent in that case.
• Makefile test target now depends on $(XCFW), ensuring the xcframework is built before tests run — good addition.
• CI adds root-level swift test after the per-package loop — matches the Makefile change.

Summary Table

Priority	Issue
🟢	Consider adding a doc comment on ViewerPR's Sendable conformance explaining it's needed for nonisolated static dedup helpers

Recommendation: Approve — the fix is well-scoped, correctly addresses the crash, has good defensive layering, and ships with solid regression tests.

🤖 Generated with Claude Code

[dgershman]

Code & Security Review

Critical Issues

None.

Security Review

Strengths:

• No external input surfaces affected — all data flows from GitHub's GraphQL API responses, which are already validated during JSON parsing
• No new shell invocations or command construction
• nonisolated static functions correctly operate on Sendable value types with no actor-isolated state access

Concerns:

• None identified

Code Quality

Strengths:

• Root cause correctly identified: viewerPRs + stalePRs concatenation can produce duplicate URLs (stale-PR follow-up race or GitHub response anomaly), and Dictionary(uniqueKeysWithValues:) traps on duplicates
• State-ranked merge (MERGED > CLOSED > OPEN) is the right precedence — a PR that transitioned mid-refresh gets its terminal state
• Backfill logic in mergePRRecords correctly handles the asymmetry where stale-PR follow-up queries return minimal records (empty checks/reviews) while viewer queries return full records
• Special handling for mergeable field (!= "UNKNOWN" vs .isEmpty) correctly accounts for its sentinel default — good attention to detail
• Belt-and-suspenders: dedup at assembly site and uniquingKeysWith: at both Dictionary call sites means a future refactor can't regress one without the other catching it
• dedupedByURL preserves first-seen order for deterministic downstream iteration
• Tests cover the important cases: exact duplicates, state precedence regardless of input order, backfill from loser to winner, three-way merge, order preservation, and the Dictionary uniquingKeysWith path directly

Minor observations (non-blocking):

• CrowTests depends on the Crow executable target — this works in SPM 6.0 but is worth noting if the project ever needs to support older toolchains
• The test Makefile target now depends on $(XCFW), which correctly ensures GhosttyKit is built before running root-level tests that transitively link it

Summary Table

Priority	Issue
Green	Test target depends on executable target (SPM 6.0 feature) — fine for this project's minimum toolchain

Recommendation: Approve — the fix is correct, well-tested (6 regression cases), and the defense-in-depth approach at both the assembly and consumption sites is solid.