mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-05 07:41:39 +00:00
fix(docker): bake build-time git SHA into the image
`hermes dump` and the startup banner both call `git rev-parse HEAD` to
report the running commit, but `.dockerignore` line 2 excludes `.git` —
so inside the published image `hermes dump` shows
`version: ... [(unknown)]` and the banner drops its `· upstream <sha>`
suffix entirely. That makes support triage from container bug reports
impossible: we can't tell which commit the user is actually running.
Fix: thread the build-time SHA through as a Docker build-arg, write it
to `/opt/hermes/.hermes_build_sha` in the image, and have a new
`hermes_cli/build_info.get_build_sha()` read it as a fallback after the
existing live-git lookup fails. Output format is unchanged in both
callsites — same 8-char short SHA whether resolved live or baked.
Wiring:
- Dockerfile: `ARG HERMES_GIT_SHA=` + write-file step after the source
copy. Empty/missing arg → no file written → callers fall through to
live git (so local `docker build` without --build-arg is unchanged).
- docker-publish.yml: passes `HERMES_GIT_SHA=${{ github.sha }}` on all
four build-push-action steps (amd64/arm64, smoke-test + final push).
- dump.py:_get_git_commit() / banner.py:get_git_banner_state(): try
live git first, fall back to baked SHA, then to legacy `(unknown)`
/ None. Banner returns `upstream == local, ahead=0` because a built
image is by definition pinned to one commit.
Coverage:
- Unit tests cover build_info (file present/absent/empty/error,
truncation, whitespace), dump (live-git wins, both fallbacks,
identical output-format regression guard), and banner (no-repo +
baked, no-repo + no-sha, shallow-clone fallback).
- tests/docker/test_dump_build_sha.py is an integration regression
guard that runs against the real image, reads
`/opt/hermes/.hermes_build_sha`, and asserts `hermes dump` surfaces
its content (or stays at `(unknown)` if no file).
- Verified end-to-end: `docker build --build-arg HERMES_GIT_SHA=abc...`
→ `docker run ... dump` reports `[abc12345]`; without the build-arg
it reports `[(unknown)]` as before.
This commit is contained in:
parent
ebe04c66cd
commit
66489f38c7
9 changed files with 488 additions and 3 deletions
104
tests/docker/test_dump_build_sha.py
Normal file
104
tests/docker/test_dump_build_sha.py
Normal file
|
|
@ -0,0 +1,104 @@
|
|||
"""Regression test: ``hermes dump`` reports a real git SHA inside the container.
|
||||
|
||||
Background: ``.dockerignore`` excludes ``.git``, so ``git rev-parse HEAD``
|
||||
fails inside the published image and ``hermes dump`` used to report
|
||||
``version: ... [(unknown)]``. The Dockerfile now writes the build-time
|
||||
``$HERMES_GIT_SHA`` build-arg to ``/opt/hermes/.hermes_build_sha`` and
|
||||
``hermes_cli/build_info.py`` reads it as a fallback.
|
||||
|
||||
CI (``.github/workflows/docker-publish.yml``) always sets the build-arg
|
||||
to ``${{ github.sha }}``. Local ``docker build`` (the ``built_image``
|
||||
fixture in ``tests/docker/conftest.py``) does NOT — so locally the file
|
||||
is absent and ``hermes dump`` correctly falls back to ``(unknown)``.
|
||||
|
||||
This test handles both cases:
|
||||
|
||||
* If ``/opt/hermes/.hermes_build_sha`` exists in the image, assert that
|
||||
``hermes dump`` surfaces its content as the version SHA (not
|
||||
``(unknown)``).
|
||||
* If the file is absent, assert the legacy behaviour (``(unknown)``)
|
||||
still holds — defensive guard against the helper accidentally
|
||||
reporting bogus data from somewhere else.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import subprocess
|
||||
|
||||
|
||||
_VERSION_LINE = re.compile(r"^version:\s+(?P<rest>.+)$", re.MULTILINE)
|
||||
_SHA_BRACKET = re.compile(r"\[(?P<sha>[^\]]+)\]\s*$")
|
||||
|
||||
|
||||
def _run_dump(image: str) -> str:
|
||||
"""Return the stdout of ``docker run <image> dump``.
|
||||
|
||||
Relies on Docker's anonymous VOLUME for ``/opt/data`` (declared by the
|
||||
Dockerfile) so the container's hermes user (UID 10000) can bootstrap
|
||||
its config. Anonymous volumes are auto-cleaned by ``--rm``, so unlike
|
||||
a host bind-mount we don't have to chown anything to UID 10000 (which
|
||||
would break cleanup on non-root hosts).
|
||||
"""
|
||||
r = subprocess.run(
|
||||
["docker", "run", "--rm", image, "dump"],
|
||||
capture_output=True, text=True, timeout=120,
|
||||
)
|
||||
assert r.returncode == 0, (
|
||||
f"hermes dump exited {r.returncode}: "
|
||||
f"stderr={r.stderr[-1000:]!r}\nstdout={r.stdout[-1000:]!r}"
|
||||
)
|
||||
return r.stdout
|
||||
|
||||
|
||||
def _read_baked_sha_from_image(image: str) -> str | None:
|
||||
"""Return the ``/opt/hermes/.hermes_build_sha`` content, or None if absent."""
|
||||
r = subprocess.run(
|
||||
[
|
||||
"docker", "run", "--rm", "--entrypoint", "cat", image,
|
||||
"/opt/hermes/.hermes_build_sha",
|
||||
],
|
||||
capture_output=True, text=True, timeout=30,
|
||||
)
|
||||
if r.returncode != 0:
|
||||
return None
|
||||
return r.stdout.strip() or None
|
||||
|
||||
|
||||
def test_dump_reports_baked_sha_when_present(built_image: str) -> None:
|
||||
"""When the image was built with ``HERMES_GIT_SHA``, dump must surface it.
|
||||
|
||||
Together with the smoke-test action (which exercises ``--help``), this
|
||||
closes the regression loop for the missing-sha bug: any future change
|
||||
that breaks the baked-file -> dump pipeline will fail CI here.
|
||||
"""
|
||||
baked = _read_baked_sha_from_image(built_image)
|
||||
stdout = _run_dump(built_image)
|
||||
|
||||
match = _VERSION_LINE.search(stdout)
|
||||
assert match, f"no `version:` line in dump output:\n{stdout[:2000]}"
|
||||
sha_match = _SHA_BRACKET.search(match.group("rest"))
|
||||
assert sha_match, (
|
||||
f"`version:` line missing [<sha>] bracket: {match.group('rest')!r}"
|
||||
)
|
||||
reported = sha_match.group("sha")
|
||||
|
||||
if baked is None:
|
||||
# Local-build path: no build-arg was passed. Verify the legacy
|
||||
# fallback ``(unknown)`` is intact — guards against the helper
|
||||
# ever inventing a SHA from thin air.
|
||||
assert reported == "(unknown)", (
|
||||
f"expected '(unknown)' when no SHA baked, got {reported!r}"
|
||||
)
|
||||
return
|
||||
|
||||
# CI path: build-arg was set, baked file exists. ``hermes dump``
|
||||
# truncates to 8 chars via ``git rev-parse --short=8`` semantics.
|
||||
assert reported != "(unknown)", (
|
||||
"baked SHA file present in image but dump still reported "
|
||||
f"'(unknown)' — the build-info fallback is broken. "
|
||||
f"Baked file content: {baked!r}"
|
||||
)
|
||||
assert reported == baked[:8], (
|
||||
f"dump reported {reported!r} but baked file contained {baked!r} "
|
||||
f"(expected first 8 chars: {baked[:8]!r})"
|
||||
)
|
||||
|
|
@ -61,3 +61,56 @@ def test_get_git_banner_state_reads_origin_and_head(tmp_path):
|
|||
state = banner.get_git_banner_state(repo_dir)
|
||||
|
||||
assert state == {"upstream": "b2f477a3", "local": "af8aad31", "ahead": 3}
|
||||
|
||||
|
||||
def test_get_git_banner_state_falls_back_to_build_sha_when_no_repo():
|
||||
"""Docker image case: no .git checkout — baked build SHA fills the gap.
|
||||
|
||||
``_resolve_repo_dir`` returns None when neither the running code's
|
||||
parent nor ``$HERMES_HOME/hermes-agent/`` is a git repo (the canonical
|
||||
case inside the published container, where .git is dockerignored).
|
||||
The banner should still report the build SHA so support bug reports
|
||||
can identify the running commit.
|
||||
"""
|
||||
from hermes_cli import banner
|
||||
|
||||
with patch.object(banner, "_resolve_repo_dir", return_value=None), \
|
||||
patch("hermes_cli.build_info.get_build_sha", return_value="abcdef12"):
|
||||
state = banner.get_git_banner_state()
|
||||
|
||||
assert state == {"upstream": "abcdef12", "local": "abcdef12", "ahead": 0}
|
||||
|
||||
|
||||
def test_get_git_banner_state_returns_none_when_no_repo_and_no_build_sha():
|
||||
"""Pip-installed wheel with neither git checkout nor baked SHA → None.
|
||||
|
||||
Banner correctly omits the upstream/local suffix in this case.
|
||||
"""
|
||||
from hermes_cli import banner
|
||||
|
||||
with patch.object(banner, "_resolve_repo_dir", return_value=None), \
|
||||
patch("hermes_cli.build_info.get_build_sha", return_value=None):
|
||||
state = banner.get_git_banner_state()
|
||||
|
||||
assert state is None
|
||||
|
||||
|
||||
def test_get_git_banner_state_falls_back_when_live_git_returns_nothing(tmp_path):
|
||||
"""Shallow clone without origin/main → still surface build SHA if baked.
|
||||
|
||||
Some install paths (e.g. ``git clone --depth 1`` without a remote) have
|
||||
a ``.git`` directory but ``git rev-parse origin/main`` fails. When that
|
||||
happens AND a baked SHA exists, return the baked one instead of None.
|
||||
"""
|
||||
from hermes_cli import banner
|
||||
|
||||
repo_dir = tmp_path / "repo"
|
||||
(repo_dir / ".git").mkdir(parents=True)
|
||||
|
||||
# All git invocations fail (returncode=1, empty stdout).
|
||||
failed = MagicMock(returncode=1, stdout="")
|
||||
with patch("hermes_cli.banner.subprocess.run", return_value=failed), \
|
||||
patch("hermes_cli.build_info.get_build_sha", return_value="cafef00d"):
|
||||
state = banner.get_git_banner_state(repo_dir)
|
||||
|
||||
assert state == {"upstream": "cafef00d", "local": "cafef00d", "ahead": 0}
|
||||
|
|
|
|||
78
tests/hermes_cli/test_build_info.py
Normal file
78
tests/hermes_cli/test_build_info.py
Normal file
|
|
@ -0,0 +1,78 @@
|
|||
"""Tests for hermes_cli.build_info — baked-in build SHA resolution.
|
||||
|
||||
The build SHA is written by the Dockerfile's ``HERMES_GIT_SHA`` build-arg
|
||||
into ``<project_root>/.hermes_build_sha``. These tests cover the read-side
|
||||
helper: missing file, malformed file, truncation, and error tolerance.
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
|
||||
def test_get_build_sha_returns_none_when_file_absent(tmp_path):
|
||||
"""Source installs: no file present → None, callers fall back to git."""
|
||||
from hermes_cli import build_info
|
||||
|
||||
missing = tmp_path / ".hermes_build_sha" # never created
|
||||
|
||||
with patch.object(build_info, "_BUILD_SHA_FILE", missing):
|
||||
assert build_info.get_build_sha() is None
|
||||
|
||||
|
||||
def test_get_build_sha_reads_baked_file(tmp_path):
|
||||
"""Docker image case: file exists with full 40-char SHA → truncated to 8."""
|
||||
from hermes_cli import build_info
|
||||
|
||||
sha_file = tmp_path / ".hermes_build_sha"
|
||||
sha_file.write_text("abcdef1234567890abcdef1234567890abcdef12\n")
|
||||
|
||||
with patch.object(build_info, "_BUILD_SHA_FILE", sha_file):
|
||||
assert build_info.get_build_sha() == "abcdef12"
|
||||
|
||||
|
||||
def test_get_build_sha_respects_short_argument(tmp_path):
|
||||
"""``short=N`` truncates to N chars; ``short<=0`` returns full SHA."""
|
||||
from hermes_cli import build_info
|
||||
|
||||
sha_file = tmp_path / ".hermes_build_sha"
|
||||
full_sha = "abcdef1234567890abcdef1234567890abcdef12"
|
||||
sha_file.write_text(full_sha + "\n")
|
||||
|
||||
with patch.object(build_info, "_BUILD_SHA_FILE", sha_file):
|
||||
assert build_info.get_build_sha(short=12) == "abcdef123456"
|
||||
assert build_info.get_build_sha(short=0) == full_sha
|
||||
assert build_info.get_build_sha(short=-1) == full_sha
|
||||
|
||||
|
||||
def test_get_build_sha_strips_whitespace(tmp_path):
|
||||
"""The Dockerfile uses ``printf '%s\\n'`` — strip the trailing newline."""
|
||||
from hermes_cli import build_info
|
||||
|
||||
sha_file = tmp_path / ".hermes_build_sha"
|
||||
sha_file.write_text(" abcdef1234567890\n\n")
|
||||
|
||||
with patch.object(build_info, "_BUILD_SHA_FILE", sha_file):
|
||||
assert build_info.get_build_sha() == "abcdef12"
|
||||
|
||||
|
||||
def test_get_build_sha_returns_none_for_empty_file(tmp_path):
|
||||
"""A whitespace-only file is treated as absent."""
|
||||
from hermes_cli import build_info
|
||||
|
||||
sha_file = tmp_path / ".hermes_build_sha"
|
||||
sha_file.write_text(" \n\n")
|
||||
|
||||
with patch.object(build_info, "_BUILD_SHA_FILE", sha_file):
|
||||
assert build_info.get_build_sha() is None
|
||||
|
||||
|
||||
def test_get_build_sha_swallows_read_errors(tmp_path):
|
||||
"""Any IO exception from the read returns None — never raises."""
|
||||
from hermes_cli import build_info
|
||||
|
||||
sha_file = tmp_path / ".hermes_build_sha"
|
||||
sha_file.write_text("abcdef1234567890\n")
|
||||
|
||||
with patch.object(build_info, "_BUILD_SHA_FILE", sha_file), \
|
||||
patch.object(Path, "read_text", side_effect=OSError("boom")):
|
||||
assert build_info.get_build_sha() is None
|
||||
118
tests/hermes_cli/test_dump_git_commit.py
Normal file
118
tests/hermes_cli/test_dump_git_commit.py
Normal file
|
|
@ -0,0 +1,118 @@
|
|||
"""Tests for hermes_cli.dump._get_git_commit — git SHA resolution for ``hermes dump``.
|
||||
|
||||
``hermes dump`` prints the running commit so support bug reports identify the
|
||||
exact version. Source installs resolve it live via ``git rev-parse``; the
|
||||
published Docker image excludes ``.git`` and falls back to the baked SHA
|
||||
written by the Dockerfile's ``HERMES_GIT_SHA`` build-arg.
|
||||
|
||||
These tests cover both paths plus the failure modes (no git, no baked file).
|
||||
"""
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
|
||||
def test_get_git_commit_uses_live_git_when_available(tmp_path):
|
||||
"""Source install: ``git rev-parse --short=8 HEAD`` wins; no fallback."""
|
||||
from hermes_cli import dump
|
||||
|
||||
repo_dir = tmp_path / "repo"
|
||||
repo_dir.mkdir()
|
||||
|
||||
git_result = MagicMock(returncode=0, stdout="deadbeef\n")
|
||||
# build_info should NOT be consulted when live git succeeds.
|
||||
with patch("hermes_cli.dump.subprocess.run", return_value=git_result) as mock_run, \
|
||||
patch("hermes_cli.build_info.get_build_sha") as mock_build:
|
||||
commit = dump._get_git_commit(repo_dir)
|
||||
|
||||
assert commit == "deadbeef"
|
||||
mock_run.assert_called_once()
|
||||
mock_build.assert_not_called()
|
||||
|
||||
|
||||
def test_get_git_commit_falls_back_to_build_sha_when_live_git_fails(tmp_path):
|
||||
"""Docker image case: live git returns non-zero → use baked SHA."""
|
||||
from hermes_cli import dump
|
||||
|
||||
repo_dir = tmp_path / "no-git-here"
|
||||
repo_dir.mkdir()
|
||||
|
||||
failed = MagicMock(returncode=128, stdout="")
|
||||
with patch("hermes_cli.dump.subprocess.run", return_value=failed), \
|
||||
patch("hermes_cli.build_info.get_build_sha", return_value="cafef00d"):
|
||||
commit = dump._get_git_commit(repo_dir)
|
||||
|
||||
assert commit == "cafef00d"
|
||||
|
||||
|
||||
def test_get_git_commit_falls_back_when_git_returns_empty_stdout(tmp_path):
|
||||
"""Edge case: git exits 0 but prints nothing — still try the baked SHA."""
|
||||
from hermes_cli import dump
|
||||
|
||||
repo_dir = tmp_path / "repo"
|
||||
repo_dir.mkdir()
|
||||
|
||||
empty = MagicMock(returncode=0, stdout="\n")
|
||||
with patch("hermes_cli.dump.subprocess.run", return_value=empty), \
|
||||
patch("hermes_cli.build_info.get_build_sha", return_value="abcdef12"):
|
||||
commit = dump._get_git_commit(repo_dir)
|
||||
|
||||
assert commit == "abcdef12"
|
||||
|
||||
|
||||
def test_get_git_commit_falls_back_when_git_raises(tmp_path):
|
||||
"""git binary missing (e.g. minimal container w/o git) → baked SHA path."""
|
||||
from hermes_cli import dump
|
||||
|
||||
repo_dir = tmp_path / "repo"
|
||||
repo_dir.mkdir()
|
||||
|
||||
with patch("hermes_cli.dump.subprocess.run", side_effect=FileNotFoundError("git")), \
|
||||
patch("hermes_cli.build_info.get_build_sha", return_value="feedface"):
|
||||
commit = dump._get_git_commit(repo_dir)
|
||||
|
||||
assert commit == "feedface"
|
||||
|
||||
|
||||
def test_get_git_commit_returns_unknown_when_neither_source_available(tmp_path):
|
||||
"""Pip-installed wheel: no git, no baked SHA → '(unknown)' (legacy contract)."""
|
||||
from hermes_cli import dump
|
||||
|
||||
repo_dir = tmp_path / "repo"
|
||||
repo_dir.mkdir()
|
||||
|
||||
failed = MagicMock(returncode=128, stdout="")
|
||||
with patch("hermes_cli.dump.subprocess.run", return_value=failed), \
|
||||
patch("hermes_cli.build_info.get_build_sha", return_value=None):
|
||||
commit = dump._get_git_commit(repo_dir)
|
||||
|
||||
assert commit == "(unknown)"
|
||||
|
||||
|
||||
def test_get_git_commit_output_format_identical_between_sources(tmp_path):
|
||||
"""Regression guard: live-git and baked-SHA outputs share the same shape.
|
||||
|
||||
Ben explicitly asked for identical output between Docker and source installs
|
||||
so support tooling that parses ``hermes dump`` doesn't have to special-case
|
||||
container builds. Both paths must return a bare 8-char SHA — no prefix,
|
||||
no suffix, no annotation.
|
||||
"""
|
||||
from hermes_cli import dump
|
||||
|
||||
repo_dir = tmp_path / "repo"
|
||||
repo_dir.mkdir()
|
||||
|
||||
# Live-git path.
|
||||
git_result = MagicMock(returncode=0, stdout="b2f477a3\n")
|
||||
with patch("hermes_cli.dump.subprocess.run", return_value=git_result):
|
||||
live = dump._get_git_commit(repo_dir)
|
||||
|
||||
# Baked-SHA path.
|
||||
failed = MagicMock(returncode=128, stdout="")
|
||||
with patch("hermes_cli.dump.subprocess.run", return_value=failed), \
|
||||
patch("hermes_cli.build_info.get_build_sha", return_value="b2f477a3"):
|
||||
baked = dump._get_git_commit(repo_dir)
|
||||
|
||||
assert live == baked == "b2f477a3"
|
||||
# Same length, same charset — no decoration in either branch.
|
||||
assert len(live) == 8
|
||||
assert all(c in "0123456789abcdef" for c in live)
|
||||
Loading…
Add table
Add a link
Reference in a new issue