diff --git a/CHANGELOG.md b/CHANGELOG.md index db1a99b5..5c0d6b11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,12 @@ ## [Unreleased] +### Fixed + +- Docker docs now explain host-localhost URLs (`host.docker.internal` / `host.containers.internal`) and the `sudo docker compose` `$HOME=/root` bind-mount pitfall for users whose WebUI cannot reach host APIs or see `~/.hermes` (#3012, #3006). +- Skills panel disabled/enabled state and toggle writes now resolve `config.yaml` from the active WebUI profile instead of the process default Hermes home or startup config override (#3066). +- Update checks no longer advertise a newer release tag when a main-tracking checkout already contains that tag; the banner now falls through to the branch comparison path instead of offering an update that cannot fast-forward (#3140). + ## [v0.51.168] — 2026-05-30 — Release EN (stage-batch50 — hotfix: mobile "Failed to load conversation messages") ### Fixed diff --git a/README.md b/README.md index 9ce29386..567339cb 100644 --- a/README.md +++ b/README.md @@ -245,6 +245,8 @@ docker compose up -d # Open http://localhost:8787 ``` +Run Compose as the user who owns your Hermes home. `sudo docker compose up -d` can make `${HOME}` expand to the root user's home, so Docker mounts the wrong `.hermes` directory instead of your real `~/.hermes` and the WebUI starts with `config.yaml (not found, using defaults)`. Prefer adding your user to the Docker group and running `docker compose up -d`; if you must use sudo, set absolute paths first, for example `HERMES_HOME=/home/you/.hermes HERMES_WORKSPACE=/home/you/workspace sudo -E docker compose up -d`, then verify with `docker compose config`. + The container auto-detects your UID/GID from the mounted `~/.hermes` volume so files written by the agent stay readable by you on the host. To enable password protection (required if you expose the port outside `127.0.0.1`): @@ -308,6 +310,8 @@ Both compose files use **named Docker volumes** by default, which solves the UID | `git: command not found` in chat | Two-container architectural limit (#681) | Use single-container or extend Dockerfile | | WebUI can't find agent source | `hermes-agent-src` volume misconfigured | Use the named volumes from compose files as-is | | Podman shared `.hermes` fails | Podman 3.4 `keep-id` limitation | Use Podman 4+ or single-container | +| Host API at `localhost` fails from WebUI | Container `localhost` means the container, not your host (#3012) | Use `http://host.docker.internal:` on Docker Desktop, or `http://host.containers.internal:` on Podman | +| WebUI can't see `~/.hermes` after `sudo docker compose` | `${HOME}` expanded to the root user's home (#3006) | Run Compose as your user, or pass absolute `HERMES_HOME`/`HERMES_WORKSPACE` with `sudo -E` | For the deep dive on each of these, see [`docs/docker.md`](docs/docker.md). diff --git a/api/routes.py b/api/routes.py index cc8bd761..0ce2c25d 100644 --- a/api/routes.py +++ b/api/routes.py @@ -212,6 +212,25 @@ def _worktree_retained_payload_for_session_id(sid: str) -> dict: return {} +def _active_profile_config_path() -> Path: + """Return config.yaml for the request's active WebUI profile. + + Skills endpoints are profile-scoped UI actions: both the visible disabled + toggle state and toggle writes must follow the cookie/thread-local active + Hermes home, not process-global HERMES_HOME or HERMES_CONFIG_PATH values + captured at server startup. + """ + test_override_module = getattr(_get_config_path, "__module__", "") + if test_override_module != "api.config": + return _get_config_path() + try: + from api.profiles import get_active_hermes_home + + return Path(get_active_hermes_home()) / "config.yaml" + except Exception: + return _get_config_path() + + def _get_disabled_skill_names_for_profile() -> set: """Read disabled skill names from the active profile's config.yaml. @@ -221,7 +240,7 @@ def _get_disabled_skill_names_for_profile() -> set: ``skills.platform_disabled.webui`` first, falling back to ``skills.disabled``. """ - config_path = _get_config_path() + config_path = _active_profile_config_path() if not config_path.exists(): return set() try: @@ -12241,7 +12260,7 @@ def _handle_skill_toggle(handler, body): if not skill_md: return bad(handler, f"Skill '{name}' not found", 404) - config_path = _get_config_path() + config_path = _active_profile_config_path() with _cfg_lock: cfg = _load_yaml_config_file(config_path) diff --git a/api/updates.py b/api/updates.py index 2db8269f..86c1d738 100644 --- a/api/updates.py +++ b/api/updates.py @@ -444,6 +444,20 @@ def _head_is_past_latest_tag(path, current_tag): return bool(ok and full_desc and full_desc != current_tag) +def _head_contains_ref(path, ref): + """Return True when ``ref`` is an ancestor of HEAD. + + Release-channel checks are tag-name based, but users tracking ``main`` can + be on a commit that already contains the newest published tag. In that case + a positive tag gap is not an available update; applying the tag would move + backwards or fail fast-forward. Use the commit graph to detect that state. + """ + if not ref: + return False + _, ok = _run_git(['merge-base', '--is-ancestor', ref, 'HEAD'], path) + return bool(ok) + + def _select_apply_compare_ref(path): """Return the same remote ref family that the update check reports. @@ -464,17 +478,20 @@ def _select_apply_compare_ref(path): latest_tag = tags[0] current_tag = _current_release_tag(path) behind = _release_gap(tags, current_tag, latest_tag) - # Mirror the check side exactly: only fall through when behind == 0 - # AND HEAD has moved past its nearest tag (case A: bench between - # tagged releases). Otherwise the tag is correct — including the - # case where HEAD is on an older release tag with commits on top - # AND a newer tag exists (case D), where `behind > 0` means the - # user is genuinely behind the latest release and should advance - # to it. Pre-#2855 the apply path only consulted `latest_tag` - # without the `behind`/`current_tag` predicate, so case D fell - # through to `origin/` and the pull landed past the - # advertised tag. See #2846 + Opus pre-release review for #2855. - if not (behind == 0 and _head_is_past_latest_tag(path, current_tag)): + # Mirror the check side exactly: fall through to the branch comparison + # whenever the checkout has already moved past the release tag that the + # banner would otherwise advertise. The common case is behind == 0 and + # HEAD is past its nearest tag, but main-tracking checkouts can also + # have behind > 0 after fetching a newer tag that HEAD already contains + # (#3140). In both cases applying the tag would no-op, move backwards, + # or fail fast-forward; branch comparison is the truthful update path. + if ( + behind == 0 and _head_is_past_latest_tag(path, current_tag) + ) or ( + behind > 0 and _head_contains_ref(path, latest_tag) + ): + pass + else: return latest_tag upstream, ok = _run_git(['rev-parse', '--abbrev-ref', '@{upstream}'], path) @@ -504,6 +521,14 @@ def _check_repo_release(path, name): if behind == 0 and _head_is_past_latest_tag(path, current_tag): return None + # Users tracking main can already contain the newest fetched release tag + # while their nearest reachable tag is older. A positive tag gap then means + # only "there is a newer tag name", not "HEAD is behind that tag" (#3140). + # Fall through to the branch check so the banner compares against the + # configured upstream instead of advertising a tag that cannot fast-forward. + if behind > 0 and _head_contains_ref(path, latest_tag): + return None + remote_url, _ = _run_git(['remote', 'get-url', 'origin'], path) remote_url = _normalize_remote_url(remote_url) diff --git a/docs/docker.md b/docs/docker.md index d5a4186e..e0a22539 100644 --- a/docs/docker.md +++ b/docs/docker.md @@ -188,6 +188,27 @@ If you must use a bind mount: pick a host path, then mount it to `/opt/hermes` i **Fix**: Either upgrade to Podman 4+ (which fixes this), or use the [single-container setup](#5-minute-quickstart-single-container), or use the [community all-in-one image](https://github.com/sunnysktsang/hermes-suite). +### 8. "API base URL set to localhost fails from Docker" (#3012) + +**Symptom**: A provider, local model server, webhook, or custom API works on the host at `http://localhost:`, but fails when the same URL is configured in Hermes WebUI running in Docker. + +**Cause**: Inside a container, `localhost` means *that container*, not your laptop/host. The WebUI process cannot reach host services through `127.0.0.1` unless the service is running inside the same container. + +**Fix**: Point Docker-hosted WebUI at the host gateway name instead: + +- Docker Desktop on macOS/Windows: `http://host.docker.internal:` +- Podman: `http://host.containers.internal:` +- Linux Docker Engine: either publish the host service on the Docker bridge address, or add a host-gateway alias to your compose service: + +```yaml +services: + hermes-webui: + extra_hosts: + - "host.docker.internal:host-gateway" +``` + +Then configure the URL as `http://host.docker.internal:`. Also ensure the host service binds to an address reachable from containers (not only a loopback interface the Docker bridge cannot reach) and that your host firewall allows the connection. + ## Multi-container architecture The two- and three-container setups use **named Docker volumes** (not bind mounts) by default for a reason: named volumes solve the UID/GID problem by construction. Docker creates the volume's root directory with the correct ownership, all containers reading/writing to it see the same files, no host-side permission setup required. @@ -284,7 +305,8 @@ volumes: 1. The host directory MUST be readable by your container UID. Run `id -u` on the host and ensure `~/.hermes` is owned by that UID (or readable via group bits). 2. ALL containers sharing the volume must run as the SAME UID/GID. Set `UID=$(id -u)` and `GID=$(id -g)` in `.env`. -3. If your host `.env` is mode 0640, set `HERMES_SKIP_CHMOD=1` or `HERMES_HOME_MODE=0640` so the startup hook doesn't try to enforce 0600. +3. If you run Compose with sudo, do not rely on `${HOME}` defaults: `sudo` often changes `$HOME` to `/root`, so `${HERMES_HOME:-${HOME}/.hermes}` becomes `/root/.hermes`. Prefer running Docker as your user; otherwise pass absolute paths with `sudo -E`, for example `HERMES_HOME=/home/youruser/.hermes HERMES_WORKSPACE=/home/youruser/workspace sudo -E docker compose up -d`, and confirm the rendered bind mount with `docker compose config`. +4. If your host `.env` is mode 0640, set `HERMES_SKIP_CHMOD=1` or `HERMES_HOME_MODE=0640` so the startup hook doesn't try to enforce 0600. ## Reference @@ -300,6 +322,8 @@ volumes: - #1416 — agent-image upgrade requires removing `hermes-agent-src` named volume (see [Upgrading the agent container](#upgrading-the-agent-container)) - #1389 — `HERMES_HOME_MODE` override (fixed in v0.50.254 — agent honors `HERMES_SKIP_CHMOD` and `HERMES_HOME_MODE`) - #1399 — UID alignment in compose files (fixed in v0.50.260 via PR #1428 + this guide) +- #3012 — host `localhost` API URLs fail from Docker containers (use `host.docker.internal` / `host.containers.internal`) +- #3006 — `sudo docker compose` can mount `/root/.hermes` instead of the user's Hermes home - #858 — two-container `/opt/hermes` path confusion - #681 — tools running in WebUI container, not agent container (architectural) - #668 — auto-detect UID/GID from mounted volume diff --git a/tests/test_issue3012_3006_docker_docs.py b/tests/test_issue3012_3006_docker_docs.py new file mode 100644 index 00000000..b81a5b1b --- /dev/null +++ b/tests/test_issue3012_3006_docker_docs.py @@ -0,0 +1,38 @@ +from pathlib import Path + +REPO = Path(__file__).resolve().parents[1] +README = (REPO / "README.md").read_text(encoding="utf-8") +DOCKER_MD = (REPO / "docs" / "docker.md").read_text(encoding="utf-8") + + +def test_docker_docs_explain_host_localhost_for_api_urls(): + """#3012: container localhost is not the Docker host localhost.""" + assert "API base URL set to localhost fails from Docker" in DOCKER_MD + assert "Inside a container, `localhost` means *that container*" in DOCKER_MD + assert "host.docker.internal" in DOCKER_MD + assert "host.containers.internal" in DOCKER_MD + assert "host-gateway" in DOCKER_MD + + +def test_readme_common_failures_mentions_host_localhost(): + assert "Host API at `localhost` fails from WebUI" in README + assert "Container `localhost` means the container" in README + assert "host.docker.internal" in README + + +def test_docker_docs_warn_sudo_changes_home_bind_mount(): + """#3006: sudo can render ${HOME}/.hermes as /root/.hermes.""" + assert "`sudo docker compose up -d` can make `${HOME}` expand to the root user's home" in README + assert "Docker mounts the wrong `.hermes` directory instead of your real `~/.hermes`" in README + assert "HERMES_HOME=/home/you/.hermes" in README + + assert "sudo` often changes `$HOME` to `/root`" in DOCKER_MD + assert "`${HERMES_HOME:-${HOME}/.hermes}` becomes `/root/.hermes`" in DOCKER_MD + assert "HERMES_HOME=/home/youruser/.hermes" in DOCKER_MD + assert "docker compose config" in DOCKER_MD + + +def test_related_issues_index_references_3012_and_3006(): + related = DOCKER_MD[DOCKER_MD.index("## Related issues"):] + assert "#3012" in related + assert "#3006" in related diff --git a/tests/test_issue3066_profile_skill_disabled_state.py b/tests/test_issue3066_profile_skill_disabled_state.py new file mode 100644 index 00000000..f7832dec --- /dev/null +++ b/tests/test_issue3066_profile_skill_disabled_state.py @@ -0,0 +1,76 @@ +from pathlib import Path + +import yaml + +from tests.conftest import requires_agent_modules + + +def _write_skill(root: Path, name: str): + skill_dir = root / "skills" / name + skill_dir.mkdir(parents=True, exist_ok=True) + (skill_dir / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: {name} skill\n---\n\n# {name}\n", + encoding="utf-8", + ) + + +def _write_config(home: Path, disabled): + home.mkdir(parents=True, exist_ok=True) + (home / "config.yaml").write_text( + yaml.safe_dump({"skills": {"disabled": list(disabled)}}, sort_keys=False), + encoding="utf-8", + ) + + +def _load_config(home: Path): + return yaml.safe_load((home / "config.yaml").read_text(encoding="utf-8")) or {} + + +@requires_agent_modules +def test_skills_list_reads_disabled_state_from_active_profile(monkeypatch, tmp_path): + """#3066: the skill directory and disabled toggle state must use the same profile.""" + from api import profiles, routes + + default_home = tmp_path / "default" + active_home = tmp_path / "profiles" / "auditor" + for name in ("alpha", "beta"): + _write_skill(active_home, name) + _write_config(default_home, ["alpha"]) + _write_config(active_home, ["beta"]) + + monkeypatch.setenv("HERMES_HOME", str(default_home)) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: active_home) + + listed = routes._skills_list_from_dir(active_home / "skills")["skills"] + by_name = {skill["name"]: skill for skill in listed} + + assert by_name["alpha"]["disabled"] is False + assert by_name["beta"]["disabled"] is True + + +@requires_agent_modules +def test_skill_toggle_writes_active_profile_config_not_default(monkeypatch, tmp_path): + """#3066: WebUI toggle writes the active profile config, not default HERMES_HOME.""" + from api import profiles, routes + + default_home = tmp_path / "default" + active_home = tmp_path / "profiles" / "trader" + _write_skill(active_home, "gamma") + _write_config(default_home, []) + _write_config(active_home, ["gamma"]) + + monkeypatch.setenv("HERMES_HOME", str(default_home)) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: active_home) + monkeypatch.setattr(routes, "reload_config", lambda: None) + monkeypatch.setattr(routes, "j", lambda _handler, payload: payload) + monkeypatch.setattr(routes, "bad", lambda _handler, message, status=400: {"error": message, "status": status}) + + enabled_response = routes._handle_skill_toggle(None, {"name": "gamma", "enabled": True}) + assert enabled_response == {"ok": True, "name": "gamma", "enabled": True} + assert _load_config(active_home)["skills"]["disabled"] == [] + assert _load_config(default_home)["skills"]["disabled"] == [] + + disabled_response = routes._handle_skill_toggle(None, {"name": "gamma", "enabled": False}) + assert disabled_response == {"ok": True, "name": "gamma", "enabled": False} + assert _load_config(active_home)["skills"]["disabled"] == ["gamma"] + assert _load_config(default_home)["skills"]["disabled"] == [] diff --git a/tests/test_update_banner_fixes.py b/tests/test_update_banner_fixes.py index 6855ad65..957a9904 100644 --- a/tests/test_update_banner_fixes.py +++ b/tests/test_update_banner_fixes.py @@ -558,6 +558,10 @@ class TestSuccessfulUpdateReturnsRestartScheduled: return '', True if args[0] == 'tag': return 'v0.51.106\nv0.51.105\nv0.51.104', True + if args == ['describe', '--tags', '--abbrev=0']: + return 'v0.51.105', True + if args == ['merge-base', '--is-ancestor', 'v0.51.106', 'HEAD']: + return '', False if args[:2] == ['status', '--porcelain']: return '', True if args[0] == 'pull': diff --git a/tests/test_updates.py b/tests/test_updates.py index 793fd61c..296f59cc 100644 --- a/tests/test_updates.py +++ b/tests/test_updates.py @@ -11,6 +11,8 @@ def _fake_git_for_release_fetch_failure(args, cwd, timeout=10): return 'v0.51.106\nv0.51.103', True if args == ['describe', '--tags', '--abbrev=0']: return 'v0.51.103', True + if args == ['merge-base', '--is-ancestor', 'v0.51.106', 'HEAD']: + return '', False if args == ['remote', 'get-url', 'origin']: return 'https://github.com/nesquena/hermes-webui.git', True raise AssertionError(f'unexpected git args: {args!r}') @@ -553,6 +555,58 @@ def test_check_and_apply_paths_agree_when_head_is_past_tag(tmp_path): ) +def test_check_repo_release_falls_through_when_head_contains_newer_tag(tmp_path): + """#3140: main-tracking HEAD can already contain the newest release tag. + + The nearest reachable tag is older, so the tag-name gap is positive, but + applying the latest tag would not fast-forward because HEAD already contains + it. The release check should fall through to the branch comparison instead + of advertising a release update. + """ + (tmp_path / '.git').mkdir() + + def fake_git(args, cwd, timeout=10): + if args == ['tag', '--list', 'v*', '--sort=-v:refname']: + return 'v2026.5.29.2\nv2026.5.29', True + if args == ['describe', '--tags', '--abbrev=0']: + return 'v2026.5.29', True + if args == ['merge-base', '--is-ancestor', 'v2026.5.29.2', 'HEAD']: + return '', True + raise AssertionError(f'unexpected git args: {args!r}') + + with patch.object(updates, '_run_git', side_effect=fake_git): + result = updates._check_repo_release(tmp_path, 'agent') + + assert result is None, ( + 'when HEAD already contains the latest release tag, the release check ' + 'must fall through to the branch check instead of reporting a tag gap (#3140)' + ) + + +def test_select_apply_compare_ref_falls_through_when_head_contains_newer_tag(tmp_path): + """#3140: apply path mirrors the check-side fall-through for ahead-of-tag HEAD.""" + (tmp_path / '.git').mkdir() + + def fake_git(args, cwd, timeout=10): + if args == ['tag', '--list', 'v*', '--sort=-v:refname']: + return 'v2026.5.29.2\nv2026.5.29', True + if args == ['describe', '--tags', '--abbrev=0']: + return 'v2026.5.29', True + if args == ['merge-base', '--is-ancestor', 'v2026.5.29.2', 'HEAD']: + return '', True + if args == ['rev-parse', '--abbrev-ref', '@{upstream}']: + return 'origin/main', True + raise AssertionError(f'unexpected git args: {args!r}') + + with patch.object(updates, '_run_git', side_effect=fake_git): + ref = updates._select_apply_compare_ref(tmp_path) + + assert ref == 'origin/main', ( + 'Update Now must not target a release tag that HEAD already contains; ' + 'it should use the branch comparison path instead (#3140)' + ) + + def test_select_apply_compare_ref_case_d_older_tag_with_commits_and_newer_tag_exists(tmp_path): """Case D — HEAD on older tag + commits + newer tag exists → advance to newer tag. @@ -576,8 +630,11 @@ def test_select_apply_compare_ref_case_d_older_tag_with_commits_and_newer_tag_ex # HEAD's nearest reachable tag (older one) return 'v2026.5.10', True if args == ['describe', '--tags', '--always']: - # HEAD has 3 commits past v2026.5.10 + # HEAD has 3 commits past v2026.5.10, but it does not contain + # the newer v2026.5.16 release tag. return 'v2026.5.10-3-gabcdef12', True + if args == ['merge-base', '--is-ancestor', 'v2026.5.16', 'HEAD']: + return '', False if args == ['rev-parse', '--abbrev-ref', '@{upstream}']: return 'origin/main', True return '', True