fix(hybrid-gate): diff the docs-only changeset from the merge-base, not the base tip - #40
Merged
Merged
Conversation
…ot the base tip
`git diff A..B` is not range notation — it is a plain comparison of two
endpoints, identical to `git diff A B`. The docs-only step fed it
`origin/${BASE_REF}..HEAD`, so against a base that keeps moving it reported
every file main changed since the branch forked, not the files the PR
proposes. A PR touching only README.md is classified NOT docs-only the moment
an unrelated .rs commit lands on main.
Measured on forkwright/kanon's `docs/2254-entry-point-truth`, which changes 2
files: the two-dot range yields 39 files, 37 of them non-doc. Three-dot yields
2. The exemption can therefore only ever fire on a branch that is exactly
up to date with its base, which is not a property any PR holds for long.
The consequence differs by gate shape. Here the PR falls through to
full-gate-build, so the cost is a wasted build. In a trailer-only gate with no
build fallback the same expression hard-blocks the PR: kanon's standalone
gate-attestation.yml carries the identical two-dot expression at line 52 and
exits 1 with "No Gate-Passed trailer found" for a docs-only PR that its own
exemption was written to let through.
Only the pull_request arm changes. The push arms are correct as two-dot:
before..after is precisely what that push changed, and all three were
re-checked against a fixture (event.before present, all-zeros, and absent) and
resolve unchanged.
Verified by extracting this workflow's own range-selection block and running it
against a fixture PR whose branch touches only README.md while the base moved
ahead with a code change:
pre-fix (origin/main): range=origin/main..HEAD files=[README.md code.rs] docs_only=false rc=1
post-fix (this branch): range=origin/main...HEAD files=[README.md] docs_only=true rc=0
The permanent assertion belongs in scripts/check_event_shape_guards.py, which
scans for pull_request-only values reaching git and would cover this class
directly. That script is still unmerged on fix/gate-push-event-trailer-sha, and
wiring it from here would collide with that branch in actionlint.yml, so this
change is scoped to the defect.
Follows up #25, which introduced the event-derived range.
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 free
to 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.
Finding
hybrid-gate.ymlcomputes its docs-only verdict withgit diff origin/${BASE_REF}..HEAD— two dots.Despite looking like range notation,
git diff A..Bis a plain comparison of two endpoints, soagainst a moving base it reports every file the BASE changed since the branch forked, not the
changeset the PR proposes.
A PR touching only
README.mdis therefore classified not docs-only the momentmainlands anunrelated Rust commit — which on an active repo is more or less immediately.
Evidence
.github/workflows/hybrid-gate.yml— the pull_request arm buildsrange="origin/${BASE_REF}..HEAD"and the next line does
changed=$(git diff --name-only "$range").Reproduced in an isolated repo: branch
featurechanges onlyREADME.md; base then gains anunrelated
lib.rscommit.main..feature(today)README.md,lib.rsmain...feature(this PR)README.mdlib.rsis a file the PR never touched. Two-dot attributes it to the PR anyway.Why this matters
This is a recovered abandoned fix, not new work. The commit sat on
fix/hybrid-gate-docs-only-diff-rangewith no PR ever opened — one of three such branches in this repo found during a sweep. The reasoning
in its commit message was already correct; it simply never landed.
It matters more now than when it was written, because
docs_onlywas exposed as aworkflow_calloutput in #35 and has since been recommended to an adopting repo as the way to skip expensive jobs.
A verdict that silently flips to
falsewhenever the base moves makes that capability unreliable inexactly the busy-repo case it exists for — and the failure is invisible: the expensive job simply
runs, which looks like normal behaviour rather than a misclassification.
The direction of the error is at least the safe one — it under-claims docs-only and never skips a
build it should have run.
Desired correction
Three-dot on the pull_request arm, so the diff is taken from the merge-base — the changeset the PR
actually proposes, and what the PR's own "Files changed" tab shows.
The push arms stay two-dot and that is deliberate:
before..afteris exactly what a push changed,and there is no merge-base to speak of. The recovered commit already carried that distinction in its
WHY comment; it is preserved verbatim.
Done when:a PR touching only markdown is classified docs-only regardless of what has landed on thebase since it forked.