fix(rebuild): keep Hermes MCP runtime active after rebuild

[apurvvkumaria]

Summary

This draft adds one-shot, credential-redacted diagnostics for the Hermes MCP request that fails immediately after a successful sandbox rebuild. The evidence distinguishes response status/body, result-token, and exact authenticated tools/call reachability failures without retrying the paid completion request. The remaining work will confirm the causal boundary and add the narrow rebuild fix before this draft is marked ready.

Related Issue

Fixes #8697

Changes

• Send exactly one authenticated Hermes /v1/chat/completions probe and preserve its HTTP status plus a redacted, byte-limited failure body while keeping successful response contents out of artifacts.
• Reuse the fixture's existing capture bound and common assertions to require one new authenticated fixture tools/call at the managed /mcp endpoint after an HTTP success.
• Add focused coverage for one-request behavior, API-key redaction, HTTP 500 evidence, duplicate status markers, and result-token validation.
• Keep the net diagnostic change to three files and 251 added lines, down from the initial 724-line draft.

Current root-cause hypothesis: the scenario's pre-existing 15-minute Shields auto-lock begins before rebuild, while the rebuild path treats both mutable-default and temporarily-unlocked postures as already down and does not capture the old timer generation. The recorded rebuild holds the lifecycle lock across the old deadline, so the timer waits; after rebuild releases that lock, overdue recovery can harden the replacement before the first tool-backed request even though config/hash and gateway-health checks passed. The new HTTP and runtime evidence will confirm whether that handoff also causes the HTTP 500 before the fix is committed.

Evidence	Current observation	Diagnostic added or remaining
Candidate	68129579827191770cfcf496715e4e272bc96129, GitHub Verified	Reused as the exact failing candidate
Live run	Run 31357091338, Hermes job 93361110199	Existing artifact confirms rebuild success followed by HTTP 500
Rebuild timing	Existing Shields window predates an 883.8-second rebuild and expires 83–89 seconds before rebuild completion	Capture the overdue timer handoff, final Shields posture, and runtime logs around the request
HTTP request	Existing artifact contains only curl exit 22 and HTTP 500	Capture one status and redacted, bounded response body without retry
MCP fixture	Pre-rebuild discovery succeeds; post-rebuild call reachability is absent from the artifact	Require exactly one authenticated post-rebuild tools/call after HTTP success
Remaining implementation	Causal boundary not yet fixed in this first commit	Add the smallest timer/runtime fix, regression tests, final docs and security reviews, and exact-head live evidence

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: the net change only improves Hermes live-E2E diagnostics and assertions; it does not change a supported product or contributor contract.
• 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: pending after the causal fix
• 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: the remaining change affects only Hermes live-E2E diagnostics and assertions; no supported CLI, API, configuration, default, or runtime contract changes.
• 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 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 — npx vitest run --project e2e-support test/e2e/support/mcp-bridge-hermes-http.test.ts (2 tests passed)
• Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — pending the causal fix
• 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: Apurv Kumaria akumaria@nvidia.com

Summary by CodeRabbit

• Tests
  • Added end-to-end coverage for Hermes MCP HTTP requests.
  • Added validation for authentication, HTTP status handling, result tokens, retries, redaction, and truncated failure output.
  • Improved adapter tool-call testing with bounded response capture and specialized validation.

[copy-pr-bot]

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.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds Hermes MCP HTTP probe helpers, integrates them into live adapter tests, and adds coverage for authenticated requests, bounded redacted failures, HTTP status validation, and result-token validation.

Changes

Hermes MCP HTTP handling

Layer / File(s)	Summary
Probe generation and response validation test/e2e/live/mcp-bridge-hermes-http.ts	The helper builds authenticated Hermes requests, limits captured output, redacts failure bodies, parses HTTP status markers, and validates result tokens.
Hermes adapter integration test/e2e/live/mcp-bridge.test.ts	The Hermes test uses the shared probe, expanded redaction values, bounded capture, and Hermes-specific response assertions. Other adapters retain their existing checks.
Probe behavior coverage test/e2e/support/mcp-bridge-hermes-http.test.ts	Mocked curl tests verify single-request behavior, authentication, redaction, truncated HTTP 500 diagnostics, marker validation, and result-token handling.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant LiveTest as Hermes live test
  participant Probe as Hermes HTTP probe
  participant Gateway as Hermes gateway
  participant Fixture as MCP fixture
  LiveTest->>Probe: Build payload and expected result-token probe
  Probe->>Gateway: Send one authenticated chat request
  Gateway->>Fixture: Invoke MCP tool
  Fixture-->>Gateway: Return tool result
  Gateway-->>Probe: Return HTTP status and response body
  Probe-->>LiveTest: Return bounded command result
  LiveTest->>Probe: Validate status, redaction, and result token

Loading

Possibly related PRs

• NVIDIA/NemoClaw#8659 — Extends and tests the same Hermes MCP HTTP probe, response validation, redaction, and adapter integration.

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

