fix(docker): close the cold-container and multi-backend gaps in attachment delivery
Follow-ups on the salvaged commit: 1. get_cache_directory_mounts() now CREATES missing staging dirs instead of skipping them. Docker snapshots the mount list at container creation, so a dir born later (first attachment, first clipboard image) dangled for the life of a persistent container. Empty bind mount costs nothing. 2. to_agent_visible_cache_path() translates per-backend instead of docker-only: docker/modal -> /root/.hermes, ssh/daytona/vercel_sandbox -> ~/.hermes (shell-expanded remotely; bytes arrive via file sync), local/ singularity keep the host path (apptainer auto-binds the host home). Mirrors the proven _agent_cache_base_for_env heuristics. Updated the two mount-list tests pinning the old skip behavior; added per-backend translation coverage.
This commit is contained in:
Teknium
84
package-lock.json
generated
84
package-lock.json
generated
@@ -1798,6 +1798,7 @@
|
||||
"os": [
|
||||
"aix"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -1814,6 +1815,7 @@
|
||||
"os": [
|
||||
"android"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -1830,6 +1832,7 @@
|
||||
"os": [
|
||||
"android"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -1846,6 +1849,7 @@
|
||||
"os": [
|
||||
"android"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -1862,6 +1866,7 @@
|
||||
"os": [
|
||||
"darwin"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -1878,6 +1883,7 @@
|
||||
"os": [
|
||||
"darwin"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -1894,6 +1900,7 @@
|
||||
"os": [
|
||||
"freebsd"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -1910,6 +1917,7 @@
|
||||
"os": [
|
||||
"freebsd"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -1926,6 +1934,7 @@
|
||||
"os": [
|
||||
"linux"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -1942,6 +1951,7 @@
|
||||
"os": [
|
||||
"linux"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -1958,6 +1968,7 @@
|
||||
"os": [
|
||||
"linux"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -1974,6 +1985,7 @@
|
||||
"os": [
|
||||
"linux"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -1990,6 +2002,7 @@
|
||||
"os": [
|
||||
"linux"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -2006,6 +2019,7 @@
|
||||
"os": [
|
||||
"linux"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -2022,6 +2036,7 @@
|
||||
"os": [
|
||||
"linux"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -2038,6 +2053,7 @@
|
||||
"os": [
|
||||
"linux"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -2054,6 +2070,7 @@
|
||||
"os": [
|
||||
"linux"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -2070,6 +2087,7 @@
|
||||
"os": [
|
||||
"netbsd"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -2086,6 +2104,7 @@
|
||||
"os": [
|
||||
"netbsd"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -2102,6 +2121,7 @@
|
||||
"os": [
|
||||
"openbsd"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -2118,6 +2138,7 @@
|
||||
"os": [
|
||||
"openbsd"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -2134,6 +2155,7 @@
|
||||
"os": [
|
||||
"openharmony"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -2150,6 +2172,7 @@
|
||||
"os": [
|
||||
"sunos"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -2166,6 +2189,7 @@
|
||||
"os": [
|
||||
"win32"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -2182,6 +2206,7 @@
|
||||
"os": [
|
||||
"win32"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -2198,6 +2223,7 @@
|
||||
"os": [
|
||||
"win32"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=18"
|
||||
}
|
||||
@@ -4709,9 +4735,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -4728,9 +4751,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -4747,9 +4767,6 @@
|
||||
"cpu": [
|
||||
"ppc64"
|
||||
],
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -4766,9 +4783,6 @@
|
||||
"cpu": [
|
||||
"s390x"
|
||||
],
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -4785,9 +4799,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -4804,9 +4815,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -5295,9 +5303,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -5314,9 +5319,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -5333,9 +5335,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -5352,9 +5351,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -5603,9 +5599,6 @@
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "Apache-2.0 OR MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -5623,9 +5616,6 @@
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "Apache-2.0 OR MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -5643,9 +5633,6 @@
|
||||
"riscv64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "Apache-2.0 OR MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -5663,9 +5650,6 @@
|
||||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "Apache-2.0 OR MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -5683,9 +5667,6 @@
|
||||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "Apache-2.0 OR MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -12196,9 +12177,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -12219,9 +12197,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -12242,9 +12217,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -12265,9 +12237,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -16534,6 +16503,7 @@
|
||||
"os": [
|
||||
"darwin"
|
||||
],
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": "^8.16.0 || ^10.6.0 || >=11.0.0"
|
||||
}
|
||||
|
||||
@@ -355,12 +355,22 @@ class TestCacheDirectoryMounts:
|
||||
assert "/root/.hermes/cache/images" in container_paths
|
||||
|
||||
def test_empty_hermes_home(self, tmp_path, monkeypatch):
|
||||
"""No cache dirs → empty list."""
|
||||
"""Empty home → every staging dir is created and mounted (#76577).
|
||||
|
||||
Docker snapshots the mount list at container creation; skipping
|
||||
not-yet-existing dirs meant the first attachment/clipboard file after
|
||||
container start dangled forever. All _CACHE_DIRS entries mount."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
assert get_cache_directory_mounts() == []
|
||||
mounts = get_cache_directory_mounts()
|
||||
container_paths = {m["container_path"] for m in mounts}
|
||||
assert "/root/.hermes/attachments" in container_paths
|
||||
assert "/root/.hermes/images" in container_paths
|
||||
assert "/root/.hermes/cache/images" in container_paths
|
||||
for mount in mounts:
|
||||
assert Path(mount["host_path"]).is_dir()
|
||||
|
||||
def test_images_upload_dir_is_mounted(self, tmp_path, monkeypatch):
|
||||
"""The flat top-level ``images/`` upload dir is mounted (#69575).
|
||||
@@ -412,12 +422,55 @@ class TestMapCachePathToContainer:
|
||||
)
|
||||
|
||||
|
||||
def test_returns_none_when_no_cache_dirs_exist(self, tmp_path, monkeypatch):
|
||||
def test_maps_path_even_when_cache_dir_missing(self, tmp_path, monkeypatch):
|
||||
"""Missing staging dirs are auto-created at mount-list time (#76577):
|
||||
Docker snapshots mounts at container creation, so a dir that appears
|
||||
later would dangle for the container's whole life. The map must
|
||||
therefore succeed (and the dir exist) even before first use."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
assert map_cache_path_to_container(str(hermes_home / "cache" / "images" / "x.png")) is None
|
||||
mapped = map_cache_path_to_container(str(hermes_home / "cache" / "images" / "x.png"))
|
||||
assert mapped == "/root/.hermes/cache/images/x.png"
|
||||
assert (hermes_home / "cache" / "images").is_dir()
|
||||
|
||||
|
||||
class TestToAgentVisiblePathPerBackend:
|
||||
"""#76577 follow-up: translation covers every backend that relocates the
|
||||
Hermes cache — not just docker — and skips the ones where the host path
|
||||
stays correct (local; singularity auto-binds the host home)."""
|
||||
|
||||
def _staged(self, tmp_path, monkeypatch):
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
(hermes_home / "attachments").mkdir(parents=True)
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
return str(hermes_home / "attachments" / "drop.zip")
|
||||
|
||||
def test_docker_maps_to_root_hermes(self, tmp_path, monkeypatch):
|
||||
staged = self._staged(tmp_path, monkeypatch)
|
||||
monkeypatch.setenv("TERMINAL_ENV", "docker")
|
||||
from tools.credential_files import to_agent_visible_cache_path
|
||||
assert to_agent_visible_cache_path(staged) == "/root/.hermes/attachments/drop.zip"
|
||||
|
||||
def test_ssh_maps_to_tilde_hermes(self, tmp_path, monkeypatch):
|
||||
staged = self._staged(tmp_path, monkeypatch)
|
||||
monkeypatch.setenv("TERMINAL_ENV", "ssh")
|
||||
from tools.credential_files import to_agent_visible_cache_path
|
||||
assert to_agent_visible_cache_path(staged) == "~/.hermes/attachments/drop.zip"
|
||||
|
||||
@pytest.mark.parametrize("backend", ["local", "singularity", ""])
|
||||
def test_untranslated_backends_keep_host_path(self, tmp_path, monkeypatch, backend):
|
||||
staged = self._staged(tmp_path, monkeypatch)
|
||||
monkeypatch.setenv("TERMINAL_ENV", backend)
|
||||
from tools.credential_files import to_agent_visible_cache_path
|
||||
assert to_agent_visible_cache_path(staged) == staged
|
||||
|
||||
def test_non_cache_path_passes_through(self, tmp_path, monkeypatch):
|
||||
self._staged(tmp_path, monkeypatch)
|
||||
monkeypatch.setenv("TERMINAL_ENV", "docker")
|
||||
from tools.credential_files import to_agent_visible_cache_path
|
||||
assert to_agent_visible_cache_path("/etc/hosts") == "/etc/hosts"
|
||||
|
||||
|
||||
class TestIterCacheFiles:
|
||||
|
||||
@@ -422,13 +422,25 @@ def get_cache_directory_mounts(
|
||||
mounts: List[Dict[str, str]] = []
|
||||
for new_subpath, old_name in _CACHE_DIRS:
|
||||
host_dir = get_hermes_dir(new_subpath, old_name)
|
||||
if host_dir.is_dir():
|
||||
# Always map to the *new* container layout regardless of host layout.
|
||||
container_path = f"{container_base.rstrip('/')}/{new_subpath}"
|
||||
mounts.append({
|
||||
"host_path": str(host_dir),
|
||||
"container_path": container_path,
|
||||
})
|
||||
if not host_dir.is_dir():
|
||||
# Create missing staging dirs instead of skipping them: Docker
|
||||
# snapshots this mount list at container CREATION, so a dir that
|
||||
# appears later (first desktop attachment, first clipboard image)
|
||||
# would dangle for the whole life of a persistent container
|
||||
# (#76577). An empty bind-mounted dir costs nothing; a missing
|
||||
# mount costs the feature. get_hermes_dir() already resolved
|
||||
# new-vs-legacy layout, so creating its answer cannot shadow a
|
||||
# populated legacy dir.
|
||||
try:
|
||||
host_dir.mkdir(parents=True, exist_ok=True)
|
||||
except OSError:
|
||||
continue # unwritable home (tests, RO mounts) — skip as before
|
||||
# Always map to the *new* container layout regardless of host layout.
|
||||
container_path = f"{container_base.rstrip('/')}/{new_subpath}"
|
||||
mounts.append({
|
||||
"host_path": str(host_dir),
|
||||
"container_path": container_path,
|
||||
})
|
||||
return mounts
|
||||
|
||||
|
||||
@@ -488,14 +500,33 @@ def to_agent_visible_cache_path(
|
||||
|
||||
Returns the input unchanged if it is not under any auto-mounted cache
|
||||
directory, or if the active terminal backend does not require path
|
||||
translation (only Docker for now).
|
||||
translation (local).
|
||||
|
||||
Per-backend base (mirrors ``_agent_cache_base_for_env`` in
|
||||
tools/image_generation_tool.py, the proven heuristics for where each
|
||||
backend's Hermes cache lands):
|
||||
|
||||
* docker / modal — bind-mounted (docker) or per-file-synced (modal) at
|
||||
``/root/.hermes`` (the *container_base* default).
|
||||
* ssh / daytona / vercel_sandbox — file-synced under the remote user's
|
||||
home; ``~/.hermes`` is shell-expanded by the remote shell, so tool
|
||||
commands resolve it regardless of the actual remote home. Previously
|
||||
these backends synced the bytes but still rendered the dangling host
|
||||
path (#76577 gap).
|
||||
* singularity — NOT translated: Apptainer auto-binds the host home, so
|
||||
the host path is directly readable and translation would dangle
|
||||
(cache dirs are not remapped into that sandbox).
|
||||
|
||||
Backend is identified by TERMINAL_ENV (same env var
|
||||
tools/terminal_tool.py reads in _get_environment_config).
|
||||
"""
|
||||
# Only Docker backend requires translation at this time. Other backends
|
||||
# (Modal, Daytona, Vercel) use different mount semantics and will be
|
||||
# addressed separately if needed. Backend is identified by TERMINAL_ENV
|
||||
# (same env var tools/terminal_tool.py reads in _get_environment_config).
|
||||
if os.environ.get("TERMINAL_ENV", "local") != "docker":
|
||||
return host_path
|
||||
backend = (os.environ.get("TERMINAL_ENV") or "local").strip().lower()
|
||||
if backend in ("docker", "modal"):
|
||||
pass # /root/.hermes default
|
||||
elif backend in ("ssh", "daytona", "vercel_sandbox"):
|
||||
container_base = "~/.hermes"
|
||||
else:
|
||||
return host_path # local, singularity, unknown: host path is correct
|
||||
|
||||
mapped = map_cache_path_to_container(host_path, container_base=container_base)
|
||||
return mapped if mapped is not None else host_path
|
||||
|
||||
Reference in New Issue
Block a user