Skip to content

fix(installer): preserve Hermes GPU fallback selection - #7710

Merged
apurvvkumaria merged 2 commits into
mainfrom
codex/hermes-gpu-fallback-events
Jul 28, 2026
Merged

fix(installer): preserve Hermes GPU fallback selection#7710
apurvvkumaria merged 2 commits into
mainfrom
codex/hermes-gpu-fallback-events

Conversation

@apurvvkumaria

@apurvvkumaria apurvvkumaria commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Source-checkout installs now retain an absolute executable selected through NEMOCLAW_OPENSHELL_BIN after refreshing user-local OpenShell discovery, while continuing to stage the existing gateway service. This prevents the Hermes GPU fallback test from silently bypassing its fault-injection wrapper and exercising the native route instead.

Related Issue

Related to #7140.
Follow-up to #6333.

Changes

  • Preserve the caller-selected absolute OpenShell executable in the source-checkout if-missing path after adding the user-local OpenShell directory to PATH.
  • Keep the feat(gateway): manage the default gateway service #7319 gateway-service staging call and the fresh/pinned OpenShell installation path unchanged.
  • Add focused E2E-support coverage proving that service staging observes the fallback wrapper, the real gateway component, and the user-local discovery path.
  • Reject restoration of a relative executable selection so invalid overrides continue to fall back to the discovered user-local OpenShell binary.

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 restores the existing documented NEMOCLAW_OPENSHELL_BIN override contract and preserves the already-documented managed gateway-service behavior.
  • 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: Exact-head nine-category security review passed with no findings: fix(installer): preserve Hermes GPU fallback selection #7710 (comment)
  • 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: At exact head 03d134564, the review confirmed that docs/reference/commands.mdx already documents the OpenShell executable override and docs/reference/architecture.mdx already documents managed gateway-service staging. Rejecting an absolute directory as a binary does not change a supported user workflow.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable; scripts/prepare-dgx-station-host.sh is unchanged.
  • Station profile/scenario: Not applicable.
  • Result: Not applicable.
  • Supporting evidence: Not applicable.

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 — hermes-gpu-startup-fallback.test.ts passed 17/17 at exact head; install-preflight.test.ts passed 94/94; CLI type-check and diff-scoped hooks 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 two-file source-checkout selection repair; focused tests and diff-scoped hooks cover the changed behavior.
  • 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

  • Bug Fixes
    • Improved installer handling when a custom OpenShell executable is configured.
    • Preserved valid absolute executable selections during managed gateway setup.
    • Added safeguards to fall back to the correct installed OpenShell binary when configured paths are relative or point to directories.
    • Improved reliability of Hermes GPU startup fallback service selection.

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

coderabbitai Bot commented Jul 28, 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: 8d426378-7a1b-4e35-91f1-089c9150c03e

📥 Commits

Reviewing files that changed from the base of the PR and between 2fb69d8 and 03d1345.

📒 Files selected for processing (2)
  • scripts/install.sh
  • test/e2e/support/hermes-gpu-startup-fallback.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • test/e2e/support/hermes-gpu-startup-fallback.test.ts
  • scripts/install.sh

📝 Walkthrough

Walkthrough

The installer preserves a valid explicitly selected OpenShell binary during macOS gateway service staging. End-to-end tests cover fallback-wrapper reporting and normalization of invalid relative or directory selections.

Changes

OpenShell service selection

Layer / File(s) Summary
Preserve valid OpenShell binary selection
scripts/install.sh
The installer captures NEMOCLAW_OPENSHELL_BIN and uses it during the macOS fallback path only when it is an absolute executable.
Validate service staging selection
test/e2e/support/hermes-gpu-startup-fallback.test.ts
End-to-end tests verify service-selection output, fallback-wrapper selection, and replacement of relative or directory paths with the absolute temporary openshell binary.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: area: install

🚥 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 is clear and matches the main installer fix around preserving the Hermes GPU fallback selection.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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/hermes-gpu-fallback-events

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

@github-code-quality

github-code-quality Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 03d1345 in the codex/hermes-gpu-fal... branch remains at 96%, unchanged from commit c3ab052 in the main branch.

