[no-ci] fix(ci): pr-metadata-check — blocked-label bug, exact match, multi-word labels#1836
Conversation
The blocked-patterns loop word-split "DO NOT MERGE" into individual words, so "DO" matched as a substring of "documentation" via grep. Replace with a single grep -qiE regex and read labels line-by-line from jq to also handle multi-word label names correctly. Made-with: Cursor
cpcloud
left a comment
There was a problem hiding this comment.
Thanks for fixing the word-splitting bug here.
I think we should still avoid the remaining grep -qiE \"blocked|do not merge\" check, since it is still doing substring matching rather than exact label matching. The workflow already has the PR labels in LABELS, so this can be an exact, case-insensitive jq check instead.
I left an inline suggestion on the changed block with one way to do that.
…tively) Co-authored-by: Phillip Cloud <417981+cpcloud@users.noreply.github.com>
|
Archiving a Cursor-generated review below. I'll try to get it to fix the "Minor UX detail" and Net change @ c6b46b5Scope: Bug fixThe previous code put Follow-up behavior change (reviewer suggestion)The fix is not only “stop splitting patterns.” The shipped logic also changes substring matching ( So compared to the old intent of “grep for blocked-ish text,” this is stricter: a label like Minor UX detailError messages use the lowercased names emitted by Unchanged in this PR
|
Pull request was converted to draft
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Shell word-splitting on $LABEL_NAMES broke multi-word GitHub label names. Read jq output with while read and build LABEL_LIST from the same jq stream. Made-with: Cursor
Emit each label's original .name from the PR payload when it matches the blocked list case-insensitively, instead of lowercased names from jq. Made-with: Cursor
|
@cpcloud this is ready for review again. I replaced the PR title and rewrote the description (there are now three changes in this PR). |
This comment has been minimized.
This comment has been minimized.
1 similar comment
|
Summary
Fixes the PR metadata workflow’s blocked-label false positive and incorrect handling of multi-word GitHub labels; tightens blocked-label detection to exact name matches (case-insensitive); error text for blocked labels uses GitHub’s spelling of the label name.
Fixes
Blocked-label false positive —
BLOCKED_PATTERNSwas unquoted in afor pattern in $BLOCKED_PATTERNSloop, so the shell split the string into tokens (blocked,DO,NOT, …). Each token was passed togrepagainst the full label name, soDOmatched the substringdoindocumentation, incorrectly reporting a blocked label. The workflow now derives blocked matches from the JSON payload withjq; error lines use each label’s original spelling from GitHub, not a lowercased form.Module / type label iteration —
LABEL_NAMESwas filled withjq -r '.[].name'but then iterated withfor label in $LABEL_NAMES, so multi-word label names were split on spaces and never matched the allowed module/type lists. Module and type checks now read label names line-by-line fromjq(and the success summary builds the label list from the samejqstream).Behavior change (intentional)
Blocked labels — Detection no longer uses substring
grepon label text. It matches only labels whose name is exactlyblockedordo not merge, case-insensitive (viajq). Custom labels that merely contain those words (e.g.blocked-by-review) are not treated as blocked; that is stricter than substring matching and matches reviewer feedback.Workflow logs reviewed while working on this PR
documentation,cuda.bindingsdocumentationflaggedenhancement,feature,cuda.bindings