Suggested reviewers: prekshivyas, cv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The changes add bounded redacted diagnostics and focused tests, but do not fix or prove the post-rebuild HTTP 500 acceptance criteria in issue #8697.	Implement the causal Hermes runtime fix and verify the first post-rebuild request, single authenticated tools/call, fail-closed behavior, logs, cleanup, and regression coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title identifies the Hermes rebuild runtime issue addressed by the diagnostics and post-rebuild validation changes.
Out of Scope Changes check	✅ Passed	The changes are limited to Hermes MCP diagnostic helpers and focused end-to-end tests described by issue #8697.
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-8697-hermes-post-rebuild-tool-call

---

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

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 0bf6625 in the codex/fix-8697-herme... branch remains at 96%, unchanged from commit fa2b577 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 0bf6625 in the codex/fix-8697-herme... branch remains at 82%, unchanged from commit fa2b577 in the main branch.

Show a code coverage summary of the most impacted files.

File	main fa2b577	codex/fix-8697-herme... 0bf6625	+/-
src/lib/policy/index.ts	67%	67%	0%
src/lib/inferen...ter-topology.ts	89%	89%	0%
src/lib/inferen...og-integrity.ts	100%	100%	0%
src/lib/inferen...ter-registry.ts	95%	96%	+1%
src/lib/inferen...-ssh-binding.ts	87%	89%	+2%
src/lib/policy/...y-validation.ts	97%	100%	+3%
src/lib/inferen...talog-loader.ts	80%	84%	+4%
src/lib/inferen...er-lifecycle.ts	70%	77%	+7%
src/lib/inferen...del-registry.ts	93%	100%	+7%
src/lib/inferen...time-context.ts	84%	93%	+9%

---

_{Updated August 10, 2026 07:38 UTC}

[github-actions]

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 · 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 terminology decisions differ; normalized E2E selections differ; severity counts match.

3 terminology differences from the second opinion

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

• status marker at test/e2e/live/mcp-bridge-hermes-http.ts:88: selected only by the second-opinion lane as define.
• result token at test/e2e/live/mcp-bridge-hermes-http.ts:97: selected only by the second-opinion lane as define.
• failure preview at test/e2e/live/mcp-bridge.test.ts:547: selected only by the second-opinion lane as define.

2 additional E2E selections from the second opinion

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

• rebuild-hermes: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
• rebuild-hermes-stale-base: 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.

2 semantic terminology decisions

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

• established — real MCP tool call at test/e2e/live/mcp-bridge-hermes-http.ts:85: Keep the established term for agent-driven MCP tool invocation.
• justified — HTTP status marker at test/e2e/live/mcp-bridge-hermes-http.ts:88: Keep this term because it identifies the status record that the assertion validates.

E2E guidance

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

Recommended E2E: cloud-inference, cloud-onboard, security-posture, mcp-bridge, mcp-bridge-dev

Workflow run details

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

[coderabbitai]

🧹 Nitpick comments (2)

test/e2e/support/mcp-bridge-hermes-http.test.ts (2)

35-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove source-text assertions.

The fake curl at Lines 54-57 and the count assertion at Line 87 already verify authentication and one request through the shell boundary. Remove these generated-script text checks.

Proposed change

-    expect(script.match(/\bcurl "\$@"/gu)).toHaveLength(1);
-    expect(script).not.toMatch(/\bretry\b/iu);
-    expect(script).toContain("Authorization: Bearer ${API_SERVER_KEY}");

As per path instructions, prefer observable outcomes through the public boundary over source-text assertions.

🤖 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/mcp-bridge-hermes-http.test.ts` around lines 35 - 37, Remove
the generated-script source-text assertions in the affected test, including the
script.match count, retry absence, and Authorization string checks. Retain the
fake curl boundary setup and the existing request-count assertion, which already
verify authentication and a single request through observable behavior.

Source: Path instructions

---

109-120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test duplicate result-token rejection.

hasResultToken requires exactly one NEMOCLAW_HERMES_MCP_RESULT_TOKEN=present marker. Add a case with two present markers and assert that assertHermesMcpHttpResponse throws. This protects the single-marker protocol rule.

🤖 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/mcp-bridge-hermes-http.test.ts` around lines 109 - 120, Add
a test beside the existing result-token assertions that passes two
NEMOCLAW_HERMES_MCP_RESULT_TOKEN=present markers to assertHermesMcpHttpResponse
and verifies it throws for violating the exactly-one-marker protocol rule.

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/e2e/support/mcp-bridge-hermes-http.test.ts`:
- Around line 35-37: Remove the generated-script source-text assertions in the
affected test, including the script.match count, retry absence, and
Authorization string checks. Retain the fake curl boundary setup and the
existing request-count assertion, which already verify authentication and a
single request through observable behavior.
- Around line 109-120: Add a test beside the existing result-token assertions
that passes two NEMOCLAW_HERMES_MCP_RESULT_TOKEN=present markers to
assertHermesMcpHttpResponse and verifies it throws for violating the
exactly-one-marker protocol rule.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 00d981ea-6284-435c-80d5-561bf3ef3016

📥 Commits

Reviewing files that changed from the base of the PR and between 04e28aa and 2793ab4.

📒 Files selected for processing (3)

• test/e2e/live/mcp-bridge-hermes-http.ts
• test/e2e/live/mcp-bridge.test.ts
• test/e2e/support/mcp-bridge-hermes-http.test.ts