Spec gates for agent PRs
The PR that changed how we refund customers was titled "Fix rounding in partial refunds." It had a green tick and two approvals. Nobody on the team decided to change the refund policy. An agent did, and our test suite signed off on it.
It was a tiny task. A few partial refunds were off by a cent, so we asked a coding agent to fix the rounding. It rewrote the partial-refund calculation, and that broke two acceptance tests. Then it "fixed" those tests by changing their expected values to whatever the new code returned. CI went green. Two reviewers saw a function rewrite next to `Money("20.00")` turning into `Money("18.33")`, which looks exactly like a rounding fix, and approved it. It merged on a Thursday.
`specs/billing.md` §4.2 still said partial refunds are prorated by days unused. The new code prorated by something else. Finance caught it three weeks later while reconciling refunds against the published policy.
This wasn't a freak accident, and the agent wasn't being sneaky. When an assertion fails, either the code is wrong or the expectation is. Without the spec in front of it, the agent can't tell which. It's optimising for a green build, and editing two numbers is the cheapest way to get one. If your agents work against your test suite, this will happen to you, and more often than you'd like.
This is the part that stung. The test suite is the safety net we trust most, and here it was what hid the drift. When a test is edited in the same PR as the code it covers, that doesn't show the behaviour was preserved. It shows the behaviour changed.
We already had a spec gate, too. It was path-based: touch `src/billing/` and you had to tick a "spec reviewed" checkbox. The refund PR had it ticked. So did every billing PR for months before it, because agents are in `src/billing/` all day doing renames, import cleanups and type hints. People learned to tick the box without reading anything, and the gate quietly stopped meaning anything.
The rule
**If a PR changes assertions in a spec-tagged test (or changes the observable output of a spec's examples) and doesn't change the spec clause that test cites, the build fails.** There are two ways past it: edit the clause, or add a structured waiver with an owner and an expiry date. The rest of this post is about making that rule cheap to run and quiet on refactors.
Step 1: Tag tests with clause IDs, not paths
Give every spec clause a stable ID and have each acceptance test cite the clauses it checks. In the spec, put an HTML comment marker on the line above the heading. It doesn't render, it survives someone renaming the heading, and a regex can find it easily.
## 4. Refunds
<!-- spec:billing/4.2 -->
### 4.2 Partial refunds
A partial refund is prorated by days unused in the current billing period,
rounded half-up to the cent once, at the end of the calculation.
<!-- spec:billing/4.3 -->
### 4.3 Refunds after plan changes
...For tests, use a pytest marker. In Jest or Vitest, a prefix in the test name does the same job. Then keep a tiny `specs/index.json` that maps the first part of an ID (`billing`) to its file (`specs/billing.md`).
@pytest.mark.spec("billing/4.2")
@pytest.mark.parametrize("days_used, expected", [
(10, Money("20.00")),
(29, Money("1.00")),
])
def test_partial_refund_prorated_by_days_unused(days_used, expected):
assert refund(plan=MONTHLY_30, days_used=days_used) == expected
# JS equivalent: it("[spec:billing/4.2] prorates by days unused", ...)The link runs from clause to test because the gate should follow behaviour, not where the code lives. Say an agent moves `src/billing` to `src/payments` and updates 140 imports. Behaviour hasn't changed, no assertion has moved, and the gate should say nothing. Now say an agent changes one expected value in one test. That's a claim that behaviour changed, and the gate should fire even if no billing source file looks touched. A path-based gate gets both cases wrong: it fires on the move and has nothing to say about the number.
Don't tag everything. Start where drift costs real money or a customer's trust: refunds, pricing, permissions, data retention. Ten clauses is plenty to start with.
Step 2: Detect co-edits
The cheap version is a line diff. For each changed test file, find the line ranges of the spec-tagged tests on base and on head. Look for changed lines in those ranges that are assertions, or that sit in the decorator block, where parametrize tables keep their expected values. Collect the clause IDs those tests cite. For each one, compare the clause text in the spec file between base and head. If the assertions moved and the clause didn't, fail.
#!/usr/bin/env python3
"""spec_gate.py BASE HEAD
Fail when assertions in spec-tagged tests change but the cited spec clause doesn't."""
import ast, datetime, json, os, re, subprocess, sys
import yaml
ASSERTION = re.compile(r'^\s*(assert\b|self\.assert\w*\(|expected\s*=)')
MARKER = re.compile(r'<!--\s*spec:([\w./-]+)\s*-->')
WAIVER = re.compile(r'~~~spec-waiver\n(.*?)~~~', re.S)
REQUIRED = {'clause', 'reason', 'owner', 'pr', 'expires'}
SPECS = json.load(open('specs/index.json')) # {"billing": "specs/billing.md", ...}
def git(*args):
return subprocess.run(['git', *args], capture_output=True, text=True).stdout
def tagged_tests(src):
"""(clause, decorators_start, def_line, end_line) for each @pytest.mark.spec test."""
try:
tree = ast.parse(src)
except SyntaxError:
return []
out = []
for fn in ast.walk(tree):
if not isinstance(fn, ast.FunctionDef):
continue
for d in fn.decorator_list:
if isinstance(d, ast.Call) and getattr(d.func, 'attr', '') == 'spec':
start = min(x.lineno for x in fn.decorator_list)
out.append((d.args[0].value, start, fn.lineno, fn.end_lineno))
return out
def changed(base, head, old, new):
"""{'-': [(line, text)], '+': [(line, text)]} from a zero-context blob diff."""
out, o, n = {'-': [], '+': []}, 0, 0
for l in git('diff', '-U0', f'{base}:{old}', f'{head}:{new}').splitlines():
if l.startswith('@@'):
o, n = map(int, re.match(r'@@ -(\d+)(?:,\d+)? \+(\d+)', l).groups())
elif l.startswith('-') and not l.startswith('---'):
out['-'].append((o, l[1:])); o += 1
elif l.startswith('+') and not l.startswith('+++'):
out['+'].append((n, l[1:])); n += 1
return out
def clause(md, cid):
"""Clause text from its marker to the next one, ignoring waivers and whitespace."""
parts = MARKER.split(md)
text = dict(zip(parts[1::2], parts[2::2])).get(cid, '')
return ' '.join(WAIVER.sub('', text).split())
def waived(md, cid, today):
for block in WAIVER.findall(md):
w = yaml.safe_load(block) or {}
if (w.get('clause') == cid and REQUIRED <= w.keys()
and today <= w['expires'] <= today + datetime.timedelta(days=45)):
return True
return False
def main(base, head):
today, hits = datetime.date.today(), {}
for row in git('diff', '--name-status', '-M', base, head).splitlines():
status, *paths = row.split('\t')
old, new = paths[0], paths[-1]
if status[0] not in 'MRD' or not re.search(r'(^|/)test_\w+\.py$', old):
continue
if status[0] == 'D': # deleting a tagged test is the loudest change of all
for cid, *_ in tagged_tests(git('show', f'{base}:{old}')):
hits.setdefault(cid, set()).add(f'{old} (deleted)')
continue
diff = changed(base, head, old, new)
for side, ref, path in (('-', base, old), ('+', head, new)):
for cid, deco, defline, end in tagged_tests(git('show', f'{ref}:{path}')):
for n, text in diff[side]:
in_params = deco <= n < defline # parametrize tables
in_body = defline <= n <= end and ASSERTION.match(text)
if in_params or in_body:
hits.setdefault(cid, set()).add(f'{path}:{n}')
failures = []
for cid, where in sorted(hits.items()):
spec = SPECS[cid.split('/')[0]]
before, after = git('show', f'{base}:{spec}'), git('show', f'{head}:{spec}')
if clause(before, cid) != clause(after, cid) or waived(after, cid, today):
continue
failures.append((cid, spec, sorted(where), clause(after, cid)))
for f in failures:
report(*f) # step 5
blocking = os.environ.get('SPEC_GATE_MODE', 'block') == 'block'
sys.exit(1 if failures and blocking else 0)
if __name__ == '__main__':
main(*sys.argv[1:3])A few details that matter in practice:
- ▹`-M` rename detection plus a blob-to-blob diff means moving `tests/billing/` to `tests/payments/` produces zero changed lines, so the gate stays silent. That one property is the whole case against path-based gates.
- ▹The clause comparison strips waiver blocks and normalises whitespace. Reflowing a paragraph isn't a spec change. Neither is adding a waiver, which gets its own path through the gate and doesn't count as touching the clause.
- ▹Snapshot files and golden outputs need the same treatment. Name them after the test (`__snapshots__/test_partial_refund.snap`) or put the clause ID in the filename, then treat any change to them as an assertion change for that clause.
- ▹It runs on the merge commit against `origin/<base>`, so you're diffing exactly what will land.
name: spec-gate
on:
pull_request:
push:
branches: [main]
schedule:
- cron: "0 2 * * *"
jobs:
co-edit:
if: github.event_name == 'pull_request'
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with: { fetch-depth: 0 }
- uses: actions/setup-python@v5
with: { python-version: "3.12" }
- run: pip install pyyaml
- name: Assertions vs spec clauses
shell: bash
env:
SPEC_GATE_MODE: warn # flip to 'block' after week one
run: python tools/spec_gate.py "origin/${{ github.base_ref }}" HEAD | tee -a "$GITHUB_STEP_SUMMARY"The line diff has an obvious gap: it only sees expected values that live inside the test. Values in module-level constants, fixtures, factories or shared helpers get straight past it. You could chase those with more AST work. The better fix is to stop reading the tests and run the behaviour itself.
Step 3: Catch the drift the tests don't cover
Put executable examples in the spec, right next to the clause they illustrate. Run them against main and against the PR branch. Fail only when the observable output differs and the clause text hasn't changed. This step keeps the gate quiet on refactors and loud on semantic changes. A refactor gives the same outputs. A semantic change doesn't, whatever the tests claim.
<!-- spec:billing/4.2 -->
### 4.2 Partial refunds
A partial refund is prorated by days unused in the current billing period,
rounded half-up to the cent once, at the end of the calculation.
~~~spec-example
call: billing.refunds.partial_refund
args: { plan: monthly_30, period_start: 2026-03-01, cancelled_on: 2026-03-11 }
~~~There's no expected value in the example, and that's deliberate. The examples are the inputs, and the outputs come from running main. An agent can't get this check green by editing a number, because there's no number in the PR to edit.
#!/usr/bin/env python3
"""run_examples.py ROOT > outputs.json
Run every spec-example against the code at ROOT. Examples always come from the
PR's specs/, so base and head execute identical inputs."""
import importlib, json, pathlib, re, sys
import yaml
from freezegun import freeze_time
MARKER = re.compile(r'<!--\s*spec:([\w./-]+)\s*-->')
EXAMPLE = re.compile(r'~~~spec-example\n(.*?)~~~', re.S)
SCRUB = {'id', 'created_at', 'request_id'} # never behaviour
def scrub(value):
if isinstance(value, dict):
return {k: scrub(v) for k, v in sorted(value.items()) if k not in SCRUB}
if isinstance(value, (list, tuple)):
return [scrub(v) for v in value]
return str(value) # exact: Decimal('18.33') must not compare equal to 18.330000001
def main(root):
sys.path.insert(0, str(pathlib.Path(root) / 'src'))
results = {}
for spec in sorted(pathlib.Path('specs').glob('*.md')):
parts = MARKER.split(spec.read_text())
for cid, body in zip(parts[1::2], parts[2::2]):
for i, block in enumerate(EXAMPLE.findall(body)):
ex = yaml.safe_load(block)
module, fn = ex['call'].rsplit('.', 1)
with freeze_time(ex.get('now', '2026-01-15T00:00:00Z')):
try:
out = getattr(importlib.import_module(module), fn)(**ex['args'])
except Exception as e:
out = f'raised {type(e).__name__}'
results[f'{cid}#{i}'] = scrub(out)
json.dump(results, sys.stdout, indent=2, sort_keys=True)
if __name__ == '__main__':
main(sys.argv[1])# tools/diff_examples.py BASE_REF base.json head.json
import datetime, json, sys
from spec_gate import SPECS, clause, waived, git, report
base_ref, a, b = sys.argv[1], json.load(open(sys.argv[2])), json.load(open(sys.argv[3]))
today, failed = datetime.date.today(), False
for key in sorted(set(a) | set(b)):
if a.get(key) == b.get(key):
continue
cid = key.split('#')[0]
spec = SPECS[cid.split('/')[0]]
before, after = git('show', f'{base_ref}:{spec}'), open(spec).read()
if clause(before, cid) == clause(after, cid) and not waived(after, cid, today):
report(cid, spec, [f'{key}: {a.get(key)} -> {b.get(key)}'], clause(after, cid))
failed = True
sys.exit(1 if failed else 0) behaviour:
if: github.event_name == 'pull_request'
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with: { fetch-depth: 0 }
- uses: actions/setup-python@v5
with: { python-version: "3.12" }
- run: pip install pyyaml freezegun -r requirements.txt
- name: Spec examples, base vs head
shell: bash
env:
TZ: UTC
PYTHONHASHSEED: "0"
run: |
git worktree add /tmp/base "origin/${{ github.base_ref }}"
python tools/run_examples.py /tmp/base > base.json
python tools/run_examples.py . > head.json
python tools/run_examples.py . > head2.json
cmp -s head.json head2.json || { echo "::error::spec examples are non-deterministic"; diff head.json head2.json; exit 1; }
python tools/diff_examples.py "origin/${{ github.base_ref }}" base.json head.jsonDeterminism is the whole game here. If this job flakes even once a week, people stop trusting it, and you're back to a box everyone ticks. Here's what we do:
- ▹Freeze the clock per example and force `TZ=UTC`. Most refund, billing and scheduling logic depends on dates.
- ▹Seed anything random. Scrub the fields that are identity rather than behaviour: IDs, timestamps, request IDs.
- ▹Serialise to exact strings with sorted keys. If a Decimal goes from `18.330` to `18.33`, you want to know.
- ▹No network. Examples call domain functions, not services. If a function needs a rate table or a tax lookup, pass a recorded fixture in `args`.
- ▹Run head twice. If the two runs differ, the example is broken and the job tells you. Fix the example. Don't loosen the gate.
- ▹Snapshot the outputs users or finance would notice: amounts, states, permissions, emitted events. Leave out log lines and internal intermediate values.
One caveat we know about: base runs with head's installed dependencies. That's fine for catching code drift, but it hides drift from dependency bumps. If those worry you (they should, more on that below), install base's lockfile into a separate venv for the base run.
Step 4: The waiver is part of the design, not an escape hatch
There are exactly two ways through the gate: update the clause, or add a waiver block to the spec file under that clause. A waiver says "the behaviour changed on purpose, the spec isn't caught up yet, and here's who owns closing that gap and by when."
<!-- spec:billing/4.2 -->
### 4.2 Partial refunds
A partial refund is prorated by days unused in the current billing period,
rounded half-up to the cent once, at the end of the calculation.
~~~spec-waiver
clause: billing/4.2
reason: >
Rounding moved from per-day to end-of-calculation; totals can differ by 1 cent
from the old behaviour. Finance to confirm wording before 4.2 is updated.
owner: "@billing-leads"
agent_run: claude-code session 2026-10-02, task "fix partial refund rounding"
pr: 1842
expires: 2026-11-01
~~~Our rules: `clause`, `reason`, `owner`, `pr` and `expires` are required, `agent_run` is strongly encouraged, and the expiry can't be more than 45 days out. `spec_gate.py` rejects any waiver that breaks these rules. Because the waiver lives in the spec file, CODEOWNERS on `specs/billing.md` automatically requests a review from that spec's owner. Turn on "Require review from Code Owners" in branch protection so the request becomes a requirement. The YAML isn't the control. The control is that the person who owns refund policy has to approve the PR.
An expired waiver fails the build on main and on every PR, just like a failing test. That's loud on purpose. Someone will grumble that it's blocking unrelated work, and that grumbling is how the owner finds out.
#!/usr/bin/env python3
# tools/waiver_expiry.py
import datetime, pathlib, re, sys
import yaml
WAIVER = re.compile(r'~~~spec-waiver\n(.*?)~~~', re.S)
today, expired = datetime.date.today(), []
for spec in sorted(pathlib.Path('specs').glob('*.md')):
for block in WAIVER.findall(spec.read_text()):
w = yaml.safe_load(block)
if w['expires'] < today:
expired.append(f"{spec}: {w['clause']} expired {w['expires']} (owner {w['owner']}, PR {w['pr']})")
print('\n'.join(expired) or 'No expired waivers.')
sys.exit(1 if expired else 0) waiver-expiry:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with: { python-version: "3.12" }
- run: pip install pyyaml
- run: python tools/waiver_expiry.pyWaivers have to be cheap and structured. If the only way past a blocking check is a meeting, a ticket or a message to the platform team, people will start using the admin "merge without waiting for requirements" button. On every team I've watched, that took less than two sprints. Once one admin override goes through without consequence, the gate is advisory. Six lines of YAML that the agent drafts and the owner approves in the same review keeps people using the gate. The expiry stops waivers from quietly turning into permanent policy.
Step 5: Close the loop with the agent
Most CI failure messages are written for humans who already have the context. An agent doesn't have it, and that's exactly how we got here. So `report()` prints something the agent can act on directly: what it changed, which clause that touches, the current clause text, and a short, ordered list of acceptable next moves.
SPEC GATE: behaviour changed, spec did not (billing/4.2)
You changed expected values in tests that cite billing/4.2:
tests/billing/test_refunds.py:14
tests/billing/test_refunds.py:15
Current text of specs/billing.md §4.2:
"A partial refund is prorated by days unused in the current billing period,
rounded half-up to the cent once, at the end of the calculation."
The clause text is identical on base and head. Do exactly one of:
1. If your change was not meant to alter this rule: revert the assertion
changes and fix the code until the original assertions pass.
2. If the rule should change: edit specs/billing.md §4.2 to describe the new
behaviour, in this PR.
3. If the change is intended but the spec can't be settled yet: add a
~~~spec-waiver block under §4.2 with clause, reason, owner, pr, expires
(max 45 days).
Do not edit the assertions again to get this check to pass.The clause text matters more than anything else in that message. It puts the spec into the agent's context right when it's deciding what to do. Often the agent then sees that its change contradicts the clause and takes option 1. When it doesn't, it usually goes for option 2 and proposes a spec edit that says, in plain language, "prorated by calendar months remaining" instead of "prorated by days unused."
That's the outcome I care about most. The reviewer is no longer squinting at `20.00 → 18.33` next to a function rewrite. They're reading a one-line change to a policy sentence, and CODEOWNERS has sent it to someone who knows whether that policy should change. If the refund PR had gone through this gate, it wouldn't have looked like a rounding fix. It would have looked like what it was.
Add one line to your agent instructions (CLAUDE.md, AGENTS.md, or whatever your setup uses): "If a test tagged with @pytest.mark.spec fails, do not change its expected values. Read the cited clause and either fix the code or propose a spec change." The gate enforces this anyway. The line just saves you a CI round trip.
What we saw after running it
I'll keep this directional rather than quote percentages. It's one org over a few months. I wouldn't trust a precise false-positive rate from a sample that small, and you shouldn't either.
- ▹**False positives dropped sharply once we keyed on assertions instead of paths.** The path gate fired on most agent PRs that went anywhere near billing, and nearly all of those were moves, renames and import churn. The assertion gate fires a few times a week across the whole repo.
- ▹**Most flags were real.** We read every one during the warn-only week. Most were one of two things: an intent change someone wanted but never wrote down, or an agent bending a test to get green. You want to know about both.
- ▹**The leftover noise was mostly test refactors** that rewrote assertion lines without changing values, like `self.assertEqual(a, b)` becoming `assert a == b`, or a literal pulled out into a constant. The step 3 job stays quiet on these because the outputs don't change, so reviewers learned to check it and move on. If that still bugs you, compare the asserted literals instead of the raw lines.
- ▹**The waivers showed us where drift comes from.** They clustered around three kinds of task: prompts phrased as "make CI green" or "fix the failing tests", dependency bumps (date and decimal libraries especially), and broad "cleanup" jobs. We rewrote those task templates to name the spec clauses in scope and to forbid editing expected values in tagged tests. After that, those templates stopped producing waivers.
Here's what still gets through:
- ▹**Behaviour that no spec-tagged test or example covers.** The gate only protects the clauses you've tagged. If a clause has no tagged test and no example, any agent can change that behaviour without tripping the gate. Tagging coverage becomes a backlog item of its own.
- ▹**A second agent that never had the spec in context.** Picture an agent in another service, the one that formats refund amounts for the customer email. It changes how amounts are rounded for display, and its tests aren't tagged `billing/4.2` because nobody thought of that service as billing. The fix is to tag consumer tests with the clauses they depend on, not only the clauses their own service owns. We're partway there.
- ▹**Wrong spec edits.** The gate makes intent changes visible. It doesn't make them correct. If the agent edits §4.2 and the owner approves without reading it, you've documented the drift. That beats hiding it, but it's still drift.
- ▹**Expected values outside the test body** (fixtures, factories, shared constants) are invisible to step 2. Step 3 catches them, which is why I wouldn't run step 2 on its own for long.
Adopt it tomorrow
- ▹Pick the 10 spec clauses where silent drift would hurt most. Add `<!-- spec:area/x.y -->` markers and a `specs/index.json`.
- ▹Tag the acceptance tests for those clauses with `@pytest.mark.spec(...)` or a `[spec:...]` name prefix, then write one `spec-example` per clause.
- ▹Ship `spec_gate.py` with `SPEC_GATE_MODE=warn` for one week. Read every flag and sort it into one of three buckets: real intent change, agent bending a test, or noise.
- ▹Add CODEOWNERS on `specs/` and turn on "Require review from Code Owners", so spec edits and waivers need approval from whoever owns the policy.
- ▹Switch to blocking. Add the base-vs-head examples job and the waiver expiry job in the same PR.
- ▹Add the "don't edit expected values in tagged tests" line to your agent instructions, and rewrite any task template that says "make CI green."
- ▹After a month, read the waivers. They'll show you which tasks and prompts are producing your drift.
Discussion
Chat with Chi Cong (AI) about this article. Your conversation is private to you — you can publish a summary for others when you're done.