Skip to content

ci(gate): fail a PR that declares itself blocked (backend#1729) - #229

Merged
LukasWodka merged 3 commits into
developfrom
feat/blocked-gate
Aug 12, 2026
Merged

ci(gate): fail a PR that declares itself blocked (backend#1729)#229
LukasWodka merged 3 commits into
developfrom
feat/blocked-gate

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

data-ingestors#468 was titled:

sec(#1528): require DB_USER/DB_PASSWORD, drop the edgeuser fallback (D10) [blocked on S2]

It collected three approvals and was merged by its author 105 seconds after S2 merged. The author did wait for the blocker they had written down — but "S2 merged" was never the real precondition (SERVICE_DB_ACCOUNTS on fleet-wide was), and nothing anywhere could tell the difference. dev and staging ingestion broke within hours and stayed broken for a day (backend#1752).

The dependency was recorded as prose, in a place nothing reads. This PR makes that sentence a merge gate: a PR whose title or labels say it is blocked fails a check.

Two open PRs carry the same shape right now:

PRTitle
client#490chore(chart): … — HOLD until v0.8.0 image
client-runtime#192DO NOT MERGE: fix(training): add configurable activeDeadlineSeconds

What it does not do

It cannot tell whether the blocker is genuinely resolved — the #468 author satisfied the sentence they wrote and the real condition was still unmet. The only claim is that clearing the marker is a deliberate act by someone who has looked. This is not "PRs are never merged early".

Design notes

  • The matching is the entire risk surface, so it lives in a tested script (scripts/blocked-marker.py), not a YAML heredoc — the same lesson as backend#1746 from the other direction.
  • Precision over recall. A missed marker costs what we have today; a false positive costs the credibility of the gate. One that fires on somebody's "unbreak dev + staging ingestion" title gets called noise, gets switched off inside a week, and then catches nothing forever. house-rules.sh names that failure mode in its own design notes. The (?<![a-z]) lookbehinds are what kill unblocked, unblocks, blocker, threshold, holder, wipe and swipe.
  • The blocked label is the zero-false-positive half; the title markers catch the unlabelled case — which is exactly what #468 was.
  • No override label, deliberately. fr-gate has skip-fr-gate because satisfying it can take a release cycle. Here the fix is editing your own title, so an escape hatch would only ever be used to skip the thinking.
  • Injection: the PR title never transits the shell. No ${{ github.event.pull_request.title }} appears anywhere — the script reads GITHUB_EVENT_PATH itself.
  • edited is in the caller trigger list on purpose: the gate reads the title, so a title edited to add or remove a marker must re-evaluate, or "I removed the marker" would never turn the check green.

No callers in this PR — on purpose

The reusable and its selftest land first, so the matching can be reviewed on its own. repo-inventory.yml records it exempt for all 20 repos with the rollout named, because that file's own header forbids adding a caller and flipping its entry to required in one PR: the .github caller state is read from its audit branch over the API, so the caller would not be there yet.

The exemption text says STAGED, NOT PARKED in so many words, and says that if the anchor is still present after the rollout PR, it has become the same finding as wip_limit_check_has_no_callers and version_bump_pr_never_wired — a reusable that shipped, was never wired up, and had a written excuse for it.

Arming is a briefing decision, not a silent flip: the two PRs in the table above go red the moment it is on. That is the gate working as intended, but it should not arrive as a surprise.

devex-bootstrap gets its own exemption rather than the shared anchor — it has no .github directory at all, so unlike the other 19 it does not clear when the rollout lands.

Test plan

CheckResult
scripts/tests/blocked-marker-selftest.py22 checks green
— real blocked titles#468, client#490, client-runtime#192, copied verbatim
— false friendsunblocked, unblocks, blocker, blocking, threshold, holder, household, wipe, swipe
— the GITHUB_EVENT_PATH path the workflow actually usescovered (it had none until I noticed)
edited payload reads the new title, not changes.fromcovered
Mutation-checked: drop the title from _from_event / drop the labelseach produces 1 failure, so neither assertion is vacuous
scripts/tests/caller-drift-selftest.py123 pass / 0 fail
load_inventory + check_source_reusables on the edited inventorypass — and fail as intended when the reusables row is removed
actionlintclean

All of the above re-run after rebasing onto develop, which carries the newer caller-drift.py from #222 and #223.

Ref: backend#1729, backend#1752, data-ingestors#468.

🤖 Generated with Claude Code


Note

Medium Risk
Introduces org-wide merge-gate matching logic that will block PRs once rolled out; false positives could force the gate off. Callers are deliberately not wired in this PR, so merge behavior is unchanged until the follow-up.

Overview
Adds a blocked merge gate so a PR that announces its own blocker (title markers like [blocked on X], DO NOT MERGE, HOLD until, WIP:, or the org blocked label) fails a check. Motivated by data-ingestors#468, which merged with three approvals while still blocked in prose.

Matching is the whole risk surface, so it lives in tested scripts/blocked-marker.py rather than YAML. Patterns favor precision over recall (lookbehinds reject false friends like unblocked, blocker, threshold, wipe). The reusable never interpolates the PR title into the shell — it reads GITHUB_EVENT_PATH. No override label by design.

No callers in this PR.blocked-gate.yml and its selftest land first for review; repo-inventory.yml lists the reusable as exempt: *blocked_gate_rollout_pending across repos (with a separate exemption for devex-bootstrap). Callers and arming follow in the rollout PR.

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

data-ingestors#468 was titled "... (D10) [blocked on S2]". It collected three
approvals and was merged by its author 105 seconds after S2 merged. The author
waited for the blocker they had written down -- but "S2 merged" was never the
real precondition, and nothing could tell the difference. dev and staging
ingestion broke within hours and stayed broken for a day (backend#1752).
The dependency was recorded as prose, in a place nothing reads. This makes that
sentence a merge gate: a PR whose title or labels say it is blocked fails a
check. Two open PRs carry the shape today -- client#490 ("HOLD until v0.8.0
image") and client-runtime#192 ("DO NOT MERGE").
What it does not do: decide whether the blocker is genuinely resolved. #468's
author satisfied the sentence they wrote and the real precondition was still
unmet. The claim is only that clearing the marker is a deliberate act.
The matching is the entire risk surface, so it lives in a tested script rather
than a YAML heredoc. Precision over recall: `unblocked`, `unblocks`, `blocker`,
`threshold`, `holder` and `wipe` must not fire, or the gate gets called noise
and switched off -- the failure mode house-rules.sh names in its own notes.
No callers in this PR, deliberately. The reusable lands first so the matching
can be reviewed alone; repo-inventory.yml records it `exempt` everywhere with
the rollout named, because this file's own header forbids adding a caller and
flipping it to `required` in one PR. Arming is a briefing decision: two live
PRs go red the moment it is on, which is the gate working.
Verified: marker selftest 22 checks green, including the GITHUB_EVENT_PATH path
the workflow actually uses; both event-path assertions mutation-checked (drop
the title / drop the labels -> each fails). caller-drift selftest 116/0.
load_inventory + check_source_reusables pass on the edited inventory, and fail
as intended when the reusables row is removed. actionlint clean.
@LukasWodkaLukasWodka self-assigned this Aug 12, 2026
…itive
Ran the matcher over 588 merged PR titles from ten tracebloc repos rather than
trusting the table I had imagined. It produced exactly one false positive:
chore(ci): retire the WIP-limit nudge
and it is the worst kind, because "WIP limit" is a domain term in THIS org --
wip-limit-check.yml is one of our own reusables, so the phrase recurs. The
(?<![a-z])/(?![a-z]) guards do not help: the character after "WIP" is a hyphen.
That is precisely the failure this gate cannot afford. A check that fires on a
normal chore PR gets called noise and switched off, and then catches nothing
ever again.
WIP-as-a-marker is a PREFIX convention (`WIP: ...`, `[WIP] ...`); WIP mid-title
is the noun. Anchoring to the start keeps every real marker and drops the false
positive. Over the same 588 titles the matcher now blocks exactly one:
data-ingestors#468 -- the PR this gate exists for.
Both new cases mutation-checked: un-anchoring `wip` fails 2 assertions.
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Measured the false-positive rate instead of asserting it — and it found one

I claimed "precision over recall" in the description, so I went and checked it against reality rather than against the table I had imagined. Ran the matcher over 588 merged PR titles from ten tracebloc repos (backend, client, client-runtime, data-ingestors, cli, tracebloc-engine, .github, release-train, e2e-test-agent, frontend-app).

It found a real false positive, and the bad kind:

chore(ci): retire the WIP-limit nudge

"WIP limit" is a domain term in this orgwip-limit-check.yml is one of our own reusables — so the phrase recurs. The (?<![a-z]) / (?![a-z]) guards do not help, because the character after WIP is a hyphen.

That is exactly the failure this gate cannot afford: fire once on a normal chore PR, get called noise, get switched off, catch nothing ever again.

Fix (571d8c0): WIP-as-a-marker is a prefix convention (WIP: …, [WIP] …); WIP mid-title is the noun. Anchored wip to the start of the title; the other four markers are unchanged, since none of them showed a false positive.

beforeafter
titles blocked, of 58861
distinct titles blocked21

The one remaining hit is data-ingestors#468 — the PR this gate exists for.

Both new assertions mutation-checked: un-anchoring wip fails 2 of them.

Also verified against the live PRs, by API rather than by transcription

PRtitle as fetchedverdict
data-ingestors#468sec(#1528): … (D10) [blocked on S2]blocked (exit 1)
client#490chore(chart): … — HOLD until v0.8.0 imageblocked (exit 1)
client-runtime#192DO NOT MERGE: fix(training): … (WS6)blocked (exit 1)

Note client-runtime#192 ends in (WS6) — it is caught by do-not-merge, not by the wip marker, so the anchoring change does not weaken it.

Comment threadscripts/blocked-marker.py
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

Comment thread.github/workflows/blocked-gate.yml Outdated
Comment thread.github/workflows/blocked-gate.yml Outdated
…imal perms; label events (Bugbot #229)
- blocked-marker.py: a GITHUB_EVENT_PATH payload with no pull_request object now
refuses (exit 2) instead of returning an empty title -> 'not blocked' -> exit
0. A non-PR or malformed event was a silent fail-OPEN; it now fails closed like
the no-event-path branch. Selftest case added.
- blocked-gate.yml: drop 'pull-requests: read' — the script reads
GITHUB_EVENT_PATH, never the API, so the wider grant would exceed a minimal
contents:read caller and fail the reusable at startup.
- Document that callers must trigger on labeled/unlabeled too, or the label half
of the gate never re-runs until the next synchronize.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 36edecf. Configure here.

@LukasWodka
LukasWodka merged commit fb2d462 into developAug 12, 2026
12 checks passed
@LukasWodka
LukasWodka deleted the feat/blocked-gate branch August 12, 2026 12:25
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@saadqbal