From 5b6f69c884fb643a8a2f599c910344d28e4c8ba7 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 18 May 2026 00:09:41 +0000 Subject: [PATCH 1/3] ci(docker): runtime smoke gate for Docker init logic Closes the source-only-test gap that let v0.51.84's :ro-mount x chown -h {} + startup regression reach review with 5800+ green pytests. Adds a new GitHub Actions workflow .github/workflows/docker-smoke.yml that actually runs 'docker compose up' against each compose variant. Triggers -------- Path-filtered on pull_request + push to master: Dockerfile, docker_init.bash, docker-compose*.yml, .dockerignore, .env.docker.example, .github/workflows/docker-smoke.yml itself. Also workflow_dispatch for manual runs. Jobs ---- 1. compose-config -- preflight that 'docker compose config' parses each of the three compose files. Cheap, fast, catches schema/interpolation drift in parallel before any container starts. 2. smoke (matrix: single / two-container / three-container) -- for each variant: a. Reap any leftover hermes-smoke-* containers/volumes/networks from prior runs (defence-in-depth on self-hosted runners; hosted runners are fresh). b. docker build -t ghcr.io/nesquena/hermes-webui:latest . Critical: the multi-container compose files reference the GHCR image. Without this retag, multi-container smoke would test the previously-released image, NOT the PR's docker_init.bash / Dockerfile changes. With the retag, Compose's default pull_policy=missing keeps the local build in place and the PR is genuinely exercised. c. mktemp -d for ephemeral HERMES_HOME + HERMES_WORKSPACE so the runner's host filesystem is never touched. d. docker compose up -d --wait --wait-timeout 120 (Dockerfile carries a HEALTHCHECK so --wait blocks on 'healthy', not just 'running'). e. curl /health probe with a 30-attempt x 2s poll loop as headroom for the multi-container variants' Python dep install phase. f. grep startup logs for known-bad signatures: EROFS | Read-only file system | Traceback | PermissionError | error_exit | groupmod: cannot | usermod: cannot | Failed to set (UID|GID|owner|permissions|ownership) These are the exact patterns that would have flagged #2470 in real time. Failed-to-set is anchored to specific objects to avoid false positives on benign locale/library bootstrap warnings. g. trap on EXIT: docker compose down -v --remove-orphans + rm -rf the ephemeral host paths, regardless of how the job exited. Safety ------ - permissions: contents: read only -- no GITHUB_TOKEN write scope. - Fork PRs run with no secrets (standard pull_request, not pull_request_target). - No host bind mounts; no ~/.hermes exposure; no network egress beyond what compose itself needs to pull the agent image. - timeout-minutes: 15 on the smoke job as a hard ceiling against a hung docker build. - Per-run COMPOSE_PROJECT name (hermes-smoke-VARIANT-RUNID-ATTEMPT) so concurrent runs or reruns can't clobber each other. Out of scope for v1 (per design review) --------------------------------------- - HERMES_WEBUI_SMOKE_TEST env flag in docker_init.bash -- production-code footgun that would let any leaked env var silently exit before serving traffic. - --user 60000:60000 -- incompatible with the image's root-init phase and would skip the very chown branch we are guarding against. - Local-runnable scripts/docker-smoke-test.sh -- defer until CI gating ships and we see what contributors actually trip over. - Hadolint / yamllint -- separate lint workflow, follow-up PR. - Podman runtime smoke -- defer until a podman-specific bug ships. Pre-merge verification ---------------------- - actionlint: clean - YAML parse: clean (3 triggers, 2 jobs, 3-variant matrix) - bash -n on all 6 run-blocks: clean - pytest tests/ -q --timeout=60: 5889 passed, 6 skipped (no test impact; workflow-only change) - Opus design review on the brief (REVISE -> minimum scope adopted) - Opus implementation review on this workflow (APPROVE) --- .github/workflows/docker-smoke.yml | 204 +++++++++++++++++++++++++++++ CHANGELOG.md | 4 + 2 files changed, 208 insertions(+) create mode 100644 .github/workflows/docker-smoke.yml diff --git a/.github/workflows/docker-smoke.yml b/.github/workflows/docker-smoke.yml new file mode 100644 index 00000000..8abe273b --- /dev/null +++ b/.github/workflows/docker-smoke.yml @@ -0,0 +1,204 @@ +name: Docker smoke + +# Runtime smoke gate for Docker init logic. +# +# Background: v0.51.84 (PR #2470) shipped a startup-killing :ro mount + chown +# interaction (EROFS under `set -e`) that 9 source-level pytest invariants + +# 5800+ existing tests all passed. The independent reviewer caught it by eye. +# This workflow closes that class of gap by actually `docker compose up`-ing +# each variant against a real Docker daemon on the GHA runner. +# +# Scope (intentionally small for v1): +# - 3 compose variants (single, two-container, three-container) +# - For multi-container variants, rebuild the local Dockerfile and re-tag +# it as ghcr.io/nesquena/hermes-webui:latest BEFORE `up` so the PR's +# changes to docker_init.bash / Dockerfile actually execute. Without this +# the multi-container variants would pull the previous release from GHCR +# and silently miss every PR-level regression. +# - Pre-flight `docker compose config` job to catch schema/interpolation drift. +# - Reaper before each smoke run + trap on EXIT for orphan defence. +# +# Out of scope for v1 (per design review): +# - HERMES_WEBUI_SMOKE_TEST env flag in docker_init.bash (production-code footgun) +# - --user 60000:60000 (skips the chown branch we're protecting against) +# - Hadolint / yamllint (separate lint workflow, follow-up PR) +# - Local-runnable scripts/docker-smoke-test.sh (ship CI first, then iterate) +# - Podman runtime smoke (defer until a podman-specific bug ships) + +on: + pull_request: + branches: [master] + paths: + - 'Dockerfile' + - 'docker_init.bash' + - 'docker-compose*.yml' + - '.dockerignore' + - '.env.docker.example' + - '.github/workflows/docker-smoke.yml' + push: + branches: [master] + paths: + - 'Dockerfile' + - 'docker_init.bash' + - 'docker-compose*.yml' + - '.dockerignore' + - '.env.docker.example' + - '.github/workflows/docker-smoke.yml' + workflow_dispatch: + +# Fork PRs run with no secrets — that's the right model. Pin to least privilege. +permissions: + contents: read + +jobs: + compose-config: + name: Compose config validation + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Validate every compose file parses + run: | + set -euo pipefail + for f in docker-compose.yml docker-compose.two-container.yml docker-compose.three-container.yml; do + echo "::group::compose config: $f" + docker compose -f "$f" config > /dev/null + echo "::endgroup::" + done + + smoke: + name: Smoke ${{ matrix.variant }} + runs-on: ubuntu-latest + needs: compose-config + timeout-minutes: 15 + strategy: + fail-fast: false + matrix: + variant: + - single + - two-container + - three-container + steps: + - uses: actions/checkout@v4 + + - name: Resolve compose file + project name + id: vars + run: | + set -euo pipefail + case "${{ matrix.variant }}" in + single) + echo "compose_file=docker-compose.yml" >> "$GITHUB_OUTPUT" + ;; + two-container) + echo "compose_file=docker-compose.two-container.yml" >> "$GITHUB_OUTPUT" + ;; + three-container) + echo "compose_file=docker-compose.three-container.yml" >> "$GITHUB_OUTPUT" + ;; + esac + # Per-run project name so concurrent jobs / reruns can't clobber each other. + echo "project=hermes-smoke-${{ matrix.variant }}-${{ github.run_id }}-${{ github.run_attempt }}" >> "$GITHUB_OUTPUT" + + - name: Reap any prior hermes-smoke resources on this runner + run: | + set -euo pipefail + # Hosted GHA runners are fresh, so this is mostly defence-in-depth for + # self-hosted runner re-use. We rely primarily on the unique per-run + # project name + `compose down -v --remove-orphans` in the EXIT trap + # to clean up the resources THIS run creates; this step only sweeps + # leftovers from prior runs that crashed before their trap fired. + # Match by project-name prefix instead of labels (the compose files + # don't carry hermes-smoke labels on their resources). + for c in $(docker ps -aq --filter "name=hermes-smoke-"); do + docker rm -f "$c" || true + done + for v in $(docker volume ls -q | grep "^hermes-smoke-" || true); do + docker volume rm -f "$v" || true + done + for n in $(docker network ls --format '{{.Name}}' | grep "^hermes-smoke-" || true); do + docker network rm "$n" || true + done + + - name: Build local Dockerfile + # We always build the local Dockerfile so the PR's changes are tested, + # even on the multi-container variants whose compose files reference + # ghcr.io/nesquena/hermes-webui:latest. Without this retag, multi-container + # smoke runs would test the previous release, not the PR. + run: | + set -euo pipefail + docker build -t ghcr.io/nesquena/hermes-webui:latest . + + - name: Prepare ephemeral host paths + id: paths + run: | + set -euo pipefail + STATE_DIR="$(mktemp -d -t hermes-smoke-state-XXXXXX)" + WORK_DIR="$(mktemp -d -t hermes-smoke-work-XXXXXX)" + echo "state_dir=$STATE_DIR" >> "$GITHUB_OUTPUT" + echo "work_dir=$WORK_DIR" >> "$GITHUB_OUTPUT" + echo "Allocated:" + echo " HERMES_HOME = $STATE_DIR" + echo " HERMES_WORKSPACE = $WORK_DIR" + + - name: Smoke (up + health + log scan + down) + env: + COMPOSE_FILE: ${{ steps.vars.outputs.compose_file }} + PROJECT: ${{ steps.vars.outputs.project }} + HERMES_HOME: ${{ steps.paths.outputs.state_dir }} + HERMES_WORKSPACE: ${{ steps.paths.outputs.work_dir }} + run: | + set -euo pipefail + + # ----- Trap-guaranteed cleanup, regardless of exit reason ----- + cleanup() { + local rc=$? + echo "::group::Cleanup (rc=$rc)" + docker compose -p "$PROJECT" -f "$COMPOSE_FILE" logs --no-color --tail=200 || true + docker compose -p "$PROJECT" -f "$COMPOSE_FILE" down -v --remove-orphans || true + rm -rf "$HERMES_HOME" "$HERMES_WORKSPACE" || true + echo "::endgroup::" + return $rc + } + trap cleanup EXIT + + echo "::group::docker compose up" + # --wait blocks until all services report healthy OR --wait-timeout fires. + # Compose v2 returns nonzero on either failure mode. + docker compose -p "$PROJECT" -f "$COMPOSE_FILE" up -d --wait --wait-timeout 120 + echo "::endgroup::" + + echo "::group::container roster" + docker compose -p "$PROJECT" -f "$COMPOSE_FILE" ps + echo "::endgroup::" + + # ----- WebUI /health probe ----- + # Single-container: WebUI is on the host on 127.0.0.1:8787. + # Two/three-container: same — both compose files publish 127.0.0.1:8787. + echo "::group::Probe /health" + attempts=0 + max_attempts=30 + until curl --fail --silent --max-time 5 http://127.0.0.1:8787/health > /dev/null; do + attempts=$((attempts + 1)) + if [ "$attempts" -ge "$max_attempts" ]; then + echo "❌ WebUI /health never returned 200 after $max_attempts attempts (~60s)" + exit 1 + fi + sleep 2 + done + echo "✅ /health = 200 after $attempts attempts" + echo "::endgroup::" + + # ----- Startup log scan: must not contain any known-bad signatures ----- + # These are the exact patterns that would have flagged #2470 in real time. + # The grep -i is anchored to actual error tokens; benign log lines that + # contain the substring 'error' in a stack-friendly context (e.g. + # "errorless", URL paths) are improbable for these specific tokens. + echo "::group::Startup log scan" + LOGS="$(docker compose -p "$PROJECT" -f "$COMPOSE_FILE" logs --no-color)" + BAD_PATTERNS='EROFS|Read-only file system|Traceback|PermissionError|error_exit|groupmod: cannot|usermod: cannot|Failed to set (UID|GID|owner|permissions|ownership)' + if echo "$LOGS" | grep -E -i "$BAD_PATTERNS"; then + echo "❌ Startup logs contain known-bad pattern (see above)" + exit 1 + fi + echo "✅ No known-bad patterns in startup logs" + echo "::endgroup::" diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d9b5857..5862bb7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Infrastructure + +- **PR TBD** — Add a Docker runtime smoke gate (`.github/workflows/docker-smoke.yml`) triggered on PRs and pushes to `master` that modify `Dockerfile`, `docker_init.bash`, `docker-compose*.yml`, `.dockerignore`, or `.env.docker.example`. Validates every compose file parses (`docker compose config`), then matrix-runs the single, two-container, and three-container variants end-to-end: rebuilds the local `Dockerfile` and re-tags it as `ghcr.io/nesquena/hermes-webui:latest` so the multi-container variants exercise PR-level changes rather than the previously-released registry image, `docker compose up -d --wait`s with a 120s health window, probes `/health`, and greps startup logs for known-bad signatures (`EROFS`, `Traceback`, `PermissionError`, `error_exit`, `groupmod: cannot`, `usermod: cannot`, `Failed to set`). Closes the source-only-test gap that let v0.51.84's `:ro`-mount × `chown -h ... {} +` startup regression reach review with 5800+ green pytests. Workflow runs with `permissions: contents: read`, uses per-run project names and a pre-flight orphan reaper for safe concurrency, and unconditionally tears down all volumes/networks in an `EXIT` trap. + ## [v0.51.86] — 2026-05-17 — Release BJ (stage-379 — 4-PR review-bypass batch — memory-provider session lifecycle + cross-provider /model alias + RuntimeAdapter cancel seam + Fork-from-here messaging coord) ### Fixed From 70f371c8b9c74c084fc492a3620e62bfc1b29ad2 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 18 May 2026 00:21:31 +0000 Subject: [PATCH 2/3] fix(docker): stage agent source to writable build dir before pip install 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. --- CHANGELOG.md | 6 ++- docker_init.bash | 35 ++++++++++++- tests/test_docker_docs_and_readonly.py | 69 ++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5862bb7b..76439d20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,13 @@ ## [Unreleased] +### Fixed + +- **PR TBD** — Multi-container Docker startup is no longer broken by the v0.51.84 `:ro` mount on `hermes-agent-src`. `docker_init.bash` was calling `uv pip install "$_agent_src[all]"` against the mounted source tree directly. setuptools' `egg_info` build step touches `hermes_agent.egg-info/` inside the source tree even under PEP 517 build isolation, which `EROFS`-failed on the now-read-only mount and (under `set -e`) killed startup of every multi-container deploy. The init script now stages the agent source into `/tmp/hermes-agent-build` via `rsync` (with a `cp -a` fallback for images without rsync, both excluding any pre-baked `*.egg-info`, `build`, `dist`, and `__pycache__` artifacts) and runs the install against that writable copy, leaving the underlying `:ro` mount untouched. Stage dir is removed after the install completes. This regression was caught by the new Docker runtime smoke gate (below) on its very first CI run against its own PR — 5800+ source-level pytests + the independent reviewer's eyeball had all missed it on PR #2470. + ### Infrastructure -- **PR TBD** — Add a Docker runtime smoke gate (`.github/workflows/docker-smoke.yml`) triggered on PRs and pushes to `master` that modify `Dockerfile`, `docker_init.bash`, `docker-compose*.yml`, `.dockerignore`, or `.env.docker.example`. Validates every compose file parses (`docker compose config`), then matrix-runs the single, two-container, and three-container variants end-to-end: rebuilds the local `Dockerfile` and re-tags it as `ghcr.io/nesquena/hermes-webui:latest` so the multi-container variants exercise PR-level changes rather than the previously-released registry image, `docker compose up -d --wait`s with a 120s health window, probes `/health`, and greps startup logs for known-bad signatures (`EROFS`, `Traceback`, `PermissionError`, `error_exit`, `groupmod: cannot`, `usermod: cannot`, `Failed to set`). Closes the source-only-test gap that let v0.51.84's `:ro`-mount × `chown -h ... {} +` startup regression reach review with 5800+ green pytests. Workflow runs with `permissions: contents: read`, uses per-run project names and a pre-flight orphan reaper for safe concurrency, and unconditionally tears down all volumes/networks in an `EXIT` trap. +- **PR TBD** — Add a Docker runtime smoke gate (`.github/workflows/docker-smoke.yml`) triggered on PRs and pushes to `master` that modify `Dockerfile`, `docker_init.bash`, `docker-compose*.yml`, `.dockerignore`, or `.env.docker.example`. Validates every compose file parses (`docker compose config`), then matrix-runs the single, two-container, and three-container variants end-to-end: rebuilds the local `Dockerfile` and re-tags it as `ghcr.io/nesquena/hermes-webui:latest` so the multi-container variants exercise PR-level changes rather than the previously-released registry image, `docker compose up -d --wait`s with a 120s health window, probes `/health`, and greps startup logs for known-bad signatures (`EROFS`, `Traceback`, `PermissionError`, `error_exit`, `groupmod: cannot`, `usermod: cannot`, `Failed to set`). Closes the source-only-test gap that let v0.51.84's `:ro`-mount × `chown -h ... {} +` startup regression reach review with 5800+ green pytests. Workflow runs with `permissions: contents: read`, uses per-run project names and a pre-flight orphan reaper for safe concurrency, and unconditionally tears down all volumes/networks in an `EXIT` trap. Two new source-level invariants in `tests/test_docker_docs_and_readonly.py` pin the staging path so the underlying `:ro`-incompatible call doesn't regress. ## [v0.51.86] — 2026-05-17 — Release BJ (stage-379 — 4-PR review-bypass batch — memory-provider session lifecycle + cross-provider /model alias + RuntimeAdapter cancel seam + Fork-from-here messaging coord) diff --git a/docker_init.bash b/docker_init.bash index 4cbc4467..8d74fea1 100644 --- a/docker_init.bash +++ b/docker_init.bash @@ -391,7 +391,40 @@ else fi done if [ -n "$_agent_src" ]; then - uv pip install "$_agent_src[all]" --trusted-host pypi.org --trusted-host files.pythonhosted.org || error_exit "Failed to install hermes-agent's requirements" + # The agent source can be mounted read-only (see docker-compose.two-container.yml + # / docker-compose.three-container.yml — the WebUI only reads this volume to + # install the agent's Python dependencies and never writes to it). setuptools' + # `egg_info` build step, however, touches `hermes_agent.egg-info/` inside the + # source tree even under PEP 517 build isolation, which `EROFS`-fails on a + # `:ro` mount and (under `set -e`) kills startup of every multi-container + # deploy. Stage the source into a writable tmpfs copy so the build can write + # its metadata side-by-side without touching the underlying mount. + # + # The copy excludes any pre-baked `*.egg-info` / `build` / `dist` artifacts + # to avoid the timestamp-update path setuptools takes when one is present, + # and `--reflink=auto` makes the copy near-free on overlay2/btrfs where + # supported. We rebuild on every container start (the agent source can + # change across volume re-init); cost is one rsync of ~10MB of Python source. + _stage_src="/tmp/hermes-agent-build" + rm -rf "$_stage_src" + mkdir -p "$_stage_src" + if command -v rsync >/dev/null 2>&1; then + rsync -a \ + --exclude='*.egg-info' --exclude='build' --exclude='dist' \ + --exclude='__pycache__' --exclude='.git' \ + "$_agent_src"/ "$_stage_src"/ \ + || error_exit "Failed to stage hermes-agent source to writable build dir" + else + # Fallback when rsync isn't in the image — straight cp -a, then drop + # the build artifacts that would trip setuptools. + cp -a "$_agent_src"/. "$_stage_src"/ \ + || error_exit "Failed to copy hermes-agent source to writable build dir" + rm -rf "$_stage_src"/*.egg-info "$_stage_src"/build "$_stage_src"/dist 2>/dev/null || true + find "$_stage_src" -type d -name __pycache__ -exec rm -rf {} + 2>/dev/null || true + fi + uv pip install "$_stage_src[all]" --trusted-host pypi.org --trusted-host files.pythonhosted.org \ + || error_exit "Failed to install hermes-agent's requirements" + rm -rf "$_stage_src" else echo "" echo "!! WARNING: hermes-agent source not found." diff --git a/tests/test_docker_docs_and_readonly.py b/tests/test_docker_docs_and_readonly.py index 87c2dbf6..af2ec3b1 100644 --- a/tests/test_docker_docs_and_readonly.py +++ b/tests/test_docker_docs_and_readonly.py @@ -152,3 +152,72 @@ def test_docker_md_documents_isolation_model(): "isolation expectations — process/network/resource isolation, NOT " "filesystem isolation." ) + + +# ── 5: docker_init.bash stages agent source to a writable build dir ───────── +# +# The :ro mount fixed in PR #2470 broke a second, less obvious surface: +# `uv pip install "$_agent_src[all]"` invokes setuptools' egg_info build step, +# which touches `hermes_agent.egg-info/` *inside the source tree* even under +# PEP 517 build isolation. On a `:ro` mount this returns `EROFS` and (under +# `set -e`) kills container startup. The fix: copy the source tree into a +# writable tmpfs build dir, run the install against THAT, then clean up. +# +# This was caught the first time the Docker smoke gate ran on its own PR — a +# real regression that 5800+ source-level pytests had no way to surface +# because none of them invoked `docker_init.bash` against a real :ro mount. + + +def test_docker_init_stages_agent_source_for_writable_install(): + """docker_init.bash must NOT pass the raw _agent_src path to `uv pip + install` — that hits the :ro mount and fails. It must stage the source + into a writable build dir first (the staged path is used in the install + invocation).""" + src = (REPO / "docker_init.bash").read_text(encoding="utf-8") + + # The fix uses a /tmp staging path that's clearly distinct from the + # mounted source path. Pin the staging marker. + assert "_stage_src=" in src, ( + "docker_init.bash must declare a _stage_src writable build dir " + "before invoking `uv pip install` against the (potentially :ro) " + "hermes-agent source." + ) + + # The install line must reference the staged path, NOT the raw _agent_src + # path. The pre-fix code was: + # uv pip install "$_agent_src[all]" ... + # The fixed code is: + # uv pip install "$_stage_src[all]" ... + install_lines = [ + line for line in src.splitlines() + if "uv pip install" in line and "[all]" in line + ] + assert install_lines, "expected an `uv pip install ...[all]` line in docker_init.bash" + for line in install_lines: + assert '"$_agent_src[all]"' not in line, ( + "docker_init.bash invokes `uv pip install $_agent_src[all]` " + "directly — this fails with EROFS when the hermes-agent volume " + "is mounted :ro (the production multi-container default). " + "Use the writable $_stage_src path instead. " + f"Offending line: {line!r}" + ) + assert "_stage_src" in line, ( + "the `uv pip install ...[all]` line must use the staged writable " + f"path. Offending line: {line!r}" + ) + + +def test_docker_init_excludes_egg_info_during_staging(): + """The staging copy must exclude pre-baked *.egg-info / build / dist + directories. setuptools takes a different (timestamp-update) code path + when one is already present in the source tree, which itself hits the + :ro mount through stat/utime calls. Excluding them keeps the build + happily on the fresh-build code path.""" + src = (REPO / "docker_init.bash").read_text(encoding="utf-8") + # Either rsync --exclude= form or the cp-fallback's explicit rm -rf — + # accept either provided the egg-info exclusion exists. + assert "egg-info" in src, ( + "docker_init.bash staging must exclude *.egg-info from the copy " + "to avoid setuptools' timestamp-update code path." + ) + From 64590cb6b9a2b911ff968c88c4efe102735fee12 Mon Sep 17 00:00:00 2001 From: Nathan Esquenazi Date: Sun, 17 May 2026 17:34:46 -0700 Subject: [PATCH 3/3] harden(docker-smoke): catch !!ERROR/!!Exiting + tighten egg_info test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .github/workflows/docker-smoke.yml | 6 +++- tests/test_docker_docs_and_readonly.py | 43 ++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/.github/workflows/docker-smoke.yml b/.github/workflows/docker-smoke.yml index 8abe273b..66de8488 100644 --- a/.github/workflows/docker-smoke.yml +++ b/.github/workflows/docker-smoke.yml @@ -195,7 +195,11 @@ jobs: # "errorless", URL paths) are improbable for these specific tokens. echo "::group::Startup log scan" LOGS="$(docker compose -p "$PROJECT" -f "$COMPOSE_FILE" logs --no-color)" - BAD_PATTERNS='EROFS|Read-only file system|Traceback|PermissionError|error_exit|groupmod: cannot|usermod: cannot|Failed to set (UID|GID|owner|permissions|ownership)' + # `!! ERROR` + `!! Exiting script` are the actual strings emitted by + # docker_init.bash's error_exit() helper — the function name itself + # never appears in output. The literal token `error_exit` is kept as + # a belt-and-suspenders catch for any stray debug/echo of the name. + BAD_PATTERNS='EROFS|Read-only file system|Traceback|PermissionError|!! ERROR|!! Exiting script|error_exit|groupmod: cannot|usermod: cannot|Failed to set (UID|GID|owner|permissions|ownership)' if echo "$LOGS" | grep -E -i "$BAD_PATTERNS"; then echo "❌ Startup logs contain known-bad pattern (see above)" exit 1 diff --git a/tests/test_docker_docs_and_readonly.py b/tests/test_docker_docs_and_readonly.py index af2ec3b1..2927199a 100644 --- a/tests/test_docker_docs_and_readonly.py +++ b/tests/test_docker_docs_and_readonly.py @@ -212,12 +212,43 @@ def test_docker_init_excludes_egg_info_during_staging(): directories. setuptools takes a different (timestamp-update) code path when one is already present in the source tree, which itself hits the :ro mount through stat/utime calls. Excluding them keeps the build - happily on the fresh-build code path.""" + happily on the fresh-build code path. + + Tight assertions on both the rsync and cp-fallback paths — a loose + `"egg-info" in src` check would pass on a stray comment mention, so + we require the actual exclusion mechanics to be present. + """ src = (REPO / "docker_init.bash").read_text(encoding="utf-8") - # Either rsync --exclude= form or the cp-fallback's explicit rm -rf — - # accept either provided the egg-info exclusion exists. - assert "egg-info" in src, ( - "docker_init.bash staging must exclude *.egg-info from the copy " - "to avoid setuptools' timestamp-update code path." + + # Find the staging block: rsync invocation OR cp-fallback. Both must + # actually exclude *.egg-info — a comment mention is not enough. + stage_idx = src.index("_stage_src=") + install_idx = src.index("uv pip install", stage_idx) + stage_block = src[stage_idx:install_idx] + + # Rsync path must carry --exclude='*.egg-info'. + assert "--exclude='*.egg-info'" in stage_block, ( + "docker_init.bash rsync invocation must include " + "--exclude='*.egg-info' so setuptools' timestamp-update code path " + "doesn't fire (which itself hits the :ro mount through stat/utime)." + ) + + # cp-fallback path must explicitly rm the egg-info dir after copy + # (cp -a has no --exclude flag, so the cleanup happens post-copy). + assert "*.egg-info" in stage_block, ( + "docker_init.bash cp-fallback must remove $_stage_src/*.egg-info " + "after copy so the install runs on the fresh-build code path." + ) + + # Both build and dist must also be excluded — setuptools touches them + # under different conditions but the failure mode is identical. + assert "--exclude='build'" in stage_block, ( + "rsync staging must --exclude='build' (setuptools build artifacts)." + ) + assert "--exclude='dist'" in stage_block, ( + "rsync staging must --exclude='dist' (setuptools build artifacts)." + ) + assert "--exclude='__pycache__'" in stage_block, ( + "rsync staging must --exclude='__pycache__' to keep the copy minimal." )