TL;DR

In July I wrote about a test-deletion sentinel: CI hard-fails if a merge request deletes tests, unless the commit message carries a [test-refactor] tag and an issue number. I was pleased with it. It turns out a tag in a commit message is a bypass that an agent (or I, on a bad day) can write in one line, and it excused every test change in the MR, not just the one I meant.

So in September I replaced it. The merge-request policy gate now allows a test removal only if an entry in a reviewed file matches one exact fingerprint: the rule that fired, the file, the test name, a hash of the old test, a hash of its replacement, and an issue number. Change one token of either test and the exception stops matching. There is no tag, no wildcard, and no bypass flag.

This post is how it works, what it caught in my own design, and where it still has holes. The code is in my cluster repo’s scripts/ci/mr_policy_guard.py.

Why this exists

The motivation is the same as in the autonomy ladder and the rival-model merge gate: an agent asked to make CI green has an easy path, which is to make the thing that turns CI red go away. Delete the failing test. Add a skip. Loosen the assertion until it passes. None of that requires malice. It is just the shortest route to the instruction “get it green.”

The house rule is that tests are never deleted to get green, and the rule has to be enforced by something that is not a language model, because a model reviewer can be skipped, rate-limited, or talked around. The gate’s own docstring says it: it is deliberately separate from the advisory LLM reviewer, and it reads the git diff and rejects mechanical regressions before an LLM is involved.

What the gate looks at

A job called policy:mr runs on every merge-request pipeline. It diffs the merge base against the head with zero context lines, then looks only at files that look like tests. For Python test files it parses both the old and the new source into an AST and compares them. It raises a finding when it sees:

  • a test file deleted, or renamed out of a test path (no exception exists for this at all)
  • a removed test function
  • fewer assertions than before, in a changed test
  • an exact comparison replaced by something looser
  • a removed pytest.param case
  • an added pytest.skip, skip, skipif, or xfail
  • a tautology like assert True or assert 1 == 1
  • a test file it cannot parse, which counts as a finding, because failing open on a syntax error would be a trivial bypass

Policy files that fail to load, and changed files it cannot read from git, produce exit code 2 and a failed job. The gate fails closed.

The old escape hatch, and what was wrong with it

The first version let a head commit message carrying [test-refactor] plus an issue reference switch off the file-deletion and removed-test findings for the entire merge request. That was a real improvement over nothing. It also had two problems, and I only found the first because a separate audit went looking.

Problem one: the tag was scoped to the whole message. In the shared CI template I use across repos, a downstream audit found that a commit which merely discussed the tag in its body exempted its own test deletions. A commit documenting the feature was disabling the gate it described. The fix, in that template, was to require the tag in the commit subject along with an issue reference.

Problem two: renames walked through. Git’s default rename detection turned git mv test_foo.py test_foo.py.disabled into one R entry, which the deletion filter never matched. Same audit. The template now uses no-rename mode so a rename shows up as a delete plus an add, which means a pure rename also needs the escape hatch. That is an accepted trade-off.

Both fixes made the tag stricter. Neither changed what it fundamentally was: a promise made in prose, checked by a regex, covering everything in the MR. In the cluster repo’s own guard, I removed it entirely.

An exception that is a fingerprint

The replacement is a YAML file, scripts/ci/mr-policy-allowlist.yaml, with a reviewed_test_changes list. Each entry has to say exactly what it is excusing:

reviewed_test_changes:
  - issue: '#NN'
    kind: test_refactor
    rule: removed_test
    path: tests/ci/test_retired_stack_gate.py
    symbol: test_rollout_loop_contains_only_actual_deployments
    replacement_symbol: test_no_ci_job_references_the_retired_stack
    old: py-token-v1:23c2b303e004...   # 64 hex digits in the real file
    new: py-token-v1:121ce8b63ff5...
    reason: The deploy job was deleted with the retirement; the guard now asserts no CI job references the retired stack.

(That is a real entry with the names generalized and the hashes shortened.)

