Repository navigation
ci: label unlabelled issues with a model and check PR descriptions - #4554
justinmclean wants to merge 9 commits into
Conversation
Issues the form leaves without an area get labels from a self-hosted model on ASF infrastructure, filtered to the allowlist. Contributor PRs fail a check when the description has no linked issue, description or test notes.
|
@justinmclean please use just title |
|
Thanks for the PR. It is labeled Slash commands (own line, regular comment) move it around the queue:
See CONTRIBUTING.md for details. |
|
@justinmclean could you extract the inline logic into standalone scripts, with the workflows calling the same code? Maintainers should be able to run them manually for a specific issue or PR. Please add |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4554 +/- ##
============================================
- Coverage 90.75% 90.72% -0.04%
- Complexity 2966 2989 +23
============================================
Files 1459 1459
Lines 347699 348348 +649
Branches 292547 293449 +902
============================================
+ Hits 315569 316034 +465
- Misses 25515 25659 +144
- Partials 6615 6655 +40
🚀 New features to boost your workflow:
|
The workflows now run scripts/ci/issue-labeler.mjs and scripts/ci/pr-description-check.mjs, so maintainers can run them by hand on any issue or PR. The labeler has --dry-run.
|
Done. The labeler and the PR check are now |
|
@hubcio could you rerun the failed Swift e2e job? |
|
@justinmclean i'm leaning towards having these scripts written in bash, because of 2 reasons:
what do you think about that? i'm not 100% set in stone about this. |
Use curl and jq instead of Node, to match the rest of scripts/ci and get shellcheck coverage. Behaviour, flags and output are unchanged.
|
Converted to bash. scripts/ci/issue-labeler.sh and scripts/ci/pr-description-check.sh, using curl and jq. I ran them side by side with the Node versions on the same cases and got the same labels, output and exit codes. |
|
/ready |
|
/skill team-review |
There was a problem hiding this comment.
Summary: .github/workflows/pr-description.yml grants only contents: read, so the new check cannot read the pull request and fails on every contributor PR; scripts/ci/issue-labeler.sh also prints model-controlled text that can inject workflow commands. The review also flags the loose issue-link regex and the enhancement label; CI was still running at review time, so no build or test result settles these findings.
Counts: critical 1, warning 2, nit 1, simplification 1
This review was generated by Claude Code 2.1.284 on deepseek-flash[1m]. Review the output before you act on it.
| types: [opened, edited, reopened, ready_for_review] | ||
|
|
||
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
critical: The workflow grants only contents: read, so the token has no pull-requests permission; scripts/ci/pr-description-check.sh calls GET /repos/apache/iggy/pulls/{n} with it and exits 1 on the 403. Add pull-requests: read to the permissions block.
There was a problem hiding this comment.
I think this is false:
“This endpoint can be used without authentication or the aforementioned permissions if only public resources are requested.”
https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#get-a-pull-request
There was a problem hiding this comment.
Checked it: with no token at all, GET /repos/apache/iggy/pulls/4554 returns 200 and the full description, so the contents: read token is enough for a public repo.
| fi | ||
| LABELS=$(jq -c .labels <<<"$result") | ||
| if [[ $LABELS == '[]' ]]; then | ||
| info "model suggested no allowed labels: $(jq -r .text <<<"$result")" |
There was a problem hiding this comment.
warning: The labeler prints the model reply through info and warning at lines 245 and 250, so a reply line that starts with :: runs as a workflow command. Replace \r, \n and % in model text before printing it.
There was a problem hiding this comment.
Fixed. info and warning now keep each message on one line, and in Actions warning escapes %, CR and LF the way workflow commands expect, so model text can't start a line with ::.
| | ($body | split("\n") | map(select(test("^\\s*#") | not)) | join(" ") | ||
| | gsub("\\s+"; " ") | sub("^ "; "") | sub(" $"; "")) as $prose | ||
| # An explicit "N/A" or "None" under the issue heading is an answer. | ||
| | (if ($body | test("#\\d+|github\\.com/apache/iggy/issues/\\d+") | not) |
There was a problem hiding this comment.
warning: The issue check accepts #\d+ anywhere, so a body with a hex color such as #007bff passes it with no linked issue. Require Closes or Relates to, or the repo issue URL, before the number.
There was a problem hiding this comment.
Fixed. A #123 now has to stand on its own, so #007bff and { no longer count as an issue link. I kept the check across the whole body so a free-form description still passes, and the old and new versions gave the same result on the last 40 PRs.
| --argjson want_type "$want_type" ' | ||
| ($areas | to_entries | map("- \(.value): \(.key)") | join("\n")) as $list | ||
| | (if $want_type | ||
| then "\nAlso pick at most one type label:\n- bug: something is broken\n- enhancement: a new feature or improvement\n" |
There was a problem hiding this comment.
nit: The prompt offers enhancement, but the feature template applies feature (.github/ISSUE_TEMPLATE/feature_request.yml:20). An issue without a type label therefore gets a different label from a templated feature request. Offer feature in the prompt and the reply filter.
There was a problem hiding this comment.
No change here. apache/iggy has no feature label, so the template's labels: ["feature"] doesn't apply anything, and existing feature requests use enhancement, which is what the labeler offers.
| TMP=$(mktemp -d) | ||
| trap 'rm -rf "$TMP"' EXIT | ||
|
|
||
| # The token goes to curl in a header file, never on its command line. |
There was a problem hiding this comment.
simplification: The token header setup and curl status handling repeat scripts/ci/issue-labeler.sh:124-149. Move one request helper into scripts/ci/lib/ and source it from both scripts.
There was a problem hiding this comment.
Done in the latest commit.
info and warning flatten or escape CR, LF and % so model text cannot start a workflow command. The PR check now needs apache#123 to stand alone, so a colour like #007bff is not taken for an issue link.
The request code moves to scripts/ci/lib/github.sh, sourced by the labeler and the PR check and checked out by both workflows. The issue-link check also accepts owner/repo#123 again.
|
@hubcio fixed the findings from the team review and replied on each. Could you take another look? /ready |
Which issue does this PR address?
N/A
Rationale
Issues opened blank, or with "Other / not sure" as the area, get no area label from the issue form, so a maintainer has to label them by hand. Contributor PRs often arrive without a linked issue or test notes, and reviewers have to ask for them.
What changed?
The issue form labelling moves into
scripts/ci/issue-labeler.mjs, which the issue workflow now runs. When the form gives no area, the script asks a self-hosted model on ASF infrastructure (LLMAO, defaultgemma4-26b) for up to three area labels, plusbugorenhancementwhen the issue has no type label. Only exact matches to the existing allowlist are applied, so a prompt injection in the issue body can at worst pick a wrong label from that list. WithoutLLMAO_API_KEYandLLMAO_BASE_URLthe model part does nothing, and model errors are warnings, not failures.A new PR Description check,
scripts/ci/pr-description-check.mjs, fails a contributor PR whose description has no linked issue (or explicit N/A), no description, or no test notes, and names each gap in the job summary. It is plain pattern matching, with no model, and the workflow skips bots, drafts and authors with write access.Both workflows check out only their script, from the default branch or the PR's base commit, without persisted credentials. Maintainers can run either script by hand on any issue or PR:
--dry-runprints the labels the labeler would add and changes nothing on GitHub. Each script's header lists the configuration it reads.Local Execution
Not yet run against the live LLMAO endpoint, which needs the secret and variable set first.
Setup (PMC member)
Create an LLMAO personal key with Project set to Apache Iggy, then add it as a secret. The command prompts for the value, so the key never appears in shell history:
gh secret set LLMAO_API_KEY -R apache/iggySet the base URL from the LLMAO page, ending in
/v1:After merge I'll check the results on the next few issues and contributor PRs.
AI Usage
Claude Code wrote the scripts, the workflow steps and the test harnesses. I reviewed the changes and the test results.