diff --git a/.agents/skills/self-review-solve/SKILL.md b/.agents/skills/self-review-solve/SKILL.md new file mode 100644 index 00000000..51168f87 --- /dev/null +++ b/.agents/skills/self-review-solve/SKILL.md @@ -0,0 +1,19 @@ +--- +name: self-review-solve +description: "Resolve the Boatstack self-review: fix open findings or run a fresh review, converge the loop, and seal the receipt." +--- + + + +This adapter exposes the canonical Yield workflow at `skills/self-review-solve`. +Read its SKILL.md, then run from the repository root: + + .yield/bin/yskill run 'skills/self-review-solve' + + Follow each returned operation exactly. Answer each operation directly: + + .yield/bin/yskill respond --value --skill 'skills/self-review-solve' + + For structured agent results, use --result-json instead of --value. + +Do not skip an operation or invent its response. diff --git a/.agents/skills/self-review/SKILL.md b/.agents/skills/self-review/SKILL.md new file mode 100644 index 00000000..c73c32c0 --- /dev/null +++ b/.agents/skills/self-review/SKILL.md @@ -0,0 +1,19 @@ +--- +name: self-review +description: "Run the Boatstack supervisory-control self-review for the current branch and report the verdict without changing code." +--- + + + +This adapter exposes the canonical Yield workflow at `skills/self-review`. +Read its SKILL.md, then run from the repository root: + + .yield/bin/yskill run 'skills/self-review' + + Follow each returned operation exactly. Answer each operation directly: + + .yield/bin/yskill respond --value --skill 'skills/self-review' + + For structured agent results, use --result-json instead of --value. + +Do not skip an operation or invent its response. diff --git a/.claude/skills/self-review-solve/SKILL.md b/.claude/skills/self-review-solve/SKILL.md new file mode 100644 index 00000000..51168f87 --- /dev/null +++ b/.claude/skills/self-review-solve/SKILL.md @@ -0,0 +1,19 @@ +--- +name: self-review-solve +description: "Resolve the Boatstack self-review: fix open findings or run a fresh review, converge the loop, and seal the receipt." +--- + + + +This adapter exposes the canonical Yield workflow at `skills/self-review-solve`. +Read its SKILL.md, then run from the repository root: + + .yield/bin/yskill run 'skills/self-review-solve' + + Follow each returned operation exactly. Answer each operation directly: + + .yield/bin/yskill respond --value --skill 'skills/self-review-solve' + + For structured agent results, use --result-json instead of --value. + +Do not skip an operation or invent its response. diff --git a/.claude/skills/self-review/SKILL.md b/.claude/skills/self-review/SKILL.md new file mode 100644 index 00000000..c73c32c0 --- /dev/null +++ b/.claude/skills/self-review/SKILL.md @@ -0,0 +1,19 @@ +--- +name: self-review +description: "Run the Boatstack supervisory-control self-review for the current branch and report the verdict without changing code." +--- + + + +This adapter exposes the canonical Yield workflow at `skills/self-review`. +Read its SKILL.md, then run from the repository root: + + .yield/bin/yskill run 'skills/self-review' + + Follow each returned operation exactly. Answer each operation directly: + + .yield/bin/yskill respond --value --skill 'skills/self-review' + + For structured agent results, use --result-json instead of --value. + +Do not skip an operation or invent its response. diff --git a/.cursor/skills/self-review-solve/SKILL.md b/.cursor/skills/self-review-solve/SKILL.md new file mode 100644 index 00000000..51168f87 --- /dev/null +++ b/.cursor/skills/self-review-solve/SKILL.md @@ -0,0 +1,19 @@ +--- +name: self-review-solve +description: "Resolve the Boatstack self-review: fix open findings or run a fresh review, converge the loop, and seal the receipt." +--- + + + +This adapter exposes the canonical Yield workflow at `skills/self-review-solve`. +Read its SKILL.md, then run from the repository root: + + .yield/bin/yskill run 'skills/self-review-solve' + + Follow each returned operation exactly. Answer each operation directly: + + .yield/bin/yskill respond --value --skill 'skills/self-review-solve' + + For structured agent results, use --result-json instead of --value. + +Do not skip an operation or invent its response. diff --git a/.cursor/skills/self-review/SKILL.md b/.cursor/skills/self-review/SKILL.md new file mode 100644 index 00000000..c73c32c0 --- /dev/null +++ b/.cursor/skills/self-review/SKILL.md @@ -0,0 +1,19 @@ +--- +name: self-review +description: "Run the Boatstack supervisory-control self-review for the current branch and report the verdict without changing code." +--- + + + +This adapter exposes the canonical Yield workflow at `skills/self-review`. +Read its SKILL.md, then run from the repository root: + + .yield/bin/yskill run 'skills/self-review' + + Follow each returned operation exactly. Answer each operation directly: + + .yield/bin/yskill respond --value --skill 'skills/self-review' + + For structured agent results, use --result-json instead of --value. + +Do not skip an operation or invent its response. diff --git a/.gitattributes b/.gitattributes index fe6e0333..4f6731b7 100644 --- a/.gitattributes +++ b/.gitattributes @@ -2,3 +2,9 @@ docs/architecture/boatstack-*.md text eol=lf docs/architecture/boatstack-*.mmd text eol=lf docs/architecture/boatstack-*.json text eol=lf docs/architecture/boatstack-standard-flow.mmd text eol=lf +# The review policy is admitted by exact bytes: the program fingerprint hashes +# the worktree assets while verification re-admits them from the base +# revision. A CRLF checkout (Windows autocrlf) would make the two disagree, +# so the policy assets and sealed receipts always materialize with LF. +.github/codex/** text eol=lf +.github/reviews/*.receipt.json text eol=lf diff --git a/.github/reviews/supervisory-review-program.receipt.json b/.github/reviews/supervisory-review-program.receipt.json new file mode 100644 index 00000000..feb4e057 --- /dev/null +++ b/.github/reviews/supervisory-review-program.receipt.json @@ -0,0 +1,276 @@ +{ + "schema_version": 1, + "instance": "supervisory-review-program", + "program": { + "id": "boatstack-reviewer", + "version": "1", + "fingerprint": "76680abd47f9e920ebfe807d3b3f226cbb48bd1a531b0a0a068cd27fd3e6f6e6" + }, + "policy": { + "prompt_path": ".github/codex/review-prompt.md", + "prompt_sha256": "66c4c7111f200de489f4fed0852b261e36cbd1e962c61320c1e4ea2dd2641986", + "schema_path": ".github/codex/review-output-schema.json", + "schema_sha256": "113b02c5cca93156692031c3dedd038e5c2b80cbc4a56337ce06e536110e3e6a", + "max_rounds": 16, + "stall_window": 3, + "weights": [ + 1000, + 100, + 10, + 1 + ] + }, + "base_ref": "origin/main", + "merge_base": "fffbab0e806fe87cd16323a708bf696c0cfceca3", + "head_commit": "fec374ddf36c0a1e2375c5e1f31eba88d3bfb4b1", + "reviewed_tree": "3aa9458d3bdfd48e74b70fc111883c2582489c31", + "generation": 3, + "rounds": [ + { + "generation": 3, + "index": 1, + "candidate_fingerprint": "66de8c7b85be8ff74b99df18518ee01dd9a9320c656b35f7bc62ddee7e44ebfc", + "reviewed_tree": "3aa9458d3bdfd48e74b70fc111883c2582489c31", + "head_commit": "fec374ddf36c0a1e2375c5e1f31eba88d3bfb4b1", + "merge_base": "fffbab0e806fe87cd16323a708bf696c0cfceca3", + "verdict": "patch is correct", + "measure": 0, + "finding_count": 0, + "priorities": [ + 0, + 0, + 0, + 0 + ], + "transition": "review.converge" + } + ], + "final_review": { + "findings": [], + "overall_correctness": "patch is correct", + "overall_explanation": "Round over fffbab0e..fec374dd. The delta since the last converged round is the sealed generation-2 receipt plus a .gitattributes pin declaring .github/codex/** and .github/reviews/*.receipt.json as text eol=lf. The pin closes a real cross-platform admission defect surfaced by CI: on Windows runners (core.autocrlf=true) the checkout materialized the policy assets with CRLF, so the seal-time worktree policy hashes (092a1752/d68222f2, verified equal to the CRLF variants) could never match the base-revision admitted blobs (66c4c711/113b02c5, the LF bytes), failing verification in TestLoopConvergesSealsAndVerifies and TestVerificationRejectsTamperingForgeryAndBypass. Adjacent-case pass: every committed blob for the pinned paths is already LF, so adding the attribute triggers no renormalization and changes no committed hash on any platform; the scratch-repo fixtures copy the checked-out assets, which now materialize as LF everywhere, and scratch git-show reads raw blobs, so worktree and revision admission agree; sealed receipts are written with LF and the receipt directory remains excluded from the reviewed tree, so the pin cannot invalidate an existing receipt. No new defect introduced by this delta was established. Model-level verification before merge: not recommended.", + "overall_confidence_score": 0.85 + }, + "control_state": { + "mode": "converged", + "revision": 15 + }, + "kernel_receipts": [ + { + "schema_version": 3, + "id": "rcp-e6d32467705e31011c25a975b966aafaa14dc02300ed96bccfe443edf50efb42", + "instance_id": "supervisory-review-program", + "prescription_id": "prx-b0bd59f4bd3768e4fe65ebf2c0b92280a8577f25c9617a7b4e6d7bd129e4422a", + "program": { + "id": "boatstack-reviewer", + "version": "1", + "fingerprint": "76680abd47f9e920ebfe807d3b3f226cbb48bd1a531b0a0a068cd27fd3e6f6e6" + }, + "transition_id": "review.findings.record", + "prior_state_revision": 1, + "attempt_state_revision": 2, + "result_state_revision": 3, + "authority_fingerprint": "e1f9d0143447daa7e9a3d11d7497659642b964a06018d671855c4496afbb30f2", + "capabilities": [ + "review.submit" + ], + "effects": [ + { + "facet": "review.round", + "operation": "review.findings.record", + "fingerprint": "c1e46f7e821c4a362181525ddc7cb015741867a179ad372b574c916cc7029476" + } + ], + "prior_observation": "9552cd99ee5c3a388a72f838b89a33a20d740c4089ee9051bb0ff8c9e3f71e92", + "result_observation": "459317767812b8de53fd8871cc1d4dea51ce827874deb4ff3edcef6c4ad71691", + "verification": "satisfied", + "committed_at": "2026-08-21T01:43:08.248706Z" + }, + { + "schema_version": 3, + "id": "rcp-ce98819998897d509f34460ab279d8f0890e0874bc4bab0db6be3f964fdf723d", + "instance_id": "supervisory-review-program", + "prescription_id": "prx-81b9b269e4dbd8656e7865f8990a90b81fc561e0544b818e7c16fe9830fc1edc", + "program": { + "id": "boatstack-reviewer", + "version": "1", + "fingerprint": "76680abd47f9e920ebfe807d3b3f226cbb48bd1a531b0a0a068cd27fd3e6f6e6" + }, + "transition_id": "review.converge", + "prior_state_revision": 3, + "attempt_state_revision": 4, + "result_state_revision": 5, + "authority_fingerprint": "2c120eaf9382cfe382abda5b83c3b5b95548e844c3e404b9905976dc35bdeeb5", + "capabilities": [ + "review.submit" + ], + "effects": [ + { + "facet": "review.round", + "operation": "review.converge", + "fingerprint": "816ff42b6b75b61238802e9a3da8fc3419c2c462b7c8f7d681b7b09015dbd343" + } + ], + "prior_observation": "d7eefc02a8a8f45cfc5583c1eebcc64adce7ff20894ed21d45edb92dfc9a1aae", + "result_observation": "a8c8d1352c96583f79679f05fc543f621d09a99b4c05b8e5758f2ce6d8198f03", + "verification": "satisfied", + "committed_at": "2026-08-21T01:48:51.282881Z" + }, + { + "schema_version": 3, + "id": "rcp-6dac967d6831833dfe32701c08f4558c36124a7ba01677edfd831b28bb6ef707", + "instance_id": "supervisory-review-program", + "prescription_id": "prx-4c03d610c22c85aa11095cb0e77f42670e1054cb49bece7eb58629a3a4be4405", + "program": { + "id": "boatstack-reviewer", + "version": "1", + "fingerprint": "76680abd47f9e920ebfe807d3b3f226cbb48bd1a531b0a0a068cd27fd3e6f6e6" + }, + "transition_id": "review.reopen", + "prior_state_revision": 5, + "attempt_state_revision": 6, + "result_state_revision": 7, + "authority_fingerprint": "612af2b28e108bf69b10cec77dd688adac1ade9e73143b6ee5be1fe88c9d3d0b", + "capabilities": [ + "review.human" + ], + "effects": [ + { + "facet": "review.round", + "operation": "review.reopen", + "fingerprint": "generation-2" + } + ], + "prior_observation": "9787090456e6836381b1d3702f630a88506c4e1ac42f658d4d5674de43b8058a", + "result_observation": "c546633650586d5243b531805e3d7460b2e7501dfbe60e2606a452dbd143786a", + "verification": "satisfied", + "committed_at": "2026-08-21T02:13:18.819099Z" + }, + { + "schema_version": 3, + "id": "rcp-0cb388ab91385fcff8a34f017a83e9c8c64a26ede241bf2479be5abf1b9d2e4b", + "instance_id": "supervisory-review-program", + "prescription_id": "prx-0ae09a9dc531642f0a52d09df7f116c328b2df61d2a6d353bb3e24f8894dfce2", + "program": { + "id": "boatstack-reviewer", + "version": "1", + "fingerprint": "76680abd47f9e920ebfe807d3b3f226cbb48bd1a531b0a0a068cd27fd3e6f6e6" + }, + "transition_id": "review.findings.record", + "prior_state_revision": 7, + "attempt_state_revision": 8, + "result_state_revision": 9, + "authority_fingerprint": "75ada7d57808df1c3a9341cf60d064cd0f82c613cd1903de417751c6b2cc144a", + "capabilities": [ + "review.submit" + ], + "effects": [ + { + "facet": "review.round", + "operation": "review.findings.record", + "fingerprint": "c4ae2b6387fe9b5e382ac9fe68d2c780f49d613323a838a20c2784895e60820a" + } + ], + "prior_observation": "410089929bd05ea5e6a61cf670e4bf7b51b912332d9312ff6a06b233b5a42622", + "result_observation": "a9ad87e432d5cfbfb439bc9635f4906058f8dd37599dbcbb0e70083f0c2a5a08", + "verification": "satisfied", + "committed_at": "2026-08-21T02:16:25.169665Z" + }, + { + "schema_version": 3, + "id": "rcp-80a66988028811f154353a6e07af1eb12cf2f58a8eebf849a848896f354e64d0", + "instance_id": "supervisory-review-program", + "prescription_id": "prx-1bdc304995ab4531cffdfc27d2cc94382086ac7bc932b86574214d3a72cab41a", + "program": { + "id": "boatstack-reviewer", + "version": "1", + "fingerprint": "76680abd47f9e920ebfe807d3b3f226cbb48bd1a531b0a0a068cd27fd3e6f6e6" + }, + "transition_id": "review.converge", + "prior_state_revision": 9, + "attempt_state_revision": 10, + "result_state_revision": 11, + "authority_fingerprint": "1f9c39e910112b5074d0169b3d1ce7d59d97eaade92e8226dd62822180ee2136", + "capabilities": [ + "review.submit" + ], + "effects": [ + { + "facet": "review.round", + "operation": "review.converge", + "fingerprint": "a8887afe41be3cf56685b18373174189c28dc22380da0152aaf8af46f59e7894" + } + ], + "prior_observation": "545b8012b511db8b58ed6f3fe5806dbd74400a2d1dde7a5388a95581c35a6e60", + "result_observation": "868a1e74eab2952c67ec0592973cb556fa36134849b09839fcc4ebc860b40a0a", + "verification": "satisfied", + "committed_at": "2026-08-21T02:18:35.533334Z" + }, + { + "schema_version": 3, + "id": "rcp-a00414357805e89f1ca93b9596d35045a8a5af3a937cd574294c2d5d4f1d2b6a", + "instance_id": "supervisory-review-program", + "prescription_id": "prx-240e898ff3b3813b88cffeb0e0eb43b17316ece34ceb691463ec4e9b2826ee8e", + "program": { + "id": "boatstack-reviewer", + "version": "1", + "fingerprint": "76680abd47f9e920ebfe807d3b3f226cbb48bd1a531b0a0a068cd27fd3e6f6e6" + }, + "transition_id": "review.reopen", + "prior_state_revision": 11, + "attempt_state_revision": 12, + "result_state_revision": 13, + "authority_fingerprint": "e958cbf68de68708cad034d304915c9a3dd2d0d3a08936b6d1974ff314cd246e", + "capabilities": [ + "review.human" + ], + "effects": [ + { + "facet": "review.round", + "operation": "review.reopen", + "fingerprint": "generation-3" + } + ], + "prior_observation": "12894eb1320889e6bcc88c1e7cbc09edfbf6d4773e861df554f09f2fb4e50fe7", + "result_observation": "b6bf0cef3a39cb57b52bd57699e2f3febe206815462027078b62d69de3b9f19b", + "verification": "satisfied", + "committed_at": "2026-08-21T02:26:32.023904Z" + }, + { + "schema_version": 3, + "id": "rcp-c01fc323bd5431330d47451fb4e1db82a715bcb371310312b80b97e02b0de091", + "instance_id": "supervisory-review-program", + "prescription_id": "prx-0ebcfcfb545ca0fc73fe7f4cdf7e1b9444165a5a64b8b31a5181106015d6a80a", + "program": { + "id": "boatstack-reviewer", + "version": "1", + "fingerprint": "76680abd47f9e920ebfe807d3b3f226cbb48bd1a531b0a0a068cd27fd3e6f6e6" + }, + "transition_id": "review.converge", + "prior_state_revision": 13, + "attempt_state_revision": 14, + "result_state_revision": 15, + "authority_fingerprint": "99aaf65d7e429d765b1b6bae85b3158a23eb427b0bcbad2d8cec87b9583cc448", + "capabilities": [ + "review.submit" + ], + "effects": [ + { + "facet": "review.round", + "operation": "review.converge", + "fingerprint": "66de8c7b85be8ff74b99df18518ee01dd9a9320c656b35f7bc62ddee7e44ebfc" + } + ], + "prior_observation": "383f96bd9dd9b8a59f450d5f804db9ce18cc6246a7bd2cfcf92e6737b98fb815", + "result_observation": "219dea3026ab1d69c00f36b14392796f5a42ee9da7c248c0cd63d6994879c8e8", + "verification": "satisfied", + "committed_at": "2026-08-21T02:27:03.244993Z" + } + ], + "honesty": { + "semantic_correctness": "not-evaluated", + "origin_authenticity": "not-proven" + }, + "sealed_at": "2026-08-21T02:27:03.663388Z", + "fingerprint": "e6ec3a381e204e7574611a9308743d1a575978e1ead795cf1271529931dcb4b9" +} diff --git a/.github/scripts/build_codex_github_review.py b/.github/scripts/build_codex_github_review.py deleted file mode 100644 index ae97c4c2..00000000 --- a/.github/scripts/build_codex_github_review.py +++ /dev/null @@ -1,191 +0,0 @@ -#!/usr/bin/env python3 -"""Build a GitHub review while preserving findings without valid diff anchors.""" - -from __future__ import annotations - -import argparse -import json -import os -import posixpath -import re -import subprocess -from pathlib import Path - - -HUNK = re.compile(r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@") - - -def normalize_path(value: str, workspace: str) -> str | None: - candidate = value.replace("\\", "/") - root = workspace.replace("\\", "/").rstrip("/") - if candidate.startswith(root + "/"): - candidate = candidate[len(root) + 1 :] - while candidate.startswith("./"): - candidate = candidate[2:] - candidate = posixpath.normpath(candidate) - if ( - not candidate - or candidate == "." - or candidate.startswith("/") - or candidate == ".." - or candidate.startswith("../") - or "/../" in candidate - ): - return None - return candidate - - -def header_path(line: str) -> str | None: - value = line[4:].split("\t", 1)[0] - if value == "/dev/null": - return None - if value.startswith(("a/", "b/")): - value = value[2:] - return value - - -def changed_lines(diff: str) -> dict[str, set[tuple[str, int]]]: - allowed: dict[str, set[tuple[str, int]]] = {"LEFT": set(), "RIGHT": set()} - old_path: str | None = None - new_path: str | None = None - old_line = 0 - new_line = 0 - in_hunk = False - for line in diff.splitlines(): - if line.startswith("diff --git "): - old_path = new_path = None - in_hunk = False - continue - if not in_hunk and line.startswith("--- "): - old_path = header_path(line) - continue - if not in_hunk and line.startswith("+++ "): - new_path = header_path(line) - continue - match = HUNK.match(line) - if match: - old_line = int(match.group(1)) - new_line = int(match.group(3)) - in_hunk = True - continue - if not in_hunk or line.startswith("\\"): - continue - if line.startswith("-"): - if old_path is not None: - allowed["LEFT"].add((old_path, old_line)) - old_line += 1 - elif line.startswith("+"): - if new_path is not None: - allowed["RIGHT"].add((new_path, new_line)) - new_line += 1 - elif line.startswith(" "): - old_line += 1 - new_line += 1 - else: - in_hunk = False - return allowed - - -def finding_body(finding: dict[str, object]) -> str: - return ( - f"[P{finding['priority']}] {finding['title']}\n\n" - f"{finding['body']}\n\nConfidence: {finding['confidence_score']}" - ) - - -def build_review( - review: dict[str, object], - *, - commit: str, - workspace: str, - allowed: dict[str, set[tuple[str, int]]], -) -> dict[str, object]: - body = ( - "Codex automated review\n\n" - f"Verdict: {review['overall_correctness']}\n" - f"Confidence: {review['overall_confidence_score']}\n\n" - f"{review['overall_explanation']}" - ) - comments: list[dict[str, object]] = [] - unanchored: list[str] = [] - for finding in review["findings"]: # type: ignore[index] - location = finding["code_location"] - path = normalize_path(location["absolute_file_path"], workspace) - side = location["side"] - start = location["line_range"]["start"] - end = location["line_range"]["end"] - valid_anchor = ( - path is not None - and side in allowed - and start <= end - and all((path, line) in allowed[side] for line in range(start, end + 1)) - ) - text = finding_body(finding) - if valid_anchor: - comment: dict[str, object] = { - "path": path, - "line": end, - "side": side, - "body": text, - } - if start != end: - comment["start_line"] = start - comment["start_side"] = side - comments.append(comment) - continue - display_path = path or "unresolved-path" - unanchored.append(f"{text}\n\nLocation: {display_path}:{start}-{end} ({side})") - if unanchored: - body += "\n\nFindings without inline diff anchors\n\n" + "\n\n---\n\n".join(unanchored) - return {"commit_id": commit, "event": "COMMENT", "body": body, "comments": comments} - - -def pull_request_diff(workspace: str, base: str, head: str) -> str: - merge_base = subprocess.run( - ["git", "merge-base", base, head], - cwd=workspace, - check=True, - text=True, - capture_output=True, - ).stdout.strip() - return subprocess.run( - [ - "git", - "-c", - "core.quotePath=false", - "diff", - "--no-ext-diff", - "--no-renames", - "--unified=0", - merge_base, - head, - ], - cwd=workspace, - check=True, - text=True, - capture_output=True, - ).stdout - - -def main() -> int: - parser = argparse.ArgumentParser() - parser.add_argument("--input", required=True) - parser.add_argument("--output", required=True) - parser.add_argument("--workspace", required=True) - parser.add_argument("--base", required=True) - parser.add_argument("--head", required=True) - args = parser.parse_args() - diff = pull_request_diff(args.workspace, args.base, args.head) - review = json.loads(Path(args.input).read_text()) - payload = build_review( - review, - commit=args.head, - workspace=os.path.abspath(args.workspace), - allowed=changed_lines(diff), - ) - Path(args.output).write_text(json.dumps(payload, indent=2) + "\n") - return 0 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/.github/tests/test_codex_review_publish.py b/.github/tests/test_codex_review_publish.py deleted file mode 100644 index df6bc87d..00000000 --- a/.github/tests/test_codex_review_publish.py +++ /dev/null @@ -1,112 +0,0 @@ -import importlib.util -import subprocess -import tempfile -import unittest -from pathlib import Path - - -SCRIPT = Path(__file__).parents[1] / "scripts" / "build_codex_github_review.py" -SPEC = importlib.util.spec_from_file_location("build_codex_github_review", SCRIPT) -MODULE = importlib.util.module_from_spec(SPEC) -assert SPEC.loader is not None -SPEC.loader.exec_module(MODULE) - - -class CodexReviewPublishTests(unittest.TestCase): - def test_invalid_inline_location_is_preserved_in_review_body(self) -> None: - diff = """diff --git a/review.go b/review.go ---- a/review.go -+++ b/review.go -@@ -2 +2 @@ --old -+new -""" - review = { - "findings": [ - self.finding("anchored", 2), - self.finding("not changed", 9), - ], - "overall_correctness": "patch is incorrect", - "overall_explanation": "One location is outside the pull request diff.", - "overall_confidence_score": 0.9, - } - payload = MODULE.build_review( - review, - commit="head", - workspace="/workspace", - allowed=MODULE.changed_lines(diff), - ) - self.assertEqual(len(payload["comments"]), 1) - self.assertEqual(payload["comments"][0]["line"], 2) - self.assertIn("Findings without inline diff anchors", payload["body"]) - self.assertIn("not changed", payload["body"]) - self.assertIn("review.go:9-9 (RIGHT)", payload["body"]) - - def test_outside_workspace_path_cannot_become_inline_comment(self) -> None: - review = { - "findings": [self.finding("outside", 2, path="/tmp/other.go")], - "overall_correctness": "patch is incorrect", - "overall_explanation": "The path is not repository-relative.", - "overall_confidence_score": 0.8, - } - payload = MODULE.build_review( - review, - commit="head", - workspace="/workspace", - allowed={"LEFT": set(), "RIGHT": {("review.go", 2)}}, - ) - self.assertEqual(payload["comments"], []) - self.assertIn("unresolved-path", payload["body"]) - - def test_pull_request_diff_excludes_base_only_changes(self) -> None: - with tempfile.TemporaryDirectory() as directory: - repository = Path(directory) - self.git(repository, "init", "-q") - self.git(repository, "config", "user.email", "review@example.invalid") - self.git(repository, "config", "user.name", "Review Test") - (repository / "base.txt").write_text("original\n") - (repository / "head.txt").write_text("original\n") - self.git(repository, "add", ".") - self.git(repository, "commit", "-q", "-m", "initial") - common = self.git(repository, "rev-parse", "HEAD") - - (repository / "base.txt").write_text("base only\n") - self.git(repository, "commit", "-q", "-am", "base") - base = self.git(repository, "rev-parse", "HEAD") - - self.git(repository, "checkout", "-q", "--detach", common) - (repository / "head.txt").write_text("head only\n") - self.git(repository, "commit", "-q", "-am", "head") - head = self.git(repository, "rev-parse", "HEAD") - - diff = MODULE.pull_request_diff(str(repository), base, head) - self.assertIn("head.txt", diff) - self.assertNotIn("base.txt", diff) - - @staticmethod - def finding(title: str, line: int, path: str = "/workspace/review.go") -> dict: - return { - "title": title, - "body": "Finding detail.", - "confidence_score": 0.95, - "priority": 1, - "code_location": { - "absolute_file_path": path, - "side": "RIGHT", - "line_range": {"start": line, "end": line}, - }, - } - - @staticmethod - def git(repository: Path, *args: str) -> str: - return subprocess.run( - ["git", *args], - cwd=repository, - check=True, - text=True, - capture_output=True, - ).stdout.strip() - - -if __name__ == "__main__": - unittest.main() diff --git a/.github/tests/test_repository_contract.py b/.github/tests/test_repository_contract.py index e7d9a5d2..1993e8d2 100644 --- a/.github/tests/test_repository_contract.py +++ b/.github/tests/test_repository_contract.py @@ -196,32 +196,41 @@ def test_active_workflows_have_no_private_upstream_authority(self) -> None: self.assertNotIn("sync/intelligence-flow-", value, workflow) self.assertNotIn("UPSTREAM.json", value, workflow) - def test_codex_review_is_secret_scoped_read_only_and_structured(self) -> None: - workflow = (REPO / ".github" / "workflows" / "codex-review.yml").read_text() + def test_review_verification_is_deterministic_and_policy_admitted(self) -> None: + # control-law: CI verifies a sealed local review receipt; no reviewer, + # model, or API key runs in CI. + workflows = REPO / ".github" / "workflows" + self.assertFalse((workflows / "codex-review.yml").exists()) + workflow = (workflows / "review-verified.yml").read_text() prompt = (REPO / ".github" / "codex" / "review-prompt.md").read_text() schema = json.loads((REPO / ".github" / "codex" / "review-output-schema.json").read_text()) self.assertIn("pull_request:", workflow) self.assertNotIn("pull_request_target", workflow) - self.assertIn("CODEX_REVIEWER_API", workflow) - self.assertIn("head.repo.full_name", workflow) - self.assertIn("not configured", workflow) self.assertIn("persist-credentials: false", workflow) - self.assertIn('git merge-base "$BASE_SHA" "$HEAD_SHA"', workflow) - self.assertIn('git show "$BASE_SHA:.github/codex/review-prompt.md"', workflow) - self.assertIn('git show "$BASE_SHA:.github/codex/review-output-schema.json"', workflow) - self.assertIn("output-schema-file: ${{ steps.policy.outputs.schema }}", workflow) - self.assertNotIn("cp .github/codex/review-prompt.md", workflow) - self.assertIn("base revision has no admitted review policy", workflow) - self.assertIn("first 200 shown", workflow) - self.assertNotIn('diff --unified=5 "$BASE_SHA" "$HEAD_SHA"', workflow) - self.assertIn("permission-profile: \":read-only\"", workflow) - self.assertIn("safety-strategy: drop-sudo", workflow) - self.assertRegex(workflow, r"openai/codex-action@[0-9a-f]{40}") - self.assertIn("pull-requests: write", workflow) self.assertIn("contents: read", workflow) - self.assertIn("gpt-5.6-sol", workflow) - self.assertIn("CODEX_REVIEW_EFFORT || 'high'", workflow) + self.assertNotIn("pull-requests: write", workflow) + self.assertNotIn("secrets.", workflow) + self.assertIn("base revision has no admitted review policy", workflow) + self.assertIn('git cat-file -e "$BASE_SHA:.github/codex/review-prompt.md"', workflow) + self.assertIn('git cat-file -e "$BASE_SHA:.github/codex/review-output-schema.json"', workflow) + self.assertIn("go build -o \"$RUNNER_TEMP/boatstack-reviewer\" ./cmd/boatstack-reviewer", workflow) + self.assertIn("--dir .github/reviews", workflow) + self.assertIn('--base "$BASE_SHA"', workflow) + self.assertIn('--head "$HEAD_SHA"', workflow) + for banned in ("codex-action", "openai-api-key", "output-schema-file"): + for active in workflows.glob("*.yml"): + self.assertNotIn(banned, active.read_text(), active) + + # The reviewer performs base-revision policy admission and binds the + # receipt-excluded head tree inside the verifier itself. + reviewer = REPO / "boatstack" / "cmd" / "boatstack-reviewer" + seal_source = (reviewer / "seal.go").read_text() + self.assertIn("loadRevisionPolicy", seal_source) + self.assertIn("reviewedTree", seal_source) + git_source = (reviewer / "gitrepo.go").read_text() + self.assertIn('receiptDirectory = ".github/reviews"', git_source) + self.assertIn("untrusted data", prompt) self.assertIn("`LEFT` for deleted lines", prompt) self.assertIn("Resolver / apply agreement", prompt) @@ -245,10 +254,11 @@ def test_codex_review_is_secret_scoped_read_only_and_structured(self) -> None: location = finding["properties"]["code_location"] self.assertIn("side", location["required"]) self.assertEqual(location["properties"]["side"]["enum"], ["LEFT", "RIGHT"]) - self.assertIn("build_codex_github_review.py", workflow) - publisher = (REPO / ".github" / "scripts" / "build_codex_github_review.py").read_text() - self.assertIn('"side": side', publisher) - self.assertIn("Findings without inline diff anchors", publisher) + self.assertFalse((REPO / ".github" / "scripts" / "build_codex_github_review.py").exists()) + anchors = (reviewer / "candidate.go").read_text() + self.assertIn("anchorFailure", anchors) + self.assertIn("changedLines", anchors) + self.assertIn("is not part of the review diff", anchors) self.assertFalse((REPO / "UPSTREAM.json").exists()) def test_release_builds_six_checksum_bound_v2_runtimes(self) -> None: @@ -633,10 +643,10 @@ def test_go_import_parser_accepts_legal_import_forms(self) -> None: ], ) - @unittest.expectedFailure - def test_kernel_runtime_has_no_production_consumer_yet(self) -> None: - # Migration task T5 must replace this marker with a permanent positive - # production-reachability assertion when the generic runtime is adopted. + def test_kernel_runtime_has_a_production_consumer(self) -> None: + # Permanent positive production-reachability assertion: the generic + # kernel runtime is adopted by boatstack-reviewer, the supervisory + # control review program. production_files = [ path for path in sorted((REPO / "boatstack").rglob("*.go")) @@ -653,8 +663,8 @@ def test_kernel_runtime_has_no_production_consumer_yet(self) -> None: if metadata["runtime_consumers"] ] self.assertTrue( - consumers, - "migration T5 has not connected the generic kernel runtime to production code", + any(consumer.startswith("boatstack/cmd/boatstack-reviewer/") for consumer in consumers), + f"the generic kernel runtime lost its production consumer: {consumers}", ) def test_invocation_runtime_is_domain_neutral(self) -> None: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ded61142..5140e145 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,7 +50,7 @@ jobs: - name: extension-conformance packages: ./extension/... ./distribution - name: surface-parity - packages: ./internal/softwaredelivery/surfaces ./sdk ./cmd/boatstack-helper + packages: ./internal/softwaredelivery/surfaces ./sdk ./cmd/boatstack-helper ./cmd/boatstack-reviewer - name: plant-integration packages: ./internal/softwaredelivery/plant ./internal/softwaredelivery/effects ./internal/softwaredelivery/engine runs-on: ubuntu-latest diff --git a/.github/workflows/codex-review.yml b/.github/workflows/codex-review.yml deleted file mode 100644 index ec8d072a..00000000 --- a/.github/workflows/codex-review.yml +++ /dev/null @@ -1,160 +0,0 @@ -# Boatstack-owned advisory review plane. -name: Codex code review - -on: - pull_request: - types: [opened, reopened, synchronize, ready_for_review] - -permissions: - contents: read - pull-requests: write - -concurrency: - group: codex-review-${{ github.event.pull_request.number }} - cancel-in-progress: true - -jobs: - review: - name: codex-review - if: github.event.pull_request.draft == false - runs-on: ubuntu-latest - env: - BASE_SHA: ${{ github.event.pull_request.base.sha }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} - PR_NUMBER: ${{ github.event.pull_request.number }} - REPOSITORY: ${{ github.repository }} - steps: - - name: Check reviewer configuration - id: configuration - env: - HEAD_REPOSITORY: ${{ github.event.pull_request.head.repo.full_name }} - REVIEWER_API_KEY: ${{ secrets.CODEX_REVIEWER_API }} - shell: bash - run: | - if [[ "$HEAD_REPOSITORY" != "$REPOSITORY" ]]; then - echo "enabled=false" >> "$GITHUB_OUTPUT" - echo "Codex review is disabled for fork pull requests." >> "$GITHUB_STEP_SUMMARY" - elif [[ -z "$REVIEWER_API_KEY" ]]; then - echo "enabled=false" >> "$GITHUB_OUTPUT" - echo "Codex review is ready but CODEX_REVIEWER_API is not configured." >> "$GITHUB_STEP_SUMMARY" - else - echo "enabled=true" >> "$GITHUB_OUTPUT" - fi - - - name: Checkout pull request merge commit - if: steps.configuration.outputs.enabled == 'true' - uses: actions/checkout@v7 - with: - fetch-depth: 0 - persist-credentials: false - ref: refs/pull/${{ github.event.pull_request.number }}/merge - - - name: Fetch exact base and head revisions - if: steps.configuration.outputs.enabled == 'true' - shell: bash - run: | - git fetch --no-tags origin \ - "+refs/pull/${PR_NUMBER}/head:refs/remotes/origin/codex-review-head" - git cat-file -e "$BASE_SHA^{commit}" - test "$(git rev-parse refs/remotes/origin/codex-review-head)" = "$HEAD_SHA" - - - name: Load admitted review policy - if: steps.configuration.outputs.enabled == 'true' - id: policy - shell: bash - run: | - prompt="$RUNNER_TEMP/codex-review-prompt.md" - schema="$RUNNER_TEMP/codex-review-output-schema.json" - if ! git cat-file -e "$BASE_SHA:.github/codex/review-prompt.md" || \ - ! git cat-file -e "$BASE_SHA:.github/codex/review-output-schema.json"; then - echo "enabled=false" >> "$GITHUB_OUTPUT" - echo "Automated review is skipped because the base revision has no admitted review policy." >> "$GITHUB_STEP_SUMMARY" - exit 0 - fi - git show "$BASE_SHA:.github/codex/review-prompt.md" > "$prompt" - git show "$BASE_SHA:.github/codex/review-output-schema.json" > "$schema" - echo "enabled=true" >> "$GITHUB_OUTPUT" - echo "path=$prompt" >> "$GITHUB_OUTPUT" - echo "schema=$schema" >> "$GITHUB_OUTPUT" - - - name: Build review prompt - if: steps.configuration.outputs.enabled == 'true' && steps.policy.outputs.enabled == 'true' - id: prompt - shell: bash - run: | - prompt="${{ steps.policy.outputs.path }}" - merge_base="$(git merge-base "$BASE_SHA" "$HEAD_SHA")" - changed_count="$(git diff --name-only "$merge_base" "$HEAD_SHA" | wc -l | tr -d ' ')" - { - echo - echo "Repository: $REPOSITORY" - echo "Pull request: $PR_NUMBER" - echo "Base revision: $BASE_SHA" - echo "Head revision: $HEAD_SHA" - echo "Merge base: $merge_base" - echo - echo "Changed files: $changed_count total (first 200 shown)" - git --no-pager diff --name-status "$merge_base" "$HEAD_SHA" | sed -n '1,200p' - echo - echo "Diff summary:" - git --no-pager diff --shortstat "$merge_base" "$HEAD_SHA" - echo - echo "Inspect the exact pull request change with:" - echo "git --no-pager diff --unified=5 $merge_base $HEAD_SHA" - echo "Read only the relevant portions needed to review the change." - } >> "$prompt" - echo "path=$prompt" >> "$GITHUB_OUTPUT" - - - name: Run structured Codex review - if: steps.configuration.outputs.enabled == 'true' && steps.policy.outputs.enabled == 'true' - id: codex - uses: openai/codex-action@dd78cb653811af44014baa08fe954e28d32c1bf9 # main, 2026-08-12 - with: - openai-api-key: ${{ secrets.CODEX_REVIEWER_API }} - prompt-file: ${{ steps.prompt.outputs.path }} - output-schema-file: ${{ steps.policy.outputs.schema }} - output-file: ${{ runner.temp }}/codex-review-output.json - permission-profile: ":read-only" - safety-strategy: drop-sudo - codex-version: "0.147.0" - model: ${{ vars.CODEX_REVIEW_MODEL || 'gpt-5.6-sol' }} - effort: ${{ vars.CODEX_REVIEW_EFFORT || 'high' }} - - - name: Validate structured review - if: steps.configuration.outputs.enabled == 'true' && steps.policy.outputs.enabled == 'true' - shell: bash - run: | - test -s "$RUNNER_TEMP/codex-review-output.json" - jq -e ' - (.findings | type == "array") and - (.overall_correctness == "patch is correct" or .overall_correctness == "patch is incorrect") and - (.overall_confidence_score >= 0 and .overall_confidence_score <= 1) - ' "$RUNNER_TEMP/codex-review-output.json" >/dev/null - - - name: Build GitHub review payload - if: steps.configuration.outputs.enabled == 'true' && steps.policy.outputs.enabled == 'true' - shell: bash - run: | - python3 .github/scripts/build_codex_github_review.py \ - --input "$RUNNER_TEMP/codex-review-output.json" \ - --output "$RUNNER_TEMP/codex-github-review.json" \ - --workspace "$GITHUB_WORKSPACE" \ - --base "$BASE_SHA" \ - --head "$HEAD_SHA" - - - name: Publish GitHub review - if: steps.configuration.outputs.enabled == 'true' && steps.policy.outputs.enabled == 'true' - env: - GH_TOKEN: ${{ github.token }} - shell: bash - run: | - jq -e 'all(.comments[]; - ((.path | length) > 0) and - (((.path | startswith("/")) or (.path | startswith("../")) or (.path | contains("/../"))) | not) and - ((.start_line // .line) <= .line) - )' \ - "$RUNNER_TEMP/codex-github-review.json" >/dev/null - gh api \ - --method POST \ - "repos/$REPOSITORY/pulls/$PR_NUMBER/reviews" \ - --input "$RUNNER_TEMP/codex-github-review.json" diff --git a/.github/workflows/review-verified.yml b/.github/workflows/review-verified.yml new file mode 100644 index 00000000..9fef0506 --- /dev/null +++ b/.github/workflows/review-verified.yml @@ -0,0 +1,80 @@ +# Boatstack-owned deterministic review verification plane. +# +# The review itself runs locally through `boatstack-reviewer`: a coding agent +# or human drives the supervisory-control review loop to convergence and +# commits the sealed receipt under .github/reviews/. This workflow runs no +# reviewer, no model, and no API key: it deterministically verifies that a +# sealed receipt binds the exact pull request head tree under the review +# policy admitted at the pull request base revision. +name: Review verified + +on: + pull_request: + types: [opened, reopened, synchronize, ready_for_review] + +permissions: + contents: read + +concurrency: + group: review-verified-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + review-verified: + name: review-verified + if: github.event.pull_request.draft == false + runs-on: ubuntu-latest + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + PR_NUMBER: ${{ github.event.pull_request.number }} + steps: + - name: Checkout pull request merge commit + uses: actions/checkout@v7 + with: + fetch-depth: 0 + persist-credentials: false + ref: refs/pull/${{ github.event.pull_request.number }}/merge + + - name: Fetch exact base and head revisions + shell: bash + run: | + git fetch --no-tags origin \ + "+refs/pull/${PR_NUMBER}/head:refs/remotes/origin/review-verified-head" + git cat-file -e "$BASE_SHA^{commit}" + test "$(git rev-parse refs/remotes/origin/review-verified-head)" = "$HEAD_SHA" + + - name: Check admitted review policy + id: policy + shell: bash + run: | + if ! git cat-file -e "$BASE_SHA:.github/codex/review-prompt.md" || \ + ! git cat-file -e "$BASE_SHA:.github/codex/review-output-schema.json"; then + echo "enabled=false" >> "$GITHUB_OUTPUT" + echo "Review verification is skipped because the base revision has no admitted review policy." >> "$GITHUB_STEP_SUMMARY" + exit 0 + fi + echo "enabled=true" >> "$GITHUB_OUTPUT" + + - name: Set up Go + if: steps.policy.outputs.enabled == 'true' + uses: actions/setup-go@v7 + with: + go-version-file: boatstack/go.mod + cache-dependency-path: boatstack/go.sum + + - name: Build the review verifier + if: steps.policy.outputs.enabled == 'true' + shell: bash + working-directory: boatstack + run: go build -o "$RUNNER_TEMP/boatstack-reviewer" ./cmd/boatstack-reviewer + + - name: Verify the sealed review receipt + if: steps.policy.outputs.enabled == 'true' + shell: bash + run: | + "$RUNNER_TEMP/boatstack-reviewer" verify \ + --repo . \ + --dir .github/reviews \ + --base "$BASE_SHA" \ + --head "$HEAD_SHA" | tee -a "$GITHUB_STEP_SUMMARY" diff --git a/.gitignore b/.gitignore index 215b19ed..f9e4ad2a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,10 @@ boatstack/boatstack-helper boatstack/boatstack-helper.exe +boatstack/boatstack-reviewer +boatstack/boatstack-reviewer.exe +.yield/ +skills/*/.yield-runs/ +skills/*/fixtures/tmp/ dist/ build/docs/ node_modules/ diff --git a/boatstack/cmd/boatstack-reviewer/candidate.go b/boatstack/cmd/boatstack-reviewer/candidate.go new file mode 100644 index 00000000..d69970cd --- /dev/null +++ b/boatstack/cmd/boatstack-reviewer/candidate.go @@ -0,0 +1,286 @@ +package main + +import ( + "bytes" + "encoding/json" + "fmt" + "path" + "regexp" + "strconv" + "strings" + + "github.com/santhosh-tekuri/jsonschema/v6" +) + +// verdictCorrect and verdictIncorrect are the only overall verdicts the +// admitted output schema allows. +const ( + verdictCorrect = "patch is correct" + verdictIncorrect = "patch is incorrect" +) + +// reviewDocument mirrors the admitted output schema. The schema bytes remain +// the contract: every candidate is validated against the exact policy schema +// before this projection is trusted. +type reviewDocument struct { + Findings []reviewFinding `json:"findings"` + OverallCorrectness string `json:"overall_correctness"` + OverallExplanation string `json:"overall_explanation"` + OverallConfidence float64 `json:"overall_confidence_score"` +} + +type reviewFinding struct { + Title string `json:"title"` + Body string `json:"body"` + ConfidenceScore float64 `json:"confidence_score"` + Priority int `json:"priority"` + CodeLocation reviewAnchor `json:"code_location"` +} + +type reviewAnchor struct { + AbsoluteFilePath string `json:"absolute_file_path"` + Side string `json:"side"` + LineRange struct { + Start int `json:"start"` + End int `json:"end"` + } `json:"line_range"` +} + +// candidateSummary is the deterministic projection of one staged candidate +// that enters the domain observation. Validity is recomputed on every +// observation from the exact staged bytes and the exact current diff; the +// proposer's own claims are never trusted. +type candidateSummary struct { + Fingerprint string `json:"fingerprint"` + ReviewedTree string `json:"reviewed_tree"` + Valid bool `json:"valid"` + InvalidReasons []string `json:"invalid_reasons,omitempty"` + Verdict string `json:"verdict,omitempty"` + Measure int `json:"measure"` + FindingCount int `json:"finding_count"` + Priorities [4]int `json:"priorities"` +} + +// candidateFingerprint identifies candidate review bytes by the sha256 of +// their compacted JSON form. Compaction is a whitespace-only normalization +// (key order and literals are preserved), so the identity survives the +// re-indentation a candidate undergoes when embedded in a sealed receipt. +// Non-JSON bytes hash as-is; they are rejected later as invalid candidates. +func candidateFingerprint(candidateBytes []byte) string { + var compact bytes.Buffer + if err := json.Compact(&compact, candidateBytes); err != nil { + return sha256Hex(candidateBytes) + } + return sha256Hex(compact.Bytes()) +} + +// evaluateCandidate performs the deterministic admission checks: exact +// schema validation against the admitted policy schema bytes, diff-anchor +// validation against the exact merge-base..head diff, and the convergence +// measure. It never trusts the proposer; an unparseable or out-of-contract +// candidate is a refusal reason, not an error. +func evaluateCandidate(policy Policy, candidateBytes []byte, stagedTree, repoRoot, diff string) candidateSummary { + summary := candidateSummary{ + Fingerprint: candidateFingerprint(candidateBytes), + ReviewedTree: stagedTree, + } + reject := func(reason string) candidateSummary { + summary.Valid = false + summary.InvalidReasons = append(summary.InvalidReasons, reason) + return summary + } + document, err := jsonschema.UnmarshalJSON(bytes.NewReader(candidateBytes)) + if err != nil { + return reject("candidate is not valid JSON: " + err.Error()) + } + schemaDocument, err := jsonschema.UnmarshalJSON(bytes.NewReader(policy.SchemaBytes)) + if err != nil { + return reject("admitted output schema does not decode: " + err.Error()) + } + compiler := jsonschema.NewCompiler() + compiler.DefaultDraft(jsonschema.Draft2020) + if err := compiler.AddResource("review-output-schema.json", schemaDocument); err != nil { + return reject("admitted output schema is not compilable: " + err.Error()) + } + schema, err := compiler.Compile("review-output-schema.json") + if err != nil { + return reject("admitted output schema is not compilable: " + err.Error()) + } + if err := schema.Validate(document); err != nil { + return reject("candidate violates the admitted output schema: " + firstLine(err.Error())) + } + var review reviewDocument + decoder := json.NewDecoder(bytes.NewReader(candidateBytes)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&review); err != nil { + return reject("candidate does not decode into the review contract: " + err.Error()) + } + summary.Verdict = review.OverallCorrectness + summary.FindingCount = len(review.Findings) + allowed := changedLines(diff) + valid := true + for index, finding := range review.Findings { + if finding.Priority >= 0 && finding.Priority <= 3 { + summary.Priorities[finding.Priority]++ + summary.Measure += policy.Weights[finding.Priority] + } + if reason := anchorFailure(finding.CodeLocation, repoRoot, allowed); reason != "" { + valid = false + summary.InvalidReasons = append(summary.InvalidReasons, + fmt.Sprintf("finding %d (%q): %s", index, finding.Title, reason)) + } + } + summary.Valid = valid + return summary +} + +func firstLine(value string) string { + if index := strings.IndexByte(value, '\n'); index >= 0 { + return value[:index] + } + return value +} + +// anchorFailure checks one finding location against the allowed changed +// lines of the exact review diff. It ports the anchor discipline of the +// retired publish script: repository-relative normalized path, declared +// LEFT/RIGHT side, and every line of the range present on that side. +func anchorFailure(anchor reviewAnchor, repoRoot string, allowed map[string]map[lineKey]bool) string { + normalized, ok := normalizeAnchorPath(anchor.AbsoluteFilePath, repoRoot) + if !ok { + return "location path does not normalize to a repository-relative path" + } + side, ok := allowed[anchor.Side] + if !ok { + return "location side is not LEFT or RIGHT" + } + start, end := anchor.LineRange.Start, anchor.LineRange.End + if start > end { + return "location line range is inverted" + } + for line := start; line <= end; line++ { + if !side[lineKey{Path: normalized, Line: line}] { + return fmt.Sprintf("line %s:%d (%s) is not part of the review diff", normalized, line, anchor.Side) + } + } + return "" +} + +func normalizeAnchorPath(value, repoRoot string) (string, bool) { + candidate := strings.ReplaceAll(value, "\\", "/") + root := strings.TrimRight(strings.ReplaceAll(repoRoot, "\\", "/"), "/") + if root != "" && strings.HasPrefix(candidate, root+"/") { + candidate = candidate[len(root)+1:] + } + for strings.HasPrefix(candidate, "./") { + candidate = candidate[2:] + } + candidate = path.Clean(candidate) + if candidate == "" || candidate == "." || strings.HasPrefix(candidate, "/") || + candidate == ".." || strings.HasPrefix(candidate, "../") || strings.Contains(candidate, "/../") { + return "", false + } + return candidate, true +} + +type lineKey struct { + Path string + Line int +} + +var hunkHeader = regexp.MustCompile(`^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@`) + +// changedLines parses a zero-context unified diff into the exact sets of +// (path, line) pairs a finding may anchor to, per diff side. +func changedLines(diff string) map[string]map[lineKey]bool { + allowed := map[string]map[lineKey]bool{ + "LEFT": {}, + "RIGHT": {}, + } + var oldPath, newPath string + var oldLine, newLine int + inHunk := false + for _, line := range strings.Split(diff, "\n") { + switch { + case strings.HasPrefix(line, "diff --git "): + oldPath, newPath = "", "" + inHunk = false + case !inHunk && strings.HasPrefix(line, "--- "): + oldPath = headerPath(line) + case !inHunk && strings.HasPrefix(line, "+++ "): + newPath = headerPath(line) + default: + if match := hunkHeader.FindStringSubmatch(line); match != nil { + oldLine = mustInt(match[1]) + newLine = mustInt(match[3]) + inHunk = true + continue + } + if !inHunk || strings.HasPrefix(line, "\\") { + continue + } + switch { + case strings.HasPrefix(line, "-"): + if oldPath != "" { + allowed["LEFT"][lineKey{Path: oldPath, Line: oldLine}] = true + } + oldLine++ + case strings.HasPrefix(line, "+"): + if newPath != "" { + allowed["RIGHT"][lineKey{Path: newPath, Line: newLine}] = true + } + newLine++ + case strings.HasPrefix(line, " "): + oldLine++ + newLine++ + default: + inHunk = false + } + } + } + return allowed +} + +func headerPath(line string) string { + value := line[4:] + if tab := strings.IndexByte(value, '\t'); tab >= 0 { + value = value[:tab] + } + if value == "/dev/null" { + return "" + } + value = strings.TrimPrefix(value, "a/") + value = strings.TrimPrefix(value, "b/") + return value +} + +func mustInt(value string) int { + parsed, err := strconv.Atoi(value) + if err != nil { + return 0 + } + return parsed +} + +// stalled reports whether recording this candidate would extend the trailing +// run of submissions without measure improvement to the policy stall window. +// The run length counts the submissions themselves: with a window of three, +// a third consecutive submission that fails to decrease the measure below +// its predecessor escalates instead of recording another round. +func stalled(policy Policy, recorded []int, candidateMeasure int) bool { + if len(recorded) == 0 { + return false + } + history := append(append([]int(nil), recorded...), candidateMeasure) + runLength := 1 + for index := len(history) - 1; index >= 1; index-- { + if history[index] < history[index-1] { + break + } + runLength++ + if runLength >= policy.StallWindow { + return true + } + } + return false +} diff --git a/boatstack/cmd/boatstack-reviewer/domain.go b/boatstack/cmd/boatstack-reviewer/domain.go new file mode 100644 index 00000000..c99ccf4e --- /dev/null +++ b/boatstack/cmd/boatstack-reviewer/domain.go @@ -0,0 +1,288 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + + "github.com/operatorstack/boatstack/boatstack/kernel" +) + +// observationValue is the canonical domain observation. Everything the +// relation needs to decide admissibility is recomputed here from the exact +// repository state, the exact staged candidate bytes, and the journal — so +// any commit, candidate change, or journal advance changes the observation +// fingerprint and stales pending prescriptions before their effect. +type observationValue struct { + Instance string `json:"instance"` + BaseRef string `json:"base_ref"` + MergeBase string `json:"merge_base"` + HeadCommit string `json:"head_commit"` + ReviewedTree string `json:"reviewed_tree"` + WorktreeDirty bool `json:"worktree_dirty"` + DirtyFingerprint string `json:"dirty_fingerprint,omitempty"` + Generation int `json:"generation"` + Candidate *candidateSummary `json:"candidate,omitempty"` + Rounds []journalRound `json:"rounds"` +} + +func (v observationValue) roundMeasures() []int { + measures := make([]int, 0, len(v.Rounds)) + for _, round := range v.Rounds { + measures = append(measures, round.Measure) + } + return measures +} + +type reviewDomain struct { + repo *gitRepo + store *fileStore + policy Policy + baseRef string +} + +func (d *reviewDomain) observeValue() (observationValue, error) { + head, err := d.repo.headCommit() + if err != nil { + return observationValue{}, err + } + mergeBase, err := d.repo.mergeBase(d.baseRef, head) + if err != nil { + return observationValue{}, err + } + reviewedTree, err := d.repo.reviewedTree(head) + if err != nil { + return observationValue{}, err + } + dirty, dirtyFingerprint, err := d.repo.worktreeStatus() + if err != nil { + return observationValue{}, err + } + journal, err := d.store.loadJournal() + if err != nil { + return observationValue{}, err + } + value := observationValue{ + Instance: d.store.initial.InstanceID, + BaseRef: d.baseRef, + MergeBase: mergeBase, + HeadCommit: head, + ReviewedTree: reviewedTree, + WorktreeDirty: dirty, + DirtyFingerprint: dirtyFingerprint, + Generation: journal.Generation, + Rounds: journal.currentRounds(), + } + if value.Rounds == nil { + value.Rounds = []journalRound{} + } + candidateBytes, meta, staged, err := d.store.loadStagedCandidate() + if err != nil { + return observationValue{}, err + } + if staged { + diff, diffErr := d.repo.pullRequestDiff(mergeBase, head) + if diffErr != nil { + return observationValue{}, diffErr + } + summary := evaluateCandidate(d.policy, candidateBytes, meta.ReviewedTree, d.repo.Root, diff) + value.Candidate = &summary + } + return value, nil +} + +func (d *reviewDomain) Observe(context.Context, string) (kernel.Observation, error) { + value, err := d.observeValue() + if err != nil { + return kernel.Observation{}, err + } + return kernel.NewObservation(value) +} + +// Admissible is the domain half of the canonical relation. Resolve and apply +// share it; nothing here mutates anything. +func (d *reviewDomain) Admissible(_ context.Context, evaluation kernel.Evaluation) (bool, string, error) { + var observed observationValue + if err := json.Unmarshal(evaluation.Observation.Value, &observed); err != nil { + return false, "", err + } + switch evaluation.Transition.Operation { + case transitionConverge, transitionRecord, transitionEscalate: + allowed, reason := submissionDisposition(d.policy, observed) + return allowed == evaluation.Transition.Operation, reason, nil + case transitionReopen: + return true, "reopening starts a fresh review generation", nil + case transitionRecover: + // A declared recovery transition stays admissible while its + // recovery state is active, including after a partial cleanup. + return evaluation.State.Recovery != nil, "recovery requires an active recovery state", nil + default: + return false, "unknown review operation", nil + } +} + +// submissionDisposition decides which single submission transition the +// current observation admits, with the refusal reason when none applies. +// Convergence, recording, and escalation are mutually exclusive by +// construction, so the relation can never reach an ambiguity frontier. +func submissionDisposition(policy Policy, observed observationValue) (string, string) { + if observed.Candidate == nil { + return "", "no candidate review is staged; run submit with --findings" + } + if observed.WorktreeDirty { + return "", "the worktree has uncommitted tracked changes; a review binds only a committed tree" + } + candidate := *observed.Candidate + if candidate.ReviewedTree != observed.ReviewedTree { + return "", "the staged candidate was produced for a different reviewed tree; re-review the current commit" + } + if !candidate.Valid { + reasons := "candidate is invalid" + for _, reason := range candidate.InvalidReasons { + reasons += "; " + reason + } + return "", reasons + } + if candidate.Verdict == verdictCorrect { + return transitionConverge, "candidate verdict accepts the patch" + } + rounds := observed.roundMeasures() + if len(rounds) >= policy.MaxRounds { + return transitionEscalate, fmt.Sprintf("round bound %d is exhausted", policy.MaxRounds) + } + if stalled(policy, rounds, candidate.Measure) { + return transitionEscalate, fmt.Sprintf("measure has not decreased for %d consecutive submissions", policy.StallWindow) + } + return transitionRecord, "candidate records open findings with a decreasing measure" +} + +// reviewOperator applies the one admitted operation. It receives only +// admitted operations from the kernel; the candidate it archives is the +// exact staged bytes the observation was computed from. +type reviewOperator struct { + store *fileStore +} + +func (o reviewOperator) Execute(_ context.Context, operation kernel.Operation) (kernel.Effect, error) { + var observed observationValue + if err := json.Unmarshal(operation.Observation.Value, &observed); err != nil { + return kernel.Effect{}, err + } + switch operation.Transition.Operation { + case transitionConverge, transitionRecord, transitionEscalate: + if observed.Candidate == nil { + return kernel.Effect{}, fmt.Errorf("no staged candidate to archive") + } + candidateBytes, meta, staged, err := o.store.loadStagedCandidate() + if err != nil { + return kernel.Effect{}, err + } + if !staged || meta.Fingerprint != observed.Candidate.Fingerprint { + return kernel.Effect{}, fmt.Errorf("staged candidate changed after resolution") + } + round := journalRound{ + CandidateFingerprint: observed.Candidate.Fingerprint, + ReviewedTree: observed.ReviewedTree, + HeadCommit: observed.HeadCommit, + MergeBase: observed.MergeBase, + Verdict: observed.Candidate.Verdict, + Measure: observed.Candidate.Measure, + FindingCount: observed.Candidate.FindingCount, + Priorities: observed.Candidate.Priorities, + Transition: operation.Transition.ID, + } + if err := o.store.archiveRound(candidateBytes, round); err != nil { + return kernel.Effect{}, err + } + return kernel.Effect{Facts: []kernel.EffectFact{{ + Facet: facetRound, + Operation: operation.Transition.Operation, + Fingerprint: observed.Candidate.Fingerprint, + }}}, nil + case transitionReopen: + if err := o.store.nextGeneration(); err != nil { + return kernel.Effect{}, err + } + journal, err := o.store.loadJournal() + if err != nil { + return kernel.Effect{}, err + } + return kernel.Effect{Facts: []kernel.EffectFact{{ + Facet: facetRound, + Operation: operation.Transition.Operation, + Fingerprint: fmt.Sprintf("generation-%d", journal.Generation), + }}}, nil + case transitionRecover: + if err := o.store.clearStagedCandidate(); err != nil { + return kernel.Effect{}, err + } + return kernel.Effect{Facts: []kernel.EffectFact{{ + Facet: facetRound, + Operation: operation.Transition.Operation, + Fingerprint: "staging-cleared", + }}}, nil + default: + return kernel.Effect{}, fmt.Errorf("unknown review operation %q", operation.Transition.Operation) + } +} + +// Verify checks the fresh post-effect observation against the transition's +// declared postcondition. The operator's own summary is never trusted. +func (d *reviewDomain) Verify(_ context.Context, evaluation kernel.Evaluation, effect kernel.Effect, target kernel.Observation) error { + var before, after observationValue + if err := json.Unmarshal(evaluation.Observation.Value, &before); err != nil { + return err + } + if err := json.Unmarshal(target.Value, &after); err != nil { + return err + } + switch evaluation.Transition.Operation { + case transitionConverge, transitionRecord, transitionEscalate: + if before.Candidate == nil { + return fmt.Errorf("no candidate was under review") + } + if after.Candidate != nil { + return fmt.Errorf("staged candidate survived its own admission") + } + if len(after.Rounds) != len(before.Rounds)+1 { + return fmt.Errorf("exactly one round must be recorded; journal grew from %d to %d", len(before.Rounds), len(after.Rounds)) + } + recorded := after.Rounds[len(after.Rounds)-1] + if recorded.CandidateFingerprint != before.Candidate.Fingerprint || + recorded.ReviewedTree != before.ReviewedTree || + recorded.Verdict != before.Candidate.Verdict || + recorded.Measure != before.Candidate.Measure || + recorded.Transition != evaluation.Transition.ID { + return fmt.Errorf("recorded round does not match the admitted candidate") + } + archived, err := d.store.roundBytes(recorded.CandidateFingerprint) + if err != nil { + return fmt.Errorf("archived candidate is unavailable: %w", err) + } + if candidateFingerprint(archived) != recorded.CandidateFingerprint { + return fmt.Errorf("archived candidate bytes do not match the recorded fingerprint") + } + if len(effect.Facts) != 1 || effect.Facts[0].Fingerprint != recorded.CandidateFingerprint { + return fmt.Errorf("effect facts do not identify the archived candidate") + } + return nil + case transitionReopen: + if after.Generation != before.Generation+1 { + return fmt.Errorf("reopen must advance exactly one generation") + } + if len(after.Rounds) != 0 { + return fmt.Errorf("a fresh generation must start with no rounds") + } + if after.Candidate != nil { + return fmt.Errorf("reopen must clear any staged candidate") + } + return nil + case transitionRecover: + if after.Candidate != nil { + return fmt.Errorf("recovery must clear the staged candidate") + } + return nil + default: + return fmt.Errorf("unknown review operation %q", evaluation.Transition.Operation) + } +} diff --git a/boatstack/cmd/boatstack-reviewer/gitrepo.go b/boatstack/cmd/boatstack-reviewer/gitrepo.go new file mode 100644 index 00000000..12acde7f --- /dev/null +++ b/boatstack/cmd/boatstack-reviewer/gitrepo.go @@ -0,0 +1,165 @@ +package main + +import ( + "bytes" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" +) + +// receiptDirectory is the tracked directory that carries sealed review +// receipts. It is excluded from the reviewed-tree binding so that committing +// a sealed receipt does not invalidate the tree it binds. +const receiptDirectory = ".github/reviews" + +type gitRepo struct { + Root string + GitDir string +} + +func openRepo(path string) (*gitRepo, error) { + root, err := gitOutput(path, "rev-parse", "--show-toplevel") + if err != nil { + return nil, fmt.Errorf("%s is not inside a Git repository: %w", path, err) + } + gitDir, err := gitOutput(path, "rev-parse", "--absolute-git-dir") + if err != nil { + return nil, err + } + return &gitRepo{Root: strings.TrimSpace(root), GitDir: strings.TrimSpace(gitDir)}, nil +} + +func gitOutput(dir string, args ...string) (string, error) { + command := exec.Command("git", args...) + command.Dir = dir + var stdout, stderr bytes.Buffer + command.Stdout = &stdout + command.Stderr = &stderr + if err := command.Run(); err != nil { + return "", fmt.Errorf("git %s: %v: %s", strings.Join(args, " "), err, strings.TrimSpace(stderr.String())) + } + return stdout.String(), nil +} + +func (r *gitRepo) output(args ...string) (string, error) { + return gitOutput(r.Root, args...) +} + +func (r *gitRepo) revParse(revision string) (string, error) { + value, err := r.output("rev-parse", "--verify", revision+"^{commit}") + if err != nil { + return "", err + } + return strings.TrimSpace(value), nil +} + +func (r *gitRepo) headCommit() (string, error) { + return r.revParse("HEAD") +} + +func (r *gitRepo) mergeBase(baseRef, head string) (string, error) { + value, err := r.output("merge-base", baseRef, head) + if err != nil { + return "", fmt.Errorf("merge base of %s and %s is unavailable: %w", baseRef, head, err) + } + return strings.TrimSpace(value), nil +} + +// reviewedTree computes the receipt-excluded tree identity of one commit: +// the git tree hash with the sealed-receipt directory removed. Two commits +// that differ only in sealed receipts share one reviewed tree, which is what +// lets a converged receipt be committed without invalidating itself. +func (r *gitRepo) reviewedTree(commit string) (string, error) { + indexFile, err := os.CreateTemp("", "boatstack-review-index-") + if err != nil { + return "", err + } + indexPath := indexFile.Name() + indexFile.Close() + os.Remove(indexPath) + defer os.Remove(indexPath) + environment := append(os.Environ(), "GIT_INDEX_FILE="+indexPath) + run := func(args ...string) (string, error) { + command := exec.Command("git", args...) + command.Dir = r.Root + command.Env = environment + var stdout, stderr bytes.Buffer + command.Stdout = &stdout + command.Stderr = &stderr + if err := command.Run(); err != nil { + return "", fmt.Errorf("git %s: %v: %s", strings.Join(args, " "), err, strings.TrimSpace(stderr.String())) + } + return stdout.String(), nil + } + if _, err := run("read-tree", commit+"^{tree}"); err != nil { + return "", err + } + if _, err := run("rm", "--cached", "-r", "-q", "--ignore-unmatch", "--", receiptDirectory); err != nil { + return "", err + } + tree, err := run("write-tree") + if err != nil { + return "", err + } + return strings.TrimSpace(tree), nil +} + +// worktreeStatus reports whether the worktree differs from HEAD. Paths under +// the sealed-receipt directory and untracked files outside it do not affect +// the review decision but tracked modifications do: a review can only bind a +// tree that a commit can reproduce. +func (r *gitRepo) worktreeStatus() (dirty bool, fingerprint string, err error) { + porcelain, err := r.output("status", "--porcelain") + if err != nil { + return false, "", err + } + var relevant []string + for _, line := range strings.Split(porcelain, "\n") { + if strings.TrimSpace(line) == "" { + continue + } + entry := line + if len(entry) >= 3 { + path := strings.TrimSpace(entry[3:]) + if renamed := strings.SplitN(path, " -> ", 2); len(renamed) == 2 { + path = renamed[1] + } + path = strings.Trim(path, "\"") + if strings.HasPrefix(path, receiptDirectory+"/") { + continue + } + if strings.HasPrefix(entry, "??") { + continue + } + } + relevant = append(relevant, entry) + } + if len(relevant) == 0 { + return false, "", nil + } + return true, sha256Hex([]byte(strings.Join(relevant, "\n"))), nil +} + +// pullRequestDiff reproduces the exact diff basis the retired CI reviewer +// used: merge-base to head, zero context, no renames, no external drivers. +func (r *gitRepo) pullRequestDiff(mergeBase, head string) (string, error) { + return r.output( + "-c", "core.quotePath=false", + "diff", "--no-ext-diff", "--no-renames", "--unified=0", + mergeBase, head, + ) +} + +func (r *gitRepo) showFile(revision, path string) ([]byte, error) { + value, err := r.output("show", revision+":"+path) + if err != nil { + return nil, err + } + return []byte(value), nil +} + +func (r *gitRepo) receiptDirectoryPath() string { + return filepath.Join(r.Root, filepath.FromSlash(receiptDirectory)) +} diff --git a/boatstack/cmd/boatstack-reviewer/main.go b/boatstack/cmd/boatstack-reviewer/main.go new file mode 100644 index 00000000..334ca462 --- /dev/null +++ b/boatstack/cmd/boatstack-reviewer/main.go @@ -0,0 +1,594 @@ +// Command boatstack-reviewer runs Boatstack's self-review as a supervisory +// control program over the domain-neutral kernel. +// +// The loop runs locally, driven by a coding agent or a human. The proposer +// is untrusted: it produces candidate review findings under the admitted +// review policy; this program owns admissibility, freshness, convergence, +// receipts, and recovery through the exact kernel relation. The sealed +// converged receipt travels with the pull request, and CI verifies it +// deterministically — no reviewer, model, or API key runs in CI. +package main + +import ( + "context" + "encoding/json" + "flag" + "fmt" + "os" + "path/filepath" + "strings" + "time" + + "github.com/operatorstack/boatstack/boatstack/kernel" +) + +func main() { + if err := run(os.Args[1:]); err != nil { + fmt.Fprintln(os.Stderr, "boatstack-reviewer:", err) + os.Exit(1) + } +} + +func run(arguments []string) error { + if len(arguments) == 0 { + return fmt.Errorf("usage: boatstack-reviewer [flags]") + } + command, rest := arguments[0], arguments[1:] + switch command { + case "resolve": + return commandResolve(rest) + case "submit": + return commandSubmit(rest) + case "status": + return commandStatus(rest) + case "show": + return commandShow(rest) + case "seal": + return commandSeal(rest) + case "verify": + return commandVerify(rest) + case "reopen": + return commandReopen(rest) + case "recover": + return commandRecover(rest) + case "reset": + return commandReset(rest) + default: + return fmt.Errorf("unknown command %q", command) + } +} + +type loopContext struct { + repo *gitRepo + policy Policy + program kernel.Program + store *fileStore + domain *reviewDomain + operator reviewOperator + clock systemClock + instance string + baseRef string +} + +func commonFlags(set *flag.FlagSet) (repoPath, delivery, baseRef, actor *string) { + repoPath = set.String("repo", ".", "repository path") + delivery = set.String("delivery", "", "control instance identity (default: derived from the current branch)") + baseRef = set.String("base", "origin/main", "review base reference") + actor = set.String("actor", "", "acting identity recorded on receipts") + return repoPath, delivery, baseRef, actor +} + +func newLoopContext(repoPath, delivery, baseRef string) (*loopContext, error) { + repo, err := openRepo(repoPath) + if err != nil { + return nil, err + } + policy, err := loadWorktreePolicy(repo.Root) + if err != nil { + return nil, err + } + program, err := compileReviewProgram(policy) + if err != nil { + return nil, err + } + instance := delivery + if instance == "" { + branch, err := repo.output("rev-parse", "--abbrev-ref", "HEAD") + if err != nil { + return nil, err + } + branch = strings.TrimSpace(branch) + if branch == "HEAD" { + return nil, fmt.Errorf("detached HEAD has no branch identity; pass --delivery") + } + instance, err = instanceIDForBranch(branch) + if err != nil { + return nil, err + } + } + store := newFileStore(repo.GitDir, instance, program.Identity()) + domain := &reviewDomain{repo: repo, store: store, policy: policy, baseRef: baseRef} + return &loopContext{ + repo: repo, + policy: policy, + program: program, + store: store, + domain: domain, + operator: reviewOperator{store: store}, + instance: instance, + baseRef: baseRef, + }, nil +} + +func (c *loopContext) runtime() (kernel.Runtime, error) { + return kernel.NewRuntime( + c.program, c.domain, c.operator, reviewCapabilities{}, + c.store, directoryLocker{path: c.store.lockPath()}, c.clock, + ) +} + +func (c *loopContext) authority(actor string, capabilities ...kernel.Capability) (kernel.Authority, error) { + return localAuthority(actor, c.clock.Now(), capabilities...) +} + +func printJSON(value any) error { + encoder := json.NewEncoder(os.Stdout) + encoder.SetIndent("", " ") + return encoder.Encode(value) +} + +// dispositionView reports which submission transition the current +// observation admits (empty when none does) and the exact domain reason. +type dispositionView struct { + Admits string `json:"admits,omitempty"` + Reason string `json:"reason"` +} + +type instructionsView struct { + PromptPath string `json:"prompt_path"` + PromptSHA256 string `json:"prompt_sha256"` + ReviewRange string `json:"review_range"` + ReviewedTree string `json:"reviewed_tree"` + SchemaPath string `json:"output_schema_path"` + SchemaSHA256 string `json:"output_schema_sha256"` + SubmitCommand string `json:"submit_command"` +} + +func commandResolve(arguments []string) error { + set := flag.NewFlagSet("resolve", flag.ContinueOnError) + repoPath, delivery, baseRef, actor := commonFlags(set) + if err := set.Parse(arguments); err != nil { + return err + } + if *actor == "" { + *actor = "reviewer" + } + loop, err := newLoopContext(*repoPath, *delivery, *baseRef) + if err != nil { + return err + } + runtime, err := loop.runtime() + if err != nil { + return err + } + authority, err := loop.authority(*actor, capabilitySubmit) + if err != nil { + return err + } + resolution, err := runtime.Resolve(context.Background(), kernel.ResolveRequest{ + InstanceID: loop.instance, + Authority: authority, + }) + if err != nil { + return err + } + observed, err := loop.domain.observeValue() + if err != nil { + return err + } + // The kernel's decision reason is generic by design; the domain's + // submission disposition names the exact blocking or admitting + // condition so a driver never has to infer it from the observation. + admits, reason := submissionDisposition(loop.policy, observed) + return printJSON(struct { + Instance string `json:"instance"` + Program kernel.ProgramIdentity `json:"program"` + State kernel.ControlState `json:"state"` + Decision kernel.Decision `json:"decision"` + Disposition dispositionView `json:"submission_disposition"` + Prescription *kernel.Prescription `json:"prescription,omitempty"` + Observation observationValue `json:"observation"` + Instructions instructionsView `json:"instructions"` + }{ + Instance: loop.instance, + Program: loop.program.Identity(), + State: resolution.State, + Decision: resolution.Decision, + Disposition: dispositionView{Admits: admits, Reason: reason}, + Prescription: resolution.Prescription, + Observation: observed, + Instructions: instructionsView{ + PromptPath: policyPromptPath, + PromptSHA256: loop.policy.PromptSHA256, + ReviewRange: observed.MergeBase + ".." + observed.HeadCommit, + ReviewedTree: observed.ReviewedTree, + SchemaPath: policySchemaPath, + SchemaSHA256: loop.policy.SchemaSHA256, + SubmitCommand: "boatstack-reviewer submit --findings --actor ", + }, + }) +} + +func commandSubmit(arguments []string) error { + set := flag.NewFlagSet("submit", flag.ContinueOnError) + repoPath, delivery, baseRef, actor := commonFlags(set) + findings := set.String("findings", "", "path to the candidate review findings JSON") + if err := set.Parse(arguments); err != nil { + return err + } + if *findings == "" { + return fmt.Errorf("submit requires --findings ") + } + loop, err := newLoopContext(*repoPath, *delivery, *baseRef) + if err != nil { + return err + } + candidateBytes, err := os.ReadFile(*findings) + if err != nil { + return err + } + head, err := loop.repo.headCommit() + if err != nil { + return err + } + reviewedTree, err := loop.repo.reviewedTree(head) + if err != nil { + return err + } + if err := loop.store.stageCandidate(candidateBytes, reviewedTree); err != nil { + return err + } + runtime, err := loop.runtime() + if err != nil { + return err + } + authority, err := loop.authority(*actor, capabilitySubmit) + if err != nil { + return err + } + request := kernel.ResolveRequest{InstanceID: loop.instance, Authority: authority} + resolution, err := runtime.Resolve(context.Background(), request) + if err != nil { + return err + } + if resolution.Decision.Kind != kernel.Prescribed || resolution.Prescription == nil { + printJSON(struct { + Instance string `json:"instance"` + State kernel.ControlState `json:"state"` + Decision kernel.Decision `json:"decision"` + }{loop.instance, resolution.State, resolution.Decision}) + return fmt.Errorf("submission refused: %s", resolution.Decision.Reason) + } + receipt, err := runtime.Apply(context.Background(), kernel.ApplyRequest{ + ResolveRequest: request, + Prescription: *resolution.Prescription, + }) + if err != nil { + return fmt.Errorf("submission did not commit: %w", err) + } + state, err := loop.store.Load(context.Background(), loop.instance) + if err != nil { + return err + } + observed, err := loop.domain.observeValue() + if err != nil { + return err + } + return printJSON(struct { + Instance string `json:"instance"` + Mode string `json:"mode"` + Receipt kernel.Receipt `json:"receipt"` + Rounds []journalRound `json:"rounds"` + Guidance string `json:"guidance"` + }{ + Instance: loop.instance, + Mode: state.Mode, + Receipt: receipt, + Rounds: observed.Rounds, + Guidance: submissionGuidance(state.Mode), + }) +} + +func submissionGuidance(mode string) string { + switch mode { + case modeConverged: + return "review converged; run `boatstack-reviewer seal` and commit the sealed receipt" + case modeFindingsOpen: + return "findings are open; fix them, commit, and submit a fresh review of the new tree" + case modeEscalated: + return "the loop escalated; a human must decide, then `boatstack-reviewer reopen --actor `" + default: + return "" + } +} + +func commandStatus(arguments []string) error { + set := flag.NewFlagSet("status", flag.ContinueOnError) + repoPath, delivery, baseRef, _ := commonFlags(set) + if err := set.Parse(arguments); err != nil { + return err + } + loop, err := newLoopContext(*repoPath, *delivery, *baseRef) + if err != nil { + return err + } + state, err := loop.store.Load(context.Background(), loop.instance) + if err != nil { + return err + } + observed, err := loop.domain.observeValue() + if err != nil { + return err + } + stale := state.Program != loop.program.Identity() + return printJSON(struct { + Instance string `json:"instance"` + Program kernel.ProgramIdentity `json:"program"` + State kernel.ControlState `json:"state"` + ProgramStale bool `json:"program_stale"` + Observation observationValue `json:"observation"` + Guidance string `json:"guidance,omitempty"` + }{ + Instance: loop.instance, + Program: loop.program.Identity(), + State: state, + ProgramStale: stale, + Observation: observed, + Guidance: func() string { + if stale { + return "the admitted policy or law changed since this state was committed; `boatstack-reviewer reset --confirm` archives it" + } + return submissionGuidance(state.Mode) + }(), + }) +} + +// commandShow prints a recorded review itself — the exact archived findings +// bytes — without resolving or changing anything. By default it shows the +// latest recorded round of the current generation; --round selects an earlier +// one, and a staged, not-yet-submitted candidate is included when present. +func commandShow(arguments []string) error { + set := flag.NewFlagSet("show", flag.ContinueOnError) + repoPath, delivery, baseRef, _ := commonFlags(set) + roundIndex := set.Int("round", 0, "round index to show (default: the latest recorded round)") + if err := set.Parse(arguments); err != nil { + return err + } + loop, err := newLoopContext(*repoPath, *delivery, *baseRef) + if err != nil { + return err + } + state, err := loop.store.Load(context.Background(), loop.instance) + if err != nil { + return err + } + journal, err := loop.store.loadJournal() + if err != nil { + return err + } + rounds := journal.currentRounds() + + var selected *journalRound + if *roundIndex > 0 { + for index := range rounds { + if rounds[index].Index == *roundIndex { + selected = &rounds[index] + break + } + } + if selected == nil { + return fmt.Errorf("generation %d has no round %d (%d recorded)", journal.Generation, *roundIndex, len(rounds)) + } + } else if len(rounds) > 0 { + selected = &rounds[len(rounds)-1] + } + + var review json.RawMessage + if selected != nil { + archived, err := loop.store.roundBytes(selected.CandidateFingerprint) + if err != nil { + return fmt.Errorf("archived review is unavailable: %w", err) + } + review = json.RawMessage(archived) + } + + var staged *struct { + Evaluation candidateSummary `json:"evaluation"` + Review json.RawMessage `json:"review"` + } + if candidateBytes, _, ok, err := loop.store.loadStagedCandidate(); err != nil { + return err + } else if ok { + observed, err := loop.domain.observeValue() + if err != nil { + return err + } + staged = &struct { + Evaluation candidateSummary `json:"evaluation"` + Review json.RawMessage `json:"review"` + }{Evaluation: *observed.Candidate, Review: json.RawMessage(candidateBytes)} + } + + if selected == nil && staged == nil { + return fmt.Errorf("instance %s has no recorded rounds and no staged candidate; run `boatstack-reviewer resolve` for instructions", loop.instance) + } + return printJSON(struct { + Instance string `json:"instance"` + Mode string `json:"mode"` + Generation int `json:"generation"` + Rounds []journalRound `json:"rounds"` + Round *journalRound `json:"round,omitempty"` + Review json.RawMessage `json:"review,omitempty"` + Staged any `json:"staged_candidate,omitempty"` + }{ + Instance: loop.instance, + Mode: state.Mode, + Generation: journal.Generation, + Rounds: rounds, + Round: selected, + Review: review, + Staged: func() any { + if staged == nil { + return nil + } + return staged + }(), + }) +} + +func commandSeal(arguments []string) error { + set := flag.NewFlagSet("seal", flag.ContinueOnError) + repoPath, delivery, baseRef, _ := commonFlags(set) + output := set.String("output", "", "sealed receipt path (default: .github/reviews/.receipt.json)") + if err := set.Parse(arguments); err != nil { + return err + } + loop, err := newLoopContext(*repoPath, *delivery, *baseRef) + if err != nil { + return err + } + receipt, err := buildSealedReceipt(loop.repo, loop.store, loop.policy, loop.program, loop.baseRef, loop.clock.Now()) + if err != nil { + return err + } + path := *output + if path == "" { + path = filepath.Join(loop.repo.receiptDirectoryPath(), loop.instance+".receipt.json") + } + if err := writeSealedReceipt(path, receipt); err != nil { + return err + } + return printJSON(struct { + Sealed string `json:"sealed"` + Fingerprint string `json:"fingerprint"` + ReviewedTree string `json:"reviewed_tree"` + Guidance string `json:"guidance"` + }{path, receipt.Fingerprint, receipt.ReviewedTree, "commit this file with the pull request; CI verifies it deterministically"}) +} + +func commandVerify(arguments []string) error { + set := flag.NewFlagSet("verify", flag.ContinueOnError) + repoPath := set.String("repo", ".", "repository path") + receiptPath := set.String("receipt", "", "sealed receipt path (default: scan --dir for the head tree)") + directory := set.String("dir", receiptDirectory, "receipt directory to scan") + baseRevision := set.String("base", "", "pull request base revision (policy admission source)") + headRevision := set.String("head", "", "pull request head revision (tree binding target)") + if err := set.Parse(arguments); err != nil { + return err + } + if *baseRevision == "" || *headRevision == "" { + return fmt.Errorf("verify requires --base and --head revisions") + } + repo, err := openRepo(*repoPath) + if err != nil { + return err + } + var receipt SealedReceipt + path := *receiptPath + if path != "" { + receipt, err = readSealedReceipt(path) + } else { + scanDir := *directory + if !filepath.IsAbs(scanDir) { + scanDir = filepath.Join(repo.Root, filepath.FromSlash(scanDir)) + } + receipt, path, err = findReceiptForHead(repo, scanDir, *headRevision) + } + if err != nil { + printJSON(verificationReport{Failures: []string{err.Error()}, Checks: []string{}, Warnings: []string{}}) + return err + } + report := verifySealedReceipt(repo, receipt, path, *baseRevision, *headRevision) + if err := printJSON(report); err != nil { + return err + } + if !report.Verified { + return fmt.Errorf("review verification failed: %s", strings.Join(report.Failures, "; ")) + } + return nil +} + +func commandReopen(arguments []string) error { + return commandRequested(arguments, "reopen", transitionReopen, capabilityHuman) +} + +func commandRecover(arguments []string) error { + return commandRequested(arguments, "recover", transitionRecover, capabilityRecover) +} + +func commandRequested(arguments []string, name, transition string, capability kernel.Capability) error { + set := flag.NewFlagSet(name, flag.ContinueOnError) + repoPath, delivery, baseRef, actor := commonFlags(set) + if err := set.Parse(arguments); err != nil { + return err + } + loop, err := newLoopContext(*repoPath, *delivery, *baseRef) + if err != nil { + return err + } + runtime, err := loop.runtime() + if err != nil { + return err + } + authority, err := loop.authority(*actor, capability) + if err != nil { + return err + } + request := kernel.ResolveRequest{ + InstanceID: loop.instance, + Authority: authority, + Requested: transition, + } + resolution, err := runtime.Resolve(context.Background(), request) + if err != nil { + return err + } + if resolution.Decision.Kind != kernel.Prescribed || resolution.Prescription == nil { + printJSON(resolution.Decision) + return fmt.Errorf("%s refused: %s", name, resolution.Decision.Reason) + } + receipt, err := runtime.Apply(context.Background(), kernel.ApplyRequest{ + ResolveRequest: request, + Prescription: *resolution.Prescription, + }) + if err != nil { + return fmt.Errorf("%s did not commit: %w", name, err) + } + return printJSON(receipt) +} + +func commandReset(arguments []string) error { + set := flag.NewFlagSet("reset", flag.ContinueOnError) + repoPath, delivery, baseRef, _ := commonFlags(set) + confirm := set.Bool("confirm", false, "confirm archiving the instance's local review state") + if err := set.Parse(arguments); err != nil { + return err + } + loop, err := newLoopContext(*repoPath, *delivery, *baseRef) + if err != nil { + return err + } + if !*confirm { + return fmt.Errorf("reset archives %s; pass --confirm to proceed", loop.store.dir) + } + if _, err := os.Stat(loop.store.dir); os.IsNotExist(err) { + return fmt.Errorf("instance %s has no local review state", loop.instance) + } + archived, err := loop.store.archive(time.Now().UTC().Format("20060102T150405Z")) + if err != nil { + return err + } + return printJSON(struct { + Archived string `json:"archived"` + }{archived}) +} diff --git a/boatstack/cmd/boatstack-reviewer/policy.go b/boatstack/cmd/boatstack-reviewer/policy.go new file mode 100644 index 00000000..ce6dc140 --- /dev/null +++ b/boatstack/cmd/boatstack-reviewer/policy.go @@ -0,0 +1,136 @@ +package main + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "os" + "path/filepath" +) + +// The admitted review policy is the pair of repository assets the previous +// CI reviewer already pinned to the pull-request base revision, plus the +// convergence bounds. All of it is hashed into the review Program's domain +// contract fingerprint: changing any part produces a different program +// identity and honestly invalidates every prior prescription and receipt. +const ( + policyPromptPath = ".github/codex/review-prompt.md" + policySchemaPath = ".github/codex/review-output-schema.json" + + // defaultMaxRounds bounds one review generation. The mined fixture + // (testdata/review_rounds.json) observed a maximum of 14 review rounds + // on one pull request; 16 covers every observed loop with headroom. + defaultMaxRounds = 16 + + // defaultStallWindow is the number of consecutive non-improving + // submissions after which the loop escalates instead of recording + // another round. The mined history shows measures normally move within + // one to two rounds of a fix landing. + defaultStallWindow = 3 +) + +// defaultWeights maps finding priority P0..P3 to its weight in the +// convergence measure V = sum(weight(priority)) over open findings. +var defaultWeights = [4]int{1000, 100, 10, 1} + +type Policy struct { + PromptPath string `json:"prompt_path"` + PromptSHA256 string `json:"prompt_sha256"` + SchemaPath string `json:"schema_path"` + SchemaSHA256 string `json:"schema_sha256"` + MaxRounds int `json:"max_rounds"` + StallWindow int `json:"stall_window"` + Weights [4]int `json:"weights"` + + PromptBytes []byte `json:"-"` + SchemaBytes []byte `json:"-"` +} + +func sha256Hex(value []byte) string { + digest := sha256.Sum256(value) + return hex.EncodeToString(digest[:]) +} + +func newPolicy(promptBytes, schemaBytes []byte) Policy { + return Policy{ + PromptPath: policyPromptPath, + PromptSHA256: sha256Hex(promptBytes), + SchemaPath: policySchemaPath, + SchemaSHA256: sha256Hex(schemaBytes), + MaxRounds: defaultMaxRounds, + StallWindow: defaultStallWindow, + Weights: defaultWeights, + PromptBytes: promptBytes, + SchemaBytes: schemaBytes, + } +} + +// loadWorktreePolicy reads the policy assets from the repository worktree. +// The local loop always reviews under the policy present in the tree being +// reviewed; CI verification separately re-admits the policy from the pull +// request base revision. +func loadWorktreePolicy(repoRoot string) (Policy, error) { + prompt, err := os.ReadFile(filepath.Join(repoRoot, filepath.FromSlash(policyPromptPath))) + if err != nil { + return Policy{}, fmt.Errorf("review policy prompt is unavailable: %w", err) + } + schema, err := os.ReadFile(filepath.Join(repoRoot, filepath.FromSlash(policySchemaPath))) + if err != nil { + return Policy{}, fmt.Errorf("review policy schema is unavailable: %w", err) + } + return newPolicy(prompt, schema), nil +} + +// loadRevisionPolicy reads the policy assets from an exact committed +// revision, mirroring the base-revision admission the retired CI reviewer +// performed with `git show "$BASE_SHA:"`. +func loadRevisionPolicy(repo *gitRepo, revision string) (Policy, error) { + prompt, err := repo.showFile(revision, policyPromptPath) + if err != nil { + return Policy{}, fmt.Errorf("revision %s has no admitted review policy prompt: %w", revision, err) + } + schema, err := repo.showFile(revision, policySchemaPath) + if err != nil { + return Policy{}, fmt.Errorf("revision %s has no admitted review policy schema: %w", revision, err) + } + return newPolicy(prompt, schema), nil +} + +// contractFingerprint is the domain contract identity compiled into the +// review Program. It covers the exact policy asset bytes and the exact +// convergence bounds; the executable transition law is separately covered +// by the kernel Program fingerprint. +func (p Policy) contractFingerprint() (string, error) { + encoded, err := json.Marshal(struct { + PromptPath string `json:"prompt_path"` + PromptSHA256 string `json:"prompt_sha256"` + SchemaPath string `json:"schema_path"` + SchemaSHA256 string `json:"schema_sha256"` + MaxRounds int `json:"max_rounds"` + StallWindow int `json:"stall_window"` + Weights [4]int `json:"weights"` + }{p.PromptPath, p.PromptSHA256, p.SchemaPath, p.SchemaSHA256, p.MaxRounds, p.StallWindow, p.Weights}) + if err != nil { + return "", err + } + return sha256Hex(encoded), nil +} + +func (p Policy) validate() error { + if len(p.PromptBytes) == 0 || len(p.SchemaBytes) == 0 { + return fmt.Errorf("review policy requires non-empty prompt and schema assets") + } + if p.PromptSHA256 != sha256Hex(p.PromptBytes) || p.SchemaSHA256 != sha256Hex(p.SchemaBytes) { + return fmt.Errorf("review policy asset hashes do not identify their exact bytes") + } + if p.MaxRounds < 1 || p.StallWindow < 1 { + return fmt.Errorf("review policy requires positive round and stall bounds") + } + for _, weight := range p.Weights { + if weight < 1 { + return fmt.Errorf("review policy requires positive priority weights") + } + } + return nil +} diff --git a/boatstack/cmd/boatstack-reviewer/program.go b/boatstack/cmd/boatstack-reviewer/program.go new file mode 100644 index 00000000..aa8f6ff3 --- /dev/null +++ b/boatstack/cmd/boatstack-reviewer/program.go @@ -0,0 +1,144 @@ +package main + +import ( + "fmt" + + "github.com/operatorstack/boatstack/boatstack/kernel" +) + +// Review control modes. The first mode is the initial mode; `converged` is +// the only marked mode, so an untargeted resolve on a converged instance +// answers MARKED instead of prescribing further work. +const ( + modeUnreviewed = "unreviewed" + modeFindingsOpen = "findings-open" + modeConverged = "converged" + modeEscalated = "escalated" +) + +// Transition identities. +const ( + transitionConverge = "review.converge" + transitionRecord = "review.findings.record" + transitionEscalate = "review.escalate" + transitionReopen = "review.reopen" + transitionRecover = "review.recover" +) + +// Capabilities. The proposer (a coding agent or a human running the loop) +// submits candidates with `review.submit`; reopening a settled generation is +// a human decision (`review.human`); recovery is explicit (`review.recover`). +const ( + capabilitySubmit = kernel.Capability("review.submit") + capabilityHuman = kernel.Capability("review.human") + capabilityRecover = kernel.Capability("review.recover") +) + +const facetRound = "review.round" + +const ( + programID = "boatstack-reviewer" + programVersion = "1" + programRuntime = "kernel-v1" +) + +// compileReviewProgram binds the executable review control law and the exact +// admitted policy into one kernel Program identity. A policy change (prompt, +// schema, bounds, weights) or a law change (any transition edit) produces a +// different fingerprint, which stales every prior prescription and makes +// prior sealed receipts honestly unverifiable against the new program. +func compileReviewProgram(policy Policy) (kernel.Program, error) { + if err := policy.validate(); err != nil { + return kernel.Program{}, err + } + contract, err := policy.contractFingerprint() + if err != nil { + return kernel.Program{}, err + } + submitSources := []string{modeUnreviewed, modeFindingsOpen} + transitions := []kernel.Transition{ + { + ID: transitionConverge, + SourceModes: submitSources, + TargetMode: modeConverged, + ObjectiveScope: kernel.ObjectiveNone, + ObjectiveMutation: kernel.PreserveObjective, + RequiredCapabilities: []kernel.Capability{capabilitySubmit}, + OwnedFacets: []string{facetRound}, + Operation: transitionConverge, + Priority: 10, + }, + { + ID: transitionRecord, + SourceModes: submitSources, + TargetMode: modeFindingsOpen, + ObjectiveScope: kernel.ObjectiveNone, + ObjectiveMutation: kernel.PreserveObjective, + RequiredCapabilities: []kernel.Capability{capabilitySubmit}, + OwnedFacets: []string{facetRound}, + Operation: transitionRecord, + Priority: 20, + }, + { + ID: transitionEscalate, + SourceModes: submitSources, + TargetMode: modeEscalated, + ObjectiveScope: kernel.ObjectiveNone, + ObjectiveMutation: kernel.PreserveObjective, + RequiredCapabilities: []kernel.Capability{capabilitySubmit}, + OwnedFacets: []string{facetRound}, + Operation: transitionEscalate, + Priority: 30, + }, + { + ID: transitionReopen, + SourceModes: []string{modeConverged, modeEscalated}, + TargetMode: modeUnreviewed, + ObjectiveScope: kernel.ObjectiveNone, + ObjectiveMutation: kernel.PreserveObjective, + RequiredCapabilities: []kernel.Capability{capabilityHuman}, + OwnedFacets: []string{facetRound}, + Operation: transitionReopen, + Priority: 40, + }, + { + ID: transitionRecover, + SourceModes: []string{modeUnreviewed, modeFindingsOpen, modeConverged, modeEscalated}, + TargetMode: modeUnreviewed, + ObjectiveScope: kernel.ObjectiveNone, + ObjectiveMutation: kernel.PreserveObjective, + RequiredCapabilities: []kernel.Capability{capabilityRecover}, + OwnedFacets: []string{facetRound}, + Operation: transitionRecover, + Priority: 5, + Recovers: []string{ + transitionConverge, + transitionRecord, + transitionEscalate, + transitionReopen, + }, + }, + } + return kernel.CompileDomainProgram( + programID, programVersion, programRuntime, contract, + modeUnreviewed, []string{modeConverged}, transitions, + ) +} + +// reviewCapabilities is the trusted capability classifier: the minimum +// capability for each concrete review operation is fixed mechanism +// configuration, never program or proposer data. +type reviewCapabilities struct{} + +func (reviewCapabilities) RequiredCapabilities(transition kernel.Transition) ([]kernel.Capability, error) { + switch transition.Operation { + case transitionConverge, transitionRecord, transitionEscalate: + return []kernel.Capability{capabilitySubmit}, nil + case transitionReopen: + return []kernel.Capability{capabilityHuman}, nil + case transitionRecover: + return []kernel.Capability{capabilityRecover}, nil + default: + return nil, fmt.Errorf("unclassified review operation %q", transition.Operation) + } +} diff --git a/boatstack/cmd/boatstack-reviewer/reviewer_test.go b/boatstack/cmd/boatstack-reviewer/reviewer_test.go new file mode 100644 index 00000000..76349488 --- /dev/null +++ b/boatstack/cmd/boatstack-reviewer/reviewer_test.go @@ -0,0 +1,791 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/operatorstack/boatstack/boatstack/kernel" +) + +// repositoryRoot is the real Boatstack repository, whose committed policy +// assets seed every scratch repository so the tests exercise the exact +// admitted contract. +func repositoryRoot(t *testing.T) string { + t.Helper() + root, err := filepath.Abs(filepath.Join("..", "..", "..")) + if err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(root, filepath.FromSlash(policyPromptPath))); err != nil { + t.Fatalf("admitted policy prompt is unavailable at %s: %v", root, err) + } + return root +} + +type scratchRepo struct { + t *testing.T + repo *gitRepo +} + +func (s *scratchRepo) git(args ...string) string { + s.t.Helper() + output, err := gitOutput(s.repo.Root, args...) + if err != nil { + s.t.Fatal(err) + } + return strings.TrimSpace(output) +} + +func (s *scratchRepo) writeFile(relative, contents string) { + s.t.Helper() + path := filepath.Join(s.repo.Root, filepath.FromSlash(relative)) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + s.t.Fatal(err) + } + if err := os.WriteFile(path, []byte(contents), 0o644); err != nil { + s.t.Fatal(err) + } +} + +func (s *scratchRepo) commitAll(message string) string { + s.t.Helper() + s.git("add", "-A") + s.git("-c", "user.email=review@test.invalid", "-c", "user.name=Review Test", "commit", "-q", "-m", message) + return s.git("rev-parse", "HEAD") +} + +// newScratchRepo builds a repository with the admitted policy on main and a +// feature branch containing one reviewable change to subject.go. +func newScratchRepo(t *testing.T) *scratchRepo { + t.Helper() + root := repositoryRoot(t) + directory := t.TempDir() + if _, err := gitOutput(directory, "init", "-q", "-b", "main"); err != nil { + t.Fatal(err) + } + repo, err := openRepo(directory) + if err != nil { + t.Fatal(err) + } + scratch := &scratchRepo{t: t, repo: repo} + for _, asset := range []string{policyPromptPath, policySchemaPath} { + contents, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(asset))) + if err != nil { + t.Fatal(err) + } + scratch.writeFile(asset, string(contents)) + } + scratch.writeFile("subject.go", "package subject\n") + scratch.commitAll("base") + scratch.git("checkout", "-qb", "feature") + scratch.writeFile("subject.go", "package subject\n\nfunc Value() int { return 1 }\n") + scratch.commitAll("change") + return scratch +} + +func newTestLoop(t *testing.T, scratch *scratchRepo, policy Policy) *loopContext { + t.Helper() + program, err := compileReviewProgram(policy) + if err != nil { + t.Fatal(err) + } + store := newFileStore(scratch.repo.GitDir, "feature", program.Identity()) + domain := &reviewDomain{repo: scratch.repo, store: store, policy: policy, baseRef: "main"} + return &loopContext{ + repo: scratch.repo, + policy: policy, + program: program, + store: store, + domain: domain, + operator: reviewOperator{store: store}, + instance: "feature", + baseRef: "main", + } +} + +func testPolicy(t *testing.T, scratch *scratchRepo) Policy { + t.Helper() + policy, err := loadWorktreePolicy(scratch.repo.Root) + if err != nil { + t.Fatal(err) + } + return policy +} + +func correctReview() string { + return `{ + "findings": [], + "overall_correctness": "patch is correct", + "overall_explanation": "No remaining actionable findings.", + "overall_confidence_score": 0.95 +}` +} + +func incorrectReview(findings ...string) string { + return fmt.Sprintf(`{ + "findings": [%s], + "overall_correctness": "patch is incorrect", + "overall_explanation": "Actionable findings remain.", + "overall_confidence_score": 0.9 +}`, strings.Join(findings, ",")) +} + +func finding(title string, priority int, path string, line int) string { + return fmt.Sprintf(`{ + "title": %q, + "body": "Invariant, failure mode, and regression test are described here in enough detail.", + "confidence_score": 0.9, + "priority": %d, + "code_location": { + "absolute_file_path": %q, + "side": "RIGHT", + "line_range": {"start": %d, "end": %d} + } + }`, title, priority, path, line, line) +} + +// submit stages candidate bytes against the current head tree and drives one +// resolve+apply through the kernel runtime, returning the resolution and the +// committed receipt when the kernel prescribed a transition. +func submit(t *testing.T, loop *loopContext, candidate string) (kernel.Resolution, *kernel.Receipt, error) { + t.Helper() + head, err := loop.repo.headCommit() + if err != nil { + t.Fatal(err) + } + tree, err := loop.repo.reviewedTree(head) + if err != nil { + t.Fatal(err) + } + if err := loop.store.stageCandidate([]byte(candidate), tree); err != nil { + t.Fatal(err) + } + runtime, err := loop.runtime() + if err != nil { + t.Fatal(err) + } + authority, err := loop.authority("test-actor", capabilitySubmit) + if err != nil { + t.Fatal(err) + } + request := kernel.ResolveRequest{InstanceID: loop.instance, Authority: authority} + resolution, err := runtime.Resolve(context.Background(), request) + if err != nil { + t.Fatal(err) + } + if resolution.Decision.Kind != kernel.Prescribed || resolution.Prescription == nil { + return resolution, nil, fmt.Errorf("refused: %s", resolution.Decision.Reason) + } + receipt, err := runtime.Apply(context.Background(), kernel.ApplyRequest{ + ResolveRequest: request, + Prescription: *resolution.Prescription, + }) + if err != nil { + return resolution, nil, err + } + return resolution, &receipt, nil +} + +func mode(t *testing.T, loop *loopContext) string { + t.Helper() + state, err := loop.store.Load(context.Background(), loop.instance) + if err != nil { + t.Fatal(err) + } + return state.Mode +} + +func TestReviewProgramControlLaw(t *testing.T) { + scratch := newScratchRepo(t) + policy := testPolicy(t, scratch) + program, err := compileReviewProgram(policy) + if err != nil { + t.Fatal(err) + } + if program.InitialMode != modeUnreviewed { + t.Fatalf("initial mode is %q", program.InitialMode) + } + if len(program.MarkedModes) != 1 || program.MarkedModes[0] != modeConverged { + t.Fatalf("marked modes are %v; only convergence is a resting point", program.MarkedModes) + } + // Recompiling under a changed policy must change the program identity, + // so prescriptions and sealed receipts can never survive a policy edit. + changed := policy + changed.MaxRounds++ + reprogram, err := compileReviewProgram(changed) + if err != nil { + t.Fatal(err) + } + if reprogram.Fingerprint == program.Fingerprint { + t.Fatal("changing the convergence bound did not change the program identity") + } +} + +func TestCalibrationCoversMinedReviewHistory(t *testing.T) { + contents, err := os.ReadFile(filepath.Join("testdata", "review_rounds.json")) + if err != nil { + t.Fatal(err) + } + var fixture struct { + PullRequests []struct { + PR int `json:"pr"` + Rounds []struct { + Verdict string `json:"verdict"` + } `json:"rounds"` + } `json:"pull_requests"` + Observed struct { + MaxRounds int `json:"max_rounds"` + } `json:"observed"` + } + if err := json.Unmarshal(contents, &fixture); err != nil { + t.Fatal(err) + } + if len(fixture.PullRequests) == 0 { + t.Fatal("fixture carries no mined pull requests") + } + longest := 0 + for _, pullRequest := range fixture.PullRequests { + if len(pullRequest.Rounds) > longest { + longest = len(pullRequest.Rounds) + } + } + if longest != fixture.Observed.MaxRounds { + t.Fatalf("fixture summary says max %d rounds but the data holds %d", fixture.Observed.MaxRounds, longest) + } + if defaultMaxRounds <= fixture.Observed.MaxRounds { + t.Fatalf("round bound %d does not cover the observed maximum of %d rounds", defaultMaxRounds, fixture.Observed.MaxRounds) + } +} + +func TestCandidateAdmissionIsSoundAndComplete(t *testing.T) { + scratch := newScratchRepo(t) + policy := testPolicy(t, scratch) + head := scratch.git("rev-parse", "HEAD") + mergeBase := scratch.git("merge-base", "main", "HEAD") + diff, err := scratch.repo.pullRequestDiff(mergeBase, head) + if err != nil { + t.Fatal(err) + } + tree, err := scratch.repo.reviewedTree(head) + if err != nil { + t.Fatal(err) + } + evaluate := func(candidate string) candidateSummary { + return evaluateCandidate(policy, []byte(candidate), tree, scratch.repo.Root, diff) + } + + // Completeness: a well-formed candidate anchored inside the diff is + // admitted, and its measure follows the priority weights. + admitted := evaluate(incorrectReview(finding("P1 on changed line", 1, "subject.go", 3))) + if !admitted.Valid { + t.Fatalf("well-formed candidate rejected: %v", admitted.InvalidReasons) + } + if admitted.Measure != policy.Weights[1] { + t.Fatalf("measure %d does not follow the priority weights", admitted.Measure) + } + + // Soundness: every out-of-contract candidate is refused with a reason. + rejections := map[string]string{ + "not JSON at all": "{", + "schema verdict enum": strings.Replace(correctReview(), "patch is correct", "looks fine", 1), + "line outside diff": incorrectReview(finding("outside", 1, "subject.go", 1)), + "unknown file": incorrectReview(finding("ghost", 1, "ghost.go", 3)), + "path escapes the repo": incorrectReview(finding("escape", 1, "../outside.go", 3)), + "absolute foreign path": incorrectReview(finding("foreign", 1, "/etc/hosts", 3)), + } + for name, candidate := range rejections { + summary := evaluate(candidate) + if summary.Valid { + t.Errorf("%s: out-of-contract candidate was admitted", name) + } + if len(summary.InvalidReasons) == 0 { + t.Errorf("%s: refusal carries no reason", name) + } + } + + // The wrong diff side is refused even when the line number exists. + leftSide := strings.Replace( + incorrectReview(finding("wrong side", 1, "subject.go", 3)), `"side": "RIGHT"`, `"side": "LEFT"`, 1) + if evaluate(leftSide).Valid { + t.Error("a finding anchored to the unchanged side was admitted") + } +} + +func TestStallLaw(t *testing.T) { + policy := Policy{StallWindow: 3} + cases := []struct { + name string + recorded []int + next int + stalled bool + }{ + {"first round never stalls", nil, 100, false}, + {"improvement resets", []int{100, 90}, 80, false}, + {"two flat submissions are tolerated", []int{100}, 100, false}, + {"three flat submissions stall", []int{100, 100}, 100, true}, + {"three non-improving submissions stall", []int{100, 110}, 120, true}, + {"an old stall does not count after progress", []int{100, 100, 100, 50}, 40, false}, + } + for _, testCase := range cases { + if got := stalled(policy, testCase.recorded, testCase.next); got != testCase.stalled { + t.Errorf("%s: stalled=%v, want %v", testCase.name, got, testCase.stalled) + } + } +} + +func TestLoopConvergesSealsAndVerifies(t *testing.T) { + scratch := newScratchRepo(t) + policy := testPolicy(t, scratch) + loop := newTestLoop(t, scratch, policy) + + // Round 1: open findings are recorded and the loop stays open. + if _, receipt, err := submit(t, loop, incorrectReview(finding("open", 1, "subject.go", 3))); err != nil { + t.Fatal(err) + } else if receipt.TransitionID != transitionRecord { + t.Fatalf("round 1 committed %q", receipt.TransitionID) + } + if mode(t, loop) != modeFindingsOpen { + t.Fatalf("mode is %q after recording findings", mode(t, loop)) + } + + // The fix lands as a commit; a stale re-submission of the old tree's + // verdict must not converge the new tree (freshness). + scratch.writeFile("subject.go", "package subject\n\nfunc Value() int { return 2 }\n") + scratch.commitAll("fix") + + // Round 2: the corrected tree converges. + if _, receipt, err := submit(t, loop, correctReview()); err != nil { + t.Fatal(err) + } else if receipt.TransitionID != transitionConverge { + t.Fatalf("round 2 committed %q", receipt.TransitionID) + } + if mode(t, loop) != modeConverged { + t.Fatalf("mode is %q after convergence", mode(t, loop)) + } + + // Seal, commit the receipt, and verify: committing the sealed receipt + // must not invalidate its own tree binding. + sealed, err := buildSealedReceipt(scratch.repo, loop.store, policy, loop.program, "main", time.Now()) + if err != nil { + t.Fatal(err) + } + receiptPath := filepath.Join(scratch.repo.receiptDirectoryPath(), "feature.receipt.json") + if err := writeSealedReceipt(receiptPath, sealed); err != nil { + t.Fatal(err) + } + scratch.commitAll("sealed review receipt") + report := verifySealedReceipt(scratch.repo, sealed, receiptPath, "main", "HEAD") + if !report.Verified { + t.Fatalf("converged receipt did not verify: %v", report.Failures) + } + + // Directory scan finds the same receipt for the head tree. + found, foundPath, err := findReceiptForHead(scratch.repo, scratch.repo.receiptDirectoryPath(), "HEAD") + if err != nil { + t.Fatal(err) + } + if found.Fingerprint != sealed.Fingerprint || foundPath != receiptPath { + t.Fatal("directory scan did not find the sealed receipt for the head tree") + } +} + +func TestStaleCandidateAndDirtyWorktreeAreRefused(t *testing.T) { + scratch := newScratchRepo(t) + policy := testPolicy(t, scratch) + loop := newTestLoop(t, scratch, policy) + + // Stale: the candidate was produced for the previous tree. + head, err := scratch.repo.headCommit() + if err != nil { + t.Fatal(err) + } + oldTree, err := scratch.repo.reviewedTree(head) + if err != nil { + t.Fatal(err) + } + if err := loop.store.stageCandidate([]byte(correctReview()), oldTree); err != nil { + t.Fatal(err) + } + scratch.writeFile("subject.go", "package subject\n\nfunc Value() int { return 3 }\n") + scratch.commitAll("moved on") + runtime, err := loop.runtime() + if err != nil { + t.Fatal(err) + } + authority, err := loop.authority("test-actor", capabilitySubmit) + if err != nil { + t.Fatal(err) + } + resolution, err := runtime.Resolve(context.Background(), kernel.ResolveRequest{ + InstanceID: loop.instance, Authority: authority, + }) + if err != nil { + t.Fatal(err) + } + if resolution.Decision.Kind == kernel.Prescribed { + t.Fatal("a candidate produced for a different tree was prescribed") + } + + // Dirty: tracked uncommitted changes refuse every submission. + scratch.writeFile("subject.go", "package subject\n\nfunc Value() int { return 4 }\n") + if _, _, err := submit(t, loop, correctReview()); err == nil { + t.Fatal("a dirty worktree accepted a submission") + } + if mode(t, loop) != modeUnreviewed { + t.Fatalf("refusals must not move the mode; mode is %q", mode(t, loop)) + } +} + +func TestStallEscalatesAndHumanReopens(t *testing.T) { + scratch := newScratchRepo(t) + policy := testPolicy(t, scratch) + loop := newTestLoop(t, scratch, policy) + + // Three submissions with non-decreasing measures: the third escalates + // instead of recording another round. + titles := []string{"first", "second", "third"} + var last *kernel.Receipt + for _, title := range titles { + _, receipt, err := submit(t, loop, incorrectReview(finding(title, 1, "subject.go", 3))) + if err != nil { + t.Fatal(err) + } + last = receipt + } + if last.TransitionID != transitionEscalate { + t.Fatalf("third non-improving submission committed %q, not escalation", last.TransitionID) + } + if mode(t, loop) != modeEscalated { + t.Fatalf("mode is %q after escalation", mode(t, loop)) + } + + // A submit-capable actor cannot reopen; reopening requires the human + // capability, and a fresh generation starts empty. + runtime, err := loop.runtime() + if err != nil { + t.Fatal(err) + } + submitOnly, err := loop.authority("test-actor", capabilitySubmit) + if err != nil { + t.Fatal(err) + } + refused, err := runtime.Resolve(context.Background(), kernel.ResolveRequest{ + InstanceID: loop.instance, Authority: submitOnly, Requested: transitionReopen, + }) + if err != nil { + t.Fatal(err) + } + if refused.Decision.Kind == kernel.Prescribed { + t.Fatal("reopen was prescribed without the human capability") + } + human, err := loop.authority("human-reviewer", capabilityHuman) + if err != nil { + t.Fatal(err) + } + request := kernel.ResolveRequest{InstanceID: loop.instance, Authority: human, Requested: transitionReopen} + resolution, err := runtime.Resolve(context.Background(), request) + if err != nil { + t.Fatal(err) + } + if resolution.Decision.Kind != kernel.Prescribed || resolution.Prescription == nil { + t.Fatalf("human reopen refused: %s", resolution.Decision.Reason) + } + if _, err := runtime.Apply(context.Background(), kernel.ApplyRequest{ + ResolveRequest: request, Prescription: *resolution.Prescription, + }); err != nil { + t.Fatal(err) + } + if mode(t, loop) != modeUnreviewed { + t.Fatalf("mode is %q after reopen", mode(t, loop)) + } + observed, err := loop.domain.observeValue() + if err != nil { + t.Fatal(err) + } + if observed.Generation != 2 || len(observed.Rounds) != 0 { + t.Fatalf("reopen did not start a fresh empty generation: generation %d, %d rounds", observed.Generation, len(observed.Rounds)) + } +} + +// converge drives a scratch repository to a sealed, committed, verified +// receipt and returns it with its path. +func converge(t *testing.T, scratch *scratchRepo, loop *loopContext) (SealedReceipt, string) { + t.Helper() + if _, _, err := submit(t, loop, correctReview()); err != nil { + t.Fatal(err) + } + sealed, err := buildSealedReceipt(scratch.repo, loop.store, loop.policy, loop.program, "main", time.Now()) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(scratch.repo.receiptDirectoryPath(), "feature.receipt.json") + if err := writeSealedReceipt(path, sealed); err != nil { + t.Fatal(err) + } + scratch.commitAll("sealed review receipt") + if report := verifySealedReceipt(scratch.repo, sealed, path, "main", "HEAD"); !report.Verified { + t.Fatalf("baseline receipt did not verify: %v", report.Failures) + } + return sealed, path +} + +func TestVerificationRejectsTamperingForgeryAndBypass(t *testing.T) { + scratch := newScratchRepo(t) + policy := testPolicy(t, scratch) + loop := newTestLoop(t, scratch, policy) + sealed, path := converge(t, scratch, loop) + + // Tampered content: any edit breaks the content identity. + tampered := sealed + tampered.Rounds = append([]journalRound(nil), sealed.Rounds...) + tampered.Rounds[0].Verdict = verdictIncorrect + if report := verifySealedReceipt(scratch.repo, tampered, path, "main", "HEAD"); report.Verified { + t.Fatal("a tampered receipt verified") + } + + // Forged verdict: rewriting the final review and re-fingerprinting the + // envelope still fails, because the kernel receipt chain committed the + // original candidate fingerprint. + forged := sealed + forged.FinalReview = json.RawMessage(strings.Replace( + string(sealed.FinalReview), "patch is correct", "patch is incorrect", 1)) + refingered, err := forged.contentFingerprint() + if err != nil { + t.Fatal(err) + } + forged.Fingerprint = refingered + report := verifySealedReceipt(scratch.repo, forged, path, "main", "HEAD") + if report.Verified { + t.Fatal("a forged final review verified") + } + + // Bypass: new commits after convergence leave the receipt bound to the + // old tree, so the new head is not review-verified. + scratch.writeFile("subject.go", "package subject\n\nfunc Value() int { return 99 }\n") + scratch.commitAll("unreviewed change") + if report := verifySealedReceipt(scratch.repo, sealed, path, "main", "HEAD"); report.Verified { + t.Fatal("a receipt for an older tree verified a new head") + } + if _, _, err := findReceiptForHead(scratch.repo, scratch.repo.receiptDirectoryPath(), "HEAD"); err == nil { + t.Fatal("directory scan bound an unreviewed head to an old receipt") + } + + // Policy drift: a base whose admitted prompt differs refuses admission. + scratch.git("checkout", "-q", "main") + scratch.writeFile(policyPromptPath, "entirely different review policy\n") + scratch.commitAll("policy change on main") + scratch.git("checkout", "-q", "feature") + if report := verifySealedReceipt(scratch.repo, sealed, path, "main", "HEAD"); report.Verified { + t.Fatal("a receipt sealed under a superseded policy verified") + } +} + +func TestVerificationRejectsReceiptDeclaredBoundsDrift(t *testing.T) { + // Regression for round 1, finding 1: a receipt sealed under weakened + // convergence bounds must not verify against the admitted policy. + scratch := newScratchRepo(t) + weakened := testPolicy(t, scratch) + weakened.MaxRounds = 1000 + weakened.StallWindow = 500 + loop := newTestLoop(t, scratch, weakened) + if _, _, err := submit(t, loop, correctReview()); err != nil { + t.Fatal(err) + } + sealed, err := buildSealedReceipt(scratch.repo, loop.store, weakened, loop.program, "main", time.Now()) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(scratch.repo.receiptDirectoryPath(), "feature.receipt.json") + if err := writeSealedReceipt(path, sealed); err != nil { + t.Fatal(err) + } + scratch.commitAll("sealed under weakened bounds") + report := verifySealedReceipt(scratch.repo, sealed, path, "main", "HEAD") + if report.Verified { + t.Fatal("a receipt sealed under weakened convergence bounds verified") + } + boundsNamed := false + for _, failure := range report.Failures { + if strings.Contains(failure, "convergence bounds") { + boundsNamed = true + } + } + if !boundsNamed { + t.Fatalf("the failure does not name the bounds drift: %v", report.Failures) + } +} + +func TestResolveSurfacesTheDomainRefusalReason(t *testing.T) { + // Regression for round 1, finding 2: an untargeted resolve must name the + // exact domain condition, not only the kernel's generic reason. + scratch := newScratchRepo(t) + reader, writer, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + original := os.Stdout + os.Stdout = writer + runErr := run([]string{"resolve", "--repo", scratch.repo.Root, "--delivery", "feature", "--base", "main"}) + os.Stdout = original + writer.Close() + output, err := io.ReadAll(reader) + if err != nil { + t.Fatal(err) + } + if runErr != nil { + t.Fatal(runErr) + } + var resolved struct { + Disposition dispositionView `json:"submission_disposition"` + } + if err := json.Unmarshal(output, &resolved); err != nil { + t.Fatalf("resolve output does not decode: %v", err) + } + if resolved.Disposition.Admits != "" { + t.Fatalf("nothing is staged but disposition admits %q", resolved.Disposition.Admits) + } + if !strings.Contains(resolved.Disposition.Reason, "no candidate review is staged") { + t.Fatalf("disposition reason does not name the staging requirement: %q", resolved.Disposition.Reason) + } +} + +func TestRecoveryClearsInterruptedEffect(t *testing.T) { + scratch := newScratchRepo(t) + policy := testPolicy(t, scratch) + loop := newTestLoop(t, scratch, policy) + + // Simulate a crash between BeginEffect and CommitTransition: the store + // holds an attempt revision with an active recovery state. + state, err := loop.store.Load(context.Background(), loop.instance) + if err != nil { + t.Fatal(err) + } + attempt := state + attempt.Revision = state.Revision + 1 + attempt.Recovery = &kernel.RecoveryState{ + PrescriptionID: "interrupted-prescription", + TransitionID: transitionRecord, + Reason: "simulated crash between effect and commit", + } + if err := loop.store.BeginEffect(context.Background(), state.Revision, attempt); err != nil { + t.Fatal(err) + } + if err := loop.store.stageCandidate([]byte(correctReview()), "half-recorded"); err != nil { + t.Fatal(err) + } + + // Ordinary submission is refused while recovery is pending; the + // declared recovery transition clears the staging and restores control. + runtime, err := loop.runtime() + if err != nil { + t.Fatal(err) + } + recoverAuthority, err := loop.authority("test-actor", capabilityRecover) + if err != nil { + t.Fatal(err) + } + request := kernel.ResolveRequest{ + InstanceID: loop.instance, Authority: recoverAuthority, Requested: transitionRecover, + } + resolution, err := runtime.Resolve(context.Background(), request) + if err != nil { + t.Fatal(err) + } + if resolution.Decision.Kind != kernel.Prescribed || resolution.Prescription == nil { + t.Fatalf("recovery refused: %s", resolution.Decision.Reason) + } + receipt, err := runtime.Apply(context.Background(), kernel.ApplyRequest{ + ResolveRequest: request, Prescription: *resolution.Prescription, + }) + if err != nil { + t.Fatal(err) + } + if receipt.TransitionID != transitionRecover { + t.Fatalf("recovery committed %q", receipt.TransitionID) + } + if mode(t, loop) != modeUnreviewed { + t.Fatalf("mode is %q after recovery", mode(t, loop)) + } + if _, _, staged, err := loop.store.loadStagedCandidate(); err != nil || staged { + t.Fatalf("recovery left staging behind (staged=%v, err=%v)", staged, err) + } +} + +func TestShowDisplaysTheRecordedReviewWithoutResolving(t *testing.T) { + scratch := newScratchRepo(t) + policy := testPolicy(t, scratch) + loop := newTestLoop(t, scratch, policy) + if _, _, err := submit(t, loop, incorrectReview(finding("visible finding", 1, "subject.go", 3))); err != nil { + t.Fatal(err) + } + + capture := func(arguments ...string) (string, error) { + t.Helper() + reader, writer, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + original := os.Stdout + os.Stdout = writer + runErr := run(append([]string{"show", "--repo", scratch.repo.Root, "--delivery", "feature"}, arguments...)) + os.Stdout = original + writer.Close() + output, err := io.ReadAll(reader) + if err != nil { + t.Fatal(err) + } + return string(output), runErr + } + + output, err := capture() + if err != nil { + t.Fatal(err) + } + var shown struct { + Mode string `json:"mode"` + Round *journalRound + Review json.RawMessage `json:"review"` + } + if err := json.Unmarshal([]byte(output), &shown); err != nil { + t.Fatalf("show output does not decode: %v", err) + } + if shown.Mode != modeFindingsOpen { + t.Fatalf("show reports mode %q", shown.Mode) + } + if !strings.Contains(string(shown.Review), "visible finding") { + t.Fatal("show did not display the recorded review findings") + } + if candidateFingerprint(shown.Review) == "" { + t.Fatal("shown review is empty") + } + + if _, err := capture("--round", "99"); err == nil { + t.Fatal("show accepted a round that was never recorded") + } +} + +func TestCLIVerifyCommandExitsNonZeroWithoutReceipt(t *testing.T) { + scratch := newScratchRepo(t) + head := scratch.git("rev-parse", "HEAD") + err := run([]string{"verify", "--repo", scratch.repo.Root, "--base", "main", "--head", head}) + if err == nil { + t.Fatal("verify succeeded with no sealed receipt present") + } + if !strings.Contains(err.Error(), "no sealed review receipt") { + t.Fatalf("failure does not tell the operator what to do: %v", err) + } +} + +func TestGitBinaryIsAvailableForThisSuite(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Fatal("this test suite requires git on PATH") + } +} diff --git a/boatstack/cmd/boatstack-reviewer/seal.go b/boatstack/cmd/boatstack-reviewer/seal.go new file mode 100644 index 00000000..9784a09a --- /dev/null +++ b/boatstack/cmd/boatstack-reviewer/seal.go @@ -0,0 +1,396 @@ +package main + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/operatorstack/boatstack/boatstack/kernel" +) + +const sealedReceiptSchemaVersion = 1 + +// SealedReceipt is the one artifact that travels with a pull request. It +// binds the exact reviewed (receipt-excluded) tree, the exact admitted +// policy, the exact program identity, the round trajectory, the final +// review bytes, and the complete kernel receipt chain. CI verifies it +// deterministically; no reviewer runs there. +type SealedReceipt struct { + SchemaVersion int `json:"schema_version"` + Instance string `json:"instance"` + Program kernel.ProgramIdentity `json:"program"` + Policy Policy `json:"policy"` + BaseRef string `json:"base_ref"` + MergeBase string `json:"merge_base"` + HeadCommit string `json:"head_commit"` + ReviewedTree string `json:"reviewed_tree"` + Generation int `json:"generation"` + Rounds []journalRound `json:"rounds"` + FinalReview json.RawMessage `json:"final_review"` + ControlState sealedControlState `json:"control_state"` + Receipts []kernel.Receipt `json:"kernel_receipts"` + Honesty sealedHonesty `json:"honesty"` + SealedAt time.Time `json:"sealed_at"` + Fingerprint string `json:"fingerprint"` +} + +type sealedControlState struct { + Mode string `json:"mode"` + Revision uint64 `json:"revision"` +} + +// sealedHonesty mirrors work-package verification: this artifact proves the +// declared review program ran to convergence over the exact bound tree. It +// never proves the review was semantically right, and never proves who or +// what actually performed the review. +type sealedHonesty struct { + SemanticCorrectness string `json:"semantic_correctness"` + OriginAuthenticity string `json:"origin_authenticity"` +} + +func (r SealedReceipt) contentFingerprint() (string, error) { + identity := r + identity.Fingerprint = "" + encoded, err := json.Marshal(identity) + if err != nil { + return "", err + } + return sha256Hex(encoded), nil +} + +// buildSealedReceipt assembles the sealed receipt from committed state. It +// refuses unless the instance is converged and the converged round binds the +// exact current reviewed tree. +func buildSealedReceipt(repo *gitRepo, store *fileStore, policy Policy, program kernel.Program, baseRef string, now time.Time) (SealedReceipt, error) { + document, err := store.loadDocument() + if err != nil { + return SealedReceipt{}, err + } + if document.State.Mode != modeConverged { + return SealedReceipt{}, fmt.Errorf("instance %s is %q, not converged; nothing to seal", document.State.InstanceID, document.State.Mode) + } + if document.State.Program != program.Identity() { + return SealedReceipt{}, fmt.Errorf("committed state belongs to program %s, not the active program %s; the admitted policy or law changed after convergence", document.State.Program.Fingerprint, program.Fingerprint) + } + journal, err := store.loadJournal() + if err != nil { + return SealedReceipt{}, err + } + rounds := journal.currentRounds() + if len(rounds) == 0 { + return SealedReceipt{}, fmt.Errorf("converged instance has no recorded rounds") + } + final := rounds[len(rounds)-1] + if final.Verdict != verdictCorrect || final.Transition != transitionConverge { + return SealedReceipt{}, fmt.Errorf("last recorded round is not the converged round") + } + head, err := repo.headCommit() + if err != nil { + return SealedReceipt{}, err + } + reviewedTree, err := repo.reviewedTree(head) + if err != nil { + return SealedReceipt{}, err + } + if reviewedTree != final.ReviewedTree { + return SealedReceipt{}, fmt.Errorf("current reviewed tree %s differs from the converged round's tree %s; re-run the review loop", reviewedTree, final.ReviewedTree) + } + finalReview, err := store.roundBytes(final.CandidateFingerprint) + if err != nil { + return SealedReceipt{}, fmt.Errorf("converged review bytes are unavailable: %w", err) + } + receipt := SealedReceipt{ + SchemaVersion: sealedReceiptSchemaVersion, + Instance: document.State.InstanceID, + Program: program.Identity(), + Policy: policy, + BaseRef: baseRef, + MergeBase: final.MergeBase, + HeadCommit: final.HeadCommit, + ReviewedTree: final.ReviewedTree, + Generation: journal.Generation, + Rounds: rounds, + FinalReview: json.RawMessage(finalReview), + ControlState: sealedControlState{Mode: document.State.Mode, Revision: document.State.Revision}, + Receipts: document.Receipts, + Honesty: sealedHonesty{ + SemanticCorrectness: "not-evaluated", + OriginAuthenticity: "not-proven", + }, + SealedAt: now.UTC(), + } + receipt.Fingerprint, err = receipt.contentFingerprint() + if err != nil { + return SealedReceipt{}, err + } + return receipt, nil +} + +func writeSealedReceipt(path string, receipt SealedReceipt) error { + encoded, err := json.MarshalIndent(receipt, "", " ") + if err != nil { + return err + } + return writeFileAtomic(path, append(encoded, '\n')) +} + +func readSealedReceipt(path string) (SealedReceipt, error) { + value, err := os.ReadFile(path) + if err != nil { + return SealedReceipt{}, err + } + var receipt SealedReceipt + if err := json.Unmarshal(value, &receipt); err != nil { + return SealedReceipt{}, fmt.Errorf("sealed receipt %s does not decode: %w", path, err) + } + return receipt, nil +} + +// verificationReport is the deterministic CI answer. +type verificationReport struct { + Verified bool `json:"verified"` + ReceiptPath string `json:"receipt_path,omitempty"` + Instance string `json:"instance,omitempty"` + Program string `json:"program_fingerprint,omitempty"` + Checks []string `json:"checks"` + Failures []string `json:"failures"` + Warnings []string `json:"warnings"` +} + +// verifySealedReceipt is the CI-side control law: +// +// a pull request head is review-verified only when a sealed receipt +// binds its exact receipt-excluded tree, under the review program +// whose policy assets are admitted at the pull request base revision, +// with an integral kernel receipt chain ending in convergence. +func verifySealedReceipt(repo *gitRepo, receipt SealedReceipt, receiptPath, baseRevision, headRevision string) verificationReport { + report := verificationReport{ + ReceiptPath: receiptPath, + Instance: receipt.Instance, + Program: receipt.Program.Fingerprint, + Checks: []string{}, + Failures: []string{}, + Warnings: []string{}, + } + pass := func(check string) { report.Checks = append(report.Checks, check) } + fail := func(format string, args ...any) { + report.Failures = append(report.Failures, fmt.Sprintf(format, args...)) + } + + if receipt.SchemaVersion != sealedReceiptSchemaVersion { + fail("sealed receipt schema version %d is unsupported", receipt.SchemaVersion) + return report + } + if fingerprint, err := receipt.contentFingerprint(); err != nil || fingerprint != receipt.Fingerprint { + fail("sealed receipt content fingerprint does not identify its exact content") + return report + } + pass("sealed receipt content identity is exact") + + // Policy admission: the assets named by the receipt must hash-match the + // assets committed at the pull request base revision. + basePolicy, err := loadRevisionPolicy(repo, baseRevision) + if err != nil { + fail("base revision policy admission failed: %v", err) + return report + } + if receipt.Policy.PromptSHA256 != basePolicy.PromptSHA256 || receipt.Policy.SchemaSHA256 != basePolicy.SchemaSHA256 { + fail("sealed receipt policy assets differ from the base-revision admitted policy (prompt %s vs %s, schema %s vs %s)", + receipt.Policy.PromptSHA256, basePolicy.PromptSHA256, receipt.Policy.SchemaSHA256, basePolicy.SchemaSHA256) + } else { + pass("policy assets match the base-revision admitted policy") + } + + // Program identity: recompile the review program from the admitted + // policy bytes under this binary's transition law and compiled + // convergence bounds. The receipt's declared bounds are never trusted: + // a receipt sealed under weakened bounds or weights must not verify. + admittedPolicy := basePolicy + if receipt.Policy.MaxRounds != admittedPolicy.MaxRounds || + receipt.Policy.StallWindow != admittedPolicy.StallWindow || + receipt.Policy.Weights != admittedPolicy.Weights { + fail("sealed receipt declares convergence bounds (rounds %d, stall %d, weights %v) that differ from the admitted policy (rounds %d, stall %d, weights %v)", + receipt.Policy.MaxRounds, receipt.Policy.StallWindow, receipt.Policy.Weights, + admittedPolicy.MaxRounds, admittedPolicy.StallWindow, admittedPolicy.Weights) + } else { + pass("convergence bounds and weights match the admitted policy") + } + program, err := compileReviewProgram(admittedPolicy) + if err != nil { + fail("admitted review program does not compile: %v", err) + return report + } + if program.Fingerprint != receipt.Program.Fingerprint { + fail("sealed receipt program fingerprint %s does not match the admitted review program %s", receipt.Program.Fingerprint, program.Fingerprint) + } else { + pass("program fingerprint matches the admitted policy and transition law") + } + + // Tree binding: the pull request head, receipts excluded, must be the + // exact tree the converged review bound. + head, err := repo.revParse(headRevision) + if err != nil { + fail("head revision %s is unavailable: %v", headRevision, err) + return report + } + reviewedTree, err := repo.reviewedTree(head) + if err != nil { + fail("reviewed tree of %s is unavailable: %v", head, err) + return report + } + if reviewedTree != receipt.ReviewedTree { + fail("sealed receipt binds tree %s but the head reviewed tree is %s; the reviewed content changed after convergence", receipt.ReviewedTree, reviewedTree) + } else { + pass("sealed receipt binds the exact head reviewed tree") + } + + // Kernel receipt chain integrity. + if len(receipt.Receipts) == 0 { + fail("sealed receipt carries no kernel receipts") + return report + } + var previousResult uint64 + for index, committed := range receipt.Receipts { + if err := committed.Validate(); err != nil { + fail("kernel receipt %d is invalid: %v", index, err) + return report + } + if committed.InstanceID != receipt.Instance { + fail("kernel receipt %d belongs to instance %q, not %q", index, committed.InstanceID, receipt.Instance) + } + if committed.Program.Fingerprint != receipt.Program.Fingerprint { + fail("kernel receipt %d was committed under program %s, not %s", index, committed.Program.Fingerprint, receipt.Program.Fingerprint) + } + if index == 0 { + previousResult = committed.ResultStateRevision + continue + } + if committed.PriorStateRevision != previousResult { + fail("kernel receipt %d does not chain from the previous committed revision", index) + } + previousResult = committed.ResultStateRevision + } + last := receipt.Receipts[len(receipt.Receipts)-1] + if last.TransitionID != transitionConverge { + fail("the final kernel receipt committed %q, not the converge transition", last.TransitionID) + } + if previousResult != receipt.ControlState.Revision || receipt.ControlState.Mode != modeConverged { + fail("sealed control state does not match the committed receipt chain") + } + if len(report.Failures) == 0 { + pass("kernel receipt chain is integral and ends in convergence") + } + + // Round trajectory: each recorded round must be committed by exactly one + // round-bearing kernel receipt with the same candidate fingerprint, in + // order, and the final round must be the convergence. + var roundReceipts []kernel.Receipt + for _, committed := range receipt.Receipts { + switch committed.TransitionID { + case transitionConverge, transitionRecord, transitionEscalate: + roundReceipts = append(roundReceipts, committed) + } + } + if len(roundReceipts) < len(receipt.Rounds) { + fail("%d rounds are recorded but only %d round-bearing kernel receipts exist", len(receipt.Rounds), len(roundReceipts)) + } else { + // The receipts for the sealed generation are the trailing ones; + // earlier generations may also have committed rounds. + tail := roundReceipts[len(roundReceipts)-len(receipt.Rounds):] + matched := true + for index, round := range receipt.Rounds { + committed := tail[index] + if committed.TransitionID != round.Transition || + len(committed.Effects) != 1 || + committed.Effects[0].Fingerprint != round.CandidateFingerprint { + fail("round %d (%s) is not committed by its kernel receipt", index+1, round.CandidateFingerprint) + matched = false + } + } + if matched { + pass("every recorded round is committed by a matching kernel receipt") + } + } + if len(receipt.Rounds) == 0 { + fail("sealed receipt records no rounds") + return report + } + finalRound := receipt.Rounds[len(receipt.Rounds)-1] + if finalRound.Transition != transitionConverge || finalRound.Verdict != verdictCorrect { + fail("the final recorded round is not a convergence") + } + if finalRound.ReviewedTree != receipt.ReviewedTree { + fail("the converged round bound tree %s, not the sealed tree %s", finalRound.ReviewedTree, receipt.ReviewedTree) + } + + // Final review bytes: exact hash correspondence and verdict recomputed + // from the bytes themselves, validated against the admitted schema. + if candidateFingerprint(receipt.FinalReview) != finalRound.CandidateFingerprint { + fail("final review bytes do not hash to the converged round's candidate fingerprint") + } else { + mergeBase, mergeErr := repo.mergeBase(baseRevision, head) + diffBase := receipt.MergeBase + if mergeErr == nil && mergeBase != receipt.MergeBase { + report.Warnings = append(report.Warnings, fmt.Sprintf( + "the review used merge base %s but the pull request merge base is %s; the bound tree is identical, the diff basis moved", receipt.MergeBase, mergeBase)) + } + diff, diffErr := repo.pullRequestDiff(diffBase, head) + if diffErr != nil { + report.Warnings = append(report.Warnings, "the recorded diff basis is not reachable here; anchor revalidation skipped") + diff = "" + } + summary := evaluateCandidate(admittedPolicy, receipt.FinalReview, receipt.ReviewedTree, repo.Root, diff) + if !summary.Valid && diffErr == nil { + fail("final review does not revalidate against the admitted schema and diff: %s", strings.Join(summary.InvalidReasons, "; ")) + } else if summary.Verdict != verdictCorrect { + fail("final review verdict is %q, not %q", summary.Verdict, verdictCorrect) + } else { + pass("final review bytes revalidate and accept the patch") + } + } + + report.Verified = len(report.Failures) == 0 + return report +} + +// findReceiptForHead scans a receipt directory for the sealed receipt that +// binds the exact head reviewed tree. +func findReceiptForHead(repo *gitRepo, directory, headRevision string) (SealedReceipt, string, error) { + head, err := repo.revParse(headRevision) + if err != nil { + return SealedReceipt{}, "", err + } + reviewedTree, err := repo.reviewedTree(head) + if err != nil { + return SealedReceipt{}, "", err + } + entries, err := os.ReadDir(directory) + if err != nil { + if os.IsNotExist(err) { + return SealedReceipt{}, "", fmt.Errorf("no sealed review receipt binds reviewed tree %s: %s does not exist; run the local review loop and commit the sealed receipt", reviewedTree, directory) + } + return SealedReceipt{}, "", err + } + var names []string + for _, entry := range entries { + if !entry.IsDir() && strings.HasSuffix(entry.Name(), ".receipt.json") { + names = append(names, entry.Name()) + } + } + sort.Strings(names) + for _, name := range names { + path := filepath.Join(directory, name) + receipt, err := readSealedReceipt(path) + if err != nil { + continue + } + if receipt.ReviewedTree == reviewedTree { + return receipt, path, nil + } + } + return SealedReceipt{}, "", fmt.Errorf("no sealed review receipt in %s binds reviewed tree %s; run the local review loop to convergence, seal, and commit the receipt", directory, reviewedTree) +} diff --git a/boatstack/cmd/boatstack-reviewer/store.go b/boatstack/cmd/boatstack-reviewer/store.go new file mode 100644 index 00000000..fde5cd93 --- /dev/null +++ b/boatstack/cmd/boatstack-reviewer/store.go @@ -0,0 +1,355 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + "time" + + "github.com/operatorstack/boatstack/boatstack/kernel" +) + +const storeSchemaVersion = 1 + +// storeDocument is the single durable transaction unit: control state and +// its committed receipts live in one file replaced atomically, so a target +// state and its receipt become visible together or not at all. +type storeDocument struct { + SchemaVersion int `json:"schema_version"` + State kernel.ControlState `json:"state"` + Receipts []kernel.Receipt `json:"receipts"` +} + +// journalDocument is the domain-owned round journal. Rounds are append-only +// across generations; a reopen starts a new generation instead of editing +// history. +type journalDocument struct { + SchemaVersion int `json:"schema_version"` + Generation int `json:"generation"` + Rounds []journalRound `json:"rounds"` +} + +type journalRound struct { + Generation int `json:"generation"` + Index int `json:"index"` + CandidateFingerprint string `json:"candidate_fingerprint"` + ReviewedTree string `json:"reviewed_tree"` + HeadCommit string `json:"head_commit"` + MergeBase string `json:"merge_base"` + Verdict string `json:"verdict"` + Measure int `json:"measure"` + FindingCount int `json:"finding_count"` + Priorities [4]int `json:"priorities"` + Transition string `json:"transition"` +} + +type stagedCandidate struct { + Fingerprint string `json:"fingerprint"` + ReviewedTree string `json:"reviewed_tree"` +} + +// fileStore owns one review control instance's durable state under the +// repository's .git directory, so nothing here can enter a commit. +type fileStore struct { + dir string + initial kernel.ControlState +} + +func newFileStore(gitDir, instanceID string, program kernel.ProgramIdentity) *fileStore { + return &fileStore{ + dir: filepath.Join(gitDir, "boatstack-review", instanceID), + initial: kernel.ControlState{ + InstanceID: instanceID, + Program: program, + Mode: modeUnreviewed, + Revision: 1, + }, + } +} + +func (s *fileStore) statePath() string { return filepath.Join(s.dir, "store.json") } +func (s *fileStore) journalPath() string { return filepath.Join(s.dir, "journal.json") } +func (s *fileStore) stagingPath() string { return filepath.Join(s.dir, "candidate.json") } +func (s *fileStore) stagingMetaPath() string { + return filepath.Join(s.dir, "candidate-meta.json") +} +func (s *fileStore) roundsDir() string { return filepath.Join(s.dir, "rounds") } +func (s *fileStore) lockPath() string { return filepath.Join(s.dir, "lock") } + +func writeFileAtomic(path string, value []byte) error { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return err + } + temp, err := os.CreateTemp(filepath.Dir(path), ".tmp-*") + if err != nil { + return err + } + tempPath := temp.Name() + if _, err := temp.Write(value); err != nil { + temp.Close() + os.Remove(tempPath) + return err + } + if err := temp.Close(); err != nil { + os.Remove(tempPath) + return err + } + if err := os.Rename(tempPath, path); err != nil { + os.Remove(tempPath) + return err + } + return nil +} + +func (s *fileStore) loadDocument() (storeDocument, error) { + value, err := os.ReadFile(s.statePath()) + if os.IsNotExist(err) { + return storeDocument{SchemaVersion: storeSchemaVersion, State: s.initial}, nil + } + if err != nil { + return storeDocument{}, err + } + var document storeDocument + if err := json.Unmarshal(value, &document); err != nil { + return storeDocument{}, fmt.Errorf("review store %s does not decode: %w", s.statePath(), err) + } + if document.SchemaVersion != storeSchemaVersion { + return storeDocument{}, fmt.Errorf("review store %s has unsupported schema version %d", s.statePath(), document.SchemaVersion) + } + return document, nil +} + +func (s *fileStore) saveDocument(document storeDocument) error { + encoded, err := json.MarshalIndent(document, "", " ") + if err != nil { + return err + } + return writeFileAtomic(s.statePath(), append(encoded, '\n')) +} + +func (s *fileStore) Load(context.Context, string) (kernel.ControlState, error) { + document, err := s.loadDocument() + if err != nil { + return kernel.ControlState{}, err + } + return document.State, nil +} + +func (s *fileStore) BeginEffect(_ context.Context, revision uint64, target kernel.ControlState) error { + document, err := s.loadDocument() + if err != nil { + return err + } + if document.State.Revision != revision { + return fmt.Errorf("stale revision: store has %d, attempt expects %d", document.State.Revision, revision) + } + document.State = target + return s.saveDocument(document) +} + +func (s *fileStore) CommitTransition(_ context.Context, revision uint64, target kernel.ControlState, receipt kernel.Receipt) error { + document, err := s.loadDocument() + if err != nil { + return err + } + if document.State.Revision != revision { + return fmt.Errorf("stale revision: store has %d, commit expects %d", document.State.Revision, revision) + } + document.State = target + document.Receipts = append(document.Receipts, receipt) + return s.saveDocument(document) +} + +func (s *fileStore) loadJournal() (journalDocument, error) { + value, err := os.ReadFile(s.journalPath()) + if os.IsNotExist(err) { + return journalDocument{SchemaVersion: storeSchemaVersion, Generation: 1}, nil + } + if err != nil { + return journalDocument{}, err + } + var document journalDocument + if err := json.Unmarshal(value, &document); err != nil { + return journalDocument{}, fmt.Errorf("review journal %s does not decode: %w", s.journalPath(), err) + } + if document.SchemaVersion != storeSchemaVersion { + return journalDocument{}, fmt.Errorf("review journal %s has unsupported schema version %d", s.journalPath(), document.SchemaVersion) + } + return document, nil +} + +func (s *fileStore) saveJournal(document journalDocument) error { + encoded, err := json.MarshalIndent(document, "", " ") + if err != nil { + return err + } + return writeFileAtomic(s.journalPath(), append(encoded, '\n')) +} + +// currentRounds returns the rounds of the active generation in order. +func (d journalDocument) currentRounds() []journalRound { + var rounds []journalRound + for _, round := range d.Rounds { + if round.Generation == d.Generation { + rounds = append(rounds, round) + } + } + return rounds +} + +func (s *fileStore) stageCandidate(candidateBytes []byte, reviewedTree string) error { + meta, err := json.MarshalIndent(stagedCandidate{ + Fingerprint: candidateFingerprint(candidateBytes), + ReviewedTree: reviewedTree, + }, "", " ") + if err != nil { + return err + } + if err := writeFileAtomic(s.stagingPath(), candidateBytes); err != nil { + return err + } + return writeFileAtomic(s.stagingMetaPath(), append(meta, '\n')) +} + +// loadStagedCandidate returns the staged candidate bytes and metadata, or ok +// false when nothing is staged. A staged candidate whose metadata is missing +// or inconsistent with the exact bytes is reported as an integrity error. +func (s *fileStore) loadStagedCandidate() (candidate []byte, meta stagedCandidate, ok bool, err error) { + candidate, err = os.ReadFile(s.stagingPath()) + if os.IsNotExist(err) { + return nil, stagedCandidate{}, false, nil + } + if err != nil { + return nil, stagedCandidate{}, false, err + } + metaBytes, err := os.ReadFile(s.stagingMetaPath()) + if err != nil { + return nil, stagedCandidate{}, false, fmt.Errorf("staged candidate metadata is unavailable: %w", err) + } + if err := json.Unmarshal(metaBytes, &meta); err != nil { + return nil, stagedCandidate{}, false, fmt.Errorf("staged candidate metadata does not decode: %w", err) + } + if meta.Fingerprint != candidateFingerprint(candidate) { + return nil, stagedCandidate{}, false, fmt.Errorf("staged candidate bytes do not match their recorded fingerprint") + } + return candidate, meta, true, nil +} + +func (s *fileStore) clearStagedCandidate() error { + for _, path := range []string{s.stagingPath(), s.stagingMetaPath()} { + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return err + } + } + return nil +} + +// archiveRound stores the exact candidate bytes content-addressed and +// appends the round record to the journal. +func (s *fileStore) archiveRound(candidateBytes []byte, round journalRound) error { + journal, err := s.loadJournal() + if err != nil { + return err + } + round.Generation = journal.Generation + round.Index = len(journal.currentRounds()) + 1 + if err := writeFileAtomic(filepath.Join(s.roundsDir(), round.CandidateFingerprint+".json"), candidateBytes); err != nil { + return err + } + journal.Rounds = append(journal.Rounds, round) + if err := s.saveJournal(journal); err != nil { + return err + } + return s.clearStagedCandidate() +} + +func (s *fileStore) roundBytes(fingerprint string) ([]byte, error) { + if !regexp.MustCompile(`^[0-9a-f]{64}$`).MatchString(fingerprint) { + return nil, fmt.Errorf("round fingerprint %q is not a sha256 identity", fingerprint) + } + return os.ReadFile(filepath.Join(s.roundsDir(), fingerprint+".json")) +} + +// archive moves the whole instance directory aside, preserving receipts and +// journal for inspection while releasing the instance identity. +func (s *fileStore) archive(timestamp string) (string, error) { + archived := s.dir + "-archived-" + timestamp + if err := os.Rename(s.dir, archived); err != nil { + return "", err + } + return archived, nil +} + +func (s *fileStore) nextGeneration() error { + journal, err := s.loadJournal() + if err != nil { + return err + } + journal.Generation++ + if err := s.saveJournal(journal); err != nil { + return err + } + return s.clearStagedCandidate() +} + +// directoryLocker serializes one control instance with an exclusive lock +// directory. A crash can leave the lock behind; the error names the exact +// path so the operator can remove a stale lock deliberately. +type directoryLocker struct{ path string } + +func (l directoryLocker) Acquire(context.Context, string) (kernel.Lock, error) { + if err := os.MkdirAll(filepath.Dir(l.path), 0o755); err != nil { + return nil, err + } + if err := os.Mkdir(l.path, 0o755); err != nil { + if os.IsExist(err) { + return nil, fmt.Errorf("another boatstack-reviewer invocation holds %s; remove it only if that process is gone", l.path) + } + return nil, err + } + return directoryLock{path: l.path}, nil +} + +type directoryLock struct{ path string } + +func (l directoryLock) Unlock() error { return os.Remove(l.path) } + +type systemClock struct{} + +func (systemClock) Now() time.Time { return time.Now() } + +var instanceSanitizer = regexp.MustCompile(`[^A-Za-z0-9._-]+`) + +// instanceIDForBranch derives a semantic control-instance identity from a +// branch name; slashes and other non-semantic characters become dashes. +func instanceIDForBranch(branch string) (string, error) { + value := instanceSanitizer.ReplaceAllString(strings.TrimSpace(branch), "-") + value = strings.Trim(value, "-._") + if value == "" { + return "", fmt.Errorf("branch %q does not yield a usable instance identity; pass --delivery", branch) + } + return value, nil +} + +// localAuthority materializes the invoking actor's capability receipts. This +// is local, self-declared authority: the sealed receipt records the actor, +// and origin authenticity remains not-proven, exactly like work-package +// verification reports. +func localAuthority(actor string, now time.Time, capabilities ...kernel.Capability) (kernel.Authority, error) { + sanitized := instanceSanitizer.ReplaceAllString(strings.TrimSpace(actor), "-") + sanitized = strings.Trim(sanitized, "-._") + if sanitized == "" { + return kernel.Authority{}, fmt.Errorf("an explicit --actor identity is required") + } + return kernel.Authority{Receipts: []kernel.AuthorityReceipt{{ + ID: "local-" + sanitized, + Subject: actor, + Fingerprint: "local-actor:" + sanitized, + Capabilities: capabilities, + IssuedAt: now.Add(-time.Second), + }}}, nil +} diff --git a/boatstack/cmd/boatstack-reviewer/testdata/review_rounds.json b/boatstack/cmd/boatstack-reviewer/testdata/review_rounds.json new file mode 100644 index 00000000..ec433c06 --- /dev/null +++ b/boatstack/cmd/boatstack-reviewer/testdata/review_rounds.json @@ -0,0 +1,748 @@ +{ + "description": "Codex automated-review rounds mined from the last 30 merged pull requests (2026-08-12 to 2026-08-21). Each round is one structured review the codex-review workflow published on the pull request. This fixture calibrates the boatstack-reviewer convergence policy: the round bound must cover the maximum observed round count.", + "mined_at": "2026-08-21", + "source": "gh api repos/operatorstack/boatstack/pulls//reviews (github-actions[bot] structured reviews)", + "pull_requests": [ + { + "pr": 205, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.96, + "submitted_at": "2026-08-12T14:10:05Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-12T14:20:59Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-12T14:32:30Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.91, + "submitted_at": "2026-08-12T14:41:32Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.91, + "submitted_at": "2026-08-12T14:53:09Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.95, + "submitted_at": "2026-08-12T15:21:34Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.96, + "submitted_at": "2026-08-12T16:00:53Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.95, + "submitted_at": "2026-08-12T16:24:11Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.96, + "submitted_at": "2026-08-12T16:37:11Z" + } + ] + }, + { + "pr": 206, + "rounds": [ + { + "verdict": "patch is correct", + "confidence": 0.97, + "submitted_at": "2026-08-12T15:47:08Z" + } + ] + }, + { + "pr": 207, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-12T16:51:01Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.94, + "submitted_at": "2026-08-12T16:58:29Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.96, + "submitted_at": "2026-08-12T17:06:12Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-12T17:14:03Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-12T17:22:43Z" + } + ] + }, + { + "pr": 208, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-12T18:07:59Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-12T18:45:09Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-12T19:17:44Z" + }, + { + "verdict": "patch is correct", + "confidence": 0.86, + "submitted_at": "2026-08-12T19:31:50Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-12T19:51:12Z" + } + ] + }, + { + "pr": 209, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.99, + "submitted_at": "2026-08-12T20:24:33Z" + }, + { + "verdict": "patch is correct", + "confidence": 0.88, + "submitted_at": "2026-08-12T20:38:52Z" + } + ] + }, + { + "pr": 210, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-12T23:07:46Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-12T23:36:32Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-12T23:56:24Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-13T00:13:40Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-13T00:29:11Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-13T00:46:20Z" + } + ] + }, + { + "pr": 211, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-13T07:31:58Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.99, + "submitted_at": "2026-08-13T07:42:42Z" + } + ] + }, + { + "pr": 212, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.99, + "submitted_at": "2026-08-13T09:13:05Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.99, + "submitted_at": "2026-08-13T09:29:20Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-13T09:46:03Z" + } + ] + }, + { + "pr": 213, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-13T11:06:55Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-13T11:19:55Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-13T11:44:01Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-13T11:57:38Z" + } + ] + }, + { + "pr": 214, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-13T12:29:50Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-13T12:48:09Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-13T12:57:42Z" + } + ] + }, + { + "pr": 215, + "rounds": [ + { + "verdict": "patch is correct", + "confidence": 0.9, + "submitted_at": "2026-08-13T14:45:49Z" + } + ] + }, + { + "pr": 216, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.99, + "submitted_at": "2026-08-14T02:01:39Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-14T02:20:10Z" + } + ] + }, + { + "pr": 217, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.96, + "submitted_at": "2026-08-14T02:31:22Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.96, + "submitted_at": "2026-08-14T02:35:08Z" + } + ] + }, + { + "pr": 218, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.94, + "submitted_at": "2026-08-14T09:42:29Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-14T10:02:53Z" + } + ] + }, + { + "pr": 219, + "rounds": [ + { + "verdict": "patch is correct", + "confidence": 0.91, + "submitted_at": "2026-08-15T03:02:47Z" + } + ] + }, + { + "pr": 220, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.96, + "submitted_at": "2026-08-15T04:03:19Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-15T04:14:00Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.94, + "submitted_at": "2026-08-15T04:26:54Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-15T04:38:20Z" + } + ] + }, + { + "pr": 221, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-15T14:12:59Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-15T14:35:29Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.94, + "submitted_at": "2026-08-15T14:51:43Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-15T15:00:53Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-15T17:16:15Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.93, + "submitted_at": "2026-08-15T17:48:12Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-15T18:03:00Z" + } + ] + }, + { + "pr": 222, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-15T21:43:00Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-15T23:19:45Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-15T23:37:24Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-16T00:12:31Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-16T00:34:16Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-16T00:42:04Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.99, + "submitted_at": "2026-08-16T01:01:27Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-16T01:32:35Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.99, + "submitted_at": "2026-08-16T01:47:03Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.96, + "submitted_at": "2026-08-16T08:06:31Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-16T08:38:12Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-16T09:11:40Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-16T09:32:45Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-16T09:49:34Z" + } + ] + }, + { + "pr": 223, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.92, + "submitted_at": "2026-08-16T13:43:11Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.99, + "submitted_at": "2026-08-16T13:55:57Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-16T16:19:30Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-16T16:52:23Z" + } + ] + }, + { + "pr": 224, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-16T20:22:08Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.94, + "submitted_at": "2026-08-16T22:36:11Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.96, + "submitted_at": "2026-08-17T00:37:17Z" + } + ] + }, + { + "pr": 225, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.99, + "submitted_at": "2026-08-17T05:28:04Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-17T06:20:04Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-17T06:31:46Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-17T06:42:01Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-17T06:50:21Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-17T07:08:29Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-17T07:19:22Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-17T07:39:34Z" + } + ] + }, + { + "pr": 226, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.99, + "submitted_at": "2026-08-17T11:23:08Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.92, + "submitted_at": "2026-08-17T11:46:40Z" + } + ] + }, + { + "pr": 227, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-17T16:21:28Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.96, + "submitted_at": "2026-08-17T16:41:52Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-17T17:06:38Z" + } + ] + }, + { + "pr": 229, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-17T19:35:05Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-17T19:57:52Z" + } + ] + }, + { + "pr": 230, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.99, + "submitted_at": "2026-08-17T21:13:47Z" + } + ] + }, + { + "pr": 231, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-18T03:14:20Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-18T03:40:59Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-18T04:07:20Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-18T04:39:17Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-18T04:52:53Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-18T05:36:29Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.94, + "submitted_at": "2026-08-18T06:00:05Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.94, + "submitted_at": "2026-08-18T06:20:16Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.96, + "submitted_at": "2026-08-18T07:06:22Z" + } + ] + }, + { + "pr": 232, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.95, + "submitted_at": "2026-08-19T04:32:15Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.94, + "submitted_at": "2026-08-19T05:27:23Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-19T06:23:18Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.96, + "submitted_at": "2026-08-19T07:17:05Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.99, + "submitted_at": "2026-08-19T08:17:01Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.91, + "submitted_at": "2026-08-19T08:57:41Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-19T09:05:16Z" + } + ] + }, + { + "pr": 233, + "rounds": [ + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-19T19:34:14Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-19T19:53:16Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.99, + "submitted_at": "2026-08-19T20:26:09Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.98, + "submitted_at": "2026-08-19T20:40:58Z" + }, + { + "verdict": "patch is incorrect", + "confidence": 0.97, + "submitted_at": "2026-08-19T20:59:20Z" + } + ] + }, + { + "pr": 234, + "rounds": [ + { + "verdict": "patch is correct", + "confidence": 0.93, + "submitted_at": "2026-08-21T00:52:33Z" + } + ] + } + ], + "observed": { + "pull_requests_with_rounds": 29, + "max_rounds": 14, + "median_rounds": 3, + "final_round_converged": 5 + } +} diff --git a/boatstack/flow/standard/completeness_test.go b/boatstack/flow/standard/completeness_test.go index b2c7a5b3..0cccc021 100644 --- a/boatstack/flow/standard/completeness_test.go +++ b/boatstack/flow/standard/completeness_test.go @@ -148,11 +148,11 @@ func TestSourceInventoryHasNoWriterOrLifecycleAuthorityOutsideOwnedPackages(t *t return true } importPath := imports[owner.Name] - if importPath == "os" && writerCalls[selector.Sel.Name] && !strings.HasPrefix(relative, "internal/softwaredelivery/effects/") && !strings.HasPrefix(relative, "internal/runtime/") { + if importPath == "os" && writerCalls[selector.Sel.Name] && !strings.HasPrefix(relative, "internal/softwaredelivery/effects/") && !strings.HasPrefix(relative, "internal/runtime/") && !reviewerWriterFile(relative) { t.Errorf("managed writer os.%s escaped effects package in %s", selector.Sel.Name, relative) } if importPath == "os/exec" && (selector.Sel.Name == "Command" || selector.Sel.Name == "CommandContext") { - if relative != "internal/softwaredelivery/effects/command_boundary.go" && relative != "internal/softwaredelivery/plant/resolver.go" && relative != "extension/subprocess/subprocess.go" && relative != "internal/runtime/exec_windows.go" && relative != "internal/runtime/flow_files.go" && relative != "internal/runtime/control_bundle.go" { + if relative != "internal/softwaredelivery/effects/command_boundary.go" && relative != "internal/softwaredelivery/plant/resolver.go" && relative != "extension/subprocess/subprocess.go" && relative != "internal/runtime/exec_windows.go" && relative != "internal/runtime/flow_files.go" && relative != "internal/runtime/control_bundle.go" && relative != "cmd/boatstack-reviewer/gitrepo.go" { t.Errorf("unclassified command boundary in %s", relative) } } @@ -453,8 +453,17 @@ func TestPackageImportsPreserveControlProgramDependencyDirection(t *testing.T) { } } +// reviewerWriterFile scopes the self-review control plane's writer boundary: +// boatstack-reviewer is its own control program on the generic kernel (never +// a software-delivery adapter), and only its durable store and its temporary +// git-index observer may write. +func reviewerWriterFile(relative string) bool { + return relative == "cmd/boatstack-reviewer/store.go" || relative == "cmd/boatstack-reviewer/gitrepo.go" +} + func classifiedProductionFile(relative string) bool { return relative == "delivery_controller.go" || relative == "program_effects.go" || relative == "program_observer.go" || strings.HasPrefix(relative, "cmd/boatstack-helper/") || + strings.HasPrefix(relative, "cmd/boatstack-reviewer/") || strings.HasPrefix(relative, "controlprogram/") || strings.HasPrefix(relative, "invocation/") || strings.HasPrefix(relative, "delivery/") || strings.HasPrefix(relative, "core/") || diff --git a/docs/index.md b/docs/index.md index c8bd0d0d..03170328 100644 --- a/docs/index.md +++ b/docs/index.md @@ -24,6 +24,7 @@ Control Program model. - [Generated files and ownership](generated-files.md) - [Runtime selection](runtime-selection.md) - [Safety boundaries](safety.md) +- [Self-review loop](self-review.md) - [Troubleshooting](troubleshooting.md) - [Public-surface contract](public-surface.md) diff --git a/docs/self-review.md b/docs/self-review.md new file mode 100644 index 00000000..e0f96f6c --- /dev/null +++ b/docs/self-review.md @@ -0,0 +1,105 @@ +# Self-review: the supervisory-control review loop + +Boatstack reviews its own pull requests with a control program built on the +domain-neutral kernel: `boatstack-reviewer` +(`boatstack/cmd/boatstack-reviewer`). The loop runs locally — normally driven +by a coding agent — and CI verifies the result deterministically. No reviewer, +model, or API key runs in CI. + +## Control story + +**Boundary:** a pull request head enters review-verified state. + +**Control law:** a head is review-verified only when a sealed receipt binds +its exact receipt-excluded tree, produced by the review program whose policy +assets are admitted at the pull request base revision, through a kernel +receipt chain that ends in convergence. + +The proposer (agent or human) is untrusted. It produces candidate findings +under the admitted review policy; the reviewer owns admissibility, freshness, +the convergence measure, receipts, and recovery through the kernel's +resolve/apply relation. + +- **Modes:** `unreviewed` → `findings-open` → `converged` (the only marked + mode), with `escalated` for bounded non-convergence. +- **Admitted policy:** `.github/codex/review-prompt.md` (review instructions) + and `.github/codex/review-output-schema.json` (output contract). Their exact + bytes, the round bound, the stall window, and the priority weights are + hashed into the program fingerprint; any change stales every prior + prescription and receipt. +- **Convergence measure:** each finding weighs by priority (P0 1000, P1 100, + P2 10, P3 1). A submission that fails to decrease the measure extends a + stall run; the loop escalates on the third consecutive non-improving + submission or after sixteen rounds in one generation. The bounds are + calibrated against mined review history + (`boatstack/cmd/boatstack-reviewer/testdata/review_rounds.json`). +- **Freshness:** every submission binds the exact committed tree it reviewed. + A dirty worktree, a new commit, or an edited candidate refuses admission + instead of recording a stale round. + +## Driving the loop (agent entry point) + +Work on a branch, commit your change, then: + +1. `boatstack-reviewer resolve --actor ` — prints the control state and + the instructions: the admitted prompt path, the exact review range + (`merge-base..HEAD`), and the output contract. Perform the review the + prompt describes over exactly that range and write the findings JSON to a + file, conforming to the output schema. +2. `boatstack-reviewer submit --findings --actor ` — the + reviewer validates the candidate (schema, diff anchors, tree binding) and + the kernel commits one transition: findings recorded, converged, or + escalated. Refusals name their reason and record nothing. +3. Fix the recorded findings, commit, and repeat. The measure must trend + down; convergence requires a fresh review of the fixed tree whose verdict + is `patch is correct`. +4. `boatstack-reviewer seal` — writes + `.github/reviews/.receipt.json`. Commit that file with the pull + request. The receipt directory is excluded from the tree binding, so + committing the receipt does not invalidate it. +5. `boatstack-reviewer show` prints a recorded review itself — the exact + archived findings of the latest round (`--round ` for earlier ones) and + any staged, not-yet-submitted candidate — without resolving or changing + anything. `status`, `reopen --actor ` (human capability, after + escalation or to re-review a settled generation), and `recover --actor + ` (after an interrupted effect) complete the surface. + +## Yield skill workflows + +Two [Yield](https://yield.operatorstack.systems/) workflows wrap this surface +so any registered coding agent drives the loop through recorded, resumable +operations instead of remembering the command order (adapters are registered +for Cursor, Codex, and Claude Code under their skill directories): + +- `skills/self-review` — one review round, no code changes: builds the + reviewer from the current tree, resolves the control state, has the agent + review exactly the admitted range under the admitted schema, submits, and + reports the recorded verdict. It verifies afterwards that no file changed. +- `skills/self-review-solve` — drives to convergence: fixes open findings in + code (committed by the agent), re-reviews the fixed tree, repeats within a + bounded attempt budget, then seals the receipt and commits it. An escalated + loop asks the human before reopening. + +Run either with `.yield/bin/yskill run 'skills/'` from the repository +root. `yskill doctor 'skills/' --test` exercises each workflow against +a scratch repository (fixture-created, sentinel-switched), so testing never +touches real review state. + +CI (`.github/workflows/review-verified.yml`) rebuilds the verifier and runs +`boatstack-reviewer verify --dir .github/reviews --base --head `, +which re-admits the policy from the base revision, recompiles the program +fingerprint, recomputes the receipt-excluded head tree, and checks the kernel +receipt chain and the final review bytes. + +## What the receipt does and does not prove + +Like work-package verification, the sealed receipt is honest about its +boundary: it proves the declared review program ran to convergence over the +exact bound tree under the admitted policy. It does not prove the review was +semantically right, and it does not prove who performed it — the receipt +records `semantic_correctness: not-evaluated` and `origin_authenticity: +not-proven`. Branch protection and human judgment remain the authority for +merging. + +Local review state lives under `.git/boatstack-review//` and never +enters a commit. diff --git a/release-notes/2026-08-21-supervisory-review-program.md b/release-notes/2026-08-21-supervisory-review-program.md new file mode 100644 index 00000000..7cf4e355 --- /dev/null +++ b/release-notes/2026-08-21-supervisory-review-program.md @@ -0,0 +1,3 @@ +### Self-review as a supervisory control program; CI verifies instead of reviewing + +Boatstack now reviews its own pull requests through boatstack-reviewer, a control program built directly on the domain-neutral kernel and the first production consumer of the generic runtime. The review loop runs locally: an untrusted proposer (coding agent or human) submits candidate findings under the admitted review policy, and the kernel commits each round only after deterministic schema, diff-anchor, tree-binding, and convergence-measure admission. A converged loop seals a receipt bound to the receipt-excluded head tree, committed with the pull request. The codex-action CI reviewer is removed; the review-verified workflow deterministically verifies the sealed receipt against the base-admitted policy with no model or API key in CI. Two Yield skill workflows (skills/self-review and skills/self-review-solve, registered for Cursor, Codex, and Claude Code) wrap the loop in recorded, resumable operations: one runs a review round and reports the verdict without changing code, the other fixes open findings, converges the loop, and seals the receipt. diff --git a/skills/self-review-solve/SKILL.md b/skills/self-review-solve/SKILL.md new file mode 100644 index 00000000..7a5a8432 --- /dev/null +++ b/skills/self-review-solve/SKILL.md @@ -0,0 +1,28 @@ +--- +name: self-review-solve +description: "Resolve the Boatstack self-review: fix open findings or run a fresh review, converge the loop, and seal the receipt." +--- + +Drive the supervisory-control self-review of the current branch to +convergence. The workflow decides from the committed control state what is +needed: open findings are fixed in code and committed, an unreviewed tree +gets a fresh review, an escalated loop asks before reopening, and a +converged loop is sealed and the receipt committed. + +Run from the repository root: + + .yield/bin/yskill run 'skills/self-review-solve' + +If `.yield/bin/yskill` is missing, install the pinned runtime first: + + go install github.com/operatorstack/yield/cmd/yskill@v0.1.38 && yskill init skills/self-review-solve --language go + +Follow each returned operation exactly. Answer it directly: + + .yield/bin/yskill respond --value --skill 'skills/self-review-solve' + +Do not skip an operation or invent a response. When the run asks you to fix +findings, edit the code, run the relevant tests, and commit before +responding. When it asks for a review, read the prompt file it names, review +only the committed range it names, and return only the schema-valid JSON +object. diff --git a/skills/self-review-solve/fixtures/responses.json b/skills/self-review-solve/fixtures/responses.json new file mode 100644 index 00000000..2beebc80 --- /dev/null +++ b/skills/self-review-solve/fixtures/responses.json @@ -0,0 +1,8 @@ +{ + "review-1": { + "findings": [], + "overall_correctness": "patch is correct", + "overall_explanation": "Fixture review of the scratch change: the single-function addition is self-contained, anchored entirely inside the review range, and introduces no defect at the reporting threshold.", + "overall_confidence_score": 0.9 + } +} diff --git a/skills/self-review-solve/fixtures/setup.sh b/skills/self-review-solve/fixtures/setup.sh new file mode 100755 index 00000000..1024f9c9 --- /dev/null +++ b/skills/self-review-solve/fixtures/setup.sh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash +# Test-only fixture: build a scratch repository so the workflow's real +# commands never touch the actual repository's review state. Runs with the +# skill directory as the working directory and YIELD_FIXTURE=1. +set -eu + +root="$(git rev-parse --show-toplevel)" +scratch="$PWD/fixtures/tmp/repo" +rm -rf "$PWD/fixtures/tmp" +mkdir -p "$scratch" + +git -C "$scratch" init -q -b main +git -C "$scratch" config user.email "self-review-fixture@example.invalid" +git -C "$scratch" config user.name "Self Review Fixture" + +mkdir -p "$scratch/.github/codex" +cp "$root/.github/codex/review-prompt.md" "$scratch/.github/codex/" +cp "$root/.github/codex/review-output-schema.json" "$scratch/.github/codex/" +printf 'package subject\n' > "$scratch/subject.go" +git -C "$scratch" add -A +git -C "$scratch" commit -qm "base" + +git -C "$scratch" checkout -qb feature +printf 'package subject\n\nfunc Value() int { return 1 }\n' > "$scratch/subject.go" +git -C "$scratch" commit -qam "change under review" + +# Regression: untracked files never affect what a review binds, so the +# workflow's gates must ignore them just as the reviewer does. +printf 'editor scratch\n' > "$scratch/untracked-note.txt" + +touch "$PWD/fixtures/tmp/active" diff --git a/skills/self-review-solve/fixtures/teardown.sh b/skills/self-review-solve/fixtures/teardown.sh new file mode 100755 index 00000000..26996337 --- /dev/null +++ b/skills/self-review-solve/fixtures/teardown.sh @@ -0,0 +1,4 @@ +#!/usr/bin/env bash +# Remove the scratch repository and the test sentinel after success or failure. +set -eu +rm -rf "$PWD/fixtures/tmp" diff --git a/skills/self-review-solve/fixtures/test.json b/skills/self-review-solve/fixtures/test.json new file mode 100644 index 00000000..09366404 --- /dev/null +++ b/skills/self-review-solve/fixtures/test.json @@ -0,0 +1,6 @@ +{ + "version": 1, + "setup": [["bash", "fixtures/setup.sh"]], + "after_response": {}, + "teardown": [["bash", "fixtures/teardown.sh"]] +} diff --git a/skills/self-review-solve/go.mod b/skills/self-review-solve/go.mod new file mode 100644 index 00000000..fe547c29 --- /dev/null +++ b/skills/self-review-solve/go.mod @@ -0,0 +1,10 @@ +module self-review-solve + +go 1.26.5 + +require github.com/operatorstack/yield v0.1.38 + +require ( + github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 // indirect + golang.org/x/text v0.14.0 // indirect +) diff --git a/skills/self-review-solve/go.sum b/skills/self-review-solve/go.sum new file mode 100644 index 00000000..b638f93e --- /dev/null +++ b/skills/self-review-solve/go.sum @@ -0,0 +1,8 @@ +github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI= +github.com/dlclark/regexp2 v1.11.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= +github.com/operatorstack/yield v0.1.38 h1:QRAcWr2Do5S0wucJiDV2EXd6QP3JpgBt1YFl/rebAEY= +github.com/operatorstack/yield v0.1.38/go.mod h1:0cwOszGHYM1O8syOxJ3+NIsgR/h5JwCC680agfTKYVs= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEVZGK7IN2kJkjTuQ= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU= +golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= +golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= diff --git a/skills/self-review-solve/main.go b/skills/self-review-solve/main.go new file mode 100644 index 00000000..47fa3665 --- /dev/null +++ b/skills/self-review-solve/main.go @@ -0,0 +1,290 @@ +// Skill workflow: drive the Boatstack supervisory-control self-review to +// convergence and seal the receipt. +// +// The workflow decides from the committed control state what is needed: +// open findings are fixed in code and committed, an unreviewed tree gets a +// fresh review, an escalated loop asks the human before reopening, and a +// converged loop is sealed. The loop is bounded; if it does not converge +// within the attempt budget the run blocks honestly instead of guessing. +package main + +import ( + "encoding/base64" + "encoding/json" + "fmt" + + "github.com/operatorstack/yield/sdk/yield" +) + +// prelude prepares every command's execution context. Commands run with the +// skill directory as the working directory. When the test sentinel exists +// (created by fixtures/setup.sh under yskill's fixture runner), commands +// operate on the scratch repository so tests never touch real review state. +const prelude = `set -eu +root="$(git rev-parse --show-toplevel)" +if [ -f fixtures/tmp/active ]; then repo="$PWD/fixtures/tmp/repo"; base=main; else repo="$root"; base=origin/main; fi +tmp="${TMPDIR:-/tmp}/boatstack-self-review-solve" +mkdir -p "$tmp" +reviewer="$tmp/boatstack-reviewer" +` + +const ( + actor = "yield-self-review-solve" + maxAttempts = 3 +) + +const fixReportSchema = `{ + "type": "object", + "required": ["summary", "committed"], + "properties": { + "summary": {"type": "string", "minLength": 1}, + "committed": {"type": "boolean"} + }, + "additionalProperties": false +}` + +type statusOutput struct { + Instance string `json:"instance"` + State struct { + Mode string `json:"mode"` + } `json:"state"` + ProgramStale bool `json:"program_stale"` + Observation struct { + WorktreeDirty bool `json:"worktree_dirty"` + ReviewedTree string `json:"reviewed_tree"` + Rounds []struct { + Index int `json:"index"` + Verdict string `json:"verdict"` + Measure int `json:"measure"` + ReviewedTree string `json:"reviewed_tree"` + } `json:"rounds"` + } `json:"observation"` +} + +type resolveOutput struct { + Instructions struct { + PromptPath string `json:"prompt_path"` + ReviewRange string `json:"review_range"` + SchemaPath string `json:"output_schema_path"` + } `json:"instructions"` +} + +type showOutput struct { + Mode string `json:"mode"` + Round struct { + Index int `json:"index"` + Verdict string `json:"verdict"` + Measure int `json:"measure"` + } `json:"round"` + Review json.RawMessage `json:"review"` +} + +func main() { + yield.Main(func(ctx *yield.Context) (yield.Outcome, error) { + build := ctx.RunCommand("build-reviewer", + prelude+`go build -C "$root/boatstack" -o "$reviewer" ./cmd/boatstack-reviewer`, 600) + ctx.Require(build.ExitCode == 0, "boatstack-reviewer builds from the current tree", build) + + observed := status(ctx, "status") + mode, measures := observed.State.Mode, observed.measures() + if mode == "converged" && observed.treeDrifted() { + // The instance converged for an older tree; new commits need a + // fresh generation. Invoking this skill is the decision to + // re-review them, so reopen without asking. + reopen := ctx.RunCommand("reopen-drift", + prelude+`"$reviewer" reopen --repo "$repo" --base "$base" --actor `+actor, 120) + ctx.Require(reopen.ExitCode == 0, + "a fresh review generation is open for the moved tree", reopen) + observed = status(ctx, "status-after-drift-reopen") + mode, measures = observed.State.Mode, observed.measures() + } + if mode == "escalated" { + answer := ctx.AskUser("reopen", + "The review loop escalated (the convergence measure stalled or the round bound was reached). Reopen a fresh review generation?", + yield.Option{Value: "yes", Label: "Reopen and continue"}, + yield.Option{Value: "no", Label: "Stop; a human will handle it"}) + if answer != "yes" { + return yield.Outcome{}, ctx.Refused("the escalated loop stays with the human") + } + reopen := ctx.RunCommand("reopen", + prelude+`"$reviewer" reopen --repo "$repo" --base "$base" --actor `+actor, 120) + ctx.Require(reopen.ExitCode == 0, "a fresh review generation is open", reopen) + observed = status(ctx, "status-after-reopen") + mode, measures = observed.State.Mode, observed.measures() + } + + for attempt := 1; mode != "converged" && attempt <= maxAttempts; attempt++ { + tag := fmt.Sprintf("-%d", attempt) + + // Fix only when the open findings still describe the current + // tree; if commits already landed since the round was recorded, + // the fixes may exist and a fresh review is what decides. + if mode == "findings-open" && !observed.treeDrifted() { + shown := show(ctx, "open-findings"+tag) + fixRaw := ctx.AgentTask("fix"+tag, + "Fix every finding of this recorded review in the repository under review: edit the code, "+ + "run the relevant tests, and commit the fixes with a clear message. Do not touch .github/reviews "+ + "and do not weaken tests to make findings disappear. Report what you changed.", + map[string]any{"round": shown.Round, "review": json.RawMessage(shown.Review)}, + json.RawMessage(fixReportSchema)) + var fix struct { + Summary string `json:"summary"` + Committed bool `json:"committed"` + } + if err := json.Unmarshal(fixRaw, &fix); err != nil { + return yield.Outcome{}, err + } + ctx.Require(fix.Committed, "the fixes are committed", fix) + // Mirror the reviewer's worktree law: untracked files never + // affect what a review can bind, so only tracked changes + // count as an uncommitted fix. + clean := ctx.RunCommand("worktree"+tag, + prelude+`git -C "$repo" status --porcelain --untracked-files=no`, 60) + ctx.Require(clean.ExitCode == 0 && clean.Stdout == "", + "the worktree is clean after the fix commit", clean) + } + + resolve := ctx.RunCommand("resolve"+tag, + prelude+`"$reviewer" resolve --repo "$repo" --base "$base" --actor `+actor, 120) + ctx.Require(resolve.ExitCode == 0, "the review control state resolves", resolve) + var resolved resolveOutput + if err := json.Unmarshal([]byte(resolve.Stdout), &resolved); err != nil { + return yield.Outcome{}, err + } + schema := ctx.RunCommand("schema"+tag, + prelude+`cat "$repo/`+resolved.Instructions.SchemaPath+`"`, 30) + ctx.Require(schema.ExitCode == 0, "the admitted output schema is readable", schema) + + review := ctx.AgentTask("review"+tag, + fmt.Sprintf("Perform the code review described by %s (in the repository under review) over exactly the range %s. "+ + "Read the prompt file first and follow it precisely. Review only committed content; do not modify any file. "+ + "Anchor every finding to changed lines of that exact range and return only the JSON object required by the schema.", + resolved.Instructions.PromptPath, resolved.Instructions.ReviewRange), + map[string]any{"review_range": resolved.Instructions.ReviewRange}, + json.RawMessage(schema.Stdout)) + + writeCandidate(ctx, "candidate"+tag, review) + submit := ctx.RunCommand("submit"+tag, + prelude+`base64 -d < "$tmp/candidate.b64" > "$tmp/candidate.json" +"$reviewer" submit --repo "$repo" --base "$base" --findings "$tmp/candidate.json" --actor `+actor, 120) + if submit.ExitCode != 0 { + return yield.Outcome{}, ctx.Blocked( + "the reviewer refused the candidate: " + submit.Stderr) + } + + observed = status(ctx, "status"+tag) + mode, measures = observed.State.Mode, observed.measures() + if mode == "escalated" { + return yield.Outcome{}, ctx.Blocked( + "the loop escalated during solving; a human must decide before reopening") + } + } + + if mode != "converged" { + return yield.Outcome{}, ctx.Blocked(fmt.Sprintf( + "the review did not converge within %d attempts (measures %v); the remaining findings need a human decision", + maxAttempts, measures)) + } + + seal := ctx.RunCommand("seal", + prelude+`"$reviewer" seal --repo "$repo" --base "$base"`, 120) + ctx.Require(seal.ExitCode == 0, "the converged review seals a receipt", seal) + var sealed struct { + Sealed string `json:"sealed"` + Fingerprint string `json:"fingerprint"` + ReviewedTree string `json:"reviewed_tree"` + } + if err := json.Unmarshal([]byte(seal.Stdout), &sealed); err != nil { + return yield.Outcome{}, err + } + + commit := ctx.RunCommand("commit-receipt", + prelude+`if [ -n "$(git -C "$repo" status --porcelain .github/reviews)" ]; then + git -C "$repo" add .github/reviews + git -C "$repo" commit -qm "Seal converged self-review receipt" + echo committed +else + echo unchanged +fi`, 60) + ctx.Require(commit.ExitCode == 0, "the sealed receipt is committed with the change", commit) + + return ctx.Complete(map[string]any{ + "mode": "converged", + "receipt": sealed.Sealed, + "fingerprint": sealed.Fingerprint, + "reviewed_tree": sealed.ReviewedTree, + "measures": measures, + "receipt_state": commit.Stdout, + "guidance": "push the branch; the review-verified CI job verifies the receipt deterministically", + }) + }) +} + +// writeCandidate stages the encoded candidate through bounded chunks so no +// single command string approaches the platform's per-argument size cap +// (MAX_ARG_STRLEN on Linux), which a large multi-finding review could +// otherwise exceed. +func writeCandidate(ctx *yield.Context, idPrefix string, review []byte) { + encoded := base64.StdEncoding.EncodeToString(review) + const chunkSize = 65536 + for i, part := 0, 1; i < len(encoded); i, part = i+chunkSize, part+1 { + end := i + chunkSize + if end > len(encoded) { + end = len(encoded) + } + redirect := ">>" + if i == 0 { + redirect = ">" + } + written := ctx.RunCommand(fmt.Sprintf("%s-part-%d", idPrefix, part), + prelude+`printf '%s' '`+encoded[i:end]+`' `+redirect+` "$tmp/candidate.b64"`, 60) + ctx.Require(written.ExitCode == 0, "the candidate chunk is staged", written) + } +} + +func (o statusOutput) measures() []int { + measures := make([]int, 0, len(o.Observation.Rounds)) + for _, round := range o.Observation.Rounds { + measures = append(measures, round.Measure) + } + return measures +} + +// treeDrifted reports whether the current reviewed tree moved past the last +// recorded round's tree — the condition under which a converged instance +// needs a fresh generation before sealing. +func (o statusOutput) treeDrifted() bool { + rounds := o.Observation.Rounds + if len(rounds) == 0 { + return false + } + return rounds[len(rounds)-1].ReviewedTree != o.Observation.ReviewedTree +} + +// status observes the committed control state; refusals here are conditions +// only a human can change. +func status(ctx *yield.Context, id string) statusOutput { + result := ctx.RunCommand(id, + prelude+`"$reviewer" status --repo "$repo" --base "$base"`, 120) + ctx.Require(result.ExitCode == 0, "the review control state is observable", result) + var observed statusOutput + if err := json.Unmarshal([]byte(result.Stdout), &observed); err != nil { + ctx.Require(false, "the status output decodes", map[string]any{"error": err.Error()}) + } + ctx.Require(!observed.ProgramStale, + "the committed state belongs to the active review program (reset required otherwise)", observed) + ctx.Require(!observed.Observation.WorktreeDirty, + "the worktree has no uncommitted tracked changes", observed) + return observed +} + +func show(ctx *yield.Context, id string) showOutput { + result := ctx.RunCommand(id, + prelude+`"$reviewer" show --repo "$repo" --base "$base"`, 60) + ctx.Require(result.ExitCode == 0, "the latest recorded review is shown", result) + var shown showOutput + if err := json.Unmarshal([]byte(result.Stdout), &shown); err != nil { + ctx.Require(false, "the show output decodes", map[string]any{"error": err.Error()}) + } + return shown +} diff --git a/skills/self-review-solve/skill.json b/skills/self-review-solve/skill.json new file mode 100644 index 00000000..939e85ec --- /dev/null +++ b/skills/self-review-solve/skill.json @@ -0,0 +1 @@ +{"version":1,"language":"go","run":["go","run","-mod=readonly","."]} diff --git a/skills/self-review/SKILL.md b/skills/self-review/SKILL.md new file mode 100644 index 00000000..1371d81c --- /dev/null +++ b/skills/self-review/SKILL.md @@ -0,0 +1,25 @@ +--- +name: self-review +description: "Run the Boatstack supervisory-control self-review for the current branch and report the verdict without changing code." +--- + +Run one review round for the current branch against `origin/main` and show +the recorded verdict. This skill never edits code: the review is performed +read-only, `boatstack-reviewer` admits or refuses the candidate, and the +round is recorded in the repository's local review store. + +Run from the repository root: + + .yield/bin/yskill run 'skills/self-review' + +If `.yield/bin/yskill` is missing, install the pinned runtime first: + + go install github.com/operatorstack/yield/cmd/yskill@v0.1.38 && yskill init skills/self-review --language go + +Follow each returned operation exactly. Answer it directly: + + .yield/bin/yskill respond --value --skill 'skills/self-review' + +Do not skip an operation or invent a response. When the run asks for the +review (the `review` agent task), read the prompt file it names, review only +the committed range it names, and return only the schema-valid JSON object. diff --git a/skills/self-review/fixtures/responses.json b/skills/self-review/fixtures/responses.json new file mode 100644 index 00000000..e3cd9534 --- /dev/null +++ b/skills/self-review/fixtures/responses.json @@ -0,0 +1,8 @@ +{ + "review": { + "findings": [], + "overall_correctness": "patch is correct", + "overall_explanation": "Fixture review of the scratch change: the single-function addition is self-contained, anchored entirely inside the review range, and introduces no defect at the reporting threshold.", + "overall_confidence_score": 0.9 + } +} diff --git a/skills/self-review/fixtures/setup.sh b/skills/self-review/fixtures/setup.sh new file mode 100755 index 00000000..1024f9c9 --- /dev/null +++ b/skills/self-review/fixtures/setup.sh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash +# Test-only fixture: build a scratch repository so the workflow's real +# commands never touch the actual repository's review state. Runs with the +# skill directory as the working directory and YIELD_FIXTURE=1. +set -eu + +root="$(git rev-parse --show-toplevel)" +scratch="$PWD/fixtures/tmp/repo" +rm -rf "$PWD/fixtures/tmp" +mkdir -p "$scratch" + +git -C "$scratch" init -q -b main +git -C "$scratch" config user.email "self-review-fixture@example.invalid" +git -C "$scratch" config user.name "Self Review Fixture" + +mkdir -p "$scratch/.github/codex" +cp "$root/.github/codex/review-prompt.md" "$scratch/.github/codex/" +cp "$root/.github/codex/review-output-schema.json" "$scratch/.github/codex/" +printf 'package subject\n' > "$scratch/subject.go" +git -C "$scratch" add -A +git -C "$scratch" commit -qm "base" + +git -C "$scratch" checkout -qb feature +printf 'package subject\n\nfunc Value() int { return 1 }\n' > "$scratch/subject.go" +git -C "$scratch" commit -qam "change under review" + +# Regression: untracked files never affect what a review binds, so the +# workflow's gates must ignore them just as the reviewer does. +printf 'editor scratch\n' > "$scratch/untracked-note.txt" + +touch "$PWD/fixtures/tmp/active" diff --git a/skills/self-review/fixtures/teardown.sh b/skills/self-review/fixtures/teardown.sh new file mode 100755 index 00000000..26996337 --- /dev/null +++ b/skills/self-review/fixtures/teardown.sh @@ -0,0 +1,4 @@ +#!/usr/bin/env bash +# Remove the scratch repository and the test sentinel after success or failure. +set -eu +rm -rf "$PWD/fixtures/tmp" diff --git a/skills/self-review/fixtures/test.json b/skills/self-review/fixtures/test.json new file mode 100644 index 00000000..09366404 --- /dev/null +++ b/skills/self-review/fixtures/test.json @@ -0,0 +1,6 @@ +{ + "version": 1, + "setup": [["bash", "fixtures/setup.sh"]], + "after_response": {}, + "teardown": [["bash", "fixtures/teardown.sh"]] +} diff --git a/skills/self-review/go.mod b/skills/self-review/go.mod new file mode 100644 index 00000000..3f27ef6c --- /dev/null +++ b/skills/self-review/go.mod @@ -0,0 +1,10 @@ +module self-review + +go 1.26.5 + +require github.com/operatorstack/yield v0.1.38 + +require ( + github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 // indirect + golang.org/x/text v0.14.0 // indirect +) diff --git a/skills/self-review/go.sum b/skills/self-review/go.sum new file mode 100644 index 00000000..b638f93e --- /dev/null +++ b/skills/self-review/go.sum @@ -0,0 +1,8 @@ +github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI= +github.com/dlclark/regexp2 v1.11.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= +github.com/operatorstack/yield v0.1.38 h1:QRAcWr2Do5S0wucJiDV2EXd6QP3JpgBt1YFl/rebAEY= +github.com/operatorstack/yield v0.1.38/go.mod h1:0cwOszGHYM1O8syOxJ3+NIsgR/h5JwCC680agfTKYVs= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEVZGK7IN2kJkjTuQ= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU= +golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= +golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= diff --git a/skills/self-review/main.go b/skills/self-review/main.go new file mode 100644 index 00000000..8581ec4c --- /dev/null +++ b/skills/self-review/main.go @@ -0,0 +1,201 @@ +// Skill workflow: run one round of the Boatstack supervisory-control +// self-review for the current branch and report the verdict. +// +// The workflow never changes code: the agent performs the review read-only, +// the boatstack-reviewer admits or refuses the candidate, and the recorded +// verdict is the result. Yield owns the order and the observed command +// results; the reviewer owns admissibility, freshness, and receipts. +package main + +import ( + "encoding/base64" + "encoding/json" + "fmt" + + "github.com/operatorstack/yield/sdk/yield" +) + +// prelude prepares every command's execution context. Commands run with the +// skill directory as the working directory. When the test sentinel exists +// (created by fixtures/setup.sh under yskill's fixture runner), commands +// operate on the scratch repository so tests never touch real review state. +const prelude = `set -eu +root="$(git rev-parse --show-toplevel)" +if [ -f fixtures/tmp/active ]; then repo="$PWD/fixtures/tmp/repo"; base=main; else repo="$root"; base=origin/main; fi +tmp="${TMPDIR:-/tmp}/boatstack-self-review" +mkdir -p "$tmp" +reviewer="$tmp/boatstack-reviewer" +` + +const actor = "yield-self-review" + +type resolveOutput struct { + Instance string `json:"instance"` + State struct { + Mode string `json:"mode"` + } `json:"state"` + Observation struct { + WorktreeDirty bool `json:"worktree_dirty"` + Rounds []struct { + Index int `json:"index"` + Verdict string `json:"verdict"` + Measure int `json:"measure"` + ReviewedTree string `json:"reviewed_tree"` + } `json:"rounds"` + } `json:"observation"` + Instructions struct { + PromptPath string `json:"prompt_path"` + ReviewRange string `json:"review_range"` + ReviewedTree string `json:"reviewed_tree"` + SchemaPath string `json:"output_schema_path"` + } `json:"instructions"` +} + +type showOutput struct { + Mode string `json:"mode"` + Round struct { + Index int `json:"index"` + Verdict string `json:"verdict"` + Measure int `json:"measure"` + FindingCount int `json:"finding_count"` + } `json:"round"` + Review json.RawMessage `json:"review"` +} + +// writeCandidate stages the encoded candidate through bounded chunks so no +// single command string approaches the platform's per-argument size cap +// (MAX_ARG_STRLEN on Linux), which a large multi-finding review could +// otherwise exceed. +func writeCandidate(ctx *yield.Context, idPrefix string, review []byte) { + encoded := base64.StdEncoding.EncodeToString(review) + const chunkSize = 65536 + for i, part := 0, 1; i < len(encoded); i, part = i+chunkSize, part+1 { + end := i + chunkSize + if end > len(encoded) { + end = len(encoded) + } + redirect := ">>" + if i == 0 { + redirect = ">" + } + written := ctx.RunCommand(fmt.Sprintf("%s-part-%d", idPrefix, part), + prelude+`printf '%s' '`+encoded[i:end]+`' `+redirect+` "$tmp/candidate.b64"`, 60) + ctx.Require(written.ExitCode == 0, "the candidate chunk is staged", written) + } +} + +func resolveState(ctx *yield.Context, id string) (resolveOutput, error) { + result := ctx.RunCommand(id, + prelude+`"$reviewer" resolve --repo "$repo" --base "$base" --actor `+actor, 120) + ctx.Require(result.ExitCode == 0, "the review control state resolves", result) + var resolved resolveOutput + if err := json.Unmarshal([]byte(result.Stdout), &resolved); err != nil { + return resolveOutput{}, err + } + return resolved, nil +} + +func main() { + yield.Main(func(ctx *yield.Context) (yield.Outcome, error) { + build := ctx.RunCommand("build-reviewer", + prelude+`go build -C "$root/boatstack" -o "$reviewer" ./cmd/boatstack-reviewer`, 600) + ctx.Require(build.ExitCode == 0, "boatstack-reviewer builds from the current tree", build) + + // Track only tracked content, mirroring the reviewer's worktree law: + // untracked files never affect what a review can bind. + before := ctx.RunCommand("worktree-before", + prelude+`git -C "$repo" status --porcelain --untracked-files=no`, 60) + ctx.Require(before.ExitCode == 0, "the repository state is observable", before) + + resolved, err := resolveState(ctx, "resolve") + if err != nil { + return yield.Outcome{}, err + } + if resolved.State.Mode == "converged" { + rounds := resolved.Observation.Rounds + if len(rounds) > 0 && rounds[len(rounds)-1].ReviewedTree == resolved.Instructions.ReviewedTree { + return ctx.Complete(map[string]any{ + "instance": resolved.Instance, + "mode": resolved.State.Mode, + "verdict": "patch is correct", + "guidance": "already converged for this tree; run boatstack-reviewer seal, or reopen to re-review", + }) + } + // The instance converged for an older tree; new commits need a + // fresh generation before a round can be recorded. + reopen := ctx.RunCommand("reopen", + prelude+`"$reviewer" reopen --repo "$repo" --base "$base" --actor `+actor, 120) + ctx.Require(reopen.ExitCode == 0, + "a fresh review generation is open for the moved tree", reopen) + if resolved, err = resolveState(ctx, "resolve-after-reopen"); err != nil { + return yield.Outcome{}, err + } + } + if resolved.State.Mode == "escalated" { + return yield.Outcome{}, ctx.Refused( + "the review loop escalated; a human must decide, then boatstack-reviewer reopen") + } + if resolved.Observation.WorktreeDirty { + return yield.Outcome{}, ctx.Refused( + "the worktree has uncommitted tracked changes; commit first — a review binds only a committed tree") + } + + schema := ctx.RunCommand("schema", + prelude+`cat "$repo/`+resolved.Instructions.SchemaPath+`"`, 30) + ctx.Require(schema.ExitCode == 0, "the admitted output schema is readable", schema) + + instruction := fmt.Sprintf( + "Perform the code review described by %s (in the repository under review) over exactly the range %s. "+ + "Read the prompt file first and follow it precisely. Review only committed content; do not modify, create, or delete any file. "+ + "Anchor every finding to changed lines of that exact range and return only the JSON object required by the schema.", + resolved.Instructions.PromptPath, resolved.Instructions.ReviewRange) + review := ctx.AgentTask("review", instruction, + map[string]any{ + "instance": resolved.Instance, + "mode": resolved.State.Mode, + "review_range": resolved.Instructions.ReviewRange, + "reviewed_tree": resolved.Instructions.ReviewedTree, + "prompt_path": resolved.Instructions.PromptPath, + }, + json.RawMessage(schema.Stdout)) + + writeCandidate(ctx, "candidate", review) + submit := ctx.RunCommand("submit", + prelude+`base64 -d < "$tmp/candidate.b64" > "$tmp/candidate.json" +"$reviewer" submit --repo "$repo" --base "$base" --findings "$tmp/candidate.json" --actor `+actor, 120) + if submit.ExitCode != 0 { + return yield.Outcome{}, ctx.Blocked( + "the reviewer refused the candidate: " + submit.Stderr) + } + + verdict := ctx.RunCommand("verdict", + prelude+`"$reviewer" show --repo "$repo" --base "$base"`, 60) + ctx.Require(verdict.ExitCode == 0, "the recorded round is shown from the store", verdict) + var shown showOutput + if err := json.Unmarshal([]byte(verdict.Stdout), &shown); err != nil { + return yield.Outcome{}, err + } + + after := ctx.RunCommand("worktree-after", + prelude+`git -C "$repo" status --porcelain --untracked-files=no`, 60) + ctx.Require(after.ExitCode == 0 && after.Stdout == before.Stdout, + "the review changed no files in the repository", map[string]any{ + "before": before.Stdout, "after": after.Stdout, + }) + + return ctx.Complete(map[string]any{ + "instance": resolved.Instance, + "mode": shown.Mode, + "round": shown.Round.Index, + "verdict": shown.Round.Verdict, + "measure": shown.Round.Measure, + "finding_count": shown.Round.FindingCount, + "review": json.RawMessage(shown.Review), + "guidance": map[string]string{ + "converged": "run boatstack-reviewer seal and commit the sealed receipt", + "findings-open": "fix the findings (or run the self-review-solve skill), commit, and review again", + "escalated": "the loop escalated; a human must decide, then boatstack-reviewer reopen", + }[shown.Mode], + }) + }) +} diff --git a/skills/self-review/skill.json b/skills/self-review/skill.json new file mode 100644 index 00000000..939e85ec --- /dev/null +++ b/skills/self-review/skill.json @@ -0,0 +1 @@ +{"version":1,"language":"go","run":["go","run","-mod=readonly","."]}