Skip to content

test(hermes): stabilize guard timing and WSL ownership - #7381

Merged
cv merged 4 commits into
mainfrom
codex/raise-hermes-guard-timeout
Jul 22, 2026
Merged

test(hermes): stabilize guard timing and WSL ownership#7381
cv merged 4 commits into
mainfrom
codex/raise-hermes-guard-timeout

Conversation

@ericksoa

@ericksoa ericksoa commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • raise the Hermes Python guard test-harness allowance from 5 seconds to 90 seconds
  • apply the shared allowance to the three runtime-config-guard helper paths only
  • correct the WSL root-only peer fixture to start from the production mutable topology, sandbox:sandbox 03770
  • assert sealing produces root:sandbox 03770, permits sandbox-group runtime-state creation, rejects unlink of root-owned sealed config, and restores the original ownership
  • let the existing config-reclaim peer write probe traverse Vitest's private temp root only for the duration of that probe, then restore its exact mode
  • leave production runtime behavior and non-guard command limits unchanged

Evidence

  • main CI job 88920894839 returned status: null after the first guard child exceeded its 5-second spawnSync limit; the runner and later guard cases remained healthy
  • four prior main runs passed the same case in about 0.3 seconds, confirming a load-sensitive harness limit rather than a Hermes behavior regression
  • current-main Platform Vitest run 29919416442 passed the complete WSL suite (1,642 files / 19,227 tests) and then reproduced the isolated root-fixture failure
  • historical run 29307612216 reproduced that fixture failure before the OpenClaw upgrade and before ci(wsl): stabilize full platform suite #7379
  • first exact-head WSL proof 29923652396 passed the complete WSL suite and both corrected Hermes root contracts; it then exposed the next root-only fixture defect
  • that config-reclaim test completed production normalization and all ownership/mode assertions; only its stepped-down write probe failed because PR test: clean up temporary files after runs #6690 made Vitest's shared temp ancestor private
  • current exact head b7414d2f7f1bfd68028e9f43c11c1cc61033e266 includes current main at f9924949922f8e554f94aeefdd23a993a801e7b4
  • current exact-head platform proof: https://github.com/NVIDIA/NemoClaw/actions/runs/29929199550
  • focused nemoclaw-start-perms suite: 16 passed, 3 Linux-root capability skips on macOS
  • npm run build:cli and npm run check:diff: passed
  • independent review of the narrow changes: no production findings

Signed-off-by: Aaron Erickson aerickson@nvidia.com

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Copilot AI review requested due to automatic review settings July 22, 2026 12:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Hermes test fixtures now use a shared 90-second subprocess timeout. Linux permission and recovery tests explicitly configure sandbox ownership and traversal, then verify ownership restoration and write-check results.

Changes

Hermes recovery test behavior

Layer / File(s) Summary
Shared guard subprocess timeout
test/helpers/hermes-restart-config-seal-fixture.ts
Adds HERMES_GUARD_TIMEOUT_MS set to 90 seconds and applies it to configuration, restart, and shield transition subprocess calls.
Sandbox ownership recovery
test/hermes-restart-config-seal-recovery.test.ts
Derives sandbox IDs, assigns fixture ownership, launches the peer with the sandbox group, and verifies restored ownership across fixture paths.
Sandbox traversal write check
test/nemoclaw-start-perms.test.ts
Temporarily enables parent-directory traversal for a sandbox write probe, restores the original mode, and validates the subprocess status and stderr.

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

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: longer Hermes guard timing and WSL ownership/permissions test fixes.
✨ 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 codex/raise-hermes-guard-timeout

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

@github-code-quality

github-code-quality Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit b7414d2 in the codex/raise-hermes-g... branch remains at 96%, unchanged from commit f992494 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit b7414d2 in the codex/raise-hermes-g... branch remains at 80%, unchanged from commit f992494 in the main branch.

Show a code coverage summary of the most impacted files.
File main f992494 codex/raise-hermes-g... b7414d2 +/-
src/lib/domain/.../connect-env.ts 97% 89% -8%
src/lib/agent/r...ime-manifest.ts 100% 100% 0%
src/lib/messagi...nnels/policy.ts 100% 100% 0%
src/lib/sandbox...rce-identity.ts 87% 87% 0%
src/lib/securit...ntial-filter.ts 93% 93% 0%
src/lib/trace.ts 94% 94% 0%
src/lib/platform.ts 84% 89% +5%

Updated July 22, 2026 14:55 UTC

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / high 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 · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

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: None

