feat(hermes): bump Hermes Agent to v2026.7.1 and enable Slack Block Kit by wscurran · Pull Request #6442 · NVIDIA/NemoClaw

wscurran · 2026-07-08T05:24:31Z

Summary

Hermes v0.18.0 is the first release with Slack Block Kit rendering and native table blocks, so this upgrade enables rich Slack responses and native Markdown-table rendering in NemoClaw's Hermes integration. Building on #5594, it pins Hermes v2026.7.1 / 0.18.0, enables platforms.slack.extra.rich_blocks, and prevents stale v0.17 bases from silently defeating the upgrade. Before merge, the v0.18 multi-architecture base must be published and the immutable final-image digest repinned; this draft safely falls back to a local v0.18 base build until then.

Related Issue

Refs #5591

Changes

Upgrade the Hermes release, semver, tarball checksum, npm integrity, manifest, and update-script fixtures to v2026.7.1 / 0.18.0.
Enable Hermes Slack rich blocks for Block Kit rendering and native table blocks, first released in Hermes v2026.7.1.
Advance generated Hermes config to schema 32 and preserve the migrated agent.verify_on_stop: false behavior.
Retarget and retain the [All Platforms][CLI&UX] --resume <sessionId> starts a new session instead of appending to existing session #5254 resumed one-shot compatibility workarounds required by Hermes v0.18.0.
Require exact Hermes-version and MCP-runtime checks when resolving base images while preserving immutable-digest provenance and cached pinned-platform reuse.
Document rich Slack rendering and add focused config, messaging, base-image, workflow, and live-E2E assertions.

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:
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: Maintainer review requested via this draft; the base-image digest repin is intentionally pending.
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
Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: 30 focused CLI tests, 6 update-script tests, and 5 focused config/workflow tests passed; pin validation and Python compilation also passed.
Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: Not run locally; draft CI will provide the broad gate and sandbox-image coverage.
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: Will Curran wcurran@nvidia.com

Summary by CodeRabbit

New Features
- Hermes now supports richer Slack rendering, including formatted block-based output for tables.
- Updated Hermes compatibility to the latest release version.
Bug Fixes
- Base image selection now rejects incompatible Hermes images more reliably.
- Hermes configuration validation is stricter, helping prevent mismatched setups during startup and upgrades.
Documentation
- Added guidance for Hermes-specific Slack rendering behavior in sandbox messaging docs.

Upgrade Hermes to v2026.7.1 (0.18.0) and enable Slack rich Block Kit output. Preserve trusted base-image resolution while the v0.18 base digest is prepared. Refs #5591 Signed-off-by: Will Curran <wcurran@nvidia.com>

copy-pr-bot · 2026-07-08T05:24:35Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

coderabbitai · 2026-07-08T05:24:39Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e29412d5-b446-495f-bbcf-c0397c583d99

📥 Commits

Reviewing files that changed from the base of the PR and between be3e7cc and a7bf039.

📒 Files selected for processing (20)

.github/actions/resolve-hermes-base-image/action.yaml
agents/hermes/Dockerfile
agents/hermes/Dockerfile.base
agents/hermes/config/hermes-config.ts
agents/hermes/hermes-wrapper.py
agents/hermes/manifest.yaml
agents/hermes/patch-session-list-preview.py
docs/manage-sandboxes/messaging-channels.mdx
src/lib/agent/base-image-hermes-resolution.test.ts
src/lib/agent/base-image-hermes.test.ts
src/lib/agent/base-image.test.ts
src/lib/agent/base-image.ts
src/lib/messaging/channels/manifests.test.ts
src/lib/messaging/channels/slack/manifest.ts
test/e2e/live/hermes-slack-e2e-helpers.ts
test/generate-hermes-config.test.ts
test/helpers/base-image-test-harness.ts
test/hermes-doctor-config-hash.test.ts
test/pr-workflow-contract.test.ts
test/update-hermes-agent-script.test.ts

📝 Walkthrough

Walkthrough

This PR upgrades the pinned Hermes Agent version from 0.17.0 to 0.18.0 across the CI resolver action, Dockerfiles, wrapper, session-preview patch script, manifest, and base-image compatibility checks, and adds Slack rich_blocks support to the Hermes messaging platform config, with corresponding test updates.

Changes

Hermes 0.18.0 version pinning and validation

Layer / File(s)	Summary
CI resolver version gate `.github/actions/resolve-hermes-base-image/action.yaml`, `test/pr-workflow-contract.test.ts`	Extracts expected Hermes semver from Dockerfile.base and validates it against candidate and locally-built images, failing on mismatch; contract tests assert new output fields and mock hermes version output.
Version pins across Dockerfiles, wrapper, manifest, patch script `agents/hermes/Dockerfile*`, `agents/hermes/hermes-wrapper.py`, `agents/hermes/manifest.yaml`, `agents/hermes/patch-session-list-preview.py`, `test/hermes-doctor-config-hash.test.ts`, `test/update-hermes-agent-script.test.ts`	Bumps version/hash constants to v2026.7.1/0.18.0, updates build-time compatibility gate and patch logic for the new session-preview query ordering, and updates related test fixtures.
Base-image compatibility validation `src/lib/agent/base-image.ts`, `src/lib/agent/base-image-hermes.test.ts`, `src/lib/agent/base-image-hermes-resolution.test.ts`, `src/lib/agent/base-image.test.ts`, `agents/hermes/config/hermes-config.ts`, `test/generate-hermes-config.test.ts`, `test/helpers/base-image-test-harness.ts`	Adds Hermes version-aware compatibility validation with MCP integration check, gates explicit override refs earlier, updates config version to 32 with verify_on_stop, and updates related tests.

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

Slack rich blocks for Hermes

Layer / File(s)	Summary
Slack manifest rich_blocks config and docs `src/lib/messaging/channels/slack/manifest.ts`, `src/lib/messaging/channels/manifests.test.ts`, `test/e2e/live/hermes-slack-e2e-helpers.ts`, `test/generate-hermes-config.test.ts`, `docs/manage-sandboxes/messaging-channels.mdx`	Adds `extra.rich_blocks: true` to the Hermes Slack platform config fragment, updates the e2e config-shape probe and unit tests, and documents Slack Block Kit rendering for Hermes.

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

Suggested labels: area: ci, chore

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: Hermes Agent version bump and enabling Slack Block Kit support.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch codex/upgrade-hermes-v0.18-block-kit

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

github-code-quality · 2026-07-08T05:25:08Z

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the codex/upgrade-hermes... 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`	`codex/upgrade-hermes...` a7bf039	+/-
`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%	—

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

github-actions · 2026-07-08T05:25:14Z

🌿 Preview your docs: https://nvidia-preview-pr-6442.docs.buildwithfern.com/nemoclaw

github-actions · 2026-07-08T05:27:54Z

E2E Advisor Recommendation

Required E2E: build-hermes-sandbox-image, hermes-e2e, rebuild-hermes, rebuild-hermes-stale-base, hermes-slack, mcp-bridge
Optional E2E: ubuntu-repo-cloud-hermes, messaging-providers

Dispatch hint: hermes-e2e,rebuild-hermes,rebuild-hermes-stale-base,hermes-slack,mcp-bridge

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

build-hermes-sandbox-image (high): Required because the PR changes Hermes Dockerfiles, the base-image resolver action, wrapper hashes, and runtime compatibility checks. This workflow job builds the actual Hermes production image and runs Hermes sandbox secret-boundary plus root-entrypoint smoke coverage against that image.
hermes-e2e (high): Required because the Hermes version/config/wrapper/session-preview changes can affect real hosted onboarding, Hermes health, CLI/session behavior, and live inference. This job exercises install.sh non-interactive Hermes onboarding and the resumed/continued one-shot session behavior covered by the wrapper/patch changes.
rebuild-hermes (high): Required because src/lib/agent/base-image.ts and Hermes base-image pins affect rebuild selection, final-image staging, runtime upgrades, and durable Hermes state preservation during nemoclaw <sandbox> rebuild --yes.
rebuild-hermes-stale-base (high): Required because this PR specifically changes stale/incompatible Hermes base rejection by adding expected-version validation. The stale-base rebuild job is the existing live guard that a cached old Hermes base is refreshed while state survives.
hermes-slack (high): Required because the PR changes the Hermes Slack channel manifest and E2E helper for rich_blocks config, while Slack setup also covers credential placeholders, secret-boundary scanning, and Slack network policy in a real Hermes sandbox.
mcp-bridge (high): Required because Hermes base-image compatibility now gates on packaged MCP Streamable HTTP support and the resolver action notes that authenticated HTTPS execution is validated by the MCP bridge live test. This covers the MCP credential/policy/security boundary against a real Hermes sandbox.

Optional E2E

ubuntu-repo-cloud-hermes (medium): Useful additional typed-target coverage for the registry-based cloud Hermes onboarding path, but the free-standing hermes-e2e job is the merge-blocking coverage for this PR.
messaging-providers (high): Optional adjacent confidence for provider/credential plumbing across messaging channels. The PR's merge-blocking messaging risk is specifically Hermes Slack, covered by hermes-slack.

New E2E recommendations

Hermes Slack rich rendering (medium): Existing Hermes Slack E2E now verifies that platforms.slack.extra.rich_blocks is rendered, but it does not appear to assert an actual Slack outbound payload containing Block Kit/table blocks. A fake Slack endpoint or controlled Slack fixture would catch regressions where config is present but runtime rendering does not emit blocks.
- Suggested test: Add Hermes Slack live or hermetic E2E coverage that sends a prompt producing a Markdown table and asserts the Slack message payload includes Block Kit/table blocks when rich_blocks is enabled.

Dispatch hint

Workflow: .github/workflows/e2e.yaml
jobs input: hermes-e2e,rebuild-hermes,rebuild-hermes-stale-base,hermes-slack,mcp-bridge

