mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
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.
This commit is contained in:
@@ -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 <path>` 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
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user