review: remove task mode; scripted dispatch is the only mode

.changeset/review-scripted-only.md

@@ -0,0 +1,5 @@
+---
+"review": minor
+---
+
+Scripted dispatch becomes the only mode: task mode is removed. The lifecycle trial (webapp#41010 vs #41013) was the acceptance instrument the scripted probe was gated on; it matched task mode's verdicts on all three rounds while running 8-17% cheaper with a tighter, deduplicated comment set, so the migration plan's slice-4 shape is now the pipeline, not the probe. The orchestrator prompt drops roughly 780 lines of turn-by-turn protocol (Phases 1-3, the task-mode Steps 4-6 and Step 9 record spec, and the budget-guardrail choreography whose shed arithmetic the dispatcher already owns); Steps 4-6 remain as short stubs naming the plan CLI as their owner so cross-references stay stable, and the thread-staging spec moves inline into the Step 3 pipeline. The ROUTING `dispatch` dial is retired: a leftover line warns and is ignored (`routing.json` always carries `dispatchMode: "scripted"`), and the Agent SDK install pre-step is unconditional. Consumers adopt the flip with their next release bump; rollback is release-level pinning. The next seeded lifecycle trial on this pin doubles as the live validation of the structured-final, open-thread-suppression, and deterministic-cache-write follow-ups, which have not yet run live together.

workflows/review/README.md

@@ -72,14 +72,12 @@ requires every finding to trace to the change: a finding whose anchor is not an
 added or modified line of the diff cannot carry a blocking label and does not post
 at all — such pre-existing observations are recorded in the run artifact only; a
 pre-existing defect the diff materially amplifies passes naturally because it
-anchors on the amplifying line. And the **budget guardrail** (a prompt discipline,
-backed in code only by the investigation-cap CLI)
-makes the orchestrator land short of the run's hard ceilings (the per-run
-AI-credits cap and the job timeout). The agent cannot see its own credit spend, so
-it tracks observable proxies (elapsed wall-clock, dispatch counts, the shared
-investigation journal) against the router's soft budget targets; nearing one, it
-sheds remaining work (each shed reviewer becomes a skipped-dimension note) and
-submits the verdict from the findings validated so far, so a run never dies at a
+anchors on the amplifying line. And the **run budget** is enforced where the
+spending happens: the dispatcher caps the roster at
+`runBudget.maxReviewerInvocations` (every capped-out reviewer becomes a
+code-rendered skipped-dimension note), the investigation-cap CLI bounds
+per-finding tool calls, and the per-sub-agent timeout is a hang backstop, so a
+run lands with whatever validated findings it has instead of dying at a
 ceiling with everything spent and nothing posted.
 
 One more gate sits after the agent itself: the **dispatch-conformance gate**
@@ -96,8 +94,34 @@ queue and fails the job. A run that skipped its own dispatch protocol (observed
 zero sub-agents dispatched, verdict submitted, nothing disclosed) becomes a
 red run that posts nothing instead of a normal-looking review; the run
 artifact keeps the original queue and the gate report for diagnosis. The gate
-proves the reviewer outputs were staged, not that a model authored them;
-script-driven dispatch (the next migration slice) is what closes that.
+proves the reviewer outputs were staged; script-driven dispatch makes skipping
+dispatch structural rather than detected.
+
+Most of the run is code rather than model turns. The prompt's Step 3 is one
+CLI invocation (`lib/dispatch.ts`) that runs triage, the reviewer fan-out
+(roster, budget cap, and planned sheds computed from `routing.json`), the
+provenance gate, the scope filter, cross-source dedup, open-thread suppression
+(a candidate describing a defect an open bot thread already tracks posts no
+duplicate; a suppressed blocking candidate still floors the verdict), and
+claim validation, inside the same firewall sandbox (the api-proxy meters and
+caps script-spawned sub-agents exactly like Task-spawned ones). Each sub-agent
+delivers its result through an in-process `submit_result` MCP tool whose input
+is validated against the agent's exact output contract at the tool boundary
+(`lib/dispatch-runner.ts`), so a drifted shape is corrected in-session instead
+of voiding the dimension; free-text finals remain the fallback. Steps 4-6 are
+code too: the submission CLI (`lib/submission.ts`) computes the verdict,
+renders the comments and the full review body, and stages
+`submission-plan.json`; the orchestrator emits safe outputs that must match the
+plan (the gate blocks any deviation), which reduces its model role to typing
+MCP calls the plan dictates. Step 9's cache record is code as well
+(`lib/cache-record.ts`, invoked once after the emission): the
+fingerprint-carrier fields are copied verbatim from staged files and
+corroborated against the safe-output queue, never serialized from the model's
+memory. The safe-output emission itself is the remaining seam: the queue is a
+run-local JSONL append that needs no credentials, but the agent sandbox mounts
+`${RUNNER_TEMP}/gh-aw` read-only, so only the safeoutputs MCP container can
+write it. Removing the seam wants a writable path into the queue (an upstream
+mount change, or a post-agent step on the host); neither is tested yet.
 
 ## Install
 
