fix(installer): preserve Hermes GPU fallback selection - #7710
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe 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. ChangesOpenShell service selection
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 03d1345 in the Show a code coverage summary of the most impacted files.
TypeScript / code-coverage/cliThe overall coverage in commit 03d1345 in the Show a code coverage summary of the most impacted files.
Updated |
Security review — exact head
|
| Severity | Finding |
|---|---|
| — | No security findings |
Detailed analysis
- Secrets and credentials — PASS. No secrets, tokens, credential values, or secret-bearing output were added. Diff-scoped gitleaks passed.
- 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. - Authentication and authorization — PASS / not applicable. No endpoint, identity, authorization, or permission behavior changed.
- Dependencies and supply chain — PASS. No dependency, lockfile, image, action, or downloaded artifact changed.
- Error handling and logging — PASS. Production logging is unchanged. Tests expose only controlled temporary fixture paths and do not serialize command arguments or credentials.
- Cryptography and data protection — PASS / not applicable. No cryptographic or persisted-data behavior changed.
- 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.
- 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.
- 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.shtest/e2e/support/hermes-gpu-startup-fallback.test.ts
Reviewed range: 8a0f36f612cd8c58e53c22fd6e52d10193ee593d...2fb69d8d1150d03118d3144ae7432975a67a6f17.
There was a problem hiding this comment.
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
📒 Files selected for processing (2)
scripts/install.shtest/e2e/support/hermes-gpu-startup-fallback.test.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported 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: 1 optional E2E recommendation
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>
Security review — exact head
|
| Severity | Finding |
|---|---|
| — | No security findings |
Detailed analysis
- Secrets and credentials — PASS. No secrets, tokens, credential values, or secret-bearing output were added. Diff-scoped gitleaks passed.
- 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. - Authentication and authorization — PASS / not applicable. No endpoint, identity, authorization, or permission behavior changed.
- Dependencies and supply chain — PASS. No dependency, lockfile, image, action, or downloaded artifact changed.
- Error handling and logging — PASS. Production logging is unchanged. Tests expose only controlled temporary fixture paths and do not serialize command arguments or credentials.
- Cryptography and data protection — PASS / not applicable. No cryptographic or persisted-data behavior changed.
- 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.
- 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.
- 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.shtest/e2e/support/hermes-gpu-startup-fallback.test.ts
Reviewed range: 8a0f36f612cd8c58e53c22fd6e52d10193ee593d...03d134564d14eccbd3a8a7b0f4e78181fa1e939d.
<!-- 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 -->
Summary
Source-checkout installs now retain an absolute executable selected through
NEMOCLAW_OPENSHELL_BINafter 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
if-missingpath after adding the user-local OpenShell directory toPATH.Type of Change
Quality Gates
NEMOCLAW_OPENSHELL_BINoverride contract and preserves the already-documented managed gateway-service behavior.Documentation Writer Review
no-docs-needed03d134564, the review confirmed thatdocs/reference/commands.mdxalready documents the OpenShell executable override anddocs/reference/architecture.mdxalready documents managed gateway-service staging. Rejecting an absolute directory as a binary does not change a supported user workflow.DGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shis unchanged.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 unavailablehermes-gpu-startup-fallback.test.tspassed 17/17 at exact head;install-preflight.test.tspassed 94/94; CLI type-check and diff-scoped hooks passed.npm testfor broad runtime/test-harness changes;npm run checkfor 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.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit