From c620e5859fe8b839a432c189160ee85e45aa3da2 Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 4 Sep 2026 21:04:39 -0400 Subject: [PATCH 1/4] =?UTF-8?q?test:=20red=20=E2=80=94=20assert=20a=20PR?= =?UTF-8?q?=20cannot=20delete=20an=20e2e=20attestation=20receipt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the failing unit, integration and e2e tests for a new `dirsql-checks attestation-guard` check, against inert stubs, plus the workflow that will invoke it. The `e2e-verify` gate is diff-based and asks only whether a branch adds or updates ITS OWN receipt, so a diff that deletes other branches' receipts passes it green. Receipts are append-only and nothing can regenerate a merged branch's, so the rule asserted here is total: any deletion under an `e2e-attestations/` directory fails, with an `allow-receipt-deletion: ` commit-body bypass mirroring changelog-gate's `skip-changelog:`. 30 tests fail against the stubs; the implementation lands next. --- .github/workflows/attestation-guard.yml | 53 +++++++ .../src/checks/attestation_guard/__init__.py | 0 .../src/checks/attestation_guard/cli.py | 19 +++ .../src/checks/attestation_guard/cli_test.py | 34 +++++ .../src/checks/attestation_guard/decide.py | 18 +++ .../checks/attestation_guard/decide_test.py | 91 ++++++++++++ .../src/checks/attestation_guard/gate.py | 23 +++ .../src/checks/attestation_guard/gate_test.py | 86 ++++++++++++ .../src/checks/attestation_guard/git_ops.py | 35 +++++ .../checks/attestation_guard/git_ops_test.py | 36 +++++ .../src/checks/attestation_guard/report.py | 12 ++ .../checks/attestation_guard/report_test.py | 47 +++++++ internals/checks/src/checks/cli.py | 2 + internals/checks/src/checks/cli_test.py | 4 + .../tests/e2e/attestation_guard_test.py | 105 ++++++++++++++ .../integration/attestation_guard_test.py | 131 ++++++++++++++++++ 16 files changed, 696 insertions(+) create mode 100644 .github/workflows/attestation-guard.yml create mode 100644 internals/checks/src/checks/attestation_guard/__init__.py create mode 100644 internals/checks/src/checks/attestation_guard/cli.py create mode 100644 internals/checks/src/checks/attestation_guard/cli_test.py create mode 100644 internals/checks/src/checks/attestation_guard/decide.py create mode 100644 internals/checks/src/checks/attestation_guard/decide_test.py create mode 100644 internals/checks/src/checks/attestation_guard/gate.py create mode 100644 internals/checks/src/checks/attestation_guard/gate_test.py create mode 100644 internals/checks/src/checks/attestation_guard/git_ops.py create mode 100644 internals/checks/src/checks/attestation_guard/git_ops_test.py create mode 100644 internals/checks/src/checks/attestation_guard/report.py create mode 100644 internals/checks/src/checks/attestation_guard/report_test.py create mode 100644 internals/checks/tests/e2e/attestation_guard_test.py create mode 100644 internals/checks/tests/integration/attestation_guard_test.py diff --git a/.github/workflows/attestation-guard.yml b/.github/workflows/attestation-guard.yml new file mode 100644 index 000000000..175b06be4 --- /dev/null +++ b/.github/workflows/attestation-guard.yml @@ -0,0 +1,53 @@ +name: Attestation Guard + +env: + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + +# E2E attestation receipts are append-only: each one records that a branch's +# e2e suite ran, and nothing can regenerate a merged branch's receipt. The +# `e2e-verify` gate only asks whether a branch adds or updates ITS OWN receipt, +# so a diff that deletes fifteen other branches' receipts passes it green +# (#1036 merged exactly that, 28 receipts, fully green). This lane closes that +# hole (#1041, #1043). +# +# Deliberately its own workflow rather than a job in an existing lane: receipts +# live under packages/, plugins/ and internals/, so no single package lane sees +# them all, and changelog-check.yml explicitly filters e2e-attestations out. +# The `**` path filter is the point -- a deletion can ride in any diff, and the +# only reliable trigger is the receipt path itself. +on: + pull_request: + paths: + - '**/e2e-attestations/**' + - 'internals/checks/src/checks/attestation_guard/**' + - '.github/workflows/attestation-guard.yml' + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + attestation-guard: + name: Attestation guard + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + + - name: Install uv + uses: astral-sh/setup-uv@v7 + with: + python-version: "3.13" + + # Logic (receipt-path detection, bypass-line scanning, the restore + # instructions) lives in internals/checks' attestation_guard check, + # colocated-tested at 100% coverage, per AGENTS.md "CI Workflows". + - name: Fail on any deleted e2e attestation receipt + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: uv run --project internals/checks dirsql-checks attestation-guard diff --git a/internals/checks/src/checks/attestation_guard/__init__.py b/internals/checks/src/checks/attestation_guard/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/internals/checks/src/checks/attestation_guard/cli.py b/internals/checks/src/checks/attestation_guard/cli.py new file mode 100644 index 000000000..3558bc49e --- /dev/null +++ b/internals/checks/src/checks/attestation_guard/cli.py @@ -0,0 +1,19 @@ +"""The attestation-guard check -- repo-only (#1043). + +Backs the `dirsql-checks attestation-guard` subcommand: fails a PR that deletes +an e2e attestation receipt. Reads BASE_SHA/HEAD_SHA from the environment (set by +`.github/workflows/attestation-guard.yml`) so the workflow step stays a one-liner. +""" + +from __future__ import annotations + +import click + +from checks.attestation_guard.gate import run + + +@click.command() +@click.option("--base-sha", envvar="BASE_SHA", required=True) +@click.option("--head-sha", envvar="HEAD_SHA", required=True) +def cli(base_sha: str, head_sha: str) -> None: + raise SystemExit(run(base_sha, head_sha)) diff --git a/internals/checks/src/checks/attestation_guard/cli_test.py b/internals/checks/src/checks/attestation_guard/cli_test.py new file mode 100644 index 000000000..1b2a23848 --- /dev/null +++ b/internals/checks/src/checks/attestation_guard/cli_test.py @@ -0,0 +1,34 @@ +"""Colocated unit tests for the attestation-guard command (isolation -- no `CliRunner`). + +Driven through `.callback`; `run` is mocked at its import site in this module. +""" + +from unittest import mock + +import pytest + +from checks.attestation_guard.cli import cli + + +def test_exits_with_runs_return_code(): + with mock.patch("checks.attestation_guard.cli.run", return_value=0) as run: + with pytest.raises(SystemExit) as exc_info: + cli.callback(base_sha="abc", head_sha="def") + run.assert_called_once_with("abc", "def") + assert exc_info.value.code == 0 + + +def test_propagates_a_nonzero_return_code(): + with mock.patch("checks.attestation_guard.cli.run", return_value=1) as run: + with pytest.raises(SystemExit) as exc_info: + cli.callback(base_sha="abc", head_sha="def") + run.assert_called_once_with("abc", "def") + assert exc_info.value.code == 1 + + +def test_declares_base_sha_and_head_sha_options_reading_the_matching_envvars(): + by_name = {option.name: option for option in cli.params} + assert by_name["base_sha"].envvar == "BASE_SHA" + assert by_name["base_sha"].required is True + assert by_name["head_sha"].envvar == "HEAD_SHA" + assert by_name["head_sha"].required is True diff --git a/internals/checks/src/checks/attestation_guard/decide.py b/internals/checks/src/checks/attestation_guard/decide.py new file mode 100644 index 000000000..213d5c9a9 --- /dev/null +++ b/internals/checks/src/checks/attestation_guard/decide.py @@ -0,0 +1,18 @@ +"""Pure decision logic for the attestation-guard check (#1043).""" + +from __future__ import annotations + + +def deleted_receipts(deleted) -> list[str]: + """Sorted paths the diff deletes from an ``e2e-attestations/`` directory.""" + return [] + + +def has_allow_line(commit_messages: str) -> bool: + """True if a commit body carries ``allow-receipt-deletion: ``.""" + return False + + +def near_miss_lines(commit_messages: str) -> list[str]: + """Bypass-shaped commit lines that are not the accepted spelling.""" + return [] diff --git a/internals/checks/src/checks/attestation_guard/decide_test.py b/internals/checks/src/checks/attestation_guard/decide_test.py new file mode 100644 index 000000000..e30ba5f14 --- /dev/null +++ b/internals/checks/src/checks/attestation_guard/decide_test.py @@ -0,0 +1,91 @@ +from checks.attestation_guard.decide import ( + deleted_receipts, + has_allow_line, + near_miss_lines, +) + + +def describe_deleted_receipts(): + def keeps_only_paths_under_an_e2e_attestations_directory(): + assert deleted_receipts( + [ + "packages/python/e2e-attestations/claude-a.json", + "packages/python/dirsql/core.py", + "README.md", + ] + ) == ["packages/python/e2e-attestations/claude-a.json"] + + def spans_every_package_root_that_holds_receipts(): + paths = [ + "internals/checks/e2e-attestations/a.json", + "plugins/dirsql-plugin-embeddings/e2e-attestations/b.json", + "packages/ts/e2e-attestations/c.json", + ] + assert deleted_receipts(paths) == sorted(paths) + + def sorts_the_result(): + assert deleted_receipts( + ["packages/ts/e2e-attestations/b.json", "packages/ts/e2e-attestations/a.json"] + ) == [ + "packages/ts/e2e-attestations/a.json", + "packages/ts/e2e-attestations/b.json", + ] + + def ignores_the_directory_entry_itself(): + assert deleted_receipts(["packages/ts/e2e-attestations/"]) == [] + + def ignores_a_lookalike_directory_name(): + assert deleted_receipts(["packages/ts/e2e-attestations-old/a.json"]) == [] + + def returns_nothing_for_an_empty_diff(): + assert deleted_receipts([]) == [] + + +def describe_has_allow_line(): + def accepts_the_bypass_with_a_reason(): + assert has_allow_line("chore: x\n\nallow-receipt-deletion: retiring pkg") is True + + def accepts_it_regardless_of_case(): + assert has_allow_line("Allow-Receipt-Deletion: retiring pkg") is True + + def accepts_it_from_any_line_not_only_a_formal_trailer(): + body = "allow-receipt-deletion: retiring pkg\n\nchore: x\n" + assert has_allow_line(body) is True + + def rejects_the_bypass_with_no_reason(): + assert has_allow_line("chore: x\n\nallow-receipt-deletion:") is False + + def rejects_the_bypass_with_only_whitespace_for_a_reason(): + assert has_allow_line("chore: x\n\nallow-receipt-deletion: ") is False + + def rejects_an_ordinary_message(): + assert has_allow_line("fix: delete a stale thing\n") is False + + +def describe_near_miss_lines(): + def names_a_bypass_spelled_with_the_wrong_verb(): + assert near_miss_lines("skip-receipt-deletion: cleanup") == [ + "skip-receipt-deletion: cleanup" + ] + + def names_a_bypass_spelled_with_the_wrong_noun(): + assert near_miss_lines("allow-attestation-deletion: cleanup") == [ + "allow-attestation-deletion: cleanup" + ] + + def names_a_bypass_missing_its_reason(): + assert near_miss_lines("allow-receipt-deletion:") == ["allow-receipt-deletion:"] + + def names_a_bypass_missing_its_colon(): + assert near_miss_lines("allow-receipt-deletion cleanup") == [ + "allow-receipt-deletion cleanup" + ] + + def does_not_name_the_accepted_spelling(): + assert near_miss_lines("allow-receipt-deletion: retiring pkg") == [] + + def strips_surrounding_whitespace(): + assert near_miss_lines(" skip-receipt-deletion: ") == ["skip-receipt-deletion:"] + + def returns_nothing_for_an_ordinary_message(): + assert near_miss_lines("fix: a thing\n") == [] diff --git a/internals/checks/src/checks/attestation_guard/gate.py b/internals/checks/src/checks/attestation_guard/gate.py new file mode 100644 index 000000000..90a72bbd1 --- /dev/null +++ b/internals/checks/src/checks/attestation_guard/gate.py @@ -0,0 +1,23 @@ +"""Orchestration for the attestation-guard check (#1043).""" + +from __future__ import annotations + +from checks.attestation_guard import git_ops +from checks.attestation_guard.decide import deleted_receipts + + +def verdict(deleted, messages: str, base_sha: str) -> int: + """0 when a well-formed bypass line is present, else 1 with diagnostics.""" + return 0 + + +def run( + base_sha: str, + head_sha: str, + *, + deleted_files=git_ops.deleted_files, + commit_messages=git_ops.commit_messages, +) -> int: + deleted = deleted_receipts(deleted_files(base_sha, head_sha)) + print("No e2e attestation receipts deleted.") + return 0 diff --git a/internals/checks/src/checks/attestation_guard/gate_test.py b/internals/checks/src/checks/attestation_guard/gate_test.py new file mode 100644 index 000000000..015cde90c --- /dev/null +++ b/internals/checks/src/checks/attestation_guard/gate_test.py @@ -0,0 +1,86 @@ +from unittest import mock + +from checks.attestation_guard.gate import run, verdict + + +def _run(deleted, *, messages="fix: a thing\n"): + return run( + "base", + "head", + deleted_files=mock.Mock(return_value=deleted), + commit_messages=mock.Mock(return_value=messages), + ) + + +def describe_run(): + def passes_when_the_diff_deletes_nothing(capsys): + assert _run([]) == 0 + assert "No e2e attestation receipts deleted." in capsys.readouterr().out + + def passes_when_the_diff_deletes_no_receipt(capsys): + assert _run(["packages/python/dirsql/core.py"]) == 0 + assert "No e2e attestation receipts deleted." in capsys.readouterr().out + + def does_not_read_commit_messages_when_nothing_was_deleted(): + messages = mock.Mock() + run( + "base", + "head", + deleted_files=mock.Mock(return_value=[]), + commit_messages=messages, + ) + messages.assert_not_called() + + def fails_when_the_diff_deletes_a_receipt(capsys): + assert _run(["packages/ts/e2e-attestations/other-branch.json"]) == 1 + assert "other-branch.json is an e2e attestation receipt" in capsys.readouterr().out + + def names_the_restore_command_with_the_base_sha(capsys): + assert _run(["packages/ts/e2e-attestations/a.json"]) == 1 + out = capsys.readouterr().out + assert "git checkout base -- packages/ts/e2e-attestations/a.json" in out + + def passes_when_a_bypass_line_is_present(capsys): + rc = _run( + ["packages/ts/e2e-attestations/a.json"], + messages="chore: retire pkg\n\nallow-receipt-deletion: package removed", + ) + assert rc == 0 + assert "permitting receipt deletion" in capsys.readouterr().out + + def fails_and_names_a_near_miss_bypass(capsys): + rc = _run( + ["packages/ts/e2e-attestations/a.json"], + messages="chore: retire pkg\n\nskip-receipt-deletion: package removed", + ) + assert rc == 1 + assert "is not the bypass line" in capsys.readouterr().out + + def queries_git_with_the_supplied_shas(): + deleted = mock.Mock(return_value=[]) + run("b1", "h1", deleted_files=deleted, commit_messages=mock.Mock()) + deleted.assert_called_once_with("b1", "h1") + + def reads_commit_messages_over_the_same_range(): + messages = mock.Mock(return_value="fix: x\n") + run( + "b1", + "h1", + deleted_files=mock.Mock(return_value=["a/e2e-attestations/x.json"]), + commit_messages=messages, + ) + messages.assert_called_once_with("b1", "h1") + + +def describe_verdict(): + def reports_every_deleted_receipt(capsys): + rc = verdict( + ["a/e2e-attestations/x.json", "b/e2e-attestations/y.json"], "fix: x", "sha" + ) + assert rc == 1 + assert capsys.readouterr().out.count("::error file=") == 2 + + def does_not_report_when_bypassed(capsys): + rc = verdict(["a/e2e-attestations/x.json"], "allow-receipt-deletion: why", "sha") + assert rc == 0 + assert "::error file=" not in capsys.readouterr().out diff --git a/internals/checks/src/checks/attestation_guard/git_ops.py b/internals/checks/src/checks/attestation_guard/git_ops.py new file mode 100644 index 000000000..d453de6cd --- /dev/null +++ b/internals/checks/src/checks/attestation_guard/git_ops.py @@ -0,0 +1,35 @@ +"""Thin git subprocess wrappers for the attestation-guard check (#1043). + +Each shells out for exactly one piece of data, so the decision logic in +`decide.py` stays subprocess-free and unit-testable. +""" + +from __future__ import annotations + +import subprocess + + +def deleted_files(base_sha: str, head_sha: str, runner=subprocess.run) -> list[str]: + # `--no-renames` so a receipt moved to another path registers as a deletion + # of the original: a receipt's path is its branch slug, so renaming one is + # as much a loss of the record as removing it. + result = runner( + ["git", "diff", "--name-only", "--diff-filter=D", "--no-renames", + f"{base_sha}...{head_sha}"], + capture_output=True, + text=True, + check=True, + ) + return [line for line in result.stdout.splitlines() if line] + + +def commit_messages(base_sha: str, head_sha: str, runner=subprocess.run) -> str: + # Raw bodies (`%B`), scanned for the bypass line without git's trailer + # parser, so the bypass works from any line of any commit in the range. + result = runner( + ["git", "log", "--format=%B", f"{base_sha}..{head_sha}"], + capture_output=True, + text=True, + check=True, + ) + return result.stdout diff --git a/internals/checks/src/checks/attestation_guard/git_ops_test.py b/internals/checks/src/checks/attestation_guard/git_ops_test.py new file mode 100644 index 000000000..b621e990e --- /dev/null +++ b/internals/checks/src/checks/attestation_guard/git_ops_test.py @@ -0,0 +1,36 @@ +from unittest import mock + +from checks.attestation_guard.git_ops import commit_messages, deleted_files + + +def describe_deleted_files(): + def asks_git_for_deletions_only_over_the_merge_base_range(): + runner = mock.Mock(return_value=mock.Mock(stdout="a.json\n")) + assert deleted_files("base", "head", runner=runner) == ["a.json"] + runner.assert_called_once_with( + ["git", "diff", "--name-only", "--diff-filter=D", "--no-renames", + "base...head"], + capture_output=True, + text=True, + check=True, + ) + + def parses_stdout_into_lines(): + runner = mock.Mock(return_value=mock.Mock(stdout="a.json\nb.json\n")) + assert deleted_files("base", "head", runner=runner) == ["a.json", "b.json"] + + def drops_blank_lines(): + runner = mock.Mock(return_value=mock.Mock(stdout="a.json\n\n")) + assert deleted_files("base", "head", runner=runner) == ["a.json"] + + +def describe_commit_messages(): + def returns_raw_bodies_via_the_two_dot_range(): + runner = mock.Mock(return_value=mock.Mock(stdout="fix: x\n")) + assert commit_messages("base", "head", runner=runner) == "fix: x\n" + runner.assert_called_once_with( + ["git", "log", "--format=%B", "base..head"], + capture_output=True, + text=True, + check=True, + ) diff --git a/internals/checks/src/checks/attestation_guard/report.py b/internals/checks/src/checks/attestation_guard/report.py new file mode 100644 index 000000000..dd2141de1 --- /dev/null +++ b/internals/checks/src/checks/attestation_guard/report.py @@ -0,0 +1,12 @@ +"""Failure output for the attestation-guard check (#1043).""" + +from __future__ import annotations + + +def restore_command(deleted, base_sha: str) -> str: + """The exact `git checkout` that puts every deleted receipt back.""" + return "" + + +def report(deleted, near_misses, base_sha: str) -> None: + """Print one annotation per deleted receipt, then how to undo them.""" diff --git a/internals/checks/src/checks/attestation_guard/report_test.py b/internals/checks/src/checks/attestation_guard/report_test.py new file mode 100644 index 000000000..548dc1785 --- /dev/null +++ b/internals/checks/src/checks/attestation_guard/report_test.py @@ -0,0 +1,47 @@ +from checks.attestation_guard.report import report, restore_command + + +def describe_restore_command(): + def names_the_base_sha_and_every_path(): + assert restore_command(["a.json", "b.json"], "abc123") == ( + "git checkout abc123 -- a.json b.json" + ) + + +def describe_report(): + def annotates_every_deleted_receipt_with_its_own_file(capsys): + report(["packages/ts/e2e-attestations/a.json"], [], "abc123") + out = capsys.readouterr().out + assert "::error file=packages/ts/e2e-attestations/a.json::" in out + assert "append-only" in out + + def prints_one_annotation_per_deleted_receipt(capsys): + report(["a/e2e-attestations/x.json", "b/e2e-attestations/y.json"], [], "abc") + out = capsys.readouterr().out + assert out.count("::error file=") == 2 + + def prints_the_exact_restore_command(capsys): + report(["a/e2e-attestations/x.json"], [], "abc123") + assert "git checkout abc123 -- a/e2e-attestations/x.json" in capsys.readouterr().out + + def counts_the_deleted_receipts(capsys): + report(["a/e2e-attestations/x.json", "a/e2e-attestations/y.json"], [], "abc") + assert "Restore 2 deleted receipt(s)" in capsys.readouterr().out + + def names_the_bypass_line_in_the_closing_advice(capsys): + report(["a/e2e-attestations/x.json"], [], "abc") + assert "allow-receipt-deletion: " in capsys.readouterr().out + + def quotes_a_near_miss_and_gives_the_exact_form(capsys): + report(["a/e2e-attestations/x.json"], ["skip-receipt-deletion: y"], "abc") + out = capsys.readouterr().out + assert "'skip-receipt-deletion: y' is not the bypass line" in out + assert "reason required" in out + + def prints_one_annotation_per_near_miss(capsys): + report(["a/e2e-attestations/x.json"], ["one:", "two:"], "abc") + assert capsys.readouterr().out.count("is not the bypass line") == 2 + + def prints_no_near_miss_annotation_when_there_is_none(capsys): + report(["a/e2e-attestations/x.json"], [], "abc") + assert "is not the bypass line" not in capsys.readouterr().out diff --git a/internals/checks/src/checks/cli.py b/internals/checks/src/checks/cli.py index 89ee15221..6475e34c0 100644 --- a/internals/checks/src/checks/cli.py +++ b/internals/checks/src/checks/cli.py @@ -9,6 +9,7 @@ import click from checks.artifact_completeness.cli import cli as artifact_completeness +from checks.attestation_guard.cli import cli as attestation_guard from checks.changelog_gate.cli import cli as changelog_gate from checks.declared_deps.cli import cli as declared_deps from checks.npm_binary_extension_load.cli import cli as npm_binary_extension_load @@ -25,6 +26,7 @@ def main() -> None: main.add_command(artifact_completeness, name="artifact-completeness") +main.add_command(attestation_guard, name="attestation-guard") main.add_command(changelog_gate, name="changelog-gate") main.add_command(declared_deps, name="declared-deps") main.add_command(npm_binary_extension_load, name="npm-binary-extension-load") diff --git a/internals/checks/src/checks/cli_test.py b/internals/checks/src/checks/cli_test.py index 6d73e99e9..5f7266602 100644 --- a/internals/checks/src/checks/cli_test.py +++ b/internals/checks/src/checks/cli_test.py @@ -10,6 +10,10 @@ def test_artifact_completeness_is_registered(): assert "artifact-completeness" in main.commands +def test_attestation_guard_is_registered(): + assert "attestation-guard" in main.commands + + def test_changelog_gate_is_registered(): assert "changelog-gate" in main.commands diff --git a/internals/checks/tests/e2e/attestation_guard_test.py b/internals/checks/tests/e2e/attestation_guard_test.py new file mode 100644 index 000000000..e11be0fdc --- /dev/null +++ b/internals/checks/tests/e2e/attestation_guard_test.py @@ -0,0 +1,105 @@ +"""E2E test for `dirsql-checks attestation-guard` through the real CLI. + +No mocking of any kind: spawns the packaged `dirsql-checks` console script as +a subprocess, run against a real scratch git repo. +""" + +from __future__ import annotations + +import shutil +import subprocess + +import pytest + +RECEIPT = '{"command": "pytest", "exit_code": 0}\n' + + +def _cli() -> str: + dirsql_checks = shutil.which("dirsql-checks") + assert dirsql_checks is not None, ( + "`dirsql-checks` console script not on PATH -- run " + "`uv run --project internals/checks pytest tests/e2e` " + "or `uv sync --project internals/checks`" + ) + return dirsql_checks + + +def _git(repo, *args: str) -> None: + subprocess.run(["git", *args], cwd=repo, check=True, capture_output=True, text=True) + + +def _head(repo) -> str: + result = subprocess.run( + ["git", "rev-parse", "HEAD"], cwd=repo, check=True, capture_output=True, text=True + ) + return result.stdout.strip() + + +def _commit(repo, message: str) -> str: + _git(repo, "add", "-A") + _git(repo, "commit", "-m", message) + return _head(repo) + + +def _write(path, text=RECEIPT): + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text) + + +def _guard(repo, base_sha, head_sha): + return subprocess.run( + [_cli(), "attestation-guard", "--base-sha", base_sha, "--head-sha", head_sha], + cwd=repo, + capture_output=True, + text=True, + timeout=30, + ) + + +@pytest.fixture +def repo(tmp_path): + _git(tmp_path, "init", "-q") + _git(tmp_path, "config", "user.email", "test@example.com") + _git(tmp_path, "config", "user.name", "Test") + (tmp_path / "README.md").write_text("hello\n") + _write(tmp_path / "packages" / "ts" / "e2e-attestations" / "old-branch.json") + base_sha = _commit(tmp_path, "initial commit") + return tmp_path, base_sha + + +def describe_dirsql_checks_attestation_guard(): + def it_exits_zero_when_the_branch_only_adds_its_own_receipt(repo): + tmp_path, base_sha = repo + _write(tmp_path / "packages" / "ts" / "e2e-attestations" / "mine.json") + head_sha = _commit(tmp_path, "attest my branch") + + proc = _guard(tmp_path, base_sha, head_sha) + assert proc.returncode == 0, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" + + def it_exits_nonzero_when_the_branch_deletes_a_foreign_receipt(repo): + tmp_path, base_sha = repo + (tmp_path / "packages" / "ts" / "e2e-attestations" / "old-branch.json").unlink() + head_sha = _commit(tmp_path, "attest (pruned a sibling)") + + proc = _guard(tmp_path, base_sha, head_sha) + assert proc.returncode == 1, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" + assert "packages/ts/e2e-attestations/old-branch.json" in proc.stdout + assert f"git checkout {base_sha} --" in proc.stdout + + def it_exits_zero_via_the_allow_receipt_deletion_line(repo): + tmp_path, base_sha = repo + (tmp_path / "packages" / "ts" / "e2e-attestations" / "old-branch.json").unlink() + head_sha = _commit(tmp_path, "retire ts\n\nallow-receipt-deletion: package removed") + + proc = _guard(tmp_path, base_sha, head_sha) + assert proc.returncode == 0, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" + assert "permitting receipt deletion" in proc.stdout + + def it_names_a_near_miss_bypass(repo): + tmp_path, base_sha = repo + (tmp_path / "packages" / "ts" / "e2e-attestations" / "old-branch.json").unlink() + head_sha = _commit(tmp_path, "retire ts\n\nskip-receipt-deletion: package removed") + + proc = _guard(tmp_path, base_sha, head_sha) + assert proc.returncode == 1, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" + assert "is not the bypass line" in proc.stdout diff --git a/internals/checks/tests/integration/attestation_guard_test.py b/internals/checks/tests/integration/attestation_guard_test.py new file mode 100644 index 000000000..7e886ed2f --- /dev/null +++ b/internals/checks/tests/integration/attestation_guard_test.py @@ -0,0 +1,131 @@ +"""Integration tests for the attestation-guard check against a real git repo. + +Exercises `gate.run` with its default collaborators (the real `git_ops` +functions, real `git` subprocess calls) over a scratch repo fixture -- never +the packaged `dirsql-checks` CLI (that's the e2e tier). +""" + +from __future__ import annotations + +import os +import subprocess + +import pytest + +from checks.attestation_guard.gate import run + +RECEIPT = '{"command": "pytest", "exit_code": 0}\n' + + +def _git(*args: str) -> None: + subprocess.run(["git", *args], check=True, capture_output=True, text=True) + + +def _head() -> str: + result = subprocess.run( + ["git", "rev-parse", "HEAD"], check=True, capture_output=True, text=True + ) + return result.stdout.strip() + + +def _commit(message: str) -> str: + _git("add", "-A") + _git("commit", "-m", message) + return _head() + + +def _write(path, text=RECEIPT): + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text) + + +@pytest.fixture +def repo(tmp_path): + original_cwd = os.getcwd() + os.chdir(tmp_path) + try: + _git("init", "-q") + _git("config", "user.email", "test@example.com") + _git("config", "user.name", "Test") + (tmp_path / "README.md").write_text("hello\n") + _write(tmp_path / "packages" / "ts" / "e2e-attestations" / "old-branch.json") + _write(tmp_path / "packages" / "python" / "e2e-attestations" / "other.json") + base_sha = _commit("initial commit") + yield tmp_path, base_sha + finally: + os.chdir(original_cwd) + + +def describe_run_against_a_real_repo(): + def it_passes_when_the_branch_only_adds_its_own_receipt(repo, capsys): + tmp_path, base_sha = repo + _write(tmp_path / "packages" / "ts" / "e2e-attestations" / "mine.json") + head_sha = _commit("attest my branch") + + assert run(base_sha, head_sha) == 0 + assert "No e2e attestation receipts deleted." in capsys.readouterr().out + + def it_passes_when_the_branch_updates_an_existing_receipt(repo, capsys): + tmp_path, base_sha = repo + _write( + tmp_path / "packages" / "ts" / "e2e-attestations" / "old-branch.json", + '{"command": "pytest", "exit_code": 0, "commit": "x"}\n', + ) + head_sha = _commit("re-attest") + + assert run(base_sha, head_sha) == 0 + + def it_fails_when_the_branch_deletes_another_branchs_receipt(repo, capsys): + tmp_path, base_sha = repo + (tmp_path / "packages" / "ts" / "e2e-attestations" / "old-branch.json").unlink() + head_sha = _commit("attest (pruned a sibling)") + + assert run(base_sha, head_sha) == 1 + out = capsys.readouterr().out + assert "packages/ts/e2e-attestations/old-branch.json" in out + assert f"git checkout {base_sha} --" in out + + def it_reports_every_deleted_receipt_across_packages(repo, capsys): + tmp_path, base_sha = repo + (tmp_path / "packages" / "ts" / "e2e-attestations" / "old-branch.json").unlink() + (tmp_path / "packages" / "python" / "e2e-attestations" / "other.json").unlink() + head_sha = _commit("prune everything") + + assert run(base_sha, head_sha) == 1 + assert capsys.readouterr().out.count("::error file=") == 2 + + def it_fails_when_a_receipt_is_renamed_away(repo, capsys): + tmp_path, base_sha = repo + attestations = tmp_path / "packages" / "ts" / "e2e-attestations" + (attestations / "old-branch.json").rename(attestations / "renamed.json") + head_sha = _commit("rename a receipt") + + assert run(base_sha, head_sha) == 1 + assert "old-branch.json" in capsys.readouterr().out + + def it_passes_when_only_source_is_deleted(repo, capsys): + tmp_path, base_sha = repo + (tmp_path / "README.md").unlink() + head_sha = _commit("drop the readme") + + assert run(base_sha, head_sha) == 0 + + def it_passes_via_the_allow_receipt_deletion_line(repo, capsys): + tmp_path, base_sha = repo + (tmp_path / "packages" / "ts" / "e2e-attestations" / "old-branch.json").unlink() + _git("add", "-A") + _git("commit", "-m", "retire ts\n\nallow-receipt-deletion: package removed") + head_sha = _head() + + assert run(base_sha, head_sha) == 0 + assert "permitting receipt deletion" in capsys.readouterr().out + + def it_names_a_near_miss_bypass_instead_of_honouring_it(repo, capsys): + tmp_path, base_sha = repo + (tmp_path / "packages" / "ts" / "e2e-attestations" / "old-branch.json").unlink() + _git("add", "-A") + _git("commit", "-m", "retire ts\n\nskip-receipt-deletion: package removed") + head_sha = _head() + + assert run(base_sha, head_sha) == 1 + assert "is not the bypass line" in capsys.readouterr().out From 4e22dbe0a374458ef28c6165579000cd551bce63 Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 4 Sep 2026 21:08:56 -0400 Subject: [PATCH 2/4] =?UTF-8?q?feat:=20green=20=E2=80=94=20implement=20the?= =?UTF-8?q?=20attestation-guard=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fills in the stubs the previous commit left inert: receipt-path detection, the `allow-receipt-deletion: ` bypass and its near-miss diagnostic, and the failure report naming every deleted receipt plus the exact `git checkout -- ` that restores them. `--no-renames` on the diff so a receipt moved to another path counts as a deletion of the original — a receipt's path is its branch slug, so renaming one loses the record as surely as removing it. --- .../src/checks/attestation_guard/decide.py | 42 +++++++++++++++++-- .../src/checks/attestation_guard/gate.py | 25 ++++++++--- .../src/checks/attestation_guard/report.py | 42 ++++++++++++++++++- .../checks/attestation_guard/report_test.py | 24 ++++++++++- 4 files changed, 121 insertions(+), 12 deletions(-) diff --git a/internals/checks/src/checks/attestation_guard/decide.py b/internals/checks/src/checks/attestation_guard/decide.py index 213d5c9a9..87112e4cc 100644 --- a/internals/checks/src/checks/attestation_guard/decide.py +++ b/internals/checks/src/checks/attestation_guard/decide.py @@ -1,18 +1,52 @@ -"""Pure decision logic for the attestation-guard check (#1043).""" +"""Pure decision logic for the attestation-guard check (#1043). + +E2E attestation receipts are append-only by convention +(``agents/reference/e2e-attestation.md``): a merged branch's receipt stays in +the directory as the record that its suite ran. The rule here is therefore +**total** -- any deletion under an ``e2e-attestations/`` directory fails, with +no carve-out for "the branch's own" receipt. A branch that re-attests +*modifies* its receipt; it never deletes one. Slug-keyed ownership would also +mis-handle the real case from #1036, where a merged child branch's receipt +rode in under a different slug and looked foreign. + +An ``allow-receipt-deletion: `` commit-body line is the bypass, +mirroring changelog-gate's ``skip-changelog:``. The reason is mandatory so a +bare word cannot disarm the gate by accident. +""" from __future__ import annotations +import re + +# Any file inside a directory named `e2e-attestations`, at any depth, so a new +# package's receipt directory is covered the day it is created. +_RECEIPT = re.compile(r"(?:.+/)?e2e-attestations/.+") + +# Scanned over raw commit bodies rather than git's trailer parser, matching +# changelog-gate: a blank line must not split the bypass off the message. +_ALLOW = re.compile(r"(?im)^[ \t]*allow-receipt-deletion:[ \t]*\S.*$") + +# Deliberately loose, so a bypass that was attempted but misspelled (wrong +# verb, wrong noun, pluralised, reason omitted) is named instead of ignored. +_NEAR = re.compile( + r"(?im)^[ \t]*(?:skip|allow)[-_ ]?(?:receipt|attestation)s?[-_ ]?deletions?[ \t]*:?.*$" +) + def deleted_receipts(deleted) -> list[str]: """Sorted paths the diff deletes from an ``e2e-attestations/`` directory.""" - return [] + return sorted(path for path in deleted if _RECEIPT.fullmatch(path)) def has_allow_line(commit_messages: str) -> bool: """True if a commit body carries ``allow-receipt-deletion: ``.""" - return False + return bool(_ALLOW.search(commit_messages)) def near_miss_lines(commit_messages: str) -> list[str]: """Bypass-shaped commit lines that are not the accepted spelling.""" - return [] + return [ + line.strip() + for line in _NEAR.findall(commit_messages) + if not _ALLOW.fullmatch(line) + ] diff --git a/internals/checks/src/checks/attestation_guard/gate.py b/internals/checks/src/checks/attestation_guard/gate.py index 90a72bbd1..7b73f2384 100644 --- a/internals/checks/src/checks/attestation_guard/gate.py +++ b/internals/checks/src/checks/attestation_guard/gate.py @@ -1,14 +1,27 @@ -"""Orchestration for the attestation-guard check (#1043).""" +"""Orchestration for the attestation-guard check (#1043). + +Fails a PR whose `base...HEAD` diff deletes any e2e attestation receipt. The +git plumbing is injected so the orchestration unit-tests without a real repo. +""" from __future__ import annotations from checks.attestation_guard import git_ops -from checks.attestation_guard.decide import deleted_receipts +from checks.attestation_guard.decide import ( + deleted_receipts, + has_allow_line, + near_miss_lines, +) +from checks.attestation_guard.report import report def verdict(deleted, messages: str, base_sha: str) -> int: """0 when a well-formed bypass line is present, else 1 with diagnostics.""" - return 0 + if has_allow_line(messages): + print("allow-receipt-deletion line present; permitting receipt deletion.") + return 0 + report(deleted, near_miss_lines(messages), base_sha) + return 1 def run( @@ -19,5 +32,7 @@ def run( commit_messages=git_ops.commit_messages, ) -> int: deleted = deleted_receipts(deleted_files(base_sha, head_sha)) - print("No e2e attestation receipts deleted.") - return 0 + if not deleted: + print("No e2e attestation receipts deleted.") + return 0 + return verdict(deleted, commit_messages(base_sha, head_sha), base_sha) diff --git a/internals/checks/src/checks/attestation_guard/report.py b/internals/checks/src/checks/attestation_guard/report.py index dd2141de1..709147145 100644 --- a/internals/checks/src/checks/attestation_guard/report.py +++ b/internals/checks/src/checks/attestation_guard/report.py @@ -1,12 +1,50 @@ -"""Failure output for the attestation-guard check (#1043).""" +"""Failure output for the attestation-guard check (#1043). + +AGENTS.md requires every CI check to emit actionable fix instructions, so this +names each wrongly-deleted receipt and prints the exact `git checkout` that +restores it, plus a near-miss diagnostic when a bypass line was attempted but +misspelled. +""" from __future__ import annotations +_ADVICE = ( + "::error::A receipt records that another branch's e2e suite ran, and " + "nothing can regenerate a merged branch's. Restore them and re-attest only " + "your own branch. If the deletion is deliberate (retiring a package), add " + "an 'allow-receipt-deletion: ' line to any commit message." +) + def restore_command(deleted, base_sha: str) -> str: """The exact `git checkout` that puts every deleted receipt back.""" - return "" + return f"git checkout {base_sha} -- {' '.join(deleted)}" + + +def deletion_annotation(path: str) -> str: + """The per-file annotation naming one wrongly-deleted receipt.""" + return ( + f"::error file={path}::{path} is an e2e attestation receipt. " + "Receipts are append-only; this PR deletes it." + ) + + +def near_miss_annotation(line: str) -> str: + """The annotation naming a bypass line that was attempted but misspelled.""" + return ( + f"::error::{line!r} is not the bypass line. The exact form is " + "'allow-receipt-deletion: ', reason required." + ) def report(deleted, near_misses, base_sha: str) -> None: """Print one annotation per deleted receipt, then how to undo them.""" + for path in deleted: + print(deletion_annotation(path)) + for line in near_misses: + print(near_miss_annotation(line)) + print( + f"::error::Restore {len(deleted)} deleted receipt(s) with: " + f"{restore_command(deleted, base_sha)}" + ) + print(_ADVICE) diff --git a/internals/checks/src/checks/attestation_guard/report_test.py b/internals/checks/src/checks/attestation_guard/report_test.py index 548dc1785..0ddf7d4b2 100644 --- a/internals/checks/src/checks/attestation_guard/report_test.py +++ b/internals/checks/src/checks/attestation_guard/report_test.py @@ -1,4 +1,9 @@ -from checks.attestation_guard.report import report, restore_command +from checks.attestation_guard.report import ( + deletion_annotation, + near_miss_annotation, + report, + restore_command, +) def describe_restore_command(): @@ -8,6 +13,23 @@ def names_the_base_sha_and_every_path(): ) +def describe_deletion_annotation(): + def names_the_file_and_the_append_only_rule(): + line = deletion_annotation("packages/ts/e2e-attestations/a.json") + assert line.startswith("::error file=packages/ts/e2e-attestations/a.json::") + assert "packages/ts/e2e-attestations/a.json is an e2e attestation receipt" in line + assert "Receipts are append-only; this PR deletes it." in line + + +def describe_near_miss_annotation(): + def quotes_the_line_and_gives_the_exact_form(): + line = near_miss_annotation("skip-receipt-deletion: y") + assert line == ( + "::error::'skip-receipt-deletion: y' is not the bypass line. " + "The exact form is 'allow-receipt-deletion: ', reason required." + ) + + def describe_report(): def annotates_every_deleted_receipt_with_its_own_file(capsys): report(["packages/ts/e2e-attestations/a.json"], [], "abc123") From b4f20fbf6faffbe9bdeb00d57d7009634e7cfb7a Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 4 Sep 2026 21:09:02 -0400 Subject: [PATCH 3/4] e2e attestation for claude/1043-receipt-deletion-guard --- .../claude-1043-receipt-deletion-guard.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 internals/checks/e2e-attestations/claude-1043-receipt-deletion-guard.json diff --git a/internals/checks/e2e-attestations/claude-1043-receipt-deletion-guard.json b/internals/checks/e2e-attestations/claude-1043-receipt-deletion-guard.json new file mode 100644 index 000000000..4b65821dd --- /dev/null +++ b/internals/checks/e2e-attestations/claude-1043-receipt-deletion-guard.json @@ -0,0 +1,7 @@ +{ + "command": "uv run python -m pytest tests/e2e -x -q", + "ran_at": 1788570542, + "exit_code": 0, + "commit": "4e22dbe0a374458ef28c6165579000cd551bce63", + "branch": "claude/1043-receipt-deletion-guard" +} From f1179a187fffba14378dded6fed436f39b5afd5d Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 4 Sep 2026 21:10:20 -0400 Subject: [PATCH 4/4] test: pin the keyword-only collaborator seam on attestation-guard's run Kills a surviving `*` -> `/` mutant on the signature: the mutant turns the positional-only marker on, which every keyword-passing test still tolerates. --- .../checks/src/checks/attestation_guard/gate_test.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internals/checks/src/checks/attestation_guard/gate_test.py b/internals/checks/src/checks/attestation_guard/gate_test.py index 015cde90c..d47634219 100644 --- a/internals/checks/src/checks/attestation_guard/gate_test.py +++ b/internals/checks/src/checks/attestation_guard/gate_test.py @@ -1,5 +1,7 @@ from unittest import mock +import pytest + from checks.attestation_guard.gate import run, verdict @@ -56,6 +58,12 @@ def fails_and_names_a_near_miss_bypass(capsys): assert rc == 1 assert "is not the bypass line" in capsys.readouterr().out + def keeps_the_injected_collaborators_keyword_only(): + # The `*` in the signature is the seam's contract: a positional third + # argument must not silently bind to `deleted_files`. + with pytest.raises(TypeError): + run("base", "head", mock.Mock(return_value=[]), mock.Mock()) + def queries_git_with_the_supplied_shas(): deleted = mock.Mock(return_value=[]) run("b1", "h1", deleted_files=deleted, commit_messages=mock.Mock())