Require positive evidence before auto-completing sessions (#181)

[dhilgaertner]

Closes #181.

Summary

• autoCompleteFinishedSessions used set-difference against openIssues, so a silently-partial GraphQL response (rate-limit, parse miss, stale-PR fetch error) flipped every candidate session to completed. Replace the two absence-based fallbacks with positive-evidence rules: PR-linked sessions need MERGED/CLOSED in prsByURL; issue-only sessions need the ticket URL in closedIssueURLs.
• fetchStalePRStates now returns [ViewerPR]? (nil on shell/parse failure) and refresh() threads prDataComplete + closedIssueURLs into both auto-complete paths. When the stale-PR fetch errors, PR-linked completion is skipped.
• Belt-and-suspenders floor guard: if any candidate has a ticket URL and openIssueURLs is empty, skip the cycle with a warning.
• Decision logic extracted into pure nonisolated static helpers (decideSessionCompletions, decideReviewCompletions) so it's covered by unit tests without a shell/Process abstraction (matches the dedupedByURL / mergePRRecords pattern from Fix IssueTracker duplicate-key crash on PR status refresh #180). The same guards apply symmetrically to review-session completion.

Test plan

• make build — clean compile
• swift test --filter IssueTracker — 20/20 pass (6 existing dedup + 14 new)
• swift test (full suite) — all pass
• Manual: with the app running, force runConsolidatedGitHubQuery to return an empty data.openIssues.nodes and confirm no active sessions flip + log line skipping auto-complete — openIssues empty with N candidate sessions
• Manual: confirm the positive path still works — merge a PR linked to an active session and observe it auto-complete on the next refresh cycle

Behavior changes worth calling out

• Sessions whose linked issue was closed more than 24h ago (outside the closed:>= window in the consolidated query) will no longer auto-complete — the user marks them manually. Matches the ticket's "positive evidence only" requirement.
• Review sessions now only auto-complete when the PR is MERGED or CLOSED. Previously, "no longer in the review-request queue for any reason" triggered completion (e.g., reviewer unassigned with PR still open). Users will manually complete those cases.

Recovery for the three sessions flipped on 2026-04-16 is unchanged from the ticket:

crow set-status --session 27664893-E932-4AB3-97D6-7C8A6F997829 active
crow set-status --session EC8B5E7F-25A6-4E9A-A321-5087B01D32CA active
crow set-status --session 1D802104-9921-49EA-81DC-2D42F4AD2C6D active

🤖 Generated with Claude Code

[dgershman]

Code & Security Review

Security Review

Strengths:

• No user-facing input handling, injection surfaces, or secrets in the changed code
• The change strictly tightens trust boundaries — removing an unsafe absence-based inference and replacing it with positive evidence checks

Concerns:

• None

Code Quality

Strengths:

• Clean extraction of decision logic into nonisolated static pure functions (decideSessionCompletions, decideReviewCompletions) — testable without mocking AppState or shell calls. Good pattern that matches existing dedupedByURL/mergePRRecords convention.
• fetchStalePRStates return type change from [ViewerPR] to [ViewerPR]? is the right primitive — threading prDataComplete through callers is clean and explicit.
• CompletionDecision and CompletionResult types carry intent (reason string, floor guard flag) without coupling to side effects.
• 14 new tests cover the full matrix: floor guard trigger/non-trigger, MERGED/CLOSED/OPEN PR states, partial fetch, missing-from-payload, issue-only paths, review sessions, and the original regression scenario.
• Comments explain the why (partial GraphQL responses, the 2026-04-16 incident) not just the what.

Minor observations (non-blocking):

• The floor guard at IssueTracker.swift:1067-1069 has a known false-positive: a user with genuinely zero open issues but active sessions with ticket URLs will skip the cycle. The PR description calls this out explicitly and it's the safer default — no action needed.
• autoCompleteFinishedReviews filters appState.sessions for .review + .active but the old code also only looked at active reviews, so this is consistent. Worth noting that .paused or .inReview review sessions won't auto-complete — seems intentional.

Summary Table

Priority	Issue
🟢	Floor guard false-positive on zero-open-issues edge case (acknowledged, safe default)
🟢	Review sessions with .paused/.inReview status excluded from auto-complete (consistent with old behavior)

Recommendation: Approve — this is a well-scoped, well-tested fix for a real production bug. The positive-evidence requirement is strictly safer than the old absence-based logic, and the belt-and-suspenders floor guard adds an extra layer of protection against future regressions.