fix(review): reject stale review evidence at completion

docs/superpowers/specs/2026-08-30-junto-design.md

@@ -119,6 +119,13 @@ bounded output tail and metadata. Successful tool output stays terse, while fail
 Any relevant code edit marks existing verdicts stale. `staleIgnore` defaults to Markdown, docs, and `.junto`
 paths. Three consecutive real failures prompt reconsideration of the plan; skipped runs do not affect the streak.
 
+Review gates additionally bind verdicts to a source/index/HEAD fingerprint, review configuration, task
+base and requirement context. Capture before and after the reviewer runs; recheck at status, transition,
+and archive boundaries. Shell and external edits are detected without a hook. Missing legacy review
+fingerprints require another review. Command gates retain their existing hook invalidation behavior.
+Hooks increment an optional per-gate invalidation version even while evidence is already stale.
+Verification preserves invalidations received during the run, including edits reverted before completion.
+
 ## 8. Hooks
 
 - `session.js` (`SessionStart`): restore the full brief, plan, and recorded decisions after startup/resume/compaction.

packages/core/src/index.ts

@@ -13,3 +13,4 @@ export * from "./review-context.js"
 export * from "./review-scope.js"
 export * from "./cli-review.js"
 export * from "./review-report.js"
+export * from "./review-freshness.js"

packages/core/src/review-freshness.ts

@@ -0,0 +1,65 @@
+import { execFileSync } from "node:child_process"
+import { createHash } from "node:crypto"
+import { closeSync, existsSync, lstatSync, openSync, readFileSync, readlinkSync, readSync, realpathSync } from "node:fs"
+import { basename, isAbsolute, join, relative, resolve } from "node:path"
+import { taskDir } from "./paths.js"
+import type { Config, Task } from "./schema.js"
+
+const IGNORED_UNTRACKED = new Set(["node_modules", "dist", "build", ".temp", ".venv", "__pycache__", ".pytest_cache", ".mypy_cache", ".ruff_cache"])
+const digest = (value: string | Buffer): string => createHash("sha256").update(value).digest("hex")
+
+function fileDigest(path: string): string {
+  const fd = openSync(path, "r")
+  try {
+    const hash = createHash("sha256")
+    const buffer = Buffer.alloc(65536)
+    let count: number
+    while ((count = readSync(fd, buffer, 0, buffer.length, null)) > 0) hash.update(buffer.subarray(0, count))
+    return hash.digest("hex")
+  } finally { closeSync(fd) }
+}
+
+/** Bind review evidence to source and policy without storing source text or reading environment files. */
+export function captureReviewFingerprint(root: string, task: Task, config: Config): string {
+  // Native resolution expands Windows 8.3 aliases used by runner temporary directories.
+  const canonicalRoot = realpathSync.native(root)
+  const git = (...args: string[]): Buffer => execFileSync("git", args, {
+    cwd: root, timeout: 30_000, maxBuffer: 64 * 1024 * 1024, stdio: ["ignore", "pipe", "pipe"], windowsHide: true,
+  })
+  if (relative(canonicalRoot, realpathSync.native(git("rev-parse", "--show-toplevel").toString().trim())) !== "") {
+    throw new Error("Review root must be the Git repository root")
+  }
+  let head: string | null = null
+  // An unborn repository is valid; other Git failures still fail the enumeration below.
+  try { head = git("rev-parse", "--verify", "HEAD").toString().trim() } catch { /* no HEAD yet */ }
+  const index = git("ls-files", "--stage", "-z")
+  const tracked = new Set(index.toString("utf8").split("\0").filter(Boolean).map(entry => entry.slice(entry.indexOf("\t") + 1)))
+  const paths = new Set([...tracked, ...git("ls-files", "--others", "--exclude-standard", "-z").toString("utf8").split("\0").filter(Boolean)])
+  const files: Array<[string, number | null, string]> = []
+  for (const path of [...paths].sort()) {
+    const parts = path.split("/")
+    if (parts[0] === ".junto" || basename(path) === ".env" || basename(path).startsWith(".env.")) continue
+    if (!tracked.has(path) && parts.some(part => IGNORED_UNTRACKED.has(part))) continue
+    const full = resolve(canonicalRoot, path)
+    const rel = relative(canonicalRoot, full)
+    if (rel === ".." || rel.startsWith("../") || rel.startsWith("..\\") || isAbsolute(rel)) throw new Error("Source path escapes review root")
+    let stat
+    try { stat = lstatSync(full) } catch (error) {
+      if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error
+      files.push([path, null, "deleted"])
+      continue
+    }
+    if (stat.isSymbolicLink()) files.push([path, stat.mode, digest(readlinkSync(full))])
+    else if (stat.isFile()) {
+      const target = relative(canonicalRoot, realpathSync.native(full))
+      if (target === ".." || target.startsWith("../") || target.startsWith("..\\") || isAbsolute(target)) throw new Error("Source path escapes review root")
+      files.push([path, stat.mode, fileDigest(full)])
+    } else throw new Error("Review fingerprints do not support source directories or submodules")
+  }
+  const context = ["brief.md", "plan.md", "review-background.md"].map(name => {
+    const path = join(taskDir(root, task.id), name)
+    return existsSync(path) ? digest(readFileSync(path)) : null
+  })
+  return digest(JSON.stringify({ version: 1, head, index: digest(index), files,
+    base: task.baseCommit, title: task.title, context, config }))
+}

