feat(langgraph): add OpenShell sandbox integration for react_agent#269
feat(langgraph): add OpenShell sandbox integration for react_agent#269rrbanda wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds an OpenShell BYOC container definition for the LangGraph ReAct agent and documentation covering image creation, sandbox startup, environment injection, endpoint verification, network policy, and cleanup. ChangesOpenShell ReAct agent deployment
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Podman
participant OpenShell
participant Uvicorn
participant LLM_API
Podman->>OpenShell: Push BYOC container image
OpenShell->>Uvicorn: Create sandbox and start uvicorn
OpenShell->>Uvicorn: Forward port and inject environment
Uvicorn->>LLM_API: Send chat completion request
LLM_API-->>Uvicorn: Return model response
Uvicorn-->>OpenShell: Return health or chat response
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
cedec00 to
de369f7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
agents/langgraph/templates/react_agent/OPENSHELL.md (1)
65-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake the policy creation copy-pasteable.
The snippet shows the YAML content and then a command that reads
policy.yaml, but doesn't explicitly instruct the user to save the file. Consolidating this into a singlecat <<EOFblock ensures it can be executed seamlessly in one go.♻️ Proposed fix
-The ReAct agent only needs outbound access to the configured LLM endpoint. Apply a restrictive policy: - -```yaml -sandbox: - network: - egress: - - host: "vllm-svc.my-ns.svc.cluster.local" - port: 8000 - methods: ["POST"] - paths: - - "/v1/chat/completions" - - "/v1/completions" -``` - -```bash +The ReAct agent only needs outbound access to the configured LLM endpoint. Apply a restrictive policy by saving it and applying it: + +```bash +cat <<EOF > policy.yaml +sandbox: + network: + egress: + - host: "vllm-svc.my-ns.svc.cluster.local" + port: 8000 + methods: ["POST"] + paths: + - "/v1/chat/completions" + - "/v1/completions" +EOF + openshell policy set langgraph-agent --policy policy.yaml --wait</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@agents/langgraph/templates/react_agent/OPENSHELL.mdaround lines 65 - 81,
Make the policy setup in the ReAct agent documentation copy-pasteable by
replacing the separate YAML and shell blocks with one shell block that writes
the policy via a heredoc to policy.yaml, then runs openshell policy set using
that file. Preserve the existing policy contents and command.</details> <!-- cr-comment:v1:553933c9a249f695f615b170 --> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@agents/langgraph/templates/react_agent/Containerfile.openshell:
- Around line 32-51: Switch to USER 1001 before both uv pip install commands in
the Containerfile, including the auth installation and the subsequent
tracing-extra installation, so all installed files are owned by the runtime
user; keep the dependency-copy and pyproject preparation steps ordered so the
installs still resolve correctly.In
@agents/langgraph/templates/react_agent/OPENSHELL.md:
- Around line 21-25: Remove the EXIT trap from the interactive shell snippet in
the OpenShell instructions and add an explicit cleanup command after the podman
build and push commands, preserving the removal of ./images and ./components.
Nitpick comments:
In@agents/langgraph/templates/react_agent/OPENSHELL.md:
- Around line 65-81: Make the policy setup in the ReAct agent documentation
copy-pasteable by replacing the separate YAML and shell blocks with one shell
block that writes the policy via a heredoc to policy.yaml, then runs openshell
policy set using that file. Preserve the existing policy contents and command.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Enterprise **Run ID**: `404d7d1b-7c21-452b-8141-d058c1880f57` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 6b6bbf92ac79b077fc09c35d12707904cbb99083 and de369f744d6f4179e88c115b5e8269e2f8ef6be0. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `agents/langgraph/templates/react_agent/Containerfile.openshell` * `agents/langgraph/templates/react_agent/OPENSHELL.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
Review Findings: • Medium: agents/langgraph/templates/react_agent/OPENSHELL.md misdescribes -e as provider-backed secret injection. The new guide says -e "injects environment variables via OpenShell providers (never • Low: agents/langgraph/templates/react_agent/Containerfile.openshell still carries the old trap 'rm -rf ./images ./components' EXIT build snippet in its header comment, even though OPENSHELL.md was |
bd3f453 to
bbcf2ee
Compare
|
@rrbanda please take a look at my previous comment at your convenience. |
Add Containerfile.openshell and deployment documentation for running
the LangGraph ReAct agent inside an NVIDIA OpenShell sandbox with
policy-enforced network isolation and filesystem constraints.
Follows the official BYOC pattern from NVIDIA/OpenShell. Handles the
components/auth path dependency by installing it separately before the
main package (avoids relative path resolution issues from /sandbox).
Tested on OpenShift 4.18 cluster:
- Image builds successfully via oc start-build (134 packages)
- /health returns {"status":"healthy","agent_initialized":true}
- /images/rh_logo.svg returns 200 (playground assets served)
- /chat/completions works end-to-end with gemini-2.5-flash via
Llama Stack endpoint
- Move USER 1001 before pip install so packages are owned by the runtime user (avoids __pycache__ permission issues) - Replace trap EXIT with explicit rm -rf in build docs (trap doesn't fire in interactive shells until terminal closes)
bbcf2ee to
7fb56fa
Compare
2 participants
TL;DR
Adds OpenShell support for the LangGraph ReAct template with one new BYOC container definition and one runbook. This does not change the existing Helm/OpenShift deployment path for
react_agent.Reviewer Guide
Review in this order:
agents/langgraph/templates/react_agent/Containerfile.openshellagents/langgraph/templates/react_agent/OPENSHELL.mdThe diff is intentionally small: one file defines the image/runtime adaptation, and the second documents build, launch, policy, and verification steps.
Summary
Containerfile.openshellfor running the LangGraph ReAct agent inside an NVIDIA OpenShell sandboxOPENSHELL.mdwith build/run instructions, network policy configuration, and verification stepsMotivation
None of the LangGraph templates currently include an OpenShell BYOC path. Adding it to
react_agentprovides the first example on the most mature LangGraph template.Key Design Decisions
Dockerfileiprouteandnftablesfor OpenShell network namespace support and optional bypass detectioncomponents/authbefore the main package so the existingtool.uv.sourcespath does not fail from/sandbox1001so installed files are owned by the runtime userReviewer Focus
Containerfile.openshell: verify the/sandboxlayout, dependency install order, and non-root runtime setupOPENSHELL.md: verify the build/run flow and policy example against current OpenShell CLI behavior-eenvironment injection inopenshell sandbox create; if we want provider-backed secret handling, that should be documented explicitly withopenshell provider createand--providerrather than implied by the current wordingTest Plan
Validated manually on OpenShift 4.18 with OpenShell v0.0.80 gateway:
oc start-buildGET /healthreturns{\"status\":\"healthy\",\"agent_initialized\":true}GET /images/rh_logo.svgreturns 200POST /chat/completionsworks end-to-end withgemini/models/gemini-2.5-flashvia a Llama Stack endpointNo automated tests were added because this PR introduces an alternate container/deployment path plus documentation, and validation was performed as an in-cluster manual run.