From 9605ab7c3d64e4601acdcf292b07240c1af6a4f7 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Tue, 6 Oct 2026 00:38:32 +0800 Subject: [PATCH 01/11] fix(ci): measure a stacked unit against its parent pull request The gate diffed every pull request against the merge commit's first parent, which is the base branch tip. For a stacked unit that base is main, so the whole unmerged chain was charged to the unit: the file-safety chain measured 512, 727, 783, 815 and 827 executable lines against a 500 cap, while each unit's own delta is 27-225 lines. The merge commit's second parent is the pull request head. When that head's own parent is another open pull request's head, the pull request is a stacked unit and the gate measures only its delta. A pull request whose parent is not another pull request head keeps the event base, so a multi-commit pull request is never charged only its last commit. An unparsable stacked map degrades to the event base instead of throwing, and the job summary records which base was used. --- .github/workflows/mutation-testing.yml | 10 ++- scripts/stryker-diff.mjs | 52 +++++++++++++-- scripts/stryker-diff.test.mjs | 87 ++++++++++++++++++++++++++ 3 files changed, 142 insertions(+), 7 deletions(-) diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml index 2da0c5c5b4..0d757d617f 100644 --- a/.github/workflows/mutation-testing.yml +++ b/.github/workflows/mutation-testing.yml @@ -8,6 +8,8 @@ on: permissions: contents: read + # The gate only reads open pull request heads to recognise a stacked unit. + pull-requests: read concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} @@ -50,9 +52,15 @@ jobs: if: github.event_name == 'pull_request' env: HEAD_SHA: ${{ github.sha }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | BASE_SHA="$(git rev-parse "$HEAD_SHA^1")" - node scripts/stryker-diff.mjs ci --base "$BASE_SHA" --head "$HEAD_SHA" + # The merge commit's second parent is the pull request head. When that head's own + # parent is another open pull request's head, this PR is a stacked unit, so the gate + # measures only this unit's delta instead of every unmerged ancestor. + PR_HEAD_SHA="$(git rev-parse "$HEAD_SHA^2" 2>/dev/null || git rev-parse "$HEAD_SHA")" + STACKED_MAP="$(gh api "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" --jq '[.[] | {number: .number, headSha: .head.sha}]' || '[]')" + node scripts/stryker-diff.mjs ci --base "$BASE_SHA" --head "$HEAD_SHA" --stacked-map "$STACKED_MAP" - name: Upload mutation reports id: mutation_report diff --git a/scripts/stryker-diff.mjs b/scripts/stryker-diff.mjs index 9742879106..5c5a122030 100644 --- a/scripts/stryker-diff.mjs +++ b/scripts/stryker-diff.mjs @@ -281,6 +281,22 @@ export function resolvePullRequestBase(repoRoot, baseSha, headSha) { return parents[0] } +// A stacked unit branch is built on the previous unit's head, so the pull request's own base (main) +// attributes every unmerged ancestor to this pull request. When the head commit's first parent is the +// head of another open pull request, that parent is the unit's real base and the diff contains only +// this unit's delta. A pull request whose parent is not another pull request's head keeps the event +// base, so a multi-commit pull request is never charged only its last commit. +export function resolveStackedUnitBase(repoRoot, eventBaseSha, prHeadSha, openPullRequests = []) { + validateSha(eventBaseSha, "base SHA") + validateSha(prHeadSha, "pull request head SHA") + const parents = git(repoRoot, ["rev-list", "--parents", "-n", "1", prHeadSha]).trim().split(/\s+/).slice(1) + if (parents.length !== 1) return { baseSha: eventBaseSha, stackedOn: null } + const parentSha = parents[0].toLowerCase() + const parent = openPullRequests.find((pr) => String(pr.headSha).toLowerCase() === parentSha) + if (!parent) return { baseSha: eventBaseSha, stackedOn: null } + return { baseSha: parentSha, stackedOn: parent.number } +} + export function selectFromGit(repoRoot, baseSha, headSha) { validateSha(baseSha, "base SHA") validateSha(headSha, "head SHA") @@ -566,12 +582,17 @@ export function formatBlockingMutants(blockingMutants, packageRoot) { } export function formatSummary(rows, advisories, manifest = {}) { - const lines = [ - "## Changed-code mutation testing", - "", + const lines = ["## Changed-code mutation testing", ""] + if (manifest.stackedOn) { + lines.push( + `Stacked unit: measured against the head of parent PR #${manifest.stackedOn}, not the event base.`, + "", + ) + } + lines.push( "| Package | Changed executable lines | Valid | Killed | Timeout | Survived | No coverage | Result |", "| --- | ---: | ---: | ---: | ---: | ---: | ---: | --- |", - ] + ) for (const row of rows) { lines.push( `| ${row.id} | ${row.changedLines} | ${row.valid} | ${row.killed} | ${row.timeout} | ${row.survived} | ${row.noCoverage} | ${row.result} |`, @@ -796,18 +817,37 @@ function argument(name) { return index === -1 ? undefined : process.argv[index + 1] } +// The workflow passes the open pull requests as a JSON array of { number, headSha }. An unparsable +// map must not change the gate's base, so it degrades to the event base instead of throwing. +export function parseStackedMap(value) { + if (!value) return [] + try { + const parsed = JSON.parse(value) + if (!Array.isArray(parsed)) return [] + return parsed.filter((entry) => entry && entry.number && /^[0-9a-f]{40}$/i.test(String(entry.headSha))) + } catch { + return [] + } +} + function main() { const command = process.argv[2] if (command !== "ci") - throw new Error("Usage: node scripts/stryker-diff.mjs ci --base --head [--reports ]") + throw new Error( + "Usage: node scripts/stryker-diff.mjs ci --base --head [--reports ] [--stacked-map ]", + ) const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..") const baseSha = argument("--base") const headSha = argument("--head") if (!baseSha || !headSha) throw new Error("--base and --head are required") + const openPullRequests = parseStackedMap(argument("--stacked-map")) + const resolved = resolveStackedUnitBase(repoRoot, baseSha, headSha, openPullRequests) + if (resolved.stackedOn) console.log(`Stacked unit: measured against the head of parent PR #${resolved.stackedOn}, not the event base.`) + const reportRoot = path.resolve(repoRoot, argument("--reports") ?? "reports/mutation") - const manifest = selectFromGit(repoRoot, baseSha, headSha) + const manifest = { ...selectFromGit(repoRoot, resolved.baseSha, headSha), stackedOn: resolved.stackedOn } if (manifest.packages.length === 0) { appendSummary([], manifest.advisories, manifest) console.log("No changed executable lines in mutation-tested packages; mutation testing is not applicable.") diff --git a/scripts/stryker-diff.test.mjs b/scripts/stryker-diff.test.mjs index 22284e598d..a84bca5cc3 100644 --- a/scripts/stryker-diff.test.mjs +++ b/scripts/stryker-diff.test.mjs @@ -25,6 +25,8 @@ import { parseNameStatus, parseVitestTestFiles, preferDirectTestFiles, + parseStackedMap, + resolveStackedUnitBase, resolveStrykerTempDir, resolveVitestBinary, shouldUseVitestRelated, @@ -58,6 +60,9 @@ describe("mutation testing workflow", () => { assert.ok(workflow.includes("HEAD_SHA: ${{ github.sha }}")) assert.ok(!workflow.includes("HEAD_SHA: ${{ github.event.pull_request.head.sha }}")) assert.ok(workflow.includes('BASE_SHA="$(git rev-parse "$HEAD_SHA^1")"')) + assert.ok(workflow.includes("pull-requests: read")) + assert.ok(workflow.includes('PR_HEAD_SHA="$(git rev-parse "$HEAD_SHA^2" 2>/dev/null || git rev-parse "$HEAD_SHA")"')) + assert.ok(workflow.includes('--stacked-map "$STACKED_MAP"')) assert.ok(!workflow.includes("github.event.pull_request.base.sha")) assert.ok(workflow.includes("steps.mutation_report.outputs.artifact-url")) assert.ok(workflow.includes("open the package's mutation.html file")) @@ -175,6 +180,88 @@ describe("pull request revision selection", () => { }) }) +describe("stacked unit base resolution", () => { + const createSyntheticStack = () => { + const repository = fs.mkdtempSync(path.join(os.tmpdir(), "mutation-stack-")) + const run = (...args) => execFileSync("git", args, { cwd: repository, encoding: "utf8" }).trim() + const write = (filePath, contents) => { + fs.mkdirSync(path.join(repository, path.dirname(filePath)), { recursive: true }) + fs.writeFileSync(path.join(repository, filePath), contents) + } + + run("init", "--quiet", "--initial-branch", "main") + run("config", "user.email", "gate@example.com") + run("config", "user.name", "Gate") + run("config", "commit.gpgsign", "false") + + write("packages/core/src/unrelated.ts", "export const unrelated = () => 1\n") + run("add", ".") + run("commit", "--quiet", "-m", "initial") + const eventBaseSha = run("rev-parse", "HEAD") + + run("checkout", "--quiet", "-b", "unit-1") + write("packages/core/src/unit1.ts", "export const unit1 = () => 1\n") + run("add", ".") + run("commit", "--quiet", "-m", "unit 1") + const parentSha = run("rev-parse", "HEAD") + + // The stacked unit is one commit on top of the parent unit's head. + write("packages/core/src/unit2.ts", "export const unit2 = () => 2\n") + run("add", ".") + run("commit", "--quiet", "-m", "unit 2") + const childSha = run("rev-parse", "HEAD") + + return { repository, eventBaseSha, parentSha, childSha } + } + + it("measures a stacked unit against the parent pull request head", () => { + const { repository, eventBaseSha, parentSha, childSha } = createSyntheticStack() + + try { + const resolved = resolveStackedUnitBase(repository, eventBaseSha, childSha, [{ number: 1, headSha: parentSha }]) + assert.equal(resolved.baseSha, parentSha) + assert.equal(resolved.stackedOn, 1) + + const manifest = selectFromGit(repository, resolved.baseSha, childSha) + assert.deepEqual( + manifest.packages.flatMap((entry) => entry.files.map((file) => file.path)), + ["packages/core/src/unit2.ts"], + ) + } finally { + fs.rmSync(repository, { recursive: true, force: true }) + } + }) + + it("keeps the event base when the parent commit is not another pull request head", () => { + const { repository, eventBaseSha, parentSha, childSha } = createSyntheticStack() + + try { + const resolved = resolveStackedUnitBase(repository, eventBaseSha, childSha, []) + assert.equal(resolved.baseSha, eventBaseSha) + assert.equal(resolved.stackedOn, null) + + // A multi-commit pull request must not be charged only its last commit. + const manifest = selectFromGit(repository, resolved.baseSha, childSha) + assert.deepEqual( + manifest.packages.flatMap((entry) => entry.files.map((file) => file.path)).sort(), + ["packages/core/src/unit1.ts", "packages/core/src/unit2.ts"], + ) + void parentSha + } finally { + fs.rmSync(repository, { recursive: true, force: true }) + } + }) + + it("degrades to the event base for an unparsable stacked map", () => { + assert.deepEqual(parseStackedMap("not json"), []) + assert.deepEqual(parseStackedMap('{"number": 1}'), []) + assert.deepEqual(parseStackedMap("[{\"number\": 1, \"headSha\": \"abc\"}]"), []) + assert.deepEqual(parseStackedMap("[{\"number\": 1, \"headSha\": \"" + "a".repeat(40) + "\"}]"), [ + { number: 1, headSha: "a".repeat(40) }, + ]) + }) +}) + describe("parseNameStatus", () => { it("parses added, modified, and renamed paths", () => { assert.deepEqual( From a42df42aca3ab2e2044e1029b43f0c37de7fde91 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Tue, 6 Oct 2026 11:08:38 +0800 Subject: [PATCH 02/11] fix(gate): pass the pull request head and keep the resolved stacked base Three gaps in the stacked-unit measurement: 1. PR_HEAD_SHA was computed but never passed, so the resolver ran on the merge commit, whose first parent is the base tip, and never detected a stacked unit. 2. The gh api fallback was a bare '[]', which bash -e executes as a program name; an API failure aborted the step instead of measuring against the event base. 3. selectFromGit re-derived the base from the merge commit, discarding the resolved stacked base. It now keeps an explicitly resolved base, and a stacked unit is measured against its own head rather than the merge commit, which also carries whatever main advanced since the parent unit. Tests: 48 passed in scripts/stryker-diff.test.mjs. --- .github/workflows/mutation-testing.yml | 9 ++--- scripts/stryker-diff.mjs | 22 +++++++++--- scripts/stryker-diff.test.mjs | 48 +++++++++++++++++++++++++- 3 files changed, 69 insertions(+), 10 deletions(-) diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml index 0d757d617f..a0d90f14e5 100644 --- a/.github/workflows/mutation-testing.yml +++ b/.github/workflows/mutation-testing.yml @@ -59,10 +59,11 @@ jobs: # parent is another open pull request's head, this PR is a stacked unit, so the gate # measures only this unit's delta instead of every unmerged ancestor. PR_HEAD_SHA="$(git rev-parse "$HEAD_SHA^2" 2>/dev/null || git rev-parse "$HEAD_SHA")" - STACKED_MAP="$(gh api "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" --jq '[.[] | {number: .number, headSha: .head.sha}]' || '[]')" - node scripts/stryker-diff.mjs ci --base "$BASE_SHA" --head "$HEAD_SHA" --stacked-map "$STACKED_MAP" - - - name: Upload mutation reports + # The fallback must itself be a command: these steps run bash with -e, so a bare '[]' is + # executed as a program name and an API failure would abort the step instead of measuring + # against the event base. + STACKED_MAP="$(gh api "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" --jq '[.[] | {number: .number, headSha: .head.sha}]')" || STACKED_MAP="$(printf '[]')" + node scripts/stryker-diff.mjs ci --base "$BASE_SHA" --head "$HEAD_SHA" --pr-head "$PR_HEAD_SHA" --stacked-map "$STACKED_MAP" id: mutation_report if: always() && github.event_name == 'pull_request' continue-on-error: true diff --git a/scripts/stryker-diff.mjs b/scripts/stryker-diff.mjs index 5c5a122030..4e3bd736ae 100644 --- a/scripts/stryker-diff.mjs +++ b/scripts/stryker-diff.mjs @@ -297,10 +297,12 @@ export function resolveStackedUnitBase(repoRoot, eventBaseSha, prHeadSha, openPu return { baseSha: parentSha, stackedOn: parent.number } } -export function selectFromGit(repoRoot, baseSha, headSha) { +export function selectFromGit(repoRoot, baseSha, headSha, options = {}) { validateSha(baseSha, "base SHA") validateSha(headSha, "head SHA") - baseSha = resolvePullRequestBase(repoRoot, baseSha, headSha) + // A stacked base was resolved from the pull request head, not from the merge commit, so it must + // survive: re-deriving it from the merge commit would charge the whole unmerged chain to this unit. + if (!options.preserveBase) baseSha = resolvePullRequestBase(repoRoot, baseSha, headSha) const mergeBase = git(repoRoot, ["merge-base", baseSha, headSha]).trim() const nameStatus = git(repoRoot, ["diff", "--name-status", "-z", "--find-renames", `${mergeBase}...${headSha}`]) const entries = parseNameStatus(nameStatus) @@ -834,7 +836,7 @@ function main() { const command = process.argv[2] if (command !== "ci") throw new Error( - "Usage: node scripts/stryker-diff.mjs ci --base --head [--reports ] [--stacked-map ]", + "Usage: node scripts/stryker-diff.mjs ci --base --head [--pr-head ] [--reports ] [--stacked-map ]", ) const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..") @@ -843,11 +845,21 @@ function main() { if (!baseSha || !headSha) throw new Error("--base and --head are required") const openPullRequests = parseStackedMap(argument("--stacked-map")) - const resolved = resolveStackedUnitBase(repoRoot, baseSha, headSha, openPullRequests) + // --pr-head is the pull request head itself. The workflow runs on GitHub's merge commit, whose + // first parent is the base tip, so the merge commit cannot identify the stacked unit; its second + // parent can. The merge commit stays the diff head. + const prHeadSha = argument("--pr-head") ?? headSha + const resolved = resolveStackedUnitBase(repoRoot, baseSha, prHeadSha, openPullRequests) if (resolved.stackedOn) console.log(`Stacked unit: measured against the head of parent PR #${resolved.stackedOn}, not the event base.`) const reportRoot = path.resolve(repoRoot, argument("--reports") ?? "reports/mutation") - const manifest = { ...selectFromGit(repoRoot, resolved.baseSha, headSha), stackedOn: resolved.stackedOn } + // A stacked unit is measured against its own head, not the merge commit: the merge commit also + // carries whatever main advanced since the parent unit, and those lines are not this unit's. + const diffHead = resolved.stackedOn ? prHeadSha : headSha + const manifest = { + ...selectFromGit(repoRoot, resolved.baseSha, diffHead, { preserveBase: Boolean(resolved.stackedOn) }), + stackedOn: resolved.stackedOn, + } if (manifest.packages.length === 0) { appendSummary([], manifest.advisories, manifest) console.log("No changed executable lines in mutation-tested packages; mutation testing is not applicable.") diff --git a/scripts/stryker-diff.test.mjs b/scripts/stryker-diff.test.mjs index a84bca5cc3..3ed6c197c5 100644 --- a/scripts/stryker-diff.test.mjs +++ b/scripts/stryker-diff.test.mjs @@ -211,7 +211,7 @@ describe("stacked unit base resolution", () => { run("commit", "--quiet", "-m", "unit 2") const childSha = run("rev-parse", "HEAD") - return { repository, eventBaseSha, parentSha, childSha } + return { repository, eventBaseSha, parentSha, childSha, run, write } } it("measures a stacked unit against the parent pull request head", () => { @@ -232,6 +232,52 @@ describe("stacked unit base resolution", () => { } }) + it("preserves the resolved stacked base when selection runs on the merge commit", () => { + const { repository, eventBaseSha, parentSha, childSha, run, write } = createSyntheticStack() + + try { + // GitHub runs the gate on its own merge commit: first parent is the base tip, second + // parent is the pull request head. + run("checkout", "--quiet", "-b", "merge-branch", eventBaseSha) + write("packages/core/src/unrelated.ts", "export const unrelated = () => 2\n") + run("add", ".") + run("commit", "--quiet", "-m", "base advance") + const advancedBase = run("rev-parse", "HEAD") + run("merge", "--no-ff", "--quiet", "-m", "merge", childSha) + const mergeSha = run("rev-parse", "HEAD") + + const resolved = resolveStackedUnitBase(repository, advancedBase, childSha, [{ number: 1, headSha: parentSha }]) + assert.equal(resolved.baseSha, parentSha) + assert.equal(resolved.stackedOn, 1) + + // Without preserving the resolved base, selection re-derives it from the merge commit and + // charges the whole unmerged chain to this unit. + const charged = selectFromGit(repository, resolved.baseSha, mergeSha) + assert.deepEqual( + charged.packages.flatMap((entry) => entry.files.map((file) => file.path)), + ["packages/core/src/unit1.ts", "packages/core/src/unit2.ts"], + ) + + // With the base preserved, the unit is measured against its own head: the merge commit also + // carries the base advance, which is not this unit's delta. + const unit = selectFromGit(repository, resolved.baseSha, childSha, { preserveBase: true }) + assert.deepEqual( + unit.packages.flatMap((entry) => entry.files.map((file) => file.path)), + ["packages/core/src/unit2.ts"], + ) + + // A non-stacked pull request still measures against the merge commit, so the base actually + // merged into is used and the base advance is not charged to the pull request. + const plain = selectFromGit(repository, advancedBase, mergeSha, { preserveBase: true }) + assert.deepEqual( + plain.packages.flatMap((entry) => entry.files.map((file) => file.path)), + ["packages/core/src/unit1.ts", "packages/core/src/unit2.ts"], + ) + } finally { + fs.rmSync(repository, { recursive: true, force: true }) + } + }) + it("keeps the event base when the parent commit is not another pull request head", () => { const { repository, eventBaseSha, parentSha, childSha } = createSyntheticStack() From 1d7384d8a490a6fe329cf712244a57125689b79e Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Tue, 6 Oct 2026 11:20:32 +0800 Subject: [PATCH 03/11] fix(gate): restore the upload step header dropped while editing the gate step The edit removed the blank line and the '- name: Upload mutation reports' header, leaving 'id: mutation_report' and a second 'if:' inside the gate step mapping, which is a duplicated YAML mapping key and made the knip job fail while loading the workflow. --- .github/workflows/mutation-testing.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml index a0d90f14e5..8eb0b0f34b 100644 --- a/.github/workflows/mutation-testing.yml +++ b/.github/workflows/mutation-testing.yml @@ -64,6 +64,8 @@ jobs: # against the event base. STACKED_MAP="$(gh api "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" --jq '[.[] | {number: .number, headSha: .head.sha}]')" || STACKED_MAP="$(printf '[]')" node scripts/stryker-diff.mjs ci --base "$BASE_SHA" --head "$HEAD_SHA" --pr-head "$PR_HEAD_SHA" --stacked-map "$STACKED_MAP" + + - name: Upload mutation reports id: mutation_report if: always() && github.event_name == 'pull_request' continue-on-error: true From a0a26cc38795c2d8639563694f017286da2e29d0 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Tue, 6 Oct 2026 11:33:22 +0800 Subject: [PATCH 04/11] fix(gate): paginate the open pull request map per_page=100 fetches only the first page, so a parent unit beyond page 1 is missing from the map and the gate falls back to the event base and charges the whole unmerged chain again - exactly the problem issue #1923 describes. gh api --paginate --slurp with a flattening jq filter covers every page, and the shape is now asserted in the workflow test. Tests: 48 passed in scripts/stryker-diff.test.mjs. --- .github/workflows/mutation-testing.yml | 3 ++- scripts/stryker-diff.test.mjs | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml index 8eb0b0f34b..dcffdb7c62 100644 --- a/.github/workflows/mutation-testing.yml +++ b/.github/workflows/mutation-testing.yml @@ -62,7 +62,8 @@ jobs: # The fallback must itself be a command: these steps run bash with -e, so a bare '[]' is # executed as a program name and an API failure would abort the step instead of measuring # against the event base. - STACKED_MAP="$(gh api "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" --jq '[.[] | {number: .number, headSha: .head.sha}]')" || STACKED_MAP="$(printf '[]')" + STACKED_MAP="$(gh api --paginate --slurp "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" --jq '[.[] | .[] | {number: .number, headSha: .head.sha}]')" + || STACKED_MAP="$(printf '[]')" node scripts/stryker-diff.mjs ci --base "$BASE_SHA" --head "$HEAD_SHA" --pr-head "$PR_HEAD_SHA" --stacked-map "$STACKED_MAP" - name: Upload mutation reports diff --git a/scripts/stryker-diff.test.mjs b/scripts/stryker-diff.test.mjs index 3ed6c197c5..01580683ad 100644 --- a/scripts/stryker-diff.test.mjs +++ b/scripts/stryker-diff.test.mjs @@ -63,6 +63,14 @@ describe("mutation testing workflow", () => { assert.ok(workflow.includes("pull-requests: read")) assert.ok(workflow.includes('PR_HEAD_SHA="$(git rev-parse "$HEAD_SHA^2" 2>/dev/null || git rev-parse "$HEAD_SHA")"')) assert.ok(workflow.includes('--stacked-map "$STACKED_MAP"')) + assert.ok(workflow.includes('--pr-head "$PR_HEAD_SHA"')) + // The open pull request map must cover every page, otherwise a parent unit beyond the first + // page is missing and the gate charges the whole unmerged chain again. + assert.ok(workflow.includes("gh api --paginate --slurp")) + assert.ok(workflow.includes("[.[] | .[] | {number: .number, headSha: .head.sha}]")) + // The fallback has to be a command: these steps run bash with -e, so a bare '[]' is executed + // as a program name instead of producing JSON. + assert.ok(workflow.includes("printf '[]'")) assert.ok(!workflow.includes("github.event.pull_request.base.sha")) assert.ok(workflow.includes("steps.mutation_report.outputs.artifact-url")) assert.ok(workflow.includes("open the package's mutation.html file")) From 6d902938b24c0a69d626c131c8de7c682a9deaae Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Tue, 6 Oct 2026 11:39:47 +0800 Subject: [PATCH 05/11] fix(gate): paginate without --slurp gh api rejects --slurp together with --jq, so the previous commit made the gate step exit 1 with 'the --slurp option is not supported with --jq or --template'. Pagination now keeps the per-page jq filter, which emits one JSON array per page, and parseStackedMap accepts that stream of page arrays as well as a single document. A parent unit beyond page 1 is therefore found again. Tests: 48 passed in scripts/stryker-diff.test.mjs, including the paginated-stream case. --- .github/workflows/mutation-testing.yml | 3 +-- scripts/stryker-diff.mjs | 24 ++++++++++++++++-------- scripts/stryker-diff.test.mjs | 23 ++++++++++++++++++++--- 3 files changed, 37 insertions(+), 13 deletions(-) diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml index dcffdb7c62..8bef2278a1 100644 --- a/.github/workflows/mutation-testing.yml +++ b/.github/workflows/mutation-testing.yml @@ -62,8 +62,7 @@ jobs: # The fallback must itself be a command: these steps run bash with -e, so a bare '[]' is # executed as a program name and an API failure would abort the step instead of measuring # against the event base. - STACKED_MAP="$(gh api --paginate --slurp "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" --jq '[.[] | .[] | {number: .number, headSha: .head.sha}]')" - || STACKED_MAP="$(printf '[]')" + STACKED_MAP="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" --jq '[.[] | {number: .number, headSha: .head.sha}]')" || STACKED_MAP="$(printf '[]')" node scripts/stryker-diff.mjs ci --base "$BASE_SHA" --head "$HEAD_SHA" --pr-head "$PR_HEAD_SHA" --stacked-map "$STACKED_MAP" - name: Upload mutation reports diff --git a/scripts/stryker-diff.mjs b/scripts/stryker-diff.mjs index 4e3bd736ae..c5ca2cfae5 100644 --- a/scripts/stryker-diff.mjs +++ b/scripts/stryker-diff.mjs @@ -819,17 +819,25 @@ function argument(name) { return index === -1 ? undefined : process.argv[index + 1] } -// The workflow passes the open pull requests as a JSON array of { number, headSha }. An unparsable -// map must not change the gate's base, so it degrades to the event base instead of throwing. +// The workflow passes the open pull requests as JSON of { number, headSha }. gh api --paginate +// emits one array per page, so the map can be a stream of arrays rather than one document; accept +// both shapes. An unparsable map must not change the gate's base, so it degrades to the event base +// instead of throwing. export function parseStackedMap(value) { if (!value) return [] - try { - const parsed = JSON.parse(value) - if (!Array.isArray(parsed)) return [] - return parsed.filter((entry) => entry && entry.number && /^[0-9a-f]{40}$/i.test(String(entry.headSha))) - } catch { - return [] + const chunks = String(value).match(/\[[^\]]*\]/g) || [String(value)] + const entries = [] + for (const chunk of chunks) { + let parsed + try { + parsed = JSON.parse(chunk) + } catch { + continue + } + if (!Array.isArray(parsed)) continue + entries.push(...parsed.filter((entry) => entry && entry.number && /^[0-9a-f]{40}$/i.test(String(entry.headSha)))) } + return entries } function main() { diff --git a/scripts/stryker-diff.test.mjs b/scripts/stryker-diff.test.mjs index 01580683ad..ce1c8c7822 100644 --- a/scripts/stryker-diff.test.mjs +++ b/scripts/stryker-diff.test.mjs @@ -66,8 +66,11 @@ describe("mutation testing workflow", () => { assert.ok(workflow.includes('--pr-head "$PR_HEAD_SHA"')) // The open pull request map must cover every page, otherwise a parent unit beyond the first // page is missing and the gate charges the whole unmerged chain again. - assert.ok(workflow.includes("gh api --paginate --slurp")) - assert.ok(workflow.includes("[.[] | .[] | {number: .number, headSha: .head.sha}]")) + // gh api rejects --slurp together with --jq, so pagination has to keep the per-page filter + // and the parser has to accept the resulting stream of page arrays. + assert.ok(workflow.includes("gh api --paginate")) + assert.ok(!workflow.includes("--slurp")) + assert.ok(workflow.includes("[.[] | {number: .number, headSha: .head.sha}]")) // The fallback has to be a command: these steps run bash with -e, so a bare '[]' is executed // as a program name instead of producing JSON. assert.ok(workflow.includes("printf '[]'")) @@ -313,7 +316,21 @@ describe("stacked unit base resolution", () => { assert.deepEqual(parseStackedMap("[{\"number\": 1, \"headSha\": \"" + "a".repeat(40) + "\"}]"), [ { number: 1, headSha: "a".repeat(40) }, ]) - }) + + // gh api --paginate emits one array per page, so a parent unit on page 2 arrives as a second + // document rather than inside the first array. + assert.deepEqual( + parseStackedMap( + "[{\"number\": 1, \"headSha\": \"" + "b".repeat(40) + "\"}]" + "[{\"number\": 2, \"headSha\": \"" + "c".repeat(40) + "\"}]", + ), + [ + { number: 1, headSha: "b".repeat(40) }, + { number: 2, headSha: "c".repeat(40) }, + ], + ) + assert.deepEqual(parseStackedMap("[{\"number\": 1, \"headSha\": \"" + "d".repeat(40) + "\"}]garbage"), [ + { number: 1, headSha: "d".repeat(40) }, + ]) }) }) describe("parseNameStatus", () => { From 134a184646a9d82f2d64bf66c936b2f1f04c242e Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Tue, 6 Oct 2026 12:52:57 +0800 Subject: [PATCH 06/11] fix(ci): keep the API token out of the step that runs the mutation gate The gate passes process.env to the Vitest discovery subprocesses, so reading the open pull request map in the same step that runs stryker-diff.mjs put GH_TOKEN inside those subprocesses. The API call now runs in its own step and only the filtered map crosses the boundary as a step output. Two follow-ups from the same review: - the ci orchestration decision is now an exported resolveCiInvocation so it can be tested directly; a synthetic merge commit proves the stacked path picks the PR head as the diff head and keeps the parent base, and that the non-stacked and unset --pr-head paths keep the merge commit. - createSyntheticStack cleans up its temp repository when setup fails halfway, which the callers could not do because they only receive the path on success. Local run: 49 tests pass (48 before, plus the ci orchestration case). --- .github/workflows/mutation-testing.yml | 26 ++++-- scripts/stryker-diff.mjs | 23 ++++-- scripts/stryker-diff.test.mjs | 107 ++++++++++++++++++------- 3 files changed, 115 insertions(+), 41 deletions(-) diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml index 8bef2278a1..b4ffeefc91 100644 --- a/.github/workflows/mutation-testing.yml +++ b/.github/workflows/mutation-testing.yml @@ -48,21 +48,37 @@ jobs: if: github.event_name == 'pull_request' run: pnpm test:mutation-ci + - name: Resolve the stacked parent from the open pull requests + if: github.event_name == 'pull_request' + id: stacked_map + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + # The fallback must itself be a command: this step runs bash with -e, so a bare '[]' is + # executed as a program name and an API failure would abort the job instead of measuring + # against the event base. + # --jq runs once per page, so a paginated call emits one JSON array per page; the gate + # reads that stream, not a single array. + STACKED_MAP="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" --jq '[.[] | {number: .number, headSha: .head.sha}]')" || STACKED_MAP="$(printf '[]')" + { + echo "stacked_map<> "$GITHUB_OUTPUT" || echo "::warning title=Stacked unit base::Could not publish the stacked map" + - name: Enforce executable-line scope and run advisory mutation testing if: github.event_name == 'pull_request' env: HEAD_SHA: ${{ github.sha }} - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # The gate passes its environment to the Vitest discovery subprocesses, so the API token + # stays in the previous step and only the filtered map crosses here. + STACKED_MAP: ${{ steps.stacked_map.outputs.stacked_map }} run: | BASE_SHA="$(git rev-parse "$HEAD_SHA^1")" # The merge commit's second parent is the pull request head. When that head's own # parent is another open pull request's head, this PR is a stacked unit, so the gate # measures only this unit's delta instead of every unmerged ancestor. PR_HEAD_SHA="$(git rev-parse "$HEAD_SHA^2" 2>/dev/null || git rev-parse "$HEAD_SHA")" - # The fallback must itself be a command: these steps run bash with -e, so a bare '[]' is - # executed as a program name and an API failure would abort the step instead of measuring - # against the event base. - STACKED_MAP="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" --jq '[.[] | {number: .number, headSha: .head.sha}]')" || STACKED_MAP="$(printf '[]')" node scripts/stryker-diff.mjs ci --base "$BASE_SHA" --head "$HEAD_SHA" --pr-head "$PR_HEAD_SHA" --stacked-map "$STACKED_MAP" - name: Upload mutation reports diff --git a/scripts/stryker-diff.mjs b/scripts/stryker-diff.mjs index c5ca2cfae5..276550a0f8 100644 --- a/scripts/stryker-diff.mjs +++ b/scripts/stryker-diff.mjs @@ -297,6 +297,18 @@ export function resolveStackedUnitBase(repoRoot, eventBaseSha, prHeadSha, openPu return { baseSha: parentSha, stackedOn: parent.number } } +// The ci command decides two things from the commit graph: which base to charge this pull request +// to, and which commit to diff against. A stacked unit diffs against its own pull request head, +// because the merge commit also carries whatever main advanced since the parent unit. +// The ci command decides two things from the commit graph: which base to charge this pull request +// to, and which commit to diff against. A stacked unit diffs against its own pull request head, +// because the merge commit also carries whatever main advanced since the parent unit. A plain +// pull request keeps the merge commit as the diff head. +export function resolveCiInvocation(repoRoot, eventBaseSha, mergeCommitSha, prHeadSha, openPullRequests = []) { + const resolved = resolveStackedUnitBase(repoRoot, eventBaseSha, prHeadSha, openPullRequests) + return { baseSha: resolved.baseSha, diffHead: resolved.stackedOn ? prHeadSha : mergeCommitSha, stackedOn: resolved.stackedOn } +} + export function selectFromGit(repoRoot, baseSha, headSha, options = {}) { validateSha(baseSha, "base SHA") validateSha(headSha, "head SHA") @@ -857,16 +869,13 @@ function main() { // first parent is the base tip, so the merge commit cannot identify the stacked unit; its second // parent can. The merge commit stays the diff head. const prHeadSha = argument("--pr-head") ?? headSha - const resolved = resolveStackedUnitBase(repoRoot, baseSha, prHeadSha, openPullRequests) - if (resolved.stackedOn) console.log(`Stacked unit: measured against the head of parent PR #${resolved.stackedOn}, not the event base.`) + const invocation = resolveCiInvocation(repoRoot, baseSha, headSha, prHeadSha, openPullRequests) + if (invocation.stackedOn) console.log(`Stacked unit: measured against the head of parent PR #${invocation.stackedOn}, not the event base.`) const reportRoot = path.resolve(repoRoot, argument("--reports") ?? "reports/mutation") - // A stacked unit is measured against its own head, not the merge commit: the merge commit also - // carries whatever main advanced since the parent unit, and those lines are not this unit's. - const diffHead = resolved.stackedOn ? prHeadSha : headSha const manifest = { - ...selectFromGit(repoRoot, resolved.baseSha, diffHead, { preserveBase: Boolean(resolved.stackedOn) }), - stackedOn: resolved.stackedOn, + ...selectFromGit(repoRoot, invocation.baseSha, invocation.diffHead, { preserveBase: Boolean(invocation.stackedOn) }), + stackedOn: invocation.stackedOn, } if (manifest.packages.length === 0) { appendSummary([], manifest.advisories, manifest) diff --git a/scripts/stryker-diff.test.mjs b/scripts/stryker-diff.test.mjs index ce1c8c7822..e2c13144a8 100644 --- a/scripts/stryker-diff.test.mjs +++ b/scripts/stryker-diff.test.mjs @@ -26,6 +26,7 @@ import { parseVitestTestFiles, preferDirectTestFiles, parseStackedMap, + resolveCiInvocation, resolveStackedUnitBase, resolveStrykerTempDir, resolveVitestBinary, @@ -78,6 +79,12 @@ describe("mutation testing workflow", () => { assert.ok(workflow.includes("steps.mutation_report.outputs.artifact-url")) assert.ok(workflow.includes("open the package's mutation.html file")) assert.ok(workflow.includes("Enforce executable-line scope and run advisory mutation testing")) + // The gate hands its environment to the Vitest discovery subprocesses, so the API token must + // stay in the step that only reads the open pull requests; the gate receives just the map. + const gateStep = workflow.slice(workflow.indexOf("Enforce executable-line scope and run advisory mutation testing")) + const gateStepBody = gateStep.slice(0, gateStep.indexOf("\n - name:")) + assert.ok(!gateStepBody.includes("GH_TOKEN")) + assert.ok(workflow.includes("STACKED_MAP: ${{ steps.stacked_map.outputs.stacked_map }}")) assert.equal(workflow.match(/continue-on-error: true/g)?.length, 1) assert.equal(workflow.match(/Could not write the job summary/g)?.length, 2) const script = fs.readFileSync(path.join(repositoryRoot, "scripts/stryker-diff.mjs"), "utf8") @@ -194,35 +201,42 @@ describe("pull request revision selection", () => { describe("stacked unit base resolution", () => { const createSyntheticStack = () => { const repository = fs.mkdtempSync(path.join(os.tmpdir(), "mutation-stack-")) - const run = (...args) => execFileSync("git", args, { cwd: repository, encoding: "utf8" }).trim() - const write = (filePath, contents) => { - fs.mkdirSync(path.join(repository, path.dirname(filePath)), { recursive: true }) - fs.writeFileSync(path.join(repository, filePath), contents) - } + // Setup can fail halfway (a missing git config, a failed commit), and the callers only get the + // repository path back on success, so cleanup has to happen here or the temp directory leaks. + try { + const run = (...args) => execFileSync("git", args, { cwd: repository, encoding: "utf8" }).trim() + const write = (filePath, contents) => { + fs.mkdirSync(path.join(repository, path.dirname(filePath)), { recursive: true }) + fs.writeFileSync(path.join(repository, filePath), contents) + } + + run("init", "--quiet", "--initial-branch", "main") + run("config", "user.email", "gate@example.com") + run("config", "user.name", "Gate") + run("config", "commit.gpgsign", "false") + + write("packages/core/src/unrelated.ts", "export const unrelated = () => 1\n") + run("add", ".") + run("commit", "--quiet", "-m", "initial") + const eventBaseSha = run("rev-parse", "HEAD") + + run("checkout", "--quiet", "-b", "unit-1") + write("packages/core/src/unit1.ts", "export const unit1 = () => 1\n") + run("add", ".") + run("commit", "--quiet", "-m", "unit 1") + const parentSha = run("rev-parse", "HEAD") + + // The stacked unit is one commit on top of the parent unit's head. + write("packages/core/src/unit2.ts", "export const unit2 = () => 2\n") + run("add", ".") + run("commit", "--quiet", "-m", "unit 2") + const childSha = run("rev-parse", "HEAD") - run("init", "--quiet", "--initial-branch", "main") - run("config", "user.email", "gate@example.com") - run("config", "user.name", "Gate") - run("config", "commit.gpgsign", "false") - - write("packages/core/src/unrelated.ts", "export const unrelated = () => 1\n") - run("add", ".") - run("commit", "--quiet", "-m", "initial") - const eventBaseSha = run("rev-parse", "HEAD") - - run("checkout", "--quiet", "-b", "unit-1") - write("packages/core/src/unit1.ts", "export const unit1 = () => 1\n") - run("add", ".") - run("commit", "--quiet", "-m", "unit 1") - const parentSha = run("rev-parse", "HEAD") - - // The stacked unit is one commit on top of the parent unit's head. - write("packages/core/src/unit2.ts", "export const unit2 = () => 2\n") - run("add", ".") - run("commit", "--quiet", "-m", "unit 2") - const childSha = run("rev-parse", "HEAD") - - return { repository, eventBaseSha, parentSha, childSha, run, write } + return { repository, eventBaseSha, parentSha, childSha, run, write } + } catch (error) { + fs.rmSync(repository, { recursive: true, force: true }) + throw error + } } it("measures a stacked unit against the parent pull request head", () => { @@ -330,7 +344,42 @@ describe("stacked unit base resolution", () => { ) assert.deepEqual(parseStackedMap("[{\"number\": 1, \"headSha\": \"" + "d".repeat(40) + "\"}]garbage"), [ { number: 1, headSha: "d".repeat(40) }, - ]) }) + ]) + }) + + it("the ci command picks the diff head from the commit graph", () => { + const { repository, eventBaseSha, parentSha, childSha, run, write } = createSyntheticStack() + + try { + run("checkout", "--quiet", "-b", "merge-branch", eventBaseSha) + write("packages/core/src/unrelated.ts", "export const unrelated = () => 2\n") + run("add", ".") + run("commit", "--quiet", "-m", "base advance") + const advancedBase = run("rev-parse", "HEAD") + run("merge", "--no-ff", "--quiet", "-m", "merge", childSha) + const mergeSha = run("rev-parse", "HEAD") + + // Stacked: base is the parent PR head and the diff head is the PR head, not the merge + // commit, so the base advance is not charged to this unit. + const stacked = resolveCiInvocation(repository, advancedBase, mergeSha, childSha, [{ number: 1, headSha: parentSha }]) + assert.equal(stacked.baseSha, parentSha) + assert.equal(stacked.diffHead, childSha) + assert.equal(stacked.stackedOn, 1) + + // Non-stacked: the merge commit stays the diff head and the base stays the event base. + const plain = resolveCiInvocation(repository, advancedBase, mergeSha, childSha, []) + assert.equal(plain.baseSha, advancedBase) + assert.equal(plain.diffHead, mergeSha) + assert.equal(plain.stackedOn, null) + + // --pr-head defaults to --head, so a plain pull request keeps the same decision. + const sameHead = resolveCiInvocation(repository, advancedBase, mergeSha, mergeSha, []) + assert.equal(sameHead.diffHead, mergeSha) + assert.equal(sameHead.stackedOn, null) + } finally { + fs.rmSync(repository, { recursive: true, force: true }) + } + }) }) describe("parseNameStatus", () => { From d444279624d4dc0b9879a18b6ce978d9dbd0a130 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Tue, 6 Oct 2026 13:47:08 +0800 Subject: [PATCH 07/11] fix(ci): read the stacked map in a job that never runs pull-request code Splitting the gh api call into its own step was not enough: within the same job the token was still visible after checkout and dependency install had executed pull-request-controlled code. The map is now read in a separate job with no checkout and no setup action, and only the filtered PR number and head SHA cross into the gate job, which has no token in its environment. Also adds the focused formatSummary coverage for the stacked-unit notice: with stackedOn set the summary names the parent PR number, and a plain pull request omits the notice. Local run: 50 tests pass. --- .github/workflows/mutation-testing.yml | 56 +++++++++++++++----------- scripts/stryker-diff.test.mjs | 28 ++++++++++--- 2 files changed, 55 insertions(+), 29 deletions(-) diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml index b4ffeefc91..5489810f6b 100644 --- a/.github/workflows/mutation-testing.yml +++ b/.github/workflows/mutation-testing.yml @@ -8,19 +8,50 @@ on: permissions: contents: read - # The gate only reads open pull request heads to recognise a stacked unit. - pull-requests: read concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} cancel-in-progress: true jobs: + # The open pull request map is read in a job that never checks out or runs pull-request + # code, so the token cannot reach anything the contributor controls. Only the filtered map + # crosses into the job that runs the gate. + stacked_map: + name: Resolve the stacked parent + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read + pull-requests: read + outputs: + stacked_map: ${{ steps.read_map.outputs.stacked_map }} + steps: + - name: Read the open pull request heads + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + # The fallback must itself be a command: this step runs bash with -e, so a bare '[]' + # is executed as a program name and an API failure would abort the job instead of + # measuring against the event base. + # --jq runs once per page, so a paginated call emits one JSON array per page; the + # gate reads that stream, not a single array. + STACKED_MAP="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" --jq '[.[] | {number: .number, headSha: .head.sha}]')" || STACKED_MAP="$(printf '[]')" + { + echo "stacked_map<> "$GITHUB_OUTPUT" || echo "::warning title=Stacked unit base::Could not publish the stacked map" + mutation-diff: name: mutation-diff + needs: stacked_map if: github.event_name == 'merge_group' || github.event.pull_request.draft == false runs-on: ubuntu-latest timeout-minutes: 30 + env: + # Only the filtered map crosses into this job; the token stays in the job that read it. + STACKED_MAP: ${{ needs.stacked_map.outputs.stacked_map }} steps: - name: Record merge-queue enforcement if: github.event_name == 'merge_group' @@ -48,31 +79,10 @@ jobs: if: github.event_name == 'pull_request' run: pnpm test:mutation-ci - - name: Resolve the stacked parent from the open pull requests - if: github.event_name == 'pull_request' - id: stacked_map - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - # The fallback must itself be a command: this step runs bash with -e, so a bare '[]' is - # executed as a program name and an API failure would abort the job instead of measuring - # against the event base. - # --jq runs once per page, so a paginated call emits one JSON array per page; the gate - # reads that stream, not a single array. - STACKED_MAP="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" --jq '[.[] | {number: .number, headSha: .head.sha}]')" || STACKED_MAP="$(printf '[]')" - { - echo "stacked_map<> "$GITHUB_OUTPUT" || echo "::warning title=Stacked unit base::Could not publish the stacked map" - - name: Enforce executable-line scope and run advisory mutation testing if: github.event_name == 'pull_request' env: HEAD_SHA: ${{ github.sha }} - # The gate passes its environment to the Vitest discovery subprocesses, so the API token - # stays in the previous step and only the filtered map crosses here. - STACKED_MAP: ${{ steps.stacked_map.outputs.stacked_map }} run: | BASE_SHA="$(git rev-parse "$HEAD_SHA^1")" # The merge commit's second parent is the pull request head. When that head's own diff --git a/scripts/stryker-diff.test.mjs b/scripts/stryker-diff.test.mjs index e2c13144a8..fb31dfdd75 100644 --- a/scripts/stryker-diff.test.mjs +++ b/scripts/stryker-diff.test.mjs @@ -79,12 +79,17 @@ describe("mutation testing workflow", () => { assert.ok(workflow.includes("steps.mutation_report.outputs.artifact-url")) assert.ok(workflow.includes("open the package's mutation.html file")) assert.ok(workflow.includes("Enforce executable-line scope and run advisory mutation testing")) - // The gate hands its environment to the Vitest discovery subprocesses, so the API token must - // stay in the step that only reads the open pull requests; the gate receives just the map. - const gateStep = workflow.slice(workflow.indexOf("Enforce executable-line scope and run advisory mutation testing")) - const gateStepBody = gateStep.slice(0, gateStep.indexOf("\n - name:")) - assert.ok(!gateStepBody.includes("GH_TOKEN")) - assert.ok(workflow.includes("STACKED_MAP: ${{ steps.stacked_map.outputs.stacked_map }}")) + // The gate hands its environment to the Vitest discovery subprocesses, so the token has to stay + // in a job that never checks out or runs pull-request code; only the filtered map crosses over. + const gateJob = workflow.slice(workflow.indexOf(" mutation-diff:")) + assert.ok(!gateJob.includes("GH_TOKEN")) + assert.ok(workflow.includes("STACKED_MAP: ${{ needs.stacked_map.outputs.stacked_map }}")) + const mapJob = workflow.slice(workflow.indexOf(" stacked_map:"), workflow.indexOf(" mutation-diff:")) + assert.ok(mapJob.includes("GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}")) + assert.ok(!mapJob.includes("actions/checkout")) + assert.ok(!mapJob.includes("setup-node-pnpm")) + assert.ok(!mapJob.includes("pnpm test:mutation-ci")) + assert.ok(workflow.includes("needs: stacked_map")) assert.equal(workflow.match(/continue-on-error: true/g)?.length, 1) assert.equal(workflow.match(/Could not write the job summary/g)?.length, 2) const script = fs.readFileSync(path.join(repositoryRoot, "scripts/stryker-diff.mjs"), "utf8") @@ -1131,6 +1136,17 @@ describe("failure output", () => { assert.equal(new Set(summary.match(/Package\dMutator\d+/g)).size, 6 * MAX_MUTANTS) assert.ok(Buffer.byteLength(summary) < 1024 * 1024) }) + + it("names the parent pull request when the unit was measured against a stacked base", () => { + const rows = [{ id: "extension", changedLines: 12, valid: 3, killed: 3, timeout: 0, survived: 0, noCoverage: 0, blocking: [], result: "Pass" }] + const summary = formatSummary(rows, [], { stackedOn: 1914 }) + assert.ok(summary.includes("Stacked unit: measured against the head of parent PR #1914, not the event base.")) + + // A plain pull request has no stacked base, so the notice must not appear. + const plain = formatSummary(rows, [], { stackedOn: null }) + assert.ok(!plain.includes("Stacked unit")) + assert.ok(plain.includes("## Changed-code mutation testing")) + }) }) describe("report evaluation", () => { From 3da494b5f4634d63207bec1ea66e5b79e2760636 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Tue, 6 Oct 2026 13:56:50 +0800 Subject: [PATCH 08/11] fix(ci): give the map step the id its job output reads `outputs.stacked_map` referenced `steps.read_map` but no step had that id, so the output was always an empty string, the gate always saw an empty map, and the stacked-unit detection never ran in CI. The workflow-shape assertions only matched strings, so the tests still passed. Publishing is also one validated record now: the value is checked before it is written, and a failed write aborts the job instead of continuing with an empty output. Local run: 50 tests pass, including assertions that the step id exists and that publication failure is not swallowed. --- .github/workflows/mutation-testing.yml | 14 +++++++++----- scripts/stryker-diff.test.mjs | 6 ++++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml index 5489810f6b..72969f4a42 100644 --- a/.github/workflows/mutation-testing.yml +++ b/.github/workflows/mutation-testing.yml @@ -28,6 +28,7 @@ jobs: stacked_map: ${{ steps.read_map.outputs.stacked_map }} steps: - name: Read the open pull request heads + id: read_map env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | @@ -37,11 +38,14 @@ jobs: # --jq runs once per page, so a paginated call emits one JSON array per page; the # gate reads that stream, not a single array. STACKED_MAP="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls?state=open&per_page=100" --jq '[.[] | {number: .number, headSha: .head.sha}]')" || STACKED_MAP="$(printf '[]')" - { - echo "stacked_map<> "$GITHUB_OUTPUT" || echo "::warning title=Stacked unit base::Could not publish the stacked map" + # Publish the whole value as one record from a validated string, and let a failed + # write abort the job: a truncated or skipped output would leave the gate measuring + # against the event base while still reporting success. + case "$STACKED_MAP" in + "["*) ;; + *) echo "::error title=Stacked unit base::The stacked map is not a JSON array" >&2; exit 1 ;; + esac + printf 'stacked_map<> "$GITHUB_OUTPUT" mutation-diff: name: mutation-diff diff --git a/scripts/stryker-diff.test.mjs b/scripts/stryker-diff.test.mjs index fb31dfdd75..a3a0e379aa 100644 --- a/scripts/stryker-diff.test.mjs +++ b/scripts/stryker-diff.test.mjs @@ -90,6 +90,12 @@ describe("mutation testing workflow", () => { assert.ok(!mapJob.includes("setup-node-pnpm")) assert.ok(!mapJob.includes("pnpm test:mutation-ci")) assert.ok(workflow.includes("needs: stacked_map")) + // The job output is only real if the step it reads from carries that id; without it the map is + // always an empty string and the gate silently falls back to the event base. + assert.ok(mapJob.includes("id: read_map")) + // Publish as one record from a validated value, and do not continue past a failed write. + assert.ok(mapJob.includes("printf 'stacked_map< Date: Tue, 6 Oct 2026 14:14:05 +0800 Subject: [PATCH 09/11] test(gate): cover the multi-parent head fallback A unit is one commit on top of its parent, so a pull request head that is itself a merge commit is not a single unit delta and keeps the event base even when one of its parents is an open pull request head. That rule was implicit; it is now stated in the resolver and asserted with a synthetic merge commit whose second parent is the parent pull request head. Local run: 50 tests pass. --- scripts/stryker-diff.mjs | 3 +++ scripts/stryker-diff.test.mjs | 14 ++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/scripts/stryker-diff.mjs b/scripts/stryker-diff.mjs index 276550a0f8..9f4bc2af2d 100644 --- a/scripts/stryker-diff.mjs +++ b/scripts/stryker-diff.mjs @@ -290,6 +290,9 @@ export function resolveStackedUnitBase(repoRoot, eventBaseSha, prHeadSha, openPu validateSha(eventBaseSha, "base SHA") validateSha(prHeadSha, "pull request head SHA") const parents = git(repoRoot, ["rev-list", "--parents", "-n", "1", prHeadSha]).trim().split(/\s+/).slice(1) + // A unit is one commit on top of its parent. A head with more than one parent is a merge on the + // unit branch itself, so its diff is not a single unit delta and the event base is kept even when + // one of those parents is an open pull request head. if (parents.length !== 1) return { baseSha: eventBaseSha, stackedOn: null } const parentSha = parents[0].toLowerCase() const parent = openPullRequests.find((pr) => String(pr.headSha).toLowerCase() === parentSha) diff --git a/scripts/stryker-diff.test.mjs b/scripts/stryker-diff.test.mjs index a3a0e379aa..1c5ece873e 100644 --- a/scripts/stryker-diff.test.mjs +++ b/scripts/stryker-diff.test.mjs @@ -387,6 +387,20 @@ describe("stacked unit base resolution", () => { const sameHead = resolveCiInvocation(repository, advancedBase, mergeSha, mergeSha, []) assert.equal(sameHead.diffHead, mergeSha) assert.equal(sameHead.stackedOn, null) + + // A unit branch that is itself a merge commit is not a single unit delta, so the fallback + // stays the event base even when one of its parents is an open pull request head. + run("checkout", "--quiet", "-b", "unit-3", advancedBase) + write("packages/core/src/unit3.ts", "export const unit3 = () => 3\n") + run("add", ".") + run("commit", "--quiet", "-m", "unit 3") + run("merge", "--no-ff", "--quiet", "-m", "unit merge", parentSha) + const multiParentHead = run("rev-parse", "HEAD") + assert.equal(run("rev-list", "--parents", "-n", "1", multiParentHead).trim().split(/\s+/).slice(1).length, 2) + const multi = resolveStackedUnitBase(repository, advancedBase, multiParentHead, [{ number: 1, headSha: parentSha }]) + assert.equal(multi.baseSha, advancedBase) + assert.equal(multi.stackedOn, null) + assert.equal(resolveCiInvocation(repository, advancedBase, multiParentHead, multiParentHead, [{ number: 1, headSha: parentSha }]).stackedOn, null) } finally { fs.rmSync(repository, { recursive: true, force: true }) } From a1a5fe90119720b0b8c30d17c9e50c5b121216e6 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Tue, 6 Oct 2026 15:45:14 +0800 Subject: [PATCH 10/11] refactor(gate): drop the duplicated comment above resolveCiInvocation The function had the same three lines twice, the second copy adding the plain-pull-request case. Kept the fuller version once. Local run: 50 tests pass. --- scripts/stryker-diff.mjs | 3 --- 1 file changed, 3 deletions(-) diff --git a/scripts/stryker-diff.mjs b/scripts/stryker-diff.mjs index 9f4bc2af2d..96ac1ec1a6 100644 --- a/scripts/stryker-diff.mjs +++ b/scripts/stryker-diff.mjs @@ -300,9 +300,6 @@ export function resolveStackedUnitBase(repoRoot, eventBaseSha, prHeadSha, openPu return { baseSha: parentSha, stackedOn: parent.number } } -// The ci command decides two things from the commit graph: which base to charge this pull request -// to, and which commit to diff against. A stacked unit diffs against its own pull request head, -// because the merge commit also carries whatever main advanced since the parent unit. // The ci command decides two things from the commit graph: which base to charge this pull request // to, and which commit to diff against. A stacked unit diffs against its own pull request head, // because the merge commit also carries whatever main advanced since the parent unit. A plain From 9dea54e3d22bee371314045979380746be7e5036 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech Date: Tue, 6 Oct 2026 16:03:18 +0800 Subject: [PATCH 11/11] fix(gate): align selection, source reads, and mutation to one tree; reject a partial stacked map Two findings from the review at head: 1. Functional Correctness (major). The gate derived selectors from the pull request head tree while Stryker mutated the checkout, which is GitHub's merge commit. When main advanced a file the unit also changed, the two trees disagree on line numbers, so selectors could target the wrong code or miss the unit's changed lines. The ci command now aligns the working tree to the diff head before mutation runs, so selection, source reads, and mutation all see one tree. The new test advances a file on the base, merges, and asserts the tree used for mutation is the unit tree. 2. Functional Correctness (minor). parseStackedMap matched bracket groups and ignored trailing garbage, so a partially readable map could still select a parent base instead of the event-base fallback. It now consumes the whole stream of page arrays and returns [] when any part cannot be parsed. Also dropped the duplicated comment block above resolveCiInvocation. Local run: 51 tests pass. --- scripts/stryker-diff.mjs | 56 +++++++++++++++++++++++++++++++---- scripts/stryker-diff.test.mjs | 37 +++++++++++++++++++++-- 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/scripts/stryker-diff.mjs b/scripts/stryker-diff.mjs index 96ac1ec1a6..4f861af332 100644 --- a/scripts/stryker-diff.mjs +++ b/scripts/stryker-diff.mjs @@ -309,6 +309,19 @@ export function resolveCiInvocation(repoRoot, eventBaseSha, mergeCommitSha, prHe return { baseSha: resolved.baseSha, diffHead: resolved.stackedOn ? prHeadSha : mergeCommitSha, stackedOn: resolved.stackedOn } } +// Selection, source reads, and Stryker must all see the same tree. The workflow checks out +// GitHub's merge commit, which also carries whatever main advanced since the parent unit, so for +// a stacked unit the working tree is moved to the unit head before mutation runs; otherwise the +// selectors come from one tree and the mutated source comes from another. +export function alignExecutionTree(repoRoot, diffHeadSha) { + validateSha(diffHeadSha, "diff head SHA") + const current = git(repoRoot, ["rev-parse", "HEAD"]).trim().toLowerCase() + const wanted = String(diffHeadSha).toLowerCase() + if (current === wanted) return false + git(repoRoot, ["checkout", "--quiet", wanted]) + return true +} + export function selectFromGit(repoRoot, baseSha, headSha, options = {}) { validateSha(baseSha, "base SHA") validateSha(headSha, "head SHA") @@ -837,17 +850,44 @@ function argument(name) { // instead of throwing. export function parseStackedMap(value) { if (!value) return [] - const chunks = String(value).match(/\[[^\]]*\]/g) || [String(value)] + // gh api --paginate emits one JSON array per page, so the input is a stream of arrays rather + // than a single array. Consume the whole stream: if any part of it cannot be parsed, the map + // is not trustworthy as a whole and the caller must fall back to the event base instead of + // keeping a partial entry that could select a parent base. + const text = String(value) const entries = [] - for (const chunk of chunks) { + let index = 0 + while (index < text.length) { + while (index < text.length && /[\s,]/.test(text[index])) index += 1 + if (index >= text.length) break + if (text[index] !== "[") return [] + let depth = 0 + let inString = false + let end = -1 + for (let i = index; i < text.length; i++) { + const char = text[i] + if (inString) { + if (char === "\\") { i += 1; continue } + if (char === "\"") inString = false + continue + } + if (char === "\"") { inString = true; continue } + if (char === "[") depth += 1 + else if (char === "]") { + depth -= 1 + if (depth === 0) { end = i; break } + } + } + if (depth !== 0 || end < 0) return [] let parsed try { - parsed = JSON.parse(chunk) + parsed = JSON.parse(text.slice(index, end + 1)) } catch { - continue + return [] } - if (!Array.isArray(parsed)) continue + if (!Array.isArray(parsed)) return [] entries.push(...parsed.filter((entry) => entry && entry.number && /^[0-9a-f]{40}$/i.test(String(entry.headSha)))) + index = end + 1 } return entries } @@ -867,10 +907,14 @@ function main() { const openPullRequests = parseStackedMap(argument("--stacked-map")) // --pr-head is the pull request head itself. The workflow runs on GitHub's merge commit, whose // first parent is the base tip, so the merge commit cannot identify the stacked unit; its second - // parent can. The merge commit stays the diff head. + // parent can. A plain pull request keeps the merge commit as the diff head; a stacked unit + // uses its own head, and the working tree is aligned to that head below. const prHeadSha = argument("--pr-head") ?? headSha const invocation = resolveCiInvocation(repoRoot, baseSha, headSha, prHeadSha, openPullRequests) if (invocation.stackedOn) console.log(`Stacked unit: measured against the head of parent PR #${invocation.stackedOn}, not the event base.`) + if (invocation.stackedOn && alignExecutionTree(repoRoot, invocation.diffHead)) { + console.log(`Aligned the working tree to the diff head ${invocation.diffHead.slice(0, 12)} so selection, source reads, and mutation run on the same tree.`) + } const reportRoot = path.resolve(repoRoot, argument("--reports") ?? "reports/mutation") const manifest = { diff --git a/scripts/stryker-diff.test.mjs b/scripts/stryker-diff.test.mjs index 1c5ece873e..e8185bcbfd 100644 --- a/scripts/stryker-diff.test.mjs +++ b/scripts/stryker-diff.test.mjs @@ -26,6 +26,7 @@ import { parseVitestTestFiles, preferDirectTestFiles, parseStackedMap, + alignExecutionTree, resolveCiInvocation, resolveStackedUnitBase, resolveStrykerTempDir, @@ -353,9 +354,10 @@ describe("stacked unit base resolution", () => { { number: 2, headSha: "c".repeat(40) }, ], ) - assert.deepEqual(parseStackedMap("[{\"number\": 1, \"headSha\": \"" + "d".repeat(40) + "\"}]garbage"), [ - { number: 1, headSha: "d".repeat(40) }, - ]) + // Trailing content cannot be consumed as a page, so the whole map is rejected rather than + // keeping a partial entry that could select a parent base. + assert.deepEqual(parseStackedMap("[{\"number\": 1, \"headSha\": \"" + "d".repeat(40) + "\"}]garbage"), []) + assert.deepEqual(parseStackedMap("[{\"number\": 1, \"headSha\": \"" + "d".repeat(40) + "\"}] [{\"number\": 2"), []) }) it("the ci command picks the diff head from the commit graph", () => { @@ -405,6 +407,35 @@ describe("stacked unit base resolution", () => { fs.rmSync(repository, { recursive: true, force: true }) } }) + + it("aligns the working tree to the diff head so selection and mutation see one tree", () => { + const { repository, eventBaseSha, parentSha, childSha, run, write } = createSyntheticStack() + + try { + run("checkout", "--quiet", "-b", "merge-branch", eventBaseSha) + // The base advance changes a file, so the merge tree differs from the unit tree. The + // selectors are derived from the unit tree, so mutation has to run on that same tree. + write("packages/core/src/unrelated.ts", "export const unrelated = () => 99\n") + run("add", ".") + run("commit", "--quiet", "-m", "base advance") + const advancedBase = run("rev-parse", "HEAD") + run("merge", "--no-ff", "--quiet", "-m", "merge", childSha) + const mergeSha = run("rev-parse", "HEAD") + + const invocation = resolveCiInvocation(repository, advancedBase, mergeSha, childSha, [{ number: 1, headSha: parentSha }]) + assert.equal(invocation.diffHead, childSha) + assert.equal(run("rev-parse", "HEAD").toLowerCase(), mergeSha.toLowerCase()) + + assert.equal(alignExecutionTree(repository, invocation.diffHead), true) + assert.equal(run("rev-parse", "HEAD").toLowerCase(), childSha.toLowerCase()) + // The tree Stryker would mutate is now the unit tree, not the merge tree. + assert.equal(fs.readFileSync(path.join(repository, "packages/core/src/unit2.ts"), "utf8").replace(/\r/g, ""), "export const unit2 = () => 2\n") + // Aligning again is a no-op. + assert.equal(alignExecutionTree(repository, invocation.diffHead), false) + } finally { + fs.rmSync(repository, { recursive: true, force: true }) + } + }) }) describe("parseNameStatus", () => {