ci: Scope path-filter and janitor diff to PR-only changes (#29993)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Declan Carroll 2026-05-07 12:41:35 +01:00 committed by GitHub
parent 0edcdcfe85
commit 8573197aef
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 97 additions and 6 deletions

View File

@ -1,6 +1,10 @@
import { describe, it } from 'node:test'; import { describe, it, before, after } from 'node:test';
import assert from 'node:assert/strict'; import assert from 'node:assert/strict';
import { matchGlob, parseFilters, evaluateFilter, runValidate } from '../ci-filter.mjs'; import { execFileSync } from 'node:child_process';
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { matchGlob, parseFilters, evaluateFilter, runValidate, getChangedFiles, getMergeBase } from '../ci-filter.mjs';
// --- matchGlob --- // --- matchGlob ---
@ -172,6 +176,70 @@ describe('evaluateFilter', () => {
}); });
}); });
// --- getChangedFiles + getMergeBase (integration, exercises real git) ---
describe('getChangedFiles', () => {
const repoDir = mkdtempSync(join(tmpdir(), 'ci-filter-'));
const remoteDir = mkdtempSync(join(tmpdir(), 'ci-filter-remote-'));
const originalCwd = process.cwd();
const git = (args: string[], cwd: string = repoDir) =>
execFileSync('git', args, { cwd, stdio: 'pipe' }).toString().trim();
before(() => {
// Bare remote so the action's `git fetch origin <ref>` works
execFileSync('git', ['init', '--bare', '-b', 'main', remoteDir], { stdio: 'pipe' });
git(['init', '-b', 'main'], repoDir);
git(['config', 'user.email', 'test@test.local']);
git(['config', 'user.name', 'test']);
git(['remote', 'add', 'origin', remoteDir]);
// Common ancestor commit
writeFileSync(join(repoDir, 'shared.ts'), 'shared\n');
git(['add', '.']);
git(['commit', '-m', 'root']);
git(['push', 'origin', 'main']);
// PR branches off main, adds a file
git(['checkout', '-b', 'pr-branch']);
writeFileSync(join(repoDir, 'pr-only.ts'), 'pr\n');
git(['add', '.']);
git(['commit', '-m', 'PR change']);
// Master drifts forward, modifying shared.ts (the pre-fix bug surface)
git(['checkout', 'main']);
writeFileSync(join(repoDir, 'shared.ts'), 'shared\ndrift-from-master\n');
git(['commit', '-am', 'master moves']);
git(['push', 'origin', 'main']);
// Sit on the PR branch as if running CI
git(['checkout', 'pr-branch']);
process.chdir(repoDir);
});
after(() => {
process.chdir(originalCwd);
rmSync(repoDir, { recursive: true, force: true });
rmSync(remoteDir, { recursive: true, force: true });
});
it('returns only PR-introduced files (master drift does not pollute)', () => {
const changed = getChangedFiles('main');
assert.deepEqual(changed, ['pr-only.ts']);
});
it('getMergeBase returns the common ancestor commit', () => {
const mergeBase = getMergeBase();
assert.match(mergeBase, /^[a-f0-9]{40}$/);
const expected = git(['merge-base', 'FETCH_HEAD', 'HEAD']);
assert.equal(mergeBase, expected);
});
it('rejects unsafe base refs', () => {
assert.throws(() => getChangedFiles('main; rm -rf /'), /Unsafe/);
assert.throws(() => getChangedFiles('main$evil'), /Unsafe/);
});
});
// --- runValidate --- // --- runValidate ---
describe('runValidate', () => { describe('runValidate', () => {

View File

@ -30,6 +30,9 @@ outputs:
base-ref: base-ref:
description: 'Resolved base ref used for the diff (filter mode only)' description: 'Resolved base ref used for the diff (filter mode only)'
value: ${{ steps.run.outputs.base-ref }} value: ${{ steps.run.outputs.base-ref }}
merge-base:
description: 'Merge-base SHA between FETCH_HEAD and HEAD (filter mode only)'
value: ${{ steps.run.outputs.merge-base }}
runs: runs:
using: 'composite' using: 'composite'

View File

@ -98,14 +98,30 @@ export function getChangedFiles(baseRef) {
if (!SAFE_REF.test(baseRef)) { if (!SAFE_REF.test(baseRef)) {
throw new Error(`Unsafe base ref: "${baseRef}"`); throw new Error(`Unsafe base ref: "${baseRef}"`);
} }
execSync(`git fetch --depth=1 origin ${baseRef}`, { stdio: 'pipe' }); // Deepen the fetch so the merge base is reachable from this shallow clone.
const output = execSync('git diff --name-only FETCH_HEAD HEAD', { encoding: 'utf-8' }); // A 2-dot diff (FETCH_HEAD HEAD) reports anything that differs in either
// direction, so files added to base-branch after the PR diverged show up as
// "changed" — spuriously triggering path-filtered jobs. The merge base
// scopes the diff to PR-only changes.
execSync(`git fetch --no-tags --prune --deepen=200 origin ${baseRef}`, { stdio: 'pipe' });
const output = execSync('git diff --name-only --merge-base FETCH_HEAD HEAD', {
encoding: 'utf-8',
});
return output return output
.split('\n') .split('\n')
.map((f) => f.trim()) .map((f) => f.trim())
.filter(Boolean); .filter(Boolean);
} }
/**
* Resolve the merge-base SHA between FETCH_HEAD and HEAD.
* Used to give downstream tools (e.g. janitor's AST diff) a stable, PR-only
* comparison point that doesn't drift when the base branch moves forward.
*/
export function getMergeBase() {
return execSync('git merge-base FETCH_HEAD HEAD', { encoding: 'utf-8' }).trim();
}
// --- Filter evaluation --- // --- Filter evaluation ---
/** /**
@ -155,7 +171,9 @@ export function runFilter() {
const filters = parseFilters(filtersInput); const filters = parseFilters(filtersInput);
const changedFiles = getChangedFiles(baseRef); const changedFiles = getChangedFiles(baseRef);
const mergeBase = getMergeBase();
console.log(`Merge base: ${mergeBase}`);
console.log(`Changed files (${changedFiles.length}):`); console.log(`Changed files (${changedFiles.length}):`);
for (const f of changedFiles) { for (const f of changedFiles) {
console.log(` ${f}`); console.log(` ${f}`);
@ -172,6 +190,7 @@ export function runFilter() {
setOutput('results', JSON.stringify(results)); setOutput('results', JSON.stringify(results));
setOutput('changed-files', changedFiles.join('\n')); setOutput('changed-files', changedFiles.join('\n'));
setOutput('base-ref', baseRef); setOutput('base-ref', baseRef);
setOutput('merge-base', mergeBase);
} }
// --- Mode: validate --- // --- Mode: validate ---

View File

@ -30,6 +30,7 @@ jobs:
e2e_performance: ${{ fromJSON(steps.ci-filter.outputs.results)['e2e-performance'] == true }} e2e_performance: ${{ fromJSON(steps.ci-filter.outputs.results)['e2e-performance'] == true }}
instance_ai_workflow_eval: ${{ fromJSON(steps.ci-filter.outputs.results)['instance-ai-workflow-eval'] == true }} instance_ai_workflow_eval: ${{ fromJSON(steps.ci-filter.outputs.results)['instance-ai-workflow-eval'] == true }}
commit_sha: ${{ steps.commit-sha.outputs.sha }} commit_sha: ${{ steps.commit-sha.outputs.sha }}
merge_base: ${{ steps.ci-filter.outputs.merge-base }}
matrix: ${{ steps.generate-matrix.outputs.matrix }} matrix: ${{ steps.generate-matrix.outputs.matrix }}
skip_tests: ${{ steps.generate-matrix.outputs.skip-tests }} skip_tests: ${{ steps.generate-matrix.outputs.skip-tests }}
steps: steps:
@ -73,7 +74,6 @@ jobs:
packages/testing/playwright/playwright-projects.ts packages/testing/playwright/playwright-projects.ts
packages/testing/playwright/package.json packages/testing/playwright/package.json
.github/workflows/test-dev-server-smoke-reusable.yml .github/workflows/test-dev-server-smoke-reusable.yml
.github/workflows/ci-pull-requests.yml
workflows: .github/** workflows: .github/**
workflow-scripts: .github/scripts/** workflow-scripts: .github/scripts/**
performance: performance:
@ -120,9 +120,10 @@ jobs:
if: fromJSON(steps.ci-filter.outputs.results).ci || fromJSON(steps.ci-filter.outputs.results).e2e if: fromJSON(steps.ci-filter.outputs.results).ci || fromJSON(steps.ci-filter.outputs.results).e2e
env: env:
CHANGED_FILES: ${{ steps.ci-filter.outputs.changed-files }} CHANGED_FILES: ${{ steps.ci-filter.outputs.changed-files }}
MERGE_BASE: ${{ steps.ci-filter.outputs.merge-base }}
run: | run: |
FILES_CSV=$(echo "$CHANGED_FILES" | tr '\n' ',' | sed 's/,$//') FILES_CSV=$(echo "$CHANGED_FILES" | tr '\n' ',' | sed 's/,$//')
MATRIX=$(node packages/testing/playwright/scripts/distribute-tests.mjs --matrix 16 --orchestrate --impact "--files=$FILES_CSV" --base=FETCH_HEAD) MATRIX=$(node packages/testing/playwright/scripts/distribute-tests.mjs --matrix 16 --orchestrate --impact "--files=$FILES_CSV" "--base=$MERGE_BASE")
echo "matrix=$MATRIX" >> "$GITHUB_OUTPUT" echo "matrix=$MATRIX" >> "$GITHUB_OUTPUT"
echo "skip-tests=$(node -e "process.stdout.write(JSON.parse(process.argv[1])[0]?.skip === true ? 'true' : 'false')" "$MATRIX")" >> "$GITHUB_OUTPUT" echo "skip-tests=$(node -e "process.stdout.write(JSON.parse(process.argv[1])[0]?.skip === true ? 'true' : 'false')" "$MATRIX")" >> "$GITHUB_OUTPUT"