From 65fe84244bc988245e25cae3396f5b3f40261f59 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Tue, 26 May 2026 17:24:38 +0300 Subject: [PATCH] fix(ci): tolerate null PR fields and honour existing area/* in pr-labeler Two correctness bugs surfaced after the initial labeler landed: 1. Null inputs. computeLabels destructured title/body/existingLabels with defaults, but destructuring defaults only fire on undefined. GitHub PR payloads carry an explicit null for an empty description, which would crash body.match() at runtime. Switched to explicit || fallbacks on rawTitle/rawBody/rawExisting so null on any of the three is treated as the empty value. 2. hasArea ignored existing area/* labels. A maintainer-added area/* followed by a scopeless retitle would re-add area/uncategorized on top of the real area, leaving the PR with both labels until the next edit cycle cleared the fallback. Now hasArea checks the new derivation AND existing labels (excluding area/uncategorized itself so a scopeless retitle of an only-uncategorized PR still reaches the fallback path). Five new tests pin the behaviour: null body, null title, null existingLabels, scopeless retitle with maintainer-added area/* (no fallback added, stale fallback removed), and the edge case where the only existing area is area/uncategorized (fallback is not frozen in place). Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- .github/scripts/pr-labeler.js | 16 ++++++++-- .github/scripts/pr-labeler.test.js | 48 ++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/.github/scripts/pr-labeler.js b/.github/scripts/pr-labeler.js index 7186102..6354261 100644 --- a/.github/scripts/pr-labeler.js +++ b/.github/scripts/pr-labeler.js @@ -127,8 +127,13 @@ export const scopeToArea = { // // Anything else (maintainer-added `area/*`, manual `kind/*`, anything // outside these two namespaces) is preserved on every run. -export function computeLabels({ title = '', body = '', existingLabels = [] } = {}) { - const existing = new Set(existingLabels) +export function computeLabels({ title: rawTitle, body: rawBody, existingLabels: rawExisting } = {}) { + // Destructuring defaults only fire on `undefined`, not `null`. GitHub PR + // payloads can carry an explicit `null` for an empty body and (less often) + // labels — fall back via `||` so `.match()` and Set iteration are safe. + const title = rawTitle || '' + const body = rawBody || '' + const existing = new Set(rawExisting || []) const toAdd = new Set() // 1. Try Conventional Commits form: type(scope)?(!)?: description @@ -186,7 +191,12 @@ export function computeLabels({ title = '', body = '', existingLabels = [] } = { } // 7. Fallback: no area/* applied -> area/uncategorized. - const hasArea = [...toAdd].some((l) => l.startsWith('area/')) + // Honour maintainer-added area/* already on the PR — if the title has no + // scope but a real area/* is already attached, the PR is categorised; do + // not pile area/uncategorized on top. + const hasArea = + [...toAdd].some((l) => l.startsWith('area/')) || + [...existing].some((l) => l.startsWith('area/') && l !== 'area/uncategorized') if (!hasArea) { toAdd.add('area/uncategorized') } diff --git a/.github/scripts/pr-labeler.test.js b/.github/scripts/pr-labeler.test.js index 2cc49c6..4d9c1f0 100644 --- a/.github/scripts/pr-labeler.test.js +++ b/.github/scripts/pr-labeler.test.js @@ -160,3 +160,51 @@ test('non-labeler labels are never removed', () => { }) assert.deepEqual(remove, []) }) + +// ── Null-safety: destructuring defaults only fire on undefined ── + +test('null body does not throw', () => { + // GitHub PR payloads carry an explicit `null` for an empty description. + const { add } = computeLabels({ title: 'feat(ui): x', body: null }) + assert.ok(add.includes('kind/feature')) + assert.ok(add.includes('area/ui')) +}) + +test('null title is treated as empty', () => { + const { add } = computeLabels({ title: null }) + assert.ok(add.includes('area/uncategorized')) +}) + +test('null existingLabels does not throw', () => { + const { add } = computeLabels({ title: 'feat(ui): x', existingLabels: null }) + assert.ok(add.includes('area/ui')) +}) + +// ── hasArea honours existing maintainer-added area/* ── + +test('retitle to scopeless when maintainer-added area/* exists: no area/uncategorized', () => { + // Maintainer manually attached area/forms. PR title is later retitled to a + // scopeless form. The labeler must not pile area/uncategorized on top of + // the already-categorised state, and must strip it if it was there from + // an earlier run. + const { add, remove } = computeLabels({ + title: 'chore: housekeeping', + existingLabels: ['area/forms', 'area/uncategorized'], + }) + assert.ok(!add.includes('area/uncategorized')) + assert.ok(remove.includes('area/uncategorized')) +}) + +test('existing only has area/uncategorized: scopeless title keeps area/uncategorized (no churn)', () => { + // The "honour existing area/*" rule must not count area/uncategorized as + // a real categorisation — otherwise a scopeless retitle would silently + // freeze the fallback in place. Expected net effect: fallback path is + // taken, area/uncategorized stays put (already-present so not re-added, + // and hasArea is false so not removed), kind/cleanup derives from chore. + const { add, remove } = computeLabels({ + title: 'chore: housekeeping', + existingLabels: ['area/uncategorized'], + }) + assert.deepEqual(add, ['kind/cleanup']) + assert.deepEqual(remove, []) +})