Skip to content

fix(hermes): preserve managed gateway restart recovery#7489

Merged
prekshivyas merged 4 commits into
mainfrom
fix/hermes-restart-health-timeout-7484
Jul 24, 2026
Merged

fix(hermes): preserve managed gateway restart recovery#7489
prekshivyas merged 4 commits into
mainfrom
fix/hermes-restart-health-timeout-7484

Conversation

@ericksoa

@ericksoa ericksoa commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

A managed Hermes restart could lose the replacement path when optional tracked children were absent under set -e, while the host reported only a generic health timeout for later launch, health, or auxiliary failures. This change keeps supervised recovery running, repairs both log streams, and returns bounded stage, PID, and fixed lifecycle evidence without trusting sandbox-writable diagnostics.

Related Issue

Fixes #7484

Changes

  • Make refresh_hermes_supervised_child_pids a total bookkeeping operation so a missing optional child cannot abort replacement launch under set -e, and repair both dashboard and gateway log streams with the other supervised auxiliaries.
  • Distinguish preparation, launch, gateway listener/health, auxiliary repair, exact-child stop, and quarantine events with fixed messages. The managed controller reads only full-matched events from a bounded descriptor-pinned tail owned by the exact re-proven supervisor UID; filesystem or process churn yields no excerpt.
  • Add staged GATEWAY_HEALTH_TIMEOUT and supervisor-loss diagnostics to the privileged helper while preserving the exact one-line SUPERVISOR_BUSY and SUPERVISOR_NOT_RUNNING contracts.
  • Classify staged supervisor loss separately on the host and apply URL plus full-secret redaction before rendering diagnostic detail.
  • Add installed-topology and adversarial coverage for non-root supervisor ownership, symlink/hardlink/FIFO and mode attacks, concurrent mutation, process churn, arbitrary or secret-bearing text, controls, suffixes, oversized lines, set -e recovery, auxiliary repair, marker compatibility, and host classification.
  • Exact-head CI/E2E must still prove the runtime boundary: stable supervisor/PID 1, a replaced gateway PID, and healthy internal API, public relay, dashboard, and forwards after restart.

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: Existing gateway lifecycle and troubleshooting pages already describe the restored managed Hermes recovery behavior and SUPERVISOR_UNAVAILABLE handling. The new bounded diagnostic fields add no command, flag, configuration, API, policy schema, or recovery action.
  • 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 review on exact head 5e2dd0603 passed with no open findings; evidence is recorded at fix(hermes): preserve managed gateway restart recovery #7489 (comment). Exact-head CI/E2E remains required before merge.
  • 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 gateway lifecycle and troubleshooting pages already cover the managed Hermes recovery and SUPERVISOR_UNAVAILABLE actions. The exact-head follow-up adds only explanatory code comments and integrates current main; it changes no documentation contract or recovery action.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit:
  • Station profile/scenario:
  • Result:
  • Supporting evidence:

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 check:diff passed 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: npx vitest run --project integration test/managed-gateway-control.test.ts test/hermes-gateway-supervisor-recovery.test.ts test/hermes-gateway-auxiliary-retry.test.ts — 53 tests passed; npx vitest run --project cli src/lib/actions/sandbox/gateway-restart.test.ts — 18 tests passed.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • 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)

Signed-off-by: Aaron Erickson aerickson@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved gateway recovery/restart behavior when supervisor services and log streams are temporarily unavailable.
    • Added clearer handling when a replacement gateway fails after health/listener readiness, including better stop-reasons.
  • Security
    • Enhanced failure diagnostics to include only bounded, sanitized excerpts, with secrets and control/terminal characters excluded.
  • Reliability
    • Gateway restart failures now preserve and surface control-stage details more consistently, including a distinct “supervisor unavailable” failure classification.
  • Tests
    • Expanded regression coverage for recovery, restart classification, and log-security behavior.

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Copilot AI review requested due to automatic review settings July 24, 2026 17:10
@ericksoa ericksoa added integration: hermes Hermes integration behavior area: observability Logging, metrics, tracing, diagnostics, or debug output area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression v0.0.95 labels Jul 24, 2026
@ericksoa ericksoa self-assigned this Jul 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Hermes recovery restores dashboard log streaming and tolerates missing optional child PIDs. Managed control emits staged, bounded diagnostics from trusted start logs, while gateway restart sanitizes evidence and classifies supervisor-unavailable failures separately.

