fix(rebuild): verify Hermes post-restore health#7117
Merged
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
added
bug-fix
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:
📝 WalkthroughWalkthroughHermes sandbox rebuilds now validate recreated identity, restore and verify gateway health, reconcile managed MCP configuration, and fail when post-restore checks remain unverified. Tests, harnesses, and documentation cover the behavior. ChangesHermes rebuild verification
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant RebuildPipeline
participant PostRestorePhase
participant MCPBridge
participant ProcessRecovery
participant RebuildResult
RebuildPipeline->>PostRestorePhase: pass target agent
PostRestorePhase->>MCPBridge: restore managed Hermes MCP configuration
PostRestorePhase->>ProcessRecovery: verify and recover gateway processes
ProcessRecovery-->>PostRestorePhase: health, recovery, and refusal flags
PostRestorePhase->>RebuildResult: accept or reject post-restore completion
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the branch. TypeScript / code-coverage/cliThe overall coverage in the branch remains at 80%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-7117.docs.buildwithfern.com/nemoclaw |
![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: 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/actions/sandbox/rebuild-hermes-post-restore.test.ts`:
- Around line 22-37: Update the test setup around
restoreMcpBridgesAfterRebuildSpy in the gateway-health failure case so MCP
bridge restoration succeeds instead of rejecting. Keep the unrecovered gateway
state from checkAndRecoverSandboxProcesses unchanged, allowing this test to
isolate gateway verification as the sole readiness blocker; leave MCP failure
coverage to the later test.
In `@src/lib/actions/sandbox/rebuild-post-restore-phase.ts`:
- Around line 129-145: Update the recreated identity checks around
recreatedRegistryAgentName and recreatedRuntimeAgentName to preserve undefined
values when registry or runtime identities are absent, and require both values
to exist and exactly equal targetAgentName before proceeding. Add a regression
case covering a missing runtime identity with targetAgentName set to OpenClaw,
ensuring the rebuild fails rather than reporting success.
🪄 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: 2b99a635-14f8-4b7c-b246-7a0ba82b587b
📒 Files selected for processing (9)
docs/manage-sandboxes/recover-rebuild-sandboxes.mdxsrc/lib/actions/sandbox/rebuild-hermes-post-restore.test.tssrc/lib/actions/sandbox/rebuild-hermes-post-restore.tssrc/lib/actions/sandbox/rebuild-mcp-phase.tssrc/lib/actions/sandbox/rebuild-pipeline.tssrc/lib/actions/sandbox/rebuild-post-restore-phase.tstest/helpers/rebuild-flow-harness.tstest/helpers/rebuild-flow-test-harness.tstest/helpers/rebuild-flow-test-support.ts
Contributor
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: 2 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
6 tasks
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts (1)
102-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrengthen test 2 against false positives from early bail.
Test 2 only asserts that two functions were not called. If
runRebuildPostRestorePhasebails early (e.g., identity mismatch from a mock setup bug), the test passes without actually exercising the hermes path. Test 1 implicitly guards against this via the fullorderassertion, but test 2 has no equivalent check.Capture the input and assert
bailwas not called:🛡️ Proposed fix
it("does not run OpenClaw session reconciliation for another agent", async () => { agentName = "hermes"; - await runRebuildPostRestorePhase(input()); + const args = input(); + await runRebuildPostRestorePhase(args); + expect(args.bail).not.toHaveBeenCalled(); expect(sessionModels.reconcileStalePinnedSessionModelsAfterRebuild).not.toHaveBeenCalled(); expect(processRecovery.executeSandboxCommand).not.toHaveBeenCalled(); });🤖 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/actions/sandbox/rebuild-post-restore-phase.test.ts` around lines 102 - 109, Strengthen the “does not run OpenClaw session reconciliation for another agent” test by capturing the value returned from input() and asserting its bail function was not called after runRebuildPostRestorePhase completes. Keep the existing negative assertions for reconcileStalePinnedSessionModelsAfterRebuild and executeSandboxCommand.
🤖 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 `@src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts`:
- Around line 102-109: Strengthen the “does not run OpenClaw session
reconciliation for another agent” test by capturing the value returned from
input() and asserting its bail function was not called after
runRebuildPostRestorePhase completes. Keep the existing negative assertions for
reconcileStalePinnedSessionModelsAfterRebuild and executeSandboxCommand.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 26992a6d-d260-4579-a08c-79e06951b44b
📒 Files selected for processing (2)
src/lib/actions/sandbox/rebuild-post-restore-phase.test.tssrc/lib/actions/sandbox/rebuild-post-restore-phase.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/actions/sandbox/rebuild-post-restore-phase.ts
Co-authored-by: Julie Yaunches <jyaunches@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
jyaunches
approved these changes
Jul 17, 2026
jyaunches
left a comment
jyaunches
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the current exact head 8afd836 against base 0c98550. The Hermes rebuild now fails closed when recreated identity, final gateway health, or managed MCP restoration cannot be verified. Focused post-restore tests pass (11/11), CLI type-check and normal hooks pass, required CI and all four selected trusted E2E scenarios are green, CodeRabbit threads are resolved, the PR Review Advisor reports zero findings, and the security review found no blocking issues.
21 tasks
apurvvkumaria
pushed a commit
that referenced
this pull request
Jul 18, 2026
<!-- markdownlint-disable MD041 --> ## Summary Add the v0.0.87 changelog entry and align the DGX Station, platform-support, and rebuild documentation with behavior merged since v0.0.86. The Station documentation retains the Deferred support status while recording the two exact factory-image qualification profiles and the post-reboot receipt compatibility fix from #7130. ## Changes - Add the v0.0.87 changelog summary, including the merged Station resume receipt fix, with links to the owning documentation pages. - Document the exact April 2026 Colossus BaseOS and June 2026 AI Developer Tools Station identities, validation boundaries, and permitted host preparation. - Synchronize those Station qualification paths into the canonical platform matrix and generated provider/platform pages. - Document how an OpenClaw rebuild clears stale managed-provider session-model pins after an inference switch. ### Source summary - [#7130](#7130) -> `docs/changelog/2026-07-17.mdx`: Document compatibility with current six-field and legacy three-field Station resume receipts after host preparation. - [#7128](#7128) -> `docs/changelog/2026-07-17.mdx`: Document restart-safe managed DCode startup and required Docker resource limits. - [#7126](#7126) -> `docs/changelog/2026-07-17.mdx`, `docs/get-started/dgx-station-preparation.mdx`, `ci/platform-matrix.json`: Document the two bounded Station factory-image qualification profiles without promoting Deferred support and synchronize the generated platform/provider references. - [#6947](#6947) -> `docs/changelog/2026-07-17.mdx`: Document streaming sandbox backup archive creation. - [#7117](#7117) -> `docs/changelog/2026-07-17.mdx`: Document Hermes post-restore gateway and managed MCP health verification. - [#7109](#7109) -> `docs/changelog/2026-07-17.mdx`, `docs/manage-sandboxes/recover-rebuild-sandboxes.mdx`: Document stale managed session-model pin reconciliation after rebuild. - [#7068](#7068) -> `docs/changelog/2026-07-17.mdx`: Document strict-provider compatibility for Hermes tool schemas. - [#6965](#6965) -> `docs/changelog/2026-07-17.mdx`: Document managed vLLM download storage estimation. - [#7114](#7114) -> `docs/changelog/2026-07-17.mdx`: Document preserved, redacted rebuild diagnostics. ## 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: Documentation-only release-prep update; the changelog, platform-generation contracts, and docs build validate the changed pages and links. - [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 — `npx vitest run test/generate-platform-docs.test.ts test/station-doc-ownership.test.ts test/changelog-docs.test.ts`: 29 passed; `python3 scripts/generate-platform-docs.py --check`: all generated tables in sync - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [x] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added filesystem-aware managed vLLM storage preflight (cold download sizing; interactive vs non-interactive capacity checks). - Improved tool-schema compatibility for strict OpenAI-compatible providers (including Gemini schema handling) using a strict single envelope. - Enhanced sandbox backup creation with streamed archive generation and incremental entry validation. - **Bug Fixes** - Strengthened rebuild/recovery checks with Hermes sandbox health validation and cleanup of stale managed-provider session pins. - Persisted onboarding startup commands with required `nproc`/`nofile` limits across sandbox recreation. - Improved replacement-image rebuild diagnostics with bounded, redacted output handling. - For OpenCLAW “rebuild while preserving state,” stale model/provider pins are cleared when appropriate. - **Documentation** - Expanded DGX Station GB300 no-OTA factory profile/qualification criteria and clarified managed vLLM provider/sandbox constraints. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
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
2 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
Rebuild no longer reports a restored Hermes sandbox as ready before its saved agent identity, managed MCP configuration, and gateway health are verified. If those postconditions cannot be restored and proven, rebuild now exits nonzero with recovery guidance instead of returning a partial success. Credit to @mercl-lau for the public reproduction and expected-result report.
Related Issue
Fixes #7084
Changes
Type of Change
Quality Gates
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablerebuild-hermes-post-restore,rebuild-mcp-phase, andrebuild-flow); CLI typecheck passed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Targeted sandbox-rebuild suites were used because this is a scoped post-restore behavior change.npm run docsbuilds without warnings (doc changes only) — Build passed with 0 errors and 2 pre-existing Fern warnings.Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
rebuilddocs to describe preserved state, MCP restore behavior, nonzero exit conditions, and recommended follow-up commands for incomplete restorations.