github-actions · 2026-07-08T05:27:55Z

E2E Target Recommendation

Required E2E targets: rebuild-hermes-stale-base, hermes-slack
Optional E2E targets: mcp-bridge

Dispatch required E2E targets:

gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=rebuild-hermes-stale-base
gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=hermes-slack

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

rebuild-hermes-stale-base: Hermes base-image resolution now rejects stale Hermes versions and Dockerfile.base was bumped. The stale-base rebuild job directly exercises cached/stale Hermes base handling and rebuild behavior.
- Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=rebuild-hermes-stale-base
hermes-slack: The PR changes Hermes Slack rendering configuration and the Hermes Slack live E2E helper. e2e.yaml wires test/e2e/live/hermes-slack-e2e.test.ts as the hermes-slack job, which is the smallest dispatch for this surface.
- Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=hermes-slack

Optional E2E targets

mcp-bridge: Optional adjacent coverage for the Hermes base-image MCP runtime compatibility probe; no MCP bridge source changed, but the Hermes base version bump could affect packaged MCP extras.
- Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=mcp-bridge

Relevant changed files

.github/actions/resolve-hermes-base-image/action.yaml
agents/hermes/Dockerfile
agents/hermes/Dockerfile.base
agents/hermes/config/hermes-config.ts
agents/hermes/hermes-wrapper.py
agents/hermes/manifest.yaml
agents/hermes/patch-session-list-preview.py
src/lib/agent/base-image.ts
src/lib/messaging/channels/slack/manifest.ts
test/e2e/live/hermes-slack-e2e-helpers.ts

github-actions · 2026-07-08T05:28:12Z

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-3: Monolith growth: Hermes-specific logic not extracted to separate module; then add or justify PRA-T1.
Open items: 3 required · 7 warnings · 2 suggestions · 8 test follow-ups
Since last review: 2 prior items resolved · 8 still apply · 3 new items found

