diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 743aa9e..6882868 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -114,15 +114,26 @@ jobs: # The script lists by prefix and asserts SWEPT AGAINST WHAT THE REMOTE # PUBLISHES, because a gate that sweeps a subset and reports success is # worse than no gate — the green is the evidence somebody acts on. - # SWEEP_BASE is passed EXPLICITLY, from the repository's own default - # branch, so the sweep does not have to guess which ref this change - # merges into. Guessing is what made this check answer differently on a - # push build than on a pull_request build: GITHUB_BASE_REF is empty on - # a push, the base fell through to the pushed branch, and the default - # branch — which carries the implementation until this merges — was - # swept and failed. + # SWEEP_BASE is passed EXPLICITLY, so the sweep does not guess which + # ref this change merges into. Guessing is what made this check answer + # differently on a push build than on a pull_request build: + # GITHUB_BASE_REF is empty on a push, the base fell through to the + # pushed branch, and the default branch — which carried the deleted + # implementation until that change merged — was swept and failed. + # + # THE PR'S OWN TARGET FIRST, the default branch only as the fallback. + # This read the default branch unconditionally, which is right for + # every pull request this repository has had and wrong for one + # targeting anything else: a PR into a release branch would have had + # that branch SWEPT — failing circularly on content the PR is changing + # — while the default branch was excluded and went unchecked. Both + # halves of that are wrong and they hide each other. + # + # `github.base_ref` is set on a pull_request and empty on a push, which + # is exactly the distinction needed: a push build has no base, and for + # it the default branch is the right answer. env: - SWEEP_BASE: ${{ github.event.repository.default_branch }} + SWEEP_BASE: ${{ github.base_ref || github.event.repository.default_branch }} SWEEP_HEAD: ${{ github.head_ref || github.ref_name }} run: | git fetch --no-tags --prune --quiet origin \ diff --git a/sweep_bypasses_test.go b/sweep_bypasses_test.go index 254d911..fd2456a 100644 --- a/sweep_bypasses_test.go +++ b/sweep_bypasses_test.go @@ -820,3 +820,51 @@ func TestTheBaseRefIsTheDefaultBranchWhicheverEventRan(t *testing.T) { require.Contains(t, explicit, "origin/main", "with badges named as the base, main is an ordinary published ref again") } + +// THE WORKFLOW MUST SWEEP THE PR'S OWN TARGET, not whatever the default branch +// happens to be. +// +// `SWEEP_BASE` read `github.event.repository.default_branch` unconditionally. +// That is right for every pull request this repository has had and wrong for +// one targeting anything else — a release branch, say. Then the release branch +// would be SWEPT, failing circularly on the very content the PR is changing, +// while the default branch was excluded and went unchecked. Both halves are +// wrong and they conceal each other: the red looks like a finding and the +// unchecked branch looks clean. +// +// `github.base_ref` is set on a pull_request and empty on a push, which is the +// distinction needed — a push build has no base, and the default branch is the +// right answer for it. +// +// WHAT THIS TEST CAN AND CANNOT DO, stated because the gap is the point: a +// GitHub expression is evaluated by GitHub, not here, so this asserts the +// expression's SHAPE — that the PR's target is consulted before the default +// branch. The script's behaviour given a base is covered by +// TestTheBaseRefIsTheDefaultBranchWhicheverEventRan, which drives both event +// shapes. Neither test runs the expression; a reviewer executed the old and new +// forms externally to establish that. +func TestTheSweepsBaseIsThePullRequestsOwnTarget(t *testing.T) { + workflow, err := os.ReadFile(filepath.Join(".github", "workflows", "go.yml")) + require.NoError(t, err) + + var base string + for _, line := range strings.Split(string(workflow), "\n") { + if key, value, found := strings.Cut(strings.TrimSpace(line), "SWEEP_BASE:"); found && key == "" { + base = strings.TrimSpace(value) + break + } + } + require.NotEmpty(t, base, "the sweep step passes no SWEEP_BASE, so it is guessing again") + + require.Contains(t, base, "github.base_ref", + "SWEEP_BASE must consult the pull request's own target. Reading only the default "+ + "branch sweeps the real target of a PR aimed anywhere else — failing circularly "+ + "on the content that PR is changing — and excludes the default branch, which then "+ + "goes unchecked.") + require.Contains(t, base, "default_branch", + "and must fall back to the default branch, because a push build has no base at all — "+ + "which is the bug this replaced") + require.Less(t, strings.Index(base, "github.base_ref"), strings.Index(base, "default_branch"), + "the order is the whole rule: the PR's target FIRST, the default branch only as the "+ + "fallback. Reversed, every pull request would sweep its own target.") +}