Skip to content

fix(sandbox): compare hermes agent version in its runtime scheme - #6089

Merged
jyaunches merged 24 commits into
mainfrom
fix/hermes-version-scheme-mismatch
Jul 1, 2026
Merged

jyaunches merged 24 commits into
mainfrom
fix/hermes-version-scheme-mismatch

Conversation

@laitingsheng

@laitingsheng laitingsheng commented Jul 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

nemoclaw <name> status on a Hermes sandbox always reported Update: v2026.6.19 available because the staleness check compared the runtime semver against the manifest calver with a scheme-blind comparator.

Related Issue

Fixes #6049

Changes

  • agents/hermes/manifest.yaml: pin expected_version to the semver the runtime reports.
  • src/lib/sandbox/version.ts: skip the comparison when runtime and expected versions are in different schemes.
  • scripts/update-hermes-agent.sh: align drift check and manifest pin with HERMES_SEMVER.
  • Tests: cover semver-match, behind, cross-scheme, and ssh-probe paths.

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: corrects an existing status line; no new command or flag.
  • 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: requesting maintainer review of src/lib/sandbox/version.ts.
  • 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)

Signed-off-by: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved Hermes runtime compatibility checks with scheme-aware version handling (semver-like vs calendar-style), including accurate stale detection for both registry and SSH-probed paths.
    • Added clearer one-time warnings when sandbox/expected versions use different version schemes.
  • Tests
    • Expanded Hermes version parsing/validation and runtime detection coverage, including exact-match, staleness, and non-update scenarios.
  • Chores
    • Updated the Hermes agent update/check script to treat the manifest’s expected version as semver.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 1, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Hermes now pins to 0.17.0, compares version schemes before staleness checks, updates the Hermes pinning script to use semver consistently, and expands tests for Hermes-specific version matching and mixed-scheme behavior.

Changes

Hermes version comparison fix

Layer / File(s) Summary
Manifest version pin
agents/hermes/manifest.yaml, src/lib/agent/defs.test.ts
expected_version changes to 0.17.0, and a test checks the Hermes runtime expectedVersion format.
Staleness comparison helper
src/lib/sandbox/version.ts
Adds version-scheme comparability helpers and routes both checkAgentVersion paths through isAgentStale.
Hermes pinning script
scripts/update-hermes-agent.sh
Updates Hermes manifest pinning, drift detection, and status output to use semver values.
Hermes version tests
src/lib/sandbox/version.test.ts
Updates the agent mock and adds Hermes-specific staleness coverage for matching, stale, mixed-scheme, and SSH-probed versions.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested reviewers: ericksoa, cv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the Hermes sandbox version-scheme comparison fix.
Linked Issues check ✅ Passed The version-scheme guard, manifest pin update, script sync, and tests address the false Hermes update warning in #6049.
Out of Scope Changes check ✅ Passed The remaining changes are directly tied to Hermes version handling and test coverage, with no unrelated scope evident.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/hermes-version-scheme-mismatch

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

@github-code-quality

github-code-quality Bot commented Jul 1, 2026 •

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the fix/hermes-version-s... 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 fix/hermes-version-s... 432065c +/-
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 fix/hermes-version-s... 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 fix/hermes-version-s... 432065c +/-
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/onboard/preflight.ts — 69% —
src/lib/shields/index.ts — 67% —
src/lib/onboard...er-gpu-patch.ts — 59% —
src/lib/actions...licy-channel.ts — 58% —
src/lib/onboard.ts — 20% —

Updated July 01, 2026 10:11 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 1, 2026 •

Copy link
Copy Markdown
Contributor

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Source-of-truth review needed: version_scheme manifest parsing.
Open items: 0 required · 6 warnings · 1 suggestion · 8 test follow-ups
Since last review: 0 prior items resolved · 7 still apply · 0 new items found

Action checklist

  • PRA-1 Resolve or justify: Source-of-truth review needed: version_scheme manifest parsing
  • PRA-2 Resolve or justify: Source-of-truth review needed: cross-scheme staleness compatibility
  • PRA-3 Resolve or justify: Invalid version_scheme manifest values are silently ignored in src/lib/agent/defs.ts:185
  • PRA-4 Resolve or justify: Scheme-mismatch warning interpolates untrusted values into stderr in src/lib/sandbox/version-scheme.ts:68
  • PRA-5 Resolve or justify: Caller-level tests still do not exercise the declared versionScheme path in src/lib/sandbox/version.test.ts:48
  • PRA-6 Resolve or justify: Issue [DGX Spark][CLI&UX] nemoclaw status falsely reports Hermes "Update: v2026.6.19 available" when installed Hermes Agent v0.17.0 IS v2026.6.19 #6049 lacks a direct status-output regression in src/lib/actions/sandbox/status-text.ts:213
  • 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: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Runtime validation
  • PRA-T6 Add or justify test follow-up: Acceptance clause
  • PRA-T7 Add or justify test follow-up: Acceptance clause
  • PRA-T8 Add or justify test follow-up: Acceptance clause
  • PRA-7 In-scope improvement: verificationFailed comment contradicts the VersionCheckResult contract in src/lib/domain/maintenance/upgrade.ts:10

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture — Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify architecture — Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-3 Resolve/justify security src/lib/agent/defs.ts:185 Treat absence as the only heuristic fallback. If `record.version_scheme !== undefined` and the value is not exactly `"semver"` or `"calendar"`, throw a manifest validation error consistent with nearby enum readers such as runtime, dashboard auth, and inference provider validation.
PRA-4 Resolve/justify security src/lib/sandbox/version-scheme.ts:68 Escape or sanitize control characters in every value used in the human-readable prefix, or render prefix values with `JSON.stringify()`-style escaping. Keep the structured JSON payload.
PRA-5 Resolve/justify correctness src/lib/sandbox/version.test.ts:48 Update the `loadAgent()` mock to return `versionScheme: "semver"` for Hermes and add cached and SSH-probed legacy-calendar observed-version cases under that declared scheme.
PRA-6 Resolve/justify acceptance src/lib/actions/sandbox/status-text.ts:213 Add a focused status rendering test for a Hermes sandbox whose cached or mocked checked runtime version and expected version are both `0.17.0`, asserting the output includes `Agent: Hermes Agent v0.17.0` and excludes both `Update:` and the rebuild command.
PRA-7 Improvement docs src/lib/domain/maintenance/upgrade.ts:10 Remove `or a scheme mismatch between the runtime and manifest versions` from the `verificationFailed` comment and, if useful, mention scheme mismatches separately as stale candidates represented by `isStale` and `schemeMismatch`.
Review findings by urgency: 0 required fixes, 6 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.

PRA-1 Resolve/justify — Source-of-truth review needed: version_scheme manifest parsing

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Add `loadAgent rejects invalid version_scheme values in manifests` for invalid string and non-string values.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: `readVersionScheme()` returns `undefined` for invalid present values.