Show a code coverage summary of the most impacted files.
File main c3ab052 codex/hermes-gpu-fal... 03d1345 +/-
nemoclaw/src/plugin-config.ts 100% 0% -100%
nemoclaw/src/index.ts 94% 94% 0%

TypeScript / code-coverage/cli

The overall coverage in commit 03d1345 in the codex/hermes-gpu-fal... branch is 80%. The coverage in commit c3ab052 in the main branch is 81%.

Show a code coverage summary of the most impacted files.
File main c3ab052 codex/hermes-gpu-fal... 03d1345 +/-
src/lib/onboard...rovider-shim.ts 67% 33% -34%
src/lib/onboard...andbox-reuse.ts 86% 61% -25%
src/lib/state/registry.ts 93% 85% -8%
src/lib/state/o...d-checkpoint.ts 90% 87% -3%
src/lib/onboard.ts 31% 31% 0%
src/lib/onboard...lers/sandbox.ts 96% 97% +1%
src/lib/onboard...itial-policy.ts 94% 95% +1%
src/lib/actions...ard-recovery.ts 93% 95% +2%
src/lib/state/r...ry-messaging.ts 48% 79% +31%
src/lib/policy/context.ts 0% 91% +91%

Updated July 28, 2026 16:45 UTC

@apurvvkumaria

Copy link
Copy Markdown
Collaborator Author

Security review — exact head 2fb69d8d1150d03118d3144ae7432975a67a6f17

Verdict

PASS — safe to merge from a security perspective, subject to required CI and maintainer review.

The change restores the existing, documented NEMOCLAW_OPENSHELL_BIN selection only when it is an absolute path to an executable file. It does not broaden the executable trust boundary, change privilege handling, or alter the fresh/pinned OpenShell installation path. The existing managed gateway-service staging remains in place and continues to resolve the real gateway sibling.

Findings

Severity Finding
No security findings

