ci(labels): adopt Kubernetes-style label scheme#16
Conversation
Mirror the label workflow already running in cozystack/cozystack: a declarative .github/labels.yml synced via EndBug/label-sync, a PR auto-labeler that derives kind/* and area/* from the Conventional Commits title, and a PR size labeler that ignores generated and vendored paths. labels.yml carries the generic categories (kind/, priority/, triage/, lifecycle/, do-not-merge/, security/, size/) and is adapted to this repo by replacing the backend-oriented area/* set (database, network, storage, virtualization, platform, etc.) with area/* that match the SPA: console, forms, k8s-client, ui, types, tenants, auth, vm, container, ci, docs, tests, plus area/uncategorized as the fallback. The labeler's authoritative-label set is narrow on purpose. Beyond the additive pass it removes exactly two labels it knows it set itself: area/uncategorized once a real area/* is derived, and kind/breaking-change once neither the conventional-commit ! marker nor a BREAKING CHANGE: footer remains. Maintainer-added labels are preserved untouched. The derivation lives in .github/scripts/pr-labeler.js as a pure function so the rules can be covered by node:test cases (.github/scripts/pr-labeler.test.js) exercising the title-edit scenarios the additive-only design used to mishandle. pr-size isIgnored is rewritten for a node workspace: pnpm-lock.yaml, package-lock.json, yarn.lock, bun.lockb, *.lock, node_modules/, dist/, build/, generated/. The Go-specific paths from cozystack (vendor/, zz_generated, .pb.go, charts/) are dropped. The validate job grows a description-vs-automation drift check: any label whose description advertises automation (auto-applied, auto-closed, auto-labeled, auto-closed) must be referenced by name in .github/workflows/ or .github/scripts/, so a description cannot promise behaviour that no code backs. The lifecycle/* descriptions and the lgtm description are reworded to reflect that they are manual markers in this repo — there is no stale-bot and no /lgtm command handler. labels.yml header notes that size/* line-count thresholds are the contract pr-size.yaml encodes. EndBug/label-sync runs with delete-other-labels: false, so the existing GitHub defaults (bug, enhancement, documentation, question, duplicate) collapse into the new kind/* and triage/* names via the aliases lists without losing references on already-tagged items. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR introduces a complete automated label management system for the repository. It defines a canonical label catalog, implements Conventional Commits-based label derivation from PR titles, adds workflows to synchronize and validate labels, and includes automatic PR size labeling based on diff line counts. ChangesPR Label Automation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 introduces a custom PR labeler script (pr-labeler.js) and associated unit tests to automate label management based on Conventional Commits and PR titles. The review feedback highlights critical improvements for robust input handling against null values, preventing duplicate fallback labels when a maintainer has manually categorized a PR, and adding a corresponding unit test to cover this scenario.
| export function computeLabels({ title = '', body = '', existingLabels = [] } = {}) { | ||
| const existing = new Set(existingLabels) | ||
| const toAdd = new Set() |
There was a problem hiding this comment.
Defensive Programming: Handle Null Inputs Safely
If title, body, or existingLabels are explicitly passed as null (which is common in GitHub Actions payloads or context objects), the destructuring default values will not apply. This can lead to runtime TypeError exceptions (e.g., TypeError: Cannot read properties of null (reading 'match') or TypeError: object is not iterable).
Using explicit logical OR (||) fallbacks ensures robust defensive programming.
| export function computeLabels({ title = '', body = '', existingLabels = [] } = {}) { | |
| const existing = new Set(existingLabels) | |
| const toAdd = new Set() | |
| export function computeLabels({ title: rawTitle = '', body: rawBody = '', existingLabels = [] } = {}) { | |
| const title = rawTitle || '' | |
| const body = rawBody || '' | |
| const existing = new Set(existingLabels || []) | |
| const toAdd = new Set() |
| const hasArea = [...toAdd].some((l) => l.startsWith('area/')) | ||
| if (!hasArea) { | ||
| toAdd.add('area/uncategorized') | ||
| } |
There was a problem hiding this comment.
Prevent Duplicate/Conflicting Fallback Labels
Currently, hasArea only checks if an area/* label is present in the newly derived toAdd set. If a PR already has a manually assigned area/* label (e.g., area/forms) and is later retitled to a title without a scope (e.g., chore: housekeeping), hasArea will be false, causing area/uncategorized to be added. This results in the PR having both a specific area label and the uncategorized fallback.
By checking both toAdd and existing (excluding area/uncategorized itself), we prevent this duplication and automatically clean up area/uncategorized as soon as a maintainer manually categorizes the PR.
| const hasArea = [...toAdd].some((l) => l.startsWith('area/')) | |
| if (!hasArea) { | |
| toAdd.add('area/uncategorized') | |
| } | |
| const hasArea = [...toAdd].some((l) => l.startsWith('area/')) || | |
| [...existing].some((l) => l.startsWith('area/') && l !== 'area/uncategorized') | |
| if (!hasArea) { | |
| toAdd.add('area/uncategorized') | |
| } |
| assert.deepEqual(remove, []) | ||
| }) |
There was a problem hiding this comment.
Add Test Case for Retitling with Existing Maintainer-Added Area Labels
Let's add a unit test to verify that area/uncategorized is not added (and is removed if present) when a PR already has a maintainer-added area/* label, even if the new title has no scope.
| assert.deepEqual(remove, []) | |
| }) | |
| assert.deepEqual(remove, []) | |
| }) | |
| test('retitle: no scope, but maintainer-added area/* exists — area/uncategorized is not added and is removed if present', () => { | |
| const { add, remove } = computeLabels({ | |
| title: 'chore: bump workflow', | |
| existingLabels: ['area/forms', 'area/uncategorized'], | |
| }) | |
| assert.ok(!add.includes('area/uncategorized')) | |
| assert.ok(remove.includes('area/uncategorized')) | |
| }) |
Maxim Kitsunoff (kitsunoff)
left a comment
There was a problem hiding this comment.
LGTM. Solid review-grade scaffolding — authoritative-vs-additive split is correct, tests cover the retitle regressions, validate job catches description-vs-automation drift, pull_request_target workflows are properly hardened.
Mirror the label workflow already running in cozystack/cozystack so this repo gets the same Kubernetes-style scheme (
kind/,area/,priority/,triage/,lifecycle/,do-not-merge/,security/,size/).What this lands
.github/labels.yml— 68 labels, declarative, validated on every PR..github/scripts/pr-labeler.js— pure label-derivation logic, extracted so the rules can be unit-tested withnode --test..github/scripts/pr-labeler.test.js— 21 cases covering happy path, retitle / stale-label removal, composite scopes, bracket form, breaking-change detection, and the "non-labeler labels are never removed" invariant..github/workflows/labels.yaml— validates schema, runs the JS unit tests, syncs labels viaEndBug/label-sync@v2on push tomain, weekly cron, andworkflow_dispatch..github/workflows/pr-labeler.yaml— deriveskind/*andarea/*from the Conventional Commits title on PR open/edit/synchronize..github/workflows/pr-size.yaml— appliessize/{XS,S,M,L,XL,XXL}based on line count, ignoring lockfiles and generated paths.What I adapted from cozystack/cozystack
The cross-cutting namespaces (
kind/,priority/,triage/,lifecycle/,do-not-merge/,security/,size/) come over verbatim. Thearea/set is rewritten for the SPA: backend-oriented areas (database, networking, storage, virtualization, monitoring, platform, kubernetes, extra, api, build, dashboard, release, testing) are out. The new set isconsole,forms,k8s-client,ui,types,tenants,auth,vm,container,ci,docs,tests, plusuncategorizedas the fallback.pr-labelerscope→area mapping covers scopes observed in the repo history (console,ui,backup,backups,external-ips,workflows,overview,forms) plus the widget and package names that show up in the codebase.pr-sizeisIgnoredis rewritten for a node workspace:pnpm-lock.yaml,package-lock.json,yarn.lock,bun.lockb,*.lock,node_modules/,dist/,build/,generated/. The Go-specific paths (vendor/,zz_generated,.pb.go, charts) are dropped.backport/backport-previousare dropped — there is no release-line backport process here.Differences from the upstream version worth flagging
area/uncategorizedonce a realarea/*is derived from the title, and removeskind/breaking-changeonce neither the!marker nor aBREAKING CHANGE:footer remains. Maintainer-added labels are never touched. This fixes the title-edit regression where a retitled PR would end up with botharea/uncategorizedand the new area, or with a stalekind/breaking-change.auto-applied,auto-closed,auto-labeled,auto-close*) must be referenced by name in.github/workflows/or.github/scripts/, so a description cannot promise behaviour that no code backs.lifecycle/*descriptions and thelgtmdescription are reworded to reflect that they are manual markers in this repo — there is no stale-bot and no/lgtmcommand handler.node --test. The workflow imports it viaawait import().Migration of existing labels
EndBug/label-syncruns withdelete-other-labels: false. The existing GitHub defaults (bug,enhancement,documentation,question,duplicate) collapse into the newkind/*andtriage/*names viaaliases:inlabels.yml— references on already-tagged items are preserved. Unmapped defaults (invalid,wontfix,help wanted,good first issue) are kept as-is.Security
Both
pull_request_targetworkflows are hardened:pr-labeler.yamlpins checkout topull_request.base.shawithpersist-credentials: false, so the labeler script can never come from an attacker-controlled PR head.pr-size.yamldoesn't check out at all. Permissions are minimal (contents: read,pull-requests: write). Both workflows have a per-PRconcurrencygroup so a stale run cannot land its mutations after a newer run.Validation
node --test: 21/21 pass.actionlintclean on all three workflows.Summary by CodeRabbit