Skip to content

fix(hermes): preserve MCP rebuild restart evidence - #8679

Merged
apurvvkumaria merged 1 commit into
mainfrom
codex/fix-8671-hermes-mcp-rebuild-intent
Aug 10, 2026
Merged

fix(hermes): preserve MCP rebuild restart evidence#8679
apurvvkumaria merged 1 commit into
mainfrom
codex/fix-8671-hermes-mcp-rebuild-intent

Conversation

@apurvvkumaria

@apurvvkumaria apurvvkumaria commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Hermes rebuilds no longer fail after a healthy gateway replacement merely because the generic restart checks persisted MCP intent before the rebuild restores the managed adapter projection. The restart result now carries explicit process-replacement and health evidence, so only that expected transitional mismatch can be superseded by MCP restoration; final MCP reconciliation remains mandatory and fail-closed.

Related Issue

Fixes #8671

Changes

  • Record restarted: true and healthPassed: true only when Hermes MCP reconciliation refuses after the supervisor restart marker and gateway health check have both passed.
  • Preserve that proven restart evidence while the rebuild restores the registered MCP provider, policy, and adapter state.
  • Keep pre-restart supervisor refusals classified as failed restarts, and keep final MCP drift, secret-boundary failures, process failures, and forward-recovery failures unverified.
  • Add positive and negative regression coverage for the transitional mismatch, post-restore drift, and the pre-restart refusal boundary.

The result provenance is required because MCP reconciliation refusal is also used for supervisor-side failures that occur before process replacement. The rebuild consumer therefore requires both explicit proofs instead of weakening reconciliation based on the failure label alone.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • 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:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: This corrects an internal rebuild state transition. Existing rebuild and MCP pages already describe restart, managed MCP restoration, and final verification in the corrected order.
  • 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: Independent nine-category security review passed with no findings; credential handling and the final fail-closed checks are unchanged.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: Existing docs/manage-sandboxes/recover-rebuild-sandboxes.mdx and docs/manage-sandboxes/manage-mcp-servers.mdx already document the corrected lifecycle and provider-backed MCP restoration.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable
  • Station profile/scenario: Not applicable
  • Result: Not applicable
  • Supporting evidence: Not applicable; no DGX Station host preparation changes.

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • 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
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification:
    • CLI rebuild, reconciliation, and restart tests: 84 passed.
    • Hermes config transaction, integrity-state, and credential-boundary integration tests: 52 passed under the repository-locked Python 3.14 dependencies.
    • Hermes MCP E2E-support lifecycle tests: 2 passed.
    • npm run typecheck:cli: passed.
    • npx prek run --from-ref main --to-ref HEAD: passed.
  • 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 localized result-contract change. A local npm run test:fast attempt was not claimed because an isolated untouched DCode finalization test currently fails two unrelated preference-merge assertions.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

The canonical live mcp-bridge (hermes) lane was not available for a PR branch because it requires the trusted main-only Hermes swap. CI and the next trusted-main lane provide the remaining process-level proof.


Signed-off-by: Apurv Kumaria akumaria@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved gateway recovery after managed MCP configuration mismatches.
    • Restarts are now recognized as successful when the gateway restarts and passes health checks, even if MCP reconciliation is refused.
    • MCP restoration and verification can continue after a healthy restart.
    • Persistent MCP drift after restoration is correctly rejected.
    • Recovery states now clearly distinguish verified success from unverified pre-restart refusals.

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@apurvvkumaria apurvvkumaria self-assigned this Aug 10, 2026
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b2ad9943-51ad-4109-939b-c8aae8c884ef

📥 Commits

Reviewing files that changed from the base of the PR and between 5003bce and 08709e9.

📒 Files selected for processing (4)
  • src/lib/actions/sandbox/gateway-restart-mcp.test.ts
  • src/lib/actions/sandbox/gateway-restart.ts
  • src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts
  • src/lib/actions/sandbox/rebuild-hermes-post-restore.ts

📝 Walkthrough

Walkthrough

Gateway restart results now distinguish MCP reconciliation refusals from supervisor integrity refusals. Hermes post-restore handling accepts successful restart evidence and continues MCP verification.

Changes

Hermes MCP restart handling