@@ -232,7 +256,6 @@ rule per line:
 # <pattern> [lens=<lens>,…] [tier=trivial|low|medium|high] [direction-dependent]
 # enable <reviewer>[,<reviewer>…]
 # re-review full|scoped|flip-gated|fast
-# dispatch task|scripted
 services/**/migrations/**  tier=high lens=data-migrations
 **/*.graphql               lens=api-federation-compat
 pkg/auth/**                tier=high direction-dependent lens=security-auth
@@ -260,36 +283,6 @@ re-review scoped
 - `re-review` sets the repo's re-review mode (see the next section). Default
   `full`; when several lines set it, the last one wins with a warning. An
   unknown mode degrades to `full`: toward more review, never less.
-- `dispatch` sets how Step 3 runs (default `task`): `task` is the
-  orchestrator's own Task-tool dispatch; `scripted` opts the repo into the
-  deterministic dispatcher (`lib/dispatch.ts`): the orchestrator invokes one
-  CLI that runs triage, the reviewer fan-out (roster, budget cap, and planned
-  sheds computed from `routing.json`), the provenance gate, the scope filter,
-  cross-source dedup, open-thread suppression (a candidate describing a
-  defect an open bot thread already tracks posts no duplicate; a suppressed
-  blocking candidate still floors the verdict), and claim validation as
-  code, inside the same firewall sandbox (the api-proxy meters and caps
-  script-spawned sub-agents exactly like Task-spawned ones). Each sub-agent
-  delivers its result through an in-process `submit_result` MCP tool whose
-  input is validated against the agent's exact output contract at the tool
-  boundary (`lib/dispatch-runner.ts`), so a drifted shape is corrected
-  in-session instead of voiding the dimension; free-text finals remain the
-  fallback. In scripted mode Steps 4-6 are code too: the submission CLI
-  (`lib/submission.ts`) computes the verdict, renders the comments and
-  the full review body, and stages `submission-plan.json`; the orchestrator
-  emits safe outputs that must match the plan (the gate blocks any
-  deviation), which reduces its model role to typing MCP calls the plan
-  dictates. Step 9's cache record is code as well (`lib/cache-record.ts`,
-  invoked once after the emission): the fingerprint-carrier fields are
-  copied verbatim from staged files and corroborated against the safe-output
-  queue, never serialized from the model's memory. The safe-output emission
-  itself is the remaining seam: the queue is a run-local JSONL append that
-  needs no credentials, but the agent sandbox mounts `${RUNNER_TEMP}/gh-aw`
-  read-only, so only the safeoutputs MCP container can write it. Removing
-  the seam wants a writable path into the queue (an upstream mount change,
-  or a post-agent step on the host); neither is tested yet. Scripted mode is
-  the production probe of the deterministic-orchestrator migration and is
-  live-trial-gated; an unknown mode degrades to `task` with a warning.
 
 Glob semantics are a practical subset of gitignore/CODEOWNERS: `**` crosses
 directories, `*` and `?` stay within a segment, a trailing `/` matches everything

workflows/review/lib/cache-record.test.ts

@@ -202,11 +202,11 @@ describe("runCacheRecordCli", () => {
         });
     });
 
-    it("no-ops without a staged plan (task mode keeps the orchestrator's write)", () => {
+    it("no-ops without a staged plan (the orchestrator's write stands)", () => {
         const fs = makeFakeFs();
         const result = runCacheRecordCli(fs, NOW);
         expect(result.written).toBe(false);
-        expect(result.reason).toMatch(/task mode/);
+        expect(result.reason).toMatch(/no submission plan staged/);
         // Benign no-op: never surfaced as a workflow warning.
         expect(result.warn).toBeUndefined();
     });

workflows/review/lib/cache-record.ts

@@ -233,7 +233,8 @@ export type CacheRecordResult = {
     /**
      * Set on refusals that indicate something WRONG (a corroboration
      * mismatch, missing staged facts), as opposed to the benign no-ops
-     * (task mode, gate-blocked run). The CLI surfaces these as `::warning`:
+     * (a gate-blocked run, a run that ended before the plan). The CLI
+     * surfaces these as `::warning`:
      * a systematic refusal permanently stales the fingerprint and forces
      * full-depth reviews indefinitely, which must not stay invisible.
      */
