Skip to content

ci: refuse a PR that does not target master, instead of running nothing (D18 #11) - #164

Merged
heydryft merged 1 commit into
masterfrom
ci/base-must-be-master
Aug 19, 2026
Merged

heydryft merged 1 commit into
masterfrom
ci/base-must-be-master

Conversation

@heydryft

@heydryft heydryft commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

The state this closes

Five PRs — #109, #113, #114, #116, #121 — are based on other PRs' branches. They trigger zero lanes and display exactly one check, comment. Nothing was compiled, tested, linted or nvcc'd. A reviewer sees no red and merges.

Why the trigger fix alone is not enough

Master already widened ci.yml's pull_request filter to '**' (91befb0cf), so the lanes now run on a stacked PR. That is half. The two changes answer different questions:

question
trigger fix ('**') did the lanes run?
this lane did they run against the code that will actually land on master?

The second is not implied by the first. A stacked PR's lanes compile head merged into another PR's branch. That base can be rebased or force-pushed out from under it, so a green there describes a tree that may never exist — and it inherits a parent that has not itself been proven, while reporting success.

So base-branch fails hard whenever github.base_ref != 'master'. Hard, not a warning: the defect class being closed is "an absence that reads as a pass", and a warning is another absence.

Where it lives, and why that matters

It is a lane of ci.yml, not a new workflow. CI complete is the single required status check in this repo's branch protection (required_status_checks.contexts = ["CI complete"]), and it aggregates its needs. A standalone workflow would not be required by branch protection and could be merged straight past.

ci-complete gains base-branch as a dependency, and EXPECTED_LANES goes 8 → 9 — the same assertion that already refuses to let the lane set shrink silently.

The if: is load-bearing

github.base_ref is empty on push, schedule and workflow_dispatch. The step carries if: github.event_name == 'pull_request'; without it every push to master would fail this lane. The job still succeeds on those events, so ci-complete sees 9 lanes on every event type.

Script body verified locally against all three cases:

base='master'                       -> exit 0   "Base is master."
base='feat/batch-admission-ragged'  -> exit 1   ::error:: ...
base=''            (push/schedule)  -> exit 1   <- why the if: exists

github.base_ref and head_ref are passed via env:, never interpolated into run: — a branch name is attacker-controlled text.

Proof it fails

A guard that has never been observed failing is the exact fault it exists to catch. PR #165 is the deliberate negative test: same lane, base set to this branch instead of master. Its Base branch lane must be RED and must take CI complete with it. This PR is the positive case — base is master, so the lane passes.

Both results are quoted in the merge report.

Note

This lane makes #109/#113/#114/#116/#121 red until they are rebased onto master. That is the intent.

…ng (D18 #11)

Five PRs — #109, #113, #114, #116, #121 — were based on other PRs' branches.
They triggered zero lanes, displayed only `comment`, and read as green while
nothing had been compiled, tested, linted or nvcc'd. A reviewer sees no red and
merges.

Widening the `pull_request` branch filter to `'**'` (already on master) fixed
half of it: the lanes now RUN on a stacked PR. It does not fix the other half.
The two answer different questions:

    trigger fix  ->  did the lanes run?
    this lane    ->  did they run against the code that will land on master?

A stacked PR's lanes compile head-merged-into-another-PR's-branch. That base can
be rebased or force-pushed under it, so its green describes a tree that may
never exist — and it inherits an unproven parent while reporting success.

`base-branch` therefore fails hard whenever `github.base_ref != 'master'`. Hard,
not a warning: the defect class being closed is "an absence that reads as a
pass", and a warning is another absence.

It is a lane of ci.yml, not a separate workflow, because `CI complete` is the
single required status check in branch protection and aggregates its `needs`. A
standalone workflow would not be required and could be merged straight past.
`ci-complete` gains it as a dependency and EXPECTED_LANES goes 8 -> 9, which is
the same assertion refusing to let the lane set shrink silently.

The step carries `if: github.event_name == 'pull_request'` because `base_ref` is
empty on push/schedule/workflow_dispatch — without it every push to master would
fail this lane. Verified by running the script body against base='master' (exit
0), base='feat/batch-admission-ragged' (exit 1) and base='' (exit 1).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SpVNMpb13HkUXqSqbN1o9H
@github-actions

Copy link
Copy Markdown
Code Metrics Report
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 Language              Files        Lines         Code     Comments       Blanks
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 C Header                  5          305          210           52           43
 CSS                       2         1181         1036           34          111
 CUDA                     73        25129        18043         4319         2767
 Dockerfile                1           39           22            8            9
 JavaScript               16         3546         2676          482          388
 Jinja2                    7          694          656            5           33
 JSON                     74         4600         4597            0            3
 Makefile                  1            6            5            0            1
 Metal Shading Lan|       33        12224         9431         1142         1651
 PowerShell                1          300          227           30           43
 Python                  145        15139        12482          811         1846
 Shell                    37         8630         5732         2323          575
 Plain Text                4         3801            0         2479         1322
 TOML                     33         1498         1294           54          150
 YAML                      3           25           23            2            0
─────────────────────────────────────────────────────────────────────────────────
 HTML                      4         2687         2604           43           40
 |- CSS                    2          543          479           37           27
 |- JavaScript             1         1233         1215           12            6
 (Total)                             4463         4298           92           73
─────────────────────────────────────────────────────────────────────────────────
 Jupyter Notebooks         4          122           83           23           16
 |- Markdown               1           60           30           22            8
 |- Python                 1          122          113            1            8
 (Total)                              304          226           46           32
─────────────────────────────────────────────────────────────────────────────────
 Markdown                199        39948            0        30698         9250
 |- BASH                  72         1654         1202          331          121
 |- C                      3           17           17            0            0
 |- CUDA                   2           84           56           16           12
 |- JSON                  18          708          708            0            0
 |- PowerShell             1            1            1            0            0
 |- Python                23         1008          787          113          108
 |- Rust                  65         2048         1713           77          258
 |- TOML                   6          207          164            0           43
 |- YAML                   4           38           33            5            0
 (Total)                            45713         4681        31240         9792
─────────────────────────────────────────────────────────────────────────────────
 Rust                    671       324354       279943        15914        28497
 |- Markdown             489        27607          471        23817         3319
 (Total)                           351961       280414        39731        31816
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 Total                  1313       479558       346053        82850        50655
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

@heydryft

Copy link
Copy Markdown
Contributor Author

Proof the guard fails, and then passes — both observed, not predicted.

A guard that has never been seen failing is the exact fault it exists to catch. Both directions are now on record.

FAILING — #165, base = ci/base-must-be-master

Run 32242984254, job Base branch:

head: ci/base-guard-proof
base: ci/base-must-be-master
##[error]This PR targets 'ci/base-must-be-master', not 'master'. ...
##[error]Process completed with exit code 1

Base branch = FAILURE on the PR.

PASSING — this PR, base = master

Base branch = SUCCESS

Same lane, same commit content, same workflow file — the only difference is the base ref. The lane also ran on #165 at all, which confirms the second half of the mechanism: for pull_request events the workflow comes from refs/pull/N/merge, so a stacked PR does execute the guard that refuses it.

Local pre-verification

The script body was exercised against all three reachable inputs before it was pushed:

base='master'                       -> exit 0   "Base is master."
base='feat/batch-admission-ragged'  -> exit 1   ::error:: ...
base=''            (push/schedule)  -> exit 1   <- why `if: github.event_name == 'pull_request'` exists

That third case is the one that would have broken every push to master had the if: been omitted.

Follow-up worth considering separately (not in this PR)

Widening the trigger to '**' means a mis-based PR now runs the full 12-job matrix before this lane declares it invalid. The queue hit 31 runs deep during this cleanup. Making the other lanes needs: base-branch would cost 1 job instead of 12 for a mis-based PR, and ci-complete already treats skipped as failure so the PR would stay correctly red. Deliberately not folded in here — this PR's guard is proven as-is and I would rather not re-open a verified change for an optimisation.

@heydryft
heydryft merged commit 79dbe68 into master Aug 19, 2026
15 checks passed
Sign up for free to 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