PRA-2 Resolve/justify — Source-of-truth review needed: cross-scheme staleness compatibility

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Add cached and SSH-probed `checkAgentVersion()` cases where the mocked Hermes agent returns `versionScheme: "semver"` and the observed version is a legacy calendar value.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: `evaluateStaleness()` fails closed on mismatches, but the `checkAgentVersion()` test mock omits `versionScheme`, so the production source-of-truth wiring is not exercised.

PRA-3 Resolve/justify — Invalid version_scheme manifest values are silently ignored

  • Location: src/lib/agent/defs.ts:185
  • Category: security
  • Problem: `readVersionScheme()` returns `undefined` for any present value other than `"semver"` or `"calendar"`, making an invalid explicit manifest field indistinguishable from an intentionally absent compatibility field.
  • Impact: The new manifest field controls sandbox staleness and rebuild routing. A typo or installed-copy tampering can downgrade an explicit lifecycle policy to shape heuristics instead of failing closed during manifest load, hiding drift in a security-sensitive sandbox lifecycle path.
  • Recommended action: Treat absence as the only heuristic fallback. If `record.version_scheme !== undefined` and the value is not exactly `"semver"` or `"calendar"`, throw a manifest validation error consistent with nearby enum readers such as runtime, dashboard auth, and inference provider validation.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `readVersionScheme()` in `src/lib/agent/defs.ts` and confirm it first allows `undefined`, then throws for any other invalid present value before returning a scheme.
  • Missing regression test: Add `loadAgent rejects invalid version_scheme values in manifests` in `src/lib/agent/defs.test.ts`, covering both `version_scheme: calver` and a non-string value such as `version_scheme: 42`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `readVersionScheme()` in `src/lib/agent/defs.ts` and confirm it first allows `undefined`, then throws for any other invalid present value before returning a scheme.
  • Evidence: `readVersionScheme(record)` currently checks `if (value === "semver" || value === "calendar") return value; return undefined;`.

PRA-4 Resolve/justify — Scheme-mismatch warning interpolates untrusted values into stderr

  • Location: src/lib/sandbox/version-scheme.ts:68
  • Category: security
  • Problem: `warnSchemeMismatch()` JSON-escapes the structured payload, but the human-readable warning prefix directly interpolates `sandboxName`, `sandboxVersion`, and `expectedVersion`.
  • Impact: A compromised sandbox, corrupted registry entry, or malformed manifest/cached version containing newlines or terminal control characters can forge additional host stderr lines, obscure the real warning, or pollute CI/support logs.
  • Recommended action: Escape or sanitize control characters in every value used in the human-readable prefix, or render prefix values with `JSON.stringify()`-style escaping. Keep the structured JSON payload.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the `process.stderr.write()` call in `warnSchemeMismatch()` and confirm all interpolated prefix values are escaped before the structured JSON payload is appended.
  • Missing regression test: Add a `version-scheme.test.ts` case that triggers a scheme mismatch with newline/control characters in the sandbox name or cached version and asserts stderr contains escaped representations rather than forged extra lines.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the `process.stderr.write()` call in `warnSchemeMismatch()` and confirm all interpolated prefix values are escaped before the structured JSON payload is appended.
  • Evidence: The payload uses `JSON.stringify({ sandbox: sandboxName, sandboxVersion, expectedVersion, ... })`, but the prefix is `warning: sandbox '${sandboxName}' agent version ${sandboxVersion} and expected version ${expectedVersion} ...`.

PRA-5 Resolve/justify — Caller-level tests still do not exercise the declared versionScheme path

  • Location: src/lib/sandbox/version.test.ts:48
  • Category: correctness
  • Problem: The `checkAgentVersion()` tests mock `loadAgent()` without returning `versionScheme`, so the caller path exercises `agent.versionScheme ?? null` as `null` and relies on shape heuristics instead of the new manifest-declared scheme.
  • Impact: The PR's primary production wiring is `agents/hermes/manifest.yaml` -> `loadAgent().versionScheme` -> `checkAgentVersion()` -> `evaluateStaleness()`. Without a caller-level test for `versionScheme: "semver"`, a future refactor can break the declared-scheme contract while the current tests still pass through the heuristic fallback.
  • Recommended action: Update the `loadAgent()` mock to return `versionScheme: "semver"` for Hermes and add cached and SSH-probed legacy-calendar observed-version cases under that declared scheme.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read the `vi.mock("../agent/defs.js")` block in `src/lib/sandbox/version.test.ts` and confirm the mocked Hermes agent includes `versionScheme: "semver"`, then inspect the cached and SSH-probed mismatch tests to ensure the observed value is calendar-shaped while the expected pin is semver-shaped.
  • Missing regression test: Add `checkAgentVersion flags cached legacy calendar Hermes runtime when manifest declares semver` and `checkAgentVersion flags SSH-probed legacy calendar Hermes runtime when manifest declares semver`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read the `vi.mock("../agent/defs.js")` block in `src/lib/sandbox/version.test.ts` and confirm the mocked Hermes agent includes `versionScheme: "semver"`, then inspect the cached and SSH-probed mismatch tests to ensure the observed value is calendar-shaped while the expected pin is semver-shaped.
  • Evidence: The mock returns `name`, `displayName`, `versionCommand`, `expectedVersion`, `stateDirs`, and `configPaths`, but no `versionScheme`.

PRA-6 Resolve/justify — Issue #6049 lacks a direct status-output regression

  • Location: src/lib/actions/sandbox/status-text.ts:213
  • Category: acceptance
  • Problem: The version checker now has unit coverage for Hermes `0.17.0` being current, but no status rendering test proves the exact user-visible regression from [DGX Spark][CLI&UX] nemoclaw status falsely reports Hermes "Update: v2026.6.19 available" when installed Hermes Agent v0.17.0 IS v2026.6.19 #6049 stays fixed.
  • Impact: A future change could still print `Update:` or a rebuild hint in `nemoclaw <hermes-sandbox> status` even when `checkAgentVersion()` reports Hermes `0.17.0` as current, reintroducing the misleading no-op rebuild prompt from the linked issue.
  • Recommended action: Add a focused status rendering test for a Hermes sandbox whose cached or mocked checked runtime version and expected version are both `0.17.0`, asserting the output includes `Agent: Hermes Agent v0.17.0` and excludes both `Update:` and the rebuild command.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search changed status tests for a Hermes `0.17.0` status case; the shortest read-only check is `grep` for `Hermes Agent v0.17.0` or a negative `Update:` assertion under `src/lib/actions/sandbox/*.test.ts`.
  • Missing regression test: Add `showSandboxStatus prints Hermes Agent v0.17.0 without Update or rebuild when current` in `src/lib/actions/sandbox/status-flow.test.ts` or an equivalent direct `printSandboxDetails` test.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search changed status tests for a Hermes `0.17.0` status case; the shortest read-only check is `grep` for `Hermes Agent v0.17.0` or a negative `Update:` assertion under `src/lib/actions/sandbox/*.test.ts`.
  • Evidence: Existing changed tests cover `checkAgentVersion()` and pure scheme logic, but searches of nearby status tests found only generic `Update:` assertions and no Hermes `0.17.0` negative rendering assertion.

