ci(main): install deps before Hermes secret-boundary Vitest (#6143)

[jason-ma-nv]

Summary

The build-hermes-sandbox-image job in .github/workflows/sandbox-images-and-e2e.yaml ran the Hermes sandbox secret-boundary Vitest test (npx vitest) before the Set up Node and Install root dependencies steps. On a clean hosted runner there is no root node_modules, so npx pulled an ad-hoc vitest that could not resolve vitest/config from the repo's vitest.config.ts, failing the job with Cannot find module 'vitest/config'. This moves the Node setup + install steps ahead of the first Vitest invocation.

Related Issue

Fixes #6143

Changes

• Move Set up Node and Install root dependencies (npm ci --ignore-scripts) to run immediately after Resolve Hermes base image, before Build Hermes production image and both Hermes Vitest steps.
• New step order: Checkout → Resolve base image → Set up Node → Install deps → Build/verify Hermes image → secret-boundary Vitest → root-entrypoint smoke Vitest.
• Net diff is a pure reorder (9 insertions / 9 deletions); no step content changed. The ordering regressed in test(e2e): retire legacy shell lanes #5756 (commit 8120223922bf).

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
• Tests not applicable — justification: CI workflow step reorder; correctness is a CI-runner behavior (dependencies present before Vitest), validated by the job's own run plus a host A/B below. No unit-testable surface.
• Docs not applicable — justification: internal CI workflow only; no user-facing surface.
• 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: touches only CI step ordering in one workflow job; the Hermes secret-boundary and smoke tests, their env, and commands are unchanged — only the position of the standard setup-node + npm ci steps moved earlier.
• 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
• Git hooks passed during commit and push
• Targeted validation: YAML parses; prek hooks (check yaml, whitespace, etc.) pass on the file; step order confirmed.
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed

Host A/B verification (local-jama@10.176.198.59)

Reproduced the exact failure and confirmed the fix's premise without running the heavy live Docker test:

• Before npm install (simulating the old ordering — Vitest with no root node_modules): npx vitest list … hermes-sandbox-secret-boundary.test.ts fails with Cannot find module 'vitest/config' — matches the reported CI error.
• After npm install (the new ordering — deps present first): the same vitest list resolves vitest.config.ts and discovers the test with no ad-hoc npx install.

(Results pasted in the completion notification.) Definitive check: this PR's own build-hermes-sandbox-image job on a fresh runner.

---

Signed-off-by: Jason Ma jama@nvidia.com

Summary by CodeRabbit

• Chores
  • Updated the sandbox image and end-to-end pipeline to prepare dependencies earlier in the run, helping the job execute more smoothly and consistently.
  • Streamlined setup steps in the workflow by removing duplicate preparation later in the process.

[coderabbitai]

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: e76e0d15-7557-4b29-ab1c-06182f5e5eca

📥 Commits

Reviewing files that changed from the base of the PR and between 9e061e0 and a80328e.

📒 Files selected for processing (1)

• .github/workflows/sandbox-images-and-e2e.yaml

---

📝 Walkthrough

Walkthrough

The build-hermes-sandbox-image job in the sandbox-images-and-e2e workflow now sets up Node.js 22 with npm cache and installs root dependencies earlier, before building the Hermes production image. The equivalent steps previously located before the smoke Vitest test were removed.

Changes

Workflow Step Reorder

Layer / File(s)	Summary
Move Node setup and dependency install earlier .github/workflows/sandbox-images-and-e2e.yaml	Node 22 setup with npm cache and npm ci --ignore-scripts are added before the Hermes production image build; the duplicate later occurrence before the smoke Vitest test is removed.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related issues

• ci(main): install dependencies before Hermes secret-boundary Vitest #6143: Addresses the dependency-ordering issue for the Hermes Vitest smoke test by moving Node setup and npm ci earlier in the same job.

🚥 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 describes the main change: installing dependencies before the Hermes secret-boundary Vitest step.
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 unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ci/6143-hermes-vitest-node-setup-order

---

_{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 the ci/6143-hermes-vites... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.

File	main	ci/6143-hermes-vites... a80328e	+/-
nemoclaw/src/se...cret-scanner.ts	—	100%	—
nemoclaw/src/commands/slash.ts	—	100%	—
nemoclaw/src/li...bprocess-env.ts	—	100%	—
nemoclaw/src/bl...eprint/state.ts	—	98%	—
nemoclaw/src/onboard/config.ts	—	98%	—
nemoclaw/src/bl...int/snapshot.ts	—	97%	—
nemoclaw/src/bl...print/runner.ts	—	95%	—
nemoclaw/src/co...ration-state.ts	—	94%	—
nemoclaw/src/bl...ate-networks.ts	—	94%	—
nemoclaw/src/index.ts	—	94%	—

TypeScript / code-coverage/cli

The overall coverage in the ci/6143-hermes-vites... branch is 68%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.

File	main	ci/6143-hermes-vites... a80328e	+/-
src/lib/shields...nsition-lock.ts	—	86%	—
src/lib/actions...dbox/rebuild.ts	—	80%	—
src/lib/actions...all/run-plan.ts	—	80%	—
src/lib/state/o...oard-session.ts	—	80%	—
src/lib/state/sandbox.ts	—	72%	—
src/lib/shields/index.ts	—	69%	—
src/lib/onboard/preflight.ts	—	69%	—
src/lib/onboard...er-gpu-patch.ts	—	59%	—
src/lib/actions...licy-channel.ts	—	58%	—
src/lib/onboard.ts	—	20%	—

---

_{Updated July 03, 2026 04:29 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.}

[github-actions]

E2E Advisor Recommendation

Required E2E: build-hermes-sandbox-image
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• build-hermes-sandbox-image (medium): This is the directly modified E2E workflow job. It builds the Hermes production sandbox image and runs the Hermes sandbox secret boundary and root entrypoint smoke live E2E tests, validating the new Node dependency setup ordering.

Optional E2E

• None.

New E2E recommendations

• None.

[github-actions]

E2E Target Recommendation

Required E2E targets: None
Optional E2E targets: None

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

• None. This PR only changes .github/workflows/sandbox-images-and-e2e.yaml, which is outside the canonical E2E target workflow .github/workflows/e2e.yaml and does not affect the typed E2E target registry, runtime support, fixtures, or target workflow machinery.

Optional E2E targets

• None.

Relevant changed files

• None.

[github-actions]

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 0 suggestions · 3 test follow-ups

Action checklist

• PRA-T1 Add or justify test follow-up: Runtime validation
• PRA-T2 Add or justify test follow-up: Runtime validation
• PRA-T3 Add or justify test follow-up: Acceptance clause

Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Runtime validation — On a fresh runner, execute `build-hermes-sandbox-image` and confirm `Run Hermes sandbox secret boundary test` starts after `npm ci --ignore-scripts` without `npm warn exec ... vitest` ad-hoc installation.. The changed behavior is CI workflow ordering for a sandbox-image job, so static review can verify step order but cannot prove fresh-runner runtime behavior.
• PRA-T2 Runtime validation — On the same job path, confirm `hermes-root-entrypoint-smoke.test.ts` still runs after the single root dependency installation and uses the repository dependency set.. The changed behavior is CI workflow ordering for a sandbox-image job, so static review can verify step order but cannot prove fresh-runner runtime behavior.
• PRA-T3 Acceptance clause — The `build-hermes-sandbox-image` job passes on a fresh hosted runner. — add test evidence or identify existing coverage. Static workflow ordering now supports the required fresh-runner behavior: dependency installation at `.github/workflows/sandbox-images-and-e2e.yaml:80` precedes both Hermes Vitest steps. Full job success is runtime infrastructure behavior and was not executed in this read-only review.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

[github-actions]

PR Review Advisor (Nemotron Ultra) — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Consider PRA-1: Workflow step reordering fixes Node.js setup timing for vitest execution.
Open items: 0 required · 0 warnings · 1 suggestion · 0 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Action checklist

• PRA-1 In-scope improvement: Workflow step reordering fixes Node.js setup timing for vitest execution in .github/workflows/sandbox-images-and-e2e.yaml:72

Findings index

ID	Severity	Category	Location	Required action
PRA-1	Improvement	correctness	.github/workflows/sandbox-images-and-e2e.yaml:72	No action needed; the fix is correct and complete. The step ordering now ensures Node.js is available for all vitest runs.

Review findings by urgency: 0 required fixes, 0 items to resolve/justify, 1 in-scope improvement

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

• None.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-1 Improvement — Workflow step reordering fixes Node.js setup timing for vitest execution

• Location: .github/workflows/sandbox-images-and-e2e.yaml:72
• Category: correctness
• Problem: The base workflow had 'Set up Node' and 'Install root dependencies' steps after the first vitest test run ('Run Hermes sandbox secret boundary test'), causing that test to fail with 'npx: not found'. The PR moves these steps before the Docker build and before any vitest invocations.
• Impact: Without this fix, the Hermes sandbox secret boundary test — which validates that sandboxed processes cannot access host secrets — would fail to execute, leaving a security regression undetected.
• Suggested action: No action needed; the fix is correct and complete. The step ordering now ensures Node.js is available for all vitest runs.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Compare base vs PR workflow: in base, 'Set up Node' (line 97) appears after 'Run Hermes sandbox secret boundary test' (line 88); in PR, 'Set up Node' (line 75) appears before 'Build Hermes production image' (line 81) and all vitest steps.
• Missing regression test: Existing workflow contract tests in test/e2e/support/hermes-secret-boundary-workflow.test.ts validate job structure; consider adding a test that simulates the workflow execution order to catch step-ordering regressions.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: Base workflow lines 88-97 vs PR workflow lines 75-91 show the reordering. The vitest command at base line 88 requires Node which is only set up at base line 97.

Since last review details

Current findings, using the urgency labels above:

PRA-1 Improvement — Workflow step reordering fixes Node.js setup timing for vitest execution

• Location: .github/workflows/sandbox-images-and-e2e.yaml:72
• Category: correctness
• Problem: The base workflow had 'Set up Node' and 'Install root dependencies' steps after the first vitest test run ('Run Hermes sandbox secret boundary test'), causing that test to fail with 'npx: not found'. The PR moves these steps before the Docker build and before any vitest invocations.
• Impact: Without this fix, the Hermes sandbox secret boundary test — which validates that sandboxed processes cannot access host secrets — would fail to execute, leaving a security regression undetected.
• Suggested action: No action needed; the fix is correct and complete. The step ordering now ensures Node.js is available for all vitest runs.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Compare base vs PR workflow: in base, 'Set up Node' (line 97) appears after 'Run Hermes sandbox secret boundary test' (line 88); in PR, 'Set up Node' (line 75) appears before 'Build Hermes production image' (line 81) and all vitest steps.
• Missing regression test: Existing workflow contract tests in test/e2e/support/hermes-secret-boundary-workflow.test.ts validate job structure; consider adding a test that simulates the workflow execution order to catch step-ordering regressions.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: Base workflow lines 88-97 vs PR workflow lines 75-91 show the reordering. The vitest command at base line 88 requires Node which is only set up at base line 97.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.