Stop completed reviews from restarting on app relaunch

[dhilgaertner]

Summary

• Session.reviewPromptDispatched is a new persisted boolean that gates whether launchClaude sends the /crow-review-pr prompt or falls through to claude --continue. Once the prompt fires for the first time, the flag flips to true and is written to the JSON store so subsequent app launches resume the existing Claude conversation instead of re-running the review.
• Existing persisted review sessions (without the new field) decode it as true, so the bug does not re-trigger on the first launch after upgrade.

Test plan

• swift test --package-path Packages/CrowCore (141 tests pass)
• swift test at root (46 tests pass)
• swift build succeeds with no new warnings
• Manual: create a review session → quit Crow mid-review → relaunch → verify the terminal shows claude --continue rather than re-executing /crow-review-pr <url>, and cat ~/.local/share/crow/store.json | jq '.sessions[] | select(.kind=="review") | {name, reviewPromptDispatched}' shows true
• Manual: brand-new review session post-fix dispatches the prompt exactly once at creation and reviewPromptDispatched becomes true in store.json

Closes #224

[dgershman]

Code & Security Review

Critical Issues

None.

Security Review

Strengths:

• No secrets, credentials, or user-controlled input flow into the new code path.
• The prompt file is constructed from an internally-generated worktree path, not from user-supplied data. The $(cat ...) shell expansion reads a local file the app itself wrote — no injection vector.
• store.mutate is protected by NSLock, so the persistence write is thread-safe.

Concerns:

• None identified.

Code Quality

Strengths:

• Clean, minimal diff — only the two files that need to change.
• Backward-compatible decoding defaults reviewPromptDispatched to true for pre-existing sessions, preventing the exact bug described in CROW-224 on first upgrade. Newly created sessions default to false in Session.init, so the flag correctly gates only the first dispatch.
• Dual-write pattern (in-memory appState + store.mutate) is consistent with existing patterns in the file (e.g., updateSessionStatus, recoverOrphan).
• The var reviewPromptJustDispatched flag inside the closure + post-closure write avoids mutating shared state inside the closure, keeping the closure side-effect-free.

Minor observations (non-blocking):

• No unit test explicitly covers reviewPromptDispatched decoding/encoding round-trip or the launchClaude branching logic. The existing 141 CrowCore tests pass, and SessionService is @MainActor-bound (hard to unit test without the full app harness), so this is understandable — noting it for awareness only.
• The createReviewSession method (line 1023) creates Session(kind: .review, ...) without explicitly passing reviewPromptDispatched: false, relying on the default. This is correct but implicit — perfectly fine given the parameter has a default value.

Summary Table

Priority	Issue
🟢 Green	Consider adding a codable round-trip test for reviewPromptDispatched in the Session test suite

Recommendation: Approve — the fix is clean, correctly handles the upgrade path, and aligns with existing patterns in the codebase. The dual-write to appState and store ensures consistency, and the backward-compat decoding default prevents re-triggering on upgrade.

[dgershman]

Code & Security Review

Security Review

Strengths:

• No user-facing input handling introduced — the new field is purely internal state.
• No secrets, credentials, or external data flows affected.
• promptPath is constructed from trusted internal state (worktree path from the persisted store), not from user input.

Concerns:

• None identified.

Code Quality

The implementation is clean and well-scoped:

1. Backward-compatible decoding (Session.swift:62): Defaulting reviewPromptDispatched to true when missing from older JSON is the correct choice — existing sessions that already ran their prompt won't re-trigger.

2. Immediately-invoked closure pattern (SessionService.swift:358-370): Mutating a local var inside an immediately-invoked closure is valid Swift and avoids the need for a separate helper function. Since SessionService is @MainActor, there are no concurrency concerns.

3. Dual state update (SessionService.swift:382-391): Updating both the in-memory appState.sessions array and the persisted store ensures consistency across the app lifecycle. The guard if reviewPromptJustDispatched, let sessionID prevents unnecessary writes.

4. Default parameter value (Session.swift:31): reviewPromptDispatched: Bool = false means new sessions start with the prompt not yet dispatched, which is the correct initial state for review sessions.

Summary Table

Priority	Issue
Green	Consider adding a unit test in CrowCore for decoding a Session JSON blob without reviewPromptDispatched to explicitly verify the ?? true fallback

Recommendation: Approve — the fix is minimal, correctly scoped, backward-compatible, and all 141 CrowCore tests pass. The architecture (MainActor serialization + JSONStore NSLock) prevents any race condition in the persist path.