@@ -264,7 +265,7 @@ export const runCacheRecordCli = (
         | undefined;
     if (plan === undefined) {
         return skip(
-            "no submission plan staged (task mode, or the run ended before the plan): the cache write stays with the orchestrator",
+            "no submission plan staged (the run ended before the plan): the cache write stays with the orchestrator",
         );
     }
     if (fs.existsSync(BLOCKED_SENTINEL_PATH)) {

workflows/review/lib/dispatch-gate-hardening.test.ts

@@ -296,19 +296,18 @@ describe("third-round nits: keep-list survivors, template coupling, summary", ()
         ).toEqual(["noop", "missing_data"]);
     });
 
-    it("review.md's Step 6 note templates still carry the phrase the gate matches", () => {
-        // Couples the disclosure matcher to the prompt templates: a Step 6
-        // reword that drops the phrase must fail here, not silently break
-        // rules 2/3 in production.
-        const reviewMd = readFileSync(
-            join(__dirname, "..", "review.md"),
-            "utf8",
+    it("the dispatcher's note templates still carry the phrase the gate matches", () => {
+        // Couples the disclosure matcher to the note templates. They lived in
+        // review.md's Step 6 while the orchestrator composed notes; with task
+        // mode removed they are code-rendered (dispatch.ts noteLine), so a
+        // template reword that drops the phrase must fail here, not silently
+        // break rules 2/3 in production.
+        const dispatchTs = readFileSync(join(__dirname, "dispatch.ts"), "utf8");
+        expect(dispatchTs).toContain(
+            "not assessed this run (shed under the ${tier}-tier run budget)",
         );
-        expect(reviewMd).toContain(
-            "not assessed this run (shed under the <tier>-tier run budget)",
-        );
-        expect(reviewMd).toContain(
-            "not assessed this run (<sub-agent> output unavailable)",
+        expect(dispatchTs).toContain(
+            "not assessed this run (${agent} output unavailable)",
         );
     });
 

workflows/review/lib/dispatch-gate.ts

@@ -542,6 +542,7 @@ export const evaluateDispatchConformance = (
                   event?: unknown;
                   body?: unknown;
                   comments?: unknown;
+                  skipSubmission?: unknown;
               })
             : undefined;
     if (planStaged !== undefined && submit === undefined) {
@@ -571,11 +572,18 @@ export const evaluateDispatchConformance = (
             );
             const planBody =
                 typeof planStaged.body === "string" ? planStaged.body : "";
-            if (
-                planStaged.event !== "APPROVE" ||
-                planComments.length > 0 ||
-                normalizeBody(planBody) !== bareApprove
-            ) {
+            // The plan CLI owns this predicate (`skipSubmission`) so the
+            // prompt and this gate cannot describe the skip differently —
+            // they diverged once, over the collapsed low-confidence section
+            // riding the body. Fall back to deriving it only for a plan
+            // staged before the field existed.
+            const planSkips =
+                typeof planStaged.skipSubmission === "boolean"
+                    ? planStaged.skipSubmission
+                    : planStaged.event === "APPROVE" &&
+                      planComments.length === 0 &&
+                      normalizeBody(planBody) === bareApprove;
+            if (!planSkips) {
                 violations.push({
                     code: "submission-plan-mismatch",
                     dimension: "verdict",

workflows/review/lib/dispatch.test.ts

@@ -313,29 +313,26 @@ describe("applyScopeFilter", () => {
 });
 
 describe("ROUTING dispatch directive", () => {
-    it("defaults to task and accepts scripted", () => {
-        expect(parseRoutingConfig("").dispatchMode).toBe("task");
+    it("always resolves to scripted, whatever the retired dial says", () => {
+        expect(parseRoutingConfig("").dispatchMode).toBe("scripted");
         expect(parseRoutingConfig("dispatch scripted\n").dispatchMode).toBe(
             "scripted",
         );
+        expect(parseRoutingConfig("dispatch task\n").dispatchMode).toBe(
+            "scripted",
+        );
     });
 
-    it("warns and keeps the default on an unknown mode", () => {
-        const config = parseRoutingConfig("dispatch warp\n");
-        expect(config.dispatchMode).toBe("task");
-        expect(config.warnings.join(" ")).toContain("unknown dispatch mode");
-    });
-
-    it("skips a dispatch line with the wrong arity", () => {
-        const config = parseRoutingConfig("dispatch task scripted\n");
-        expect(config.dispatchMode).toBe("task");
-        expect(config.warnings.join("\n")).toContain("exactly one");
-    });
-
-    it("lets the last of duplicate dispatch lines win, with a warning", () => {
-        const config = parseRoutingConfig("dispatch task\ndispatch scripted\n");
-        expect(config.dispatchMode).toBe("scripted");
-        expect(config.warnings.join("\n")).toContain("duplicate dispatch");
+    it("warns that a leftover dispatch line is obsolete", () => {
+        expect(
+            parseRoutingConfig("dispatch scripted\n").warnings.join(" "),
+        ).toContain("obsolete");
+        // A `task` (or unknown) value gets the stronger retired-dial wording.
+        for (const line of ["dispatch task\n", "dispatch warp\n"]) {
+            const config = parseRoutingConfig(line);
+            expect(config.dispatchMode).toBe("scripted");
+            expect(config.warnings.join(" ")).toContain("retired");
+        }
     });
 });
 
@@ -673,13 +670,12 @@ describe("re-review hardening (slice 2 feedback)", () => {
         );
     });
 
-    it("warns on dispatch directive arity and duplicates (last one wins)", () => {
+    it("stays scripted whatever a leftover dispatch line says", () => {
         const arity = parseRoutingConfig("dispatch task scripted\n");
-        expect(arity.dispatchMode).toBe("task");
-        expect(arity.warnings.join(" ")).toContain("exactly one");
+        expect(arity.dispatchMode).toBe("scripted");
+        expect(arity.warnings.join(" ")).toContain("retired");
         const dupe = parseRoutingConfig("dispatch task\ndispatch scripted\n");
         expect(dupe.dispatchMode).toBe("scripted");
-        expect(dupe.warnings.join(" ")).toContain("duplicate dispatch");
     });
 
     it("emits dispatchMode through the router CLI's routing.json", () => {

workflows/review/lib/rereview-mode.ts

@@ -343,8 +343,9 @@ export const findLatestStamp = (
 
 /**
  * Reconstruct a stamp from the Step 9 cache-memory record (the fallback
- * fingerprint carrier; see the module header). The record is model-written
- * in task mode, so every field is validated and any gap returns null: a
+ * fingerprint carrier; see the module header). Records written before the
+ * cache-record CLI landed are model-written, so every field is validated and
+ * any gap returns null: a
  * fingerprint we cannot trust anchors nothing, and the depth decision
  * degrades to `full`. The executed depth is not recorded there, so the
  * reconstructed stamp carries `full` (the field is informational; no

workflows/review/lib/router-dispatch-mode.test.ts

@@ -4,9 +4,10 @@ import {runCli} from "./router";
 import {ROUTING_CONFIG_PATH} from "./routing-config";
 
 /**
- * The `dispatch` directive's CLI wiring, split from router.test.ts for its
- * max-lines budget: the ROUTING line reaches `routing.json`'s `dispatchMode`
- * so the orchestrator can read the repo's mode from one staged surface.
+ * The retired `dispatch` directive's CLI wiring, split from router.test.ts
+ * for its max-lines budget: `routing.json` always carries `scripted`, and a
+ * leftover ROUTING line earns a visible obsolete warning rather than
+ * changing behaviour.
  * The fs fixture is a small local copy of router.test.ts's, the same way
  * dispatch-gate-hardening.test.ts copies its parent's.
  */
@@ -51,22 +52,24 @@ const fakeFs = (inputs: Record<string, string>) => {
 };
 
 describe("runCli: dispatch mode", () => {
-    it("surfaces a configured dispatch scripted line in routing.json", () => {
+    it("always emits scripted (the dial is retired), warning on a leftover line", () => {
         const {fs} = fakeFs({
             ["/tmp/gh-aw/review/files.json"]: JSON.stringify([
                 {path: "a.ts", status: "modified"},
             ]),
             [ROUTING_CONFIG_PATH]: "dispatch scripted",
         });
-        expect(runCli(fs).dispatchMode).toBe("scripted");
+        const routing = runCli(fs);
+        expect(routing.dispatchMode).toBe("scripted");
+        expect(routing.routingConfig.warnings.join(" ")).toContain("obsolete");
     });
 
-    it("defaults to task without a dispatch line", () => {
+    it("emits scripted without a dispatch line", () => {
         const {fs} = fakeFs({
             ["/tmp/gh-aw/review/files.json"]: JSON.stringify([
                 {path: "a.ts", status: "modified"},
             ]),
         });
-        expect(runCli(fs).dispatchMode).toBe("task");
+        expect(runCli(fs).dispatchMode).toBe("scripted");
     });
 });