Action checklist

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`	Required	architecture	`src/lib/agent/base-image.ts:1`	Extract the five Hermes-specific functions and two constants into src/lib/agent/hermes-base-image.ts. Update imports in base-image.ts, base-image-hermes.test.ts, base-image-hermes-resolution.test.ts, and base-image.test.ts. Verify base-image.ts returns to ~373 lines and all tests pass.
`PRA-4`	Required	security	`.github/actions/resolve-hermes-base-image/action.yaml:52`	Update regex to `sed -nE 's/.[vV]?([0-9]+\.[0-9]+\.[0-9]+)./\1/p; s/^([0-9]+\.[0-9]+\.[0-9]+)$/\1/p'` to make 'v' prefix optional and case-insensitive. Add integration test in pr-workflow-contract.test.ts parameterizing mocked docker output across 'Hermes Agent 0.18.0', 'Hermes Agent v0.18.0 (2026.7.1)', 'hermes v0.18.0', '0.18.0'.
`PRA-5`	Required	security	`agents/hermes/Dockerfile:206`	Run sha256sum agents/hermes/hermes-wrapper.py locally and confirm output matches the ARG value. If not, update the ARG in Dockerfile line 206.
`PRA-6`	Resolve/justify	acceptance	`agents/hermes/config/hermes-config.ts:109`	Add a complete source-of-truth comment block near verify_on_stop: false answering all five questions explicitly.
`PRA-7`	Resolve/justify	tests	`src/lib/adapters/openshell/client.test.ts:64`	Add test cases to client.test.ts parseVersionFromText suite for all four Hermes v0.18.0 output variants, asserting each returns '0.18.0'.
`PRA-8`	Resolve/justify	correctness	`test/generate-hermes-config.test.ts:1`	Either: (a) Add a test in generate-hermes-config.test.ts that loads a v30/v31 config fixture, runs the generator, and asserts verify_on_stop: false in output; or (b) Document in hermes-config.ts that NemoClaw only generates fresh configs at v32 and migration is upstream Hermes' responsibility.
`PRA-9`	Resolve/justify	tests	`test/pr-workflow-contract.test.ts:1290`	Extend the existing 'rejects a pulled Hermes base without MCP HTTP imports' test or add a new parameterized test case that mocks docker run --entrypoint /usr/local/bin/hermes with outputs: 'Hermes Agent 0.18.0', 'Hermes Agent v0.18.0 (2026.7.1)', 'hermes v0.18.0', '0.18.0' — each should extract '0.18.0' and pass validation.
`PRA-10`	Resolve/justify	security	`src/lib/agent/base-image.ts:135`	Verify parseVersionFromText correctly extracts version from 'hermes --version' output format. The function's executable-aware parsing looks for the executable name as a word boundary; 'Hermes Agent' may not match 'hermes'. Consider passing versionCommand as 'hermes' (just the binary name) or ensure the parsing works for Hermes' actual output format.
`PRA-11`	Improvement	correctness	`src/lib/messaging/channels/slack/manifest.ts:104`	No action required — feature is well-scoped. Verify e2e test in test/e2e/live/hermes-slack-e2e-helpers.ts validates the rich_blocks config is present in generated config.yaml.
`PRA-12`	Improvement	scope	`src/lib/messaging/channels/manifests.test.ts:470`	No immediate action. Monitor for further growth. If more channels add render validation tests, extract a test helper.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

`PRA-3` Required — Monolith growth: Hermes-specific logic not extracted to separate module

Location: src/lib/agent/base-image.ts:1
Category: architecture
Problem: Five Hermes-specific functions (getHermesPinnedRemoteBaseRef, hermesFinalDockerfileAcceptsBase, hermesBaseImageIsCompatible, hermesBaseImageSupportsMcp, plus constants HERMES_OFFICIAL_BASE_DIGEST_REF and HERMES_MCP_RUNTIME_PROBE_OK) remain in base-image.ts, growing it from 373 to 403 lines. Prior review required extraction to src/lib/agent/hermes-base-image.ts.
Impact: Continued accumulation of agent-specific logic in base-image.ts makes the file harder to maintain, test, and reason about. Future agent additions will compound the problem. Violates monolith growth guardrail.
Required action: Extract the five Hermes-specific functions and two constants into src/lib/agent/hermes-base-image.ts. Update imports in base-image.ts, base-image-hermes.test.ts, base-image-hermes-resolution.test.ts, and base-image.test.ts. Verify base-image.ts returns to ~373 lines and all tests pass.
Expected follow-up: Fix before merge or get explicit maintainer override.
Verification: wc -l src/lib/agent/base-image.ts should show ~373 lines; new file src/lib/agent/hermes-base-image.ts should exist with extracted functions; npm test passes
Missing regression test: No new test needed — existing base-image-hermes.test.ts and base-image-hermes-resolution.test.ts cover the extracted logic. Verify they still pass after extraction.
Done when: The required change is committed and verification passes: wc -l src/lib/agent/base-image.ts should show ~373 lines; new file src/lib/agent/hermes-base-image.ts should exist with extracted functions; npm test passes.
Evidence: base-image.ts lines 88-156 contain getHermesPinnedRemoteBaseRef, hermesFinalDockerfileAcceptsBase, hermesBaseImageIsCompatible, hermesBaseImageSupportsMcp; line 180+ contains Hermes branch of createAgentBaseImageResolutionOptions; constants at lines 25-30

`PRA-4` Required — Resolver hermes_version() regex rejects valid Hermes --version output formats

Location: .github/actions/resolve-hermes-base-image/action.yaml:52
Category: security
Problem: The hermes_version() function regex `sed -nE 's/.*v([0-9]+\.[0-9]+\.[0-9]+).*/\1/p; s/^([0-9]+\.[0-9]+\.[0-9]+)$/\1/p'` only matches versions with 'v' prefix or bare version numbers. It will NOT match 'Hermes Agent 0.18.0' (no 'v'), 'hermes v0.18.0' (lowercase), or 'Hermes Agent v0.18.0 (2026.7.1)' (parenthetical). This causes valid images to be rejected, falling back to local builds.
Impact: A legitimate Hermes base image with a variant version string format would be incorrectly rejected, increasing CI time via unnecessary local builds. Conversely, if the regex incorrectly matches a non-version string, a stale image could pass.
Required action: Update regex to `sed -nE 's/.*[vV]?([0-9]+\.[0-9]+\.[0-9]+).*/\1/p; s/^([0-9]+\.[0-9]+\.[0-9]+)$/\1/p'` to make 'v' prefix optional and case-insensitive. Add integration test in pr-workflow-contract.test.ts parameterizing mocked docker output across 'Hermes Agent 0.18.0', 'Hermes Agent v0.18.0 (2026.7.1)', 'hermes v0.18.0', '0.18.0'.
Expected follow-up: Fix before merge or get explicit maintainer override.
Verification: Read action.yaml lines 52-54 and verify sed pattern handles all four formats. Run the new integration test to confirm each extracts '0.18.0'.
Missing regression test: Integration test in pr-workflow-contract.test.ts mocking docker run --entrypoint /usr/local/bin/hermes with outputs: 'Hermes Agent 0.18.0', 'Hermes Agent v0.18.0 (2026.7.1)', 'hermes v0.18.0', '0.18.0' — each should extract '0.18.0' and pass validation
Done when: The required change is committed and verification passes: Read action.yaml lines 52-54 and verify sed pattern handles all four formats. Run the new integration test to confirm each extracts '0.18.0'.
Evidence: action.yaml:52-54 hermes_version() function with sed -nE 's/.*v([0-9]+\.[0-9]+\.[0-9]+).*/\1/p; s/^([0-9]+\.[0-9]+\.[0-9]+)$/\1/p'

`PRA-5` Required — Wrapper SHA256 hash must be verified against actual hermes-wrapper.py content

Location: agents/hermes/Dockerfile:206
Category: security
Problem: Dockerfile declares ARG NEMOCLAW_HERMES_WRAPPER_SHA256=ec8b0e4d6254175929d5a02240acd6af25e94774a30b36ba5c6f5a69c32fb9d7. This must match the SHA256 of agents/hermes/hermes-wrapper.py in the PR to maintain supply-chain integrity. Cannot verify in this environment (no sha256sum).
Impact: If the hash doesn't match, the build will fail (correct behavior). If it matches but the file was tampered with in a way that preserves the hash (collision), supply-chain integrity is compromised. Verification is a required gate.
Required action: Run sha256sum agents/hermes/hermes-wrapper.py locally and confirm output matches the ARG value. If not, update the ARG in Dockerfile line 206.
Expected follow-up: Fix before merge or get explicit maintainer override.
Verification: Execute: sha256sum agents/hermes/hermes-wrapper.py and compare to ARG value in Dockerfile line 206
Missing regression test: Build-time sha256sum -c check in Dockerfile lines 246-248 enforces this on every build
Done when: The required change is committed and verification passes: Execute: sha256sum agents/hermes/hermes-wrapper.py and compare to ARG value in Dockerfile line 206.
Evidence: Dockerfile line 206: ARG NEMOCLAW_HERMES_WRAPPER_SHA256=ec8b0e4d6254175929d5a02240acd6af25e94774a30b36ba5c6f5a69c32fb9d7; lines 246-248: sha256sum -c verification

Review findings by urgency: 3 required fixes, 7 items to resolve/justify, 2 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 — Source-of-truth review needed: agents/hermes/config/hermes-config.ts:109 (verify_on_stop: false)

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: generate-hermes-config.test.ts:382 — expect(config.agent?.verify_on_stop).toBe(false)
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: hermes-config.ts lines 105-112 comment only partially addresses 5 questions; generate-hermes-config.test.ts:382 verifies fresh config value

`PRA-2` Resolve/justify — Source-of-truth review needed: .github/actions/resolve-hermes-base-image/action.yaml:52 (hermes_version regex)

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: Missing — pr-workflow-contract.test.ts only mocks one format
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: action.yaml:52 regex pattern; pr-workflow-contract.test.ts:1290 mocks only one format

`PRA-6` Resolve/justify — verify_on_stop: false source-of-truth comment incomplete

Location: agents/hermes/config/hermes-config.ts:109
Category: acceptance
Problem: The verify_on_stop: false setting has a comment but it does not answer all five source-of-truth questions: (1) Invalid state: Hermes defaults verify_on_stop to 'auto' which may prompt unexpectedly; (2) Source boundary: Hermes upstream default; (3) Why not fixed upstream: NemoClaw controls config generation, Hermes default may change; (4) Regression test: generate-hermes-config.test.ts verifies fresh v32 config has verify_on_stop: false; (5) Removal condition: When Hermes upstream changes default to false or NemoClaw stops generating config. Current comment only addresses (1) and (4) partially.
Impact: Without complete source-of-truth documentation, future maintainers cannot evaluate whether this workaround is still needed or can be removed. Risk of stale behavior persisting.
Recommended action: Add a complete source-of-truth comment block near verify_on_stop: false answering all five questions explicitly.
Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
Verification: Read hermes-config.ts lines 105-112 and verify comment block covers all five source-of-truth questions
Missing regression test: generate-hermes-config.test.ts:382 already verifies fresh config has verify_on_stop: false; no new test needed
Done when: The risk is fixed or explicitly justified in the PR. Verification: Read hermes-config.ts lines 105-112 and verify comment block covers all five source-of-truth questions.
Evidence: hermes-config.ts lines 105-112: comment mentions 'Config v30→32 migrations switch the old implicit/auto behavior off' but lacks full 5-question block

`PRA-7` Resolve/justify — parseVersionFromText missing unit tests for Hermes v0.18.0 output variants

Location: src/lib/adapters/openshell/client.test.ts:64
Category: tests
Problem: parseVersionFromText test only covers 'Hermes Agent v0.17.0 (2026.6.19)'. Missing tests for Hermes v0.18.0 output variants: 'Hermes Agent 0.18.0', 'Hermes Agent v0.18.0 (2026.7.1)', 'hermes v0.18.0', '0.18.0'. The resolver action and base-image.ts both depend on this function for version extraction.
Impact: Version extraction may fail on valid Hermes v0.18.0 output formats, causing resolver to reject valid images or base-image validation to fail incorrectly.
Recommended action: Add test cases to client.test.ts parseVersionFromText suite for all four Hermes v0.18.0 output variants, asserting each returns '0.18.0'.
Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
Verification: Read client.test.ts lines 60-70 and verify test cases for all four formats exist and pass
Missing regression test: Four new test cases in client.test.ts: expect(parseVersionFromText('Hermes Agent 0.18.0')).toBe('0.18.0'); expect(parseVersionFromText('Hermes Agent v0.18.0 (2026.7.1)')).toBe('0.18.0'); expect(parseVersionFromText('hermes v0.18.0')).toBe('0.18.0'); expect(parseVersionFromText('0.18.0')).toBe('0.18.0')
Done when: The risk is fixed or explicitly justified in the PR. Verification: Read client.test.ts lines 60-70 and verify test cases for all four formats exist and pass.
Evidence: client.test.ts:64 only tests 'Hermes Agent v0.17.0 (2026.6.19)'

`PRA-8` Resolve/justify — Config migration behavior for verify_on_stop: false undocumented/untested

Location: test/generate-hermes-config.test.ts:1
Category: correctness
Problem: Config migration behavior for verify_on_stop: false is undocumented/untested. No test loads a v30/v31 config fixture, runs the generator, and asserts verify_on_stop: false in output. The comment in hermes-config.ts claims 'Config v30→32 migrations switch the old implicit/auto behavior off' but there's no migration code visible in the generator — fresh configs just start at v32 with false.
Impact: The migration claim is unverified. If NemoClaw ever needs to migrate existing v30/v31 configs, the behavior is unknown and untested.
Recommended action: Either: (a) Add a test in generate-hermes-config.test.ts that loads a v30/v31 config fixture, runs the generator, and asserts verify_on_stop: false in output; or (b) Document in hermes-config.ts that NemoClaw only generates fresh configs at v32 and migration is upstream Hermes' responsibility.
Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
Verification: Search generate-hermes-config.test.ts for 'v30', 'v31', 'migration', or 'verify_on_stop' fixture test — should find one or the other
Missing regression test: Test case loading v30/v31 config fixture (or documented justification that migration is not NemoClaw's responsibility)
Done when: The risk is fixed or explicitly justified in the PR. Verification: Search generate-hermes-config.test.ts for 'v30', 'v31', 'migration', or 'verify_on_stop' fixture test — should find one or the other.
Evidence: hermes-config.ts:105-108 comment claims migration behavior; generate-hermes-config.test.ts has no v30/v31 fixture test

`PRA-9` Resolve/justify — Resolver action integration test only covers single Hermes version string format

Location: test/pr-workflow-contract.test.ts:1290
Category: tests
Problem: Resolver action integration test only mocks a single Hermes version string format: 'Hermes Agent v0.18.0 (2026.7.1)'. It does not test variant formats that the regex must handle. The test passes because the mocked format matches the current regex, but other valid formats would fail in production.
Impact: Resolver action may reject valid base images in CI when Hermes outputs a different version format, causing unnecessary local builds and CI delays.
Recommended action: Extend the existing 'rejects a pulled Hermes base without MCP HTTP imports' test or add a new parameterized test case that mocks docker run --entrypoint /usr/local/bin/hermes with outputs: 'Hermes Agent 0.18.0', 'Hermes Agent v0.18.0 (2026.7.1)', 'hermes v0.18.0', '0.18.0' — each should extract '0.18.0' and pass validation.
Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
Verification: Read pr-workflow-contract.test.ts around line 1290 and verify parameterized test covering all four variant formats
Missing regression test: Parameterized integration test in pr-workflow-contract.test.ts exercising resolver action against mocked docker returning the four variant version string formats
Done when: The risk is fixed or explicitly justified in the PR. Verification: Read pr-workflow-contract.test.ts around line 1290 and verify parameterized test covering all four variant formats.
Evidence: pr-workflow-contract.test.ts:1290 mocks only 'Hermes Agent v0.18.0 (2026.7.1)'

`PRA-10` Resolve/justify — hermesBaseImageIsCompatible versionCommand may not match executable name in Hermes output

Location: src/lib/agent/base-image.ts:135
Category: security
Problem: New function hermesBaseImageIsCompatible() calls parseVersionFromText(versionOutput, 'hermes --version') but the versionCommand parameter 'hermes --version' may not match the actual executable name in the version output. parseVersionFromText uses the executable name to find the version on the correct line. If Hermes outputs 'Hermes Agent v0.18.0' without 'hermes' in the executable position, the executable-aware parsing may not find the version.
Impact: Version extraction may silently fail or return null for valid Hermes output, causing base image compatibility check to fail incorrectly.
Recommended action: Verify parseVersionFromText correctly extracts version from 'hermes --version' output format. The function's executable-aware parsing looks for the executable name as a word boundary; 'Hermes Agent' may not match 'hermes'. Consider passing versionCommand as 'hermes' (just the binary name) or ensure the parsing works for Hermes' actual output format.
Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
Verification: Check parseVersionFromText implementation in client.ts:62-85 — it splits versionCommand by whitespace and takes first token, then splits by '/' and takes last part. For 'hermes --version', executable='hermes'. Then it searches for '\bhermes\b' case-insensitive in each line. 'Hermes Agent v0.18.0' contains 'Hermes' not 'hermes' — case-insensitive match should work. But need to verify.
Missing regression test: Unit test in base-image-hermes.test.ts or client.test.ts verifying hermesBaseImageIsCompatible correctly extracts version from actual Hermes --version output
Done when: The risk is fixed or explicitly justified in the PR. Verification: Check parseVersionFromText implementation in client.ts:62-85 — it splits versionCommand by whitespace and takes first token, then splits by '/' and takes last part. For 'hermes --version', executable='hermes'. Then it searches for '\bhermes\b' case-insensitive in each line. 'Hermes Agent v0.18.0' contains 'Hermes' not 'hermes' — case-insensitive match should work. But need to verify.
Evidence: base-image.ts:135 calls parseVersionFromText(versionOutput, 'hermes --version'); client.ts:62-85 executable-aware parsing logic

💡 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-11` Improvement — Slack Block Kit rich_blocks feature well-scoped — verify e2e coverage

