diff --git a/BACKLOG.md b/BACKLOG.md index cf90cc43..48bc78e6 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -37,7 +37,7 @@ CC-001/CC-002 were consumed by PR #24 fix bundle inline, with no standalone entr | CC-515 | ✅ closed 2026-07-29 | `gate_assurance_v3` immutable subject 與 artifact／subject／policy 三軸 shared verifier；downstream scope／closure producers 分屬 CC-518／CC-517 | arch/gate | 2026-07-23 | pr:#454 | P1 | design | | CC-516 | ⏸ deferred | evidence-gated thin delivery wrapper 評估；只組合既有 primitives，不建立 workflow engine/FSM | ux/process | 2026-07-23 | — | P3 | spike | | CC-517 | 🔵 active | maintainer `/ship`：primary review、structured remediation closure 與 conditional targeted confirmation | process/gate | 2026-07-23 | — | P1 | design | -| CC-518 | 🔵 active | gate scope manifest v1：immutable subject、changed paths、paired tests、signals 與 bounded expansion | ops/gate | 2026-07-23 | — | P1 | design | +| CC-518 | ✅ closed 2026-07-29 | gate scope manifest v1：immutable subject、changed paths、paired tests、signals 與 bounded expansion | ops/gate | 2026-07-23 | pr:#455 | P1 | design | | CC-519 | 🔵 active | selected-reviewer coverage／finding contract：declared coverage、stable IDs 與 actionable fix boundary | ops/gate | 2026-07-23 | — | P1 | design | | CC-520 | 🔵 active | synthesis parity 與 remediation seed：findings union、root-cause grouping、coverage matrix 與 no-silent-drop | ops/gate | 2026-07-23 | — | P1 | design | | CC-521 | 🔵 active | test-gap matrix、protocol recovery 與 live recall evaluation 分層 | ops/test | 2026-07-23 | — | P2 | design | @@ -2021,7 +2021,9 @@ P1，排入 v0.11.0 delivery assurance correctness。 --- -## CC-518 — gate scope manifest v1 🔵 active +## CC-518 — gate scope manifest v1 ✅ 2026-07-29 + +**See**: pr:#455 **Problem**: reviewers 目前主要從 diff list 與個別 prompt 探索 scope；renamed/ untracked paths、paired tests、sensitive signals 與 bounded adjacent context 沒有一份 @@ -2080,6 +2082,13 @@ early stop，finding 也常缺少受影響 behavior、fix boundary 與 verificat 5. schema/contract deterministic tests涵蓋 sequential logical sections、parallel session evidence、blocker no-early-stop、missing checklist、invalid stable ID與 evidence-less blocker。 +6. `gate_reviewer_result_v1.verdict` 是唯一 machine verdict；Markdown heading + 只作 human presentation，重複或缺少 heading 不得讓已完成 review 變成格式失敗。 + Shell 必須從已驗證 JSON verdict 機械聚合 GO／NO-GO，並與 final result parity。 +7. Current scope manifests carry an immutable reviewer reference index with + repository path、subject/base snapshot、line count 與 content SHA-256。 + Coverage evidence refs 與 finding source 必須命中該 index；不存在、scope 外或 + 超出 snapshot line count 的引用使 protocol `INCOMPLETE`。 **Done-when**: selected reviewer outputs 都是 schema-complete、coverage-declared， 且 finding 可直接作 remediation input；不對未選 reviewer 或模型 recall 作虛假保證。 diff --git a/CHANGELOG.md b/CHANGELOG.md index 8676789e..72f46a74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,24 @@ Versions follow [Semantic Versioning](https://semver.org/). ### Added +- **Selected-reviewer coverage and finding contract (CC-519).** Every selected + reviewer now emits a scope-bound `gate_reviewer_result_v1` JSON report with + an explicit eleven-surface checklist, evidence/reasons, stable finding IDs, + hard-gate/origin classification, affected behavior, failure mode, minimum + fix boundary, and verification expectation. The JSON verdict replaces + Markdown headings as the machine source of truth, so duplicate headings no + longer abort a completed review. Legacy role values such as `pass` are mapped + to the common enum in reviewer instructions, role-specific prose is confined + to rationale/findings, and diagnostics distinguish top-level, coverage, + finding, evidence-reference, and verdict failures. Current scope manifests + include a subject/base snapshot reference index with line counts and content + digests; nonexistent, out-of-scope, or out-of-range reviewer references fail + before synthesis. Missing/malformed sections, incomplete checklists, invalid + IDs, evidence-less blockers, and aggregate verdict drift fail closed as + protocol `INCOMPLETE`. Completed reports are preserved in + `pr_gate_result_v3`; v1/v2 and pre-index v3 results remain readable under + their legacy contracts. + - **Immutable gate scope manifest (CC-518).** `pmctl gate run` now creates a content-addressed `gate_scope_manifest_v1` before reviewer dispatch, bound to the immutable gate subject and linked from `gate_assurance_v3`. It declares diff --git a/DECISIONS.md b/DECISIONS.md index 728e60b0..c1995627 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -7,6 +7,62 @@ H2 標題格式：## YYYY-MM-DD: <短描述> 與 BACKLOG closure 對應的 entry，內文首行寫：Closes: BACKLOG.md#-NNN --> +## 2026-07-29: structured-reviewer-verdicts-replace-markdown-heading-parsing + +Relates: CC-519 + +**Context**: Parallel reviewer sessions can finish their substantive work yet +cause the gate to stop before synthesis when Markdown headings are duplicated, +omitted, or reformatted. In one observed run, four reviewer artifacts repeated +the same canonical heading while their actual conclusions remained readable; +the operation correctly failed closed as protocol incomplete, but the heading +count neither represented reviewer coverage nor preserved an actionable, +machine-verifiable finding contract. Requiring another exact prose marker would +keep transport formatting coupled to review semantics. A later sequential run +exposed a second producer ambiguity: qa-tester used legacy `pass`, +architecture-reviewer added legacy `alignment`, and security/risk reviewers put +legacy narrative prose in `verdict`. The verifier correctly stopped the run as +`INCOMPLETE`, but its generic coverage/finding diagnostic hid the actual verdict +and top-level-key violations. + +**Decision**: Every selected reviewer emits one scope-bound +`gate_reviewer_result_v1` JSON object. It carries the complete declared-surface +checklist, stable actionable findings, and one closed-enum `verdict`. +`reviewer_result_v1.verdict` is the only canonical reviewer verdict; Markdown +headings are optional presentation and are ignored for machine decisions. +The gate shell validates every JSON report before synthesis, calculates +GO／NO-GO from those verdicts, preserves the original reports in +`pr_gate_result_v3`, and verifies the aggregate against the final result. +Sequential combined sessions and parallel per-reviewer sessions use the same +logical report contract; topology and independence remain separate assurance +coordinates. Current scope manifests also carry a content-addressed reviewer +reference index. Every coverage evidence path and finding source must resolve +to that declared subject/base snapshot, and a line reference cannot exceed the +indexed line count. The manifest itself is admitted by its verified artifact +digest; arbitrary or nonexistent repository paths fail before synthesis. + +**Alternatives considered**: (a) Continue requiring exactly one Markdown +heading——否決，duplicate/missing prose remains a false protocol failure after +substantive review completes. (b) Loosen the heading regex and take the first +match——否決，conflicting later text could be silently discarded. (c) Let +synthesis infer verdicts from prose——否決，synthesis is another model session +and cannot establish deterministic input parity. + +**Constraints introduced**: Missing, duplicate, malformed, scope-digest-mismatched, +or coverage-incomplete JSON reports make the operation `INCOMPLETE`, not +reviewer `NO-GO`. A blocker cannot early-stop remaining coverage cells. +Pre-existing/caution findings cannot block, and a blocking finding requires +source evidence plus fix/verification boundaries. Coverage declaration does +not guarantee semantic completeness or model recall. Legacy result v1/v2 +artifacts remain readable but cannot claim selected-reviewer protocol evidence. +Legacy role vocabulary must be mapped into the closed common verdict enum, role +narrative belongs in `rationale` or findings, and verifier diagnostics must name +the failing contract component without reclassifying it as a reviewer verdict. +Legacy scope manifests without a reference index remain readable for historical +result verification; current producers always emit the index and enforce it. + +--- + ## 2026-07-28: gate-assurance-v3-separates-validity-freshness-applicability Relates: CC-515 diff --git a/MILESTONES.md b/MILESTONES.md index be68e05f..421de194 100644 --- a/MILESTONES.md +++ b/MILESTONES.md @@ -118,7 +118,7 @@ | 票 | 摘要 | 狀態 | |----|------|------| -| CC-518 | `gate_scope_manifest_v1`：immutable subject、changed/renamed/untracked、paired tests、signals、bounded expansion/truncation | 🔵 | +| CC-518 | `gate_scope_manifest_v1`：immutable subject、changed/renamed/untracked、paired tests、signals、bounded expansion/truncation | ✅ pr:#455 | | CC-519 | selected-reviewer coverage/finding contract；sequential logical sections 與 parallel session isolation 分開 | 🔵 | | CC-520 | synthesis findings-union parity、root-cause grouping、coverage matrix、remediation seed、no silent drop | 🔵 | | CC-521 | actionable test-gap matrix + bounded protocol recovery；seeded live recall 僅作 quality evaluation | 🔵 | diff --git a/agents/architecture-reviewer.md b/agents/architecture-reviewer.md index 60a48296..fd1583df 100644 --- a/agents/architecture-reviewer.md +++ b/agents/architecture-reviewer.md @@ -6,7 +6,7 @@ tools: Read, Bash, Glob, Grep # Output brevity -Output is parsed by the main thread, not read directly by the user. No preamble, no closing summary — the structured YAML block is the complete response. English only. Each finding field (`issue`, `suggest`): one sentence max. +Output is parsed by the main thread, not read directly by the user. No preamble, no closing summary — the structured contract selected by the caller is the complete response. English only. Each finding field (`issue`, `suggest`): one sentence max. Judge whether a change *fits* the module, the layer, the system as-is. @@ -34,10 +34,26 @@ Out of scope: style (critic), security/risk (separate), tests (qa-tester), featu 1. Read brief + `git -C diff`. 2. Read what *was* the design: `ARCHITECTURE.md` if present, the changed module's directory layout, how peers handle similar concerns. 3. Use the canonical-memory provenance/context supplied by the gate brief for prior decisions that bind this change. Never infer a host-local memory path; if the brief reports unavailable or query-failed, state that limitation rather than falling back. -4. Note in `alignment` that no `conceptual_map` was provided; the review fell back to diff inspection. +4. Note in the contract `rationale` (or legacy `alignment` when no shared + contract is supplied) that no `conceptual_map` was provided and the review + fell back to diff inspection. # Output +When the caller supplies the shared `reviewer_result_v1` contract, it fully replaces the legacy format below. Emit exactly one fenced JSON object with only +the nine contract keys (`kind`, `schema_version`, `reviewer`, +`scope_manifest_sha256`, `coverage_claim`, `coverage`, `findings`, `verdict`, +`rationale`). Complete every declared coverage surface even after finding a +blocker and map structural concerns/suggested boundaries to the common +actionable finding fields. `verdict` must be exactly +`approve|advise|block-soft|block`; put alignment prose in `rationale`. +Evidence paths must come from the caller's declared reference index, with line +numbers inside the indexed snapshot. Do not +add `status`, `summary`, or `alignment` as top-level keys. Every finding ID uses +the exact `architecture-reviewer-FNNN` prefix. Do not emit separate legacy +YAML. Only when the caller does not supply that contract, use the legacy +standalone format below. + ``` status: approve | advise | block-soft summary: diff --git a/agents/critic.md b/agents/critic.md index 5cfb5a32..17e63ea9 100644 --- a/agents/critic.md +++ b/agents/critic.md @@ -6,7 +6,7 @@ tools: Read, Bash, Glob, Grep # Output brevity -Output is parsed by the main thread, not read directly by the user. No preamble, no closing summary — the structured YAML block is the complete response. English only. Each finding field (`issue`, `suggest`): one sentence max. +Output is parsed by the main thread, not read directly by the user. No preamble, no closing summary — the structured contract selected by the caller is the complete response. English only. Each finding field (`issue`, `suggest`): one sentence max. Find what's wrong, weak, or missed. Do not validate. @@ -28,6 +28,19 @@ Find what's wrong, weak, or missed. Do not validate. # Output +When the caller supplies the shared `reviewer_result_v1` contract, it fully replaces the legacy format below. Emit exactly one fenced JSON object with only +the nine contract keys (`kind`, `schema_version`, `reviewer`, +`scope_manifest_sha256`, `coverage_claim`, `coverage`, `findings`, `verdict`, +`rationale`). Complete every declared coverage surface even after finding a +blocker and map `issue` to the common actionable finding fields. `verdict` must +be exactly `approve|advise|block-soft|block`; put explanatory prose in +`rationale`. Evidence paths must come from the caller's declared reference +index, with line numbers inside the indexed snapshot. Every finding ID uses +the exact `critic-FNNN` prefix. Do not add +`status`, `summary`, `over_scope`, or `missed` as +top-level keys and do not emit separate legacy YAML. Only when the caller does +not supply that contract, use the legacy standalone format below. + ``` status: approve | advise | block-soft summary: diff --git a/agents/qa-tester.md b/agents/qa-tester.md index e01aeead..26c26a3c 100644 --- a/agents/qa-tester.md +++ b/agents/qa-tester.md @@ -6,7 +6,7 @@ tools: Read, Edit, Write, Bash, Glob, Grep # Output brevity -Output is parsed by the main thread, not read directly by the user. No preamble, no closing summary — the structured YAML block is the complete response. English only. Each finding field: one sentence max. +Output is parsed by the main thread, not read directly by the user. No preamble, no closing summary — the structured contract selected by the caller is the complete response. English only. Each finding field: one sentence max. Testing rules — categories, layer choice, anti-patterns — come from the configured QA rules directory, not your training data. @@ -62,6 +62,22 @@ Run A if no tests exist, then B. End state: runnable suite with explicit categor # Output +When the caller supplies the shared `reviewer_result_v1` contract, it fully replaces the legacy format below. Emit exactly one fenced JSON object with only +the nine contract keys (`kind`, `schema_version`, `reviewer`, +`scope_manifest_sha256`, `coverage_claim`, `coverage`, `findings`, `verdict`, +`rationale`). Complete every declared coverage surface even after finding a +blocker and map test gaps/audit findings to the common actionable finding +fields. `verdict` must be exactly `approve|advise|block-soft|block`: map legacy +`pass` to `approve` and a gate-blocking `needs-tests` outcome to `block`; put +matrix/run/audit details in `rationale` or common findings. Evidence paths must +come from the caller's declared reference index, with line numbers inside the +indexed snapshot. Never emit `pass`, +`needs-tests`, or prose as the JSON verdict. Every finding ID uses the exact +`qa-tester-FNNN` prefix. Do not add `status`, `summary`, `matrix`, `run`, or +`audit_findings` as top-level keys and do not emit separate legacy YAML. Only +when the caller does not supply that contract, use the legacy standalone +format below. + ``` status: pass | block | needs-tests summary: diff --git a/agents/risk-reviewer.md b/agents/risk-reviewer.md index 4cdfb1be..bb912810 100644 --- a/agents/risk-reviewer.md +++ b/agents/risk-reviewer.md @@ -6,7 +6,7 @@ tools: Read, Bash, Glob, Grep # Output brevity -Output is parsed by the main thread, not read directly by the user. No preamble, no closing summary — the structured YAML block is the complete response. English only. Each finding field (`risk`, `failure_mode`, `fix`): one sentence max. +Output is parsed by the main thread, not read directly by the user. No preamble, no closing summary — the structured contract selected by the caller is the complete response. English only. Each finding field (`risk`, `failure_mode`, `fix`): one sentence max. HARD GATE. Think about what happens if the change is wrong, not whether it's conceptually correct (that's critic / architecture / security territory). @@ -37,6 +37,22 @@ A `block` halts the PR until either (1) code/process is fixed (re-review) or (2) # Output +When the caller supplies the shared `reviewer_result_v1` contract, it fully replaces the legacy format below. Emit exactly one fenced JSON object with only +the nine contract keys (`kind`, `schema_version`, `reviewer`, +`scope_manifest_sha256`, `coverage_claim`, `coverage`, `findings`, `verdict`, +`rationale`). Complete every declared coverage surface even after finding a +blocker and map risk/failure mode/mitigation to the common actionable finding +fields. `verdict` must be exactly `approve|advise|block-soft|block`: map legacy +`pass` and `pass-not-applicable` to `approve`, and put reversibility/override +prose in `rationale` or the common finding fields. Evidence paths must come +from the caller's declared reference index, with line numbers inside the +indexed snapshot. Never emit `pass`, +`pass-not-applicable`, or prose as the JSON verdict. Do not add `status`, +`summary`, `reversibility`, or `override_path` as top-level keys. Every finding +ID uses the exact `risk-reviewer-FNNN` prefix, never `risk-FNNN`. Do not emit +separate legacy YAML. Only when the caller does not supply that contract, use +the legacy standalone format below. + ``` status: pass | block | pass-not-applicable summary: diff --git a/agents/security-reviewer.md b/agents/security-reviewer.md index ca41dbef..93f422ac 100644 --- a/agents/security-reviewer.md +++ b/agents/security-reviewer.md @@ -6,7 +6,7 @@ tools: Read, Bash, Glob, Grep # Output brevity -Output is parsed by the main thread, not read directly by the user. No preamble, no closing summary — the structured YAML block is the complete response. English only. Each finding field (`issue`, `impact`, `fix`): one sentence max. +Output is parsed by the main thread, not read directly by the user. No preamble, no closing summary — the structured contract selected by the caller is the complete response. English only. Each finding field (`issue`, `impact`, `fix`): one sentence max. HARD GATE. A `block` halts the PR until either (1) code is fixed (re-review) or (2) the user explicitly overrides with recorded justification. @@ -37,6 +37,21 @@ HARD GATE. A `block` halts the PR until either (1) code is fixed (re-review) or # Output +When the caller supplies the shared `reviewer_result_v1` contract, it fully replaces the legacy format below. Emit exactly one fenced JSON object with only +the nine contract keys (`kind`, `schema_version`, `reviewer`, +`scope_manifest_sha256`, `coverage_claim`, `coverage`, `findings`, `verdict`, +`rationale`). Complete every declared coverage surface even after finding a +blocker and map security impact/remediation to the common actionable finding +fields. `verdict` must be exactly `approve|advise|block-soft|block`: map legacy +`pass` and `pass-not-applicable` to `approve`, and put explanatory prose in +`rationale`. Evidence paths must come from the caller's declared reference +index, with line numbers inside the indexed snapshot. Never emit `pass`, +`pass-not-applicable`, or prose as the JSON +verdict. Every finding ID uses the exact `security-reviewer-FNNN` prefix. Do +not add `status`, `summary`, or `override_path` as top-level keys and do not +emit separate legacy YAML. Only when the caller does not supply that contract, +use the legacy standalone format below. + ``` status: pass | block | pass-not-applicable summary: diff --git a/commands/pr-gate.md b/commands/pr-gate.md index 451990a2..6f0e8d69 100644 --- a/commands/pr-gate.md +++ b/commands/pr-gate.md @@ -49,6 +49,23 @@ dispatch. `--accept-scope-truncation` permits the run to continue while recording `accepted_truncation`, omitted counts, and reasons; it does not claim complete semantic or call-graph coverage. +Each selected reviewer must emit one `gate_reviewer_result_v1` JSON block bound +to that manifest digest. It declares every required surface as +`examined|not_applicable|uncertain`, includes evidence/reason for every cell, +continues after blockers, and gives every actionable finding a stable ID, +source, failure mode, minimum fix boundary, and verification expectation. +Every evidence path must appear in the manifest's immutable +`reference_index.entries` (or identify the digest-verified manifest itself), and +line references must not exceed the indexed snapshot's line count. +`reviewer_result_v1.verdict` is the only canonical reviewer verdict; Markdown +headings are presentation only, so duplicate or omitted headings cannot turn a +completed review into a format failure. Missing or malformed JSON reports stop +as protocol `INCOMPLETE`, not `Final: NO-GO`. The only valid verdicts are +`approve`, `advise`, `block-soft`, and `block`; legacy reviewer wording such as +`pass` or `pass-not-applicable` must be emitted as `approve`. Diagnostics +distinguish invalid JSON, top-level/binding, coverage, finding, +evidence-reference, and verdict contracts. + ## Step 1 - Invoke pmctl directly Call the bare `pmctl` command with no resolution preamble — an installed diff --git a/core/README.md b/core/README.md index 755416f7..4baf5bd1 100644 --- a/core/README.md +++ b/core/README.md @@ -25,7 +25,11 @@ Gate assurance definitions are split deliberately: - `schema/gate-scope-manifest.schema.json` defines the content-addressed, immutable-subject-bound scope declaration shared by every selected reviewer. It records exact changed inputs, bounded review hints, and explicit - truncation rather than claiming a complete call graph. + truncation rather than claiming a complete call graph. Current producers also + index allowed evidence paths with snapshot line counts and content digests. +- `schema/gate-reviewer-result.schema.json` defines the selected-reviewer + coverage checklist and actionable finding contract. Its JSON verdict is the + machine source of truth; Markdown headings are presentation only. - `schema/gate-verification.schema.json` defines the shared three-axis assessment returned to gate consumers. diff --git a/core/schema/gate-reviewer-result.schema.json b/core/schema/gate-reviewer-result.schema.json new file mode 100644 index 00000000..e4ab49cf --- /dev/null +++ b/core/schema/gate-reviewer-result.schema.json @@ -0,0 +1,502 @@ +{ + "title": "Gate Reviewer Result", + "description": "A schema-complete selected-reviewer report bound to one immutable gate scope manifest. Coverage declaration calibrates the review; it does not claim semantic or defect completeness.", + "type": "object", + "required": [ + "kind", + "schema_version", + "reviewer", + "scope_manifest_sha256", + "coverage_claim", + "coverage", + "findings", + "verdict", + "rationale" + ], + "properties": { + "kind": { + "const": "gate_reviewer_result_v1" + }, + "schema_version": { + "const": 1 + }, + "reviewer": { + "enum": [ + "critic", + "qa-tester", + "architecture-reviewer", + "security-reviewer", + "risk-reviewer" + ] + }, + "scope_manifest_sha256": { + "type": "string", + "pattern": "^[a-f0-9]{64}$" + }, + "coverage_claim": { + "const": "declared-scope-checklist-not-review-completeness" + }, + "coverage": { + "type": "array", + "minItems": 11, + "maxItems": 11, + "items": { + "$ref": "#/definitions/coverageEntry" + }, + "allOf": [ + { + "contains": { + "properties": { + "surface": { + "const": "changed_files" + } + }, + "required": [ + "surface" + ] + } + }, + { + "contains": { + "properties": { + "surface": { + "const": "paired_tests" + } + }, + "required": [ + "surface" + ] + } + }, + { + "contains": { + "properties": { + "surface": { + "const": "sensitive_signals" + } + }, + "required": [ + "surface" + ] + } + }, + { + "contains": { + "properties": { + "surface": { + "const": "public_interface" + } + }, + "required": [ + "surface" + ] + } + }, + { + "contains": { + "properties": { + "surface": { + "const": "schema" + } + }, + "required": [ + "surface" + ] + } + }, + { + "contains": { + "properties": { + "surface": { + "const": "config" + } + }, + "required": [ + "surface" + ] + } + }, + { + "contains": { + "properties": { + "surface": { + "const": "install" + } + }, + "required": [ + "surface" + ] + } + }, + { + "contains": { + "properties": { + "surface": { + "const": "ci" + } + }, + "required": [ + "surface" + ] + } + }, + { + "contains": { + "properties": { + "surface": { + "const": "release" + } + }, + "required": [ + "surface" + ] + } + }, + { + "contains": { + "properties": { + "surface": { + "const": "migration" + } + }, + "required": [ + "surface" + ] + } + }, + { + "contains": { + "properties": { + "surface": { + "const": "bounded_expansion" + } + }, + "required": [ + "surface" + ] + } + } + ] + }, + "findings": { + "type": "array", + "items": { + "$ref": "#/definitions/finding" + } + }, + "verdict": { + "enum": [ + "approve", + "advise", + "block-soft", + "block" + ] + }, + "rationale": { + "type": "string", + "minLength": 1 + } + }, + "allOf": [ + { + "if": { + "properties": { + "verdict": { + "const": "block-soft" + } + } + }, + "then": { + "properties": { + "findings": { + "contains": { + "properties": { + "hard_gate_class": { + "const": "soft_block" + } + }, + "required": [ + "hard_gate_class" + ] + } + } + } + } + }, + { + "if": { + "properties": { + "verdict": { + "const": "block" + } + } + }, + "then": { + "properties": { + "findings": { + "contains": { + "properties": { + "hard_gate_class": { + "const": "hard_block" + } + }, + "required": [ + "hard_gate_class" + ] + } + } + } + } + } + ], + "additionalProperties": false, + "definitions": { + "relativePath": { + "type": "string", + "minLength": 1, + "pattern": "^(?!/)", + "not": { + "pattern": "(^|/)\\.\\.(/|$)" + } + }, + "evidenceRef": { + "type": "object", + "required": [ + "path" + ], + "properties": { + "path": { + "$ref": "#/definitions/relativePath" + }, + "line": { + "type": [ + "integer", + "null" + ], + "minimum": 1 + }, + "symbol": { + "type": [ + "string", + "null" + ], + "minLength": 1 + } + }, + "anyOf": [ + { + "required": [ + "line" + ], + "properties": { + "line": { + "type": "integer", + "minimum": 1 + } + } + }, + { + "required": [ + "symbol" + ], + "properties": { + "symbol": { + "type": "string", + "minLength": 1 + } + } + } + ], + "additionalProperties": false + }, + "coverageEntry": { + "type": "object", + "required": [ + "surface", + "status", + "evidence_refs", + "reason" + ], + "properties": { + "surface": { + "enum": [ + "changed_files", + "paired_tests", + "sensitive_signals", + "public_interface", + "schema", + "config", + "install", + "ci", + "release", + "migration", + "bounded_expansion" + ] + }, + "status": { + "enum": [ + "examined", + "not_applicable", + "uncertain" + ] + }, + "evidence_refs": { + "type": "array", + "items": { + "$ref": "#/definitions/evidenceRef" + } + }, + "reason": { + "type": "string", + "minLength": 1 + } + }, + "allOf": [ + { + "if": { + "properties": { + "status": { + "const": "examined" + } + } + }, + "then": { + "properties": { + "evidence_refs": { + "minItems": 1 + } + } + } + } + ], + "additionalProperties": false + }, + "finding": { + "type": "object", + "required": [ + "id", + "reviewer", + "severity", + "hard_gate_class", + "origin", + "source", + "affected_behavior", + "why_it_matters", + "failure_mode", + "minimum_fix_boundary", + "verification_expectation" + ], + "properties": { + "id": { + "type": "string", + "pattern": "^(critic|qa-tester|architecture-reviewer|security-reviewer|risk-reviewer)-F[0-9]{3}$" + }, + "reviewer": { + "enum": [ + "critic", + "qa-tester", + "architecture-reviewer", + "security-reviewer", + "risk-reviewer" + ] + }, + "severity": { + "enum": [ + "critical", + "high", + "medium", + "low" + ] + }, + "hard_gate_class": { + "enum": [ + "none", + "soft_block", + "hard_block" + ] + }, + "origin": { + "enum": [ + "diff_caused", + "pre_existing", + "uncertain", + "caution" + ] + }, + "source": { + "$ref": "#/definitions/evidenceRef" + }, + "affected_behavior": { + "type": "string", + "minLength": 1 + }, + "why_it_matters": { + "type": "string", + "minLength": 1 + }, + "failure_mode": { + "type": "string", + "minLength": 1 + }, + "minimum_fix_boundary": { + "type": "string", + "minLength": 1 + }, + "verification_expectation": { + "type": "string", + "minLength": 1 + } + }, + "allOf": [ + { + "if": { + "properties": { + "hard_gate_class": { + "enum": [ + "soft_block", + "hard_block" + ] + } + } + }, + "then": { + "properties": { + "severity": { + "enum": [ + "critical", + "high" + ] + }, + "origin": { + "enum": [ + "diff_caused", + "uncertain" + ] + } + } + } + }, + { + "if": { + "properties": { + "origin": { + "enum": [ + "pre_existing", + "caution" + ] + } + } + }, + "then": { + "properties": { + "hard_gate_class": { + "const": "none" + } + } + } + } + ], + "additionalProperties": false + } + } +} diff --git a/core/schema/gate-scope-manifest.schema.json b/core/schema/gate-scope-manifest.schema.json index 1619be9a..8a444228 100644 --- a/core/schema/gate-scope-manifest.schema.json +++ b/core/schema/gate-scope-manifest.schema.json @@ -292,6 +292,27 @@ }, "additionalProperties": false }, + "reference_index": { + "description": "Optional on legacy v1 artifacts; current producers emit this immutable subject/base snapshot index and reviewer protocols bind every evidence reference to it.", + "type": "object", + "required": [ + "claim", + "entries" + ], + "properties": { + "claim": { + "const": "declared-review-reference-set" + }, + "entries": { + "type": "array", + "minItems": 1, + "items": { + "$ref": "#/definitions/referenceEntry" + } + } + }, + "additionalProperties": false + }, "truncation": { "type": "object", "required": [ @@ -850,6 +871,34 @@ }, "additionalProperties": false }, + "referenceEntry": { + "type": "object", + "required": [ + "path", + "snapshot", + "line_count", + "sha256" + ], + "properties": { + "path": { + "$ref": "#/definitions/path" + }, + "snapshot": { + "enum": [ + "subject", + "base" + ] + }, + "line_count": { + "type": "integer", + "minimum": 0 + }, + "sha256": { + "$ref": "#/definitions/sha256" + } + }, + "additionalProperties": false + }, "scopeCounts": { "type": "object", "required": [ diff --git a/docs/architecture/script-variable-consumers.tsv b/docs/architecture/script-variable-consumers.tsv index a3a8237d..5e4fe3f1 100644 --- a/docs/architecture/script-variable-consumers.tsv +++ b/docs/architecture/script-variable-consumers.tsv @@ -25,8 +25,8 @@ CLAUDE_CONFIG_TEST_* CLAUDE_CONFIG_TEST_PREFLIGHT_HOME tests/lib/test-suite-runn CLAUDE_CONFIG_TEST_* CLAUDE_CONFIG_TEST_PREFLIGHT_HOME tests/shell/test-install.sh test CLAUDE_CONFIG_TEST_* CLAUDE_CONFIG_TEST_PREFLIGHT_HOME tests/shell/test-run-all-tests.sh test CLAUDE_DISPATCH_TIMEOUT CLAUDE_DISPATCH_TIMEOUT adapters/claude/dispatch.sh production -CLAUDE_GATE_STUB_* CLAUDE_GATE_STUB_CALLED_MARKER tests/shell/test-pr-gate-profile.sh test -CLAUDE_GATE_STUB_* CLAUDE_GATE_STUB_MODE tests/shell/test-pr-gate-profile.sh test +CLAUDE_GATE_STUB_* CLAUDE_GATE_STUB_CALLED_MARKER tests/lib/test-pr-gate-fixture.sh test +CLAUDE_GATE_STUB_* CLAUDE_GATE_STUB_MODE tests/lib/test-pr-gate-fixture.sh test CLAUDE_HOME CLAUDE_HOME hosts/claude/bin/install-guards.sh production CLAUDE_HOME CLAUDE_HOME hosts/claude/bin/uninstall-guards.sh production CLAUDE_HOME CLAUDE_HOME hosts/claude/lib/doctor.sh production @@ -40,16 +40,20 @@ CODEX_DISPATCH_TIMEOUT CODEX_DISPATCH_TIMEOUT adapters/codex/dispatch.sh product CODEX_DISPATCH_TIMEOUT CODEX_DISPATCH_TIMEOUT runtime/lib/pmctl-dispatch.sh production CODEX_GATE_STUB_* CODEX_GATE_STUB_ tests/shell/test-pr-gate.sh test CODEX_GATE_STUB_* CODEX_GATE_STUB_BOLD_FINAL tests/shell/test-pr-gate.sh test +CODEX_GATE_STUB_* CODEX_GATE_STUB_CALLED_MARKER tests/lib/test-pr-gate-fixture.sh test CODEX_GATE_STUB_* CODEX_GATE_STUB_CALLED_MARKER tests/shell/test-pr-gate-profile.sh test CODEX_GATE_STUB_* CODEX_GATE_STUB_CONFLICTING_VERDICT tests/shell/test-pr-gate.sh test CODEX_GATE_STUB_* CODEX_GATE_STUB_CROSS_TAMPER_REVIEWER tests/shell/test-pr-gate.sh test CODEX_GATE_STUB_* CODEX_GATE_STUB_CROSS_TAMPER_VICTIM tests/shell/test-pr-gate.sh test +CODEX_GATE_STUB_* CODEX_GATE_STUB_DUPLICATE_HEADING tests/shell/test-pr-gate.sh test CODEX_GATE_STUB_* CODEX_GATE_STUB_FRONTMATTER_FINAL tests/shell/test-pr-gate.sh test CODEX_GATE_STUB_* CODEX_GATE_STUB_HEADER_ONLY_VERDICT tests/shell/test-pr-gate.sh test CODEX_GATE_STUB_* CODEX_GATE_STUB_INJECT_FILE tests/shell/test-pr-gate.sh test -CODEX_GATE_STUB_* CODEX_GATE_STUB_MODE tests/shell/test-pr-gate-profile.sh test +CODEX_GATE_STUB_* CODEX_GATE_STUB_MODE tests/lib/test-pr-gate-fixture.sh test CODEX_GATE_STUB_* CODEX_GATE_STUB_MODE tests/shell/test-pr-gate.sh test CODEX_GATE_STUB_* CODEX_GATE_STUB_MULTIPLE_VERDICTS tests/shell/test-pr-gate.sh test +CODEX_GATE_STUB_* CODEX_GATE_STUB_PROTOCOL_MUTATION tests/lib/test-pr-gate-fixture.sh test +CODEX_GATE_STUB_* CODEX_GATE_STUB_PROTOCOL_MUTATION tests/shell/test-pr-gate.sh test CODEX_GATE_STUB_* CODEX_GATE_STUB_SYNTHESIS_EXTRA_FINAL tests/shell/test-pr-gate.sh test CODEX_GATE_STUB_* CODEX_GATE_STUB_SYNTHESIS_FINAL tests/shell/test-pr-gate.sh test CODEX_GATE_STUB_* CODEX_GATE_STUB_SYNTHESIS_INJECT_FILE tests/shell/test-pr-gate.sh test diff --git a/docs/review-model.md b/docs/review-model.md index 5c834dd5..edd6dbae 100644 --- a/docs/review-model.md +++ b/docs/review-model.md @@ -203,9 +203,11 @@ Changes under `core/policy/` are themselves a full-tier signal with architecture, security, and risk coverage, so the governance tables cannot quietly lower their own future review floor through a small edit. -The final producer writes `pr_gate_result_v2` Markdown plus a sibling -`gate_assurance_v3` JSON envelope. The Markdown contains human findings and a -bounded relative `gate_assurance` pointer; the shell-owned envelope records +After reviewer dispatch completes, the final producer writes +`pr_gate_result_v3` Markdown plus a sibling `gate_assurance_v3` JSON envelope. +A pre-dispatch fail-fast route has no reviewer protocol and intentionally +remains `pr_gate_result_v2`. The Markdown contains human findings and a bounded +relative `gate_assurance` pointer; the shell-owned envelope records requested/resolved coordinates, selected/skipped coverage, actual dispatch outcomes, run IDs, and the evidence status behind independence claims. The v3 envelope adds an immutable subject: stable Git common-directory repository @@ -239,6 +241,42 @@ parallel, and synthesis briefs all carry the same artifact digest, so execution mode cannot silently change the declared review scope. The manifest is a scope declaration, not proof that a reviewer understood or exhaustively reviewed it. +Every selected reviewer emits one fenced `gate_reviewer_result_v1` JSON object +bound to the shared scope-manifest SHA-256. Its checklist covers changed files, +paired tests, sensitive signals, explicit surface flags, and bounded expansion; +each cell is `examined`, `not_applicable`, or `uncertain`, with evidence +references and a reason. A blocker does not terminate the checklist early. +Findings carry reviewer-prefixed stable IDs, severity, hard-gate class, origin, +source path plus line or symbol, affected behavior, failure mode, minimum fix +boundary, and verification expectation. Pre-existing issues and cautions cannot +be blocking. + +Current scope manifests include a `declared-review-reference-set` index for the +subject/base snapshots supplied to reviewers. Each entry records path, snapshot +kind, line count, and content SHA-256. Coverage references and finding sources +must use an indexed path (or the digest-verified manifest itself); line +references must be within the indexed snapshot. Nonexistent, out-of-scope, or +out-of-range references make the protocol `INCOMPLETE` before synthesis. +Legacy pre-index manifests remain readable for historical result verification. + +The JSON `verdict` is the only canonical reviewer verdict. Markdown headings +are optional presentation: duplicate, missing, or differently formatted +headings do not invalidate an otherwise unique schema-complete report. The +shell validates each report before synthesis, computes GO/NO-GO from the JSON +verdicts, preserves the original reports in the final result, and verifies +their aggregate against `Final:`. Missing, duplicate, malformed, or +coverage-incomplete reports make the operation `INCOMPLETE`; they are not a +reviewer `NO-GO`. Coverage declaration calibrates what was examined and never +claims model recall or defect completeness. + +The verdict vocabulary is exactly `approve`, `advise`, `block-soft`, or +`block`. Reviewer prompts map legacy `pass` and `pass-not-applicable` wording +to `approve`, and place narrative conclusions in `rationale` rather than +inventing role-specific top-level keys. Protocol failures identify the first +invalid layer as JSON, top-level/binding, coverage, finding, +evidence-reference, or verdict so a format error is not misdiagnosed as missing +coverage. + `pmctl gate verify [--cd ] [--consumer ] [--json]` returns three independent axes: @@ -270,16 +308,19 @@ Do not mutate the reviewed tree between gate finalization and `gate wait` or `ship finish`. A formatter, test artifact, commit, or other included file write correctly changes `subject_current` to `fail`; the consumer refuses the result and reports the drift reason instead of silently authorizing a different tree. -Legacy v1 results retain historical detached-wait completion behavior. A v2 GO -cannot satisfy the wait path's named embedded consumer, and neither v1 nor v2 -is publication authorization; `ship finish` requires a current, applicable v3 +Legacy result v1/v2 artifacts remain readable under their historical +contracts. Result v3 is required for selected-reviewer protocol evidence. +No gate result version is publication authorization by itself; `ship finish` +requires a current, applicable gate-assurance v3 assessment. The producer publishes the sidecar before atomically replacing the -self-contained v1 result with the v2 result that references it, so interruption -cannot strand a v2 result with a missing sidecar. The protected attestation is -published afterward; verification uses a bounded retry when it observes that -in-flight canonical finalization. Legacy +self-contained staging v1 result with the bound result that references it, so +interruption cannot strand a result with a missing sidecar. A completed +selected-reviewer route becomes result v3; a pre-dispatch fail-fast result with +no reviewer protocol remains result v2. The protected attestation is published +afterward; verification uses a bounded retry when it observes that in-flight +canonical finalization. Legacy `pr_gate_result_v1` and unbound `gate_assurance_v1` artifacts remain structurally readable, but verification reports `assurance: unavailable`; consumers must not infer mode, coverage, or independence from them. Earlier diff --git a/docs/sandbox-limitations.md b/docs/sandbox-limitations.md index dc925bbe..144e761e 100644 --- a/docs/sandbox-limitations.md +++ b/docs/sandbox-limitations.md @@ -30,7 +30,7 @@ Understanding the boundary helps you write `self_verify` blocks that actually wo > warning. Pass `--allow-hooks` only for repositories and branches you trust. > > ```bash -> bash runtime/bin/pr-gate.sh --cd . --allow-hooks +> pmctl gate run --cd . --allow-hooks --lifecycle foreground > ``` The gate lifecycle hooks let the main thread start and stop infrastructure @@ -60,7 +60,7 @@ around the gate run. Place hook scripts in `.pm-dispatch/` at your project root: Depending on your use case, choose the right approach: **(A) Teardown that must always run** (e.g., `docker compose down`, even if a reviewer fails): -Wrap `pr-gate.sh` in a shell function or CI step that uses a `trap` in the +Wrap the foreground gate invocation in a shell function or CI step that uses a `trap` in the **caller**, not inside `pre-gate.sh`. The hook exits after setup; any `trap` inside it cleans up immediately, not after the gate finishes. @@ -74,7 +74,7 @@ trap cleanup EXIT docker compose -f docker-compose.test.yml up -d db until docker compose -f docker-compose.test.yml exec -T db pg_isready -q; do sleep 1; done -bash runtime/bin/pr-gate.sh --cd . --allow-hooks +pmctl gate run --cd . --allow-hooks --lifecycle foreground ``` The teardown trap is now in scope for the full gate run. Alternatively, use a @@ -150,7 +150,7 @@ make test-integration Then run the gate: ```bash -bash runtime/bin/pr-gate.sh --cd . --allow-hooks +pmctl gate run --cd . --allow-hooks --lifecycle foreground ``` **Why this works**: `pre-gate.sh` runs as the main-thread user with full Docker and diff --git a/runtime/bin/pr-gate.sh b/runtime/bin/pr-gate.sh index c88b866e..d67c288a 100755 --- a/runtime/bin/pr-gate.sh +++ b/runtime/bin/pr-gate.sh @@ -1012,9 +1012,101 @@ _gate_scope_flags_resolve() { ' } +_gate_scope_reference_index_collect() { + local changed_paths_json="$1" paired_tests_json="$2" + local sensitive_signals_json="$3" flags_json="$4" + local expansion_file="$5" output="$6" + local paths_file content_file sorted_file path snapshot line_count digest + local reference_fd + paths_file="$(mktemp "${TMPDIR:-/tmp}/gate-scope-reference-paths.XXXXXX")" \ + || return 2 + content_file="$(mktemp "${TMPDIR:-/tmp}/gate-scope-reference-content.XXXXXX")" \ + || { + rm -f -- "$paths_file" + return 2 + } + sorted_file="$(mktemp "${TMPDIR:-/tmp}/gate-scope-reference-index.XXXXXX")" \ + || { + rm -f -- "$paths_file" "$content_file" + return 2 + } + : > "$output" + if ! jq -jnr \ + --argjson changed "$changed_paths_json" \ + --argjson paired "$paired_tests_json" \ + --argjson signals "$sensitive_signals_json" \ + --argjson flags "$flags_json" \ + --slurpfile expansion "$expansion_file" ' + ([$changed[]] + + [$paired[] | .source_path, .test_path] + + [$signals[] | .matches[]] + + [$flags[] | .paths[]] + + [$expansion[0][] | .path] | + unique | sort | .[]) + "\u0000" + ' > "$paths_file"; then + rm -f -- "$paths_file" "$content_file" "$sorted_file" + return 2 + fi + + exec {reference_fd}< "$paths_file" || { + rm -f -- "$paths_file" "$content_file" "$sorted_file" + return 2 + } + while IFS= read -r -d '' path <&"$reference_fd"; do + snapshot=subject + if [[ "$POLICY_DIFF_KIND" != fixed-head && -L "$WORK_DIR/$path" ]]; then + readlink -n -- "$WORK_DIR/$path" > "$content_file" || { + rm -f -- "$paths_file" "$content_file" "$sorted_file" + return 2 + } + elif _gate_scope_path_exists "$path"; then + _gate_scope_path_content "$path" > "$content_file" || { + rm -f -- "$paths_file" "$content_file" "$sorted_file" + return 2 + } + elif git -C "$WORK_DIR" cat-file -e \ + "${GATE_BINDING_BASE_COMMIT}:$path" 2>/dev/null; then + snapshot=base + git -C "$WORK_DIR" show \ + "${GATE_BINDING_BASE_COMMIT}:$path" > "$content_file" || { + rm -f -- "$paths_file" "$content_file" "$sorted_file" + return 2 + } + else + continue + fi + line_count="$(awk 'END { print NR+0 }' "$content_file")" + digest="$(_gate_sha256_stream < "$content_file")" || { + rm -f -- "$paths_file" "$content_file" "$sorted_file" + return 2 + } + jq -nc --arg path "$path" --arg snapshot "$snapshot" \ + --argjson line_count "$line_count" --arg sha256 "$digest" '{ + path:$path, + snapshot:$snapshot, + line_count:$line_count, + sha256:$sha256 + }' >> "$output" || { + rm -f -- "$paths_file" "$content_file" "$sorted_file" + return 2 + } + done + exec {reference_fd}<&- + + jq -s 'unique_by(.path) | sort_by(.path)' "$output" > "$sorted_file" || { + rm -f -- "$paths_file" "$content_file" "$sorted_file" + return 2 + } + mv -- "$sorted_file" "$output" || { + rm -f -- "$paths_file" "$content_file" "$sorted_file" + return 2 + } + rm -f -- "$paths_file" "$content_file" +} + _gate_scope_manifest_write() { local destination="$1" changes_json="$2" policy_json="$3" - local hunks_file binary_file expansion_file manifest_tmp + local hunks_file binary_file expansion_file reference_file manifest_tmp local changed_paths_json renamed_paths_json untracked_paths_json local paired_tests_json sensitive_signals_json flags_json local truncation_occurred=false truncation_accepted=false @@ -1028,10 +1120,14 @@ _gate_scope_manifest_write() { rm -f -- "$hunks_file" "$binary_file" return 2 } - manifest_tmp="$(mktemp "${destination}.tmp.XXXXXX")" || { + reference_file="$(mktemp "${TMPDIR:-/tmp}/gate-scope-references.XXXXXX")" || { rm -f -- "$hunks_file" "$binary_file" "$expansion_file" return 2 } + manifest_tmp="$(mktemp "${destination}.tmp.XXXXXX")" || { + rm -f -- "$hunks_file" "$binary_file" "$expansion_file" "$reference_file" + return 2 + } changed_paths_json="$(jq -c '[ .[] | .old_path, .new_path | select(. != null) @@ -1045,21 +1141,31 @@ _gate_scope_manifest_write() { ] | unique | sort' <<<"$changes_json")" || return 2 _gate_scope_hunks_collect "$changes_json" "$hunks_file" "$binary_file" || { - rm -f -- "$hunks_file" "$binary_file" "$expansion_file" "$manifest_tmp" + rm -f -- "$hunks_file" "$binary_file" "$expansion_file" \ + "$reference_file" "$manifest_tmp" return 2 } paired_tests_json="$(_gate_scope_paired_tests_collect "$changed_paths_json")" || { - rm -f -- "$hunks_file" "$binary_file" "$expansion_file" "$manifest_tmp" + rm -f -- "$hunks_file" "$binary_file" "$expansion_file" \ + "$reference_file" "$manifest_tmp" return 2 } _gate_scope_expansions_collect "$changed_paths_json" "$expansion_file" || { - rm -f -- "$hunks_file" "$binary_file" "$expansion_file" "$manifest_tmp" + rm -f -- "$hunks_file" "$binary_file" "$expansion_file" \ + "$reference_file" "$manifest_tmp" return 2 } sensitive_signals_json="$(jq -c '[ .matched_signals[] | select(.source == "path-regex") ] | sort_by(.id)' <<<"$policy_json")" || return 2 flags_json="$(_gate_scope_flags_resolve "$changed_paths_json")" || return 2 + _gate_scope_reference_index_collect \ + "$changed_paths_json" "$paired_tests_json" "$sensitive_signals_json" \ + "$flags_json" "$expansion_file" "$reference_file" || { + rm -f -- "$hunks_file" "$binary_file" "$expansion_file" \ + "$reference_file" "$manifest_tmp" + return 2 + } if [[ "$GATE_SCOPE_OMITTED_DIFF_HUNKS" -gt 0 \ || "$GATE_SCOPE_OMITTED_EXPANSION_SOURCES" -gt 0 \ @@ -1107,6 +1213,7 @@ _gate_scope_manifest_write() { --argjson paired_tests "$paired_tests_json" \ --argjson sensitive_signals "$sensitive_signals_json" \ --argjson flags "$flags_json" --slurpfile expansion "$expansion_file" \ + --slurpfile references "$reference_file" \ --argjson truncation_occurred "$truncation_occurred" \ --argjson truncation_accepted "$truncation_accepted" \ --argjson omitted_hunks "$GATE_SCOPE_OMITTED_DIFF_HUNKS" \ @@ -1154,6 +1261,10 @@ _gate_scope_manifest_write() { entries:$expansion[0], included_paths:([$expansion[0][].path] | unique | sort) }, + reference_index:{ + claim:"declared-review-reference-set", + entries:$references[0] + }, truncation:{ occurred:$truncation_occurred, budgets:{ @@ -1182,26 +1293,29 @@ _gate_scope_manifest_write() { digest:("") } }' > "$manifest_tmp"; then - rm -f -- "$hunks_file" "$binary_file" "$expansion_file" "$manifest_tmp" + rm -f -- "$hunks_file" "$binary_file" "$expansion_file" \ + "$reference_file" "$manifest_tmp" return 2 fi content_digest="$(jq -cS 'del(.content.digest)' "$manifest_tmp" | _gate_sha256_stream)" || { - rm -f -- "$hunks_file" "$binary_file" "$expansion_file" "$manifest_tmp" + rm -f -- "$hunks_file" "$binary_file" "$expansion_file" \ + "$reference_file" "$manifest_tmp" return 2 } jq --arg digest "$content_digest" '.content.digest=$digest' \ "$manifest_tmp" > "${manifest_tmp}.final" || { rm -f -- "$hunks_file" "$binary_file" "$expansion_file" \ - "$manifest_tmp" "${manifest_tmp}.final" + "$reference_file" "$manifest_tmp" "${manifest_tmp}.final" return 2 } mv -- "${manifest_tmp}.final" "$destination" || { rm -f -- "$hunks_file" "$binary_file" "$expansion_file" \ - "$manifest_tmp" "${manifest_tmp}.final" + "$reference_file" "$manifest_tmp" "${manifest_tmp}.final" return 2 } - rm -f -- "$hunks_file" "$binary_file" "$expansion_file" "$manifest_tmp" + rm -f -- "$hunks_file" "$binary_file" "$expansion_file" \ + "$reference_file" "$manifest_tmp" } # Resolve risk/policy once for every gate consumer. The function owns the @@ -1616,46 +1730,30 @@ verify_reviewer_artifact_hashes() { done } -# _gate_reviewer_verdict_extract -# -# Parallel reviewer briefs already require a canonical heading. Base-pinned -# reviewer definitions additionally use a lower-case, narrative `verdict:` -# field, so requiring a second upper-case Verdict line creates two competing -# output contracts. Treat the unique, reviewer-matched heading as authoritative. -# An optional legacy `Verdict:` marker remains accepted only when it is itself -# unique, valid, and agrees with the heading. -_gate_reviewer_verdict_extract() { - local reviewer="${1:-}" artifact="${2:-}" - local heading_count valid_heading_count explicit_count valid_explicit_count - local heading_verdict explicit_verdict - [[ $# -eq 2 && -n "$reviewer" && -s "$artifact" ]] || return 1 - - heading_count="$(grep -cE "^## ${reviewer} -- " "$artifact" || true)" - valid_heading_count="$( - grep -cE "^## ${reviewer} -- (approve|advise|block-soft|block)$" \ - "$artifact" || true - )" - [[ "$heading_count" -eq 1 && "$valid_heading_count" -eq 1 ]] || return 1 - heading_verdict="$( - grep -oE "^## ${reviewer} -- (approve|advise|block-soft|block)$" \ - "$artifact" | awk '{print $4}' - )" - - explicit_count="$(grep -cE '^Verdict:' "$artifact" || true)" - if [[ "$explicit_count" -gt 0 ]]; then - valid_explicit_count="$( - grep -cE '^Verdict: (approve|advise|block-soft|block)([. ]|$)' \ - "$artifact" || true - )" - [[ "$explicit_count" -eq 1 && "$valid_explicit_count" -eq 1 ]] || return 1 - explicit_verdict="$( - grep -oE '^Verdict: (approve|advise|block-soft|block)([. ]|$)' \ - "$artifact" | awk '{print $2}' | tr -d '. ' - )" - [[ "$explicit_verdict" == "$heading_verdict" ]] || return 1 +_gate_reviewer_protocol_append_blocks() { + local destination="$1" artifact reviewer verdict + shift + if grep -q '^```reviewer_result_v1$' "$destination"; then + printf 'Error: reviewer protocol INCOMPLETE: synthesis emitted machine-owned reviewer blocks\n' >&2 + return 1 fi - - printf '%s\n' "$heading_verdict" + { + printf '\n## Reviewer Protocol Evidence\n' + printf 'Validated selected-reviewer reports are preserved verbatim below.\n\n' + for artifact in "$@"; do + reviewer="$( + _gate_reviewer_protocol_documents "$artifact" | + jq -sr '.[0].reviewer // empty' + )" || return 1 + verdict="$( + _gate_reviewer_protocol_verdict_extract "$artifact" "$reviewer" + )" || return 1 + printf '## %s -- %s\n' "$reviewer" "$verdict" + # shellcheck disable=SC2016 # Literal Markdown fence delimiters. + sed -n '/^```reviewer_result_v1$/,/^```$/p' "$artifact" + printf '\n' + done + } >> "$destination" } # _kill_process_tree [signal] -- signal a process AND all its descendants. @@ -1993,6 +2091,302 @@ else ' "$result_file" } + _gate_reviewer_protocol_surfaces() { + printf '%s\n' \ + changed_files \ + paired_tests \ + sensitive_signals \ + public_interface \ + schema \ + config \ + install \ + ci \ + release \ + migration \ + bounded_expansion + } + + _gate_reviewer_protocol_reference_index_json() { + local manifest_file="$1" expected_sha="$2" + local actual_sha manifest_ref line_count + if [[ ! -f "$manifest_file" || -L "$manifest_file" ]]; then + printf 'Error: reviewer protocol reference manifest is missing or unsafe: %s\n' \ + "$manifest_file" >&2 + return 1 + fi + actual_sha="$(_gate_result_sha256_file "$manifest_file")" || return $? + if [[ "$actual_sha" != "$expected_sha" ]]; then + printf 'Error: reviewer protocol reference manifest digest mismatch: %s\n' \ + "$manifest_file" >&2 + return 1 + fi + manifest_ref=".gate-results/$(basename "$manifest_file")" + line_count="$(awk 'END { print NR+0 }' "$manifest_file")" + jq -c \ + --arg manifest_ref "$manifest_ref" \ + --arg manifest_sha "$actual_sha" \ + --argjson manifest_lines "$line_count" ' + if .reference_index.claim != "declared-review-reference-set" or + (.reference_index.entries | type) != "array" or + (.reference_index.entries | length) == 0 + then error("missing reviewer reference index: claim=" + + (.reference_index.claim // "") + " entries=" + + ((.reference_index.entries // []) | length | tostring)) + else + ([.reference_index.entries[] | { + path:.path, + line_count:.line_count, + sha256:.sha256 + }] + [{ + path:$manifest_ref, + line_count:$manifest_lines, + sha256:$manifest_sha + }] | unique_by(.path) | sort_by(.path)) + end + ' "$manifest_file" || { + printf 'Error: reviewer protocol reference manifest has no valid index: %s\n' \ + "$manifest_file" >&2 + return 1 + } + } + + _gate_reviewer_protocol_document_verify() { + local document="$1" expected_reviewer="$2" expected_scope_sha="$3" + local reference_index_json="${4:-null}" + local surfaces_json validation + GATE_REVIEWER_PROTOCOL_DOCUMENT_ERROR="" + surfaces_json="$(_gate_reviewer_protocol_surfaces | jq -Rsc ' + split("\n") | map(select(length > 0)) + ')" || return 2 + validation="$(jq -r \ + --arg reviewer "$expected_reviewer" \ + --arg scope_sha "$expected_scope_sha" \ + --argjson surfaces "$surfaces_json" \ + --argjson references "$reference_index_json" ' + def only_keys($allowed): + type == "object" and ((keys_unsorted - $allowed) | length) == 0; + def exact_keys($required): + type == "object" and ((keys_unsorted | sort) == ($required | sort)); + def nonempty: type == "string" and length > 0; + def relative_path: + nonempty and (startswith("/") | not) and + (test("(^|/)\\.\\.(/|$)") | not); + def evidence_ref: + only_keys(["path","line","symbol"]) and + (.path | relative_path) and + (.line == null or (.line | type == "number" and . >= 1 and floor == .)) and + (.symbol == null or (.symbol | nonempty)) and + (.line != null or .symbol != null); + def coverage_entry: + only_keys(["surface","status","evidence_refs","reason"]) and + (.surface | IN($surfaces[])) and + (.status | IN("examined","not_applicable","uncertain")) and + (.reason | nonempty) and + (.evidence_refs | type == "array" and all(.[]; evidence_ref)) and + (if .status == "examined" + then (.evidence_refs | length) > 0 + else true + end); + def finding: + only_keys(["id","reviewer","severity","hard_gate_class","origin","source", + "affected_behavior","why_it_matters","failure_mode", + "minimum_fix_boundary","verification_expectation"]) and + .reviewer == $reviewer and + (.id | test("^" + $reviewer + "-F[0-9]{3}$")) and + (.severity | IN("critical","high","medium","low")) and + (.hard_gate_class | IN("none","soft_block","hard_block")) and + (.origin | IN("diff_caused","pre_existing","uncertain","caution")) and + (.source | evidence_ref) and + (.affected_behavior | nonempty) and + (.why_it_matters | nonempty) and + (.failure_mode | nonempty) and + (.minimum_fix_boundary | nonempty) and + (.verification_expectation | nonempty) and + (if .hard_gate_class == "none" + then true + else (.severity | IN("critical","high")) and + (.origin | IN("diff_caused","uncertain")) + end) and + (if (.origin | IN("pre_existing","caution")) + then .hard_gate_class == "none" + else true + end); + def envelope_contract: + exact_keys(["kind","schema_version","reviewer","scope_manifest_sha256", + "coverage_claim","coverage","findings","verdict","rationale"]) and + .kind == "gate_reviewer_result_v1" and .schema_version == 1 and + .reviewer == $reviewer and .scope_manifest_sha256 == $scope_sha and + .coverage_claim == "declared-scope-checklist-not-review-completeness"; + def coverage_contract: + .coverage | type == "array" and length == ($surfaces | length) and + all(.[]; coverage_entry) and + ([.[].surface] | sort) == ($surfaces | sort); + def finding_contract: + .findings | type == "array" and all(.[]; finding) and + ([.[].id] | length) == ([.[].id] | unique | length); + def verdict_contract: + (.verdict | IN("approve","advise","block-soft","block")) and + (.rationale | nonempty) and + (if .verdict == "block-soft" + then any(.findings[]; .hard_gate_class == "soft_block") + elif .verdict == "block" + then any(.findings[]; .hard_gate_class == "hard_block") + else all(.findings[]; .hard_gate_class == "none") + end); + def bound_evidence_ref: + .path as $path | + ($references | map(select(.path == $path)) | first) as $entry | + $entry != null and + (if .line != null then .line <= $entry.line_count else true end); + def evidence_reference_contract: + all(.coverage[].evidence_refs[]; bound_evidence_ref) and + all(.findings[].source; bound_evidence_ref); + if (envelope_contract | not) + then "invalid top-level or binding contract" + elif (coverage_contract | not) + then "invalid coverage contract" + elif (finding_contract | not) + then "invalid finding contract" + elif $references != null and (evidence_reference_contract | not) + then "invalid evidence reference contract" + elif (verdict_contract | not) + then "invalid verdict contract" + else "ok" + end + ' "$document" 2>/dev/null)" || { + GATE_REVIEWER_PROTOCOL_DOCUMENT_ERROR="invalid JSON document" + return 1 + } + if [[ "$validation" != ok ]]; then + GATE_REVIEWER_PROTOCOL_DOCUMENT_ERROR="$validation" + return 1 + fi + } + + _gate_reviewer_protocol_documents() { + local artifact="$1" + awk ' + $0 == "```reviewer_result_v1" { + if (inside) exit 2 + inside=1 + next + } + inside && $0 == "```" { + inside=0 + print "" + next + } + inside { print } + END { if (inside) exit 2 } + ' "$artifact" + } + + _gate_reviewer_protocol_verdict_extract() { + local artifact="$1" reviewer="$2" verdicts count + verdicts="$( + _gate_reviewer_protocol_documents "$artifact" | + jq -r --arg reviewer "$reviewer" \ + 'select(.reviewer == $reviewer) | .verdict // empty' + )" || return 1 + count="$(printf '%s\n' "$verdicts" | awk 'NF { count++ } END { print count+0 }')" + [[ "$count" -eq 1 ]] || return 1 + printf '%s\n' "$verdicts" + } + + _gate_reviewer_protocol_final_extract() { + local artifact="$1" + _gate_reviewer_protocol_documents "$artifact" | + jq -sr ' + map(.verdict) as $verdicts | + if ($verdicts | any(. == "block" or . == "block-soft")) + then "NO-GO" + else "GO" + end + ' + } + + gate_reviewer_protocol_verify() { + local artifact=${1-} selected=${2-} scope_sha=${3-} scope_manifest=${4-} + local tmp_dir line block="" in_block=false count=0 reviewer expected document + local seen=" " reference_index_json=null + [[ $# -ge 3 && $# -le 4 && -s "$artifact" && -n "$selected" \ + && "$scope_sha" =~ ^[a-f0-9]{64}$ ]] || { + printf 'Error: reviewer protocol INCOMPLETE: invalid verifier inputs\n' >&2 + return 2 + } + if [[ -n "$scope_manifest" ]]; then + reference_index_json="$( + _gate_reviewer_protocol_reference_index_json \ + "$scope_manifest" "$scope_sha" + )" || return $? + fi + tmp_dir="$(mktemp -d "${TMPDIR:-/tmp}/gate-reviewer-protocol.XXXXXX")" \ + || return 2 + while IFS= read -r line || [[ -n "$line" ]]; do + if [[ "$line" == '```reviewer_result_v1' ]]; then + if [[ "$in_block" == true ]]; then + printf 'Error: reviewer protocol INCOMPLETE: nested result block in %s\n' \ + "$artifact" >&2 + rm -rf -- "$tmp_dir" + return 1 + fi + in_block=true + block="" + continue + fi + if [[ "$in_block" == true && "$line" == '```' ]]; then + count=$((count + 1)) + printf '%s\n' "$block" > "$tmp_dir/$count.json" + in_block=false + block="" + continue + fi + if [[ "$in_block" == true ]]; then + block="${block}${block:+$'\n'}${line}" + fi + done < "$artifact" + if [[ "$in_block" == true ]]; then + printf 'Error: reviewer protocol INCOMPLETE: unclosed result block in %s\n' \ + "$artifact" >&2 + rm -rf -- "$tmp_dir" + return 1 + fi + if [[ "$count" -eq 0 ]]; then + printf 'Error: reviewer protocol INCOMPLETE: no reviewer_result_v1 block in %s\n' \ + "$artifact" >&2 + rm -rf -- "$tmp_dir" + return 1 + fi + for document in "$tmp_dir"/*.json; do + reviewer="$(jq -r '.reviewer // empty' "$document" 2>/dev/null)" || reviewer="" + if [[ -z "$reviewer" || " $selected " != *" $reviewer "* \ + || "$seen" == *" $reviewer "* ]]; then + printf 'Error: reviewer protocol INCOMPLETE: unexpected or duplicate reviewer %s in %s\n' \ + "${reviewer:-}" "$artifact" >&2 + rm -rf -- "$tmp_dir" + return 1 + fi + if ! _gate_reviewer_protocol_document_verify \ + "$document" "$reviewer" "$scope_sha" "$reference_index_json"; then + printf 'Error: reviewer protocol INCOMPLETE: %s for %s in %s\n' \ + "${GATE_REVIEWER_PROTOCOL_DOCUMENT_ERROR:-invalid reviewer document}" \ + "$reviewer" "$artifact" >&2 + rm -rf -- "$tmp_dir" + return 1 + fi + seen="${seen}${reviewer} " + done + for expected in $selected; do + if [[ "$seen" != *" $expected "* ]]; then + printf 'Error: reviewer protocol INCOMPLETE: missing selected reviewer %s in %s\n' \ + "$expected" "$artifact" >&2 + rm -rf -- "$tmp_dir" + return 1 + fi + done + rm -rf -- "$tmp_dir" + } + _gate_result_sha256_stream() { if command -v sha256sum >/dev/null 2>&1; then sha256sum | awk '{print $1}' @@ -2242,9 +2636,10 @@ else (.matched == ((.paths | length) > 0)) and all(.paths[]; . as $path | ($changed | index($path)) != null); - only_keys(["kind","schema_version","status","subject","selection", - "changes","diff","paired_tests","sensitive_signals","flags", - "expansion","truncation","content"]) and + ((keys - ["reference_index"]) | sort) == + (["kind","schema_version","status","subject","selection", + "changes","diff","paired_tests","sensitive_signals","flags", + "expansion","truncation","content"] | sort) and .kind == "gate_scope_manifest_v1" and .schema_version == 1 and (.status | IN("complete","accepted_truncation","incomplete")) and (.subject | @@ -2354,9 +2749,9 @@ else ($flags.ci | scope_flag($changed)) and ($flags.release | scope_flag($changed)) and ($flags.migration | scope_flag($changed)))) and - (.expansion | - only_keys(["claim","entries","included_paths"]) and - .claim == "bounded-hints-not-complete-call-graph" and + (.expansion | + only_keys(["claim","entries","included_paths"]) and + .claim == "bounded-hints-not-complete-call-graph" and (.entries | type == "array" and all(.[]; only_keys(["path","reason","source","evidence","limit"]) and @@ -2369,10 +2764,29 @@ else only_keys(["kind","maximum"]) and (.kind | IN("per-source","per-symbol","global")) and (.maximum | type == "number" and . >= 1 and floor == .)))) and - (.included_paths | paths_unique) and - (. as $expansion | - ([.entries[].path] | unique | exact_set($expansion.included_paths)))) and - (. as $manifest | .truncation | + (.included_paths | paths_unique) and + (. as $expansion | + ([.entries[].path] | unique | exact_set($expansion.included_paths)))) and + (. as $manifest | + ([$manifest.changes.changed_paths[]] + + [$manifest.paired_tests[] | .source_path,.test_path] + + [$manifest.sensitive_signals[] | .matches[]] + + [$manifest.flags[] | .paths[]] + + [$manifest.expansion.included_paths[]] | unique) as $allowed | + ($manifest.reference_index == null or + ($manifest.reference_index | + only_keys(["claim","entries"]) and + .claim == "declared-review-reference-set" and + (.entries | type == "array" and length > 0) and + ([.entries[].path] | strings_unique) and + all(.entries[]; + only_keys(["path","snapshot","line_count","sha256"]) and + (.path | safe_path) and + (.path as $path | ($allowed | index($path)) != null) and + (.snapshot | IN("subject","base")) and + (.line_count | type == "number" and . >= 0 and floor == .) and + (.sha256 | test("^[a-f0-9]{64}$")))))) and + (. as $manifest | .truncation | only_keys(["occurred","budgets","omitted","reasons","acceptance"]) and (.occurred | type == "boolean") and (.budgets | scope_counts and all(.[]; . >= 1)) and @@ -2905,6 +3319,8 @@ else gate_result_verify() { local result_file=${1-} expected_final=${2-} route_label=${3-gate} local version pointer result_parent assurance_file body_final + local selected_reviewers scope_sha scope_artifact scope_manifest + local assurance_kind protocol_final [[ $# -ge 1 && $# -le 3 ]] || { printf 'gate-result-verify: gate_result_verify expects [expected_final] [route_label]\n' >&2 return 2 @@ -2918,18 +3334,54 @@ else export GATE_RESULT_ASSURANCE return 0 ;; - pr_gate_result_v2) + pr_gate_result_v2 | pr_gate_result_v3) pointer="$(_gate_result_frontmatter_value "$result_file" gate_assurance)" if [[ -z "$pointer" || "$pointer" == */* || "$pointer" == "." || "$pointer" == ".." \ || ! "$pointer" =~ ^[A-Za-z0-9._-]+\.json$ ]]; then - printf 'Error: pr_gate_result_v2 requires a bounded sibling gate_assurance pointer: %s\n' \ - "$result_file" >&2 + printf 'Error: %s requires a bounded sibling gate_assurance pointer: %s\n' \ + "$version" "$result_file" >&2 return 1 fi result_parent="$(cd "$(dirname "$result_file")" && pwd -P)" || return 1 assurance_file="$result_parent/$pointer" body_final=$(grep -E '^Final: (GO|NO-GO)$' "$result_file" | awk '{print $2}') gate_assurance_verify "$result_file" "$assurance_file" "$body_final" || return $? + if [[ "$version" == pr_gate_result_v3 ]]; then + assurance_kind="$(jq -r '.kind // empty' "$assurance_file" 2>/dev/null)" + selected_reviewers="$(jq -r \ + '.coordinates.coverage.selected // [] | join(" ")' \ + "$assurance_file" 2>/dev/null)" + scope_sha="$(jq -r \ + '.evidence.scope_manifest.sha256 // empty' \ + "$assurance_file" 2>/dev/null)" + scope_artifact="$(jq -r \ + '.evidence.scope_manifest.artifact // empty' \ + "$assurance_file" 2>/dev/null)" + scope_manifest="$result_parent/$scope_artifact" + if [[ "$assurance_kind" != gate_assurance_v3 \ + || -z "$selected_reviewers" \ + || ! "$scope_sha" =~ ^[a-f0-9]{64}$ ]]; then + printf 'Error: pr_gate_result_v3 requires verified selected-reviewer scope evidence: %s\n' \ + "$assurance_file" >&2 + return 1 + fi + if jq -e '.reference_index != null' "$scope_manifest" >/dev/null 2>&1; then + gate_reviewer_protocol_verify \ + "$result_file" "$selected_reviewers" "$scope_sha" \ + "$scope_manifest" || return $? + else + gate_reviewer_protocol_verify \ + "$result_file" "$selected_reviewers" "$scope_sha" || return $? + fi + protocol_final="$( + _gate_reviewer_protocol_final_extract "$result_file" + )" || return 1 + if [[ "$protocol_final" != "$body_final" ]]; then + printf 'Error: reviewer protocol verdict (%s) contradicts gate Final: (%s): %s\n' \ + "$protocol_final" "$body_final" "$result_file" >&2 + return 1 + fi + fi if [[ "${GATE_ASSURANCE_BOUND:-false}" == true ]]; then GATE_RESULT_ASSURANCE=verified else @@ -4104,6 +4556,7 @@ _gate_assurance_destination_check "$ASSURANCE_FILE" || exit 2 GATE_OUTPUT_EXISTED=false [[ -e "$OUTPUT_FILE" ]] && GATE_OUTPUT_EXISTED=true touch "$OUTPUT_FILE" +REVIEWER_PROTOCOL_COMPLETE=false # Track all brief files for EXIT cleanup BRIEF_FILES=() @@ -4354,14 +4807,17 @@ gate_finalize_assurance() { "$result_file" >&2 return 1 } - awk -v pointer="$ASSURANCE_POINTER" ' + local result_version=pr_gate_result_v2 + [[ "$REVIEWER_PROTOCOL_COMPLETE" == true ]] \ + && result_version=pr_gate_result_v3 + awk -v pointer="$ASSURANCE_POINTER" -v result_version="$result_version" ' /^---$/ { fence++ print next } fence == 1 && /^gate_result_version:/ { - print "gate_result_version: pr_gate_result_v2" + print "gate_result_version: " result_version print "gate_assurance: " pointer next } @@ -4457,8 +4913,8 @@ gate_finalize_assurance() { } # Publish the sidecar before the v2 Markdown result that references it. A verifier # racing this boundary sees either the original self-contained v1 result or - # the complete v2 pair; a host failure cannot strand a v2 result with a - # permanently missing sidecar. + # the complete bound pair; a host failure cannot strand a v2/v3 result with + # a permanently missing sidecar. mv -- "$assurance_tmp" "$assurance_file" || { rm -f -- "$assurance_tmp" "$result_tmp" return 1 @@ -4766,7 +5222,7 @@ done <<< "$ALL_REVIEW_FILES" DIFF_FILE_ENTRIES="${DIFF_FILE_ENTRIES} - read: ${SCOPE_MANIFEST_PATH}"$'\n' printf -v SCOPE_MANIFEST_CONTEXT_BLOCK \ - ' Declared scope manifest (machine-owned; use this digest in every coverage claim):\n status: %s\n artifact: %s\n artifact_sha256: %s\n content_digest: %s\n subject_fingerprint: %s\n expansion_claim: bounded-hints-not-complete-call-graph\n' \ + ' Declared scope manifest (machine-owned; use this digest in every coverage claim):\n status: %s\n artifact: %s\n artifact_sha256: %s\n content_digest: %s\n subject_fingerprint: %s\n expansion_claim: bounded-hints-not-complete-call-graph\n reference_claim: declared-review-reference-set\n' \ "$SCOPE_MANIFEST_STATUS" "$SCOPE_MANIFEST_PATH" "$SCOPE_MANIFEST_DIGEST" \ "$SCOPE_MANIFEST_CONTENT_DIGEST" "$GATE_BINDING_SUBJECT_FINGERPRINT" say 'pr-gate: scope manifest status=%s sha256=%s expansions=%s artifact=%s\n' \ @@ -4792,6 +5248,52 @@ if [[ "$SCOPE_MANIFEST_STATUS" == incomplete ]]; then exit 3 fi +REVIEWER_PROTOCOL_SURFACES="$( + _gate_reviewer_protocol_surfaces | awk ' + NR == 1 { out=$0; next } + { out=out "," $0 } + END { print out } + ' +)" || exit 2 +# shellcheck disable=SC2016 # Literal Markdown fence delimiters in reviewer prose. +printf -v REVIEWER_PROTOCOL_INSTRUCTIONS \ + '%s\n' \ + ' Selected-reviewer protocol (mandatory; protocol completeness is machine-validated):' \ + ' - Emit exactly one JSON block opened by ```reviewer_result_v1 and closed by ```.' \ + ' - kind=gate_reviewer_result_v1, schema_version=1, reviewer=.' \ + ' - The JSON object has exactly these nine top-level keys: kind, schema_version,' \ + ' reviewer, scope_manifest_sha256, coverage_claim, coverage, findings, verdict,' \ + ' rationale. No role-specific or legacy top-level keys are allowed.' \ + " - scope_manifest_sha256=${SCOPE_MANIFEST_DIGEST}." \ + ' - Every evidence_refs[].path and finding source.path must be either a path in' \ + ' reference_index.entries[] from the declared scope manifest or the manifest' \ + " reference .gate-results/$(basename "$SCOPE_MANIFEST_PATH")." \ + ' A line reference must not exceed that index entry line_count. Arbitrary,' \ + ' nonexistent, or out-of-scope repository paths make the protocol INCOMPLETE.' \ + ' - coverage_claim=declared-scope-checklist-not-review-completeness.' \ + " - coverage contains each surface exactly once: ${REVIEWER_PROTOCOL_SURFACES}." \ + ' - Every coverage entry has surface, status=examined|not_applicable|uncertain,' \ + ' evidence_refs, and a non-empty reason. Every evidence ref has a relative path' \ + ' plus line or symbol (both are allowed). examined requires at least one ref;' \ + ' never use silence for not_applicable or uncertain.' \ + ' - Continue through every coverage surface after finding a blocker; do not early-stop.' \ + ' - findings is an array. Each finding requires id=-FNNN, reviewer,' \ + ' using the exact prefix critic-FNNN, qa-tester-FNNN,' \ + ' architecture-reviewer-FNNN, security-reviewer-FNNN, or' \ + ' risk-reviewer-FNNN. Abbreviations such as risk-F001 are invalid.' \ + ' severity=critical|high|medium|low, hard_gate_class=none|soft_block|hard_block,' \ + ' origin=diff_caused|pre_existing|uncertain|caution, source={path,line,symbol},' \ + ' affected_behavior, why_it_matters, failure_mode, minimum_fix_boundary, and' \ + ' verification_expectation. source needs path plus line or symbol.' \ + ' - pre_existing/caution findings are non-blocking. block-soft needs a soft_block' \ + ' finding; block needs a hard_block finding; approve/advise may contain only none.' \ + ' - verdict is exactly approve|advise|block-soft|block. Map legacy pass and' \ + ' pass-not-applicable to approve; map needs-tests to block. Never put pass,' \ + ' pass-not-applicable, needs-tests, or prose in verdict; prose belongs in rationale.' \ + ' - Legacy fields such as status, summary, matrix, run, audit_findings, over_scope,' \ + ' missed, alignment, reversibility, and override_path must not appear at top level.' \ + ' - Do not claim semantic completeness or coverage for an unselected reviewer.' + if [[ "$SKIP_PREFLIGHT_TESTS" != "true" && -n "$TEST_CMD_OVERRIDE" ]]; then # pr-gate.sh is designed to be copied standalone into any repo (copy-mode -- # see the file header), so it must not hardcode any repo-specific test @@ -5154,6 +5656,8 @@ constraints: You will write to this same file multiple times in this session (once per reviewer, then once for synthesis) -- that is expected. Do not create or write any other file. - Create parent directories for ${OUTPUT_FILE} if needed (mkdir -p). - Only cite files in the verified reference index or the diff list. Read a file before citing its sections; do not invent citations. + - reviewer_result_v1.verdict is the only machine verdict. Markdown reviewer + headings are presentation only; do not rely on their count or wording. context: Tier: ${TIER} @@ -5164,6 +5668,7 @@ context: Scope: ${SCOPE:-none} Date: $(date '+%Y-%m-%d') ${GATE_ASSURANCE_CONTEXT_BLOCK}${SCOPE_MANIFEST_CONTEXT_BLOCK}${GATE_OVERRIDES_CONTEXT_BLOCK}${TEST_EVIDENCE_CONTEXT_BLOCK} +${REVIEWER_PROTOCOL_INSTRUCTIONS} ${MEMORY_CONTEXT_BLOCK} Verified reference files (exist in working tree -- check before citing): ${REPO_REF_INDEX} @@ -5178,7 +5683,7 @@ task: and internalize their specific review criteria and verdict scale. 2. Review the changed files from that reviewer's perspective only. 3. Produce a structured findings block: - - Findings with severity (low/medium/high) and location + - The complete mandatory reviewer_result_v1 coverage/finding JSON block - Explicit verdict: approve | advise | block-soft | block 4. IMMEDIATELY write/append that reviewer's "## {reviewer} -- {verdict}" section to ${OUTPUT_FILE} before moving to the next reviewer. On the FIRST reviewer, create the @@ -5190,6 +5695,8 @@ task: content in-context until the end -- write each section as soon as it is done, so a later reviewer's slowness (e.g. a long test run) cannot destroy earlier reviewers' already-completed verdicts if this session is later interrupted or times out. + The JSON verdict inside reviewer_result_v1 is canonical even if a Markdown + heading is accidentally duplicated or omitted. After all reviewers, synthesize as project-pm would: 5. Identify cross-reviewer overlaps (same issue raised by multiple reviewers) @@ -5227,7 +5734,9 @@ output_format: | **Not reviewed**: ${SKIPPED_DISPLAY} ## {reviewer-name} -- {verdict} - {findings, one per bullet, with [severity] and file:line} + \`\`\`reviewer_result_v1 + {one JSON object satisfying the selected-reviewer protocol above} + \`\`\` (repeat for each reviewer in order) @@ -5301,7 +5810,8 @@ BRIEF_EOF if [[ -s "$OUTPUT_FILE" ]]; then _SEQ_COMPLETED=() _SEQ_INCOMPLETE=() for r in $REVIEWERS; do - if grep -qE "^## ${r} -- " "$OUTPUT_FILE"; then + if _gate_reviewer_protocol_verdict_extract \ + "$OUTPUT_FILE" "$r" >/dev/null 2>&1; then _SEQ_COMPLETED+=("$r") else _SEQ_INCOMPLETE+=("$r") @@ -5322,7 +5832,15 @@ BRIEF_EOF # non-empty, carry exactly one Final: GO|NO-GO line that agrees with the # frontmatter final: field). Same checks the parallel synthesis route and # `pmctl gate verify` enforce. - gate_result_verify "$OUTPUT_FILE" "" "sequential gate" || exit 1 + gate_reviewer_protocol_verify \ + "$OUTPUT_FILE" "$REVIEWERS" "$SCOPE_MANIFEST_DIGEST" \ + "$SCOPE_MANIFEST_PATH" || exit 1 + SEQ_PROTOCOL_FINAL="$( + _gate_reviewer_protocol_final_extract "$OUTPUT_FILE" + )" || exit 1 + REVIEWER_PROTOCOL_COMPLETE=true + gate_result_verify \ + "$OUTPUT_FILE" "$SEQ_PROTOCOL_FINAL" "sequential gate" || exit 1 else @@ -5390,6 +5908,8 @@ constraints: If that call exits nonzero, abort and report the guard denial -- do NOT write the file. - Create parent directories if needed (mkdir -p). - Only cite files in the verified reference index or the diff list. Read a file before citing its sections; do not invent citations. + - reviewer_result_v1.verdict is the only machine verdict. A Markdown heading + is optional presentation and may not act as a second output contract. context: Tier: ${TIER} @@ -5399,6 +5919,7 @@ context: Scope: ${SCOPE:-none} Date: $(date '+%Y-%m-%d') ${GATE_ASSURANCE_CONTEXT_BLOCK}${SCOPE_MANIFEST_CONTEXT_BLOCK}${GATE_OVERRIDES_CONTEXT_BLOCK}${TEST_EVIDENCE_CONTEXT_BLOCK} +${REVIEWER_PROTOCOL_INSTRUCTIONS} ${MEMORY_CONTEXT_BLOCK} Verified reference files (exist in working tree -- check before citing): ${REPO_REF_INDEX} @@ -5411,8 +5932,8 @@ task: 2. Review the changed files strictly from the ${r} perspective only. Do not attempt to cover other reviewer dimensions. 3. Write a structured findings block with: - - Findings: [severity] file:line -- description (low/medium/high) - - Exactly one heading with the canonical verdict: + - The complete mandatory reviewer_result_v1 coverage/finding JSON block + - Optionally one human-readable heading: ## ${r} -- approve | advise | block-soft | block - One-sentence rationale for your verdict @@ -5420,16 +5941,17 @@ task: output_format: | ## ${r} -- {verdict} - {structured findings that follow the agent definition} + \`\`\`reviewer_result_v1 + {one JSON object satisfying the selected-reviewer protocol above} + \`\`\` - The heading is the machine verdict. If you also emit an upper-case - Verdict: line, it must appear exactly once and match the heading. + The JSON verdict is canonical. Do not emit an upper-case Verdict: marker. self_verify: - cmd: "test -f ${REVIEWER_OUTPUT}" acceptance: - - ${REVIEWER_OUTPUT} exists with exactly one canonical ${r} verdict heading + - ${REVIEWER_OUTPUT} exists with exactly one reviewer_result_v1 JSON block RBRIEF_EOF REVIEWER_DISPATCH_CMD="$(dispatch_via "$EXECUTOR" "$REVIEWER_BRIEF" "$WORK_DIR" "$DISPATCH_MODEL" "$DISPATCH_SANDBOX" "$DISPATCH_APPROVAL" "$TIMEOUT" "$DISPATCH_ISOLATION" "$DISPATCH_EFFORT")" || exit 2 @@ -5513,26 +6035,28 @@ RBRIEF_EOF exit 1 fi - # Verify every reviewer output contains one unambiguous canonical verdict - # before synthesis. The role-matched heading is authoritative; an optional - # upper-case Verdict marker must be unique and agree with it. - INVALID_OUTPUTS=() + # Validate the schema-complete reviewer contract before synthesis and read + # its JSON verdict. Markdown headings are presentation only: duplicate or + # omitted headings cannot turn a completed review into a protocol failure. + PROTOCOL_INVALID_OUTPUTS=() REVIEWER_VERDICTS=() for i in "${!REVIEWER_OUTPUT_FILES[@]}"; do rf="${REVIEWER_OUTPUT_FILES[$i]}" r="${REVIEWER_NAMES[$i]}" - if reviewer_verdict="$(_gate_reviewer_verdict_extract "$r" "$rf")"; then + if gate_reviewer_protocol_verify \ + "$rf" "$r" "$SCOPE_MANIFEST_DIGEST" "$SCOPE_MANIFEST_PATH" \ + && reviewer_verdict="$( + _gate_reviewer_protocol_verdict_extract "$rf" "$r" + )"; then REVIEWER_VERDICTS+=("$reviewer_verdict") else - INVALID_OUTPUTS+=("$r") + PROTOCOL_INVALID_OUTPUTS+=("$r") fi done - if [[ "${#INVALID_OUTPUTS[@]}" -gt 0 ]]; then - printf 'Error: reviewer output has an invalid or ambiguous canonical verdict for: %s\n' \ - "${INVALID_OUTPUTS[*]}" >&2 - printf 'Expected: exactly one matching heading: ## -- approve|advise|block-soft|block\n' >&2 - printf 'Any upper-case Verdict: marker must be unique and match that heading.\n' >&2 - printf 'Gate aborted -- use --sequential to diagnose.\n' >&2 + if [[ "${#PROTOCOL_INVALID_OUTPUTS[@]}" -gt 0 ]]; then + printf 'Error: reviewer protocol INCOMPLETE for: %s\n' \ + "${PROTOCOL_INVALID_OUTPUTS[*]}" >&2 + printf 'Every selected reviewer must complete the declared-surface checklist and actionable finding contract.\n' >&2 exit 1 fi @@ -5616,6 +6140,8 @@ constraints: - The Gate Conclusion MUST contain exactly: Final: ${SHELL_FINAL} This is pre-computed from the reviewer verdicts and must not be overridden. - Only cite files in the verified reference index or reviewer findings; do not invent citations. + - Do not emit or copy any reviewer_result_v1 fenced block. The gate shell + validates and appends the original reviewer protocol blocks after synthesis. context: Tier: ${TIER} @@ -5681,7 +6207,8 @@ output_format: | **Not reviewed**: ${SKIPPED_DISPLAY} ## {reviewer-name} -- {verdict} - {Copy findings from that reviewer's findings block above, one bullet per finding with [severity] and file:line} + {Summarize findings from that reviewer, one bullet per finding with stable ID, + severity, and file:line. Do not copy the reviewer_result_v1 fenced block.} Verdict: {verdict from reviewer findings}. {rationale} @@ -5791,6 +6318,13 @@ SBRIEF_P2 printf 'Error: synthesis session modified working tree -- possible prompt injection.\n' >&2 exit 1 fi + + _gate_reviewer_protocol_append_blocks \ + "$OUTPUT_FILE" "${REVIEWER_OUTPUT_FILES[@]}" || exit 1 + gate_reviewer_protocol_verify \ + "$OUTPUT_FILE" "$REVIEWERS" "$SCOPE_MANIFEST_DIGEST" \ + "$SCOPE_MANIFEST_PATH" || exit 1 + REVIEWER_PROTOCOL_COMPLETE=true fi fi @@ -5930,10 +6464,12 @@ elif [[ -x "$_POST_GATE_HOOK" ]]; then fi fi -# Replace the executor-authored staging frontmatter with a v2 pointer and write -# the machine-owned assurance sidecar only after every deterministic rewrite -# and explicitly enabled post-gate hook is complete. The shared verifier then -# checks result/pointer/envelope parity before publication or relocation. +# Replace the executor-authored staging frontmatter with a bound pointer and +# write the machine-owned assurance sidecar only after every deterministic +# rewrite and explicitly enabled post-gate hook is complete. Completed reviewer +# routes publish result v3; pre-dispatch fail-fast routes without reviewer +# protocol remain v2. The shared verifier then checks result/pointer/envelope +# parity before publication or relocation. gate_finalize_assurance "$OUTPUT_FILE" "$ASSURANCE_FILE" || exit 2 # ── Relocate result to run dir (post-verification) ─────────────────────────── diff --git a/runtime/lib/gate-result-verify.sh b/runtime/lib/gate-result-verify.sh index 22f0b1df..1ab516a7 100644 --- a/runtime/lib/gate-result-verify.sh +++ b/runtime/lib/gate-result-verify.sh @@ -4,7 +4,9 @@ # pr_gate_result_v1 remains valid legacy structural evidence. It proves only # frontmatter/body verdict parity and is reported as assurance=unavailable. # pr_gate_result_v2 points at a sibling assurance JSON envelope owned by the -# gate shell. Legacy gate_assurance_v1 remains readable but non-authorizing; +# gate shell. pr_gate_result_v3 additionally requires one schema-complete +# reviewer protocol block per selected reviewer. Legacy gate_assurance_v1 +# remains readable but non-authorizing; # gate_assurance_v2 adds result bindings and protected dispatch attestation. # gate_assurance_v3 adds an immutable subject plus digest-bound evidence links. @@ -55,6 +57,311 @@ _gate_result_frontmatter_value() { ' "$result_file" } +_gate_reviewer_protocol_surfaces() { + printf '%s\n' \ + changed_files \ + paired_tests \ + sensitive_signals \ + public_interface \ + schema \ + config \ + install \ + ci \ + release \ + migration \ + bounded_expansion +} + +_gate_reviewer_protocol_reference_index_json() { + local manifest_file="$1" expected_sha="$2" + local actual_sha manifest_ref line_count + if [[ ! -f "$manifest_file" || -L "$manifest_file" ]]; then + printf 'Error: reviewer protocol reference manifest is missing or unsafe: %s\n' \ + "$manifest_file" >&2 + return 1 + fi + actual_sha="$(_gate_result_sha256_file "$manifest_file")" || return $? + if [[ "$actual_sha" != "$expected_sha" ]]; then + printf 'Error: reviewer protocol reference manifest digest mismatch: %s\n' \ + "$manifest_file" >&2 + return 1 + fi + manifest_ref=".gate-results/$(basename "$manifest_file")" + line_count="$(awk 'END { print NR+0 }' "$manifest_file")" + jq -c \ + --arg manifest_ref "$manifest_ref" \ + --arg manifest_sha "$actual_sha" \ + --argjson manifest_lines "$line_count" ' + if .reference_index.claim != "declared-review-reference-set" or + (.reference_index.entries | type) != "array" or + (.reference_index.entries | length) == 0 + then error("missing reviewer reference index: claim=" + + (.reference_index.claim // "") + " entries=" + + ((.reference_index.entries // []) | length | tostring)) + else + ([.reference_index.entries[] | { + path:.path, + line_count:.line_count, + sha256:.sha256 + }] + [{ + path:$manifest_ref, + line_count:$manifest_lines, + sha256:$manifest_sha + }] | unique_by(.path) | sort_by(.path)) + end + ' "$manifest_file" || { + printf 'Error: reviewer protocol reference manifest has no valid index: %s\n' \ + "$manifest_file" >&2 + return 1 + } +} + +_gate_reviewer_protocol_document_verify() { + local document="$1" expected_reviewer="$2" expected_scope_sha="$3" + local reference_index_json="${4:-null}" + local surfaces_json validation + GATE_REVIEWER_PROTOCOL_DOCUMENT_ERROR="" + surfaces_json="$(_gate_reviewer_protocol_surfaces | jq -Rsc ' + split("\n") | map(select(length > 0)) + ')" || return 2 + validation="$(jq -r \ + --arg reviewer "$expected_reviewer" \ + --arg scope_sha "$expected_scope_sha" \ + --argjson surfaces "$surfaces_json" \ + --argjson references "$reference_index_json" ' + def only_keys($allowed): + type == "object" and ((keys_unsorted - $allowed) | length) == 0; + def exact_keys($required): + type == "object" and ((keys_unsorted | sort) == ($required | sort)); + def nonempty: type == "string" and length > 0; + def relative_path: + nonempty and (startswith("/") | not) and + (test("(^|/)\\.\\.(/|$)") | not); + def evidence_ref: + only_keys(["path","line","symbol"]) and + (.path | relative_path) and + (.line == null or (.line | type == "number" and . >= 1 and floor == .)) and + (.symbol == null or (.symbol | nonempty)) and + (.line != null or .symbol != null); + def coverage_entry: + only_keys(["surface","status","evidence_refs","reason"]) and + (.surface | IN($surfaces[])) and + (.status | IN("examined","not_applicable","uncertain")) and + (.reason | nonempty) and + (.evidence_refs | type == "array" and all(.[]; evidence_ref)) and + (if .status == "examined" + then (.evidence_refs | length) > 0 + else true + end); + def finding: + only_keys(["id","reviewer","severity","hard_gate_class","origin","source", + "affected_behavior","why_it_matters","failure_mode", + "minimum_fix_boundary","verification_expectation"]) and + .reviewer == $reviewer and + (.id | test("^" + $reviewer + "-F[0-9]{3}$")) and + (.severity | IN("critical","high","medium","low")) and + (.hard_gate_class | IN("none","soft_block","hard_block")) and + (.origin | IN("diff_caused","pre_existing","uncertain","caution")) and + (.source | evidence_ref) and + (.affected_behavior | nonempty) and + (.why_it_matters | nonempty) and + (.failure_mode | nonempty) and + (.minimum_fix_boundary | nonempty) and + (.verification_expectation | nonempty) and + (if .hard_gate_class == "none" + then true + else (.severity | IN("critical","high")) and + (.origin | IN("diff_caused","uncertain")) + end) and + (if (.origin | IN("pre_existing","caution")) + then .hard_gate_class == "none" + else true + end); + def envelope_contract: + exact_keys(["kind","schema_version","reviewer","scope_manifest_sha256", + "coverage_claim","coverage","findings","verdict","rationale"]) and + .kind == "gate_reviewer_result_v1" and .schema_version == 1 and + .reviewer == $reviewer and .scope_manifest_sha256 == $scope_sha and + .coverage_claim == "declared-scope-checklist-not-review-completeness"; + def coverage_contract: + .coverage | type == "array" and length == ($surfaces | length) and + all(.[]; coverage_entry) and + ([.[].surface] | sort) == ($surfaces | sort); + def finding_contract: + .findings | type == "array" and all(.[]; finding) and + ([.[].id] | length) == ([.[].id] | unique | length); + def verdict_contract: + (.verdict | IN("approve","advise","block-soft","block")) and + (.rationale | nonempty) and + (if .verdict == "block-soft" + then any(.findings[]; .hard_gate_class == "soft_block") + elif .verdict == "block" + then any(.findings[]; .hard_gate_class == "hard_block") + else all(.findings[]; .hard_gate_class == "none") + end); + def bound_evidence_ref: + .path as $path | + ($references | map(select(.path == $path)) | first) as $entry | + $entry != null and + (if .line != null then .line <= $entry.line_count else true end); + def evidence_reference_contract: + all(.coverage[].evidence_refs[]; bound_evidence_ref) and + all(.findings[].source; bound_evidence_ref); + if (envelope_contract | not) + then "invalid top-level or binding contract" + elif (coverage_contract | not) + then "invalid coverage contract" + elif (finding_contract | not) + then "invalid finding contract" + elif $references != null and (evidence_reference_contract | not) + then "invalid evidence reference contract" + elif (verdict_contract | not) + then "invalid verdict contract" + else "ok" + end + ' "$document" 2>/dev/null)" || { + GATE_REVIEWER_PROTOCOL_DOCUMENT_ERROR="invalid JSON document" + return 1 + } + if [[ "$validation" != ok ]]; then + GATE_REVIEWER_PROTOCOL_DOCUMENT_ERROR="$validation" + return 1 + fi +} + +_gate_reviewer_protocol_documents() { + local artifact="$1" + awk ' + $0 == "```reviewer_result_v1" { + if (inside) exit 2 + inside=1 + next + } + inside && $0 == "```" { + inside=0 + print "" + next + } + inside { print } + END { if (inside) exit 2 } + ' "$artifact" +} + +_gate_reviewer_protocol_verdict_extract() { + local artifact="$1" reviewer="$2" verdicts count + verdicts="$( + _gate_reviewer_protocol_documents "$artifact" | + jq -r --arg reviewer "$reviewer" \ + 'select(.reviewer == $reviewer) | .verdict // empty' + )" || return 1 + count="$(printf '%s\n' "$verdicts" | awk 'NF { count++ } END { print count+0 }')" + [[ "$count" -eq 1 ]] || return 1 + printf '%s\n' "$verdicts" +} + +_gate_reviewer_protocol_final_extract() { + local artifact="$1" + _gate_reviewer_protocol_documents "$artifact" | + jq -sr ' + map(.verdict) as $verdicts | + if ($verdicts | any(. == "block" or . == "block-soft")) + then "NO-GO" + else "GO" + end + ' +} + +# gate_reviewer_protocol_verify +# [scope-manifest] +# +# Extract every exact reviewer_result_v1 fenced JSON block and verify that the +# selected reviewer set is represented once, with the complete declared-surface +# checklist and actionable finding fields. When the linked scope manifest has a +# reference index, every evidence path and line is bound to that immutable set. +# The optional fourth argument preserves readability for legacy v3 artifacts +# produced before reference indexes existed. +gate_reviewer_protocol_verify() { + local artifact=${1-} selected=${2-} scope_sha=${3-} scope_manifest=${4-} + local tmp_dir line block="" in_block=false count=0 reviewer expected document + local seen=" " reference_index_json=null + [[ $# -ge 3 && $# -le 4 && -s "$artifact" && -n "$selected" \ + && "$scope_sha" =~ ^[a-f0-9]{64}$ ]] || { + printf 'Error: reviewer protocol INCOMPLETE: invalid verifier inputs\n' >&2 + return 2 + } + if [[ -n "$scope_manifest" ]]; then + reference_index_json="$( + _gate_reviewer_protocol_reference_index_json \ + "$scope_manifest" "$scope_sha" + )" || return $? + fi + tmp_dir="$(mktemp -d "${TMPDIR:-/tmp}/gate-reviewer-protocol.XXXXXX")" \ + || return 2 + while IFS= read -r line || [[ -n "$line" ]]; do + if [[ "$line" == '```reviewer_result_v1' ]]; then + if [[ "$in_block" == true ]]; then + printf 'Error: reviewer protocol INCOMPLETE: nested result block in %s\n' \ + "$artifact" >&2 + rm -rf -- "$tmp_dir" + return 1 + fi + in_block=true + block="" + continue + fi + if [[ "$in_block" == true && "$line" == '```' ]]; then + count=$((count + 1)) + printf '%s\n' "$block" > "$tmp_dir/$count.json" + in_block=false + block="" + continue + fi + if [[ "$in_block" == true ]]; then + block="${block}${block:+$'\n'}${line}" + fi + done < "$artifact" + if [[ "$in_block" == true ]]; then + printf 'Error: reviewer protocol INCOMPLETE: unclosed result block in %s\n' \ + "$artifact" >&2 + rm -rf -- "$tmp_dir" + return 1 + fi + if [[ "$count" -eq 0 ]]; then + printf 'Error: reviewer protocol INCOMPLETE: no reviewer_result_v1 block in %s\n' \ + "$artifact" >&2 + rm -rf -- "$tmp_dir" + return 1 + fi + for document in "$tmp_dir"/*.json; do + reviewer="$(jq -r '.reviewer // empty' "$document" 2>/dev/null)" || reviewer="" + if [[ -z "$reviewer" || " $selected " != *" $reviewer "* \ + || "$seen" == *" $reviewer "* ]]; then + printf 'Error: reviewer protocol INCOMPLETE: unexpected or duplicate reviewer %s in %s\n' \ + "${reviewer:-}" "$artifact" >&2 + rm -rf -- "$tmp_dir" + return 1 + fi + if ! _gate_reviewer_protocol_document_verify \ + "$document" "$reviewer" "$scope_sha" "$reference_index_json"; then + printf 'Error: reviewer protocol INCOMPLETE: %s for %s in %s\n' \ + "${GATE_REVIEWER_PROTOCOL_DOCUMENT_ERROR:-invalid reviewer document}" \ + "$reviewer" "$artifact" >&2 + rm -rf -- "$tmp_dir" + return 1 + fi + seen="${seen}${reviewer} " + done + for expected in $selected; do + if [[ "$seen" != *" $expected "* ]]; then + printf 'Error: reviewer protocol INCOMPLETE: missing selected reviewer %s in %s\n' \ + "$expected" "$artifact" >&2 + rm -rf -- "$tmp_dir" + return 1 + fi + done + rm -rf -- "$tmp_dir" +} + _gate_result_sha256_stream() { if command -v sha256sum >/dev/null 2>&1; then sha256sum | awk '{print $1}' @@ -362,9 +669,10 @@ gate_scope_manifest_verify() { (.matched == ((.paths | length) > 0)) and all(.paths[]; . as $path | ($changed | index($path)) != null); - only_keys(["kind","schema_version","status","subject","selection", - "changes","diff","paired_tests","sensitive_signals","flags", - "expansion","truncation","content"]) and + ((keys - ["reference_index"]) | sort) == + (["kind","schema_version","status","subject","selection", + "changes","diff","paired_tests","sensitive_signals","flags", + "expansion","truncation","content"] | sort) and .kind == "gate_scope_manifest_v1" and .schema_version == 1 and (.status | IN("complete","accepted_truncation","incomplete")) and (.subject | @@ -474,9 +782,9 @@ gate_scope_manifest_verify() { ($flags.ci | scope_flag($changed)) and ($flags.release | scope_flag($changed)) and ($flags.migration | scope_flag($changed)))) and - (.expansion | - only_keys(["claim","entries","included_paths"]) and - .claim == "bounded-hints-not-complete-call-graph" and + (.expansion | + only_keys(["claim","entries","included_paths"]) and + .claim == "bounded-hints-not-complete-call-graph" and (.entries | type == "array" and all(.[]; only_keys(["path","reason","source","evidence","limit"]) and @@ -489,10 +797,29 @@ gate_scope_manifest_verify() { only_keys(["kind","maximum"]) and (.kind | IN("per-source","per-symbol","global")) and (.maximum | type == "number" and . >= 1 and floor == .)))) and - (.included_paths | paths_unique) and - (. as $expansion | - ([.entries[].path] | unique | exact_set($expansion.included_paths)))) and - (. as $manifest | .truncation | + (.included_paths | paths_unique) and + (. as $expansion | + ([.entries[].path] | unique | exact_set($expansion.included_paths)))) and + (. as $manifest | + ([$manifest.changes.changed_paths[]] + + [$manifest.paired_tests[] | .source_path,.test_path] + + [$manifest.sensitive_signals[] | .matches[]] + + [$manifest.flags[] | .paths[]] + + [$manifest.expansion.included_paths[]] | unique) as $allowed | + ($manifest.reference_index == null or + ($manifest.reference_index | + only_keys(["claim","entries"]) and + .claim == "declared-review-reference-set" and + (.entries | type == "array" and length > 0) and + ([.entries[].path] | strings_unique) and + all(.entries[]; + only_keys(["path","snapshot","line_count","sha256"]) and + (.path | safe_path) and + (.path as $path | ($allowed | index($path)) != null) and + (.snapshot | IN("subject","base")) and + (.line_count | type == "number" and . >= 0 and floor == .) and + (.sha256 | test("^[a-f0-9]{64}$")))))) and + (. as $manifest | .truncation | only_keys(["occurred","budgets","omitted","reasons","acceptance"]) and (.occurred | type == "boolean") and (.budgets | scope_counts and all(.[]; . >= 1)) and @@ -1164,6 +1491,8 @@ gate_assurance_authorization_verify() { gate_result_verify() { local result_file=${1-} expected_final=${2-} route_label=${3-gate} local version pointer result_parent assurance_file body_final + local selected_reviewers scope_sha scope_artifact scope_manifest + local assurance_kind protocol_final [[ $# -ge 1 && $# -le 3 ]] || { printf 'gate-result-verify: gate_result_verify expects [expected_final] [route_label]\n' >&2 return 2 @@ -1177,18 +1506,54 @@ gate_result_verify() { export GATE_RESULT_ASSURANCE return 0 ;; - pr_gate_result_v2) + pr_gate_result_v2 | pr_gate_result_v3) pointer="$(_gate_result_frontmatter_value "$result_file" gate_assurance)" if [[ -z "$pointer" || "$pointer" == */* || "$pointer" == "." || "$pointer" == ".." \ || ! "$pointer" =~ ^[A-Za-z0-9._-]+\.json$ ]]; then - printf 'Error: pr_gate_result_v2 requires a bounded sibling gate_assurance pointer: %s\n' \ - "$result_file" >&2 + printf 'Error: %s requires a bounded sibling gate_assurance pointer: %s\n' \ + "$version" "$result_file" >&2 return 1 fi result_parent="$(cd "$(dirname "$result_file")" && pwd -P)" || return 1 assurance_file="$result_parent/$pointer" body_final=$(grep -E '^Final: (GO|NO-GO)$' "$result_file" | awk '{print $2}') gate_assurance_verify "$result_file" "$assurance_file" "$body_final" || return $? + if [[ "$version" == pr_gate_result_v3 ]]; then + assurance_kind="$(jq -r '.kind // empty' "$assurance_file" 2>/dev/null)" + selected_reviewers="$(jq -r \ + '.coordinates.coverage.selected // [] | join(" ")' \ + "$assurance_file" 2>/dev/null)" + scope_sha="$(jq -r \ + '.evidence.scope_manifest.sha256 // empty' \ + "$assurance_file" 2>/dev/null)" + scope_artifact="$(jq -r \ + '.evidence.scope_manifest.artifact // empty' \ + "$assurance_file" 2>/dev/null)" + scope_manifest="$result_parent/$scope_artifact" + if [[ "$assurance_kind" != gate_assurance_v3 \ + || -z "$selected_reviewers" \ + || ! "$scope_sha" =~ ^[a-f0-9]{64}$ ]]; then + printf 'Error: pr_gate_result_v3 requires verified selected-reviewer scope evidence: %s\n' \ + "$assurance_file" >&2 + return 1 + fi + if jq -e '.reference_index != null' "$scope_manifest" >/dev/null 2>&1; then + gate_reviewer_protocol_verify \ + "$result_file" "$selected_reviewers" "$scope_sha" \ + "$scope_manifest" || return $? + else + gate_reviewer_protocol_verify \ + "$result_file" "$selected_reviewers" "$scope_sha" || return $? + fi + protocol_final="$( + _gate_reviewer_protocol_final_extract "$result_file" + )" || return 1 + if [[ "$protocol_final" != "$body_final" ]]; then + printf 'Error: reviewer protocol verdict (%s) contradicts gate Final: (%s): %s\n' \ + "$protocol_final" "$body_final" "$result_file" >&2 + return 1 + fi + fi if [[ "${GATE_ASSURANCE_BOUND:-false}" == true ]]; then GATE_RESULT_ASSURANCE=verified else diff --git a/runtime/lib/pmctl-gate.sh b/runtime/lib/pmctl-gate.sh index 6f9bda85..541f2e63 100644 --- a/runtime/lib/pmctl-gate.sh +++ b/runtime/lib/pmctl-gate.sh @@ -80,7 +80,7 @@ _pmctl_gate_result_verifier_load() { && declare -F _gate_result_sha256_file >/dev/null 2>&1 } -# A v2 result and its bound sidecar cannot be renamed into place as one +# A bound result and its sidecar cannot be renamed into place as one # filesystem operation. If a verifier races the producer between those # renames, wait briefly for the sidecar (and, for verified independence, # its protected attestation) instead of turning a valid in-flight @@ -91,7 +91,8 @@ _pmctl_gate_wait_for_assurance_publication() { local attempt=0 max_attempts=20 version="$(_gate_result_frontmatter_value "$result_file" gate_result_version)" - [[ "$version" == pr_gate_result_v2 ]] || return 0 + [[ "$version" == pr_gate_result_v2 || "$version" == pr_gate_result_v3 ]] \ + || return 0 result_parent="$(cd "$(dirname "$result_file")" 2>/dev/null && pwd -P)" || return 0 # Only canonical run-layout results have an asynchronous producer # finalization lifecycle. Ad-hoc and copy-mode results fail immediately. diff --git a/tests/bin/run-tests.sh b/tests/bin/run-tests.sh index a7b4ed9c..1f1c77f6 100755 --- a/tests/bin/run-tests.sh +++ b/tests/bin/run-tests.sh @@ -217,11 +217,16 @@ map_path() { add_suite test-pmctl-task; behavioral=1 ;; core/schema/preflight-evidence.schema.json) add_suite test-pr-gate; behavioral=1 ;; - core/schema/gate-assurance.schema.json|core/schema/gate-policy-override.schema.json|core/schema/gate-scope-manifest.schema.json|core/schema/gate-verification.schema.json) + core/schema/gate-assurance.schema.json|core/schema/gate-policy-override.schema.json|core/schema/gate-reviewer-result.schema.json|core/schema/gate-scope-manifest.schema.json|core/schema/gate-verification.schema.json) add_suite test-core-schemas; add_suite test-pr-gate add_suite test-pmctl-gate; behavioral=1 ;; + runtime/bin/pr-gate.sh) + add_suite test-pr-gate-profile; behavioral=1 ;; runtime/lib/gate-result-verify.sh) - add_suite test-pr-gate; add_suite test-pmctl-gate; behavioral=1 ;; + add_suite test-pr-gate; add_suite test-pr-gate-profile + add_suite test-pmctl-gate; behavioral=1 ;; + tests/lib/test-pr-gate-fixture.sh) + add_suite test-pr-gate; add_suite test-pr-gate-profile; behavioral=1 ;; tools/generate-gate-result-verifier-fallback.sh) add_suite test-pr-gate; behavioral=1 ;; core/policy/gate-tiers.tsv|core/policy/gate-modes.tsv|core/policy/gate-pass-kinds.tsv|core/policy/gate-policy-consumers.tsv|core/policy/gate-policy-signals.tsv) diff --git a/tests/lib/test-pr-gate-fixture.sh b/tests/lib/test-pr-gate-fixture.sh new file mode 100644 index 00000000..1827afc9 --- /dev/null +++ b/tests/lib/test-pr-gate-fixture.sh @@ -0,0 +1,183 @@ +#!/usr/bin/env bash +# Shared test fixtures for the pr-gate shell suites. + +create_agents() { + local home="$1" + shift + mkdir -p "$home/.claude/agents" + local reviewer + for reviewer in "$@"; do + printf '# %s\n\nReviewer fixture.\n' "$reviewer" \ + > "$home/.claude/agents/$reviewer.md" + done +} + +pr_gate_fixture_write_reviewer_protocol() { + local brief_file="$1" output_path="$2" reviewer="$3" verdict="$4" + local mutation="${5:-${CODEX_GATE_STUB_PROTOCOL_MUTATION:-none}}" + local scope_sha scope_path evidence_path hard_gate_class findings_json + scope_sha="$(awk '$1 == "artifact_sha256:" { print $2; exit }' "$brief_file")" + scope_path="$(awk '$1 == "artifact:" { print $2; exit }' "$brief_file")" + evidence_path=".gate-results/$(basename "$scope_path")" + hard_gate_class=none + case "$verdict" in + block-soft) hard_gate_class=soft_block ;; + block) hard_gate_class=hard_block ;; + esac + findings_json='[]' + if [[ "$hard_gate_class" != none ]]; then + findings_json="$(jq -nc \ + --arg id "${reviewer}-F001" --arg reviewer "$reviewer" \ + --arg evidence_path "$evidence_path" \ + --arg hard_gate_class "$hard_gate_class" '[{ + id:$id, + reviewer:$reviewer, + severity:"high", + hard_gate_class:$hard_gate_class, + origin:"diff_caused", + source:{path:$evidence_path,line:1,symbol:null}, + affected_behavior:"The changed behavior is blocked by the fixture finding.", + why_it_matters:"The fixture models an actionable gate failure.", + failure_mode:"The reviewed behavior fails when exercised.", + minimum_fix_boundary:"Correct the changed behavior without expanding scope.", + verification_expectation:"Run the focused regression for the changed behavior." + }]')" + fi + if [[ "$mutation" == invalid-id ]]; then + findings_json="$(jq -nc --arg reviewer "$reviewer" \ + --arg evidence_path "$evidence_path" '[{ + id:"invalid-id", + reviewer:$reviewer, + severity:"low", + hard_gate_class:"none", + origin:"caution", + source:{path:$evidence_path,line:1,symbol:null}, + affected_behavior:"The fixture finding exercises stable-ID validation.", + why_it_matters:"Unstable finding IDs cannot seed deterministic remediation.", + failure_mode:"Downstream consumers cannot preserve finding identity.", + minimum_fix_boundary:"Emit the reviewer-prefixed stable identifier.", + verification_expectation:"Validate the corrected protocol artifact." + }]')" + fi + jq -nc \ + --arg reviewer "$reviewer" \ + --arg scope_sha "$scope_sha" \ + --arg evidence_path "$evidence_path" \ + --arg verdict "$verdict" \ + --arg mutation "$mutation" \ + --argjson findings "$findings_json" ' + ["changed_files","paired_tests","sensitive_signals","public_interface", + "schema","config","install","ci","release","migration", + "bounded_expansion"] as $surfaces | + { + kind:"gate_reviewer_result_v1", + schema_version:1, + reviewer:$reviewer, + scope_manifest_sha256:$scope_sha, + coverage_claim:"declared-scope-checklist-not-review-completeness", + coverage:($surfaces | map({ + surface:.,status:"examined", + evidence_refs:[{path:$evidence_path,line:1,symbol:null}], + reason:"Fixture examined this declared surface." + })), + findings:$findings, + verdict:$verdict, + rationale:"Fixture reviewer completed every declared coverage surface." + } + | if $mutation == "missing-surface" then .coverage = .coverage[:-1] + elif $mutation == "evidence-less-blocker" + then .findings[0].source = {path:"",line:null,symbol:null} + elif $mutation == "out-of-scope-reference" + then .coverage[0].evidence_refs[0].path = "not-declared-by-scope.md" + elif $mutation == "out-of-range-line" + then .coverage[0].evidence_refs[0].line = 999999 + elif $mutation == "qa-legacy-pass" and $reviewer == "qa-tester" + then .verdict = "pass" + elif $mutation == "critic-extra-top-level" and $reviewer == "critic" + then .alignment = "Legacy role-specific field." + elif $mutation == "risk-short-id" and $reviewer == "risk-reviewer" + then .findings = [{ + id:"risk-F001", + reviewer:"risk-reviewer", + severity:"low", + hard_gate_class:"none", + origin:"caution", + source:{path:$evidence_path,line:1}, + affected_behavior:"The fixture models an abbreviated finding ID.", + why_it_matters:"Abbreviated IDs cannot preserve reviewer identity.", + failure_mode:"Downstream remediation loses deterministic identity.", + minimum_fix_boundary:"Use the complete reviewer role as the prefix.", + verification_expectation:"Validate the corrected finding ID." + }] + else . + end + ' | { + printf '```reviewer_result_v1\n' + cat + printf '\n```\n' + } >> "$output_path" +} + +pr_gate_fixture_profile_dispatch() { + local executor="$1" + shift + local brief_file="" mode marker output_path reviewer_name selected_csv selected_reviewer + case "$executor" in + codex) + mode="${CODEX_GATE_STUB_MODE:-success}" + marker="${CODEX_GATE_STUB_CALLED_MARKER:-}" + ;; + claude) + mode="${CLAUDE_GATE_STUB_MODE:-success}" + marker="${CLAUDE_GATE_STUB_CALLED_MARKER:-}" + ;; + *) + printf 'unknown profile fixture executor: %s\n' "$executor" >&2 + return 2 + ;; + esac + + while [[ $# -gt 0 ]]; do + case "$1" in + --brief-file) brief_file="$2"; shift 2 ;; + --cd | --timeout | --model | --isolation) shift 2 ;; + *) shift ;; + esac + done + + [[ -n "$marker" ]] && printf 'called\n' > "$marker" + printf 'DISPATCH_STUB:%s\n' "$mode" + [[ "$mode" != exit99 ]] || return 99 + [[ -n "$brief_file" ]] || return 0 + + output_path="" + if [[ -f "$brief_file" ]]; then + output_path="$(grep -o '\- new:.*' "$brief_file" | head -1 | awk '{print $NF}' || true)" + fi + [[ -n "$output_path" ]] || return 0 + mkdir -p "$(dirname "$output_path")" + + if [[ "$brief_file" == *-synthesis.md ]]; then + printf -- '---\ngate_result_version: pr_gate_result_v1\nfinal: GO\ntier: standard\nmode: parallel\nmost_severe: advise\nreviewers:\n critic: advise\nescalation:\n recommended: false\n reviewers: []\n reason: []\n---\n\n# PR-Gate Result — stub tier\n**Date**: 2026-05-17\n**Reviewers**: stub\n**Not reviewed**: none\n\n## cross-check\nnone\n\n## Gate Conclusion\n**Overall verdict**: advise\n**Most severe individual verdict**: advise\nFinal: GO\n' > "$output_path" + return 0 + fi + + reviewer_name="$(awk '$1 == "Reviewer:" { print $2; exit }' "$brief_file")" + if [[ -n "$reviewer_name" ]]; then + printf '## %s -- advise\n\nstatus: advise\nfindings: []\nverdict: Stub output.\n' \ + "$reviewer_name" > "$output_path" + pr_gate_fixture_write_reviewer_protocol \ + "$brief_file" "$output_path" "$reviewer_name" advise + return 0 + fi + + printf -- '---\ngate_result_version: pr_gate_result_v1\nfinal: GO\ntier: standard\nmode: sequential\nmost_severe: advise\nreviewers:\n critic: advise\nescalation:\n recommended: false\n reviewers: []\n reason: []\n---\n\n## stub-reviewer — advise\nVerdict: advise. Stub output.\n' > "$output_path" + selected_csv="$(awk '$1 == "coverage.selected:" { print $2; exit }' "$brief_file")" + : "${selected_csv:=critic}" + for selected_reviewer in ${selected_csv//,/ }; do + printf '\n## %s -- advise\n' "$selected_reviewer" >> "$output_path" + pr_gate_fixture_write_reviewer_protocol \ + "$brief_file" "$output_path" "$selected_reviewer" advise + done + printf 'Final: GO\n' >> "$output_path" +} diff --git a/tests/shell/test-commands.sh b/tests/shell/test-commands.sh index ac672cda..e4cfbc72 100755 --- a/tests/shell/test-commands.sh +++ b/tests/shell/test-commands.sh @@ -234,6 +234,32 @@ for agent_file in "$AGENTS_DIR"/*.md; do "$agent_file" "Output brevity" "English only" done +# ── selected-reviewer JSON contract ────────────────────────────────────────── + +for reviewer_name in critic qa-tester architecture-reviewer security-reviewer risk-reviewer; do + reviewer_file="$AGENTS_DIR/$reviewer_name.md" + should_run "agent/$reviewer_name: shared protocol replaces legacy format" \ + && assert_file_contains \ + "agent/$reviewer_name: shared protocol replaces legacy format" \ + "$reviewer_file" "fully replaces the legacy format" \ + && pass "agent/$reviewer_name: shared protocol replaces legacy format" + should_run "agent/$reviewer_name: protocol has exact verdict enum" \ + && assert_file_contains \ + "agent/$reviewer_name: protocol has exact verdict enum" \ + "$reviewer_file" 'approve|advise|block-soft|block' \ + && pass "agent/$reviewer_name: protocol has exact verdict enum" + should_run "agent/$reviewer_name: role-specific prose is not top-level" \ + && assert_file_contains \ + "agent/$reviewer_name: role-specific prose is not top-level" \ + "$reviewer_file" "top-level keys" \ + && pass "agent/$reviewer_name: role-specific prose is not top-level" + should_run "agent/$reviewer_name: finding ID uses full reviewer prefix" \ + && assert_file_contains \ + "agent/$reviewer_name: finding ID uses full reviewer prefix" \ + "$reviewer_file" "$reviewer_name-FNNN" \ + && pass "agent/$reviewer_name: finding ID uses full reviewer prefix" +done + # ── next-step uncertainty router contract (discover/research/spike family) ──── DISCOVER="$COMMANDS_DIR/discover.md" diff --git a/tests/shell/test-core-schemas.sh b/tests/shell/test-core-schemas.sh index 20fe56bb..7e5bec38 100755 --- a/tests/shell/test-core-schemas.sh +++ b/tests/shell/test-core-schemas.sh @@ -295,6 +295,11 @@ case_enum_sync "$CORE_DIR/schema/review.schema.json" \ "$CORE_DIR/policy/reviewer-policy.yaml" \ "reviewers" +case_enum_sync "$CORE_DIR/schema/gate-reviewer-result.schema.json" \ + '.properties.reviewer.enum' \ + "$CORE_DIR/policy/reviewer-policy.yaml" \ + "reviewers" + # verdicts are reviewer-policy.yaml's verdicts list case_enum_sync "$CORE_DIR/schema/review.schema.json" \ '.properties.findings.items.properties.verdict.enum' \ @@ -815,6 +820,23 @@ _gate_scope_manifest_valid_instance() { }], included_paths:["tests/shell/test-example.sh"] }, + reference_index:{ + claim:"declared-review-reference-set", + entries:[ + { + path:"runtime/bin/example.sh", + snapshot:"subject", + line_count:40, + sha256:("f" * 64) + }, + { + path:"tests/shell/test-example.sh", + snapshot:"subject", + line_count:80, + sha256:("0" * 64) + } + ] + }, truncation:{ occurred:false, budgets:{ @@ -1361,6 +1383,164 @@ case_gate_assurance_non_user_policy_approver_rejected() { rm -f "$tmpf" } +_gate_reviewer_result_valid_instance() { + jq -n ' + ["changed_files","paired_tests","sensitive_signals","public_interface", + "schema","config","install","ci","release","migration", + "bounded_expansion"] as $surfaces | + { + kind:"gate_reviewer_result_v1", + schema_version:1, + reviewer:"critic", + scope_manifest_sha256:("a" * 64), + coverage_claim:"declared-scope-checklist-not-review-completeness", + coverage:($surfaces | map({ + surface:., + status:"examined", + evidence_refs:[{path:"runtime/bin/example.sh",line:42,symbol:null}], + reason:"The reviewer examined this declared surface." + })), + findings:[{ + id:"critic-F001", + reviewer:"critic", + severity:"high", + hard_gate_class:"soft_block", + origin:"diff_caused", + source:{path:"runtime/bin/example.sh",line:42,symbol:null}, + affected_behavior:"The changed command can emit an incomplete result.", + why_it_matters:"Consumers could accept evidence that was not reviewed.", + failure_mode:"The incomplete artifact reaches a publish decision.", + minimum_fix_boundary:"Reject the malformed reviewer result before synthesis.", + verification_expectation:"Run the malformed-protocol fixture." + }], + verdict:"block-soft", + rationale:"Every declared surface was completed after recording the blocker." + } + ' +} + +case_gate_reviewer_result_valid_instance() { + local name="gate-reviewer-result: canonical blocker with complete coverage validates" + should_run "$name" || return 0 + local schema_file="$CORE_DIR/schema/gate-reviewer-result.schema.json" tmpf + tmpf="$(mktemp /tmp/gate-reviewer-result-valid-XXXXXX.json)" + _gate_reviewer_result_valid_instance > "$tmpf" + if jsonschema -i "$tmpf" "$schema_file" >/dev/null 2>&1; then + pass "$name" + else + fail "$name" "schema rejected a canonical reviewer result" + fi + rm -f "$tmpf" +} + +case_gate_reviewer_result_missing_surface_rejected() { + local name="gate-reviewer-result: missing coverage surface is rejected" + should_run "$name" || return 0 + local schema_file="$CORE_DIR/schema/gate-reviewer-result.schema.json" tmpf + tmpf="$(mktemp /tmp/gate-reviewer-result-missing-surface-XXXXXX.json)" + _gate_reviewer_result_valid_instance | + jq '.coverage = .coverage[:-1]' > "$tmpf" + if jsonschema -i "$tmpf" "$schema_file" >/dev/null 2>&1; then + fail "$name" "schema accepted an incomplete reviewer coverage checklist" + else + pass "$name" + fi + rm -f "$tmpf" +} + +case_gate_reviewer_result_invalid_stable_id_rejected() { + local name="gate-reviewer-result: invalid stable finding ID is rejected" + should_run "$name" || return 0 + local schema_file="$CORE_DIR/schema/gate-reviewer-result.schema.json" tmpf + tmpf="$(mktemp /tmp/gate-reviewer-result-invalid-id-XXXXXX.json)" + _gate_reviewer_result_valid_instance | + jq '.findings[0].id = "finding-one"' > "$tmpf" + if jsonschema -i "$tmpf" "$schema_file" >/dev/null 2>&1; then + fail "$name" "schema accepted an unstable finding ID" + else + pass "$name" + fi + rm -f "$tmpf" +} + +case_gate_reviewer_result_evidence_less_blocker_rejected() { + local name="gate-reviewer-result: evidence-less blocker is rejected" + should_run "$name" || return 0 + local schema_file="$CORE_DIR/schema/gate-reviewer-result.schema.json" tmpf + tmpf="$(mktemp /tmp/gate-reviewer-result-no-source-XXXXXX.json)" + _gate_reviewer_result_valid_instance | + jq '.findings[0].source = {path:"",line:null,symbol:null}' > "$tmpf" + if jsonschema -i "$tmpf" "$schema_file" >/dev/null 2>&1; then + fail "$name" "schema accepted a blocker without source evidence" + else + pass "$name" + fi + rm -f "$tmpf" +} + +case_gate_reviewer_result_preexisting_blocker_rejected() { + local name="gate-reviewer-result: pre-existing issue cannot be a blocker" + should_run "$name" || return 0 + local schema_file="$CORE_DIR/schema/gate-reviewer-result.schema.json" tmpf + tmpf="$(mktemp /tmp/gate-reviewer-result-preexisting-block-XXXXXX.json)" + _gate_reviewer_result_valid_instance | + jq '.findings[0].origin = "pre_existing"' > "$tmpf" + if jsonschema -i "$tmpf" "$schema_file" >/dev/null 2>&1; then + fail "$name" "schema accepted a pre-existing blocker" + else + pass "$name" + fi + rm -f "$tmpf" +} + +case_gate_reviewer_result_omitted_unused_symbol_valid() { + local name="gate-reviewer-result: evidence ref may omit unused symbol" + should_run "$name" || return 0 + local schema_file="$CORE_DIR/schema/gate-reviewer-result.schema.json" tmpf + tmpf="$(mktemp /tmp/gate-reviewer-result-omitted-symbol-XXXXXX.json)" + _gate_reviewer_result_valid_instance | + jq 'del(.coverage[0].evidence_refs[0].symbol)' > "$tmpf" + if jsonschema -i "$tmpf" "$schema_file" >/dev/null 2>&1; then + pass "$name" + else + fail "$name" "schema rejected an evidence ref with path and line" + fi + rm -f "$tmpf" +} + +case_gate_reviewer_result_missing_line_and_symbol_rejected() { + local name="gate-reviewer-result: evidence ref needs line or symbol" + should_run "$name" || return 0 + local schema_file="$CORE_DIR/schema/gate-reviewer-result.schema.json" tmpf + tmpf="$(mktemp /tmp/gate-reviewer-result-no-location-XXXXXX.json)" + _gate_reviewer_result_valid_instance | + jq 'del(.coverage[0].evidence_refs[0].line, + .coverage[0].evidence_refs[0].symbol)' > "$tmpf" + if jsonschema -i "$tmpf" "$schema_file" >/dev/null 2>&1; then + fail "$name" "schema accepted an evidence ref without line or symbol" + else + pass "$name" + fi + rm -f "$tmpf" +} + +case_gate_reviewer_result_abbreviated_reviewer_id_rejected() { + local name="gate-reviewer-result: abbreviated reviewer finding ID is rejected" + should_run "$name" || return 0 + local schema_file="$CORE_DIR/schema/gate-reviewer-result.schema.json" tmpf + tmpf="$(mktemp /tmp/gate-reviewer-result-short-id-XXXXXX.json)" + _gate_reviewer_result_valid_instance | + jq '.reviewer = "risk-reviewer" | + .findings[0].reviewer = "risk-reviewer" | + .findings[0].id = "risk-F001"' > "$tmpf" + if jsonschema -i "$tmpf" "$schema_file" >/dev/null 2>&1; then + fail "$name" "schema accepted an abbreviated reviewer finding ID" + else + pass "$name" + fi + rm -f "$tmpf" +} + _gate_policy_override_valid_instance() { jq -n '{ kind:"gate_policy_override_v1", @@ -1461,6 +1641,14 @@ case_gate_verification_duplicate_reason_rejected case_gate_verification_invalid_consumer_rejected case_gate_assurance_invalid_outcome_rejected case_gate_assurance_non_user_policy_approver_rejected +case_gate_reviewer_result_valid_instance +case_gate_reviewer_result_missing_surface_rejected +case_gate_reviewer_result_invalid_stable_id_rejected +case_gate_reviewer_result_evidence_less_blocker_rejected +case_gate_reviewer_result_preexisting_blocker_rejected +case_gate_reviewer_result_omitted_unused_symbol_valid +case_gate_reviewer_result_missing_line_and_symbol_rejected +case_gate_reviewer_result_abbreviated_reviewer_id_rejected case_gate_policy_override_valid_instance case_gate_policy_override_non_user_approver_rejected case_gate_policy_override_extra_key_rejected diff --git a/tests/shell/test-pmctl-gate.sh b/tests/shell/test-pmctl-gate.sh index a9dba701..f92f3ff9 100755 --- a/tests/shell/test-pmctl-gate.sh +++ b/tests/shell/test-pmctl-gate.sh @@ -617,6 +617,58 @@ _mk_gate_result_v3_verified() { _refresh_gate_result_v3_attestation "$path" } +_append_gate_reviewer_protocol_fixture() { + local path="$1" reviewer="$2" scope_sha + scope_sha="$(jq -r '.evidence.scope_manifest.sha256' \ + "${path}.assurance.json")" + { + printf '```reviewer_result_v1\n' + jq -nc --arg reviewer "$reviewer" --arg scope_sha "$scope_sha" ' + ["changed_files","paired_tests","sensitive_signals","public_interface", + "schema","config","install","ci","release","migration", + "bounded_expansion"] as $surfaces | + { + kind:"gate_reviewer_result_v1", + schema_version:1, + reviewer:$reviewer, + scope_manifest_sha256:$scope_sha, + coverage_claim:"declared-scope-checklist-not-review-completeness", + coverage:($surfaces | map({ + surface:., + status:"examined", + evidence_refs:[{path:"README.md",line:1,symbol:null}], + reason:"Fixture examined this declared surface." + })), + findings:[], + verdict:"approve", + rationale:"Fixture completed every declared surface." + } + ' + printf '```\n' + } >> "$path" +} + +_refresh_gate_result_protocol_v3_binding() { + local path="$1" sidecar="${1}.assurance.json" result_sha + result_sha="$(sha256sum "$path" | awk '{print $1}')" + jq --arg result_sha "$result_sha" \ + '.bindings.result_sha256 = $result_sha' \ + "$sidecar" > "${sidecar}.tmp" + mv "${sidecar}.tmp" "$sidecar" + _refresh_gate_result_v3_attestation "$path" +} + +_mk_gate_result_protocol_v3_verified() { + local path="$1" bound_repo="${2:-$_GATE_VERIFY_REPO}" + _mk_gate_result_v3_verified "$path" "$bound_repo" + sed -i \ + 's/^gate_result_version: pr_gate_result_v2$/gate_result_version: pr_gate_result_v3/' \ + "$path" + _append_gate_reviewer_protocol_fixture "$path" critic + _append_gate_reviewer_protocol_fixture "$path" qa-tester + _refresh_gate_result_protocol_v3_binding "$path" +} + _attach_gate_scope_manifest_v3() { local path="$1" sidecar="${1}.assurance.json" local manifest manifest_digest manifest_sha @@ -683,6 +735,15 @@ _attach_gate_scope_manifest_v3() { entries:[], included_paths:[] }, + reference_index:{ + claim:"declared-review-reference-set", + entries:[{ + path:"README.md", + snapshot:"subject", + line_count:1, + sha256:("f" * 64) + }] + }, truncation:{ occurred:false, budgets:{ @@ -840,6 +901,81 @@ case_verify_v2_assurance() { fi } +# Behavior: canonical verification accepts a fully bound result v3 reviewer protocol. +# Steps: +# 1. Build a verified result v3 with linked scope and reviewer evidence. +# 2. Run canonical pmctl gate verification against the generated result. +# 3. Assert verification succeeds and reports verified machine assurance. +case_verify_result_v3_reviewer_protocol() { + local name="gate/verify: result v3 reviewer protocol exits 0" + should_run "$name" || return 0 + local result out code + result="$(_gate_verify_result_path result-v3-reviewer-protocol)" + _mk_gate_result_protocol_v3_verified "$result" "$_GATE_VERIFY_REPO" + set +e + out="$(_run_canonical_gate_verify "$result" 2>&1)" + code=$? + set -e + if [[ "$code" -eq 0 && "$out" == *"assurance: verified"* ]]; then + pass "$name" + else + fail "$name" "code=$code out=$out" + fi +} + +case_verify_result_v3_invalid_evidence_reference() { + # Behavior: recomputing every result/assurance binding cannot authorize a + # reviewer evidence path that is absent from the linked scope index. + # Steps: + # 1. Build a fully verified result v3 with indexed reviewer evidence. + # 2. Replace one reviewer path with an out-of-scope path and re-attest it. + # 3. Assert canonical pmctl verification rejects the protocol reference. + local name="gate/verify: result v3 out-of-scope evidence reference exits 1" + should_run "$name" || return 0 + local result out code + result="$(_gate_verify_result_path result-v3-invalid-evidence-reference)" + _mk_gate_result_protocol_v3_verified "$result" "$_GATE_VERIFY_REPO" + sed -i \ + '0,/"path":"README.md"/s//"path":"outside.md"/' \ + "$result" + _refresh_gate_result_protocol_v3_binding "$result" + set +e + out="$(_run_canonical_gate_verify "$result" 2>&1)" + code=$? + set -e + if [[ "$code" -eq 1 \ + && "$out" == *"invalid evidence reference contract for critic"* ]]; then + pass "$name" + else + fail "$name" "code=$code out=$out" + fi +} + +case_verify_result_v3_protocol_final_mismatch() { + local name="gate/verify: result v3 protocol verdict mismatch exits 1" + should_run "$name" || return 0 + local result sidecar out code + result="$(_gate_verify_result_path result-v3-protocol-mismatch)" + sidecar="${result}.assurance.json" + _mk_gate_result_protocol_v3_verified "$result" "$_GATE_VERIFY_REPO" + sed -i \ + -e 's/^final: GO$/final: NO-GO/' \ + -e 's/^Final: GO$/Final: NO-GO/' \ + "$result" + jq '.result.final = "NO-GO"' "$sidecar" > "${sidecar}.tmp" + mv "${sidecar}.tmp" "$sidecar" + _refresh_gate_result_protocol_v3_binding "$result" + set +e + out="$(_run_canonical_gate_verify "$result" 2>&1)" + code=$? + set -e + if [[ "$code" -eq 1 && "$out" == *"reviewer protocol verdict (GO) contradicts gate Final: (NO-GO)"* ]]; then + pass "$name" + else + fail "$name" "code=$code out=$out" + fi +} + case_verify_v2_without_policy_remains_readable() { local name="gate/verify: pre-policy v2 assurance remains readable" should_run "$name" || return 0 @@ -2495,6 +2631,9 @@ case_pmctl_routing case_help_bypasses_detached_default case_verify_valid case_verify_v2_assurance +case_verify_result_v3_reviewer_protocol +case_verify_result_v3_invalid_evidence_reference +case_verify_result_v3_protocol_final_mismatch case_verify_v2_without_policy_remains_readable case_verify_v2_named_consumer_is_not_authorizing case_verify_v2_canonical_authorization diff --git a/tests/shell/test-pr-gate-profile.sh b/tests/shell/test-pr-gate-profile.sh index 4c591239..1da52dbb 100755 --- a/tests/shell/test-pr-gate-profile.sh +++ b/tests/shell/test-pr-gate-profile.sh @@ -7,6 +7,8 @@ ORIGINAL_PATH="$PATH" # shellcheck source=tests/lib/test-harness.sh . "$SCRIPT_DIR/../lib/test-harness.sh" +# shellcheck source=tests/lib/test-pr-gate-fixture.sh +. "$SCRIPT_DIR/../lib/test-pr-gate-fixture.sh" # shellcheck source=runtime/lib/portable.sh . "$REPO_ROOT/runtime/lib/portable.sh" th_init "$@" @@ -42,6 +44,8 @@ create_runner() { cp -R "$REPO_ROOT/agents" "$dir/agents" mkdir -p "$dir/lib" cp -R "$REPO_ROOT/runtime/lib/." "$dir/lib/" + cp "$REPO_ROOT/tests/lib/test-pr-gate-fixture.sh" \ + "$dir/lib/test-pr-gate-fixture.sh" local cmd for cmd in bash git date readlink dirname basename cp mkdir touch ln cat grep sort wc awk sed mktemp rm mv find head tail tr true false sha256sum shasum jq; do src="$(command -v "$cmd" 2>/dev/null || true)" @@ -55,119 +59,33 @@ create_runner() { cat > "$dir/adapters/codex/dispatch.sh" <<'STUB_EOF' #!/usr/bin/env bash set -euo pipefail - -brief_file="" -MODE="${CODEX_GATE_STUB_MODE:-success}" -MARKER="${CODEX_GATE_STUB_CALLED_MARKER:-}" - -while [[ $# -gt 0 ]]; do - case "$1" in - --brief-file) brief_file="$2"; shift 2 ;; - --cd|--timeout) shift 2 ;; - *) shift ;; - esac - : -done - -[[ -n "$MARKER" ]] && printf 'called\n' > "$MARKER" -printf 'DISPATCH_STUB:%s\n' "$MODE" - -if [[ "$MODE" == "exit99" ]]; then - exit 99 -fi - -if [[ -z "$brief_file" ]]; then - exit 0 -fi - -output_path="" -if [[ -f "$brief_file" ]]; then -output_path=$(grep -o '\- new:.*' "$brief_file" | head -1 | awk '{print $NF}' || true) -fi - -if [[ -n "$output_path" ]]; then - mkdir -p "$(dirname "$output_path")" - if [[ "$brief_file" == *-synthesis.md ]]; then - printf -- '---\ngate_result_version: pr_gate_result_v1\nfinal: GO\ntier: standard\nmode: parallel\nmost_severe: advise\nreviewers:\n critic: advise\nescalation:\n recommended: false\n reviewers: []\n reason: []\n---\n\n# PR-Gate Result — stub tier\n**Date**: 2026-05-17\n**Reviewers**: stub\n**Not reviewed**: none\n\n## cross-check\nnone\n\n## Gate Conclusion\n**Overall verdict**: advise\n**Most severe individual verdict**: advise\nFinal: GO\n' > "$output_path" - elif reviewer_name="$(awk '$1 == "Reviewer:" { print $2; exit }' "$brief_file")" \ - && [[ -n "$reviewer_name" ]]; then - printf '## %s -- advise\n\nstatus: advise\nfindings: []\nverdict: Stub output.\n' \ - "$reviewer_name" > "$output_path" - else - printf -- '---\ngate_result_version: pr_gate_result_v1\nfinal: GO\ntier: standard\nmode: sequential\nmost_severe: advise\nreviewers:\n critic: advise\nescalation:\n recommended: false\n reviewers: []\n reason: []\n---\n\n## stub-reviewer — advise\nVerdict: advise. Stub output.\nFinal: GO\n' > "$output_path" - fi +runner_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +# shellcheck source=tests/lib/test-pr-gate-fixture.sh +if [[ -f "$runner_root/lib/test-pr-gate-fixture.sh" ]]; then + . "$runner_root/lib/test-pr-gate-fixture.sh" +else + . "$runner_root/runtime/lib/test-pr-gate-fixture.sh" fi - -exit 0 +pr_gate_fixture_profile_dispatch codex "$@" STUB_EOF chmod +x "$dir/adapters/codex/dispatch.sh" - # claude adapter stub (CC-383): claude now dispatches a real subprocess too. - # Same result-writing behavior as the codex stub, but keyed on a SEPARATE - # called-marker (CLAUDE_GATE_STUB_CALLED_MARKER) so a test can assert the - # claude route does NOT invoke the codex adapter. mkdir -p "$dir/adapters/claude" cat > "$dir/adapters/claude/dispatch.sh" <<'CLAUDE_STUB_EOF' #!/usr/bin/env bash set -euo pipefail - -brief_file="" -MODE="${CLAUDE_GATE_STUB_MODE:-success}" -MARKER="${CLAUDE_GATE_STUB_CALLED_MARKER:-}" - -while [[ $# -gt 0 ]]; do - case "$1" in - --brief-file) brief_file="$2"; shift 2 ;; - --cd|--timeout|--model|--isolation) shift 2 ;; - *) shift ;; - esac - : -done - -[[ -n "$MARKER" ]] && printf 'called\n' > "$MARKER" -printf 'DISPATCH_STUB:%s\n' "$MODE" - -if [[ "$MODE" == "exit99" ]]; then - exit 99 -fi - -if [[ -z "$brief_file" ]]; then - exit 0 -fi - -output_path="" -if [[ -f "$brief_file" ]]; then -output_path=$(grep -o '\- new:.*' "$brief_file" | head -1 | awk '{print $NF}' || true) +runner_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +# shellcheck source=tests/lib/test-pr-gate-fixture.sh +if [[ -f "$runner_root/lib/test-pr-gate-fixture.sh" ]]; then + . "$runner_root/lib/test-pr-gate-fixture.sh" +else + . "$runner_root/runtime/lib/test-pr-gate-fixture.sh" fi - -if [[ -n "$output_path" ]]; then - mkdir -p "$(dirname "$output_path")" - if [[ "$brief_file" == *-synthesis.md ]]; then - printf -- '---\ngate_result_version: pr_gate_result_v1\nfinal: GO\ntier: standard\nmode: parallel\nmost_severe: advise\nreviewers:\n critic: advise\nescalation:\n recommended: false\n reviewers: []\n reason: []\n---\n\n# PR-Gate Result — stub tier\n**Date**: 2026-05-17\n**Reviewers**: stub\n**Not reviewed**: none\n\n## cross-check\nnone\n\n## Gate Conclusion\n**Overall verdict**: advise\n**Most severe individual verdict**: advise\nFinal: GO\n' > "$output_path" - elif reviewer_name="$(awk '$1 == "Reviewer:" { print $2; exit }' "$brief_file")" \ - && [[ -n "$reviewer_name" ]]; then - printf '## %s -- advise\n\nstatus: advise\nfindings: []\nverdict: Stub output.\n' \ - "$reviewer_name" > "$output_path" - else - printf -- '---\ngate_result_version: pr_gate_result_v1\nfinal: GO\ntier: standard\nmode: sequential\nmost_severe: advise\nreviewers:\n critic: advise\nescalation:\n recommended: false\n reviewers: []\n reason: []\n---\n\n## stub-reviewer — advise\nVerdict: advise. Stub output.\nFinal: GO\n' > "$output_path" - fi -fi - -exit 0 +pr_gate_fixture_profile_dispatch claude "$@" CLAUDE_STUB_EOF chmod +x "$dir/adapters/claude/dispatch.sh" } -create_agents() { - local home="$1" - shift - mkdir -p "$home/.claude/agents" - local reviewer - for reviewer in "$@"; do - printf '# %s\n\nReviewer fixture.\n' "$reviewer" > "$home/.claude/agents/$reviewer.md" - done -} - create_repo() { local repo="$1" git init -q -b main "$repo" diff --git a/tests/shell/test-pr-gate.sh b/tests/shell/test-pr-gate.sh index 5917c66d..2f335f5c 100755 --- a/tests/shell/test-pr-gate.sh +++ b/tests/shell/test-pr-gate.sh @@ -30,6 +30,8 @@ export PATH="$_codex_stub_bin:$PATH" # shellcheck source=tests/lib/test-harness.sh . "$SCRIPT_DIR/../lib/test-harness.sh" +# shellcheck source=tests/lib/test-pr-gate-fixture.sh +. "$SCRIPT_DIR/../lib/test-pr-gate-fixture.sh" legacy_tmp_root="$TMP_ROOT" th_init "$@" TMP_ROOT="$tmp_root" @@ -66,6 +68,8 @@ create_runner() { cp -R "$REPO_ROOT/agents" "$dir/agents" mkdir -p "$dir/lib" cp -R "$REPO_ROOT/runtime/lib/." "$dir/lib/" + cp "$REPO_ROOT/tests/lib/test-pr-gate-fixture.sh" \ + "$dir/lib/test-pr-gate-fixture.sh" mkdir -p "$dir/core/policy" cp "$REPO_ROOT/core/policy/isolation-level.yaml" "$dir/core/policy/isolation-level.yaml" cp "$REPO_ROOT/core/policy/gate-tiers.tsv" "$dir/core/policy/gate-tiers.tsv" @@ -77,6 +81,13 @@ create_runner() { cat > "$dir/adapters/codex/dispatch.sh" <<'STUB_EOF' #!/usr/bin/env bash set -euo pipefail +runner_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +# shellcheck source=tests/lib/test-pr-gate-fixture.sh +if [[ -f "$runner_root/lib/test-pr-gate-fixture.sh" ]]; then + . "$runner_root/lib/test-pr-gate-fixture.sh" +else + . "$runner_root/runtime/lib/test-pr-gate-fixture.sh" +fi # Capture raw dispatch args for isolation-forwarding tests. if [[ -n "${CODEX_GATE_CAPTURE_DISPATCH_ARGS:-}" ]]; then @@ -190,6 +201,10 @@ if [[ "${CODEX_GATE_STUB_TAMPER_SCOPE:-}" == "1" && "$brief_file" != *-synthesis [[ -n "$scope_path" ]] && printf '\n' >> "$scope_path" fi +write_reviewer_protocol_stub() { + pr_gate_fixture_write_reviewer_protocol "$brief_file" "$@" +} + # Simulate prefix-only verdict (loose regex bypass): writes "Verdict: approved" (invalid token # with the right prefix) to verify the anchored regex rejects it. # CODEX_GATE_STUB_VERDICT_PREFIX_ONLY=1: write an invalid prefix verdict instead of a valid one. @@ -203,9 +218,9 @@ if [[ "${CODEX_GATE_STUB_VERDICT_PREFIX_ONLY:-}" == "1" && "$brief_file" != *-sy exit 0 fi -# Simulate the structured output emitted by base-pinned reviewer definitions: -# the machine verdict is in the canonical heading and the definition's -# lower-case `verdict:` field is narrative rather than a second token. +# Simulate a legacy presentation heading plus a narrative lower-case +# `verdict:` field. The appended reviewer_result_v1 JSON remains the only +# machine verdict. if [[ "${CODEX_GATE_STUB_HEADER_ONLY_VERDICT:-}" == "1" \ && "$brief_file" != *-synthesis.md ]]; then output_path=$(grep -o '\- new:.*' "$brief_file" | head -1 | awk '{print $NF}') @@ -213,12 +228,13 @@ if [[ "${CODEX_GATE_STUB_HEADER_ONLY_VERDICT:-}" == "1" \ mkdir -p "$(dirname "$output_path")" printf '## %s -- advise\n\nstatus: advise\nfindings: []\nverdict: Structured narrative.\n' \ "$reviewer_name" > "$output_path" + write_reviewer_protocol_stub "$output_path" "$reviewer_name" advise fi exit 0 fi -# Simulate a conflicting optional legacy Verdict marker. The heading remains -# authoritative, but disagreement must abort rather than silently choose one. +# Simulate conflicting legacy presentation markers without a protocol block. +# The gate must fail closed because no canonical reviewer_result_v1 exists. if [[ "${CODEX_GATE_STUB_CONFLICTING_VERDICT:-}" == "1" \ && "$brief_file" != *-synthesis.md ]]; then output_path=$(grep -o '\- new:.*' "$brief_file" | head -1 | awk '{print $NF}') @@ -307,6 +323,15 @@ Required fixes before GO: none Rationale: Stub gate output. STUB_GATE_EOF + + if grep -q '^goal: Sequential ' "$brief_file"; then + local selected_csv selected_reviewer + selected_csv="$(awk '$1 == "coverage.selected:" { print $2; exit }' "$brief_file")" + for selected_reviewer in ${selected_csv//,/ }; do + printf '\n## %s -- advise\n' "$selected_reviewer" >> "$output_path" + write_reviewer_protocol_stub "$output_path" "$selected_reviewer" advise + done + fi } # Determine effective mode: synthesis briefs can have their own mode override. @@ -363,12 +388,16 @@ case "$effective_mode" in **Reviewers**: stub **Not reviewed**: none +## critic -- advise ## critic -- advise - stub finding, completed before timeout -## qa-tester -- pass +## qa-tester -- approve +## qa-tester -- approve - stub finding, completed before timeout (this reviewer then stalled running tests) PARTIAL_EOF + write_reviewer_protocol_stub "$output_path" critic advise + write_reviewer_protocol_stub "$output_path" qa-tester approve fi fi exit 124 @@ -403,6 +432,12 @@ PARTIAL_EOF stub_verdict="${CODEX_GATE_STUB_VERDICT:-advise}" printf '## %s -- %s\nVerdict: %s. Stub output.\n' \ "$reviewer_name" "$stub_verdict" "$stub_verdict" > "$output_path" + write_reviewer_protocol_stub \ + "$output_path" "$reviewer_name" "$stub_verdict" + if [[ "${CODEX_GATE_STUB_DUPLICATE_HEADING:-}" == "1" ]]; then + printf '## %s -- %s\n' "$reviewer_name" "$stub_verdict" \ + >> "$output_path" + fi if [[ "$(basename "$output_path")" == pr-gate-result-* || "$(basename "$output_path")" == gate-* ]]; then printf 'Final: GO\n' >> "$output_path" fi @@ -474,16 +509,6 @@ PMCTL_STUB_EOF chmod +x "$dir/bin/pmctl" } -create_agents() { - local home="$1" - shift - mkdir -p "$home/.claude/agents" - local reviewer - for reviewer in "$@"; do - printf '# %s\n\nReviewer fixture.\n' "$reviewer" > "$home/.claude/agents/$reviewer.md" - done -} - write_managed_gitignore() { printf '.agent-trace/\n.gate-briefs/\n.gate-results/\n.agents/\n' > .gitignore } @@ -2113,7 +2138,7 @@ test_sequential_combined_brief_validates() { local code=$? set -e if [[ "$code" -ne 0 ]]; then - fail "$name" "gate exit $code, expected 0" + fail "$name" "gate exit $code, expected 0; stderr: $(cat "$err" 2>/dev/null)" return fi set +e @@ -2151,7 +2176,7 @@ test_parallel_reviewer_brief_validates() { local code=$? set -e if [[ "$code" -ne 0 ]]; then - fail "$name" "gate exit $code, expected 0" + fail "$name" "gate exit $code, expected 0; stderr: $(cat "$err" 2>/dev/null)" return fi set +e @@ -2596,13 +2621,13 @@ test_synthesis_no_output_aborts_gate() { pass "$name" } -# Behavior: a reviewer output file without a valid Verdict line fails the -# gate before synthesis (guards against malformed or manipulated output). +# Behavior: a reviewer output file without a reviewer_result_v1 block fails +# the gate before synthesis (guards against malformed or incomplete output). # Steps: # 1. Create a minimal repo (express tier, docs change) # 2. CODEX_GATE_STUB_MODE=no-verdict: reviewer writes output but no Verdict line # 3. Run gate in explicit parallel mode -# 4. Assert non-zero exit and "exactly one valid Verdict line" in stderr +# 4. Assert non-zero exit and protocol INCOMPLETE in stderr test_reviewer_invalid_verdict_aborts_gate() { local name="reviewer-invalid-verdict-aborts-gate" should_run "$name" || return 0 @@ -2619,11 +2644,11 @@ test_reviewer_invalid_verdict_aborts_gate() { local code=$? set -e if [[ "$code" -eq 0 ]]; then - fail "$name" "expected non-zero exit when reviewer output has no valid Verdict line" + fail "$name" "expected non-zero exit when reviewer output has no protocol verdict" return fi assert_file_contains "$name" "$err" \ - "invalid or ambiguous canonical verdict" || return + "reviewer protocol INCOMPLETE" || return pass "$name" } @@ -2654,8 +2679,8 @@ test_reviewer_heading_only_verdict_is_accepted() { pass "$name" } -# Behavior: when an optional upper-case Verdict marker conflicts with the -# canonical heading, the gate fails closed before synthesis. +# Behavior: legacy heading/Verdict-only output without a reviewer_result_v1 +# block fails closed before synthesis. test_reviewer_heading_and_explicit_verdict_must_agree() { local name="reviewer-heading-and-explicit-verdict-must-agree" should_run "$name" || return 0 @@ -2677,7 +2702,7 @@ test_reviewer_heading_and_explicit_verdict_must_agree() { return fi assert_file_contains "$name" "$err" \ - "invalid or ambiguous canonical verdict" || return + "reviewer protocol INCOMPLETE" || return assert_not_contains "$name" "$out" "[synthesis]" || return pass "$name" } @@ -2712,13 +2737,13 @@ test_reviewer_no_output_aborts_gate() { pass "$name" } -# Behavior: sequential mode exiting 0 without writing the gate result file -# fails the gate before reporting a result. +# Behavior: sequential mode exiting 0 without writing reviewer protocol +# evidence fails closed before reporting a result. # Steps: # 1. Create a minimal repo (express tier, docs change) # 2. CODEX_GATE_STUB_MODE=no-output: dispatch exits 0 without output # 3. Run gate in policy-selected sequential mode -# 4. Assert non-zero exit and "sequential gate did not produce" in stderr +# 4. Assert non-zero exit and protocol INCOMPLETE in stderr test_sequential_no_output_aborts_gate() { local name="sequential-no-output-aborts-gate" should_run "$name" || return 0 @@ -2738,17 +2763,17 @@ test_sequential_no_output_aborts_gate() { fail "$name" "expected non-zero exit when sequential dispatch produces no output" return fi - assert_file_contains "$name" "$err" "sequential gate did not produce" || return + assert_file_contains "$name" "$err" "reviewer protocol INCOMPLETE" || return pass "$name" } -# Behavior: sequential mode output without a valid Final line fails the -# gate before reporting a result. +# Behavior: sequential mode output without reviewer protocol evidence fails +# closed before legacy Final-line parsing can report a result. # Steps: # 1. Create a minimal repo (express tier, docs change) # 2. CODEX_GATE_STUB_MODE=no-verdict: dispatch writes output but no Final line # 3. Run gate in policy-selected sequential mode -# 4. Assert non-zero exit and "must contain exactly one Final" in stderr +# 4. Assert non-zero exit and protocol INCOMPLETE in stderr test_sequential_no_final_line_aborts_gate() { local name="sequential-no-final-line-aborts-gate" should_run "$name" || return 0 @@ -2768,7 +2793,7 @@ test_sequential_no_final_line_aborts_gate() { fail "$name" "expected non-zero exit when sequential output has no valid Final line" return fi - assert_file_contains "$name" "$err" "must contain exactly one Final" || return + assert_file_contains "$name" "$err" "reviewer protocol INCOMPLETE" || return pass "$name" } @@ -2816,7 +2841,13 @@ test_sequential_timeout_preserves_partial_result() { return fi assert_file_contains "$name" "$result" "## critic -- advise" || return - assert_file_contains "$name" "$result" "## qa-tester -- pass" || return + assert_file_contains "$name" "$result" "## qa-tester -- approve" || return + local protocol_count + protocol_count="$(grep -c '^```reviewer_result_v1$' "$result" || true)" + [[ "$protocol_count" -eq 2 ]] || { + fail "$name" "expected two completed reviewer protocol blocks, got $protocol_count" + return + } pass "$name" } @@ -3361,12 +3392,12 @@ test_preflight_artifact_tamper_aborts_gate() { fi } -# Behavior: the manifest artifact is re-hashed during assurance finalization, -# so a reviewer cannot change declared scope while retaining the original -# digest supplied to every selected reviewer. +# Behavior: reviewer protocol verification re-hashes the scope manifest before +# accepting evidence, so a reviewer cannot change declared scope while +# retaining the original digest supplied to every selected reviewer. # Steps: # 1. Run a sequential gate whose adapter appends to the scope artifact. -# 2. Assert finalization rejects the linked digest before reporting success. +# 2. Assert protocol verification rejects the linked digest before success. test_scope_manifest_tamper_aborts_gate() { local name="scope-manifest/reviewer-tamper-aborts-gate" should_run "$name" || return 0 @@ -3385,7 +3416,8 @@ test_scope_manifest_tamper_aborts_gate() { local code=$? set -e if [[ "$code" -ne 0 ]] \ - && grep -Fq "linked scope_manifest evidence digest mismatch" "$err"; then + && grep -Fq "reviewer protocol reference manifest digest mismatch" \ + "$err"; then pass "$name" else fail "$name" "tampered scope manifest was accepted: code=$code err=$(cat "$err")" @@ -3625,9 +3657,8 @@ TWRAP_EOF pass "$name" } -# Behavior: a verdict line with a valid prefix but invalid suffix is -# rejected -- e.g. "Verdict: approved" must not be accepted as "Verdict: -# approve". +# Behavior: legacy prose using a verdict-like prefix cannot substitute for a +# schema-complete reviewer_result_v1 verdict. # Steps: # 1. Create a minimal repo (express tier, docs change) # 2. CODEX_GATE_STUB_VERDICT_PREFIX_ONLY=1: stub writes "Verdict: approved" (not "approve") @@ -3654,7 +3685,7 @@ test_verdict_prefix_rejected() { return fi assert_file_contains "$name" "$err" \ - "invalid or ambiguous canonical verdict" || return + "reviewer protocol INCOMPLETE" || return pass "$name" } @@ -3802,10 +3833,8 @@ test_synthesis_multiple_final_lines_aborts_gate() { pass "$name" } -# Behavior: a reviewer artifact with more than one valid Verdict: line is -# rejected. The gate must fail closed on ambiguous reviewer output -- -# silently taking the first match would allow a more-severe later verdict -# to be ignored. +# Behavior: multiple legacy Verdict lines cannot substitute for the unique +# verdict field in a schema-complete reviewer_result_v1 block. # Steps: # 1. Create a minimal repo (express tier, docs change) # 2. CODEX_GATE_STUB_MULTIPLE_VERDICTS=1: stub writes "Verdict: approve" then "Verdict: block" @@ -3832,7 +3861,7 @@ test_multiple_verdict_lines_aborts_gate() { return fi assert_file_contains "$name" "$err" \ - "invalid or ambiguous canonical verdict" || return + "reviewer protocol INCOMPLETE" || return pass "$name" } @@ -3903,8 +3932,8 @@ test_gate_result_frontmatter_and_escalation() { fi local frontmatter frontmatter="$(sed -n "1,${frontmatter_end}p" "$result")" - if ! printf '%s\n' "$frontmatter" | grep -q '^gate_result_version: pr_gate_result_v2$'; then - fail "$name" "frontmatter missing gate_result_version: pr_gate_result_v2" + if ! printf '%s\n' "$frontmatter" | grep -q '^gate_result_version: pr_gate_result_v3$'; then + fail "$name" "frontmatter missing gate_result_version: pr_gate_result_v3" return fi if ! printf '%s\n' "$frontmatter" | grep -q '^gate_assurance: result.md.assurance.json$'; then @@ -4660,7 +4689,7 @@ test_untracked_binary_routes_to_standard() { # review hints; every parallel reviewer and synthesis receive its same digest. # Steps: # 1. Create a feature diff with a rename, shared-helper edit, paired test, -# sensitive old path, and untracked schema/migration inputs. +# sensitive old path, and untracked schema/migration/symlink inputs. # 2. Run a parallel gate and inspect the linked machine-owned manifest. # 3. Assert its self-digest, assurance link, flags/signals/expansions, and the # digest captured independently from every dispatch brief. @@ -4672,6 +4701,7 @@ test_scope_manifest_complete_and_shared_across_parallel_dispatch() { local runner="$dir/runner" out="$dir/out" err="$dir/err" local captures="$dir/scope-captures" result assurance manifest local artifact_digest content_digest captured_count captured_unique + local symlink_digest mkdir -p "$dir" create_runner "$runner" create_agents "$home" critic qa-tester architecture-reviewer security-reviewer risk-reviewer @@ -4694,9 +4724,10 @@ test_scope_manifest_complete_and_shared_across_parallel_dispatch() { printf 'shared_token() { printf new; }\n' > runtime/lib/shared.sh git add app/login.ts runtime/lib/shared.sh git commit -q -m change - mkdir -p core/schema migrations + mkdir -p core/schema migrations docs printf '{"type":"object"}\n' > core/schema/sample.schema.json printf 'ALTER TABLE example ADD COLUMN active boolean;\n' > migrations/001.sql + ln -s ../runtime/lib/shared.sh docs/shared-link.md ) set +e @@ -4720,7 +4751,10 @@ test_scope_manifest_complete_and_shared_across_parallel_dispatch() { fail "$name" "missing linked scope manifest" return } - if ! jq -e ' + symlink_digest="$( + printf '%s' '../runtime/lib/shared.sh' | sha256sum | awk '{print $1}' + )" + if ! jq -e --arg symlink_digest "$symlink_digest" ' .kind == "gate_scope_manifest_v1" and .schema_version == 1 and .status == "complete" and (.changes.entries | any( @@ -4728,6 +4762,7 @@ test_scope_manifest_complete_and_shared_across_parallel_dispatch() { .old_path == "app/auth.ts" and .new_path == "app/login.ts")) and (.changes.untracked_paths == [ "core/schema/sample.schema.json", + "docs/shared-link.md", "migrations/001.sql" ]) and (.paired_tests | any( @@ -4749,10 +4784,23 @@ test_scope_manifest_complete_and_shared_across_parallel_dispatch() { (.expansion.entries | any( .path == "runtime/bin/use-shared.sh" and .reason == "call-site-hint")) and + .reference_index.claim == "declared-review-reference-set" and + (.reference_index.entries | any( + .path == "app/login.ts" and .snapshot == "subject" and + .line_count == 1 and (.sha256 | test("^[a-f0-9]{64}$")))) and + (.reference_index.entries | any( + .path == "app/auth.ts" and .snapshot == "base" and + .line_count == 1 and (.sha256 | test("^[a-f0-9]{64}$")))) and + (.reference_index.entries | any( + .path == "runtime/bin/use-shared.sh" and .snapshot == "subject")) and + (.reference_index.entries | any( + .path == "docs/shared-link.md" and .snapshot == "subject" and + .line_count == 1 and .sha256 == $symlink_digest)) and (.truncation.occurred == false) ' "$manifest" >/dev/null; then fail "$name" "manifest omitted required scope facts: $(jq -c '{ - status,changes,paired_tests,sensitive_signals,flags,expansion,truncation + status,changes,paired_tests,sensitive_signals,flags,expansion, + reference_index,truncation }' "$manifest" 2>/dev/null)" return fi @@ -5247,7 +5295,7 @@ test_post_gate_hook_runs() { fi local result_path result_path="$(awk -F'result: ' '/^result: /{path=$2} END{print path}' "$out")" - assert_file_contains "$name" "$result_path" "gate_result_version: pr_gate_result_v2" || return + assert_file_contains "$name" "$result_path" "gate_result_version: pr_gate_result_v3" || return if [[ ! -s "${result_path}.assurance.json" ]]; then fail "$name" "assurance was not finalized after the successful post-gate hook" return @@ -6109,8 +6157,8 @@ test_parallel_synthesis_brief_ascii_separator() { # CODEX_GATE_CAPTURE_REVIEWER_FILTER selects the critic brief from the # policy-complete parallel dispatch. # Steps: run the gate with generic docs coverage, and assert the captured -# critic brief contains "Executor: codex" and "file:line --" -# using ASCII dashes, and no UTF-8 em dash byte sequence is present. +# critic brief contains "Executor: codex" and an ASCII "--" constraint, +# and no UTF-8 em dash byte sequence is present. test_parallel_reviewer_brief_ascii_separator() { local name="parallel-reviewer-brief-ascii-separator" should_run "$name" || return 0 @@ -6139,7 +6187,7 @@ test_parallel_reviewer_brief_ascii_separator() { fi # Reviewer brief heading format must use ASCII -- not em dash assert_file_contains "$name" "$reviewer_brief" "Executor: codex" || return - assert_file_contains "$name" "$reviewer_brief" "file:line --" || return + assert_file_contains "$name" "$reviewer_brief" "denial -- do NOT" || return # No em dash bytes (UTF-8 E2 80 94) must remain in the reviewer brief if grep -q $'\xe2\x80\x94' "$reviewer_brief" 2>/dev/null; then fail "$name" "em dash (U+2014) found in parallel reviewer brief" @@ -8091,6 +8139,7 @@ if [[ -n "${CODEX_GATE_REVIEWER_DEFS_MARKER:-}" ]]; then fi output_path="$(printf '%s\n' "$brief" | grep -o '\- new:.*' | head -1 | awk '{print $NF}')" if [[ -n "$output_path" ]]; then + scope_sha="$(printf '%s\n' "$brief" | awk '$1 == "artifact_sha256:" { print $2; exit }')" mkdir -p "$(dirname "$output_path")" cat > "$output_path" <<'GATE_RESULT_EOF' --- @@ -8101,7 +8150,7 @@ mode: sequential most_severe: approve reviewers: critic: approve - qa-tester: pass + qa-tester: approve escalation: recommended: false reviewers: [] @@ -8122,6 +8171,30 @@ Required fixes before GO: none **Reason**: - none GATE_RESULT_EOF + for reviewer in critic qa-tester; do + printf '```reviewer_result_v1\n' >> "$output_path" + jq -nc --arg reviewer "$reviewer" --arg scope_sha "$scope_sha" ' + ["changed_files","paired_tests","sensitive_signals","public_interface", + "schema","config","install","ci","release","migration", + "bounded_expansion"] as $surfaces | + { + kind:"gate_reviewer_result_v1", + schema_version:1, + reviewer:$reviewer, + scope_manifest_sha256:$scope_sha, + coverage_claim:"declared-scope-checklist-not-review-completeness", + coverage:($surfaces | map({ + surface:.,status:"examined", + evidence_refs:[{path:"README.md",line:1,symbol:null}], + reason:"Fixture examined this declared surface." + })), + findings:[], + verdict:"approve", + rationale:"Fixture reviewer completed every declared coverage surface." + } + ' >> "$output_path" + printf '\n```\n' >> "$output_path" + done fi printf 'fake Codex reviewer completed\n' > "$last" printf '%s\n' '{"type":"turn.completed","usage":{"input_tokens":10,"output_tokens":5}}' @@ -8516,6 +8589,433 @@ test_shared_gate_reviewer_content_host_boundary_ratchet() { pass "$name" } +# Behavior: the runtime reviewer checklist cannot drift from the canonical +# schema surface vocabulary. +# Steps: source the shared verifier, extract and sort both vocabularies, then +# assert byte-for-byte equality. +test_reviewer_protocol_surfaces_match_schema() { + local name="reviewer-protocol/surfaces-match-schema" + should_run "$name" || return 0 + local runtime_surfaces schema_surfaces + # shellcheck source=runtime/lib/gate-result-verify.sh + . "$REPO_ROOT/runtime/lib/gate-result-verify.sh" + runtime_surfaces="$(_gate_reviewer_protocol_surfaces | LC_ALL=C sort)" + schema_surfaces="$(jq -r ' + .definitions.coverageEntry.properties.surface.enum[] + ' "$REPO_ROOT/core/schema/gate-reviewer-result.schema.json" | LC_ALL=C sort)" + if [[ "$runtime_surfaces" != "$schema_surfaces" ]]; then + fail "$name" "runtime reviewer surfaces drifted from the canonical schema" + return + fi + pass "$name" +} + +# Behavior: sequential review preserves one complete protocol document per +# selected reviewer even though all reviewers share one session. +# Steps: run a two-reviewer sequential gate, count protocol blocks and surfaces, +# then assert result v3 plus both normalized presentation sections. +test_sequential_reviewer_protocol_has_independent_logical_sections() { + local name="reviewer-protocol/sequential-logical-sections" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home" + local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner" + local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" + local result="$TMP_ROOT/$name/result.md" code block_count surface_count + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic qa-tester + create_repo "$repo" docs + set +e + run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --reviewers critic,qa-tester --mode sequential --output "$result" + code=$? + set -e + [[ "$code" -eq 0 ]] || { + fail "$name" "exit $code, expected 0: $(cat "$err" 2>/dev/null)" + return + } + block_count="$(grep -c '^```reviewer_result_v1$' "$result" || true)" + surface_count="$(grep -o '"surface":"' "$result" | wc -l | tr -d ' ')" + [[ "$block_count" -eq 2 && "$surface_count" -eq 22 ]] || { + fail "$name" "expected 2 complete reviewer blocks, got blocks=$block_count surfaces=$surface_count" + return + } + assert_file_contains "$name" "$result" "gate_result_version: pr_gate_result_v3" || return + assert_file_contains "$name" "$result" "## critic -- advise" || return + assert_file_contains "$name" "$result" "## qa-tester -- advise" || return + pass "$name" +} + +# Behavior: parallel review preserves per-reviewer session independence while +# enforcing the same reviewer-result contract as sequential mode. +# Steps: run a two-reviewer parallel gate, inspect its assurance topology and +# captured brief, then assert both protocol blocks and mandatory instructions. +test_parallel_reviewer_protocol_preserves_session_topology() { + local name="reviewer-protocol/parallel-session-topology" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home" + local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner" + local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" + local result="$TMP_ROOT/$name/result.md" assurance code block_count + local reviewer_brief="$TMP_ROOT/$name/reviewer-brief.md" + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic qa-tester + create_repo "$repo" docs + set +e + CODEX_GATE_CAPTURE_REVIEWER_BRIEF="$reviewer_brief" \ + CODEX_GATE_CAPTURE_REVIEWER_FILTER=critic \ + run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --reviewers critic,qa-tester --mode parallel --output "$result" + code=$? + set -e + [[ "$code" -eq 0 ]] || { + fail "$name" "exit $code, expected 0: $(cat "$err" 2>/dev/null)" + return + } + assurance="${result}.assurance.json" + block_count="$(grep -c '^```reviewer_result_v1$' "$result" || true)" + if [[ "$block_count" -ne 2 ]] || ! jq -e ' + .coordinates.mode.resolved == "parallel" and + .coordinates.mode.topology == "per-reviewer-sessions" and + .coordinates.coverage.selected == ["critic","qa-tester"] and + [.dispatch.outcomes[].role] == ["reviewer","reviewer","synthesis"] + ' "$assurance" >/dev/null; then + fail "$name" "parallel reviewer protocol or topology evidence was incomplete" + return + fi + assert_file_contains "$name" "$reviewer_brief" \ + "exactly these nine top-level keys" || return + assert_file_contains "$name" "$reviewer_brief" \ + "Map legacy pass" || return + assert_file_contains "$name" "$reviewer_brief" \ + "must not appear at top level" || return + assert_file_contains "$name" "$reviewer_brief" \ + "risk-reviewer-FNNN" || return + assert_file_contains "$name" "$reviewer_brief" \ + "reference_index.entries[]" || return + assert_file_contains "$name" "$reviewer_brief" \ + "out-of-scope repository paths make the protocol INCOMPLETE" || return + pass "$name" +} + +# Behavior: a selected reviewer that omits one declared surface is protocol +# incomplete and cannot reach synthesis. +# Steps: mutate one parallel reviewer report to remove a surface, run the gate, +# then assert a nonzero protocol error and no synthesis marker. +test_reviewer_protocol_missing_surface_is_incomplete() { + local name="reviewer-protocol/missing-surface-incomplete" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home" + local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner" + local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" code + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic qa-tester + create_repo "$repo" docs + set +e + CODEX_GATE_STUB_PROTOCOL_MUTATION=missing-surface \ + run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --reviewers critic,qa-tester --mode parallel + code=$? + set -e + [[ "$code" -ne 0 ]] || { + fail "$name" "incomplete coverage unexpectedly passed" + return + } + assert_file_contains "$name" "$err" "reviewer protocol INCOMPLETE" || return + assert_not_contains "$name" "$out" "[synthesis]" || return + pass "$name" +} + +# Behavior: an actionable finding without the reviewer-prefixed stable ID is +# protocol incomplete before synthesis. +# Steps: emit an invalid-ID parallel fixture, run the gate, then assert the +# protocol failure and absence of synthesis. +test_reviewer_protocol_invalid_stable_id_is_incomplete() { + local name="reviewer-protocol/invalid-stable-id-incomplete" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home" + local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner" + local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" code + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic qa-tester + create_repo "$repo" docs + set +e + CODEX_GATE_STUB_PROTOCOL_MUTATION=invalid-id \ + run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --reviewers critic,qa-tester --mode parallel + code=$? + set -e + [[ "$code" -ne 0 ]] || { + fail "$name" "invalid stable ID unexpectedly passed" + return + } + assert_file_contains "$name" "$err" "reviewer protocol INCOMPLETE" || return + assert_not_contains "$name" "$out" "[synthesis]" || return + pass "$name" +} + +# Behavior: a blocking finding without a usable source reference is protocol +# incomplete rather than a reviewer NO-GO. +# Steps: emit a blocker whose source path is empty, run the parallel gate, then +# assert protocol failure before synthesis. +test_reviewer_protocol_evidence_less_blocker_is_incomplete() { + local name="reviewer-protocol/evidence-less-blocker-incomplete" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home" + local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner" + local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" code + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic qa-tester + create_repo "$repo" docs + set +e + CODEX_GATE_STUB_VERDICT=block \ + CODEX_GATE_STUB_PROTOCOL_MUTATION=evidence-less-blocker \ + run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --reviewers critic,qa-tester --mode parallel + code=$? + set -e + [[ "$code" -ne 0 ]] || { + fail "$name" "evidence-less blocker unexpectedly passed" + return + } + assert_file_contains "$name" "$err" "reviewer protocol INCOMPLETE" || return + assert_not_contains "$name" "$out" "[synthesis]" || return + pass "$name" +} + +# Behavior: a legacy QA `pass` token is diagnosed as a verdict-contract error, +# not as a coverage or finding failure. +# Steps: mutate the QA JSON verdict, run the parallel gate, then assert the +# precise diagnostic and no synthesis marker. +test_reviewer_protocol_legacy_pass_reports_verdict_contract() { + local name="reviewer-protocol/legacy-pass-diagnostic" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home" + local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner" + local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" code + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic qa-tester + create_repo "$repo" docs + set +e + CODEX_GATE_STUB_PROTOCOL_MUTATION=qa-legacy-pass \ + run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --reviewers critic,qa-tester --mode parallel + code=$? + set -e + [[ "$code" -ne 0 ]] || { + fail "$name" "legacy qa pass token unexpectedly passed" + return + } + assert_file_contains "$name" "$err" \ + "invalid verdict contract for qa-tester" || return + assert_not_contains "$name" "$err" \ + "malformed coverage or finding contract" || return + assert_not_contains "$name" "$out" "[synthesis]" || return + pass "$name" +} + +# Behavior: role-specific legacy top-level fields are rejected by the common +# reviewer envelope contract. +# Steps: add an architecture-style field to the critic fixture, run the gate, +# then assert the top-level diagnostic and no synthesis marker. +test_reviewer_protocol_extra_role_field_reports_top_level_contract() { + local name="reviewer-protocol/extra-role-field-diagnostic" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home" + local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner" + local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" code + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic qa-tester + create_repo "$repo" docs + set +e + CODEX_GATE_STUB_PROTOCOL_MUTATION=critic-extra-top-level \ + run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --reviewers critic,qa-tester --mode parallel + code=$? + set -e + [[ "$code" -ne 0 ]] || { + fail "$name" "legacy role-specific top-level field unexpectedly passed" + return + } + assert_file_contains "$name" "$err" \ + "invalid top-level or binding contract for critic" || return + assert_not_contains "$name" "$out" "[synthesis]" || return + pass "$name" +} + +# Behavior: an abbreviated reviewer identity in a stable finding ID is rejected +# before synthesis. +# Steps: emit `risk-F001` for risk-reviewer, run the parallel gate, then assert +# the finding-contract diagnostic and no synthesis marker. +test_reviewer_protocol_abbreviated_finding_id_is_incomplete() { + local name="reviewer-protocol/abbreviated-finding-id" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home" + local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner" + local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" code + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic qa-tester risk-reviewer + create_repo "$repo" docs + set +e + CODEX_GATE_STUB_PROTOCOL_MUTATION=risk-short-id \ + run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --reviewers critic,qa-tester,risk-reviewer --mode parallel + code=$? + set -e + [[ "$code" -ne 0 ]] || { + fail "$name" "abbreviated risk finding ID unexpectedly passed" + return + } + assert_file_contains "$name" "$err" \ + "invalid finding contract for risk-reviewer" || return + assert_not_contains "$name" "$out" "[synthesis]" || return + pass "$name" +} + +# Behavior: a parallel reviewer cannot cite a syntactically valid repository +# path that is absent from the scope manifest reference index. +# Steps: mutate one coverage reference to an out-of-scope path, run the +# parallel gate, then assert evidence-reference INCOMPLETE before synthesis. +test_parallel_reviewer_protocol_out_of_scope_reference_is_incomplete() { + local name="reviewer-protocol/parallel-out-of-scope-reference" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home" + local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner" + local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" code + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic qa-tester + create_repo "$repo" docs + set +e + CODEX_GATE_STUB_PROTOCOL_MUTATION=out-of-scope-reference \ + run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --reviewers critic,qa-tester --mode parallel + code=$? + set -e + [[ "$code" -ne 0 ]] || { + fail "$name" "out-of-scope evidence reference unexpectedly passed" + return + } + assert_file_contains "$name" "$err" \ + "invalid evidence reference contract for critic" || return + assert_not_contains "$name" "$out" "[synthesis]" || return + pass "$name" +} + +# Behavior: a sequential reviewer cannot cite a line beyond the immutable +# reference-index snapshot even when the repository path itself is in scope. +# Steps: mutate one coverage line beyond line_count, run the sequential gate, +# then assert evidence-reference INCOMPLETE before machine acceptance. +test_sequential_reviewer_protocol_out_of_range_line_is_incomplete() { + local name="reviewer-protocol/sequential-out-of-range-line" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home" + local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner" + local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" code + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic qa-tester + create_repo "$repo" docs + set +e + CODEX_GATE_STUB_PROTOCOL_MUTATION=out-of-range-line \ + run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --reviewers critic,qa-tester --mode sequential + code=$? + set -e + [[ "$code" -ne 0 ]] || { + fail "$name" "out-of-range evidence line unexpectedly passed" + return + } + assert_file_contains "$name" "$err" \ + "invalid evidence reference contract for critic" || return + assert_not_contains "$name" "$out" "[synthesis]" || return + pass "$name" +} + +# Behavior: duplicate human presentation headings cannot invalidate a unique, +# schema-complete JSON reviewer verdict. +# Steps: duplicate each raw heading, run the parallel gate, then assert the +# shell-owned result normalizes presentation and reports no protocol error. +test_reviewer_protocol_duplicate_heading_uses_json_verdict() { + local name="reviewer-protocol/duplicate-heading-json-verdict" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home" + local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner" + local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" + local result="$TMP_ROOT/$name/result.md" code raw heading_count + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic qa-tester + create_repo "$repo" docs + set +e + CODEX_GATE_STUB_DUPLICATE_HEADING=1 \ + run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --reviewers critic,qa-tester --mode parallel --output "$result" + code=$? + set -e + [[ "$code" -eq 0 ]] || { + fail "$name" "duplicate presentation heading blocked valid JSON verdict: $(cat "$err" 2>/dev/null)" + return + } + for raw in "$repo"/.gate-results/reviewer-*.md; do + heading_count="$(grep -cE '^## .* -- advise$' "$raw" || true)" + [[ "$heading_count" -eq 2 ]] || { + fail "$name" "fixture did not create duplicate reviewer headings in $raw" + return + } + done + heading_count="$(grep -cE '^## (critic|qa-tester) -- advise$' "$result" || true)" + [[ "$heading_count" -eq 2 ]] || { + fail "$name" "machine-owned result did not normalize reviewer presentation headings" + return + } + assert_not_contains "$name" "$err" "reviewer protocol INCOMPLETE" || return + pass "$name" +} + +# Behavior: a blocker still completes every declared coverage surface and +# becomes a formal reviewer NO-GO rather than protocol incomplete. +# Steps: emit two complete blocking reviewer reports, run the parallel gate, +# then assert 22 surfaces, result v3, and Final NO-GO. +test_reviewer_protocol_blocker_completes_remaining_surfaces() { + local name="reviewer-protocol/blocker-no-early-stop" + should_run "$name" || return 0 + local dir="$TMP_ROOT/$name" home="$TMP_ROOT/$name/home" + local repo="$TMP_ROOT/$name/repo" runner="$TMP_ROOT/$name/runner" + local out="$TMP_ROOT/$name/out" err="$TMP_ROOT/$name/err" + local result="$TMP_ROOT/$name/result.md" code surface_count + mkdir -p "$dir" + create_runner "$runner" + create_agents "$home" critic qa-tester + create_repo "$repo" docs + set +e + CODEX_GATE_STUB_VERDICT=block CODEX_GATE_STUB_SYNTHESIS_FINAL=NO-GO \ + run_gate "$home" "$runner" "$repo" "$out" "$err" \ + --base main --reviewers critic,qa-tester --mode parallel --output "$result" + code=$? + set -e + [[ "$code" -eq 1 ]] || { + fail "$name" "blocker run exit $code, expected reviewer NO-GO exit 1" + return + } + surface_count="$(grep -o '"surface":"' "$result" | wc -l | tr -d ' ')" + [[ "$surface_count" -eq 22 ]] || { + fail "$name" "blocker early-stopped coverage: expected 22 surfaces, got $surface_count" + return + } + assert_file_contains "$name" "$result" "gate_result_version: pr_gate_result_v3" || return + assert_file_contains "$name" "$result" "Final: NO-GO" || return + assert_not_contains "$name" "$err" "reviewer protocol INCOMPLETE" || return + pass "$name" +} + run_test test_repo_owned_reviewers_and_canonical_memory_on_clean_home run_test test_pmctl_codex_gate_uses_production_memory_on_clean_home run_test test_invalid_canonical_memory_does_not_fallback @@ -8530,5 +9030,18 @@ run_test test_gate_memory_runtime_closes_query_failure run_test test_gate_memory_runtime_omits_over_budget_context run_test test_gate_memory_runtime_closes_unexpected_success_status run_test test_shared_gate_reviewer_content_host_boundary_ratchet +run_test test_reviewer_protocol_surfaces_match_schema +run_test test_sequential_reviewer_protocol_has_independent_logical_sections +run_test test_parallel_reviewer_protocol_preserves_session_topology +run_test test_reviewer_protocol_missing_surface_is_incomplete +run_test test_reviewer_protocol_invalid_stable_id_is_incomplete +run_test test_reviewer_protocol_evidence_less_blocker_is_incomplete +run_test test_reviewer_protocol_legacy_pass_reports_verdict_contract +run_test test_reviewer_protocol_extra_role_field_reports_top_level_contract +run_test test_reviewer_protocol_abbreviated_finding_id_is_incomplete +run_test test_parallel_reviewer_protocol_out_of_scope_reference_is_incomplete +run_test test_sequential_reviewer_protocol_out_of_range_line_is_incomplete +run_test test_reviewer_protocol_duplicate_heading_uses_json_verdict +run_test test_reviewer_protocol_blocker_completes_remaining_surfaces th_summary diff --git a/tests/shell/test-run-tests.sh b/tests/shell/test-run-tests.sh index 79099d02..29dfe25b 100755 --- a/tests/shell/test-run-tests.sh +++ b/tests/shell/test-run-tests.sh @@ -284,6 +284,26 @@ case_gate_assurance_contract_maps_runtime_verifiers() { fi } +case_pr_gate_protocol_contract_maps_profile_and_verifiers() { + local name=pr-gate-protocol-contract-maps-profile-and-verifiers + local repo out status=0 args + args="$TMP_ROOT/$name.args" + repo="$(make_fixture "$name")" + out=$(RUN_TESTS_ARGS_LOG="$args" "$repo/tests/bin/run-tests.sh" \ + --path runtime/bin/pr-gate.sh \ + --path runtime/lib/gate-result-verify.sh \ + --path core/schema/gate-reviewer-result.schema.json \ + --path tests/lib/test-pr-gate-fixture.sh --list 2>&1) || status=$? + if [[ "$status" -eq 0 && "$out" == *"test-core-schemas"* && + "$out" == *"test-pr-gate"* && "$out" == *"test-pr-gate-profile"* && + "$out" == *"test-pmctl-gate"* && "$out" == *"test-layer-boundaries"* && + "$out" != *"coverage gaps"* && ! -e "$args" ]]; then + pass "$name" + else + fail "$name" "status=$status out=$out" + fi +} + case_high_fanout_escalates_full() { local name=high-fanout-escalates-full repo out status=0 args args="$TMP_ROOT/$name.args" @@ -461,6 +481,7 @@ case_prompt_context_timeout_contract_maps_all_consumers case_evidence_contract_maps_to_runner_regression case_gate_assurance_policy_maps_gate_consumers case_gate_assurance_contract_maps_runtime_verifiers +case_pr_gate_protocol_contract_maps_profile_and_verifiers case_high_fanout_escalates_full case_repeated_high_fanout_escalation_succeeds case_unknown_path_fails_without_test_evidence diff --git a/tools/generate-gate-result-verifier-fallback.sh b/tools/generate-gate-result-verifier-fallback.sh index 6166f78f..0093b3e3 100755 --- a/tools/generate-gate-result-verifier-fallback.sh +++ b/tools/generate-gate-result-verifier-fallback.sh @@ -26,6 +26,13 @@ trap cleanup EXIT functions=( gate_result_verdict_verify _gate_result_frontmatter_value + _gate_reviewer_protocol_surfaces + _gate_reviewer_protocol_reference_index_json + _gate_reviewer_protocol_document_verify + _gate_reviewer_protocol_documents + _gate_reviewer_protocol_verdict_extract + _gate_reviewer_protocol_final_extract + gate_reviewer_protocol_verify _gate_result_sha256_stream _gate_result_sha256_file _gate_subject_common_dir