Repository navigation
fix(ci): harden the PR labeler's title parsing and map recurring scopes - #4666
Conversation
Both maps are plain object literals, so a title typed or scoped `constructor`, `toString` or `__proto__` resolved to an Object.prototype member. That value is truthy, landed in the label set and killed the step on `l.startsWith`, leaving the PR with no labels at all. The new contract test runs the workflow's own script under node with stubbed context, so it exercises the code as shipped rather than a copy of it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
A backport of a backport is titled with two `[Backport ...]` prefixes. Only the first was stripped, so the second was parsed as a bracket scope: the real scope and the kind were lost, and the area/release added by the strip suppressed the area/uncategorized fallback, so the PR looked labelled correctly. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
Over the full PR history a set of scopes recurring three or more times still fell through to area/uncategorized. Most of them have an obvious home and were only missing a row. Subsystem names such as `networking` or `storage` are now accepted as the scope of their own area, since contributors reach for those as often as for a package name. `dx`, `hack` and `cozypkg` describe developer tooling, which no existing area covers: area/build is image builds, area/testing is tests. They get a new area/dx rather than being filed under either. The contract test also pins that every map target exists in labels.yml, since the label sync workflow creates only what that file defines, and that no key is written twice, since a repeated key silently overrides the earlier one. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
github-actions
Bot
added
area/ci
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: cozystack/cozystack/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe pull request adds area labels and scope mappings, updates backport-title parsing and mapping lookups in the PR labeler, and adds a Bats contract suite for the workflow script. ChangesPull request labeling
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The change broadens scope labeling and hardens title parsing, with contract tests covering the changed behavior. No actionable merge-blocking risk is established; merge after normal checks pass. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
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 |
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
LGTM
Checked the parsing hardening by running the workflow's own github-script block under node, at the merge base and at this head, over the 60 most recent real PR titles plus an adversarial set. Contract suite is green (8/8).
What the diff actually changes in behaviour, confirmed by diffing base against head output:
- Double
[Backport ...]prefixes now parse. Before,[Backport 1.2] [Backport 1.3] fix(linstor): xkept onlyarea/releaseand dropped both the scope and the kind. Now it yieldsarea/release, area/storage, kind/bug. - A scope or type named after an
Object.prototypemember no longer kills the job.fix(constructor): xthrewl.startsWith is not a functionat the base;Object.hasOwnturns that intoarea/uncategorized, kind/bug. - Everything else is byte-identical.
type(scope): x,type!: x,type(scope)!: x, compositefix(a, b): x, and[scope]bracket titles all produce the same labels they did before. On the 60 real titles the only three differences were titles that used to land inarea/uncategorizedand now resolve correctly:controllerto platform,workflowsto ci,monitoring-agentsto monitoring.
The scope map is clean. 130 keys, no duplicates, and every one of the 22 target areas is defined in labels.yml. I spot-checked the less obvious additions against the package tree: ouroboros ships etchosts/coredns/networkpolicy, so networking is right, and dashboard-controller is a real scope from the changelogs. The tests run the extracted script rather than a copy of it, which is the right call.
Two non-blocking notes, neither worth a change:
- [NIT] Adjacent backport prefixes with no space between them (
[Backport a][Backport b] ...) still fall to area/uncategorized, because the strip regex wants whitespace after each]. This is pre-existing, and tooling-generated backports always put the space in. - [NIT] The contract does not pin the breaking-change forms or composite-scope parsing. The PR does not touch those paths and I verified they still hold, so it is optional coverage.
Merged current origin/main into the head before signing off; the contract suite stays green there too.
2 participants
What this PR does
Fixes three problems in
.github/workflows/pr-labeler.yaml. Each one either killed the job or put a PR in the wrong area, and none of them showed up anywhere.Object.hasOwn. A title typed or scopedconstructor,toString,hasOwnPropertyor__proto__used to resolve to anObject.prototypemember and crash the step onl.startsWith, which left the PR with no labels at all.[Backport ...]prefix is now stripped, not only the first one. With two prefixes, the second was read as a bracket scope, so the real scope and thekind/*label were both lost. Thearea/releaselabel hid the problem by keepingarea/uncategorizedfrom firing.networking,storage,database,virtualization,testing,dependencies).Scope-to-area decisions:
area/database:database,etcd-operator,opensearch,opensearch-operatorarea/networking:networking,kubeovn,multus,kilo,hetzner-robotlb,ouroboros(it rewrites the CoreDNS Corefile to fix hairpin NAT, so it is networking, not platform)area/virtualization:virtualization,vm,kubevirt-operator,kubevirt-cdi,kubevirt-instancetypesarea/platform:operator,cozystack-operator,controller,cozystack-controller,cozystack-basics,flux-shard-operator,migrations,scheduler,cozystack-scheduler,reloader,bootbox, andlineage. I putlineagehere on purpose: no existing label covers it more precisely, and it is too narrow for an area of its own.area/storage:storage,piraeus-operator,backupstrategy-controllerarea/release:changelog,changelogsarea/ci:workflows,labelerarea/dependencies:dependencies,renovatearea/api:registry(the cozystack-api storage layer)area/dashboard:dashboard-controllerarea/monitoring:monitoring-agentsarea/keycloak:keycloak-configurearea/testing:testingarea/dx, added to.github/labels.yml:dx,hack,cozypkg. These are developer tooling (hack/ scripts, package Makefiles, cozypkg).area/buildcovers image builds andarea/testingcovers tests, and neither describes this work.Left unmapped on purpose:
http-cache: the application was removed from the tree.openbao: already mapped on main.apps,system,docs: catch-all scopes that don't point to one area.fix,bugfix: malformed titles.security,rbac,governance,codeowners,maintenance,images,readme,telemetry,cluster-autoscaler: no single area fits any of them.I did not add spelling normalisation.
kubeovngot its own row, since it is the only alternate spelling that recurs.A new contract test,
hack/pr-labeler-contract.bats, pulls the workflow's ownscript:block out of the file and runs it under node with stubbedcontext,coreandgithub, so it tests the shipped code, not a copy. On main it is red on the prototype titles (THROW: l.startsWith is not a function), on the double backport (onlyarea/release), on the newly mapped scopes and on the area-name scopes. It also checks that every map target exists inlabels.ymland that no key is written twice. The suite fails rather than skips when node is missing. The older node-based suites skip, so this is the first one that needs node on the unit-test runner.Some things stay out of this PR. The scope table in
docs/agents/contributing.mdis a summary. It has no row forarea/dxand does not mention that an area's own name works as a scope. Separately, the singularmigrationstill maps toarea/virtualizationas it did before, while the newmigrationsmaps toarea/platform.A backport PR whose scope is unmapped still never gets
area/uncategorized, even with a single prefix. Thearea/releaseadded by the strip satisfies the fallback check. This behaviour is the same on main and is not changed here.The test that checks map targets against
labels.ymlpasses by itself if the map literal cannot be extracted. The duplicate-key and area-name tests go red in that case, so the suite as a whole still fails.Closes #4125, closes #4124, closes #4122
Screenshots
Not a UI change.
Downstream repositories
Release note
Summary by CodeRabbit
area/dxlabel for issues and pull requests related to developer tooling.