Reviewer-agent gate: independent adversarial review before a task surfaces to the operator

crates/voro/src/dispatch.rs

@@ -52,12 +52,49 @@ task; drop it for a proposal that stands on its own. Finish
 with your work committed on a branch and a PR-ready `--summary` on `done` — what
 changed, why, and how you verified it — since `voro pr` opens the pull request
 straight from that summary. Never modify the database with raw SQL, which would
-bypass the state machine and event log.{branch}{docs}
+bypass the state machine and event log.{branch}{docs}{review}
 
 ---
 
 ";
 
+/// The `{review}` block, rendered into every dispatched prompt (DESIGN.md §8).
+/// A writing agent's self-review shares the session's context and so ratifies
+/// its blind spots rather than catching them, which leaves the operator the
+/// first independent reader of every diff; a reviewer with no authorship context
+/// placed ahead of `done` spends the agent's tokens on that instead. The
+/// mechanism is named agent-agnostically for the same reason
+/// [`BRANCH_ISOLATE_SENTENCE`] is, and names the concrete tool as specifically:
+/// the harness's own subagent where it has one, a deliberate separate pass where
+/// it does not. Rendered last, after the linked documents it counts among the
+/// reviewer's inputs. Voro cannot verify the review happened (it is told, not
+/// observing), so this is a prompt-level mandate with the standing of the branch
+/// block.
+const REVIEW_GATE_TEMPLATE: &str = "\n\n\
+Before you call `voro done {task_id}{db}`, commit your work — the review below
+reads a diff — and have it read by a reviewer that carries no authorship
+context, one that did not watch you write it. Use your harness's own subagent
+mechanism for that where it has one — in Claude Code that is the `Agent` tool,
+which spawns a subagent on a context of its own, carrying only the brief you
+give it. Lacking such a mechanism, do the review as a deliberate separate pass
+over the review inputs alone, setting the assumptions you worked under aside.
+
+The reviewer gets two inputs and nothing else: this task body, together with any
+documents named above, and the diff of your work branch against the project's
+base branch. Not your notes, and not this session's transcript — a reader who
+sees only what the operator will see is the whole point. Its brief is
+adversarial: look for grounds to reject this work against the acceptance
+criteria in the task body, rather than for confirmation that it is done.
+
+Address every finding before you call `done` — fix it, or rebut it with
+evidence. One pass per `done`: do not send the fixes back for a second review.
+A finding you fixed needs no record, since the fix is in the diff; a finding you
+rebutted rather than fixed goes in the `--summary`, the finding and your
+rebuttal in a sentence each, because rejecting a reviewer's finding is a
+judgement the operator should see rather than one you settle alone. That summary
+is the body of the pull request `voro pr` opens, so write those lines for that
+reader too; it is otherwise the PR-ready account it always was.";
+
 /// The `{docs}` block for a task linked to plan or design documents (DESIGN.md
 /// §3/§8). Each is named at its resolved location — absolute for a path, since
 /// a linked doc may live in another project's checkout entirely — so the agent
@@ -276,6 +313,10 @@ fn render_preamble(
             ("{db}", db_flag.as_str()),
         ],
     );