Changes

Hermes restart recovery

Layer / File(s) Summary
Supervised replacement recovery
agents/hermes/start.sh, test/hermes-gateway-auxiliary-retry.test.ts, test/hermes-gateway-supervisor-recovery.test.ts
Auxiliary repair starts dashboard logging before gateway logging, explicitly succeeds when optional child PIDs are absent, distinguishes replacement validation failures, and updates recovery tests for the new flow and diagnostics.
Managed-control failure diagnostics
scripts/managed-gateway-control.py, test/managed-gateway-control.test.ts
Managed control validates supervisor-owned start-log metadata, reads bounded sanitized excerpts, attaches stages to health timeouts, and emits best-effort supervisor and gateway diagnostics with security-focused harness coverage.
Restart failure classification and sanitization
src/lib/actions/sandbox/gateway-restart.ts, src/lib/actions/sandbox/gateway-restart.test.ts
Gateway restart sanitization removes ANSI controls and URL tokens, and staged supervisor-unavailable output maps to a dedicated failure layer with regression tests for bounded evidence.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant restartSandboxGateway
  participant managed_gateway_control
  participant HermesSupervisor
  participant StartLog
  Client->>restartSandboxGateway: request managed gateway restart
  restartSandboxGateway->>managed_gateway_control: invoke restart control
  managed_gateway_control->>HermesSupervisor: replace gateway and await health
  HermesSupervisor-->>managed_gateway_control: health timeout or supervisor-unavailable error
  managed_gateway_control->>StartLog: read bounded trusted diagnostic tail
  StartLog-->>managed_gateway_control: sanitized evidence
  managed_gateway_control-->>restartSandboxGateway: staged failure output
  restartSandboxGateway-->>Client: classified failure and redacted detail
Loading

Suggested labels: area: e2e

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address the issue by preserving supervisor recovery, restoring log tails, adding bounded diagnostics, and covering the failure cases in tests.
Out of Scope Changes check ✅ Passed The added diagnostics and test updates support the restart recovery objectives; no clearly unrelated changes stand out.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main change: improving Hermes managed gateway restart recovery.
✨ 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 fix/hermes-restart-health-timeout-7484

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

@github-code-quality

github-code-quality Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 5e2dd06 in the fix/hermes-restart-h... branch remains at 96%, unchanged from commit fd07634 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 5e2dd06 in the fix/hermes-restart-h... branch remains at 80%, unchanged from commit 88b3e35 in the main branch.

Show a code coverage summary of the most impacted files.
File main 88b3e35 fix/hermes-restart-h... 5e2dd06 +/-
src/lib/messagi...nnels/policy.ts 100% 100% 0%
src/lib/onboard...der-metadata.ts 100% 100% 0%
src/lib/sandbox...rce-identity.ts 87% 87% 0%
src/lib/securit...ntial-filter.ts 93% 93% 0%
src/lib/shields...nsition-lock.ts 89% 89% 0%
src/lib/trace.ts 94% 94% 0%
src/lib/actions...eway-restart.ts 93% 95% +2%
src/lib/platform.ts 84% 89% +5%

Updated July 24, 2026 18:01 UTC

Comment thread scripts/managed-gateway-control.py Fixed
Comment thread scripts/managed-gateway-control.py Fixed
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · high 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 E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: full-e2e, hermes-e2e, security-posture, onboard-repair, onboard-resume

