Review objections must run: --review-executable

CHANGELOG.md

@@ -2,6 +2,19 @@
 
 ## Unreleased
 
+### Added
+
+- **`--review-executable`** (experimental, also `MAAT_REVIEW_EXECUTABLE=1`):
+  every `--review` objection must carry a read-only command that demonstrates
+  it. Maat runs it on a throwaway copy of the tree, with no API keys, tokens or
+  provider variables in its environment; an objection counts only when its
+  command reads the work (names a project path, or runs the project's tests or
+  build) and exits non-zero, or prints the value it named without that value
+  being in the command itself. A demonstrated objection shows the command
+  failed, not that it tested what the objection says. With no copy of the tree
+  available the command is not run. Each objection is recorded as counted,
+  refuted (the reviewer was wrong) or unchecked (it could not be run).
+
 ### Changed
 
 - **OpenCode runs OpenCode Zen models only.** The OpenCode backend (worker and

src/cli.tsx

@@ -162,6 +162,16 @@ options
                      gate; "verified" then needs a drafted check that asserts a
                      value, ran the work, and failed before the work began
                      (also MAAT_REVIEW_ADVISORY=1)
+  --review-executable  experimental: every --review objection must carry a
+                     read-only command that demonstrates it; Maat runs it on a
+                     copy of the tree and an objection whose command is missing,
+                     mutates, or passes on the work is a note, not a veto. A
+                     failure counts only from a command that reads the work
+                     (names a project path or runs its tests); it shows the
+                     command failed, not that it tested what the objection
+                     says. Commands run without Maat's credentials, and with
+                     no throwaway copy of the tree they are not run and do not
+                     count (also MAAT_REVIEW_EXECUTABLE=1)
   --signout          before an unattended claim is judged, put each stated
                      requirement to the model once beside the commands it ran
                      (off by default: 60 rounds rescued no task)
@@ -279,6 +289,8 @@ type Args = {
   revealStuck?: boolean;
   /** `--review-advisory`: see EngineConfig.reviewAdvisory. */
   reviewAdvisory?: boolean;
+  /** `--review-executable`: see EngineConfig.reviewExecutable. */
+  reviewExecutable?: boolean;
   /** `--signout`: see EngineConfig.signOut. */
   signout?: boolean;
   /** `--arbiter-model` / `--dispute-votes`: see EngineConfig.dispute. */
@@ -515,6 +527,9 @@ export function parseArgs(argv: string[], stored: StoredEndpoint = {}): Args {
       case "--review-advisory":
         out.reviewAdvisory = true;
         break;
+      case "--review-executable":
+        out.reviewExecutable = true;
+        break;
       case "--signout":
         out.signout = true;
         break;
@@ -822,6 +837,7 @@ function engineFor(args: Args, session = false, extra: { files?: FileAccess } =
     batch: args.batch === true,
     ...(args.revealStuck === true ? { revealOnStuck: true } : args.revealStuck === false ? { revealOnStuck: false } : {}),
     ...(args.reviewAdvisory || env("REVIEW_ADVISORY") === "1" ? { reviewAdvisory: true } : {}),
+    ...(args.reviewExecutable || env("REVIEW_EXECUTABLE") === "1" ? { reviewExecutable: true } : {}),
     ...(args.signout ? { signOut: true } : {}),
     ...(args.arbiterModel || args.disputeVotes ? { dispute: { model: args.arbiterModel, votes: args.disputeVotes } } : {}),
     ...(args.review ? { review: { votes: args.review, reasoningEffort: args.reasoningChecks ?? args.reasoning } } : {}),

src/credenv.ts

@@ -0,0 +1,23 @@
+/**
+ * An environment for a command a model wrote, without Maat's credentials.
+ *
+ * A reviewer's objection command is model-authored shell text, and the
+ * reviewer read the worker's claim and output, so the worker can steer it.
+ * It runs in a throwaway copy of the tree, but with Maat's own environment
+ * it could read every API key Maat holds (`curl -d "$(env)" …`). This drops
+ * anything that names itself a credential, and the provider variables Maat
+ * reads its keys and endpoints from. PATH, HOME, locale and the like stay:
+ * the command still has to run.
+ */
+const SECRET_NAME = /(KEY|TOKEN|SECRET|PASSWORD|PASSWD|CREDENTIAL|AUTH|COOKIE|SESSION|DSN|DATABASE_URL|PRIVATE)/i;
+const PROVIDER_PREFIX = /^(MAAT_|MOLT_|OPENAI|ANTHROPIC|OPENROUTER|XAI|GROK|GEMINI|GOOGLE|AWS|AZURE|GCP|DEEPSEEK|MISTRAL|GROQ|TOGETHER|FIREWORKS|HF_|HUGGING|OPENCODE|GH_|GITHUB|NPM_|CLOUDFLARE|CF_)/i;
+
+export function credentialFreeEnv(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
+  const out: NodeJS.ProcessEnv = {};
+  for (const [k, v] of Object.entries(env)) {
+    if (v === undefined) continue;
+    if (SECRET_NAME.test(k) || PROVIDER_PREFIX.test(k)) continue;
+    out[k] = v;
+  }
+  return out;
+}

src/engine.ts

@@ -18,9 +18,11 @@
  *  - Shedding is two-phase: archive first, mutate second.
  *  - Nothing is summarized by a model, ever.
  */
-import { runCommand } from "./run.js";
+import { bashPath, runCommand } from "./run.js";
+import { copyTreeOrWhy } from "./scratch.js";
 import { describeStart, listBackground, startBackground, stopBackground } from "./background.js";
-import { reviewClaim, type Review } from "./review.js";
+import { objectionLine, readsTheWork, reviewClaim, type ExecutableReview, type ObjectionRun, type Review } from "./review.js";
+import { credentialFreeEnv } from "./credenv.js";
 import { judgeEffort, judgeTarget } from "./judge.js";
 import { tierOf } from "./tiers.js";
 import { arbitrate, parseDisputes, type Ruling } from "./dispute.js";
@@ -1347,6 +1349,15 @@ export type EngineConfig = {
    * failed before the work (tiers.ts `reviewAdvisory`). Off by default.
    */
   reviewAdvisory?: boolean;
+  /**
+   * Experimental (`--review-executable`, MAAT_REVIEW_EXECUTABLE=1): each
+   * objection the independent review raises must carry a read-only shell
+   * command that demonstrates it. Maat runs it on a throwaway copy of the tree;
+   * an objection counts only when its command ran and showed the failure.
+   * The rest are recorded as unsubstantiated notes and veto nothing
+   * (src/review.ts). Off by default.
+   */
+  reviewExecutable?: boolean;
   /**
    * Requirement sign-out (src/signout.ts): one round per unattended turn that
    * lists each stated requirement beside the commands run for it. Off unless
@@ -3586,6 +3597,61 @@ export class Engine {
     return this.markGuards(await runBar(bar, this.barContext(claim)));
   }
 
+  /** The review's executable-objection runner, when `reviewExecutable` is on; else nothing. */
+  private executableReview(): { executable?: ExecutableReview } {
+    if (this.cfg.reviewExecutable !== true) return {};
+    return { executable: { run: (command) => this.runObjection(command) } };
+  }
+
+  /**
+   * Run one reviewer's objection command as a task check runs: in a throwaway
+   * copy of the tree (src/scratch.ts), under bash, never past the turn's clock.
+   * The command has already passed lint L15 (it does not change the work).
+   *
+   * Never on the work itself. A reviewer's command is written by a model that
+   * read the task text, which may be anyone's, and it exists to probe the
+   * work, not to protect it; the lint is a reading of the command, not a
+   * guarantee. With no copy (a worktree's .git pointer, a tree over the copy
+   * limits, MAAT_CHECK_COPY=0) the objection is not run and does not count:
+   * "could not be run safely".
+   */
+  private async runObjection(command: string): Promise<ObjectionRun> {
+    const tried = process.env.MAAT_CHECK_COPY === "0" ? { why: "MAAT_CHECK_COPY=0 turns throwaway copies off" } : await copyTreeOrWhy(this.cwd);
+    if ("why" in tried) {
+      return { code: null, stdout: "", stderr: "", notRun: `could not be run safely: no throwaway copy of the tree (${tried.why})` };
+    }
+    const copy = tried;
+    try {
+      const left = this.timeLeftMs();
+      if (left !== undefined && left <= 0) {
+        return { code: null, stdout: "", stderr: "", notRun: "the time budget is spent" };
+      }
+      const r = await runCommand(command, {
+        cwd: copy.dir,
+        shell: bashPath() ?? true,
+        timeoutMs: left !== undefined ? Math.max(1, Math.min(30_000, left)) : 30_000,
+        maxBuffer: 1024 * 1024,
+        // The reviewer's command never sees Maat's credentials (credentialFreeEnv).
+        env: credentialFreeEnv(process.env),
+        signal: this.running?.signal,
+      });
+      const fix = (t: string) => copy.unmap(t);
+      return { code: r.code, stdout: fix(r.stdout), stderr: fix(r.stderr), timedOut: r.timedOut, readsWork: readsTheWork(command, copy.dir) };
+    } finally {
+      await copy.cleanup();
+    }
+  }
+
+
+  /** Executable mode: every objection that did not count, as an info line each. */
+  private *unsubstantiated(review: Review | null): Generator<EngineEvent> {
+    for (const o of review?.objections ?? []) {
+      if (o.result === "demonstrated") continue;
+      // The reviewer's command and its output can quote a hidden check.
+      yield { kind: "info", text: this.maskWithheld(`objection not counted (reviewer ${o.vote}): ${objectionLine(o)}`) };
+    }
+  }
+
   /** tierOf's advisory-review arguments: empty unless `reviewAdvisory`. */
   private advisoryTier(): { reviewAdvisory?: true; guards?: ReadonlySet<string> } {
     return this.cfg.reviewAdvisory === true ? { reviewAdvisory: true, guards: this.passedBeforeWork } : {};
@@ -4327,7 +4393,7 @@ export class Engine {
     if (outcome === "verified" && this.cfg.review && inTurn !== undefined) {
       // Reviewed inside the turn, and nothing changed after.
       review = inTurn;
-      this.cfg.journal?.append("review", review ? { confirmed: review.confirmed, votes: review.votes, violations: review.violations.length } : { ran: false });
+      this.cfg.journal?.append("review", review ? { confirmed: review.confirmed, votes: review.votes, violations: review.violations.length, ...(review.objections ? { objections: review.objections } : {}) } : { ran: false });
       if (review && !review.confirmed) {
         yield { kind: "info", text: `passed its checks, unconfirmed: the reviewer (${review.votes}) found ` + review.violations.map((v) => `"${v.quote}" — ${v.evidence}`).join("; ") };
       }
@@ -4352,8 +4418,11 @@ export class Engine {
           // And every retry and pause inside it, not just one ask.
           deadlineAt: this.deadlineAt(),
         },
+        ...this.executableReview(),
       }).catch(() => null);
-      this.cfg.journal?.append("review", review ? { confirmed: review.confirmed, votes: review.votes, violations: review.violations.length } : { ran: false });
+      this.cfg.journal?.append("review", review ? { confirmed: review.confirmed, votes: review.votes, violations: review.violations.length, ...(review.objections ? { objections: review.objections } : {}) } : { ran: false });
+      if (review?.objections && receiptPath) this.cfg.receipts?.amendReview(basename(receiptPath), review.objections);
+      yield* this.unsubstantiated(review);
       if (!review) {
         yield { kind: "info", text: "the independent review could not be run; the claim stands as checked" };
       } else if (!review.confirmed) {
@@ -4435,7 +4504,7 @@ export class Engine {
       ...(this.turnProviderStall ? { providerStall: true } : {}),
       ...(checksDisagree ? { checksDisagree: failing.map((r) => r.name) } : {}),
       ...(outcome === "verified" && this.turnRevealed.length ? { revealed: [...this.turnRevealed] } : {}),
-      ...(review ? { review: { confirmed: review.confirmed, votes: review.votes, violations: review.violations } } : {}),
+      ...(review ? { review: { confirmed: review.confirmed, votes: review.votes, violations: review.violations, ...(review.objections ? { objections: review.objections } : {}) } } : {}),
       ...(opened !== undefined ? { case: opened } : {}),
     };
   }
@@ -7550,11 +7619,16 @@ export class Engine {
             ...(this.timeLeftMs() !== undefined ? { timeoutMs: Math.max(1_000, this.timeLeftMs()!) } : {}),
             deadlineAt: this.deadlineAt(),
           },
+          ...this.executableReview(),
         }).catch(() => null);
+        if (review?.objections) {
+          if (lastReceipt) this.cfg.receipts?.amendReview(lastReceipt, review.objections);
+          yield* this.unsubstantiated(review);
+        }
         if (review && !review.confirmed && review.violations.length && this.cfg.reviewAdvisory !== true) {
           reviewNudged = true;
           this.turnReview = undefined;
-          log?.append("review", { confirmed: false, votes: review.votes, violations: review.violations.length, nudged: true });
+          log?.append("review", { confirmed: false, votes: review.votes, violations: review.violations.length, nudged: true, ...(review.objections ? { objections: review.objections } : {}) });
           yield {
             kind: "info",
             text:
@@ -7566,7 +7640,10 @@ export class Engine {
             content:
               `[molt] Your checks passed, but independent reviewers who read only the task and your ` +
               `receipt found the work contradicts the task:\n` +
-              review.violations.map((v) => `- the task says "${v.quote}" — ${v.evidence}`).join("\n") +
+              (review.objections
+                ? // Executable mode: an objection's command output may quote a hidden check.
+                  this.maskWithheld(review.violations.map((v) => `- the task says "${v.quote}" — ${v.evidence}`).join("\n"))
+                : review.violations.map((v) => `- the task says "${v.quote}" — ${v.evidence}`).join("\n")) +
               `\nIf they are right, fix it and say done again. If they are wrong, say in one line why, ` +
               `and say done again without changing anything.`,
           });

src/receipts.ts

@@ -14,6 +14,7 @@ import { MIN_SECRET_CHARS, redact } from "./redact.js";
 import type { BarResult } from "./types.js";
 import { stateDir } from "./statedir.js";
 import { WITHHELD, maskDeep, maskText } from "./withhold.js";
+import type { Objection } from "./review.js";
 
 /** Where a receipt's full text goes when hidden commands were masked in it (src/withhold.ts). */
 export const FULL_DIR = "full";
@@ -89,6 +90,12 @@ export type ReceiptRecord = {
   tierReason?: string;
   /** The strongest evidence class among the passing checks. */
   evidence?: string;
+  /**
+   * `--review-executable`: each reviewer objection, the command that came with
+   * it and what running it showed (src/review.ts). Added after the receipt is
+   * written (`amendReview`), like the tier.
+   */
+  objections?: Objection[];
 };
 
 /** What `repair()` changed, and what it left alone. */
@@ -743,6 +750,36 @@ export class Receipts {
     }
   }
 
+  /**
+   * Record the independent review's objections on a receipt's index row: each
+   * with its command and result (`--review-executable`). Masked like every
+   * other write while hidden commands are withheld. The receipt file itself,
+   * hash-bound when it was written, is never rewritten.
+   */
+  amendReview(file: string, objections: readonly Objection[]): boolean {
+    try {
+      if (!existsSync(this.indexPath)) return false;
+      let hit = false;
+      const out = readFileSync(this.indexPath, "utf8")
+        .split("\n")
+        .map((l) => {
+          if (!l.trim()) return l;
+          try {
+            const r = JSON.parse(l) as ReceiptRecord;
+            if (r.file !== file) return l;
+            hit = true;
+            return redact(JSON.stringify({ ...r, objections: maskDeep([...objections], this.withheld) }), this.secrets);
+          } catch {
+            return l;
+          }
+        });
+      if (hit) writeFileSync(this.indexPath, out.join("\n"), "utf8");
+      return hit;
+    } catch {
+      return false;
+    }
+  }
+
   /**
    * Reconcile the index against the files on disk.
    *