From ed4b66c2299e082bb798e5df6c4898caba92539e Mon Sep 17 00:00:00 2001
From: Radu Topala <radu.topala@trisoft.ro>
Date: Mon, 27 Jul 2026 20:01:43 +0300
Subject: [PATCH] fix(review): deliver code-review findings to the Review panel
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A PR review run reported its findings and the panel stayed empty. Three
defects stacked up between the agent's tool call and the ingest path.

1. collectOutput picked the streaming reader off a hand-listed subset of
   the callbacks (onTurn / onThinking / onToolResult). A review request
   sets none of those, so it fell through to the batch read, which
   re-scans the logs with an empty streamCallbacks and drops every
   callback. The findings were on the stream with nothing listening.
   Replaced with an exhaustive streamCallbacks.any(), which also restores
   stream-derived activity events for requests that set only OnActivity.

2. Tool inputs reached callers only as summarizeToolInput's chat-facing
   summary, which is empty for any tool it has no case for — including
   ReportFindings. Added ToolUse.Raw / onToolUseRaw / OnToolUseRaw so a
   caller that needs to decode arguments gets the verbatim JSON, and
   pointed the review runner at it.

3. Review runs forked the built-in /code-review command into a silent
   subagent instead of running it inline. Inline requires
   CLAUDE_CODE_REPORT_FINDINGS plus an available ReportFindings tool, so
   review mode now drops that tool from the batch disallow list and
   passes the env via --settings. Container env is applied *under* the
   user's settings file, so a host CLAUDE_CODE_SUBAGENT_MODEL would
   otherwise overwrite ours; --settings lands in a scope that wins.

Tests cover the raw channel, the streaming gate per callback, and a
review-shaped request end to end, plus the real findings payload.
Docs updated to describe ReportFindings as the default reporting channel
with the MCP tool as the override path.
---
 .gitignore                               |   3 +
 docs/configuration.md                    |   4 +-
 docs/events.md                           |   2 +-
 docs/review.md                           |  36 +++++---
 internal/agent/types.go                  |  21 +++++
 internal/api/review_handler.go           |  28 ++++---
 internal/api/review_handler_test.go      |  45 +++++++++-
 internal/api/review_service.go           |  23 ++++--
 internal/config/config.go                |   8 +-
 internal/container/claude_cmd.go         |  41 ++++++++-
 internal/container/runner.go             |   3 +-
 internal/container/runner_mounts_test.go |  31 +++++++
 internal/container/runner_retry_test.go  |  16 ++--
 internal/container/runner_shell_test.go  | 101 +++++++++++++++++++++++
 internal/container/runner_stream_test.go |  34 ++++++++
 internal/container/stream.go             |  34 ++++++--
 internal/review/parser.go                |  47 +++++++++++
 internal/review/parser_test.go           |  57 +++++++++++++
 internal/review/runner.go                |  30 +++++--
 internal/review/runner_test.go           |  61 +++++++++++++-
 20 files changed, 562 insertions(+), 63 deletions(-)

diff --git a/.gitignore b/.gitignore
index 67b95736..423d8ce1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -49,6 +49,9 @@ memory/
 tickets/
 .worktrees/
 
+# Throwaway working files (binary dumps, captured payloads) — never committed
+.scratch/
+
 # Component test artifacts
 test/component/screenshots/
 
diff --git a/docs/configuration.md b/docs/configuration.md
index 19b411f5..eace2d1e 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -48,7 +48,7 @@ Exactly one of these should be set. OAuth takes precedence if both are provided.
 | `claude_effort` | `string` | `""` | Reasoning-effort level passed to the Claude CLI as `--effort`. Empty leaves it unset (model default). Options: `"low"`, `"medium"`, `"high"`, `"xhigh"`, `"max"`. |
 
 Both `claude_model` and `claude_effort` can additionally be **overridden per channel/thread/worktree on demand** from the chat composer's model pill (persisted on the channel row, applied from the next run) — see [Chat: Model / Effort Override](chat.md#model--effort-override). The per-channel override beats the merged config value.
-| `claude_batch_disallowed_tools` | `[]string` | `["ScheduleWakeup", "CronCreate", "CronDelete", "CronList", "Monitor", "ReportFindings"]` | Claude Code tool names passed to `--disallowedTools` for **batch** (`--print`) agent runs only — the interactive terminal is unaffected. The defaults rely on a persistent harness that one-shot containers lack: `ScheduleWakeup`/`Cron*` schedule re-invocations that never fire, and `Monitor` arms a background watcher whose cross-turn events are dropped when the container exits at end of turn — so they silently park work that never resumes. `ReportFindings` reports code-review findings into a harness UI channel batch runs don't render — Loop's review flow uses the `report_review_findings` MCP tool instead. Override to tune the list (e.g. to re-enable a tool). |
+| `claude_batch_disallowed_tools` | `[]string` | `["ScheduleWakeup", "CronCreate", "CronDelete", "CronList", "Monitor", "ReportFindings"]` | Claude Code tool names passed to `--disallowedTools` for **batch** (`--print`) agent runs only — the interactive terminal is unaffected. The defaults rely on a persistent harness that one-shot containers lack: `ScheduleWakeup`/`Cron*` schedule re-invocations that never fire, and `Monitor` arms a background watcher whose cross-turn events are dropped when the container exits at end of turn — so they silently park work that never resumes. `ReportFindings` reports code-review findings into a harness UI channel batch runs don't render. Review runs are the exception: they drop `ReportFindings` from this list, because the built-in `/code-review` command only runs inline (rather than forking a silent subagent) when it can call that tool, and the daemon reads the findings off the stream. Override to tune the list (e.g. to re-enable a tool). |
 | `claude_retry` | `object` | see below | Automatic recovery from Claude API limits on batch agent runs. Keys: `max_attempts` (`5`) extra attempts after the first failure (`0` disables), `backoff_base_sec` (`5`) first-retry delay that doubles each attempt, `backoff_max_sec` (`120`) per-attempt delay ceiling — these govern in-process exponential backoff for **transient** errors (rate limiting, overload, 5xx). `session_limit_auto_continue` (`true`) handles a **session limit** error (`You've hit your session limit · resets <time>`) by scheduling a one-shot retry at the announced reset time that resumes the session with `continue`; it is persisted via the scheduler so it survives daemon restarts. Terminal errors (usage/quota exhaustion, auth, billing) are never retried. |
 | `keep_mcp_configs` | `bool` | `false` | When true, preserves MCP config JSON files after container runs. Useful for debugging MCP server configuration. |
 
@@ -272,7 +272,7 @@ Enables and configures the Review panel (see [review.md](review.md)).
 | `prompt` | `string` | Inline prompt text. Mutually exclusive with `prompt_path`. |
 | `prompt_path` | `string` | Path to a prompt file, resolved as `~/.loop/review/{prompt_path}`. Mutually exclusive with `prompt`. |
 
-Both prompt fields empty (the default) uses the daemon's built-in default prompt: the bare `/code-review` slash command, with the output contract (report findings via the `report_review_findings` MCP tool for actionable issues only) carried in the system prompt.
+Both prompt fields empty (the default) uses the daemon's built-in default prompt: the bare `/code-review` slash command, with the output contract (report actionable findings via the built-in `ReportFindings` tool, each carrying a file and a 1-based line) carried in the system prompt. A configured non-slash prompt gets no system prompt, so it must carry its own contract — see [review.md](review.md#required-output-format).
 
 #### Playground share
 
