From 823385e5cb6d07e04e855db5376a91f3dc2e4056 Mon Sep 17 00:00:00 2001 From: finn763 <165816600+finn763@users.noreply.github.com> Date: Tue, 29 Sep 2026 17:00:46 +0800 Subject: [PATCH] fix(agent): keep generated images/videos out of swept media caches (Closes #126445) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated image/video output now writes under cache/generated/images|videos instead of the swept inbound caches (cache/images, cache/videos). The hourly gateway housekeeping deletes 24h-old files in those dirs; for base64-returning providers the swept file was the only copy, breaking transcript references, the Artifacts panel and later edits. Rebased onto current main, where the writers delegate to agent/provider_media, so the media kind is now the generated/ subpath. cache/generated is added to tools/credential_files._CACHE_DIRS: without a mount/sync entry the Docker, SSH and Modal backends never receive the bytes and the provider result loses agent_visible_image. All callers consume the returned absolute paths, so no call-site changes were needed; inbound download paths (get_image_cache_dir / get_video_cache_dir) and the sweeper are untouched. RED: tests/agent/test_generated_media_durable.py (9 failed on main) — 25h-backdated generated files deleted by the real cleanup_image_cache / cleanup_video_cache, and no mount mapping for cache/generated. tests/tools/test_image_generation_artifacts.py ::test_real_generated_image_is_agent_visible_under_ssh_env covers the sandbox regression with the real writer. Green: 533 passed, 19 skipped. Co-authored-by: Froraut <79215537+Froraut@users.noreply.github.com> (cherry picked from commit fd23181f22232f66aec7076d6bf0868ed45661f2) --- agent/image_gen_provider.py | 14 ++- agent/pet/generate/orchestrate.py | 4 +- agent/video_gen_provider.py | 16 ++- plugins/image_gen/meta-ai/plugin.yaml | 2 +- plugins/image_gen/openai-codex/plugin.yaml | 2 +- plugins/image_gen/openai/plugin.yaml | 2 +- tests/agent/test_generated_media_durable.py | 116 ++++++++++++++++++ tests/agent/test_save_url_image.py | 7 +- .../image_gen/test_meta_ai_provider.py | 2 +- .../image_gen/test_openai_codex_provider.py | 2 +- .../plugins/image_gen/test_openai_provider.py | 2 +- .../tools/test_image_generation_artifacts.py | 42 +++++++ tools/credential_files.py | 6 + 13 files changed, 197 insertions(+), 20 deletions(-) create mode 100644 tests/agent/test_generated_media_durable.py diff --git a/agent/image_gen_provider.py b/agent/image_gen_provider.py index ce7523006c..23911c4efc 100644 --- a/agent/image_gen_provider.py +++ b/agent/image_gen_provider.py @@ -75,9 +75,15 @@ def normalize_reference_images(value: Any) -> Optional[List[str]]: return [item.strip() for item in value if isinstance(item, str) and item.strip()] or None +# Generated deliverables live in ``cache/generated//``, OUTSIDE the +# transient inbound caches the gateway housekeeping sweeps after 24h (#126445): +# for base64-returning providers the cached file is the ONLY copy of the image. +_GENERATED_IMAGE_KIND = "generated/images" + + def save_b64_image(b64_data: str, *, prefix: str = "image", extension: str = "png") -> Path: - """Decode base64 image data into ``$HERMES_HOME/cache/images/``; return the path.""" - return provider_media.save_b64("images", b64_data, prefix=prefix, extension=extension) + """Decode base64 image data into ``$HERMES_HOME/cache/generated/images/``; return the path.""" + return provider_media.save_b64(_GENERATED_IMAGE_KIND, b64_data, prefix=prefix, extension=extension) _URL_IMAGE_CONTENT_TYPES = { @@ -88,10 +94,10 @@ _URL_IMAGE_CONTENT_TYPES = { def save_url_image( url: str, *, prefix: str = "image", timeout: float = 60.0, max_bytes: int = 25 * 1024 * 1024, ) -> Path: - """Download an (often ephemeral) image URL into ``$HERMES_HOME/cache/images/``. Raises on + """Download an (often ephemeral) image URL into ``$HERMES_HOME/cache/generated/images/``. Raises on network / HTTP / oversize / empty errors so callers can fall back to the bare URL.""" return provider_media.save_url( - "images", url, prefix=prefix, timeout=timeout, max_bytes=max_bytes, + _GENERATED_IMAGE_KIND, url, prefix=prefix, timeout=timeout, max_bytes=max_bytes, chunk_size=64 * 1024, content_types=_URL_IMAGE_CONTENT_TYPES, url_extensions=("png", "jpg", "jpeg", "webp", "gif"), default_extension="png", label="Image", empty_error="Image at {url} returned 0 bytes; refusing to cache.", diff --git a/agent/pet/generate/orchestrate.py b/agent/pet/generate/orchestrate.py index aec37df703..9175064ede 100644 --- a/agent/pet/generate/orchestrate.py +++ b/agent/pet/generate/orchestrate.py @@ -75,7 +75,7 @@ def _harden_transparency(path: Path) -> Path: out = path if path.suffix.lower() == ".png" else path.with_suffix(".png") keyed.save(out, format="PNG") if out != path: - _unlink_quietly(path) # nothing else prunes cache/images outside the gateway loop + _unlink_quietly(path) # nothing else prunes cache/generated/images outside the gateway loop return out except Exception as exc: # noqa: BLE001 - cosmetic; fall back to the raw image logger.debug("base draft transparency hardening failed for %s: %s", path, exc) @@ -199,7 +199,7 @@ def _generate_row(spec: tuple[str, int, int], *, base: Path, label: str, style: logger.warning("pet hatch %r: row %r attempt %d/%d failed: %s", slug, state, attempt + 1, _ROW_GEN_ATTEMPTS, exc) finally: # Strips are intermediates already decoded into memory; nothing - # prunes cache/images outside the gateway loop, so drop them now. + # prunes cache/generated/images outside the gateway loop, so drop them now. for strip in strips: _unlink_quietly(Path(strip)) logger.warning("pet hatch %r: row %r gave up after %.1fs: %s", slug, state, time.monotonic() - t0, last_exc) diff --git a/agent/video_gen_provider.py b/agent/video_gen_provider.py index 805974727a..dfe18adf6e 100644 --- a/agent/video_gen_provider.py +++ b/agent/video_gen_provider.py @@ -69,14 +69,20 @@ class VideoGenProvider(CatalogProviderBase): pass; providers that honor it report ``upscaled: True`` in ``extra``.""" +# Generated deliverables live in ``cache/generated//``, OUTSIDE the +# transient inbound caches the gateway housekeeping sweeps after 24h (#126445): +# for base64-returning providers the cached file is the ONLY copy of the video. +_GENERATED_VIDEO_KIND = "generated/videos" + + def save_b64_video(b64_data: str,*, prefix: str="video", extension: str="mp4") -> Path: - """Decode base64 video data into ``$HERMES_HOME/cache/videos/``; return the path.""" - return provider_media.save_b64("videos", b64_data, prefix=prefix, extension=extension) + """Decode base64 video data into ``$HERMES_HOME/cache/generated/videos/``; return the path.""" + return provider_media.save_b64(_GENERATED_VIDEO_KIND, b64_data, prefix=prefix, extension=extension) def save_bytes_video(raw: bytes,*, prefix: str="video", extension: str="mp4") -> Path: """Write raw video bytes (e.g. an HTTP download body) to the cache.""" - return provider_media.save_bytes("videos", raw, prefix=prefix, extension=extension) + return provider_media.save_bytes(_GENERATED_VIDEO_KIND, raw, prefix=prefix, extension=extension) _URL_VIDEO_CONTENT_TYPES = { @@ -94,12 +100,12 @@ def save_url_video( require_video_content_type: bool = False, trusted_origin: bool = False, ) -> Path: - """Download an (often ephemeral) video URL into ``$HERMES_HOME/cache/videos/``; + """Download an (often ephemeral) video URL into ``$HERMES_HOME/cache/generated/videos/``; raises on network / HTTP / oversize / empty errors so callers can fall back to the URL. ``trusted_origin`` is only for URLs built from the operator's configured provider ``base_url`` (see ``provider_media.save_url``).""" return provider_media.save_url( - "videos", url, prefix=prefix, timeout=timeout, max_bytes=max_bytes, + _GENERATED_VIDEO_KIND, url, prefix=prefix, timeout=timeout, max_bytes=max_bytes, chunk_size=256 * 1024, content_types=_URL_VIDEO_CONTENT_TYPES, url_extensions=("mp4", "webm", "mov", "mkv"), default_extension="mp4", label="Video", empty_error="Video at {url} was empty (0 bytes).", diff --git a/plugins/image_gen/meta-ai/plugin.yaml b/plugins/image_gen/meta-ai/plugin.yaml index 2d5bb36649..0032c878b4 100644 --- a/plugins/image_gen/meta-ai/plugin.yaml +++ b/plugins/image_gen/meta-ai/plugin.yaml @@ -1,6 +1,6 @@ name: meta-ai-image-gen version: 1.0.0 -description: "Meta Model API image generation backend (muse-image). OpenAI-compatible /v1/images/generations. Saves images to $HERMES_HOME/cache/images/." +description: "Meta Model API image generation backend (muse-image). OpenAI-compatible /v1/images/generations. Saves images to $HERMES_HOME/cache/generated/images/." author: Meta Platforms, Inc. kind: backend requires_env: diff --git a/plugins/image_gen/openai-codex/plugin.yaml b/plugins/image_gen/openai-codex/plugin.yaml index b90db889e6..b79db2e239 100644 --- a/plugins/image_gen/openai-codex/plugin.yaml +++ b/plugins/image_gen/openai-codex/plugin.yaml @@ -1,5 +1,5 @@ name: openai-codex version: 1.0.0 -description: "OpenAI image generation backed by ChatGPT/Codex OAuth (gpt-image-2 via the native Codex images/generations and images/edits endpoints). Saves generated images to $HERMES_HOME/cache/images/." +description: "OpenAI image generation backed by ChatGPT/Codex OAuth (gpt-image-2 via the native Codex images/generations and images/edits endpoints). Saves generated images to $HERMES_HOME/cache/generated/images/." author: NousResearch kind: backend diff --git a/plugins/image_gen/openai/plugin.yaml b/plugins/image_gen/openai/plugin.yaml index 5e3db55b48..7169746cc0 100644 --- a/plugins/image_gen/openai/plugin.yaml +++ b/plugins/image_gen/openai/plugin.yaml @@ -1,6 +1,6 @@ name: openai version: 1.0.0 -description: "OpenAI image generation backend (GPT Image 2 and GPT Image 2.5 Flare/Sunburst). Saves generated images to $HERMES_HOME/cache/images/." +description: "OpenAI image generation backend (GPT Image 2 and GPT Image 2.5 Flare/Sunburst). Saves generated images to $HERMES_HOME/cache/generated/images/." author: NousResearch kind: backend requires_env: diff --git a/tests/agent/test_generated_media_durable.py b/tests/agent/test_generated_media_durable.py new file mode 100644 index 0000000000..43a83979c6 --- /dev/null +++ b/tests/agent/test_generated_media_durable.py @@ -0,0 +1,116 @@ +"""Generated image/video output must survive the gateway media sweep (#126445). + +``save_b64_image`` / ``save_*_video`` write the ONLY copy of base64-returning +provider output. The hourly gateway housekeeping deletes 24h-old files from +``cache/images`` / ``cache/videos`` (transient inbound media), so generated +deliverables live in ``cache/generated//`` instead — otherwise the +transcript and the Artifacts panel point at deleted files a day later, with no +way to recover the bytes. + +Living outside the swept dirs means ``cache/generated`` needs its own +credential-files mount/sync entry: Docker/SSH/Modal backends only reach paths +that map through ``tools.credential_files``, and an unmapped path also drops +``agent_visible_image`` from the provider result. + +RED on main: every test here fails — the writers target the swept caches and +``cache/generated`` is not mounted at all. +""" + +from __future__ import annotations + +import base64 +import os +import time +from pathlib import Path + +PNG_1PX = bytes.fromhex( + "89504e470d0a1a0a0000000d49484452000000010000000108020000009077" + "53de00000010494441547801635c0e000000feff03000006000557bfabd400" + "00000049454e44ae426082" +) + + +def _backdate_hours(path, hours: float = 25.0) -> None: + old = time.time() - hours * 3600 + os.utime(path, (old, old)) + + +class TestGeneratedImageSurvivesSweep: + def test_old_generated_image_not_swept(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + from agent.image_gen_provider import save_b64_image + from gateway.platforms.base import cleanup_image_cache + + path = save_b64_image(base64.b64encode(PNG_1PX).decode(), prefix="red_test") + _backdate_hours(path) + removed = cleanup_image_cache(max_age_hours=24) + assert path.exists(), ( + f"25h-old generated image was swept ({removed} removed): {path}" + ) + + def test_generated_image_outside_swept_dir(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + from agent.image_gen_provider import save_b64_image + from gateway.platforms.base import get_image_cache_dir + + path = save_b64_image(base64.b64encode(PNG_1PX).decode(), prefix="red_test") + assert path.parent != get_image_cache_dir() + assert path.exists() + + +class TestGeneratedVideoSurvivesSweep: + def test_old_generated_video_not_swept(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + from agent.video_gen_provider import save_b64_video + from gateway.platforms.base import cleanup_video_cache + + path = save_b64_video(base64.b64encode(b"fake-video-bytes").decode(), prefix="red_test") + _backdate_hours(path) + removed = cleanup_video_cache(max_age_hours=24) + assert path.exists(), ( + f"25h-old generated video was swept ({removed} removed): {path}" + ) + + def test_generated_video_outside_swept_dir(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + from agent.video_gen_provider import save_b64_video + from gateway.platforms.base import get_video_cache_dir + + path = save_b64_video(base64.b64encode(b"fake-video-bytes").decode(), prefix="red_test") + assert path.parent != get_video_cache_dir() + assert path.exists() + + +class TestGeneratedMediaReachesSandbox: + """Out-of-sweep is only half the job: remote backends must still see the file.""" + + def test_generated_dir_is_mounted_from_the_writers_location(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + from agent.image_gen_provider import save_b64_image + from tools.credential_files import get_cache_directory_mounts + + path = save_b64_image(base64.b64encode(PNG_1PX).decode(), prefix="red_test") + mounts = {m["container_path"]: m["host_path"] for m in get_cache_directory_mounts()} + assert "/root/.hermes/cache/generated" in mounts, sorted(mounts) + assert path.is_relative_to(Path(mounts["/root/.hermes/cache/generated"]).resolve()) + + def test_generated_image_maps_into_the_sandbox(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + from agent.image_gen_provider import save_b64_image + from tools.credential_files import map_cache_path_to_container + + path = save_b64_image(base64.b64encode(PNG_1PX).decode(), prefix="red_test") + rel = f"cache/generated/images/{path.name}" + assert map_cache_path_to_container(str(path)) == f"/root/.hermes/{rel}" + assert map_cache_path_to_container(str(path), container_base="~/.hermes") == f"~/.hermes/{rel}" + + def test_generated_video_is_in_the_per_file_sync_list(self, tmp_path, monkeypatch): + """Modal uploads file-by-file through iter_cache_files().""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + from agent.video_gen_provider import save_bytes_video + from tools.credential_files import iter_cache_files + + path = save_bytes_video(b"fake-video-bytes", prefix="red_test") + # container paths are POSIX; normalize the host separator pytest sees on Windows + entries = {e["container_path"].replace("\\", "/") for e in iter_cache_files("~/.hermes")} + assert f"~/.hermes/cache/generated/videos/{path.name}" in entries diff --git a/tests/agent/test_save_url_image.py b/tests/agent/test_save_url_image.py index bdaa0e79de..eeac26396c 100644 --- a/tests/agent/test_save_url_image.py +++ b/tests/agent/test_save_url_image.py @@ -113,9 +113,10 @@ class TestSaveUrlImage: assert path.exists() assert path.read_bytes() == PNG_1PX - # The cache directory must be under HERMES_HOME — gateway cleanup - # relies on this being the canonical location. - assert os.path.join("cache", "images") in str(path) + # Durable generated-media dir: under HERMES_HOME but OUTSIDE the swept + # inbound caches — gateway cleanup must never delete the only copy of a + # generated deliverable (#126445). + assert os.path.join("cache", "generated", "images") in str(path) assert path.suffix == ".png" def test_404_raises(self, http_server): diff --git a/tests/plugins/image_gen/test_meta_ai_provider.py b/tests/plugins/image_gen/test_meta_ai_provider.py index 0a9d955aab..6999468307 100644 --- a/tests/plugins/image_gen/test_meta_ai_provider.py +++ b/tests/plugins/image_gen/test_meta_ai_provider.py @@ -203,7 +203,7 @@ class TestGenerate: saved = Path(result["image"]) assert saved.exists() - assert saved.parent == tmp_path / "cache" / "images" + assert saved.parent == tmp_path / "cache" / "generated" / "images" assert saved.read_bytes() == png_bytes call_kwargs = fake_client.images.generate.call_args.kwargs diff --git a/tests/plugins/image_gen/test_openai_codex_provider.py b/tests/plugins/image_gen/test_openai_codex_provider.py index 00a2a5abf2..bbcf2d11d9 100644 --- a/tests/plugins/image_gen/test_openai_codex_provider.py +++ b/tests/plugins/image_gen/test_openai_codex_provider.py @@ -205,7 +205,7 @@ class TestGenerate: assert result["reported_size"] == "1254x1254" assert result["imagegen_request_id"] == "req_abc" saved = Path(result["image"]) - assert saved.exists() and saved.parent == tmp_path / "cache" / "images" + assert saved.exists() and saved.parent == tmp_path / "cache" / "generated" / "images" assert saved.name.startswith("openai_codex_") (request,) = codex_backend["requests"] diff --git a/tests/plugins/image_gen/test_openai_provider.py b/tests/plugins/image_gen/test_openai_provider.py index d70023fcec..ae76d63011 100644 --- a/tests/plugins/image_gen/test_openai_provider.py +++ b/tests/plugins/image_gen/test_openai_provider.py @@ -248,7 +248,7 @@ class TestGenerate: saved = Path(result["image"]) assert saved.exists() - assert saved.parent == tmp_path / "cache" / "images" + assert saved.parent == tmp_path / "cache" / "generated" / "images" assert saved.read_bytes() == png_bytes call_kwargs = fake_client.images.generate.call_args.kwargs diff --git a/tests/tools/test_image_generation_artifacts.py b/tests/tools/test_image_generation_artifacts.py index ae2e9bede3..45506979ec 100644 --- a/tests/tools/test_image_generation_artifacts.py +++ b/tests/tools/test_image_generation_artifacts.py @@ -1,3 +1,4 @@ +import base64 import concurrent.futures import json import threading @@ -147,3 +148,44 @@ def test_handle_image_generate_postprocesses_plugin_result(monkeypatch, tmp_path assert seen_task_ids == ["plugin-task"] assert result["agent_visible_image"] == "/home/remote/.hermes/cache/images/plugin.png" + + +def test_real_generated_image_is_agent_visible_under_ssh_env(monkeypatch, tmp_path): + """#126445: the writers now target ``cache/generated/``, so that dir needs + its own credential-files mount entry — otherwise the SSH/Docker/Modal agent + never receives the bytes and the result silently loses ``agent_visible_image``. + + Uses the real ``save_b64_image`` writer (not a hand-made ``cache/images`` + fixture), which is exactly the hole the sibling test above cannot see. + """ + from agent.image_gen_provider import save_b64_image + from tools import image_generation_tool + + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + + image_path = save_b64_image(base64.b64encode(b"png").decode(), prefix="xai_real") + + sync_calls = [] + + class FakeSyncManager: + def sync(self, *, force=False): + sync_calls.append(force) + + env = SimpleNamespace( + _remote_home="/home/remotesshuser", + _sync_manager=FakeSyncManager(), + ) + monkeypatch.setattr(image_generation_tool, "_active_terminal_env", lambda task_id: env) + + raw = json.dumps({"success": True, "image": str(image_path)}) + result = json.loads( + image_generation_tool._postprocess_image_generate_result(raw, task_id="task-1") + ) + + assert result["host_image"] == str(image_path) + assert result["agent_visible_image"] == ( + f"/home/remotesshuser/.hermes/cache/generated/images/{image_path.name}" + ) + assert sync_calls == [True] diff --git a/tools/credential_files.py b/tools/credential_files.py index 89872fff05..66b256a152 100644 --- a/tools/credential_files.py +++ b/tools/credential_files.py @@ -250,6 +250,12 @@ _CACHE_DIRS: list[tuple[str, str]] = [ ("cache/web", "web_cache"), ("cache/delegation", "delegation_cache"), ("cache/spillover", "cache/spillover"), # oversized tool results; host side is canonical + # Generated image/video deliverables (#126445). They are deliberately kept + # out of the swept inbound caches above, so they need their own mount/sync + # entry — without it Docker/SSH/Modal backends never see the file and the + # provider result loses ``agent_visible_image``. No legacy alias exists, so + # both tuple slots match. Parent entry: covers every generated media kind. + ("cache/generated", "cache/generated"), # Flat top-level desktop staging dirs (tui_gateway attach RPCs; no legacy alias), # mounted so vision/file tools in sandboxes reach uploads and dropped files. # Mount it so vision can reach uploads inside sandbox containers (#69575). No legacy alias exists, so