Location: src/lib/messaging/channels/slack/manifest.ts:104
Category: correctness
Problem: New Slack Block Kit rendering feature (rich_blocks: true) added to Hermes platform config. This is a net-positive feature but the manifest change adds a new JSON fragment render target. The extra.rich_blocks is a static boolean, so credential leakage or injection risk is low.
Impact: Minimal — feature is well-scoped and static. E2E test validates the config is present.
Suggested action: No action required — feature is well-scoped. Verify e2e test in test/e2e/live/hermes-slack-e2e-helpers.ts validates the rich_blocks config is present in generated config.yaml.
Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
Verification: Read test/e2e/live/hermes-slack-e2e-helpers.ts lines 260-275 — configProbe asserts platforms.slack.extra.rich_blocks is true
Missing regression test: E2E test already covers this in configProbe assertion
Done when: The local improvement is applied, or the PR notes why it should be deferred.
Evidence: slack/manifest.ts:104 adds extra: { rich_blocks: true }; manifests.test.ts:470 asserts render fragment; hermes-slack-e2e-helpers.ts configProbe validates

`PRA-12` Improvement — Test file growth acceptable but monitor for extraction opportunity

Location: src/lib/messaging/channels/manifests.test.ts:470
Category: scope
Problem: Test file grew by 13 lines (918→931) adding Slack rich_blocks render assertion. Prior review noted to monitor growth beyond ~1000 lines. Current growth is acceptable but if additional channels add similar render validation tests, consider extracting a test helper module for manifest render assertions.
Impact: Low — current growth is justified. Risk of future bloat if more channels add similar tests.
Suggested action: No immediate action. Monitor for further growth. If more channels add render validation tests, extract a test helper.
Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
Verification: wc -l src/lib/messaging/channels/manifests.test.ts — currently 931 lines
Missing regression test: None — monitoring only
Done when: The local improvement is applied, or the PR notes why it should be deferred.
Evidence: manifests.test.ts grew from 918 to 931 lines; Slack rich_blocks assertion added at line ~470

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 — parseVersionFromText extracts 0.18.0 from Hermes Agent 0.18.0 (no v prefix). Infrastructure/sandbox paths changed: action.yaml, Dockerfiles, hermes-config.ts, wrapper.py. Runtime validation needed for resolver version regex, wrapper hash, parseVersionFromText variants, config migration, and executable matching.
PRA-T2 Runtime validation — parseVersionFromText extracts 0.18.0 from Hermes Agent v0.18.0 (2026.7.1). Infrastructure/sandbox paths changed: action.yaml, Dockerfiles, hermes-config.ts, wrapper.py. Runtime validation needed for resolver version regex, wrapper hash, parseVersionFromText variants, config migration, and executable matching.
PRA-T3 Runtime validation — parseVersionFromText extracts 0.18.0 from hermes v0.18.0 (lowercase). Infrastructure/sandbox paths changed: action.yaml, Dockerfiles, hermes-config.ts, wrapper.py. Runtime validation needed for resolver version regex, wrapper hash, parseVersionFromText variants, config migration, and executable matching.
PRA-T4 Runtime validation — parseVersionFromText extracts 0.18.0 from 0.18.0 (bare). Infrastructure/sandbox paths changed: action.yaml, Dockerfiles, hermes-config.ts, wrapper.py. Runtime validation needed for resolver version regex, wrapper hash, parseVersionFromText variants, config migration, and executable matching.
PRA-T5 Runtime validation — resolver action accepts Hermes Agent 0.18.0 / hermes v0.18.0 / 0.18.0 formats. Infrastructure/sandbox paths changed: action.yaml, Dockerfiles, hermes-config.ts, wrapper.py. Runtime validation needed for resolver version regex, wrapper hash, parseVersionFromText variants, config migration, and executable matching.
PRA-T6 parseVersionFromText missing unit tests for Hermes v0.18.0 output variants — Add test cases to client.test.ts parseVersionFromText suite for all four Hermes v0.18.0 output variants, asserting each returns '0.18.0'.
PRA-T7 Resolver action integration test only covers single Hermes version string format — Extend the existing 'rejects a pulled Hermes base without MCP HTTP imports' test or add a new parameterized test case that mocks docker run --entrypoint /usr/local/bin/hermes with outputs: 'Hermes Agent 0.18.0', 'Hermes Agent v0.18.0 (2026.7.1)', 'hermes v0.18.0', '0.18.0' — each should extract '0.18.0' and pass validation.
PRA-T8 Acceptance clause — Exact Hermes-version + MCP checks in resolver — add test evidence or identify existing coverage. action.yaml: expected_hermes_version from Dockerfile.base, hermes_version() call; pr-workflow-contract.test.ts mocks hermes version output; BUT regex gap rejects valid formats (see blocker finding)

Since last review details

Current findings, using the urgency labels above:

`PRA-1` Resolve/justify — Source-of-truth review needed: agents/hermes/config/hermes-config.ts:109 (verify_on_stop: false)

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: generate-hermes-config.test.ts:382 — expect(config.agent?.verify_on_stop).toBe(false)
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: hermes-config.ts lines 105-112 comment only partially addresses 5 questions; generate-hermes-config.test.ts:382 verifies fresh config value

`PRA-2` Resolve/justify — Source-of-truth review needed: .github/actions/resolve-hermes-base-image/action.yaml:52 (hermes_version regex)

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: Missing — pr-workflow-contract.test.ts only mocks one format
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: action.yaml:52 regex pattern; pr-workflow-contract.test.ts:1290 mocks only one format

`PRA-3` Required — Monolith growth: Hermes-specific logic not extracted to separate module

Location: src/lib/agent/base-image.ts:1
Category: architecture
Problem: Five Hermes-specific functions (getHermesPinnedRemoteBaseRef, hermesFinalDockerfileAcceptsBase, hermesBaseImageIsCompatible, hermesBaseImageSupportsMcp, plus constants HERMES_OFFICIAL_BASE_DIGEST_REF and HERMES_MCP_RUNTIME_PROBE_OK) remain in base-image.ts, growing it from 373 to 403 lines. Prior review required extraction to src/lib/agent/hermes-base-image.ts.
Impact: Continued accumulation of agent-specific logic in base-image.ts makes the file harder to maintain, test, and reason about. Future agent additions will compound the problem. Violates monolith growth guardrail.
Required action: Extract the five Hermes-specific functions and two constants into src/lib/agent/hermes-base-image.ts. Update imports in base-image.ts, base-image-hermes.test.ts, base-image-hermes-resolution.test.ts, and base-image.test.ts. Verify base-image.ts returns to ~373 lines and all tests pass.
Expected follow-up: Fix before merge or get explicit maintainer override.
Verification: wc -l src/lib/agent/base-image.ts should show ~373 lines; new file src/lib/agent/hermes-base-image.ts should exist with extracted functions; npm test passes
Missing regression test: No new test needed — existing base-image-hermes.test.ts and base-image-hermes-resolution.test.ts cover the extracted logic. Verify they still pass after extraction.
Done when: The required change is committed and verification passes: wc -l src/lib/agent/base-image.ts should show ~373 lines; new file src/lib/agent/hermes-base-image.ts should exist with extracted functions; npm test passes.
Evidence: base-image.ts lines 88-156 contain getHermesPinnedRemoteBaseRef, hermesFinalDockerfileAcceptsBase, hermesBaseImageIsCompatible, hermesBaseImageSupportsMcp; line 180+ contains Hermes branch of createAgentBaseImageResolutionOptions; constants at lines 25-30

`PRA-4` Required — Resolver hermes_version() regex rejects valid Hermes --version output formats

Location: .github/actions/resolve-hermes-base-image/action.yaml:52
Category: security
Problem: The hermes_version() function regex `sed -nE 's/.*v([0-9]+\.[0-9]+\.[0-9]+).*/\1/p; s/^([0-9]+\.[0-9]+\.[0-9]+)$/\1/p'` only matches versions with 'v' prefix or bare version numbers. It will NOT match 'Hermes Agent 0.18.0' (no 'v'), 'hermes v0.18.0' (lowercase), or 'Hermes Agent v0.18.0 (2026.7.1)' (parenthetical). This causes valid images to be rejected, falling back to local builds.
Impact: A legitimate Hermes base image with a variant version string format would be incorrectly rejected, increasing CI time via unnecessary local builds. Conversely, if the regex incorrectly matches a non-version string, a stale image could pass.
Required action: Update regex to `sed -nE 's/.*[vV]?([0-9]+\.[0-9]+\.[0-9]+).*/\1/p; s/^([0-9]+\.[0-9]+\.[0-9]+)$/\1/p'` to make 'v' prefix optional and case-insensitive. Add integration test in pr-workflow-contract.test.ts parameterizing mocked docker output across 'Hermes Agent 0.18.0', 'Hermes Agent v0.18.0 (2026.7.1)', 'hermes v0.18.0', '0.18.0'.
Expected follow-up: Fix before merge or get explicit maintainer override.
Verification: Read action.yaml lines 52-54 and verify sed pattern handles all four formats. Run the new integration test to confirm each extracts '0.18.0'.
Missing regression test: Integration test in pr-workflow-contract.test.ts mocking docker run --entrypoint /usr/local/bin/hermes with outputs: 'Hermes Agent 0.18.0', 'Hermes Agent v0.18.0 (2026.7.1)', 'hermes v0.18.0', '0.18.0' — each should extract '0.18.0' and pass validation
Done when: The required change is committed and verification passes: Read action.yaml lines 52-54 and verify sed pattern handles all four formats. Run the new integration test to confirm each extracts '0.18.0'.
Evidence: action.yaml:52-54 hermes_version() function with sed -nE 's/.*v([0-9]+\.[0-9]+\.[0-9]+).*/\1/p; s/^([0-9]+\.[0-9]+\.[0-9]+)$/\1/p'