Detailed analysis

  1. Secrets and credentials — PASS. No secrets, tokens, credential values, or secret-bearing output were added. Diff-scoped gitleaks passed.
  2. Input validation — PASS. Restoration requires both an absolute path (/*) and an executable file (-x). Regression coverage confirms a relative executable selection is rejected in favor of normal user-local discovery.
  3. Authentication and authorization — PASS / not applicable. No endpoint, identity, authorization, or permission behavior changed.
  4. Dependencies and supply chain — PASS. No dependency, lockfile, image, action, or downloaded artifact changed.
  5. Error handling and logging — PASS. Production logging is unchanged. Tests expose only controlled temporary fixture paths and do not serialize command arguments or credentials.
  6. Cryptography and data protection — PASS / not applicable. No cryptographic or persisted-data behavior changed.
  7. Configuration and service integrity — PASS. The existing gateway-service staging call is retained. Tests verify it receives the preserved wrapper selection, the real sibling gateway binary, and the user-local-first discovery path.
  8. Security testing — PASS. Focused tests cover accepted absolute selection, rejected relative selection, service staging, real gateway resolution, and PATH behavior. Existing fallback tests continue to assert that secret markers and credential-shaped values are absent from artifacts.
  9. System security — PASS. The forced/fresh install route is unchanged, no new privilege boundary is introduced, and the change remains inside the existing caller-controlled executable override contract.

Files reviewed

  • scripts/install.sh
  • test/e2e/support/hermes-gpu-startup-fallback.test.ts

Reviewed range: 8a0f36f612cd8c58e53c22fd6e52d10193ee593d...2fb69d8d1150d03118d3144ae7432975a67a6f17.

@coderabbitai coderabbitai Bot 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.

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 `@scripts/install.sh`:
- Around line 1540-1541: Reject absolute directory paths in the OpenShell
override validation by requiring both a regular file and executable permissions
in the install script condition around explicit_openshell_bin. Add an
absolute-directory test case to hermes-gpu-startup-fallback.test.ts that
verifies the fallback resolves to ${realDir}/openshell.
🪄 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: 60c5bc1f-f80c-4343-a38b-94590761dc08

📥 Commits

Reviewing files that changed from the base of the PR and between 8a0f36f and 2fb69d8.

📒 Files selected for processing (2)
  • scripts/install.sh
  • test/e2e/support/hermes-gpu-startup-fallback.test.ts

Comment thread scripts/install.sh Outdated
@github-actions

github-actions Bot commented Jul 28, 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 · 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 match; 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: cloud-inference, cloud-onboard, security-posture

1 optional E2E recommendation
  • hermes-gpu-startup

Workflow run details

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

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

Copy link
Copy Markdown
Collaborator Author

Security review — exact head 03d134564d14eccbd3a8a7b0f4e78181fa1e939d

Verdict

PASS — safe to merge from a security perspective, subject to required CI and maintainer review.

The change restores the existing, documented NEMOCLAW_OPENSHELL_BIN selection only when it is an absolute path to an executable regular file. It does not broaden the executable trust boundary, change privilege handling, or alter the fresh/pinned OpenShell installation path. The existing managed gateway-service staging remains in place and continues to resolve the real gateway sibling.

Findings

Severity Finding
No security findings

Detailed analysis

  1. Secrets and credentials — PASS. No secrets, tokens, credential values, or secret-bearing output were added. Diff-scoped gitleaks passed.
  2. Input validation — PASS. Restoration requires an absolute path (/*) to a regular file (-f) with executable permissions (-x). Regression coverage confirms both relative executable selections and absolute directories are rejected in favor of normal user-local discovery.
  3. Authentication and authorization — PASS / not applicable. No endpoint, identity, authorization, or permission behavior changed.
  4. Dependencies and supply chain — PASS. No dependency, lockfile, image, action, or downloaded artifact changed.
  5. Error handling and logging — PASS. Production logging is unchanged. Tests expose only controlled temporary fixture paths and do not serialize command arguments or credentials.
  6. Cryptography and data protection — PASS / not applicable. No cryptographic or persisted-data behavior changed.
  7. Configuration and service integrity — PASS. The existing gateway-service staging call is retained. Tests verify it receives the preserved wrapper selection, the real sibling gateway binary, and the user-local-first discovery path.
  8. Security testing — PASS. Focused tests cover accepted absolute-file selection, rejected relative selection, rejected absolute-directory selection, service staging, real gateway resolution, and PATH behavior. Existing fallback tests continue to assert that secret markers and credential-shaped values are absent from artifacts.
  9. System security — PASS. The forced/fresh install route is unchanged, no new privilege boundary is introduced, and the change remains inside the existing caller-controlled executable override contract.

Files reviewed

  • scripts/install.sh
  • test/e2e/support/hermes-gpu-startup-fallback.test.ts

Reviewed range: 8a0f36f612cd8c58e53c22fd6e52d10193ee593d...03d134564d14eccbd3a8a7b0f4e78181fa1e939d.

@apurvvkumaria
apurvvkumaria merged commit ba4ef68 into main Jul 28, 2026
120 of 127 checks passed
@apurvvkumaria
apurvvkumaria deleted the codex/hermes-gpu-fallback-events branch July 28, 2026 17:07
@cjagwani cjagwani added v0.0.98 and removed v0.0.97 labels Jul 28, 2026
@wscurran wscurran added area: e2e End-to-end tests, nightly failures, or validation infrastructure area: install Install, setup, prerequisites, or uninstall flow area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression integration: hermes Hermes integration behavior labels Jul 28, 2026
@cjagwani cjagwani mentioned this pull request Jul 28, 2026
23 tasks
cv pushed a commit that referenced this pull request Jul 28, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Add the canonical dated changelog entry for NemoClaw v0.0.97 before the
release plan captures `origin/main`.
The entry groups the user-visible and maintainer-facing changes since
v0.0.96 while preserving the Deferred dual-Station status, experimental
runtime-identity boundary, and pending physical IGX validation.

## Changes

- Add `docs/changelog/2026-07-28.mdx` with the parser-safe MDX SPDX
comment and exact `## v0.0.97` heading.
- Summarize the 43 merged PRs in the release range, omitting
internal-only changes from the public entry and linking each grouped
change to its most specific published documentation.
- Keep the experimental Okta reference explicitly opt-in and outside
normal onboarding, keep the two-Station path Deferred, and state that
physical IGX Orin validation remains pending.

### Source summary

- [#7440](#7440),
[#7443](#7443), and
[#7445](#7445) ->
`docs/changelog/2026-07-28.mdx`: Document read-only host readiness
reports and fail-closed platform qualification.
- [#7030](#7030) ->
`docs/changelog/2026-07-28.mdx`: Document the Deferred trusted
two-Station vLLM evaluation.
- [#7265](#7265) ->
`docs/changelog/2026-07-28.mdx`: Document the bounded experimental
direct-runner Okta runtime-identity reference.
- [#7711](#7711) and
[#7648](#7648) ->
`docs/changelog/2026-07-28.mdx`: Document compatible-endpoint reasoning
effort and retired NVIDIA Build model paths.
- [#7746](#7746),
[#7763](#7763), and
[#7681](#7681) ->
`docs/changelog/2026-07-28.mdx`: Document safe compatible-provider
creation, replacement refusal, and narrow OpenShell bridge URL handling.
- [#7641](#7641),
[#7690](#7690),
[#7631](#7631), and
[#7710](#7710) ->
`docs/changelog/2026-07-28.mdx`: Document paused-container recovery,
recreation journaling, pre-mutation uninstall checks, and
source-checkout OpenShell selection.
- [#7624](#7624) and
[#7762](#7762) ->
`docs/changelog/2026-07-28.mdx`: Document Jetson release diagnostics and
bounded render-device group propagation.
- [#7639](#7639),
[#7760](#7760),
[#7721](#7721), and
[#7761](#7761) ->
`docs/changelog/2026-07-28.mdx`: Document Telegram, MCP media-type,
Hermes image-mode, and locked-restart fixes.
- [#7653](#7653) and
[#7680](#7680) ->
`docs/changelog/2026-07-28.mdx`: Document Deep Agents policy tasks and
the bounded Claude Code OAuth path.
- [#7679](#7679) ->
`docs/changelog/2026-07-28.mdx`: Document the checksum-bound libssh2 and
Python HTMLParser backports.
- [#7655](#7655),
[#7651](#7651),
[#7664](#7664),
[#7666](#7666),
[#7670](#7670),
[#7719](#7719), and
[#7741](#7741) ->
`docs/changelog/2026-07-28.mdx`: Document exact candidate E2E evidence,
Launchable selection, diagnostic consolidation, and trusted WSL
validation.

## 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,
MDX header, heading uniqueness, and release-entry structure.
- [ ] 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: The committed `docs/changelog/2026-07-28.mdx` blob exactly
matches the reviewed file. Completeness, factual accuracy, link shape,
parser-safe MDX header, one-sentence-per-line style, `.docs-skip`
compliance, and bounded product claims passed.
- Agent: Codex Desktop documentation writer subagent
<!-- docs-review-head-sha: da6aa27 -->
<!-- docs-review-agents-blob-sha: be20a09 -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable; this PR changes only the dated
changelog.
- Station profile/scenario: Not applicable.
- Result: Not applicable.
- Supporting evidence: Not applicable.

## 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/changelog-docs.test.ts` passed 6/6.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — not applicable to this doc-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) —
completed with 0 errors and 2 pre-existing 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)
— native changelog entries use the required parser-safe MDX SPDX comment
and intentionally have no frontmatter.

---
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>


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

## Summary by CodeRabbit

* **New Features**
* Added improved host readiness reporting and Jetson onboarding
guidance.
* Added controls for reasoning effort with compatible endpoints and
enhanced managed MCP discovery.
  * Improved Deep Agents task publication and preset support.
* **Bug Fixes**
* Hardened provider switching, sandbox recovery, uninstall behavior, and
Telegram connectivity.
* Improved container image integrity checks, media-type handling, and
checksum validation.
  * Enhanced vLLM evaluation behavior and release diagnostics.
* **Documentation**
  * Added the NemoClaw v0.0.97 changelog.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: e2e End-to-end tests, nightly failures, or validation infrastructure area: install Install, setup, prerequisites, or uninstall flow 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.

4 participants