Uh oh!
There was an error while loading. Please reload this page.
fix(fr-gate): gate the diff — re-land #72 onto develop - #73
Merged
Conversation
…to 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
LukasWodka
commented
Jul 26, 2026
ContributorAuthor
bugbot run |
LukasWodka
commented
Jul 26, 2026
ContributorAuthor
👋 Heads-up — Code review queue is at 41 / 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.) |
Uh oh!
There was an error while loading. Please reload this page.
…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
commented
Jul 26, 2026
ContributorAuthor
bugbot run |
There was a problem hiding this comment.
✅ 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 815605b. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka added a commit
that referenced
this pull request
Jul 27, 2026
commits/{sha}/pulls lists EVERY PR whose branch contains a commit, so a sync
merge ("Merge develop into feature") was listed for the feature PR. The old
non-empty check let that sync merge enter MERGES and mark a direct-push-to-base
it happened to pull in as "covered", skipping Pass 2 and reopening the Bugbot
#73 fail-open hole. Require merge_commit_sha==sha so only true PR merges vouch;
any other merge drops its payload to Pass 2 for individual attribution.
Verified on real history: backend sync merge ba80b3a yields no merge_commit_sha
match (drops to Pass 2), while every 'Merge pull request #N' matches #N.
Addresses Bugbot 'Sync merges reopen attribution hole'.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>LukasWodka added a commit
that referenced
this pull request
Jul 27, 2026
…90) * fix(fr-gate): attribute items via commits/pulls, not commit subjects Replaces the commit-subject '(#N)' scrape (both the item list and the attribution vouch) with GitHub's authoritative commit->PR attribution (repos/{repo}/commits/{sha}/pulls). The scrape was unreliable three ways under this org's '<subject> (#issue) (#PR)' convention: 1. it grabbed ISSUE numbers -> pullRequest() NOT_FOUND -> gate failed CLOSED (backend#1228: 16 issue-refs, e.g. #1005); 2. it grabbed bare '#N' MENTIONS of unrelated PRs (#234/#472 style); and 3. it MISSED a commit's real PR when the (#PR) wasn't in the subject (rebase-merges / issue-only squash subjects) -> a naive NOT_FOUND-skip then failed OPEN, since UNATTRIB counts an (#issue) subject as attributed and the real PR is never gated (Bugbot High on the first cut of this PR). commits/{sha}/pulls returns the actual MERGED PR(s) that introduced each commit -- the single source of truth for both PRS and UNATTRIB. One call per commit not already vouched by a PR-merge (M^1..M^2); retry; fail CLOSED on a persistent API error. The --depth=200 base fetch bounds the commit set and the call count. Validated against backend#1228's exact failing range (staging 0e4af47d .. develop 8ddbbef2, 41 non-merge commits): yields 39 real merged PRs, ZERO issue-ref leaks, ZERO false-unattributable, drops the #234/#472 mention-noise, and correctly keeps #1191 (a genuine containing PR). actionlint clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(fr-gate): only true PR-merges vouch (match merge_commit_sha) commits/{sha}/pulls lists EVERY PR whose branch contains a commit, so a sync merge ("Merge develop into feature") was listed for the feature PR. The old non-empty check let that sync merge enter MERGES and mark a direct-push-to-base it happened to pull in as "covered", skipping Pass 2 and reopening the Bugbot #73 fail-open hole. Require merge_commit_sha==sha so only true PR merges vouch; any other merge drops its payload to Pass 2 for individual attribution. Verified on real history: backend sync merge ba80b3a yields no merge_commit_sha match (drops to Pass 2), while every 'Merge pull request #N' matches #N. Addresses Bugbot 'Sync merges reopen attribution hole'. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-lands #72 onto develop
#72 (gate-the-diff) was reviewed and merged, but into its stacked base
fix/1266, notdevelop.#71 merged to develop first; the child PR was never retargeted, so its attribution code landed on the
now-orphaned
fix/1266branch and never reached develop. Verified:UNATTRIBappears 0 times indevelop's
fr-gate.yml.This ports exactly that delta. The diff against develop's current
fr-gate.ymlis purely additive— the attribution block plus its
UNATTRIBwiring — because develop already carries #71's fail-closedand robustness fixes (via the earlier squash). Nothing is removed or re-applied.
What it does (unchanged from #72)
Every non-merge commit in
BASE..HEADmust carry a(#N)or be introduced by a PR merge (inM^1..M^2), else the gate fails closed. Includes the Bugbot #72 fix — vouch only commits the mergeactually added, so a direct push absorbed by a feature branch is still flagged. Fixture-tested,
actionlint clean.
Process note
This is the stacked-PR merge-order hazard: when a stack's base merges first, the child must be
retargeted to the new base before it merges, or it merges into the wrong branch. I should have
retargeted #72 the moment #71 merged. Closing #72's branch after this lands.
Note
Medium Risk
Changes required promotion-gate logic for staging/main merges; incorrect git attribution could block valid releases, though
skip-fr-gateremains an audited override.Overview
Extends the FR promotion gate so it no longer trusts PR numbers parsed only from commit subjects. Every non-merge commit in
BASE..HEADmust either reference a PR in its subject or be vouched by a PR merge in range (commits introduced onM^2but not already onM^1).Sync merges like
Merge branch 'develop' into featurecannot vouch for commits, closing a path where direct pushes could be laundered through feature-branch merges. Unattributed SHAs are exported asunattributedand fail the gate closed alongside blocked/missing/unreadable kanban items, with guidance to open a PR or useskip-fr-gate.This re-lands the #72 “gate the diff” behavior onto
develop(additive on top of existing fail-closed kanban checks).Reviewed by Cursor Bugbot for commit 815605b. Bugbot is set up for automated code reviews on this repo. Configure here.