`PRA-5` Required — Wrapper SHA256 hash must be verified against actual hermes-wrapper.py content

Location: agents/hermes/Dockerfile:206
Category: security
Problem: Dockerfile declares ARG NEMOCLAW_HERMES_WRAPPER_SHA256=ec8b0e4d6254175929d5a02240acd6af25e94774a30b36ba5c6f5a69c32fb9d7. This must match the SHA256 of agents/hermes/hermes-wrapper.py in the PR to maintain supply-chain integrity. Cannot verify in this environment (no sha256sum).
Impact: If the hash doesn't match, the build will fail (correct behavior). If it matches but the file was tampered with in a way that preserves the hash (collision), supply-chain integrity is compromised. Verification is a required gate.
Required action: Run sha256sum agents/hermes/hermes-wrapper.py locally and confirm output matches the ARG value. If not, update the ARG in Dockerfile line 206.
Expected follow-up: Fix before merge or get explicit maintainer override.
Verification: Execute: sha256sum agents/hermes/hermes-wrapper.py and compare to ARG value in Dockerfile line 206
Missing regression test: Build-time sha256sum -c check in Dockerfile lines 246-248 enforces this on every build
Done when: The required change is committed and verification passes: Execute: sha256sum agents/hermes/hermes-wrapper.py and compare to ARG value in Dockerfile line 206.
Evidence: Dockerfile line 206: ARG NEMOCLAW_HERMES_WRAPPER_SHA256=ec8b0e4d6254175929d5a02240acd6af25e94774a30b36ba5c6f5a69c32fb9d7; lines 246-248: sha256sum -c verification

`PRA-6` Resolve/justify — verify_on_stop: false source-of-truth comment incomplete

Location: agents/hermes/config/hermes-config.ts:109
Category: acceptance
Problem: The verify_on_stop: false setting has a comment but it does not answer all five source-of-truth questions: (1) Invalid state: Hermes defaults verify_on_stop to 'auto' which may prompt unexpectedly; (2) Source boundary: Hermes upstream default; (3) Why not fixed upstream: NemoClaw controls config generation, Hermes default may change; (4) Regression test: generate-hermes-config.test.ts verifies fresh v32 config has verify_on_stop: false; (5) Removal condition: When Hermes upstream changes default to false or NemoClaw stops generating config. Current comment only addresses (1) and (4) partially.
Impact: Without complete source-of-truth documentation, future maintainers cannot evaluate whether this workaround is still needed or can be removed. Risk of stale behavior persisting.
Recommended action: Add a complete source-of-truth comment block near verify_on_stop: false answering all five questions explicitly.
Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
Verification: Read hermes-config.ts lines 105-112 and verify comment block covers all five source-of-truth questions
Missing regression test: generate-hermes-config.test.ts:382 already verifies fresh config has verify_on_stop: false; no new test needed
Done when: The risk is fixed or explicitly justified in the PR. Verification: Read hermes-config.ts lines 105-112 and verify comment block covers all five source-of-truth questions.
Evidence: hermes-config.ts lines 105-112: comment mentions 'Config v30→32 migrations switch the old implicit/auto behavior off' but lacks full 5-question block

`PRA-7` Resolve/justify — parseVersionFromText missing unit tests for Hermes v0.18.0 output variants

Location: src/lib/adapters/openshell/client.test.ts:64
Category: tests
Problem: parseVersionFromText test only covers 'Hermes Agent v0.17.0 (2026.6.19)'. Missing tests for Hermes v0.18.0 output variants: 'Hermes Agent 0.18.0', 'Hermes Agent v0.18.0 (2026.7.1)', 'hermes v0.18.0', '0.18.0'. The resolver action and base-image.ts both depend on this function for version extraction.
Impact: Version extraction may fail on valid Hermes v0.18.0 output formats, causing resolver to reject valid images or base-image validation to fail incorrectly.
Recommended action: Add test cases to client.test.ts parseVersionFromText suite for all four Hermes v0.18.0 output variants, asserting each returns '0.18.0'.
Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
Verification: Read client.test.ts lines 60-70 and verify test cases for all four formats exist and pass
Missing regression test: Four new test cases in client.test.ts: expect(parseVersionFromText('Hermes Agent 0.18.0')).toBe('0.18.0'); expect(parseVersionFromText('Hermes Agent v0.18.0 (2026.7.1)')).toBe('0.18.0'); expect(parseVersionFromText('hermes v0.18.0')).toBe('0.18.0'); expect(parseVersionFromText('0.18.0')).toBe('0.18.0')
Done when: The risk is fixed or explicitly justified in the PR. Verification: Read client.test.ts lines 60-70 and verify test cases for all four formats exist and pass.
Evidence: client.test.ts:64 only tests 'Hermes Agent v0.17.0 (2026.6.19)'

`PRA-8` Resolve/justify — Config migration behavior for verify_on_stop: false undocumented/untested

