fix(gateway): respawn openclaw gateway after a managed restart (#6868) - #6910
Conversation
…A#6868) Signed-off-by: Atulya Singh <atulyarajsingh@gmail.com>
📝 WalkthroughWalkthroughThe change adds managed expected-exit leases, validates them during gateway respawn, publishes leases for all managed agents, and improves config restart failure guidance with corresponding tests. ChangesManaged gateway restart flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ManagedController
participant GatewaySupervisor
participant Gateway
participant RespawnLoop
ManagedController->>GatewaySupervisor: publish expected-exit lease
ManagedController->>Gateway: signal old gateway
Gateway-->>RespawnLoop: report clean exit
RespawnLoop->>GatewaySupervisor: validate pid and start identity
GatewaySupervisor-->>RespawnLoop: return authorization result
RespawnLoop->>Gateway: respawn authorized gateway
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: None 2 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/openclaw-managed-restart-respawn.test.ts (1)
177-203: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExercise the actual entrypoint instead of extracted shell fragments.
These tests can pass even if sourcing or surrounding lifecycle wiring prevents the shipped guards from executing. Use a sandboxed
nemoclaw-start.shharness and assert the gateway process is respawned or stopped through that public boundary.As per path instructions, prefer observable outcomes through the public boundary over source-text or private-shape assertions.
Also applies to: 239-312
🤖 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/openclaw-managed-restart-respawn.test.ts` around lines 177 - 203, Replace the extractRespawnGuards source-text testing in the respawn tests with a sandboxed nemoclaw-start.sh harness that executes the real entrypoint and observes gateway lifecycle outcomes. Cover both guard paths, asserting leased exits respawn the gateway and non-leased or terminal exits stop it through the public boundary; remove source-marker and extracted-fragment assertions.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.
Nitpick comments:
In `@test/openclaw-managed-restart-respawn.test.ts`:
- Around line 177-203: Replace the extractRespawnGuards source-text testing in
the respawn tests with a sandboxed nemoclaw-start.sh harness that executes the
real entrypoint and observes gateway lifecycle outcomes. Cover both guard paths,
asserting leased exits respawn the gateway and non-leased or terminal exits stop
it through the public boundary; remove source-marker and extracted-fragment
assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9093f075-7976-4c6f-be70-6c4c4470f668
📒 Files selected for processing (6)
scripts/lib/gateway-supervisor.shscripts/managed-gateway-control.pyscripts/nemoclaw-start.shsrc/lib/sandbox/config.tstest/config-set.test.tstest/openclaw-managed-restart-respawn.test.ts
|
✨ Thanks for the fix. The managed restart now leases the exit and respawns the gateway so Related open issues: Related open issues: |
Co-authored-by: Atulya Singh <atulyarajsingh@gmail.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Resolved the current main merge conflict append-only in signed commit 8b755d7 while preserving the original author as co-author. The resolution keeps current managed-controller diagnostics and extends the existing exact root-authorized exit lease to OpenClaw restarts. Validation: 95 focused tests passed; repository hooks, CLI typecheck, and plugin typecheck passed; independent security review passed with no blocker; existing docs already cover the behavior. Current-head CI and automated review are now being monitored. |
Co-authored-by: Atulya Singh <atulyarajsingh@gmail.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
left a comment
There was a problem hiding this comment.
Approved at exact head acda1f5 after resolving the main merge conflict and the source-shape gate. Independent security review passed; the expected-exit lease remains root-owned, exact-process-bound, single-use, and fail-closed. The follow-up test refactor preserves all positive and negative authorization cases while reducing TypeScript IfStatements from 8 to 0. Final exact-head result: 37 checks passed, 0 failed, 0 pending; advisor ledger is clear and review threads are resolved.
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical dated changelog entry for NemoClaw `v0.0.89` before the release plan captures the tag commit. The entry summarizes the user-visible Station preparation, inference, recovery, policy-disclosure, and CLI-containment changes merged since `v0.0.88`. ## Changes - Add `docs/changelog/2026-07-20.mdx` with the exact `## v0.0.89` release heading, parser-safe SPDX comment, summary, and detailed bullets. - Link each shipped theme to the most specific published OpenClaw documentation routes. - Keep internal E2E, advisory-registry, and review-workflow refactors out of the user-facing release summary. Source summary: - #7214, #7241, #7237, #7223, #7204, #7202, #7183, and #7090 -> `docs/changelog/2026-07-20.mdx`: Summarize qualified DGX Station identity, package-state, PackageKit, DKMS, and reboot-handoff fixes. - #7242, #7221, #7186, #7164, and #6874 -> `docs/changelog/2026-07-20.mdx`: Summarize inference endpoint provenance, provider attachment, managed vLLM cleanup and progress, and Ollama selection guidance. - #7225, #7216, #7192, #7136, #7096, and #6910 -> `docs/changelog/2026-07-20.mdx`: Summarize sandbox readiness, recovery guidance, rebuilt model routing, durable Hermes state, and gateway restart behavior. - #7187 -> `docs/changelog/2026-07-20.mdx`: Summarize complete effective messaging-preset egress disclosure before policy mutation. - #7218, #7165, and #7184 -> `docs/changelog/2026-07-20.mdx`: Summarize structured output containment, terminal-safe route display, and Hermes forward cleanup. ## 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) ## Quality Gates - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: This PR adds release-history prose only; the focused changelog contract test validates its required structure and routes. - [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 - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `mise exec node@22.23.1 -- npx vitest run test/changelog-docs.test.ts` (6 passed). - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not applicable to this doc-only release entry. - [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) — `mise exec node@22.23.1 -- npm run docs` completed with 0 errors and 2 existing site-wide 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) — the native changelog entry uses the required parser-safe MDX SPDX comment and does not use frontmatter. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Expanded DGX Station installation support for qualified GB300 and OTA-upgraded environments. * Preserved selected inference providers, endpoints, model pins, and tuning settings during sandbox creation and rebuilds. * Improved sandbox recovery by validating availability and stability before restarting services. * Added clearer policy output showing the complete effective messaging egress configuration. * **Bug Fixes** * Hardened status and inference route displays by sanitizing terminal control characters. * Improved Hermes uninstall behavior by stopping detached dashboard forwarding. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Summary
nemoclaw <sandbox> config set --key … --value … --restartwrote the config to disk and then hung until the user killed it (or, if they waited, failed withGATEWAY_HEALTH_TIMEOUTafter ~150s), leaving the config written but never applied to the running agent. The managed restart path terminated the openclaw gateway with SIGTERM and then only waited for a replacement — but openclaw exits0on SIGTERM, andnemoclaw-startreads a clean exit as an intentional shutdown and deliberately does not respawn. The restart therefore killed the gateway with the one signal that guaranteed it would not come back. After this change the root controller leases the exit it is about to cause, the entrypoint relaunches the gateway, andconfig set --restartcompletes normally.Related Issue
Fixes #6868
Root cause
The managed path (
gateway-control.shroutes tomanaged-gateway-control.pywhen PID 1 is/opt/openshell/bin/openshell-sandbox) has no explicitrestartbranch: it falls through to_terminate_gateway()+_wait_for_healthy_gateway(), and never launches a replacement. Its comment states the assumption — "PID 1 continuously supervises the managed gateway" — but the process that would respawn it isnemoclaw-start, whose respawn loop treatsRC == 0as an intentional stop andexit 0s.The mechanism was already solved for Hermes by the expected-exit lease (
_publish_expected_exit_lease), which authorizes one exact gateway exit while the root controller is live. It was gated behindspec.name == "hermes", so openclaw never got one.Evidence gathered against
ghcr.io/nvidia/nemoclaw/sandbox-base:latest(OpenClaw 2026.6.10 / aa69b12, aarch64 — the exact build in the report):openclaw gateway run --port … --allow-unconfigured, healthy on HTTP 200) sent the same SIGTERM_terminate_gatewaysends exits with status 0.openshell-sandboxv0.0.72 as PID 1 reaps children but never respawns them (its CLI has no restart policy; killing its child propagates the exit).Changes
scripts/managed-gateway-control.py— publish the expected-exit lease for every managed agent, not only Hermes, so the entrypoint can distinguish a host-requested restart from a self-requested shutdown. The lease is published before the pidfd signal and cleared after the health wait, so it is live exactly while the controller waits.scripts/lib/gateway-supervisor.sh— addgateway_control_exit_was_host_authorized(plus the controller argv/liveness proofs) to the shared PID 1 control primitives both entrypoints already source. It validates the root-owned0:0 711directory and0:0 444marker, requires the lease to name the exact pid + start identity, and requires the named controller to still be the live root helper, so an orphaned or mismatched marker fails closed as an ordinary crash.scripts/nemoclaw-start.sh— both openclaw respawn loops relaunch a leased exit instead ofexit 0. An unleased clean exit still stops, so genuine shutdowns are unchanged.src/lib/sandbox/config.ts— when the restart does not complete, say plainly that the config was written to disk but not applied to the running agent, and give the idempotent retry (nemoclaw <sandbox> gateway restart) instead of a generic failure line.No new abstraction, configuration, fallback, or compatibility path: this wires openclaw into the existing lease mechanism and its existing consumer (
nemoclaw-start), protected by the tests below.Type of Change
Quality Gates
config set --restartbehavior; no documented interface, flag, or output contract changes.python3 -I <helper> restart|recover <64-hex nonce>.Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run test/openclaw-managed-restart-respawn.test.ts test/config-set.test.ts src/lib/actions/sandbox/gateway-restart.test.ts→ 92 passed.npm run typecheck:cliandnpm --prefix nemoclaw run typecheck→ 0 errors.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Regression coverage
test/openclaw-managed-restart-respawn.test.tsis the test that should have caught this. Each case was confirmed to fail against the unfixed code and pass after:expected false to be true— no lease published).Not reproduced
The report notes the container stays
Up (healthy). By this mechanismnemoclaw-startexiting should take the container down with it; NemoClaw sets no Docker restart policy (OpenShell creates the container), so this is most likely OpenShell restarting it. That leg was not verified and is not relied on by this fix.Signed-off-by: Atulya Singh atulyarajsingh@gmail.com
Summary by CodeRabbit
New Features
Bug Fixes
Tests