packages/core/src/review.ts

@@ -484,6 +484,7 @@ export interface RunReviewGateOptions {
   /** Severities that fail the gate. Defaults to critical and high. */
   failOn?: ReviewSeverity[]
   runner: string
+  reviewFingerprint?: string
 }
 
 export const DEFAULT_FAIL_ON: ReviewSeverity[] = ["critical", "high"]
@@ -578,6 +579,7 @@ export async function runReviewGate(opts: RunReviewGateOptions): Promise<Verdict
     outputBytes: Buffer.byteLength(logOutput, "utf-8"),
     outputFile: `verdicts/${name}.log`,
     runner,
+    ...(opts.reviewFingerprint ? { reviewFingerprint: opts.reviewFingerprint } : {}),
     ...(reason ? { reason } : {}),
   }
 

packages/core/src/schema.ts

@@ -35,6 +35,7 @@ export const gateStatusSchema = z.object({
   required: z.boolean(),
   verdict: z.string().nullable(),
   stale: z.boolean(),
+  invalidationVersion: z.number().int().min(0).optional(),
   failStreak: z.number().int().min(0),
 })
 
@@ -170,6 +171,8 @@ export interface VerdictFile {
   outputFile: string
   runner: string
   reason?: string
+  /** Present on source-bound reviews; legacy reviews must be rerun. */
+  reviewFingerprint?: string
 }
 
 export function parseTask(raw: unknown): Task {

packages/mcp/src/tools/advance.ts

@@ -2,6 +2,7 @@ import { existsSync, readFileSync } from "node:fs"
 import { join } from "node:path"
 import {
   canEnter,
+  captureReviewFingerprint,
   gateStateSchema,
   readActiveId,
   readConfig,
@@ -21,20 +22,34 @@ import { resolveTaskPolicy } from "./policy.js"
 export function buildTransitionContext(root: string, task: Task, config: Config): TransitionContext {
   const dir = taskDir(root, task.id)
 
-  const readState = (rel: string | null): GateState | null => {
+  let fingerprint: string | undefined
+  const readState = (name: string, rel: string | null): GateState | null => {
     if (rel === null) return null
     const path = join(dir, rel)
     if (!existsSync(path)) return null
     try {
-      return gateStateSchema.parse(JSON.parse(readFileSync(path, "utf-8")).state)
+      const verdict = JSON.parse(readFileSync(path, "utf-8"))
+      if (config.gates[name]?.type === "review" || verdict.reviewFingerprint !== undefined) {
+        fingerprint ??= captureReviewFingerprint(root, task, config)
+        if (verdict.reviewFingerprint !== fingerprint) {
+          const gate = task.gates[name]
+          if (gate) gate.stale = true
+          return null
+        }
+      }
+      return gateStateSchema.parse(verdict.state)
     } catch {
+      if (config.gates[name]?.type === "review") {
+        const gate = task.gates[name]
+        if (gate) gate.stale = true
+      }
       return null
     }
   }
 
   const verdictStates: Record<string, GateState | null> = {}
   for (const [name, status] of Object.entries(task.gates)) {
-    verdictStates[name] = readState(status.verdict)
+    verdictStates[name] = readState(name, status.verdict)
   }
 
   const brief = join(dir, "brief.md")

packages/mcp/src/tools/task.ts

@@ -3,10 +3,11 @@ import { join } from "node:path"
 import { execa } from "execa"
 import {
   juntoDir, readActiveId, readConfig, readTask, requiredPhases, setActiveId, taskDir, writeTask,
-  SCHEMA_VERSION,
+  SCHEMA_VERSION, canEnter,
   type GateStatus, type Size, type Task,
 } from "@junto/core"
 import type { ToolContext } from "../context.js"
+import { buildTransitionContext } from "./advance.js"
 
 export type TaskToolInput =
   | { action: "start", title: string, size: Size, gates?: string[] }
@@ -128,6 +129,12 @@ function finish(ctx: ToolContext): string {
     )
   }
   const from = taskDir(ctx.root, id)
+  const config = readConfig(ctx.root)
+  const transition = buildTransitionContext(ctx.root, task, config)
+  if (Object.keys(task.gates).some(name => config.gates[name]?.type === "review" || task.gates[name]?.stale)) {
+    const check = canEnter({ ...task, phase: "verify" }, "done", transition)
+    if (!check.ok) throw new Error(`Cannot archive task. ${check.reason}`)
+  }
   const to = join(juntoDir(ctx.root), "archive", id)
 
   // Safety net for an archive directory created manually between start and finish.

packages/mcp/src/tools/verify.ts

@@ -1,8 +1,8 @@
 import {
-  CliReviewProvider, OpenCodeReviewProvider, ScopedReviewProvider, readActiveId, readConfig, readTask, resolveReviewScopes, runGate, runReviewGate,
+  captureReviewFingerprint, CliReviewProvider, OpenCodeReviewProvider, ScopedReviewProvider, readActiveId, readConfig, readTask, resolveReviewScopes, runGate, runReviewGate, updateTask,
   writeReviewBackground, writeTask,
 } from "@junto/core"
-import type { GateSpec, Task, VerdictFile } from "@junto/core"
+import type { Config, GateSpec, Task, VerdictFile } from "@junto/core"
 import type { ToolContext } from "../context.js"
 import { resolveTaskPolicy } from "./policy.js"
 
@@ -30,15 +30,26 @@ function render(v: VerdictFile, streak: number): string {
     + `\`\`\`\n${v.outputTail}\n\`\`\`${hint}`
 }
 
-async function runReview(ctx: ToolContext, task: Task, name: string, spec: GateSpec): Promise<VerdictFile> {
-  const scopes = await resolveReviewScopes(ctx.root, task.baseCommit)
+async function runReview(ctx: ToolContext, task: Task, name: string, spec: GateSpec, config: Config): Promise<VerdictFile> {
   const backgroundFile = writeReviewBackground(ctx.root, task.id, task.title)
+  const fingerprint = captureReviewFingerprint(ctx.root, task, config)
+  const scopes = await resolveReviewScopes(ctx.root, task.baseCommit)
+  const provider = new ScopedReviewProvider(spec.provider === "cli" ? new CliReviewProvider(spec.timeoutMs)
+    : new OpenCodeReviewProvider({ ...(spec.timeoutMs ? { timeoutMs: spec.timeoutMs } : {}) }), scopes)
   return runReviewGate({
     root: ctx.root,
     taskId: task.id,
     name,
-    provider: new ScopedReviewProvider(spec.provider === "cli" ? new CliReviewProvider(spec.timeoutMs)
-      : new OpenCodeReviewProvider({ ...(spec.timeoutMs ? { timeoutMs: spec.timeoutMs } : {}) }), scopes),
+    reviewFingerprint: fingerprint,
+    provider: { async review(context) {
+      const result = await provider.review(context)
+      const current = readTask(ctx.root, task.id)
+      if (captureReviewFingerprint(ctx.root, current, readConfig(ctx.root)) !== fingerprint
+        || (current.gates[name]?.invalidationVersion ?? 0) !== (task.gates[name]?.invalidationVersion ?? 0)) {
+        result.error = { kind: "incomplete", message: "Source, policy or requirement context changed during review; rerun the review." }
+      }
+      return result
+    } },
     context: {
       ...(backgroundFile ? { backgroundFile } : {}),
     },
@@ -68,11 +79,11 @@ export async function verifyTool(ctx: ToolContext, input: { gates?: string[] }):
     }
 
     // A failed scope lookup or spawn must not leave a previous passing verdict current.
-    status.stale = true
-    writeTask(ctx.root, task)
+    const started = updateTask(ctx.root, id, current => { const gate = current.gates[name]; if (gate) gate.stale = true })
+    status.invalidationVersion = started.gates[name]?.invalidationVersion ?? 0
 
     const verdict = spec.type === "review"
-      ? await runReview(ctx, task, name, spec)
+      ? await runReview(ctx, task, name, spec, config)
       : await runGate({ root: ctx.root, taskId: id, name, spec, runner: ctx.runner })
 
     // Build the path from the validated name rather than coupling to outputFile formatting.
@@ -84,7 +95,13 @@ export async function verifyTool(ctx: ToolContext, input: { gates?: string[] }):
     sections.push(render(verdict, status.failStreak))
 
     // Persist after every gate so a later failure cannot discard completed evidence.
-    writeTask(ctx.root, task)
+    updateTask(ctx.root, id, current => {
+      const version = current.gates[name]?.invalidationVersion ?? 0
+      current.gates[name] = { ...status, invalidationVersion: version, stale: version !== status.invalidationVersion }
+      if (verdict.reviewFingerprint) {
+        current.gates[name].stale ||= captureReviewFingerprint(ctx.root, current, readConfig(ctx.root)) !== verdict.reviewFingerprint
+      }
+    })
   }
 
   return sections.join("\n\n")

packages/mcp/test/verify-review-ocr.test.ts

@@ -1,13 +1,14 @@
 import { execFileSync } from "node:child_process"
-import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"
+import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"
 import { tmpdir } from "node:os"
 import { join } from "node:path"
-import { afterEach, beforeEach, describe, expect, it } from "vitest"
-import { readActiveId, readTask, writeTask } from "@junto/core"
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"
+import { captureReviewFingerprint, OpenCodeReviewProvider, readActiveId, readConfig, readTask, updateTask, writeTask } from "@junto/core"
 import { taskTool } from "../src/tools/task.js"
 import { verifyTool } from "../src/tools/verify.js"
 import { advanceTool } from "../src/tools/advance.js"
 import { resolvePlan } from "../src/tools/plan.js"
+import { statusTool } from "../src/tools/status.js"
 
 /**
  * A real child process stands in for `ocr`, so these tests exercise argv, environment, exit codes and
@@ -85,13 +86,107 @@ beforeEach(() => {
 })
 
 afterEach(() => {
+  vi.restoreAllMocks()
   if (savedBin === undefined) delete process.env.OPEN_CODE_REVIEW_BIN
   else process.env.OPEN_CODE_REVIEW_BIN = savedBin
   rmSync(root, { recursive: true, force: true })
   rmSync(bin, { recursive: true, force: true })
 })
 
 describe("verify with a review gate and a real reviewer process", () => {
+  it("accepts filesystem aliases of the repository but rejects a nested root", async () => {
+    const id = await startTask()
+    const task = readTask(root, id)
+    const config = readConfig(root)
+    const alias = join(bin, "repository-alias")
+    symlinkSync(root, alias, process.platform === "win32" ? "junction" : "dir")
+    expect(captureReviewFingerprint(alias, task, config)).toBe(captureReviewFingerprint(root, task, config))
+    if (process.platform === "win32") {
+      expect(captureReviewFingerprint(root.toUpperCase(), task, config)).toBe(captureReviewFingerprint(root, task, config))
+    }
+    expect(() => captureReviewFingerprint(join(root, "src"), task, config)).toThrow(/repository root/)
+  })
+
+  it.each(["source", "index", "head", "policy", "brief", "plan", "legacy"])("refuses stale %s evidence at status and completion", async change => {
+    installFakeOcr({ status: "complete", comments: [] })
+    const id = await startTask()
+    await advanceTool(ctx(), { to: "verify" })
+    await verifyTool(ctx(), {})
+    const dir = join(root, ".junto", "tasks", id)
+    if (change === "source") writeFileSync(join(root, "src/auth/login.ts"), "throw new Error('new bug')\n")
+    if (change === "index") git("add", "src/auth/login.ts")
+    if (change === "head") git("commit", "--allow-empty", "-qm", "move head")
+    if (change === "policy") {
+      const path = join(root, ".junto", "config.json")
+      const config = JSON.parse(readFileSync(path, "utf8"))
+      config.gates["code-review"].failOn = ["critical", "high", "medium"]
+      writeFileSync(path, JSON.stringify(config))
+    }
+    if (change === "brief" || change === "plan") writeFileSync(join(dir, `${change}.md`), "Changed requirement")
+    if (change === "legacy") {
+      const path = join(dir, "verdicts/code-review.json")
+      const verdict = JSON.parse(readFileSync(path, "utf8"))
+      delete verdict.reviewFingerprint
+      writeFileSync(path, JSON.stringify(verdict))
+    }
+    const stored = readFileSync(join(dir, "task.json"), "utf8")
+    expect(await statusTool(ctx())).toContain("stale")
+    expect(readFileSync(join(dir, "task.json"), "utf8")).toBe(stored)
+    await expect(advanceTool(ctx(), { to: "done" })).rejects.toThrow(/stale/)
+  })
+
+  it("requires fresh evidence when archiving and permits completion after re-review", async () => {
+    installFakeOcr({ status: "complete", comments: [] })
+    await startTask()
+    await advanceTool(ctx(), { to: "verify" })
+    await verifyTool(ctx(), {})
+    await advanceTool(ctx(), { to: "done" })
+    writeFileSync(join(root, "src/auth/login.ts"), "export const changed = true\n")
+    await expect(taskTool(ctx(), { action: "finish" })).rejects.toThrow(/stale/)
+    await verifyTool(ctx(), {})
+    expect(await taskTool(ctx(), { action: "finish" })).toContain("Archived")
+  })
+
+  it("fails when source changes while the reviewer runs", async () => {
+    await startTask()
+    vi.spyOn(OpenCodeReviewProvider.prototype, "review").mockImplementation(async () => {
+      writeFileSync(join(root, "src/auth/login.ts"), "throw new Error('changed during review')\n")
+      return { provider: "open-code-review", findings: [] }
+    })
+    expect(await verifyTool(ctx(), {})).toContain("changed during review")
+    const id = readActiveId(root)
+    expect(readTask(root, id!).gates["code-review"]?.stale).toBe(true)
+  })
+
+  it("preserves hook invalidations even when an in-flight edit is reverted", async () => {
+    const id = await startTask()
+    vi.spyOn(OpenCodeReviewProvider.prototype, "review").mockImplementation(async () => {
+      const path = join(root, "src/auth/login.ts")
+      const original = readFileSync(path)
+      writeFileSync(path, "throw new Error('temporary edit')\n")
+      updateTask(root, id, task => {
+        const gate = task.gates["code-review"]!
+        gate.invalidationVersion = (gate.invalidationVersion ?? 0) + 1
+        gate.stale = true
+      })
+      writeFileSync(path, original)
+      return { provider: "open-code-review", findings: [] }
+    })
+    expect(await verifyTool(ctx(), {})).toContain("changed during review")
+    expect(readTask(root, id).gates["code-review"]?.stale).toBe(true)
+  })
+
+  it("does not archive an old review after changing its configured gate type", async () => {
+    installFakeOcr({ status: "complete", comments: [] })
+    await startTask()
+    await advanceTool(ctx(), { to: "verify" })
+    await verifyTool(ctx(), {})
+    await advanceTool(ctx(), { to: "done" })
+    writeFileSync(join(root, ".junto/config.json"), JSON.stringify({ schemaVersion: 1,
+      gates: { "code-review": { type: "command", argv: ["node", "-e", "process.exit(0)"], required: true } } }))
+    await expect(taskTool(ctx(), { action: "finish" })).rejects.toThrow(/stale/)
+  })
+
   it("passes on a clean review and hands the reviewer the requirement background", async () => {
     installFakeOcr({ status: "complete", comments: null })
     const id = await startTask()