💡 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-7 Improvement — verificationFailed comment contradicts the VersionCheckResult contract

  • Location: src/lib/domain/maintenance/upgrade.ts:10
  • Category: docs
  • Problem: `SandboxVersionCheck.verificationFailed` says it is true for scheme mismatches and that `classifyUpgradeableSandboxes` treats those as unknown candidates, but `VersionCheckResult` now documents and implements scheme mismatches as `isStale: true`, `schemeMismatch: true`, and `verificationFailed: false`.
  • Impact: The stale-vs-unknown upgrade contract is easy to misread. A maintainer could route scheme mismatches to the unknown bucket later, silently changing the fail-closed rebuild behavior this PR is trying to establish.
  • Suggested action: Remove `or a scheme mismatch between the runtime and manifest versions` from the `verificationFailed` comment and, if useful, mention scheme mismatches separately as stale candidates represented by `isStale` and `schemeMismatch`.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Compare the comment on `SandboxVersionCheck.verificationFailed` in `src/lib/domain/maintenance/upgrade.ts` with the `VersionCheckResult.verificationFailed` and `schemeMismatch` comments in `src/lib/sandbox/version.ts`.
  • Missing regression test: Existing `upgrade.test.ts` coverage already asserts scheme-mismatched sandboxes are classified as stale, not unknown; no new test is needed if the comment is corrected.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: `upgrade.ts` says scheme mismatch makes `verificationFailed` true/unknown, while `version.ts` says scheme mismatches do NOT set `verificationFailed` and instead set `schemeMismatch` with `isStale`.
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 — showSandboxStatus prints Hermes Agent v0.17.0 without Update or rebuild when version check is current. The PR changes sandbox lifecycle/version probing, manifest configuration, status rendering, and an installer/update script. Unit coverage is broad for pure comparison and `checkAgentVersion()`, but status-output acceptance, manifest invalid-value rejection, declared-scheme caller wiring, and log sanitization remain unproven.
  • PRA-T2 Runtime validation — checkAgentVersion uses declared Hermes versionScheme semver to flag a cached legacy calendar observed version as schemeMismatch. The PR changes sandbox lifecycle/version probing, manifest configuration, status rendering, and an installer/update script. Unit coverage is broad for pure comparison and `checkAgentVersion()`, but status-output acceptance, manifest invalid-value rejection, declared-scheme caller wiring, and log sanitization remain unproven.
  • PRA-T3 Runtime validation — checkAgentVersion uses declared Hermes versionScheme semver to flag an SSH-probed legacy calendar observed version as schemeMismatch. The PR changes sandbox lifecycle/version probing, manifest configuration, status rendering, and an installer/update script. Unit coverage is broad for pure comparison and `checkAgentVersion()`, but status-output acceptance, manifest invalid-value rejection, declared-scheme caller wiring, and log sanitization remain unproven.
  • PRA-T4 Runtime validation — loadAgent rejects version_scheme calver and non-string version_scheme values. The PR changes sandbox lifecycle/version probing, manifest configuration, status rendering, and an installer/update script. Unit coverage is broad for pure comparison and `checkAgentVersion()`, but status-output acceptance, manifest invalid-value rejection, declared-scheme caller wiring, and log sanitization remain unproven.
  • PRA-T5 Runtime validation — warnSchemeMismatch escapes newline and control characters in sandbox name and observed version prefix. The PR changes sandbox lifecycle/version probing, manifest configuration, status rendering, and an installer/update script. Unit coverage is broad for pure comparison and `checkAgentVersion()`, but status-output acceptance, manifest invalid-value rejection, declared-scheme caller wiring, and log sanitization remain unproven.
  • PRA-T6 Acceptance clause — `nemoclaw <hermes-sandbox> status` displays those two equivalent labels as if v2026.6.19 were a newer version available for upgrade — telling the user to rebuild for an update that does not exist. — add test evidence or identify existing coverage. `checkAgentVersion()` now compares Hermes against the semver expected pin and tests cover `0.17.0` as not stale, but no status rendering regression asserts the `Update:` and rebuild lines are absent for Hermes `0.17.0`.
  • PRA-T7 Acceptance clause — Running `nemoclaw <name> rebuild` in response would pull the SAME image (no-op upgrade); the misleading nudge wastes user time and erodes trust in status output. — add test evidence or identify existing coverage. The version-check path returns `isStale: false` for Hermes `0.17.0`, which should suppress the rebuild hint, but this is not directly proven at the status-output layer.
  • PRA-T8 Acceptance clause — (a) Show a single unified version line and NO "Update available" prompt, e.g. `Agent: Hermes Agent v0.17.0 (build v2026.6.19)` — add test evidence or identify existing coverage. The chosen implementation prints the semver runtime line via `printAgentVersion()` and avoids `Update:` when `isStale` is false; it does not include the optional build annotation, and the no-Update status output lacks a direct test.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: version_scheme manifest parsing

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Add `loadAgent rejects invalid version_scheme values in manifests` for invalid string and non-string values.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: `readVersionScheme()` returns `undefined` for invalid present values.

PRA-2 Resolve/justify — Source-of-truth review needed: cross-scheme staleness compatibility

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Add cached and SSH-probed `checkAgentVersion()` cases where the mocked Hermes agent returns `versionScheme: "semver"` and the observed version is a legacy calendar value.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: `evaluateStaleness()` fails closed on mismatches, but the `checkAgentVersion()` test mock omits `versionScheme`, so the production source-of-truth wiring is not exercised.

PRA-3 Resolve/justify — Invalid version_scheme manifest values are silently ignored

  • Location: src/lib/agent/defs.ts:185
  • Category: security
  • Problem: `readVersionScheme()` returns `undefined` for any present value other than `"semver"` or `"calendar"`, making an invalid explicit manifest field indistinguishable from an intentionally absent compatibility field.
  • Impact: The new manifest field controls sandbox staleness and rebuild routing. A typo or installed-copy tampering can downgrade an explicit lifecycle policy to shape heuristics instead of failing closed during manifest load, hiding drift in a security-sensitive sandbox lifecycle path.
  • Recommended action: Treat absence as the only heuristic fallback. If `record.version_scheme !== undefined` and the value is not exactly `"semver"` or `"calendar"`, throw a manifest validation error consistent with nearby enum readers such as runtime, dashboard auth, and inference provider validation.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `readVersionScheme()` in `src/lib/agent/defs.ts` and confirm it first allows `undefined`, then throws for any other invalid present value before returning a scheme.
  • Missing regression test: Add `loadAgent rejects invalid version_scheme values in manifests` in `src/lib/agent/defs.test.ts`, covering both `version_scheme: calver` and a non-string value such as `version_scheme: 42`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `readVersionScheme()` in `src/lib/agent/defs.ts` and confirm it first allows `undefined`, then throws for any other invalid present value before returning a scheme.
  • Evidence: `readVersionScheme(record)` currently checks `if (value === "semver" || value === "calendar") return value; return undefined;`.

