handle_upload_extract() used Path(s.workspace) as the extraction root,
bypassing HERMES_WEBUI_ATTACHMENT_DIR entirely. Route through
_session_attachment_dir(session_id) so archives land alongside
single-file uploads and session cleanup covers them.
Add tests and CHANGELOG entry.
Ref #2247
Two non-blocking observations from the review, both addressed:
1. The bad-pattern grep listed `error_exit` as a literal token, but the
`error_exit()` function at docker_init.bash:5-10 only echoes the
strings `"!! ERROR: "` and `"!! Exiting script (ID: $$)"` — the
function name itself never appears in container logs. So
`grep -E -i "error_exit"` would only fire on stray debug prints of
the name, not on actual failures. The other patterns
(`Failed to set (UID|GID|...)`, `groupmod: cannot`, etc.) DO catch
real error_exit output, so this wasn't a coverage gap — just a dead
token.
Add `!! ERROR` and `!! Exiting script` to the bad-pattern set so the
grep actually matches the function's output. Keep the literal
`error_exit` token as belt-and-suspenders for any debug/echo of the
name.
2. `test_docker_init_excludes_egg_info_during_staging` was a single
`assert "egg-info" in src` check. That passes if any occurrence
appears — including the explanatory comment block above the staging
logic. A maintainer removing the `--exclude='*.egg-info'` from
rsync but keeping the comment would slip past the test.
Tighten to:
- scope to the staging block (between `_stage_src=` and the
`uv pip install` line) so comments outside that window can't
satisfy the assertion;
- require the literal `--exclude='*.egg-info'` rsync flag;
- require `*.egg-info` in the block so the cp-fallback cleanup is
also pinned;
- additionally require `--exclude='build'`, `--exclude='dist'`,
`--exclude='__pycache__'` so all four setuptools-touchable
artifact dirs stay excluded.
Verified:
- tests/test_docker_docs_and_readonly.py — 11/11 pass.
- YAML parses cleanly via `yaml.safe_load`.
- Full suite: 5770 passed, 0 failed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Docker smoke gate added in this same PR caught a real production
regression on its very first CI run. v0.51.84 (PR #2470) mounted
hermes-agent-src read-only on the WebUI side and widened the chown
prune to keep the read-only walk happy, but missed that the WebUI's
startup also runs:
uv pip install "$_agent_src[all]"
against the same now-read-only mount. setuptools' egg_info step writes
hermes_agent.egg-info/ inside the source tree even under PEP 517 build
isolation (this is by design -- PEP 517 isolates the BUILD environment,
not the source tree's metadata directory). On a :ro mount this returns
EROFS, the install fails, error_exit fires, and every multi-container
deploy dies at startup. The smoke gate flagged it on both the
two-container and three-container variants.
The fix
-------
Stage the agent source into a writable build dir under /tmp BEFORE
invoking pip install, then point pip at the staged copy.
_stage_src="/tmp/hermes-agent-build"
rm -rf "$_stage_src" && mkdir -p "$_stage_src"
rsync -a --exclude='*.egg-info' --exclude='build' --exclude='dist' \
--exclude='__pycache__' --exclude='.git' \
"$_agent_src"/ "$_stage_src"/
uv pip install "$_stage_src[all]" ...
rm -rf "$_stage_src"
The exclusion list matters: when setuptools sees a pre-baked *.egg-info,
build, or dist directory, it takes a timestamp-update code path that
also reads/writes inside that directory -- which itself fails on a :ro
source. Excluding them keeps the build on the fresh-build path
unconditionally.
rsync is in the production image (Dockerfile line 41-44). For users
running custom WebUI images without rsync, the script falls back to
cp -a + post-copy rm -rf of the same artifacts.
Tests
-----
Two new source-level invariants in tests/test_docker_docs_and_readonly.py:
test_docker_init_stages_agent_source_for_writable_install
-- asserts _stage_src=... is declared
-- asserts every `uv pip install ...[all]` line uses _stage_src,
NOT raw $_agent_src
test_docker_init_excludes_egg_info_during_staging
-- asserts the staging path excludes *.egg-info (rsync exclude
form or cp-fallback's explicit rm -rf both pass)
These would have caught the v0.51.84 regression at the source level
(once written; they're new). The Docker runtime smoke gate is the
durable defence for the broader class of :ro x init-script
interactions, since source-level invariants only catch what they're
written to catch.
Verification
------------
- pytest tests/test_docker_docs_and_readonly.py: 11 passed (9 existing
+ 2 new)
- pytest tests/ -q --timeout=60: 5891 passed, 6 skipped (was 5889;
delta is exactly the 2 new tests)
- bash -n docker_init.bash: clean
Once this lands, the Docker smoke gate's two/three-container variants
should go green, completing the self-validating loop.
PR #2470 introduces a `:ro` mount for the `hermes-agent-src` named volume
on the WebUI side of `docker-compose.{two,three}-container.yml`. The
WebUI's docker_init.bash unconditionally runs `chown_home_hermeswebui`
which walks `/home/hermeswebui` with `find -exec chown -h {} +`,
pruning only `/home/hermeswebui/.hermes/hermes-agent/.git/objects` (the
narrow #2237 fix for macOS bind mounts).
With the new `:ro` mount, every other file inside the hermes-agent
subtree is also on a read-only filesystem. `chown` returns `EROFS`,
`find -exec ... +` propagates the non-zero exit, and the wrapping
`chown_home_hermeswebui || error_exit "..."` under `set -e` kills the
container before the WebUI server can run.
Verified locally:
$ /usr/bin/find /tmp/ftest -exec false {} +
$ echo $?
1
So `find` does propagate `-exec` command failures, which the existing
`|| error_exit` then catches.
The WebUI never writes to the agent source — `uv pip install
/home/hermeswebui/.hermes/hermes-agent` is a pure read. So aligning
ownership inside the agent subtree was always a nicety, not a
requirement. Widen the prune to skip the entire
`/home/hermeswebui/.hermes/hermes-agent` path. This also subsumes the
original #2237 case (the `.git/objects` packs are inside the now-pruned
subtree) without needing a separate carve-out.
Test updates:
- Renamed `test_home_chown_skips_hermes_agent_git_objects` →
`test_home_chown_skips_hermes_agent_subtree`, and pinned the broader
prune target (`-path ".../hermes-agent" -prune`).
- Added `test_home_chown_helper_documents_readonly_mount_compat` so a
future maintainer narrowing the prune back to `.git/objects` (and
re-introducing the EROFS failure mode) trips a regression.
Verified:
- `tests/test_issue2237_docker_chown_git_objects.py` 4/4 pass.
- `tests/test_docker_docs_and_readonly.py` 9/9 pass.
- Full suite: 5738 passed, 0 failed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The hermes-agent-src named volume in the two- and three-container compose
files is initialised from the agent image's /opt/hermes on first `up` and
Docker reuses it verbatim on every subsequent `up` — even after a fresh
`docker pull` of the agent image. This was the root cause of #1416 (the
'missing entrypoint' symptom was a stale cached volume hiding the new
image's source tree).
Changes:
- Add an 'Upgrading the agent container' section to docs/docker.md with
the canonical `down → docker volume rm → pull → up -d` recipe, plus the
same pointer as a comment block in both multi-container compose files
near the volume declarations.
- Switch the WebUI's hermes-agent-src mount to `:ro` in both multi-container
compose files. The WebUI only reads this volume to install the agent's
Python deps at startup; mounting it read-only enforces that at the kernel
layer and brings the actual mount mode in line with the existing
docs/docker.md architecture diagram (which already labelled this edge as
read-only).
- Align the workspace bind default in both multi-container compose files
with the single-container convention — `${HERMES_WORKSPACE:-${HOME}/workspace}`
instead of `${HERMES_WORKSPACE:-~/workspace}` — so the default resolves
the same way across Linux, macOS, WSL2, and Docker Desktop on Windows.
- Add a 'What the multi-container setup isolates (and what it doesn't)'
section to docs/docker.md to frame the two/three-container setups as
process/network/resource isolation, not filesystem isolation, so users
don't reach for multi-container expecting a trust boundary it doesn't
provide.
- Cross-link #1416 from the Related issues section.
Adds 9 regression tests in tests/test_docker_docs_and_readonly.py covering:
- :ro on the WebUI side of hermes-agent-src in both files
- agent side stays read-write (still needs to populate /opt/hermes on first run)
- ${HOME} (not ~) in workspace bind defaults in both files
- single-container file already uses ${HOME} (pin to prevent drift)
- docs/docker.md has the 'Upgrading the agent container' section + recipe
- compose files reference docs/docker.md + show the upgrade step inline
- docs/docker.md frames the isolation model honestly
Test suite: 42 passed (33 existing Docker tests + 9 new). No behaviour
change for users who set HERMES_WORKSPACE explicitly, and no migration is
required for existing deployments — Docker rebinds the existing volume
read-only on next `up`. Users upgrading the agent image should now follow
the documented `docker volume rm hermes-agent-src` recipe.
Closes#1416 (documented upgrade procedure) and addresses the read-only
half of the multi-container coupling concern raised on #2453.
r.kulbaev