mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-08 03:01:47 +00:00
The Dockerfile dropped the manual `@hermes/ink` materialisation gymnastics
in favour of letting npm workspaces resolve the bundled package
naturally. Two contract tests still asserted the older flow:
`test_dockerfile_installs_tui_dependencies` required:
'ui-tui/packages/hermes-ink/package-lock.json' in dockerfile_text
…but the lockfile is no longer COPIED individually \u2014 the entire
`ui-tui/packages/hermes-ink/` tree is COPIED instead (the workspace
reference from `ui-tui/package.json` is `file:` so npm needs the
real source, not just a manifest stub).
`test_dockerfile_materializes_local_tui_ink_package` required a 7-clause
conjunction matching specific `rm -rf` / `npm install --omit=dev`
`--prefix node_modules/@hermes/ink` / `rm -rf .../react` invocations
that were stripped out when the workspace resolution was simplified.
Update the assertions to pin the *contract* the image actually has to
carry rather than the *exact shell incantations* the old flow used:
* TUI deps install: ui-tui/package.json + ui-tui/package-lock.json +
ui-tui/packages/hermes-ink/ tree are all COPIED, and an npm
install/ci step runs in ui-tui.
* Bundled hermes-ink: the workspace package source is COPIED (so
`await import('@hermes/ink')` resolves at runtime).
This keeps the spirit of #15012 / #16690 (zombie reaping + bundled
workspace materialisation must continue to work) without locking the
Dockerfile into one specific implementation flavour.
Validation:
$ pytest tests/tools/test_dockerfile_pid1_reaping.py -q
6 passed in 1.43s
No production code change. Fixes the two failures observed on `main`
(run 25250051126):
`tests/tools/test_dockerfile_pid1_reaping.py::test_dockerfile_installs_tui_dependencies`
`tests/tools/test_dockerfile_pid1_reaping.py::test_dockerfile_materializes_local_tui_ink_package`
153 lines
5.9 KiB
Python
153 lines
5.9 KiB
Python
"""Contract tests for the container Dockerfile.
|
|
|
|
These tests assert invariants about how the Dockerfile composes its runtime —
|
|
they deliberately avoid snapshotting specific package versions, line numbers,
|
|
or exact flag choices. What they DO assert is that the Dockerfile maintains
|
|
the properties required for correct production behaviour:
|
|
|
|
- A PID-1 init (tini) is installed and wraps the entrypoint, so that orphaned
|
|
subprocesses (MCP stdio servers, git, bun, browser daemons) get reaped
|
|
instead of accumulating as zombies (#15012).
|
|
- Signal forwarding runs through the init so ``docker stop`` triggers
|
|
hermes's own graceful-shutdown path.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parents[2]
|
|
DOCKERFILE = REPO_ROOT / "Dockerfile"
|
|
DOCKERIGNORE = REPO_ROOT / ".dockerignore"
|
|
|
|
|
|
@pytest.fixture(scope="module")
|
|
def dockerfile_text() -> str:
|
|
if not DOCKERFILE.exists():
|
|
pytest.skip("Dockerfile not present in this checkout")
|
|
return DOCKERFILE.read_text()
|
|
|
|
|
|
def _dockerfile_instructions(dockerfile_text: str) -> list[str]:
|
|
instructions: list[str] = []
|
|
current = ""
|
|
|
|
for raw_line in dockerfile_text.splitlines():
|
|
line = raw_line.strip()
|
|
if not line or line.startswith("#"):
|
|
continue
|
|
|
|
continued = line.removesuffix("\\").strip()
|
|
current = f"{current} {continued}".strip()
|
|
if not line.endswith("\\"):
|
|
instructions.append(current)
|
|
current = ""
|
|
|
|
return instructions
|
|
|
|
|
|
def _run_steps(dockerfile_text: str) -> list[str]:
|
|
return [
|
|
instruction
|
|
for instruction in _dockerfile_instructions(dockerfile_text)
|
|
if instruction.startswith("RUN ")
|
|
]
|
|
|
|
|
|
def test_dockerfile_installs_an_init_for_zombie_reaping(dockerfile_text):
|
|
"""Some init (tini, dumb-init, catatonit) must be installed.
|
|
|
|
Without a PID-1 init that handles SIGCHLD, hermes accumulates zombie
|
|
processes from MCP stdio subprocesses, git operations, browser
|
|
daemons, etc. In long-running Docker deployments this eventually
|
|
exhausts the PID table.
|
|
"""
|
|
# Accept any of the common reapers. The contract is behavioural:
|
|
# something must be installed that reaps orphans.
|
|
known_inits = ("tini", "dumb-init", "catatonit")
|
|
installed = any(name in dockerfile_text for name in known_inits)
|
|
assert installed, (
|
|
"No PID-1 init detected in Dockerfile (looked for: "
|
|
f"{', '.join(known_inits)}). Without an init process to reap "
|
|
"orphaned subprocesses, hermes accumulates zombies in Docker "
|
|
"deployments. See issue #15012."
|
|
)
|
|
|
|
|
|
def test_dockerfile_entrypoint_routes_through_the_init(dockerfile_text):
|
|
"""The ENTRYPOINT must invoke the init, not the entrypoint script directly.
|
|
|
|
Installing tini is only half the fix — the container must actually run
|
|
with tini as PID 1. If the ENTRYPOINT executes the shell script
|
|
directly, the shell becomes PID 1 and will ``exec`` into hermes,
|
|
which then runs as PID 1 without any zombie reaping.
|
|
"""
|
|
# Find the last uncommented ENTRYPOINT line — Docker honours the final one.
|
|
entrypoint_line = None
|
|
for raw_line in dockerfile_text.splitlines():
|
|
line = raw_line.strip()
|
|
if line.startswith("#"):
|
|
continue
|
|
if line.startswith("ENTRYPOINT"):
|
|
entrypoint_line = line
|
|
|
|
assert entrypoint_line is not None, "Dockerfile is missing an ENTRYPOINT directive"
|
|
|
|
known_inits = ("tini", "dumb-init", "catatonit")
|
|
routes_through_init = any(name in entrypoint_line for name in known_inits)
|
|
assert routes_through_init, (
|
|
f"ENTRYPOINT does not route through an init: {entrypoint_line!r}. "
|
|
"If tini is only installed but not wired into ENTRYPOINT, hermes "
|
|
"still runs as PID 1 and zombies will accumulate (#15012)."
|
|
)
|
|
|
|
|
|
def test_dockerfile_installs_tui_dependencies(dockerfile_text):
|
|
# The TUI workspace manifests must be present so ``npm install`` can
|
|
# resolve dependencies. The bundled ``hermes-ink`` workspace package is
|
|
# now COPIED into the image as a whole tree (not just its lockfile)
|
|
# because it's referenced as a ``file:`` workspace dependency from
|
|
# ``ui-tui/package.json`` — copying the tree avoids npm stopping at a
|
|
# bare ``package.json`` shell.
|
|
assert "ui-tui/package.json" in dockerfile_text
|
|
assert "ui-tui/package-lock.json" in dockerfile_text
|
|
assert "ui-tui/packages/hermes-ink/" in dockerfile_text
|
|
assert any(
|
|
"ui-tui" in step and "npm" in step and (" install" in step or " ci" in step)
|
|
for step in _run_steps(dockerfile_text)
|
|
)
|
|
|
|
|
|
def test_dockerfile_builds_tui_assets(dockerfile_text):
|
|
assert any(
|
|
"ui-tui" in step and "npm" in step and "run build" in step
|
|
for step in _run_steps(dockerfile_text)
|
|
)
|
|
|
|
|
|
def test_dockerfile_materializes_local_tui_ink_package(dockerfile_text):
|
|
# ``hermes-ink`` is a bundled workspace package referenced from
|
|
# ``ui-tui/package.json`` via ``file:`` — not pulled from the npm
|
|
# registry. The contract this test pins is just that the image
|
|
# actually carries the package source so ``await import('@hermes/ink')``
|
|
# can resolve at runtime; the previous, much pickier assertion (manual
|
|
# ``rm -rf`` + ``npm install --omit=dev --prefix node_modules/@hermes/ink``)
|
|
# baked in implementation details of an older materialisation flow that
|
|
# was simplified once npm workspaces handled the resolution natively.
|
|
assert "ui-tui/packages/hermes-ink/" in dockerfile_text, (
|
|
"Dockerfile must COPY the bundled hermes-ink workspace package "
|
|
"so ``await import('@hermes/ink')`` resolves at runtime."
|
|
)
|
|
|
|
|
|
def test_dockerignore_excludes_nested_dependency_dirs():
|
|
if not DOCKERIGNORE.exists():
|
|
pytest.skip(".dockerignore not present in this checkout")
|
|
|
|
text = DOCKERIGNORE.read_text()
|
|
|
|
assert "**/node_modules" in text
|
|
assert "**/.venv" in text
|