fix(ci): tolerate null PR fields and honour existing area/* in pr-labeler#17
Conversation
…eler 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 <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
Warning Review limit reached
More reviews will be available in 31 minutes and 20 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request improves the robustness of the PR labeler script by adding null-safety checks to the destructured parameters of computeLabels, preventing runtime errors when GitHub payloads contain explicit null values. Additionally, it updates the fallback logic to honor existing, maintainer-added area/* labels (excluding area/uncategorized) so that the fallback label is not redundantly applied. Comprehensive unit tests have been added to verify these changes. No review comments were provided.
Maxim Kitsunoff (kitsunoff)
left a comment
There was a problem hiding this comment.
LGTM. Null-safety at the function boundary is the right call (defence in depth even with caller-side coercion), and excluding area/uncategorized from the hasArea check avoids freezing the fallback in place.
Follow-up to #16. Addresses two correctness bugs in
computeLabels(.github/scripts/pr-labeler.js) caught in review.The bugs
Null inputs crash
body.match().computeLabelsdestructuredtitle/body/existingLabelswith= ''defaults. Destructuring defaults only fire onundefined. GitHub PR payloads carry an explicitnullfor an empty body (and, less commonly, for missing fields), which would slip past the default and crash on the first.match()or Set iteration. Switched to explicitrawTitle || ''/rawBody || ''/new Set(rawExisting || [])sonullon any input is treated as the empty value.hasAreaignored existing area/ labels.* A maintainer-addedarea/formsfollowed by a scopeless retitle (e.g.chore: housekeeping) would re-addarea/uncategorizedon top of the real area. The next labeler run would clean it up, but the PR carried both labels until then. FixedhasAreato also consider already-attached realarea/*, witharea/uncategorizeditself excluded so a scopeless retitle of an only-uncategorized PR still routes through the fallback path.Tests
Five new cases in
pr-labeler.test.js, all pinning behaviour that fails without the corresponding fix:null body does not thrownull title is treated as emptynull existingLabels does not throwretitle to scopeless when maintainer-added area/* exists: no area/uncategorized— stale fallback gets removedexisting only has area/uncategorized: scopeless title keeps area/uncategorized (no churn)— fallback path still taken, no flapping26 tests pass (
node --test .github/scripts/*.test.js).Note
pr-labeler.yamlalready coercespr.title || '',pr.body || '',(pr.labels || [])at the caller, so fix #1 is defense-in-depth at the function boundary. Worth keeping — the function contract should not rely on every caller doing the coercion.