Skip to content

fix(fr-gate): gate the diff, not just commit subjects - #72

Merged
LukasWodka merged 2 commits into
fix/1266-fr-gate-fail-closedfrom
fix/1265-gate-the-diff
Jul 26, 2026
Merged

fix(fr-gate): gate the diff, not just commit subjects#72
LukasWodka merged 2 commits into
fix/1266-fr-gate-fail-closedfrom
fix/1265-gate-the-diff

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Closes tracebloc/backend#1265. RFC-BACKEND-0008 D27-L1.

The defect

Item discovery greps commit subjects for (#N):

PRS=$(git log --format='%s'"$BASE_SHA..$HEAD_SHA"| grep -oE '\(#[0-9]+\)|Merge pull request #[0-9]+' ...)

Any commit whose subject lacks a marker — hand-written, rebase-merged, or folded inside a merge —
is invisible. design-system#70 (merged 2026-07-23) shipped 13 commits over 9 source files; one
carried (#67); the gate logged "Items in promotion: 67 → PASSED" and the other 12 commits reached
main unevaluated
.

The fix

An attribution pass over every non-merge commit in BASE..HEAD. A commit is attributable if:

  1. its own subject carries a (#N) / Merge pull request #N, or
  2. it is reachable from the 2nd parent of a merge in range — it entered via a PR merge even if
    its own subject is bare.

Anything left is code that reached the range without a reviewed PR. It now fails the gate closed,
with a message pointing at the override for legitimate release-mechanics commits.

Normal flow is unaffected: squash- and rebase-merges to develop each produce one (#N) commit, so
they attribute by subject.

Verification

Against a git fixture — a subject-ref commit, two commits vouched for by their PR merge, and one
direct-push commit — only the direct push flags. actionlint passes.

Stacking

Based on fix/1266-fr-gate-fail-closed (#71), not develop — both edit fr-gate.yml and would
otherwise conflict. Merge #71 first, then this. After #71 merges, this PR's base auto-retargets;
if GitHub shows a conflict, rebase onto the new develop.

Together, #71 and this close the two halves of how the gate was passing things it never checked: #71
stops it passing on an unreadable item, this stops it passing over an unattributed commit.

Full end-to-end (real promotion + live kanban token) will run in CI; the attribution logic is
fixture-tested above.


Note

Medium Risk
Changes promotion-blocking CI logic for staging/main; incorrect merge ancestry rules could false-fail legitimate promotions, but scope is limited to fr-gate.yml with an existing override label.

Overview
The FR gate no longer relies only on PR numbers scraped from commit subjects. It now scans every non-merge commit in the promotion range and fails closed when any commit cannot be tied to a reviewed PR.

A commit counts as attributable if its subject includes (#N) / Merge pull request #N, or if it was introduced by a merge in range (ancestor of the merge’s second parent but not of the first parent, so base-branch direct pushes are not vouched). Leftover SHAs are emitted as unattributed and block the check alongside blocked/missing/unreadable kanban items, with guidance to open a PR or use skip-fr-gate for legitimate release mechanics.

Squash and rebase merges that already put (#N) in the subject should behave as before; the change targets invisible commits inside merge promotions and other paths without subject markers.

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

Item discovery greps commit SUBJECTS for (#N). Any commit whose subject has no
marker — a hand-written commit, a rebase-merged commit, or the commits folded
inside a merge — was invisible to the gate. design-system#70 shipped 12 such
commits to main under a single blessed (#67): the gate saw one item, passed,
and the other 12 were never evaluated.
Adds an attribution pass over every non-merge commit in BASE..HEAD. A commit is
attributable if its own subject carries a (#N), OR it is reachable from the 2nd
parent of a merge in range (i.e. it entered via a PR merge). Whatever is left is
code that reached the range without a reviewed PR — a direct push, or a release
branch edited after its last PR — and it now fails the gate closed.
Squash- and rebase-merges to develop each produce a single (#N)-bearing commit,
so the normal flow attributes cleanly; only genuinely unreviewed commits flag.
Verified against a git fixture: a subject-ref commit, two commits vouched for by
their PR merge, and one direct-push commit — only the direct push is flagged.
actionlint passes.
Refs RFC-BACKEND-0008 D27-L1 (tracebloc/backend#1265)
@LukasWodkaLukasWodka self-assigned this Jul 26, 2026
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

👋 Heads-up — Code review queue is at 42 / 30

Above the WIP limit. The team convention is to review existing PRs before opening new work.

Open PRs currently in Code review (oldest first):

Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.)

Comment thread.github/workflows/fr-gate.yml
Bugbot on #72: the merge-vouching path treated any ancestor of a merge's 2nd
parent as attributable. A direct push to the base branch becomes an ancestor
of ^2 once a feature branch merges the base in and is then PR-merged, so it was
silently vouched despite never being in a PR.
Tighten to: reachable from ^2 AND NOT from ^1 — i.e. among the commits the
merge actually added. Verified against a fixture reproducing exactly that
pattern: the old check flagged nothing, the new check flags the direct push,
and the normal feature-merge fixture still flags only its own direct push (no
false positives introduced).
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Bugbot finding addressed — real false-negative. The merge-vouch path accepted any ancestor of ^2; a direct push to the base branch becomes an ancestor of ^2 once a feature branch merges the base in, so it slipped through.

Tightened to M^1..M^2 (reachable from ^2and not from ^1 — the commits the merge actually introduced). Verified on a fixture reproducing the exact pattern: old check flagged nothing, new check flags the direct push, and the normal feature-merge case still flags only its own direct push. actionlint green.

bugbot run

@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 dc5466c. Configure here.

@LukasWodka
LukasWodka merged commit e3aff70 into fix/1266-fr-gate-fail-closedJul 26, 2026
2 checks passed
LukasWodka added a commit that referenced this pull request Jul 26, 2026
…ing hole
Bugbot #73: the attribution loop vouched a commit via ANY merge in range whose
M^1..M^2 contained it. A routine 'Merge branch develop into feature' sync merge
is itself in range, and a direct push on develop sits in that sync merge's
S^1..S^2 — so the push was laundered into attribution, reopening the exact
fail-open the #72 ^1 check closed, on merge-commit workflows.
Fix: a merge may vouch only if its own subject carries a (#N) or
'Merge pull request #N'. Sync merges (no PR ref) no longer vouch.
Reproduced with a fixture where the feature branches before the direct push and
syncs develop in: the pre-fix logic MISSES the push (the hole), the fix FLAGS
it. Earlier fixtures (basic direct push, feature-absorbs-base) still flag
correctly. actionlint clean.
Refs RFC-BACKEND-0008 D27-L1 (tracebloc/backend#1265)
LukasWodka added a commit that referenced this pull request Jul 26, 2026
* fix(fr-gate): gate the diff, not just commit subjects (re-land #72 onto develop)
#72 was reviewed and merged, but into its stacked base (fix/1266) rather than
develop — #71 merged to develop first and the child was never retargeted, so
the attribution code never reached develop (stacked-PR merge-order hazard).
This ports exactly that delta onto develop: the only change vs develop's
current fr-gate.yml is the additive attribution block + its wiring (verified by
diff — all additions, nothing removed). develop already carries #71's
fail-closed + robustness fixes via the earlier squash.
Attribution: every non-merge commit in BASE..HEAD must carry a (#N) or be
introduced by a PR merge (in M^1..M^2), else the gate fails closed. Fixture-
tested incl. the Bugbot #72 false-negative (direct push absorbed by a feature
branch). actionlint clean.
Refs RFC-BACKEND-0008 D27-L1 (tracebloc/backend#1265), re-lands #72
* fix(fr-gate): only PR-merges may vouch — close the sync-merge laundering hole
Bugbot #73: the attribution loop vouched a commit via ANY merge in range whose
M^1..M^2 contained it. A routine 'Merge branch develop into feature' sync merge
is itself in range, and a direct push on develop sits in that sync merge's
S^1..S^2 — so the push was laundered into attribution, reopening the exact
fail-open the #72 ^1 check closed, on merge-commit workflows.
Fix: a merge may vouch only if its own subject carries a (#N) or
'Merge pull request #N'. Sync merges (no PR ref) no longer vouch.
Reproduced with a fixture where the feature branches before the direct push and
syncs develop in: the pre-fix logic MISSES the push (the hole), the fix FLAGS
it. Earlier fixtures (basic direct push, feature-absorbs-base) still flag
correctly. actionlint clean.
Refs RFC-BACKEND-0008 D27-L1 (tracebloc/backend#1265)
@LukasWodka
LukasWodka deleted the fix/1265-gate-the-diff branch August 1, 2026 21:45
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.

1 participant

@LukasWodka