+    let review_block = render(
+        REVIEW_GATE_TEMPLATE,
+        &[("{task_id}", task_id.as_str()), ("{db}", db_flag.as_str())],
+    );
     // A task with no linked document renders no block at all, so an unlinked
     // dispatch's prompt is byte-for-byte what it was before docs existed.
     let docs_block = if docs.is_empty() {
@@ -298,6 +339,7 @@ fn render_preamble(
         RETURN_PATH_PREAMBLE_TEMPLATE,
         &[
             ("{branch}", branch_block.as_str()),
+            ("{review}", review_block.as_str()),
             ("{docs}", docs_block.as_str()),
             ("{task_id}", task_id.as_str()),
             ("{db}", db_flag.as_str()),
@@ -1577,6 +1619,114 @@ mod tests {
         }
     }
 
+    /// The review gate's text, whitespace-flattened so an assertion reads a
+    /// phrase rather than the constant's hard line wrapping, and sliced from the
+    /// gate's own opening so an ordering assertion cannot match the branch
+    /// block's wording instead (both say "Lacking such a mechanism").
+    fn review_gate(rendered: &str) -> String {
+        let gate = &rendered[rendered
+            .find("Before you call `voro done")
+            .unwrap_or_else(|| panic!("no review gate in the preamble: {rendered}"))..];
+        gate.split_whitespace().collect::<Vec<_>>().join(" ")
+    }
+
+    #[test]
+    fn preamble_mandates_an_independent_review_before_done() {
+        // Every dispatched prompt carries the gate, whatever else the preamble
+        // renders: the self-review it replaces shares the session's blind spots,
+        // and the operator is otherwise the diff's first independent reader.
+        let docs = [("Plan".to_string(), "/tmp/plan.md".to_string())];
+        let scratch = PathBuf::from("/tmp/scratch/voro.db");
+        for db in [Store::default_db_path(), scratch] {
+            // every other rendered verb carries the store it must write to, so
+            // the gate's own `done` reference does too
+            let done = if db == Store::default_db_path() {
+                "voro done 62".to_string()
+            } else {
+                format!("voro done 62 --db {}", shell_quote(&db))
+            };
+            for branch in [None, Some("feat/parser")] {
+                for docs in [&[][..], &docs[..]] {
+                    let gate = review_gate(&render_preamble(62, &db, branch, docs));
+                    assert!(
+                        gate.starts_with(&format!("Before you call `{done}`")),
+                        "{gate}"
+                    );
+                    // committed first, or the reviewer reads an empty diff
+                    assert!(gate.contains("commit your work"), "{gate}");
+                    // harness subagent first, named as concretely as the
+                    // worktree tool is; manual pass second as the fallback
+                    let subagent = gate
+                        .find("in Claude Code that is the `Agent` tool")
+                        .unwrap();
+                    let manual = gate.find("Lacking such a mechanism").unwrap();
+                    assert!(subagent < manual, "{gate}");
+                    assert!(gate.contains("no authorship context"), "{gate}");
+                    // reviewer inputs: the brief and the branch diff, nothing else
+                    assert!(
+                        gate.contains(
+                            "this task body, together with any documents named above, and the \
+                             diff of your work branch against the project's base branch"
+                        ),
+                        "{gate}"
+                    );
+                    assert!(gate.contains("not this session's transcript"), "{gate}");
+                    // adversarial brief, not a confirmation pass
+                    assert!(gate.contains("brief is adversarial"), "{gate}");
+                    assert!(gate.contains("look for grounds to reject"), "{gate}");
+                    assert!(
+                        gate.contains("acceptance criteria in the task body"),
+                        "{gate}"
+                    );
+                    // fix or rebut every finding, one pass only
+                    assert!(gate.contains("fix it, or rebut it with evidence"), "{gate}");
+                    assert!(gate.contains("One pass per `done`"), "{gate}");
+                    // rebutted findings ride in the summary; fixed ones need no record
+                    assert!(
+                        gate.contains("rebutted rather than fixed goes in the `--summary`"),
+                        "{gate}"
+                    );
+                    assert!(gate.contains("needs no record"), "{gate}");
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn dispatched_prompt_puts_the_review_gate_ahead_of_the_task_body() {
+        let (mut store, ctx, project) = fixture("cat {prompt_file}");
+        let id = ready_task(&mut store, &project);
+
+        dispatch(&mut store, &ctx, id, None).unwrap();
+
+        let prompt = std::fs::read_to_string(prompt_files(&ctx).pop().unwrap()).unwrap();
+        let gate = prompt.find("Before you call `voro done").unwrap();
+        assert!(gate < prompt.find("# Do the thing").unwrap(), "{prompt}");
+    }
+
+    #[test]
+    fn planning_and_refine_prompts_carry_no_review_gate() {
+        // Their deliverable is a task body, not a diff, so there is nothing for
+        // an adversarial reviewer to read a branch against.
+        let db = Store::default_db_path();
+        let refine = |template| render_refine_prompt(template, 62, &db, "## seed\n", "tighten it");
+        let prompts = [
+            render_planning_prompt("augere", &db),
+            refine(REFINE_PROMPT_TEMPLATE),
+            refine(REFINE_PLAN_PROMPT_TEMPLATE),
+        ];
+        for prompt in prompts {
+            for marker in [
+                "Before you call `voro done",
+                "adversarial",
+                "no authorship context",
+                "subagent",
+            ] {
+                assert!(!prompt.contains(marker), "{marker} in: {prompt}");
+            }
+        }
+    }
+
     #[test]
     fn preamble_names_a_task_s_linked_documents_at_their_resolved_locations() {
         // A task linked to a plan gets it handed over rather than having to

docs/DESIGN.md

@@ -301,6 +301,8 @@ Naming the id literally is what makes the return path survive the launch style t
 
 **Linked documents** (§3) ride the same preamble mechanism as branch names, and for the same reason: the dispatcher already owns the prompt file, so the plan a task derives from can be handed over rather than left to be rediscovered from hints in the body. A task carrying document links renders an extra block naming each one at its *resolved* location, ahead of the body separator so it is read before the task itself — absolute for a path, because a linked document may live in another project's checkout entirely and a location relative to the session's working directory would point at nothing. A task with no links renders no block at all, so an unlinked dispatch's prompt is byte-for-byte what it was before documents existed. Voro neither reads nor parses the document: it names it, exactly as it names a branch, and what the agent does with it is the agent's business. Registering a document stays a CLI affair (`doc add`/`remove`, plus `--doc` on `add` and `set`, where it replaces the whole list as `--blocked-by` does), but *linking* one does not: `c` on a selected task — on the cockpit, in the task browser, and inside the browser's detail popup — opens a picker over every registered document with the ones the task already cites ticked, and ⏎ links or unlinks the highlighted one in place through the same store calls `doc link`/`doc unlink` make, leaving the picker open so several can be toggled in one visit. Linking earns the key that registration does not because the moment a link most wants making is while triaging a proposal in the queue, which is exactly where the operator already is, whereas registration is a rarer and wordier act — a location, a title, sometimes a repo — with no such pull. The picker spans every project's documents rather than the task's own, since a task in any project may cite any plan (§3), with the owning project's name on the ones that are not the task's and the task's own listed first. That picker is the whole of the TUI's librarianship: there is no documents screen, and a document's own row — its title, its location, the tasks it backs — remains `doc list`/`doc show`, which keeps the cockpit about attention.
 
+**An adversarial review before `done`** is the last block the preamble renders, after the branch and document blocks and still ahead of the body separator, and every dispatch carries it — a redispatch included, since it renders the same preamble. A task reported done today carries only the writing agent's self-assessment, so the operator, whose attention the whole queue exists to ration, is the first independent reader of every diff. The self-review cannot fill that gap by construction: it shares the session's context and therefore its blind spots, so the commonest cause of a rejection, a misread of the acceptance criteria, ratifies itself rather than being caught. The preamble therefore requires the agent to have the work read, before it may call `done`, by a reviewer carrying no authorship context. The mechanism is named agent-agnostically and as concretely as the worktree instruction beside it: the harness's own subagent where it has one (in Claude Code, the `Agent` tool, which spawns a subagent on a context of its own), and where it has none, a deliberate separate pass over the review inputs alone. Those inputs are the task body with whatever documents it links, and the diff of the work branch against the base — which is why the block renders after the document list rather than before it, and why it tells the agent to commit before reviewing — and deliberately not the session transcript or the agent's notes, a reader who sees only what the operator will see being the point. The brief is adversarial: look for grounds to reject the work against the body's acceptance criteria rather than for confirmation that it is done. The agent then addresses every finding, fixing it or rebutting it with evidence, in one pass per `done` with no re-review, which keeps the gate a fixed cost rather than an open-ended loop. A fixed finding needs no record, since the fix is in the diff; a finding *rebutted* rather than fixed goes in the completion summary alongside the rebuttal, because rejecting a reviewer's finding is a judgement the operator should see rather than one the agent settles alone — and because that summary is the body `pr` opens the pull request with (below), the block says so, so the rebuttal is written for the reader who will actually meet it. Nothing about this is Voro-side: it launches no reviewer, records no verdict, and adds no sub-state, because it is told rather than observing (above) and so cannot verify the review happened at all — the block has exactly the standing of the branch-isolation instructions beside it. The planning and refine prompts render no such block, their deliverable being a task body rather than a diff.
+
 **Branch names** flow through dispatch in both directions, and Voro runs no git in either — it only passes a name in and records one back. A task carries an optional `branch` (schema §5): the *intended* name a human sets with `voro set --branch`, which is the mechanism for attaching a task to an existing branch as much as for naming a fresh one. When set, dispatch renders it into the prompt preamble — telling the agent to create or check out that branch itself before working, since the agent knows the checkout's state better than the dispatcher and Voro deliberately never touches the working tree. Either way — whether a human named the branch or the agent chooses its own — the preamble tells the agent to register that branch with `voro set <id> --branch NAME` the moment it creates or checks it out, so Voro records the real branch while the task is still `running` (letting reconcile, attach, `voro pr`, and the UI reflect it, and capturing it even if the agent never reaches a clean `done`) rather than only learning it at completion. The reverse direction is the *reported* name: `voro done --branch NAME` (and, belt-and-braces, the `SessionEnd` hook in [`agent-integration.md`](agent-integration.md)) records the branch the work actually landed on, overwriting any intended name — and re-confirms the early-registered name for the assigned case. The intended name is a suggestion the agent may follow or override; the reported name is the source of truth. Storing it on the task rather than the session means it survives redispatch and reads naturally beside `pr_url`, so a task correlates with its PR and its branch at a glance; Voro never reads the checkout's HEAD to infer it, consistent with the task-state-versus-session-state boundary above.
 
 **Opening the PR** is `pr`'s second job. On a `review` task with a tracked `pr_url` it is unchanged — jump to the PR in a browser (§11c). On one with *none* it *creates* the PR from the done-time state the two directions above capture: it asserts the task is in `review` and carries both a branch and a completion summary (erroring, network-free, on whichever is missing), pushes the branch to `origin`, opens a ready-for-review (non-draft) GitHub PR whose title is the task title and whose body is that summary, and records the URL through the same `set --pr` write path. No state change — the task stays `review` until a human accepts. The description is captured at `done` while the agent's context is hot, and the rest is mechanical. Crucially `pr` is operator-invoked, so Voro pushing on the operator's behalf preserves the trust model — the *dispatched agent* still cannot publish work (the one deliberate rule), the human running `pr` is the gate, and the PR page is where the diff gets reviewed. The CLI confirms interactively before pushing (`--yes` skips it); the TUI shows the same confirmation as a modal, and on confirming it *also* jumps to the new PR in the browser, since creating one is all but always followed by looking at it and the operator would otherwise press the key twice. That chained open is cosmetic, not part of the create: the URL is recorded either way, so a browser that will not launch is reported beside the URL rather than as a failed create. The CLI leaves the chain to the operator, who is already at a shell. The forge-specific half — push plus `gh pr create` — sits behind one seam in the `voro` crate, and that seam is where the two review media meet: `open` and `pr` are the same operation in two media — get the task's diff in front of the human, locally or on GitHub — so `pr` (one verb, one TUI key) *is* the per-project "show me this task's diff" action. Which medium it uses is the project's **review action** (`projects.review_action`, §5): `auto` (the unconfigured default) uses GitHub when the checkout is a GitHub repo — probed through `gh repo view`, with a missing or unauthenticated `gh` reading as "not GitHub" — and a `voro.toml` viewer otherwise; `pr` pins the GitHub flow, erroring on a checkout that cannot take one; `viewer[:name]` pins a viewer (§11a). On the viewer medium the task's diff is opened in that viewer exactly as `open` does — allowed on `review` and `running`, demanding no branch or summary and confirming nothing, since nothing is pushed. `open` survives as the explicit always-viewer spelling, for reaching the local diff even on a GitHub project. Because a dispatched agent works in a throwaway worktree on the task's branch (§11), the diff lives there, not in the primary checkout, so `open` runs the viewer in that worktree when the task's branch has a live one, falling back to the task's resolved repo (§3) when it has no branch or no worktree. The viewer template is filled with `{path}` (that resolved directory), `{branch}` (the task's branch, empty when none), and `{base}` (the checkout's default branch, read from `refs/remotes/origin/HEAD` with a `main` fallback) so it can express a diff range like `{base}...{branch}` rather than a bare directory; a template using none of these is substituted unchanged. The action is set per project with `voro project action` or the projects screen's picker (`v`), and viewers are defined as `[viewers.<name>]` tables in `voro.toml` (§5) surfaced by `voro viewer list`. The medium decision itself (`ReviewAction::resolve`) plus the pure precondition check and plan assembly live in `voro-core` (tested); the seam supplies only the GitHub probe and the git/`gh` I/O, in the `voro` crate.