fix(review): notify worker on changes_requested instead of relying on SCM poll (#337)

[neversettle17-101]

What #337 required

review.Engine.Submit only persisted the reviewer's verdict/body via UpdateReviewRunResult — it never told the worker session anything. On a changes_requested verdict the worker was left to discover the feedback through the SCM poll loop (observe/scm needsReviewRefresh), which is gated on GitHub's reviewDecision. That decision never becomes CHANGES_REQUESTED for self-reviews or COMMENT-state reviews, so the nudge frequently never fired.

This PR makes it event-driven: when verdict == VerdictChangesRequested, Submit now messages the worker's live pane directly through ports.AgentMessenger.Send — the same mechanism lifecycle.Manager uses for its SCM nudges. The messenger is wired into the review engine in daemon/lifecycle_wiring.go (the per-daemon messenger already in scope). Approved verdicts do not message the worker. The body is reviewer-authored text pasted into a PTY, so it is run through domain.SanitizeControlChars first, matching the lifecycle reaction path.

We deliberately did not touch the generic SCM-comment poller — re-injecting every PR comment from anyone is out of scope.

Extended flow (review-id round-trip)

So the worker knows which review to address:

1. The reviewer agent posts its review with gh api --method POST repos/{owner}/{repo}/pulls/{number}/reviews ... --jq '.id' and passes that id through a new ao review submit --review-id flag (prompt updated in internal/review/prompt.go). It posts via gh api rather than gh pr review because that is the only way to attach inline comments, and the POST response already carries the created review's id — so the id is captured from the call the reviewer makes anyway, exactly (no array-ordering, pagination, or read-back heuristics).
2. The daemon stores the id on the review_run row (new github_review_id column, migration 0016 — latest before this was 0015) and includes it in the changes-requested message to the worker.
3. After addressing the feedback, the worker replies on the review referencing that id with what it changed, and resolves the inline review comment threads it addressed.

The id flows: CLI flag → SubmitReviewInput.githubReviewId → service → Engine.Submit → UpdateReviewRunResult (now also sets the column) and into the worker message. It is optional throughout: if the reviewer can't read the id back, everything still works without it (the reply/resolve instruction is simply omitted).

AO internal review vs. external SCM review (worker message)

The worker now gets two clearly different nudges depending on origin:

• External GitHub reviewer (lifecycle SCM loop, unchanged): "A reviewer left feedback on your PR. Address it and push."
• AO internal review (Engine.Submit, this PR): identifies itself as an AO code reviewer (not an external GitHub PR reviewer), and — when the review id is known — asks the worker, after pushing the fix, to reply on that review referencing its id with what it changed and to resolve the review comment threads it addressed.

"Resolve conversation" tradeoff

GitHub's resolveReviewThread (GraphQL) only applies to inline review-comment threads tied to a line, not to a top-level PR review object. The reviewer's prompt has it post inline comments for specific findings alongside the top-level request-changes verdict, so the per-finding threads are resolvable — the worker is told to resolve those after addressing them. The top-level review object itself is not resolvable, so for that we use option (a): the worker replies on the review referencing the id (a plain comment) rather than claiming to "resolve" it.

I rejected the heavier option (b) — restructuring around domain.ReviewRun to track per-line comment threads and their resolution state in AO — because it fights the one-verdict/one-body-per-run model and isn't needed: the worker agent can run resolveReviewThread itself against the threads the reviewer already created.

Tests

• internal/review: hermetic unit tests with a fake AgentMessenger — changes_requested messages the worker with body + review id, is identified as an AO internal review, and carries the reply/resolve instruction; the instruction is omitted when no id is supplied; approved does not message; messenger error is surfaced (happy-path-only, per scope). Existing Submit tests updated for the new signature.
• internal/storage/sqlite/store: round-trips github_review_id through insert/update/get.
• Regenerated openapi.yaml and frontend/src/api/schema.ts (code-first API contract); gen/* via sqlc.
• go build ./..., go vet ./... clean. Affected packages pass go test -count=1. (Pre-existing zellij runtime integration tests fail locally on IPC-socket-path-length / local-binary-path — environment-specific, unrelated.)

Out of scope / follow-ups (deferred per issue)

• Resiliency: retry on message-send failure, dead/terminated worker session, double-submit idempotency. Today Submit surfaces a send error to the caller and does nothing fancier — happy path only.
• AO-side tracking of which threads were resolved, if richer per-finding state is wanted later.

🤖 Generated with Claude Code

[neversettle17-101]

Verdict: changes_requested (posted as a COMMENT review — GitHub does not allow REQUEST_CHANGES on your own PR).

The core change (event-driven worker nudge on changes_requested + github_review_id round-trip) is solid, well-plumbed, and well-tested. Requesting one change to the new reviewer-agent prompt before merge — details below and inline.

Blocking: the documented gh api command won't attach inline comments

gh api's -f/-F flags build a flat body and only treat a key ending in [] as an array-append of scalars. comments[][path]=... is taken as a literal top-level field, not as "append an object to the comments array", so the body becomes {"comments[][path]": ...} rather than {"comments":[{"path":...,"line":...,"body":...}]}. GitHub will 422 / drop those, so the review posts with no inline comments — which is the whole reason the latest commits switched from gh pr review to gh api. Use --input with a JSON body instead (see inline comment).

Non-blocking

If messenger.Send fails, Submit returns an error after UpdateReviewRunResult already marked the run complete; a retried ao review submit then fails the status='running' guard, leaving the run recorded but the worker un-nudged. Explicitly deferred (resiliency/idempotency) in the PR description, so acceptable — just flagging.

Everything else — migration 0016 numbering, sqlc gen, store round-trip, wiring, and the hermetic tests — looks good. go build, go vet, gofmt, and affected tests pass locally.

[neversettle17-101]

Verdict: changes_requested (posted as a COMMENT review — GitHub does not allow REQUEST_CHANGES on your own PR).

Re-review at fcc441e

My earlier blocking finding is resolved — prompt.go now posts the review with an --input - JSON heredoc, so inline comments form a proper "comments": [ {path, line, body} ] array GitHub accepts, instead of the comments[][path] flat-field form gh api can't serialize. The approve/COMMENT fallback prose and --jq '.id' capture are intact. go build clean. The core feature remains solid and well-tested.

Blocking: a stray review.md was committed into the PR

The PR now adds a 42-line review.md at the repo root (added in 5df20c9, reformatted by fcc441e). Its contents are a code-review writeup of this very PR — a reviewer-agent scratch artifact that leaked onto the worker branch. It's unrelated to the feature and shouldn't merge. Please git rm review.md, and consider gitignoring the reviewer's scratch path so this can't recur. See inline comment.

Non-blocking (deferred per PR description)

If messenger.Send fails, Submit returns an error after UpdateReviewRunResult already marked the run complete; a retried ao review submit then fails the status='running' guard. Explicitly deferred (resiliency/idempotency) — flagging only.

Once review.md is removed, this is good to merge.

[neversettle17-101]

Verdict: approved (posted as a COMMENT review — GitHub does not allow the PR author's account to submit APPROVE).

Re-review at 154b99e

Both of my prior findings are fully resolved, with sensible extra hardening.

• Stray review.md (was blocking) — fixed. 40d3d19 removes it and .gitignore now ignores /review.md. Better still, the root cause is gone: ao review submit --body - reads the review from stdin (io.ReadAll(cmd.InOrStdin())) and the reviewer prompt passes it on a heredoc, so nothing is written into the checkout at all.
• Submit error after DB commit (was non-blocking) — fixed. 600ea1b notifies the worker before UpdateReviewRunResult, so a failed messenger.Send leaves the run running and a retried submit runs again instead of tripping the status='running' guard. Trade-off documented in the comment (matches lifecycle sendOnce); new test asserts both stays-running and successful retry.
• event=COMMENT always (237548f/154b99e): correct — the reviewer posts from the PR author's own account, where GitHub rejects APPROVE and REQUEST_CHANGES. The verdict still reaches AO via step 2.
• Earlier gh api inline-comments fix (JSON --input body) is in place.

Verification: go build ./..., go vet, gofmt clean; go test ./internal/review/... ./internal/cli/... pass incl. new stdin-body + retry tests; diff vs main is focused, migration correctly numbered 0016, no stray files.

Nice work — ready to merge.