Bind completion evidence to the exact change

.noxroot/skills/independent-review/SKILL.md

@@ -16,6 +16,9 @@ For automated mode, emit exactly one JSON object and no prose:
 
 ```json
 {
+  "schemaVersion": 2,
+  "taskId": "copy from task package",
+  "changeId": "copy from task package",
   "decision": "approved|changes-requested|blocked",
   "summary": "factual summary",
   "findings": [

docs/adapters.md

@@ -23,13 +23,15 @@ The executable must understand this protocol. A vendor CLI name alone does not m
 use documented arguments or a wrapper that translates the task package. Install and authenticate
 that tool separately. Noxroot does not supply provider accounts, credentials, or model access.
 
-Roles are `worker`, `repair`, and `reviewer`. An automated reviewer must write exactly one JSON
-object to standard output with `decision`, `summary`, `findings`, and `learningCandidates`. Findings
-require severity, evidence, and required outcome; optional paths are repository-relative. Prose,
-additional text, missing fields, unknown fields, truncated output, nonzero exit, and a decision
-printed only on standard error all block approval. Diagnostics remain separate on standard error.
-Every invocation is a fresh process. Noxroot does not use a shell, interpolate repository text into
-arguments, bypass permissions, or promise undocumented vendor flags.
+Roles are `worker`, `repair`, and `reviewer`. An automated reviewer must write exactly one version 2
+JSON object to standard output. It must copy `taskId` and `changeId` from the reviewer package, then
+provide `decision`, `summary`, `findings`, and `learningCandidates`. This binding prevents an older
+or unrelated review from approving the current change. Findings require severity, evidence, and
+required outcome; optional paths are repository-relative. Prose, additional text, missing fields,
+unknown fields, a mismatched id, truncated output, nonzero exit, and a decision printed only on
+standard error all block approval. Diagnostics remain separate on standard error. Every invocation
+is a fresh process. Noxroot does not use a shell, interpolate repository text into arguments, bypass
+permissions, or promise undocumented vendor flags.
 
 Before delegated implementation, preflight resolves the configured executable, validates literal
 arguments, checks repository write access and a committed Git baseline, and confirms executables for

docs/architecture.md

@@ -49,8 +49,15 @@ evidence belongs in `.noxroot/knowledge/`.
 
 Guided orchestration is a two-command lifecycle. Start persists repository identity, clean revision,
 bounded context, effective autonomy, and a hash of the approved verification policy. Finish derives
-the real diff and affected checks from that snapshot, then emits a portable reviewer package or a
-strict decision. Local state is never treated as application runtime state.
+the real changed paths and approved checks from that snapshot. A full-content change id establishes
+freshness without retaining file contents; separately bounded and redacted diff evidence supports
+display and review. Reviewer decisions must repeat the package's task and change ids. Local state is
+never treated as application runtime state.
+
+Changed-path and Git metadata capture fails closed when bounded process output cannot represent the
+complete change. Unsupported changed directory or submodule surfaces cannot complete. Identity is
+checked again after approved commands and after review so a concurrently changing repository cannot
+inherit earlier evidence. The same task/change binding applies to guided and delegated reviewers.
 
 Completed and approved local records are pruned by age and count after a run finishes. Running,
 incomplete, failed, blocked, review-pending, and malformed recovery evidence is never removed by
@@ -72,15 +79,16 @@ state, memory, and user data are not Noxroot project knowledge. The MVP uses gen
 detection, approved native tests/evals, and the command-adapter protocol; framework-specific
 semantic modules are deferred.
 
-Controlled learning consumes deterministic verification evidence or already parsed structured
-reviewer candidates. Deterministic signatures deduplicate Noxroot-owned knowledge; first creation
-also updates the index. Every proposed entry names its confirmation date and source task. Per-file
-and total corpus bounds prevent accumulated Markdown from silently consuming future context.
-Learning writes are capped at 1,000,000 bytes across Markdown files, including nested files and
-index growth. The limit is rechecked when a proposal is applied. Symbolic-link destinations are
-refused. A full destination requires deliberate consolidation before another write. Canonical
-`.noxroot/skills/*/SKILL.md` files are short, standards-compatible procedures selected through
-ordinary routing, not a new skill runtime or vendor-specific tree.
+Controlled learning consumes the current approved review's structured candidates. It rejects legacy,
+failed, superseded, or subsequently edited task evidence. Deterministic signatures deduplicate
+Noxroot-owned knowledge; first creation also updates the index. Every proposed entry names its
+confirmation date and source task. Per-file and total corpus bounds prevent accumulated Markdown
+from silently consuming future context. Learning writes are capped at 1,000,000 bytes across
+Markdown files, including nested files and index growth. The limit is rechecked when a proposal is
+applied. Symbolic-link destinations are refused. A full destination requires deliberate
+consolidation before another write. Canonical `.noxroot/skills/*/SKILL.md` files are short,
+standards-compatible procedures selected through ordinary routing, not a new skill runtime or
+vendor-specific tree.
 
 Trust boundaries are described in [security.md](security.md). Public behavior belongs in tests
 before it is claimed in the README.

docs/commands.md

@@ -160,11 +160,17 @@ eligible task is active; multiple tasks require an explicit id. Finish validates
 and the policy snapshot, computes the actual diff, and runs matching approved checks. Routine
 checked changes become `completed` without a reviewer. User-facing, security-sensitive, and
 unusually broad diffs produce a review package and may become `review-pending`. Only a schema-valid
-reviewer can produce `approved`. No matching or available check becomes `incomplete`: local handoff
-can continue, but approval cannot. Finish also reports a deterministic documentation/learning
-assessment without a new model call. When no deterministic documentation signal exists,
-documentation is reported as `not-assessed`; an empty deterministic learning assessment is reported
-as `no-candidate`, not as proof that no documentation could help.
+reviewer response bound to the package's task and full-change ids can produce `approved`. A valid
+response for an older or unrelated change is rejected. No matching or available check becomes
+`incomplete`: local handoff can continue, but approval cannot. Finish also reports a deterministic
+documentation/learning assessment without a new model call. When no deterministic documentation
+signal exists, documentation is reported as `not-assessed`; an empty deterministic learning
+assessment is reported as `no-candidate`, not as proof that no documentation could help.
+
+External review evidence must be an untracked regular JSON file under `.noxroot/local/`. Linked,
+tracked, oversized, mismatched, and malformed evidence is rejected; rejected contents are not kept
+in task state. If a check or reviewer changes the repository, prior verification and review evidence
+becomes stale and `finish` must be run again.
 
 ## `learn`
 
@@ -173,7 +179,10 @@ as `no-candidate`, not as proof that no documentation could help.
 become project knowledge merely because it occurred once. Proposals show evidence, expected value,
 duplication/conflict results, content, and whether an executable guardrail is better. `--apply`
 requires confirmation; the first learnings file and index link are written in the same operation.
-Raw prose, task text, sessions, user data, secrets, and external human docs are not converted into
-knowledge. New entries carry a confirmation date and source task id. Noxroot refuses another entry
-when the destination would exceed `context.documentWarningBytes`; existing knowledge must then be
-consolidated or superseded deliberately.
+Candidates are eligible only after an approved review, and only while the complete change still
+matches that approval. Earlier reviewer calls and candidates from failed or superseded attempts are
+not reused. Freshness is checked again after application confirmation. Raw prose, task text,
+sessions, user data, secrets, and external human docs are not converted into knowledge. New entries
+carry a confirmation date and source task id. Noxroot refuses another entry when the destination
+would exceed `context.documentWarningBytes`; existing knowledge must then be consolidated or
+superseded deliberately.

docs/security.md

@@ -31,8 +31,11 @@ Guided start requires a clean committed baseline. Finish validates repository id
 policy snapshot, derives actual changed paths, includes bounded tracked and new-file diff evidence,
 and treats zero matching checks or unavailable executables as blockers. Diff evidence records the
 path but omits contents for suspected secrets, configured sensitive paths, and symlinks; the same
-redaction applies to connected-agent reviewer packages. Reviewer files are resolved inside the
-repository and must satisfy the same strict JSON contract as command reviewers.
+redaction applies to connected-agent reviewer packages. Freshness uses a separate full-change hash;
+file contents are streamed into it, not retained in task state. Reviewer files are resolved inside
+the dedicated untracked `.noxroot/local/` directory without following links and must satisfy the
+same strict bound JSON contract as command reviewers. Invalid reviewer-file contents are discarded
+rather than persisted as diagnostics.
 
 Negative guarantees are release blockers. A newly discovered path to a preview write, child command,
 agent call, network attempt, secret disclosure, or path escape requires a regression test before

src/adapters/agents.ts

@@ -28,6 +28,9 @@ export interface AgentResult {
 
 export const reviewerResponseSchema = z
   .object({
+    schemaVersion: z.literal(2),
+    taskId: z.string().trim().min(1).max(200),
+    changeId: z.string().regex(/^[a-f0-9]{64}$/),
     decision: z.enum(["approved", "changes-requested", "blocked"]),
     summary: z.string().trim().min(1).max(2_000),
     findings: z
@@ -95,11 +98,21 @@ export class ManualAgentAdapter implements AgentAdapter {
   }
 }
 
-export function parseReviewerResponse(output: string): ReviewerResponse | undefined {
+export function parseReviewerResponse(
+  output: string,
+  expected?: { taskId: string; changeId: string },
+): ReviewerResponse | undefined {
   try {
     const decoded: unknown = JSON.parse(output);
     const parsed = reviewerResponseSchema.safeParse(decoded);
-    return parsed.success ? parsed.data : undefined;
+    if (!parsed.success) return undefined;
+    if (
+      expected &&
+      (parsed.data.taskId !== expected.taskId || parsed.data.changeId !== expected.changeId)
+    ) {
+      return undefined;
+    }
+    return parsed.data;
   } catch {
     return undefined;
   }
@@ -171,9 +184,14 @@ export class CommandAgentAdapter implements AgentAdapter {
         exitCode: evidence.exitCode,
       };
       if (request.role === "reviewer") {
+        const candidate = request.package as { taskId?: unknown; changeId?: unknown };
+        const expected =
+          typeof candidate.taskId === "string" && typeof candidate.changeId === "string"
+            ? { taskId: candidate.taskId, changeId: candidate.changeId }
+            : undefined;
         const review =
-          evidence.exitCode === 0 && !evidence.outputTruncated
-            ? parseReviewerResponse(evidence.stdout)
+          evidence.exitCode === 0 && !evidence.outputTruncated && expected
+            ? parseReviewerResponse(evidence.stdout, expected)
             : undefined;
         if (review) {
           result.review = review;

src/adapters/vcs.ts

@@ -1,4 +1,6 @@
-import { lstat, mkdir, readFile, realpath } from "node:fs/promises";
+import { createHash } from "node:crypto";
+import { createReadStream } from "node:fs";
+import { lstat, mkdir, readFile, readlink, realpath } from "node:fs/promises";
 import path from "node:path";
 import { runProcess } from "./process.js";
 import { prepareStateRoot } from "../state/local.js";
@@ -18,6 +20,13 @@ export interface RepositoryBaseline {
   branch: string;
 }
 
+export interface ChangeIdentity {
+  schemaVersion: 1;
+  baselineRevision: string;
+  changedPaths: string[];
+  changeId: string;
+}
+
 function taskSlug(task: string): string {
   const slug = task
     .toLowerCase()
@@ -70,6 +79,88 @@ export async function revisionInCurrentHistory(root: string, revision: string):
   return result.exitCode === 0;
 }
 
+export async function isTrackedPath(root: string, relativePath: string): Promise<boolean> {
+  const result = await git(root, ["ls-files", "--error-unmatch", "--", relativePath]);
+  return result.exitCode === 0;
+}
+
+export async function identifyChange(
+  root: string,
+  baselineRevision: string,
+  changedPaths: string[],
+): Promise<ChangeIdentity> {
+  const normalizedPaths = [
+    ...new Set(changedPaths.map((value) => value.replaceAll("\\", "/"))),
+  ].sort();
+  const hash = createHash("sha256");
+  hash.update("noxroot-change-identity-v1\0");
+  hash.update(baselineRevision);
+  const trackedMetadata = await git(
+    root,
+    ["diff", "--raw", "--no-abbrev", "-z", baselineRevision, "--"],
+    1_000_000,
+  );
+  if (trackedMetadata.exitCode !== 0 || trackedMetadata.outputTruncated) {
+    throw new Error("Complete Git change metadata could not be captured safely.");
+  }
+  const untrackedMetadata = await git(
+    root,
+    ["ls-files", "--others", "--exclude-standard", "-z"],
+    1_000_000,
+  );
+  if (untrackedMetadata.exitCode !== 0 || untrackedMetadata.outputTruncated) {
+    throw new Error("Complete untracked change metadata could not be captured safely.");
+  }
+  const untrackedPaths = new Set(untrackedMetadata.stdout.split("\0").filter(Boolean));
+  hash.update("\0git-raw\0");
+  hash.update(trackedMetadata.stdout);
+
+  for (const relative of normalizedPaths) {
+    const absolute = resolveWithin(root, relative);
+    hash.update("\0path\0");
+    hash.update(relative);
+    let entry;
+    try {
+      entry = await lstat(absolute);
+    } catch (error) {
+      if ((error as NodeJS.ErrnoException).code === "ENOENT") {
+        hash.update("\0deleted");
+        continue;
+      }
+      throw error;
+    }
+
+    if (entry.isSymbolicLink()) {
+      hash.update("\0symlink\0");
+      hash.update(await readlink(absolute));
+    } else if (entry.isFile()) {
+      hash.update(`\0file\0size:${entry.size}\0`);
+      if (untrackedPaths.has(relative)) {
+        hash.update(entry.mode & 0o111 ? "executable\0" : "not-executable\0");
+      }
+      await new Promise<void>((resolve, reject) => {
+        const stream = createReadStream(absolute);
+        stream.on("data", (chunk) => hash.update(chunk));
+        stream.on("error", reject);
+        stream.on("end", resolve);
+      });
+    } else if (entry.isDirectory()) {
+      throw new Error(
+        `Complete change identity does not support changed directory or submodule path: ${relative}`,
+      );
+    } else {
+      hash.update("\0other");
+    }
+  }
+
+  return {
+    schemaVersion: 1,
+    baselineRevision,
+    changedPaths: normalizedPaths,
+    changeId: hash.digest("hex"),
+  };
+}
+
 function matchesSensitivePath(relative: string, patterns: string[]): boolean {
   const normalized = relative.replaceAll("\\", "/");
   return patterns.some((value) => {
@@ -88,6 +179,7 @@ export async function diffFromRevision(
   root: string,
   revision: string,
   sensitivePaths: string[] = [],
+  excludedPaths: string[] = [],
 ): Promise<string> {
   const tracked = await git(root, ["diff", "--name-only", "-z", revision, "--"], 100_000);
   if (tracked.exitCode !== 0) return `Diff unavailable: ${tracked.stderr.trim()}`;
@@ -114,6 +206,7 @@ export async function diffFromRevision(
       "--",
       ".",
       ...protectedTracked.map((entry) => `:(top,exclude,literal)${entry.path}`),
+      ...excludedPaths.map((entry) => `:(top,exclude,literal)${entry}`),
     ],
     100_000,
   );
@@ -130,10 +223,12 @@ export async function diffFromRevision(
     remaining -= Buffer.byteLength(bounded);
   }
   for (const relative of untracked.stdout.split("\0").filter(Boolean).sort()) {
+    if (excludedPaths.includes(relative)) continue;
     if (remaining <= 0) break;
     const absolute = resolveWithin(root, relative);
     const file = await lstat(absolute);
-    const header = `\ndiff --git a/${relative} b/${relative}\nnew file mode ${file.isSymbolicLink() ? "120000" : "100644"}\n--- /dev/null\n+++ b/${relative}\n`;
+    const mode = file.isSymbolicLink() ? "120000" : file.mode & 0o111 ? "100755" : "100644";
+    const header = `\ndiff --git a/${relative} b/${relative}\nnew file mode ${mode}\n--- /dev/null\n+++ b/${relative}\n`;
     let body: string;
     if (isSuspectedSecret(relative) || matchesSensitivePath(relative, sensitivePaths)) {
       body = `Content omitted for sensitive path ${relative}.\n`;

src/cli.ts

@@ -12,6 +12,7 @@ import { configuredAgent, ManualAgentAdapter } from "./adapters/agents.js";
 import {
   boundedDiff,
   captureRepositoryBaseline,
+  identifyChange,
   prepareIsolatedWorktree,
   revisionInCurrentHistory,
 } from "./adapters/vcs.js";
@@ -61,6 +62,7 @@ import {
   executeVerification,
   planVerification,
   selectVerification,
+  unmatchedVerificationPaths,
 } from "./verification/index.js";
 
 const DESCRIPTION =
@@ -1006,6 +1008,19 @@ export function createProgram(customIo?: Partial<Io>): Command {
                 });
               },
               diff: () => boundedDiff(worktree, config?.sensitivePaths ?? []),
+              changeId: async () => {
+                const actualChanged = await changedFiles(worktree.path, worktree.baseRevision, {
+                  strict: true,
+                });
+                return (await identifyChange(worktree.path, worktree.baseRevision, actualChanged))
+                  .changeId;
+              },
+              unmatchedPaths: async () => {
+                const actualChanged = await changedFiles(worktree.path, worktree.baseRevision, {
+                  strict: true,
+                });
+                return unmatchedVerificationPaths(checks, actualChanged);
+              },
             },
           );
           const recordPath = await writeRunRecord(root, id, record);
@@ -1138,9 +1153,24 @@ export function createProgram(customIo?: Partial<Io>): Command {
         io.stderr("Learning application cancelled; durable knowledge was not changed.\n");
         return;
       }
+      const refreshed = await proposeLearnings(common.root, record);
+      const refreshedBySignature = new Map(
+        refreshed.proposals
+          .filter(
+            (proposal) => proposal.duplication === "not-found" && proposal.conflict === "none",
+          )
+          .map((proposal) => [proposal.signature, proposal]),
+      );
+      if (applicable.some((proposal) => !refreshedBySignature.has(proposal.signature))) {
+        throw new Error(
+          "Learning stopped because the approved change or its eligible proposals changed; review it again.",
+        );
+      }
       const applied: string[] = [];
       for (const proposal of applicable) {
-        applied.push(...(await applyLearning(common.root, proposal)));
+        applied.push(
+          ...(await applyLearning(common.root, refreshedBySignature.get(proposal.signature)!)),
+        );
       }
       if (common.json) writeJson(io, { ...result, applied });
       else io.stdout(`Applied ${applicable.length} proposal(s).\n`);