Skip to content

Commit 88f1783

Browse files
BKPepeclaude
andcommitted
cloudflare-worker: take back the labels that stopped being true
The bot applied "add package", "drop package" and release/<version> whenever the condition held, and never took any of them off again. A branch that dropped its new package during review, or was retargeted from one release to another, kept a label saying otherwise - and the nightly scan and everyone reading the list of pull requests believed it. Only "stale" and the guidelines label were ever withdrawn. All three are read straight off the pull request's own content, so the bot now withdraws them on the same terms it applies them. Labels matched from labeler.yml are deliberately left alone, the way GitHub's own labeler action leaves them by default: a path label is as often put on by hand as derived, and taking those back would fight whoever set them. A removal names the label the way the repository spells it rather than the way the constant is written - openwrt/packages calls it "Add package" - which the comparison, being case-insensitive, hid until now. Deploying is also held back until the tests pass. deploy.yml and tests.yml both triggered on a push to main, so they started in the same second and raced: the Worker went live whether or not the suite ever went green. Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 3162f4d commit 88f1783

4 files changed

Lines changed: 151 additions & 8 deletions

File tree

.github/workflows/deploy.yml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,31 @@ permissions:
99
contents: read
1010

1111
jobs:
12+
# The tests also run from tests.yml on the same push, but a separate
13+
# workflow cannot hold this one back: both start in the same second and the
14+
# Worker would go live whether or not the suite ever went green. Running
15+
# them here as well is what makes the deployment wait for an answer.
16+
test:
17+
name: Run Tests
18+
runs-on: ubuntu-slim
19+
steps:
20+
- name: Checkout code
21+
uses: actions/checkout@v7
22+
23+
- name: Setup Node.js
24+
uses: actions/setup-node@v6
25+
with:
26+
node-version: 24
27+
28+
- name: Run Unit Tests
29+
run: npm test
30+
working-directory: cloudflare-worker
31+
env:
32+
FORCE_COLOR: 1
33+
1234
deploy:
1335
name: Deploy Worker
36+
needs: test
1437
runs-on: ubuntu-slim
1538
environment:
1639
name: production

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ Scans the contribution tree for nested downstream patch targets:
6969
### Automated Triage & Stale PR Management
7070

7171
* **`not following guidelines`**: A high-visibility tag automatically attached to the PR if any critical validation check drops a failure blueprint. Clears itself upon a successful push.
72-
* **`add package` / `drop package`**: Dynamically analyzes unified diff targets to label tracking trees introducing or purging software packages.
73-
* **Stable Branch Tracking**: Auto-generates matching grey release tags (e.g., `release/24.10`, `release/25.12`) whenever a PR targets an active release backport branch.
72+
* **`add package` / `drop package`**: Dynamically analyzes unified diff targets to label tracking trees introducing or purging software packages. Both are withdrawn again once the branch stops adding or dropping a package, so a label never outlives what it describes.
73+
* **Stable Branch Tracking**: Auto-generates matching grey release tags (e.g., `release/24.10`, `release/25.12`) whenever a PR targets an active release backport branch, and removes a release tag that no longer matches after the pull request is retargeted. Labels matched from `labeler.yml` are only ever added, the way GitHub's own labeler action leaves them by default — a path label is as often set by hand as derived.
7474
* **Issue Labeller**: Replaces the GitHub Actions `issue-labeller.yml` workflow. When a bug-report issue is opened with the trigger label, the bot validates form fields and applies labels based on a declarative `.github/issue-labeller.yml` configuration file (same spirit as `labeler.yml` for PRs — label name → list of conditions). Supports template variables (`{major}`, `{segment0}`, etc.), format validation (regex), existence checks (tag/path via GraphQL), substring matching, and presence checks. Falls back to sensible defaults if no config file exists. Disabled by default — enable per-repository with `"enable_issue_labeller": true`.
7575
* **Stale PR Cleanup**: A daily scheduled cron task (05:30 UTC) scans all repositories where the App is installed. If explicitly enabled in a repository's configuration (\`"enable_stale_bot": true\`), it marks PRs containing the \`not following guidelines\` label as \`stale\` (with a warning comment) after 14 days of inactivity, and closes them after another 14 days of silence. Only contributor activity resets the countdown: pushed commits, force-pushes, reopens, and comments or reviews from people. Comments from GitHub Apps, `*[bot]` accounts and the machine accounts listed in `stale_ignored_users` are ignored, so an automated review can never keep a dead PR alive forever. Pushing new commits also removes the `stale` label immediately via the webhook, without waiting for the nightly scan. The scan asks GitHub for each repository's configuration and its labelled pull requests in two GraphQL queries, timelines included, so its request count no longer grows with the number of stale pull requests. A pull request whose timeline is longer than the fetched window (the most recent 100 entries) is left untouched rather than judged on incomplete history.
7676

cloudflare-worker/src/index.js

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1472,11 +1472,16 @@ async function handleWebhook(request, env) {
14721472
}
14731473

14741474
const currentPrLabels = new Set((data.pull_request?.labels || []).map(l => l.name.toLowerCase()));
1475+
// Comparisons are case-insensitive, but a removal has to name the label the
1476+
// way the repository spells it: openwrt/packages calls it "Add package",
1477+
// and the constant here is lower case.
1478+
const currentPrLabelNames = new Map((data.pull_request?.labels || []).map(l => [l.name.toLowerCase(), l.name]));
1479+
const spelledAsOnPr = (name) => currentPrLabelNames.get(name.toLowerCase()) || name;
14751480

14761481
// New commits or a reopen are contributor activity: drop the stale marker
14771482
// right away instead of waiting for the nightly scan to notice it.
14781483
if ((data.action === 'synchronize' || data.action === 'reopened') && currentPrLabels.has('stale')) {
1479-
labelOperations.push(() => removeLabel('stale'));
1484+
labelOperations.push(() => removeLabel(spelledAsOnPr('stale')));
14801485
}
14811486

14821487
if (!allPassed) {
@@ -1487,27 +1492,49 @@ async function handleWebhook(request, env) {
14871492
} else {
14881493
// Delete validation failure label if present
14891494
if (currentPrLabels.has(LABEL_GUIDELINES.toLowerCase())) {
1490-
labelOperations.push(() => removeLabel(LABEL_GUIDELINES));
1495+
labelOperations.push(() => removeLabel(spelledAsOnPr(LABEL_GUIDELINES)));
14911496
}
14921497
}
14931498

1499+
// The three labels below are read straight off the pull request's own
1500+
// content, so the bot both applies and withdraws them: a branch that
1501+
// dropped its new package, or was retargeted to another release, would
1502+
// otherwise keep a label that stopped being true. Labels matched from
1503+
// labeler.yml are left alone, the way GitHub's own labeler action leaves
1504+
// them by default - a path label is as often put on by hand as derived,
1505+
// and taking those back would fight whoever set them.
14941506
if (CONFIG.add_package_label && state.isNewPackage && !currentPrLabels.has(LABEL_ADD_PACKAGE.toLowerCase())) {
14951507
labelOperations.push(() => ensureLabel(LABEL_ADD_PACKAGE, '0e7490', 'Introduces a new package Makefile build script'));
14961508
labelsToAdd.push(LABEL_ADD_PACKAGE);
14971509
}
14981510

1511+
if (CONFIG.add_package_label && !state.isNewPackage && currentPrLabels.has(LABEL_ADD_PACKAGE.toLowerCase())) {
1512+
labelOperations.push(() => removeLabel(spelledAsOnPr(LABEL_ADD_PACKAGE)));
1513+
}
1514+
14991515
if (CONFIG.drop_package_label && state.isDroppedPackage && !currentPrLabels.has(LABEL_DROP_PACKAGE.toLowerCase())) {
15001516
labelOperations.push(() => ensureLabel(LABEL_DROP_PACKAGE, '3b82f6', 'Removes an existing package Makefile from the tracking tree'));
15011517
labelsToAdd.push(LABEL_DROP_PACKAGE);
15021518
}
15031519

1504-
if (CONFIG.branch_labeling && /^openwrt-\d{2}\.\d{2}$/.test(baseBranch)) {
1505-
const version = baseBranch.split('-')[1];
1506-
const labelName = `release/${version}`;
1507-
if (!currentPrLabels.has(labelName.toLowerCase())) {
1520+
if (CONFIG.drop_package_label && !state.isDroppedPackage && currentPrLabels.has(LABEL_DROP_PACKAGE.toLowerCase())) {
1521+
labelOperations.push(() => removeLabel(spelledAsOnPr(LABEL_DROP_PACKAGE)));
1522+
}
1523+
1524+
if (CONFIG.branch_labeling) {
1525+
const releaseMatch = baseBranch.match(/^openwrt-(\d{2}\.\d{2})$/);
1526+
const labelName = releaseMatch ? `release/${releaseMatch[1]}` : null;
1527+
if (labelName && !currentPrLabels.has(labelName.toLowerCase())) {
15081528
labelOperations.push(() => ensureLabel(labelName, '6b7280', `Pull request targets the stable release branch ${labelName}`));
15091529
labelsToAdd.push(labelName);
15101530
}
1531+
// Retargeting a pull request to another branch leaves the old release
1532+
// label behind, saying it goes somewhere it no longer goes.
1533+
for (const existing of currentPrLabels) {
1534+
if (/^release\/\d{2}\.\d{2}$/.test(existing) && existing !== labelName?.toLowerCase()) {
1535+
labelOperations.push(() => removeLabel(spelledAsOnPr(existing)));
1536+
}
1537+
}
15111538
}
15121539

15131540
if (CONFIG.enable_labeler_yml && labelerText) {

cloudflare-worker/test/index.test.js

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5896,6 +5896,99 @@ diff --git a/utils/mypkg/patches/00${i}-fix.patch b/utils/mypkg/patches/00${i}-f
58965896
}
58975897
});
58985898

5899+
// Runs one webhook on a pull request that already carries the given labels,
5900+
// and reports which labels the run took off.
5901+
async function labelsRemovedFor({ labels, baseRef = 'main', patch }) {
5902+
const payload = JSON.stringify({
5903+
action: 'synchronize',
5904+
pull_request: {
5905+
number: 123, title: 'mypkg: update to 1.2.3', body: 'Update',
5906+
labels: labels.map(name => ({ name })),
5907+
base: { ref: baseRef, sha: 'basesha' }, head: { ref: 'feature-branch', sha: 'headsha' },
5908+
user: { login: 'johndoe', type: 'User' },
5909+
commits_url: 'https://api.github.com/repos/test/repo/pulls/123/commits',
5910+
url: 'https://api.github.com/repos/test/repo/pulls/123'
5911+
},
5912+
installation: { id: 456 }, repository: { full_name: 'test/repo' }
5913+
});
5914+
const secret = 'mysecret';
5915+
const signature = await calculateHmac(secret, payload);
5916+
const removed = [];
5917+
5918+
fetchMock = async (url, options) => {
5919+
const method = options?.method || 'GET';
5920+
if (url.includes('/access_tokens')) return new Response(JSON.stringify({ token: 'mocktoken' }), { status: 200 });
5921+
if (url.includes('/formalities.json')) {
5922+
return new Response(JSON.stringify({ check_branch: false, enable_comments: false, require_linked_github_account: false, require_body: false, check_uci_config: false, check_pkg_release: false }), { status: 200 });
5923+
}
5924+
{ const lr = graphqlLabelsHandler(url, options, labels); if (lr) return lr; }
5925+
if (url.includes('/pulls/123/commits')) {
5926+
return new Response(JSON.stringify([{
5927+
sha: 'sha123', html_url: 'https://github.com/test/repo/commit/sha123',
5928+
commit: { message: 'mypkg: update to 1.2.3\n\nA description of the change.\n\nSigned-off-by: John Doe <john@doe.com>', author: { name: 'John Doe', email: 'john@doe.com' }, committer: { name: 'John Doe', email: 'john@doe.com' } }
5929+
}]), { status: 200 });
5930+
}
5931+
if (url.match(/\/repos\/test\/repo\/commits\/sha123/)) return new Response(patch, { status: 200 });
5932+
if (url.includes('/issues/123/labels/') && method === 'DELETE') {
5933+
removed.push(decodeURIComponent(url.split('/labels/')[1]));
5934+
return new Response('[]', { status: 200 });
5935+
}
5936+
if (url.includes('/issues/123/comments')) return new Response(JSON.stringify([]), { status: 200 });
5937+
return new Response(JSON.stringify({}), { status: 201 });
5938+
};
5939+
5940+
const originalImportKey = crypto.subtle.importKey;
5941+
crypto.subtle.importKey = async (format, keyData, algorithm, extractable, keyUsages) =>
5942+
algorithm.name === 'RSASSA-PKCS1-v1_5' ? { type: 'private', extractable: false, algorithm, usages: keyUsages }
5943+
: originalImportKey.call(crypto.subtle, format, keyData, algorithm, extractable, keyUsages);
5944+
const originalSign = crypto.subtle.sign;
5945+
crypto.subtle.sign = async (algorithm, key, data) =>
5946+
algorithm === 'RSASSA-PKCS1-v1_5' ? new ArrayBuffer(256) : originalSign.call(crypto.subtle, algorithm, key, data);
5947+
5948+
try {
5949+
const response = await worker.fetch(new Request('http://localhost/webhook', {
5950+
method: 'POST', body: payload,
5951+
headers: { 'x-hub-signature-256': signature, 'x-github-event': 'pull_request' }
5952+
}), { WEBHOOK_SECRET: secret, APP_ID: '12345', PRIVATE_KEY: 'YW55Y29udGVudA==' }, {});
5953+
assert.strictEqual(response.status, 200, await response.text());
5954+
return removed;
5955+
} finally {
5956+
crypto.subtle.importKey = originalImportKey;
5957+
crypto.subtle.sign = originalSign;
5958+
fetchMock = null;
5959+
}
5960+
}
5961+
5962+
const plainPatch = 'diff --git a/README b/README\n--- a/README\n+++ b/README\n@@ -1 +1 @@\n-a\n+b\n';
5963+
5964+
test('takes back the "add package" label when the branch no longer adds one', async () => {
5965+
// Spelled the way openwrt/packages spells it, which is not how the
5966+
// constant in the bot is written.
5967+
const removed = await labelsRemovedFor({ labels: ['Add package'], patch: plainPatch });
5968+
assert.deepStrictEqual(removed, ['Add package']);
5969+
});
5970+
5971+
test('keeps the "add package" label while the branch still adds one', async () => {
5972+
const addsPackage = 'diff --git a/utils/mypkg/Makefile b/utils/mypkg/Makefile\nnew file mode 100644\n--- /dev/null\n+++ b/utils/mypkg/Makefile\n@@ -0,0 +1,2 @@\n+include $(TOPDIR)/rules.mk\n+PKG_NAME:=mypkg\n';
5973+
const removed = await labelsRemovedFor({ labels: ['Add package'], patch: addsPackage });
5974+
assert.deepStrictEqual(removed, []);
5975+
});
5976+
5977+
test('takes back a release label after the pull request is retargeted', async () => {
5978+
const removed = await labelsRemovedFor({ labels: ['release/24.10'], baseRef: 'openwrt-25.12', patch: plainPatch });
5979+
assert.deepStrictEqual(removed, ['release/24.10']);
5980+
});
5981+
5982+
test('keeps the release label that matches the base branch', async () => {
5983+
const removed = await labelsRemovedFor({ labels: ['release/25.12'], baseRef: 'openwrt-25.12', patch: plainPatch });
5984+
assert.deepStrictEqual(removed, []);
5985+
});
5986+
5987+
test('leaves a label matched from labeler.yml alone', async () => {
5988+
const removed = await labelsRemovedFor({ labels: ['target/ath79'], patch: plainPatch });
5989+
assert.deepStrictEqual(removed, [], 'a path label is as often set by hand as derived');
5990+
});
5991+
58995992
test('labeler.yml integration: handles missing (404) .github/labeler.yml gracefully', async () => {
59005993
const originalImportKey = crypto.subtle.importKey;
59015994
crypto.subtle.importKey = async (format, keyData, algorithm, extractable, keyUsages) => {

0 commit comments

Comments
 (0)