diff --git a/.noxroot/skills/independent-review/SKILL.md b/.noxroot/skills/independent-review/SKILL.md index e559681..beaefb2 100644 --- a/.noxroot/skills/independent-review/SKILL.md +++ b/.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": [ diff --git a/docs/adapters.md b/docs/adapters.md index c977ece..8a8fcf3 100644 --- a/docs/adapters.md +++ b/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 diff --git a/docs/architecture.md b/docs/architecture.md index cdfd5b7..8e325ba 100644 --- a/docs/architecture.md +++ b/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. diff --git a/docs/commands.md b/docs/commands.md index f771155..cbb7de0 100644 --- a/docs/commands.md +++ b/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. diff --git a/docs/security.md b/docs/security.md index e220605..7f61900 100644 --- a/docs/security.md +++ b/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 diff --git a/src/adapters/agents.ts b/src/adapters/agents.ts index 851decd..1fc6682 100644 --- a/src/adapters/agents.ts +++ b/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; diff --git a/src/adapters/vcs.ts b/src/adapters/vcs.ts index 5c84405..da43de0 100644 --- a/src/adapters/vcs.ts +++ b/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 { + 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 { + 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((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 { 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`; diff --git a/src/cli.ts b/src/cli.ts index 7aa489c..e97b9ef 100644 --- a/src/cli.ts +++ b/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): 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): 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`); diff --git a/src/core/intent.ts b/src/core/intent.ts index e68683a..9670077 100644 --- a/src/core/intent.ts +++ b/src/core/intent.ts @@ -1,12 +1,32 @@ import type { TaskIntent } from "../model.js"; -const EXCLUSION = /\b(?:do not|don't|must not|never|without|except(?:ing)?|exclude|avoid)\b/i; +const EXCLUSION = + /\b(?:do not|don't|must not|never|without|except(?:ing)?|exclude|avoid(?:ing)?)\b/i; const ACCEPTANCE = /\b(?:acceptance|must|should|when|so that|ensure|verify)\b/i; const AUTHORITY = /\b(push|merge|deploy|publish|release)\b/gi; +const POSITIVE_ACTION = + /^(?:add|build|change|create|document|ensure|fix|implement|improve|preserve|refactor|remove|run|support|test|update|write)\b/i; + +function splitConjunctions(value: string): string[] { + const parts = value.split(/\s+and\s+/i); + if (parts.length === 1) return parts; + const result: string[] = []; + for (const part of parts) { + const previous = result.at(-1); + const boundary = + EXCLUSION.test(part) || + (previous !== undefined && EXCLUSION.test(previous) && POSITIVE_ACTION.test(part)); + if (boundary || previous === undefined) result.push(part); + else result[result.length - 1] = `${previous} and ${part}`; + } + return result; +} function clauses(task: string): string[] { return task .split(/(?:\r?\n|[.;](?:\s|$))/) + .flatMap((value) => value.split(/\s+(?:but|while)\s+/i)) + .flatMap(splitConjunctions) .flatMap((value) => { const trimmed = value.trim(); const without = /\bwithout\b/i.exec(trimmed); diff --git a/src/core/proposals.ts b/src/core/proposals.ts index e86b026..f6e06d1 100644 --- a/src/core/proposals.ts +++ b/src/core/proposals.ts @@ -75,7 +75,7 @@ Inspect the diff independently of worker rationale. Check acceptance criteria, c For automated mode, emit exactly one JSON object and no prose: \`\`\`json -{"decision":"approved|changes-requested|blocked","summary":"factual summary","findings":[{"severity":"critical|high|medium|low","path":"optional/path","evidence":"specific evidence","requiredOutcome":"required result"}],"learningCandidates":[]} +{"schemaVersion":2,"taskId":"copy from task package","changeId":"copy from task package","decision":"approved|changes-requested|blocked","summary":"factual summary","findings":[{"severity":"critical|high|medium|low","path":"optional/path","evidence":"specific evidence","requiredOutcome":"required result"}],"learningCandidates":[]} \`\`\` `; diff --git a/src/knowledge/learn.ts b/src/knowledge/learn.ts index 4ff046b..015357a 100644 --- a/src/knowledge/learn.ts +++ b/src/knowledge/learn.ts @@ -2,9 +2,11 @@ import { createHash } from "node:crypto"; import { lstat, mkdir, readFile, readdir, rename, stat, writeFile } from "node:fs/promises"; import path from "node:path"; import type { ReviewerResponse } from "../adapters/agents.js"; +import { identifyChange, isTrackedPath, type ChangeIdentity } from "../adapters/vcs.js"; import { loadConfig } from "../config/load.js"; import type { RunRecord } from "../orchestration/run.js"; import { resolveWithin } from "../security/paths.js"; +import { changedFiles } from "../verification/index.js"; export type LearningKind = "knowledge" | "decision" | "procedure" | "verification"; @@ -146,10 +148,32 @@ function structuredCandidates(run: RunRecord): ReviewerResponse["learningCandida learningCandidates?: ReviewerResponse["learningCandidates"]; } ).learningCandidates; - return [ - ...(direct ?? []), - ...run.calls.flatMap((call) => call.result.review?.learningCandidates ?? []), - ]; + return direct ?? []; +} + +async function currentApprovedChange(root: string, run: RunRecord): Promise { + const guided = run as RunRecord & { + mode?: unknown; + baseline?: { revision?: unknown }; + changeIdentity?: ChangeIdentity; + reviewEvidencePath?: string; + }; + if ( + run.status !== "approved" || + guided.mode !== "guided" || + typeof guided.baseline?.revision !== "string" || + !guided.changeIdentity + ) { + return false; + } + if (guided.reviewEvidencePath && (await isTrackedPath(root, guided.reviewEvidencePath))) { + return false; + } + const changedPaths = ( + await changedFiles(root, guided.baseline.revision, { strict: true }) + ).filter((changedPath) => changedPath !== guided.reviewEvidencePath); + const current = await identifyChange(root, guided.baseline.revision, changedPaths); + return current.changeId === guided.changeIdentity.changeId; } function fromReviewer( @@ -190,6 +214,14 @@ ${candidate.content.trim()} } export async function proposeLearnings(root: string, run: RunRecord): Promise { + if (!(await currentApprovedChange(root, run))) { + return { + taskId: run.id, + proposals: [], + rejected: [], + message: "Learning requires an approved review of the current unchanged diff", + }; + } const candidates = structuredCandidates(run) .map(fromReviewer) .filter((candidate): candidate is Candidate => candidate !== undefined); diff --git a/src/orchestration/guided.ts b/src/orchestration/guided.ts index 70c25a7..2426ccd 100644 --- a/src/orchestration/guided.ts +++ b/src/orchestration/guided.ts @@ -1,13 +1,25 @@ import { createHash } from "node:crypto"; -import { readFile } from "node:fs/promises"; +import { constants } from "node:fs"; +import { open, realpath } from "node:fs/promises"; import path from "node:path"; import type { AgentAdapter, AgentResult, ReviewerResponse } from "../adapters/agents.js"; import { parseReviewerResponse } from "../adapters/agents.js"; -import { captureRepositoryBaseline, diffFromRevision } from "../adapters/vcs.js"; +import { + captureRepositoryBaseline, + diffFromRevision, + identifyChange, + isTrackedPath, + type ChangeIdentity, +} from "../adapters/vcs.js"; import type { ContextPackage, VerificationCommand, VerificationResult } from "../model.js"; import { cliCommand } from "../invocation.js"; -import { resolveWithin } from "../security/paths.js"; -import { changedFiles, executeVerification, selectVerification } from "../verification/index.js"; +import { isWithin, resolveWithin } from "../security/paths.js"; +import { + changedFiles, + executeVerification, + selectVerification, + unmatchedVerificationPaths, +} from "../verification/index.js"; import type { EffectiveAutonomy } from "./autonomy.js"; import type { RunRecord } from "./run.js"; import { assessReviewNeed, type ReviewAssessment } from "./review.js"; @@ -24,6 +36,10 @@ export interface GuidedRunRecord extends RunRecord { startedAt: string; finishedAt?: string; changedPaths?: string[]; + unmatchedChangedPaths?: string[]; + reviewEvidencePath?: string; + changeIdentity?: ChangeIdentity; + /** @deprecated Read-only compatibility with 0.1 task records. */ diffHash?: string; reviewerPackage?: unknown; learningCandidates?: ReviewerResponse["learningCandidates"]; @@ -53,6 +69,26 @@ function samePath(left: string, right: string): boolean { return normalize(left) === normalize(right); } +async function readReviewerEvidence(root: string, relativePath: string): Promise { + const target = resolveWithin(root, relativePath); + const canonical = await realpath(target); + if (!isWithin(await realpath(root), canonical) || !samePath(target, canonical)) { + throw new Error("Reviewer evidence must not use a symbolic link or leave the repository."); + } + const handle = await open(target, constants.O_RDONLY | (constants.O_NOFOLLOW ?? 0)); + try { + const metadata = await handle.stat(); + if (!metadata.isFile() || metadata.size > 1_000_000) { + throw new Error( + "Reviewer evidence must be a regular JSON file no larger than 1000000 bytes.", + ); + } + return await handle.readFile("utf8"); + } finally { + await handle.close(); + } +} + function continuationNextAction( record: GuidedRunRecord, changedPaths: string[], @@ -87,16 +123,24 @@ export async function inspectGuidedContinuation( record: GuidedRunRecord, sensitivePaths: string[] = [], ): Promise { - const changedPaths = await changedFiles(root, record.baseline.revision); - const currentDiff = await diffFromRevision(root, record.baseline.revision, sensitivePaths); - const currentDiffHash = createHash("sha256").update(currentDiff).digest("hex"); + const changedPaths = ( + await changedFiles(root, record.baseline.revision, { strict: true }) + ).filter((changedPath) => changedPath !== record.reviewEvidencePath); + const currentIdentity = await identifyChange(root, record.baseline.revision, changedPaths); const latestChecks = record.verification.at(-1) ?? []; let status: ContinuationVerificationStatus; let summary: string; - if (!record.diffHash) { + if (!record.changeIdentity && !record.diffHash) { status = "not-run"; summary = "Not run for the current diff."; - } else if (record.diffHash !== currentDiffHash) { + } else if ( + record.changeIdentity + ? record.changeIdentity.changeId !== currentIdentity.changeId + : record.diffHash !== + createHash("sha256") + .update(await diffFromRevision(root, record.baseline.revision, sensitivePaths)) + .digest("hex") + ) { status = "stale"; summary = "Stale because the diff changed afterward."; } else if ( @@ -224,28 +268,93 @@ export async function finishGuidedRun(input: { if (policyHash(record.trustedVerificationPolicy) !== record.verificationPolicyHash) { throw new Error("The recorded verification policy snapshot is invalid."); } - const changedPaths = await changedFiles(input.root, record.baseline.revision); + const reviewEvidencePath = input.reviewFile + ? path.relative(input.root, resolveWithin(input.root, input.reviewFile)).replaceAll("\\", "/") + : undefined; + if ( + reviewEvidencePath && + (!reviewEvidencePath.toLowerCase().startsWith(".noxroot/local/") || + !reviewEvidencePath.toLowerCase().endsWith(".json")) + ) { + throw new Error("Reviewer evidence must be an untracked JSON file under .noxroot/local/."); + } + if (reviewEvidencePath && (await isTrackedPath(input.root, reviewEvidencePath))) { + throw new Error("Reviewer evidence must not be a tracked repository file."); + } + const changedPaths = ( + await changedFiles(input.root, record.baseline.revision, { strict: true }) + ).filter((changedPath) => changedPath !== reviewEvidencePath); for (const changedPath of changedPaths) resolveWithin(input.root, changedPath); + const changeIdentity = await identifyChange(input.root, record.baseline.revision, changedPaths); const diff = await diffFromRevision( input.root, record.baseline.revision, input.sensitivePaths ?? [], + reviewEvidencePath ? [reviewEvidencePath] : [], ); - const diffHash = createHash("sha256").update(diff).digest("hex"); + const changedAfterDiff = ( + await changedFiles(input.root, record.baseline.revision, { strict: true }) + ).filter((changedPath) => changedPath !== reviewEvidencePath); + const identityAfterDiff = await identifyChange( + input.root, + record.baseline.revision, + changedAfterDiff, + ); + if (identityAfterDiff.changeId !== changeIdentity.changeId) { + const stale: GuidedRunRecord = { + ...record, + status: "incomplete", + changedPaths, + changeIdentity, + verification: [...record.verification, []], + verificationGaps: ["The repository changed while review evidence was captured."], + handoff: "", + }; + stale.handoff = guidedHandoff(stale, []); + return stale; + } const reviewAssessment = assessReviewNeed(changedPaths, diff, record.task); const commands = selectVerification(record.trustedVerificationPolicy, changedPaths); + const unmatchedChangedPaths = unmatchedVerificationPaths( + record.trustedVerificationPolicy, + changedPaths, + ); const checks = await executeVerification(input.root, commands, { ...(input.signal === undefined ? {} : { signal: input.signal }), }); + const freshRecord = { ...record }; + delete freshRecord.learningCandidates; + delete freshRecord.reviewerPackage; + delete freshRecord.reviewDecision; + delete freshRecord.finishedAt; const next: GuidedRunRecord = { - ...record, + ...freshRecord, changedPaths, - diffHash, + unmatchedChangedPaths, + ...(reviewEvidencePath === undefined ? {} : { reviewEvidencePath }), + changeIdentity, reviewAssessment, verification: [...record.verification, checks], verificationGaps: [], }; + const changedAfterChecks = ( + await changedFiles(input.root, record.baseline.revision, { strict: true }) + ).filter((changedPath) => changedPath !== reviewEvidencePath); + const identityAfterChecks = await identifyChange( + input.root, + record.baseline.revision, + changedAfterChecks, + ); + if (identityAfterChecks.changeId !== changeIdentity.changeId) { + next.status = "incomplete"; + next.verificationGaps = [ + "The repository changed while approved checks ran; their evidence is stale.", + ]; + next.handoff = guidedHandoff(next, checks); + return next; + } + if (changedPaths.length === 0) { next.status = "blocked"; next.verificationGaps = ["No repository change was detected from the recorded baseline."]; @@ -258,6 +367,12 @@ export async function finishGuidedRun(input: { next.handoff = guidedHandoff(next, checks); return next; } + if (unmatchedChangedPaths.length > 0) { + next.status = "incomplete"; + next.verificationGaps = [`No approved check applies to: ${unmatchedChangedPaths.join(", ")}.`]; + next.handoff = guidedHandoff(next, checks); + return next; + } if (checks.some((result) => result.status === "unavailable")) { next.status = "incomplete"; next.verificationGaps = checks @@ -285,6 +400,9 @@ export async function finishGuidedRun(input: { } const reviewerPackage = { + schemaVersion: 2, + taskId: record.id, + changeId: changeIdentity.changeId, task: record.task, context: record.context, changedPaths, @@ -292,6 +410,9 @@ export async function finishGuidedRun(input: { verification: checks, reviewAssessment, responseContract: { + schemaVersion: 2, + taskId: record.id, + changeId: changeIdentity.changeId, decision: "approved | changes-requested | blocked", summary: "short factual summary", findings: ["severity, optional path, evidence, requiredOutcome"], @@ -301,8 +422,11 @@ export async function finishGuidedRun(input: { next.reviewerPackage = reviewerPackage; let reviewResult: AgentResult | undefined; if (input.reviewFile) { - const source = await readFile(resolveWithin(input.root, input.reviewFile), "utf8"); - const review = parseReviewerResponse(source); + const source = await readReviewerEvidence(input.root, input.reviewFile); + const review = parseReviewerResponse(source, { + taskId: record.id, + changeId: changeIdentity.changeId, + }); reviewResult = review ? { invoked: false, @@ -317,7 +441,8 @@ export async function finishGuidedRun(input: { invoked: false, status: "failed", summary: "External reviewer file was not one schema-valid JSON response.", - output: source, + output: "", + diagnostics: "Rejected reviewer evidence was not retained.", exitCode: null, reviewDecision: "blocked", }; @@ -341,6 +466,23 @@ export async function finishGuidedRun(input: { } next.calls = [...record.calls, { role: "reviewer", result: reviewResult }]; const review = reviewResult.review; + const changedAfterReview = ( + await changedFiles(input.root, record.baseline.revision, { strict: true }) + ).filter((changedPath) => changedPath !== reviewEvidencePath); + const identityAfterReview = await identifyChange( + input.root, + record.baseline.revision, + changedAfterReview, + ); + if (identityAfterReview.changeId !== changeIdentity.changeId) { + next.status = "blocked"; + next.verificationGaps = [ + "The repository changed during review; verification and the reviewer decision are stale.", + ]; + next.learningCandidates = []; + next.handoff = guidedHandoff(next, checks); + return next; + } next.reviewDecision = reviewResult.reviewDecision; next.learningCandidates = review?.learningCandidates ?? []; next.status = diff --git a/src/orchestration/run.ts b/src/orchestration/run.ts index a026225..03b4736 100644 --- a/src/orchestration/run.ts +++ b/src/orchestration/run.ts @@ -47,6 +47,26 @@ export interface OrchestrationRequest { export interface OrchestrationDependencies { verify: () => Promise; diff: () => Promise; + changeId: () => Promise; + unmatchedPaths: () => Promise; +} + +async function stableReviewEvidence( + dependencies: OrchestrationDependencies, +): Promise<{ diff: string; changeId: string } | undefined> { + const before = await dependencies.changeId(); + const diff = await dependencies.diff(); + return (await dependencies.changeId()) === before ? { diff, changeId: before } : undefined; +} + +async function stableVerificationCoverage( + dependencies: OrchestrationDependencies, +): Promise<{ changeId: string; unmatchedPaths: string[] } | undefined> { + const before = await dependencies.changeId(); + const unmatchedPaths = await dependencies.unmatchedPaths(); + return (await dependencies.changeId()) === before + ? { changeId: before, unmatchedPaths } + : undefined; } function passed(results: VerificationResult[]): boolean { @@ -153,8 +173,48 @@ export async function orchestrateRun( return { ...partial, handoff: handoff(partial) }; } + let coverage = await stableVerificationCoverage(dependencies); + if (!coverage) { + verificationGaps.push("The repository changed while verification coverage was inspected."); + } else if (coverage.unmatchedPaths.length > 0) { + verificationGaps.push(`No approved check applies to: ${coverage.unmatchedPaths.join(", ")}.`); + } + if (verificationGaps.length > 0) { + const partial: Omit = { + id: request.id, + task: request.task, + status: "incomplete", + ...(request.branch === undefined ? {} : { branch: request.branch }), + worktree: request.cwd, + calls, + verification, + verificationGaps, + }; + return { ...partial, handoff: handoff(partial) }; + } + let verificationChangeId = coverage!.changeId; let checkResults = await dependencies.verify(); + let verificationStable = + verificationChangeId !== undefined && (await dependencies.changeId()) === verificationChangeId; verification.push(checkResults); + if (!verificationStable) { + verificationGaps.push( + verificationChangeId + ? "The repository changed while approved checks ran; their evidence is stale." + : "A complete change id was unavailable for verification.", + ); + const partial: Omit = { + id: request.id, + task: request.task, + status: "incomplete", + ...(request.branch === undefined ? {} : { branch: request.branch }), + worktree: request.cwd, + calls, + verification, + verificationGaps, + }; + return { ...partial, handoff: handoff(partial) }; + } if (checkResults.length === 0) { verificationGaps.push("No approved deterministic checks matched the change."); const partial: Omit = { @@ -205,8 +265,44 @@ export async function orchestrateRun( }); calls.push({ role: "repair", result: repair }); if (repair.status !== "completed") break; + coverage = await stableVerificationCoverage(dependencies); + if (!coverage || coverage.unmatchedPaths.length > 0) { + verificationStable = false; + verificationGaps.push( + coverage + ? `No approved check applies to: ${coverage.unmatchedPaths.join(", ")}.` + : "The repository changed while verification coverage was inspected.", + ); + break; + } + verificationChangeId = coverage.changeId; checkResults = await dependencies.verify(); + verificationStable = + verificationChangeId !== undefined && + (await dependencies.changeId()) === verificationChangeId; verification.push(checkResults); + if (!verificationStable) { + verificationGaps.push( + verificationChangeId + ? "The repository changed while approved checks ran; their evidence is stale." + : "A complete change id was unavailable for verification.", + ); + break; + } + } + + if (!verificationStable) { + const partial: Omit = { + id: request.id, + task: request.task, + status: "incomplete", + ...(request.branch === undefined ? {} : { branch: request.branch }), + worktree: request.cwd, + calls, + verification, + verificationGaps, + }; + return { ...partial, handoff: handoff(partial) }; } if (!passed(checkResults)) { @@ -223,7 +319,24 @@ export async function orchestrateRun( return { ...partial, handoff: handoff(partial) }; } - const reviewDiff = await dependencies.diff(); + const initialReviewEvidence = await stableReviewEvidence(dependencies); + if (!initialReviewEvidence || initialReviewEvidence.changeId !== verificationChangeId) { + const partial: Omit = { + id: request.id, + task: request.task, + status: "incomplete", + ...(request.branch === undefined ? {} : { branch: request.branch }), + worktree: request.cwd, + calls, + verification, + verificationGaps: [ + ...verificationGaps, + "The repository changed while review evidence was captured.", + ], + }; + return { ...partial, handoff: handoff(partial) }; + } + const reviewDiff = initialReviewEvidence.diff; const reviewAssessment = assessReviewNeed([], reviewDiff, request.task); if (!reviewAssessment.required) { const partial: Omit = { @@ -270,9 +383,13 @@ export async function orchestrateRun( return { ...partial, handoff: handoff(partial) }; } + let reviewChangeId: string | undefined = initialReviewEvidence.changeId; let review = await request.adapter.invoke({ role: "reviewer", package: { + schemaVersion: 2, + taskId: request.id, + changeId: reviewChangeId, original: request.context, diff: reviewDiff, reviewAssessment, @@ -304,29 +421,102 @@ export async function orchestrateRun( }); calls.push({ role: "repair", result: repair }); if (repair.status === "completed") { - checkResults = await dependencies.verify(); - verification.push(checkResults); + coverage = await stableVerificationCoverage(dependencies); + if (!coverage || coverage.unmatchedPaths.length > 0) { + verificationStable = false; + verificationGaps.push( + coverage + ? `No approved check applies to: ${coverage.unmatchedPaths.join(", ")}.` + : "The repository changed while verification coverage was inspected.", + ); + } else { + verificationChangeId = coverage.changeId; + checkResults = await dependencies.verify(); + verificationStable = (await dependencies.changeId()) === verificationChangeId; + verification.push(checkResults); + } if ( + verificationStable && passed(checkResults) && calls.filter((call) => call.role === "reviewer").length < request.budgets.reviewerCalls ) { - review = await request.adapter.invoke({ - role: "reviewer", - package: { - original: request.context, - diff: await dependencies.diff(), - verification: checkResults, - priorFindings: review.output, - }, - cwd: request.cwd, - repositoryRoot: request.repositoryRoot, - ...(request.signal === undefined ? {} : { signal: request.signal }), - }); - calls.push({ role: "reviewer", result: review }); + const repairedReviewEvidence = await stableReviewEvidence(dependencies); + reviewChangeId = + repairedReviewEvidence?.changeId === verificationChangeId + ? repairedReviewEvidence.changeId + : undefined; + if (repairedReviewEvidence?.changeId === verificationChangeId) { + review = await request.adapter.invoke({ + role: "reviewer", + package: { + schemaVersion: 2, + taskId: request.id, + changeId: reviewChangeId, + original: request.context, + diff: repairedReviewEvidence.diff, + verification: checkResults, + priorFindings: review.output, + }, + cwd: request.cwd, + repositoryRoot: request.repositoryRoot, + ...(request.signal === undefined ? {} : { signal: request.signal }), + }); + calls.push({ role: "reviewer", result: review }); + } } } } + if (!verificationStable) { + const partial: Omit = { + id: request.id, + task: request.task, + status: "incomplete", + ...(request.branch === undefined ? {} : { branch: request.branch }), + worktree: request.cwd, + calls, + verification, + verificationGaps: [ + ...verificationGaps, + "The repository changed while approved checks ran; their evidence is stale.", + ], + }; + return { ...partial, handoff: handoff(partial) }; + } + + if (!reviewChangeId) { + const partial: Omit = { + id: request.id, + task: request.task, + status: "incomplete", + ...(request.branch === undefined ? {} : { branch: request.branch }), + worktree: request.cwd, + calls, + verification, + verificationGaps: [ + ...verificationGaps, + "The repository changed while review evidence was captured.", + ], + }; + return { ...partial, handoff: handoff(partial) }; + } + if ((await dependencies.changeId()) !== reviewChangeId) { + const partial: Omit = { + id: request.id, + task: request.task, + status: "blocked", + ...(request.branch === undefined ? {} : { branch: request.branch }), + worktree: request.cwd, + calls, + verification, + verificationGaps: [ + ...verificationGaps, + "The repository changed during review; verification and the reviewer decision are stale.", + ], + }; + return { ...partial, handoff: handoff(partial) }; + } + const status: RunRecord["status"] = review.status !== "completed" ? "failed" diff --git a/src/state/local.ts b/src/state/local.ts index 346e06d..2370442 100644 --- a/src/state/local.ts +++ b/src/state/local.ts @@ -46,7 +46,10 @@ export async function localStateRoot(root: string): Promise { if (!(await pathType(path.join(root, ".git")))) return legacy; const local = await setupDestination(root, ".noxroot/local"); if (await pathType(legacy)) { - if (await pathType(local)) { + // Review evidence is intentionally placed under .noxroot/local, including in + // repositories whose older task records still live under .git/noxroot. Only + // a runs entry makes the local directory a competing task-state store. + if (await pathType(path.join(local, "runs"))) { throw new TaskStateError( "Two task-state directories exist. Stop and reconcile the existing records before retrying; neither directory was changed.", ); diff --git a/src/verification/index.ts b/src/verification/index.ts index 63426a5..e7c574a 100644 --- a/src/verification/index.ts +++ b/src/verification/index.ts @@ -5,10 +5,13 @@ import { scanRepository } from "../detection/scan.js"; import type { VerificationCommand, VerificationResult } from "../model.js"; import { runProcess, type ProcessRequest } from "../adapters/process.js"; -function matches(pattern: string, changedPath: string): boolean { +export function matchesVerificationPath(pattern: string, changedPath: string): boolean { const normalized = changedPath.replaceAll("\\", "/"); if (pattern === "**/*" || pattern === "**") return true; - if (pattern.endsWith("/**")) return normalized.startsWith(pattern.slice(0, -3)); + if (pattern.endsWith("/**")) { + const directory = pattern.slice(0, -3).replace(/\/$/, ""); + return normalized === directory || normalized.startsWith(`${directory}/`); + } if (pattern.startsWith("**/*.")) return normalized.endsWith(pattern.slice(4)); if (!pattern.includes("*")) return normalized === pattern; const escaped = pattern @@ -26,11 +29,23 @@ export function selectVerification( if (changedPaths.length === 0) return []; return commands.filter((command) => command.appliesTo.some((pattern) => - changedPaths.some((changedPath) => matches(pattern, changedPath)), + changedPaths.some((changedPath) => matchesVerificationPath(pattern, changedPath)), ), ); } +export function unmatchedVerificationPaths( + commands: VerificationCommand[], + changedPaths: string[], +): string[] { + return changedPaths.filter( + (changedPath) => + !commands.some((command) => + command.appliesTo.some((pattern) => matchesVerificationPath(pattern, changedPath)), + ), + ); +} + export async function planVerification( root: string, changedPaths: string[] = [], @@ -113,7 +128,11 @@ export async function executeVerification( return results; } -export async function changedFiles(root: string, baseRevision?: string): Promise { +export async function changedFiles( + root: string, + baseRevision?: string, + options: { strict?: boolean } = {}, +): Promise { try { const result = await runProcess({ executable: "git", @@ -123,7 +142,10 @@ export async function changedFiles(root: string, baseRevision?: string): Promise timeoutMs: 10_000, outputLimitBytes: 1_000_000, }); - if (result.exitCode !== 0) return []; + if (result.exitCode !== 0 || result.outputTruncated) { + if (options.strict) throw new Error("Complete changed-path metadata could not be captured."); + return []; + } const parts = result.stdout.split("\0").filter(Boolean); const files: string[] = []; for (let index = 0; index < parts.length; index += 1) { @@ -131,7 +153,10 @@ export async function changedFiles(root: string, baseRevision?: string): Promise if (!entry || entry.length < 4) continue; const status = entry.slice(0, 2); files.push(entry.slice(3).replaceAll("\\", "/")); - if ((status.includes("R") || status.includes("C")) && parts[index + 1]) index += 1; + if ((status.includes("R") || status.includes("C")) && parts[index + 1]) { + files.push(parts[index + 1]!.replaceAll("\\", "/")); + index += 1; + } } if (baseRevision) { const committed = await runProcess({ @@ -142,12 +167,15 @@ export async function changedFiles(root: string, baseRevision?: string): Promise timeoutMs: 10_000, outputLimitBytes: 1_000_000, }); - if (committed.exitCode === 0) { + if (committed.exitCode === 0 && !committed.outputTruncated) { files.push(...committed.stdout.split("\0").filter(Boolean)); + } else if (options.strict) { + throw new Error("Complete committed-path metadata could not be captured."); } } return [...new Set(files.map((file) => file.replaceAll("\\", "/")))].sort(); - } catch { + } catch (error) { + if (options.strict) throw error; return []; } } diff --git a/tests/acceptance/p1-p2-live.mjs b/tests/acceptance/p1-p2-live.mjs index 38a332d..4ef743b 100644 --- a/tests/acceptance/p1-p2-live.mjs +++ b/tests/acceptance/p1-p2-live.mjs @@ -160,6 +160,9 @@ if (mode === "prepare") { state.firstDiff = git(state.app, ["diff"]); // Scripted review input validates plumbing, not an independent model's discovery of the lesson. await save(path.join(state.app, ".noxroot/local/fixture-review.json"), { + schemaVersion: 2, + taskId: state.firstTask, + changeId: finished.record.changeIdentity.changeId, decision: "approved", summary: "Scripted acceptance review: exact millisecond delays and invalid-attempt regression passed.", diff --git a/tests/agent-review.test.ts b/tests/agent-review.test.ts index 7387291..f4d3904 100644 --- a/tests/agent-review.test.ts +++ b/tests/agent-review.test.ts @@ -8,6 +8,9 @@ import type { ProcessEvidence } from "../src/model.js"; function response(decision: ReviewerResponse["decision"]): ReviewerResponse { return { + schemaVersion: 2, + taskId: "task-fixture", + changeId: "a".repeat(64), decision, summary: `${decision} from deterministic fixture`, findings: @@ -66,6 +69,22 @@ describe("strict reviewer protocol", () => { expect(parseReviewerResponse(output)).toBeUndefined(); }); + it("rejects a valid decision bound to another task or change", () => { + const approved = response("approved"); + expect( + parseReviewerResponse(JSON.stringify(approved), { + taskId: "another-task", + changeId: approved.changeId, + }), + ).toBeUndefined(); + expect( + parseReviewerResponse(JSON.stringify(approved), { + taskId: approved.taskId, + changeId: "b".repeat(64), + }), + ).toBeUndefined(); + }); + it("parses only stdout and blocks invalid output even when stderr says approved", async () => { const adapter = new CommandAgentAdapter( "fixture", diff --git a/tests/autonomy-guided.test.ts b/tests/autonomy-guided.test.ts index d51840c..dbf5531 100644 --- a/tests/autonomy-guided.test.ts +++ b/tests/autonomy-guided.test.ts @@ -4,7 +4,11 @@ import { CommanderError } from "commander"; import { afterEach, describe, expect, it } from "vitest"; import { effectiveAutonomy } from "../src/orchestration/autonomy.js"; import { finishGuidedRun, startGuidedRun } from "../src/orchestration/guided.js"; -import { ManualAgentAdapter } from "../src/adapters/agents.js"; +import { + ManualAgentAdapter, + type AgentAdapter, + type AgentRequest, +} from "../src/adapters/agents.js"; import { runProcess } from "../src/adapters/process.js"; import { temporaryDirectory } from "./helpers.js"; import { createProgram } from "../src/cli.js"; @@ -179,7 +183,12 @@ commands: const pending = await cli(["finish", "--json", "--root", root]); const pendingValue = JSON.parse(pending.stdout) as { - record: { status: string; calls: unknown[] }; + record: { + id: string; + status: string; + calls: unknown[]; + changeIdentity: { changeId: string }; + }; completion: { documentation: { status: string }; learning: { status: string } }; }; expect(pendingValue.record.status).toBe("completed"); @@ -191,6 +200,9 @@ commands: await writeFile( reviewPath, JSON.stringify({ + schemaVersion: 2, + taskId: pendingValue.record.id, + changeId: pendingValue.record.changeIdentity.changeId, decision: "approved", summary: "The actual diff and affected check passed.", findings: [], @@ -557,10 +569,46 @@ agents: {default: manual, adapters: {manual: {type: manual}}} expect(pending.reviewerPackage).toBeUndefined(); expect(pending.verification.at(-1)?.[0]?.status).toBe("passed"); - const reviewPath = "review.json"; + const reviewPath = ".noxroot/local/review.json"; + await mkdir(path.join(root, ".noxroot", "local"), { recursive: true }); + await expect( + finishGuidedRun({ + root, + record: pending, + adapter: new ManualAgentAdapter(), + reviewAuthorized: false, + reviewFile: "src/value.ts", + }), + ).rejects.toThrow("untracked JSON file under .noxroot/local"); + await writeFile( + path.join(root, reviewPath), + JSON.stringify({ + schemaVersion: 2, + taskId: pending.id, + changeId: "0".repeat(64), + decision: "approved", + summary: "This approval belongs to a different change.", + findings: [], + learningCandidates: [], + }), + ); + const mismatched = await finishGuidedRun({ + root, + record: pending, + adapter: new ManualAgentAdapter(), + reviewAuthorized: false, + reviewFile: reviewPath, + }); + expect(mismatched.status).toBe("blocked"); + expect(mismatched.calls.at(-1)?.result.output).toBe(""); + expect(mismatched.calls.at(-1)?.result.diagnostics).toContain("not retained"); + await writeFile( path.join(root, reviewPath), JSON.stringify({ + schemaVersion: 2, + taskId: pending.id, + changeId: pending.changeIdentity!.changeId, decision: "approved", summary: "The bounded change and check evidence are acceptable.", findings: [], @@ -616,6 +664,108 @@ agents: {default: manual, adapters: {manual: {type: manual}}} expect(JSON.stringify(finished.reviewerPackage)).toContain("