fix(agent): keep generated images/videos out of swept media caches (Closes #126445)
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)
This commit is contained in:
@@ -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/<media>/``, 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.",
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -69,14 +69,20 @@ class VideoGenProvider(CatalogProviderBase):
|
||||
pass; providers that honor it report ``upscaled: True`` in ``extra``."""
|
||||
|
||||
|
||||
# Generated deliverables live in ``cache/generated/<media>/``, 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).",
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
116
tests/agent/test_generated_media_durable.py
Normal file
116
tests/agent/test_generated_media_durable.py
Normal file
@@ -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/<kind>/`` 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
|
||||
@@ -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):
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"]
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user