auto review

docs/operations.md

@@ -296,6 +296,7 @@ Every new snapshot of a task gets a review: a separate agent session on the vend
 
 - **Starting.** After `takeSnapshot` creates a snapshot, `afterSnapshot` runs `startReviewer`: it picks the vendor opposite the task's latest implementer session (`reviewVendor`) and that vendor's model in `reviewModels`, checks the snapshot's commit out detached at `worktrees/<project-id>/review-<review-id>`, creates an `agent_sessions` row with `role` `reviewer` on the task (created by the task's creator), a `reviews` row (`running`, one per snapshot), and sends `reviewPrompt` as its first turn (actor `byre`): the change as `git diff` from the previous snapshot (the merge-base with the default branch for the first) with its changed files, the merge-base diff of all the task's work for context, and the task's first message to its agent.
 - **Running.** Reviewer sessions get `reviewGuidance` and only the `submit_review` tool, whose arguments (`reviewSchema`: `change`, a few sentences of Markdown on what the change does, required and non-empty; a Markdown `summary`, which the guidance and schema require to be exactly "No significant findings." when there are no findings and otherwise a bulleted list with one line per finding; `findings`, each with `severity`, `category`, `title`, `detail`, `path`, `line`, `end_line` and `images`; and top-level `images`) become the review's `result` and mark it `done`. The guidance encourages, without requiring, Mermaid blocks for a data flow, call graph, state machine or schema the change alters, and screenshots when the change alters something visible.
+- **Forwarding.** When a submitted review has findings, byre sends them to the task's live agent as a `byre` message (`forwardFindings` and `findingsMessage` in `internal/agents/review.go`): each finding's severity, title, location and detail, under a preamble asking the agent to check each one against the code and its brief, act on those that hold up, and say in its final message which it acted on and which it left and why. It goes through `Manager.Message`, so it is queued like any message while the agent is mid-turn. A task with no live agent session gets nothing, and the log says `review findings not forwarded`. Forwarding doesn't set any finding's state; **fix** and **ignore** stay available.
 - **Screenshots.** Reviewer sessions also get the Playwright MCP server (`playwrightMCP` in `internal/agents/review.go`: `npx @playwright/mcp` at a pinned version, headless Chrome, isolated profile, absolute file paths), passed to Claude as a `stdio` entry in `--mcp-config` and to Codex as `mcp_servers` in the thread's `config`. Its automatically named files go to `worktrees/<project-id>/review-<review-id>-media`, removed with the checkout. Each `images` entry is `{path, caption}`, the path absolute or relative to the checkout; at `submit_review` byre reads every one, refuses the whole submission (nothing recorded) for a missing file or one that isn't one of `store.ImageTypes` within `store.MaxBlobBytes`, then stores each as a blob and replaces its `path` with `blob`, since the files are deleted with the checkout. They run without the Stop-hook bounce, mid-turn delivery, end-of-turn commit, snapshots or turn summaries, and their `user.message` events carry no `head`. When the reviewer's turn ends byre completes its session, kills its process and removes the worktree; a review still `running` then fails with why (no submission, with the final message; an error; an interrupt). A process that exits mid-turn fails the review the same way. Merging or discarding the task fails its running reviews ("the task was closed") and ends their reviewers. On startup, a running reviewer resumes like any session, and a running review whose reviewer isn't `running` fails.
 - **Questions.** A question's `reviews` is the review of the first snapshot the task took after the question was filed, the one at the end of the turn that asked it, computed when the item is read; the Inbox tab shows it under the question once it starts.
 - **Events.** `review.started`, `review.finished` (state `done`, `failed` or `skipped`; a skipped review emits only `review.finished`) and `review.finding` (a finding's state set, with the user as actor) carry the whole review, with the task's ids and no `agent_session_id`. A reviewer's own events carry its session id and the task's ids: the transcript skips every event of a session whose `session.created` has `role` `reviewer`, and draws each `review.finished` as a note with its outcome; the lead's `read_transcript` leaves the reviewer's events out and shows each `review.finished` with its summary and findings (`reviewLine`). `GET /api/tasks/{taskId}/sessions` and everything that looks for the task's agent (`ListSessions`) see only implementer sessions.

internal/agents/review.go

@@ -325,9 +325,62 @@ func (m *Manager) submitReview(ctx context.Context, l *live, raw json.RawMessage
 	if err != nil {
 		return "", err
 	}
+	if len(findings) > 0 {
+		go m.forwardFindings(l.taskID, l.reviewID)
+	}
 	return "Review recorded. End your turn now with one line.", nil
 }
 
+func (m *Manager) forwardFindings(taskID, reviewID string) {
+	ctx := context.Background()
+	r, err := m.store.GetReview(ctx, reviewID)
+	if err != nil {
+		slog.Error("load review to forward", "review", reviewID, "err", err)
+		return
+	}
+	sessions, err := m.store.ListSessions(ctx, taskID)
+	if err != nil {
+		slog.Error("list sessions to forward review", "review", reviewID, "err", err)
+		return
+	}
+	for _, s := range sessions {
+		if s.State == api.AgentSessionStateCompleted {
+			continue
+		}
+		_, err = m.Message(ctx, s.Id, "byre", findingsMessage(r), "")
+		if err != nil {
+			slog.Error("forward review findings", "review", reviewID, "session", s.Id, "err", err)
+			return
+		}
+		slog.Info("review findings forwarded", "review", reviewID, "session", s.Id)
+		return
+	}
+	slog.Info("review findings not forwarded, task has no live agent", "review", reviewID, "task", taskID)
+}
+
+func findingsMessage(r api.Review) string {
+	res := map[string]any{}
+	if r.Result != nil {
+		res = *r.Result
+	}
+	b := &strings.Builder{}
+	fmt.Fprintf(b, "A reviewer looked at snapshot #%d of your work and raised the findings below. Reviewers can be wrong or miss context: check each one against the code and your brief, act on the ones that hold up, and in your final message say briefly which you acted on and which you're leaving and why.", r.SnapshotN)
+	findings, _ := res["findings"].([]any)
+	for _, f := range findings {
+		fm, _ := f.(map[string]any)
+		where := ""
+		if p, ok := fm["path"].(string); ok && p != "" {
+			where = p
+			if line, ok := fm["line"].(float64); ok && line > 0 {
+				where += fmt.Sprintf(":%d", int(line))
+			}
+			where = " (`" + where + "`)"
+		}
+		fmt.Fprintf(b, "\n\n**[%v] %v**%s\n\n%s", fm["severity"], fm["title"], where, strings.TrimSpace(fmt.Sprint(fm["detail"])))
+	}
+	return b.String()
+}
+
 type reviewImage struct {
 	entry       map[string]any
 	contentType string