Skip to content

feat(ci): required conformance gate — a contract change cannot merge on a failed audit - #173

Merged
LukasWodka merged 4 commits into
developfrom
ci/1608-conformance-gate
Aug 6, 2026
Merged

feat(ci): required conformance gate — a contract change cannot merge on a failed audit#173
LukasWodka merged 4 commits into
developfrom
ci/1608-conformance-gate

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

First increment of backend#1608.

The hole

caller-drift.yml has run on PRs touching repo-inventory.yml since #1415, and it correctly fails when the inventory disagrees with reality. But it is not a required status check.github/develop requires only actionlint. So a PR that adds a repo to the train with drift goes red and can still be merged. The gate existed in trigger form with no teeth.

That is the concrete version of the worry driving #1608: we keep adding repos to the train, and nothing stops one arriving that doesn't meet the contract.

Why not just mark caller-drift required

It carries a paths: filter. A path-filtered required check never reports on a PR that misses the filter, leaving it pending forever — the trap actionlint.yml's own header documents. Dropping the filter would run a 20-repo API audit on every PR in this repo, which is exactly why the filter exists.

So this adds a job that always runs and always reports:

  • PR doesn't touch the contract → green in seconds, zero audit API calls
  • PR does touch it → requires caller-drift's verdict for that exact head sha

Two details that are load-bearing

It keys on the workflow file, not the check name. Check-run names are not unique across workflows — caller-drift.yml and standards-sync.yml both expose a job called selftest, so polling by name would happily accept the wrong workflow's verdict. actions/workflows/caller-drift.yml/runs?head_sha=… cannot be confused that way.

--paginate on the changed-files query. A PR over one page would otherwise look like it touches only its first 30 files, and a truncated read reporting "not guarded" is precisely the silent pass this file exists to prevent. An empty list is treated as unreadable, not as "no files".

Every non-success branch is red with a named reason: no run within the poll budget, non-success conclusion, unreadable API, unreadable file list.

Verification

Path matching is exact (grep -qxF), dry-run offline:

README.md -> not guarded
repo-inventory.yml -> GUARDED
scripts/caller-drift.py + README-> GUARDED
docs/repo-inventory.yml -> not guarded <- suffix/prefix cannot sneak in
repo-inventory.yml.bak -> not guarded

This PR edits repo-inventory.yml's header on purpose, so it exercises its own guarded path rather than shipping a gate whose interesting branch never ran. Expect the gate to wait for caller-drift here and pass on its success — which also proves the poll works against a real concurrent run. caller-drift has been green on all five most recent runs, so arming imports no backlog.

After merge

Add gate to .github/develop's required checks. Not done in this PR: the check must exist on the base branch before it can be required, or every open PR blocks on a status that will never report.

Parent backend#1405.


Note

Medium Risk
Changes merge policy for contract files and depends on GitHub Actions API polling; misconfiguration could block PRs or allow bypass if not added to required checks after merge.

Overview
Adds .github/workflows/conformance-gate.yml, a PR check meant to be required so inventory/contract edits cannot merge while caller-drift only ran optionally. The job always reports: if the PR does not touch guarded paths it exits green with no audit API calls; if it touches repo-inventory.yml, scripts/caller-drift.py, related selftests, or caller-drift.yml, it pollsactions/workflows/caller-drift.yml/runs?head_sha=… until that workflow completes with success on the PR head (40m poll budget, 45m job timeout).

Guard detection is fail-closed: paginated changed-files (current and previous names for renames), empty/unreadable file lists fail, and guarded-path matching uses here-string grep instead of a pipe to avoid SIGPIPE false “not guarded” greens on large PRs.

repo-inventory.yml documents the new gate in its header and flips .githubcode-quality.yml from exempt to required, reflecting that the hosting repo now runs the code-quality caller.

Reviewed by Cursor Bugbot for commit 2022566. Bugbot is set up for automated code reviews on this repo. Configure here.

