Skip to content

Commit ef5856b

Browse files
brandonpaytonclaude
andcommitted
[CI] Label PRs from git diff instead of the pulls.listFiles API
The `paths` job used actions/labeler, which lists a PR's changed files via GitHub's pulls.listFiles API. That makes GitHub generate the full PR diff, which times out ("Sorry, this diff is taking too long to generate") on PRs that change a lot of binary data — e.g. recompiled PHP.wasm builds. As a required check, the failed job then blocks the merge, and retrying doesn't help. (#3812 already handled this for the package-and-type job, but actions/labeler's internal call can't be wrapped.) List the changed files with `git diff --name-only base...head` instead — it compares tree object hashes and never serializes blob content, so it is instant regardless of PR size — and apply the same path labels. The label-matching logic lives in a small, unit-tested module rather than inline in the workflow, so it can be exercised locally: packages/meta/src/pr-labels/match-path-labels.mjs # pure logic packages/meta/src/pr-labels/match-path-labels.test.mjs # node:test packages/meta/bin/label-pr-paths.mjs # thin I/O runner Run the tests with: node --test "packages/meta/src/pr-labels/**/*.test.mjs" Scope kept deliberately small: - Only the failing `paths` job changes; `package-and-type` is untouched. - The module implements only `any-glob-to-any-file` — the one labeler.yml feature this repo uses — with matching identical to actions/labeler v5 (minimatch, { dot: true }). Any other labeler rule throws (fail-loud) rather than silently mislabeling, so a future config change is caught. `git diff --name-only` runs no PR code (it only reads paths), so reading the fork head stays safe under pull_request_target. Actions stay pinned to commit SHAs; minimatch/js-yaml are version-pinned and installed in isolation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3144994 commit ef5856b

4 files changed

Lines changed: 323 additions & 9 deletions

File tree

.github/workflows/auto-label-prs.yml

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@ name: Auto-label PRs
22

33
# PR labeling has two parts:
44
#
5-
# 1. `paths` job — runs actions/labeler against .github/labeler.yml to
6-
# apply [Aspect], [Focus], [Feature], and [Type] Documentation
7-
# labels. These have no count limit and a wide refactor can match
8-
# many of them legitimately.
5+
# 1. `paths` job — applies [Aspect], [Focus], [Feature], and
6+
# [Type] Documentation labels from the path globs in
7+
# .github/labeler.yml. It lists changed files with `git diff` (not
8+
# GitHub's pulls.listFiles API, which times out on large binary PRs)
9+
# and matches them with packages/meta/src/pr-labels/match-path-labels.mjs.
10+
# These labels have no count limit; a wide refactor can match many
11+
# of them legitimately.
912
#
1013
# 2. `package-and-type` job — applies the `[Package][...]` labels and
1114
# a single inferred `[Type]` label. This one needs custom logic:
@@ -43,13 +46,30 @@ jobs:
4346
contents: read # Required for actions/labeler to read the configuration file.
4447
pull-requests: write # Required to apply labels to pull requests.
4548
steps:
46-
# Pinned to a commit SHA, not a tag: this job runs with
49+
# We do NOT use actions/labeler here: it lists changed files via
50+
# GitHub's pulls.listFiles API, which makes GitHub generate the full
51+
# PR diff and times out on PRs with large binary churn (recompiled
52+
# PHP.wasm builds), failing this required check and blocking merges.
53+
# Instead we list files with `git diff` (size-independent) and apply
54+
# the same path labels. The matching logic — and its unit tests —
55+
# live in packages/meta/src/pr-labels/match-path-labels.{mjs,test.mjs}.
56+
#
57+
# Actions are pinned to commit SHAs, not tags: this job runs with
4758
# pull-requests:write, so a moved tag would be a supply-chain
48-
# foothold. Bump deliberately when upgrading.
49-
- uses: actions/labeler@8558fd74291d67161a8a78ce36a881fa63b766a9 # v5.0.0
59+
# foothold. Bump deliberately when upgrading. `git diff` runs no PR
60+
# code — it only reads paths — so reading the fork head is safe.
61+
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
5062
with:
51-
configuration-path: .github/labeler.yml
52-
sync-labels: false
63+
fetch-depth: 0 # Full base history so the merge-base is present.
64+
# minimatch + js-yaml, pinned and installed in isolation (not the
65+
# repo's deps). match-path-labels.mjs finds them via NODE_PATH.
66+
- name: Install label matcher libraries
67+
run: npm install --no-save --no-audit --no-fund --prefix "$RUNNER_TEMP/labeltools" minimatch@9.0.5 js-yaml@4.1.0
68+
- name: Apply path-based labels
69+
run: node packages/meta/bin/label-pr-paths.mjs
70+
env:
71+
NODE_PATH: ${{ runner.temp }}/labeltools/node_modules
72+
GITHUB_TOKEN: ${{ github.token }}
5373

