Merged
Conversation
Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com>
Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com>
Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com>
merge changes from main
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds two new GitHub Actions workflows: Changes
Sequence Diagram(s)sequenceDiagram
participant GH as "GitHub (PR event)"
participant Actions as "GitHub Actions Runner"
participant GHCLI as "gh CLI"
participant Node as "Node.js / npm"
participant Docker as "Docker Engine"
participant Tests as "Test scripts"
GH->>Actions: pull_request (opened/reopened/synchronize)
Actions->>GHCLI: run pr-limit check (pull_request_target workflow)
alt author has >10 open PRs
GHCLI->>GH: post comment & close PR
GHCLI-->>Actions: exit non-zero
else author within limit
Actions->>Node: setup Node 22, install deps, build plugin
Node->>Tests: run root unit tests
Node->>Tests: run nemoclaw unit tests
Actions->>Docker: build sandbox image (test/Dockerfile.sandbox)
Docker->>Tests: run sandbox E2E tests
Tests-->>Actions: report results
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/pr.yaml (1)
27-31: Consider using environment variables for GitHub context values.Direct interpolation of
${{ ... }}expressions into shell scripts can be a script injection vector. While GitHub restricts usernames to safe characters, using environment variables is the safer pattern for shell script inputs.♻️ Suggested safer approach
- name: Check open PR count for author env: GH_TOKEN: ${{ github.token }} + AUTHOR: ${{ github.event.pull_request.user.login }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} run: | - AUTHOR="${{ github.event.pull_request.user.login }}" - PR_NUMBER="${{ github.event.pull_request.number }}" - REPO="${{ github.repository }}" - OPEN_COUNT=$(gh pr list --repo "$REPO" --author "$AUTHOR" --state open --json number --jq 'length')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr.yaml around lines 27 - 31, The workflow currently injects GitHub context expressions directly into the shell as AUTHOR, PR_NUMBER, and REPO which can be an injection vector; instead define those values as step or job environment variables (e.g., map github.event.pull_request.user.login, github.event.pull_request.number, and github.repository into env: AUTHOR, PR_NUMBER, REPO or use built‑in vars like GITHUB_ACTOR) and then reference the safe env vars ($AUTHOR, $PR_NUMBER, $REPO) inside the run block so the shell sees only environment values rather than raw `${{ ... }}` expressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/pr.yaml:
- Around line 27-31: The workflow currently injects GitHub context expressions
directly into the shell as AUTHOR, PR_NUMBER, and REPO which can be an injection
vector; instead define those values as step or job environment variables (e.g.,
map github.event.pull_request.user.login, github.event.pull_request.number, and
github.repository into env: AUTHOR, PR_NUMBER, REPO or use built‑in vars like
GITHUB_ACTOR) and then reference the safe env vars ($AUTHOR, $PR_NUMBER, $REPO)
inside the run block so the shell sees only environment values rather than raw
`${{ ... }}` expressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bd4de385-0c93-466a-b93d-49b9db95eeba
📒 Files selected for processing (1)
.github/workflows/pr.yaml
Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr.yaml:
- Around line 10-13: Workflow-level permission pull-requests: write is overly
broad; remove or change the workflow-level pull-requests permission to read
(keep contents: read) and add a job-level permissions block to the single job
that needs PR mutation (the check-pr-limit job) granting pull-requests: write so
only that job gets write access; update both places called out in the review
(the top-level permissions block and the duplicate block at lines 19-23) so no
other jobs inherit write scope.
- Line 20: The workflow currently restricts the check to only the 'opened'
action; update the job/step with the name "check-pr-limit" to run for all
relevant PR events by replacing the single-action guard with a pull_request
check that includes opened, synchronize, and reopened (for example: if:
github.event_name == 'pull_request' && (github.event.action == 'opened' ||
github.event.action == 'synchronize' || github.event.action == 'reopened')) so
the PR-limit policy runs on updates as well.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 303adc3a-eb00-47eb-9f81-c4fd3631357c
📒 Files selected for processing (1)
.github/workflows/pr.yaml
Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com>
Member
jacobtomlinson
left a comment
jacobtomlinson
left a comment
There was a problem hiding this comment.
Awesome! Can you split the PR closer into a separate workflow file?
Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com>
Contributor
Author
@jacobtomlinson Done! Makes it easier for permission scoping too. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-limit.yaml:
- Around line 6-8: The workflow trigger uses "pull_request" which provides a
read-only GITHUB_TOKEN for forked PRs; change the trigger to
"pull_request_target" so actions that mutate PRs (the comment/close steps
referenced in the job that runs after the on: block) run with the repository's
write-scoped GITHUB_TOKEN; update any steps that checkout code (replace uses of
actions/checkout with ref handling to avoid executing untrusted code from the PR
head) and ensure permissions for contents: write and pull-requests: write are
set for the job that performs the comment/close operations (refer to the
workflow trigger and the job that creates PR comments/close actions).
In @.github/workflows/pr.yaml:
- Around line 17-58: The PR workflow starts tests immediately (jobs test-unit
and test-e2e-sandbox) but cannot depend on check-pr-limit because needs: only
works within the same workflow; move the limit check into this workflow or
convert the existing pr-limit.yaml into a reusable workflow and call it from
this workflow so tests can depend on it. Specifically, add a job named
check-pr-limit inside this file (or call the reusable workflow) and then update
test-unit and test-e2e-sandbox to declare needs: [check-pr-limit] so they wait
for the limit check to complete before running.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6668483a-dea3-4046-a0f9-ddda7fd28ac6
📒 Files selected for processing (2)
.github/workflows/pr-limit.yaml.github/workflows/pr.yaml
Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com>
ericksoa
approved these changes
Mar 18, 2026
Contributor
ericksoa
left a comment
ericksoa
left a comment
There was a problem hiding this comment.
LGTM — unit tests, sandbox E2E, and PR rate limit all look good.
Ryuketsukami
pushed a commit
to Ryuketsukami/NemoClaw
that referenced
this pull request
Mar 24, 2026
* add PR CI workflows Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com> * changed npm to install Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com> * merged both PR checks into one Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com> * moved variables from run to env Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com> * scoping read permissions Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com> * split pr closer into a separate file Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com> * changed action to pull_request_target Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com> --------- Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com>
jessesanford
pushed a commit
to jessesanford/NemoClaw
that referenced
this pull request
Mar 24, 2026
* add PR CI workflows Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com> * changed npm to install Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com> * merged both PR checks into one Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com> * moved variables from run to env Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com> * scoping read permissions Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com> * split pr closer into a separate file Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com> * changed action to pull_request_target Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com> --------- Signed-off-by: Jaya Venkatesh <jjayabaskar@nvidia.com>
mafueee
pushed a commit
to mafueee/NemoClaw
that referenced
this pull request
Mar 28, 2026
…dy (NVIDIA#261) * fix(proxy): stream inference responses instead of buffering entire body The inference.local proxy path called response.bytes().await which buffered the entire upstream response before sending anything to the client. For streaming SSE responses this inflated TTFB from sub-second to the full generation time, causing clients with TTFB timeouts to abort. Add a streaming proxy variant that returns response headers immediately and forwards body chunks incrementally using HTTP chunked transfer encoding. Non-streaming responses and mock routes continue to work through the existing buffered path. Closes NVIDIA#260 * docs: update architecture docs and example for inference streaming * test(example): add direct NVIDIA endpoint tests via L7 TLS intercept Expand inference example to 4 test cases: inference.local and direct endpoint, each streaming and non-streaming. The direct path exercises the L7 REST relay (relay_chunked) to verify it already streams correctly. NVIDIA_API_KEY is picked up from the sandbox env when started with --provider nvidia.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit