fix(recover): restore managed OpenClaw config mode - #7972
Merged
Conversation
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
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:
📝 WalkthroughWalkthroughThe changes add managed-health diagnostics to sandbox recovery, validate legacy keepalive container replacement in E2E tests, and apply mode ChangesSandbox recovery
State restore permissions
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant RecoveryTest as Gateway recovery E2E test
participant Fixture as Legacy keepalive fixture
participant Docker as Docker sandbox
participant Gateway as Recovered gateway
RecoveryTest->>Fixture: Recreate pinned legacy container
Fixture->>Docker: Start replacement with sleep infinity
Docker-->>Fixture: Return replacement identity
Fixture-->>RecoveryTest: Return validated recreation result
RecoveryTest->>Docker: Restart replacement
Docker->>Gateway: Run managed startup and health checks
Gateway-->>RecoveryTest: Return recovery and inference results
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 25f1cdb in the TypeScript / code-coverage/cliThe overall coverage in commit 25f1cdb in the Show a code coverage summary of the most impacted files.
Updated |
prekshivyas
marked this pull request as ready for review
July 31, 2026 04:57
![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: 1
🤖 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`:
- Around line 1259-1281: The relaunch recovery flow must use
confirmRelaunchedManagedHealth for managed probes in both
waitForRecoveredSandboxGateway calls, rather than passing
confirmRecoveredSandboxGatewayManaged directly. Update the relaunch probe
callbacks so definitive failures populate relaunchedManagedHealthFailureDetail
before the wait returns, and add a regression test covering a managed probe
failure during waitForRecoveredSandboxGateway.
🪄 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: d212c9fb-e092-4193-bf20-3b4f60a0f8fc
📒 Files selected for processing (4)
src/lib/actions/sandbox/process-recovery.tssrc/lib/state/state-file-restore-mode.test.tssrc/lib/state/state-file-restore.tstest/process-recovery-supervisor-relaunch.test.ts
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported 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: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
cv
approved these changes
Jul 31, 2026
sandl99
approved these changes
Jul 31, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/state/state-file-restore-mode.test.ts`:
- Around line 51-55: Add a separate fixture for an ordinary copied state file
with a non-managed path, then invoke the restore flow against that fixture and
assert its resulting mode is 0o640. Keep the existing runRestore(false) coverage
intact, and focus the new assertion on behavior rather than the specific
implementation.
In `@test/e2e/support/gateway-guard-legacy-keepalive-fixture.test.ts`:
- Around line 107-112: Bound the synchronous child process in the “loads the
real recreation dependency through the standalone tsx entrypoint” test by adding
a finite spawnSync timeout and configuring SIGKILL on timeout. Preserve the
existing command, fixture arguments, and UTF-8 encoding.
🪄 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: 7840a593-1e73-4639-8d30-fcd895826c7c
📒 Files selected for processing (8)
src/lib/actions/sandbox/process-recovery.tssrc/lib/state/state-file-restore-mode.test.tssrc/lib/state/state-file-restore.tstest/e2e/live/gateway-guard-legacy-keepalive-fixture.tstest/e2e/live/gateway-guard-recovery.test.tstest/e2e/mock-parity.jsontest/e2e/support/gateway-guard-legacy-keepalive-fixture.test.tstest/process-recovery-supervisor-relaunch.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- test/e2e/mock-parity.json
- src/lib/state/state-file-restore.ts
- test/process-recovery-supervisor-relaunch.test.ts
- src/lib/actions/sandbox/process-recovery.ts
- test/e2e/live/gateway-guard-legacy-keepalive-fixture.ts
Comment on lines
+51
to
+55
| it("keeps the restricted mode for ordinary copied state files", () => { | ||
| const { configPath } = runRestore(false); | ||
|
|
||
| expect(mode(configPath)).toBe(0o640); | ||
| }); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test a non-managed copied state file.
runRestore(false) still restores openclaw.json from STATE_FILE. This does not verify the 0640 mode for an ordinary copied state file. Add a second fixture with a non-managed path and assert its mode is 0640.
Based on PR objectives, ordinary copied state files must retain mode 0640. As per path instructions, review tests for behavioral confidence rather than implementation lock-in.
🤖 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/state/state-file-restore-mode.test.ts` around lines 51 - 55, Add a
separate fixture for an ordinary copied state file with a non-managed path, then
invoke the restore flow against that fixture and assert its resulting mode is
0o640. Keep the existing runRestore(false) coverage intact, and focus the new
assertion on behavior rather than the specific implementation.
Source: Path instructions
Comment on lines
+107
to
+112
| it("loads the real recreation dependency through the standalone tsx entrypoint", () => { | ||
| const result = spawnSync( | ||
| process.execPath, | ||
| ["--import", "tsx", FIXTURE_PATH, "fixture-import-probe", "f".repeat(64)], | ||
| { encoding: "utf8" }, | ||
| ); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound the synchronous fixture process.
spawnSync can block the E2E support worker indefinitely if tsx or the Docker lookup stalls. Set a bounded timeout and use SIGKILL so the synchronous child process terminates reliably.
Proposed fix
["--import", "tsx", FIXTURE_PATH, "fixture-import-probe", "f".repeat(64)],
- { encoding: "utf8" },
+ {
+ encoding: "utf8",
+ timeout: 30_000,
+ killSignal: "SIGKILL",
+ },As per path instructions, E2E subprocess boundaries require bounded timeouts and SIGKILL for synchronous calls.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it("loads the real recreation dependency through the standalone tsx entrypoint", () => { | |
| const result = spawnSync( | |
| process.execPath, | |
| ["--import", "tsx", FIXTURE_PATH, "fixture-import-probe", "f".repeat(64)], | |
| { encoding: "utf8" }, | |
| ); | |
| it("loads the real recreation dependency through the standalone tsx entrypoint", () => { | |
| const result = spawnSync( | |
| process.execPath, | |
| ["--import", "tsx", FIXTURE_PATH, "fixture-import-probe", "f".repeat(64)], | |
| { | |
| encoding: "utf8", | |
| timeout: 30_000, | |
| killSignal: "SIGKILL", | |
| }, | |
| ); |
🤖 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/support/gateway-guard-legacy-keepalive-fixture.test.ts` around lines
107 - 112, Bound the synchronous child process in the “loads the real recreation
dependency through the standalone tsx entrypoint” test by adding a finite
spawnSync timeout and configuring SIGKILL on timeout. Preserve the existing
command, fixture arguments, and UTF-8 encoding.
Source: Path instructions
23 tasks
sandl99
added a commit
that referenced
this pull request
Jul 31, 2026
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical July 30 release entry for `v0.0.99` before the release tag is captured. The entry covers all 37 merged PRs since `v0.0.98` and bounds experimental or dormant work without presenting it as supported behavior. ## Changes - Adds `docs/changelog/2026-07-30.mdx` with the exact `## v0.0.99` heading, parser-safe MDX SPDX comment, summary, detailed release bullets, and published documentation routes. - Records user-visible recovery, snapshot, shared-route, Hermes, readiness, inference, image, documentation, and release E2E changes. - States that the managed-image selection and startup-profile contracts remain dormant and do not activate buildless onboarding. Source summary: - [#7972](#7972) -> `docs/changelog/2026-07-30.mdx`: Records restored managed OpenClaw configuration modes during recovery. - [#7834](#7834) -> `docs/changelog/2026-07-30.mdx`: Records clone-bound pairing verification after snapshot restore. - [#7975](#7975) -> `docs/changelog/2026-07-30.mdx`: Records managed startup recovery coverage. - [#7960](#7960) -> `docs/changelog/2026-07-30.mdx`: Records dormant startup-profile coordination without activating a supported surface. - [#7856](#7856) -> `docs/changelog/2026-07-30.mdx`: Records persistence of the credential-free OpenClaw startup command. - [#7959](#7959) -> `docs/changelog/2026-07-30.mdx`: Records dormant startup-profile construction without changing onboarding. - [#7946](#7946) -> `docs/changelog/2026-07-30.mdx`: Records the internal startup-profile schema and transport contract. - [#7951](#7951) -> `docs/changelog/2026-07-30.mdx`: Records platform-pull cleanup before managed-image validation. - [#7949](#7949) -> `docs/changelog/2026-07-30.mdx`: Records rejection of retained Hermes `uv` build cache metadata. - [#7597](#7597) -> `docs/changelog/2026-07-30.mdx`: Records separate command and agent first-turn latency evidence. - [#7931](#7931) -> `docs/changelog/2026-07-30.mdx`: Records focused E2E replacement evidence for retired selectors. - [#7950](#7950) -> `docs/changelog/2026-07-30.mdx`: Records exclusion of build-only BuildKit telemetry from the Deep Agents Code probe. - [#7665](#7665) -> `docs/changelog/2026-07-30.mdx`: Records consolidated priority 2 E2E coverage. - [#7911](#7911) -> `docs/changelog/2026-07-30.mdx`: Records the corrected NVIDIA DORI installation pin. - [#7934](#7934) -> `docs/changelog/2026-07-30.mdx`: Records the staging image-family wait before Brev Launchable deployment. - [#7772](#7772) -> `docs/changelog/2026-07-30.mdx`: Records dormant managed-image selection contracts without activating buildless onboarding. - [#7941](#7941) -> `docs/changelog/2026-07-30.mdx`: Records corrected agent-specific provider and policy guidance. - [#7819](#7819) -> `docs/changelog/2026-07-30.mdx`: Records removal of empty Deep Agents Code provider-switch sections. - [#7932](#7932) -> `docs/changelog/2026-07-30.mdx`: Records independent credential-generation E2E execution. - [#7840](#7840) -> `docs/changelog/2026-07-30.mdx`: Records shared-route preservation and pre-delete peer validation during upgrades. - [#7874](#7874) -> `docs/changelog/2026-07-30.mdx`: Records the split between pre-tag release entries and post-tag Announcements. - [#7876](#7876) -> `docs/changelog/2026-07-30.mdx`: Records the writable Hermes runtime root within lockdown. - [#7756](#7756) -> `docs/changelog/2026-07-30.mdx`: Records validated multi-platform managed-image publication. - [#7914](#7914) -> `docs/changelog/2026-07-30.mdx`: Records accepted `uv` version metadata in Hermes image validation. - [#7686](#7686) -> `docs/changelog/2026-07-30.mdx`: Records the explicitly experimental Microsoft Entra runtime identity reference. - [#7869](#7869) -> `docs/changelog/2026-07-30.mdx`: Records classified gateway relaunch quarantine and rebuild guidance. - [#7814](#7814) -> `docs/changelog/2026-07-30.mdx`: Records state restore into replacement sandboxes and SQLite write verification. - [#7839](#7839) -> `docs/changelog/2026-07-30.mdx`: Records quieter onboarding test execution without a user-facing behavior claim. - [#7854](#7854) -> `docs/changelog/2026-07-30.mdx`: Records generalized agent-selection guidance. - [#7845](#7845) -> `docs/changelog/2026-07-30.mdx`: Records isolated CDI test evidence without a user-facing behavior claim. - [#7843](#7843) -> `docs/changelog/2026-07-30.mdx`: Records the corrected Omni sub-agent model ID. - [#7908](#7908) -> `docs/changelog/2026-07-30.mdx`: Records reviewed Hermes and Deep Agents Code dependency pins. - [#7887](#7887) -> `docs/changelog/2026-07-30.mdx`: Records rejection of a symlinked DGX Station release marker. - [#7747](#7747) -> `docs/changelog/2026-07-30.mdx`: Records the internal compute-driver separation without a user-facing behavior claim. - [#7660](#7660) -> `docs/changelog/2026-07-30.mdx`: Records atomic publication of rebuild recovery manifests. - [#7661](#7661) -> `docs/changelog/2026-07-30.mdx`: Records bounded local inference health-response retention. - [#7654](#7654) -> `docs/changelog/2026-07-30.mdx`: Records state preservation across supervisor relaunch recovery. ## 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 - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated changelog contract, SPDX comment, version heading, and published routes. - [ ] Tests not applicable — justification: - [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: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/changelog/2026-07-30.mdx`; the documentation-only diff passed review against `WRITING.md`, the controlled word list, and `docs/CONTRIBUTING.md`. The review covered terminology, structure, active voice, release meaning, product-scope boundaries, and link and code presentation. Changelog tests passed 6/6, and the docs build reported 0 errors with 2 pre-existing warnings. - Agent: Codex CLI <!-- docs-review-head-sha: 200940f --> <!-- docs-review-agents-blob-sha: c052d60 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## 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 validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run test/changelog-docs.test.ts` passed 6/6 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable to this documentation-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) — result: Build passed with 0 errors and 2 pre-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) --- Signed-off-by: San Dang <sdang@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.99 covering snapshot restoration, sandbox recovery, gateway route upgrades, and Hermes security updates. * Documented experimental Microsoft Entra runtime identity support and enhanced readiness checks. * Added details on managed image validation, trusted CI image promotion, and end-to-end release evidence. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
wscurran
added
bug-fix
This was referenced Aug 3, 2026
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
4 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
Sandbox recovery restored managed
openclaw.jsonthrough the generic state-file path at mode0640, so the trusted restart preflight rejected the recreated sandbox even though its gateway process and HTTP endpoint were healthy. This change restores managed OpenClaw configuration at the required0660mode and preserves the classified managed-health failure detail when recovery still fails. The live regression target now deliberately recreates a legacy keepalive container because current onboarding correctly persistsnemoclaw-start.Changes
openclaw.json,.last-good, and.config-hashfiles at mode0660while retaining mode0640for ordinary copied state files.GATEWAY_UNSAFE_CONFIG_PATHrecovery failure.nemoclaw-startrestart path, then recreate only the identity-pinned sandbox container withsleep infinityand prove the legacy migration, with fail-closed fixture coverage, standalonetsxloader coverage, and mock/live parity mapping.Type of Change
Quality Gates
660 sandbox:sandboxposture, final managed-health checks, and the unsafe-config-path failure layer. The E2E follow-up changes only internal test setup.Documentation Writer Review
no-docs-neededDGX Station Hardware Evidence
Verification
Signed-off-by:line and every published commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailabletest:changedpassed 412 tests with 7 skipped but hit three known macOSsystemctl --userfailures inherited from merged test(e2e): stabilize managed startup recovery #7975; focused lifecycle coverage passed. Exact run 30610342220 selectedgateway-guard-recoveryand exposed a standalonetsximport failure before legacy recreation; commit0f47a7b78fixes that fixture failure. Merge commit6da6b0de9resolves currentmainby retaining both modern and legacy routes;25f1cdbb1includes the concurrent automated conflict-resolution history without changing the reviewed tree. Exact-head run https://github.com/NVIDIA/NemoClaw/actions/runs/30612753934 passed the full selected matrix;gateway-guard-recoverypassed all 10 phases, including persisted-startup recovery and legacy keepalive recreation/recovery.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not applicable to this narrow recovery fix and target-specific E2E fixture; the affected suites and diff-scoped validation passed.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Prekshi Vyas prekshiv@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests