feat: support multi-PR review runs

[neversettle17-101]

Summary

• keep the review row session-scoped while moving trigger/list behavior to PR-scoped review_run records
• add migration 0020 for the (session_id, pr_url, target_sha) non-failed unique index and session/PR/SHA lookup
• add pure per-PR review planning plus multi-PR trigger batching through one reviewer terminal
• expose current PR review state as reviews on list/trigger responses; run records remain internal except submit's run response and latestRun on each review state
• include the full created-run PR queue in reviewer prompts so the shared reviewer knows the multi-PR context

Go Rewrite Notes

• Legacy behavior ported: AO review triggering/submission loop behavior from the review/lifecycle flow, adapted for multiple PRs per worker session described in issue Support multi-PR reviews per worker session #395.
• Go service owner: core review planning/execution lives in backend/internal/review; HTTP-facing submission and lifecycle delivery stay in backend/internal/service/review and lifecycle reactions.
• Preserved invariants: review remains one row per worker session, review_run owns PR-specific facts, failed runs are retryable, current-head running/approved runs are reused/skipped, and worker notifications use the stored run PR URL.

Tests

• cd backend && go build ./...
• cd backend && go vet ./...
• cd backend && go test ./...

Refs #395

[neversettle17-101]

AO review — feat: support multi-PR review runs

Verdict: Changes requested (one medium correctness concern + a small leak; the rest is solid).

This is a clean, well-tested refactor from single-PR to multi-PR review runs. The pure Plan() shared between the trigger and list paths is a nice design, the migration carefully dedupes before adding the (session_id, pr_url, target_sha) unique index and folds in batch_id, and the batch-gated delivery via drainBatch + ApplyReviewBatch is reasonable. Build is green and go test ./internal/review/... ./internal/service/review/... ./internal/lifecycle/... ./internal/httpd/controllers/... ./internal/storage/sqlite/... all pass.

Concerns below — the batch-delivery one is worth confirming is intended before merge.

Medium

• A never-terminal run blocks delivery of the whole batch. drainBatch returns early as soon as any run in the batch is still running. Delivery only ever happens on a Submit call, and a failed/superseded status is only produced by launch failures or a newer-commit trigger — not by a reviewer that dies or simply never submits one PR's run. So if the shared reviewer pane crashes after reviewing PR1 but before submitting PR2, PR1's changes_requested feedback is never delivered to the worker, and there's no sweeper to force the stuck run terminal. Single-PR triggers didn't couple PRs this way. Is there an intended recovery path (timeout / reviewer-death reaction) I'm missing?

Low

• drainLocks grows unbounded. lockBatchDrain adds a *sync.Mutex per (workerID, batchID) and never removes it, so the map leaks one entry per batch over the daemon's lifetime. Consider deleting the entry once the batch is fully delivered, or using a sharded/striped lock keyed by a bounded hash.
• firstReusableRun can return an unrelated PR's run. When nothing new is created it returns the first review state with any LatestRun, regardless of PR or eligibility, into the legacy TriggerResult.Run. Harmless given Created:false and that Reviews now carries the real state, but the picked run is arbitrary across PRs.

Notes (non-blocking)

• upsertReview now always writes PRURL: "", leaving domain.Review.PRURL a permanently-empty field. Fine for the multi-PR model, but the field is now dead — worth a follow-up to drop it or document why it stays.
• Re-triggering a changes_requested PR at its current head relies on the unique-index ErrDuplicateReviewRun fallback to no-op; works, but does an extra insert attempt each trigger.

[neversettle17-101]

AO re-review — feat: support multi-PR review runs (backend)

Verdict: Approve (both prior blocking concerns resolved; only minor notes remain).

Thanks for the rework in fix: submit multi-pr reviews as one batch and fix: make queued reviews autonomous — this addresses both findings from my previous review:

• Stuck-run no longer blocks batch delivery (was my medium concern). Delivery is now scoped to the runs in a single reviewer CLI submission (SubmitMany → deliverSubmitted/deliverableRuns) rather than gated on the whole batch having no running run. A missing/stuck result for another PR in the same trigger can no longer withhold feedback. The new TestSubmitBatchRunDoesNotWaitForOtherRunningRuns and TestSubmitManySendsCombinedChangesRequested lock this in.
• drainLocks leak removed (was my low concern). The per-(worker,batch) mutex map and drainBatch are gone entirely.
• The prompt/CLI now drive an autonomous multi-PR queue with a single batched ao review submit --reviews -, and submitOne is idempotent for already-complete/delivered runs, so a retried submission recovers cleanly.

Build green; go test ./internal/review/... ./internal/service/review/... ./internal/lifecycle/... ./internal/httpd/controllers/... ./internal/cli/... ./internal/storage/sqlite/... all pass.

Minor notes (non-blocking)

• SubmitMany persists results item-by-item before delivering. If item N is invalid (bad verdict / run not running), items 0..N-1 are already complete in the store when the call aborts with no delivery. Idempotency makes a full retry recover, but pre-validating every item before the first UpdateReviewRunResult would avoid leaving a batch half-committed.
• ReviewIndex is now effectively dead. Since the per-run notify loop was removed in Trigger, only created[0] is ever launched (index 0) and reviewQueueText no longer reads spec.ReviewIndex. Consider dropping the field from LaunchSpec/ports.ReviewInvocation to avoid a misleading knob.
• HTTP submit accepts both shapes. The controller takes either top-level runId/verdict/... or a reviews[] array; the CLI guards against combining them but the API doesn't. Harmless (array wins, top-level ignored) but worth a note.

[neversettle17-101]

Results are persisted one at a time here; if a later item fails validation in submitOne, earlier items are already marked complete while the whole call returns an error with no delivery. submitOne's idempotency lets a full retry recover, but validating all items up front (before any UpdateReviewRunResult) would avoid leaving a batch half-committed.

[neversettle17-101]

ReviewIndex is now always 0 (the per-run notify loop was removed, so only created[0] is launched) and reviewQueueText no longer reads it. Consider removing the field from LaunchSpec/ReviewInvocation so it doesn't read as a live knob.