Workflow run details

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

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Copilot AI review requested due to automatic review settings July 22, 2026 13:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@ericksoa ericksoa changed the title test(hermes): allow slow config guard helpers test(hermes): stabilize guard timing and WSL ownership Jul 22, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@test/hermes-restart-config-seal-recovery.test.ts`:
- Around line 362-371: The capability gate for this test must also verify that
the sandbox account exists before running. Update the surrounding it.runIf
condition to include successful id -u sandbox and id -g sandbox checks, reusing
the same command options as the existing sandboxUidResult and sandboxGidResult
calls so unsupported runners skip the test.
🪄 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: 80009b2c-dba7-4175-816b-458b7330ba40

📥 Commits

Reviewing files that changed from the base of the PR and between 532e160 and 6a8995c.

📒 Files selected for processing (1)
  • test/hermes-restart-config-seal-recovery.test.ts

Comment on lines +362 to +371
const sandboxUidResult = spawnSync("id", ["-u", "sandbox"], {
encoding: "utf-8",
timeout: 5000,
});
const sandboxGidResult = spawnSync("id", ["-g", "sandbox"], {
encoding: "utf-8",
timeout: 5000,
});
expect(sandboxUidResult.status, sandboxUidResult.stderr).toBe(0);
expect(sandboxGidResult.status, sandboxGidResult.stderr).toBe(0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Include the sandbox account in the capability gate.

The existing it.runIf condition checks Linux, root, and setpriv, but not whether the sandbox user/group exists. On a runner lacking that account, both id calls fail and the test fails instead of being skipped. Add equivalent id -u/-g sandbox checks to the gate or provision the account in CI.

🤖 Prompt for 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.

In `@test/hermes-restart-config-seal-recovery.test.ts` around lines 362 - 371, The
capability gate for this test must also verify that the sandbox account exists
before running. Update the surrounding it.runIf condition to include successful
id -u sandbox and id -g sandbox checks, reusing the same command options as the
existing sandboxUidResult and sandboxGidResult calls so unsupported runners skip
the test.

ericksoa added 2 commits July 22, 2026 07:33
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Copilot AI review requested due to automatic review settings July 22, 2026 14:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@cv
cv merged commit c6f46d4 into main Jul 22, 2026
97 of 103 checks passed
@cv
cv deleted the codex/raise-hermes-guard-timeout branch July 22, 2026 15:18
@ericksoa

Copy link
Copy Markdown
Contributor Author

Post-merge validation is complete.

No production permission behavior was changed by this follow-up.

@cv cv mentioned this pull request Jul 22, 2026
22 tasks
cv added a commit that referenced this pull request Jul 22, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Adds the canonical `## v0.0.92` release entry to
`docs/changelog/2026-07-22.mdx` before the release plan is generated.
The entry summarizes all ten pull requests merged after v0.0.91,
including the OpenClaw security update and Jaeger runtime regression
coverage.

## Changes

- Added the canonical v0.0.92 changelog entry.
- Recorded the user-visible, security, documentation, CI, and test
changes in the release range.
- #7280 -> `docs/changelog/2026-07-22.mdx`: OpenClaw 2026.7.1 and
Node.js 22.23.1 security/runtime update.
- #7378 -> `docs/changelog/2026-07-22.mdx`: canonical macOS watcher path
validation.
- #7379 -> `docs/changelog/2026-07-22.mdx`: stabilized full WSL platform
validation.
- #7380 -> `docs/changelog/2026-07-22.mdx`: bounded swap for hosted
Hermes image exports.
- #7100 -> `docs/changelog/2026-07-22.mdx`: semantic progress phases for
live E2E tests.
- #7376 -> `docs/changelog/2026-07-22.mdx`: restored v0.0.91 changelog
history and corrected tagged guidance.
- #7374 -> `docs/changelog/2026-07-22.mdx`: reviewed Homebrew formula
transition for installer integrity checks.
- #7346 -> `docs/changelog/2026-07-22.mdx`: provider-neutral headless
server deployment guidance.
- #7381 -> `docs/changelog/2026-07-22.mdx`: stabilized Hermes guard
timing and WSL ownership fixtures.
- #7339 -> `docs/changelog/2026-07-22.mdx`: real-artifact Jaeger header
remediation regression coverage.

## 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 canonical dated changelog
and release heading contract.
- [ ] 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:

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable
- 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 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 a
changelog-only change.
- [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) — passed
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)

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


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

* **Documentation**
* Added `v0.0.92` release notes covering sandboxing updates
(OpenClaw/Node.js bumps, integrity pinning remediation, mcporter
handling, and upgrade validation).
* Updated deployment guidance for provider-neutral headless installs,
and improved live E2E test reporting plus phase-plan validation.
* Tightened installer integrity-check messaging during an OpenShell
Homebrew transition and expanded platform/image validation (including
macOS/WSL timing) and hosted image export behavior.
* Restored the previously missed `v0.0.91` changelog entry and release
validation guidance.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions integration: hermes Hermes integration behavior platform: wsl Affects Windows Subsystem for Linux labels Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions integration: hermes Hermes integration behavior platform: wsl Affects Windows Subsystem for Linux

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants