fix(onboard): avoid literal proc wildcard in GPU sandbox policy - #3436
Conversation
Signed-off-by: zyang-dev <267119621+zyang-dev@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughGPU policy generation for direct GPU sandboxes now filters ChangesGPU Policy /proc Filtering and Proof Commands
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
ericksoa
left a comment
There was a problem hiding this comment.
Approving the narrow GPU procfs policy/proof fix. The current WSL failure is in unrelated Ollama auth-proxy selection tests; this PR is limited to direct-GPU policy generation and proof command shape.
## Summary `NEMOCLAW_SANDBOX_READY_TIMEOUT` has been a recognised env var since #2849, but no documentation accompanied it — `docs/reference/commands.md`, `docs/reference/troubleshooting.md`, and the inference / deployment guides only mention the companion `NEMOCLAW_LOCAL_INFERENCE_TIMEOUT` (added in #1620 and documented at that time). Operators hitting `Sandbox '<name>' was created but did not become ready within 180s` have no doc-grep path to the workaround, and the two timeouts are easy to conflate. This closes the documentation gap left by #2849. Originally tried under #3435; closed because that PR mis-framed the docs as resolving #3344 / #3416 (the root cause of both was the GPU policy bug fixed in #3436, not a timeout misconfiguration). The docs themselves still have value as a follow-up to the env-var introductions, so reopening as a new PR with the correct framing. ## Related Issue <!-- Not closing any issue; this addresses the doc-gap surfaced while investigating #3344 and #3416 (both already fixed in code by #3436). --> ## Changes - `docs/reference/commands.md`: add `NEMOCLAW_SANDBOX_READY_TIMEOUT` and `NEMOCLAW_LOCAL_INFERENCE_TIMEOUT` to the Onboard Timeouts table. - `docs/reference/troubleshooting.md`: new troubleshooting entry "Sandbox onboard times out with 'did not become ready within Ns'" that distinguishes the readiness wait from the inference-probe budget, with a worked example. - `docs/inference/use-local-inference.md`: cross-link the two timeouts from the existing `NEMOCLAW_LOCAL_INFERENCE_TIMEOUT` section so readers of either knob land on the other. - `docs/deployment/deploy-to-remote-gpu.md`: new "First-Run Readiness Budget" section calling out DGX Station / cloud-VM / large-quantised-model conditions that exceed the default and showing how to raise it. No code changes — the readiness behaviour is unchanged. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [ ] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [x] `make docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a “First-Run Readiness Budget” note for remote GPU hosts explaining longer initial sandbox build/upload times and advice to increase NEMOCLAW_SANDBOX_READY_TIMEOUT. * Clarified that NEMOCLAW_LOCAL_INFERENCE_TIMEOUT applies to inference-server validation while sandbox readiness uses NEMOCLAW_SANDBOX_READY_TIMEOUT (default 180s). * Expanded examples for exporting both timeouts and onboarding timeout messaging. * Added troubleshooting guidance and inspection steps when sandbox readiness timeouts delete partial sandboxes. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3440) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Summary
Fix direct-GPU sandbox onboarding by removing NemoClaw’s create-time procfs entries that OpenShell should own at runtime. This avoids the literal
/proc/self/task/*/commpolicy path crash and makes GPU verification compatible with newer OpenShell command validation.Related Issue
Fixes #3416
Changes
/procand/proc/self/task/*/commfrom direct-GPU create policies so OpenShell can apply GPU procfs enrichment./proc/<pid>/task/<tid>/commentry instead of a short-lived child process.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: zyang-dev 267119621+zyang-dev@users.noreply.github.com
Summary by CodeRabbit
Improvements
/procfilesystem entries based on actual usage patterns rather than static paths.Tests
/procentry removal from policies.