fix(review): concurrent Trigger double-spawns reviewers (no serialization, no unique constraint)

[codebanditssss]

Bug

Concurrent Trigger calls for the same worker session at the same head SHA double-spawn reviewers and insert duplicate running review runs. The idempotency guard is a read-then-write with no serialization and no backing DB constraint.

Source: Live code analysis (bug hunt) | Reported by: @codebanditssss | Analyzed against: 198b797
Confidence: High — confirmed no unique index on review_run(session_id, target_sha) in migration 0012.

Reproduction

1. Issue two near-simultaneous POST /api/v1/sessions/{id}/reviews/trigger for the same worker at the same head SHA.
2. Both pass the GetReviewRunBySessionAndSHA idempotency check (neither has inserted yet), both spawn the deterministic review-<id> zellij session, and both InsertReviewRun → two running runs for one commit.

Root Cause

backend/internal/review/review.go:121 (Trigger) checks for an existing run (line ~151) then, much later, calls e.launcher.Spawn(...) (~173) and e.store.InsertReviewRun(...) (~211) with no lock/singleflight/transaction between check and write. reviewerHandleID(workerID) is deterministic ("review-"+workerID, launcher.go:62), so both spawns target the same zellij session id. backend/internal/storage/sqlite/migrations/0012_add_review_tables.sql declares UNIQUE only on the parent review.session_id, not on review_run(session_id, target_sha). The route POST /sessions/{id}/reviews/trigger (controllers/reviews.go:67) has no per-session locking.

Fix

Serialize triggers per worker session (keyed mutex / singleflight in the engine) and add a partial UNIQUE index on review_run(session_id, target_sha) so a duplicate InsertReviewRun fails and the engine can fall back to the existing run.

Impact

• Two reviewer processes spawn against the same handle (second errors or duplicates), and the DB holds two running runs for one commit, defeating the idempotency design and confusing the UI's reviewer handle/run list.

Related

• #140 — LCM intent-driven notification service foundation (adjacent review/lifecycle surface)