`expandRunsOn` enumerated a multi-axis `strategy.matrix` one key at a time,
producing partial realization contexts. A true `os × shell` matrix therefore
left `${{ matrix.shell }}` unresolvable against any `{ os: ... }`-only context,
firing spurious `UNRESOLVABLE_MATRIX` violations and leaving shell-pinning
coverage incomplete on Cartesian jobs.
Enumerate the full GitHub Actions cross-product of all base-list matrix keys
(every `matrix.<k>` array, excluding the `include`/`exclude` control keys) via
a named `cartesianProduct` helper. Each realization's context now carries a
value for every matrix key, so `${{ matrix.<key> }}` resolves per realization.
Single-axis matrices keep byte-for-byte identical output; only multi-axis
matrices change shape. The `include` and `exclude` blocks are unchanged
(full tuple-aware exclude is a documented out-of-scope follow-up).
Tests: updated the `os × shell` test to assert post-fix behavior (4 step
realizations, 2 WRONG_SHELL_FOR_OS, 0 UNRESOLVABLE_MATRIX); added a compliant
`os × node-version` cross-product test; added a fast-check property test that
the realization count equals the product of axis lengths and that no axis key
is dropped from any context.
Closes #435
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -36,6 +36,26 @@ function runnerDefault(runner) {
|
||||
return 'bash'; // ubuntu-* and macos-* both default to bash on GHA
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Matrix expansion helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
// Build the GitHub Actions Cartesian product of the given base-list matrix
|
||||
// keys. Returns one context object per realized job, each mapping every key
|
||||
// to a stringified value. A single key yields one context per value (identical
|
||||
// to the legacy base-list expansion); N keys yield the full cross-product.
|
||||
// Values are stringified to match runner-label comparison and matrix-expression
|
||||
// resolution, which operate on strings.
|
||||
function cartesianProduct(keys, matrix) {
|
||||
return keys.reduce(
|
||||
(contexts, key) =>
|
||||
contexts.flatMap((context) =>
|
||||
matrix[key].map((value) => ({ ...context, [key]: String(value) })),
|
||||
),
|
||||
[{}],
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Matrix expansion
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -88,19 +108,32 @@ function expandRunsOn(runsOnRaw, matrix) {
|
||||
}
|
||||
}
|
||||
|
||||
// Collect values from matrix.<key> list (e.g. matrix.os: [ubuntu, macos])
|
||||
// These base-list entries have no extra context beyond the key itself.
|
||||
// Each entry is pushed unconditionally — deduplicating by runner alone
|
||||
// would collapse distinct Cartesian rows (e.g. duplicate os values paired
|
||||
// with different shell values) and hide policy violations on later rows.
|
||||
// GitHub Actions realizes one job per element of the Cartesian product of all
|
||||
// base-list matrix keys (every matrix.<k> that is an array, excluding the
|
||||
// include/exclude control keys), and each realized job's context carries a
|
||||
// value for EVERY matrix key — so ${{ matrix.<key> }} references (e.g. in a
|
||||
// shell: field) resolve against any realization, not only the runs-on key.
|
||||
// Single-axis matrices yield exactly one realization per value, identical to
|
||||
// the prior behavior; only multi-axis matrices change shape.
|
||||
// Each realization is pushed unconditionally — deduplicating by runner label
|
||||
// would collapse distinct Cartesian rows (e.g. matrix.os: [macos-latest,
|
||||
// macos-latest] paired with different shells) and hide policy violations.
|
||||
if (Array.isArray(matrix[key])) {
|
||||
for (const val of matrix[key]) {
|
||||
const runner = String(val);
|
||||
realizations.push({ runner, resolvable: true, context: { [key]: runner } });
|
||||
const baseListKeys = Object.keys(matrix).filter(
|
||||
(k) => k !== 'include' && k !== 'exclude' && Array.isArray(matrix[k]),
|
||||
);
|
||||
for (const context of cartesianProduct(baseListKeys, matrix)) {
|
||||
realizations.push({ runner: context[key], resolvable: true, context });
|
||||
}
|
||||
}
|
||||
|
||||
// matrix.exclude: remove matches
|
||||
// matrix.exclude: remove matches by runner label (first match only).
|
||||
// KNOWN LIMITATION (out of scope for #435, tracked as a follow-up): this
|
||||
// matches on the runs-on key's runner label rather than the full exclude
|
||||
// tuple, so a multi-axis exclude like { os: macos-latest, shell: bash } can
|
||||
// remove the wrong cross-product cell. Full GitHub Actions tuple-match
|
||||
// (including the include-rows-are-not-excluded rule) is deferred; #435 scopes
|
||||
// only the base-list cross-product expansion above.
|
||||
if (Array.isArray(matrix.exclude)) {
|
||||
for (const excl of matrix.exclude) {
|
||||
if (excl && excl[key] != null) {
|
||||
|
||||
@@ -492,24 +492,20 @@ jobs:
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Test 8a — Counter-test: Cartesian matrix os × shell — dedup must not collapse rows by runner alone
|
||||
// Test 8a — Cartesian matrix os × shell — full cross-product expansion resolves
|
||||
// matrix.shell per realization
|
||||
// (mechanism: matrix.os: [macos-latest, macos-latest] with matrix.shell: [zsh, bash]
|
||||
// and runs-on: ${{ matrix.os }}, step shell: ${{ matrix.shell }}.
|
||||
// The base-list path in expandRunsOn previously deduped by runner alone, collapsing
|
||||
// both macos-latest rows into one. Post-fix: each entry is pushed unconditionally,
|
||||
// producing 2 realizations from the base-list os array.
|
||||
// GitHub Actions realizes a 2×2 grid (4 jobs). After the Cartesian-product fix,
|
||||
// expandRunsOn must produce 4 realizations, each carrying BOTH os AND shell in
|
||||
// context so that ${{ matrix.shell }} resolves per realization.
|
||||
//
|
||||
// NOTE: Cartesian cross-product expansion (expanding the full os × shell grid so
|
||||
// that each realization carries BOTH os and shell in its context) is not yet
|
||||
// implemented in expandRunsOn. The base-list path only records { os: runner } in
|
||||
// context, so ${{ matrix.shell }} on the step cannot be resolved and the linter
|
||||
// emits UNRESOLVABLE_MATRIX. The ideal post-Cartesian-expansion behavior would be
|
||||
// 2 WRONG_SHELL_FOR_OS violations (the bash rows). That is a separate follow-up bug.
|
||||
//
|
||||
// This test validates the dedupe fix only: 2 violations must be produced (not 1),
|
||||
// proving the base-list path no longer collapses duplicate runner values.
|
||||
// Expected post-fix behavior:
|
||||
// - 4 total step-results (2 os × 2 shell)
|
||||
// - exactly 2 WRONG_SHELL_FOR_OS violations — the two bash cells on macos-latest
|
||||
// - ZERO UNRESOLVABLE_MATRIX violations (matrix.shell is now fully resolved)
|
||||
// ---------------------------------------------------------------------------
|
||||
describe('Cartesian matrix os × shell — dedup must not collapse rows by runner alone', () => {
|
||||
describe('Cartesian matrix os × shell — full cross-product expansion resolves matrix.shell per realization', () => {
|
||||
const CARTESIAN_MATRIX_YAML = `
|
||||
name: Cartesian Matrix
|
||||
jobs:
|
||||
@@ -525,44 +521,160 @@ jobs:
|
||||
run: echo hi
|
||||
`;
|
||||
|
||||
test('Cartesian matrix os × shell — dedup must not collapse rows by runner alone', () => {
|
||||
test('2×2 Cartesian product yields 4 step-results, 2 WRONG_SHELL_FOR_OS violations, 0 UNRESOLVABLE_MATRIX', () => {
|
||||
const result = inspectWorkflow(CARTESIAN_MATRIX_YAML, { filePath: '<synthetic-cartesian-matrix>' });
|
||||
|
||||
const violations = result.jobs
|
||||
.flatMap(j => j.steps)
|
||||
.filter(s => s.violation !== null);
|
||||
const allSteps = result.jobs.flatMap(j => j.steps);
|
||||
const violations = allSteps.filter(s => s.violation !== null);
|
||||
const unresolvable = violations.filter(s => s.violation === VIOLATION.UNRESOLVABLE_MATRIX);
|
||||
const wrongShell = violations.filter(s => s.violation === VIOLATION.WRONG_SHELL_FOR_OS);
|
||||
|
||||
// The dedupe fix ensures both macos-latest entries in matrix.os are expanded
|
||||
// independently, yielding 2 realizations — not 1 (as the old dedup-by-runner
|
||||
// guard would produce). Each realization's ${{ matrix.shell }} is currently
|
||||
// UNRESOLVABLE_MATRIX because the base-list path doesn't yet carry shell context
|
||||
// (Cartesian cross-product is a separate follow-up fix).
|
||||
// 4 step-results: 2 os values × 2 shell values = 4 realizations, each with 1 step
|
||||
assert.strictEqual(
|
||||
violations.length,
|
||||
allSteps.length,
|
||||
4,
|
||||
`Expected 4 step-results (2×2 Cartesian product) but got ${allSteps.length}. All steps: ` +
|
||||
allSteps.map(s => `runner=${s.runner} shell=${s.effectiveShell} violation=${s.violation}`).join(', ')
|
||||
);
|
||||
|
||||
// Core regression proof: ZERO UNRESOLVABLE_MATRIX (matrix.shell now resolves)
|
||||
assert.strictEqual(
|
||||
unresolvable.length,
|
||||
0,
|
||||
`Expected 0 UNRESOLVABLE_MATRIX violations but got ${unresolvable.length}: ` +
|
||||
unresolvable.map(v => `runner=${v.runner} shell=${v.effectiveShell} type=${v.violation}`).join(', ')
|
||||
);
|
||||
|
||||
// Exactly 2 WRONG_SHELL_FOR_OS: the two bash cells on macos-latest
|
||||
assert.strictEqual(
|
||||
wrongShell.length,
|
||||
2,
|
||||
`Expected exactly 2 violations (dedup fix: both macos-latest rows preserved) but got ${violations.length}: ` +
|
||||
`Expected exactly 2 WRONG_SHELL_FOR_OS violations (bash on macos-latest) but got ${wrongShell.length}: ` +
|
||||
violations.map(v => `runner=${v.runner} shell=${v.effectiveShell} type=${v.violation}`).join(', ')
|
||||
);
|
||||
|
||||
for (const v of violations) {
|
||||
for (const v of wrongShell) {
|
||||
assert.strictEqual(
|
||||
v.runner,
|
||||
'macos-latest',
|
||||
`Expected violation runner to be macos-latest but got ${v.runner}`
|
||||
);
|
||||
// UNRESOLVABLE_MATRIX because Cartesian cross-product expansion is not yet
|
||||
// implemented; ${{ matrix.shell }} cannot be resolved from base-list context.
|
||||
// When Cartesian expansion is added, these will become WRONG_SHELL_FOR_OS
|
||||
// (for the bash rows) and compliant (for the zsh rows).
|
||||
assert.strictEqual(
|
||||
v.violation,
|
||||
VIOLATION.UNRESOLVABLE_MATRIX,
|
||||
`Expected UNRESOLVABLE_MATRIX (shell key absent from base-list context) but got ${v.violation}`
|
||||
v.effectiveShell,
|
||||
'bash',
|
||||
`Expected effectiveShell to be bash (the violating cell) but got ${v.effectiveShell}`
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Test 8b — Cartesian matrix os × node-version — compliant cross-product carries
|
||||
// extra axis without violations
|
||||
// (mechanism: matrix.os: [ubuntu-latest, ubuntu-latest] with
|
||||
// matrix.node-version: [22, 24], step shell: bash (literal).
|
||||
// The cross-product yields 4 realizations. ubuntu-latest + bash = compliant.
|
||||
// Pre-fix: only os is expanded → 2 step-results. Post-fix: 4 step-results.
|
||||
// This is the fail-first signal for the Cartesian expansion fix.
|
||||
// ---------------------------------------------------------------------------
|
||||
describe('Cartesian matrix os × node-version — compliant cross-product yields 4 step-results with 0 violations', () => {
|
||||
const CARTESIAN_COMPLIANT_YAML = `
|
||||
name: Cartesian Compliant
|
||||
jobs:
|
||||
build:
|
||||
runs-on: \${{ matrix.os }}
|
||||
strategy:
|
||||
matrix:
|
||||
os: [ubuntu-latest, ubuntu-latest]
|
||||
node-version: [22, 24]
|
||||
steps:
|
||||
- name: Run tests
|
||||
shell: bash
|
||||
run: npm test
|
||||
`;
|
||||
|
||||
test('2×2 Cartesian product yields 4 step-results (fail-first signal pre-fix: 2) and 0 violations', () => {
|
||||
const result = inspectWorkflow(CARTESIAN_COMPLIANT_YAML, { filePath: '<synthetic-cartesian-compliant>' });
|
||||
|
||||
const allSteps = result.jobs.flatMap(j => j.steps);
|
||||
const violations = allSteps.filter(s => s.violation !== null);
|
||||
|
||||
// Pre-fix: only os is expanded → 2 step-results; post-fix: 4
|
||||
assert.strictEqual(
|
||||
allSteps.length,
|
||||
4,
|
||||
`Expected 4 step-results (2 os × 2 node-version Cartesian product) but got ${allSteps.length}. ` +
|
||||
`If you see 2, the Cartesian expansion fix is not yet applied. All steps: ` +
|
||||
allSteps.map(s => `runner=${s.runner} shell=${s.effectiveShell} violation=${s.violation}`).join(', ')
|
||||
);
|
||||
|
||||
// ubuntu-latest + literal bash = compliant; no violations expected
|
||||
assert.strictEqual(
|
||||
violations.length,
|
||||
0,
|
||||
`Expected 0 violations (ubuntu-latest + bash is compliant) but got ${violations.length}: ` +
|
||||
violations.map(v => `runner=${v.runner} shell=${v.effectiveShell} type=${v.violation}`).join(', ')
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Test 8c — Property-based: for any 1–3 base-list axes with 1–3 values each,
|
||||
// step count === Cartesian product of axis lengths
|
||||
// (fast-check is a devDependency: fast-check ^4.8.0)
|
||||
// ---------------------------------------------------------------------------
|
||||
const fc = require('fast-check');
|
||||
|
||||
describe('property-based: step count equals product of all axis lengths for arbitrary base-list matrices', () => {
|
||||
test('step count === product(axisLengths) for 1–3 axes with 1–3 values each', () => {
|
||||
// Axis keys named k0, k1, k2; values restricted to [A-Za-z0-9-] to keep YAML well-formed.
|
||||
// runs-on references ${{ matrix.k0 }}; the single step's shell references the
|
||||
// LAST matrix axis key (${{ matrix.kLast }}) so that a dropped axis key would
|
||||
// surface as UNRESOLVABLE_MATRIX rather than silently resolving.
|
||||
const axisValueArb = fc.stringMatching(/^[A-Za-z][A-Za-z0-9-]{0,7}$/);
|
||||
const axisArb = fc.array(axisValueArb, { minLength: 1, maxLength: 3 });
|
||||
const matrixArb = fc.array(axisArb, { minLength: 1, maxLength: 3 });
|
||||
|
||||
fc.assert(
|
||||
fc.property(matrixArb, (axes) => {
|
||||
// Build YAML matrix block
|
||||
const keys = axes.map((_, i) => `k${i}`);
|
||||
const lastKey = keys[keys.length - 1];
|
||||
const matrixLines = keys.map((k, i) => ` ${k}: [${axes[i].join(', ')}]`);
|
||||
|
||||
const yaml = [
|
||||
'name: PropertyTest',
|
||||
'jobs:',
|
||||
' build:',
|
||||
' runs-on: ${{ matrix.k0 }}',
|
||||
' strategy:',
|
||||
' matrix:',
|
||||
...matrixLines,
|
||||
' steps:',
|
||||
' - name: Run tests',
|
||||
` shell: \${{ matrix.${lastKey} }}`,
|
||||
' run: echo hi',
|
||||
].join('\n');
|
||||
|
||||
const result = inspectWorkflow(yaml, { filePath: '<property-test>' });
|
||||
const allSteps = result.jobs.flatMap(j => j.steps);
|
||||
const actualSteps = allSteps.length;
|
||||
const expectedSteps = axes.reduce((acc, axis) => acc * axis.length, 1);
|
||||
|
||||
// Every realization must carry the last axis key in context so that
|
||||
// ${{ matrix.kLast }} resolves. If the key is absent from any
|
||||
// realization's context, effectiveShell fires UNRESOLVABLE_MATRIX.
|
||||
const noUnresolvable = allSteps.every(
|
||||
s => s.violation !== VIOLATION.UNRESOLVABLE_MATRIX,
|
||||
);
|
||||
|
||||
return actualSteps === expectedSteps && noUnresolvable;
|
||||
}),
|
||||
{ seed: 42, numRuns: 100 }
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Test 8 — Counter-test: two macos-latest matrix.include rows where
|
||||
// row 1 has shell: zsh (compliant) and row 2 has shell: bash (violation).
|
||||
|
||||
Reference in New Issue
Block a user