From 1e485a8e5c7f496d1933749fc7154d5ac068a67c Mon Sep 17 00:00:00 2001 From: "DESKTOP-R5R1H1R\\rafal" Date: Fri, 24 Jul 2026 01:26:35 +0200 Subject: [PATCH] fix(review): bind capture evidence to reviewed target Ensure provider-owned subject context and strict evidence schemas survive capture while rejecting post-review substitutions. --- .../fixtures/capabilities-v1.4.fixture.json | 1 + .../v1/fixtures/start-v2.fixture.json | 6 + .../v1/fixtures/status-v2.fixture.json | 1 + .../verification-evidence.fixture.json | 12 + .../v1/schemas/start-v2.schema.json | 5 + .../v1/schemas/status-v2.schema.json | 6 + .../schemas/verification-evidence.schema.json | 45 ++++ docs/review-authority-threat-model.md | 2 +- docs/review-integration.md | 10 +- internal/assets/assets_test.go | 5 +- .../plugins/review-result-artifacts.ts | 75 ++++++- .../review_result_artifacts_behavior_test.go | 208 ++++++++++++++++++ .../skills/_shared/review-ledger-contract.md | 2 +- internal/cli/review_artifact.go | 64 +++++- internal/cli/review_artifact_test.go | 154 +++++++++++++ internal/cli/review_capabilities.go | 1 + internal/cli/review_capabilities_test.go | 4 +- internal/cli/review_facade.go | 47 +++- internal/cli/review_next_transition.go | 37 +++- internal/cli/review_next_transition_test.go | 121 ++++++++-- internal/cli/review_operation_contract.go | 11 +- internal/cli/review_schema.go | 66 +++++- internal/cli/review_schema_test.go | 90 ++++++++ internal/cli/review_start_context_test.go | 11 + internal/cli/review_start_contract.go | 66 +++--- internal/cli/review_status_contract.go | 9 +- .../sdd/review_ledger_contract_test.go | 4 +- .../reviewtransaction/artifact_admission.go | 66 +++++- .../artifact_admission_test.go | 4 + scripts/test-review-contract-package.sh | 2 + 30 files changed, 1061 insertions(+), 74 deletions(-) create mode 100644 contracts/review-integration/v1/fixtures/verification-evidence.fixture.json create mode 100644 contracts/review-integration/v1/schemas/verification-evidence.schema.json create mode 100644 internal/assets/review_result_artifacts_behavior_test.go diff --git a/contracts/review-integration/v1/fixtures/capabilities-v1.4.fixture.json b/contracts/review-integration/v1/fixtures/capabilities-v1.4.fixture.json index fe49a7fc3..dbe3d8401 100644 --- a/contracts/review-integration/v1/fixtures/capabilities-v1.4.fixture.json +++ b/contracts/review-integration/v1/fixtures/capabilities-v1.4.fixture.json @@ -59,6 +59,7 @@ "gentle-ai.review-integration.repair/v1", "gentle-ai.review-integration.start/v2", "gentle-ai.review-integration.status/v2", + "gentle-ai.review-verification-evidence/v1", "gentle-ai.review-receipt/v1", "gentle-ai.review-receipt/v2", "gentle-ai.review-result-artifact/v2", diff --git a/contracts/review-integration/v1/fixtures/start-v2.fixture.json b/contracts/review-integration/v1/fixtures/start-v2.fixture.json index 5aec892fe..92c44dde1 100644 --- a/contracts/review-integration/v1/fixtures/start-v2.fixture.json +++ b/contracts/review-integration/v1/fixtures/start-v2.fixture.json @@ -70,6 +70,12 @@ "selected_order": 3 } ], + "reviewer_task_bindings": [ + "GENTLE_AI_REVIEW_BINDING {\"lineage\":\"review-start-fixture\",\"target\":\"sha256:136dc6556e3bac8c2e7f7af7cc5ec361f449e383a997638128729059fefa06a5\",\"lens\":\"review-risk\",\"order\":0,\"revision\":\"sha256:3411d049f39e891b50adfc76473199b473927d10245a227a3bc967eae0caf68c\",\"repository_context\":\"rctx1_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\"subject_hash\":\"sha256:38a55988f6b3ae6f520690d52fd29255f425a4048b0f8694de3a80c5daf265cd\"}", + "GENTLE_AI_REVIEW_BINDING {\"lineage\":\"review-start-fixture\",\"target\":\"sha256:136dc6556e3bac8c2e7f7af7cc5ec361f449e383a997638128729059fefa06a5\",\"lens\":\"review-resilience\",\"order\":1,\"revision\":\"sha256:3411d049f39e891b50adfc76473199b473927d10245a227a3bc967eae0caf68c\",\"repository_context\":\"rctx1_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\"subject_hash\":\"sha256:06d86f981a8c0b16c2179696d467c8157580214ad72c952adee423fe596d2c21\"}", + "GENTLE_AI_REVIEW_BINDING {\"lineage\":\"review-start-fixture\",\"target\":\"sha256:136dc6556e3bac8c2e7f7af7cc5ec361f449e383a997638128729059fefa06a5\",\"lens\":\"review-readability\",\"order\":2,\"revision\":\"sha256:3411d049f39e891b50adfc76473199b473927d10245a227a3bc967eae0caf68c\",\"repository_context\":\"rctx1_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\"subject_hash\":\"sha256:beacbf1eed52620ef7686087f8944cbc2304d71abf13281267efe9e4de20fb1e\"}", + "GENTLE_AI_REVIEW_BINDING {\"lineage\":\"review-start-fixture\",\"target\":\"sha256:136dc6556e3bac8c2e7f7af7cc5ec361f449e383a997638128729059fefa06a5\",\"lens\":\"review-reliability\",\"order\":3,\"revision\":\"sha256:3411d049f39e891b50adfc76473199b473927d10245a227a3bc967eae0caf68c\",\"repository_context\":\"rctx1_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\"subject_hash\":\"sha256:61a10b133e3f7a8ccad1d0f1d58989df75d2330ad530690137de625bfd10eec8\"}" + ], "candidate_diff": { "encoding": "base64", "data": "ZGlmZiAtLWdpdCBhL3NjcmlwdHMvZGVwbG95LnNoIGIvc2NyaXB0cy9kZXBsb3kuc2gKbmV3IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMC4uMjhiM2FjMTI4ZmJiZWFmZmY0OWEzMjYxMDVmOGVhYzg1YzZjNzdhMQotLS0gL2Rldi9udWxsCisrKyBiL3NjcmlwdHMvZGVwbG95LnNoCkBAIC0wLDAgKzEgQEAKK2VjaG8gZGVwbG95Cg==", diff --git a/contracts/review-integration/v1/fixtures/status-v2.fixture.json b/contracts/review-integration/v1/fixtures/status-v2.fixture.json index 47bead838..7bf7c0cc6 100644 --- a/contracts/review-integration/v1/fixtures/status-v2.fixture.json +++ b/contracts/review-integration/v1/fixtures/status-v2.fixture.json @@ -151,6 +151,7 @@ "value": "0" } ], + "reviewer_task_binding": "GENTLE_AI_REVIEW_BINDING {\"lineage\":\"review-status-fixture\",\"target\":\"sha256:e6faad1cb9ceb2db170e0ec9ea5394b44c3991dff6cbd02fb6787539715968f6\",\"lens\":\"review-reliability\",\"order\":0,\"revision\":\"sha256:81cdad8f3182d4919147d3c4fe8f614814068e5ef68417e3dcbd5e326dd3e390\",\"repository_context\":\"rctx1_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\"subject_hash\":\"sha256:fe57db2ea45a33c7e852d039e8af3291c3bc31911f0c92a710b5fef547b1999d\"}", "artifact_subject": { "schema": "gentle-ai.review-artifact-subject/v1", "subject_hash": "sha256:fe57db2ea45a33c7e852d039e8af3291c3bc31911f0c92a710b5fef547b1999d", diff --git a/contracts/review-integration/v1/fixtures/verification-evidence.fixture.json b/contracts/review-integration/v1/fixtures/verification-evidence.fixture.json new file mode 100644 index 000000000..817354506 --- /dev/null +++ b/contracts/review-integration/v1/fixtures/verification-evidence.fixture.json @@ -0,0 +1,12 @@ +{ + "schema": "gentle-ai.review-verification-evidence/v1", + "outcome": "passed", + "checks": [ + { + "name": "focused tests", + "status": "passed", + "command": "go test ./internal/cli", + "evidence": ["ok github.com/gentleman-programming/gentle-ai/internal/cli"] + } + ] +} diff --git a/contracts/review-integration/v1/schemas/start-v2.schema.json b/contracts/review-integration/v1/schemas/start-v2.schema.json index d979f1843..12c3356e5 100644 --- a/contracts/review-integration/v1/schemas/start-v2.schema.json +++ b/contracts/review-integration/v1/schemas/start-v2.schema.json @@ -101,6 +101,11 @@ "maxItems": 4, "items": {"$ref": "#/$defs/artifact_subject"} }, + "reviewer_task_bindings": { + "type": "array", + "maxItems": 4, + "items": {"type": "string", "pattern": "^GENTLE_AI_REVIEW_BINDING \\{"} + }, "candidate_diff": {"$ref": "#/$defs/frozen_candidate_diff"}, "changed_path_manifest": { "type": "array", diff --git a/contracts/review-integration/v1/schemas/status-v2.schema.json b/contracts/review-integration/v1/schemas/status-v2.schema.json index 81e484503..6dbe516f2 100644 --- a/contracts/review-integration/v1/schemas/status-v2.schema.json +++ b/contracts/review-integration/v1/schemas/status-v2.schema.json @@ -142,6 +142,7 @@ "properties": { "name": { "type": "string", "pattern": "^[a-z0-9_]+$" }, "schema": { "type": "string", "minLength": 1 }, "capture_operation": { "type": "string", "minLength": 1 }, "arguments": { "type": "array", "minItems": 1, "items": { "$ref": "#/$defs/transition_argument" } }, + "reviewer_task_binding": { "type": "string", "pattern": "^GENTLE_AI_REVIEW_BINDING \\{" }, "artifact_subject": { "$ref": "artifact-subject.schema.json" }, "candidate_diff": { "$ref": "start-v2.schema.json#/$defs/frozen_candidate_diff" }, "changed_path_manifest": { "type": "array", "uniqueItems": true, "items": { "$ref": "start-v2.schema.json#/$defs/changed_path" } }, @@ -159,10 +160,15 @@ "else": { "allOf": [ { "not": { "required": ["artifact_subject"] } }, + { "not": { "required": ["reviewer_task_binding"] } }, { "not": { "required": ["candidate_diff"] } }, { "not": { "required": ["changed_path_manifest"] } } ] } + }, + { + "if": { "properties": { "capture_operation": { "const": "review.capture-evidence" } }, "required": ["capture_operation"] }, + "then": { "properties": { "schema": { "const": "gentle-ai.review-verification-evidence/v1" } } } } ] }, diff --git a/contracts/review-integration/v1/schemas/verification-evidence.schema.json b/contracts/review-integration/v1/schemas/verification-evidence.schema.json new file mode 100644 index 000000000..1dae731e7 --- /dev/null +++ b/contracts/review-integration/v1/schemas/verification-evidence.schema.json @@ -0,0 +1,45 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://gentle-ai.dev/contracts/review-integration/v1/schemas/verification-evidence.schema.json", + "title": "Gentle AI final verification evidence", + "type": "object", + "additionalProperties": false, + "required": ["schema", "outcome", "checks"], + "properties": { + "schema": { "const": "gentle-ai.review-verification-evidence/v1" }, + "outcome": { "enum": ["passed", "failed"] }, + "checks": { + "type": "array", + "minItems": 1, + "items": { + "type": "object", + "additionalProperties": false, + "required": ["name", "status", "evidence"], + "properties": { + "name": { "type": "string", "pattern": "\\S" }, + "status": { "enum": ["passed", "failed"] }, + "command": { "type": "string", "pattern": "\\S" }, + "evidence": { + "type": "array", + "minItems": 1, + "items": { "type": "string", "pattern": "\\S" } + } + } + } + } + }, + "examples": [ + { + "schema": "gentle-ai.review-verification-evidence/v1", + "outcome": "passed", + "checks": [ + { + "name": "focused tests", + "status": "passed", + "command": "go test ./internal/cli", + "evidence": ["ok github.com/gentleman-programming/gentle-ai/internal/cli"] + } + ] + } + ] +} diff --git a/docs/review-authority-threat-model.md b/docs/review-authority-threat-model.md index 4c122f788..7fdc3db85 100644 --- a/docs/review-authority-threat-model.md +++ b/docs/review-authority-threat-model.md @@ -41,7 +41,7 @@ Frozen intended-untracked membership proves what entered the reviewed candidate ## Review Input Schemas -Run `gentle-ai review schema reviewer`, `gentle-ai review schema refuter`, `gentle-ai review schema validator`, or `gentle-ai review schema final-verification-incident` to print a versioned JSON Schema. Final verification evidence remains arbitrary non-empty bytes and therefore has no invented JSON contract. The incident schema is different: its native parser accepts only compact canonical JSON plus one LF, rejects unknown fields, and admits only `procedural_tooling_failure`. Unknown JSON fields and semantic violations remain rejected before authority changes. +Run `gentle-ai review schema reviewer`, `gentle-ai review schema refuter`, `gentle-ai review schema validator`, `gentle-ai review schema verification-evidence`, or `gentle-ai review schema final-verification-incident` to print a versioned JSON Schema. Provider-owned final verification evidence uses `gentle-ai.review-verification-evidence/v1`: the provider records an overall `passed` or `failed` outcome and one or more named checks with explicit evidence, while the native capture operation supplies lineage, target, revision, canonical bytes, and hashes. Pass the payload as a file to the `review.capture-evidence` transition emitted by `review status --next-transition`; once captured, lineage-only `review finalize` discovers it and preserves exact authority bindings. The incident schema is different: its native parser accepts only compact canonical JSON plus one LF, rejects unknown fields, and admits only `procedural_tooling_failure`. Unknown JSON fields and semantic violations remain rejected before authority changes. An ordinary bounded lineage permits exactly one changed-target correction attempt. The initial lenses and frozen finding IDs execute once, while that correction records its snapshot, validation checks, and changed-line charge without expanding immutable genesis paths or the frozen budget. Consuming the attempt exhausts ordinary correction even when its measured delta is zero; a later change requires an authorized successor rather than another fix transition. Historical multi-attempt records remain readable, but cannot append another attempt. diff --git a/docs/review-integration.md b/docs/review-integration.md index 4236268d7..79a8e8866 100644 --- a/docs/review-integration.md +++ b/docs/review-integration.md @@ -253,6 +253,12 @@ When a release merge retains an approved `current-changes` candidate but expands Malformed reviewer JSON, missing required reviewer arrays, canonicalization failures, and selected-lens mismatches are deterministic preflight failures. Negotiated finalize reports `invalid_request`, `mutation_outcome: not_started`, `retry_safe: true`, `replayability: not_replayable`, and `next_action: correct_request`, while preserving a valid requested lineage for target-scoped recovery. Correct the payload before retrying; do not run authority repair. +### Complete final verification from validating + +When negotiated status emits `collect / verification_evidence_required`, use its exact `review.capture-evidence` operation and lineage, target, and expected-revision arguments. Obtain the public payload contract with `gentle-ai review schema verification-evidence`, write the provider-owned `gentle-ai.review-verification-evidence/v1` payload to a file, and pass that file with `--input`. The provider records check names, statuses, optional commands, and concrete result evidence; native capture canonicalizes the JSON and derives all bindings and hashes. + +After capture, status emits `execute / captured_verification_evidence_ready`. Lineage-only `review finalize` also discovers the same canonical evidence after restart, derives pass or failure from structured evidence, and publishes or discovers the terminal receipt idempotently. Do not replay `--captured-results` after authority leaves `reviewing`: negotiated finalize rejects that stale selector without mutation and directs the caller to `review.status` for the current transition. + ### Reopen unusable validating results without another budget `gentle-ai review reopen-results` is a bounded maintenance operation for an uncorrected validating authority whose historical reviewer artifact was unadmitted or whose preserved evidence says candidate inspection was unavailable. It never starts a lineage or recalculates target, tier, lenses, changed-line count, or correction budget. @@ -304,8 +310,8 @@ Pi adoption, fallback retirement, package pinning, and Pi release sequencing are Each release archive contains: -- `contracts/review-integration/v1/schemas/` — twenty strict JSON Schemas, including preserved capability protocols v1.0–v1.3, current v1.4, versioned START/status/result-artifact contracts, final-verification incident, classified repair, provider subject/admission, and targeted validation. -- `contracts/review-integration/v1/fixtures/` — twenty-four deterministic conformance fixtures, including all five capability minors, preserved v1 plus current v2 START/status examples, the final-verification incident and retry projection, classified repair preflight, and typed failure envelopes. +- `contracts/review-integration/v1/schemas/` — twenty-one strict JSON Schemas, including preserved capability protocols v1.0–v1.3, current v1.4, versioned START/status/result-artifact contracts, final-verification evidence and incident, classified repair, provider subject/admission, and targeted validation. +- `contracts/review-integration/v1/fixtures/` — twenty-five deterministic conformance fixtures, including all five capability minors, preserved v1 plus current v2 START/status examples, final-verification evidence, the final-verification incident and retry projection, classified repair preflight, and typed failure envelopes. - `docs/review-integration.md` — this ownership and consumption guide. Repository maintainers can verify source inventory or a complete GoReleaser snapshot: diff --git a/internal/assets/assets_test.go b/internal/assets/assets_test.go index 528c0ab7a..c1ad1eefb 100644 --- a/internal/assets/assets_test.go +++ b/internal/assets/assets_test.go @@ -464,7 +464,10 @@ func TestReviewResultArtifactsPluginContract(t *testing.T) { `artifact_subject`, `candidate_diff`, `changed_path_manifest`, - `output.args.prompt = await injectReviewerContext(`, + `const injected = await injectReviewerContext(`, + `retainedPreflight.set(bindingKey(parseBinding(output.args.prompt, output.args.subagent_type)), injected.preflight)`, + `result = enrichedReviewerResult(binding, result, preflight)`, + `inspection: { status: "completed", paths }`, `"--lineage", binding.lineage`, `"--target", binding.target`, `"--lens", binding.lens`, diff --git a/internal/assets/opencode/plugins/review-result-artifacts.ts b/internal/assets/opencode/plugins/review-result-artifacts.ts index 065bdf4f2..41a6ff4cd 100644 --- a/internal/assets/opencode/plugins/review-result-artifacts.ts +++ b/internal/assets/opencode/plugins/review-result-artifacts.ts @@ -27,6 +27,18 @@ interface ReviewCapturePreflight { changed_path_manifest: Array> } +function validManifestPath(path: unknown): path is string { + return typeof path === "string" && path !== "" && !path.startsWith("/") && !path.includes("\\") && + path.split("/").every((segment) => segment !== "" && segment !== "." && segment !== "..") +} + +function bindingKey(binding: ReviewBinding): string { + return JSON.stringify([ + binding.lineage, binding.target, binding.lens, binding.order, + binding.revision ?? "", binding.repository_context ?? "", + ]) +} + function parseBinding(prompt: unknown, lens: string): ReviewBinding { const match = BINDING.exec(typeof prompt === "string" ? prompt : "") if (!match) throw new Error("review task is missing GENTLE_AI_REVIEW_BINDING") @@ -139,9 +151,11 @@ async function preflightCapture(cwd: string, binding: ReviewBinding): Promise const subject = value.artifact_subject as Record | undefined + const manifest = value.changed_path_manifest as Array> | undefined if (!subject || typeof subject.subject_hash !== "string" || !/^sha256:[a-f0-9]{64}$/.test(subject.subject_hash) || !value.candidate_diff || typeof value.candidate_diff !== "object" || Array.isArray(value.candidate_diff) || - !Array.isArray(value.changed_path_manifest) || value.changed_path_manifest.some((entry) => !entry || typeof entry !== "object" || Array.isArray(entry))) { + !Array.isArray(manifest) || manifest.some((entry) => !entry || typeof entry !== "object" || Array.isArray(entry) || !validManifestPath(entry.path)) || + new Set(manifest.map((entry) => entry.path)).size !== manifest.length) { throw new Error("review capture preflight returned incomplete frozen candidate context") } if (binding.subject_hash && subject.subject_hash !== binding.subject_hash) { @@ -169,10 +183,13 @@ async function preflightCapture(cwd: string, binding: ReviewBinding): Promise { +async function injectReviewerContext(prompt: string, lens: string, cwd: string): Promise<{prompt: string, preflight?: ReviewCapturePreflight}> { const binding = parseBinding(prompt, lens) const preflight = await preflightCapture(cwd, binding) - if (!preflight) return prompt + if (!preflight) { + if (binding.subject_hash) throw new Error("provider-bound review task requires capture preflight") + return { prompt } + } const injectedBinding = { ...binding, subject_hash: preflight.artifact_subject.subject_hash } const boundPrompt = prompt.replace(BINDING, `GENTLE_AI_REVIEW_BINDING ${JSON.stringify(injectedBinding)}\n`) const frozen = JSON.stringify({ @@ -180,7 +197,40 @@ async function injectReviewerContext(prompt: string, lens: string, cwd: string): candidate_diff: preflight.candidate_diff, changed_path_manifest: preflight.changed_path_manifest, }) - return `${boundPrompt.trimEnd()}\n${FROZEN_CONTEXT}${frozen}` + return { prompt: `${boundPrompt.trimEnd()}\n${FROZEN_CONTEXT}${frozen}`, preflight } +} + +function enrichedReviewerResult(binding: ReviewBinding, result: string, preflight: ReviewCapturePreflight | undefined): string { + if (!preflight) { + if (binding.subject_hash) throw new Error("review task is missing retained provider context") + return result + } + if (binding.subject_hash !== preflight.artifact_subject.subject_hash) { + throw new Error("review task binding does not match retained provider context") + } + const paths = preflight.changed_path_manifest.map((entry) => entry.path) + if (paths.some((path) => typeof path !== "string" || path === "")) { + throw new Error("retained provider context contains a malformed changed-path manifest") + } + let parsed: unknown + try { + parsed = JSON.parse(result) + } catch { + throw new Error("reviewer result is not strict JSON") + } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + throw new Error("reviewer result must be an object") + } + const value = parsed as Record + if (Object.keys(value).sort().join(",") !== "evidence,findings" || !Array.isArray(value.findings) || !Array.isArray(value.evidence)) { + throw new Error("reviewer result must contain only findings and evidence") + } + return JSON.stringify({ + subject_hash: preflight.artifact_subject.subject_hash, + inspection: { status: "completed", paths }, + findings: value.findings, + evidence: value.evidence, + }) } function preserveResult(cwd: string, binding: ReviewBinding, raw: string, cls?: string): Promise { @@ -250,18 +300,24 @@ async function preservedCaptureFailure(cwd: string, binding: ReviewBinding, raw: } } -const ReviewResultArtifactsPlugin: Plugin = async ({ directory, worktree }) => ({ +const ReviewResultArtifactsPlugin: Plugin = async ({ directory, worktree }) => { + const retainedPreflight = new Map() + return { "tool.execute.before": async (input, output) => { if (input.tool !== "task" || typeof output.args?.subagent_type !== "string" || !REVIEW_AGENTS.has(output.args.subagent_type) || !BINDING.test(output.args.prompt)) return if (output.args.background === true) { throw new Error("bound review tasks must run in the foreground for native result capture") } - output.args.prompt = await injectReviewerContext( + const injected = await injectReviewerContext( output.args.prompt, output.args.subagent_type, captureCwd(worktree, directory), ) + output.args.prompt = injected.prompt + if (injected.preflight) { + retainedPreflight.set(bindingKey(parseBinding(output.args.prompt, output.args.subagent_type)), injected.preflight) + } }, "tool.execute.after": async (input, output) => { if (input.tool !== "task" || typeof input.args?.subagent_type !== "string" || !REVIEW_AGENTS.has(input.args.subagent_type)) return @@ -283,11 +339,16 @@ const ReviewResultArtifactsPlugin: Plugin = async ({ directory, worktree }) => ( throw await preservedCaptureFailure(cwd, binding, output.output, cause) } try { + const key = bindingKey(binding) + const preflight = retainedPreflight.get(key) + retainedPreflight.delete(key) + result = enrichedReviewerResult(binding, result, preflight) output.output = await captureResult(cwd, binding, result) } catch (cause) { throw await preservedCaptureFailure(cwd, binding, result, cause) } }, -}) + } +} export default ReviewResultArtifactsPlugin diff --git a/internal/assets/review_result_artifacts_behavior_test.go b/internal/assets/review_result_artifacts_behavior_test.go new file mode 100644 index 000000000..d6a590ba6 --- /dev/null +++ b/internal/assets/review_result_artifacts_behavior_test.go @@ -0,0 +1,208 @@ +package assets + +import ( + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +type nativePluginInvocation struct { + Args []string `json:"args"` + Stdin string `json:"stdin"` +} + +func TestReviewResultArtifactsPluginEnrichesCaptureFromProviderContext(t *testing.T) { + if testing.Short() { + t.Skip("requires Node.js") + } + if _, err := exec.LookPath("node"); err != nil { + t.Skip("requires Node.js") + } + t.Run("findings and evidence receive provider-owned capture metadata", func(t *testing.T) { + invocations, output := runReviewResultArtifactsHarness(t, false, false, false) + if output != `{"captured":true}` { + t.Fatalf("plugin output = %q", output) + } + if len(invocations) != 2 || !containsArgument(invocations[0].Args, "--preflight") || + invocations[1].Args[0] != "review" || invocations[1].Args[1] != "capture-result" { + t.Fatalf("native invocations = %#v", invocations) + } + var capture map[string]any + if err := json.Unmarshal([]byte(invocations[1].Stdin), &capture); err != nil { + t.Fatalf("decode capture stdin: %v\n%s", err, invocations[1].Stdin) + } + want := map[string]any{ + "subject_hash": "sha256:" + strings.Repeat("a", 64), + "inspection": map[string]any{"status": "completed", "paths": []any{"z-last.go", "a-first.go"}}, + "findings": []any{}, + "evidence": []any{"inspected exact frozen candidate"}, + } + if !equalJSON(capture, want) { + t.Fatalf("capture stdin = %#v, want %#v", capture, want) + } + }) + + t.Run("binding drift fails before capture", func(t *testing.T) { + invocations, output := runReviewResultArtifactsHarness(t, true, false, false) + if len(invocations) != 2 || !containsArgument(invocations[0].Args, "--preflight") || + invocations[1].Args[0] != "review" || invocations[1].Args[1] != "preserve-result" { + t.Fatalf("binding drift reached capture: %#v", invocations) + } + if !strings.Contains(output, "repository_context_capture_failed") || !strings.Contains(output, "preserved for recovery") { + t.Fatalf("binding drift error = %q", output) + } + }) + + t.Run("provider binding fails before launch when preflight is unavailable", func(t *testing.T) { + invocations, output := runReviewResultArtifactsHarness(t, false, true, false) + if len(invocations) != 1 || !containsArgument(invocations[0].Args, "--preflight") { + t.Fatalf("missing preflight launched reviewer or capture: %#v", invocations) + } + if !strings.Contains(output, "requires capture preflight") { + t.Fatalf("missing preflight error = %q", output) + } + }) + + t.Run("capture failure preserves the provider-enriched replay payload", func(t *testing.T) { + invocations, output := runReviewResultArtifactsHarness(t, false, false, true) + if len(invocations) != 3 || invocations[1].Args[1] != "capture-result" || invocations[2].Args[1] != "preserve-result" { + t.Fatalf("capture recovery invocations = %#v", invocations) + } + if invocations[1].Stdin != invocations[2].Stdin || !strings.Contains(invocations[2].Stdin, `"subject_hash":"sha256:`) || + !strings.Contains(invocations[2].Stdin, `"inspection":{"status":"completed"`) { + t.Fatalf("preserved replay payload = %q, capture payload = %q", invocations[2].Stdin, invocations[1].Stdin) + } + if !strings.Contains(output, "preserved for recovery") { + t.Fatalf("capture recovery error = %q", output) + } + }) +} + +func runReviewResultArtifactsHarness(t *testing.T, drift, unsupportedPreflight, captureFail bool) ([]nativePluginInvocation, string) { + t.Helper() + dir := t.TempDir() + plugin := filepath.Join(dir, "review-result-artifacts.mts") + if err := os.WriteFile(plugin, []byte(MustRead("opencode/plugins/review-result-artifacts.ts")), 0o600); err != nil { + t.Fatal(err) + } + mockSource := filepath.Join(dir, "mock-gentle-ai.go") + if err := os.WriteFile(mockSource, []byte(`package main +import ( + "encoding/json" + "io" + "os" + "slices" +) +func main() { + stdin, _ := io.ReadAll(os.Stdin) + invocation, _ := json.Marshal(map[string]any{"args": os.Args[1:], "stdin": string(stdin)}) + log, _ := os.OpenFile(os.Getenv("GENTLE_AI_TEST_LOG"), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600) + if log != nil { _, _ = log.Write(append(invocation, '\n')); _ = log.Close() } + if slices.Contains(os.Args[1:], "--preflight") && os.Getenv("GENTLE_AI_TEST_UNSUPPORTED_PREFLIGHT") == "true" { + _, _ = os.Stderr.WriteString("flag provided but not defined: -preflight") + os.Exit(1) + } + if slices.Contains(os.Args[1:], "--preflight") { _, _ = os.Stdout.WriteString(os.Getenv("GENTLE_AI_TEST_PREFLIGHT")); return } + if len(os.Args) > 2 && os.Args[1] == "review" && os.Args[2] == "capture-result" && os.Getenv("GENTLE_AI_TEST_CAPTURE_FAIL") == "true" { + _, _ = os.Stderr.WriteString("simulated capture failure") + os.Exit(1) + } + _, _ = os.Stdout.WriteString("{\"captured\":true}") +} +`), 0o600); err != nil { + t.Fatal(err) + } + mock := filepath.Join(dir, "gentle-ai") + if strings.EqualFold(filepath.Ext(os.Args[0]), ".exe") { + mock += ".exe" + } + build := exec.Command("go", "build", "-o", mock, mockSource) + if output, err := build.CombinedOutput(); err != nil { + t.Fatalf("build native mock: %v\n%s", err, output) + } + harness := filepath.Join(dir, "harness.mjs") + if err := os.WriteFile(harness, []byte(` +import pluginFactory from "./review-result-artifacts.mts" +const subject = "sha256:" + "a".repeat(64) +const binding = { + lineage: "review-provider-binding", target: "sha256:" + "b".repeat(64), lens: "review-reliability", order: 0, + revision: "sha256:" + "c".repeat(64), repository_context: "rctx1_" + "d".repeat(64), subject_hash: subject, +} +const args = {subagent_type: binding.lens, prompt: `+"`"+`GENTLE_AI_REVIEW_BINDING ${JSON.stringify(binding)} +Review the frozen candidate.`+"`"+`, background: false} +const hooks = await pluginFactory({directory: process.cwd(), worktree: process.cwd()}) +try { + await hooks["tool.execute.before"]({tool: "task"}, {args}) +} catch (error) { + process.stdout.write(error instanceof Error ? error.message : String(error)) + process.exit(0) +} +if (process.env.GENTLE_AI_TEST_DRIFT === "true") args.prompt = args.prompt.replace(subject, "sha256:" + "e".repeat(64)) +const output = {output: '{"findings":[],"evidence":["inspected exact frozen candidate"]}'} +try { + await hooks["tool.execute.after"]({tool: "task", args}, output) + process.stdout.write(output.output) +} catch (error) { + process.stdout.write(error instanceof Error ? error.message : String(error)) +} +`), 0o600); err != nil { + t.Fatal(err) + } + logPath := filepath.Join(dir, "native.jsonl") + preflight, err := json.Marshal(map[string]any{ + "artifact_subject": map[string]any{"subject_hash": "sha256:" + strings.Repeat("a", 64)}, + "candidate_diff": map[string]any{"encoding": "base64", "content": "ZGlmZg=="}, + "changed_path_manifest": []any{ + map[string]any{"path": "z-last.go"}, map[string]any{"path": "a-first.go"}, + }, + }) + if err != nil { + t.Fatal(err) + } + command := exec.Command("node", harness) + command.Dir = dir + command.Env = append(os.Environ(), + "PATH="+dir+string(os.PathListSeparator)+os.Getenv("PATH"), + "GENTLE_AI_TEST_LOG="+logPath, + "GENTLE_AI_TEST_PREFLIGHT="+string(preflight), + "GENTLE_AI_TEST_DRIFT="+map[bool]string{false: "false", true: "true"}[drift], + "GENTLE_AI_TEST_UNSUPPORTED_PREFLIGHT="+map[bool]string{false: "false", true: "true"}[unsupportedPreflight], + "GENTLE_AI_TEST_CAPTURE_FAIL="+map[bool]string{false: "false", true: "true"}[captureFail], + ) + output, err := command.CombinedOutput() + if err != nil { + t.Fatalf("run plugin harness: %v\n%s", err, output) + } + payload, err := os.ReadFile(logPath) + if err != nil { + t.Fatal(err) + } + lines := strings.Split(strings.TrimSpace(string(payload)), "\n") + invocations := make([]nativePluginInvocation, 0, len(lines)) + for _, line := range lines { + var invocation nativePluginInvocation + if err := json.Unmarshal([]byte(line), &invocation); err != nil { + t.Fatal(err) + } + invocations = append(invocations, invocation) + } + return invocations, string(output) +} + +func containsArgument(args []string, want string) bool { + for _, arg := range args { + if arg == want { + return true + } + } + return false +} + +func equalJSON(left, right any) bool { + leftJSON, _ := json.Marshal(left) + rightJSON, _ := json.Marshal(right) + return string(leftJSON) == string(rightJSON) +} diff --git a/internal/assets/skills/_shared/review-ledger-contract.md b/internal/assets/skills/_shared/review-ledger-contract.md index 83dda490b..c2f837750 100644 --- a/internal/assets/skills/_shared/review-ledger-contract.md +++ b/internal/assets/skills/_shared/review-ledger-contract.md @@ -6,7 +6,7 @@ Parent orchestrator and native CLI only. Never pass this contract to a reviewer, Call `gentle-ai review start` once. The native facade discovers the repository root and untracked scope, derives the immutable target, selects zero lenses for low risk, one focus lens for standard risk, or canonical 4R for high risk, and freezes the original line count, tier, and correction budget `min(200, ceil(original_changed_lines / 2))`. Goldens stay in snapshot identity but not that count. Correction and compatible base advance never recalculate risk or open review. -Run each selected lens once in the foreground. Prefix its prompt with START's exact `GENTLE_AI_REVIEW_BINDING`, including `subject_hash`. Return one JSON object echoing `subject_hash`; require `inspection.status: "completed"`, all manifest paths in order as `inspection.paths`, `findings`/`evidence`, and severe `evidence_class`/`causal_disposition`; access failure is not completion. `gentle-ai review capture-result` follows the native transition; handles are cwd-independent and legacy bindings need `--cwd`. Pass manifests in lens order with repeated `--result-artifact-file ` arguments, BOM-less UTF-8 on Windows PowerShell 5.1. The POSIX inline `--result-artifact ''` form remains compatible; so does provider-owned `--captured-results`; never pass raw `--result`. Native Go validates, canonicalizes, persists, hashes, reopens, and binds results; models never construct canonical bytes or hashes. Freeze merged findings. Only `introduced`, `behavior-activated`, or `worsened` with changed-hunk, candidate-created-path, differential-test, or before/after proof may block. Route `pre-existing` and `base-only` to follow-ups; `unknown` escalates. WARNING/SUGGESTION remain `info`. Deterministic blockers need no refuter; inferential blockers share one read-only refuter batch. Judgment Day uses two independent judges. +Run each selected lens once in the foreground. Prefix its prompt with START's exact provider-rendered `GENTLE_AI_REVIEW_BINDING`, including `subject_hash`. The reviewer returns only `findings` and `evidence`, with severe findings carrying `evidence_class` and `causal_disposition`; models never author hashes, manifests, or inspection metadata. The managed adapter derives top-level `subject_hash` and completed ordered `inspection.paths` from retained provider context before `gentle-ai review capture-result`; access failure is not completion. Handles are cwd-independent and legacy bindings need `--cwd`. Pass manifests in lens order with repeated `--result-artifact-file ` arguments, BOM-less UTF-8 on Windows PowerShell 5.1. The POSIX inline `--result-artifact ''` form remains compatible; so does provider-owned `--captured-results`; never pass raw `--result`. Native Go validates, canonicalizes, persists, hashes, reopens, and binds results; models never construct canonical bytes or hashes. Freeze merged findings. Only `introduced`, `behavior-activated`, or `worsened` with changed-hunk, candidate-created-path, differential-test, or before/after proof may block. Route `pre-existing` and `base-only` to follow-ups; `unknown` escalates. WARNING/SUGGESTION remain `info`. Deterministic blockers need no refuter; inferential blockers share one read-only refuter batch. Judgment Day uses two independent judges. Before each lens, append the exact immutable candidate diff and changed-path manifest from START; if unavailable, stop. diff --git a/internal/cli/review_artifact.go b/internal/cli/review_artifact.go index 7cb1a9868..3639ed78c 100644 --- a/internal/cli/review_artifact.go +++ b/internal/cli/review_artifact.go @@ -59,6 +59,14 @@ func RunReviewCaptureEvidence(args []string, stdout io.Writer) error { if err != nil || len(payload) == 0 || len(payload) > reviewResultArtifactLimit { return reviewPreflightError(errors.New("final verification evidence is required")) } + var evidence *reviewVerificationEvidence + if reviewVerificationEvidencePayload(payload) { + parsed, canonical, parseErr := parseReviewVerificationEvidence(payload) + if parseErr != nil { + return reviewPreflightError(parseErr) + } + evidence, payload = &parsed, canonical + } dir := filepath.Join(store.Dir, reviewtransaction.CompactFinalEvidenceDir) if err := ensureReviewerArtifactDir(dir); err != nil { return err @@ -98,20 +106,45 @@ func RunReviewCaptureEvidence(args []string, stdout io.Writer) error { return err } } - return encodeReviewJSON(stdout, map[string]any{"schema": "gentle-ai.review-verification-evidence/v1", "capability": "review.native_final_evidence", "sha256": facadePayloadHash(payload), "lineage_id": state.LineageID, "target_identity": state.CurrentSnapshot.Identity, "revision": record.Revision}) + result := map[string]any{"schema": reviewVerificationEvidenceSchemaName, "capability": "review.native_final_evidence", "sha256": facadePayloadHash(payload), "lineage_id": state.LineageID, "target_identity": state.CurrentSnapshot.Identity, "revision": record.Revision} + if evidence != nil { + result["outcome"] = evidence.Outcome + } + return encodeReviewJSON(stdout, result) +} + +func reviewVerificationEvidencePayload(payload []byte) bool { + var identity struct { + Schema string `json:"schema"` + } + return json.Unmarshal(payload, &identity) == nil && identity.Schema == reviewVerificationEvidenceSchemaName } func readCapturedFinalEvidence(storeDir string, state reviewtransaction.CompactState, revision string) ([]byte, error) { + payload, found, err := discoverCapturedFinalEvidence(storeDir) + if err != nil { + return nil, err + } + if !found { + return nil, errors.New("captured final evidence is unavailable or unsafe") + } + return payload, nil +} + +func discoverCapturedFinalEvidence(storeDir string) ([]byte, bool, error) { path := filepath.Join(storeDir, reviewtransaction.CompactFinalEvidenceDir, reviewtransaction.CompactFinalEvidenceFile) info, err := os.Lstat(path) + if os.IsNotExist(err) { + return nil, false, nil + } if err != nil || !info.Mode().IsRegular() || info.Mode()&os.ModeSymlink != 0 || !reviewArtifactModeSafe(info.Mode(), false) { - return nil, errors.New("captured final evidence is unavailable or unsafe") + return nil, false, errors.New("captured final evidence is unavailable or unsafe") } payload, err := os.ReadFile(path) if err != nil || len(payload) == 0 || len(payload) > reviewResultArtifactLimit { - return nil, errors.New("captured final evidence is invalid") + return nil, false, errors.New("captured final evidence is invalid") } - return payload, nil + return payload, true, nil } type reviewResultArtifact struct { @@ -296,23 +329,32 @@ func RunReviewCaptureResult(args []string, stdout io.Writer) error { if result.Findings == nil || result.Evidence == nil { return reviewPreflightError(errors.New("reviewer result requires explicit findings and evidence arrays")) } - if _, err := prepareCompactReviewerResults(reviewtransaction.CompactState{SelectedLenses: []string{*lens}}, []facadeReviewerResult{result}, facadeRefuterResult{}); err != nil { + if result.Lens != "" { + providedLens, lensErr := nativeFacadeReviewerLens(result.Lens) + if lensErr != nil || providedLens != *lens { + return reviewPreflightError(fmt.Errorf("reviewer result lens %q does not match selected lens %q", result.Lens, *lens)) + } + } + nativeResult := result.nativeLensResult() + nativeResult.Lens = *lens + canonical, err := reviewtransaction.NewCanonicalArtifactLensResult(nativeResult) + if err != nil { return reviewPreflightError(err) } + nativeResult = canonical.Result() + result = result.withCanonicalLensResult(nativeResult) canonicalResult, err := json.Marshal(result) if err != nil { return err } canonicalResult = append(canonicalResult, '\n') - nativeResult := result.nativeLensResult() - nativeResult.Lens = *lens candidateCausalIDs, err := verifiedCandidateCausalFindingIDs(ctx, root, state.InitialSnapshot, nativeResult) if err != nil { return reviewPreflightError(err) } _, admission, err := reviewtransaction.AdmitArtifact(reviewtransaction.ArtifactAdmissionRequest{ ExpectedSubject: subject, FrozenContext: frozen, EchoedSubjectHash: result.SubjectHash, - Inspection: result.Inspection, Result: nativeResult, CandidateCausalFindingIDs: candidateCausalIDs, + Inspection: result.Inspection, CanonicalResult: &canonical, CandidateCausalFindingIDs: candidateCausalIDs, RawPayload: rawPayload, CanonicalPayload: canonicalResult, }) if err != nil { @@ -779,9 +821,13 @@ func decodeAdmittedReviewerResult(payload []byte, expected reviewtransaction.Art canonical = append(canonical, '\n') native := envelope.Result.nativeLensResult() native.Lens = expected.Lens + canonicalResult, err := reviewtransaction.NewCanonicalArtifactLensResult(native) + if err != nil { + return facadeReviewerResult{}, err + } result, revalidated, err := reviewtransaction.AdmitArtifact(reviewtransaction.ArtifactAdmissionRequest{ ExpectedSubject: expected, FrozenContext: frozen, EchoedSubjectHash: envelope.Result.SubjectHash, - Inspection: envelope.Result.Inspection, Result: native, + Inspection: envelope.Result.Inspection, CanonicalResult: &canonicalResult, CandidateCausalFindingIDs: envelope.Admission.CandidateCausalFindingIDs, RawPayload: canonical, CanonicalPayload: canonical, }) diff --git a/internal/cli/review_artifact_test.go b/internal/cli/review_artifact_test.go index 03a801df6..ccee00cfb 100644 --- a/internal/cli/review_artifact_test.go +++ b/internal/cli/review_artifact_test.go @@ -202,6 +202,160 @@ func TestReviewCaptureResultFinalizePreservesCausalClassification(t *testing.T) } } +func TestReviewCaptureResultCanonicalFindingIDMatrixHarness(t *testing.T) { + repo, started, store, record := newArtifactReview(t, true) + wantIDs := map[string]string{ + reviewtransaction.LensRisk: "R1-001", + reviewtransaction.LensResilience: "R4-001", + reviewtransaction.LensReadability: "R2-001", + reviewtransaction.LensReliability: "R3-001", + } + for order, lens := range record.State.SelectedLenses { + result := admittedReviewerResultForTest(t, repo, record, lens, order) + severity := "CRITICAL" + if lens == reviewtransaction.LensRisk || lens == reviewtransaction.LensReliability { + severity = "BLOCKER" + } + class := reviewtransaction.EvidenceDeterministic + causality := reviewtransaction.CausalIntroduced + if lens == reviewtransaction.LensResilience { + severity, class, causality = "WARNING", "", "" + } + result.Findings = []facadeFinding{{ + Location: "service-token.ts:1", Severity: severity, Claim: "candidate-specific finding", + ProofRefs: []string{"service-token.ts:1 changed hunk"}, EvidenceClass: class, CausalDisposition: causality, + }} + input := filepath.Join(t.TempDir(), fmt.Sprintf("%02d-%s.json", order, lens)) + writeReviewCLIJSON(t, input, result) + raw, err := os.ReadFile(input) + if err != nil { + t.Fatal(err) + } + args := []string{ + "--cwd", repo, "--lineage", started.LineageID, "--target", record.State.InitialSnapshot.Identity, + "--lens", lens, "--order", fmt.Sprint(order), "--input", input, + } + var captured, replay bytes.Buffer + if err := RunReviewCaptureResult(args, &captured); err != nil { + t.Fatalf("capture omitted %s finding ID: %v", lens, err) + } + if err := RunReviewCaptureResult(args, &replay); err != nil || captured.String() != replay.String() { + t.Fatalf("exact raw replay for %s changed: %v", lens, err) + } + var artifact reviewResultArtifact + decodeStrictReviewJSON(t, captured.Bytes(), &artifact) + envelopePayload, err := os.ReadFile(artifact.Path) + if err != nil { + t.Fatal(err) + } + var envelope admittedReviewerResult + decodeStrictReviewJSON(t, envelopePayload, &envelope) + if got := envelope.Result.Findings[0].ID; got != wantIDs[lens] { + t.Fatalf("%s canonical finding ID = %q, want %q", lens, got, wantIDs[lens]) + } + wantCausal := []string{} + if facadeSevere(severity) { + wantCausal = []string{wantIDs[lens]} + } + if !reflect.DeepEqual(envelope.Admission.CandidateCausalFindingIDs, wantCausal) || envelope.Admission.RawSHA256 != facadePayloadHash(raw) { + t.Fatalf("%s admission = %#v, raw hash %q", lens, envelope.Admission, facadePayloadHash(raw)) + } + } + loaded, err := store.Load() + if err != nil { + t.Fatal(err) + } + entries, err := os.ReadDir(filepath.Join(store.Dir, reviewtransaction.CompactReviewerResultsDir)) + if err != nil { + t.Fatal(err) + } + if loaded.Revision != record.Revision || loaded.State.State != reviewtransaction.StateReviewing || len(entries) != 8 { + t.Fatalf("four-slot capture mutated lifecycle or missed artifact/digest files: record=%#v entries=%d", loaded, len(entries)) + } +} + +func TestReviewCaptureResultCanonicalFindingIDOwnership(t *testing.T) { + for _, test := range []struct { + name string + findings []facadeFinding + wantIDs []string + wantCausal []string + wantReject bool + }{ + { + name: "explicit ID preserved", wantIDs: []string{"R3-explicit"}, wantCausal: []string{"R3-explicit"}, + findings: []facadeFinding{{ID: "R3-explicit", Location: "tracked.txt:1", Severity: "CRITICAL", Claim: "candidate failure", ProofRefs: []string{"tracked.txt:1 changed hunk"}, EvidenceClass: reviewtransaction.EvidenceDeterministic, CausalDisposition: reviewtransaction.CausalIntroduced}}, + }, + { + name: "multiple omitted IDs keep order", wantIDs: []string{"R3-001", "R3-002"}, wantCausal: []string{}, + findings: []facadeFinding{ + {Location: "tracked.txt:1", Severity: "WARNING", Claim: "first implicit identity", ProofRefs: []string{"tracked.txt:1 changed hunk"}}, + {Location: "tracked.txt:1", Severity: "WARNING", Claim: "second implicit identity", ProofRefs: []string{"tracked.txt:1 changed hunk"}}, + }, + }, + { + name: "explicit and implicit collision rejected", wantReject: true, + findings: []facadeFinding{ + {Location: "tracked.txt:1", Severity: "WARNING", Claim: "implicit identity", ProofRefs: []string{"tracked.txt:1 changed hunk"}}, + {ID: "R3-001", Location: "tracked.txt:1", Severity: "WARNING", Claim: "explicit collision", ProofRefs: []string{"tracked.txt:1 changed hunk"}}, + }, + }, + { + name: "duplicate explicit IDs rejected", wantReject: true, + findings: []facadeFinding{ + {ID: "R3-duplicate", Location: "tracked.txt:1", Severity: "WARNING", Claim: "first explicit identity", ProofRefs: []string{"tracked.txt:1 changed hunk"}}, + {ID: "R3-duplicate", Location: "tracked.txt:1", Severity: "WARNING", Claim: "duplicate explicit identity", ProofRefs: []string{"tracked.txt:1 changed hunk"}}, + }, + }, + { + name: "wrong lens prefix rejected", wantReject: true, + findings: []facadeFinding{{ID: "R1-001", Location: "tracked.txt:1", Severity: "WARNING", Claim: "cross-lens identity", ProofRefs: []string{"tracked.txt:1 changed hunk"}}}, + }, + } { + t.Run(test.name, func(t *testing.T) { + repo, started, store, record := newArtifactReview(t, false) + lens := record.State.SelectedLenses[0] + result := admittedReviewerResultForTest(t, repo, record, lens, 0) + result.Findings = test.findings + input := filepath.Join(t.TempDir(), "result.json") + writeReviewCLIJSON(t, input, result) + var output bytes.Buffer + err := RunReviewCaptureResult([]string{ + "--cwd", repo, "--lineage", started.LineageID, "--target", record.State.InitialSnapshot.Identity, + "--lens", lens, "--order", "0", "--input", input, + }, &output) + if test.wantReject { + if err == nil { + t.Fatal("invalid finding identity was captured") + } + if _, statErr := os.Stat(filepath.Join(store.Dir, reviewtransaction.CompactReviewerResultsDir)); !os.IsNotExist(statErr) { + t.Fatalf("rejection consumed result slot: %v", statErr) + } + assertArtifactRevision(t, store, record.Revision) + return + } + if err != nil { + t.Fatal(err) + } + var artifact reviewResultArtifact + decodeStrictReviewJSON(t, output.Bytes(), &artifact) + payload, err := os.ReadFile(artifact.Path) + if err != nil { + t.Fatal(err) + } + var envelope admittedReviewerResult + decodeStrictReviewJSON(t, payload, &envelope) + gotIDs := make([]string, len(envelope.Result.Findings)) + for index, finding := range envelope.Result.Findings { + gotIDs[index] = finding.ID + } + if !reflect.DeepEqual(gotIDs, test.wantIDs) || !reflect.DeepEqual(envelope.Admission.CandidateCausalFindingIDs, test.wantCausal) { + t.Fatalf("canonical identities = %v admission=%v, want %v/%v", gotIDs, envelope.Admission.CandidateCausalFindingIDs, test.wantIDs, test.wantCausal) + } + }) + } +} + func TestReviewFinalizeArtifactFiles(t *testing.T) { for _, tt := range []struct { name string diff --git a/internal/cli/review_capabilities.go b/internal/cli/review_capabilities.go index e340a19a3..d8fd4cb91 100644 --- a/internal/cli/review_capabilities.go +++ b/internal/cli/review_capabilities.go @@ -204,6 +204,7 @@ func reviewCapabilitiesStaticSurface() ReviewCapabilitiesResult { ReviewIntegrationRepairSchema, ReviewIntegrationStartSchema, ReviewIntegrationStatusSchema, + reviewVerificationEvidenceSchemaName, reviewtransaction.ReceiptSchema, reviewtransaction.CompactReceiptSchema, reviewResultArtifactSchema, diff --git a/internal/cli/review_capabilities_test.go b/internal/cli/review_capabilities_test.go index 773562c76..ace531040 100644 --- a/internal/cli/review_capabilities_test.go +++ b/internal/cli/review_capabilities_test.go @@ -138,7 +138,7 @@ func TestReviewCapabilitiesAdvertisesOnlyNativeSurface(t *testing.T) { if !slices.Equal(result.Operations, wantOperations) || !slices.Equal(result.Gates, wantGates) || !slices.Equal(result.Projections, wantProjections) { t.Fatalf("capability surface = operations %v gates %v projections %v", result.Operations, result.Gates, result.Projections) } - if !slices.Contains(result.Schemas, reviewResultArtifactSchema) || !slices.Contains(result.Schemas, ReviewIntegrationOperationSchema) || !slices.Contains(result.Schemas, ReviewIntegrationStartSchema) || !slices.Contains(result.Schemas, ReviewIntegrationStatusSchema) || !slices.Contains(result.Schemas, ReviewIntegrationProjectionSchema) || !slices.Contains(result.Schemas, ReviewIntegrationRepairSchema) || !slices.Contains(result.Schemas, reviewtransaction.AuthorityRepairAssessmentSchema) || !slices.Contains(result.Schemas, reviewtransaction.FinalVerificationIncidentSchema) { + if !slices.Contains(result.Schemas, reviewResultArtifactSchema) || !slices.Contains(result.Schemas, ReviewIntegrationOperationSchema) || !slices.Contains(result.Schemas, ReviewIntegrationStartSchema) || !slices.Contains(result.Schemas, ReviewIntegrationStatusSchema) || !slices.Contains(result.Schemas, ReviewIntegrationProjectionSchema) || !slices.Contains(result.Schemas, ReviewIntegrationRepairSchema) || !slices.Contains(result.Schemas, reviewtransaction.AuthorityRepairAssessmentSchema) || !slices.Contains(result.Schemas, reviewtransaction.FinalVerificationIncidentSchema) || !slices.Contains(result.Schemas, reviewVerificationEvidenceSchemaName) { t.Fatalf("capability schemas do not advertise the negotiated provider surface: %v", result.Schemas) } if result.Bootstrap == nil || result.Bootstrap.Command != "gentle-ai review status --cwd --contract gentle-ai.review-integration/v1 --next-transition" || @@ -515,7 +515,7 @@ func TestReviewIntegrationDocumentationMatchesRuntimeContract(t *testing.T) { document := string(payload) for _, required := range []string{ "`stop`", "`legacy_v1_read_only`", "`mutation_outcome`", "`not_started`", "`unknown`", "`committed`", - "twenty strict JSON Schemas", "twenty-four deterministic conformance fixtures", + "twenty-one strict JSON Schemas", "twenty-five deterministic conformance fixtures", "Legacy-v1 never reports `publication_pending`", "retry and replay disabled", "Historical `ordinary_4r` legacy status omits `frozen`", "START, finalize, BIND-SDD, invalidation, and direct append", "`native_frozen_candidate_context`", "`candidate_diff`", "`changed_path_manifest`", diff --git a/internal/cli/review_facade.go b/internal/cli/review_facade.go index 179e7852a..0761908f5 100644 --- a/internal/cli/review_facade.go +++ b/internal/cli/review_facade.go @@ -120,6 +120,12 @@ type ReviewFacadeReceiptPublicationError struct { Cause error `json:"-"` } +type staleCapturedResultsError struct{} + +func (*staleCapturedResultsError) Error() string { + return "captured reviewer results are accepted only while reviewing; query review.status --next-transition for the current native transition" +} + func (err *ReviewFacadeReceiptPublicationError) Error() string { return fmt.Sprintf( "write compact review receipt: %v (mutation_outcome: %s, replayability: %s, lineage: %s, request_digest: %s)", @@ -1324,12 +1330,15 @@ func runReviewFacadeFinalize(ctx context.Context, args []string, stdout io.Write if terminalAtEntry && !facadeFinalizeReplayInputsEmpty(resultPaths, resultArtifacts, resultArtifactFiles, *capturedResults, *capturedEvidence, *validationPath, *refuterPath, *evidencePath, *correctionLines, *failed, *tracePath) { return errors.New("terminal review finalize accepts no review inputs; exact replay requires only --lineage") } - if state.State != reviewtransaction.StateReviewing && (len(resultArtifacts) != 0 || len(resultArtifactFiles) != 0 || len(resultPaths) != 0) { + if state.State != reviewtransaction.StateReviewing && (len(resultArtifacts) != 0 || len(resultArtifactFiles) != 0 || len(resultPaths) != 0 || *capturedResults) { pending, pendingErr := store.PendingFinalizeAttempt() if pendingErr != nil { return pendingErr } if terminalAtEntry || pending == nil { + if *capturedResults { + return reviewPreflightError(&staleCapturedResultsError{}) + } return reviewPreflightError(errors.New("reviewer results are accepted only while the authority is reviewing")) } } @@ -1410,6 +1419,7 @@ func runReviewFacadeFinalize(ctx context.Context, args []string, stdout io.Write } } var evidence []byte + verificationFailed := *failed if strings.TrimSpace(*evidencePath) != "" { evidence, err = readFacadeBytes(*evidencePath) if err != nil { @@ -1422,6 +1432,21 @@ func runReviewFacadeFinalize(ctx context.Context, args []string, stdout io.Write return reviewPreflightError(err) } } + if state.State == reviewtransaction.StateValidating && len(evidence) == 0 { + var found bool + evidence, found, err = discoverCapturedFinalEvidence(store.Dir) + if err != nil { + return reviewPreflightError(err) + } + if !found { + evidence = nil + } + } + if len(evidence) > 0 { + if captured, _, parseErr := parseReviewVerificationEvidence(evidence); parseErr == nil { + verificationFailed = captured.Outcome == reviewVerificationFailed + } + } if terminalComplete { if err := reviewFacadeSyncDirectory(filepath.Dir(store.FinalizeAttemptJournalPath())); err != nil { return fmt.Errorf("sync completed finalize journal directory: %w", err) @@ -1444,7 +1469,7 @@ func runReviewFacadeFinalize(ctx context.Context, args []string, stdout io.Write return reviewPreflightError(err) } } - replayRequest := facadeFinalizeAttemptRequestForCandidate(record, state.CurrentSnapshot, reviewerResults, validation, refuter, replayEvidence, *correctionLines, *failed) + replayRequest := facadeFinalizeAttemptRequestForCandidate(record, state.CurrentSnapshot, reviewerResults, validation, refuter, replayEvidence, *correctionLines, verificationFailed) attempt, attemptLoaded, err = store.ReconcileFinalizeAttempt(ctx, replayRequest) if err != nil { return err @@ -1463,7 +1488,7 @@ func runReviewFacadeFinalize(ctx context.Context, args []string, stdout io.Write return reviewPreflightError(fmt.Errorf("validate FINALIZE current snapshot: %v", err)) } } - plan, err := prepareFacadeFinalizePlan(ctx, root, record.Revision, state, reviewerResults, refuter, validation, evidence, *correctionLines, *failed) + plan, err := prepareFacadeFinalizePlan(ctx, root, record.Revision, state, reviewerResults, refuter, validation, evidence, *correctionLines, verificationFailed) if err != nil { return reviewPreflightError(err) } @@ -1477,7 +1502,7 @@ func runReviewFacadeFinalize(ctx context.Context, args []string, stdout io.Write if !terminalAtEntry && pendingAtEntry == nil && len(plan.Transitions) == 0 { return encodeCompactFacadeFinalize(stdout, negotiated, *actionEligibility, *nextTransition, state, record.Revision, store, "continue the current review state", reviewFinalizeOutputContext{Context: ctx, Repo: root}) } - request := facadeFinalizeAttemptRequestForCandidate(record, plan.Candidate, reviewerResults, validation, refuter, plan.Evidence, *correctionLines, *failed) + request := facadeFinalizeAttemptRequestForCandidate(record, plan.Candidate, reviewerResults, validation, refuter, plan.Evidence, *correctionLines, verificationFailed) if !terminalAtEntry && pendingAtEntry != nil && !attemptLoaded { attempt, attemptLoaded, err = store.ReconcileFinalizeAttempt(ctx, request) if err != nil { @@ -2210,6 +2235,20 @@ func (result facadeReviewerResult) nativeLensResult() reviewtransaction.LensResu return reviewtransaction.LensResult{Lens: result.Lens, Findings: findings, Evidence: result.Evidence} } +func (result facadeReviewerResult) withCanonicalLensResult(canonical reviewtransaction.LensResult) facadeReviewerResult { + result.Lens = canonical.Lens + result.Findings = make([]facadeFinding, len(canonical.Findings)) + for index, finding := range canonical.Findings { + result.Findings[index] = facadeFinding{ + ID: finding.ID, Lens: finding.Lens, Location: finding.Location, Severity: finding.Severity, + Claim: finding.Claim, ProofRefs: append([]string(nil), finding.ProofRefs...), + EvidenceClass: finding.EvidenceClass, CausalDisposition: finding.CausalDisposition, + } + } + result.Evidence = append([]string(nil), canonical.Evidence...) + return result +} + func (result facadeValidationResult) native(tx reviewtransaction.Transaction) (reviewtransaction.ScopedValidationResult, error) { if len(result.OriginalCriteria.Evidence) == 0 || len(result.CorrectionRegression.Evidence) == 0 { return reviewtransaction.ScopedValidationResult{}, errors.New("targeted validation requires original_criteria and correction_regression evidence") diff --git a/internal/cli/review_next_transition.go b/internal/cli/review_next_transition.go index 1ae6c50a5..3190b22fd 100644 --- a/internal/cli/review_next_transition.go +++ b/internal/cli/review_next_transition.go @@ -1,6 +1,8 @@ package cli import ( + "encoding/json" + "errors" "fmt" "strings" @@ -41,6 +43,7 @@ type ReviewTransitionInput struct { Schema string `json:"schema"` CaptureOperation string `json:"capture_operation"` Arguments []ReviewTransitionArgument `json:"arguments"` + ReviewerTaskBinding string `json:"reviewer_task_binding,omitempty"` ArtifactSubject *reviewtransaction.ArtifactSubject `json:"artifact_subject,omitempty"` CandidateDiff *reviewtransaction.FrozenCandidateDiff `json:"candidate_diff,omitempty"` ChangedPathManifest *[]reviewtransaction.ChangedPathManifestEntry `json:"changed_path_manifest,omitempty"` @@ -64,6 +67,34 @@ type ReviewTransitionBinding struct { RepositoryContext string `json:"repository_context,omitempty"` } +const reviewTaskBindingPrefix = "GENTLE_AI_REVIEW_BINDING " + +type reviewTaskBinding struct { + Lineage string `json:"lineage"` + Target string `json:"target"` + Lens string `json:"lens"` + Order int `json:"order"` + Revision string `json:"revision"` + RepositoryContext string `json:"repository_context"` + SubjectHash string `json:"subject_hash"` +} + +func renderReviewTaskBinding(binding ReviewTransitionBinding, subject reviewtransaction.ArtifactSubject) (string, error) { + if reviewtransaction.ValidateArtifactSubject(subject) != nil || binding.LineageID != subject.LineageID || + binding.Revision != subject.AuthorityRevision || binding.TargetIdentity != subject.TargetIdentity || + reviewtransaction.ValidateReviewRepositoryContextHandle(binding.RepositoryContext) != nil { + return "", errors.New("review task binding does not match provider-owned transition context") + } + payload, err := json.Marshal(reviewTaskBinding{ + Lineage: binding.LineageID, Target: binding.TargetIdentity, Lens: subject.Lens, Order: subject.SelectedOrder, + Revision: binding.Revision, RepositoryContext: binding.RepositoryContext, SubjectHash: subject.SubjectHash, + }) + if err != nil { + return "", fmt.Errorf("encode provider-owned review task binding: %w", err) + } + return reviewTaskBindingPrefix + string(payload), nil +} + // ReviewTransitionArtifact deliberately excludes the provider-owned path. The // native finalize command discovers the immutable captured bytes itself. type ReviewTransitionArtifact struct { @@ -160,7 +191,7 @@ func newReviewNextTransition(status ReviewTargetStatusResult, selectedLenses []s return reviewExecuteTransition("native_low_risk_verification", "review.finalize", []ReviewTransitionArgument{{Name: "lineage", Value: binding.LineageID}}, []ReviewTransitionArgument{{Name: "state", Value: "validating"}, {Name: "risk_level", Value: "low"}}, binding, nil) } return reviewCollectTransition("verification_evidence_required", ReviewTransitionInput{ - Name: "evidence", Schema: "gentle-ai.review-verification-evidence/v1", CaptureOperation: "review.capture-evidence", + Name: "evidence", Schema: reviewVerificationEvidenceSchemaName, CaptureOperation: "review.capture-evidence", Arguments: reviewBindingArguments(binding), }) case reviewtransaction.StateInvalidated: @@ -269,6 +300,10 @@ func reviewCaptureInput(binding ReviewTransitionBinding, lens string, order int, if manifest == nil { manifest = []reviewtransaction.ChangedPathManifestEntry{} } + taskBinding, err := renderReviewTaskBinding(binding, subject) + if err == nil { + input.ReviewerTaskBinding = taskBinding + } input.ArtifactSubject, input.CandidateDiff, input.ChangedPathManifest = &subject, &diff, &manifest } return input diff --git a/internal/cli/review_next_transition_test.go b/internal/cli/review_next_transition_test.go index 24f3bc0c2..b448b4902 100644 --- a/internal/cli/review_next_transition_test.go +++ b/internal/cli/review_next_transition_test.go @@ -15,21 +15,32 @@ import ( "github.com/gentleman-programming/gentle-ai/internal/reviewtransaction" ) -func TestValidatingEvidenceCollectionUnblocksFinalizeAndPreCommit(t *testing.T) { - repo, started, _, record, _ := capturedArtifact(t) +func TestReviewEvidenceRuntimeHarnessReachesTerminalReceipt(t *testing.T) { + repo, started, store, record, _ := capturedArtifact(t) finalize := []string{"--contract", ReviewIntegrationContractV1, "--next-transition", "--cwd", repo, "--lineage", started.LineageID, "--captured-results"} var first bytes.Buffer if err := RunReviewFacadeFinalize(finalize, &first); err != nil { t.Fatal(err) } - var repeated bytes.Buffer - if err := RunReviewFacadeFinalize(finalize, &repeated); err != nil { + before, err := store.Load() + if err != nil { + t.Fatal(err) + } + var stale bytes.Buffer + err = RunReview(append([]string{"finalize"}, finalize...), &stale) + if err == nil { + t.Fatal("stale --captured-results finalize succeeded") + } + failure := decodeReviewIntegrationFailure(t, stale.Bytes()) + if failure.Code != "invalid_request" || failure.MutationOutcome != ReviewMutationNotStarted || failure.NextAction != "review.status" || failure.LineageID != started.LineageID { + t.Fatalf("stale captured-results failure = %#v", failure) + } + after, err := store.Load() + if err != nil { t.Fatal(err) } - var repeatedResult ReviewIntegrationFinalizeResult - decodeStrictReviewJSON(t, decodeReviewOperationEnvelope(t, repeated.Bytes()).Result, &repeatedResult) - if repeatedResult.State != reviewtransaction.StateValidating || repeatedResult.NextTransition == nil || repeatedResult.NextTransition.Kind != reviewNextTransitionCollect || repeatedResult.NextTransition.ReasonCode != "verification_evidence_required" { - t.Fatalf("repeated finalize made no-progress recommendation = %#v", repeatedResult) + if !reflect.DeepEqual(before, after) { + t.Fatalf("stale captured-results mutated authority:\nbefore=%#v\nafter=%#v", before, after) } statusArgs := []string{"status", "--contract", ReviewIntegrationContractV1, "--next-transition", "--cwd", repo, "--lineage", started.LineageID} @@ -39,16 +50,31 @@ func TestValidatingEvidenceCollectionUnblocksFinalizeAndPreCommit(t *testing.T) } var status ReviewTargetStatusResult decodeStrictReviewJSON(t, waiting.Bytes(), &status) - if status.NextTransition == nil || status.NextTransition.Kind != reviewNextTransitionCollect || status.NextTransition.Collect == nil || len(status.NextTransition.Collect.Inputs) != 1 || status.NextTransition.Collect.Inputs[0].CaptureOperation != "review.capture-evidence" { + if status.NextTransition == nil || status.NextTransition.Kind != reviewNextTransitionCollect || status.NextTransition.ReasonCode != "verification_evidence_required" || status.NextTransition.Collect == nil || len(status.NextTransition.Collect.Inputs) != 1 { t.Fatalf("validating status = %#v", status.NextTransition) } + input := status.NextTransition.Collect.Inputs[0] + if input.Name != "evidence" || input.Schema != reviewVerificationEvidenceSchemaName || input.CaptureOperation != "review.capture-evidence" || + !reflect.DeepEqual(input.Arguments, []ReviewTransitionArgument{{Name: "lineage", Value: started.LineageID}, {Name: "expected-revision", Value: status.Authority.Revision}, {Name: "target", Value: reviewAuthorityTargetIdentity(status)}}) { + t.Fatalf("verification evidence input = %#v", input) + } evidence := filepath.Join(t.TempDir(), "evidence.txt") - if err := os.WriteFile(evidence, []byte("verification passed\n"), 0o600); err != nil { + writeReviewCLIJSON(t, evidence, reviewVerificationEvidence{ + Schema: reviewVerificationEvidenceSchemaName, + Outcome: reviewVerificationPassed, + Checks: []reviewVerificationCheck{{Name: "focused tests", Status: reviewVerificationPassed, Command: "go test ./internal/cli", Evidence: []string{"ok github.com/gentleman-programming/gentle-ai/internal/cli"}}}, + }) + captureArgs := []string{"capture-evidence", "--cwd", repo, "--lineage", started.LineageID, "--target", record.State.InitialSnapshot.Identity, "--expected-revision", status.Authority.Revision, "--input", evidence} + var captured, recaptured bytes.Buffer + if err := RunReview(captureArgs, &captured); err != nil { t.Fatal(err) } - if err := RunReview([]string{"capture-evidence", "--cwd", repo, "--lineage", started.LineageID, "--target", record.State.InitialSnapshot.Identity, "--expected-revision", status.Authority.Revision, "--input", evidence}, &bytes.Buffer{}); err != nil { + if err := RunReview(captureArgs, &recaptured); err != nil { t.Fatal(err) } + if captured.String() != recaptured.String() { + t.Fatalf("idempotent evidence capture changed output:\n%s\n%s", captured.String(), recaptured.String()) + } var ready bytes.Buffer if err := RunReview(statusArgs, &ready); err != nil { t.Fatal(err) @@ -58,7 +84,7 @@ func TestValidatingEvidenceCollectionUnblocksFinalizeAndPreCommit(t *testing.T) t.Fatalf("evidence-ready status = %#v", status.NextTransition) } var terminal bytes.Buffer - if err := RunReviewFacadeFinalize([]string{"--contract", ReviewIntegrationContractV1, "--next-transition", "--cwd", repo, "--lineage", started.LineageID, "--captured-evidence"}, &terminal); err != nil { + if err := RunReviewFacadeFinalize([]string{"--contract", ReviewIntegrationContractV1, "--next-transition", "--cwd", repo, "--lineage", started.LineageID}, &terminal); err != nil { t.Fatal(err) } var finalized ReviewIntegrationFinalizeResult @@ -66,6 +92,26 @@ func TestValidatingEvidenceCollectionUnblocksFinalizeAndPreCommit(t *testing.T) if finalized.State != reviewtransaction.StateApproved { t.Fatalf("captured evidence finalize state = %q, want approved", finalized.State) } + receiptPayload, err := os.ReadFile(store.ReceiptPath()) + if err != nil { + t.Fatal(err) + } + receipt, err := reviewtransaction.ParseCompactReceipt(receiptPayload) + if err != nil { + t.Fatal(err) + } + if receipt.LineageID != started.LineageID || receipt.InitialReviewTree != record.State.InitialSnapshot.CandidateTree || receipt.FinalCandidateTree != record.State.CurrentSnapshot.CandidateTree || receipt.PathsDigest != record.State.InitialSnapshot.PathsDigest { + t.Fatalf("terminal receipt lost lineage or target identity = %#v", receipt) + } + var replay bytes.Buffer + if err := RunReviewFacadeFinalize([]string{"--contract", ReviewIntegrationContractV1, "--next-transition", "--cwd", repo, "--lineage", started.LineageID}, &replay); err != nil { + t.Fatal(err) + } + var replayed ReviewIntegrationFinalizeResult + decodeStrictReviewJSON(t, decodeReviewOperationEnvelope(t, replay.Bytes()).Result, &replayed) + if replayed.State != reviewtransaction.StateApproved || replayed.StoreRevision != finalized.StoreRevision { + t.Fatalf("terminal replay = %#v, original revision %q", replayed, finalized.StoreRevision) + } runReviewCLIGit(t, repo, "add", "tracked.txt") if err := RunReview([]string{"validate", "--cwd", repo, "--lineage", started.LineageID, "--gate", string(reviewtransaction.GatePreCommit)}, &bytes.Buffer{}); err != nil { t.Fatalf("pre-commit after captured evidence: %v", err) @@ -278,7 +324,24 @@ func historicalRoutingCandidate(value int) string { } func TestNegotiatedRestartStatusSuppliesFrozenContextForEveryMissingReviewer(t *testing.T) { - repo, started, _, record := newArtifactReview(t, true) + repo := initReviewCLIRepo(t) + if err := os.WriteFile(filepath.Join(repo, "tracked.txt"), []byte("provider-bound candidate\n"), 0o644); err != nil { + t.Fatal(err) + } + started := runNegotiatedReviewStart(t, repo, "review-provider-task-binding") + store, err := reviewtransaction.CompactAuthoritativeStore(context.Background(), repo, started.LineageID) + if err != nil { + t.Fatal(err) + } + record, err := store.Load() + if err != nil { + t.Fatal(err) + } + resumed := runNegotiatedReviewStart(t, repo, started.LineageID) + if resumed.Action != string(reviewtransaction.CompactStartResumed) || + !reflect.DeepEqual(resumed.ReviewerTaskBindings, started.ReviewerTaskBindings) { + t.Fatalf("resumed START task bindings changed:\ncreated=%#v\nresumed=%#v", started.ReviewerTaskBindings, resumed.ReviewerTaskBindings) + } var output bytes.Buffer if err := RunReview([]string{ "status", "--contract", ReviewIntegrationContractV1, "--next-transition", @@ -288,6 +351,20 @@ func TestNegotiatedRestartStatusSuppliesFrozenContextForEveryMissingReviewer(t * } var status ReviewTargetStatusResult decodeStrictReviewJSON(t, output.Bytes(), &status) + legacyPayload, err := json.Marshal(status) + if err != nil { + t.Fatal(err) + } + var legacyStatus ReviewTargetStatusResult + if err := json.Unmarshal(legacyPayload, &legacyStatus); err != nil { + t.Fatal(err) + } + for index := range legacyStatus.NextTransition.Collect.Inputs { + legacyStatus.NextTransition.Collect.Inputs[index].ReviewerTaskBinding = "" + } + if err := legacyStatus.Validate(); err != nil { + t.Fatalf("Validate() rejected additive-minor STATUS without reviewer task bindings: %v", err) + } if status.NextTransition == nil || status.NextTransition.Collect == nil || len(status.NextTransition.Collect.Inputs) != len(record.State.SelectedLenses) { t.Fatalf("restart transition = %#v", status.NextTransition) @@ -297,6 +374,16 @@ func TestNegotiatedRestartStatusSuppliesFrozenContextForEveryMissingReviewer(t * t.Fatal(err) } for order, input := range status.NextTransition.Collect.Inputs { + if len(started.ReviewerTaskBindings) != len(record.State.SelectedLenses) || + input.ReviewerTaskBinding != started.ReviewerTaskBindings[order] || + !strings.HasPrefix(input.ReviewerTaskBinding, "GENTLE_AI_REVIEW_BINDING {") || + strings.Contains(input.ReviewerTaskBinding, repo) || strings.Contains(input.ReviewerTaskBinding, "tracked.txt") { + t.Fatalf("restart reviewer binding %d = %q, START bindings %#v", order, input.ReviewerTaskBinding, started.ReviewerTaskBindings) + } + var binding map[string]any + if err := json.Unmarshal([]byte(strings.TrimPrefix(input.ReviewerTaskBinding, "GENTLE_AI_REVIEW_BINDING ")), &binding); err != nil { + t.Fatalf("decode reviewer binding %d: %v", order, err) + } payload, err := json.Marshal(input) if err != nil { t.Fatal(err) @@ -327,6 +414,14 @@ func TestNegotiatedRestartStatusSuppliesFrozenContextForEveryMissingReviewer(t * subject.SelectedOrder != order || subject.CandidateDiffSHA256 != wantContext.CandidateDiff.SHA256 { t.Fatalf("restart subject %d = %#v", order, subject) } + wantBinding := map[string]any{ + "lens": subject.Lens, "lineage": subject.LineageID, "order": float64(subject.SelectedOrder), + "repository_context": started.RepositoryContext.Handle, "revision": subject.AuthorityRevision, + "subject_hash": subject.SubjectHash, "target": subject.TargetIdentity, + } + if !reflect.DeepEqual(binding, wantBinding) { + t.Fatalf("restart reviewer binding %d = %#v, want %#v", order, binding, wantBinding) + } if !reflect.DeepEqual(diff, wantContext.CandidateDiff) || !reflect.DeepEqual(manifest, wantContext.ChangedPathManifest) { t.Fatalf("restart context %d differs from frozen candidate\ngot diff=%#v manifest=%#v\nwant diff=%#v manifest=%#v", order, diff, manifest, wantContext.CandidateDiff, wantContext.ChangedPathManifest) } diff --git a/internal/cli/review_operation_contract.go b/internal/cli/review_operation_contract.go index abf5bf75c..811427563 100644 --- a/internal/cli/review_operation_contract.go +++ b/internal/cli/review_operation_contract.go @@ -47,7 +47,7 @@ type reviewIntegrationOperationMetadata struct { var reviewIntegrationOperationRegistry = []reviewIntegrationOperationMetadata{ {Command: "bind-sdd", Operation: ReviewIntegrationOperationBindSDD, Label: "Review BIND-SDD", ValueFlags: []string{"cwd", "change", "lineage", "expected-binding-revision"}, MutatesAuthority: true, JoinOnTimeout: true, TimeoutRetryable: true}, {Command: "capabilities", Operation: "review.capabilities", Label: "Review CAPABILITIES"}, - {Command: "finalize", Operation: ReviewIntegrationOperationFinalize, Label: "Review FINALIZE", ValueFlags: []string{"cwd", "lineage", "validation", "refuter", "evidence", "trace", "result"}, BoolFlags: []string{"failed"}, IntFlags: []string{"correction-lines"}, MutatesAuthority: true}, + {Command: "finalize", Operation: ReviewIntegrationOperationFinalize, Label: "Review FINALIZE", ValueFlags: []string{"cwd", "lineage", "validation", "refuter", "evidence", "trace", "result", "result-artifact", "result-artifact-file"}, BoolFlags: []string{"failed", "captured-results", "captured-evidence", "action-eligibility", "next-transition"}, IntFlags: []string{"correction-lines"}, MutatesAuthority: true}, {Command: "repair", Operation: "review.repair", Label: "Review REPAIR", ValueFlags: []string{"cwd", "class", "lineage", "expected-revision", "cause", "disposition", "repository-binding", "actor", "reason", "maintainer-authorization"}, BoolFlags: []string{"preflight"}, MutatesAuthority: true, JoinOnTimeout: true, ReadOnlyFlag: "preflight"}, {Command: "retry-final-verification", Operation: ReviewIntegrationOperationRetryFinalVerification, Label: "Review RETRY-FINAL-VERIFICATION", ValueFlags: []string{"cwd", "predecessor-lineage", "expected-predecessor-revision", "successor-lineage", "incident", "actor", "reason", "maintainer-authorization"}, MutatesAuthority: true, JoinOnTimeout: true}, {Command: "start", Operation: "review.start", Label: "Review START", ValueFlags: []string{"cwd", "target", "lineage", "policy", "focus", "base-ref", "projection", "trace"}, BoolFlags: []string{"committed-only", "workspace-overlay"}, MutatesAuthority: true}, @@ -450,6 +450,15 @@ func newReviewIntegrationFailure(operation string, args []string, runErr error) if errors.As(runErr, &preflight) { preflightFailure := newReviewIntegrationPreflightFailure(operation, "invalid_request", "The negotiated review request is invalid.") preflightFailure.LineageID = failure.LineageID + var staleResults *staleCapturedResultsError + if errors.As(runErr, &staleResults) { + preflightFailure.Message = "Captured reviewer results are valid only while reviewing; query the current native next transition." + preflightFailure.AuthorityApplicability = "current_target" + preflightFailure.RetrySafe = false + preflightFailure.Replayability = reviewtransaction.ReplayabilityNotReplayable + preflightFailure.RequiredInputs = []string{"lineage_id"} + preflightFailure.NextAction = "review.status" + } return preflightFailure } var legacy *reviewtransaction.LegacyReadOnlyError diff --git a/internal/cli/review_schema.go b/internal/cli/review_schema.go index 8e427faf3..90f7a6409 100644 --- a/internal/cli/review_schema.go +++ b/internal/cli/review_schema.go @@ -1,16 +1,41 @@ package cli import ( + "bytes" "encoding/json" "errors" "fmt" "io" + "strings" ) const reviewReviewerSchema = `{"$schema":"https://json-schema.org/draft/2020-12/schema","$id":"https://gentle-ai.dev/schema/review/reviewer/v1","title":"Gentle AI reviewer result","type":"object","additionalProperties":false,"required":["subject_hash","inspection","findings","evidence"],"properties":{"subject_hash":{"type":"string","pattern":"^sha256:[0-9a-f]{64}$"},"inspection":{"type":"object","additionalProperties":false,"required":["status","paths"],"properties":{"status":{"const":"completed"},"paths":{"type":"array","uniqueItems":true,"items":{"type":"string","minLength":1}}}},"lens":{"type":"string","enum":["risk","resilience","readability","reliability","review-risk","review-resilience","review-readability","review-reliability"]},"findings":{"type":"array","items":{"type":"object","additionalProperties":false,"required":["location","severity","claim","proof_refs"],"allOf":[{"if":{"properties":{"severity":{"enum":["BLOCKER","CRITICAL"]}},"required":["severity"]},"then":{"required":["evidence_class","causal_disposition"]}}],"properties":{"id":{"type":"string","pattern":"^R[1-4]-[A-Za-z0-9][A-Za-z0-9._-]*$"},"lens":{"type":"string","enum":["risk","resilience","readability","reliability"]},"location":{"type":"string","minLength":1},"severity":{"type":"string","enum":["BLOCKER","CRITICAL","WARNING","SUGGESTION"]},"claim":{"type":"string","minLength":1},"proof_refs":{"type":"array","minItems":1,"items":{"type":"string","pattern":"\\S","not":{"pattern":"^\\s*(?:[nN]/[aA]|[nN][aA]|[nN][oO][nN][eE]|[tT][oO][dD][oO]|[tT][bB][dD]|[pP][aA][sS][sS]|[pP][aA][sS][sS][eE][dD]|[sS][uU][cC][cC][eE][sS][sS]|[pP][lL][aA][cC][eE][hH][oO][lL][dD][eE][rR])\\s*$"}}},"evidence_class":{"type":"string","enum":["deterministic","inferential","insufficient"]},"causal_disposition":{"type":"string","enum":["introduced","behavior-activated","worsened","pre-existing","base-only","unknown"]}}}},"evidence":{"type":"array","minItems":1,"items":{"type":"string","pattern":"\\S","not":{"pattern":"^\\s*(?:[nN]/[aA]|[nN][aA]|[nN][oO][nN][eE]|[tT][oO][dD][oO]|[tT][bB][dD]|[pP][aA][sS][sS]|[pP][aA][sS][sS][eE][dD]|[sS][uU][cC][cC][eE][sS][sS]|[pP][lL][aA][cC][eE][hH][oO][lL][dD][eE][rR])\\s*$"}}}},"examples":[{"subject_hash":"sha256:0000000000000000000000000000000000000000000000000000000000000000","inspection":{"status":"completed","paths":["internal/example.go"]},"findings":[],"evidence":["reviewed the complete candidate scope"]}]}` +const ( + reviewVerificationEvidenceSchemaName = "gentle-ai.review-verification-evidence/v1" + reviewVerificationEvidenceSchemaID = "https://gentle-ai.dev/contracts/review-integration/v1/schemas/verification-evidence.schema.json" + reviewVerificationPassed = "passed" + reviewVerificationFailed = "failed" +) + +const reviewVerificationEvidenceSchema = `{"$schema":"https://json-schema.org/draft/2020-12/schema","$id":"https://gentle-ai.dev/contracts/review-integration/v1/schemas/verification-evidence.schema.json","title":"Gentle AI final verification evidence","type":"object","additionalProperties":false,"required":["schema","outcome","checks"],"properties":{"schema":{"const":"gentle-ai.review-verification-evidence/v1"},"outcome":{"enum":["passed","failed"]},"checks":{"type":"array","minItems":1,"items":{"type":"object","additionalProperties":false,"required":["name","status","evidence"],"properties":{"name":{"type":"string","pattern":"\\S"},"status":{"enum":["passed","failed"]},"command":{"type":"string","pattern":"\\S"},"evidence":{"type":"array","minItems":1,"items":{"type":"string","pattern":"\\S"}}}}}},"examples":[{"schema":"gentle-ai.review-verification-evidence/v1","outcome":"passed","checks":[{"name":"focused tests","status":"passed","command":"go test ./internal/cli","evidence":["ok github.com/gentleman-programming/gentle-ai/internal/cli"]}]}]}` + +type reviewVerificationEvidence struct { + Schema string `json:"schema"` + Outcome string `json:"outcome"` + Checks []reviewVerificationCheck `json:"checks"` +} + +type reviewVerificationCheck struct { + Name string `json:"name"` + Status string `json:"status"` + Command string `json:"command,omitempty"` + Evidence []string `json:"evidence"` +} + var reviewInputSchemas = map[string]json.RawMessage{ "reviewer": json.RawMessage(reviewReviewerSchema), + "verification-evidence": json.RawMessage(reviewVerificationEvidenceSchema), "final-verification-incident": json.RawMessage(`{"$schema":"https://json-schema.org/draft/2020-12/schema","$id":"https://gentle-ai.dev/contracts/review-integration/v1/schemas/final-verification-incident.schema.json","title":"Gentle AI final-verification tooling incident","type":"object","additionalProperties":false,"required":["schema","class","lineage_id","terminal_revision","validating_revision","target_identity","failed_evidence_hash","finalize_request_digest"],"properties":{"schema":{"const":"gentle-ai.review-final-verification-incident/v1"},"class":{"const":"procedural_tooling_failure"},"lineage_id":{"type":"string","maxLength":128,"pattern":"^[a-z0-9]+(?:-[a-z0-9]+)*$"},"terminal_revision":{"$ref":"#/$defs/sha256"},"validating_revision":{"$ref":"#/$defs/sha256"},"target_identity":{"$ref":"#/$defs/sha256"},"failed_evidence_hash":{"$ref":"#/$defs/sha256"},"finalize_request_digest":{"$ref":"#/$defs/sha256"}},"$defs":{"sha256":{"type":"string","pattern":"^sha256:[0-9a-f]{64}$"}}}`), "refuter": json.RawMessage(`{"$schema":"https://json-schema.org/draft/2020-12/schema","$id":"https://gentle-ai.dev/schema/review/refuter/v1","title":"Gentle AI refuter result","type":"object","additionalProperties":false,"required":["results"],"properties":{"results":{"type":"array","items":{"type":"object","additionalProperties":false,"required":["finding_id","outcome","proof_refs"],"properties":{"finding_id":{"type":"string"},"outcome":{"type":"string","enum":["corroborated","refuted","inconclusive"]},"proof_refs":{"type":"array","minItems":1,"items":{"type":"string","pattern":"\\S"}}}}}},"examples":[{"results":[]}]}`), "validator": json.RawMessage(`{"$schema":"https://json-schema.org/draft/2020-12/schema","$id":"https://gentle-ai.dev/schema/review/validator/v1","title":"Gentle AI targeted validator result","type":"object","additionalProperties":false,"required":["original_criteria","correction_regression","follow_ups"],"properties":{"original_criteria":{"$ref":"#/$defs/check"},"correction_regression":{"$ref":"#/$defs/check"},"follow_ups":{"type":"array","items":{"type":"object","additionalProperties":false,"required":["observation","proof_refs"],"properties":{"observation":{"type":"string"},"proof_refs":{"type":"array","minItems":1,"items":{"type":"string","pattern":"\\S"}}}}}},"$defs":{"check":{"type":"object","additionalProperties":false,"required":["passed","evidence"],"properties":{"passed":{"type":"boolean"},"evidence":{"type":"array","minItems":1,"items":{"type":"string"}}}}},"examples":[{"original_criteria":{"passed":true,"evidence":["acceptance test passed"]},"correction_regression":{"passed":true,"evidence":["regression test passed"]},"follow_ups":[]}]}`), @@ -18,7 +43,7 @@ var reviewInputSchemas = map[string]json.RawMessage{ func RunReviewSchema(args []string, stdout io.Writer) error { if len(args) != 1 { - return errors.New("review schema requires exactly one of reviewer, refuter, validator, or final-verification-incident") + return errors.New("review schema requires exactly one of reviewer, refuter, validator, verification-evidence, or final-verification-incident") } document, ok := reviewInputSchemas[args[0]] if !ok { @@ -30,3 +55,42 @@ func RunReviewSchema(args []string, stdout io.Writer) error { } return encodeReviewJSON(stdout, value) } + +func parseReviewVerificationEvidence(payload []byte) (reviewVerificationEvidence, []byte, error) { + var evidence reviewVerificationEvidence + decoder := json.NewDecoder(bytes.NewReader(payload)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&evidence); err != nil { + return evidence, nil, fmt.Errorf("decode final verification evidence: %w", err) + } + if err := decoder.Decode(&struct{}{}); err != io.EOF { + return evidence, nil, errors.New("decode final verification evidence: trailing data") + } + if evidence.Schema != reviewVerificationEvidenceSchemaName || (evidence.Outcome != reviewVerificationPassed && evidence.Outcome != reviewVerificationFailed) || len(evidence.Checks) == 0 { + return evidence, nil, errors.New("invalid final verification evidence identity, outcome, or checks") + } + failed := false + for index, check := range evidence.Checks { + if strings.TrimSpace(check.Name) == "" || strings.TrimSpace(check.Name) != check.Name || + (check.Status != reviewVerificationPassed && check.Status != reviewVerificationFailed) || len(check.Evidence) == 0 { + return evidence, nil, fmt.Errorf("invalid final verification check %d", index+1) + } + if check.Command != "" && (strings.TrimSpace(check.Command) == "" || strings.TrimSpace(check.Command) != check.Command) { + return evidence, nil, fmt.Errorf("invalid final verification check %d command", index+1) + } + for _, proof := range check.Evidence { + if strings.TrimSpace(proof) == "" { + return evidence, nil, fmt.Errorf("invalid final verification check %d evidence", index+1) + } + } + failed = failed || check.Status == reviewVerificationFailed + } + if (evidence.Outcome == reviewVerificationFailed) != failed { + return evidence, nil, errors.New("final verification outcome does not match check statuses") + } + canonical, err := json.Marshal(evidence) + if err != nil { + return evidence, nil, err + } + return evidence, append(canonical, '\n'), nil +} diff --git a/internal/cli/review_schema_test.go b/internal/cli/review_schema_test.go index 9cbaae84b..a173be0b9 100644 --- a/internal/cli/review_schema_test.go +++ b/internal/cli/review_schema_test.go @@ -76,6 +76,96 @@ func TestReviewerSchemaMatchesProviderAdmissionEnvelope(t *testing.T) { } } +func TestVerificationEvidenceSchemaIsPublicAndComplete(t *testing.T) { + var output bytes.Buffer + if err := RunReviewSchema([]string{"verification-evidence"}, &output); err != nil { + t.Fatal(err) + } + var schema map[string]any + if err := json.Unmarshal(output.Bytes(), &schema); err != nil { + t.Fatal(err) + } + if schema["additionalProperties"] != false || schema["$id"] != reviewVerificationEvidenceSchemaID { + t.Fatalf("verification evidence schema header = %#v", schema) + } + for _, field := range []string{"schema", "outcome", "checks"} { + if !containsString(schemaStringArray(t, schema["required"]), field) { + t.Fatalf("verification evidence required fields = %#v, missing %q", schema["required"], field) + } + } + properties := schema["properties"].(map[string]any) + if properties["schema"].(map[string]any)["const"] != reviewVerificationEvidenceSchemaName { + t.Fatalf("verification evidence identity = %#v", properties["schema"]) + } + check := properties["checks"].(map[string]any)["items"].(map[string]any) + if check["additionalProperties"] != false { + t.Fatalf("verification check schema is not closed = %#v", check) + } + for _, field := range []string{"name", "status", "evidence"} { + if !containsString(schemaStringArray(t, check["required"]), field) { + t.Fatalf("verification check required fields = %#v, missing %q", check["required"], field) + } + } + contractPayload, err := os.ReadFile(filepath.Join("..", "..", "contracts", "review-integration", "v1", "schemas", "verification-evidence.schema.json")) + if err != nil { + t.Fatal(err) + } + var contract map[string]any + if err := json.Unmarshal(contractPayload, &contract); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(schema, contract) { + t.Fatal("runtime and packaged verification evidence schemas differ") + } + fixture, err := os.ReadFile(filepath.Join("..", "..", "contracts", "review-integration", "v1", "fixtures", "verification-evidence.fixture.json")) + if err != nil { + t.Fatal(err) + } + evidence, canonical, err := parseReviewVerificationEvidence(fixture) + if err != nil { + t.Fatal(err) + } + if evidence.Outcome != reviewVerificationPassed || len(evidence.Checks) != 1 || len(canonical) == 0 || canonical[len(canonical)-1] != '\n' { + t.Fatalf("verification evidence fixture = %#v, canonical=%q", evidence, canonical) + } +} + +func TestVerificationEvidencePayloadValidation(t *testing.T) { + for _, test := range []struct { + name string + payload string + }{ + {name: "unknown field", payload: `{"schema":"gentle-ai.review-verification-evidence/v1","outcome":"passed","checks":[{"name":"tests","status":"passed","evidence":["ok"],"hash":"invented"}]}`}, + {name: "outcome mismatch", payload: `{"schema":"gentle-ai.review-verification-evidence/v1","outcome":"passed","checks":[{"name":"tests","status":"failed","evidence":["exit 1"]}]}`}, + {name: "empty evidence", payload: `{"schema":"gentle-ai.review-verification-evidence/v1","outcome":"failed","checks":[{"name":"tests","status":"failed","evidence":[]}]}`}, + } { + t.Run(test.name, func(t *testing.T) { + if _, _, err := parseReviewVerificationEvidence([]byte(test.payload)); err == nil { + t.Fatal("invalid verification evidence accepted") + } + }) + } +} + +func TestVerificationEvidencePayloadDetectionPreservesOpaqueCompatibility(t *testing.T) { + for _, test := range []struct { + name string + payload string + want bool + }{ + {name: "public contract", payload: `{"schema":"gentle-ai.review-verification-evidence/v1","outcome":"passed","checks":[]}`, want: true}, + {name: "legacy text", payload: "verification passed\n"}, + {name: "legacy JSON", payload: `{"tool":"legacy","result":"passed"}`}, + {name: "malformed legacy bytes", payload: `{verification passed`}, + } { + t.Run(test.name, func(t *testing.T) { + if got := reviewVerificationEvidencePayload([]byte(test.payload)); got != test.want { + t.Fatalf("public evidence detection = %t, want %t", got, test.want) + } + }) + } +} + func containsString(values []string, want string) bool { for _, value := range values { if value == want { diff --git a/internal/cli/review_start_context_test.go b/internal/cli/review_start_context_test.go index 14b67c271..777b1160f 100644 --- a/internal/cli/review_start_context_test.go +++ b/internal/cli/review_start_context_test.go @@ -117,12 +117,22 @@ func TestNegotiatedReviewStartContextValidationDistinguishesMissingAndEmpty(t *t writeReviewStartCandidate(t, repo, "tracked.txt", "candidate\n", 0o644) writeReviewStartCandidate(t, repo, "z.txt", "second candidate\n", 0o644) valid := runNegotiatedReviewStart(t, repo, "review-start-context-validation") + legacy := valid + legacy.ReviewerTaskBindings = nil + if err := legacy.Validate(); err != nil { + t.Fatalf("Validate() rejected additive-minor START without reviewer task bindings: %v", err) + } for _, test := range []struct { name string mutate func(*ReviewIntegrationStartResult) }{ {name: "missing artifact subjects", mutate: func(result *ReviewIntegrationStartResult) { result.ArtifactSubjects = nil }}, + {name: "reviewer task binding mismatch", mutate: func(result *ReviewIntegrationStartResult) { + bindings := append([]string(nil), result.ReviewerTaskBindings...) + bindings[0] = strings.Replace(bindings[0], `"order":0`, `"order":1`, 1) + result.ReviewerTaskBindings = bindings + }}, {name: "artifact subject mismatch", mutate: func(result *ReviewIntegrationStartResult) { subjects := append([]reviewtransaction.ArtifactSubject(nil), result.ArtifactSubjects...) subjects[0].SubjectHash = "sha256:" + strings.Repeat("0", 64) @@ -197,6 +207,7 @@ func TestNegotiatedReviewStartContextValidationDistinguishesMissingAndEmpty(t *t valid.RiskLevel = reviewtransaction.RiskLow valid.SelectedLenses = []string{} valid.ArtifactSubjects = []reviewtransaction.ArtifactSubject{} + valid.ReviewerTaskBindings = []string{} valid.ChangedFiles = 0 valid.ChangedLines = 0 valid.CorrectionBudget = 0 diff --git a/internal/cli/review_start_contract.go b/internal/cli/review_start_contract.go index f157208f7..87f1ed071 100644 --- a/internal/cli/review_start_contract.go +++ b/internal/cli/review_start_contract.go @@ -17,28 +17,29 @@ const ReviewIntegrationStartSchemaID = "https://gentle-ai.dev/contracts/review-i // ReviewIntegrationStartResult is the explicitly negotiated START response. // The legacy ReviewFacadeStartResult remains byte- and schema-compatible. type ReviewIntegrationStartResult struct { - Schema string `json:"schema"` - Contract string `json:"contract"` - Operation string `json:"operation"` - Action string `json:"action"` - LensesRequired bool `json:"lenses_required"` - LineageID string `json:"lineage_id"` - State reviewtransaction.State `json:"state"` - RiskLevel reviewtransaction.RiskLevel `json:"risk_level"` - SelectedLenses []string `json:"selected_lenses"` - Projection reviewtransaction.Projection `json:"projection"` - TargetMode reviewtransaction.TargetKind `json:"target_mode,omitempty"` - TargetIdentity string `json:"target_identity,omitempty"` - BaseTree string `json:"base_tree,omitempty"` - CandidateTree string `json:"candidate_tree,omitempty"` - ChangedFiles int `json:"changed_files"` - ChangedLines int `json:"changed_lines"` - CorrectionBudget int `json:"correction_budget"` - RiskReasons []reviewtransaction.RiskReason `json:"risk_reasons"` - ArtifactSubjects []reviewtransaction.ArtifactSubject `json:"artifact_subjects"` - CandidateDiff *reviewtransaction.FrozenCandidateDiff `json:"candidate_diff,omitempty"` - ChangedPathManifest *[]reviewtransaction.ChangedPathManifestEntry `json:"changed_path_manifest,omitempty"` - RepositoryContext *ReviewRepositoryContextReference `json:"repository_context,omitempty"` + Schema string `json:"schema"` + Contract string `json:"contract"` + Operation string `json:"operation"` + Action string `json:"action"` + LensesRequired bool `json:"lenses_required"` + LineageID string `json:"lineage_id"` + State reviewtransaction.State `json:"state"` + RiskLevel reviewtransaction.RiskLevel `json:"risk_level"` + SelectedLenses []string `json:"selected_lenses"` + Projection reviewtransaction.Projection `json:"projection"` + TargetMode reviewtransaction.TargetKind `json:"target_mode,omitempty"` + TargetIdentity string `json:"target_identity,omitempty"` + BaseTree string `json:"base_tree,omitempty"` + CandidateTree string `json:"candidate_tree,omitempty"` + ChangedFiles int `json:"changed_files"` + ChangedLines int `json:"changed_lines"` + CorrectionBudget int `json:"correction_budget"` + RiskReasons []reviewtransaction.RiskReason `json:"risk_reasons"` + ArtifactSubjects []reviewtransaction.ArtifactSubject `json:"artifact_subjects"` + ReviewerTaskBindings []string `json:"reviewer_task_bindings,omitempty"` + CandidateDiff *reviewtransaction.FrozenCandidateDiff `json:"candidate_diff,omitempty"` + ChangedPathManifest *[]reviewtransaction.ChangedPathManifestEntry `json:"changed_path_manifest,omitempty"` + RepositoryContext *ReviewRepositoryContextReference `json:"repository_context,omitempty"` } // ReviewRepositoryContextReference is the path-free provider context that a @@ -64,7 +65,7 @@ func newReviewIntegrationStartResult(legacy ReviewFacadeStartResult, assessment State: legacy.State, RiskLevel: legacy.RiskLevel, SelectedLenses: append([]string{}, legacy.SelectedLenses...), Projection: legacy.Projection, ChangedFiles: legacy.ChangedFiles, ChangedLines: legacy.ChangedLines, CorrectionBudget: legacy.CorrectionBudget, RiskReasons: append([]reviewtransaction.RiskReason{}, assessment.Reasons...), - ArtifactSubjects: []reviewtransaction.ArtifactSubject{}, RepositoryContext: repositoryContext, + ArtifactSubjects: []reviewtransaction.ArtifactSubject{}, ReviewerTaskBindings: []string{}, RepositoryContext: repositoryContext, } if targetMode == reviewtransaction.TargetBaseWorkspaceOverlay { result.TargetMode = targetMode @@ -91,6 +92,7 @@ func newReviewIntegrationStartResult(legacy ReviewFacadeStartResult, assessment SelectedLenses: append([]string{}, legacy.SelectedLenses...), } result.ArtifactSubjects = make([]reviewtransaction.ArtifactSubject, len(legacy.SelectedLenses)) + result.ReviewerTaskBindings = make([]string, len(legacy.SelectedLenses)) for order, lens := range legacy.SelectedLenses { result.ArtifactSubjects[order], err = reviewtransaction.NewArtifactSubject( subjectState, repositoryContext.Revision, *frozenContext, lens, order, "", @@ -98,6 +100,13 @@ func newReviewIntegrationStartResult(legacy ReviewFacadeStartResult, assessment if err != nil { return ReviewIntegrationStartResult{}, fmt.Errorf("derive artifact subject %d: %w", order, err) } + result.ReviewerTaskBindings[order], err = renderReviewTaskBinding(ReviewTransitionBinding{ + LineageID: legacy.LineageID, Revision: repositoryContext.Revision, + TargetIdentity: repositoryContext.TargetIdentity, RepositoryContext: repositoryContext.Handle, + }, result.ArtifactSubjects[order]) + if err != nil { + return ReviewIntegrationStartResult{}, fmt.Errorf("render reviewer task binding %d: %w", order, err) + } } } } @@ -191,10 +200,10 @@ func (result ReviewIntegrationStartResult) Validate() error { return errors.New("negotiated START repository context does not match the active reviewing authority") } if needsRepositoryContext { - if len(result.ArtifactSubjects) != len(result.SelectedLenses) { + if len(result.ArtifactSubjects) != len(result.SelectedLenses) || result.ReviewerTaskBindings != nil && len(result.ReviewerTaskBindings) != len(result.SelectedLenses) { return errors.New("negotiated START requires one provider artifact subject per selected lens") } - } else if len(result.ArtifactSubjects) != 0 { + } else if len(result.ArtifactSubjects) != 0 || len(result.ReviewerTaskBindings) != 0 { return errors.New("negotiated START cannot expose artifact subjects outside an active reviewing authority") } if result.RepositoryContext != nil { @@ -232,6 +241,13 @@ func (result ReviewIntegrationStartResult) Validate() error { if digestErr != nil || subject.ChangedPathManifestSHA256 != manifestDigest { return fmt.Errorf("negotiated START artifact subject %d does not match changed-path manifest", order) } + wantBinding, bindingErr := renderReviewTaskBinding(ReviewTransitionBinding{ + LineageID: result.LineageID, Revision: result.RepositoryContext.Revision, + TargetIdentity: result.RepositoryContext.TargetIdentity, RepositoryContext: result.RepositoryContext.Handle, + }, subject) + if bindingErr != nil || result.ReviewerTaskBindings != nil && result.ReviewerTaskBindings[order] != wantBinding { + return fmt.Errorf("negotiated START reviewer task binding %d does not match frozen authority", order) + } } } return nil diff --git a/internal/cli/review_status_contract.go b/internal/cli/review_status_contract.go index 4be119fe8..95dd39010 100644 --- a/internal/cli/review_status_contract.go +++ b/internal/cli/review_status_contract.go @@ -774,7 +774,14 @@ func (transition ReviewNextTransition) Validate() error { if _, err := input.CandidateDiff.Bytes(); err != nil { return errors.New("review capture transition candidate diff is invalid") } - } else if input.ArtifactSubject != nil || input.CandidateDiff != nil || input.ChangedPathManifest != nil { + wantBinding, bindingErr := renderReviewTaskBinding(ReviewTransitionBinding{ + LineageID: arguments["lineage"], Revision: arguments["expected-revision"], TargetIdentity: arguments["target"], + RepositoryContext: arguments["repository-context"], + }, *subject) + if bindingErr != nil || input.ReviewerTaskBinding != "" && input.ReviewerTaskBinding != wantBinding { + return errors.New("review capture transition task binding is invalid") + } + } else if input.ArtifactSubject != nil || input.CandidateDiff != nil || input.ChangedPathManifest != nil || input.ReviewerTaskBinding != "" { return errors.New("non-reviewer collection transition contains frozen reviewer context") } if input.CaptureOperation == "external.run_targeted_validation" && input.ValidationRequest == nil { diff --git a/internal/components/sdd/review_ledger_contract_test.go b/internal/components/sdd/review_ledger_contract_test.go index a70a214fa..6fd011e85 100644 --- a/internal/components/sdd/review_ledger_contract_test.go +++ b/internal/components/sdd/review_ledger_contract_test.go @@ -181,8 +181,8 @@ func TestOpenCodeRenderedReviewProtocolCost(t *testing.T) { wantChars int maxCharacters int }{ - {name: "standard", agents: []string{"review-reliability"}, beforeChars: 42_301, wantChars: 7_085, maxCharacters: 7_200}, - {name: "full-4R", agents: []string{"review-risk", "review-resilience", "review-readability", "review-reliability"}, beforeChars: 106_998, wantChars: 14_078, maxCharacters: 16_000}, + {name: "standard", agents: []string{"review-reliability"}, beforeChars: 42_301, wantChars: 7_181, maxCharacters: 7_200}, + {name: "full-4R", agents: []string{"review-risk", "review-resilience", "review-readability", "review-reliability"}, beforeChars: 106_998, wantChars: 14_174, maxCharacters: 16_000}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/reviewtransaction/artifact_admission.go b/internal/reviewtransaction/artifact_admission.go index ab5a5ef0d..d0a2dc177 100644 --- a/internal/reviewtransaction/artifact_admission.go +++ b/internal/reviewtransaction/artifact_admission.go @@ -55,6 +55,7 @@ type ArtifactAdmissionRequest struct { EchoedSubjectHash string Inspection ArtifactInspection Result LensResult + CanonicalResult *CanonicalArtifactLensResult // CandidateCausalFindingIDs is the canonical set whose claimed candidate // causality the provider verified against repository-derived changed-line // evidence before admission. @@ -63,6 +64,29 @@ type ArtifactAdmissionRequest struct { CanonicalPayload []byte } +// CanonicalArtifactLensResult carries one provider-canonicalized result across +// repository causal verification and artifact admission without regenerating +// finding identities at either boundary. +type CanonicalArtifactLensResult struct { + result LensResult + valid bool +} + +func NewCanonicalArtifactLensResult(result LensResult) (CanonicalArtifactLensResult, error) { + canonical, err := CanonicalCompactLensResult(result) + if err != nil { + return CanonicalArtifactLensResult{}, err + } + if err := validateCanonicalArtifactFindingIDs(canonical); err != nil { + return CanonicalArtifactLensResult{}, err + } + return CanonicalArtifactLensResult{result: cloneArtifactLensResult(canonical), valid: true}, nil +} + +func (canonical CanonicalArtifactLensResult) Result() LensResult { + return cloneArtifactLensResult(canonical.result) +} + // ArtifactAdmissionError exposes the stable native decision without requiring // callers to parse diagnostic prose. type ArtifactAdmissionError struct { @@ -157,9 +181,17 @@ func AdmitArtifact(request ArtifactAdmissionRequest) (LensResult, ArtifactAdmiss if !equalStrings(inspectionPaths, wantPaths) { return fail(ArtifactAdmissionIncomplete, "reviewer inspection did not cover the complete frozen path manifest") } - canonical, err := CanonicalCompactLensResult(request.Result) - if err != nil { - return fail(ArtifactAdmissionIncomplete, err.Error()) + canonical := LensResult{} + if request.CanonicalResult != nil { + if !request.CanonicalResult.valid { + return fail(ArtifactAdmissionIncomplete, "canonical reviewer result is invalid") + } + canonical = request.CanonicalResult.Result() + } else { + canonical, err = CanonicalCompactLensResult(request.Result) + if err != nil { + return fail(ArtifactAdmissionIncomplete, err.Error()) + } } wantPrefix := map[string]string{LensRisk: "R1-", LensReadability: "R2-", LensReliability: "R3-", LensResilience: "R4-"}[canonical.Lens] seenFindingIDs := make(map[string]struct{}, len(canonical.Findings)) @@ -212,6 +244,34 @@ func AdmitArtifact(request ArtifactAdmissionRequest) (LensResult, ArtifactAdmiss return canonical, admission, nil } +func cloneArtifactLensResult(result LensResult) LensResult { + clone := result + clone.Findings = append([]Finding(nil), result.Findings...) + for index := range clone.Findings { + clone.Findings[index].ProofRefs = append([]string(nil), result.Findings[index].ProofRefs...) + } + clone.Evidence = append([]string(nil), result.Evidence...) + return clone +} + +func validateCanonicalArtifactFindingIDs(result LensResult) error { + wantPrefix := map[string]string{LensRisk: "R1-", LensReadability: "R2-", LensReliability: "R3-", LensResilience: "R4-"}[result.Lens] + seen := make(map[string]struct{}, len(result.Findings)) + for index, finding := range result.Findings { + if !artifactFindingID.MatchString(finding.ID) { + return fmt.Errorf("lens result finding[%d] ID does not match the native ASCII schema", index) + } + if !strings.HasPrefix(finding.ID, wantPrefix) { + return fmt.Errorf("lens result finding[%d] ID is not bound to %q", index, result.Lens) + } + if _, duplicate := seen[finding.ID]; duplicate { + return fmt.Errorf("lens result finding[%d] repeats finding ID %q", index, finding.ID) + } + seen[finding.ID] = struct{}{} + } + return nil +} + func payloadSHA256(payload []byte) string { sum := sha256.Sum256(payload) return "sha256:" + hex.EncodeToString(sum[:]) diff --git a/internal/reviewtransaction/artifact_admission_test.go b/internal/reviewtransaction/artifact_admission_test.go index a332235b4..6e2f6a4fb 100644 --- a/internal/reviewtransaction/artifact_admission_test.go +++ b/internal/reviewtransaction/artifact_admission_test.go @@ -100,6 +100,10 @@ func TestAdmitArtifactRequiresCompletedBoundInScopeInspection(t *testing.T) { {name: "non ASCII finding id", mutate: func(r *ArtifactAdmissionRequest) { r.Result.Findings[0].ID = "R3-é" }, decision: ArtifactAdmissionBindingMismatch}, + {name: "invalid canonical token", mutate: func(r *ArtifactAdmissionRequest) { + r.Result = LensResult{} + r.CanonicalResult = &CanonicalArtifactLensResult{} + }, decision: ArtifactAdmissionIncomplete}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { diff --git a/scripts/test-review-contract-package.sh b/scripts/test-review-contract-package.sh index dc44416b7..2e8335fe4 100755 --- a/scripts/test-review-contract-package.sh +++ b/scripts/test-review-contract-package.sh @@ -40,6 +40,7 @@ expected_contract = [ contract_root / "fixtures/status-v2-unrelated.fixture.json", contract_root / "fixtures/status-v2.fixture.json", contract_root / "fixtures/status.fixture.json", + contract_root / "fixtures/verification-evidence.fixture.json", contract_root / "schemas/admitted-result.schema.json", contract_root / "schemas/artifact-subject.schema.json", contract_root / "schemas/authority-repair-assessment.schema.json", @@ -60,6 +61,7 @@ expected_contract = [ contract_root / "schemas/status-v2.schema.json", contract_root / "schemas/status.schema.json", contract_root / "schemas/targeted-validation-request.schema.json", + contract_root / "schemas/verification-evidence.schema.json", ] expected_names = sorted(path.as_posix() for path in expected_contract)