PRA-4 Resolve/justify — Scheme-mismatch warning interpolates untrusted values into stderr

  • Location: src/lib/sandbox/version-scheme.ts:68
  • Category: security
  • Problem: `warnSchemeMismatch()` JSON-escapes the structured payload, but the human-readable warning prefix directly interpolates `sandboxName`, `sandboxVersion`, and `expectedVersion`.
  • Impact: A compromised sandbox, corrupted registry entry, or malformed manifest/cached version containing newlines or terminal control characters can forge additional host stderr lines, obscure the real warning, or pollute CI/support logs.
  • Recommended action: Escape or sanitize control characters in every value used in the human-readable prefix, or render prefix values with `JSON.stringify()`-style escaping. Keep the structured JSON payload.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the `process.stderr.write()` call in `warnSchemeMismatch()` and confirm all interpolated prefix values are escaped before the structured JSON payload is appended.
  • Missing regression test: Add a `version-scheme.test.ts` case that triggers a scheme mismatch with newline/control characters in the sandbox name or cached version and asserts stderr contains escaped representations rather than forged extra lines.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the `process.stderr.write()` call in `warnSchemeMismatch()` and confirm all interpolated prefix values are escaped before the structured JSON payload is appended.
  • Evidence: The payload uses `JSON.stringify({ sandbox: sandboxName, sandboxVersion, expectedVersion, ... })`, but the prefix is `warning: sandbox '${sandboxName}' agent version ${sandboxVersion} and expected version ${expectedVersion} ...`.

PRA-5 Resolve/justify — Caller-level tests still do not exercise the declared versionScheme path

  • Location: src/lib/sandbox/version.test.ts:48
  • Category: correctness
  • Problem: The `checkAgentVersion()` tests mock `loadAgent()` without returning `versionScheme`, so the caller path exercises `agent.versionScheme ?? null` as `null` and relies on shape heuristics instead of the new manifest-declared scheme.
  • Impact: The PR's primary production wiring is `agents/hermes/manifest.yaml` -> `loadAgent().versionScheme` -> `checkAgentVersion()` -> `evaluateStaleness()`. Without a caller-level test for `versionScheme: "semver"`, a future refactor can break the declared-scheme contract while the current tests still pass through the heuristic fallback.
  • Recommended action: Update the `loadAgent()` mock to return `versionScheme: "semver"` for Hermes and add cached and SSH-probed legacy-calendar observed-version cases under that declared scheme.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read the `vi.mock("../agent/defs.js")` block in `src/lib/sandbox/version.test.ts` and confirm the mocked Hermes agent includes `versionScheme: "semver"`, then inspect the cached and SSH-probed mismatch tests to ensure the observed value is calendar-shaped while the expected pin is semver-shaped.
  • Missing regression test: Add `checkAgentVersion flags cached legacy calendar Hermes runtime when manifest declares semver` and `checkAgentVersion flags SSH-probed legacy calendar Hermes runtime when manifest declares semver`.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read the `vi.mock("../agent/defs.js")` block in `src/lib/sandbox/version.test.ts` and confirm the mocked Hermes agent includes `versionScheme: "semver"`, then inspect the cached and SSH-probed mismatch tests to ensure the observed value is calendar-shaped while the expected pin is semver-shaped.
  • Evidence: The mock returns `name`, `displayName`, `versionCommand`, `expectedVersion`, `stateDirs`, and `configPaths`, but no `versionScheme`.

PRA-6 Resolve/justify — Issue #6049 lacks a direct status-output regression

  • Location: src/lib/actions/sandbox/status-text.ts:213
  • Category: acceptance
  • Problem: The version checker now has unit coverage for Hermes `0.17.0` being current, but no status rendering test proves the exact user-visible regression from [DGX Spark][CLI&UX] nemoclaw status falsely reports Hermes "Update: v2026.6.19 available" when installed Hermes Agent v0.17.0 IS v2026.6.19 #6049 stays fixed.
  • Impact: A future change could still print `Update:` or a rebuild hint in `nemoclaw <hermes-sandbox> status` even when `checkAgentVersion()` reports Hermes `0.17.0` as current, reintroducing the misleading no-op rebuild prompt from the linked issue.
  • Recommended action: Add a focused status rendering test for a Hermes sandbox whose cached or mocked checked runtime version and expected version are both `0.17.0`, asserting the output includes `Agent: Hermes Agent v0.17.0` and excludes both `Update:` and the rebuild command.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search changed status tests for a Hermes `0.17.0` status case; the shortest read-only check is `grep` for `Hermes Agent v0.17.0` or a negative `Update:` assertion under `src/lib/actions/sandbox/*.test.ts`.
  • Missing regression test: Add `showSandboxStatus prints Hermes Agent v0.17.0 without Update or rebuild when current` in `src/lib/actions/sandbox/status-flow.test.ts` or an equivalent direct `printSandboxDetails` test.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search changed status tests for a Hermes `0.17.0` status case; the shortest read-only check is `grep` for `Hermes Agent v0.17.0` or a negative `Update:` assertion under `src/lib/actions/sandbox/*.test.ts`.
  • Evidence: Existing changed tests cover `checkAgentVersion()` and pure scheme logic, but searches of nearby status tests found only generic `Update:` assertions and no Hermes `0.17.0` negative rendering assertion.

PRA-7 Improvement — verificationFailed comment contradicts the VersionCheckResult contract

  • Location: src/lib/domain/maintenance/upgrade.ts:10
  • Category: docs
  • Problem: `SandboxVersionCheck.verificationFailed` says it is true for scheme mismatches and that `classifyUpgradeableSandboxes` treats those as unknown candidates, but `VersionCheckResult` now documents and implements scheme mismatches as `isStale: true`, `schemeMismatch: true`, and `verificationFailed: false`.
  • Impact: The stale-vs-unknown upgrade contract is easy to misread. A maintainer could route scheme mismatches to the unknown bucket later, silently changing the fail-closed rebuild behavior this PR is trying to establish.
  • Suggested action: Remove `or a scheme mismatch between the runtime and manifest versions` from the `verificationFailed` comment and, if useful, mention scheme mismatches separately as stale candidates represented by `isStale` and `schemeMismatch`.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Compare the comment on `SandboxVersionCheck.verificationFailed` in `src/lib/domain/maintenance/upgrade.ts` with the `VersionCheckResult.verificationFailed` and `schemeMismatch` comments in `src/lib/sandbox/version.ts`.
  • Missing regression test: Existing `upgrade.test.ts` coverage already asserts scheme-mismatched sandboxes are classified as stale, not unknown; no new test is needed if the comment is corrected.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: `upgrade.ts` says scheme mismatch makes `verificationFailed` true/unknown, while `version.ts` says scheme mismatches do NOT set `verificationFailed` and instead set `schemeMismatch` with `isStale`.

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 Jul 1, 2026 •

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Informational