Workflow run details

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

ericksoa added 2 commits July 24, 2026 10:15
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
…alth-timeout-7484

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Copilot AI review requested due to automatic review settings July 24, 2026 17:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@ericksoa

Copy link
Copy Markdown
Contributor Author

Exact-head follow-up is now published at 22ef8ae789004298dc94ec8e60aed1b04e0d4ff2 and rebased forward via a signed/DCO merge of current main (a63ac38ed). GitHub reports all three PR commits as Verified.

The follow-up adds explanatory comments to the intentionally best-effort diagnostic fallback, resolving the duplicate empty-handler findings without changing behavior. Independent nine-category security review is GO with no findings. Fresh validation passed: 53/53 focused integration tests, 18/18 focused CLI tests, Python compile, ShellCheck, CLI typecheck, and full npm run check:diff. Documentation review is no-docs-needed at this exact head (AGENTS.md blob 9c9b36d7f).

Fresh exact-head CI/E2E is now running; this note records local/review evidence, not a claim that remote gates have completed.

…alth-timeout-7484

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Copilot AI review requested due to automatic review settings July 24, 2026 17:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@ericksoa

Copy link
Copy Markdown
Contributor Author

Fresh exact head 5e2dd06036b715125139fdcebe7eb44f6c0d5f14 is now published with current main (88b3e35ed); GitHub reports all four PR commits Verified and the PR mergeable. The net PR diff remains the same seven gateway implementation/test paths.

The prior selected E2E run was terminal on 22ef8ae: Hermes E2E, both security-posture lanes, and onboard-resume passed; onboard-repair lost its hosted runner; full-e2e hit the known current-main cold-onboard performance class tracked in #6660 (BuildKit layer export 122.6s, max silence 110s). That failure occurred before runtime and is outside this PR’s Hermes restart paths, but it was a real assertion, so I did not rerun the same SHA. This current-main merge creates the policy-compliant fresh exact-head evaluation.

Independent security review is GO with all nine categories passing. Fresh local validation passed: 53/53 focused integration tests, 18/18 focused CLI tests, Python compile, ShellCheck, CLI typecheck, and full npm run check:diff. Documentation review is no-docs-needed at 5e2dd0603 (AGENTS.md blob 9c9b36d7f). Fresh remote CI/E2E is now running.

@ericksoa

Copy link
Copy Markdown
Contributor Author

Exact-head E2E completed on 5e2dd06036b715125139fdcebe7eb44f6c0d5f14: 5/6 selected jobs passed — hermes-e2e, Hermes and OpenClaw security-posture, onboard-repair, and onboard-resume. This includes every restart-specific runtime/security lane; the prior onboard-repair runner-loss result did not recur.

The sole failure was the systemic #6660 full-e2e cold-build budget: install and expected first turn exited 0, BuildKit was used without fallback, but sandbox/root-to-first-turn measured 256.760s/310.049s vs 171s/228s, layer export took 96.6s, and max silence was 79s vs 60s. The controller correctly recorded no same-SHA retry reason. Comparative evidence is at #6660 (comment).

The existing layer-collapse fix #7486 reduces x86 layer export to 7.4s, but its prior gate did not select full-e2e; the prerequisite deterministic risk-selector fix is now in progress so #7486 can obtain the missing cold-path proof before this PR starts another exact-head cycle.

@prekshivyas
prekshivyas merged commit fac91e6 into main Jul 24, 2026
79 of 81 checks passed
@prekshivyas
prekshivyas deleted the fix/hermes-restart-health-timeout-7484 branch July 24, 2026 19:54
@senthilr-nv senthilr-nv mentioned this pull request Jul 25, 2026
23 tasks
senthilr-nv added a commit that referenced this pull request Jul 25, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Adds the canonical pre-tag `## v0.0.95` release entry to
`docs/changelog/2026-07-24.mdx`, before the existing v0.0.94 entry. The
entry summarizes approved user-visible changes merged since v0.0.94 and
excludes internal-only prerequisites.