A finding is excused only when all of these match: the rule that fired, the literal file path, the test name, the fingerprint of the old test, the fingerprint of the replacement, and the replacement’s name. A few more constraints make it hard to abuse:

  • The file has to have exactly those nine fields. Extra or missing fields are an error.
  • There are only two kinds. test_refactor can excuse only a removed_test. backend_dependency_skip can excuse only a conditional skip.
  • The issue must look like #123, the reason cannot be blank, and the path cannot contain a wildcard.
  • Fingerprints must be absent or py-token-v1: plus 64 hex characters.
  • Each entry is single-use within a run. Duplicates are rejected, and one entry cannot excuse two findings.

The second kind exists because of a real case: tests that need a PostgreSQL backend skip themselves when it is not configured, and a separate required CI job runs them against a real database. Those skips are legitimate, and each one is listed with a reason that says where the test actually runs.

What “fingerprint” means

The fingerprint is a SHA-256 over the Python token stream of the test function, excluding comments, indentation, and newlines. This is the real function, trimmed:

def node_fingerprint(source: str, node: ast.AST | None) -> str:
    if node is None:
        return "absent"
    lines = source.splitlines(keepends=True)
    snippet = "".join(lines[node.lineno - 1 : node.end_lineno])
    items = []
    for item in tokenize.generate_tokens(io.StringIO(snippet).readline):
        if item.type not in {token.COMMENT, token.INDENT, token.DEDENT,
                             token.NEWLINE, token.NL, token.ENDMARKER}:
            items.append(f"{token.tok_name[item.type]}:{item.string}")
    return "py-token-v1:" + hashlib.sha256("\n".join(items).encode()).hexdigest()

I ran it on three versions of a two-line test to check the property I care about. The original and a version with an added comment and odd spacing gave the same fingerprint. Changing == to >= gave a different one. So reformatting does not break an exception, and any change to what the test actually does does.

That is the whole point. The reviewed entry says “this exact old test was replaced by this exact new test.” If an agent later tweaks the replacement into something weaker, the hash no longer matches and the gate fails again. A human approving the exception approved a specific pair of code snippets, not a category.

The secret scanner objected

This was the funniest part. A 64-character hex string is exactly what a secret scanner is for. The fingerprints in the exceptions file tripped the repo’s secret scan, so each one had to be allowlisted in the scanner’s config, by exact value, with a comment saying “reviewed fingerprint, not a credential.”

The exceptions file needed exceptions, and the same rule applied to them: exact match, no pattern. I did not add a “64 hex characters” regex, because that would also whitelist a real leaked key.

What it caught, and the limits

I do not have a tidy statistic for how many deletions this has blocked. What I can say from the history is how it has been used: the entries in the file today are a handful of reviewed removals (one where a test was renamed and strengthened, a few conditional database skips, and two where a retired service’s gate tests were replaced by one test asserting nothing references the retired stack). That is the shape I wanted. Every legitimate case has a name and a reason in a diff.

The honest limits:

  1. The exceptions file lives in the same repo. An agent could add an entry for its own deletion in the same MR. The gate does not stop that. What it does is turn the bypass into a conspicuous change to one file, with an issue number and a reason, in the diff a human reviews before merge. I did not find a code-owner rule protecting that file, so today the protection is review, not enforcement.
  2. The exact-match path is Python only. Other test files fall back to line-based heuristics with no reviewed-exception path, so they stay strict by default.
  3. The issue field is checked for shape, not truth. #123 passes whether or not issue 123 says anything relevant.
  4. It does not judge whether a test is any good. It only catches tests getting weaker. A bad test that existed before stays bad.

I also have not measured how often agents try the shortcuts. The gate exists because the failure mode is obvious and cheap to prevent, not because I have a catch rate to report.

What I would copy

  • Make the exception as narrow as the thing you are excusing. If the exception is a category (“test refactors”), it will be applied to things you did not intend.
  • Match on content, not on a claim about content. A tag says “trust me.” A hash says “this exact thing.”
  • Fail closed on every parse and load error. A gate that passes when it cannot read its own policy is decoration.
  • Make exceptions cost a visible diff. The point is not to make them impossible. The point is that each one is a line someone has to read.

The commits for this rework landed over about 75 minutes one early morning, authored by an agent, which is its own small irony. It is still doing its job because it is mechanical, narrow, and boring, and none of those depend on who wrote it.