…on a failed audit
caller-drift.yml has run on PRs touching repo-inventory.yml since backend#1415,
and it correctly fails when the inventory disagrees with reality. But it is not
a required status check: .github/develop requires only actionlint. So a PR that
adds a repo to the train with drift went RED and could still be merged. The gate
existed in trigger form and had no teeth.
It cannot simply be marked required as-is. It carries a paths: filter, and a
path-filtered required check never reports on PRs that miss the filter, leaving
them pending forever - the trap actionlint.yml's own header documents. Dropping
the filter would instead run a 20-repo API audit on every PR in this repo, which
is the reason the filter is there.
So this adds a job that ALWAYS runs and ALWAYS reports. On a PR that does not
touch the contract it is green in seconds with zero audit API calls. On a PR
that does, it requires caller-drift's verdict for that exact head sha.
It keys on the WORKFLOW FILE, not the check name. Check-run names are not unique
across workflows - caller-drift.yml and standards-sync.yml both expose a job
called selftest - so polling by name would happily accept the wrong workflow's
verdict. Querying actions/workflows/caller-drift.yml/runs?head_sha=... cannot be
confused that way.
Fail-closed on every non-success branch, each with a named reason: no run found
within the poll budget, a non-success conclusion, an unreadable API, or a
changed-file list that came back empty (unreadable, not "no files"). The
--paginate on the files query matters: a PR over one page would otherwise look
like it touches only its first 30 files, and a truncated read reporting "not
guarded" is precisely the silent pass this file exists to prevent.
The path match is exact (grep -qxF), verified offline against the cases that
matter: docs/repo-inventory.yml and repo-inventory.yml.bak do NOT count as
touching the contract.
This PR edits repo-inventory.yml's header on purpose, so it exercises its own
guarded path rather than shipping a gate whose interesting branch never ran.
Refs backend#1608, backend#1415.
Caught by the gate this PR adds, on its own first run. The audit reported:
.github: code-quality.yml is marked `exempt` but a caller exists
(code-quality-caller.yml). The exemption is stale.
Correct, and the drift is an hour old: .github#171 landed the caller for
backend#1603 and never flipped this entry. That is precisely the follow-up
caller-drift.py's docstring tells you to do -- "land the caller first, flip
the entry in a follow-up" -- because the inventory is read from the checkout
while repo state is read from the audit branch, so doing both in one commit
fails.
So the sequence worked exactly as designed: the caller landed, the entry went
stale, and the very next inventory-touching PR was refused until it was
reconciled. That is the whole point of the gate, demonstrated on itself
rather than asserted in a comment.
The exemption text is replaced with the remediation history rather than
deleted, matching how the pii-gate and e2e-test-agent entries record theirs --
the reason the exemption existed is evidence, not noise.
Comment thread.github/workflows/conformance-gate.yml Outdated
…hit into a miss
Bugbot, .github#173. Reproduced before fixing:
20,000 filenames (~349 KB) with the guarded path FIRST
printf '%s\n' "$FILES" | grep -qxF repo-inventory.yml
-> pipeline rc 141, `if` takes the else branch, gate reports NOT GUARDED
grep -q exits on its first match and closes the pipe. With enough left to
write, printf takes SIGPIPE and exits 141; pipefail then makes the PIPELINE
141, so a real match reads as a miss. The gate would report "not guarded" and
pass GREEN on precisely the PRs most likely to matter -- the failure gets MORE
likely as the diff gets bigger, and it fails OPEN, which is the one direction
this file exists to rule out.
A here-string is a single command, so the status is grep's own. Verified: same
input, same guarded path, correctly GUARDED.
Swept the rest of the repo for the pattern. fr-gate.yml has two instances
(`echo "$LABELS" | grep -q ...`), and both are safe on two independent counts:
a PR's label JSON is orders of magnitude under the 64 KB pipe buffer, so echo
finishes before grep can exit; and a false negative there means the gate does
NOT skip, i.e. it fails CLOSED. Same pattern, opposite consequence -- left
alone deliberately rather than changed for symmetry.
The `| head -1` chains in version-bump-gate.yml and the kanban workflows are
likewise safe: each is either terminated by `|| true` or fed by output far
under the buffer.
Refs backend#1608, backend#1409 (same defect class in the PII gate).
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Confirmed and fixed in c4c4480. Reproduced before fixing, because a claim about SIGPIPE deserves a measurement:

20,000 filenames (~349 KB), guarded path FIRST
printf '%s\n' "$FILES" | grep -qxF repo-inventory.yml
-> pipeline rc 141
-> if-branch: NOT GUARDED <- silent pass
grep -qxF repo-inventory.yml <<< "$FILES"
-> if-branch: GUARDED <- correct