## Changes

- Adds the v0.0.95 summary and detailed bullets for gateway lifecycle,
recovery, state transfer, inference compatibility, sandbox security,
Discord policy, and E2E evidence.
- Links each user-facing theme to the most specific published
documentation.
- Records the release entry in the shared native changelog used by the
OpenClaw, Hermes, and Deep Agents guides.

Source summary:

- [#7246](#7246),
[#7228](#7228),
[#7267](#7267),
[#7489](#7489),
[#7509](#7509),
[#7351](#7351), and
[#7290](#7290) ->
`docs/changelog/2026-07-24.mdx`: Gateway authority, forward teardown and
retry, managed recovery, Hermes restart recovery, scoped uninstall, and
orphan-aware backup behavior.
- [#7344](#7344) and
[#7416](#7416) ->
`docs/changelog/2026-07-24.mdx`: Atomic SQLite restore and host download
verification.
- [#7476](#7476),
[#7347](#7347),
[#7281](#7281),
[#7485](#7485),
[#7491](#7491), and
[#7422](#7422) ->
`docs/changelog/2026-07-24.mdx`: Windows Ollama reuse, CDI fallback,
bounded OpenRouter connection setup, Nemotron-3 request compatibility,
and managed Deep Agents retry and provider-error behavior.
- [#6884](#6884),
[#7481](#7481),
[#6878](#6878),
[#7467](#7467),
[#7502](#7502),
[#7503](#7503),
[#7504](#7504), and
[#7486](#7486) ->
`docs/changelog/2026-07-24.mdx`: Trusted base-image overrides, local
rebuild images, runtime validation, config preservation, reviewed
package updates, and fewer final-image payload layers.
- [#7303](#7303) ->
`docs/changelog/2026-07-24.mdx`: Scoped Discord application-command
management.
- [#7488](#7488),
[#7465](#7465),
[#7497](#7497),
[#7464](#7464),
[#7501](#7501),
[#7494](#7494), and
[#7493](#7493) ->
`docs/changelog/2026-07-24.mdx`: Selected-test risk signals, retry
cleanup, full root-image validation, direct-main Hermes setup, executed
PR-gate evidence, nightly history, and runner wait reporting.
- [#7447](#7447) is an internal
pinned-runtime prerequisite and is intentionally excluded from canonical
supported-integration documentation.
- [#7370](#7370) adds
maintainer-only advisory reconciliation tooling and does not change
supported user behavior.
- [#7495](#7495) updates existing
documentation and does not add a new v0.0.95 behavior claim.

## 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 structure,
heading uniqueness, and published links.
- [ ] 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-24.mdx`; writing rules,
documentation style, factual release meaning, and published links
reviewed at exact head `58b02f2bf`.
- Agent: Codex documentation writer reviewer
<!-- docs-review-head-sha: 58b02f2 -->
<!-- docs-review-agents-blob-sha: 9c9b36d -->

## 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 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 — command/result or justification: `npx
vitest run test/changelog-docs.test.ts` passed 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:
- [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) — the
build passed with 0 errors and 2 Fern 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: Senthil Ravichandran <senthilr@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Documentation**
  * Added a new v0.0.95 changelog entry above v0.0.94.
* Documented improved externally supervised gateway lifecycle ownership.
  * Improved snapshot restore reliability and SQLite state handling.
  * Tightened CLI `backup-all` behavior and host artifact verification.
* Updated Windows onboarding guidance (including Ollama service reuse
and CDI directory fallback).
* Noted inference compatibility fixes, deeper agent failure
classification, stricter base-image validation, updated Discord bot
command permissions, and refined E2E release automation evidence
handling.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: observability Logging, metrics, tracing, diagnostics, or debug output 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][Sandbox] managed gateway restart can health-timeout after the supervised gateway exits

4 participants