mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-23 20:25:56 +00:00
Validate merged integration ranges against in-range browser receipts (#2330)
The maintainer coordinator merges reviewed candidates onto a main that keeps moving and then ran the browser verification guard once over the whole range with the merge tip as the commit. No lane can produce a receipt bound to that merge's parent and covering upstream frontend work too, so 13 of 13 coordinator browser-guard rollbacks since 28 September failed on base_sha. This adds a range mode where every changed frontend file's final content must match a valid receipt from a non-merge commit inside the range, each validated against its own commit. Merge-authored receipts never count, and the per-commit and staged modes are unchanged.
This commit is contained in:
@@ -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 <range-base> --commit <range-tip>`. 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
|
||||
|
||||
@@ -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.",
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user