Exactly as described: grep -q exits on first match and closes the pipe, printf takes SIGPIPE and exits 141, and pipefail promotes that to the pipeline's status, so a real hit reads as a miss.

What makes it worse than a normal bug and worth the Medium: the gate would report "not guarded" and pass green, and the failure gets more likely as the diff gets bigger — so it would have been most wrong on exactly the large refactor PRs where an inventory change is most likely to be buried. It fails open, the one direction this file exists to rule out. A here-string is one command, so the status is grep's own.

I swept the rest of the repo for the pattern rather than fixing only the reported line. Two instances in fr-gate.yml:

ifecho"$LABELS"| grep -q '"skip-fr-gate"';thenifecho"$LABELS"| grep -q '"hotfix"';then

Both are safe, on two independent counts, and I left them alone deliberately rather than changing them for symmetry:

  1. A PR's label JSON is orders of magnitude below the 64 KB pipe buffer, so echo finishes writing before grep can exit — SIGPIPE never fires.
  2. Even if it did, a false negative there means the gate does not skip. It fails closed. Same pattern, opposite consequence.

The | head -1 chains in version-bump-gate.yml and the kanban workflows are likewise safe — each is either terminated by || true or fed by output far under the buffer.

Good find. It survived my own review because I was looking at the logic of the match and not at what pipefail does to a pipeline whose consumer exits early.

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c4c4480. Configure here.

Comment thread.github/workflows/conformance-gate.yml
Comment thread.github/workflows/conformance-gate.yml Outdated
…at it waits for
Two Bugbot findings on .github#173.
HIGH - a rename escaped the contract guard. The changed-file probe read only
.filename, and a rename reports the NEW path there. So `git mv
repo-inventory.yml elsewhere.yml` sailed through as "not guarded": the
contract file moves out from under the audit and the gate waves it past -
precisely the silent pass this workflow exists to close. Now reads BOTH
.filename and .previous_filename, which is the same reason promote-repo.sh's
publishable_delta matches both fields (a file renamed OUT of a published tree
is still a publish-path change). I had read that comment while building the
version-bump gate and failed to carry it across.
Proven on a synthetic rename payload, since no open PR has one:
old jq (.filename only) -> NOT GUARDED <- the hole
new jq (+ .previous_filename) -> GUARDED
and verified against a real PR that the `// empty` guard emits nothing extra
when previous_filename is absent.
MEDIUM - the poll budget was shorter than the thing it waits for. 40x30s = 20
minutes, while caller-drift's worst case is selftest (timeout 5m) + audit
(timeout 30m) = 35m. A legitimately slow audit would have made the gate go red
as though no verdict ever arrived, blocking a contract change until someone
re-ran it by hand. Fail-closed is right; fail-closed on a budget that cannot
cover the wait is just fail-annoying, and a gate people have to re-run by hand
is a gate they campaign to remove. Budget is now 80x30s = 40m with the job
timeout raised to 45m so the named error message surfaces rather than an
opaque job timeout.
Refs backend#1608.
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Both fixed in 2022566. Both were real, and the first one is the better catch of the two.

Rename escaping the guard (High) — confirmed

I read only .filename, and a rename reports the new path there. So git mv repo-inventory.yml elsewhere.yml sailed through as "not guarded" — the contract file moves out from under the audit and the gate waves it past. Exactly the silent pass this workflow exists to close, via a path I did not think about.

Proven on a synthetic rename payload, since no open PR has one:

old jq .filename only -> NOT GUARDED <- the hole
new jq + .previous_filename -> GUARDED