diff --git a/docs/events.md b/docs/events.md
index cda513cc..d1fc9482 100644
--- a/docs/events.md
+++ b/docs/events.md
@@ -776,7 +776,7 @@ The renderer treats event delivery as best-effort: on every WebSocket reconnect
 
 ### `review.comment`
 
-Emitted once per finding the review agent reports through the `report_review_findings` MCP tool during a run. Deduplicated by comment id upstream so each id arrives at most once.
+Emitted once per finding the review agent reports during a run, via either the built-in `ReportFindings` tool (read off the agent's stream) or the `report_review_findings` MCP tool. Deduplicated by comment id upstream so each id arrives at most once.
 
 **Payload schema:**
 
diff --git a/docs/review.md b/docs/review.md
index b53e5247..4dfd6f9c 100644
--- a/docs/review.md
+++ b/docs/review.md
@@ -56,9 +56,12 @@ per-global / per-project / per-worktree the same way as `github.gh_user`.
    operator can see `review iter 2/3 — running`, `fixing — iter 2/3`,
    `paused at gate — …`, `done — 0 comments remaining`, or
    `stopped — no progress (same findings)` without leaving the Review
-   panel. The agent reports findings by calling the
-   `report_review_findings` MCP tool, which posts them straight into the
-   review session; each comment is broadcast to the FE as it arrives.
+   panel. The agent reports findings by calling `ReportFindings`; the
+   daemon reads that tool call off the agent's output stream and posts
+   each finding into the review session, broadcasting it to the FE as it
+   arrives. An override prompt can instead use the
+   `report_review_findings` MCP tool — see
+   [Required output format](#required-output-format).
 3. **Push** — each comment ships with **Push** (single) and a **Push all
    (N)** affordance in the header (when at least one comment is unpushed).
    The backend uses `gh api ... /pulls/N/comments` against the captured
@@ -151,9 +154,22 @@ error; setting neither uses the daemon's built-in default prompt.
 
 ### Required output format
 
-Findings travel through the `report_review_findings` MCP tool (available
-in every agent container), so an override prompt must instruct the agent
-to call it with the full findings list. Each finding carries:
+Findings reach the daemon two ways, both landing in the same ingest path.
+
+The default prompt is the bare `/code-review` slash command, which
+reports through Claude Code's own **`ReportFindings`** tool. The daemon
+intercepts that tool call on the agent's stream, so nothing has to round
+-trip through HTTP. Findings need a repo-relative `file` and a 1-based
+`line`: the tool's schema treats `line` as optional, but a finding
+without one can't be anchored in the diff and is dropped, so the default
+system prompt requires it. `summary` and `failure_scenario` are joined
+into the comment body.
+
+An override prompt that is *not* a slash command gets no system prompt
+from the daemon, so it must state its own contract. Either instruct the
+agent to call `ReportFindings` as above, or use the
+**`report_review_findings`** MCP tool (registered in every agent
+container) with the full findings list. Each MCP finding carries:
 
 - `path` — repo-relative file path.
 - `line` — the 1-based line on the indicated side of the diff.
@@ -165,12 +181,8 @@ to call it with the full findings list. Each finding carries:
 
 Malformed findings (empty path/body, non-positive line) are skipped, and
 the daemon deduplicates by a stable content hash of path/line/body, so
-re-reporting the same finding — in the same call or a later run — is
-safe. The prompt should also tell the agent NOT to use the harness's
-`ReportFindings` tool: a successful call there swallows the findings
-into a UI channel the daemon never reads (the default prompt does this,
-and batch runs additionally deny the tool via
-`claude_batch_disallowed_tools`).
+re-reporting the same finding — in the same call, over both channels, or
+in a later run — is safe.
 
 ## See also
 
diff --git a/internal/agent/types.go b/internal/agent/types.go
index d70e4a41..c45f581a 100644
--- a/internal/agent/types.go
+++ b/internal/agent/types.go
@@ -22,6 +22,22 @@ type AgentRequest struct {
 	// for this run when non-empty (per-channel on-demand override).
 	Model  string `json:"model,omitempty"`
 	Effort string `json:"effort,omitempty"`
+	// ReviewMode runs the request as a code-review pass, so that the built-in
+	// /code-review command executes in the main session instead of forking a
+	// subagent. It takes ReportFindings out of claude_batch_disallowed_tools
+	// and passes --settings with two env keys (see reviewModeSettings):
+	//
+	//   - CLAUDE_CODE_REPORT_FINDINGS=1. The command runs inline only when
+	//     this is set AND ReportFindings is available AND the output format is
+	//     stream-json (batch runs always are); otherwise it forks. A fork emits
+	//     nothing on the parent stream — the run looks hung for minutes — and
+	//     its findings never reach a tool call the parent can see.
+	//   - CLAUDE_CODE_SUBAGENT_MODEL=inherit, so any subagent that does get
+	//     spawned runs on this request's model. This key outranks every other
+	//     source of a subagent's model, so a host ~/.claude/settings.json
+	//     setting it to e.g. "sonnet" (that mount is shared with the container)
+	//     otherwise silently downgrades the run.
+	ReviewMode bool `json:"review_mode,omitempty"`
 	// OnTurn is called for each assistant turn's text content during streaming.
 	// When set, the runner follows container logs in real-time instead of waiting
 	// for the container to exit. When nil, the runner uses the existing
@@ -31,6 +47,11 @@ type AgentRequest struct {
 	// toolUseID is the per-block id from the assistant message; pairs with
 	// the toolUseID delivered by OnToolResult.
 	OnToolUse func(toolUseID, name, input string) `json:"-"`
+	// OnToolUseRaw sees the same invocations as OnToolUse, but rawInput is the
+	// tool's argument JSON verbatim instead of the chat-facing summary — which
+	// is lossy, and empty for tools whose schema the summarizer doesn't know.
+	// Set this (not OnToolUse) when the arguments have to be decoded.
+	OnToolUseRaw func(toolUseID, name, rawInput string) `json:"-"`
 	// OnActivity is called for model detection and system events (subagent progress).
 	OnActivity func(activity, detail string) `json:"-"`
 	// OnThinking is called for each "thinking" content block emitted by the
diff --git a/internal/api/review_handler.go b/internal/api/review_handler.go
index aa40cdf6..ebbcbb52 100644
--- a/internal/api/review_handler.go
+++ b/internal/api/review_handler.go
@@ -743,17 +743,23 @@ func (s *reviewService) handleReviewRun(w http.ResponseWriter, r *http.Request)
 const defaultReviewPrompt = `/code-review`
 
 // defaultReviewSystemPrompt carries the review-panel output contract when
-// the prompt is the bare /code-review slash command: findings are reported
-// through Loop's report_review_findings MCP tool, which POSTs them straight
-// into the daemon's review session (persist + broadcast to the panel). The
-// explicit ReportFindings ban is load-bearing: the skill's own instructions
-// steer the model toward that harness tool, which exists even in --print
-// runs — a successful call would swallow every finding into a channel Loop
-// never reads, so the run would look clean with zero comments. Belt and
-// braces, the tool is also stripped via claude_batch_disallowed_tools.
-const defaultReviewSystemPrompt = `You are reviewing a GitHub pull request for an external review pipeline. When the review completes, report every finding by calling the mcp__loop__report_review_findings tool once with the full list. Each finding needs the repo-relative path, the 1-based line, side (RIGHT for added/modified lines — the default; LEFT only for lines removed from the base), and a one-paragraph body: the bug, the concrete inputs/state that trigger it, and the wrong output or crash.
-
-Report findings ONLY through that tool — do not use the ReportFindings tool, do not emit XML blocks, and do not print the findings as your reply. If there are no findings, skip the tool call. Do not fix anything yourself — the user triages comments from the Review panel.`
+// the prompt is the bare /code-review slash command. The command reports
+// through Claude Code's own ReportFindings tool, and a review run enables
+// it (agent.AgentRequest.ReviewMode) precisely so it can: Loop reads the
+// tool_use straight off the agent's stream and ingests each finding. So
+// this prompt reinforces the skill's own contract instead of competing
+// with it — steering the model to Loop's MCP tool here would fight the
+// skill's instructions, and pulling ReportFindings back out would make
+// the command fork into a silent subagent again.
+//
+// The `line` requirement is the one addition that matters: ReportFindings
+// treats line as optional, but a finding without one can't be placed in
+// the diff and gets dropped on ingest.
+const defaultReviewSystemPrompt = `You are reviewing a GitHub pull request for an external review pipeline. Report every finding by calling the ReportFindings tool once with the full list.
+
+Every finding MUST carry a repo-relative ` + "`file`" + ` and a 1-based ` + "`line`" + ` in the current revision — a finding without a line is discarded, so pick the most relevant line rather than omitting it. Write ` + "`summary`" + ` as the one-line defect and ` + "`failure_scenario`" + ` as the concrete inputs or state that trigger it plus the wrong output or crash; both are shown to the reviewer.
+
+Do not print the findings as your reply and do not emit XML blocks — the tool call is the only channel that reaches the reviewer. If there are no findings, skip the call. Do not fix anything yourself: the user triages comments from the Review panel.`
 
 // buildReviewContext renders the per-PR context block appended to the
 // configured review prompt. Each known field gets its own labelled line so
diff --git a/internal/api/review_handler_test.go b/internal/api/review_handler_test.go
index 0877a13a..c9378b75 100644
--- a/internal/api/review_handler_test.go
+++ b/internal/api/review_handler_test.go
@@ -1324,10 +1324,13 @@ type mockReviewRunner struct {
 	// runWithCtxFn, when set, takes precedence over runFn so tests can
 	// observe ctx cancellation (used by the runReviewAsync timeout test).
 	runWithCtxFn func(ctx context.Context) (*agent.AgentResponse, error)
-	done         chan struct{} // closed after Run returns
+	// findings, when set, are handed to the run's onComment callback the
+	// way the agent's ReportFindings tool_use would deliver them.
+	findings []*review.Comment
+	done     chan struct{} // closed after Run returns
 }
 
-func (m *mockReviewRunner) Run(ctx context.Context, _, dirPath, parentDirPath, systemPrompt, prompt string) (*agent.AgentResponse, error) {
+func (m *mockReviewRunner) Run(ctx context.Context, _, dirPath, parentDirPath, systemPrompt, prompt string, onComment func(*review.Comment)) (*agent.AgentResponse, error) {
 	m.mu.Lock()
 	m.calls++
 	m.lastDir = dirPath
@@ -1337,7 +1340,13 @@ func (m *mockReviewRunner) Run(ctx context.Context, _, dirPath, parentDirPath, s
 	ctxFn := m.runWithCtxFn
 	fn := m.runFn
 	done := m.done
+	findings := m.findings
 	m.mu.Unlock()
+	if onComment != nil {
+		for _, c := range findings {
+			onComment(c)
+		}
+	}
 	defer func() {
 		if done != nil {
 			close(done)
@@ -1561,6 +1570,34 @@ func (s *ReviewHandlerSuite) TestIngestCommentsLoopDirFallback() {
 	require.Equal(s.T(), "LEFT", s.rs.Get("ch1").Comments[0].Side)
 }
 
+// Findings the agent reports through the built-in ReportFindings tool
+// reach the panel via the run's onComment callback — the same ingest path
+// the report_review_findings MCP endpoint uses, so duplicates collapse.
+func (s *ReviewHandlerSuite) TestRunIngestsStreamedFindings() {
+	s.wireReadySession()
+	s.store.On("GetChannel", mock.Anything, "ch1").Return(&db.Channel{ChannelID: "ch1", DirPath: "/repo"}, nil).Maybe()
+	runner := &mockReviewRunner{
+		done: make(chan struct{}),
+		findings: []*review.Comment{
+			review.NewComment("a.go", 3, "", "leak\n\nfd stays open"),
+			review.NewComment("a.go", 3, "", "leak\n\nfd stays open"), // same id -> deduped
+			review.NewComment("b.go", 9, "", "panic\n\nnil deref"),
+		},
+	}
+	s.srv.review.setAgent(runner, "", "")
+
+	w := httptest.NewRecorder()
+	s.mux.ServeHTTP(w, httptest.NewRequest("POST", "/api/channels/ch1/review/run", nil))
+	require.Equal(s.T(), http.StatusAccepted, w.Code)
+	<-runner.done
+
+	comments := s.rs.Get("ch1").Comments
+	require.Len(s.T(), comments, 2)
+	require.Equal(s.T(), "a.go", comments[0].Path)
+	require.Equal(s.T(), "leak\n\nfd stays open", comments[0].Body)
+	require.Equal(s.T(), "b.go", comments[1].Path)
+}
+
 // When a gh_user is configured, the run prompt includes a switch hint so
 // the agent can `gh auth switch -u <user>` before running gh commands.
 func (s *ReviewHandlerSuite) TestRunPromptIncludesConfiguredGHUser() {
@@ -1651,8 +1688,8 @@ func (s *ReviewHandlerSuite) TestRunUsesDefaultPromptWhenUnconfigured() {
 	require.Equal(s.T(), http.StatusAccepted, w.Code)
 	<-runner.done
 	require.Equal(s.T(), defaultReviewPrompt, runner.lastUser)
-	require.Contains(s.T(), runner.lastSys, "report_review_findings")
-	require.Contains(s.T(), runner.lastSys, "do not use the ReportFindings tool")
+	require.Contains(s.T(), runner.lastSys, "calling the ReportFindings tool")
+	require.Contains(s.T(), runner.lastSys, "1-based `line`")
 	require.Contains(s.T(), runner.lastSys, "Pull request under review:")
 }
 
diff --git a/internal/api/review_service.go b/internal/api/review_service.go
index e70c7ba2..2f504858 100644
--- a/internal/api/review_service.go
+++ b/internal/api/review_service.go
@@ -56,13 +56,15 @@ type GitHubReview interface {
 	DeletePRReviewComment(ctx context.Context, workdir, ghUser string, slug githubapi.RepoSlug, commentID int64) error
 }
 
-// ReviewRunner kicks off a single review pass. Findings arrive out of
-// band: the agent reports them through the report_review_findings MCP
-// tool, which POSTs to the review-comments endpoint (ingestComment).
-// Satisfied by *review.Runner; held as an interface so tests can drive
-// the handler without a real agent container.
+// ReviewRunner kicks off a single review pass. Findings arrive either
+// through onComment — the built-in ReportFindings tool, picked off the
+// agent's stream as it runs — or out of band through the
+// report_review_findings MCP tool, which POSTs to the review-comments
+// endpoint. Both funnel into ingestComment. Satisfied by *review.Runner;
+// held as an interface so tests can drive the handler without a real
+// agent container.
 type ReviewRunner interface {
-	Run(ctx context.Context, channelID, dirPath, parentDirPath, systemPrompt, prompt string) (*agent.AgentResponse, error)
+	Run(ctx context.Context, channelID, dirPath, parentDirPath, systemPrompt, prompt string, onComment func(*review.Comment)) (*agent.AgentResponse, error)
 }
 
 // refreshReviewSession fast-forwards the worktree to the PR's current
@@ -223,7 +225,14 @@ func (s *reviewService) runReviewAsync(runCtx context.Context, channelID, worktr
 		ctx, cancel = context.WithTimeout(ctx, s.runTimeout)
 		defer cancel()
 	}
-	_, err := s.runner.Run(ctx, channelID, worktreePath, parentDirPath, systemPrompt, prompt)
+	// Findings stream in as the agent reports them, so the panel fills up
+	// during the run rather than all at once at the end. ingestComment is
+	// safe to call concurrently and dedups by stable comment id, so a
+	// finding that also arrives over MCP lands once.
+	onComment := func(c *review.Comment) {
+		s.ingestComment(channelID, worktreePath, parentDirPath, c)
+	}
+	_, err := s.runner.Run(ctx, channelID, worktreePath, parentDirPath, systemPrompt, prompt, onComment)
 	if err != nil {
 		msg := err.Error()
 		// Re-shape ctx-deadline into a user-readable message. errors.Is
diff --git a/internal/config/config.go b/internal/config/config.go
index a11842a2..f24e36ca 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -511,9 +511,11 @@ func stringDefault(val, def string) string {
 // watch dies mid-stream, dropping the remaining events. Override via the
 // `claude_batch_disallowed_tools` config key (global/project/worktree).
 // ReportFindings is denied for a different reason: it reports code-review
-// findings into a harness UI channel that batch runs don't render — a
-// successful call swallows the findings invisibly. Loop's review flow
-// reports through the report_review_findings MCP tool instead.
+// findings into a harness UI channel that batch runs don't render, so an
+// ordinary batch run that calls it swallows the findings invisibly. Review
+// runs are the exception and re-enable it: the built-in /code-review command
+// only runs inline when it can call that tool, and the review runner reads
+// the findings off the stream — see agent.AgentRequest.ReviewMode.
 func DefaultBatchDisallowedTools() []string {
 	return []string{"ScheduleWakeup", "CronCreate", "CronDelete", "CronList", "Monitor", "ReportFindings"}
 }
diff --git a/internal/container/claude_cmd.go b/internal/container/claude_cmd.go
index 49fb2982..27cae922 100644
--- a/internal/container/claude_cmd.go
+++ b/internal/container/claude_cmd.go
@@ -56,11 +56,40 @@ func buildBaseClaudeCmd(cfg *config.Config, mcpConfigPath, sessionID, agentID st
 // restrictions) from the next turn onward.
 const planModePromptPrefix = "Call the EnterPlanMode tool before doing anything else, then follow the plan-mode instructions that follow.\n\n"
 
+// reportFindingsTool is Claude Code's built-in code-review reporting tool.
+// Denied by default in batch runs (see config.DefaultBatchDisallowedTools),
+// re-enabled for review runs.
+const reportFindingsTool = "ReportFindings"
+
+// withoutTool returns tools with name removed, preserving order. The input
+// slice is never mutated — it belongs to the cached config.
+func withoutTool(tools []string, name string) []string {
+	out := make([]string, 0, len(tools))
+	for _, t := range tools {
+		if t != name {
+			out = append(out, t)
+		}
+	}
+	return out
+}
+
+// reviewModeSettings is the --settings payload for a review run. It carries
+// env, not container env vars, because Claude Code applies each settings
+// scope over process.env with a plain assign, in the fixed order
+// userSettings → flagSettings → policySettings. A container-level env var is
+// therefore *lower* precedence than ~/.claude/settings.json, which Loop
+// bind-mounts from the host into every agent container: a host setting of
+// e.g. CLAUDE_CODE_SUBAGENT_MODEL=sonnet silently overwrote it. --settings is
+// the flagSettings scope, so it lands after the user's file and wins.
+//
+// Both keys are load-bearing; see agent.AgentRequest.ReviewMode.
+const reviewModeSettings = `{"env":{"CLAUDE_CODE_REPORT_FINDINGS":"1","CLAUDE_CODE_SUBAGENT_MODEL":"inherit"}}`
+
 // buildClaudeCmd assembles the Claude CLI command with all flags for batch mode.
 func buildClaudeCmd(cfg *config.Config, mcpConfigPath string, req *agent.AgentRequest) []string {
 	// Per-channel on-demand overrides beat the merged config's model/effort.
 	// Shallow-copy so the cached config is never mutated.
-	if req.Model != "" || req.Effort != "" {
+	if req.Model != "" || req.Effort != "" || req.ReviewMode {
 		override := *cfg
 		if req.Model != "" {
 			override.ClaudeModel = req.Model
@@ -68,9 +97,19 @@ func buildClaudeCmd(cfg *config.Config, mcpConfigPath string, req *agent.AgentRe
 		if req.Effort != "" {
 			override.ClaudeEffort = req.Effort
 		}
+		// A review run is the one batch case that wants ReportFindings: it's
+		// how the built-in code-review command hands its findings back, and
+		// keeping it available is half of what makes that command run inline
+		// rather than fork. See agent.AgentRequest.ReviewMode.
+		if req.ReviewMode {
+			override.ClaudeBatchDisallowedTools = withoutTool(cfg.ClaudeBatchDisallowedTools, reportFindingsTool)
+		}
 		cfg = &override
 	}
 	cmd := buildBaseClaudeCmd(cfg, mcpConfigPath, req.SessionID, req.AgentID, req.ForkSession, false, cfg.ExtraDirs)
+	if req.ReviewMode {
+		cmd = append(cmd, "--settings", reviewModeSettings)
+	}
 	// Deny tools that only make sense in a persistent interactive harness.
 	// In one-shot `--print` mode the container exits at end of turn, so tools
 	// like ScheduleWakeup / Cron* schedule re-invocations that never fire —
diff --git a/internal/container/runner.go b/internal/container/runner.go
index a6b16a5d..926ea09f 100644
--- a/internal/container/runner.go
+++ b/internal/container/runner.go
@@ -486,6 +486,7 @@ func (r *DockerRunner) runOnce(ctx context.Context, req *agent.AgentRequest) (*a
 	claudeResp, err := r.collectOutput(ctx, containerID, streamCallbacks{
 		onTurn:       req.OnTurn,
 		onToolUse:    req.OnToolUse,
+		onToolUseRaw: req.OnToolUseRaw,
 		onActivity:   req.OnActivity,
 		onThinking:   req.OnThinking,
 		onToolResult: req.OnToolResult,
@@ -728,7 +729,7 @@ func (r *DockerRunner) createAndStartContainer(
 // collectOutput reads container logs (streaming or batch) and waits for exit.
 // Returns the parsed Claude response or an error.
 func (r *DockerRunner) collectOutput(ctx context.Context, containerID string, cb streamCallbacks) (*claudeResponse, error) {
-	if cb.onTurn != nil || cb.onThinking != nil || cb.onToolResult != nil {
+	if cb.any() {
 		return r.collectStreamingOutput(ctx, containerID, cb)
 	}
 	return r.collectBatchOutput(ctx, containerID)
diff --git a/internal/container/runner_mounts_test.go b/internal/container/runner_mounts_test.go
index 25017a47..ece24525 100644
--- a/internal/container/runner_mounts_test.go
+++ b/internal/container/runner_mounts_test.go
@@ -430,6 +430,37 @@ func (s *RunnerSuite) TestRunClaudeModelConfig() {
 	}
 }
 
+// A review run carries its two overrides on the claude command line, not in
+// the container env. Container env is the losing end of Claude Code's
+// settings precedence: every settings scope is assigned over process.env, so
+// the bind-mounted ~/.claude/settings.json here would win and pin subagents
+// to sonnet. The config's own env still reaches the container untouched —
+// review mode routes around it rather than fighting it there.
+func (s *RunnerSuite) TestRunReviewModeSettings() {
+	s.client = new(MockDockerClient)
+	s.cfg.Envs = map[string]string{"CLAUDE_CODE_SUBAGENT_MODEL": "sonnet"}
+	s.runner = NewDockerRunner(s.client, s.cfg, nil)
+	s.applyMockDefaults()
+
+	ctx := context.Background()
+	req := &agent.AgentRequest{
+		Messages:   []agent.AgentMessage{{Role: "user", Content: "/code-review"}},
+		ChannelID:  "ch-1",
+		ReviewMode: true,
+	}
+	check := func(cfg *ContainerConfig) bool {
+		return slices.Contains(cfg.Cmd, reviewModeSettings) &&
+			findEnv(cfg.Env, "CLAUDE_CODE_SUBAGENT_MODEL") == "sonnet" &&
+			findEnv(cfg.Env, "CLAUDE_CODE_REPORT_FINDINGS") == ""
+	}
+	s.setupMockRun(ctx, mock.MatchedBy(check), testContainerName, testJSONOK)
+
+	resp, err := s.runner.Run(ctx, req)
+	require.NoError(s.T(), err)
+	require.Equal(s.T(), "ok", resp.Response)
+	s.client.AssertExpectations(s.T())
+}
+
 func (s *RunnerSuite) TestRunClaudeEffortConfig() {
 	tests := []struct {
 		name        string
diff --git a/internal/container/runner_retry_test.go b/internal/container/runner_retry_test.go
index 79d8379d..8eeb4dd8 100644
--- a/internal/container/runner_retry_test.go
+++ b/internal/container/runner_retry_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"io"
 	"sync/atomic"
 	"time"
 
@@ -14,20 +15,23 @@ import (
 	"github.com/radutopala/loop/internal/config"
 )
 
-// setupMockAttempts wires N sequential non-streaming container runs, each
-// returning the corresponding jsonOutput. Distinct container IDs per attempt
-// keep the ContainerLogs/Wait expectations unambiguous. All attempts share the
-// deterministic container name from osRandRead.
+// setupMockAttempts wires N sequential container runs, each returning the
+// corresponding jsonOutput. Distinct container IDs per attempt keep the
+// logs/Wait expectations unambiguous. All attempts share the deterministic
+// container name from osRandRead. Whether a run reads the logs once at exit
+// or follows them live is up to the request's callbacks (streamCallbacks.any),
+// so both readers are wired and each is optional; the tests assert on the
+// parsed response, which only a consumed stream can produce.
 func (s *RunnerSuite) setupMockAttempts(ctx context.Context, jsonOutputs ...string) {
 	s.client.On("NetworkEnsure", ctx, mock.Anything).Maybe().Return(nil)
 	for i, out := range jsonOutputs {
 		cid := fmt.Sprintf("container-attempt-%d", i)
-		reader := bytes.NewReader([]byte(out))
 		waitCh := make(chan WaitResponse, 1)
 		waitCh <- WaitResponse{StatusCode: 0}
 		errCh := make(chan error, 1)
 		s.client.On("ContainerCreate", ctx, mock.MatchedBy(func(*ContainerConfig) bool { return true }), testContainerName).Return(cid, nil).Once()
-		s.client.On("ContainerLogs", ctx, cid).Return(reader, nil)
+		s.client.On("ContainerLogs", ctx, cid).Maybe().Return(bytes.NewReader([]byte(out)), nil)
+		s.client.On("ContainerLogsFollow", ctx, cid).Maybe().Return(io.NopCloser(bytes.NewReader([]byte(out))), nil)
 		s.client.On("ContainerStart", ctx, cid).Return(nil)
 		s.client.On("ContainerWait", ctx, cid).Return((<-chan WaitResponse)(waitCh), (<-chan error)(errCh))
 	}
diff --git a/internal/container/runner_shell_test.go b/internal/container/runner_shell_test.go
index d01993aa..e5b69db3 100644
--- a/internal/container/runner_shell_test.go
+++ b/internal/container/runner_shell_test.go
@@ -3,6 +3,7 @@ package container
 import (
 	"archive/tar"
 	"context"
+	"encoding/json"
 	"errors"
 	"io"
 	"os"
@@ -59,6 +60,62 @@ func (s *RunnerSuite) TestRunWithOnTurnStreaming() {
 	s.client.AssertExpectations(s.T())
 }
 
+// A review request sets OnToolUseRaw and nothing else. Before collectOutput
+// asked streamCallbacks.any(), that shape fell through to the batch read,
+// which drops every callback — so ReportFindings findings never reached the
+// review panel even though the tool call was right there on the stream.
+func (s *RunnerSuite) TestRunWithOnlyOnToolUseRawStreams() {
+	ctx := context.Background()
+
+	var raws []string
+	req := &agent.AgentRequest{
+		ChannelID: "ch-1",
+		OnToolUseRaw: func(_, name, rawInput string) {
+			raws = append(raws, name+":"+rawInput)
+		},
+	}
+
+	streamOutput := `{"type":"assistant","message":{"content":[{"type":"tool_use","id":"tu-1","name":"ReportFindings","input":{"findings":[{"file":"a.go","line":2}]}}]}}
+{"type":"result","result":"done","session_id":"sess-raw","is_error":false}
+`
+
+	waitCh := make(chan WaitResponse, 1)
+	waitCh <- WaitResponse{StatusCode: 0}
+	errCh := make(chan error, 1)
+
+	s.client.On("ContainerCreate", ctx, mock.AnythingOfType("*container.ContainerConfig"), testContainerName).Return(testContainerID, nil)
+	s.client.On("ContainerStart", ctx, testContainerID).Return(nil)
+	s.client.On("ContainerLogsFollow", ctx, testContainerID).Return(io.NopCloser(strings.NewReader(streamOutput)), nil)
+	s.client.On("ContainerWait", ctx, testContainerID).Return((<-chan WaitResponse)(waitCh), (<-chan error)(errCh))
+
+	_, err := s.runner.Run(ctx, req)
+	require.NoError(s.T(), err)
+	require.Equal(s.T(), []string{`ReportFindings:{"findings":[{"file":"a.go","line":2}]}`}, raws)
+	s.client.AssertNotCalled(s.T(), "ContainerLogs", mock.Anything, mock.Anything)
+	s.client.AssertExpectations(s.T())
+}
+
+func (s *RunnerSuite) TestStreamCallbacksAny() {
+	tests := []struct {
+		name string
+		cb   streamCallbacks
+		want bool
+	}{
+		{"empty", streamCallbacks{}, false},
+		{"onTurn", streamCallbacks{onTurn: func(string) {}}, true},
+		{"onToolUse", streamCallbacks{onToolUse: func(_, _, _ string) {}}, true},
+		{"onToolUseRaw", streamCallbacks{onToolUseRaw: func(_, _, _ string) {}}, true},
+		{"onActivity", streamCallbacks{onActivity: func(_, _ string) {}}, true},
+		{"onThinking", streamCallbacks{onThinking: func(string) {}}, true},
+		{"onToolResult", streamCallbacks{onToolResult: func(_, _ string, _ bool) {}}, true},
+	}
+	for _, tt := range tests {
+		s.Run(tt.name, func() {
+			require.Equal(s.T(), tt.want, tt.cb.any())
+		})
+	}
+}
+
 func (s *RunnerSuite) TestRunWithOnTurnFollowError() {
 	ctx := context.Background()
 	req := &agent.AgentRequest{
@@ -683,6 +740,50 @@ func (s *RunnerSuite) TestBuildClaudeCmdModelEffortOverride() {
 	require.Equal(s.T(), "medium", cfg.ClaudeEffort)
 }
 
+// TestBuildClaudeCmdReviewMode verifies a review run keeps ReportFindings
+// available (the built-in code-review command only runs inline — same
+// session, same model, streamed output — when it can call that tool) while
+// the other batch denials stand, and that the shared config is untouched.
+func (s *RunnerSuite) TestBuildClaudeCmdReviewMode() {
+	disallowed := config.DefaultBatchDisallowedTools()
+	cfg := &config.Config{ClaudeBinPath: "claude", ClaudeBatchDisallowedTools: disallowed}
+	req := &agent.AgentRequest{
+		ChannelID: "ch-1",
+		Messages:  []agent.AgentMessage{{Role: "user", Content: "/code-review"}},
+	}
+
+	got := strings.Join(buildClaudeCmd(cfg, "/work/.loop/mcp-ch-1.json", req), " ")
+	require.Contains(s.T(), got, "ReportFindings")
+	require.NotContains(s.T(), got, "--settings")
+
+	req.ReviewMode = true
+	cmd := buildClaudeCmd(cfg, "/work/.loop/mcp-ch-1.json", req)
+	got = strings.Join(cmd, " ")
+	require.NotContains(s.T(), got, "ReportFindings")
+	require.Contains(s.T(), got, "--disallowedTools ScheduleWakeup,CronCreate,CronDelete,CronList,Monitor")
+	require.Equal(s.T(), disallowed, cfg.ClaudeBatchDisallowedTools)
+
+	// --settings, not container env: settings scopes are assigned over
+	// process.env in the order userSettings → flagSettings, so a bind-mounted
+	// ~/.claude/settings.json outranks anything the container exports.
+	i := slices.Index(cmd, "--settings")
+	require.NotEqual(s.T(), -1, i, "review runs pass --settings")
+	require.Equal(s.T(), reviewModeSettings, cmd[i+1])
+
+	var settings struct {
+		Env map[string]string `json:"env"`
+	}
+	require.NoError(s.T(), json.Unmarshal([]byte(reviewModeSettings), &settings))
+	require.Equal(s.T(), map[string]string{
+		"CLAUDE_CODE_REPORT_FINDINGS": "1",
+		"CLAUDE_CODE_SUBAGENT_MODEL":  "inherit",
+	}, settings.Env)
+
+	// The flag must precede --print: --disallowedTools is variadic and would
+	// otherwise swallow it.
+	require.Less(s.T(), i, slices.Index(cmd, "--print"))
+}
+
 func (s *RunnerSuite) TestBuildClaudeCmdPermissionPromptTool() {
 	cfg := &config.Config{ClaudeBinPath: "claude"}
 	req := &agent.AgentRequest{
diff --git a/internal/container/runner_stream_test.go b/internal/container/runner_stream_test.go
index 891af2ba..5af9551f 100644
--- a/internal/container/runner_stream_test.go
+++ b/internal/container/runner_stream_test.go
@@ -242,6 +242,40 @@ func TestScanStreamJSONOnToolUse(t *testing.T) {
 	require.Equal(t, []string{"Bash:go test"}, tools)
 }
 
+// onToolUseRaw gets the input JSON untouched. The ReportFindings case is the
+// reason it exists: summarizeToolInput has no case for it and none of the
+// fallback keys match, so onToolUse alone hands the consumer an empty string.
+func TestScanStreamJSONOnToolUseRaw(t *testing.T) {
+	input := `{"type":"assistant","message":{"content":[{"type":"tool_use","id":"tu-1","name":"ReportFindings","input":{"findings":[{"file":"a.go","line":2}]}}]}}
+{"type":"result","result":"OK","session_id":"s1","is_error":false}
+`
+	var summaries, raws []string
+	cb := streamCallbacks{
+		onToolUse:    func(_, name, in string) { summaries = append(summaries, name+":"+in) },
+		onToolUseRaw: func(id, name, raw string) { raws = append(raws, id+":"+name+":"+raw) },
+	}
+	resp, err := scanStreamJSON(strings.NewReader(input), cb)
+	require.NoError(t, err)
+	require.Equal(t, "OK", resp.Result)
+	require.Equal(t, []string{"ReportFindings:"}, summaries)
+	require.Equal(t, []string{`tu-1:ReportFindings:{"findings":[{"file":"a.go","line":2}]}`}, raws)
+}
+
+// The raw callback still fires when onToolUse is unset — the two are dispatched
+// from one loop guarded by "either is set".
+func TestScanStreamJSONOnToolUseRawOnly(t *testing.T) {
+	input := `{"type":"assistant","message":{"content":[{"type":"tool_use","id":"tu-1","name":"Bash","input":{"command":"ls"}}]}}
+{"type":"result","result":"OK","session_id":"s1","is_error":false}
+`
+	var raws []string
+	cb := streamCallbacks{
+		onToolUseRaw: func(_, name, raw string) { raws = append(raws, name+":"+raw) },
+	}
+	_, err := scanStreamJSON(strings.NewReader(input), cb)
+	require.NoError(t, err)
+	require.Equal(t, []string{`Bash:{"command":"ls"}`}, raws)
+}
+
 func TestScanStreamJSONOnActivity(t *testing.T) {
 	t.Run("model detected from assistant events", func(t *testing.T) {
 		input := `{"type":"assistant","message":{"model":"claude-opus-4-6","content":[{"type":"text","text":"Hello"}]}}
diff --git a/internal/container/stream.go b/internal/container/stream.go
index 259f5ce7..9266b58d 100644
--- a/internal/container/stream.go
+++ b/internal/container/stream.go
@@ -90,15 +90,19 @@ type ToolUse struct {
 	ID    string // per-block tool_use id, pairs with the matching tool_result
 	Name  string
 	Input string // short summary of the input
+	Raw   string // the tool input JSON, verbatim
 }
 
 // extractToolUses returns tool_use content blocks from an assistant message.
+// Input is a chat-facing summary and is lossy — summarizeToolInput returns ""
+// for any tool whose schema it doesn't know. Raw keeps the untouched JSON for
+// callers that need to decode the arguments (see onToolUseRaw).
 func (m *assistantMessage) extractToolUses() []ToolUse {
 	var tools []ToolUse
 	for _, c := range m.Message.Content {
 		if c.Type == "tool_use" && c.Name != "" {
 			summary := summarizeToolInput(c.Name, c.Input)
-			tools = append(tools, ToolUse{ID: c.ID, Name: c.Name, Input: summary})
+			tools = append(tools, ToolUse{ID: c.ID, Name: c.Name, Input: summary, Raw: string(c.Input)})
 		}
 	}
 	return tools
@@ -169,13 +173,28 @@ func summarizeToolInput(name string, raw json.RawMessage) string {
 
 // streamCallbacks holds optional callbacks for scanStreamJSON.
 type streamCallbacks struct {
-	onTurn       func(string)
-	onToolUse    func(toolUseID, name, input string)
+	onTurn    func(string)
+	onToolUse func(toolUseID, name, input string)
+	// onToolUseRaw sees the same tool_use blocks as onToolUse but with the
+	// input JSON verbatim rather than summarized, for callers that decode it.
+	onToolUseRaw func(toolUseID, name, rawInput string)
 	onActivity   func(activity, detail string)
 	onThinking   func(text string)
 	onToolResult func(toolUseID, output string, isError bool)
 }
 
+// any reports whether at least one callback is set. collectOutput uses it to
+// pick between following the container's logs live and reading them once at
+// exit — with nothing to deliver mid-run the cheaper batch read wins, and it
+// drops the callbacks entirely. Every field must be listed: a callback left
+// out here is silently never invoked for a request that sets only that one.
+// A review request sets only onToolUseRaw, and that is exactly how the
+// ReportFindings findings went missing.
+func (cb streamCallbacks) any() bool {
+	return cb.onTurn != nil || cb.onToolUse != nil || cb.onToolUseRaw != nil ||
+		cb.onActivity != nil || cb.onThinking != nil || cb.onToolResult != nil
+}
+
 // userEventMaxBytes caps the size of "user" stream-json lines we will fully
 // read. Above this we drain the line (without buffering it) — this protects
 // against multi-MB screenshot tool_results. Below it, we parse the line so
@@ -343,9 +362,14 @@ func scanStreamJSON(r io.Reader, cb streamCallbacks) (*claudeResponse, error) {
 					cb.onThinking(text)
 				}
 			}
-			if cb.onToolUse != nil {
+			if cb.onToolUse != nil || cb.onToolUseRaw != nil {
 				for _, tu := range msg.extractToolUses() {
-					cb.onToolUse(tu.ID, tu.Name, tu.Input)
+					if cb.onToolUse != nil {
+						cb.onToolUse(tu.ID, tu.Name, tu.Input)
+					}
+					if cb.onToolUseRaw != nil {
+						cb.onToolUseRaw(tu.ID, tu.Name, tu.Raw)
+					}
 				}
 			}
 		case "user":
diff --git a/internal/review/parser.go b/internal/review/parser.go
index ad870083..d27d1a8c 100644
--- a/internal/review/parser.go
+++ b/internal/review/parser.go
@@ -3,10 +3,57 @@ package review
 import (
 	"crypto/sha1"
 	"encoding/hex"
+	"encoding/json"
 	"fmt"
 	"strings"
 )
 
+// ReportFindingsTool is the built-in Claude Code tool the code-review
+// command calls to hand its findings to the host. Loop watches the agent's
+// stream for a tool_use block with this name and turns its input into
+// review comments — see ParseReportFindings.
+const ReportFindingsTool = "ReportFindings"
+
+// reportFindings mirrors the ReportFindings tool's input schema. Only the
+// fields the review panel renders are decoded; `level`, `short_summary`,
+// `category`, `verdict` and `outcome` are ignored.
+type reportFindings struct {
+	Findings []struct {
+		File            string `json:"file"`
+		Line            int    `json:"line"`
+		Summary         string `json:"summary"`
+		FailureScenario string `json:"failure_scenario"`
+	} `json:"findings"`
+}
+
+// ParseReportFindings converts a ReportFindings tool_use input into review
+// comments. Findings the panel can't place — no file, no positive line, no
+// text — are dropped rather than failing the batch, matching NewComment's
+// best-effort contract. A body pairs the one-line summary with the concrete
+// failure scenario, which is what the panel shows per comment. The tool
+// carries no LEFT/RIGHT side; every finding is reported against the head
+// revision, so NewComment's RIGHT default is correct.
+func ParseReportFindings(input string) []*Comment {
+	var parsed reportFindings
+	if err := json.Unmarshal([]byte(input), &parsed); err != nil {
+		return nil
+	}
+	comments := make([]*Comment, 0, len(parsed.Findings))
+	for _, f := range parsed.Findings {
+		body := strings.TrimSpace(f.Summary)
+		if scenario := strings.TrimSpace(f.FailureScenario); scenario != "" {
+			if body != "" {
+				body += "\n\n"
+			}
+			body += scenario
+		}
+		if c := NewComment(f.File, f.Line, "", body); c != nil {
+			comments = append(comments, c)
+		}
+	}
+	return comments
+}
+
 // NewComment builds a Comment from the raw finding fields the agent
 // reports through the report_review_findings MCP tool, normalizing side
 // (anything but LEFT becomes RIGHT) and deriving the stable dedup ID.
diff --git a/internal/review/parser_test.go b/internal/review/parser_test.go
index e4f0bed2..6fcbcf47 100644
--- a/internal/review/parser_test.go
+++ b/internal/review/parser_test.go
@@ -53,3 +53,60 @@ func (s *ParserSuite) TestNewCommentStableID() {
 	require.Equal(s.T(), a.ID, b.ID)
 	require.NotEqual(s.T(), a.ID, c.ID)
 }
+
+func (s *ParserSuite) TestParseReportFindingsBody() {
+	tests := []struct {
+		name    string
+		finding string
+		want    string
+	}{
+		{
+			name:    "summary and scenario",
+			finding: `{"file":"a.go","line":1,"summary":"leak","failure_scenario":"fd stays open"}`,
+			want:    "leak\n\nfd stays open",
+		},
+		{
+			name:    "summary only",
+			finding: `{"file":"a.go","line":1,"summary":"leak"}`,
+			want:    "leak",
+		},
+		{
+			name:    "scenario only",
+			finding: `{"file":"a.go","line":1,"failure_scenario":"fd stays open"}`,
+			want:    "fd stays open",
+		},
+	}
+	for _, tc := range tests {
+		s.Run(tc.name, func() {
+			got := ParseReportFindings(`{"findings":[` + tc.finding + `]}`)
+			require.Len(s.T(), got, 1)
+			require.Equal(s.T(), tc.want, got[0].Body)
+			// ReportFindings carries no side; findings are against head.
+			require.Equal(s.T(), "RIGHT", got[0].Side)
+		})
+	}
+}
+
+// A verbatim ReportFindings payload captured off a real review run. The
+// first implementation parsed hand-written fixtures fine but ingested
+// nothing in production, because the stream handed it a summarized (empty)
+// input rather than this JSON — so pin the real shape, extra keys included.
+func (s *ParserSuite) TestParseReportFindingsRealPayload() {
+	const raw = `{"findings":[{"file":"internal/idfree/memory_store.go","line":48,"summary":"File-monitor/reload machinery duplicated verbatim across store packages","short_summary":"Duplicated file-monitor/reload machinery","failure_scenario":"MemoryStore.loadFile/checkAndReload/Monitor plus the pollInterval/openFile/lastMod fields are a line-for-line copy of internal/offer and internal/publisher.","category":"reuse"},{"file":"internal/idfree/memory_store.go","line":180,"summary":"parseModels accepts a JSON top-level null, silently yielding an empty store","short_summary":"Top-level JSON null yields empty store silently","failure_scenario":"If the file content is the literal ` + "`null`" + `, json.Unmarshal succeeds leaving parsed nil with no error.","category":"correctness"}]}`
+
+	got := ParseReportFindings(raw)
+	require.Len(s.T(), got, 2)
+	require.Equal(s.T(), "internal/idfree/memory_store.go", got[0].Path)
+	require.Equal(s.T(), 48, got[0].Line)
+	require.Contains(s.T(), got[0].Body, "duplicated verbatim across store packages")
+	require.Contains(s.T(), got[0].Body, "line-for-line copy")
+	require.Equal(s.T(), 180, got[1].Line)
+	require.NotEqual(s.T(), got[0].ID, got[1].ID)
+}
+
+func (s *ParserSuite) TestParseReportFindingsRejects() {
+	require.Nil(s.T(), ParseReportFindings("{"), "malformed json")
+	require.Empty(s.T(), ParseReportFindings(`{"findings":[]}`), "no findings")
+	require.Empty(s.T(), ParseReportFindings(`{"findings":[{"file":"a.go","line":1}]}`), "no text")
+	require.Empty(s.T(), ParseReportFindings(`{"findings":[{"line":1,"summary":"x"}]}`), "no file")
+}
diff --git a/internal/review/runner.go b/internal/review/runner.go
index 855b74ec..8a033b05 100644
--- a/internal/review/runner.go
+++ b/internal/review/runner.go
@@ -15,10 +15,12 @@ type AgentRunner interface {
 }
 
 // Runner drives a single review pass: it builds an AgentRequest pointed
-// at the PR worktree and runs it to completion. Findings don't travel
-// through the agent's stdout — the agent reports them itself via the
-// report_review_findings MCP tool, which POSTs back into the daemon's
-// review-comments endpoint. Persistence and broadcasting live there.
+// at the PR worktree and runs it to completion. Findings reach the daemon
+// two ways, both landing in the same ingest path: the built-in
+// ReportFindings tool, intercepted live off the agent's stream (onComment
+// below), and — for a user-configured review prompt that doesn't run the
+// built-in command — the report_review_findings MCP tool, which POSTs
+// back into the daemon's review-comments endpoint.
 type Runner struct {
 	Agent AgentRunner
 }
@@ -31,8 +33,10 @@ type Runner struct {
 // the container, and the agent dies on startup. systemPrompt + prompt
 // are passed straight through to the agent (the caller is expected to
 // have resolved the configured review prompt and assembled the diff
-// context).
-func (r *Runner) Run(ctx context.Context, channelID, dirPath, parentDirPath, systemPrompt, prompt string) (*agent.AgentResponse, error) {
+// context). onComment, when set, receives each finding the agent reports
+// through the built-in ReportFindings tool, in the order reported; it runs
+// on the stream-reading goroutine, so it must not block for long.
+func (r *Runner) Run(ctx context.Context, channelID, dirPath, parentDirPath, systemPrompt, prompt string, onComment func(*Comment)) (*agent.AgentResponse, error) {
 	if r.Agent == nil {
 		return nil, errors.New("review runner: agent not configured")
 	}
@@ -42,6 +46,20 @@ func (r *Runner) Run(ctx context.Context, channelID, dirPath, parentDirPath, sys
 		ParentDirPath: parentDirPath,
 		SystemPrompt:  systemPrompt,
 		Prompt:        prompt,
+		ReviewMode:    true,
+	}
+	if onComment != nil {
+		// OnToolUseRaw, not OnToolUse: the latter carries a chat-facing
+		// summary, which is empty for ReportFindings because the summarizer
+		// has no case for it. Only the raw form has the findings to decode.
+		req.OnToolUseRaw = func(_, name, rawInput string) {
+			if name != ReportFindingsTool {
+				return
+			}
+			for _, c := range ParseReportFindings(rawInput) {
+				onComment(c)
+			}
+		}
 	}
 	return r.Agent.Run(ctx, req)
 }
diff --git a/internal/review/runner_test.go b/internal/review/runner_test.go
index f0da13bb..33aa232d 100644
--- a/internal/review/runner_test.go
+++ b/internal/review/runner_test.go
@@ -26,7 +26,7 @@ func TestRunnerSuite(t *testing.T) { suite.Run(t, new(RunnerSuite)) }
 
 func (s *RunnerSuite) TestRunNoAgentConfigured() {
 	r := &Runner{}
-	_, err := r.Run(context.Background(), "ch", "/dir", "/parent", "sys", "p")
+	_, err := r.Run(context.Background(), "ch", "/dir", "/parent", "sys", "p", nil)
 	require.ErrorContains(s.T(), err, "agent not configured")
 }
 
@@ -34,20 +34,73 @@ func (s *RunnerSuite) TestRunAgentErrorPropagates() {
 	a := new(mockAgentRunner)
 	a.On("Run", mock.Anything, mock.Anything).Return(nil, errors.New("boom"))
 	r := &Runner{Agent: a}
-	_, err := r.Run(context.Background(), "ch", "/dir", "/parent", "sys", "p")
+	_, err := r.Run(context.Background(), "ch", "/dir", "/parent", "sys", "p", nil)
 	require.ErrorContains(s.T(), err, "boom")
 }
 
+// A nil onComment leaves OnToolUse unset so the container runner keeps its
+// cheaper non-streaming path.
 func (s *RunnerSuite) TestRunBuildsRequest() {
 	a := new(mockAgentRunner)
 	a.On("Run", mock.Anything, mock.MatchedBy(func(req *agent.AgentRequest) bool {
 		return req.ChannelID == "ch" && req.DirPath == "/wt" && req.ParentDirPath == "/repo" &&
-			req.SystemPrompt == "sys" && req.Prompt == "p"
+			req.SystemPrompt == "sys" && req.Prompt == "p" && req.ReviewMode && req.OnToolUseRaw == nil
 	})).Return(&agent.AgentResponse{Response: "done"}, nil)
 
 	r := &Runner{Agent: a}
-	resp, err := r.Run(context.Background(), "ch", "/wt", "/repo", "sys", "p")
+	resp, err := r.Run(context.Background(), "ch", "/wt", "/repo", "sys", "p", nil)
 	require.NoError(s.T(), err)
 	require.Equal(s.T(), "done", resp.Response)
 	a.AssertExpectations(s.T())
 }
+
+// The ReportFindings tool_use is fanned out to onComment one comment at a
+// time; every other tool the agent uses is ignored.
+func (s *RunnerSuite) TestRunForwardsReportFindings() {
+	tests := []struct {
+		name     string
+		toolName string
+		input    string
+		want     []string
+	}{
+		{
+			name:     "report findings",
+			toolName: ReportFindingsTool,
+			input:    `{"level":"high","findings":[{"file":"a.go","line":3,"summary":"leak","failure_scenario":"fd stays open"},{"file":"b.go","line":9,"summary":"panic","failure_scenario":"nil deref"}]}`,
+			want:     []string{"leak\n\nfd stays open", "panic\n\nnil deref"},
+		},
+		{
+			name:     "unplaceable findings dropped",
+			toolName: ReportFindingsTool,
+			input:    `{"findings":[{"file":"a.go","summary":"no line"},{"file":"b.go","line":2,"summary":"kept","failure_scenario":"boom"}]}`,
+			want:     []string{"kept\n\nboom"},
+		},
+		{
+			name:     "other tool ignored",
+			toolName: "Bash",
+			input:    `{"command":"git diff"}`,
+		},
+		{
+			name:     "malformed input ignored",
+			toolName: ReportFindingsTool,
+			input:    "not json",
+		},
+	}
+	for _, tc := range tests {
+		s.Run(tc.name, func() {
+			a := new(mockAgentRunner)
+			a.On("Run", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
+				req, _ := args.Get(1).(*agent.AgentRequest)
+				req.OnToolUseRaw("tu-1", tc.toolName, tc.input)
+			}).Return(&agent.AgentResponse{}, nil)
+
+			var got []string
+			r := &Runner{Agent: a}
+			_, err := r.Run(context.Background(), "ch", "/wt", "/repo", "sys", "p", func(c *Comment) {
+				got = append(got, c.Body)
+			})
+			require.NoError(s.T(), err)
+			require.Equal(s.T(), tc.want, got)
+		})
+	}
+}