From 02c69551621ac82a29c08ac9b8104832bec16128 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 3 Sep 2026 15:00:04 -0400 Subject: [PATCH] chore(#4241): add merge_group trigger to test.yml (#4242) * chore(ci): add merge_group trigger to test.yml (#4241) GitHub's merge queue fires the `merge_group` event for the temporary merge-group commit it creates when a PR is added to the queue - not `pull_request` or `push`. Without this trigger, `required-tests` (the registered "Required tests" branch-protection check) never schedules for a queued PR, permanently stalling the queue on a check that never runs. This is workflow-side prerequisite wiring only; enabling the merge queue itself is a separate manual branch-protection step. * fix(#4241): pin AUDIT_BASELINE_REF for merge_group events too Code review on this branch caught that AUDIT_BASELINE_REF's ternary only branched on pull_request/push, so a merge_group run silently fell through to '' -- scripts/npm-audit-baseline.cjs's resolveBaselineRef() documents its origin/next live-tip fallback as unreachable from CI specifically because AUDIT_BASELINE_REF is "always set by test.yml". Reopens the exact race #4196 fixed, but only for merge-queue runs. Extends all three AUDIT_BASELINE_REF pins (test, test-inert, test-full) to also branch on merge_group, using github.event.merge_group.base_sha (confirmed against GitHub's own webhook payload schema: "the SHA of the merge group's parent commit") -- the base tip the temporary merge-group commit was built against. Adds a regression test asserting every AUDIT_BASELINE_REF pin branches on merge_group with the correct field. --------- Co-authored-by: sim --- .github/workflows/test.yml | 39 +++++++++++++++++++++++++++++--- tests/ci-test-scope.test.cjs | 43 ++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6e082b253..92731bc1f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,6 +11,15 @@ on: branches: - next - main + # #4241: GitHub's merge queue fires `merge_group`, not `pull_request` or + # `push`, for the temporary merge-group commit it creates. Without this + # trigger, `required-tests` (the registered "Required tests" branch- + # protection check) never schedules for a queued PR and the queue stalls + # waiting on a check that never runs. See: + # https://docs.github.com/en/actions/how-tos/using-features-for-actions/using-merge-queues + # This is prerequisite wiring only — the merge queue itself is enabled via + # a separate, manual "Require merge queue" branch-protection toggle. + merge_group: workflow_dispatch: concurrency: @@ -223,7 +232,15 @@ jobs: # even when a rebase-merged PR lands as multiple discrete commits in # one push (HEAD~1 would be wrong there: it could already contain an # earlier commit's newly-introduced vulnerable package, masking it). - AUDIT_BASELINE_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || (github.event_name == 'push' && github.event.before) || '' }} + # #4241: a merge_group event carries no pull_request/push context, so + # without this arm it silently fell through to the '' branch -- + # resolveBaselineRef()'s documented-unreachable origin/next live-tip + # fallback (npm-audit-baseline.cjs), reopening the exact race #4196 + # fixed, but only for merge-queue runs. github.event.merge_group.base_sha + # is "the SHA of the merge group's parent commit" (GitHub's merge_group + # webhook payload) -- the base tip the temporary merge-group commit was + # built against, pinned for the life of the run same as the other two arms. + AUDIT_BASELINE_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || (github.event_name == 'push' && github.event.before) || (github.event_name == 'merge_group' && github.event.merge_group.base_sha) || '' }} strategy: fail-fast: false matrix: @@ -463,7 +480,15 @@ jobs: # even when a rebase-merged PR lands as multiple discrete commits in # one push (HEAD~1 would be wrong there: it could already contain an # earlier commit's newly-introduced vulnerable package, masking it). - AUDIT_BASELINE_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || (github.event_name == 'push' && github.event.before) || '' }} + # #4241: a merge_group event carries no pull_request/push context, so + # without this arm it silently fell through to the '' branch -- + # resolveBaselineRef()'s documented-unreachable origin/next live-tip + # fallback (npm-audit-baseline.cjs), reopening the exact race #4196 + # fixed, but only for merge-queue runs. github.event.merge_group.base_sha + # is "the SHA of the merge group's parent commit" (GitHub's merge_group + # webhook payload) -- the base tip the temporary merge-group commit was + # built against, pinned for the life of the run same as the other two arms. + AUDIT_BASELINE_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || (github.event_name == 'push' && github.event.before) || (github.event_name == 'merge_group' && github.event.merge_group.base_sha) || '' }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -566,7 +591,15 @@ jobs: # even when a rebase-merged PR lands as multiple discrete commits in # one push (HEAD~1 would be wrong there: it could already contain an # earlier commit's newly-introduced vulnerable package, masking it). - AUDIT_BASELINE_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || (github.event_name == 'push' && github.event.before) || '' }} + # #4241: a merge_group event carries no pull_request/push context, so + # without this arm it silently fell through to the '' branch -- + # resolveBaselineRef()'s documented-unreachable origin/next live-tip + # fallback (npm-audit-baseline.cjs), reopening the exact race #4196 + # fixed, but only for merge-queue runs. github.event.merge_group.base_sha + # is "the SHA of the merge group's parent commit" (GitHub's merge_group + # webhook payload) -- the base tip the temporary merge-group commit was + # built against, pinned for the life of the run same as the other two arms. + AUDIT_BASELINE_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || (github.event_name == 'push' && github.event.before) || (github.event_name == 'merge_group' && github.event.merge_group.base_sha) || '' }} strategy: fail-fast: false matrix: diff --git a/tests/ci-test-scope.test.cjs b/tests/ci-test-scope.test.cjs index 98652a7a5..3ef36d099 100644 --- a/tests/ci-test-scope.test.cjs +++ b/tests/ci-test-scope.test.cjs @@ -599,6 +599,49 @@ describe('test-full shard matrix parity (#1212)', () => { 'required-tests must `needs: test-full` so all shard legs aggregate into the gate', ); }); + + test('workflow triggers on merge_group (#4241)', () => { + // GitHub's merge queue fires `merge_group`, not `pull_request` or `push`, + // for the temporary merge-group commit it creates. Without this trigger + // the whole workflow — and therefore `required-tests` — never schedules + // for a queued PR, silently stalling the merge queue on a check that + // never runs. `on.merge_group` has no required config, so its presence + // as a key (even with a `null`/empty value) is what matters here. + const text = fs.readFileSync(path.join(WORKFLOWS_DIR, 'test.yml'), 'utf8'); + const doc = yaml.load(text); + assert.ok( + Object.prototype.hasOwnProperty.call(doc.on, 'merge_group'), + 'test.yml must trigger `on: merge_group` so required-tests schedules for merge-queue commits', + ); + }); + + test('every AUDIT_BASELINE_REF pin covers merge_group, not just pull_request/push (#4241)', () => { + // scripts/npm-audit-baseline.cjs's resolveBaselineRef() documents its + // origin/next live-tip fallback as UNREACHABLE from CI because + // AUDIT_BASELINE_REF is "always set by test.yml". A merge_group event + // carries neither pull_request nor push context, so a ternary that only + // branches on those two silently falls through to '' for a merge-queue + // run -- reopening the exact origin/next-can-advance-mid-run race #4196 + // fixed, but only for merge_group. Every job that sets AUDIT_BASELINE_REF + // must also branch on merge_group, using merge_group.base_sha (the base + // tip the temporary merge-group commit was built against). + const text = fs.readFileSync(path.join(WORKFLOWS_DIR, 'test.yml'), 'utf8'); + const doc = yaml.load(text); + + const jobsUnderTest = Object.entries(doc.jobs).filter( + ([, job]) => job.env && typeof job.env.AUDIT_BASELINE_REF === 'string', + ); + assert.ok(jobsUnderTest.length >= 3, `expected at least 3 jobs to pin AUDIT_BASELINE_REF, got ${jobsUnderTest.length}`); + + for (const [name, job] of jobsUnderTest) { + const expr = job.env.AUDIT_BASELINE_REF; + assert.ok( + expr.includes("github.event_name == 'merge_group'") && expr.includes('github.event.merge_group.base_sha'), + `job ${name}: AUDIT_BASELINE_REF must branch on merge_group using ` + + `github.event.merge_group.base_sha, got: ${expr}`, + ); + } + }); }); describe('emitted-provenance selection (#1691 drift guard, retargeted by #2724)', () => {