and verified against a real PR that (.previous_filename // empty) emits nothing extra when the field is absent, so ordinary PRs are unaffected.

You are right that the precedent was already here: promote-repo.sh's publishable_delta matches both fields, because a file renamed out of a published tree is still a publish-path change. I read that exact comment yesterday while building the version-bump gate and did not carry it across.

Poll budget shorter than the audit (Medium) — confirmed, arithmetic checked

gate budget 40 x 30s = 20 min
caller-drift selftest (timeout 5m) + audit (30m) = 35 min worst case

So a legitimately slow audit would have made the gate go red as though no verdict ever arrived, blocking a contract change until someone re-ran it by hand. Now 80 × 30s = 40 min, job timeout raised 25 → 45 so the named error surfaces instead of an opaque job timeout.

Worth stating the principle, because it generalises to every fail-closed poll we have: fail-closed on a budget that cannot cover the wait is just fail-annoying — and a gate people must re-run by hand is a gate they campaign to remove. The budget has to exceed the worst case of the thing it waits on, and it should be re-derived whenever that thing's timeout changes.

That is three genuine findings from Bugbot on this PR — SIGPIPE, rename, budget — all of which survived my own review. The common thread is that I checked the logic of each step and not the behaviour of the machinery underneath it: what pipefail does to an early-exiting consumer, what the files API reports for a rename, and what the thing I poll can actually take.

@LukasWodka
LukasWodka merged commit 3b650a6 into developAug 6, 2026
10 of 12 checks passed

@saqlainsyed007saqlainsyed007 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid, well-reasoned gate — the fail-closed discipline (pagination, previous_filename for renames, here-string vs. SIGPIPE, absence≠success) is exactly right, and editing repo-inventory.yml here so the interesting branch actually runs is a nice touch.

Two issues to address (details inline):

  1. GUARDED and caller-drift.yml's paths: are two copies of one invariant with nothing keeping them in sync. Both divergence directions are harmful — one is the exact silent-pass this file exists to prevent.
  2. The poll's real wall-clock exceeds the nominal 40m budget (80 sleeps + 80 API round-trips + one wasted trailing sleep), narrowing the 40→45m margin the design leans on so the "opaque job timeout" it tried to avoid can still fire first.

Requesting changes on #1 in particular.

HEAD_SHA: ${{ github.event.pull_request.head.sha }}
# Changing any of these changes what the org's repos are contractually
# required to have, so the audit must have passed on this exact head.
GUARDED: |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole gate's correctness rests on GUARDED being identical to caller-drift.yml's pull_request.paths:, and nothing enforces that. They are two hand-maintained copies of one invariant:

repo-inventory.yml
scripts/caller-drift.py
scripts/tests/caller-drift-selftest.py
.github/workflows/caller-drift.yml

They happen to match today, but a future edit to either list silently breaks the gate in one of two ways:

  • Path in caller-drift.yml but not in GUARDED → a PR touching only that path runs the audit but the gate reports "not guarded" and passes green without waiting. That is precisely the silent pass this file was written to prevent.
  • Path in GUARDED but not in caller-drift.yml → the gate declares the PR guarded, polls for a run that will never be triggered, and fails closed after 40m, blocking the PR with no way to make it go green.

Given how carefully every other branch here is made fail-closed, this unguarded coupling is the weak point. Suggest at minimum a cross-referencing comment in both files ("keep in lockstep with …"), and ideally an assertion in caller-drift-selftest.py that the two sets are equal — or derive GUARDED from the workflow's paths: so there is a single source of truth.

fi
i=$((i + 1))
echo " audit status='${STATUS:-<no run yet>}' - waiting (${i}/${POLL_TRIES})"
sleep "${POLL_SECONDS}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two timing issues in the poll loop.

  1. Wasted trailing sleep. On the 80th attempt the body increments i to 80, prints waiting (80/80) as if it will keep waiting, then sleeps 30s before the while condition finally fails. That last sleep buys nothing and the message is misleading. Guard it, e.g. [ "$i" -lt "${POLL_TRIES}" ] && sleep "${POLL_SECONDS}".

  2. Real budget > nominal 40m. The comment above sizes the budget as 80×30s = 40m and sets timeout-minutes: 45 specifically "so the budget's own error message is what a caller sees rather than an opaque job timeout." But wall-clock is 80 sleeps plus 80 gh api round-trips (plus the wasted sleep above) — comfortably 42–43m+ and variable under API latency. Against caller-drift's stated 35m worst case with any queueing, the 45m job timeout can trip before the poll budget's named error fires, defeating the exact reason the timeout was set above the budget. Either lower POLL_TRIES so real wall-clock stays under 45m with margin, or raise timeout-minutes to sit clearly above the true worst case.

@LukasWodka
LukasWodka deleted the ci/1608-conformance-gate branch August 14, 2026 13:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@LukasWodka@saqlainsyed007