From 4e67b8117d28af2368fb6eacab1e6e4b68b15c1a Mon Sep 17 00:00:00 2001 From: rcourtman Date: Thu, 6 Aug 2026 21:46:18 +0100 Subject: [PATCH] fix(governance): exempt pure prettier reformats from browser proof A `make format` sweep re-lays-out already-committed files without changing a single token, so it cannot change what renders. The browser verification guard still demanded a fresh receipt for it, which would mean recording routes, viewports, states and interactions nobody exercised in order to describe a diff with no visual delta. A guard that can only be satisfied by an untrue receipt teaches people to write untrue receipts. Exempt a path only when its new content is byte-identical to prettier's output for its committed content. That is provable, not a judgement call: if the two match, the sole difference from HEAD is layout. Fails closed everywhere else -- added or deleted files, unreadable blobs, prettier missing, or any non-identical output all fall through and still require the receipt. A reformat that also changes a value is covered by a test and still blocks. --- .../browser_verification_guard.py | 69 +++++++++++++++++++ .../browser_verification_guard_test.py | 67 ++++++++++++++++++ 2 files changed, 136 insertions(+) diff --git a/scripts/release_control/browser_verification_guard.py b/scripts/release_control/browser_verification_guard.py index 152ed8f6b..9b0ad13cc 100644 --- a/scripts/release_control/browser_verification_guard.py +++ b/scripts/release_control/browser_verification_guard.py @@ -12,6 +12,8 @@ import subprocess import sys from typing import Iterable, Sequence +import format_staged_frontend + REPO_ROOT = Path(__file__).resolve().parents[2] RECEIPT_PATH = "frontend-modern/browser-verification.json" @@ -79,6 +81,62 @@ def frontend_runtime_paths(paths: Iterable[str]) -> list[str]: return sorted({path for path in paths if is_user_visible_frontend_source(path)}) +def prettier_format( + prettier: Path, + path: str, + content: bytes, + *, + repo_root: Path = REPO_ROOT, +) -> bytes | None: + # --stdin-filepath drives parser selection and config resolution, so this + # matches what `prettier --write ` would produce. + try: + result = subprocess.run( + [str(prettier), "--stdin-filepath", str(repo_root / path)], + cwd=repo_root / "frontend-modern", + check=True, + capture_output=True, + input=content, + ) + except (subprocess.CalledProcessError, OSError): + return None + return result.stdout + + +def formatting_only_paths( + paths: Sequence[str], + *, + commit: str | None, + repo_root: Path = REPO_ROOT, +) -> set[str]: + """Paths whose new content is exactly prettier's output for the old content. + + A `make format` sweep re-lays-out already-committed files without changing + a single token, so it cannot change what renders. Demanding fresh browser + proof for that would mean recording a receipt describing an interaction + matrix nobody exercised, for a diff with no visual delta. + + 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. + """ + prettier = format_staged_frontend.prettier_bin() + if prettier is None: + return set() + + base_revision = 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}" + new_content = git_blob_bytes(new_object, repo_root=repo_root) + old_content = git_blob_bytes(f"{base_revision}:{path}", repo_root=repo_root) + if new_content is None or old_content is None or new_content == old_content: + continue + if prettier_format(prettier, path, old_content, repo_root=repo_root) == new_content: + formatting_only.add(path) + return formatting_only + + def load_receipt_text( *, commit: str | None, @@ -233,6 +291,17 @@ def main(argv: Sequence[str] | None = None) -> int: return 0 changed_frontend_paths = frontend_runtime_paths(paths) + if changed_frontend_paths: + reformatted = formatting_only_paths(changed_frontend_paths, commit=args.commit) + if reformatted: + print( + f"Browser verification guard: {len(reformatted)} path(s) are prettier-only " + "reformats of their committed content, with no visual delta to verify." + ) + changed_frontend_paths = [ + path for path in changed_frontend_paths if path not in reformatted + ] + if not changed_frontend_paths: print("Browser verification guard skipped (no user-visible frontend source changes).") return 0 diff --git a/scripts/release_control/browser_verification_guard_test.py b/scripts/release_control/browser_verification_guard_test.py index 9c2bbd7bb..1833e9714 100644 --- a/scripts/release_control/browser_verification_guard_test.py +++ b/scripts/release_control/browser_verification_guard_test.py @@ -3,15 +3,22 @@ from __future__ import annotations from io import StringIO +import os +from pathlib import Path +import subprocess +import tempfile import unittest from unittest.mock import patch from browser_verification_guard import ( RECEIPT_PATH, + formatting_only_paths, frontend_runtime_paths, main, validate_receipt, ) +from format_staged_frontend_test import REAL_PRETTIER +from repo_file_io import strip_local_git_env BASE_SHA = "a" * 40 @@ -116,5 +123,65 @@ class BrowserVerificationGuardTest(unittest.TestCase): self.assertTrue(any("content_sha256" in error for error in errors)) +class FormattingOnlyExemptionTest(unittest.TestCase): + """A prettier sweep has no visual delta, but a real edit must still block.""" + + def build_repo(self, tmpdir: str, old: str, new: str) -> Path: + repo_root = Path(tmpdir) + source = repo_root / CHANGED_PATH + source.parent.mkdir(parents=True) + source.write_text(old, encoding="utf-8") + env = strip_local_git_env(os.environ.copy()) + + def git(*args: str) -> None: + subprocess.run( + ["git", *args], cwd=repo_root, check=True, capture_output=True, env=env + ) + + git("init") + git("add", CHANGED_PATH) + git("-c", "user.email=t@example.com", "-c", "user.name=t", "commit", "-m", "seed") + source.write_text(new, encoding="utf-8") + git("add", CHANGED_PATH) + return repo_root + + def resolve(self, repo_root: Path) -> set[str]: + # Under the pre-commit hook, GIT_DIR and GIT_INDEX_FILE are exported + # and would point this temp repo's plumbing at the real repository. + with patch.dict("os.environ", strip_local_git_env(os.environ.copy()), clear=True): + with patch("browser_verification_guard.REPO_ROOT", repo_root): + return formatting_only_paths([CHANGED_PATH], commit=None, repo_root=repo_root) + + @unittest.skipUnless(REAL_PRETTIER.exists(), "prettier not installed under frontend-modern") + def test_exempts_a_pure_reformat(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + repo_root = self.build_repo(tmpdir, "const x = {a:1}\n", "const x = { a: 1 };\n") + with patch.dict( + "os.environ", {"PULSE_PRETTIER_BIN": str(REAL_PRETTIER)}, clear=False + ): + self.assertEqual(self.resolve(repo_root), {CHANGED_PATH}) + + @unittest.skipUnless(REAL_PRETTIER.exists(), "prettier not installed under frontend-modern") + def test_still_requires_proof_when_a_reformat_also_changes_a_value(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + # Formatted exactly as prettier would, but 1 became 2. The guard + # must not treat a semantic edit as a cosmetic one. + repo_root = self.build_repo(tmpdir, "const x = {a:1}\n", "const x = { a: 2 };\n") + with patch.dict( + "os.environ", {"PULSE_PRETTIER_BIN": str(REAL_PRETTIER)}, clear=False + ): + self.assertEqual(self.resolve(repo_root), set()) + + def test_fails_closed_when_prettier_is_unavailable(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + repo_root = self.build_repo(tmpdir, "const x = {a:1}\n", "const x = { a: 1 };\n") + with patch.dict( + "os.environ", + {"PULSE_PRETTIER_BIN": str(repo_root / "missing-prettier")}, + clear=False, + ): + self.assertEqual(self.resolve(repo_root), set()) + + if __name__ == "__main__": unittest.main()