5474
package-and-type:
5575
if: github.repository == 'WordPress/wordpress-playground'
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
#!/usr/bin/env node
2+
//
3+
// Thin I/O wrapper around match-path-labels.mjs for the Auto-label workflow's
4+
// `paths` job. The label-matching logic lives in (and is unit-tested by)
5+
// ../src/pr-labels/match-path-labels.{mjs,test.mjs}; this file only does the
6+
// side effects the workflow needs and is intentionally kept trivial.
7+
//
8+
// It exists because actions/labeler lists changed files via GitHub's
9+
// pulls.listFiles API, which times out on PRs with a lot of binary churn
10+
// (recompiled PHP.wasm builds). See match-path-labels.mjs for the full why.
11+
//
12+
// Expects (all provided by the workflow):
13+
// GITHUB_EVENT_PATH pull_request_target event payload
14+
// GITHUB_REPOSITORY "owner/repo"
15+
// GITHUB_TOKEN token with pull-requests:write
16+
// NODE_PATH dir holding the isolated minimatch + js-yaml install
17+
import { readFileSync } from 'node:fs';
18+
import { execFileSync } from 'node:child_process';
19+
import { createRequire } from 'node:module';
20+
import { matchPathLabels } from '../src/pr-labels/match-path-labels.mjs';
21+
22+
const yaml = createRequire(import.meta.url)('js-yaml');
23+
24+
const event = JSON.parse(readFileSync(process.env.GITHUB_EVENT_PATH, 'utf8'));
25+
const pr = event.pull_request;
26+
if (!pr) {
27+
throw new Error('No pull_request in the event payload.');
28+
}
29+
30+
// List changed files with git — compares tree hashes, so it is instant
31+
// regardless of PR size, unlike pulls.listFiles which forces GitHub to
32+
// generate the (possibly enormous) diff.
33+
const git = (...args) => execFileSync('git', args).toString();
34+
execFileSync(
35+
'git',
36+
['fetch', '--no-tags', 'origin', `refs/pull/${pr.number}/head`],
37+
{
38+
stdio: 'inherit',
39+
}
40+
);
41+
const head = git('rev-parse', 'FETCH_HEAD').trim();
42+
const changedFiles = git('diff', '--name-only', `${pr.base.sha}...${head}`)
43+
.split('\n')
44+
.filter(Boolean);
45+
46+
const config = yaml.load(readFileSync('.github/labeler.yml', 'utf8'));
47+
const labels = matchPathLabels(changedFiles, config);
48+
console.log(
49+
`Changed files: ${changedFiles.length}. Labels: ${JSON.stringify(labels)}`
50+
);
51+
52+
if (labels.length === 0) {
53+
process.exit(0);
54+
}
55+
56+
const [owner, repo] = process.env.GITHUB_REPOSITORY.split('/');
57+
const response = await fetch(
58+
`https://api.github.com/repos/${owner}/${repo}/issues/${pr.number}/labels`,
59+
{
60+
method: 'POST',
61+
headers: {
62+
Authorization: `Bearer ${process.env.GITHUB_TOKEN}`,
63+
Accept: 'application/vnd.github+json',
64+
'X-GitHub-Api-Version': '2022-11-28',
65+
},
66+
body: JSON.stringify({ labels }),
67+
}
68+
);
69+
if (!response.ok) {
70+
throw new Error(
71+
`Failed to add labels (${response.status}): ${await response.text()}`
72+
);
73+
}
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
import { createRequire } from 'node:module';
2+
3+
// minimatch is resolved at runtime so this module has no build step: locally
4+
// (and in tests) it comes from node_modules; in the Auto-label workflow it is
5+
// installed in isolation and found via NODE_PATH. createRequire honors
6+
// NODE_PATH, unlike a bare ESM `import`.
7+
const { minimatch } = createRequire(import.meta.url)('minimatch');
8+
9+
/**
10+
* Compute the path-based labels for a pull request.
11+
*
12+
* WHY this exists (instead of just using actions/labeler):
13+
* actions/labeler reads a PR's changed files from GitHub's `pulls.listFiles`
14+
* API, which makes GitHub generate the full PR diff. That request times out
15+
* ("Sorry, this diff is taking too long to generate") on PRs that change a
16+
* lot of binary data — e.g. recompiled PHP.wasm builds — and the action then
17+
* fails the `paths` job, which is a required check, and blocks the merge.
18+
* The caller lists changed files with `git diff` (which compares tree hashes
19+
* and is instant regardless of PR size) and passes them here, so labeling
20+
* no longer depends on GitHub's diff generation.
21+
*
22+
* WHAT is supported:
23+
* Only the one labeler feature .github/labeler.yml actually uses —
24+
*
25+
* <label>:
26+
* - changed-files:
27+
* - any-glob-to-any-file: [ ...globs ]
28+
*
29+
* i.e. "apply <label> if any glob matches any changed file". Glob matching
30+
* uses minimatch with `{ dot: true }`, identical to actions/labeler v5, so
31+
* results match for this config.
32+
*
33+
* Every other labeler feature (any-glob-to-all-files, all-globs-to-any-file,
34+
* all-globs-to-all-files, base-branch, head-branch, or more than one entry
35+
* under a label) is intentionally NOT implemented. Rather than silently
36+
* ignore such a rule and mislabel PRs, this throws — so a future labeler.yml
37+
* change fails loudly and whoever makes it extends this module (and its
38+
* tests) instead.
39+
*
40+
* @param {string[]} changedFiles - Paths changed in the PR.
41+
* @param {Record<string, unknown>} config - Parsed .github/labeler.yml.
42+
* @returns {string[]} Labels whose globs match at least one changed file.
43+
*/
44+
export function matchPathLabels(changedFiles, config) {
45+
const labels = [];
46+
for (const [label, rules] of Object.entries(config ?? {})) {
47+
const globs = globsForLabel(label, rules);
48+
const matched = globs.some((glob) =>
49+
changedFiles.some((file) => minimatch(file, glob, { dot: true }))
50+
);
51+
if (matched) {
52+
labels.push(label);
53+
}
54+
}
55+
return labels;
56+
}
57+
58+
/**
59+
* Extract the `any-glob-to-any-file` globs for one label, rejecting any config
60+
* shape this module does not implement (see WHAT is supported, above).
61+
*/
62+
function globsForLabel(label, rules) {
63+
if (!Array.isArray(rules) || rules.length !== 1) {
64+
throw unsupported(label, 'expected exactly one `changed-files` entry');
65+
}
66+
const [rule] = rules;
67+
const ruleKeys = Object.keys(rule ?? {});
68+
if (ruleKeys.length !== 1 || ruleKeys[0] !== 'changed-files') {
69+
throw unsupported(
70+
label,
71+
`only \`changed-files\` is supported, saw ${JSON.stringify(ruleKeys)}`
72+
);
73+
}
74+
const clauses = rule['changed-files'];
75+
if (!Array.isArray(clauses)) {
76+
throw unsupported(label, '`changed-files` must be a list');
77+
}
78+
79+
const globs = [];
80+
for (const clause of clauses) {
81+
const clauseKeys = Object.keys(clause ?? {});
82+
if (
83+
clauseKeys.length !== 1 ||
84+
clauseKeys[0] !== 'any-glob-to-any-file'
85+
) {
86+
throw unsupported(
87+
label,
88+
`only \`any-glob-to-any-file\` is supported, saw ${JSON.stringify(
89+
clauseKeys
90+
)}`
91+
);
92+
}
93+
const value = clause['any-glob-to-any-file'];
94+
if (Array.isArray(value)) {
95+
globs.push(...value);
96+
} else if (typeof value === 'string') {
97+
globs.push(value);
98+
} else {
99+
throw unsupported(
100+
label,
101+
'`any-glob-to-any-file` must be a string or list of globs'
102+
);
103+
}
104+
}
105+
return globs;
106+
}
107+
108+
function unsupported(label, detail) {
109+
return new Error(
110+
`Unsupported .github/labeler.yml rule for "${label}": ${detail}. ` +
111+
'packages/meta/src/pr-labels/match-path-labels.mjs implements only ' +
112+
'`changed-files: [{ any-glob-to-any-file: [...] }]`. Extend it and its ' +
113+
'tests to support the new rule.'
114+
);
115+
}
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
import { test } from 'node:test';
2+
import assert from 'node:assert/strict';
3+
import { matchPathLabels } from './match-path-labels.mjs';
4+
5+
// Run locally with:
6+
// node --test "packages/meta/src/pr-labels/**/*.test.mjs"
7+
8+
const anyGlob = (globs) => [
9+
{ 'changed-files': [{ 'any-glob-to-any-file': globs }] },
10+
];
11+
12+
const config = {
13+
'[Aspect] Browser': anyGlob(['packages/php-wasm/web/**']),
14+
'[Type] Documentation': anyGlob(['packages/docs/**', '**/*.md']),
15+
'[Aspect] Service Worker': anyGlob(['**/service-worker*.{ts,js}']),
16+
'[Aspect] Sqlite': anyGlob(['**/sqlite*/**']),
17+
};
18+
19+
test('matches a directory prefix glob', () => {
20+
assert.deepEqual(matchPathLabels(['packages/php-wasm/web/x.ts'], config), [
21+
'[Aspect] Browser',
22+
]);
23+
});
24+
25+
test('matches a suffix glob (**/*.md)', () => {
26+
assert.deepEqual(matchPathLabels(['README.md'], config), [
27+
'[Type] Documentation',
28+
]);
29+
});
30+
31+
test('matches a brace-expansion glob ({ts,js})', () => {
32+
assert.deepEqual(matchPathLabels(['a/b/service-worker.js'], config), [
33+
'[Aspect] Service Worker',
34+
]);
35+
});
36+
37+
test('matches a mid-path wildcard glob (**/sqlite*/**)', () => {
38+
assert.deepEqual(
39+
matchPathLabels(['packages/x/sqlite-integration/y.php'], config),
40+
['[Aspect] Sqlite']
41+
);
42+
});
43+
44+
test('applies every matching label and ignores non-matching files', () => {
45+
const labels = matchPathLabels(
46+
['packages/php-wasm/web/x.ts', 'docs/guide.md', 'unrelated/file.txt'],
47+
config
48+
);
49+
assert.deepEqual(labels.sort(), [
50+
'[Aspect] Browser',
51+
'[Type] Documentation',
52+
]);
53+
});
54+
55+
test('returns [] when nothing matches', () => {
56+
assert.deepEqual(matchPathLabels(['unrelated/file.txt'], config), []);
57+
});
58+
59+
test('includes dotfiles (dot: true, matching actions/labeler default)', () => {
60+
assert.deepEqual(
61+
matchPathLabels(['packages/docs/.eslintrc'], {
62+
'[Type] Documentation': anyGlob(['packages/docs/**']),
63+
}),
64+
['[Type] Documentation']
65+
);
66+
});
67+
68+
test('tolerates a null/empty config', () => {
69+
assert.deepEqual(matchPathLabels(['a'], null), []);
70+
assert.deepEqual(matchPathLabels(['a'], {}), []);
71+
});
72+
73+
// Fail-loud guard: any config shape beyond `any-glob-to-any-file` must throw
74+
// rather than silently mislabel.
75+
test('throws on an unsupported match type', () => {
76+
assert.throws(
77+
() =>
78+
matchPathLabels(['a/x'], {
79+
X: [
80+
{
81+
'changed-files': [
82+
{ 'all-globs-to-any-file': ['a/**'] },
83+
],
84+
},
85+
],
86+
}),
87+
/only `any-glob-to-any-file` is supported/
88+
);
89+
});
90+
91+
test('throws on a non-changed-files rule (e.g. base-branch)', () => {
92+
assert.throws(
93+
() => matchPathLabels([], { X: [{ 'base-branch': ['main'] }] }),
94+
/only `changed-files` is supported/
95+
);
96+
});
97+
98+
test('throws when a label has more than one entry (AND semantics unsupported)', () => {
99+
assert.throws(
100+
() =>
101+
matchPathLabels([], {
102+
X: [...anyGlob(['a/**']), ...anyGlob(['b/**'])],
103+
}),
104+
/exactly one `changed-files` entry/
105+
);
106+
});

0 commit comments

Comments
 (0)