Merge posture: Informational / low confidence
Primary next action: Resolve or justify PRA-1: PR review advisor unavailable.
Open items: 0 required · 1 warning · 0 suggestions · 1 test follow-up
Top item: PR review advisor unavailable

Action checklist

  • PRA-1 Resolve or justify: PR review advisor unavailable
  • PRA-T1 Add or justify test follow-up: Runtime validation

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify correctness — Re-run the PR Review Advisor or perform a manual review.
Review findings by urgency: 0 required fixes, 1 item to resolve/justify, 0 in-scope improvements

⚠️ 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.

PRA-1 Resolve/justify — PR review advisor unavailable

  • Location: not file-specific
  • Category: correctness
  • Problem: The automated advisor could not complete: PR review advisor SDK provider error: orient-drift: 403 <html> <head><title>403 Forbidden</title></head> <body> <center><h1>403 Forbidden</h1></center> </body> </html>; security: 403 <html> <head><title>403 Forbidden</title></head> <body> <center><h1>403 Forbidden</h1></center> </body> </html>; acceptance-correctness-tests: 403 <html> <head><title>403 Forbidden</title></head> <body> <center><h1>403 Forbidden</h1></center> </body> </html>; synthesize-json: 403 <html> <head><title>403 Forbidden</title></head> <body> <center><h1>403 Forbidden</h1></center> </body> </html>
  • Impact: Automated review evidence is incomplete, so human review must cover the changed code manually.
  • Recommended action: Re-run the PR Review Advisor or perform a manual review.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the workflow logs and raw advisor artifact for the execution failure.
  • Missing regression test: No regression test recommendation is available because the advisor did not complete.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the workflow logs and raw advisor artifact for the execution failure.
  • Evidence: PR review advisor SDK provider error: orient-drift: 403 <html> <head><title>403 Forbidden</title></head> <body> <center><h1>403 Forbidden</h1></center> </body> </html>; security: 403 <html> <head><title>403 Forbidden</title></head> <body> <center><h1>403 Forbidden</h1></center> </body> </html>; acceptance-correctness-tests: 403 <html> <head><title>403 Forbidden</title></head> <body> <center><h1>403 Forbidden</h1></center> </body> </html>; synthesize-json: 403 <html> <head><title>403 Forbidden</title></head> <body> <center><h1>403 Forbidden</h1></center> </body> </html>

💡 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.

  • None.
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 — Add or identify targeted runtime/integration validation for the changed behavior; do not report external E2E job pass/fail here.. Runtime/sandbox/infrastructure paths need behavioral runtime validation: agents/hermes/manifest.yaml, agents/langchain-deepagents-code/manifest.yaml, agents/openclaw/manifest.yaml, scripts/update-hermes-agent.sh, src/lib/actions/sandbox/status-text.ts, src/lib/agent/defs.ts, src/lib/domain/maintenance/upgrade.ts, src/lib/sandbox/version-scheme.ts.

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.

@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.

🧹 Nitpick comments (2)
src/lib/agent/defs.test.ts (1)

92-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test hardcodes the same magic threshold used in production code.

This test re-derives the major version and asserts major < 1000, duplicating the CALENDAR_VERSION_MIN_MAJOR constant defined in src/lib/sandbox/version.ts. If that threshold ever changes, this test won't track it and could silently diverge from the real staleness logic it's meant to protect.

Consider exporting CALENDAR_VERSION_MIN_MAJOR from version.ts and importing it here instead of re-declaring 1000.

