fix(cli): don't treat any container as the Docker image for updates (#35139)
detect_install_method() returned "docker" for any container (is_container()), before the .git check. Both supported installs already self-identify via the .install_method stamp read first: the curl installer (scripts/install.sh) git-clones and stamps "git"; the published nousresearch/hermes-agent image stamps "docker" at boot via docker/stage2-hook.sh. An unsupported manual install dropped into a container has no stamp, so the bare container check hijacked it to "docker" and 'hermes update' bailed with the docker-pull guidance. Drop the redundant is_container() -> docker fallback. Unstamped installs now fall through to the .git/pip checks like any off-path install; both supported paths are unaffected because the stamp wins first. Fixes #34397.
This commit is contained in:
@ -285,9 +285,22 @@ def detect_install_method(project_root: Optional[Path] = None) -> str:
|
||||
Resolution order:
|
||||
1. Stamped ``~/.hermes/.install_method`` file (written by installers)
|
||||
2. HERMES_MANAGED env / .managed marker (NixOS, Homebrew)
|
||||
3. Container detection (/.dockerenv, /run/.containerenv, cgroup)
|
||||
4. .git directory presence -> 'git'
|
||||
5. Fallback -> 'pip'
|
||||
3. .git directory presence -> 'git'
|
||||
4. Fallback -> 'pip'
|
||||
|
||||
Note: running inside a container is NOT treated as "docker" on its own.
|
||||
The two supported install paths both self-identify via the
|
||||
``.install_method`` stamp (caught by step 1), so neither relies on
|
||||
container detection here:
|
||||
- the curl installer (scripts/install.sh, the README/website install
|
||||
command) git-clones the repo and stamps ``git``;
|
||||
- the published ``nousresearch/hermes-agent`` image stamps ``docker``
|
||||
at boot via ``docker/stage2-hook.sh``.
|
||||
An unsupported manual install dropped into a container (no stamp) was
|
||||
wrongly classified as the published image by bare container detection,
|
||||
so ``hermes update`` bailed with "doesn't apply inside the Docker
|
||||
container". Without that fallback such installs fall through to the
|
||||
``.git``/pip checks and behave like any off-path install. See issue #34397.
|
||||
"""
|
||||
stamp = get_hermes_home() / ".install_method"
|
||||
try:
|
||||
@ -299,9 +312,6 @@ def detect_install_method(project_root: Optional[Path] = None) -> str:
|
||||
managed = get_managed_system()
|
||||
if managed:
|
||||
return managed.lower().replace(" ", "-")
|
||||
from hermes_constants import is_container
|
||||
if is_container():
|
||||
return "docker"
|
||||
if project_root is None:
|
||||
project_root = Path(__file__).parent.parent.resolve()
|
||||
if (project_root / ".git").is_dir():
|
||||
|
||||
@ -48,12 +48,32 @@ def test_stamp_file_takes_precedence(tmp_path):
|
||||
assert detect_install_method(project_root=tmp_path) == "docker"
|
||||
|
||||
|
||||
def test_docker_detected_via_dockerenv(tmp_path):
|
||||
def test_container_without_stamp_is_not_docker(tmp_path):
|
||||
"""An unstamped install in a generic container must NOT be flagged as docker.
|
||||
|
||||
Regression for issue #34397. The two supported installs both stamp
|
||||
``.install_method`` (the curl installer -> ``git``, covered by
|
||||
``test_stamp_file_takes_precedence``; the published image -> ``docker``),
|
||||
so neither hits this path. An unsupported manual install dropped into a
|
||||
container has no stamp and was wrongly classified as the published Docker
|
||||
image, so ``hermes update`` refused to run. With a ``.git`` checkout it
|
||||
must resolve to ``git``.
|
||||
"""
|
||||
(tmp_path / ".git").mkdir()
|
||||
with patch("hermes_cli.config.get_managed_system", return_value=None), \
|
||||
patch("hermes_cli.config.get_hermes_home", return_value=tmp_path), \
|
||||
patch("hermes_constants.is_container", return_value=True):
|
||||
from hermes_cli.config import detect_install_method
|
||||
assert detect_install_method(project_root=tmp_path) == "docker"
|
||||
assert detect_install_method(project_root=tmp_path) == "git"
|
||||
|
||||
|
||||
def test_container_pip_install_without_stamp_is_pip(tmp_path):
|
||||
"""Container + no .git + no stamp -> pip, not docker (issue #34397)."""
|
||||
with patch("hermes_cli.config.get_managed_system", return_value=None), \
|
||||
patch("hermes_cli.config.get_hermes_home", return_value=tmp_path), \
|
||||
patch("hermes_constants.is_container", return_value=True):
|
||||
from hermes_cli.config import detect_install_method
|
||||
assert detect_install_method(project_root=tmp_path) == "pip"
|
||||
|
||||
|
||||
def test_recommended_update_command_docker():
|
||||
|
||||
Reference in New Issue
Block a user