Skip to content

chore(promote): develop → main — fr-gate ancestry-only fix - #110

Merged
LukasWodka merged 4 commits into
mainfrom
develop
Jul 30, 2026
Merged

chore(promote): develop → main — fr-gate ancestry-only fix#110
LukasWodka merged 4 commits into
mainfrom
develop

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Promotes the fr-gate fix from #109. Reusable workflows only take effect from main, so this is what makes it live.

What it fixes: the no-items fallback gated the promotion PR itself, which is unsatisfiable by construction — set-pr-status parks that PR in Code review, and the only way out is merging, which the gate blocks. data-ingestors deadlocked on exactly this in today's prod hop and was reported as a governance failure (❌ #423 — Status='Code review') rather than a bug.

An empty item set now splits by what the promotion actually contributes, measured three-dot via the compare API:

ConditionVerdict
0 files contributed and every commit accounted forpass (ancestry-only — nothing shipped)
any unattributable commitfail closed
>0 files but nothing attributedfail closed (gate the promotion PR)
count unavailable (-1)fail closed

Two fail-opens Bugbot caught during review, both real:

  1. The original early exit 0 jumped over the UNATTRIB fail-closed check ~150 lines below it, so a tree-identical range containing an unverifiable commit would have passed. Now requires UNATTRIB empty.
  2. The emptiness check used two-dotgit diff, which counts files the base moved on alone — 6 on the data-ingestors case where the correct three-dot answer is 0. It would not have fixed its own motivating bug. Now uses the compare API, matching how the premise was verified in the first place.

Verified against live repos:data-ingestors compare(master...staging).files = 0 → passes; client compare(main...staging).files = 36 → does not. actionlint + shellcheck clean.


Note

Medium Risk
Changes promotion merge-gating behavior for empty attribution cases; incorrect emptiness detection could pass ungated content or block valid promotions, though the design fails closed on API errors and requires empty UNATTRIB.

Overview
Fixes a deadlock in the reusable FR gate when a release-train promotion has no attributable PRs but also ships no new file content.

The Discover items step now records changed_files using the GitHub compare API with three-dot semantics (base...head), with retries and fail-closed behavior if the count cannot be obtained. The verify step receives CHANGED_FILES and, when the item list is empty, passes only if changed_files is 0 and UNATTRIB is empty—otherwise it still gates the promotion PR or fails on unattributable commits. This separates ancestry-only mirror merges (base already has every byte) from real content or broken attribution, without using two-dot diffs that miscount files on diverged branches.

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

The no-items fallback gated the promotion PR itself, which is
unsatisfiable BY CONSTRUCTION: set-pr-status parks that PR in 'Code
review', and it can only leave that column by merging -- which this gate
blocks. data-ingestors deadlocked on exactly this in today's prod hop.
It happened because the range's only new commit was the train's own
mirror merge, whose content master already held (a manual promotion had
carried it there first): 1 commit ahead, 0 files changed. Both associated
PRs were correctly filtered as train plumbing, leaving no cargo -- so the
fallback fired on a promotion that ships nothing.
Now distinguished by whether the range changes any file:
0 files -> ancestry-only, nothing shipped, nothing to gate -> pass
>0 files -> real content we could not attribute -> gate the promotion
PR, i.e. fail closed and force a human look
An unset file count defaults to the fail-closed branch, so a failure to
compute it can never turn into a pass.
The early exit sat ~150 lines above the UNATTRIB fail-closed check, so a
tree-identical range containing a commit we could not verify (e.g.
commits/{sha}/pulls erroring three times) would have passed the gate --
and my PR body wrongly claimed that check was untouched.
0 files changed means 'nothing shipped' only if every commit was
successfully accounted for, so the ancestry pass now requires UNATTRIB to
be empty as well.
The emptiness check used a two-dot git diff, which counts files the BASE
moved on alone -- so it measured 'how do the two tips differ', not 'what
did this promotion contribute'. On data-ingestors that reports 6 files
(master's own commits that staging lacks) where the correct three-dot
answer is 0, so the ancestry-only branch would never have been taken and
the deadlock this PR exists to fix would have survived.
I had verified the premise with the compare API (three-dot) but
implemented local two-dot git -- validating with one method and shipping
another. Now uses the compare API directly, so the check and its
verification are the same operation.
The API also avoids local three-dot's silent merge-base pick: this repo's
history has multiple merge bases after the mirror reconciliations. Any
failure to count yields -1, which takes the fail-closed branch.
fix(fr-gate): an ancestry-only promotion has nothing to gate
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

👋 Heads-up — Code review queue is at 35 / 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.)

@LukasWodka
LukasWodka merged commit cac7284 into mainJul 30, 2026
6 checks passed
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