As per path instructions for **/*.test.{ts,js,mts,mjs,cts,cjs}, tests should "Flag copied production algorithms... and conditionals that make a test pass without exercising its claim."

♻️ Proposed fix
-    const major = Number.parseInt(String(hermes.expectedVersion).split(".")[0] ?? "", 10);
-    expect(major).toBeLessThan(1000);
+    const major = Number.parseInt(String(hermes.expectedVersion).split(".")[0] ?? "", 10);
+    expect(major).toBeLessThan(CALENDAR_VERSION_MIN_MAJOR);

(requires exporting CALENDAR_VERSION_MIN_MAJOR from src/lib/sandbox/version.ts and importing it in this test file)

🤖 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 `@src/lib/agent/defs.test.ts` around lines 92 - 98, The Hermes version test is
duplicating the production calendar-version threshold with a hardcoded major
version check, so it can drift from the real staleness rule. Export
CALENDAR_VERSION_MIN_MAJOR from version.ts and import that constant into
defs.test.ts, then use it in the hermes.expectedVersion assertion instead of
re-deriving or hardcoding 1000.

Source: Path instructions

src/lib/sandbox/version.ts (1)

95-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Document the scheme-mismatch tradeoff.

versionsComparable correctly suppresses false positives when schemes differ (the bug this PR fixes), but it also means a genuine update will not be flagged once an agent's version scheme changes again (e.g., a future switch back to calendar versioning) until both sides use the same scheme. That's a reasonable tradeoff, but it's worth a short comment explaining the assumption so a future reader doesn't mistake isAgentStale returning false for "definitely up to date."

📝 Suggested comment
+// Cross-scheme versions (e.g. calendar "2026.6.19" vs semver "0.17.0") cannot be
+// meaningfully ordered by versionGte. Treat them as "not stale" rather than risk a
+// false-positive update notice; a real update will only be detected once both the
+// installed runtime and the manifest pin use the same scheme again.
 function versionsComparable(left: string, right: string): boolean {
🤖 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 `@src/lib/sandbox/version.ts` around lines 95 - 108, Add a short comment near
versionsComparable/isAgentStale explaining the scheme-mismatch assumption: when
sandboxVersion and expectedVersion use different versioning schemes,
isAgentStale intentionally returns false to avoid false positives, but that also
means a real update may be missed until both sides share the same scheme again.
Reference versionsComparable, isAgentStale, and CALENDAR_VERSION_MIN_MAJOR in
the comment so future readers don’t read false as “definitely up to date.”
🤖 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.

Nitpick comments:
In `@src/lib/agent/defs.test.ts`:
- Around line 92-98: The Hermes version test is duplicating the production
calendar-version threshold with a hardcoded major version check, so it can drift
from the real staleness rule. Export CALENDAR_VERSION_MIN_MAJOR from version.ts
and import that constant into defs.test.ts, then use it in the
hermes.expectedVersion assertion instead of re-deriving or hardcoding 1000.

In `@src/lib/sandbox/version.ts`:
- Around line 95-108: Add a short comment near versionsComparable/isAgentStale
explaining the scheme-mismatch assumption: when sandboxVersion and
expectedVersion use different versioning schemes, isAgentStale intentionally
returns false to avoid false positives, but that also means a real update may be
missed until both sides share the same scheme again. Reference
versionsComparable, isAgentStale, and CALENDAR_VERSION_MIN_MAJOR in the comment
so future readers don’t read false as “definitely up to date.”

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0c9b8e86-3f5c-4f52-8cd2-861e40837376

📥 Commits

Reviewing files that changed from the base of the PR and between e4b9111 and f74b771.

📒 Files selected for processing (4)
  • agents/hermes/manifest.yaml
  • src/lib/agent/defs.test.ts
  • src/lib/sandbox/version.test.ts
  • src/lib/sandbox/version.ts

@github-actions

github-actions Bot commented Jul 1, 2026 •

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: hermes-e2e, rebuild-hermes, upgrade-stale-sandbox, sandbox-operations
Optional E2E: cloud-onboard, rebuild-openclaw, sessions-agents-cli, rebuild-hermes-stale-base

Dispatch hint: hermes-e2e,rebuild-hermes,upgrade-stale-sandbox,sandbox-operations

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • hermes-e2e (high): Required because the Hermes manifest expected_version scheme changed and the new staleness logic must be validated against a real Hermes onboard/runtime/status flow. This catches failures where a live Hermes semver runtime is incorrectly treated as stale or unverifiable.
  • rebuild-hermes (high): Required because the PR changes Hermes version pins and rebuild-time version comparison semantics. This job exercises a real old Hermes sandbox rebuild, reads the manifest expected_version, runs hermes --version, and verifies runtime/state after rebuild.
  • upgrade-stale-sandbox (high): Required because upgrade classification changed for stale, unknown, and verification-failed sandbox version checks. This is the existing live coverage for upgrade-sandboxes detecting stale registered metadata and rebuilding a real sandbox instead of silently skipping it.
  • sandbox-operations (high): Required because status rendering and OpenClaw calendar-scheme manifest handling changed. This job exercises real OpenClaw sandbox list/status/recovery operations and will catch status/checkAgentVersion regressions on the default agent path.

Optional E2E

  • cloud-onboard (high): Useful additional confidence for the full hosted OpenClaw onboarding path because agent definition loading and OpenClaw manifest metadata changed, but the PR does not modify onboarding state-machine/resume code.
  • rebuild-openclaw (high): Useful adjacent coverage for OpenClaw rebuild/version behavior after adding version_scheme: calendar to the OpenClaw manifest.
  • sessions-agents-cli (high): Useful confidence for multi-agent CLI/session behavior after changing shared agent definition loading and adding manifest version_scheme metadata.
  • rebuild-hermes-stale-base (high): Useful extra Hermes confidence for stale base cache rebuild behavior and installed-copy/version pin interactions, but the core Hermes rebuild/version path is already covered by required rebuild-hermes.

New E2E recommendations

  • Hermes scheme-mismatch upgrade coverage (high): Existing live upgrade-stale-sandbox coverage is OpenClaw-based. This PR specifically fixes Hermes calendar-vs-semver drift, but there is no dedicated live E2E that seeds a Hermes sandbox/registry with a legacy calendar agentVersion against the new semver manifest and proves status/upgrade-sandboxes routes it through rebuild.
    • Suggested test: Add a Hermes upgrade-stale or rebuild fixture that creates/uses a Hermes sandbox with legacy calendar cached agentVersion, asserts scheme-mismatch stale guidance, runs the rebuild/upgrade path, and verifies the registry/runtime version is rewritten to semver.

Dispatch hint

  • Workflow: .github/workflows/e2e.yaml
  • jobs input: hermes-e2e,rebuild-hermes,upgrade-stale-sandbox,sandbox-operations

@github-actions

github-actions Bot commented Jul 1, 2026 •

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: ubuntu-repo-cloud-openclaw, ubuntu-repo-cloud-langchain-deepagents-code, rebuild-hermes, upgrade-stale-sandbox
Optional E2E targets: None

Dispatch required E2E targets:

  • gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-openclaw
  • gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-langchain-deepagents-code
  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=rebuild-hermes
  • gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=upgrade-stale-sandbox

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • ubuntu-repo-cloud-openclaw: OpenClaw manifest version-scheme metadata and shared sandbox version/status staleness handling changed; this live-supported typed target exercises the primary Ubuntu Docker OpenClaw onboarding, status, smoke, inference, and credential surface.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-openclaw
  • ubuntu-repo-cloud-langchain-deepagents-code: The LangChain Deep Agents Code manifest gained explicit semver scheme metadata and the agent definition/version plumbing changed; this is the smallest live-supported typed target for the terminal-agent manifest/runtime surface.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-langchain-deepagents-code
  • rebuild-hermes: The PR changes Hermes version scheme handling, stale cached version behavior, and rebuild-facing version checks; rebuild-hermes explicitly validates rebuilding an old Hermes sandbox to the manifest expected version and refreshing registry metadata.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=rebuild-hermes
  • upgrade-stale-sandbox: Shared upgrade classification and sandbox version staleness logic changed; this live job exercises upgrade-sandboxes detection and rebuild of a stale OpenClaw sandbox across real Docker/OpenShell boundaries.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=upgrade-stale-sandbox

Optional E2E targets

  • None.

Relevant changed files

  • agents/hermes/manifest.yaml
  • agents/langchain-deepagents-code/manifest.yaml
  • agents/openclaw/manifest.yaml
  • scripts/update-hermes-agent.sh
  • src/lib/actions/sandbox/status-text.ts
  • src/lib/agent/defs.ts
  • src/lib/domain/maintenance/upgrade.ts
  • src/lib/sandbox/version-scheme.ts
  • src/lib/sandbox/version.ts

@laitingsheng laitingsheng added bug-fix PR fixes a bug or regression NV QA Bugs found by the NVIDIA QA Team area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery integration: hermes Hermes integration behavior labels Jul 1, 2026
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…smatch

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

@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

🧹 Nitpick comments (1)
src/lib/sandbox/version.ts (1)

114-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Keep the staleness helper pure; move stderr output to the boundary.

isAgentStale now writes to process.stderr through warnSchemeMismatch, which makes version comparison depend on host I/O. Return a skip/reason from the helper and let the action/CLI formatting layer decide whether to print it. As per path instructions, “helpers for version comparison/staleness detection within src/lib/sandbox/**” should be “small pure helpers” with “no direct host calls.”

Also applies to: 131-134

🤖 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 `@src/lib/sandbox/version.ts` around lines 114 - 120, The staleness/version
comparison path is doing host I/O inside the helper, specifically
`warnSchemeMismatch` is writing directly to `process.stderr` from within
`isAgentStale` flow. Refactor the sandbox version helpers in
`src/lib/sandbox/version.ts` so `isAgentStale` and `warnSchemeMismatch` stay
pure and only return a skip/reason or warning signal, then move the actual
stderr printing to the action/CLI formatting boundary that consumes the result.
Keep the deduping logic in the helper, but remove any direct process calls from
these version comparison helpers.

Source: Path instructions

🤖 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 `@src/lib/sandbox/version.ts`:
- Around line 102-105: The calendar version check in isCalendarVersion currently
only matches bare numeric tags, so v-prefixed calendar releases still fall
through as non-calendar. Update CALENDAR_VERSION_PATTERN in version.ts to accept
an optional leading v while keeping the existing numeric calendar format, and
keep the logic in isCalendarVersion using that pattern so v2026.6.19 is
classified correctly. Verify the change still excludes non-calendar semver tags
like v0.17.0.

---

Nitpick comments:
In `@src/lib/sandbox/version.ts`:
- Around line 114-120: The staleness/version comparison path is doing host I/O
inside the helper, specifically `warnSchemeMismatch` is writing directly to
`process.stderr` from within `isAgentStale` flow. Refactor the sandbox version
helpers in `src/lib/sandbox/version.ts` so `isAgentStale` and
`warnSchemeMismatch` stay pure and only return a skip/reason or warning signal,
then move the actual stderr printing to the action/CLI formatting boundary that
consumes the result. Keep the deduping logic in the helper, but remove any
direct process calls from these version comparison helpers.
🪄 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: a101e8d0-1282-4044-87f4-9afb61d4a6ce

📥 Commits

Reviewing files that changed from the base of the PR and between 7e15816 and 606b4d5.

📒 Files selected for processing (1)
  • src/lib/sandbox/version.ts

Comment thread src/lib/sandbox/version.ts Outdated
…smatch log

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…bucket

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…fixtures

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
)

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…endar vs semver

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…re-read

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…ime and manifest

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
… year range

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…ismatch line

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…d structured warn payload

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

@prekshivyas prekshivyas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the core logic in the new version-scheme.ts: shape classifier (calendar 2020–2099/3000–9999 vs semver), and evaluateStaleness fails closed on a cross-scheme pair (marks stale + one-shot structured warning) rather than comparing blindly; otherwise defers to versionGte. Root cause is fixed by pinning the manifest expected_version to the runtime's semver. Sound and well-tested (semver match/behind, cross-scheme, ssh-probe). Note: this is the heaviest change in the batch and modifies the sensitive src/lib/sandbox/version.ts path — the author-requested maintainer review of that orchestration should still be recorded per the template. Approving on the scheme-comparator logic + green advisors/CodeRabbit/E2E/CI.

@jyaunches
jyaunches merged commit 2ab9e52 into main Jul 1, 2026
116 of 118 checks passed
@jyaunches
jyaunches deleted the fix/hermes-version-scheme-mismatch branch July 1, 2026 19:35
apurvvkumaria pushed a commit that referenced this pull request Jul 1, 2026
## Summary

After #6089 merged (re-pinning the Hermes manifest to semver `0.17.0`),
the `rebuild-hermes` live test started failing on main. The regex at
line 696 was extracting a calver version in parentheses — `(2026.6.19)`
— but the version output now uses a semver tag format — `v0.17.0`. The
two never match.

## Related Issue

Regression introduced by #6089 (merged as `2ab9e525`). The live
`rebuild-hermes` test doesn't run on PRs so it merged green.

## Changes

- `test/e2e/live/rebuild-hermes.test.ts:696` — change
`/\((\d+\.\d+\.\d+)\)/` to `/v(\d+\.\d+\.\d+)/`

## Type of Change

- [x] Code change (feature, bug fix, or refactor)

## Quality Gates

- [x] Tests added or updated for changed behavior
- [x] Docs not applicable — justification: live test fix, no user-facing
change
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging) — no

## Verification

- [x] Git hooks passed during commit and push
- [x] No secrets, API keys, or credentials committed

---
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>

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

* **Tests**
* Updated end-to-end Hermes version parsing to match the latest CLI
output format.
* Added new test coverage for `nemoclaw-start` gateway preload selection
and persistent gateway log mirroring, including refusal on unsafe
symlinked log directories.
* Removed overlapping preload/log hardening suites from the existing
`nemoclaw-start` test file.
* Introduced a helper to extract shell functions from script sources for
more reliable testing.
* **Chores**
* Adjusted the `nemoclaw-start` test file size budget to reflect the new
suite layout.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@miyoungc miyoungc mentioned this pull request Jul 2, 2026
3 tasks done
ericksoa pushed a commit that referenced this pull request Jul 2, 2026
## Summary
- Add the `v0.0.72` release-note section with links to the deeper docs
pages for installer recovery, command diagnostics, inference, policy,
and sandbox repair changes.
- Document the custom preset `allowed_ips` guard for user-authored
policy files.

## Related Issue
None.

## Source summary
- #6132 -> `docs/about/release-notes.mdx`: Summarizes installer and
upgrade recovery before generic onboarding, with links to quickstart and
lifecycle docs.
- #6087 -> `docs/network-policy/customize-network-policy.mdx`: Documents
that user-authored custom presets reject `allowed_ips` for ordinary
endpoints; also summarized in release notes.
- #5975 -> `docs/about/release-notes.mdx`: Summarizes safer curl-based
inference probes that keep API keys out of process arguments.
- #6044 -> `docs/about/release-notes.mdx`: Summarizes compact `channels
status` configuration reporting.
- #6096 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw EC2
metadata discovery disablement and links to security guidance.
- #5980 and #5991 -> `docs/about/release-notes.mdx`: Summarizes `exec`
multiline argument rejection and recovery guidance.
- #6023 -> `docs/about/release-notes.mdx`: Summarizes
registered-provider diagnostics for `inference set` failures.
- #6074 -> `docs/about/release-notes.mdx`: Summarizes the refreshed
NVIDIA Endpoints featured-model selection behavior.
- #5969 -> `docs/about/release-notes.mdx`: Summarizes `credentials add`
provider credential registration.
- #6060 -> `docs/about/release-notes.mdx`: Summarizes mutable OpenClaw
config permission restoration after `exec`.
- #6134 -> `docs/about/release-notes.mdx`: Summarizes restored Tavily
access for managed Python workflows.
- #6089 -> `docs/about/release-notes.mdx`: Summarizes Hermes runtime
version-scheme comparison during upgrade checks.
- #6131 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw gateway
watchdog recovery behavior.
- #5976 and #5990 -> `docs/about/release-notes.mdx`: Summarizes prompt
stdin EOF cancellation behavior during onboarding.
- #5540 -> `docs/about/release-notes.mdx`: Summarizes clarified
host-level and per-sandbox status command scope.
- #5978 and #6018 -> `docs/about/release-notes.mdx`: Summarizes
policy-denial log breadcrumbs in connect shells.

## Testing
- `npm run docs:sync-agent-variants`
- `npm run docs`
- Commit hooks passed during `git commit`, including commitlint and
gitleaks.
- Pre-push hook passed during `git push`, including TypeScript CLI and
package/tag version sync.

## Checklist
- [x] Documentation updated.
- [x] `npm run docs` completed with 0 errors and 1 existing Fern
warning.
- [x] No source code or generated build artifacts committed.

Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>

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

## Summary by CodeRabbit

* **Documentation**
* Added release notes for v0.0.72 covering improved installer recovery,
clearer CLI diagnostics, safer inference setup and provider switching,
better credential handling, stronger policy boundaries, and more robust
runtime repair behavior.
* Updated network policy guidance to clarify when `allowed_ips` can be
used, including a specific exception for the sandbox-to-host bridge
endpoint.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
…DIA#6089)

<!-- markdownlint-disable MD041 -->
## Summary
`nemoclaw <name> status` on a Hermes sandbox always reported `Update:
v2026.6.19 available` because the staleness check compared the runtime
semver against the manifest calver with a scheme-blind comparator.

## Related Issue
Fixes NVIDIA#6049

## Changes
- `agents/hermes/manifest.yaml`: pin `expected_version` to the semver
the runtime reports.
- `src/lib/sandbox/version.ts`: skip the comparison when runtime and
expected versions are in different schemes.
- `scripts/update-hermes-agent.sh`: align drift check and manifest pin
with `HERMES_SEMVER`.
- Tests: cover semver-match, behind, cross-scheme, and ssh-probe paths.

## 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: corrects an existing `status`
line; no new command or flag.
- [x] 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: requesting maintainer
review of `src/lib/sandbox/version.ts`.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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 (doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>


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

* **Bug Fixes**
* Improved Hermes runtime compatibility checks with scheme-aware version
handling (semver-like vs calendar-style), including accurate stale
detection for both registry and SSH-probed paths.
* Added clearer one-time warnings when sandbox/expected versions use
different version schemes.
* **Tests**
* Expanded Hermes version parsing/validation and runtime detection
coverage, including exact-match, staleness, and non-update scenarios.
* **Chores**
* Updated the Hermes agent update/check script to treat the manifest’s
expected version as semver.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
…IA#6138)

## Summary

After NVIDIA#6089 merged (re-pinning the Hermes manifest to semver `0.17.0`),
the `rebuild-hermes` live test started failing on main. The regex at
line 696 was extracting a calver version in parentheses — `(2026.6.19)`
— but the version output now uses a semver tag format — `v0.17.0`. The
two never match.

## Related Issue

Regression introduced by NVIDIA#6089 (merged as `2ab9e525`). The live
`rebuild-hermes` test doesn't run on PRs so it merged green.

## Changes

- `test/e2e/live/rebuild-hermes.test.ts:696` — change
`/\((\d+\.\d+\.\d+)\)/` to `/v(\d+\.\d+\.\d+)/`

## Type of Change

- [x] Code change (feature, bug fix, or refactor)

## Quality Gates

- [x] Tests added or updated for changed behavior
- [x] Docs not applicable — justification: live test fix, no user-facing
change
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging) — no

## Verification

- [x] Git hooks passed during commit and push
- [x] No secrets, API keys, or credentials committed

---
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>

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

* **Tests**
* Updated end-to-end Hermes version parsing to match the latest CLI
output format.
* Added new test coverage for `nemoclaw-start` gateway preload selection
and persistent gateway log mirroring, including refusal on unsafe
symlinked log directories.
* Removed overlapping preload/log hardening suites from the existing
`nemoclaw-start` test file.
* Introduced a helper to extract shell functions from script sources for
more reliable testing.
* **Chores**
* Adjusted the `nemoclaw-start` test file size budget to reflect the new
suite layout.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary
- Add the `v0.0.72` release-note section with links to the deeper docs
pages for installer recovery, command diagnostics, inference, policy,
and sandbox repair changes.
- Document the custom preset `allowed_ips` guard for user-authored
policy files.

## Related Issue
None.

## Source summary
- NVIDIA#6132 -> `docs/about/release-notes.mdx`: Summarizes installer and
upgrade recovery before generic onboarding, with links to quickstart and
lifecycle docs.
- NVIDIA#6087 -> `docs/network-policy/customize-network-policy.mdx`: Documents
that user-authored custom presets reject `allowed_ips` for ordinary
endpoints; also summarized in release notes.
- NVIDIA#5975 -> `docs/about/release-notes.mdx`: Summarizes safer curl-based
inference probes that keep API keys out of process arguments.
- NVIDIA#6044 -> `docs/about/release-notes.mdx`: Summarizes compact `channels
status` configuration reporting.
- NVIDIA#6096 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw EC2
metadata discovery disablement and links to security guidance.
- NVIDIA#5980 and NVIDIA#5991 -> `docs/about/release-notes.mdx`: Summarizes `exec`
multiline argument rejection and recovery guidance.
- NVIDIA#6023 -> `docs/about/release-notes.mdx`: Summarizes
registered-provider diagnostics for `inference set` failures.
- NVIDIA#6074 -> `docs/about/release-notes.mdx`: Summarizes the refreshed
NVIDIA Endpoints featured-model selection behavior.
- NVIDIA#5969 -> `docs/about/release-notes.mdx`: Summarizes `credentials add`
provider credential registration.
- NVIDIA#6060 -> `docs/about/release-notes.mdx`: Summarizes mutable OpenClaw
config permission restoration after `exec`.
- NVIDIA#6134 -> `docs/about/release-notes.mdx`: Summarizes restored Tavily
access for managed Python workflows.
- NVIDIA#6089 -> `docs/about/release-notes.mdx`: Summarizes Hermes runtime
version-scheme comparison during upgrade checks.
- NVIDIA#6131 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw gateway
watchdog recovery behavior.
- NVIDIA#5976 and NVIDIA#5990 -> `docs/about/release-notes.mdx`: Summarizes prompt
stdin EOF cancellation behavior during onboarding.
- NVIDIA#5540 -> `docs/about/release-notes.mdx`: Summarizes clarified
host-level and per-sandbox status command scope.
- NVIDIA#5978 and NVIDIA#6018 -> `docs/about/release-notes.mdx`: Summarizes
policy-denial log breadcrumbs in connect shells.

## Testing
- `npm run docs:sync-agent-variants`
- `npm run docs`
- Commit hooks passed during `git commit`, including commitlint and
gitleaks.
- Pre-push hook passed during `git push`, including TypeScript CLI and
package/tag version sync.

## Checklist
- [x] Documentation updated.
- [x] `npm run docs` completed with 0 errors and 1 existing Fern
warning.
- [x] No source code or generated build artifacts committed.

Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>

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

## Summary by CodeRabbit

* **Documentation**
* Added release notes for v0.0.72 covering improved installer recovery,
clearer CLI diagnostics, safer inference setup and provider switching,
better credential handling, stronger policy boundaries, and more robust
runtime repair behavior.
* Updated network policy guidance to clarify when `allowed_ips` can be
used, including a specific exception for the sandbox-to-host bridge
endpoint.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery 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.

[DGX Spark][CLI&UX] nemoclaw status falsely reports Hermes "Update: v2026.6.19 available" when installed Hermes Agent v0.17.0 IS v2026.6.19

3 participants