Location: test/generate-hermes-config.test.ts:1
Category: correctness
Problem: Config migration behavior for verify_on_stop: false is undocumented/untested. No test loads a v30/v31 config fixture, runs the generator, and asserts verify_on_stop: false in output. The comment in hermes-config.ts claims 'Config v30→32 migrations switch the old implicit/auto behavior off' but there's no migration code visible in the generator — fresh configs just start at v32 with false.
Impact: The migration claim is unverified. If NemoClaw ever needs to migrate existing v30/v31 configs, the behavior is unknown and untested.
Recommended action: Either: (a) Add a test in generate-hermes-config.test.ts that loads a v30/v31 config fixture, runs the generator, and asserts verify_on_stop: false in output; or (b) Document in hermes-config.ts that NemoClaw only generates fresh configs at v32 and migration is upstream Hermes' responsibility.
Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
Verification: Search generate-hermes-config.test.ts for 'v30', 'v31', 'migration', or 'verify_on_stop' fixture test — should find one or the other
Missing regression test: Test case loading v30/v31 config fixture (or documented justification that migration is not NemoClaw's responsibility)
Done when: The risk is fixed or explicitly justified in the PR. Verification: Search generate-hermes-config.test.ts for 'v30', 'v31', 'migration', or 'verify_on_stop' fixture test — should find one or the other.
Evidence: hermes-config.ts:105-108 comment claims migration behavior; generate-hermes-config.test.ts has no v30/v31 fixture test

`PRA-9` Resolve/justify — Resolver action integration test only covers single Hermes version string format

Location: test/pr-workflow-contract.test.ts:1290
Category: tests
Problem: Resolver action integration test only mocks a single Hermes version string format: 'Hermes Agent v0.18.0 (2026.7.1)'. It does not test variant formats that the regex must handle. The test passes because the mocked format matches the current regex, but other valid formats would fail in production.
Impact: Resolver action may reject valid base images in CI when Hermes outputs a different version format, causing unnecessary local builds and CI delays.
Recommended action: Extend the existing 'rejects a pulled Hermes base without MCP HTTP imports' test or add a new parameterized test case that mocks docker run --entrypoint /usr/local/bin/hermes with outputs: 'Hermes Agent 0.18.0', 'Hermes Agent v0.18.0 (2026.7.1)', 'hermes v0.18.0', '0.18.0' — each should extract '0.18.0' and pass validation.
Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
Verification: Read pr-workflow-contract.test.ts around line 1290 and verify parameterized test covering all four variant formats
Missing regression test: Parameterized integration test in pr-workflow-contract.test.ts exercising resolver action against mocked docker returning the four variant version string formats
Done when: The risk is fixed or explicitly justified in the PR. Verification: Read pr-workflow-contract.test.ts around line 1290 and verify parameterized test covering all four variant formats.
Evidence: pr-workflow-contract.test.ts:1290 mocks only 'Hermes Agent v0.18.0 (2026.7.1)'

`PRA-10` Resolve/justify — hermesBaseImageIsCompatible versionCommand may not match executable name in Hermes output

Location: src/lib/agent/base-image.ts:135
Category: security
Problem: New function hermesBaseImageIsCompatible() calls parseVersionFromText(versionOutput, 'hermes --version') but the versionCommand parameter 'hermes --version' may not match the actual executable name in the version output. parseVersionFromText uses the executable name to find the version on the correct line. If Hermes outputs 'Hermes Agent v0.18.0' without 'hermes' in the executable position, the executable-aware parsing may not find the version.
Impact: Version extraction may silently fail or return null for valid Hermes output, causing base image compatibility check to fail incorrectly.
Recommended action: Verify parseVersionFromText correctly extracts version from 'hermes --version' output format. The function's executable-aware parsing looks for the executable name as a word boundary; 'Hermes Agent' may not match 'hermes'. Consider passing versionCommand as 'hermes' (just the binary name) or ensure the parsing works for Hermes' actual output format.
Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
Verification: Check parseVersionFromText implementation in client.ts:62-85 — it splits versionCommand by whitespace and takes first token, then splits by '/' and takes last part. For 'hermes --version', executable='hermes'. Then it searches for '\bhermes\b' case-insensitive in each line. 'Hermes Agent v0.18.0' contains 'Hermes' not 'hermes' — case-insensitive match should work. But need to verify.
Missing regression test: Unit test in base-image-hermes.test.ts or client.test.ts verifying hermesBaseImageIsCompatible correctly extracts version from actual Hermes --version output
Done when: The risk is fixed or explicitly justified in the PR. Verification: Check parseVersionFromText implementation in client.ts:62-85 — it splits versionCommand by whitespace and takes first token, then splits by '/' and takes last part. For 'hermes --version', executable='hermes'. Then it searches for '\bhermes\b' case-insensitive in each line. 'Hermes Agent v0.18.0' contains 'Hermes' not 'hermes' — case-insensitive match should work. But need to verify.
Evidence: base-image.ts:135 calls parseVersionFromText(versionOutput, 'hermes --version'); client.ts:62-85 executable-aware parsing logic

`PRA-11` Improvement — Slack Block Kit rich_blocks feature well-scoped — verify e2e coverage

Location: src/lib/messaging/channels/slack/manifest.ts:104
Category: correctness
Problem: New Slack Block Kit rendering feature (rich_blocks: true) added to Hermes platform config. This is a net-positive feature but the manifest change adds a new JSON fragment render target. The extra.rich_blocks is a static boolean, so credential leakage or injection risk is low.
Impact: Minimal — feature is well-scoped and static. E2E test validates the config is present.
Suggested action: No action required — feature is well-scoped. Verify e2e test in test/e2e/live/hermes-slack-e2e-helpers.ts validates the rich_blocks config is present in generated config.yaml.
Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
Verification: Read test/e2e/live/hermes-slack-e2e-helpers.ts lines 260-275 — configProbe asserts platforms.slack.extra.rich_blocks is true
Missing regression test: E2E test already covers this in configProbe assertion
Done when: The local improvement is applied, or the PR notes why it should be deferred.
Evidence: slack/manifest.ts:104 adds extra: { rich_blocks: true }; manifests.test.ts:470 asserts render fragment; hermes-slack-e2e-helpers.ts configProbe validates

`PRA-12` Improvement — Test file growth acceptable but monitor for extraction opportunity

Location: src/lib/messaging/channels/manifests.test.ts:470
Category: scope
Problem: Test file grew by 13 lines (918→931) adding Slack rich_blocks render assertion. Prior review noted to monitor growth beyond ~1000 lines. Current growth is acceptable but if additional channels add similar render validation tests, consider extracting a test helper module for manifest render assertions.
Impact: Low — current growth is justified. Risk of future bloat if more channels add similar tests.
Suggested action: No immediate action. Monitor for further growth. If more channels add render validation tests, extract a test helper.
Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
Verification: wc -l src/lib/messaging/channels/manifests.test.ts — currently 931 lines
Missing regression test: None — monitoring only
Done when: The local improvement is applied, or the PR notes why it should be deferred.
Evidence: manifests.test.ts grew from 918 to 931 lines; Slack rich_blocks assertion added at line ~470

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.

Keep changed test files branch-free for the codebase growth guard. Signed-off-by: Will Curran <wcurran@nvidia.com>

github-actions · 2026-07-08T05:34:10Z

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-2: Repin the final Hermes base digest to a v0.18-compatible image; then add or justify PRA-T1.
Open items: 1 required · 3 warnings · 0 suggestions · 7 test follow-ups
Since last review: 0 prior items resolved · 4 still apply · 0 new items found

Action checklist

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`	Required	acceptance	`agents/hermes/Dockerfile:9`	Publish the Hermes 0.18 base image, update `ARG BASE_IMAGE` in `agents/hermes/Dockerfile` to the reviewed immutable digest for that artifact, and keep the semver, MCP, layout, and glibc resolver checks intact.
`PRA-3`	Resolve/justify	security	`agents/hermes/Dockerfile.base:32`	Add or link maintainer-visible advisory/dependency-review evidence for the Hermes 0.18 release lockfiles and npm package set, or add a repeatable automated dependency-review artifact if that is the repository's chosen control.
`PRA-4`	Resolve/justify	architecture	`src/lib/agent/base-image.ts:121`	Move the Hermes-only constants and helper functions into a small local helper such as `src/lib/agent/hermes-base-image.ts`, then have `ensureAgentBaseImage` call that helper while preserving the explicit-override, pinned-provenance, semver, and MCP checks exactly.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

`PRA-2` Required — Repin the final Hermes base digest to a v0.18-compatible image

Location: agents/hermes/Dockerfile:9
Category: acceptance
Problem: The PR advances the Hermes base build and manifest to Hermes 0.18.0, and the PR's own acceptance text says the v0.18 multi-architecture base must be published and the immutable final-image digest repinned before merge. The final Dockerfile still points at the existing immutable ghcr.io/nvidia/nemoclaw/hermes-sandbox-base@sha256:d6ce... trust anchor. The new resolver correctly rejects a stale base by semver and falls back locally, but that means the reviewed remote-base trust anchor is not yet proving the upgraded artifact.
Impact: Merging with the old digest would either force local fallback builds for Hermes 0.18 or leave the reproducible, reviewed remote base-image path out of sync with the runtime this PR claims to ship.
Required action: Publish the Hermes 0.18 base image, update `ARG BASE_IMAGE` in `agents/hermes/Dockerfile` to the reviewed immutable digest for that artifact, and keep the semver, MCP, layout, and glibc resolver checks intact.
Expected follow-up: Fix before merge or get explicit maintainer override.
Verification: Read `agents/hermes/Dockerfile:9` and compare it with `agents/hermes/Dockerfile.base:32-35`; the final Dockerfile pin should reference the newly published base whose `/usr/local/bin/hermes --version` reports `0.18.0`.
Missing regression test: Add a focused contract or manifest mapping that fails when the final Dockerfile's tracked Hermes base pin is not known to satisfy the `HERMES_SEMVER` declared in `Dockerfile.base`; the current tests dynamically accept/mock the tracked digest and therefore do not prove it was repinned to a 0.18-compatible artifact.
Done when: The required change is committed and verification passes: Read `agents/hermes/Dockerfile:9` and compare it with `agents/hermes/Dockerfile.base:32-35`; the final Dockerfile pin should reference the newly published base whose `/usr/local/bin/hermes --version` reports `0.18.0`.
Evidence: `agents/hermes/Dockerfile.base` declares `HERMES_VERSION=v2026.7.1` and `HERMES_SEMVER=0.18.0`, while `agents/hermes/Dockerfile` still declares `ARG BASE_IMAGE=ghcr.io/nvidia/nemoclaw/hermes-sandbox-base@sha256:d6ce792eb302a73fc7e32e6ade18dcf21aaa425bcecdde8999e90b437fb6186a`.

Review findings by urgency: 1 required fix, 3 items 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 — Source-of-truth review needed: Hermes base image remote pin and local fallback

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: Existing tests cover stale-version rejection and fallback mechanics, but not that the final Dockerfile pin itself has been advanced to a v0.18-compatible digest.
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: `agents/hermes/Dockerfile:9` still pins the old `sha256:d6ce...` digest while `agents/hermes/Dockerfile.base` declares `HERMES_SEMVER=0.18.0`; this is covered by the blocker finding.

`PRA-3` Resolve/justify — Record dependency-risk evidence for the upgraded Hermes runtime

Location: agents/hermes/Dockerfile.base:32
Category: security
Problem: This PR upgrades a broad third-party agent runtime from Hermes v2026.6.19 / 0.17.0 to v2026.7.1 / 0.18.0. The Dockerfile preserves strong identity controls through tarball SHA256, npm integrity, `uv sync --frozen`, and `npm ci`, but the diff does not include maintainer-visible evidence that the new Python/npm dependency set was reviewed for known advisories.
Impact: A vulnerable dependency introduced by the upstream Hermes release could enter the sandbox base image despite checksum pinning; checksums prove artifact identity, not that the identified dependency set is free of known CVEs or accepted risk.
Recommended action: Add or link maintainer-visible advisory/dependency-review evidence for the Hermes 0.18 release lockfiles and npm package set, or add a repeatable automated dependency-review artifact if that is the repository's chosen control.
Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
Verification: Inspect `agents/hermes/Dockerfile.base:32-35` for the changed Hermes release pins and confirm there is corresponding review evidence covering the new tarball/npm/lockfile dependency set.
Missing regression test: Existing checksum and integrity gates prove selected artifact identity. They do not prove advisory review; add a repeatable advisory/dependency-review artifact or CI scan covering the Hermes release lockfiles if available.
Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect `agents/hermes/Dockerfile.base:32-35` for the changed Hermes release pins and confirm there is corresponding review evidence covering the new tarball/npm/lockfile dependency set.
Evidence: The diff changes `HERMES_VERSION`, `HERMES_SEMVER`, `HERMES_TARBALL_SHA256`, and `HERMES_NPM_INTEGRITY`, which changes the upstream runtime dependency set shipped inside a high-risk sandbox base image.

`PRA-4` Resolve/justify — Extract Hermes-specific base-image trust policy from the generic provisioning hotspot

Location: src/lib/agent/base-image.ts:121
Category: architecture
Problem: `src/lib/agent/base-image.ts` is already a large provisioning module and this PR adds more Hermes-only trust-boundary behavior: official digest recognition, final-Dockerfile pin parsing, pinned-platform provenance acceptance, semver probing, MCP probing, and explicit override rejection. The validation is necessary, but keeping this agent-specific policy in the generic module makes future agent-neutral refactors more likely to weaken a Hermes security boundary accidentally.
Impact: The caller/callee contract for accepting Hermes base images becomes harder to audit, especially with active overlapping work in the same generic provisioning file.
Recommended action: Move the Hermes-only constants and helper functions into a small local helper such as `src/lib/agent/hermes-base-image.ts`, then have `ensureAgentBaseImage` call that helper while preserving the explicit-override, pinned-provenance, semver, and MCP checks exactly.
Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
Verification: Read the Hermes-specific block in `src/lib/agent/base-image.ts` from `HERMES_OFFICIAL_BASE_DIGEST_REF` through `createAgentBaseImageResolutionOptions`; it can be extracted without changing resolver behavior.
Missing regression test: The changed tests in `src/lib/agent/base-image-hermes.test.ts` and `src/lib/agent/base-image-hermes-resolution.test.ts` already cover the security-sensitive behavior; keep those passing after extraction and do not weaken the Docker probe argument assertions.
Done when: The risk is fixed or explicitly justified in the PR. Verification: Read the Hermes-specific block in `src/lib/agent/base-image.ts` from `HERMES_OFFICIAL_BASE_DIGEST_REF` through `createAgentBaseImageResolutionOptions`; it can be extracted without changing resolver behavior.
Evidence: Drift analysis flagged `src/lib/agent/base-image.ts` as a large-file hotspot that grew by 30 lines, and the added lines are Hermes-specific base-image trust policy.

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

Simplification opportunities: 1 possible cut

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

PRA-4 shrink (src/lib/agent/base-image.ts:121): Hermes-specific digest regex, final Dockerfile pin parsing, compatibility probes, and acceptance policy from the generic `base-image.ts` module.
- Replacement: A focused Hermes base-image helper module imported by the generic provisioning path.
- Safety boundary: Do not remove or relax trust-boundary validation: exact official digest matching, pinned provenance, explicit override rejection, semver equality, MCP import probing, and existing negative tests must remain.

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 — Final Hermes Dockerfile base pin is known to report HERMES_SEMVER 0.18.0. The PR changes sandbox Dockerfiles, a GitHub composite action, runtime config generation, wrapper compatibility code, and base-image trust policy. Unit and contract tests cover most mechanics, but the final remote base artifact/version proof and dependency-review evidence are still missing.
PRA-T2 Runtime validation — Published Hermes v0.18 base digest passes semver, glibc, layout, and MCP import probes without falling back locally. The PR changes sandbox Dockerfiles, a GitHub composite action, runtime config generation, wrapper compatibility code, and base-image trust policy. Unit and contract tests cover most mechanics, but the final remote base artifact/version proof and dependency-review evidence are still missing.
PRA-T3 Runtime validation — Hermes 0.18 dependency/advisory review artifact covers the Python and npm lockfile/package set. The PR changes sandbox Dockerfiles, a GitHub composite action, runtime config generation, wrapper compatibility code, and base-image trust policy. Unit and contract tests cover most mechanics, but the final remote base artifact/version proof and dependency-review evidence are still missing.
PRA-T4 Runtime validation — Hermes base-image helper extraction preserves stale-version rejection, explicit override rejection, pinned provenance acceptance, and MCP probe arguments. The PR changes sandbox Dockerfiles, a GitHub composite action, runtime config generation, wrapper compatibility code, and base-image trust policy. Unit and contract tests cover most mechanics, but the final remote base artifact/version proof and dependency-review evidence are still missing.
PRA-T5 Acceptance clause — Refs [NemoClaw] Dependency Updates (Hermes, OpenShell, OpenClaw) #5591 — add test evidence or identify existing coverage. The deterministic validation context did not include linked issue clauses or comments for [NemoClaw] Dependency Updates (Hermes, OpenShell, OpenClaw) #5591, so there are no issue acceptance clauses to map literally.
PRA-T6 Acceptance clause — Before merge, the v0.18 multi-architecture base must be published and the immutable final-image digest repinned; this draft safely falls back to a local v0.18 base build until then. — add test evidence or identify existing coverage. `agents/hermes/Dockerfile:9` still pins `ghcr.io/nvidia/nemoclaw/hermes-sandbox-base@sha256:d6ce792eb302a73fc7e32e6ade18dcf21aaa425bcecdde8999e90b437fb6186a` while `Dockerfile.base` declares Hermes 0.18.0.
PRA-T7 Hermes base image remote pin and local fallback — Existing tests cover stale-version rejection and fallback mechanics, but not that the final Dockerfile pin itself has been advanced to a v0.18-compatible digest.. `agents/hermes/Dockerfile:9` still pins the old `sha256:d6ce...` digest while `agents/hermes/Dockerfile.base` declares `HERMES_SEMVER=0.18.0`; this is covered by the blocker finding.

Since last review details

Current findings, using the urgency labels above:

`PRA-1` Resolve/justify — Source-of-truth review needed: Hermes base image remote pin and local fallback

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: Existing tests cover stale-version rejection and fallback mechanics, but not that the final Dockerfile pin itself has been advanced to a v0.18-compatible digest.
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: `agents/hermes/Dockerfile:9` still pins the old `sha256:d6ce...` digest while `agents/hermes/Dockerfile.base` declares `HERMES_SEMVER=0.18.0`; this is covered by the blocker finding.

`PRA-2` Required — Repin the final Hermes base digest to a v0.18-compatible image

Location: agents/hermes/Dockerfile:9
Category: acceptance
Problem: The PR advances the Hermes base build and manifest to Hermes 0.18.0, and the PR's own acceptance text says the v0.18 multi-architecture base must be published and the immutable final-image digest repinned before merge. The final Dockerfile still points at the existing immutable ghcr.io/nvidia/nemoclaw/hermes-sandbox-base@sha256:d6ce... trust anchor. The new resolver correctly rejects a stale base by semver and falls back locally, but that means the reviewed remote-base trust anchor is not yet proving the upgraded artifact.
Impact: Merging with the old digest would either force local fallback builds for Hermes 0.18 or leave the reproducible, reviewed remote base-image path out of sync with the runtime this PR claims to ship.
Required action: Publish the Hermes 0.18 base image, update `ARG BASE_IMAGE` in `agents/hermes/Dockerfile` to the reviewed immutable digest for that artifact, and keep the semver, MCP, layout, and glibc resolver checks intact.
Expected follow-up: Fix before merge or get explicit maintainer override.
Verification: Read `agents/hermes/Dockerfile:9` and compare it with `agents/hermes/Dockerfile.base:32-35`; the final Dockerfile pin should reference the newly published base whose `/usr/local/bin/hermes --version` reports `0.18.0`.
Missing regression test: Add a focused contract or manifest mapping that fails when the final Dockerfile's tracked Hermes base pin is not known to satisfy the `HERMES_SEMVER` declared in `Dockerfile.base`; the current tests dynamically accept/mock the tracked digest and therefore do not prove it was repinned to a 0.18-compatible artifact.
Done when: The required change is committed and verification passes: Read `agents/hermes/Dockerfile:9` and compare it with `agents/hermes/Dockerfile.base:32-35`; the final Dockerfile pin should reference the newly published base whose `/usr/local/bin/hermes --version` reports `0.18.0`.
Evidence: `agents/hermes/Dockerfile.base` declares `HERMES_VERSION=v2026.7.1` and `HERMES_SEMVER=0.18.0`, while `agents/hermes/Dockerfile` still declares `ARG BASE_IMAGE=ghcr.io/nvidia/nemoclaw/hermes-sandbox-base@sha256:d6ce792eb302a73fc7e32e6ade18dcf21aaa425bcecdde8999e90b437fb6186a`.

`PRA-3` Resolve/justify — Record dependency-risk evidence for the upgraded Hermes runtime

Location: agents/hermes/Dockerfile.base:32
Category: security
Problem: This PR upgrades a broad third-party agent runtime from Hermes v2026.6.19 / 0.17.0 to v2026.7.1 / 0.18.0. The Dockerfile preserves strong identity controls through tarball SHA256, npm integrity, `uv sync --frozen`, and `npm ci`, but the diff does not include maintainer-visible evidence that the new Python/npm dependency set was reviewed for known advisories.
Impact: A vulnerable dependency introduced by the upstream Hermes release could enter the sandbox base image despite checksum pinning; checksums prove artifact identity, not that the identified dependency set is free of known CVEs or accepted risk.
Recommended action: Add or link maintainer-visible advisory/dependency-review evidence for the Hermes 0.18 release lockfiles and npm package set, or add a repeatable automated dependency-review artifact if that is the repository's chosen control.
Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
Verification: Inspect `agents/hermes/Dockerfile.base:32-35` for the changed Hermes release pins and confirm there is corresponding review evidence covering the new tarball/npm/lockfile dependency set.
Missing regression test: Existing checksum and integrity gates prove selected artifact identity. They do not prove advisory review; add a repeatable advisory/dependency-review artifact or CI scan covering the Hermes release lockfiles if available.
Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect `agents/hermes/Dockerfile.base:32-35` for the changed Hermes release pins and confirm there is corresponding review evidence covering the new tarball/npm/lockfile dependency set.
Evidence: The diff changes `HERMES_VERSION`, `HERMES_SEMVER`, `HERMES_TARBALL_SHA256`, and `HERMES_NPM_INTEGRITY`, which changes the upstream runtime dependency set shipped inside a high-risk sandbox base image.

`PRA-4` Resolve/justify — Extract Hermes-specific base-image trust policy from the generic provisioning hotspot

Location: src/lib/agent/base-image.ts:121
Category: architecture
Problem: `src/lib/agent/base-image.ts` is already a large provisioning module and this PR adds more Hermes-only trust-boundary behavior: official digest recognition, final-Dockerfile pin parsing, pinned-platform provenance acceptance, semver probing, MCP probing, and explicit override rejection. The validation is necessary, but keeping this agent-specific policy in the generic module makes future agent-neutral refactors more likely to weaken a Hermes security boundary accidentally.
Impact: The caller/callee contract for accepting Hermes base images becomes harder to audit, especially with active overlapping work in the same generic provisioning file.
Recommended action: Move the Hermes-only constants and helper functions into a small local helper such as `src/lib/agent/hermes-base-image.ts`, then have `ensureAgentBaseImage` call that helper while preserving the explicit-override, pinned-provenance, semver, and MCP checks exactly.
Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
Verification: Read the Hermes-specific block in `src/lib/agent/base-image.ts` from `HERMES_OFFICIAL_BASE_DIGEST_REF` through `createAgentBaseImageResolutionOptions`; it can be extracted without changing resolver behavior.
Missing regression test: The changed tests in `src/lib/agent/base-image-hermes.test.ts` and `src/lib/agent/base-image-hermes-resolution.test.ts` already cover the security-sensitive behavior; keep those passing after extraction and do not weaken the Docker probe argument assertions.
Done when: The risk is fixed or explicitly justified in the PR. Verification: Read the Hermes-specific block in `src/lib/agent/base-image.ts` from `HERMES_OFFICIAL_BASE_DIGEST_REF` through `createAgentBaseImageResolutionOptions`; it can be extracted without changing resolver behavior.
Evidence: Drift analysis flagged `src/lib/agent/base-image.ts` as a large-file hotspot that grew by 30 lines, and the added lines are Hermes-specific base-image trust policy.

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.

wscurran · 2026-07-08T05:34:24Z

@ericksoa fyi, after some research, discovered that the latest version of Hermes (7/1) supports markdown in Slack.

wscurran · 2026-07-08T05:48:04Z

Way over-engineered, trying again.

feat(hermes): upgrade to v0.18 with Slack Block Kit

2973f3b

Upgrade Hermes to v2026.7.1 (0.18.0) and enable Slack rich Block Kit output. Preserve trusted base-image resolution while the v0.18 base digest is prepared. Refs #5591 Signed-off-by: Will Curran <wcurran@nvidia.com>

wscurran self-assigned this Jul 8, 2026

test(hermes): linearize base-image mocks

a7bf039

Keep changed test files branch-free for the codebase growth guard. Signed-off-by: Will Curran <wcurran@nvidia.com>

wscurran requested a review from ericksoa July 8, 2026 05:30

wscurran added v0.0.76 integration: hermes Hermes integration behavior integration: slack Slack integration or channel behavior labels Jul 8, 2026

wscurran marked this pull request as ready for review July 8, 2026 05:35

wscurran closed this Jul 8, 2026

wscurran removed the request for review from ericksoa July 8, 2026 05:48

wscurran removed integration: slack Slack integration or channel behavior integration: hermes Hermes integration behavior v0.0.76 labels Jul 8, 2026

Uh oh!

Conversation

wscurran commented Jul 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Type of Change

Quality Gates

Verification

Summary by CodeRabbit

Uh oh!

copy-pr-bot Bot commented Jul 8, 2026

Uh oh!

coderabbitai Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

github-code-quality Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Overview

TypeScript / code-coverage/plugin

Uh oh!

github-actions Bot commented Jul 8, 2026

Uh oh!

github-actions Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

E2E Advisor Recommendation

E2E Recommendation Advisor

Required E2E

Optional E2E

New E2E recommendations

Dispatch hint

Uh oh!

github-actions Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

E2E Target Recommendation

E2E Target Advisor

Required E2E targets

Optional E2E targets

Relevant changed files

Uh oh!

github-actions Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review Advisor (Nemotron Ultra) — Changes requested

Action checklist

Findings index

🚨 Required before merge

PRA-3 Required — Monolith growth: Hermes-specific logic not extracted to separate module

PRA-4 Required — Resolver hermes_version() regex rejects valid Hermes --version output formats

PRA-5 Required — Wrapper SHA256 hash must be verified against actual hermes-wrapper.py content

⚠️ Resolve or justify before merge

PRA-1 Resolve/justify — Source-of-truth review needed: agents/hermes/config/hermes-config.ts:109 (verify_on_stop: false)

PRA-2 Resolve/justify — Source-of-truth review needed: .github/actions/resolve-hermes-base-image/action.yaml:52 (hermes_version regex)

PRA-6 Resolve/justify — verify_on_stop: false source-of-truth comment incomplete

PRA-7 Resolve/justify — parseVersionFromText missing unit tests for Hermes v0.18.0 output variants

PRA-8 Resolve/justify — Config migration behavior for verify_on_stop: false undocumented/untested

PRA-9 Resolve/justify — Resolver action integration test only covers single Hermes version string format

PRA-10 Resolve/justify — hermesBaseImageIsCompatible versionCommand may not match executable name in Hermes output

💡 In-scope improvements

PRA-11 Improvement — Slack Block Kit rich_blocks feature well-scoped — verify e2e coverage

PRA-12 Improvement — Test file growth acceptable but monitor for extraction opportunity

PRA-1 Resolve/justify — Source-of-truth review needed: agents/hermes/config/hermes-config.ts:109 (verify_on_stop: false)

PRA-2 Resolve/justify — Source-of-truth review needed: .github/actions/resolve-hermes-base-image/action.yaml:52 (hermes_version regex)

PRA-3 Required — Monolith growth: Hermes-specific logic not extracted to separate module

PRA-4 Required — Resolver hermes_version() regex rejects valid Hermes --version output formats

PRA-5 Required — Wrapper SHA256 hash must be verified against actual hermes-wrapper.py content

PRA-6 Resolve/justify — verify_on_stop: false source-of-truth comment incomplete

PRA-7 Resolve/justify — parseVersionFromText missing unit tests for Hermes v0.18.0 output variants

PRA-8 Resolve/justify — Config migration behavior for verify_on_stop: false undocumented/untested

PRA-9 Resolve/justify — Resolver action integration test only covers single Hermes version string format

PRA-10 Resolve/justify — hermesBaseImageIsCompatible versionCommand may not match executable name in Hermes output

PRA-11 Improvement — Slack Block Kit rich_blocks feature well-scoped — verify e2e coverage

PRA-12 Improvement — Test file growth acceptable but monitor for extraction opportunity

Uh oh!

github-actions Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review Advisor — Changes requested

Action checklist

wscurran commented Jul 8, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading

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

edited

Loading

github-actions Bot commented Jul 8, 2026 •

edited

Loading

github-actions Bot commented Jul 8, 2026 •

edited

Loading

github-actions Bot commented Jul 8, 2026 •

edited

Loading

`PRA-3` Required — Monolith growth: Hermes-specific logic not extracted to separate module

`PRA-4` Required — Resolver hermes_version() regex rejects valid Hermes --version output formats

`PRA-5` Required — Wrapper SHA256 hash must be verified against actual hermes-wrapper.py content

`PRA-1` Resolve/justify — Source-of-truth review needed: agents/hermes/config/hermes-config.ts:109 (verify_on_stop: false)

`PRA-2` Resolve/justify — Source-of-truth review needed: .github/actions/resolve-hermes-base-image/action.yaml:52 (hermes_version regex)

`PRA-6` Resolve/justify — verify_on_stop: false source-of-truth comment incomplete

`PRA-7` Resolve/justify — parseVersionFromText missing unit tests for Hermes v0.18.0 output variants

`PRA-8` Resolve/justify — Config migration behavior for verify_on_stop: false undocumented/untested

`PRA-9` Resolve/justify — Resolver action integration test only covers single Hermes version string format

`PRA-10` Resolve/justify — hermesBaseImageIsCompatible versionCommand may not match executable name in Hermes output

`PRA-11` Improvement — Slack Block Kit rich_blocks feature well-scoped — verify e2e coverage

`PRA-12` Improvement — Test file growth acceptable but monitor for extraction opportunity

`PRA-1` Resolve/justify — Source-of-truth review needed: agents/hermes/config/hermes-config.ts:109 (verify_on_stop: false)

`PRA-2` Resolve/justify — Source-of-truth review needed: .github/actions/resolve-hermes-base-image/action.yaml:52 (hermes_version regex)

`PRA-3` Required — Monolith growth: Hermes-specific logic not extracted to separate module

`PRA-4` Required — Resolver hermes_version() regex rejects valid Hermes --version output formats

`PRA-5` Required — Wrapper SHA256 hash must be verified against actual hermes-wrapper.py content

`PRA-6` Resolve/justify — verify_on_stop: false source-of-truth comment incomplete

`PRA-7` Resolve/justify — parseVersionFromText missing unit tests for Hermes v0.18.0 output variants

`PRA-8` Resolve/justify — Config migration behavior for verify_on_stop: false undocumented/untested

`PRA-9` Resolve/justify — Resolver action integration test only covers single Hermes version string format

`PRA-10` Resolve/justify — hermesBaseImageIsCompatible versionCommand may not match executable name in Hermes output

`PRA-11` Improvement — Slack Block Kit rich_blocks feature well-scoped — verify e2e coverage

`PRA-12` Improvement — Test file growth acceptable but monitor for extraction opportunity

github-actions Bot commented Jul 8, 2026 •

edited

Loading

`PRA-2` Required — Repin the final Hermes base digest to a v0.18-compatible image

`PRA-1` Resolve/justify — Source-of-truth review needed: Hermes base image remote pin and local fallback

`PRA-3` Resolve/justify — Record dependency-risk evidence for the upgraded Hermes runtime

`PRA-4` Resolve/justify — Extract Hermes-specific base-image trust policy from the generic provisioning hotspot

`PRA-1` Resolve/justify — Source-of-truth review needed: Hermes base image remote pin and local fallback

`PRA-2` Required — Repin the final Hermes base digest to a v0.18-compatible image

`PRA-3` Resolve/justify — Record dependency-risk evidence for the upgraded Hermes runtime

`PRA-4` Resolve/justify — Extract Hermes-specific base-image trust policy from the generic provisioning hotspot