Skip to content

fix(cli): write Hermes light skin over stdin on macOS - #6837

Merged
cv merged 3 commits into
mainfrom
fix/hermes-light-skin-macos-exec
Jul 14, 2026
Merged

fix(cli): write Hermes light skin over stdin on macOS#6837
cv merged 3 commits into
mainfrom
fix/hermes-light-skin-macos-exec

Conversation

@laitingsheng

@laitingsheng laitingsheng commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

On macOS, nemohermes connect failed to write the managed Hermes light terminal skin: the skin helper passed a multi-line shell script as a sh -c argument, and OpenShell's macOS exec rejects command arguments containing a newline or carriage return. The write and remove helpers now stream the script over stdin via sh -s, so the skin is written as intended on macOS while Linux behaviour stays unchanged.

Related Issue

Fixes #6834

Changes

  • writeHermesLightSkinFile and removeHermesLightSkinFile now invoke openshell sandbox exec ... -- sh -s with the script supplied on stdin (input) and stdin piped, instead of sh -c "<multi-line script>". This mirrors the existing stdin pattern already used in tunnel/sandbox-gateway-stop.ts.
  • Updated the connect-hermes-light-theme test harness helpers and mock arms to read the script from the exec stdin rather than the -c argv slot.
  • Added a regression test asserting the forwarded argv is exactly sh -s, no argv element carries a newline or carriage return, and the multi-line script travels over stdin.

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: internal exec-transport fix; no user-facing behaviour, CLI surface, flags, or output change (the skin is written as originally intended).
  • 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: transport-only change (exec argv -c → stdin sh -s); the executed script is byte-identical and still runs as the sandbox user, with no change to privileges, credentials, or network surface. Requesting maintainer sensitive-path review.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration 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: npx vitest run src/lib/actions/sandbox/connect-hermes-light-theme.test.ts → 11/11 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: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved the reliability and security of Hermes light-skin setup and cleanup operations.
    • Prevented multi-line shell scripts and sensitive values from appearing in command arguments or logs.
    • Strengthened failure handling when applying or removing the light skin, while preserving existing cleanup behavior.

openshell sandbox exec rejects command arguments containing a newline or
carriage return on macOS, so the multi-line managed light-skin script never
reached the sandbox. Feed the script over stdin with `sh -s` instead of a
`sh -c` argument; the write and remove paths now pipe the script through
exec stdin.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@laitingsheng laitingsheng self-assigned this Jul 14, 2026
@coderabbitai

coderabbitai Bot commented Jul 14, 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: 123e14dc-1841-4c19-8be9-d94ad48bf555

📥 Commits

Reviewing files that changed from the base of the PR and between 2adc848 and b83c3c4.

📒 Files selected for processing (2)
  • src/lib/actions/sandbox/connect-hermes-light-skin.ts
  • src/lib/actions/sandbox/connect-hermes-light-theme.test.ts

📝 Walkthrough

Walkthrough

Hermes light-skin write and removal scripts now run through openshell stdin using sh -s. Tests inspect structured call options and verify multiline scripts are excluded from argv.

Changes

Hermes light-skin execution

Layer / File(s) Summary
Route skin scripts through stdin
src/lib/actions/sandbox/connect-hermes-light-skin.ts
Write and removal operations pass scripts through input with piped stdin while preserving existing error handling.
Validate structured openshell calls
src/lib/actions/sandbox/connect-hermes-light-theme.test.ts
Tests extract scripts from call options, update cleanup mocks, and verify multiline scripts are sent through stdin rather than argv.

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

Suggested reviewers: cjagwani

🚥 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 matches the main change: writing the Hermes light skin over stdin on macOS.
Linked Issues check ✅ Passed The PR addresses the macOS multiline-script rejection by switching to stdin and updates tests to verify the new shell input path.
Out of Scope Changes check ✅ Passed The changes stay focused on the macOS Hermes light-skin write/remove flow and related test updates.
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 fix/hermes-light-skin-macos-exec

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

@laitingsheng laitingsheng removed their assignment Jul 14, 2026
@laitingsheng laitingsheng added integration: hermes Hermes integration behavior area: cli Command line interface, flags, terminal UX, or output bug-fix PR fixes a bug or regression labels Jul 14, 2026
@github-code-quality

github-code-quality Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the main branch.

TypeScript / code-coverage/cli

The overall coverage in the fix/hermes-light-ski... branch remains at 79%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.
File main 20130dd fix/hermes-light-ski... 261e7fa +/-
src/lib/actions...e-validation.ts 81% 83% +2%
src/lib/adapters/http/probe.ts 94% 97% +3%
src/lib/adapter.../docker/pull.ts 83% 86% +3%
src/lib/onboard...eway-process.ts 90% 93% +3%
src/lib/sandbox...rce-identity.ts 89% 94% +5%
src/lib/onboard...shboard-port.ts 89% 94% +5%
src/lib/messagi...l-validation.ts 70% 77% +7%
src/lib/policy/tiers.ts 80% 94% +14%
src/lib/onboard...licy-presets.ts 81% 100% +19%
src/lib/hermes-dashboard.ts 63% 95% +32%

Updated July 14, 2026 17:42 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / medium 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 · medium confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Failed

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: onboard-repair, onboard-resume

1 optional E2E recommendation
  • hermes-e2e

Workflow run details

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

@laitingsheng laitingsheng added the NV QA Bugs found by the NVIDIA QA Team label Jul 14, 2026
@cv cv added the v0.0.83 label Jul 14, 2026
@cv
cv merged commit 7ce6d95 into main Jul 14, 2026
50 of 51 checks passed
@cv
cv deleted the fix/hermes-light-skin-macos-exec branch July 14, 2026 18:21
jyaunches added a commit that referenced this pull request Jul 14, 2026
## Summary

Add v0.0.83 release notes to `docs/about/release-notes.mdx` for pre-tag
release prep.

## Source Summary

- #6773 -> `docs/about/release-notes.mdx`: Shared inference route
changes are explicit and fail-safe; status shows recorded route, live
route, and drift.
- #6875 -> `docs/about/release-notes.mdx`: DGX Station GB300 express
setup restored; vLLM storage preflight narrowed.
- #6770 -> `docs/about/release-notes.mdx`: Risky Spark vLLM server
warning during onboarding.
- #6856 -> `docs/about/release-notes.mdx`: Re-onboard reuse preserves
tier-default brave/tavily presets.
- #6867 -> `docs/about/release-notes.mdx`: Unreachable custom endpoint
routed through transport-recovery path.
- #6860 -> `docs/about/release-notes.mdx`: Rebuild preflight uses
model-aware token field for o-series/GPT-5.
- #6845 -> `docs/about/release-notes.mdx`: Corporate CA anchored for
image build TLS.
- #6833 -> `docs/about/release-notes.mdx`: SSH ControlMaster-delegated
forwards recognized in fallback.
- #6837 -> `docs/about/release-notes.mdx`: Hermes light skin writes via
stdin on macOS.

## 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
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: doc-only release notes
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed
- [ ] Non-success, skipped, or missing CI check accepted by maintainer

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as Verified in GitHub
- [x] Normal pre-commit, commit-msg, and pre-push hooks passed
- [x] `npm run docs` passes with 0 errors

Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com>

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

Labels

area: cli Command line interface, flags, terminal UX, or output bug-fix PR fixes a bug or regression integration: hermes Hermes integration behavior NV QA Bugs found by the NVIDIA QA Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[macOS][CLI&UX] nemohermes connect fails to write Hermes light terminal skin — openshell exec rejects multi-line script on macOS

3 participants