From 1f70c20b1486309dacdec2c28dd50bb87ccd90eb Mon Sep 17 00:00:00 2001 From: Alan-TheGentleman Date: Sat, 1 Aug 2026 16:05:25 +0200 Subject: [PATCH 1/3] feat(review): derive exact capture slots as a pure module providerReviewerProjection already validated every review.capture-result binding field but the code lived inline in extensions/gentle-ai.ts. Extract it into lib/review-result-capture.ts as deriveCaptureSlots, the single reusable validator Wave 1's FINALIZE capture phase will call, plus assertLensSlotBijection for the lens/slot fail-closed check. --- lib/review-result-capture.ts | 147 +++++++++++++ tests/review-result-capture.test.ts | 328 ++++++++++++++++++++++++++++ 2 files changed, 475 insertions(+) create mode 100644 lib/review-result-capture.ts create mode 100644 tests/review-result-capture.test.ts diff --git a/lib/review-result-capture.ts b/lib/review-result-capture.ts new file mode 100644 index 000000000..abb46ed2c --- /dev/null +++ b/lib/review-result-capture.ts @@ -0,0 +1,147 @@ +// Pure exact-slot capture derivation (Wave 1, #2028 host behavior, Design +// Decision 1). This is the single validator for the provider-issued +// `review.capture-result` binding fields: lineage, target, authority +// revision, repository context (base/candidate tree), changed-path manifest +// hash, lens, order, and subject hash. `providerReviewerProjection` +// (extensions/gentle-ai.ts) and the FINALIZE capture phase both consume this +// module instead of re-validating the binding themselves, so exactly one +// validator exists. +import { isCanonicalProcessString } from "./native-review-cli.ts"; +import { CandidateViewError } from "./review-candidate-view.ts"; +import type { ReviewArtifactSubjectV2, ReviewCollectInputV3, ReviewStatusV3 } from "./review-integration-v2.ts"; + +export const CAPTURE_SLOT_ARGUMENT = { + LINEAGE: "lineage", + EXPECTED_REVISION: "expected-revision", + TARGET: "target", + LENS: "lens", + ORDER: "order", + SUBJECT_HASH: "subject-hash", +} as const; +export type CaptureSlotArgument = (typeof CAPTURE_SLOT_ARGUMENT)[keyof typeof CAPTURE_SLOT_ARGUMENT]; + +export interface CaptureSlot { + readonly slotKey: string; + readonly lineageId: string; + readonly authorityRevision: string; + readonly targetIdentity: string; + readonly lens: ReviewArtifactSubjectV2["lens"]; + readonly selectedOrder: number; + readonly subjectHash: string; + // Provider-issued, forwarded byte-identical. Never recomposed. + readonly argumentTokens: readonly string[]; +} + +export interface CaptureSlotDerivation { + readonly slots: readonly CaptureSlot[]; + // The first `review.capture-result` collect input, already validated + // identical to every other input's frozen manifest binding. Callers that + // need the changed-path manifest itself (rather than the six-field slot + // binding) read it from here instead of re-deriving or re-validating it. + readonly first: ReviewCollectInputV3; +} + +const MANIFEST_INPUT_DIVERGENCE = "manifest-input-divergence"; + +export function captureSlotKey(lineageId: string, authorityRevision: string, targetIdentity: string, lens: string, selectedOrder: number, subjectHash: string): string { + return [lineageId, authorityRevision, targetIdentity, lens, String(selectedOrder), subjectHash].join("|"); +} + +function captureArgumentValue(name: CaptureSlotArgument, input: ReviewCollectInputV3): string { + const matches = input.arguments.filter((argument) => argument.name === name); + if (matches.length !== 1 || !isCanonicalProcessString(matches[0]?.value)) { + throw new CandidateViewError(`provider reviewer collect input requires exactly one canonical ${name} argument`, MANIFEST_INPUT_DIVERGENCE); + } + return matches[0]!.value; +} + +function captureArgumentTokens(input: ReviewCollectInputV3): readonly string[] { + return input.arguments.map((argument) => { + if (typeof argument.token !== "string" || argument.token.length === 0) { + throw new CandidateViewError("provider reviewer collect input argument token must be a non-empty string", MANIFEST_INPUT_DIVERGENCE); + } + return argument.token; + }); +} + +/** + * Derives one `CaptureSlot` per `review.capture-result` collect input offered + * by fresh STATUS, validating that every input agrees on the frozen + * candidate (lineage, authority revision, target, base/candidate tree, + * changed-path manifest hash) and that lens/order/subject-hash are pairwise + * distinct. Pure, no I/O — throws `CandidateViewError` with reason + * `manifest-input-divergence` on any disagreement, exactly as the pre-Wave-1 + * `providerReviewerProjection` did before this module existed. + */ +export function deriveCaptureSlots(status: ReviewStatusV3): CaptureSlotDerivation { + const authority = status.authority; + const inputs = status.nextTransition?.kind === "collect" + ? status.nextTransition.collect?.inputs.filter((input) => input.captureOperation === "review.capture-result") ?? [] + : []; + if (authority === undefined || inputs.length === 0) { + throw new CandidateViewError("provider status did not supply reviewer collect inputs for the frozen candidate", MANIFEST_INPUT_DIVERGENCE); + } + const first = inputs[0]!; + if (first.artifactSubject === undefined || first.baseTree === undefined || first.candidateTree === undefined || first.changedPathManifest === undefined) { + throw new CandidateViewError("provider reviewer collect input omitted its frozen artifact subject or manifest", MANIFEST_INPUT_DIVERGENCE); + } + const manifestBytes = JSON.stringify(first.changedPathManifest); + const manifestHash = first.artifactSubject.changedPathManifestSha256; + const seenLenses = new Set(); + const seenOrders = new Set(); + const seenSubjects = new Set(); + const slots: CaptureSlot[] = []; + for (const input of inputs) { + const subject = input.artifactSubject; + if ( + subject === undefined || input.baseTree === undefined || input.candidateTree === undefined || input.changedPathManifest === undefined || + input.baseTree !== status.projection.baseTree || input.candidateTree !== status.projection.currentCandidateTree || + subject.baseTree !== input.baseTree || subject.candidateTree !== input.candidateTree || + subject.lineageId !== authority.lineageId || subject.authorityRevision !== authority.revision || subject.targetIdentity !== status.targetIdentity || + subject.changedPathManifestSha256 !== manifestHash || JSON.stringify(input.changedPathManifest) !== manifestBytes || + captureArgumentValue(CAPTURE_SLOT_ARGUMENT.LINEAGE, input) !== subject.lineageId || + captureArgumentValue(CAPTURE_SLOT_ARGUMENT.EXPECTED_REVISION, input) !== subject.authorityRevision || + captureArgumentValue(CAPTURE_SLOT_ARGUMENT.TARGET, input) !== subject.targetIdentity || + captureArgumentValue(CAPTURE_SLOT_ARGUMENT.LENS, input) !== subject.lens || + captureArgumentValue(CAPTURE_SLOT_ARGUMENT.ORDER, input) !== String(subject.selectedOrder) || + captureArgumentValue(CAPTURE_SLOT_ARGUMENT.SUBJECT_HASH, input) !== subject.subjectHash || + seenLenses.has(subject.lens) || seenOrders.has(subject.selectedOrder) || seenSubjects.has(subject.subjectHash) + ) { + throw new CandidateViewError("provider reviewer collect inputs disagree on their frozen manifest binding", MANIFEST_INPUT_DIVERGENCE); + } + seenLenses.add(subject.lens); + seenOrders.add(subject.selectedOrder); + seenSubjects.add(subject.subjectHash); + slots.push(Object.freeze({ + slotKey: captureSlotKey(subject.lineageId, subject.authorityRevision, subject.targetIdentity, subject.lens, subject.selectedOrder, subject.subjectHash), + lineageId: subject.lineageId, + authorityRevision: subject.authorityRevision, + targetIdentity: subject.targetIdentity, + lens: subject.lens, + selectedOrder: subject.selectedOrder, + subjectHash: subject.subjectHash, + argumentTokens: captureArgumentTokens(input), + })); + } + return Object.freeze({ slots: Object.freeze(slots), first }); +} + +/** + * Fails closed before any capture runs unless every entry in `lensLabels` + * (one per `review_result.lens_results[]` entry) maps to exactly one slot in + * `slots` (outstanding STATUS-offered slots) — a missing lens, an extra + * lens, or a duplicate all reject. + */ +export function assertLensSlotBijection(lensLabels: readonly string[], slots: readonly CaptureSlot[]): void { + const slotLenses = new Set(slots.map((slot) => slot.lens as string)); + const seen = new Set(); + for (const lens of lensLabels) { + if (!slotLenses.has(lens) || seen.has(lens)) { + throw new CandidateViewError(`review result lens ${lens} does not match exactly one outstanding capture slot`, "capture-lens-bijection-violation"); + } + seen.add(lens); + } + if (seen.size !== slotLenses.size) { + throw new CandidateViewError("review result is missing a capture slot for a selected lens", "capture-lens-bijection-violation"); + } +} diff --git a/tests/review-result-capture.test.ts b/tests/review-result-capture.test.ts new file mode 100644 index 000000000..fb74223de --- /dev/null +++ b/tests/review-result-capture.test.ts @@ -0,0 +1,328 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { CandidateViewError } from "../lib/review-candidate-view.ts"; +import type { ReviewArtifactSubjectV2, ReviewCollectInputV3, ReviewStatusV3, ReviewTransitionArgumentV3 } from "../lib/review-integration-v2.ts"; +import { CAPTURE_SLOT_ARGUMENT, type CaptureSlot, assertLensSlotBijection, deriveCaptureSlots } from "../lib/review-result-capture.ts"; + +const LINEAGE = "capture-lineage"; +const REVISION = `sha256:${"1".repeat(64)}`; +const TARGET = `sha256:${"2".repeat(64)}`; +const BASE_TREE = "b".repeat(40); +const CANDIDATE_TREE = "c".repeat(40); +const MANIFEST = Object.freeze([ + { path: "app.ts", status: "M" as const, oldMode: "100644", newMode: "100644", deleted: false, typeChanged: false, modeOnly: false, intendedUntracked: false }, +]); +const MANIFEST_SHA = `sha256:${"3".repeat(64)}`; + +interface SubjectOverrides { + lineageId?: string; + authorityRevision?: string; + targetIdentity?: string; + baseTree?: string; + candidateTree?: string; + changedPathManifestSha256?: string; + lens?: ReviewArtifactSubjectV2["lens"]; + selectedOrder?: number; + subjectHash?: string; +} + +function subject(overrides: SubjectOverrides = {}): ReviewArtifactSubjectV2 { + return { + schema: "gentle-ai.review-artifact-subject/v2", + subjectHash: overrides.subjectHash ?? `sha256:${"4".repeat(64)}`, + lineageId: overrides.lineageId ?? LINEAGE, + authorityRevision: overrides.authorityRevision ?? REVISION, + targetIdentity: overrides.targetIdentity ?? TARGET, + baseTree: overrides.baseTree ?? BASE_TREE, + candidateTree: overrides.candidateTree ?? CANDIDATE_TREE, + changedPathManifestSha256: overrides.changedPathManifestSha256 ?? MANIFEST_SHA, + lens: overrides.lens ?? "review-reliability", + selectedOrder: overrides.selectedOrder ?? 0, + }; +} + +function argumentsFor(bound: ReviewArtifactSubjectV2, argumentOverrides: Partial> = {}): readonly ReviewTransitionArgumentV3[] { + const values: Record = { + lineage: bound.lineageId, + "expected-revision": bound.authorityRevision, + target: bound.targetIdentity, + lens: bound.lens, + order: String(bound.selectedOrder), + "subject-hash": bound.subjectHash, + ...argumentOverrides, + }; + return Object.entries(values).map(([name, value]) => ({ name, value, token: `--${name}=${value}` })); +} + +function collectInput(overrides: { + subjectOverrides?: SubjectOverrides; + argumentOverrides?: Partial>; + omitArtifactSubject?: boolean; + omitBaseTree?: boolean; + omitCandidateTree?: boolean; + omitManifest?: boolean; + tokenOverrides?: Partial>; +} = {}): ReviewCollectInputV3 { + const bound = subject(overrides.subjectOverrides); + const args = argumentsFor(bound, overrides.argumentOverrides).map((argument) => + overrides.tokenOverrides && argument.name in overrides.tokenOverrides + ? { ...argument, token: overrides.tokenOverrides[argument.name] } + : argument, + ); + return { + name: "reviewer_result", + schema: "https://gentle-ai.dev/schema/review/reviewer/v1", + captureOperation: "review.capture-result", + arguments: args, + ...(overrides.omitArtifactSubject ? {} : { artifactSubject: bound }), + ...(overrides.omitBaseTree ? {} : { baseTree: bound.baseTree }), + ...(overrides.omitCandidateTree ? {} : { candidateTree: bound.candidateTree }), + ...(overrides.omitManifest ? {} : { changedPathManifest: MANIFEST }), + }; +} + +function status(inputs: readonly ReviewCollectInputV3[]): ReviewStatusV3 { + return { + contract: "gentle-ai.review-integration/v2", + applicability: "current_target", + authority: { version: "compact-v2", lineageId: LINEAGE, state: "reviewing", generation: 1, revision: REVISION }, + receipt: { status: "expected_missing" }, + action: "finalize", + replayability: "not_replayable", + targetIdentity: TARGET, + projection: { + schema: "gentle-ai.review-integration.projection/v1", + kind: "current-changes", + projection: "workspace", + baseTree: BASE_TREE, + initialReviewTree: CANDIDATE_TREE, + currentCandidateTree: CANDIDATE_TREE, + pathsDigest: "digest", + paths: ["app.ts"], + intendedUntracked: [], + intendedUntrackedProof: "proof", + initialSnapshotIdentity: "identity", + currentSnapshotIdentity: "identity", + }, + repair: { + schema: "gentle-ai.review-authority-repair-assessment/v1", + status: "unsupported", + counts: { lineages: 0, compactLineages: 0, legacyLineages: 0, events: 0, bytes: 0, eligibleCandidates: 0, unsupportedLineages: 0, conflicts: 0 }, + supportedOperations: ["review/complete-fix", "review/validate-fix"], + authorizationSchema: "gentle-ai.review-repair-authorization/v1", + }, + candidates: [], + nextTransition: inputs.length === 0 ? undefined : { kind: "collect", reasonCode: "reviewer_results_required", collect: { inputs } }, + raw: {}, + }; +} + +test("CAPTURE_SLOT_ARGUMENT names the six provider binding fields", () => { + assert.deepEqual(CAPTURE_SLOT_ARGUMENT, { + LINEAGE: "lineage", + EXPECTED_REVISION: "expected-revision", + TARGET: "target", + LENS: "lens", + ORDER: "order", + SUBJECT_HASH: "subject-hash", + }); +}); + +test("deriveCaptureSlots derives one slot per collect input with a canonical slotKey", () => { + const derivation = deriveCaptureSlots(status([collectInput()])); + assert.equal(derivation.slots.length, 1); + const slot = derivation.slots[0]!; + assert.equal(slot.lineageId, LINEAGE); + assert.equal(slot.authorityRevision, REVISION); + assert.equal(slot.targetIdentity, TARGET); + assert.equal(slot.lens, "review-reliability"); + assert.equal(slot.selectedOrder, 0); + assert.equal(slot.subjectHash, `sha256:${"4".repeat(64)}`); + assert.equal(slot.slotKey, [LINEAGE, REVISION, TARGET, "review-reliability", "0", `sha256:${"4".repeat(64)}`].join("|")); + assert.deepEqual(slot.argumentTokens, [ + `--lineage=${LINEAGE}`, + `--expected-revision=${REVISION}`, + `--target=${TARGET}`, + "--lens=review-reliability", + "--order=0", + `--subject-hash=sha256:${"4".repeat(64)}`, + ]); +}); + +test("deriveCaptureSlots derives distinct slots for multiple lenses with distinct selectedOrder", () => { + const first = collectInput({ subjectOverrides: { lens: "review-risk", selectedOrder: 0, subjectHash: `sha256:${"5".repeat(64)}` } }); + const second = collectInput({ subjectOverrides: { lens: "review-reliability", selectedOrder: 1, subjectHash: `sha256:${"6".repeat(64)}` } }); + const derivation = deriveCaptureSlots(status([first, second])); + assert.equal(derivation.slots.length, 2); + assert.deepEqual(derivation.slots.map((slot) => slot.lens), ["review-risk", "review-reliability"]); + assert.deepEqual(derivation.slots.map((slot) => slot.selectedOrder), [0, 1]); +}); + +test("deriveCaptureSlots rejects when no review.capture-result collect input is offered", () => { + assert.throws( + () => deriveCaptureSlots(status([])), + (error: unknown) => error instanceof CandidateViewError && error.reason === "manifest-input-divergence", + ); +}); + +test("deriveCaptureSlots rejects a collect input missing its artifact subject or manifest", () => { + for (const overrides of [{ omitArtifactSubject: true }, { omitBaseTree: true }, { omitCandidateTree: true }, { omitManifest: true }]) { + assert.throws( + () => deriveCaptureSlots(status([collectInput(overrides)])), + (error: unknown) => error instanceof CandidateViewError && error.reason === "manifest-input-divergence", + `expected rejection for ${JSON.stringify(overrides)}`, + ); + } +}); + +test("deriveCaptureSlots rejects each of lineage/target/revision/repository-context mismatching independently against the frozen authority and projection", () => { + // Lens, order, and subject-hash have no authoritative comparison target of + // their own for a single collect input beyond the argument-echo check + // (covered separately below) and pairwise uniqueness (covered by the + // duplicate-lens/order/subjectHash tests) — they are two of the six + // argument fields, not fields with an independent frozen-authority source. + const mismatches: SubjectOverrides[] = [ + { lineageId: "other-lineage" }, + { targetIdentity: `sha256:${"9".repeat(64)}` }, + { authorityRevision: `sha256:${"8".repeat(64)}` }, + { baseTree: "d".repeat(40) }, + { candidateTree: "e".repeat(40) }, + ]; + for (const subjectOverrides of mismatches) { + // The input's own top-level baseTree/candidateTree mirror the status + // projection, but the artifact subject inside it disagrees — this is + // exactly the repository-context / identity divergence the six-field + // validator must reject before any slot is trusted. + const bound = subject(subjectOverrides); + const drifted: ReviewCollectInputV3 = { + name: "reviewer_result", + schema: "https://gentle-ai.dev/schema/review/reviewer/v1", + captureOperation: "review.capture-result", + arguments: argumentsFor(bound), + artifactSubject: bound, + baseTree: BASE_TREE, + candidateTree: CANDIDATE_TREE, + changedPathManifest: MANIFEST, + }; + assert.throws( + () => deriveCaptureSlots(status([drifted])), + (error: unknown) => error instanceof CandidateViewError && error.reason === "manifest-input-divergence", + `expected rejection for ${JSON.stringify(subjectOverrides)}`, + ); + } +}); + +test("deriveCaptureSlots rejects a second collect input whose manifest hash disagrees with the first", () => { + const first = collectInput({ subjectOverrides: { lens: "review-risk", selectedOrder: 0, subjectHash: `sha256:${"f".repeat(64)}` } }); + const driftedSubject = subject({ lens: "review-reliability", selectedOrder: 1, subjectHash: `sha256:${"0".repeat(64)}`, changedPathManifestSha256: `sha256:${"1".repeat(64)}` }); + const second: ReviewCollectInputV3 = { + name: "reviewer_result", + schema: "https://gentle-ai.dev/schema/review/reviewer/v1", + captureOperation: "review.capture-result", + arguments: argumentsFor(driftedSubject), + artifactSubject: driftedSubject, + baseTree: BASE_TREE, + candidateTree: CANDIDATE_TREE, + changedPathManifest: MANIFEST, + }; + assert.throws( + () => deriveCaptureSlots(status([first, second])), + (error: unknown) => error instanceof CandidateViewError && error.reason === "manifest-input-divergence", + ); +}); + +test("deriveCaptureSlots rejects an argument value that disagrees with the artifact subject", () => { + assert.throws( + () => deriveCaptureSlots(status([collectInput({ argumentOverrides: { lens: "review-risk" } })])), + (error: unknown) => error instanceof CandidateViewError && error.reason === "manifest-input-divergence", + ); +}); + +test("deriveCaptureSlots rejects an argument token that is missing, empty, or non-string", () => { + for (const tokenOverrides of [{ lineage: undefined }, { lineage: "" }]) { + assert.throws( + () => deriveCaptureSlots(status([collectInput({ tokenOverrides })])), + (error: unknown) => error instanceof CandidateViewError, + `expected rejection for ${JSON.stringify(tokenOverrides)}`, + ); + } +}); + +test("deriveCaptureSlots rejects a duplicate lens across collect inputs", () => { + const first = collectInput({ subjectOverrides: { selectedOrder: 0, subjectHash: `sha256:${"a".repeat(64)}` } }); + const second = collectInput({ subjectOverrides: { selectedOrder: 1, subjectHash: `sha256:${"b".repeat(64)}` } }); + assert.throws( + () => deriveCaptureSlots(status([first, second])), + (error: unknown) => error instanceof CandidateViewError && error.reason === "manifest-input-divergence", + ); +}); + +test("deriveCaptureSlots rejects a duplicate selectedOrder across collect inputs", () => { + const first = collectInput({ subjectOverrides: { lens: "review-risk", subjectHash: `sha256:${"c".repeat(64)}` } }); + const second = collectInput({ subjectOverrides: { lens: "review-reliability", subjectHash: `sha256:${"d".repeat(64)}` } }); + assert.throws( + () => deriveCaptureSlots(status([first, second])), + (error: unknown) => error instanceof CandidateViewError && error.reason === "manifest-input-divergence", + ); +}); + +test("deriveCaptureSlots rejects a duplicate subjectHash across collect inputs", () => { + const shared = `sha256:${"e".repeat(64)}`; + const first = collectInput({ subjectOverrides: { lens: "review-risk", selectedOrder: 0, subjectHash: shared } }); + const second = collectInput({ subjectOverrides: { lens: "review-reliability", selectedOrder: 1, subjectHash: shared } }); + assert.throws( + () => deriveCaptureSlots(status([first, second])), + (error: unknown) => error instanceof CandidateViewError && error.reason === "manifest-input-divergence", + ); +}); + +function slotFixture(overrides: Partial = {}): CaptureSlot { + return { + slotKey: "slot", + lineageId: LINEAGE, + authorityRevision: REVISION, + targetIdentity: TARGET, + lens: "review-reliability", + selectedOrder: 0, + subjectHash: `sha256:${"4".repeat(64)}`, + argumentTokens: ["--lens=review-reliability"], + ...overrides, + }; +} + +test("assertLensSlotBijection accepts a lens_results set matching outstanding slots exactly once each", () => { + const slots = [slotFixture({ lens: "review-risk", slotKey: "risk" }), slotFixture({ lens: "review-reliability", slotKey: "reliability" })]; + assert.doesNotThrow(() => assertLensSlotBijection(["review-risk", "review-reliability"], slots)); +}); + +test("assertLensSlotBijection rejects a lens missing from outstanding slots", () => { + const slots = [slotFixture({ lens: "review-reliability" })]; + assert.throws( + () => assertLensSlotBijection(["review-risk"], slots), + (error: unknown) => error instanceof CandidateViewError && error.reason === "capture-lens-bijection-violation", + ); +}); + +test("assertLensSlotBijection rejects an extra lens_results entry beyond outstanding slots", () => { + const slots = [slotFixture({ lens: "review-risk" })]; + assert.throws( + () => assertLensSlotBijection(["review-risk", "review-reliability"], slots), + (error: unknown) => error instanceof CandidateViewError && error.reason === "capture-lens-bijection-violation", + ); +}); + +test("assertLensSlotBijection rejects a duplicate lens in lens_results", () => { + const slots = [slotFixture({ lens: "review-risk", slotKey: "a" }), slotFixture({ lens: "review-reliability", slotKey: "b" })]; + assert.throws( + () => assertLensSlotBijection(["review-risk", "review-risk"], slots), + (error: unknown) => error instanceof CandidateViewError && error.reason === "capture-lens-bijection-violation", + ); +}); + +test("assertLensSlotBijection rejects a slot count larger than lens_results (an outstanding slot with no submitted lens)", () => { + const slots = [slotFixture({ lens: "review-risk", slotKey: "a" }), slotFixture({ lens: "review-reliability", slotKey: "b" })]; + assert.throws( + () => assertLensSlotBijection(["review-risk"], slots), + (error: unknown) => error instanceof CandidateViewError && error.reason === "capture-lens-bijection-violation", + ); +}); From ad3441f85ee1da1414806447472e3ddb6911426a Mon Sep 17 00:00:00 2001 From: Alan-TheGentleman Date: Sat, 1 Aug 2026 16:05:40 +0200 Subject: [PATCH 2/3] feat(review): capture reviewer results at FINALIZE and drop dead result staging Pi ran review lenses but never captured their documents through native admission: providerReviewerProjection validated the review.capture-result binding and then threw the work away, and NativeReviewCliV216.finalize() staged lensResults into a tmp file no argv ever consumed. Route providerReviewerProjection through deriveCaptureSlots so exactly one validator exists, and add a FINALIZE capture phase that, for every review_result.lens_results entry: fails closed on any lens/slot mismatch before capturing anything, calls captureResult() with the candidate root as cwd and the provider's own argument tokens forwarded verbatim in ascending selectedOrder, and re-checks each admitted manifest against its slot's subject hash and lens. Transport is selected per run: every admitted manifest carrying a path selects resultArtifactFiles in ascending order, any manifest carrying a reference selects capturedResults instead, and the retired --result flag is never emitted. Also removes the dead lensResults staging from NativeReviewCliV216.finalize (the negotiated production client) and regenerates its runtime mirror. The legacy plain-CLI NativeReviewCliV214 keeps its own working --result staging untouched, since Wave 1 only targets the negotiated FINALIZE transport. --- extensions/gentle-ai.ts | 117 ++++---- lib/native-review-cli.ts | 16 +- runtime/native-review-cli.mjs | 16 +- .../review-controller-native-recovery.test.ts | 82 ++++++ .../review-controller-native-routing.test.ts | 255 +++++++++++++++--- .../review-controller-workspace-root.test.ts | 64 ++++- 6 files changed, 458 insertions(+), 92 deletions(-) diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index 197a43ada..ba402e5c3 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -116,7 +116,9 @@ import { NATIVE_REVIEW_MODE_SOURCE, NATIVE_REVIEW_RECONCILE_ANOMALIES, sanitizeForeignNativeReviewDiagnostics, + type NativeReviewAdmittedResultManifest, type NativeReviewCli, + type NativeReviewFinalizeCapturedResults, type NativeFinalizeResult, type NativeReviewModeOperation, type NativeReviewModeSource, @@ -126,6 +128,7 @@ import { } from "../lib/native-review-cli.ts"; import type { ReviewConsentV2, ReviewStatusV3 } from "../lib/review-integration-v2.ts"; import { assertDistinctCorrectionEvidence, resolveCorrectionStep, type CorrectionEvidence, type CorrectionOutcome, type CorrectionStep } from "../lib/review-correction-lifecycle.ts"; +import { assertLensSlotBijection, deriveCaptureSlots } from "../lib/review-result-capture.ts"; const GRAPH_V1_ORDINARY_READ_ONLY = "Graph-v1 ordinary review authority is read-only; use native compact-v2 review operations"; import { @@ -4884,60 +4887,24 @@ function resolveReviewControllerWorkspaceRoot(requested: string | undefined, ses return resolved; } -function providerArgumentValue(name: string, input: NonNullable["collect"]>["inputs"][number]): string { - const matches = input.arguments.filter((argument) => argument.name === name); - if (matches.length !== 1 || !isCanonicalProcessString(matches[0]?.value)) { - throw new CandidateViewError(`provider reviewer collect input requires exactly one canonical ${name} argument`, "manifest-input-divergence"); - } - return matches[0]!.value; -} - +// The six-field capture-slot binding (lineage, target, authority revision, +// repository context, subject hash, lens, order) is validated exactly once, +// by `deriveCaptureSlots` (lib/review-result-capture.ts, Design Decision 1). +// This projection only reads the already-validated `first` collect input to +// build the candidate manifest descriptor; it performs no binding comparison +// of its own beyond the intended-untracked subset check below, which is +// specific to `NativeCandidateProjectionDescriptor` construction and not part +// of the capture-slot binding. function providerReviewerProjection(status: ReviewStatusV3): NativeCandidateProjectionDescriptor { - const authority = status.authority; - const inputs = status.nextTransition?.kind === "collect" - ? status.nextTransition.collect?.inputs.filter((input) => input.captureOperation === "review.capture-result") ?? [] - : []; - if (authority === undefined || inputs.length === 0) { - throw new CandidateViewError("provider status did not supply reviewer collect inputs for the frozen candidate", "manifest-input-divergence"); - } - const first = inputs[0]!; - if (first.artifactSubject === undefined || first.baseTree === undefined || first.candidateTree === undefined || first.changedPathManifest === undefined) { - throw new CandidateViewError("provider reviewer collect input omitted its frozen artifact subject or manifest", "manifest-input-divergence"); - } - const manifestBytes = JSON.stringify(first.changedPathManifest); - const manifestHash = first.artifactSubject.changedPathManifestSha256; - const seenLenses = new Set(); - const seenOrders = new Set(); - const seenSubjects = new Set(); - for (const input of inputs) { - const subject = input.artifactSubject; - if ( - subject === undefined || input.baseTree === undefined || input.candidateTree === undefined || input.changedPathManifest === undefined || - input.baseTree !== status.projection.baseTree || input.candidateTree !== status.projection.currentCandidateTree || - subject.baseTree !== input.baseTree || subject.candidateTree !== input.candidateTree || - subject.lineageId !== authority.lineageId || subject.authorityRevision !== authority.revision || subject.targetIdentity !== status.targetIdentity || - subject.changedPathManifestSha256 !== manifestHash || JSON.stringify(input.changedPathManifest) !== manifestBytes || - providerArgumentValue("lineage", input) !== subject.lineageId || - providerArgumentValue("expected-revision", input) !== subject.authorityRevision || - providerArgumentValue("target", input) !== subject.targetIdentity || - providerArgumentValue("lens", input) !== subject.lens || - providerArgumentValue("order", input) !== String(subject.selectedOrder) || - providerArgumentValue("subject-hash", input) !== subject.subjectHash || - seenLenses.has(subject.lens) || seenOrders.has(subject.selectedOrder) || seenSubjects.has(subject.subjectHash) - ) { - throw new CandidateViewError("provider reviewer collect inputs disagree on their frozen manifest binding", "manifest-input-divergence"); - } - seenLenses.add(subject.lens); - seenOrders.add(subject.selectedOrder); - seenSubjects.add(subject.subjectHash); - } - const intendedUntracked = first.changedPathManifest.filter((entry) => entry.intendedUntracked).map((entry) => entry.path).sort(); + const { first } = deriveCaptureSlots(status); + const manifest = first.changedPathManifest!; + const intendedUntracked = manifest.filter((entry) => entry.intendedUntracked).map((entry) => entry.path).sort(); if (JSON.stringify(intendedUntracked) !== JSON.stringify([...status.projection.intendedUntracked].sort())) { throw new CandidateViewError("provider manifest intended-untracked fields disagree with its projection", "manifest-intended-untracked-not-subset"); } return { ...status.projection, - manifest: first.changedPathManifest.map((entry) => ({ + manifest: manifest.map((entry) => ({ path: entry.path, status: entry.status, oldMode: entry.oldMode, @@ -4946,11 +4913,58 @@ function providerReviewerProjection(status: ReviewStatusV3): NativeCandidateProj typeChanged: entry.typeChanged, modeOnly: entry.modeOnly, })), - manifestSha256: manifestHash, + manifestSha256: first.artifactSubject!.changedPathManifestSha256, providerManifestHashVerified: true, }; } +/** + * FINALIZE capture phase (Wave 1, #2028 host behavior, Design Decisions 1 + * and 4). Pi has no `tool.execute.after` analogue, so capture is + * status-mediated at FINALIZE entry instead of host-hook-time: each + * `review_result.lens_results[]` entry is bound to exactly one outstanding + * `review.capture-result` slot (fail-closed bijection, before any capture + * runs), captured in ascending `selectedOrder` with the provider's own + * argument tokens forwarded byte-identical, then re-checked against the + * admitted manifest's subject hash and lens. Transport is selected per run: + * every admitted manifest carrying `path` selects `resultArtifactFiles` in + * ascending order; any manifest carrying `reference` selects + * `capturedResults: true`. The retired `--result` transport is never used. + */ +async function captureReviewerResults( + nativeReviewCli: NativeReviewCli, + status: ReviewStatusV3, + lensResults: NonNullable["review_result"]>["lens_results"], + cwd: string, + signal: AbortSignal | undefined, +): Promise { + if (nativeReviewCli.captureResult === undefined) { + throw new CandidateViewError("native reviewer result capture is unavailable", "capture-unavailable"); + } + const { slots } = deriveCaptureSlots(status); + const lensLabels = lensResults.map((document, index) => document.lens ?? `lens-${index}`); + assertLensSlotBijection(lensLabels, slots); + const orderedSlots = [...slots].sort((left, right) => left.selectedOrder - right.selectedOrder); + const manifests: NativeReviewAdmittedResultManifest[] = []; + for (const slot of orderedSlots) { + const index = lensLabels.indexOf(slot.lens); + const document = toNativeReviewerDocument(lensResults[index]!); + const manifest = await nativeReviewCli.captureResult({ + argumentTokens: slot.argumentTokens, + resultDocument: JSON.stringify(document), + cwd, + ...(signal === undefined ? {} : { signal }), + }); + if (manifest.subjectHash !== slot.subjectHash || (manifest.lens !== undefined && manifest.lens !== slot.lens)) { + throw new CandidateViewError("admitted capture manifest does not match the requested capture slot", "capture-manifest-binding-drift"); + } + manifests.push(manifest); + } + return manifests.every((manifest) => manifest.path !== undefined) + ? { resultArtifactFiles: manifests.map((manifest) => manifest.path!) } + : { capturedResults: true }; +} + function correctionOutcome(input: ReturnType): CorrectionOutcome | undefined { if (input.final_verification_outcome !== undefined) return input.final_verification_outcome; if (input.final_verification_passed === undefined) return undefined; @@ -5538,10 +5552,13 @@ async function executeReviewControllerOperation( throw new Error("Native FINALIZE refuter material is invalid without inferential candidate-caused severe findings"); } } + const capturedTransport: NativeReviewFinalizeCapturedResults | undefined = input.review_result === undefined + ? undefined + : await captureReviewerResults(nativeReviewCli, negotiatedStatus!, input.review_result.lens_results, candidateView?.root ?? defaultCwd, signal); nativeResult = await nativeReviewCli.finalize({ cwd: candidateView?.root ?? defaultCwd, ...(parameters.lineageId === undefined ? {} : { lineageId: parameters.lineageId }), - ...(input.review_result === undefined ? {} : { lensResults: input.review_result.lens_results.map((document, index) => ({ lens: document.lens ?? `lens-${index}`, document: toNativeReviewerDocument(document) })) }), + ...(capturedTransport ?? {}), ...(input.refuter_batch === undefined ? {} : { refuterDocument: toNativeRefuterDocument(input.refuter_batch) }), ...(input.correction_line_forecast === undefined ? {} : { correctionLines: input.correction_line_forecast }), ...(input.validation === undefined && input.validation_proof === undefined ? {} : { validationDocument: toNativeValidatorDocument(input.validation ?? input.validation_proof!) }), diff --git a/lib/native-review-cli.ts b/lib/native-review-cli.ts index 1a9b8e7e0..3dca30886 100644 --- a/lib/native-review-cli.ts +++ b/lib/native-review-cli.ts @@ -135,6 +135,10 @@ export interface NativeReviewCli { // evidence-first correction lifecycle's collection step. repair?(request: NativeReviewRepairRequest): Promise; captureEvidence?(request: NativeReviewCaptureEvidenceRequest): Promise; + // `review capture-result`: the FINALIZE capture phase's collection step + // (Wave 1, #2028 host behavior, Design Decision 1). Optional, exactly like + // captureEvidence above, since older negotiated clients may not implement it. + captureResult?(request: NativeReviewCaptureResultRequest): Promise; // Dark until a negotiated version reports the `mode` capability true // (Design Decision #7, organic-rdd-parity). Plain versioned CLI operation, // outside the negotiated review-integration protocol — same shape as @@ -2148,11 +2152,19 @@ export class NativeReviewCliV216 implements NativeReviewCli { async finalize(request: NativeFinalizeRequest): Promise { if (request.evidenceDocument !== undefined && request.evidenceDocument.length === 0) throw new TypeError("Native FINALIZE evidence must contain at least one byte"); - const needsStaging = request.lensResults !== undefined || request.refuterDocument !== undefined || request.validationDocument !== undefined || request.evidenceDocument !== undefined; + // `lensResults`/`resultFiles` are accepted only by the legacy plain-CLI + // client (NativeReviewCliV214) and its retired `--result` argv. The + // negotiated V216 client never had a caller for them (Wave 1, #2028 host + // behavior): reviewer results reach authority exclusively through + // `review capture-result`, and FINALIZE either discovers them + // (`--captured-results`) or is handed each admitted manifest explicitly + // (`--result-artifact-file`). Staging `lensResults` into a tmp document + // here would write candidate-derived content to disk for an argv that + // never consumes it, so it is intentionally ignored. + const needsStaging = request.refuterDocument !== undefined || request.validationDocument !== undefined || request.evidenceDocument !== undefined; const directory = needsStaging ? await mkdtemp(join(tmpdir(), "gentle-ai-finalize-")) : undefined; try { if (directory !== undefined) await chmod(directory, 0o700); - const resultFiles = directory !== undefined && request.lensResults !== undefined ? await Promise.all(request.lensResults.map((entry, index) => this.stageDocument(directory, `result-${index}`, entry.document))) : request.resultFiles ?? []; const refuterFile = directory !== undefined && request.refuterDocument !== undefined ? await this.stageDocument(directory, "refuter", request.refuterDocument) : request.refuterFile; const validationFile = directory !== undefined && request.validationDocument !== undefined ? await this.stageDocument(directory, "validation", request.validationDocument) : request.validationFile; const evidenceFile = directory !== undefined && request.evidenceDocument !== undefined ? await this.stageEvidence(directory, request.evidenceDocument) : request.evidenceFile; diff --git a/runtime/native-review-cli.mjs b/runtime/native-review-cli.mjs index c612d75ff..139c2b85c 100644 --- a/runtime/native-review-cli.mjs +++ b/runtime/native-review-cli.mjs @@ -133,6 +133,10 @@ export const NATIVE_SDD_ARTIFACT_STATE = { + + + + @@ -2149,11 +2153,19 @@ export class NativeReviewCliV216 { async finalize(request ) { if (request.evidenceDocument !== undefined && request.evidenceDocument.length === 0) throw new TypeError("Native FINALIZE evidence must contain at least one byte"); - const needsStaging = request.lensResults !== undefined || request.refuterDocument !== undefined || request.validationDocument !== undefined || request.evidenceDocument !== undefined; + // `lensResults`/`resultFiles` are accepted only by the legacy plain-CLI + // client (NativeReviewCliV214) and its retired `--result` argv. The + // negotiated V216 client never had a caller for them (Wave 1, #2028 host + // behavior): reviewer results reach authority exclusively through + // `review capture-result`, and FINALIZE either discovers them + // (`--captured-results`) or is handed each admitted manifest explicitly + // (`--result-artifact-file`). Staging `lensResults` into a tmp document + // here would write candidate-derived content to disk for an argv that + // never consumes it, so it is intentionally ignored. + const needsStaging = request.refuterDocument !== undefined || request.validationDocument !== undefined || request.evidenceDocument !== undefined; const directory = needsStaging ? await mkdtemp(join(tmpdir(), "gentle-ai-finalize-")) : undefined; try { if (directory !== undefined) await chmod(directory, 0o700); - const resultFiles = directory !== undefined && request.lensResults !== undefined ? await Promise.all(request.lensResults.map((entry, index) => this.stageDocument(directory, `result-${index}`, entry.document))) : request.resultFiles ?? []; const refuterFile = directory !== undefined && request.refuterDocument !== undefined ? await this.stageDocument(directory, "refuter", request.refuterDocument) : request.refuterFile; const validationFile = directory !== undefined && request.validationDocument !== undefined ? await this.stageDocument(directory, "validation", request.validationDocument) : request.validationFile; const evidenceFile = directory !== undefined && request.evidenceDocument !== undefined ? await this.stageEvidence(directory, request.evidenceDocument) : request.evidenceFile; diff --git a/tests/review-controller-native-recovery.test.ts b/tests/review-controller-native-recovery.test.ts index e046a63a2..c2549f934 100644 --- a/tests/review-controller-native-recovery.test.ts +++ b/tests/review-controller-native-recovery.test.ts @@ -884,6 +884,58 @@ test("captureResult passes the provider tokens through verbatim and carries no - assert.equal(argv.length, 2 + tokens.length + 2); }); +test("captureResult forwards a documentation-like admitted path byte-identical and refuses a dual-locator manifest (Wave 1, threat: Documentation-like paths)", async (t) => { + t.after(() => clearNativeReviewCapabilitiesCacheForTesting()); + const tokens = [ + "--lineage=review-1d5aadacc600e167", + "--expected-revision=sha256:" + "c".repeat(64), + "--target=sha256:" + "d".repeat(64), + "--repository-context=rctx1_" + "e".repeat(64), + "--lens=review-reliability", + "--order=0", + "--subject-hash=sha256:" + "a".repeat(64), + ]; + const resultDocument = JSON.stringify({ subject_hash: "sha256:" + "a".repeat(64), inspection: { status: "completed", paths: ["a.ts"] }, findings: [], evidence: ["reviewed the complete frozen candidate scope"] }); + + // A locator containing "..", an absolute path, or an executable-looking + // name is opaque provider data returned to the same provider that issued + // it — Pi never opens, stats, classifies, or executes it, and forwards it + // byte-identical. None of these locators exist on disk in this test. + for (const dangerousPath of ["../../../../etc/passwd", "/etc/shadow", "/usr/bin/env sh -c 'rm -rf /'"]) { + const manifest = { + schema: "gentle-ai.review-result-artifact/v2", + capability: "review.native_result_artifact", + subject_hash: "sha256:" + "a".repeat(64), + admission_decision: "completed", + lens: "review-reliability", + path: dangerousPath, + }; + const { adapter } = queuedAdapter([{ stdout: JSON.stringify(manifest) }]); + const cli = new NativeReviewCliV216(adapter, "/package/.gentle-ai/gentle-ai", 30_000, 1024 * 1024, async () => undefined, () => "dcc846103b16d365eaeeb9d7f289c23fc4f2897f23def1cb3fe7f05557b64705"); + const captured = await cli.captureResult({ argumentTokens: tokens, resultDocument }); + assert.equal(captured.path, dangerousPath, "the locator must be forwarded byte-identical, unexamined"); + assert.equal(captured.reference, undefined); + } + + // A manifest carrying both a `path` and a `reference` is refused: Pi + // cannot forward two locators to FINALIZE for one slot. + const dualLocator = { + schema: "gentle-ai.review-result-artifact/v2", + capability: "review.native_result_artifact", + subject_hash: "sha256:" + "a".repeat(64), + admission_decision: "completed", + lens: "review-reliability", + path: "../escape.json", + reference: "rref1_" + "b".repeat(64), + }; + const { adapter: dualAdapter } = queuedAdapter([{ stdout: JSON.stringify(dualLocator) }]); + const dualCli = new NativeReviewCliV216(dualAdapter, "/package/.gentle-ai/gentle-ai", 30_000, 1024 * 1024, async () => undefined, () => "dcc846103b16d365eaeeb9d7f289c23fc4f2897f23def1cb3fe7f05557b64705"); + await assert.rejects( + dualCli.captureResult({ argumentTokens: tokens, resultDocument }), + (error: unknown) => error instanceof NativeReviewCliError && error.code === NATIVE_REVIEW_ERROR_CODE.SCHEMA_INCOMPATIBLE, + ); +}); + test("captureEvidence stages exact bytes, uses the closed outcome argv, and decodes the native record", async (t) => { t.after(() => clearNativeReviewCapabilitiesCacheForTesting()); const capabilities = v2Fixture>("capabilities.fixture.json"); @@ -959,3 +1011,33 @@ test("negotiated finalize never emits the retired --result flag", async (t) => { assert.ok(argv.some((token) => token === "--captured-results" || token.startsWith("--captured-results=")), "finalize must tell the provider to discover the captured results"); assert.ok(argv.includes("--contract"), "finalize IS negotiated, unlike capture-result"); }); + +test("negotiated finalize no longer stages lensResults into a reviewer-document tmp file (Wave 1, dead-staging removal)", async (t) => { + t.after(() => clearNativeReviewCapabilitiesCacheForTesting()); + const capabilities = v2Fixture>("capabilities.fixture.json"); + const capabilitiesBody = { ...capabilities, package: { ...(capabilities.package as Record), version: GENTLE_AI_VERSION } }; + const finalizeBody = { + schema: "gentle-ai.review-integration.operation/v2", + contract: "gentle-ai.review-integration/v2", + operation: "review.finalize", + result: { operation: "review/finalize", lineage_id: "review-1d5aadacc600e167", state: "approved", action: "validate delivery", store_revision: "sha256:" + "f".repeat(64) }, + }; + const { adapter, calls } = queuedAdapter([ + { stdout: JSON.stringify(capabilitiesBody) }, + { stdout: JSON.stringify(finalizeBody) }, + ]); + let cleanupInvocations = 0; + const cli = new NativeReviewCliV216(adapter, "/package/.gentle-ai/gentle-ai", 30_000, 1024 * 1024, async () => { cleanupInvocations += 1; }, () => "dcc846103b16d365eaeeb9d7f289c23fc4f2897f23def1cb3fe7f05557b64705"); + + // `lensResults` is accepted only for the legacy plain-CLI client + // (NativeReviewCliV214); the negotiated V216 client must not stage it into + // a tmp document, must not create a staging directory for it at all + // (proven here by the cleanup hook never firing), and must not reference + // it in argv — it is dead input here. + await cli.finalize({ cwd: "/repo", lineageId: "review-1d5aadacc600e167", lensResults: [{ lens: "review-risk", document: { id: "risk" } }], capturedResults: true }); + + assert.equal(cleanupInvocations, 0, "lensResults alone must not create any staging directory to clean up"); + const argv = calls[1]!.arguments; + assert.equal(argv.includes("--result"), false); + assert.equal(argv.some((token) => token.startsWith("--result=")), false); +}); diff --git a/tests/review-controller-native-routing.test.ts b/tests/review-controller-native-routing.test.ts index 20bd61e47..3cbdd8429 100644 --- a/tests/review-controller-native-routing.test.ts +++ b/tests/review-controller-native-routing.test.ts @@ -23,7 +23,7 @@ import { CandidateViewRegistry, deriveChangedPathManifest } from "../lib/review- import { inspectLegacyReviewAuthorityV1 } from "../lib/review-legacy-detector.ts"; import { resolveRepositoryAuthorityV1 } from "../lib/review-repository.ts"; import { NATIVE_REVIEW_REMEDIATION, classifyNativeReviewRemediation } from "../lib/native-review-remediation.ts"; -import type { AuthorityRepairAssessmentV1, ReviewStatusV3 } from "../lib/review-integration-v2.ts"; +import type { AuthorityRepairAssessmentV1, ReviewArtifactSubjectV2, ReviewStatusV3 } from "../lib/review-integration-v2.ts"; interface RegisteredTool { execute: ( @@ -279,6 +279,7 @@ function fakeNative(overrides: Partial = {}): NativeReviewCli { ? candidateStartTargetStatus(request) : candidateFinalizeTargetStatus(request, lineageId); }, + captureResult: defaultCaptureResult, ...overrides, }; } @@ -427,28 +428,40 @@ function candidateFinalizeTargetStatus(request: Parameters ({ ...entry, status: entry.status as "A" | "D" | "M" | "T", intendedUntracked: status.projection.intendedUntracked.includes(entry.path), })); - const subject = { - schema: "gentle-ai.review-artifact-subject/v2" as const, - subjectHash: `sha256:${"8".repeat(64)}`, - lineageId: status.authority!.lineageId, - authorityRevision: status.authority!.revision, - targetIdentity: status.targetIdentity, - baseTree: status.projection.baseTree, - candidateTree: status.projection.currentCandidateTree, - changedPathManifestSha256: manifestHash, - lens: "review-reliability" as const, - selectedOrder: 0, - }; - status.nextTransition = { - kind: "collect", - reasonCode: "reviewer_results_required", - collect: { inputs: [{ + const inputs = slots.map((slot, index) => { + const subject = { + schema: "gentle-ai.review-artifact-subject/v2" as const, + subjectHash: slot.subjectHash ?? `sha256:${"89abcdef01234567"[index % 16]!.repeat(64)}`, + lineageId: status.authority!.lineageId, + authorityRevision: status.authority!.revision, + targetIdentity: status.targetIdentity, + baseTree: status.projection.baseTree, + candidateTree: status.projection.currentCandidateTree, + changedPathManifestSha256: manifestHash, + lens: slot.lens ?? "review-reliability", + selectedOrder: slot.selectedOrder ?? index, + }; + return { name: "reviewer_result", schema: "https://gentle-ai.dev/schema/review/reviewer/v1", captureOperation: "review.capture-result", @@ -457,18 +470,46 @@ function bindReviewerManifest(status: ReviewStatusV3, cwd: string, manifestHash { name: "expected-revision", value: subject.authorityRevision, token: `--expected-revision=${subject.authorityRevision}` }, { name: "target", value: subject.targetIdentity, token: `--target=${subject.targetIdentity}` }, { name: "lens", value: subject.lens, token: `--lens=${subject.lens}` }, - { name: "order", value: "0", token: "--order=0" }, + { name: "order", value: String(subject.selectedOrder), token: `--order=${subject.selectedOrder}` }, { name: "subject-hash", value: subject.subjectHash, token: `--subject-hash=${subject.subjectHash}` }, ], artifactSubject: subject, baseTree: subject.baseTree, candidateTree: subject.candidateTree, changedPathManifest: manifest, - }] }, - }; + }; + }); + status.nextTransition = { kind: "collect", reasonCode: "reviewer_results_required", collect: { inputs } }; return status; } +function bindReviewerManifest(status: ReviewStatusV3, cwd: string, manifestHash = `sha256:${"7".repeat(64)}`): ReviewStatusV3 { + return bindReviewerManifests(status, cwd, [{ lens: "review-reliability", selectedOrder: 0, subjectHash: `sha256:${"8".repeat(64)}` }], manifestHash); +} + +/** + * Decodes the provider-issued `--name=value` argument tokens `captureResult` + * receives verbatim, and returns a matching admitted manifest. This mirrors + * exactly what a real `gentle-ai review capture-result` invocation would + * admit for the fixtures `bindReviewerManifests` produces above. + */ +function defaultCaptureResult(request: Parameters>[0]): ReturnType> extends Promise ? Promise : never { + const decoded = new Map(request.argumentTokens.map((token) => { + const withoutPrefix = token.replace(/^--/, ""); + const separator = withoutPrefix.indexOf("="); + return [withoutPrefix.slice(0, separator), withoutPrefix.slice(separator + 1)] as const; + })); + const lens = decoded.get("lens")!; + const subjectHash = decoded.get("subject-hash")!; + return Promise.resolve({ + schema: "gentle-ai.review-result-artifact/v2", + subjectHash, + admissionDecision: "completed", + lens, + path: `/opaque/capture/${lens}`, + }); +} + function bindCorrectionCollection(status: ReviewStatusV3): ReviewStatusV3 { status.nextTransition = { kind: "collect", @@ -566,8 +607,7 @@ function assertNoPublicDestructiveResetMaterial(value: unknown): void { } test("new ordinary START and native-lineage FINALIZE use exactly one native call and stable envelopes", async (t) => { - const cwd = mkdtempSync(join(tmpdir(), "gentle-pi-native-controller-")); - t.after(() => rmSync(cwd, { recursive: true, force: true })); + const cwd = repository(t); let starts = 0; let finalizes = 0; const { controller } = runtime(fakeNative({ @@ -575,6 +615,9 @@ test("new ordinary START and native-lineage FINALIZE use exactly one native call starts += 1; return { lineageId: "native-lineage", state: "reviewing", riskLevel: "medium", selectedLenses: ["review-reliability"], changedFiles: 2, changedLines: 7, correctionBudget: 4, action: "created", lensesRequired: true }; }, + targetStatus: async (request) => request.lineageId === undefined + ? candidateStartTargetStatus(request) + : bindReviewerManifests(candidateFinalizeTargetStatus(request, "native-lineage"), request.cwd), finalize: async () => { finalizes += 1; return { lineageId: "native-lineage", state: "approved", action: "approved", storeRevision: "r1", receiptPath: "/opaque/receipt" }; @@ -582,7 +625,7 @@ test("new ordinary START and native-lineage FINALIZE use exactly one native call })); const start = await controller.execute("start", { operation: "start", input: JSON.stringify({ mode: "ordinary" }) }, undefined, undefined, context(cwd)); assert.deepEqual(start.details, { operation: "start", result: { lineage_id: "native-lineage", state: "reviewing", risk_tier: "medium", selected_lenses: ["review-reliability"], changed_files: 2, original_changed_lines: 7, correction_budget: 4, action: "created", lenses_required: true }, workspace_root: cwd }); - const finalize = await controller.execute("finalize", { operation: "finalize", lineageId: "native-lineage", input: JSON.stringify({ review_result: { lens_results: [{ findings: [], evidence: ["complete candidate reviewed"] }] } }) }, undefined, undefined, context(cwd)); + const finalize = await controller.execute("finalize", { operation: "finalize", lineageId: "native-lineage", input: JSON.stringify({ review_result: { lens_results: [{ lens: "review-reliability", findings: [], evidence: ["complete candidate reviewed"] }] } }) }, undefined, undefined, context(cwd)); assert.deepEqual(finalize.details, { operation: "finalize", result: { lineage_id: "native-lineage", state: "approved", action: "approved", store_revision: "r1", receipt_path: "/opaque/receipt" } }); assert.equal(starts, 1); assert.equal(finalizes, 1); @@ -600,12 +643,12 @@ test("native FINALIZE resolves STATUS and mutation against the verified frozen c statusRoots.push(request.cwd); if (request.cwd === cwd) return targetStatusFixture({ applicability: "unrelated", action: "start" }); const candidate = candidateViews.resolveForFinalize(request.lineageId); - return targetStatusFixture({ + return bindReviewerManifests(targetStatusFixture({ lineageId: request.lineageId, baseTree: candidate.baseTree, currentCandidateTree: candidate.candidateTree, paths: candidate.paths, - }); + }), request.cwd); }, start: async () => ({ lineageId: "candidate-root-finalize", @@ -629,7 +672,7 @@ test("native FINALIZE resolves STATUS and mutation against the verified frozen c const result = await controller.execute("candidate-root-finalize", { operation: "finalize", lineageId: "candidate-root-finalize", - input: JSON.stringify({ review_result: { lens_results: [{ findings: [], evidence: ["candidate reviewed"] }] } }), + input: JSON.stringify({ review_result: { lens_results: [{ lens: "review-reliability", findings: [], evidence: ["candidate reviewed"] }] } }), }, undefined, undefined, context(cwd)); assert.deepEqual(statusRoots, [candidateRoot]); assert.equal(finalizeRoot, candidateRoot); @@ -1393,6 +1436,9 @@ test("native FINALIZE derives and requires the trusted refuter request before in let finalizes = 0; const { controller } = runtime(fakeNative({ start: async () => ({ lineageId: "native-lineage", state: "reviewing", riskLevel: "medium", selectedLenses: ["review-risk"], changedFiles: 1, changedLines: 1, correctionBudget: 1, action: "created", lensesRequired: true }), + targetStatus: async (request) => request.lineageId === undefined + ? candidateStartTargetStatus(request) + : bindReviewerManifests(candidateFinalizeTargetStatus(request, "native-lineage"), request.cwd, [{ lens: "review-risk", selectedOrder: 0 }]), finalize: async () => { finalizes += 1; return { lineageId: "native-lineage", state: "validating", action: "continue", storeRevision: "r1" }; @@ -1421,6 +1467,9 @@ test("native FINALIZE emits exact v2.1.4 process documents and failed verificati const requests: Parameters[0][] = []; const { controller } = runtime(fakeNative({ start: async () => ({ lineageId: "native-lineage", state: "reviewing", riskLevel: "medium", selectedLenses: ["review-risk"], changedFiles: 1, changedLines: 1, correctionBudget: 1, action: "created", lensesRequired: true }), + targetStatus: async (request) => request.lineageId === undefined + ? candidateStartTargetStatus(request) + : bindReviewerManifests(candidateFinalizeTargetStatus(request, "native-lineage"), request.cwd, [{ lens: "review-risk", selectedOrder: 0 }]), finalize: async (request) => { requests.push(request); return { lineageId: "native-lineage", state: "approved", action: "approved", storeRevision: "r1" }; @@ -1442,7 +1491,9 @@ test("native FINALIZE emits exact v2.1.4 process documents and failed verificati assert.deepEqual(requests, [{ cwd, lineageId: "native-lineage", - lensResults: [{ lens: "review-risk", document: { lens: "risk", findings: [{ ...finding, lens: "risk" }], evidence: ["complete candidate reviewed"] } }], + // Reviewer results now reach FINALIZE only as admitted capture + // manifests (Wave 1, #2028 host behavior) — never as `lensResults`. + resultArtifactFiles: ["/opaque/capture/review-risk"], refuterDocument: { results: refuterBatch.results }, validationDocument: { original_criteria: { passed: false, evidence: ["acceptance still fails"] }, correction_regression: { passed: true, evidence: ["regression suite passes"] }, follow_ups: [{ observation: "Track the remaining failure", proof_refs: ["differential-test:candidate still fails"] }] }, evidenceDocument: " focused verification failed\n\n", @@ -1450,6 +1501,120 @@ test("native FINALIZE emits exact v2.1.4 process documents and failed verificati }]); }); +test("FINALIZE capture phase fails closed on a missing, extra, or duplicate lens before any capture runs (Wave 1, lens/slot bijection)", async (t) => { + const cwd = repository(t); + let captures = 0; + let finalizes = 0; + const { controller } = runtime(fakeNative({ + start: async () => ({ lineageId: "native-lineage", state: "reviewing", riskLevel: "medium", selectedLenses: ["review-reliability"], changedFiles: 1, changedLines: 1, correctionBudget: 1, action: "created", lensesRequired: true }), + targetStatus: async (request) => request.lineageId === undefined + ? candidateStartTargetStatus(request) + : bindReviewerManifests(candidateFinalizeTargetStatus(request, "native-lineage"), request.cwd, [{ lens: "review-reliability", selectedOrder: 0 }]), + captureResult: async (request) => { captures += 1; return defaultCaptureResult(request); }, + finalize: async () => { finalizes += 1; return { lineageId: "native-lineage", state: "approved", action: "approved", storeRevision: "r1" }; }, + })); + await controller.execute("bijection-start", { operation: "start", input: JSON.stringify({ mode: "ordinary" }) }, undefined, undefined, context(cwd)); + for (const lensResults of [ + [{ lens: "review-risk", findings: [], evidence: ["reviewed"] }], // extra/mismatched lens, no slot offered for it + [{ lens: "review-reliability", findings: [], evidence: ["a"] }, { lens: "review-reliability", findings: [], evidence: ["b"] }], // duplicate lens + [], // missing lens: one outstanding slot, zero submitted results + ]) { + const result = await controller.execute("bijection-attempt", { operation: "finalize", lineageId: "native-lineage", input: JSON.stringify({ review_result: { lens_results: lensResults } }) }, undefined, undefined, context(cwd)); + const details = result.details as { outcome?: string }; + assert.equal(details.outcome, "native-operation-failed", JSON.stringify(details)); + } + assert.equal(captures, 0, "no capture may run before the lens/slot bijection is proven"); + assert.equal(finalizes, 0); +}); + +test("FINALIZE capture phase binds cwd to the candidate root and forwards provider tokens verbatim in ascending order (Wave 1, threat: Git repository selection)", async (t) => { + const cwd = repository(t); + writeFileSync(join(cwd, "app.ts"), "export const value = 2;\n"); + const candidateViews = new CandidateViewRegistry(); + const captureRequests: Parameters>[0][] = []; + const { controller } = runtime(fakeNative({ + start: async () => ({ lineageId: "capture-cwd-lineage", state: "reviewing", riskLevel: "high", selectedLenses: ["review-risk", "review-reliability"], changedFiles: 1, changedLines: 1, correctionBudget: 1, action: "created", lensesRequired: true }), + targetStatus: async (request) => { + if (request.lineageId === undefined) return candidateStartTargetStatus(request); + const candidate = candidateViews.resolveForFinalize(request.lineageId); + return bindReviewerManifests(targetStatusFixture({ + lineageId: request.lineageId, + baseTree: candidate.baseTree, + currentCandidateTree: candidate.candidateTree, + paths: candidate.paths, + }), request.cwd, [ + { lens: "review-reliability", selectedOrder: 1 }, + { lens: "review-risk", selectedOrder: 0 }, + ]); + }, + captureResult: async (request) => { captureRequests.push(request); return defaultCaptureResult(request); }, + finalize: async () => ({ lineageId: "capture-cwd-lineage", state: "approved", action: "approved", storeRevision: "r1" }), + }), undefined, undefined, undefined, candidateViews); + await controller.execute("capture-cwd-start", { operation: "start", input: JSON.stringify({ mode: "ordinary" }) }, undefined, undefined, context(cwd)); + const candidateRoot = candidateViews.resolveForFinalize("capture-cwd-lineage").root; + // Submitted in descending lens order; capture must still run ascending by + // the provider's own selectedOrder (review-risk=0 before review-reliability=1). + await controller.execute("capture-cwd-finalize", { + operation: "finalize", + lineageId: "capture-cwd-lineage", + input: JSON.stringify({ review_result: { lens_results: [ + { lens: "review-reliability", findings: [], evidence: ["reviewed"] }, + { lens: "review-risk", findings: [], evidence: ["reviewed"] }, + ] } }), + }, undefined, undefined, context(cwd)); + assert.equal(captureRequests.length, 2); + assert.deepEqual(captureRequests.map((request) => request.cwd), [candidateRoot, candidateRoot]); + for (const request of captureRequests) { + assert.ok(request.argumentTokens.every((token) => typeof token === "string" && token.length > 0)); + } + assert.deepEqual(captureRequests.map((request) => request.argumentTokens.find((token) => token.startsWith("--lens="))), ["--lens=review-risk", "--lens=review-reliability"]); +}); + +test("FINALIZE selects resultArtifactFiles when every admitted manifest carries a path, ascending by selectedOrder, and capturedResults when any carries a reference (Wave 1, transport selection)", async (t) => { + const cwd = repository(t); + writeFileSync(join(cwd, "app.ts"), "export const value = 2;\n"); + const pathTransportRequests: Parameters[0][] = []; + const { controller: pathController } = runtime(fakeNative({ + start: async () => ({ lineageId: "transport-path-lineage", state: "reviewing", riskLevel: "high", selectedLenses: ["review-risk", "review-reliability"], changedFiles: 1, changedLines: 1, correctionBudget: 1, action: "created", lensesRequired: true }), + targetStatus: async (request) => request.lineageId === undefined + ? candidateStartTargetStatus(request) + : bindReviewerManifests(candidateFinalizeTargetStatus(request, "transport-path-lineage"), request.cwd, [ + { lens: "review-reliability", selectedOrder: 1 }, + { lens: "review-risk", selectedOrder: 0 }, + ]), + finalize: async (request) => { pathTransportRequests.push(request); return { lineageId: "transport-path-lineage", state: "approved", action: "approved", storeRevision: "r1" }; }, + })); + await pathController.execute("transport-path-start", { operation: "start", input: JSON.stringify({ mode: "ordinary" }) }, undefined, undefined, context(cwd)); + await pathController.execute("transport-path-finalize", { + operation: "finalize", + lineageId: "transport-path-lineage", + input: JSON.stringify({ review_result: { lens_results: [ + { lens: "review-reliability", findings: [], evidence: ["reviewed"] }, + { lens: "review-risk", findings: [], evidence: ["reviewed"] }, + ] } }), + }, undefined, undefined, context(cwd)); + assert.deepEqual(pathTransportRequests[0]?.resultArtifactFiles, ["/opaque/capture/review-risk", "/opaque/capture/review-reliability"]); + assert.equal(pathTransportRequests[0]?.capturedResults, undefined); + + const referenceTransportRequests: Parameters[0][] = []; + const { controller: referenceController } = runtime(fakeNative({ + start: async () => ({ lineageId: "transport-reference-lineage", state: "reviewing", riskLevel: "medium", selectedLenses: ["review-reliability"], changedFiles: 1, changedLines: 1, correctionBudget: 1, action: "created", lensesRequired: true }), + targetStatus: async (request) => request.lineageId === undefined + ? candidateStartTargetStatus(request) + : bindReviewerManifests(candidateFinalizeTargetStatus(request, "transport-reference-lineage"), request.cwd, [{ lens: "review-reliability", selectedOrder: 0 }]), + captureResult: async () => ({ schema: "gentle-ai.review-result-artifact/v2", subjectHash: `sha256:${"8".repeat(64)}`, admissionDecision: "completed", lens: "review-reliability", reference: `rref1_${"c".repeat(64)}` }), + finalize: async (request) => { referenceTransportRequests.push(request); return { lineageId: "transport-reference-lineage", state: "approved", action: "approved", storeRevision: "r1" }; }, + })); + await referenceController.execute("transport-reference-start", { operation: "start", input: JSON.stringify({ mode: "ordinary" }) }, undefined, undefined, context(cwd)); + await referenceController.execute("transport-reference-finalize", { + operation: "finalize", + lineageId: "transport-reference-lineage", + input: JSON.stringify({ review_result: { lens_results: [{ lens: "review-reliability", findings: [], evidence: ["reviewed"] }] } }), + }, undefined, undefined, context(cwd)); + assert.equal(referenceTransportRequests[0]?.capturedResults, true); + assert.equal(referenceTransportRequests[0]?.resultArtifactFiles, undefined); +}); + test("native FINALIZE rejects unpublished reviewer enums and empty arrays before native calls", async (t) => { const cwd = repository(t); let finalizes = 0; @@ -1530,14 +1695,24 @@ test("controller rejects zero-length final evidence before native calls", async test("repeated native FINALIZE keeps initial lenses one-shot", async (t) => { const cwd = repository(t); const requests: Parameters[0][] = []; - const { controller } = runtime(fakeNative({ finalize: async (request) => { - requests.push(request); - return { lineageId: "native-lineage", state: "correction_required", action: "continue correction", storeRevision: `r${requests.length}` }; - } })); - await controller.execute("initial", { operation: "finalize", lineageId: "native-lineage", input: JSON.stringify({ review_result: { lens_results: [{ findings: [], evidence: ["complete candidate reviewed"] }] } }) }, undefined, undefined, context(cwd)); + const { controller } = runtime(fakeNative({ + targetStatus: async (request) => request.lineageId === undefined + ? candidateStartTargetStatus(request) + : bindReviewerManifests(candidateFinalizeTargetStatus(request, "native-lineage"), request.cwd), + finalize: async (request) => { + requests.push(request); + return { lineageId: "native-lineage", state: "correction_required", action: "continue correction", storeRevision: `r${requests.length}` }; + }, + })); + await controller.execute("initial", { operation: "finalize", lineageId: "native-lineage", input: JSON.stringify({ review_result: { lens_results: [{ lens: "review-reliability", findings: [], evidence: ["complete candidate reviewed"] }] } }) }, undefined, undefined, context(cwd)); await controller.execute("retry", { operation: "finalize", lineageId: "native-lineage", input: JSON.stringify({ correction_line_forecast: 1 }) }, undefined, undefined, context(cwd)); - assert.equal(requests[0]?.lensResults?.length, 1); - assert.equal(requests[1]?.lensResults, undefined); + // Reviewer results now reach FINALIZE only through admitted capture + // manifests (Wave 1, #2028 host behavior) — one-shot initial lenses means + // the second (correction) FINALIZE carries no capture transport at all. + assert.deepEqual(requests[0]?.resultArtifactFiles, ["/opaque/capture/review-reliability"]); + assert.equal(requests[0]?.capturedResults, undefined); + assert.equal(requests[1]?.resultArtifactFiles, undefined); + assert.equal(requests[1]?.capturedResults, undefined); }); test("native error has no compact fallback and ambiguous mutation demands target status", async (t) => { @@ -2734,6 +2909,9 @@ test("native pre-commit rejects an unproven staged projection before native auth const candidateViews = new CandidateViewRegistry(); let validations = 0; const { controller } = runtime(fakeNative({ + targetStatus: async (request) => request.lineageId === undefined + ? candidateStartTargetStatus(request) + : bindReviewerManifests(candidateFinalizeTargetStatus(request, request.lineageId), request.cwd), validate: async () => { validations += 1; return { allowed: true, result: "allow", action: "continue", reason: "native allow must not bypass Pi projection checks", gateContext: nativeGateContext() }; @@ -2741,7 +2919,7 @@ test("native pre-commit rejects an unproven staged projection before native auth }), undefined, undefined, undefined, candidateViews); const started = await controller.execute("start", { operation: "start", input: JSON.stringify({ mode: "ordinary" }) }, undefined, undefined, context(cwd)); const lineageId = (started.details as { result: { lineage_id: string } }).result.lineage_id; - await controller.execute("finalize", { operation: "finalize", lineageId, input: JSON.stringify({ review_result: { lens_results: [{ findings: [], evidence: ["candidate reviewed"] }] } }) }, undefined, undefined, context(cwd)); + await controller.execute("finalize", { operation: "finalize", lineageId, input: JSON.stringify({ review_result: { lens_results: [{ lens: "review-reliability", findings: [], evidence: ["candidate reviewed"] }] } }) }, undefined, undefined, context(cwd)); writeFileSync(join(cwd, "app.ts"), "export const value = 3;\n"); git(cwd, "add", "--", "app.ts", "initially-untracked.ts"); const result = await controller.execute("validate", { operation: "validate", lineageId, idempotencyKey: "projection-drift", command: "git commit -m native", input: "{}" }, undefined, undefined, context(cwd)); @@ -2756,6 +2934,9 @@ test("native pre-commit binds the exact tracked and initially-untracked projecti const candidateViews = new CandidateViewRegistry(); let validations = 0; const native = fakeNative({ + targetStatus: async (request) => request.lineageId === undefined + ? candidateStartTargetStatus(request) + : bindReviewerManifests(candidateFinalizeTargetStatus(request, request.lineageId), request.cwd), validate: async () => { validations += 1; return { allowed: true, result: "allow", action: "continue", reason: "ok", gateContext: nativeGateContext("native-lineage", "r1", git(cwd, "write-tree")) }; @@ -2764,7 +2945,7 @@ test("native pre-commit binds the exact tracked and initially-untracked projecti const { controller, toolCall } = runtime(native, undefined, undefined, undefined, candidateViews); const started = await controller.execute("start", { operation: "start", input: JSON.stringify({ mode: "ordinary" }) }, undefined, undefined, context(cwd)); const lineageId = (started.details as { result: { lineage_id: string } }).result.lineage_id; - await controller.execute("finalize", { operation: "finalize", lineageId, input: JSON.stringify({ review_result: { lens_results: [{ findings: [], evidence: ["candidate reviewed"] }] } }) }, undefined, undefined, context(cwd)); + await controller.execute("finalize", { operation: "finalize", lineageId, input: JSON.stringify({ review_result: { lens_results: [{ lens: "review-reliability", findings: [], evidence: ["candidate reviewed"] }] } }) }, undefined, undefined, context(cwd)); git(cwd, "add", "--", "app.ts", "initially-untracked.ts"); const command = "git commit -m exact-projection"; const allowed = await controller.execute("validate", { operation: "validate", lineageId, idempotencyKey: "exact-projection", command, input: "{}" }, undefined, undefined, context(cwd)); diff --git a/tests/review-controller-workspace-root.test.ts b/tests/review-controller-workspace-root.test.ts index a33c2a73f..bff03651c 100644 --- a/tests/review-controller-workspace-root.test.ts +++ b/tests/review-controller-workspace-root.test.ts @@ -7,7 +7,7 @@ import test from "node:test"; import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent"; import { createGentleAiExtension } from "../extensions/gentle-ai.ts"; import type { NativeReviewCli } from "../lib/native-review-cli.ts"; -import { CandidateViewRegistry } from "../lib/review-candidate-view.ts"; +import { CandidateViewRegistry, deriveChangedPathManifest } from "../lib/review-candidate-view.ts"; import type { AuthorityRepairAssessmentV1, ReviewStatusV3 } from "../lib/review-integration-v2.ts"; interface RegisteredTool { @@ -86,10 +86,69 @@ function fakeNative(overrides: Partial = {}): NativeReviewCli { targetStatus: async (request) => request.lineageId === undefined ? candidateStartTargetStatus(request) : candidateFinalizeTargetStatus(request, request.lineageId), + captureResult: defaultCaptureResult, ...overrides, }; } +/** + * Binds `status.nextTransition` to one `review.capture-result` collect input + * for lens `review-reliability`, matching the exact-slot binding + * `deriveCaptureSlots` (lib/review-result-capture.ts) validates. + */ +function bindReviewerManifest(status: ReviewStatusV3, cwd: string): ReviewStatusV3 { + const manifest = deriveChangedPathManifest(cwd, status.projection.baseTree, status.projection.currentCandidateTree).map((entry) => ({ + ...entry, + status: entry.status as "A" | "D" | "M" | "T", + intendedUntracked: status.projection.intendedUntracked.includes(entry.path), + })); + const subject = { + schema: "gentle-ai.review-artifact-subject/v2" as const, + subjectHash: `sha256:${"8".repeat(64)}`, + lineageId: status.authority!.lineageId, + authorityRevision: status.authority!.revision, + targetIdentity: status.targetIdentity, + baseTree: status.projection.baseTree, + candidateTree: status.projection.currentCandidateTree, + changedPathManifestSha256: `sha256:${"7".repeat(64)}`, + lens: "review-reliability" as const, + selectedOrder: 0, + }; + status.nextTransition = { + kind: "collect", + reasonCode: "reviewer_results_required", + collect: { inputs: [{ + name: "reviewer_result", + schema: "https://gentle-ai.dev/schema/review/reviewer/v1", + captureOperation: "review.capture-result", + arguments: [ + { name: "lineage", value: subject.lineageId, token: `--lineage=${subject.lineageId}` }, + { name: "expected-revision", value: subject.authorityRevision, token: `--expected-revision=${subject.authorityRevision}` }, + { name: "target", value: subject.targetIdentity, token: `--target=${subject.targetIdentity}` }, + { name: "lens", value: subject.lens, token: `--lens=${subject.lens}` }, + { name: "order", value: "0", token: "--order=0" }, + { name: "subject-hash", value: subject.subjectHash, token: `--subject-hash=${subject.subjectHash}` }, + ], + artifactSubject: subject, + baseTree: subject.baseTree, + candidateTree: subject.candidateTree, + changedPathManifest: manifest, + }] }, + }; + return status; +} + +function defaultCaptureResult(request: Parameters>[0]): ReturnType> extends Promise ? Promise : never { + const decoded = new Map(request.argumentTokens.map((token) => { + const withoutPrefix = token.replace(/^--/, ""); + const separator = withoutPrefix.indexOf("="); + return [withoutPrefix.slice(0, separator), withoutPrefix.slice(separator + 1)] as const; + })); + const lens = decoded.get("lens")!; + const subjectHash = decoded.get("subject-hash")!; + return Promise.resolve({ schema: "gentle-ai.review-result-artifact/v2", subjectHash, admissionDecision: "completed", lens, path: `/opaque/capture/${lens}` }); +} + const UNSUPPORTED_REPAIR_ASSESSMENT: AuthorityRepairAssessmentV1 = { schema: "gentle-ai.review-authority-repair-assessment/v1", status: "unsupported", @@ -360,6 +419,9 @@ test("FINALIZE fails closed when the frozen projection belongs to a different wo let finalizes = 0; const { controller } = runtime(fakeNative({ start: async () => ({ lineageId: "finalize-lineage", state: "reviewing", riskLevel: "medium", selectedLenses: ["review-reliability"], changedFiles: 1, changedLines: 1, correctionBudget: 1, action: "created", lensesRequired: true }), + targetStatus: async (request) => request.lineageId === undefined + ? candidateStartTargetStatus(request) + : bindReviewerManifest(candidateFinalizeTargetStatus(request, request.lineageId), request.cwd), finalize: async () => { finalizes += 1; return { lineageId: "finalize-lineage", state: "approved", action: "approved", storeRevision: "r1" }; From 960df1167676cd73ee7f578c994c505d05dc1a7d Mon Sep 17 00:00:00 2001 From: Alan-TheGentleman Date: Sat, 1 Aug 2026 16:05:44 +0200 Subject: [PATCH 3/3] docs(sdd): mark Wave 1 W1 tasks complete Records RED/GREEN evidence and the two scoped deviations (bijection checks only outstanding slots until W2's record map exists; the dead lensResults staging deletion is scoped to NativeReviewCliV216, not the still-used legacy NativeReviewCliV214 client) for all 14 W1 tasks. --- .../gentle-ai-main-behavior-parity/tasks.md | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/openspec/changes/gentle-ai-main-behavior-parity/tasks.md b/openspec/changes/gentle-ai-main-behavior-parity/tasks.md index 04853aeb6..31be90e7a 100644 --- a/openspec/changes/gentle-ai-main-behavior-parity/tasks.md +++ b/openspec/changes/gentle-ai-main-behavior-parity/tasks.md @@ -51,20 +51,20 @@ branch (feature-branch-chain), never squashed into one. ## W1: Exact-slot capture, admission, transport (fakes only, no foundation dependency) -- [ ] W1.1 RED `tests/review-result-capture.test.ts` (new): `deriveCaptureSlots` — each of lineage/target/authority-revision/repository-context/subject-hash/lens/order mismatching independently rejects; duplicate lens, order, or subject hash rejects; canonical `slotKey` join of the six fields; ascending `selectedOrder`. -- [ ] W1.2 GREEN `lib/review-result-capture.ts` (new): `CaptureSlot`, `CAPTURE_SLOT_ARGUMENT` const, `deriveCaptureSlots(status)` — pure, extracted from `providerReviewerProjection`. -- [ ] W1.3 Refactor `extensions/gentle-ai.ts:4895` (`providerReviewerProjection`): call `deriveCaptureSlots` so exactly one validator exists; keep existing manifest/intended-untracked mapping behavior-preserving. -- [ ] W1.4 RED `tests/review-controller-native-routing.test.ts`: lens↔slot bijection — a missing lens, an extra lens, or a duplicate fails closed before any capture runs. -- [ ] W1.5 GREEN `extensions/gentle-ai.ts`: FINALIZE capture-phase entry — fresh `targetStatus`, `deriveCaptureSlots`, bijection check against outstanding ∪ recorded slots, ascending-order execution loop. -- [ ] W1.6 RED (threat: Git repository selection + Subprocess invocation) `tests/review-controller-native-routing.test.ts` + `tests/native-review-cli.test.ts`: capture cwd equals `candidateView.root`; tokens carrying `--repository-context` plus an explicit cwd rejected; argument tokens stay discrete argv elements verbatim, never composed into a shell string; empty/non-string tokens refused. -- [ ] W1.7 GREEN: wire the capture phase to `lib/native-review-cli.ts` `captureResult()` (`:2287`) with `candidateView.root` and verbatim `argumentTokens`; no shell, fixed executable, existing bounded timeout/buffer in `invoke()`. -- [ ] W1.8 RED (threat: Documentation-like paths) `tests/native-review-cli.test.ts`: an admitted-manifest `path` containing `..`, an absolute path, or an executable-looking name is forwarded byte-identical with zero Pi-side filesystem access; a manifest carrying both `path` and `reference` is refused. -- [ ] W1.9 GREEN: confirm `decodeNativeAdmittedResultManifest`'s exactly-one-locator rule is exercised end-to-end from capture through FINALIZE transport; no filesystem access added. -- [ ] W1.10 RED `tests/review-controller-native-routing.test.ts`: transport selection — every slot's manifest carries `path` ⇒ `resultArtifactFiles[]` in ascending `selectedOrder`; any manifest carries `reference` or any slot is `COMMITTED` ⇒ `capturedResults: true`; retired `--result` is never emitted. -- [ ] W1.11 GREEN `extensions/gentle-ai.ts`: FINALIZE per-run transport selection (Decision 4). -- [ ] W1.12 RED `tests/native-review-cli.test.ts`: `finalize()` no longer stages `lensResults` into `resultFiles`; `NativeFinalizeRequest` drops `resultFiles`/`lensResults`; no reviewer-document tmp file is written that argv never consumes. -- [ ] W1.13 GREEN `lib/native-review-cli.ts` (`:2149-2195`, `:408-422`): delete the dead staging and the two fields; regenerate `runtime/native-review-cli.mjs` via `node scripts/build-git-commit-transaction-runner.mjs --write`; verify with `pnpm run check:transaction-runner`. -- [ ] W1.14 Verify: `pnpm test` — W1 scope green; only the 3 pre-existing RDD-off failures in `tests/native-review-parity-runtime.test.ts` remain, unchanged (expected environment state, not a regression). +- [x] W1.1 RED `tests/review-result-capture.test.ts` (new): `deriveCaptureSlots` — each of lineage/target/authority-revision/repository-context/subject-hash/lens/order mismatching independently rejects; duplicate lens, order, or subject hash rejects; canonical `slotKey` join of the six fields; ascending `selectedOrder`. +- [x] W1.2 GREEN `lib/review-result-capture.ts` (new): `CaptureSlot`, `CAPTURE_SLOT_ARGUMENT` const, `deriveCaptureSlots(status)` — pure, extracted from `providerReviewerProjection`. Also ships `assertLensSlotBijection` (used by W1.4/W1.5) in the same pure module. +- [x] W1.3 Refactor `extensions/gentle-ai.ts:4895` (`providerReviewerProjection`): call `deriveCaptureSlots` so exactly one validator exists; keep existing manifest/intended-untracked mapping behavior-preserving. Verified behavior-preserving by running the full pre-existing `review-controller-native-routing`/`review-controller-native-recovery`/`native-review-cli`/`review-candidate-view` suite (320/320) unchanged before and after the refactor. +- [x] W1.4 RED `tests/review-controller-native-routing.test.ts`: lens↔slot bijection — a missing lens, an extra lens, or a duplicate fails closed before any capture runs. +- [x] W1.5 GREEN `extensions/gentle-ai.ts`: FINALIZE capture-phase entry — fresh `targetStatus` (the FINALIZE handler's existing STATUS call), `deriveCaptureSlots`, bijection check (`assertLensSlotBijection`), ascending-order execution loop. Bijection checks against outstanding slots only in W1; the `∪ recorded slots` half of Decision 1 activates in W2 once the `CaptureSlotRecord` map exists — W1 has no persisted record state to union with. +- [x] W1.6 RED (threat: Git repository selection + Subprocess invocation) `tests/review-controller-native-routing.test.ts` + `tests/review-controller-native-recovery.test.ts`: capture cwd equals `candidateView.root`; argument tokens stay discrete argv elements verbatim, never composed into a shell string, in ascending `selectedOrder`. (`--repository-context`-plus-`--cwd` rejection and empty/non-string token refusal were already covered pre-Wave-1 by `captureResult()`'s own guards — exercised end-to-end by the existing `captureResult passes the provider tokens through verbatim...` test, now reachable from the controller for the first time.) +- [x] W1.7 GREEN: wire the capture phase to `lib/native-review-cli.ts` `captureResult()` (`:2287`) with `candidateView.root` and verbatim `argumentTokens`; no shell, fixed executable, existing bounded timeout/buffer in `invoke()`. Added `captureResult?` to the `NativeReviewCli` interface (previously implemented only on `NativeReviewCliV216`, never declared, so no interface caller could reach it). +- [x] W1.8 RED (threat: Documentation-like paths) `tests/review-controller-native-recovery.test.ts`: an admitted-manifest `path` containing `..`, an absolute path, or an executable-looking name is forwarded byte-identical with zero Pi-side filesystem access; a manifest carrying both `path` and `reference` is refused. +- [x] W1.9 GREEN: confirmed `decodeNativeAdmittedResultManifest`'s exactly-one-locator rule already enforces this (pre-existing code) and is now exercised end-to-end from capture through FINALIZE transport by the W1.10/11 controller tests; no filesystem access added anywhere in the capture phase. +- [x] W1.10 RED `tests/review-controller-native-routing.test.ts`: transport selection — every slot's manifest carries `path` ⇒ `resultArtifactFiles[]` in ascending `selectedOrder`; any manifest carries `reference` ⇒ `capturedResults: true`; retired `--result` is never emitted. (The `COMMITTED`-slot half of this rule is W2 scope — no slot can be `COMMITTED` until W2's record map exists.) +- [x] W1.11 GREEN `extensions/gentle-ai.ts`: FINALIZE per-run transport selection (Decision 4). +- [x] W1.12 RED `tests/review-controller-native-recovery.test.ts`: `NativeReviewCliV216.finalize()` no longer stages `lensResults` into a reviewer-document tmp file; no `--result`/`--result=` argv is ever emitted. **Deviation**: `resultFiles`/`lensResults` stay declared on the shared `NativeFinalizeRequest` type because `NativeReviewCliV214.finalize()` (the legacy plain-CLI client, still directly tested in `tests/native-review-cli.test.ts` for v2.1.3–v2.1.5 fixture compatibility) genuinely consumes them in its own `--result` argv — that staging is NOT dead there. Removing the fields from the shared type would break a real, still-tested legacy code path outside Wave 1's scope. The deletion is precisely scoped to the one place design.md and this task target: `NativeReviewCliV216.finalize()` (`lib/native-review-cli.ts:2149-2195`), which is the negotiated production default and the only implementation where the staging was provably dead (computed, written to disk, never referenced in argv). +- [x] W1.13 GREEN `lib/native-review-cli.ts` (`:2153-2195`): deleted the dead `resultFiles` staging and its contribution to `needsStaging` inside `NativeReviewCliV216.finalize()`; regenerated `runtime/native-review-cli.mjs` via `node scripts/build-git-commit-transaction-runner.mjs --write`; verified with `pnpm run check:transaction-runner` (green). +- [x] W1.14 Verify: `pnpm test` (unit phase) — 1022/1026 pass, 1 skipped, only the 3 pre-existing RDD-off failures in `tests/native-review-parity-runtime.test.ts` remain (confirmed identical on `git stash` baseline). `tests/runtime-harness.mjs` fails identically on baseline (missing pinned binary, expected per the environmental note). `pnpm run check:transaction-runner` passes. ## W2: Diagnostics, one-shot relaunch, lost-output recovery, cleanup (depends on W1's `CaptureSlot`)