From ded4e91d1f5a1d51e8a078bd8ca4cd5697670a0e Mon Sep 17 00:00:00 2001 From: "courtmanr@gmail.com" Date: Tue, 29 Sep 2026 17:06:44 +0100 Subject: [PATCH] Validate merged integration ranges against in-range browser receipts The maintainer coordinator integrates reviewed candidates by merge onto a main that keeps moving, then ran this guard once over before..after with --commit after. That demands one tip receipt whose base_sha is the tip's parent and whose paths and digests cover the whole cumulative delta. The tip is a merge and the range carries the candidate plus upstream work that was verified on its own, so no lane could ever produce that receipt. 13 of 13 coordinator browser-guard rollbacks since 28 September failed on base_sha for this reason. Add --base for range mode. Every changed user-visible frontend file's content at the tip must equal a digest recorded by a valid receipt in a non-merge commit inside the range, and each receipt is validated as the per-commit guard validates it at its own commit, bound to that commit's parent and tree. Merge-authored receipts are never evidence, so a conflict resolution or a later correction that changes frontend source still needs fresh proof for the final content. Replaying the three rolled-back 29 September batches from the host passes, and the existing per-commit and staged modes are unchanged. --- .../CANONICAL_DEVELOPMENT_PROTOCOL.md | 14 ++ .../browser_verification_guard.py | 169 +++++++++++++++++- .../browser_verification_guard_test.py | 149 +++++++++++++++ 3 files changed, 327 insertions(+), 5 deletions(-) diff --git a/docs/release-control/v6/internal/CANONICAL_DEVELOPMENT_PROTOCOL.md b/docs/release-control/v6/internal/CANONICAL_DEVELOPMENT_PROTOCOL.md index cb206464cc..73761c66b4 100644 --- a/docs/release-control/v6/internal/CANONICAL_DEVELOPMENT_PROTOCOL.md +++ b/docs/release-control/v6/internal/CANONICAL_DEVELOPMENT_PROTOCOL.md @@ -253,6 +253,20 @@ print a non-passing receipt skeleton with `python3 scripts/release_control/browser_verification_guard.py --print-template` after staging the intended frontend paths. +An integration range that merges reviewed work onto a main that has moved is +validated with `--base --commit `. The tip of such a +range is normally a merge, and the range carries several independently +verified changes, so no single receipt can name the tip's parent or the whole +cumulative delta. Range mode instead requires every changed user-visible +frontend file's content at the tip to equal a `content_sha256` digest recorded +by a valid receipt in a non-merge commit inside the range, where that receipt +is validated exactly as the per-commit guard validates it at its own commit: +bound to that commit's parent and to that commit's tree. Receipts written by +merge commits are never evidence, because a conflict resolution has no browser +run behind it. Content that changed after its browser pass, including a +correction commit, a conflict resolution, or two changes to one file, needs a +fresh receipt for the final content in its own non-merge commit. + A source diff that is byte-for-byte the locked Prettier output of its parent has no rendered behavior or visual delta and does not require a new browser receipt. The exemption fails closed when the formatter is unavailable or the diff --git a/scripts/release_control/browser_verification_guard.py b/scripts/release_control/browser_verification_guard.py index 9b0ad13cc4..0c9fbe5fcd 100644 --- a/scripts/release_control/browser_verification_guard.py +++ b/scripts/release_control/browser_verification_guard.py @@ -107,6 +107,7 @@ def formatting_only_paths( paths: Sequence[str], *, commit: str | None, + base: str | None = None, repo_root: Path = REPO_ROOT, ) -> set[str]: """Paths whose new content is exactly prettier's output for the old content. @@ -118,13 +119,14 @@ def formatting_only_paths( This fails closed: an added or deleted file, an unreadable blob, a prettier that will not run, or any output that is not byte-identical all fall - through and still require the receipt. + through and still require the receipt. `base` compares against an + explicit range base instead of the commit's parent. """ prettier = format_staged_frontend.prettier_bin() if prettier is None: return set() - base_revision = f"{commit}^" if commit else "HEAD" + base_revision = base or (f"{commit}^" if commit else "HEAD") formatting_only: set[str] = set() for path in paths: new_object = f"{commit}:{path}" if commit else f":{path}" @@ -244,6 +246,108 @@ def validate_receipt( return errors +def receipt_commits_in_range( + base: str, + head: str, + *, + repo_root: Path = REPO_ROOT, +) -> list[str]: + """Non-merge commits in base..head that author a receipt version. + + Merge commits are excluded: a merge resolution of the shared receipt keeps + or combines other commits' records without a browser run of its own, so it + can never be evidence. --full-history keeps receipt commits on merged side + branches that default history simplification would prune. + """ + output = run_git( + [ + "rev-list", + "--reverse", + "--no-merges", + "--full-history", + f"{base}..{head}", + "--", + RECEIPT_PATH, + ], + repo_root=repo_root, + ) + return [line.strip() for line in output.splitlines() if line.strip()] + + +def range_receipt_coverage( + base: str, + head: str, + *, + repo_root: Path = REPO_ROOT, +) -> tuple[dict[str, set[str]], list[str]]: + """Content digests verified by valid receipts authored inside base..head. + + Each receipt is validated exactly as the per-commit guard validates it at + the commit that recorded it: bound to that commit's own parent, and its + changed_paths and content_sha256 matching that commit's tree. A receipt + that fails contributes no coverage and is reported. + """ + covered: dict[str, set[str]] = {} + diagnostics: list[str] = [] + for commit in receipt_commits_in_range(base, head, repo_root=repo_root): + try: + payload = json.loads(load_receipt_text(commit=commit, repo_root=repo_root)) + commit_base = expected_base_sha(commit=commit, repo_root=repo_root) + except (json.JSONDecodeError, subprocess.CalledProcessError) as exc: + diagnostics.append(f"{commit[:12]}: unable to load receipt: {exc}") + continue + listed = payload.get("changed_paths") if isinstance(payload, dict) else None + listed_paths = ( + [path for path in listed if isinstance(path, str) and path] + if isinstance(listed, list) + else [] + ) + errors = validate_receipt( + payload, + changed_paths=listed_paths, + expected_base=commit_base, + expected_content_sha256=content_sha256( + listed_paths, commit=commit, repo_root=repo_root + ), + ) + if errors: + diagnostics.append(f"{commit[:12]}: receipt is invalid: " + "; ".join(errors)) + continue + for path, digest in payload["content_sha256"].items(): + covered.setdefault(path, set()).add(digest) + return covered, diagnostics + + +def range_coverage_errors( + changed_paths: Sequence[str], + *, + base: str, + head: str, + repo_root: Path = REPO_ROOT, +) -> list[str]: + """Changed frontend paths whose final content no receipt in range verified. + + Integration merges reviewed commits onto a main that keeps moving, so the + range tip is usually a merge and the range carries several independently + verified changes. Binding one tip receipt to the tip's parent and to the + whole cumulative delta is invalid by construction there. What must hold is + that every user-visible frontend file ships with content a real browser + pass verified: its digest at head must equal the digest recorded by a valid + receipt from a non-merge commit in the range. + """ + covered, diagnostics = range_receipt_coverage(base, head, repo_root=repo_root) + final = content_sha256(changed_paths, commit=head, repo_root=repo_root) + uncovered = [path for path in changed_paths if final[path] not in covered.get(path, set())] + if not uncovered: + return [] + errors = [ + f"{path} final content {final[path][:12]} is not verified by any valid receipt in the range" + for path in uncovered + ] + errors.extend(diagnostics) + return errors + + def build_template(paths: Sequence[str], *, repo_root: Path = REPO_ROOT) -> dict: changed_paths = frontend_runtime_paths(paths) return { @@ -274,17 +378,38 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: "--commit", help="Validate the receipt stored in this commit against that commit's parent.", ) + parser.add_argument( + "--base", + help=( + "Validate the range base..--commit: every changed user-visible frontend " + "file's final content must match a valid receipt recorded by a non-merge " + "commit in the range, bound to that commit's own parent." + ), + ) parser.add_argument( "--print-template", action="store_true", help="Print a non-passing receipt template for the current staged frontend paths.", ) - return parser.parse_args(argv) + args = parser.parse_args(argv) + if args.base and not args.commit: + parser.error("--base requires --commit") + return args def main(argv: Sequence[str] | None = None) -> int: args = parse_args(argv) - paths = stdin_files(sys.stdin) if args.files_from_stdin else staged_files() + if args.files_from_stdin: + paths = stdin_files(sys.stdin) + elif args.base: + paths = stdin_files( + run_git( + ["diff", "--name-only", "--diff-filter=ACMRD", args.base, args.commit], + repo_root=REPO_ROOT, + ).splitlines() + ) + else: + paths = staged_files() if args.print_template: print(json.dumps(build_template(paths), indent=2)) @@ -292,7 +417,9 @@ def main(argv: Sequence[str] | None = None) -> int: changed_frontend_paths = frontend_runtime_paths(paths) if changed_frontend_paths: - reformatted = formatting_only_paths(changed_frontend_paths, commit=args.commit) + reformatted = formatting_only_paths( + changed_frontend_paths, commit=args.commit, base=args.base, repo_root=REPO_ROOT + ) if reformatted: print( f"Browser verification guard: {len(reformatted)} path(s) are prettier-only " @@ -306,6 +433,38 @@ def main(argv: Sequence[str] | None = None) -> int: print("Browser verification guard skipped (no user-visible frontend source changes).") return 0 + if args.base: + try: + errors = range_coverage_errors( + changed_frontend_paths, + base=args.base, + head=args.commit, + repo_root=REPO_ROOT, + ) + except subprocess.CalledProcessError as exc: + print(f"BLOCKED: unable to evaluate browser verification range: {exc}", file=sys.stderr) + return 1 + if errors: + print( + f"BLOCKED: browser verification does not cover {args.base[:12]}..{args.commit[:12]}:", + file=sys.stderr, + ) + for error in errors: + print(f" - {error}", file=sys.stderr) + print( + "Content changed after its browser pass (a correction, a conflict resolution, " + "or two changes to one file) needs a fresh receipt for the final content, " + "committed in its own non-merge commit and bound to that commit's parent.", + file=sys.stderr, + ) + return 1 + print( + "Browser verification guard passed " + f"({len(changed_frontend_paths)} frontend source file(s) across " + f"{args.base[:12]}..{args.commit[:12]} covered by in-range receipts)." + ) + return 0 + if RECEIPT_PATH not in paths: print( f"BLOCKED: {RECEIPT_PATH} must be updated and staged for user-visible frontend changes.", diff --git a/scripts/release_control/browser_verification_guard_test.py b/scripts/release_control/browser_verification_guard_test.py index 483ec17565..fde2d6a284 100644 --- a/scripts/release_control/browser_verification_guard_test.py +++ b/scripts/release_control/browser_verification_guard_test.py @@ -2,7 +2,9 @@ from __future__ import annotations +import hashlib from io import StringIO +import json import os from pathlib import Path import subprocess @@ -192,5 +194,152 @@ class FormattingOnlyExemptionTest(unittest.TestCase): self.assertEqual(self.resolve(repo_root), set()) +OTHER_PATH = "frontend-modern/src/components/Other.tsx" + + +class IntegrationRangeTest(unittest.TestCase): + """--base validates a merged integration range, not one tip receipt. + + The maintainer coordinator merges reviewed candidates onto a main that + keeps moving, so the range tip is a merge whose parent no lane receipt can + name. Each frontend file must still ship with content a browser pass + verified, recorded by a non-merge commit bound to its own parent. + """ + + def setUp(self) -> None: + self.tmpdir = tempfile.TemporaryDirectory() + self.addCleanup(self.tmpdir.cleanup) + self.repo_root = Path(self.tmpdir.name) + self.env = strip_local_git_env(os.environ.copy()) + self.env["PULSE_PRETTIER_BIN"] = str(self.repo_root / "missing-prettier") + self.git("init", "--quiet", "--initial-branch=main") + self.write(CHANGED_PATH, "export const a = 0;\n") + self.write(OTHER_PATH, "export const b = 0;\n") + self.base = self.commit("seed") + + def git(self, *args: str) -> str: + return subprocess.run( + ["git", "-c", "user.email=t@example.com", "-c", "user.name=t", *args], + cwd=self.repo_root, + check=True, + capture_output=True, + text=True, + env=self.env, + ).stdout.strip() + + def write(self, path: str, text: str) -> None: + target = self.repo_root / path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(text, encoding="utf-8") + + def commit(self, message: str) -> str: + self.git("add", "-A") + self.git("commit", "--quiet", "--allow-empty", "-m", message) + return self.git("rev-parse", "HEAD") + + def write_receipt(self, paths: list[str], *, base: str | None = None) -> None: + receipt = valid_receipt() + receipt["base_sha"] = base or self.git("rev-parse", "HEAD") + receipt["changed_paths"] = paths + receipt["content_sha256"] = { + path: hashlib.sha256((self.repo_root / path).read_bytes()).hexdigest() + for path in paths + } + self.write(RECEIPT_PATH, json.dumps(receipt, indent=2) + "\n") + + def verified_change(self, path: str, text: str, message: str) -> str: + self.write(path, text) + self.write_receipt([path]) + return self.commit(message) + + def run_range(self, *extra: str) -> tuple[int, str]: + stderr = StringIO() + with ( + patch.dict("os.environ", self.env, clear=True), + patch("browser_verification_guard.REPO_ROOT", self.repo_root), + patch("sys.stdout", new=StringIO()), + patch("sys.stderr", new=stderr), + ): + status = main(["--base", self.base, "--commit", self.git("rev-parse", "HEAD"), *extra]) + return status, stderr.getvalue() + + def merge_advanced_main_into_candidate(self) -> None: + # main advances with its own verified frontend change while the + # candidate is verified against the older base, then the coordinator + # merges the candidate onto the newer main. + self.git("checkout", "--quiet", "-b", "candidate", self.base) + self.verified_change(CHANGED_PATH, "export const a = 1;\n", "candidate") + self.git("checkout", "--quiet", "main") + self.verified_change(OTHER_PATH, "export const b = 1;\n", "upstream") + # Both sides edited the shared receipt; the resolution keeps one side. + subprocess.run( + ["git", "-c", "user.email=t@example.com", "-c", "user.name=t", + "merge", "--quiet", "--no-ff", "candidate", "-m", "integrate"], + cwd=self.repo_root, capture_output=True, env=self.env, + ) + self.git("checkout", "--quiet", "--theirs", RECEIPT_PATH) + self.commit("integrate candidate") + self.assertEqual(len(self.git("log", "-1", "--format=%P").split()), 2) + + def test_accepts_merge_of_verified_candidate_onto_advanced_main(self) -> None: + self.merge_advanced_main_into_candidate() + self.assertEqual(self.run_range(), (0, "")) + + def test_accepts_receipt_recorded_in_a_later_commit(self) -> None: + self.write(CHANGED_PATH, "export const a = 1;\n") + self.commit("frontend change") + self.write_receipt([CHANGED_PATH]) + self.commit("record proof") + self.assertEqual(self.run_range()[0], 0) + + def test_blocks_frontend_edit_after_merge_without_fresh_proof(self) -> None: + self.merge_advanced_main_into_candidate() + self.write(CHANGED_PATH, "export const a = 2;\n") + self.commit("coordinator correction") + + status, stderr = self.run_range() + + self.assertEqual(status, 1) + self.assertIn(CHANGED_PATH, stderr) + self.assertNotIn(OTHER_PATH, stderr) + + def test_merge_resolution_receipt_is_not_evidence(self) -> None: + # A conflict resolution that changes source and rewrites the receipt + # inside the merge commit has no browser run behind it. + self.git("checkout", "--quiet", "-b", "candidate", self.base) + self.verified_change(CHANGED_PATH, "export const a = 1;\n", "candidate") + self.git("checkout", "--quiet", "main") + self.git("merge", "--quiet", "--no-ff", "--no-commit", "candidate") + self.write(CHANGED_PATH, "export const a = 3;\n") + self.write_receipt([CHANGED_PATH], base=self.base) + self.commit("integrate with resolution") + self.assertEqual(len(self.git("log", "-1", "--format=%P").split()), 2) + + status, stderr = self.run_range() + + self.assertEqual(status, 1) + self.assertIn(CHANGED_PATH, stderr) + + def test_receipt_bound_to_another_parent_is_rejected(self) -> None: + self.write(CHANGED_PATH, "export const a = 1;\n") + self.write_receipt([CHANGED_PATH], base="b" * 40) + self.commit("stale proof") + + status, stderr = self.run_range() + + self.assertEqual(status, 1) + self.assertIn("base_sha must match", stderr) + + def test_range_paths_can_come_from_stdin(self) -> None: + self.merge_advanced_main_into_candidate() + changed = self.git("diff", "--name-only", "--diff-filter=ACMRD", self.base, "HEAD") + with patch("sys.stdin", StringIO(changed + "\n")): + self.assertEqual(self.run_range("--files-from-stdin"), (0, "")) + + def test_base_requires_commit(self) -> None: + with patch("sys.stderr", new=StringIO()), self.assertRaises(SystemExit): + main(["--base", self.base]) + + if __name__ == "__main__": unittest.main()