CLAUDE.md, PR babysitter, local dev skills + symlinks for Codex/Cursor#15613
Conversation
Signed-off-by: Piotr Żelasko <petezor@gmail.com>
|
/claude review |
| "matcher": "Write|Edit", | ||
| "hooks": [ | ||
| { | ||
| "type": "command", |
There was a problem hiding this comment.
Bug: The directory-filter case patterns (docs/*, nemo/collections/*, etc.) are relative, but tool_input.file_path from Claude Code's Write/Edit tools is always an absolute path (e.g. /home/user/NeMo/nemo/collections/asr/foo.py). Since bash case matching requires docs/* to match from the start of the string, none of these exclusions will ever trigger — so black/isort will run on every .py file, including the collections that pyproject.toml extend-exclude is supposed to skip.
One fix: strip the path to repo-relative before the filter, e.g.:
| "type": "command", | |
| "command": "jq -r '.tool_input.file_path' | { read -r f; f=\"${f#\"$(git rev-parse --show-toplevel)/\"}\"; case \"$f\" in *.py) ;; *) exit 0;; esac; case \"$f\" in docs/*|external/*|examples/*) exit 0;; nemo/collections/speechlm2/*) ;; nemo/collections/*) exit 0;; esac; black --quiet \"$f\"; isort --quiet \"$f\"; } 2>/dev/null || true", |
Or use wildcard prefixes like */docs/* in the patterns.
|
[🤖]: Hi @pzelasko 👋, We wanted to let you know that a CICD pipeline for this PR just finished successfully. So it might be time to merge this PR or get some approvals. |
Signed-off-by: Piotr Żelasko <petezor@gmail.com>
|
@nithinraok @vmendelev I addressed your reviews.
|
|
I'm taking over this PR. I'll ping the author if I can't figure it out. |
1 similar comment
|
I'm taking over this PR. I'll ping the author if I can't figure it out. |
Signed-off-by: Piotr Żelasko <petezor@gmail.com>
CLAUDE.md, PR babysitter, local dev skills + symlinks for Codex/Cursor
|
/claude review |
nithinraok
left a comment
There was a problem hiding this comment.
Great work towards automating!
Signed-off-by: Piotr Żelasko <petezor@gmail.com>
|
[🤖]: Hi @pzelasko 👋, We wanted to let you know that a CICD pipeline for this PR just finished successfully. So it might be time to merge this PR or get some approvals. |
Signed-off-by: Piotr Żelasko <petezor@gmail.com>
|
Following offline discussion about this PR with @chtruong814, we decided to make the PR babysitter GH action less autonomous for now. New logic: When there is a check failure, and |
chtruong814
left a comment
There was a problem hiding this comment.
I just focused on the github action. main concern is the authorization check and ensuring this overall workflow does not run if someone is not a speech team member.
| issue_comment: | ||
| types: [created] | ||
| schedule: | ||
| - cron: '*/5 * * * *' |
There was a problem hiding this comment.
Why does this need to run on a schedule?
There was a problem hiding this comment.
It was to detect the 👍🏻 emoji. I decided to drop this to simplify - approval can now be only done via comment.
| issues: write | ||
| id-token: write | ||
|
|
||
| jobs: |
There was a problem hiding this comment.
We probably should have checks that this is only running on branches or PRs for internal users. The claude code action should also have a check on the user that triggers the event has write access. But there's a lot going on here, and I think we should ensure nothing happens if it's for forks/external contributors.
There was a problem hiding this comment.
Hardened every step with a "preflight" verification
| contains(github.event.pull_request.labels.*.name, 'Has Babysitter')) || | ||
| (github.event_name == 'pull_request_review' && | ||
| contains(github.event.review.body, '@claude') && | ||
| contains(github.event.pull_request.labels.*.name, 'Has Babysitter')) |
There was a problem hiding this comment.
We may need to check for a specific @claude /command because there may be other claude actions in this repo that could inadvertently trigger this.
There was a problem hiding this comment.
The other actions are /claude so they don't conflict. I'd rather keep this as natural language thing, we might want to evolve this into a chat later.
| } | ||
| } catch (e) { | ||
| core.setFailed(`${username} is not a member of NVIDIA Speech Team`); | ||
| } |
There was a problem hiding this comment.
oh great. you do have this check.
| github.event_name == 'pull_request' && | ||
| github.event.action == 'labeled' && | ||
| github.event.label.name == 'Agent Plan Approved' && | ||
| contains(github.event.pull_request.labels.*.name, 'Has Babysitter') |
There was a problem hiding this comment.
I think anyone can add a label? So this would get triggered if someone adds Has Babysitter and then Agent Plan Approved. I guess someone manually adding the label is also approving? And in theory nothing happens if there's no ` comment?
There was a problem hiding this comment.
Hardened against this
| context.payload.comment?.user?.login || | ||
| context.payload.review?.user?.login; | ||
| try { | ||
| const res = await github.rest.teams.getMembershipForUserInOrg({ |
There was a problem hiding this comment.
I feel this should probably happen as a prequisite for all the jobs in the workflow?
Addresses Charlie's review on #15613. Adds per-job preflights that verify the acting user (label sender / approval commenter / execute-fix sender) is an active NVIDIA-NeMo/speech_team member, and fork-guards every PR-scoped job so the babysitter never runs on forks. Also blocks manual-label bypass by requiring a bot-authored plan comment before execute-fix proceeds, and drops the cron+reaction approval path in favor of natural-language replies. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@chtruong814 addressed review, see 318c08b |
|
Bypassing CI - the PR doesn't touch any code |
Important
The
Update branchbutton must only be pressed in very rare occassions.An outdated branch is never blocking the merge of a PR.
Please reach out to the automation team before pressing that button.
What does this PR do ?
/verify,/babysit-prand/fix-issue- intended for local development use with a harness if that's preferable to triggering it through GH actionsPR babysitter GH action logic:
When there is a check failure, and
Has Babysitterlabel is set, the agent will debug and post a comment with fix plan, pinging the PR author. The author can respond in a comment or react with a 👍🏻 emoji on the agent's comment. The agent determines the response and if approved, implements the plan. At any time this can be disabled by removingHas Babysitterlabel. The agent usesAgent Plan Pending ApprovalandAgent Plan Approvedlabels on the to manage state, and cleans them up after PR is merged.Collection: [Note which collection this PR will affect]
Changelog
Usage
# Add a code snippet demonstrating how to use thisGitHub Actions CI
The Jenkins CI system has been replaced by GitHub Actions self-hosted runners.
The GitHub Actions CI will run automatically when the "Run CICD" label is added to the PR.
To re-run CI remove and add the label again.
To run CI on an untrusted fork, a NeMo user with write access must first click "Approve and run".
Before your PR is "Ready for review"
Pre checks:
PR Type:
If you haven't finished some of the above items you can still open "Draft" PR.
Who can review?
Anyone in the community is free to review the PR once the checks have passed.
Contributor guidelines contains specific people who can review PRs to various areas.
Additional Information