Skip to content

test(e2e): preserve Hermes retired-state parity - #5997

Closed
jyaunches wants to merge 1 commit into
mainfrom
test/hermes-retired-state-parity
Closed

test(e2e): preserve Hermes retired-state parity#5997
jyaunches wants to merge 1 commit into
mainfrom
test/hermes-retired-state-parity

Conversation

@jyaunches

@jyaunches jyaunches commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Preserve the post-#5949 Hermes retired-state contract at the live Vitest image boundary before the legacy shell entrypoint is removed.

Related Issue

Refs #5849
Refs #5919
Refs #5949

Changes

  • Fail the live Hermes production-image inspection when either retired /sandbox/.openclaw or /sandbox/.hermes-data state exists.
  • Keep the assertion at the same real built-image boundary as the legacy shell check.

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: test parity only; no product or user-facing behavior changes.
  • 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: pending independent review of the Hermes image boundary.
  • 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, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • 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)

Targeted verification:

  • npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/hermes-secret-boundary-workflow.test.ts — 2 tests passed.
  • NEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest list --project e2e-scenarios-live test/e2e-scenario/live/hermes-sandbox-secret-boundary.test.ts — live test collected successfully.
  • npm run build:cli && npm run typecheck:cli — passed.
  • npx biome check test/e2e-scenario/live/hermes-sandbox-secret-boundary.test.ts — passed.
  • Exact live Docker/image execution is delegated to the selective hermes-sandbox-secret-boundary-vitest workflow job.

Signed-off-by: Julie Yaunches jyaunches@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Strengthened sandbox boundary checks by rejecting retired Hermes sandbox state paths when they are present or linked.
    • Added clearer failure reporting when an invalid path is detected, helping prevent outdated sandbox files from passing validation.

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

coderabbitai Bot commented Jun 29, 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: 30b08878-b6c0-4e74-b1fc-7aface465caa

📥 Commits

Reviewing files that changed from the base of the PR and between e0b9e1e and 4a0ed8f.

📒 Files selected for processing (1)
  • test/e2e-scenario/live/hermes-sandbox-secret-boundary.test.ts

📝 Walkthrough

Walkthrough

The e2e test for Hermes sandbox secret boundary adds a pre-check loop in the inline Python inspection script. Before the existing checks for /sandbox/.hermes/.env and /sandbox/.hermes/config.yaml, the script now iterates over /sandbox/.openclaw and /sandbox/.hermes-data; if either path exists or is a symlink, it prints to stderr and exits with code 1.

Changes

Hermes Sandbox Secret Boundary Test

Layer / File(s) Summary
Retired path pre-check in image inspection script
test/e2e-scenario/live/hermes-sandbox-secret-boundary.test.ts
Adds a loop over /sandbox/.openclaw and /sandbox/.hermes-data that reports the offending path to stderr and exits 1 if either path exists or is a symlink, before the existing boundary checks run.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5949: Directly overlaps with retired-path boundary enforcement logic in the same Hermes sandbox secret-boundary test file.

Suggested labels

integration: hermes, area: sandbox

🚥 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 concise and clearly matches the test change around Hermes retired-state parity.
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 test/hermes-retired-state-parity

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

@github-code-quality

github-code-quality Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the test/hermes-retired-... 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 test/hermes-retired-... 4a0ed8f +/-
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 test/hermes-retired-... 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 test/hermes-retired-... 4a0ed8f +/-
src/lib/actions...all/run-plan.ts 80%
src/lib/state/o...oard-session.ts 79%
src/lib/actions...dbox/rebuild.ts 74%
src/lib/state/sandbox.ts 72%
src/lib/shields/index.ts 70%
src/lib/onboard/preflight.ts 69%
src/lib/actions...licy-channel.ts 59%
src/lib/onboard...er-gpu-patch.ts 59%
src/lib/policy/index.ts 52%
src/lib/onboard.ts 20%

Updated June 29, 2026 16:19 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • None. No E2E run is required because this is a tests-only PR modifying an existing E2E scenario assertion. It cannot directly affect runtime user flows or security boundaries unless the test is executed separately by the author for validation.

Optional E2E

  • None.

New E2E recommendations

  • None.

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: hermes-sandbox-secret-boundary-vitest
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=hermes-sandbox-secret-boundary-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • hermes-sandbox-secret-boundary-vitest: Focused free-standing Vitest job wired for changed live test test/e2e-scenario/live/hermes-sandbox-secret-boundary.test.ts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=hermes-sandbox-secret-boundary-vitest

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • test/e2e-scenario/live/hermes-sandbox-secret-boundary.test.ts

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: No advisor follow-up required beyond maintainer review.
Open items: 0 required · 0 warnings · 0 suggestions · 0 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

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

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: No advisor follow-up required beyond maintainer review.
Open items: 0 required · 0 warnings · 0 suggestions · 0 test follow-ups

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

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ✅ All requested jobs passed

Run: 28386410772
Workflow ref: test/hermes-retired-state-parity
Requested scenarios: (default — all supported)
Requested jobs: hermes-sandbox-secret-boundary-vitest
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
hermes-sandbox-secret-boundary-vitest ✅ success

@wscurran wscurran added area: e2e End-to-end tests, nightly failures, or validation infrastructure chore Build, CI, dependency, or tooling maintenance integration: hermes Hermes integration behavior labels Jul 1, 2026
@wscurran

wscurran commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

@cv cv added the v0.0.76 label Jul 7, 2026
@cv

cv commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Closing as superseded by #5756. The same retired-state assertion is already on main in test/e2e/live/hermes-sandbox-secret-boundary.test.ts, and this branch now targets the retired path and conflicts.

@cv cv closed this Jul 7, 2026
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 chore Build, CI, dependency, or tooling maintenance integration: hermes Hermes integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants