fix(sandbox): keep Homebrew on Linuxbrew prefix#4459
Merged
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
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:
📝 WalkthroughWalkthroughReplace symlink with a ChangesHomebrew wrapper script and PATH provisioning
sequenceDiagram
participant BuildStep
participant Wrapper
participant Linuxbrew
participant ProfileD
participant Sandbox
BuildStep->>Wrapper: write wrapper script
BuildStep->>BuildStep: run brew --prefix and brew --version checks
BuildStep->>ProfileD: create runtime PATH rule file
Sandbox->>Wrapper: invoke brew (via gosu sandbox)
Wrapper->>Linuxbrew: exec linuxbrew bin/brew with args
Linuxbrew->>Sandbox: return brew output
🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels: Suggested reviewers:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-4459.docs.buildwithfern.com/nemoclaw |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Scenario Advisor RecommendationRequired scenario E2E: None Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
Contributor
PR Review AdvisorFindings: 0 needs attention, 0 worth checking, 0 nice ideas This is an automated advisory review. A human maintainer must make the final merge decision. |
Contributor
|
Actionable comments posted: 0 |
ericksoa
added
bug
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
|
Actionable comments posted: 0 |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26597813048
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/e2e/test-network-policy.sh (1)
339-345: ⚡ Quick winAssert wrapper resolution explicitly in TC-NET-11
Line 339 validates
brew --prefix, but not thatbrewresolves to/usr/local/bin/brew(the wrapper contract fromDockerfile.baseLine 233-Line 288). Add a direct path assertion to catch PATH-order regressions.Proposed diff
- response=$(sandbox_exec "bash -lc 'set -e; export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_ENV_HINTS=1; brew --prefix; brew install --quiet hello; command -v hello; hello'" 2>&1) || true + response=$(sandbox_exec "bash -lc 'set -e; export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_ENV_HINTS=1; command -v brew; brew --prefix; brew install --quiet hello; command -v hello; hello'" 2>&1) || true @@ - if echo "$response" | grep -q "/home/linuxbrew/.linuxbrew" \ + if echo "$response" | grep -q "/usr/local/bin/brew" \ + && echo "$response" | grep -q "/home/linuxbrew/.linuxbrew" \ && echo "$response" | grep -q "/home/linuxbrew/.linuxbrew/bin/hello" \ && echo "$response" | grep -q "Hello, world!"; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/test-network-policy.sh` around lines 339 - 345, The test calls sandbox_exec and captures response but never verifies that the brew executable resolves to the wrapper path; update the sandbox_exec command string (in the same invocation that runs "brew --prefix; brew install ...; command -v hello; hello") to also run "command -v brew" so the path is emitted, then add an assertion alongside the existing greps to check response contains "/usr/local/bin/brew" (i.e. include a grep for that path in the if condition that currently checks for "/home/linuxbrew/.linuxbrew", "/home/linuxbrew/.linuxbrew/bin/hello", and "Hello, world!").
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/e2e/test-network-policy.sh`:
- Around line 339-345: The test calls sandbox_exec and captures response but
never verifies that the brew executable resolves to the wrapper path; update the
sandbox_exec command string (in the same invocation that runs "brew --prefix;
brew install ...; command -v hello; hello") to also run "command -v brew" so the
path is emitted, then add an assertion alongside the existing greps to check
response contains "/usr/local/bin/brew" (i.e. include a grep for that path in
the if condition that currently checks for "/home/linuxbrew/.linuxbrew",
"/home/linuxbrew/.linuxbrew/bin/hello", and "Hello, world!").
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 22732571-17f9-41e8-85c7-54327aaca5a9
📒 Files selected for processing (2)
Dockerfile.basetest/e2e/test-network-policy.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- Dockerfile.base
Contributor
|
Actionable comments posted: 0 |
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26598137061
|
Contributor
|
Actionable comments posted: 0 |
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26598419637
|
Contributor
Selective E2E Results — ❌ Some jobs failedRun: 26598669807
|
cv
approved these changes
May 28, 2026
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26599920696
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26601603327
|
12 tasks
miyoungc
added a commit
that referenced
this pull request
May 29, 2026
## Summary Refreshes the NemoClaw documentation for the v0.0.54 release and regenerates user skills from the Fern MDX source. Also keeps the Fern CLI pin current so local docs checks use the upgraded Fern version. ## Related Issue <!-- No single related issue. This is release-prep documentation catch-up. --> ## Changes - #4403 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document Telegram, Discord, and Slack post-rebuild bridge verification and summarize channel activation fixes. - #4222 -> `docs/about/release-notes.mdx`: Include Slack generated channel enablement in the v0.0.54 messaging summary. - #4346 -> `docs/get-started/windows-preparation.mdx`, `docs/about/release-notes.mdx`: Document safer Windows bootstrap behavior for Ubuntu first-run and Docker Desktop WSL integration. - #4416 -> `docs/inference/use-local-inference.mdx`, `docs/about/release-notes.mdx`: Document the Docker Desktop WSL requirement for Windows-host Ollama. - #4442 -> `docs/about/release-notes.mdx`: Summarize the optional NemoHermes native web dashboard and related environment variables. - #4426 -> `docs/about/release-notes.mdx`: Summarize copy-paste recovery hints for invalid sandbox names and missing NVIDIA API keys. - #4459 -> `docs/about/release-notes.mdx`: Summarize the Linuxbrew prefix fix for sandbox Homebrew usage. - #4450 -> `docs/about/release-notes.mdx`: Summarize `/nemoclaw` slash command startup activation. - #4468 -> `docs/about/release-notes.mdx`: Summarize scope-upgrade approval recovery. - #4325 -> `docs/about/release-notes.mdx`: Summarize the narrowed `web_fetch` host-gateway allowance. - #4474 -> `docs/about/release-notes.mdx`: Summarize Hermes Provider smoke-check behavior for OAuth versus Nous API key setup. - Refresh generated `.agents/skills/nemoclaw-user-*` references from `docs/` and update `fern/fern.config.json` to Fern `5.41.2`. ## 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 <!-- Check each item you ran and confirmed. Leave unchecked items you skipped. Doc-only changes do not require npm test unless you ran it. --> - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `npm run 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) --- <!-- DCO sign-off required by CI. Run: git config user.name && git config user.email --> Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Optional NemoHermes native web dashboard (configurable port and TUI) * GPU memory cleanup now unloads Ollama models when switching providers or stopping services * **Bug Fixes** * Improved sandbox name validation with suggested slug recovery * Windows-host Ollama now requires Docker Desktop WSL integration and exits with remediation guidance when unsupported * **Documentation** * Clarified quickstart/onboard flow, installer TTY/non‑TTY guidance, Hermes Docker prerequisites, sandbox hardening, and channels add rebuild checks <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4539?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4539?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
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
Fixes #3757 as a follow-up to #3916. The base image now exposes
brewon the normal sandboxPATHwithout making Homebrew infer/usr/localas its prefix.Problem
#3916 correctly baked Homebrew into
/home/linuxbrew/.linuxbrew, but it exposed the entry point with a plain/usr/local/bin/brewsymlink. In a fresh sandbox that made Homebrew report/usr/localas the prefix, which broke the QA path in two ways:brew install --quiet hellotried to create locks under root-owned/usr/local/var/homebrew./usr/localprefix.After fixing the prefix, installed formula commands also need to be reachable in sandbox shell sessions without asking users to source
brew shellenvmanually.Changes
/usr/local/bin/brewsymlink with a wrapper that execs/home/linuxbrew/.linuxbrew/bin/brew, preserving the Linuxbrew prefix./usr/local/bin/brew --prefixand sandbox login-shellbrew --prefixboth return/home/linuxbrew/.linuxbrew./home/linuxbrew/.linuxbrew/binthrough/etc/profile.d/nemoclaw-linuxbrew.shonly for thesandboxuser, while deliberately avoiding a global DockerENV PATHentry for the sandbox-writable Linuxbrew prefix.brew, installhello, resolvehelloon PATH, and run it.Validation
npm run build:clinpm run typecheck:clinpx vitest run test/policies.test.tsnpm run source-shape:checkhadolint Dockerfile.basebash -n test/e2e/test-network-policy.shshellcheck test/e2e/test-network-policy.shgit diff --checkPATH:/home/linuxbrew/.linuxbrew/bin/usr/local/bin/brew --prefix->/home/linuxbrew/.linuxbrew/home/linuxbrew/.linuxbrew/binbrew install --quiet hellosucceedscommand -v hello->/home/linuxbrew/.linuxbrew/bin/hellohello->Hello, world!Local caveats
docker build -f Dockerfile.base ...could not run on this host because Docker has no working buildx/BuildKit component; the legacy builder fails on the existingRUN --mountstep before reaching this change.npx vitest run test/nemoclaw-start.test.ts test/policies.test.tspassed the related policy coverage but still hit existing local fixture failures intest/nemoclaw-start.test.tsbecausenemoclaw/node_modules/json5is absent in this worktree.test/policies.test.tspasses by itself.npx prek run --files Dockerfile.base docs/network-policy/integration-policy-examples.mdx test/policies.test.tspassed the visible file hooks including hadolint, gitleaks, docs-to-skills, and source-shape, then failed in the full CLI test hook on the same unrelatedtest/nemoclaw-start.test.tsbaseline-capture failures plus a coverage temp-file ENOENT.Summary by CodeRabbit
Chores
Documentation
Tests