From 16827de45d45363326ee33ff850d9c54c1b5804d Mon Sep 17 00:00:00 2001 From: wassname <1103714+wassname@users.noreply.github.com> Date: Fri, 3 Jul 2026 12:13:14 +0800 Subject: [PATCH] judge goes strictly read-only: no bash, never re-runs verify; reviews evidence discipline instead Re-running verify was fine for 'npm test' but a footgun for ML workflows where verify may be a 10-hour training run -- and bash made 'read-only' nominal anyway (it could mutate). The agent now runs verify itself and saves the output as evidence. The judge checks, in order: anything here / quoted+attributed / provenance / quotes match disk / substance. Matches the cooperative-but- confused threat model: reading real artifacts catches confusion; execution only defended against deliberate forgery, which is out of scope. Co-Authored-By: Claudypoo <288921227+claudypoo@users.noreply.github.com> --- README.md | 17 ++++++++------ src/index.ts | 16 ++++++++------ src/prompts.ts | 49 +++++++++++++++++++++++++---------------- test/judge-args.test.ts | 4 +++- 4 files changed, 52 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 47efb22..80c1895 100644 --- a/README.md +++ b/README.md @@ -86,18 +86,21 @@ Latency target came from the SLO review; keep the existing client API. ## Signing off a goal (`CompleteGoal`) -`CompleteGoal(goal)` is the one blessed tool. It spawns a read-only `pi` subprocess (`-p ---no-session --no-extensions`, tools `read,bash,grep,find,ls`, edit/write excluded) with the whole -plan file and the claimed goal. The judge finds the goal, re-derives from the cited artifacts rather -than trusting claims, runs `verify` if the goal names one, and returns `VERDICT: accept | reject` -plus what's missing. It inspects the live working tree, not HEAD: uncommitted work counts, and -committing before sign-off is for durable evidence, not for the judge's visibility. +`CompleteGoal(goal)` is the one blessed tool. It spawns a strictly read-only `pi` subprocess (`-p +--no-session --no-extensions`, tools `read,grep,find,ls` -- no bash, no edit/write) with the whole +plan file and the claimed goal. The judge cannot execute anything, so it never re-runs your verify +command (which may be a 10-hour training job); the agent runs `verify` itself and saves the output +as evidence. The judge reviews evidence discipline in order -- anything here at all? each item +quoted and attributed? provenance visible (how was this produced)? do the quotes match the cited +files on disk? -- and only then the substance, returning `VERDICT: accept | reject` plus what's +missing. It reads the live working tree, not HEAD: uncommitted work counts, and committing before +sign-off is for durable evidence, not for the judge's visibility. - accept: a sign-off line is appended to `## Log` and the goal is ticked `[x]` in the same write (exact goal-line match only; on wording drift the result asks the agent to tick it). The tool-written log line is the audit trail; a hand-tick without one shows in the diff. - every run saves the judge's full transcript to `.pi/judge/.md`, referenced from the log - line, so "did the judge really re-run verify?" stays answerable after the fact. + line, so "what did the judge actually check?" stays answerable after the fact. - reject: the goal stays open and the agent gets the missing list. - judge ran but failed/errored/timed out, or returned no VERDICT line: accepted inconclusive, logged as such. There is no pre-emptive "no model" path -- a null judgeModel just omits diff --git a/src/index.ts b/src/index.ts index 9e273d9..4066536 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,12 +9,13 @@ * 1. memory — inject plan.md verbatim every turn (survives compaction; byte-identical when * unchanged so the KV cache holds; stale copies stripped by the context hook) * 2. format — a skeleton convention taught in planDrafting (prompts.ts), not validated - * 3. eyes — CompleteGoal spawns a read-only pi subprocess (--no-session) that gets the whole - * plan file plus the claimed goal, finds the goal itself (tolerates wording drift), - * runs the goal's verify command itself, and returns VERDICT: accept|reject + * 3. eyes — CompleteGoal spawns a strictly read-only pi subprocess (--no-session, no bash) + * that gets the whole plan file plus the claimed goal, finds the goal itself + * (tolerates wording drift), checks the evidence (including the agent's saved + * verify output) against the repo, and returns VERDICT: accept|reject * * The judge subsumes what v1 did in code: goal matching (no findGoal), evidence validation (a - * placeholder gets rejected in words), verify execution, and format reading. The extension's only + * placeholder gets rejected in words), and format reading. The extension's only * writes are the sign-off: append a log line to ## Log (the audit trail) and tick the goal [x] when * an exact goal line matches (on drift the agent ticks, and the result says so). A hand-tick * without a matching tool-written log line is visible in the diff either way. @@ -39,9 +40,10 @@ const PLAN_CONTEXT = "pi-goals-context"; // injected plan/guidance, stale copies const STATUS_KEY = "pi-goals"; const WIDGET_KEY = "pi-goals-widget"; const PLAN_REL = ".pi/plan.md"; -// Judge toolset: read-only inspection plus bash (git log, cat, running the goal's verify command). -// edit/write are excluded so the judge cannot modify anything. Names match pi's tool registry. -const JUDGE_TOOLS = ["read", "bash", "grep", "find", "ls"]; +// Judge toolset: strictly read-only, NO bash -- the judge can never execute or mutate anything, and +// in particular never re-runs a verify command (which may be a 10-hour training job). The agent runs +// verify itself and saves the output as evidence; the judge reads it. Names match pi's tool registry. +const JUDGE_TOOLS = ["read", "grep", "find", "ls"]; const JUDGE_BLOCKED_TOOLS = ["edit", "write"]; const JUDGE_TIMEOUT_MS = 600_000; // Plan mode is read-only by convention AND a light gate: edit/write are blocked (except plan.md, diff --git a/src/prompts.ts b/src/prompts.ts index 15373f3..0f5d740 100644 --- a/src/prompts.ts +++ b/src/prompts.ts @@ -46,7 +46,8 @@ directly by the human and a judge model, so clarity beats conformance; small dev 1. [ ] goal: - subtle failure mode: - discriminator: - - verify: + - verify: - tasks: 1. [ ] - evidence: (empty until sign-off) @@ -95,12 +96,14 @@ export const completeGoalDescription = "plan file: each item pairs a durable artifact with a short read of it (a quoted+linked log, a " + "table plus how to read it, a metric plus what it shows -- not a bare claim). Quote verbatim from " + "output you actually observed; never reconstruct numbers from memory. If you couldn't see an " + - "output, rerun it or write that you couldn't -- an honest gap beats a plausible fabrication. The " + - "read must show success POSITIVELY happened, not just that failures were avoided. Then call this with the goal's " + - "text (the line after 'goal:'; small wording drift is fine). A fresh read-only judge inspects the " + - "LIVE WORKING TREE (uncommitted changes included; committing first is for durability, not " + - "visibility), runs the goal's verify command if it names one, and returns accept or reject with " + - "what's missing. On accept (or if the judge itself failed), a sign-off line is appended to ## Log " + + "output, rerun it or write that you couldn't -- an honest gap beats a plausible fabrication. If " + + "the goal names a verify: command, run it yourself first and save its output to a file cited in " + + "the evidence: the judge cannot execute anything and will reject a claimed pass with no saved " + + "output. The read must show success POSITIVELY happened, not just that failures were avoided. " + + "Then call this with the goal's text (the line after 'goal:'; small wording drift is fine). A " + + "fresh strictly-read-only judge inspects the LIVE WORKING TREE (uncommitted changes included; " + + "committing first is for durability, not visibility) and returns accept or reject with what's " + + "missing. On accept (or if the judge itself failed), a sign-off line is appended to ## Log " + "and the goal is ticked [x] for you; the result says if you must tick it yourself. On reject the " + "goal stays open."; @@ -113,17 +116,25 @@ export const completeGoalParamDescription = "The goal's text: the line after 'go * reads discriminator/failure modes/evidence itself (no parser between). * ──────────────────────────────────────────────────────────────────────── */ export const judgeSystem = `\ -You are a read-only reviewer signing off a coding goal. Do not trust claims; verify. -Use read/grep/find/ls/bash to inspect the repository and the cited artifacts yourself. Re-read the -files, logs, and diffs the evidence points to; if something asserted isn't on disk, you can't -confirm it. Spot-check the evidence quotes against the artifacts: a quote or number that doesn't -match what's actually there means the evidence was reconstructed from memory, not observed -- -reject and ask for re-observed evidence, even if the goal itself otherwise looks met. Evidence -discipline is part of the goal. If the goal names a verify: command, run it and check it really tests the discriminator -rather than passing tautologically. Judge whether the evidence shows the goal POSITIVELY succeeded --- the discriminator's success signal is actually present, not just that the named failure modes -were dodged; a run can rule out every trap and still have produced nothing. Then check each subtle -failure mode is genuinely ruled out, not just unmentioned. +You are a strictly read-only reviewer signing off a coding goal. You cannot execute anything: judge +by reading (read/grep/find/ls). Never re-run the work or its verify command -- it may be a 10-hour +job; the agent must bring you its saved output. Your job is evidence discipline, checked in order: + +1. Anything here? An empty or placeholder evidence: list -> reject: "there's nothing here -- fill + the evidence and try again." +2. Quoted and attributed? Each item needs a source (file path / command) plus a verbatim quote of + what was observed, plus a one-line read. A bare claim -> reject: "you didn't quote and + attribute it." +3. Provenance? It must be visible HOW each result was produced (the command run, where its output + was saved). Results with no origin -> reject: "I see the results, but how did you get them?" +4. Spot-check: open the cited files. A quote or number that doesn't match what's on disk means the + evidence was reconstructed from memory, not observed -> reject and ask for re-observed + evidence, even if the goal otherwise looks met. +5. Substance, only once 1-4 hold: does the evidence show the discriminator's success signal + POSITIVELY happened -- not just that the named failure modes were dodged; a run can rule out + every trap and still have produced nothing. Is each subtle failure mode genuinely ruled out, + not just unmentioned? If the goal names a verify: command, its saved output must be among the + evidence, and the command must actually test the discriminator rather than pass tautologically. Finish with exactly these two lines and nothing after: VERDICT: accept | reject @@ -143,5 +154,5 @@ modes, verify command, and evidence list from the file itself. ${p.plan} --- end plan file --- -Inspect the repo and artifacts, run verify if present, then give your VERDICT.`; +Read the cited artifacts (you cannot execute anything), then give your VERDICT.`; } diff --git a/test/judge-args.test.ts b/test/judge-args.test.ts index 9704cac..0601132 100644 --- a/test/judge-args.test.ts +++ b/test/judge-args.test.ts @@ -23,7 +23,9 @@ describe("buildJudgeArgs", () => { expect(args).toContain("--no-session"); expect(args).toContain("--no-extensions"); // a broken global extension must not take down sign-offs expect(args).toContain("--tools"); - expect(args.some((a) => a.startsWith("read,bash,grep,find,ls"))).toBe(true); + expect(args.some((a) => a.startsWith("read,grep,find,ls"))).toBe(true); + // no bash: the judge must never be able to execute (or re-run a 10-hour verify) or mutate + expect(args.some((a) => a.includes("bash"))).toBe(false); expect(args).toContain("--exclude-tools"); expect(args.some((a) => a.includes("edit") && a.includes("write"))).toBe(true); }