Skip to content

fix(hermes): root staged OpenShell build context - #9915

Merged
prekshivyas merged 1 commit into
fix/hermes-openshell-sandbox-absencefrom
fix/hermes-openshell-build-context-root
Aug 21, 2026
Merged

fix(hermes): root staged OpenShell build context#9915
prekshivyas merged 1 commit into
fix/hermes-openshell-sandbox-absencefrom
fix/hermes-openshell-build-context-root

Conversation

@senthilr-nv

@senthilr-nv senthilr-nv commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • stage the rendered Hermes Dockerfile at the root of its reviewed build context
  • preserve source authority against agents/hermes/Dockerfile
  • bind onboarding fixtures to the new root-context Dockerfile receipt

What changed

OpenShell 0.0.106 derives the build context from the parent directory of the path passed to sandbox create --from. Hermes previously staged the Dockerfile under agents/hermes/, which excluded root-level reviewed inputs such as tools/mcp-tool-discovery-runtime/.../BUNDLED_PACKAGES.json from that inferred context.

The source Dockerfile remains reviewed at agents/hermes/Dockerfile, while the rendered staged copy is now published as <context>/Dockerfile. All allowlisted source assets remain below the inferred context root.

Why

Fresh Hermes Portable onboarding reached sandbox image creation but failed when Podman could not find the reviewed MCP bundle referenced by the Dockerfile. The asset was staged correctly; it was outside the narrower context OpenShell inferred from the nested Dockerfile path.

How tested

  • Hermes build-context and onboarding suites: 64 passed
  • affected CLI/plugin/E2E-support tests: 172 passed
  • codebase growth guardrails: 32 passed
  • CLI typecheck and CLI/plugin builds passed
  • Oxlint, formatting, repository architecture/safety checks, commit hooks, and pre-push hooks passed

Risks

The staged context receipt changes its Dockerfile relative path from agents/hermes/Dockerfile to Dockerfile. Context manifest hashing and transaction authority remain fail-closed, so an older staged generation cannot be silently reused.

Refs #9211

Signed-off-by: Senthil Ravichandran senthilr@nvidia.com

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
@senthilr-nv senthilr-nv self-assigned this Aug 21, 2026
@copy-pr-bot

copy-pr-bot Bot commented Aug 21, 2026

Copy link
Copy Markdown

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

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7f4e7340-a5cd-4816-8921-4243e8535e2a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

@github-code-quality

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit c021932 in the fix/hermes-openshell... branch remains at 96%, unchanged from commit c7af373 in the fix/hermes-openshell... branch.

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Blocking findings reported

Advisor assessment: Blockers require maintainer review
Next action: Review the blockers below.
Findings: 1 blocker · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 1 blocker · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings differ; normalized terminology decisions match; normalized E2E selections differ; Nemotron reported 1 fewer blocker, the same number of warnings, the same number of suggestions.
2 additional E2E selections from the second opinion

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

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

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite for the commit under review.

Recommended E2E: None

Manual-only E2E: onboard-repair, onboard-resume, cloud-onboard
The manual PR workflow does not run these selectors for the commit under review. Run them from reviewed code on main.

1 optional E2E recommendation
  • hermes-e2e

Blockers

PRA-1 Blocker — Update the installer-admission test for the root staged Dockerfile

  • Location: test/install-hermes-portable-active.test.ts:277
  • Category: security
  • Problem: The installer-admission integration test still expects the staged Dockerfile below `agents/hermes`, although the changed build-context plan stages and dispatches it as `Dockerfile` at the context root.
  • Impact: The checked-in installer/onboarding integration test rejects the intended `--from` value and no longer verifies the security-relevant staged build-context dispatch contract.
  • Fix: Change the integration assertion to `path.join(buildContextPath, "Dockerfile")`.
  • Verification: Compare the `createSandbox` assertion at `test/install-hermes-portable-active.test.ts:274-280` with the `dockerfilePath` returned by `materializeContext`.
  • Test coverage: Keep the installer-admission integration assertion and make it require `path.join(buildContextPath, "Dockerfile")`, so the real build-context plan and onboarding dispatch share one tested path contract.
  • Evidence: `test/install-hermes-portable-active.test.ts:274-278` expects `path.join(buildContextPath, "agents", "hermes", "Dockerfile")`. `src/lib/onboard/experimental/hermes-portable-build-context.ts:1315-1319` returns `dockerfilePath` from `authority.dockerfileRelativePath`, which now equals `Dockerfile`. `src/lib/onboard/experimental/hermes-portable-build-context.test.ts:115-126` verifies the root Dockerfile and absence of the former staged path.

Workflow run details

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

@senthilr-nv
senthilr-nv marked this pull request as ready for review August 21, 2026 18:18
@senthilr-nv senthilr-nv added bug-fix PR fixes a bug or regression integration: hermes Hermes integration behavior area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow area: packaging Packages, images, registries, installers, or distribution platform: container Affects Docker, containerd, Podman, or images v0.0.114 labels Aug 21, 2026
@prekshivyas
prekshivyas merged commit 074ca72 into fix/hermes-openshell-sandbox-absence Aug 21, 2026
80 of 86 checks passed
@prekshivyas
prekshivyas deleted the fix/hermes-openshell-build-context-root branch August 21, 2026 18:51
senthilr-nv pushed a commit that referenced this pull request Aug 21, 2026
<!-- markdownlint-disable MD041 -->
## Summary

The Hermes portable installer test now expects the root-level Dockerfile
that the reviewed build-context producer supplies to OpenShell. This
repairs the stale assertion left after #9915 moved that Dockerfile to
the build-context root.

## Related Issue

Related to #9211.

## Changes

- Expect `<build-context>/Dockerfile` in the portable Hermes installer
fixture.
- Preserve the existing assertion that `sandbox create --from` uses the
reviewed build context.

## Type of Change

- [x] 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

- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior, justification:
- [ ] Tests not applicable, justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable, justification: This test-only change aligns
with existing accepted production behavior and changes no user-facing
contract.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded, reviewer/approval link/justification: The nine-category review
found no production, credential, policy, authorization, dependency,
logging, or runtime-state change. The assertion restores coverage for
the reviewed build-context boundary.
- [ ] 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: `no-docs-needed`
- Evidence: `test/install-hermes-portable-active.test.ts` now expects
the existing root-level staged Hermes Dockerfile path. The merge commit
integrates base commit `0b427dec350d83568dfcf54bd01e86354807ed98`
without changing the test-only PR diff. No command, default,
configuration, API, policy, or documented behavior changes.
- Agent: Codex Desktop
<!-- docs-review-head-sha: 54c8652 -->
<!-- docs-review-agents-blob-sha: 513518c -->

## 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 validate:pr` passed after refreshing `origin/main` 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 --project installer-integration
test/install-hermes-portable-active.test.ts` passed 3 tests.
- [ ] 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
one-line test assertion repair.
- [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)
- [ ] 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: Rebecca Sliter <571084+rsliter@users.noreply.github.com>


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

## Summary by CodeRabbit

* **Tests**
* Updated the Hermes portable onboarding test to reflect the
Dockerfile’s generated location at the build-context root.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow area: packaging Packages, images, registries, installers, or distribution bug-fix PR fixes a bug or regression integration: hermes Hermes integration behavior platform: container Affects Docker, containerd, Podman, or images

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants