fix(sandbox): add host-mediated gateway restart#5874
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:
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 (21)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (11)
📝 WalkthroughWalkthroughAdds a host-mediated ChangesGateway lifecycle and shields flow
Estimated code review effort🎯 5 (Critical) | ⏱️ ~180 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the branch is 96%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the branch is 68%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
Updated |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-5874.docs.buildwithfern.com/nemoclaw |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
![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: 4
🧹 Nitpick comments (2)
docs/manage-sandboxes/runtime-controls.mdx (1)
60-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
$$nemoclawon this shared page.These Hermes-only references hard-code
nemohermesfor a host command that also exists on the shared alias surface. Use$$nemoclawhere so the generated OpenClaw and Hermes variants render the correct command name consistently.
As per coding guidelines,Use $$nemoclaw for host CLI command examples on shared OpenClaw and Hermes pages.As per path instructions,ask the author to use $$nemoclaw instead so generated OpenClaw and Hermes docs render the right command name.Based on learnings, use a concrete alias only when the command is intentionally OpenClaw-specific.Also applies to: 70-70
🤖 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/runtime-controls.mdx` at line 60, The runtime-controls table currently hard-codes the host command as nemohermes on a shared OpenClaw/Hermes page, which should instead use the shared alias surface. Update the affected entry in the docs content that references the gateway restart command to use $$nemoclaw so both generated variants render the correct host CLI name consistently, and apply the same fix to the other referenced occurrence in this section.Sources: Coding guidelines, Path instructions, Learnings
docs/reference/commands.mdx (1)
1334-1334: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
$$nemoclawin the shared reference page.This note is on a shared docs page, and
gateway restartis not Hermes-only. Hard-codingnemohermeshere breaks the variant-friendly command style used elsewhere in this page.
As per coding guidelines,Use $$nemoclaw for host CLI command examples on shared OpenClaw and Hermes pages.As per path instructions,ask the author to use $$nemoclaw instead so generated OpenClaw and Hermes docs render the right command name.Based on learnings, concrete aliases are fine when the command is intentionally agent-specific, which is not the case here.🤖 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/commands.mdx` at line 1334, Replace the hard-coded Hermes CLI name in the shared docs example with the shared host-command placeholder used elsewhere on this page. Update the command example in the reference docs so it uses $$nemoclaw instead of nemohermes, keeping the wording variant-friendly for both OpenClaw and Hermes and preserving the existing gateway restart guidance.Sources: Coding guidelines, Path instructions, Learnings
🤖 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 `@src/lib/actions/sandbox/process-recovery.test.ts`:
- Around line 295-299: The test is reading failure-only fields from
`restartSandboxGateway` without narrowing the union first, so `result.detail` is
not type-safe after `toMatchObject`. Update the assertion in
`process-recovery.test.ts` to explicitly narrow on `result.ok` for the failure
branch before accessing `detail`, while keeping the existing
`restartSandboxGateway` and `deps.buildOpenClawGatewayRestartScript`
expectations intact.
In `@src/lib/actions/sandbox/process-recovery.ts`:
- Around line 617-624: The success log in process recovery is not honoring
quiet, so `process-recovery.ts` still prints the gateway restart message
unconditionally. Update the `processRecovery` restart path around the
`forwardRecovered` checks so the final `console.log` for “Gateway restarted;
health passed; forwards checked/recovered” only runs when `quiet` is false,
matching the existing quiet handling used for the earlier status messages.
- Around line 593-599: The wedge diagnostics path is bypassing the injected exec
behavior by passing the direct sandbox exec helper instead of the dependency
override. In the process-recovery flow that calls waitForRecoveredSandboxGateway
and printGatewayWedgeDiagnostics, make sure the diagnostics invocation uses
deps.executeSandboxExecCommand when present (falling back to the default helper
only if not injected) so callers can fully control exec behavior during
health-timeout recovery.
In `@src/lib/agent/runtime.ts`:
- Around line 282-291: In buildOpenClawGatewayRestartScript and the
gatewayRootGosuLaunchCommand flow, move the root check ahead of any
state-mutating restart steps so OpenClaw verifies root before log setup, lock
removal, or stale-process termination. Use the existing root-check logic from
gatewayRootGosuLaunchCommand and ensure the non-root path exits before
buildGatewayLogSetup, buildGatewayGuardRecoveryLines, rm -rf lock cleanup, and
buildGatewayStopLines are executed.
---
Nitpick comments:
In `@docs/manage-sandboxes/runtime-controls.mdx`:
- Line 60: The runtime-controls table currently hard-codes the host command as
nemohermes on a shared OpenClaw/Hermes page, which should instead use the shared
alias surface. Update the affected entry in the docs content that references the
gateway restart command to use $$nemoclaw so both generated variants render the
correct host CLI name consistently, and apply the same fix to the other
referenced occurrence in this section.
In `@docs/reference/commands.mdx`:
- Line 1334: Replace the hard-coded Hermes CLI name in the shared docs example
with the shared host-command placeholder used elsewhere on this page. Update the
command example in the reference docs so it uses $$nemoclaw instead of
nemohermes, keeping the wording variant-friendly for both OpenClaw and Hermes
and preserving the existing gateway restart guidance.
🪄 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: 4573e314-45ce-43a2-b097-901ac1580774
📒 Files selected for processing (17)
agents/hermes/Dockerfiledocs/manage-sandboxes/install-plugins-hermes.mdxdocs/manage-sandboxes/lifecycle.mdxdocs/manage-sandboxes/runtime-controls.mdxdocs/reference/commands-nemohermes.mdxdocs/reference/commands.mdxdocs/reference/troubleshooting.mdxsrc/commands/sandbox/gateway/restart.tssrc/commands/sandbox/oclif-command-adapters.test.tssrc/lib/actions/sandbox/process-recovery.test.tssrc/lib/actions/sandbox/process-recovery.tssrc/lib/agent/runtime.test.tssrc/lib/agent/runtime.tssrc/lib/cli/command-registry.test.tssrc/lib/cli/public-argv-translation.test.tssrc/lib/cli/public-display-defaults.tstest/hermes-doctor-config-hash.test.ts
Contributor
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
Contributor
PR Review Advisor — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
Review findings by urgency: 0 required fixes, 3 items to resolve/justify, 0 in-scope improvements
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 28253422289
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/lib/actions/sandbox/process-recovery.ts`:
- Line 401: The restart script is being built with the wrong port value:
`buildHermesGatewayRestartScript` expects the Hermes health/API port for
`_NEMOCLAW_RESTART_HEALTH_PORT`, but `process-recovery.ts` is currently passing
`dashboardPort`. Update the call site to pass the actual Hermes health port from
the recovery context instead of the dashboard port, and make sure the value used
here matches the fixture’s health probe port so
`buildHermesGatewayRestartScript` launches socat/health recovery against the
correct endpoint.
- Around line 395-405: The Hermes recovery path in recoverSandboxProcesses still
prints failure details even when callers request quiet mode, so thread the quiet
flag into this helper and suppress the printGatewayRestartFailure calls when
quiet is true. Update the existing checkAndRecoverSandboxProcesses call site to
pass { quiet }, and use the recoverSandboxProcesses and
printGatewayRestartFailure symbols to locate the affected Hermes branch.
In `@src/lib/agent/runtime.ts`:
- Around line 364-367: The restart validation in runtime.ts should fail closed
instead of falling back to resolving Hermes from PATH. Update the logic in the
validationSteps construction so that if AGENT_BIN at binaryPath is missing or
not executable, it exits with AGENT_MISSING rather than calling command -v on
binaryName. Keep the check within the restart flow that relaunches as gateway,
and ensure the code only accepts the trusted binaryPath for locating the
executable.
🪄 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: e725fd75-9d5c-4add-9d95-ab5fbe819868
📒 Files selected for processing (4)
src/lib/actions/sandbox/process-recovery.tssrc/lib/agent/runtime.test.tssrc/lib/agent/runtime.tstest/process-recovery.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/agent/runtime.test.ts
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 28254560749
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 28254817942
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 28255266980
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 28255490504
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 28257003744
|
Contributor
✅ Action performedReview finished.
|
Contributor
Vitest E2E Target Results — ✅ All selected jobs passedRun: 28461753584
|
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile (1)
1140-1145: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse cmdline proof instead of
commfor the OpenClaw gateway fallback.Line 1144 can reject a valid npm-installed OpenClaw gateway whose process
commisnode, causing Docker to restart an otherwise healthy local gateway after curl exit 7.
The PID/starttime proof already prevents PID reuse; validate/proc/$gwpid/cmdlinefor the OpenClaw gateway command instead.As per path instructions,
Dockerfile*must be treated as a security boundary and process identity must be traced across the changed flow.Proposed fix
- case "$(ps -p "$gwpid" -o comm= 2>/dev/null)" in openclaw*) ;; *) exit 1 ;; esac; \ + gwcmdline="$(tr '\0' ' ' <"/proc/$gwpid/cmdline" 2>/dev/null)" || exit 1; \ + printf '%s' "$gwcmdline" | grep -Eq '(^|[[:space:]/])(openclaw|openclaw[.]mjs)([[:space:]]+gateway[[:space:]]+run|[[:space:]]*$)|openclaw-gateway' || exit 1; \ [ -s /tmp/gateway.log ]🤖 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 `@Dockerfile` around lines 1140 - 1145, The OpenClaw gateway fallback is validating process identity with ps ... comm in the gateway check, which can misidentify npm-launched gateways whose comm is node. Update the gateway proof in the Dockerfile block that sets gwidentity/gwstate/gwcurrent to validate /proc/$gwpid/cmdline instead of comm, and keep the existing PID/starttime check as the process identity guard for the fallback flow.Source: Path instructions
🧹 Nitpick comments (2)
docs/security/openshell-0.0.71-gateway-auth-review.mdx (1)
19-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit these multi-sentence bullets to one sentence per source line.
Several new bullets pack multiple sentences onto the same MDX line, which breaks the docs source-formatting rule and makes later diffs harder to review. As per coding guidelines,
Keep one sentence per line in Markdown and MDX source files.Also applies to: 34-35, 52-59
🤖 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/security/openshell-0.0.71-gateway-auth-review.mdx` around lines 19 - 24, The MDX bullets in this review note combine multiple sentences on a single source line, violating the one-sentence-per-line formatting rule. Update the affected bullets in the security doc so each sentence is on its own line, keeping the existing content intact while reflowing the text around the OpenShell gateway auth, Docker-hosted compatibility, Hermes env-file, JWT recovery, sandbox recovery, and sessions RPC helper bullets.Source: Coding guidelines
src/lib/onboard/docker-driver-gateway-config-auth-contract.test.ts (1)
22-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid locking this test to review-note prose.
This assertion list fails on harmless wording edits without proving the gateway auth contract changed. Prefer a small set of machine-readable version markers here and leave the behavioral contract to the TOML/JWT tests below. As per path instructions,
Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions.🤖 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 `@src/lib/onboard/docker-driver-gateway-config-auth-contract.test.ts` around lines 22 - 53, The `keeps the OpenShell gateway auth source review aligned with the generated config` test is over-coupled to review-note prose and will fail on wording-only changes. Replace the long `reviewNote.toContain(...)` checklist with a minimal set of stable, machine-readable version markers in this spec, and keep the real behavioral assertions in the TOML/JWT contract tests such as `openshell_server::config_file::load()` and the gateway auth source contract tests.Source: Path instructions
🤖 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 `@agents/hermes/start.sh`:
- Around line 2434-2448: The crash-quarantine counter in
record_hermes_managed_gateway_exit is counting intentional controller-driven
restarts the same as unexpected exits, which can wrongly quarantine managed
recovery flows. Update the exit-recording path used by
supervise_hermes_gateway_current_user so host-requested restarts are excluded
from HERMES_MANAGED_GATEWAY_EXIT_TIMES, or explicitly reset/skip the budget for
those cases before the >=5 check. Make sure the restart/recovery loop that
triggers gateway restart does not call into the quarantine logic for expected
exits, while true failures still increment and can call
quarantine_hermes_managed_gateway_relaunch.
In `@src/lib/actions/sandbox/hermes-secret-boundary-recovery.ts`:
- Around line 67-69: The Hermes boundary check in
hermes-secret-boundary-recovery.ts only treats GATEWAY_PID as success, so update
the success condition in the helper that inspects result.stdout/result.stderr to
also accept ALREADY_RUNNING as a valid authenticated marker. Keep the existing
refusal behavior for unexpected output, but ensure the same recovery contract
used elsewhere is honored in this live Hermes path.
In `@src/lib/actions/sandbox/process-recovery.ts`:
- Around line 519-527: The recovery flow in waitForRecoveredSandboxGateway is
still hardcoding executeGatewaySupervisorAction inside managedProbeImpl, which
bypasses the caller-injected supervisor action from restartSandboxGateway.
Update the managed settle probe to use the same injected
deps.requestGatewaySupervisorAction path that restartSandboxGateway receives,
and thread that dependency through confirmRecoveredSandboxGatewayManaged so the
recovery wait stays fully mocked when callers override it.
---
Outside diff comments:
In `@Dockerfile`:
- Around line 1140-1145: The OpenClaw gateway fallback is validating process
identity with ps ... comm in the gateway check, which can misidentify
npm-launched gateways whose comm is node. Update the gateway proof in the
Dockerfile block that sets gwidentity/gwstate/gwcurrent to validate
/proc/$gwpid/cmdline instead of comm, and keep the existing PID/starttime check
as the process identity guard for the fallback flow.
---
Nitpick comments:
In `@docs/security/openshell-0.0.71-gateway-auth-review.mdx`:
- Around line 19-24: The MDX bullets in this review note combine multiple
sentences on a single source line, violating the one-sentence-per-line
formatting rule. Update the affected bullets in the security doc so each
sentence is on its own line, keeping the existing content intact while reflowing
the text around the OpenShell gateway auth, Docker-hosted compatibility, Hermes
env-file, JWT recovery, sandbox recovery, and sessions RPC helper bullets.
In `@src/lib/onboard/docker-driver-gateway-config-auth-contract.test.ts`:
- Around line 22-53: The `keeps the OpenShell gateway auth source review aligned
with the generated config` test is over-coupled to review-note prose and will
fail on wording-only changes. Replace the long `reviewNote.toContain(...)`
checklist with a minimal set of stable, machine-readable version markers in this
spec, and keep the real behavioral assertions in the TOML/JWT contract tests
such as `openshell_server::config_file::load()` and the gateway auth source
contract tests.
🪄 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: 57a68fc4-17b0-4cc6-abd5-1266ef89751c
📒 Files selected for processing (47)
Dockerfileagents/hermes/Dockerfileagents/hermes/runtime-config-guard.pyagents/hermes/start.shagents/hermes/validate-env-secret-boundary.pydocs/about/release-notes.mdxdocs/index.ymldocs/manage-sandboxes/install-plugins-hermes.mdxdocs/manage-sandboxes/lifecycle.mdxdocs/manage-sandboxes/runtime-controls.mdxdocs/reference/commands-nemohermes.mdxdocs/reference/commands.mdxdocs/reference/troubleshooting.mdxdocs/security/best-practices.mdxdocs/security/openshell-0.0.71-gateway-auth-review.mdxdocs/security/tcb-boundary.mdxscripts/checks/no-test-dist-imports.tsscripts/gateway-control.shscripts/lib/gateway-supervisor.shscripts/managed-gateway-control.pyscripts/nemoclaw-start.shscripts/openclaw-config-guard.pysrc/commands/sandbox/gateway/restart.tssrc/lib/actions/inference-set-endpoint-security.test.tssrc/lib/actions/inference-set.test.tssrc/lib/actions/inference-set.tssrc/lib/actions/sandbox/connect-flow.test.tssrc/lib/actions/sandbox/connect.tssrc/lib/actions/sandbox/gateway-restart.test.tssrc/lib/actions/sandbox/gateway-restart.tssrc/lib/actions/sandbox/hermes-secret-boundary-recovery.test.tssrc/lib/actions/sandbox/hermes-secret-boundary-recovery.tssrc/lib/actions/sandbox/process-recovery.test.tssrc/lib/actions/sandbox/process-recovery.tssrc/lib/agent/gateway-restart-scripts.tssrc/lib/cli/public-display-defaults.tssrc/lib/onboard/docker-driver-gateway-config-auth-contract.test.tssrc/lib/sandbox/build-context.tssrc/lib/sandbox/config.tssrc/lib/sandbox/privileged-exec.test.tssrc/lib/sandbox/privileged-exec.tssrc/lib/shields/index.test.tssrc/lib/shields/index.tssrc/lib/shields/legacy-hermes-compat.test.tstest/cli/connect-recovery-settle.test.tstest/cli/connect-recovery.test.tstest/e2e-gateway-isolation.sh
💤 Files with no reviewable changes (7)
- src/lib/sandbox/build-context.ts
- src/lib/shields/index.test.ts
- src/lib/sandbox/privileged-exec.test.ts
- src/lib/shields/legacy-hermes-compat.test.ts
- src/lib/sandbox/privileged-exec.ts
- src/lib/sandbox/config.ts
- src/lib/shields/index.ts
✅ Files skipped from review due to trivial changes (1)
- docs/reference/commands-nemohermes.mdx
🚧 Files skipped from review as they are similar to previous changes (14)
- src/lib/cli/public-display-defaults.ts
- src/lib/actions/sandbox/connect.ts
- src/commands/sandbox/gateway/restart.ts
- docs/manage-sandboxes/install-plugins-hermes.mdx
- docs/reference/troubleshooting.mdx
- scripts/checks/no-test-dist-imports.ts
- docs/manage-sandboxes/lifecycle.mdx
- src/lib/agent/gateway-restart-scripts.ts
- src/lib/actions/sandbox/gateway-restart.test.ts
- docs/manage-sandboxes/runtime-controls.mdx
- agents/hermes/validate-env-secret-boundary.py
- scripts/lib/gateway-supervisor.sh
- docs/reference/commands.mdx
- src/lib/actions/sandbox/gateway-restart.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
Vitest E2E Target Results — ❌ Some jobs failedRun: 28467231522
|
jyaunches
approved these changes
Jun 30, 2026
8 tasks
3 tasks
2 tasks
26 tasks
jyaunches
pushed a commit
that referenced
this pull request
Jun 30, 2026
…arning to file (#6065) ## Summary Two E2E regressions introduced by #5874: - **`runtime-overrides`**: `reconcile_agent_model_with_provider` ran after `apply_model_override` and overwrote `agents.defaults.model.primary` with an `inference/`-prefixed variant because `qualify()` unconditionally prepends `inference/`. The comment at the call site already documented that the explicit override should win; this adds the guard to enforce it. - **`issue-2478-crash-loop-recovery`**: `restore_openclaw_runtime_guard_chain` wrote its warning only to stderr. The new host-mediated recovery path (`docker exec … nemoclaw-gateway-control recover`) runs inside PID 1, not a fresh sandbox exec, so stderr never reached `/tmp/gateway.log`. Mirrors the warning to the gateway log file using the same `_NEMOCLAW_GATEWAY_LOG` seam the watchdog already uses. ## Test plan - [ ] Trigger the `runtime-overrides` E2E target — expect `primaryModel(modelOverride)` to return `"anthropic/claude-sonnet-4-6"` (not `"inference/anthropic/…"`) - [ ] Trigger the `issue-2478-crash-loop-recovery` E2E target — expect `[gateway-recovery] WARNING: .*restoring library guards from packaged preloads` to appear in `/tmp/gateway.log` - [ ] Full main E2E run to confirm no regressions Signed-off-by: Preksha Vijayvargiya <prekshiv@nvidia.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Preserved explicit model overrides so they are not replaced by later reconciliation. * Added startup guard-chain warnings to the gateway log as well as stderr for easier troubleshooting. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Preksha Vijayvargiya <prekshiv@nvidia.com>
21 tasks
jyaunches
added a commit
that referenced
this pull request
Jun 30, 2026
<!-- markdownlint-disable MD041 --> ## Summary Refreshes the public documentation for NemoClaw v0.0.71 after scanning commits since v0.0.70. Adds release notes and fills the remaining doc gaps for Windows bootstrap diagnostics, OpenClaw agent auto-relock warnings, auto-pair cadence tuning, and plugin-install recovery hints. ## Changes - `docs/about/release-notes.mdx`: adds the v0.0.71 release-note section, grouped by gateway recovery, OpenShell auth, policy provenance, day-two maintenance, messaging/inference, and Windows setup. - `docs/get-started/windows-preparation.mdx`: documents sanitized WSL install output and reboot gating in the Windows bootstrap. - `docs/reference/commands.mdx`: documents the host `agent` wrapper's shields auto-relock warning and OpenClaw auto-pair watcher tuning variables. - `docs/reference/troubleshooting.mdx`: adds plugin-install network failure recovery guidance and updates Windows WSL troubleshooting for sanitized install logs and reboot-required handling. Source summary: - #6065 -> `docs/about/release-notes.mdx`: Notes explicit model override preservation and gateway-log guard-chain recovery diagnostics. - #5874 -> `docs/about/release-notes.mdx`: Summarizes host-mediated `recover` and `gateway restart`, linking to lifecycle, command, troubleshooting, and trusted-boundary docs already added by the source PR. - #5596 -> `docs/about/release-notes.mdx`: Summarizes OpenShell 0.0.71 gateway auth, loopback binding, and compatibility-container docs already added by the source PR. - #5797 and #5798 -> `docs/about/release-notes.mdx`: Summarizes `policy-list` provenance, Restricted tier suppression, and Balanced tier weather behavior already reflected in policy docs. - #5784 -> `docs/about/release-notes.mdx`: Summarizes `--destroy-user-data` and the safe `--yes` uninstall behavior already documented in lifecycle and command docs. - #6034 -> `docs/about/release-notes.mdx`: Summarizes custom Dockerfile warm-build cache behavior already documented in the command reference. - #5951 -> `docs/reference/commands.mdx`: Documents the stderr-only host `agent` wrapper warning after recent shields auto-relock. - #5387 -> `docs/reference/commands.mdx`: Documents OpenClaw auto-pair watcher cadence and fast-reentry tuning variables. - #5835 -> `docs/reference/troubleshooting.mdx`: Adds recovery guidance for OpenClaw plugin-install network failures. - #5995 and #5956 -> `docs/about/release-notes.mdx`: Summarizes Microsoft Teams final-message delivery and runtime mention hints already covered by messaging docs. - #5716 -> `docs/about/release-notes.mdx`: Summarizes non-interactive Ollama loopback safety already covered by local inference docs. - #5505, #5527, and #5528 -> `docs/about/release-notes.mdx`: Summarizes compatible local endpoint, model task-fit, and model capability audit docs. - #6009 -> `docs/get-started/windows-preparation.mdx`, `docs/reference/troubleshooting.mdx`: Documents sanitized Windows bootstrap WSL output and reboot-required gating. - #6055 -> no additional source doc page change needed beyond the already-merged quickstart update; release notes did not duplicate routine quickstart cleanup. No matching v0.0.71 GitHub announcement discussion was found in the latest 20 discussions, so this refresh is based on the commit scan and existing source PR docs. ## 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) ## Quality Gates <!-- Check all that apply. For any "covered by existing tests", "not applicable", or waiver entry, add a brief justification on the same line or in the Changes section. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: docs-only refresh with no runtime behavior changes. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## 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. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [ ] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — ran `npm run docs`; Fern reported 0 errors and 2 existing warnings. - [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 is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new release-notes entry covering gateway recovery, authentication, network policy/provenance output, uninstall safety, Windows bootstrap diagnostics, messaging defaults, and inference setup guidance. * Clarified Windows preparation steps around reboot behavior and redacting troubleshooting transcripts. * Expanded command reference details for OpenClaw wrapper behavior and new auto-pair tuning options. * Improved troubleshooting guidance for plugin installation issues, WSL repair/reboot cases, and install timing problems. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
21 tasks
13 tasks
jyaunches
pushed a commit
that referenced
this pull request
Jul 6, 2026
…st it (#6086) ## Summary Two regressions from #5874 (fixed in #6065) only surfaced in **live E2E targets that don't run on PR CI**. This PR closes that class of gap: it audits the live suite for behavior-critical assertions that are cheaply mockable, backfills them as fast units that run on **every** PR, and adds a guard so pure-unit blocks can't hide in live files again. ## What's here **1. The two direct #6065 regression fences** (mocked shell-units) - `reconcile`: an explicit `NEMOCLAW_MODEL_OVERRIDE` survives a divergent gateway model and the stale in-file fallback; normal drift-correction still runs when unset. - `guard recovery`: the restore warning is mirrored into `_NEMOCLAW_GATEWAY_LOG` (the marker the crash-loop E2E polls), and stays silent when the chain is healthy. **2. High-priority mockable backfill** (security/recovery class) - ollama-auth-proxy: Bearer enforcement, no `/api/tags` bypass (#3338), header stripping, non-ASCII auth no-crash (#4820), backend 502. - `config get`: credential redaction + `gateway`-key omission (the `nvapi-` regression class). - device approval policy: scope-upgrade allowlist gate, gateway-env stripping, recover-failed rejection paths (#4462). - shields audit JSONL: credentials redacted before persistence. - hermes env secret boundary: value-shape (not key-name) discriminator; raw secrets rejected without echoing. - dashboard bind: `NEMOCLAW_DASHBOARD_BIND` opt-in incl. negative cases (#3259). - whatsapp compact QR: package shape-detection + terminal-only `small` (#4522). **3. Medium/low backfill** - ollama token-file lifecycle (0600 / persisted / divergent-repair); extra-placeholder-keys canonical placeholder + accepted-keys breadcrumb; hermes `remove_stale_gateway_file` symlink-safety; token-rotation selective-rebuild naming; `_validate_port` fail-closed; snapshot `help` branch. **4. Regression guard** - `scripts/checks/no-unit-blocks-in-live-e2e.ts` bans the vitest `it(...)` primitive inside `test/e2e/live/**` (that glob is uncollected on PR CI, so such blocks never run). Wired into the checks registry with its own unit test. - Relocated the existing offenders (skill-agent + messaging-compatible-endpoint classifier blocks, plus the bare-`test(` unit cases in common-egress + openclaw-inference-switch) into importable `test/e2e/support` modules with PR-collected tests; the live tests import them unchanged. ## Notes - Minimal behavior-preserving refactor to `whatsapp-qr-compact.ts` to export its pure helpers (the preload still auto-installs on require); `tsconfig.runtime-preloads.json` excludes the new co-located test from the shipped preload build. - `nemoclaw-start.sh`: made two possibly-empty-array iterations bash-3.2-safe via the existing `"${arr[@]+...}"` idiom so the shell-unit harnesses run on stock macOS bash. - **Deferred (2 low-value items):** the install.sh "Resolved install ref:" log assertion and the OpenClaw anthropic plain-baseUrl assertion both land in legacy-budget-capped test files where the growth guardrail forbids bumping the budget; the anthropic contract is already covered on the Hermes side and enforced live on the OpenClaw side. ## Verification - Every new/changed test verified green individually across the `cli`, `integration`, and `e2e-support` projects. - `npm run checks` (incl. the new live-unit-block guard), test-file-size budget, gitleaks, and CLI typecheck all pass. - The full `test-cli` pre-commit hook was skipped locally only because it trips on **pre-existing** macOS bash 3.2 failures in untouched shell-harness suites (`select`/`set -u`); CI runs bash 5.x, where they are green. --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved startup robustness for environment parsing and background launch behavior. * WhatsApp compact-QR rendering more consistently uses the compact “terminal” style. * Dashboard remote bind activates only when explicitly opted in via `NEMOCLAW_DASHBOARD_BIND=0.0.0.0`. * Tightened audit/config redaction to prevent secret leakage and omit gateway details. * **Tests** * Expanded coverage for guard-chain recovery warnings, model override precedence, Hermes env-boundary hardening, and proxy/policy correctness. * **Chores** * Added a CI safeguard to prevent unit-test primitives from being included in live E2E tests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
12 tasks
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
5 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
Adds a supported host-mediated
gateway restartcommand for NemoClaw-managed OpenClaw and Hermes gateways and routes automatic recovery through the same topology-specific authenticated controller or supervisor path. The lifecycle acts only on proven process and listener identity, applies configuration and state changes transactionally, and fails closed when privileged execution or built-in health probes are ambiguous.Related Issue
Fixes #2426
Fixes #5253
Supersedes #5416
Changes
sandbox:gateway:restartplus publicnemoclaw <name> gateway restartandnemohermes <name> gateway restartrouting while keeping healthyrecoveridempotent.CAP_DAC_OVERRIDE.0600lifecycle lock and root-owned0444authorization bound to the live root controller; all orphaned, mismatched, unexpected, and failed replacement exits still count.GATEWAY_PID=rather than accepting the legacy custom-scriptALREADY_RUNNINGmarker.main.Type of Change
Quality Gates
Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Exact-head evidence for
8dbe420d3d835546683cbfa67fec0537791ad7c1:d86c60c325a3dbc74fc42f74447bc00f89be4400addresses the current CodeRabbit correctness and documentation findings. Signed merge commitc9061c68fd82b1b73044383bc4e8962808644744incorporatesmainate10462ff3e2e0727350a2532fc7bb7edc64116b2without rebasing or rewriting the verified history. Signed cleanup commit8dbe420d3d835546683cbfa67fec0537791ad7c1addresses the resulting CodeQL/code-quality diagnostics.npm run build:cli,npm run typecheck:cli, Biome, ShellCheck, Python compilation, branch diff checks, test-size/project/title guards, andnpm run docs:check-agent-variantspass.npm run docscompletes with 0 errors and two existing Fern warnings; the warning-free checkbox remains unchecked.Verified; all three new commits contain the DCO sign-off declaration.test-clihook and full-npm testboxes remain unchecked; exact-head CI is required for those broader platform lanes.Signed-off-by: Aaron Erickson aerickson@nvidia.com
Summary by CodeRabbit
New Features
gateway restartcommand for sandboxes with forced reload and post-restart health/forward verification.Bug Fixes
Documentation
recover,gateway restart, shields windows, and Hermes recovery constraints.