Layer / File(s) Summary
Gateway restart result contract
src/lib/actions/sandbox/gateway-restart.ts, src/lib/actions/sandbox/gateway-restart-mcp.test.ts
The MCP reconciliation refusal variant records restarted: true and healthPassed: true. Supervisor integrity refusals continue to omit both properties.
Hermes post-restore outcome handling
src/lib/actions/sandbox/rebuild-hermes-post-restore.ts, src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts
Hermes post-restore handling classifies an MCP reconciliation refusal as restarted only when both success flags are true. Tests cover successful recovery, persistent drift, and pre-restart refusal.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HermesPostRestore
  participant GatewayRestart
  participant MCPReconciliation
  HermesPostRestore->>GatewayRestart: restart gateway after state restore
  GatewayRestart->>MCPReconciliation: reconcile managed MCP intent
  MCPReconciliation-->>GatewayRestart: refusal with restarted and healthPassed
  GatewayRestart-->>HermesPostRestore: restart result
  HermesPostRestore->>HermesPostRestore: classify result and continue verification
Loading

Possibly related PRs

  • NVIDIA/NemoClaw#8472: Both modify Hermes post-restore gateway restart handling and its tests.
  • NVIDIA/NemoClaw#8661: Both modify Hermes post-restore restart verification and MCP reconciliation handling.

Suggested labels: integration: hermes, area: sandbox, bug-fix

Suggested reviewers: ericksoa, cv

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes preserving restart evidence for the Hermes MCP rebuild fix.
Linked Issues check ✅ Passed The changes address issue #8671 by preserving healthy restart evidence and enabling MCP restoration after temporary reconciliation refusal.
Out of Scope Changes check ✅ Passed All changes support the Hermes MCP rebuild and restart-reconciliation objectives in issue #8671.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-8671-hermes-mcp-rebuild-intent

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 08709e9 in the codex/fix-8671-herme... branch remains at 96%, unchanged from commit 5003bce in the main branch.

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized terminology decisions differ; normalized E2E selections differ; severity counts match.
1 terminology difference from the second opinion

Advisory only. These are normalized differences from the primary terminology receipt.

  • MCP refusal before gateway replacement at src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts:123: selected only by the second-opinion lane as justified.
1 additional E2E selection from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

  • hermes-e2e: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

Since last review: 0 prior items resolved · 0 still apply · 0 new items found

2 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

  • justified — restart evidence at src/lib/actions/sandbox/rebuild-hermes-post-restore.test.ts:89: Retain restart evidence for the paired restart and health result because it distinguishes completed replacement from a pre-restart refusal.
  • established — MCP reconciliation refusal at src/lib/actions/sandbox/gateway-restart.ts:77: Retain the established failure-layer term and use the explicit result fields to distinguish post-restart refusal.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite against this exact revision.

Recommended E2E: managed-image-multiarch-startup, managed-image-protected-runtime, onboard-repair, onboard-resume, rebuild-hermes, rebuild-openclaw, state-backup-restore

1 optional E2E recommendation
  • mcp-bridge

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@apurvvkumaria
apurvvkumaria merged commit d7ac19b into main Aug 10, 2026
65 of 69 checks passed
@apurvvkumaria
apurvvkumaria deleted the codex/fix-8671-hermes-mcp-rebuild-intent branch August 10, 2026 04:35
apurvvkumaria added a commit that referenced this pull request Aug 10, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Align the Hermes restart-drift regression test with the explicit restart
and health evidence added by #8679. This fixes the exact-result
assertion that failed after the production behavior merged.

## Related Issue
Follow-up to #8679 and #8671.

## Changes
- Expect `restarted: true` after the supervisor returns a valid restart
marker.
- Expect `healthPassed: true` after the recovered Hermes gateway passes
its health check.
- Preserve the assertions that no post-refusal mutations run and
secret-like output remains redacted.

## Type of Change

- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: This changes only an exact
test expectation for behavior already merged in #8679; runtime behavior,
CLI output, configuration, and recovery guidance are unchanged.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: Codex security review
passed with no findings; the test continues to require restart and
health evidence, forbid post-refusal mutations, and verify redaction.
- [ ] 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: `no-docs-needed`
- Evidence: The change is assertion-only and does not alter user-facing
behavior or existing recovery guidance.
- Agent: Codex Desktop
<!-- docs-review-head-sha: feb2e02 -->
<!-- docs-review-agents-blob-sha: c4923a3 -->

## 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:
affected CLI suites passed 33/33 under Python 3.14; the #8678
stale-recovery case passed 1/1 on the updated base; CLI type-check
passed.
- [ ] 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
- [ ] `npm run docs` builds without warnings (doc changes only)
- [ ] 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: Apurv Kumaria <akumaria@nvidia.com>

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@wscurran wscurran added bug-fix PR fixes a bug or regression area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery integration: hermes Hermes integration behavior labels Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression integration: hermes Hermes integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hermes MCP rebuild loses persisted managed intent on OpenShell v0.0.101

3 participants