docs(manage-sandboxes): refactor sandbox management guides - #6789
Conversation
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
|
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:
📝 WalkthroughWalkthroughThis PR reorganizes NemoClaw documentation navigation, adds sandbox lifecycle, state, messaging, and managed MCP guides, refreshes cross-references and redirects, and strengthens published-route and ChangesDocumentation overhaul
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
|
🌿 Preview your docs: https://nvidia-preview-pr-6789.docs.buildwithfern.com/nemoclaw |
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron is a non-blocking second opinion. Its prose, findings, and E2E guidance do not change the primary assessment above and remain in workflow artifacts only. E2E guidanceAdvisory only: coverage and selector recommendations are non-authoritative. E2E / PR Gate independently computes and dispatches trusted jobs without consuming this output. Recommended coverage:
4 optional coverage items · 4 optional selectors · 0 new-test recommendations
1 warning · 0 optional suggestionsWarningsThese merit maintainer attention but do not block by themselves.
|
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
scripts/sync-agent-variant-docs.ts (1)
97-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winError messages omit file context, only give body-relative line numbers.
index + 1is relative tobodyafter frontmatter and theAgentOnlyimport line have already been stripped, so it won't match the actual line number in the source.mdxfile. Combined with the lack ofsourcePathin the thrown message, locating the offending block across the many doc files this script processes (see downstreamrenderAgentVariantPage(source, variant, { sourcePath })callers) will require manual guesswork.♻️ Suggested fix: include source path context at the call site
const { frontmatter, body } = splitFrontmatter(source); const commandsReference = isCommandsReferenceSource(options.sourcePath); const renderedFrontmatter = renderFrontmatter(frontmatter, variant, commandsReference); - let renderedBody = stripAgentOnlyBlocksForVariant( - body.replace(/^import \{ AgentOnly \} from "\.\.\/_components\/AgentGuide";\n\n?/m, ""), - variant, - ); + let renderedBody; + try { + renderedBody = stripAgentOnlyBlocksForVariant( + body.replace(/^import \{ AgentOnly \} from "\.\.\/_components\/AgentGuide";\n\n?/m, ""), + variant, + ); + } catch (error) { + const location = options.sourcePath ? ` (${options.sourcePath})` : ""; + throw new Error(`${(error as Error).message}${location}`); + }Also applies to: 121-129
🤖 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 `@scripts/sync-agent-variant-docs.ts` around lines 97 - 101, Update the AgentOnly validation errors in the body-processing loop and the related checks around lines 121–129 to include sourcePath and translate body-relative indices to actual source-file line numbers, accounting for stripped frontmatter and import lines. Preserve the existing error conditions while making both messages identify the source file and correct line.test/agent-variant-docs.test.ts (1)
101-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood coverage for nested blocks; consider covering the other two new throw paths too.
This test confirms the nested-block rejection, but
stripAgentOnlyBlocksForVariantalso now throws on an unexpected</AgentOnly>closing tag and on an unclosed block at end-of-input (scripts/sync-agent-variant-docs.tslines 121-129). Adding sibling tests for those would lock in the new strict-validation behavior.🤖 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/agent-variant-docs.test.ts` around lines 101 - 115, Extend the AgentOnly validation tests alongside the existing nested-block case to cover both additional throw paths in stripAgentOnlyBlocksForVariant: an unexpected closing </AgentOnly> and an AgentOnly block that remains unclosed at end-of-input. Assert renderAgentVariantPage throws the corresponding validation errors while preserving the existing nested-block test.docs/deployment/set-up-mcp-bridge.mdx (1)
4-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFilename no longer matches page topic.
The frontmatter title/description now describe "About Managed MCP Servers" rather than an MCP bridge setup guide, but the file is still named
set-up-mcp-bridge.mdx. Consider renaming the file to match its new topic for maintainability, if not already planned elsewhere in the stack.🤖 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 `@docs/deployment/set-up-mcp-bridge.mdx` around lines 4 - 6, Rename the documentation file currently named set-up-mcp-bridge.mdx to match its “About Managed MCP Servers” topic, and update any references, navigation entries, or links that depend on the old filename so they point to the renamed page.docs/reference/enterprise-readiness.mdx (1)
84-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLink text out of sync with the renamed target page.
The destination was updated to
review-sandbox-hardening, but the link text still reads "Sandbox Hardening" whiledocs/about/how-it-works.mdxuses "Review Sandbox Hardening" for the same target.✏️ Suggested fix
-For container-level hardening beyond the entrypoint defaults, refer to [Sandbox Hardening](../manage-sandboxes/configure-sandboxes/review-sandbox-hardening). +For container-level hardening beyond the entrypoint defaults, refer to [Review Sandbox Hardening](../manage-sandboxes/configure-sandboxes/review-sandbox-hardening).🤖 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 `@docs/reference/enterprise-readiness.mdx` around lines 84 - 86, Update the link text in the AgentOnly enterprise-readiness references to “Review Sandbox Hardening” while preserving its existing destination and surrounding deployment links.docs/manage-sandboxes/backup-restore.mdx (1)
155-171: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDocument the SSH-unreachable / strict-backup env vars.
backupAll()insrc/lib/actions/maintenance.tsalso handles running-but-SSH-unreachable sandboxes viaNEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUPand gates strict pre-upgrade backups viaNEMOCLAW_REQUIRE_ALL_SANDBOX_BACKUPS(exiting non-zero on any skip/fail), each with distinct remediation messaging. None of this is covered here, so a user hitting the unreachable-sandbox path during a pre-upgradebackup-all(seeupdate-sandboxes.mdx's "requires a fresh backup of every registered sandbox") has no doc guidance for that specific failure mode.📝 Proposed addition
When a backup fails, NemoClaw identifies the affected state item and reports `permission denied`, `tar read error`, or `absent after extraction` when available. Use `$$nemoclaw <name> snapshot list` and `$$nemoclaw <name> snapshot restore` to inspect or restore one sandbox's bundles later. + +If a running sandbox's in-sandbox SSH endpoint does not answer, `backup-all` reports it as unreachable and fails by default. +Set `NEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP=1` to skip it and accept losing any uncommitted state since the last successful backup, or restore its gateway health and rerun. +`NEMOCLAW_REQUIRE_ALL_SANDBOX_BACKUPS=1` makes `backup-all` exit non-zero if any sandbox is skipped or fails, for callers that must not proceed without a complete backup set.🤖 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 `@docs/manage-sandboxes/backup-restore.mdx` around lines 155 - 171, Update the “Back Up Every Registered Sandbox” documentation to describe NEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP for running but SSH-unreachable sandboxes, including its remediation guidance, and NEMOCLAW_REQUIRE_ALL_SANDBOX_BACKUPS for strict pre-upgrade backups that exit non-zero on any skipped or failed backup. Explain their distinct behaviors and reference the required fresh-backup pre-upgrade workflow.
🤖 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.
Inline comments:
In `@docs/about/release-notes.mdx`:
- Line 77: Update the About Managed MCP Servers link in the release-notes entry
to use the same variant-relative path pattern as the surrounding entries,
replacing the hard-coded OpenClaw route while leaving the NemoClaw Quickstart
with Hermes link unchanged.
---
Nitpick comments:
In `@docs/deployment/set-up-mcp-bridge.mdx`:
- Around line 4-6: Rename the documentation file currently named
set-up-mcp-bridge.mdx to match its “About Managed MCP Servers” topic, and update
any references, navigation entries, or links that depend on the old filename so
they point to the renamed page.
In `@docs/manage-sandboxes/backup-restore.mdx`:
- Around line 155-171: Update the “Back Up Every Registered Sandbox”
documentation to describe NEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP for running
but SSH-unreachable sandboxes, including its remediation guidance, and
NEMOCLAW_REQUIRE_ALL_SANDBOX_BACKUPS for strict pre-upgrade backups that exit
non-zero on any skipped or failed backup. Explain their distinct behaviors and
reference the required fresh-backup pre-upgrade workflow.
In `@docs/reference/enterprise-readiness.mdx`:
- Around line 84-86: Update the link text in the AgentOnly enterprise-readiness
references to “Review Sandbox Hardening” while preserving its existing
destination and surrounding deployment links.
In `@scripts/sync-agent-variant-docs.ts`:
- Around line 97-101: Update the AgentOnly validation errors in the
body-processing loop and the related checks around lines 121–129 to include
sourcePath and translate body-relative indices to actual source-file line
numbers, accounting for stripped frontmatter and import lines. Preserve the
existing error conditions while making both messages identify the source file
and correct line.
In `@test/agent-variant-docs.test.ts`:
- Around line 101-115: Extend the AgentOnly validation tests alongside the
existing nested-block case to cover both additional throw paths in
stripAgentOnlyBlocksForVariant: an unexpected closing </AgentOnly> and an
AgentOnly block that remains unclosed at end-of-input. Assert
renderAgentVariantPage throws the corresponding validation errors while
preserving the existing nested-block test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 68ee2764-be7e-47ba-92ff-fb982faeba55
📒 Files selected for processing (57)
.agents/skills/nemoclaw-maintainer-refactor-docs/SKILL.mdREADME.mddocs/about/how-it-works.mdxdocs/about/release-notes.mdxdocs/configure-agents/progressive-tool-disclosure.mdxdocs/deployment/brev-web-ui.mdxdocs/deployment/deploy-to-remote-gpu.mdxdocs/deployment/install-openclaw-plugins.mdxdocs/deployment/sandbox-hardening.mdxdocs/deployment/set-up-mcp-bridge.mdxdocs/get-started/quickstart-hermes.mdxdocs/get-started/quickstart-langchain-deepagents-code.mdxdocs/get-started/quickstart.mdxdocs/index.ymldocs/inference/declarative-agents-manifest.mdxdocs/inference/set-up-sub-agent.mdxdocs/manage-sandboxes/add-channels-after-onboarding.mdxdocs/manage-sandboxes/add-mcp-server.mdxdocs/manage-sandboxes/backup-restore.mdxdocs/manage-sandboxes/enable-channels-during-onboarding.mdxdocs/manage-sandboxes/gateway-lifecycle-control.mdxdocs/manage-sandboxes/install-plugins-hermes.mdxdocs/manage-sandboxes/lifecycle.mdxdocs/manage-sandboxes/manage-mcp-servers.mdxdocs/manage-sandboxes/manage-messaging-channels.mdxdocs/manage-sandboxes/messaging-channels.mdxdocs/manage-sandboxes/recover-rebuild-sandboxes.mdxdocs/manage-sandboxes/run-sandboxes.mdxdocs/manage-sandboxes/runtime-controls.mdxdocs/manage-sandboxes/set-up-discord.mdxdocs/manage-sandboxes/set-up-microsoft-teams.mdxdocs/manage-sandboxes/set-up-slack.mdxdocs/manage-sandboxes/set-up-telegram.mdxdocs/manage-sandboxes/set-up-wechat.mdxdocs/manage-sandboxes/set-up-whatsapp.mdxdocs/manage-sandboxes/transfer-state-manually.mdxdocs/manage-sandboxes/uninstall-nemoclaw.mdxdocs/manage-sandboxes/update-sandboxes.mdxdocs/manage-sandboxes/workspace-files.mdxdocs/network-policy/integration-policy-examples.mdxdocs/reference/cli-selection-guide.mdxdocs/reference/commands.mdxdocs/reference/enterprise-readiness.mdxdocs/reference/host-files-and-state.mdxdocs/reference/network-policies.mdxdocs/reference/troubleshoot-mcp-servers.mdxdocs/reference/troubleshooting.mdxdocs/security/best-practices.mdxdocs/security/credential-rotation.mdxdocs/security/credential-storage.mdxdocs/security/openshell-0.0.72-compatibility-review.mdxfern/docs.ymlscripts/check-docs-published-routes.tsscripts/sync-agent-variant-docs.tstest/agent-variant-docs.test.tstest/check-docs-links.test.tstest/check-docs-published-routes.test.ts
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Summary
Refactors Manage Sandboxes from oversized pages and a nested Extend Sandboxes branch into focused lifecycle, messaging, MCP server, plugin, backup, and recovery pages.
Adds journey-based variant navigation, direct redirects, canonical troubleshooting ownership, concise prose, and route checks that preserve published paths.
Changes
Type of Change
Quality Gates
Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — not run; focused route tests and the docs build cover this documentation refactornpm run docsbuilds without warnings (doc changes only) — build passed with 0 errors and 1 Fern warningSigned-off-by: Miyoung Choi miyoungc@nvidia.com
Summary by CodeRabbit