fix(hermes): scope prebaked dependencies to supported integrations#2846
Conversation
This change introduces UV to sync the .[all] target for all python dependencies. This was introduced due to missing dependencies such as `discord.py` in the logs. Additionally runs npm install to get the JS dependencies for the TUI and web dashboards. (cherry picked from commit 2a1a9d7)
|
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. |
📝 WalkthroughWalkthroughHermes agent is updated from v2026.4.8/v2026.4.13 to v2026.4.23 with a new deterministic build pipeline using Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 8/10 reviews remaining, refill in 6 minutes and 8 seconds. Comment |
Selective E2E Results — ❌ Some jobs failedRun: 25219741538
|
Selective E2E Results — ❌ Some jobs failedRun: 25220440120
|
Selective E2E Results — ❌ Some jobs failedRun: 25221059901
|
Selective E2E Results — ❌ Some jobs failedRun: 25221685089
|
Selective E2E Results — ❌ Some jobs failedRun: 25222192970
|
Selective E2E Results — ✅ All requested jobs passedRun: 25222726016
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@agents/hermes/Dockerfile`:
- Around line 214-228: The Dockerfile currently makes /sandbox/.hermes
group-writable with the chmod 770 line; change that to keep the config root
non-group-writable (chmod 750) so /sandbox/.hermes remains 750 sandbox:sandbox
while leaving config.yaml and .env as 640; additionally, create and chown
dedicated writable subdirectories for gateway runtime state (e.g., pid, logs,
socket or state dirs like /sandbox/.hermes/pid or /sandbox/.hermes/runtime) and
grant those gateway-writable permissions (chown to gateway and chmod 770/750 as
appropriate) so gateway can write runtime files without giving group write on
the config root.
In `@agents/hermes/start.sh`:
- Around line 42-54: The current _START_LOG creation (variable _START_LOG and
the block that truncates and chowns it before the exec redirection/tee) risks
following attacker-controlled symlinks; change to an atomic safe-create pattern:
open a new temporary file in the same directory (e.g., create a unique tmp
name), set ownership and permissions on that temp file, then atomically rename
it to the final _START_LOG path (or call the planned restricted-log helper)
before running exec > >(tee -a "$_START_LOG") 2> >(tee -a "$_START_LOG" >&2);
ensure the logic for both root and non-root branches uses the same
safe-create/rename flow and avoid using shell truncation (:) on the final path
so symlinks cannot be followed.
In `@test/e2e/test-rebuild-hermes.sh`:
- Around line 33-35: The test seeds an old hermes version using
OLD_HERMES_VERSION but the final assertion still checks a hardcoded version;
introduce a single constant (e.g., OLD_HERMES_REGISTRY_VERSION) to hold the
seeded registry version and use it both where the fixture is set (replacing
OLD_HERMES_VERSION usage if needed) and in the final REGISTRY_VERSION assertion
to compare against the same value; update any other occurrences (notably the
other two spots mentioned) to reference OLD_HERMES_REGISTRY_VERSION so the seed
and the final guard come from the same source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 766247a0-11e5-4977-922b-3bad7a378121
📒 Files selected for processing (9)
agents/hermes/Dockerfileagents/hermes/Dockerfile.baseagents/hermes/manifest.yamlagents/hermes/policy-additions.yamlagents/hermes/policy-permissive.yamlagents/hermes/start.shtest/e2e/test-hermes-e2e.shtest/e2e/test-rebuild-hermes.shtest/sandbox-provisioning.test.ts
Selective E2E Results — ✅ All requested jobs passedRun: 25223575974
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@agents/hermes/start.sh`:
- Around line 66-67: The mv invocation in agents/hermes/start.sh currently uses
mv -f "$tmp" "$path" which will follow a destination symlink-to-directory and
can be exploited; change the publish step to use mv -T (atomic replace of
destination) so the temp file replaces the target path rather than being moved
into a symlinked directory — update the mv command that handles the "$tmp" ->
"$path" replacement to use mv -T (and keep the existing -f behavior) and ensure
the cleanup branch still removes "$tmp" on failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6904c474-9a14-46b7-8df2-3297ba037477
📒 Files selected for processing (6)
agents/hermes/Dockerfileagents/hermes/Dockerfile.baseagents/hermes/start.shtest/e2e/test-rebuild-hermes.shtest/e2e/test-runtime-overrides.shtest/sandbox-provisioning.test.ts
✅ Files skipped from review due to trivial changes (2)
- test/e2e/test-runtime-overrides.sh
- agents/hermes/Dockerfile.base
🚧 Files skipped from review as they are similar to previous changes (2)
- agents/hermes/Dockerfile
- test/e2e/test-rebuild-hermes.sh
| if ! mv -f "$tmp" "$path"; then | ||
| rm -f "$tmp" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the actual file to see the context
head -80 agents/hermes/start.sh | tail -40Repository: NVIDIA/NemoClaw
Length of output: 1102
🏁 Script executed:
# Check if the file has SPDX headers
head -5 agents/hermes/start.shRepository: NVIDIA/NemoClaw
Length of output: 266
🏁 Script executed:
# Get more context around the function in question
sed -n '45,80p' agents/hermes/start.shRepository: NVIDIA/NemoClaw
Length of output: 898
🌐 Web query:
GNU coreutils mv -T flag symlink directory behavior
💡 Result:
The GNU coreutils mv command with the -T (or --no-target-directory) flag does not treat the destination operand specially if it is a directory or a symbolic link to a directory. Normally, without -T, if the destination is a directory or symlink to a directory, mv moves the source into that directory (e.g., mv source dest becomes mv source dest/source). With -T, mv treats the destination as a normal file path, attempting to rename or move the source directly to that exact path, even if dest exists as a directory or symlink to one. This enables overwriting or replacing a directory symlink atomically with another file or symlink. This behavior holds regardless of whether the destination is a symlink to a directory; the check for special treatment explicitly includes "a directory or a symbolic link to a directory". The mv source code confirms that when no_target_directory is set, it skips the directory detection logic for the last operand and performs a direct rename operation. Note that symlink handling also depends on trailing slashes (use --strip-trailing-slashes if needed) and the underlying rename(2) syscall behavior, which varies by OS/kernel (e.g., modern Linux fails with ENOTDIR for symlink-to-dir sources with trailing slash, while others may dereference).
Citations:
- 1: https://www.gnu.org/s/coreutils/manual/html_node/Target-directory.html
- 2: https://www.gnu.org/software/coreutils/manual/html_node/mv-invocation.html
- 3: https://www.gnu.org/s/coreutils/manual/html_node/mv-invocation.html
- 4: https://github.com/coreutils/coreutils/blob/master/src/mv.c
- 5: https://blog.dvl.pl/article/2009/04/18/atomic-symlink-replacement/
🏁 Script executed:
# Check for Dockerfile or system requirements to understand target environment
find . -name "Dockerfile*" -o -name "*.dockerfile" | head -5Repository: NVIDIA/NemoClaw
Length of output: 175
🏁 Script executed:
# Check if other shell scripts use GNU-specific mv flags
rg "mv\s+-[a-zA-Z]*T" --type shRepository: NVIDIA/NemoClaw
Length of output: 41
🏁 Script executed:
# Check for any POSIX compatibility notes or system requirements
rg "POSIX|portability|compatibility" README.md 2>/dev/null | head -20Repository: NVIDIA/NemoClaw
Length of output: 41
🏁 Script executed:
# See if there are any CI/build configs that indicate target platform
find . -name ".github" -type d -exec find {} -name "*.yml" -o -name "*.yaml" \; | head -5Repository: NVIDIA/NemoClaw
Length of output: 354
🏁 Script executed:
# Check the Hermes Dockerfile to see base image
cat agents/hermes/Dockerfile | head -20Repository: NVIDIA/NemoClaw
Length of output: 908
🏁 Script executed:
# Also check the base image
cat agents/hermes/Dockerfile.base | head -20Repository: NVIDIA/NemoClaw
Length of output: 1042
🏁 Script executed:
# Check where prepare_restricted_log is called (should be 2+ times)
rg "prepare_restricted_log" agents/hermes/start.sh -nRepository: NVIDIA/NemoClaw
Length of output: 311
🏁 Script executed:
# Verify the second usage for gateway.log
sed -n '500,570p' agents/hermes/start.shRepository: NVIDIA/NemoClaw
Length of output: 3064
Use mv -T when publishing the restricted log file.
On line 66, mv -f treats a destination symlink-to-directory as a directory, so a pre-planted /tmp/nemoclaw-start.log or /tmp/gateway.log link can redirect the temp file into an attacker-chosen directory instead of replacing the log path. Because this helper is reused for both restricted logs, the /tmp hardening is incomplete.
🔒 Proposed hardening
- if ! mv -f "$tmp" "$path"; then
+ if ! mv -fT "$tmp" "$path"; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@agents/hermes/start.sh` around lines 66 - 67, The mv invocation in
agents/hermes/start.sh currently uses mv -f "$tmp" "$path" which will follow a
destination symlink-to-directory and can be exploited; change the publish step
to use mv -T (atomic replace of destination) so the temp file replaces the
target path rather than being moved into a symlinked directory — update the mv
command that handles the "$tmp" -> "$path" replacement to use mv -T (and keep
the existing -f behavior) and ensure the cleanup branch still removes "$tmp" on
failure.
Selective E2E Results — ✅ All requested jobs passedRun: 25227294718
|
## Summary Daily release-prep documentation refresh for merged PRs from the past 24 hours. This updates user-facing docs for Telegram mention-only mode, in-sandbox messaging shutdown, Hermes onboarding/runtime behavior, and compatible-endpoint smoke validation, then bumps the docs metadata to 0.0.33 after tag v0.0.32. ## Related Issue None. ## Changes - #2417 / c7e49ad: Document `TELEGRAM_REQUIRE_MENTION` for Telegram group-chat replies in `docs/manage-sandboxes/messaging-channels.md` and `docs/reference/commands.md`. - #1977 / 69403e0: Update `nemoclaw tunnel stop` and deprecated `nemoclaw stop` docs to explain that NemoClaw also attempts to stop the in-sandbox OpenClaw gateway and messaging polling. - #2781 / b83ffe2, #2859 / 4df8be6, and #2846 / 0968dfd: Refresh the Hermes quickstart for the default `my-hermes` sandbox name, cross-agent same-name guard, agent type visibility in `nemoclaw list`, Brave prompt omission, and supported prebaked Hermes integrations. - #2849 / fd240ff: Document the Telegram plus OpenAI-compatible endpoint `inference.local` smoke check in inference options and troubleshooting. - Bump `docs/versions1.json` and `docs/project.json` from 0.0.32 to 0.0.33 for daily release preparation. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Verification - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [x] `make docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Additional checks run: - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --dry-run` - `git diff --check` - `make docs` passed with the existing local version-switcher read message. - Full `npx prek run --all-files` and `npm test` were skipped for this doc-only automation run. Commit and pre-push hooks otherwise passed docs, lint, secret, and conversion checks until the local `Test (skills YAML)` hook failed because `vitest/config` is not installed in this fresh worktree. --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated Hermes quickstart: default sandbox name is "hermes"; guidance to use distinct sandbox names, note same-name reuse is prevented, Hermes wizard does not request Brave Web Search, and sandbox listings now show agent type. * Clarified provider onboarding: bounded in-sandbox smoke check runs when Telegram messaging is enabled. * Expanded Telegram docs: added TELEGRAM_REQUIRE_MENTION (DMs still governed by TELEGRAM_ALLOWED_IDS), onboarding examples, stop-messaging/tunnel behavior, and troubleshooting. * Promoted docs to version 0.0.33. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> Co-authored-by: Cursor <cursoragent@cursor.com>
<!-- markdownlint-disable MD041 --> ## Summary Follows up on NousResearch's dependency-bootstrap proposal in #2834 and the maintainer-side #2846 by carrying the remaining root Node dependency install into the Hermes base image. The existing `uv sync` step covers the supported Python integration extras; this adds deterministic `npm ci` from the Hermes root lockfile so browser tooling such as `agent-browser` is available in the extracted Hermes tree. ## Credit Thanks to Ben Barclay and the NousResearch team for the original PR #2834. They identified the Hermes dependency-bootstrap gap and proposed the direction this follow-up carries into the current NemoClaw base-image flow. ## Related Issue Follow-up to #2834 and #2846. ## Changes - Install Hermes root Node dependencies with `npm ci --prefer-offline --no-audit --no-fund` after the existing `uv sync` step. - Remove transient `/tmp/camoufox-*` installer downloads in the same Docker layer so large temporary archives do not persist into the image. - Document why the root Node install belongs in the Hermes base image. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Additional validation: - `git diff --check` - `docker build --progress=plain --output=type=cacheonly -f agents/hermes/Dockerfile.base .` - validated the `uv sync` step - validated the new `npm ci` step - validated `/usr/local/bin/hermes --version` reports `Hermes Agent v0.11.0 (2026.4.23)` --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Optimized the container image build by removing temporary build artifacts to reduce clutter and image size. * Made tooling dependencies for browser-related features explicit so tooling is reliably present during build. * Ensured Node package installation runs deterministically during image creation to improve build stability and reproducibility. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Ben Barclay <ben@nousresearch.com>
This is a maintainer experiment that builds on Ben Barclay's PR #2834. Ben identified the right install gap: the Hermes image needs the dependencies required for supported runtime integrations, otherwise users hit missing-package failures after onboarding.
This branch keeps that direction while narrowing the dependency surface for NemoClaw:
v2026.4.23/0.11.0uv sync --frozen --no-dev --extra messaging --extra webby default instead of[all]/usr/local/bin/hermesfinal-image contract used by policy and e2e checks/tmp/gateway.logfrom inside thegatewayuser context so capability-dropping does not block startup2026.4.23The intent is to prebake the Hermes dependencies that map to NemoClaw-supported onboarding integrations today: Telegram, Discord, Slack, and the API/health runtime. Larger or unsupported integrations should stay out of the base image and be installed by the agent workflow when enabled.
Local validation completed:
docker build --progress=plain -f agents/hermes/Dockerfile.base -t nemoclaw-hermes-base-experiment .docker build --progress=plain -f agents/hermes/Dockerfile.base --build-arg HERMES_VERSION=v2026.4.13 --build-arg HERMES_TARBALL_SHA256=5e4529b8cb6e4821eb916b81517e48125109b1764d6d1e68a204a9f0ddf2d98c --build-arg HERMES_UV_EXTRAS=messaging -t nemoclaw-hermes-old-base-e2e-rebuild-test .docker build --progress=plain -f agents/hermes/Dockerfile --build-arg BASE_IMAGE=nemoclaw-hermes-base-experiment -t nemoclaw-hermes-final-experiment .hermesto/usr/local/bin/hermesand reportsHermes Agent v0.11.0 (2026.4.23)telegram,discord,slack_bolt,fastapi,uvicornfaster_whisper,mautrix,boto3,dingtalk_stream,lark_oapi{"status":"ok","platform":"hermes-agent"}through port8642npm test -- test/sandbox-provisioning.test.ts src/lib/agent-defs.test.ts src/lib/sandbox-version.test.tsnpm run validate:configsnpm run typecheck:clinpx prek run --all-files --stage pre-pushFocused e2e passed on this branch: https://github.com/NVIDIA/NemoClaw/actions/runs/25222726016
Earlier focused e2e on this experiment found four useful issues that this branch now addresses: first the image contract expected
/usr/local/bin/hermes, then the gateway needed writable Hermes runtime state under/sandbox/.hermes, then the entrypoint opened/tmp/gateway.logbefore switching to thegatewayuser. The rebuild e2e also needed its old-version fixture pinned to the matching tarball checksum and extras profile. The final OpenShell-specific blocker was that the Hermes executable is a symlink into /opt/hermes, so the Hermes policy now grants read-only access to that venv path.Related context:
Summary by CodeRabbit
New Features
Chores