fix(onboard): normalize blueprint build permissions#3664
Merged
Conversation
Signed-off-by: Revant <revant.h.patel@gmail.com>
Contributor
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughNormalizes read/execute permissions for the ChangesBlueprint permission normalization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
cv
reviewed
May 17, 2026
Signed-off-by: Revant <revant.h.patel@gmail.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/sandbox-build-context.test.ts (1)
49-140: ⚡ Quick winAdd a legacy-staging regression alongside the optimized one.
This covers
stageOptimizedSandboxBuildContext(), but the same permission-normalization hook was also added tostageLegacySandboxBuildContext(). Right now a future refactor could drop the legacy call without tripping these tests, so it would be worth adding a small counterpart that stages the same restrictive manifest tree through the legacy path and asserts the copied modes there too.🤖 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/sandbox-build-context.test.ts` around lines 49 - 140, The test only exercises stageOptimizedSandboxBuildContext but the same permission-normalization exists in stageLegacySandboxBuildContext; add a sibling test (or extend this test) that calls stageLegacySandboxBuildContext(sourceRoot, tmpDir) to stage the same restrictive blueprint tree and assert that staged manifest directory and files (the equivalents of stagedManifestDir, stagedManifest, stagedPlugin) have modes "755" and "644" respectively; reuse the same fixture setup and cleanup, and mirror the expect assertions so the legacy path is covered.
🤖 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/sandbox-build-context.test.ts`:
- Around line 49-140: The test only exercises stageOptimizedSandboxBuildContext
but the same permission-normalization exists in stageLegacySandboxBuildContext;
add a sibling test (or extend this test) that calls
stageLegacySandboxBuildContext(sourceRoot, tmpDir) to stage the same restrictive
blueprint tree and assert that staged manifest directory and files (the
equivalents of stagedManifestDir, stagedManifest, stagedPlugin) have modes "755"
and "644" respectively; reuse the same fixture setup and cleanup, and mirror the
expect assertions so the legacy path is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: df44bc51-2d8c-4ecf-9ad6-052d87151b98
📒 Files selected for processing (2)
src/lib/sandbox/build-context.tstest/sandbox-build-context.test.ts
Signed-off-by: Revant <revant.h.patel@gmail.com>
cv
approved these changes
May 17, 2026
12 tasks
ericksoa
pushed a commit
that referenced
this pull request
May 18, 2026
## Summary Updates the NemoClaw documentation for the v0.0.45 release by summarizing the user-facing changes merged since v0.0.44 and bumping the docs version metadata. Refreshes generated user skills so agent-facing references match the source docs. ## Changes - Added v0.0.45 release notes covering onboarding recovery, local inference, channel cleanup, share mount diagnostics, uninstall cleanup, and security redaction updates. - Updated command and troubleshooting docs for sandbox name limits, GPU gateway reuse, DNS preflight behavior, channel removal cleanup, and share mount path validation. - Bumped docs version metadata to 0.0.45 and regenerated NemoClaw user skills from the docs. - Source summary: #3672 -> `docs/reference/commands.md`: documented channel removal detaching bridge providers and un-applying channel policy presets. - Source summary: #3678 -> `docs/about/release-notes.md`: documented Ollama streamed usage accounting in the release notes. - Source summary: #3670 -> `docs/reference/commands.md`, `docs/reference/troubleshooting.md`: documented safe GPU gateway replacement behavior. - Source summary: #3664 -> `docs/about/release-notes.md`: documented blueprint permission normalization in the release notes. - Source summary: #3181 -> `docs/reference/troubleshooting.md`: documented GPU toolkit guidance when host drivers work but passthrough is disabled. - Source summary: #3554 -> `docs/about/release-notes.md`: documented host `openshell-gateway` cleanup during uninstall. - Source summary: #3651 -> `docs/reference/troubleshooting.md`: documented the uncached `.invalid` DNS preflight probe. - Source summary: #3643 -> `docs/reference/commands.md`: included existing `NEMOCLAW_PROVIDER` interactive-mode behavior in generated docs. - Source summary: #3647 -> `docs/reference/commands.md`: documented remote sandbox path verification for `share mount`. - Source summary: #3646 -> `docs/reference/commands.md`: included existing local writable mount target guidance in generated docs. - Source summary: #3642 -> `docs/inference/use-local-inference.md`, `docs/reference/commands.md`: documented managed-vLLM model override and gated-model token checks. - Source summary: #3639 -> `docs/reference/commands.md`: documented the 63-character sandbox name limit. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] 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 - [x] No secrets, API keys, or credentials committed - [x] 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) Commit hooks passed for the staged files. A standalone `npx prek run --all-files` attempt was blocked by sandbox access to `/Users/miyoungc/.cache/prek/prek.log`, so that checkbox is left unchecked. --- <!-- 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 * **Documentation** * Enhanced CLI command reference documentation with clearer guidance on onboarding, GPU passthrough, inference configuration, channel removal, and shared mounts. * Improved troubleshooting sections with better DNS resolution and GPU passthrough remediation steps. * Added documentation for overriding managed vLLM model selection. * Updated release notes for v0.0.45 reflecting infrastructure and workflow improvements. * **Version Bump** * Released v0.0.45. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3755?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 -->
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
Normalize NemoClaw blueprint permissions during sandbox build staging and inside the OpenClaw Docker image so model-specific setup manifests remain readable when config generation runs as the non-root
sandboxuser. This prevents Kimi K2.6 onboarding builds from depending on restrictive host checkout file modes.Related Issue
Fixes #3634
Changes
/opt/nemoclaw-blueprintbeforegenerate-openclaw-config.pyruns.model-specific-setupmanifest modes and the Dockerfile permission-normalization block.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Passed locally:
npm run build:clinpx vitest run test/sandbox-build-context.test.ts test/sandbox-provisioning.test.tsnpm run typecheck:cligit diff --checknpm testfailed in unrelated existing tests: twotest/generate-openclaw-config.test.tsnumeric env-var assertions receiveInfinityinstead of falling back to defaults, and one e2e helper timeout appeared in the full root run. The focused regression tests for this fix passed in both targeted runs and the larger test runs.npx prek run --all-filesand the push hook failed on the same unrelated CLI test failure intest/generate-openclaw-config.test.ts; lint, schema, security, docs, plugin tests, source-shape, skills YAML, and TypeScript checks passed. The commit and push used hook bypasses only after those unrelated failures blocked publishing.Signed-off-by: Revant revant.h.patel@gmail.com
Summary by CodeRabbit
Bug Fixes
Tests