diff --git a/agent/skill_commands.py b/agent/skill_commands.py index ab0dad92e168c..101fc1755a812 100644 --- a/agent/skill_commands.py +++ b/agent/skill_commands.py @@ -8,7 +8,7 @@ import logging import os import re -from pathlib import Path +from pathlib import Path, PurePosixPath from typing import Any, Dict, Optional from hermes_constants import display_hermes_home @@ -296,19 +296,23 @@ def _build_skill_message( # Done before anything else so downstream blocks (setup notes, # supporting-file hints) see the expanded content. skills_cfg = _load_skills_config() - if skills_cfg.get("template_vars", True): - content = _substitute_template_vars(content, skill_dir, session_id) if skills_cfg.get("inline_shell", False): timeout = int(skills_cfg.get("inline_shell_timeout", 10) or 10) - content = _expand_inline_shell(content, skill_dir, timeout) + content = _expand_inline_shell(content, skill_dir, timeout, session_id, skills_cfg.get("template_vars", True)) + if skills_cfg.get("template_vars", True): + content = _substitute_template_vars(content, skill_dir, session_id) + + from agent.skill_path_mapping import map_skill_dir_for_backend + mapped_dir = map_skill_dir_for_backend(skill_dir, task_id=session_id) if skill_dir else None + hint_dir = PurePosixPath(mapped_dir) if mapped_dir and mapped_dir != str(skill_dir) else skill_dir parts = [activation_note, "", content.strip()] # ── Inject the absolute skill directory so the agent can reference # bundled scripts without an extra skill_view() round-trip. ── if skill_dir: parts.append("") - parts.append(f"[Skill directory: {skill_dir}]") + parts.append(f"[Skill directory: {hint_dir}]") parts.append( "Resolve any relative paths in this skill (e.g. `scripts/foo.js`, " "`templates/config.yaml`) against that directory, then run them " @@ -364,11 +368,11 @@ def _build_skill_message( parts.append("") parts.append("[This skill has supporting files:]") for sf in supporting: - parts.append(f"- {sf} -> {skill_dir / sf}") + parts.append(f"- {sf} -> {hint_dir / sf}") parts.append( f'\nLoad any of these with skill_view(name="{skill_view_target}", ' f'file_path=""), or run scripts directly by absolute path ' - f"(e.g. `node {skill_dir}/scripts/foo.js`)." + f"(e.g. `node {hint_dir}/scripts/foo.js`)." ) if user_instruction: diff --git a/agent/skill_path_mapping.py b/agent/skill_path_mapping.py new file mode 100644 index 0000000000000..092934539273c --- /dev/null +++ b/agent/skill_path_mapping.py @@ -0,0 +1,171 @@ +"""Map host skill directory paths to backend-visible paths. + +When the active terminal backend is remote (Docker, SSH, Daytona, Singularity, +Modal), the skills tree lives at a different filesystem location inside the +sandbox than on the host. Skill content that references the skill directory +(``${HERMES_SKILL_DIR}``, ``[Skill directory: ...]``, supporting-file hints) +must use the backend-visible path, or the agent will try to run bundled +scripts via paths that do not exist in the sandbox (hermes-agent#41541, +#73842). + +The authoritative mount layout is computed by +``tools.credential_files.get_skills_directory_layout()``; this module consumes +it with a longest-prefix match and falls back to the host path whenever the +backend is local or unknown, so behavior on local backends is unchanged. +""" + +from __future__ import annotations + +import logging +import os +from pathlib import Path, PurePosixPath +from typing import Any + +logger = logging.getLogger(__name__) + +# Container backends whose skills mount root is /root/.hermes inside the +# sandbox. Mirrors the class names in tools/environments/*.py. +_CONTAINER_ENV_CLASSES = { + "DockerEnvironment", + "SingularityEnvironment", + "ModalEnvironment", + "ManagedModalEnvironment", +} + +# TERMINAL_ENV values that map to the /root/.hermes container layout even +# before a live environment object exists (used at first skill render). +_CONTAINER_BACKEND_NAMES = {"docker", "singularity", "modal"} + +# Remote backends whose .hermes root is only known from the live environment +# (SSH/Daytona resolve a remote home at connect time). +_REMOTE_BACKEND_NAMES = {"ssh", "daytona"} + + +def _active_terminal_env(task_id: str | None) -> Any: + """Return the live terminal environment for *task_id*, or None.""" + if not task_id: + return None + try: + from tools.terminal_tool import get_active_env + + return get_active_env(task_id) + except Exception: + logger.debug("Could not resolve active terminal env", exc_info=True) + return None + + +def _backend_name() -> str: + return str(os.getenv("TERMINAL_ENV", "local")).strip().lower() or "local" + + +def _hermes_base_for_env(env: Any, backend_name: str) -> str | None: + """Resolve the backend-visible ``.hermes`` root, or None if unknown. + + None means "the host path is the backend path" (local/unknown backend) or + the backend root cannot be determined without a live environment. + """ + if env is not None: + remote_home = getattr(env, "_remote_home", None) + if remote_home: + return f"{str(remote_home).rstrip('/')}/.hermes" + if type(env).__name__ in _CONTAINER_ENV_CLASSES: + return "/root/.hermes" + return None + if backend_name in _CONTAINER_BACKEND_NAMES: + return "/root/.hermes" + return None + + +def map_skill_dir_for_backend( + host_skill_dir: Path | str | None, + task_id: str | None = None, +) -> str: + """Translate *host_skill_dir* to the path the agent sees on the backend. + + Longest-prefix-matches the host path against the existing skills mount + layout (``get_skills_directory_layout``) and returns the corresponding + backend-visible path (POSIX form, since container/remote paths are + POSIX). Falls back to the host path unchanged when: + + - the backend is local or unknown (TERMINAL_ENV unset / "local"), + - the backend root cannot be determined without a live environment + (SSH/Daytona before first connect), + - the directory is not under any known skills mount, or + - the mount layout cannot be resolved. + """ + if host_skill_dir is None: + return "" + host = str(host_skill_dir) + if _backend_name() == "sprites": + return _map_sprites_skill_dir(host) + base = _hermes_base_for_env(_active_terminal_env(task_id), _backend_name()) + if not base: + return host + try: + from tools.credential_files import get_skills_directory_layout + + mounts = get_skills_directory_layout(container_base=base) + except Exception: + logger.debug("Could not resolve skills directory mount layout", exc_info=True) + return host + if not mounts: + return host + + # Longest-prefix match against mount host paths. Normalize separators so + # Windows hosts (backslash paths) match against POSIX container prefixes. + # On Windows, filesystems are case-insensitive, so comparisons are + # lowercased there; on POSIX hosts the match stays case-sensitive. + host_norm = host.replace("\\", "/") + case_fold = os.name == "nt" + best_prefix: str | None = None + best_container: str | None = None + for m in mounts: + # Match against the canonical source path AND the actual mount + # source: when symlinks are present the mount host_path is a + # sanitized copy while agent-visible skill dirs live under the + # canonical skills tree (source_path). + for key in ("source_path", "host_path"): + candidate = m.get(key) + if not candidate: + continue + prefix = str(candidate).rstrip("/").replace("\\", "/") + match_norm = host_norm if not case_fold else host_norm.lower() + prefix_norm = prefix if not case_fold else prefix.lower() + if match_norm == prefix_norm or match_norm.startswith(prefix_norm + "/"): + if best_prefix is None or len(prefix) > len(best_prefix): + best_prefix = prefix + best_container = m["container_path"] + if best_container is None: + return host + + rel = host_norm[len(best_prefix):].lstrip("/") + if not rel: + return best_container + return f"{best_container.rstrip('/')}/{rel}" + + +def _map_sprites_skill_dir(host: str) -> str: + """Use the same roots as Sprites' secret-free skills sync (no host mounts).""" + from agent.skill_utils import get_external_skills_dirs + from tools.credential_files import _resolve_hermes_home + + roots = [(_resolve_hermes_home() / "skills", "/skills")] + roots.extend((root, f"/skills/external_skills/{index}") + for index, root in enumerate(get_external_skills_dirs())) + path = Path(host).absolute() + for root, target in sorted(roots, key=lambda item: len(item[0].parts), reverse=True): + try: + relative = path.relative_to(root.absolute()) + except ValueError: + continue + # The sync skips symlinks below each catalog root. Never advertise + # a path to bytes that are intentionally excluded from the projection. + current = root + if ".." in relative.parts: + return host + for part in relative.parts: + current = current / part + if current.is_symlink(): + return host + return str(PurePosixPath(target) / relative.as_posix()) + return host diff --git a/agent/skill_preprocessing.py b/agent/skill_preprocessing.py index 19c6eeb80fb3d..e3493a1a77152 100644 --- a/agent/skill_preprocessing.py +++ b/agent/skill_preprocessing.py @@ -40,6 +40,8 @@ def substitute_template_vars( content: str, skill_dir: Path | None, session_id: str | None, + *, + runtime_paths: bool = True, ) -> str: """Replace ${HERMES_SKILL_DIR} / ${HERMES_SESSION_ID} in skill content. @@ -49,7 +51,11 @@ def substitute_template_vars( if not content: return content + from agent.skill_path_mapping import map_skill_dir_for_backend + skill_dir_str = str(skill_dir) if skill_dir else None + if skill_dir and runtime_paths: + skill_dir_str = map_skill_dir_for_backend(skill_dir, task_id=session_id) def _replace(match: re.Match) -> str: token = match.group(1) @@ -107,6 +113,8 @@ def expand_inline_shell( content: str, skill_dir: Path | None, timeout: int, + session_id: str | None = None, + template_vars: bool = False, ) -> str: """Replace every !`cmd` snippet in ``content`` with its stdout. @@ -120,6 +128,10 @@ def _replace(match: re.Match) -> str: cmd = match.group(1).strip() if not cmd: return "" + if template_vars: + # Inline preprocessing still runs on the host. Runtime path + # translation applies only to instructions handed to the agent. + cmd = substitute_template_vars(cmd, skill_dir, session_id, runtime_paths=False) return run_inline_shell(cmd, skill_dir, timeout) return _INLINE_SHELL_RE.sub(_replace, content) @@ -136,9 +148,9 @@ def preprocess_skill_content( return content cfg = skills_cfg if isinstance(skills_cfg, dict) else load_skills_config() - if cfg.get("template_vars", True): - content = substitute_template_vars(content, skill_dir, session_id) if cfg.get("inline_shell", False): timeout = int(cfg.get("inline_shell_timeout", 10) or 10) - content = expand_inline_shell(content, skill_dir, timeout) + content = expand_inline_shell(content, skill_dir, timeout, session_id, cfg.get("template_vars", True)) + if cfg.get("template_vars", True): + content = substitute_template_vars(content, skill_dir, session_id) return content diff --git a/agent/skill_utils.py b/agent/skill_utils.py index eea78d6a07c05..bf876ec1984ee 100644 --- a/agent/skill_utils.py +++ b/agent/skill_utils.py @@ -749,6 +749,21 @@ def _resolve_dotpath(config: Dict[str, Any], dotted_key: str): return current +_HOME_VAR_RE = re.compile(r"\$(?:\{HOME\}|HOME)(?=$|[/\\])") + + +def _expand_skill_config_path(value: str) -> str: + """Expand skill defaults against tool HOME, not the gateway account.""" + from hermes_constants import get_subprocess_home + + tool_home = "/home" if os.getenv("TERMINAL_ENV", "").strip().lower() == "sprites" else get_subprocess_home() + if tool_home: + if value == "~" or value.startswith(("~/", "~\\")): + value = tool_home + value[1:] + value = _HOME_VAR_RE.sub(lambda _match: tool_home, value) + return os.path.expanduser(os.path.expandvars(value)) + + def resolve_skill_config_values( config_vars: List[Dict[str, Any]], ) -> Dict[str, Any]: @@ -771,8 +786,8 @@ def resolve_skill_config_values( value = var.get("default", "") # Expand ~ in path-like values - if isinstance(value, str) and ("~" in value or "${" in value): - value = os.path.expanduser(os.path.expandvars(value)) + if isinstance(value, str) and ("~" in value or "$" in value): + value = _expand_skill_config_path(value) resolved[logical_key] = value diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index e55c91b4dd181..5a460d8e72c8c 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -1826,7 +1826,7 @@ def cache_media_bytes( ``application/octet-stream``); only images that fail validation (``cache_image_from_bytes`` raises ValueError) return None. """ - from tools.credential_files import to_agent_visible_cache_path + from tools.credential_files import publish_cache_path ext = _resolve_media_ext(filename, mime_type) mime = (mime_type or "").lower() @@ -1847,18 +1847,18 @@ def cache_media_bytes( except ValueError: return None out_mime = mime if mime.startswith("image/") else SUPPORTED_IMAGE_DOCUMENT_TYPES.get(img_ext, "image/jpeg") - return CachedMedia(to_agent_visible_cache_path(path), out_mime, "image", display) + return CachedMedia(publish_cache_path(path), out_mime, "image", display) if is_video: vid_ext = ext if ext in SUPPORTED_VIDEO_TYPES else ".mp4" path = cache_video_from_bytes(data, ext=vid_ext) - return CachedMedia(to_agent_visible_cache_path(path), SUPPORTED_VIDEO_TYPES.get(vid_ext, "video/mp4"), "video", display) + return CachedMedia(publish_cache_path(path), SUPPORTED_VIDEO_TYPES.get(vid_ext, "video/mp4"), "video", display) if is_audio: aud_ext = ext if ext in {".ogg", ".mp3", ".wav", ".m4a", ".opus", ".flac"} else ".ogg" path = cache_audio_from_bytes(data, ext=aud_ext) out_mime = mime if mime.startswith("audio/") else f"audio/{aud_ext.lstrip('.')}" - return CachedMedia(to_agent_visible_cache_path(path), out_mime, "audio", display) + return CachedMedia(publish_cache_path(path), out_mime, "audio", display) # Any other file type is cached and surfaced to the agent as a local path # so it can be inspected with terminal / read_file / etc. Authorization to @@ -1873,7 +1873,7 @@ def cache_media_bytes( out_mime = SUPPORTED_DOCUMENT_TYPES[ext] else: out_mime = mime if mime else "application/octet-stream" - return CachedMedia(to_agent_visible_cache_path(path), out_mime, "document", display or fallback_name) + return CachedMedia(publish_cache_path(path), out_mime, "document", display or fallback_name) class MessageType(Enum): diff --git a/gateway/run.py b/gateway/run.py index 7888315d2ae89..c4c4b957eb5fc 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -12858,7 +12858,7 @@ async def _prepare_inbound_message_text( # language. The hardcoded send has therefore been removed. if audio_file_paths: - from tools.credential_files import to_agent_visible_cache_path as _to_agent_path + from tools.credential_files import publish_cache_path as _to_agent_path for _apath in audio_file_paths: _basename = os.path.basename(_apath) _parts = _basename.split("_", 2) @@ -12877,7 +12877,7 @@ async def _prepare_inbound_message_text( message_text = f"{_note}\n\n{message_text}" if video_paths: - from tools.credential_files import to_agent_visible_cache_path as _to_agent_path + from tools.credential_files import publish_cache_path as _to_agent_path for _vpath in video_paths: _basename = os.path.basename(_vpath) _parts = _basename.split("_", 2) @@ -12897,7 +12897,7 @@ async def _prepare_inbound_message_text( if event.media_urls: import mimetypes as _mimetypes - from tools.credential_files import to_agent_visible_cache_path + from tools.credential_files import publish_cache_path _TEXT_EXTENSIONS = {".txt", ".md", ".csv", ".log", ".json", ".xml", ".yaml", ".yml", ".toml", ".ini", ".cfg"} for i, path in enumerate(event.media_urls): @@ -12934,10 +12934,9 @@ async def _prepare_inbound_message_text( display_name = parts[2] if len(parts) >= 3 else basename display_name = re.sub(r'[^\w.\- ]', '_', display_name) - # Translate host cache path to in-container path if running under Docker backend. - # This ensures the agent receives a path it can open inside its sandbox, as the - # cache directories are auto-mounted at /root/.hermes/cache/* by get_cache_directory_mounts(). - agent_path = to_agent_visible_cache_path(path) + # Publish before advertising the path: copying backends need + # the bytes on the remote filesystem, while Docker uses mounts. + agent_path = publish_cache_path(path) context_note = _build_document_context_note(display_name, agent_path, mtype) message_text = f"{context_note}\n\n{message_text}" diff --git a/tests/agent/test_skill_utils.py b/tests/agent/test_skill_utils.py index 4a860a2077da9..4db08b9365f19 100644 --- a/tests/agent/test_skill_utils.py +++ b/tests/agent/test_skill_utils.py @@ -167,9 +167,12 @@ def test_skill_config_raw_cache_invalidates_on_config_edit(tmp_path, monkeypatch skill_utils._external_dirs_cache_clear() assert get_disabled_skill_names() == {"old-skill"} + previous_mtime = config_path.stat().st_mtime_ns config_path.write_text("skills:\n disabled: [new-skill]\n", encoding="utf-8") import os - os.utime(config_path, None) + # Fast equal-sized writes can retain the same timestamp on Linux. This + # tests invalidation after a metadata change, not filesystem clock ticks. + os.utime(config_path, ns=(previous_mtime + 1_000_000_000,) * 2) assert get_disabled_skill_names() == {"new-skill"} diff --git a/tests/gateway/test_api_server_omnio_turn_event_log.py b/tests/gateway/test_api_server_omnio_turn_event_log.py index 827213f88a684..198bd1b0b0750 100644 --- a/tests/gateway/test_api_server_omnio_turn_event_log.py +++ b/tests/gateway/test_api_server_omnio_turn_event_log.py @@ -1431,9 +1431,14 @@ async def test_finalize_hook_failure_still_emits_terminal_promptly( monkeypatch: pytest.MonkeyPatch, hook_status: int | str, ) -> None: + release_hook = asyncio.Event() + async def finalize(_request: web.Request) -> web.Response: if hook_status == "timeout": - await asyncio.sleep(1.0) + # The hook cannot complete until after the terminal event. A + # missing request deadline therefore fails the bounded terminal + # wait, without measuring unrelated CI scheduling latency. + await release_hook.wait() return web.json_response({"annotations": []}) return web.json_response({"annotations": []}, status=hook_status) @@ -1447,25 +1452,30 @@ async def finalize(_request: web.Request) -> web.Response: api_server_module._OMNIO_TURN_FINALIZE_HOOK_ENV, str(server.make_url("/internal/turn-finalize")), ) - started_at = asyncio.get_running_loop().time() - with patch.object( - adapter, - "_create_agent", - return_value=_agent( - lambda **_kwargs: { - "final_response": "Report: /brand/report.pdf", - "messages": [], - } - ), - ): - started, events = await _run_without_http_server( + try: + with patch.object( adapter, - {"input": "make report", "turn_id": "turn-1"}, - ) - elapsed = asyncio.get_running_loop().time() - started_at + "_create_agent", + return_value=_agent( + lambda **_kwargs: { + "final_response": "Report: /brand/report.pdf", + "messages": [], + } + ), + ), patch.object( + api_server_module, + "_request_turn_finalize_annotations", + wraps=api_server_module._request_turn_finalize_annotations, + ) as request_finalize: + started, events = await _run_without_http_server( + adapter, + {"input": "make report", "turn_id": "turn-1"}, + ) + request_finalize.assert_awaited_once() + finally: + release_hook.set() assert started.status == 202 - assert elapsed < 0.5 assert events[-1]["type"] == "response.completed" assert not any( event["type"] == "response.output_text.annotation.added" for event in events diff --git a/tests/gateway/test_video_context_note.py b/tests/gateway/test_video_context_note.py index 19ca1dba8e51b..aea8abc97e015 100644 --- a/tests/gateway/test_video_context_note.py +++ b/tests/gateway/test_video_context_note.py @@ -34,15 +34,16 @@ async def test_video_attachment_adds_path_note_without_document_wording(): ) with patch( - "tools.credential_files.to_agent_visible_cache_path", + "tools.credential_files.publish_cache_path", side_effect=lambda path: path, - ): + ) as publish: result = await runner._prepare_inbound_message_text( event=event, source=source, history=[], ) + publish.assert_called_once_with("/tmp/video_clip.mp4") assert "video attachment" in result assert "/tmp/video_clip.mp4" in result assert "video analysis or media tool" in result diff --git a/tests/tools/test_code_execution_stdout_recovery.py b/tests/tools/test_code_execution_stdout_recovery.py new file mode 100644 index 0000000000000..f1c2922c3bd7f --- /dev/null +++ b/tests/tools/test_code_execution_stdout_recovery.py @@ -0,0 +1,139 @@ +"""Recover clipped execution output without repeating the original command.""" +import json +import os +import subprocess +from pathlib import Path + +import pytest + +from tools import code_execution_tool as execution + + +@pytest.fixture(autouse=True) +def isolated_home(tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "profile")) + monkeypatch.setenv("TERMINAL_ENV", "local") + + +def test_local_execution_retains_middle_and_does_not_repeat(tmp_path, monkeypatch): + monkeypatch.setattr(execution, "_load_config", lambda: {"timeout": 10}) + receipt = tmp_path / "executions.txt" + code = ( + f"with open({str(receipt)!r}, 'a') as f: f.write('ran\\n')\n" + "print('head\\n' * 12000)\n" + "print('MIDDLE_RECORD_617')\n" + "print('tail\\n' * 12000)\n" + ) + result = json.loads(execution.execute_code(code, enabled_tools=[])) + assert result["status"] == "success" + assert "MIDDLE_RECORD_617" not in result["output"] + saved = Path(result["stdout_spill_path"]).read_text() + assert "MIDDLE_RECORD_617" in saved + assert saved.startswith("head\n") and saved.endswith("tail\n\n") + assert result["stdout_spill_truncated"] is False + assert receipt.read_text() == "ran\n" + + +def test_short_output_does_not_create_artifact(): + output, metadata = execution._truncate_stdout_text("small\n") + assert output == "small\n" + assert "stdout_spill_path" not in metadata + + +def test_full_output_is_redacted_before_it_is_saved(): + secret = "ghp_" + "a" * 36 + _, metadata = execution._truncate_stdout_text("start\n" * 12000 + secret + "\nend\n" * 12000) + saved = Path(metadata["stdout_spill_path"]).read_text() + assert secret not in saved + assert "start" in saved and "end" in saved + + +def test_spill_limit_is_bytes_and_reported_as_partial(monkeypatch): + monkeypatch.setattr(execution, "MAX_SPILLED_STDOUT_BYTES", 60_000) + _, metadata = execution._truncate_stdout_text("ééé\n" * 15000) + data = Path(metadata["stdout_spill_path"]).read_bytes() + data.decode("utf-8") + assert len(data) <= 60_000 + assert metadata["stdout_spill_truncated"] is True + assert "partial" in metadata["warning"].lower() + assert "FULL output" not in metadata["warning"] + + +def test_storage_failure_preserves_result_without_unreadable_path(monkeypatch): + def unavailable(*args, **kwargs): + raise OSError("disk full") + monkeypatch.setattr(execution, "_spill_full_stdout", unavailable) + output, metadata = execution._truncate_stdout_text("start\n" * 15000 + "END") + assert output.endswith("END") + assert metadata["stdout_truncated"] is True + assert "stdout_spill_path" not in metadata + assert "unavailable" in metadata["warning"] + + +def test_symlink_cache_is_not_followed(tmp_path, monkeypatch): + home = tmp_path / "profile" + outside = tmp_path / "outside" + home.mkdir() + outside.mkdir() + (home / "cache").symlink_to(outside, target_is_directory=True) + _, metadata = execution._truncate_stdout_text("data\n" * 15000) + assert "stdout_spill_path" not in metadata + assert not list(outside.iterdir()) + + +@pytest.mark.parametrize("backend", ["ssh", "modal", "docker"]) +@pytest.mark.skipif(os.name == "nt", reason="The remote-filesystem fixture runs a POSIX shell") +def test_remote_execution_retains_recovery_on_execution_filesystem(tmp_path, monkeypatch, backend): + remote = tmp_path / "remote" + remote.mkdir() + + class FileEnvironment: + def get_temp_dir(self): + return str(remote) + + def write_file_content(self, path, content): + target = Path(path) + assert target.is_relative_to(remote) + target.write_text(content) + return True + + def execute(self, command, cwd=None, timeout=30): + result = subprocess.run( + command, shell=True, executable="/bin/bash", cwd=cwd or remote, + env={"PATH": os.environ["PATH"], "HOME": str(remote)}, + capture_output=True, text=True, timeout=timeout, + ) + return {"output": result.stdout + result.stderr, "returncode": result.returncode} + + monkeypatch.setenv("TERMINAL_ENV", backend) + monkeypatch.setattr(execution, "_load_config", lambda: {"timeout": 10}) + monkeypatch.setattr(execution, "_get_or_create_env", lambda task: (FileEnvironment(), backend)) + receipt = remote / "executions.txt" + code = ( + f"with open({str(receipt)!r}, 'a') as f: f.write('ran\\n')\n" + "print('head\\n' * 12000)\n" + "print('REMOTE_MIDDLE_RECORD')\n" + "print('tail\\n' * 12000)\n" + ) + result = json.loads(execution._execute_remote(code, "recovery-test", [])) + assert result["status"] == "success", result + assert result["exit_code"] == 0 + assert "REMOTE_MIDDLE_RECORD" not in result["output"] + saved = Path(result["stdout_spill_path"]) + assert saved.is_relative_to(remote), result + assert "REMOTE_MIDDLE_RECORD" in saved.read_text() + assert receipt.read_text() == "ran\n" + + +def test_remote_publication_failure_does_not_return_a_host_path(monkeypatch): + class UnavailableEnvironment: + def execute(self, *args, **kwargs): + raise OSError("remote unavailable") + + output, metadata = execution._truncate_stdout_text( + "start\n" * 15000 + "END", env=UnavailableEnvironment(), + ) + assert output.endswith("END") + assert metadata["stdout_truncated"] is True + assert "stdout_spill_path" not in metadata + assert "unavailable" in metadata["warning"] diff --git a/tests/tools/test_delegation_artifact_delivery.py b/tests/tools/test_delegation_artifact_delivery.py index d28d4b5512a2c..ae916fcfebe8a 100644 --- a/tests/tools/test_delegation_artifact_delivery.py +++ b/tests/tools/test_delegation_artifact_delivery.py @@ -1,4 +1,5 @@ -"""Worker artifacts cross the real HTTP client into a separate filesystem.""" +"""Harness cache artifacts (worker output, stdout recovery, stored pages, +screenshots) cross the HTTP client into a separate Toolbox filesystem.""" import json import threading from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer @@ -7,13 +8,18 @@ import pytest -from tools.credential_files import to_agent_visible_cache_path +from tools.credential_files import ( + _CACHE_DIRS, OMNIO_TOOLBOX_CACHE_BASE, from_agent_visible_cache_path, + to_agent_visible_cache_path, +) from tools.delegate_tool import _spill_summary_to_file from tools.delegation_live_log import create_live_transcripts +from tools.environments import file_sync from tools.environments.file_sync import ( - FileSyncManager, SPRITES_DELEGATION_ROOT, iter_sprites_delegation_files, + FileSyncManager, SPRITES_CACHE_ROOT, SPRITES_DELEGATION_ROOT, iter_sprites_cache_files, ) from tools.environments.sprites import SpritesEnvironment, SpritesFileOperations, SpritesToolboxError +from tools.code_execution_tool import _truncate_stdout_text @pytest.fixture @@ -46,11 +52,27 @@ def do_POST(self): else: try: content = path.read_text() - result = {"content": content, "totalLines": len(content.splitlines())} + if len(content.encode()) > 2 * 1024 * 1024: + result = {"error": f"File too large: {payload['path']}"} + else: + result = {"content": content, "totalLines": len(content.splitlines())} except FileNotFoundError: result = {"error": "file not found"} self.reply(result) + def do_GET(self): + query = parse_qs(urlsplit(self.path).query) + assert self.headers["X-Omnio-Brand"] == "brand-a" + assert self.headers["Authorization"] == "Bearer test-token" + path = toolbox / query["path"][0].lstrip("/") + requests.append({"operation": "raw-read", "path": query["path"][0]}) + if not path.is_file(): + self.send_error(404) + return + self.send_response(200) + self.end_headers() + self.wfile.write(path.read_bytes()) + def do_PUT(self): query = parse_qs(urlsplit(self.path).query) assert query["overwrite"] == ["true"] @@ -76,11 +98,15 @@ def reply(self, result): env.brand = "brand-a" env.timeout = 5 env.cwd = "/brand" - env._delegation_sync_lock = threading.Lock() - env._delegation_sync_manager = FileSyncManager( - iter_sprites_delegation_files, env._upload_delegation_artifact, - env._delete_delegation_artifacts, + env._cache_sync_lock = threading.Lock() + env._cache_sync_manager = FileSyncManager( + iter_sprites_cache_files, env._upload_cache_file, env._delete_cache_files, ) + from tools import file_tools, terminal_tool + + monkeypatch.setattr(terminal_tool, "_active_environments", {"default": env}) + monkeypatch.setattr(file_tools, "_file_ops_cache", {"default": SpritesFileOperations(env)}) + monkeypatch.setattr(terminal_tool, "_last_activity", {}) yield home, toolbox, env, requests server.shutdown() server.server_close() @@ -91,16 +117,158 @@ def test_summary_path_can_be_read_completely_from_toolbox(pair): home, toolbox, env, requests = pair expected = "worker instruction\n" * 3000 + "FINAL RESULT" path = _spill_summary_to_file(0, expected) - assert path.startswith("/tmp/.omnio-session/cache/delegation/") + assert path.startswith("/tmp/omnio-session/cache/delegation/") + # Publishing the path already pushed the file: a consumer that reads the + # Toolbox directly (the proxy's deliverable warm-up) finds it at once. + assert [item["operation"] for item in requests] == ["raw-write"] + assert (toolbox / path.lstrip("/")).read_text() == expected result = SpritesFileOperations(env).read_file_raw(path) assert result.content == expected - assert (toolbox / path.lstrip("/")).read_text() == expected + + +def test_published_paths_never_contain_hidden_segments(pair): + home, toolbox, env, requests = pair + path = _spill_summary_to_file(0, "deliverable") + assert not any(segment.startswith(".") for segment in path.strip("/").split("/")) + assert SPRITES_CACHE_ROOT == f"{OMNIO_TOOLBOX_CACHE_BASE}/cache" + + +def test_cold_publication_creates_transport_before_returning_path(pair, monkeypatch): + from tools import file_tools, terminal_tool + from tools.credential_files import publish_cache_path + + home, toolbox, env, requests = pair + monkeypatch.setattr(terminal_tool, "_active_environments", {}) + monkeypatch.setattr(file_tools, "_file_ops_cache", {}) + monkeypatch.setenv("OMNIO_TOOLBOX_URL", env.toolbox_url) + monkeypatch.setenv("OMNIO_TOOLBOX_BEARER", env.bearer_token) + monkeypatch.setenv("OMNIO_TOOLBOX_BRAND", env.brand) + monkeypatch.setenv("TERMINAL_CWD", "/brand") + monkeypatch.setattr(terminal_tool, "_start_cleanup_thread", lambda: None) + # Session shell setup is unrelated to publishing: the file transport and + # environment factory remain real, with no pre-created active environment. + monkeypatch.setattr(SpritesEnvironment, "init_session", lambda self: None) + shot = home / "cache/screenshots/first.png" + shot.parent.mkdir(parents=True) + shot.write_bytes(b"screenshot bytes") + + path = publish_cache_path(str(shot)) + + assert (toolbox / path.lstrip("/")).read_bytes() == shot.read_bytes() + assert [item["operation"] for item in requests] == ["raw-write"] + + +@pytest.mark.parametrize("filename,mime", [ + ("fresh.mp4", "video/mp4"), + ("fresh.mp3", "audio/mpeg"), + ("fresh.txt", "text/plain"), +]) +def test_gateway_attachments_are_published_before_direct_consumers(pair, filename, mime): + from gateway.platforms.base import cache_media_bytes + + home, toolbox, env, requests = pair + data = b"new attachment content" + + media = cache_media_bytes(data, filename=filename, mime_type=mime) + + assert media is not None + assert (toolbox / media.path.lstrip("/")).read_bytes() == data + assert [item["operation"] for item in requests] == ["raw-write"] + + +@pytest.mark.parametrize("offset", [1, 2]) +def test_paged_read_bounds_memory_for_long_selected_and_skipped_lines(offset): + import tracemalloc + from tools.tool_output_limits import get_max_line_length + + class LargeLineEnvironment: + cwd = "/brand" + + def stream_file_bytes(self, path): + chunk = b"x" * (1024 * 1024) + for _ in range(24): + yield chunk + yield b"\nrequested line\n" + + tracemalloc.start() + try: + result = SpritesFileOperations(LargeLineEnvironment())._read_large_text_page( + "/tmp/long-line.txt", offset, 1, + ) + _, peak = tracemalloc.get_traced_memory() + finally: + tracemalloc.stop() + + assert result.error is None + assert result.total_lines == 2 + assert result.file_size == 24 * 1024 * 1024 + len(b"\nrequested line\n") + if offset == 1: + assert result.content == "1|" + "x" * get_max_line_length() + "... [truncated]" + else: + assert result.content == "2|requested line" + assert peak < 12 * 1024 * 1024 + + +def test_publish_flush_failure_falls_back_to_read_time_sync(pair, monkeypatch): + home, toolbox, env, requests = pair + original = env._write_raw_artifact + attempts = [] + + def flaky(path, data): + attempts.append(path) + if len(attempts) == 1: + raise SpritesToolboxError("Artifact transfer failed") + return original(path, data) + + monkeypatch.setattr(env, "_write_raw_artifact", flaky) + path = _spill_summary_to_file(0, "eventually") + assert path.startswith("/tmp/omnio-session/cache/delegation/") # publish never returns a host path + assert len(attempts) == 1 and not requests # the publish-time push failed quietly + assert SpritesFileOperations(env).read_file_raw(path).content == "eventually" + assert len(attempts) == 2 + + +@pytest.mark.parametrize("chunk_size", [1, 3, 7]) +def test_streamed_pages_preserve_utf8_bom_crlf_and_empty_lines(chunk_size): + data = "\ufefffirst 🧪\r\n\r\nlast é".encode() + + class SplitEnvironment: + cwd = "/brand" + + def stream_file_bytes(self, path): + for index in range(0, len(data), chunk_size): + yield data[index:index + chunk_size] + + result = SpritesFileOperations(SplitEnvironment())._read_large_text_page( + "/tmp/utf8.txt", 1, 3, + ) + assert result.error is None + assert result.content == "1|first 🧪\n2|\n3|last é" + assert result.total_lines == 3 + assert result.file_size == len(data) + + +@pytest.mark.parametrize("streaming", [False, True]) +def test_raw_reads_normalize_transport_failures(pair, monkeypatch, streaming): + from tools.environments import sprites + + _, _, env, _ = pair + + def disconnected(*args, **kwargs): + raise ConnectionResetError("connection lost") + + monkeypatch.setattr(sprites._URL_OPENER, "open", disconnected) + with pytest.raises(SpritesToolboxError, match="unreachable"): + if streaming: + list(env.stream_file_bytes("/tmp/example.txt")) + else: + env.read_file_bytes("/tmp/example.txt", max_bytes=100) def test_live_log_refreshes_before_each_parent_read(pair): home, toolbox, env, requests = pair _, writers, paths = create_live_transcripts([{"goal": "write"}]) - assert paths[0].startswith("/tmp/.omnio-session/cache/delegation/") + assert paths[0].startswith("/tmp/omnio-session/cache/delegation/") writers[0].assistant_text("FIRST OBSERVATION") assert "FIRST OBSERVATION" in SpritesFileOperations(env).read_file_raw(paths[0]).content writers[0].assistant_text("FINAL OBSERVATION") @@ -115,36 +283,223 @@ def test_profile_credentials_other_caches_and_symlinks_are_not_transferred(pair) cache = home / "cache" / "delegation" (cache / "secret.json").symlink_to(secret) (cache / "outside").symlink_to(home, target_is_directory=True) + # Files at the cache ROOT (model metadata, encrypted secret caches) are + # not part of the projected set: only the listed subdirectories cross. (home / "cache" / "other.json").write_text("private") - env.sync_delegation_artifacts() + (home / "cache" / "bws_cache.enc.json").write_text("encrypted") + env.sync_cache_files() assert [item["path"] for item in requests] == [path] assert to_agent_visible_cache_path(str(secret)) == str(secret) assert to_agent_visible_cache_path(str(cache / ".." / "other.json")) == str(cache / ".." / "other.json") +def test_every_cache_subdirectory_maps_to_the_toolbox_and_back(pair): + home, toolbox, env, requests = pair + for subpath, _old in _CACHE_DIRS: + host = home / subpath / "nested" / "artifact.bin" + host.parent.mkdir(parents=True, exist_ok=True) + host.write_bytes(b"\x00\x01binary\xff") + agent = to_agent_visible_cache_path(str(host)) + assert agent == f"{SPRITES_CACHE_ROOT}/{subpath.removeprefix('cache/')}/nested/artifact.bin" + assert from_agent_visible_cache_path(agent) == str(host) + env.sync_cache_files() + landed = sorted(item["path"] for item in requests) + assert landed == sorted( + f"{SPRITES_CACHE_ROOT}/{subpath.removeprefix('cache/')}/nested/artifact.bin" + for subpath, _old in _CACHE_DIRS + ) + for subpath, _old in _CACHE_DIRS: + copy = toolbox / f"{SPRITES_CACHE_ROOT}/{subpath.removeprefix('cache/')}/nested/artifact.bin".lstrip("/") + assert copy.read_bytes() == b"\x00\x01binary\xff" + + +def test_symlinked_profile_home_still_maps(tmp_path, monkeypatch): + real = tmp_path / "real-home" + (real / "cache" / "web").mkdir(parents=True) + link = tmp_path / "linked-home" + link.symlink_to(real, target_is_directory=True) + monkeypatch.setenv("HERMES_HOME", str(link)) + monkeypatch.setenv("TERMINAL_ENV", "sprites") + page = link / "cache" / "web" / "page.md" + page.write_text("stored") + assert to_agent_visible_cache_path(str(page)) == f"{SPRITES_CACHE_ROOT}/web/page.md" + assert to_agent_visible_cache_path(str(page.resolve())) == f"{SPRITES_CACHE_ROOT}/web/page.md" + + +def test_paths_are_not_translated_on_other_backends(tmp_path, monkeypatch): + home = tmp_path / "home" + (home / "cache" / "web").mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(home)) + page = home / "cache" / "web" / "page.md" + page.write_text("stored") + for backend in ("local", "ssh", "modal"): + monkeypatch.setenv("TERMINAL_ENV", backend) + assert to_agent_visible_cache_path(str(page)) == str(page) + assert from_agent_visible_cache_path(f"{SPRITES_CACHE_ROOT}/web/page.md") == f"{SPRITES_CACHE_ROOT}/web/page.md" + + +def test_oversized_cache_files_stay_on_the_harness(pair, monkeypatch): + home, toolbox, env, requests = pair + monkeypatch.setattr(file_sync, "SPRITES_CACHE_FILE_MAX_BYTES", 1024) + videos = home / "cache" / "videos" + videos.mkdir(parents=True) + (videos / "small.mp4").write_bytes(b"v" * 512) + (videos / "huge.mp4").write_bytes(b"v" * 4096) + assert [remote for _host, remote in iter_sprites_cache_files()] == [f"{SPRITES_CACHE_ROOT}/videos/small.mp4"] + env.sync_cache_files() + assert [item["path"] for item in requests] == [f"{SPRITES_CACHE_ROOT}/videos/small.mp4"] + + +def test_text_artifacts_are_redacted_and_media_is_byte_exact(pair): + home, toolbox, env, requests = pair + secret = "ghp_" + "b" * 36 + web = home / "cache" / "web" + web.mkdir(parents=True) + (web / "page.md").write_text(f"title\ntoken {secret}\n") + shots = home / "cache" / "screenshots" + shots.mkdir(parents=True) + png = b"\x89PNG\r\n\x1a\n" + bytes(range(256)) + secret.encode() + (shots / "shot.png").write_bytes(png) + env.sync_cache_files() + stored_page = (toolbox / f"{SPRITES_CACHE_ROOT}/web/page.md".lstrip("/")).read_text() + assert secret not in stored_page and "title" in stored_page + assert (toolbox / f"{SPRITES_CACHE_ROOT}/screenshots/shot.png".lstrip("/")).read_bytes() == png + + +def test_raw_reads_under_the_cache_refresh_the_projection_first(pair): + home, toolbox, env, requests = pair + shots = home / "cache" / "screenshots" + shots.mkdir(parents=True) + (shots / "shot.png").write_bytes(b"\x89PNG first") + agent_path = to_agent_visible_cache_path(str(shots / "shot.png")) + assert env.read_file_bytes(agent_path, max_bytes=64) == b"\x89PNG first" + (shots / "shot.png").write_bytes(b"\x89PNG second, longer") + assert env.read_file_bytes(agent_path, max_bytes=64) == b"\x89PNG second, longer" + # A read outside the projected cache does not trigger a sync round. + before = len(requests) + (toolbox / "brand").mkdir() + (toolbox / "brand" / "notes.txt").write_text("brand") + assert env.read_file_bytes("/brand/notes.txt", max_bytes=64) == b"brand" + assert [item["operation"] for item in requests[before:]] == ["raw-read"] + + +def test_web_and_browser_footers_name_the_toolbox_path(pair): + home, toolbox, env, requests = pair + from tools.web_tools import _truncate_with_footer + from tools.browser_tool import _truncate_snapshot + + page, truncated = _truncate_with_footer("line\n" * 5000, "https://example.com/doc", 2000) + assert truncated + assert f"{SPRITES_CACHE_ROOT}/web/" in page + assert str(home) not in page + snapshot = _truncate_snapshot("- element\n" * 5000, max_chars=2000) + assert f"{SPRITES_CACHE_ROOT}/web/browser-snapshot-" in snapshot + assert str(home) not in snapshot + + def test_large_artifact_uses_raw_upload_without_truncation(pair): home, toolbox, env, requests = pair expected = "result line\n" * 210_000 + "FINAL RECORD" path = _spill_summary_to_file(0, expected) - env.sync_delegation_artifacts() + env.sync_cache_files() assert (toolbox / path.lstrip("/")).read_text() == expected assert requests[0]["operation"] == "raw-write" + assert SpritesFileOperations(env).read_file_raw(path).content == expected + + +def test_stdout_recovery_is_readable_through_toolbox_file_tools(pair): + home, toolbox, env, requests = pair + secret = "ghp_" + "a" * 36 + expected = "before\n" * 200_000 + "MIDDLE_RECORD\n" + secret + "\nafter\n" * 200_000 + output, metadata = _truncate_stdout_text(expected) + assert "MIDDLE_RECORD" not in output + path = metadata["stdout_spill_path"] + assert path.startswith("/tmp/omnio-session/cache/exec/") + # Host-side canonical copy, projected to the Toolbox as the path is published. + assert len(list(home.glob("cache/exec/*"))) == 1 + assert [item["operation"] for item in requests] == ["raw-write"] + result = SpritesFileOperations(env).read_file_raw(path) + assert "MIDDLE_RECORD" in result.content + assert secret not in result.content + assert result.content.startswith("before\n") and result.content.endswith("after\n") + assert (toolbox / path.lstrip("/")).read_text() == result.content + assert requests[0]["operation"] == "raw-write" + assert metadata["stdout_spill_truncated"] is False + page = SpritesFileOperations(env).read_file(path, offset=200_001, limit=1) + assert page.error is None + assert "MIDDLE_RECORD" in page.content + assert "before" not in page.content and "after" not in page.content + assert page.truncated is True + assert "offset=200002" in page.hint + + +def test_text_above_the_raw_ceiling_pages_by_streaming(pair): + home, toolbox, env, requests = pair + path = "/tmp/omnio-session/cache/exec/too-large.txt" + local = toolbox / path.lstrip("/") + local.parent.mkdir(parents=True) + # ~6.9 MB, 600,000 lines: above the 2 MiB JSON read AND the 5 MiB whole-read ceiling. + local.write_text("".join(f"line {i:06d}\n" for i in range(600_000))) + ops = SpritesFileOperations(env) + + whole = ops.read_file_raw(path) + assert whole.error and "exceeds" in whole.error and "offset/limit" in whole.error + assert not whole.content + + middle = ops.read_file(path, offset=300_001, limit=2) + assert middle.error is None + assert middle.content == "300001|line 300000\n300002|line 300001" + assert middle.total_lines == 600_000 and middle.truncated is True + assert middle.hint == "Use offset=300003 to continue reading" + + tail = ops.read_file(path, offset=599_999, limit=10) + assert tail.content == "599999|line 599998\n600000|line 599999" + assert tail.truncated is False and tail.hint is None + + beyond = ops.read_file(path, offset=700_000, limit=10) + assert beyond.error and "beyond the end" in beyond.error + + # The JSON reads were all refused as too large; every byte the pages and + # the whole-read attempt returned came from the streaming GET. + assert sum(1 for item in requests if item["operation"] == "raw-read") == 4 + assert not any(item["operation"] == "raw-write" for item in requests) + + +def test_failed_stdout_transfer_surfaces_as_a_read_error_not_a_host_path(pair, monkeypatch): + home, toolbox, env, requests = pair + + def failed(*args): + raise SpritesToolboxError("Artifact transfer failed") + + monkeypatch.setattr(env, "_write_raw_artifact", failed) + output, metadata = _truncate_stdout_text("before\n" * 15000 + "END") + assert output.endswith("END") + path = metadata["stdout_spill_path"] + assert path.startswith("/tmp/omnio-session/cache/exec/") + assert str(home) not in path + result = SpritesFileOperations(env).read_file_raw(path) + assert result.error and "temporarily unavailable" in result.error + assert not requests def test_invalid_artifact_destination_is_rejected(pair): home, toolbox, env, requests = pair - with pytest.raises(SpritesToolboxError, match="outside delegation"): - env._upload_delegation_artifact(str(home / "auth.json"), SPRITES_DELEGATION_ROOT + "/../../auth.json") + with pytest.raises(SpritesToolboxError, match="outside the Toolbox cache"): + env._upload_cache_file(str(home / "auth.json"), SPRITES_DELEGATION_ROOT + "/../../../auth.json") assert not requests def test_failed_transfer_is_reported_and_retried(pair, monkeypatch): home, toolbox, env, requests = pair - path = _spill_summary_to_file(0, "complete") - original = env.write_file_content - monkeypatch.setattr(env, "write_file_content", lambda *args: False) + original = env._write_raw_artifact + + def failed(*args): + raise SpritesToolboxError("Artifact transfer failed") + + monkeypatch.setattr(env, "_write_raw_artifact", failed) + path = _spill_summary_to_file(0, "complete") # publish-time push fails quietly with pytest.raises(SpritesToolboxError, match="transfer failed"): - env.sync_delegation_artifacts() - monkeypatch.setattr(env, "write_file_content", original) - env.sync_delegation_artifacts() + env.sync_cache_files() + monkeypatch.setattr(env, "_write_raw_artifact", original) + env.sync_cache_files() assert (toolbox / path.lstrip("/")).read_text() == "complete" diff --git a/tests/tools/test_skill_runtime_contract.py b/tests/tools/test_skill_runtime_contract.py new file mode 100644 index 0000000000000..5511c412c515b --- /dev/null +++ b/tests/tools/test_skill_runtime_contract.py @@ -0,0 +1,256 @@ +"""Skill viewing, management, and execution share a backend path contract.""" +import json +from pathlib import Path + +import pytest + +from agent import skill_utils +from tools import skills_tool, skill_manager_tool + + +@pytest.fixture +def catalog(tmp_path, monkeypatch): + root = tmp_path / 'profile' / 'skills' + root.mkdir(parents=True) + monkeypatch.setenv('HERMES_HOME', str(root.parent)) + monkeypatch.setenv('TERMINAL_ENV', 'local') + monkeypatch.setattr(skills_tool, 'SKILLS_DIR', root) + monkeypatch.setattr(skill_manager_tool, 'SKILLS_DIR', root) + monkeypatch.setattr(skill_utils, 'get_external_skills_dirs', lambda: []) + monkeypatch.setattr(skill_utils, 'get_all_skills_dirs', lambda: [root]) + return root + + +def write_skill(root, relative, body='Use the helper.'): + directory = root / relative + directory.mkdir(parents=True) + (directory / 'SKILL.md').write_text( + '---\nname: helper\ndescription: Test helper\n---\n# Usage\n' + body + ) + return directory + + +@pytest.mark.parametrize('consumer', ['command', 'file']) +def test_warm_toolbox_receives_new_helper_before_use(catalog, monkeypatch, consumer): + import threading + from tools.environments import file_sync + from tools.environments.sprites import SpritesEnvironment, SpritesFileOperations + + directory = write_skill(catalog, 'ops/helper') + remote = {} + monkeypatch.setattr(file_sync, '_monotonic', lambda: 100.0) + env = SpritesEnvironment.__new__(SpritesEnvironment) + env.cwd = '/brand' + env._cache_sync_lock = threading.Lock() + env._sync_manager = file_sync.FileSyncManager( + get_files_fn=lambda: file_sync.iter_sprites_sync_files('/skills'), + upload_fn=lambda host, target: remote.update({target: Path(host).read_bytes()}), + delete_fn=lambda paths: [remote.pop(path, None) for path in paths], + ) + env._sync_manager.sync(force=True) + helper = directory / 'run.py' + helper.write_text("print('new helper')\n") + target = '/skills/ops/helper/run.py' + # Still inside the normal sync throttle window, with a warm environment. + assert target not in remote + if consumer == 'command': + env._before_execute() + else: + env.file_request = lambda payload: {'content': remote[payload['path']].decode()} + result = SpritesFileOperations(env)._files({'operation': 'read', 'path': target}) + assert result['content'] == helper.read_text() + assert remote[target] == helper.read_bytes() + + +def test_failed_skill_sync_prevents_execution_with_stale_helpers(catalog): + import threading + from tools.environments.file_sync import FileSyncManager, iter_sprites_sync_files + from tools.environments.sprites import SpritesEnvironment, SpritesToolboxError + + write_skill(catalog, 'helper') + env = SpritesEnvironment.__new__(SpritesEnvironment) + env._cache_sync_lock = threading.Lock() + def fail_upload(host, target): + raise SpritesToolboxError('offline') + env._sync_manager = FileSyncManager( + get_files_fn=lambda: iter_sprites_sync_files('/skills'), + upload_fn=fail_upload, + delete_fn=lambda paths: None, + ) + with pytest.raises(SpritesToolboxError, match='offline'): + env._before_execute() + + +def test_skill_view_publishes_runtime_directory_but_internal_load_keeps_host(catalog, monkeypatch): + directory = write_skill(catalog, 'ops/helper', 'python ${HERMES_SKILL_DIR}/scripts/run.py') + scripts = directory / 'scripts' + scripts.mkdir() + (scripts / 'run.py').write_text("print('helper executed')\n") + monkeypatch.setenv('TERMINAL_ENV', 'sprites') + result = json.loads(skills_tool.skill_view('ops/helper')) + assert result['skill_dir'] == '/skills/ops/helper' + assert '/skills/ops/helper/scripts/run.py' in result['content'] + from tools.credential_files import iter_skills_files + assert any(f['container_path'] == result['skill_dir'] + '/scripts/run.py' + for f in iter_skills_files('/skills')) + internal = json.loads(skills_tool.skill_view('ops/helper', preprocess=False)) + assert internal['skill_dir'] == str(directory) + + +def test_categorized_skill_management_edits_host_file(catalog): + directory = write_skill(catalog, 'ops/helper') + result = json.loads(skill_manager_tool.skill_manage( + action='patch', name='ops/helper', old_string='Use the helper.', new_string='Run the helper.' + )) + assert result['success'], result + assert 'Run the helper.' in (directory / 'SKILL.md').read_text() + + +def test_directory_request_lists_available_support_files(catalog): + directory = write_skill(catalog, 'helper') + (directory / 'references').mkdir() + (directory / 'references' / 'guide.md').write_text('Guide') + result = json.loads(skills_tool.skill_view('helper', file_path='references')) + assert result['success'] is False + assert 'references/guide.md' in result['available_files']['references'] + + +def test_identical_same_root_copy_prefers_shallow_skill(catalog): + directory = write_skill(catalog, 'helper') + write_skill(catalog, 'ops/helper') + result = json.loads(skills_tool.skill_view('helper')) + assert result['success'], result + assert result['skill_dir'] == str(directory) + + +def test_distinct_same_root_skills_still_refuse_collision(catalog): + write_skill(catalog, 'helper', 'Custom implementation') + write_skill(catalog, 'ops/helper', 'Built-in implementation') + result = json.loads(skills_tool.skill_view('helper')) + assert result['success'] is False + assert 'Ambiguous' in result['error'] + + +@pytest.mark.parametrize('value', ['~/brand/config', '$HOME/brand/config', '${HOME}/brand/config']) +def test_skill_config_uses_toolbox_home(catalog, monkeypatch, value): + monkeypatch.setenv('TERMINAL_ENV', 'sprites') + monkeypatch.setattr(skill_utils, '_load_raw_config', lambda: {}) + result = skill_utils.resolve_skill_config_values([{'key': 'path', 'default': value}]) + assert result['path'] == '/home/brand/config' + + +def test_local_skill_directory_is_unchanged(catalog): + directory = write_skill(catalog, 'helper') + assert json.loads(skills_tool.skill_view('helper'))['skill_dir'] == str(directory) + + +def test_external_skill_runtime_path_matches_sync_destination(catalog, tmp_path, monkeypatch): + from agent.skill_path_mapping import map_skill_dir_for_backend + from tools.credential_files import iter_skills_files + external = tmp_path / 'external' + directory = write_skill(external, 'ops/helper') + monkeypatch.setattr(skill_utils, 'get_external_skills_dirs', lambda: [external]) + monkeypatch.setenv('TERMINAL_ENV', 'sprites') + runtime = map_skill_dir_for_backend(directory) + assert runtime == '/skills/external_skills/0/ops/helper' + assert any(f['container_path'] == runtime + '/SKILL.md' for f in iter_skills_files('/skills')) + + +def test_other_brand_skill_path_is_not_translated(catalog, tmp_path, monkeypatch): + from agent.skill_path_mapping import map_skill_dir_for_backend + other = write_skill(tmp_path / 'other-profile' / 'skills', 'helper') + monkeypatch.setenv('TERMINAL_ENV', 'sprites') + assert map_skill_dir_for_backend(other) == str(other) + + +def test_equal_depth_identical_copies_remain_ambiguous(catalog): + write_skill(catalog, 'one/helper') + write_skill(catalog, 'two/helper') + assert json.loads(skills_tool.skill_view('helper'))['success'] is False + + +def test_identical_cross_root_copies_remain_ambiguous(catalog, tmp_path, monkeypatch): + write_skill(catalog, 'helper') + external = tmp_path / 'external' + write_skill(external, 'nested/helper') + monkeypatch.setattr(skill_utils, 'get_external_skills_dirs', lambda: [external]) + assert json.loads(skills_tool.skill_view('helper'))['success'] is False + + +def test_slash_command_advertises_runtime_support_paths(catalog, monkeypatch): + from agent.skill_commands import _build_skill_message + directory = write_skill(catalog, 'ops/helper') + monkeypatch.setenv('TERMINAL_ENV', 'sprites') + result = _build_skill_message( + {'content': 'python ${HERMES_SKILL_DIR}/scripts/run.py', + 'linked_files': {'scripts': ['scripts/run.py']}}, directory, 'Activated' + ) + assert '[Skill directory: /skills/ops/helper]' in result + assert '/skills/ops/helper/scripts/run.py' in result + assert str(directory) not in result + assert 'name="ops/helper"' in result + + +def test_inline_shell_keeps_host_paths_while_instructions_use_toolbox(catalog, monkeypatch): + from agent.skill_preprocessing import preprocess_skill_content + directory = write_skill(catalog, 'helper') + (directory / 'host.txt').write_text('HOST_CONTENT') + monkeypatch.setenv('TERMINAL_ENV', 'sprites') + content = '!`cat "${HERMES_SKILL_DIR}/host.txt"`\nRun ${HERMES_SKILL_DIR}/scripts/helper.py' + rendered = preprocess_skill_content(content, directory, skills_cfg={'inline_shell': True}) + assert rendered == 'HOST_CONTENT\nRun /skills/helper/scripts/helper.py' + + +def test_docker_skill_path_uses_the_existing_mount(catalog, monkeypatch): + from agent.skill_path_mapping import map_skill_dir_for_backend + directory = write_skill(catalog, 'ops/helper') + monkeypatch.setenv('TERMINAL_ENV', 'docker') + assert map_skill_dir_for_backend(directory) == '/root/.hermes/skills/ops/helper' + + +def test_skill_config_preserves_non_home_environment_variables(catalog, monkeypatch): + monkeypatch.setenv('TERMINAL_ENV', 'sprites') + monkeypatch.setenv('ASSET_ROOT', '/assets') + monkeypatch.setattr(skill_utils, '_load_raw_config', lambda: {}) + assert skill_utils.resolve_skill_config_values([ + {'key': 'path', 'default': '${ASSET_ROOT}/config'}, + ]) == {'path': '/assets/config'} + + +def test_categorized_external_skill_can_be_managed(catalog, tmp_path, monkeypatch): + external = tmp_path / 'external' + directory = write_skill(external, 'ops/helper') + monkeypatch.setattr(skill_utils, 'get_all_skills_dirs', lambda: [catalog, external]) + assert skill_manager_tool._find_skill('ops/helper') == {'path': directory} + + +def test_missing_reference_still_returns_real_inventory(catalog): + directory = write_skill(catalog, 'helper') + (directory / 'references').mkdir() + (directory / 'references' / 'actual.md').write_text('Actual instructions') + result = json.loads(skills_tool.skill_view('helper', file_path='references/missing.md')) + assert result['success'] is False + assert result['available_files']['references'] == ['references/actual.md'] + + +@pytest.mark.parametrize('name', ['omnio/helper', 'omnio:helper']) +def test_stale_category_suggests_installed_name_without_silent_fallback(catalog, name): + write_skill(catalog, 'marketing/research/helper') + result = json.loads(skills_tool.skill_view(name)) + assert result['success'] is False + assert result['matching_skills'][0]['name'] == 'helper' + assert json.loads(skills_tool.skill_view(result['matching_skills'][0]['name']))['success'] + + +def test_path_mapping_does_not_replace_a_live_sanitized_mount(catalog, tmp_path, monkeypatch): + from agent.skill_path_mapping import map_skill_dir_for_backend + from tools import credential_files + directory = write_skill(catalog, 'helper') + (catalog / 'excluded-link').symlink_to(tmp_path / 'outside') + monkeypatch.setattr(credential_files, '_safe_skills_tempdir', None) + monkeypatch.setenv('TERMINAL_ENV', 'docker') + mounted = credential_files.get_skills_directory_mount()[0] + mounted_file = Path(mounted['host_path']) / 'helper' / 'SKILL.md' + assert mounted_file.is_file() + assert map_skill_dir_for_backend(directory) == '/root/.hermes/skills/helper' + assert mounted_file.is_file(), 'Rendering a path must not delete an existing bind mount source' diff --git a/tools/browser_tool.py b/tools/browser_tool.py index 57f510c5c5207..c3a041570039e 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -3218,6 +3218,24 @@ def _store_full_snapshot(snapshot_text: str) -> Optional[str]: return None +def _agent_visible_stored_path(stored_path: Optional[str]) -> Optional[str]: + """Render a stored cache path where the AGENT's read_file sees it. + + ``read_file`` runs inside the active terminal backend, so a footer that + names the host path dangles on Docker and on the Omnio Toolbox (upstream + #72389). ``publish_cache_path`` is a no-op on backends that keep host + paths and pushes the file first on backends that copy the cache. + """ + if not stored_path: + return stored_path + try: + from tools.credential_files import publish_cache_path + + return publish_cache_path(stored_path) + except Exception: # noqa: BLE001 — a failed translation must not lose the pointer + return stored_path + + def _extract_relevant_content( snapshot_text: str, user_task: Optional[str] = None @@ -3228,7 +3246,7 @@ def _extract_relevant_content( the pointer lets the agent read anything the summary dropped). Falls back to simple truncation when no auxiliary text model is configured. """ - stored_path = _store_full_snapshot(snapshot_text) + stored_path = _agent_visible_stored_path(_store_full_snapshot(snapshot_text)) stored_note = ( f'\n\n[Summarized from a {len(snapshot_text):,}-char snapshot. Full snapshot ' f'saved to: {stored_path} — read it with read_file if anything is missing.]' @@ -3304,7 +3322,7 @@ def _truncate_snapshot(snapshot_text: str, max_chars: int = SNAPSHOT_SUMMARIZE_T if len(snapshot_text) <= max_chars: return snapshot_text - stored_path = _store_full_snapshot(snapshot_text) + stored_path = _agent_visible_stored_path(_store_full_snapshot(snapshot_text)) lines = snapshot_text.split('\n') result: list[str] = [] diff --git a/tools/code_execution_tool.py b/tools/code_execution_tool.py index 0458e147738a2..5206610d464d9 100644 --- a/tools/code_execution_tool.py +++ b/tools/code_execution_tool.py @@ -114,6 +114,7 @@ def _sandbox_module_name() -> str: DEFAULT_MAX_TOOL_CALLS = 50 MAX_STDOUT_BYTES = 50_000 # 50 KB MAX_STDERR_BYTES = 10_000 # 10 KB +MAX_SPILLED_STDOUT_BYTES = 5_000_000 def _assemble_stdout_result( @@ -152,13 +153,96 @@ def _assemble_stdout_result( if truncated: metadata["warning"] = ( "execute_code stdout was truncated; the script did run, but only " - "the captured head/tail output is included. Re-run only with " - "narrower output if the omitted data is required." + "the captured head/tail output is included." ) return stdout_text, metadata -def _truncate_stdout_text(stdout_text: str) -> Tuple[str, Dict[str, Any]]: +def _sanitize_stdout(stdout_text: str) -> str: + from agent.redact import redact_sensitive_text + from tools.ansi_strip import strip_ansi + + return redact_sensitive_text(strip_ansi(stdout_text), code_file=True) + + +def _spill_full_stdout(stdout_text: str, *, env=None) -> str: + """Save recovery output where this execution's file tools can read it. + + Adapted from upstream #97043. Local execution and Sprites use the host + cache and its existing publisher. Other remote executions pass their + environment so recovery does not rely on cache mounts or path translation. + Callers sanitize and bound the text before any disk or transport write. + """ + if env is not None: + publish = getattr(env, "write_output_artifact", None) + if callable(publish): + return publish(stdout_text) + path = f"{_env_temp_dir(env)}/stdout_{uuid.uuid4().hex}.txt" + _ship_file_to_remote(env, path, stdout_text) + return path + + from hermes_constants import get_hermes_dir, get_hermes_home + + spill_dir = get_hermes_dir("cache/exec", "exec_spill") + # Refuse links in the cache path before mkdir can follow them. The profile + # root itself remains the caller's configured location. + home = get_hermes_home() + for component in (spill_dir, *spill_dir.parents): + if component == home: + break + if component.is_symlink(): + raise OSError("stdout cache contains a symlink") + spill_dir.mkdir(parents=True, exist_ok=True, mode=0o700) + # Exclusive, unpredictable names avoid overwriting a prior artifact or + # following a pre-existing file symlink. mkstemp creates private files. + fd, path = tempfile.mkstemp(prefix="stdout_", suffix=".txt", dir=spill_dir) + with os.fdopen(fd, "w", encoding="utf-8") as output: + output.write(stdout_text) + from tools.credential_files import publish_cache_path + + return publish_cache_path(path) + + +def _add_stdout_spill(metadata: Dict[str, Any], captured: bytes, *, + total_bytes: int, env=None) -> None: + """Attach best-effort recovery without changing the execution outcome.""" + if not metadata["stdout_truncated"]: + return + try: + bounded = captured[:MAX_SPILLED_STDOUT_BYTES] + partial = total_bytes > len(bounded) + if partial: + # A cut credential may no longer match the redactor. Keep complete + # lines when capture stopped at the storage ceiling. + bounded = bounded[:bounded.rfind(b"\n") + 1] + sanitized = _sanitize_stdout(bounded.decode("utf-8", errors="replace")) + data = sanitized.encode("utf-8") + if len(data) > MAX_SPILLED_STDOUT_BYTES: + partial = True + marker = "\n[OUTPUT CAPTURE LIMIT: this artifact is partial.]\n" if partial else "" + budget = MAX_SPILLED_STDOUT_BYTES - len(marker.encode("utf-8")) + text = data[:budget].decode("utf-8", errors="ignore") + if not text: + raise OSError("No complete output fits in the recovery artifact") + path = _spill_full_stdout(text + marker, env=env) + if not isinstance(path, str) or not path: + raise OSError("Backend did not return an output artifact path") + metadata["stdout_spill_path"] = path + metadata["stdout_spill_truncated"] = partial + qualifier = "A partial copy of captured stdout" if partial else "Captured stdout" + metadata["warning"] += ( + f" {qualifier} was saved to {path}; page the artifact instead of " + "re-running the script." + ) + except Exception: + logger.debug("Could not publish execute_code stdout recovery", exc_info=True) + metadata["warning"] += ( + " The recovery artifact is unavailable. Do not repeat commands " + "with side effects just to recover output." + ) + + +def _truncate_stdout_text(stdout_text: str, *, env=None) -> Tuple[str, Dict[str, Any]]: """Cap a complete stdout string by bytes using the same head/tail policy.""" stdout_bytes = stdout_text.encode("utf-8", errors="replace") if len(stdout_bytes) <= MAX_STDOUT_BYTES: @@ -166,11 +250,13 @@ def _truncate_stdout_text(stdout_text: str) -> Tuple[str, Dict[str, Any]]: head_bytes = int(MAX_STDOUT_BYTES * 0.4) tail_bytes = MAX_STDOUT_BYTES - head_bytes - return _assemble_stdout_result( + text, metadata = _assemble_stdout_result( stdout_bytes[:head_bytes], stdout_bytes[-tail_bytes:], total_bytes=len(stdout_bytes), ) + _add_stdout_spill(metadata, stdout_bytes, total_bytes=len(stdout_bytes), env=env) + return text, metadata # Environment variable scrubbing rules (shared between the local + remote # backends). Secret-substring block is applied first; anything left must @@ -1511,7 +1597,12 @@ def _execute_remote( # --- Post-process output (same as local path) --- - stdout_text, stdout_metadata = _truncate_stdout_text(stdout_text) + # Sprites publishes via the shared cache projection. Keep #114's direct + # transfer for all other remote backends, including already-warm containers + # that may have been created before the stdout cache directory existed. + stdout_text, stdout_metadata = _truncate_stdout_text( + stdout_text, env=None if env_type == "sprites" else env, + ) # Strip ANSI escape sequences from tools.ansi_strip import strip_ansi @@ -1836,6 +1927,7 @@ def _drain(pipe, chunks, max_bytes): logger.debug("Error reading process output: %s", e, exc_info=True) stdout_total_bytes = [0] # mutable ref for total bytes seen + stdout_recovery = bytearray() def _drain_head_tail(pipe, head_chunks, tail_chunks, head_bytes, tail_bytes, total_ref): """Drain stdout keeping both head and tail data.""" @@ -1849,6 +1941,11 @@ def _drain_head_tail(pipe, head_chunks, tail_chunks, head_bytes, tail_bytes, tot if not data: break total_ref[0] += len(data) + # Retain the middle before the head/tail display discards + # it. Memory stays bounded even for an infinite writer. + remaining = MAX_SPILLED_STDOUT_BYTES - len(stdout_recovery) + if remaining > 0: + stdout_recovery.extend(data[:remaining]) # Fill head buffer first if head_collected < head_bytes: keep = min(len(data), head_bytes - head_collected) @@ -1928,6 +2025,10 @@ def _drain_head_tail(pipe, head_chunks, tail_chunks, head_bytes, tail_bytes, tot b"".join(stdout_tail_chunks), total_bytes=stdout_total_bytes[0], ) + _add_stdout_spill( + stdout_metadata, bytes(stdout_recovery), + total_bytes=stdout_total_bytes[0], + ) exit_code = proc.returncode if proc.returncode is not None else -1 duration = round(time.monotonic() - exec_start, 2) @@ -2363,7 +2464,10 @@ def build_execute_code_schema(enabled_sandbox_tools: set = None, f"{_timeout_s // 60}-minute" if _timeout_s % 60 == 0 else f"{_timeout_s}s" ) limits_note = ( - f"{_timeout_note} timeout, {MAX_STDOUT_BYTES // 1000}KB stdout cap, " + f"{_timeout_note} timeout, {MAX_STDOUT_BYTES // 1000}KB inline stdout head/tail " + f"(larger captured output is saved to a recovery artifact, up to " + f"{MAX_SPILLED_STDOUT_BYTES // 1_000_000}MB; page the returned path instead " + f"of re-running the script), " f"max {_max_calls} tool calls per script" ) diff --git a/tools/credential_files.py b/tools/credential_files.py index 0cb8d746a8ab5..59f20dcea4d63 100644 --- a/tools/credential_files.py +++ b/tools/credential_files.py @@ -259,17 +259,32 @@ def get_skills_directory_mount( symlinks are present (the common case), the original directory is returned directly with zero overhead. - Returns a list of dicts with ``host_path`` and ``container_path`` keys. - The local skills dir mounts at ``/skills``, external dirs + Returns ``host_path`` (possibly sanitized), canonical ``source_path``, + and ``container_path`` for each directory. The local skills dir mounts at ``/skills``, external dirs at ``/external_skills/``. """ + mounts = get_skills_directory_layout(container_base) + for mount in mounts: + mount["host_path"] = _safe_skills_path(Path(mount["source_path"])) + return mounts + + +def get_skills_directory_layout( + container_base: str = "/root/.hermes", +) -> list[Dict[str, str]]: + """Read the source/destination layout without preparing bind mounts. + + Unlike get_skills_directory_mount, this never replaces a sanitized tree + that a running environment may still have mounted. + """ mounts = [] hermes_home = _resolve_hermes_home() skills_dir = hermes_home / "skills" if skills_dir.is_dir(): - host_path = _safe_skills_path(skills_dir) + host_path = str(skills_dir) mounts.append({ "host_path": host_path, + "source_path": str(skills_dir), "container_path": f"{container_base.rstrip('/')}/skills", }) @@ -278,9 +293,10 @@ def get_skills_directory_mount( from agent.skill_utils import get_external_skills_dirs for idx, ext_dir in enumerate(get_external_skills_dirs()): if ext_dir.is_dir(): - host_path = _safe_skills_path(ext_dir) + host_path = str(ext_dir) mounts.append({ "host_path": host_path, + "source_path": str(ext_dir), "container_path": f"{container_base.rstrip('/')}/external_skills/{idx}", }) except ImportError: @@ -400,8 +416,47 @@ def iter_skills_files( ("cache/screenshots", "browser_screenshots"), ("cache/web", "web_cache"), ("cache/delegation", "delegation_cache"), + # execute_code stdout recovery artifacts (upstream #97043). + ("cache/exec", "exec_spill"), ] +# Where the paired Omnio Toolbox sees the harness cache set. The Toolbox has +# no ``~/.hermes``: the harness runs on the Omnio sprite and only projects the +# agent-facing cache subdirectories into the Brand's private ``/tmp``, so +# ``/cache/web/x.md`` is read there as +# ``/tmp/omnio-session/cache/web/x.md``. The root is deliberately not a dot +# directory: files under it are deliverables the reply may hand over as +# ``sandbox:`` links, and the Omnio proxy refuses hidden path segments. Every +# cache producer that hands the model a path goes through +# :func:`publish_cache_path`; the Sprites environment syncs the same set before +# commands and reads. +OMNIO_TOOLBOX_CACHE_BASE = "/tmp/omnio-session" + +def publish_cache_path(host_path: str) -> str: + """Return the path the AGENT should be given for a host cache file, with + the file already readable there. + + On backends that project the cache by copying (the Omnio Toolbox), the + copy is pushed before the path is handed out, including before the first + terminal/file call. Reuse the file tools' lazy environment acquisition so + publication and later reads share the same transport and sync state. + Docker sees its bind mount live; other backends keep their existing paths. + A failed push retains the translated path for a later read-time retry. + """ + agent_path = to_agent_visible_cache_path(host_path) + if _terminal_backend() == "sprites" and agent_path != host_path: + try: + from tools.file_tools import _get_file_ops + + _get_file_ops().env.sync_cache_files() + except Exception: + logger.warning("Cache publication failed; read-time sync will retry", exc_info=True) + return agent_path + + +def _terminal_backend() -> str: + return (os.environ.get("TERMINAL_ENV", "local") or "local").strip().lower() + def get_cache_directory_mounts( container_base: str = "/root/.hermes", @@ -441,12 +496,21 @@ def map_cache_path_to_container( regardless of the host OS. """ path = Path(host_path) + resolved_path: Optional[Path] = None for mount in get_cache_directory_mounts(container_base=container_base): host_dir = Path(mount["host_path"]) try: rel = path.relative_to(host_dir) except ValueError: - continue + # A profile home reached through a symlink (or a caller that + # already resolved its path) must still map: compare the + # resolved forms before giving up on this mount. + try: + if resolved_path is None: + resolved_path = path.resolve() + rel = resolved_path.relative_to(host_dir.resolve()) + except (OSError, ValueError): + continue return posixpath.join(mount["container_path"], rel.as_posix()) return None @@ -458,11 +522,15 @@ def from_agent_visible_cache_path( """Translate a sandbox/container cache path back to its host path. Inverse of :func:`to_agent_visible_cache_path`. Returns the input unchanged - when the active backend is not Docker, or when the path is not under any - auto-mounted cache directory — the caller then treats a still-container - path as "no host file" and falls back to an in-container read. + when the active backend does not project the cache (only Docker and the + Omnio Toolbox do), or when the path is not under any projected cache + directory — the caller then treats a still-container path as "no host + file" and falls back to an in-container read. """ - if os.environ.get("TERMINAL_ENV", "local") != "docker": + backend = _terminal_backend() + if backend == "sprites": + container_base = OMNIO_TOOLBOX_CACHE_BASE + elif backend != "docker": return container_path path = Path(container_path) @@ -481,25 +549,30 @@ def to_agent_visible_cache_path( ) -> str: """Translate a host cache path to its mounted path inside the sandbox. - 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 (Docker and Sprites). + Returns the input unchanged if it is not under any projected cache + directory, or if the active terminal backend does not project the cache. + + * ``docker`` bind-mounts the cache set at *container_base*. + * ``sprites`` (the paired Omnio Toolbox) receives a synced copy of the + cache set under :data:`OMNIO_TOOLBOX_CACHE_BASE`; the host path is + resolved first so a profile home reached through a symlink still maps. + * Every other backend keeps host paths (Modal, Daytona and SSH sync the + cache too, but nothing translates for them in this fork yet). + + The backend is identified by ``TERMINAL_ENV`` (the same env var + ``tools/terminal_tool.py`` reads in ``_get_environment_config``). """ - # Docker mounts caches; Sprites copies only delegation artifacts. Other - # backends (Modal, Daytona) 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") == "sprites": - from hermes_constants import get_hermes_dir - from tools.environments.file_sync import SPRITES_DELEGATION_ROOT - - root = get_hermes_dir("cache/delegation", "delegation_cache") + backend = _terminal_backend() + if backend == "sprites": try: - relative = Path(host_path).resolve().relative_to(root.resolve()) - except ValueError: - return host_path - return posixpath.join(SPRITES_DELEGATION_ROOT, relative.as_posix()) - if os.environ.get("TERMINAL_ENV", "local") != "docker": + resolved = str(Path(host_path).resolve()) + except OSError: + resolved = host_path + mapped = map_cache_path_to_container( + resolved, container_base=OMNIO_TOOLBOX_CACHE_BASE + ) + return mapped if mapped is not None else host_path + if backend != "docker": return host_path mapped = map_cache_path_to_container(host_path, container_base=container_base) @@ -536,4 +609,3 @@ def iter_cache_files( def clear_credential_files() -> None: """Reset the skill-scoped registry (e.g. on session reset).""" _get_registered().clear() - diff --git a/tools/delegate_tool.py b/tools/delegate_tool.py index dc4ce7915398e..8d16e9efe6365 100644 --- a/tools/delegate_tool.py +++ b/tools/delegate_tool.py @@ -1811,8 +1811,8 @@ def _spill_summary_to_file(task_index: int, summary: str) -> Optional[str]: ts = _dt.datetime.now().strftime("%Y%m%d_%H%M%S_%f") path = cache_dir / f"subagent-summary-{task_index}-{ts}.txt" path.write_text(summary, encoding="utf-8") - from tools.credential_files import to_agent_visible_cache_path - return to_agent_visible_cache_path(str(path)) + from tools.credential_files import publish_cache_path + return publish_cache_path(str(path)) except Exception as exc: logger.debug("Failed to spill subagent summary to file: %s", exc) return None diff --git a/tools/delegation_live_log.py b/tools/delegation_live_log.py index 556b88b590edd..95d1bb976e224 100644 --- a/tools/delegation_live_log.py +++ b/tools/delegation_live_log.py @@ -336,8 +336,8 @@ def create_live_transcripts( ) writers.append(w if w.path is not None else None) if w.path is not None: - from tools.credential_files import to_agent_visible_cache_path - paths.append(to_agent_visible_cache_path(str(w.path))) + from tools.credential_files import publish_cache_path + paths.append(publish_cache_path(str(w.path))) if not paths: return None, [None] * n, [] _write_manifest(deleg_id, task_list, paths) diff --git a/tools/environments/file_sync.py b/tools/environments/file_sync.py index ef211ae605055..e0c6e001bf4a5 100644 --- a/tools/environments/file_sync.py +++ b/tools/environments/file_sync.py @@ -108,26 +108,59 @@ def iter_sprites_sync_files( ] -SPRITES_DELEGATION_ROOT = "/tmp/.omnio-session/cache/delegation" - - -def iter_sprites_delegation_files() -> list[tuple[str, str]]: - """Only regular delegation artifacts may cross the harness boundary.""" +# Where the harness cache set lands on the paired Toolbox (see +# ``tools.credential_files.OMNIO_TOOLBOX_CACHE_BASE``); the delegation root is +# kept as a named constant because the delegation tooling documents it. +SPRITES_CACHE_ROOT = "/tmp/omnio-session/cache" +SPRITES_DELEGATION_ROOT = f"{SPRITES_CACHE_ROOT}/delegation" + +# Per-file ceiling for the Toolbox cache projection. The cache can hold media +# (videos, screenshots); anything above this stays host-only and is logged so +# the gap is visible instead of silently stalling every command's sync. +SPRITES_CACHE_FILE_MAX_BYTES = 50 * 1024 * 1024 +_oversized_cache_files_warned: set[str] = set() + + +def iter_sprites_cache_files() -> list[tuple[str, str]]: + """Enumerate the harness cache files projected onto the paired Toolbox. + + Mirrors :func:`iter_cache_files` with the Toolbox layout: every + ``credential_files._CACHE_DIRS`` subdirectory lands under + :data:`SPRITES_CACHE_ROOT`. Only regular files below a non-symlinked + directory chain cross the boundary, and files above + :data:`SPRITES_CACHE_FILE_MAX_BYTES` are skipped (once-logged). Text + redaction happens in the uploader, not here. + """ from hermes_constants import get_hermes_dir + from tools.credential_files import _CACHE_DIRS, OMNIO_TOOLBOX_CACHE_BASE - root = get_hermes_dir("cache/delegation", "delegation_cache") - if root.is_symlink() or not root.is_dir(): - return [] - files = [] - for path in root.rglob("*"): - if path.is_symlink() or not path.is_file(): - continue - relative = path.relative_to(root) - if any(parent.is_symlink() for parent in path.parents if parent != root and root in parent.parents): - continue - if path.suffix not in {".txt", ".log", ".json"}: + files: list[tuple[str, str]] = [] + for new_subpath, old_name in _CACHE_DIRS: + root = get_hermes_dir(new_subpath, old_name) + if root.is_symlink() or not root.is_dir(): continue - files.append((str(path), f"{SPRITES_DELEGATION_ROOT}/{relative.as_posix()}")) + remote_root = f"{OMNIO_TOOLBOX_CACHE_BASE}/{new_subpath}" + for path in root.rglob("*"): + if path.is_symlink() or not path.is_file(): + continue + relative = path.relative_to(root) + if any(parent.is_symlink() for parent in path.parents if parent != root and root in parent.parents): + continue + try: + size = path.stat().st_size + except OSError: + continue + if size > SPRITES_CACHE_FILE_MAX_BYTES: + key = str(path) + if key not in _oversized_cache_files_warned: + _oversized_cache_files_warned.add(key) + logger.warning( + "file_sync: %s is %d bytes, above the %d-byte Toolbox cache " + "ceiling; it stays on the harness only", + key, size, SPRITES_CACHE_FILE_MAX_BYTES, + ) + continue + files.append((str(path), f"{remote_root}/{relative.as_posix()}")) return files diff --git a/tools/environments/sprites.py b/tools/environments/sprites.py index d72dc6216709a..db94ba4e0e028 100644 --- a/tools/environments/sprites.py +++ b/tools/environments/sprites.py @@ -1,6 +1,7 @@ """Omnio toolbox Sprite execution environment.""" import base64 +import codecs import http.client import json import logging @@ -12,12 +13,13 @@ import urllib.parse import urllib.request import uuid +from collections.abc import Iterator from pathlib import Path from typing import Any from tools.environments.base import BaseEnvironment, _ThreadedProcessHandle from tools.environments.file_sync import ( - FileSyncManager, SPRITES_DELEGATION_ROOT, iter_sprites_delegation_files, + FileSyncManager, SPRITES_CACHE_ROOT, iter_sprites_cache_files, iter_sprites_sync_files, ) from tools.file_operations import ( @@ -39,8 +41,25 @@ _MAX_SKILL_BATCH_FILES = 200 _MAX_SKILL_BATCH_BYTES = 16 * 1024 * 1024 _MAX_FILE_CONTENT_BYTES = 2 * 1024 * 1024 +# Whole-file text reads above the JSON cap go through the raw stream up to +# this ceiling (matches the largest stdout recovery artifact); larger files +# must be paged with read_file offset/limit. +_MAX_RAW_TEXT_READ_BYTES = 5 * 1024 * 1024 _EXEC_PREDISPATCH_RETRY_DELAYS_SECONDS = (2.0, 4.0) _EXEC_RETRY_MIN_REQUEST_BUDGET_SECONDS = 1.0 +# Cache artifacts with these suffixes are text the model or a worker wrote; +# they are secret-redacted before they land on the Toolbox. +_REDACTED_CACHE_SUFFIXES = frozenset({ + ".txt", ".log", ".json", ".md", ".csv", ".html", ".htm", ".xml", ".yaml", ".yml", +}) + + +def _is_toolbox_cache_path(path: str) -> bool: + """True for paths inside the projected harness cache on the Toolbox.""" + return path == SPRITES_CACHE_ROOT or path.startswith(SPRITES_CACHE_ROOT + "/") + + +_STREAM_CHUNK_BYTES = 1024 * 1024 def _expand_toolbox_home(path: str) -> str: @@ -257,11 +276,15 @@ def __init__( delete_fn=self._sprites_delete, bulk_upload_fn=self._sprites_bulk_upload, ) - self._delegation_sync_lock = threading.Lock() - self._delegation_sync_manager = FileSyncManager( - get_files_fn=iter_sprites_delegation_files, - upload_fn=self._upload_delegation_artifact, - delete_fn=self._delete_delegation_artifacts, + # The harness cache set (web pages, screenshots, worker artifacts, + # stdout recovery, ...) is projected under SPRITES_CACHE_ROOT so the + # paths `to_agent_visible_cache_path` publishes are readable on the + # Toolbox. Synced before every command and before reads under it. + self._cache_sync_lock = threading.Lock() + self._cache_sync_manager = FileSyncManager( + get_files_fn=iter_sprites_cache_files, + upload_fn=self._upload_cache_file, + delete_fn=self._delete_cache_files, ) self._sync_manager.sync(force=True) self.init_session() @@ -418,7 +441,7 @@ def read_file_bytes(self, path: str, *, max_bytes: int) -> bytes: if max_bytes < 1: raise ValueError("max_bytes must be positive") path = _canonicalize_toolbox_path(path) - self.sync_delegation_artifacts() + self.sync_projected_path(path) query = urllib.parse.urlencode({"path": path}) request = urllib.request.Request( f"{self.toolbox_url}/files?{query}", @@ -448,6 +471,46 @@ def read_file_bytes(self, path: str, *, max_bytes: int) -> bytes: f"Toolbox API /files is unreachable: {exc}" ) from exc + def stream_file_bytes( + self, path: str, *, chunk_size: int = _STREAM_CHUNK_BYTES + ) -> Iterator[bytes]: + """Yield a Toolbox file's raw bytes in bounded chunks. + + The raw ``GET /files`` route streams any size; consuming it in chunks + keeps memory bounded to the caller's window, which is what lets file + tools page through text the JSON read refuses as too large. + """ + path = _canonicalize_toolbox_path(path) + self.sync_projected_path(path) + query = urllib.parse.urlencode({"path": path}) + request = urllib.request.Request( + f"{self.toolbox_url}/files?{query}", + headers={ + "Authorization": f"Bearer {self.bearer_token}", + "X-Omnio-Brand": self.brand, + }, + method="GET", + ) + try: + with _URL_OPENER.open(request, timeout=self.timeout) as response: + while True: + chunk = response.read(chunk_size) + if not chunk: + return + yield chunk + except urllib.error.HTTPError as exc: + detail = exc.read(_MAX_ERROR_BYTES).decode("utf-8", errors="replace") + raise SpritesToolboxError( + f"Toolbox API /files failed with HTTP {exc.code}: " + f"{detail or exc.reason}", + detail=detail or str(exc.reason), + http_status=exc.code, + ) from exc + except (OSError, http.client.HTTPException) as exc: + raise SpritesToolboxError( + f"Toolbox API /files is unreachable: {exc}" + ) from exc + def get_temp_dir(self) -> str: return "/tmp/.hermes-session" @@ -533,22 +596,29 @@ def _sprites_delete(self, remote_paths: list[str]) -> None: ) self.file_request({"operation": "deleteSkills", "path": remote_path, "missingOk": True}) - def _upload_delegation_artifact(self, host_path: str, remote_path: str) -> None: + def _upload_cache_file(self, host_path: str, remote_path: str) -> None: + """Project one harness cache file onto the Toolbox. + + Text artifacts (worker transcripts, stored pages, stdout recovery) are + secret-redacted first: the Toolbox is readable by model-authored code, + and these files carry exactly the data that tends to hold keys. Media + crosses byte-for-byte through the raw endpoint. + """ remote_path = _canonicalize_toolbox_path(remote_path) - if not remote_path.startswith(SPRITES_DELEGATION_ROOT + "/"): - raise SpritesToolboxError("Refused artifact outside delegation cache") - from tools.delegation_live_log import _redact - - content = _redact(Path(host_path).read_text(encoding="utf-8")) - if len(content.encode("utf-8")) <= _MAX_FILE_CONTENT_BYTES: - if not self.write_file_content(remote_path, content): - raise SpritesToolboxError("Delegation artifact transfer failed") - return - self._write_large_delegation_artifact(remote_path, content) - - def _write_large_delegation_artifact(self, path: str, content: str) -> None: - """The existing atomic raw-file endpoint avoids the JSON write cap.""" - data = content.encode("utf-8") + if not _is_toolbox_cache_path(remote_path): + raise SpritesToolboxError("Refused artifact outside the Toolbox cache") + source = Path(host_path) + if source.is_symlink() or not source.is_file(): + raise SpritesToolboxError(f"Refused non-file cache artifact: {host_path}") + data = source.read_bytes() + if source.suffix.lower() in _REDACTED_CACHE_SUFFIXES: + from tools.delegation_live_log import _redact + + data = _redact(data.decode("utf-8", errors="replace")).encode("utf-8") + self._write_raw_artifact(remote_path, data) + + def _write_raw_artifact(self, path: str, data: bytes) -> None: + """The atomic raw-file endpoint: binary-safe, no JSON write cap.""" query = urllib.parse.urlencode({ "path": path, "overwrite": "true", "maxBytes": len(data), }) @@ -562,28 +632,51 @@ def _write_large_delegation_artifact(self, path: str, content: str) -> None: with _URL_OPENER.open(request, timeout=self.timeout) as response: result = json.loads(response.read(_MAX_RESPONSE_BYTES + 1)) if result.get("bytesWritten") != len(data): - raise SpritesToolboxError("Delegation artifact transfer was incomplete") + raise SpritesToolboxError("Artifact transfer was incomplete") except (OSError, http.client.HTTPException, ValueError) as exc: - raise SpritesToolboxError("Delegation artifact transfer failed") from exc + raise SpritesToolboxError("Artifact transfer failed") from exc - def _delete_delegation_artifacts(self, paths: list[str]) -> None: + def _delete_cache_files(self, paths: list[str]) -> None: for path in paths: path = _canonicalize_toolbox_path(path) - if not path.startswith(SPRITES_DELEGATION_ROOT + "/"): - raise SpritesToolboxError("Refused artifact outside delegation cache") - result = self.file_request({"operation": "delete", "path": path}) + if not _is_toolbox_cache_path(path): + raise SpritesToolboxError("Refused artifact outside the Toolbox cache") + result = self.file_request({"operation": "delete", "path": path, "missingOk": True}) if result.get("error"): - raise SpritesToolboxError("Delegation artifact removal failed") + raise SpritesToolboxError("Artifact removal failed") - def sync_delegation_artifacts(self, *, raise_on_error: bool = True) -> None: - manager = getattr(self, "_delegation_sync_manager", None) + def sync_cache_files(self, *, raise_on_error: bool = True) -> None: + """Push new or changed harness cache files to the Toolbox now. + + Forced (not rate-limited): callers invoke it right before a read under + :data:`SPRITES_CACHE_ROOT`, where a stale copy would be a wrong answer. + """ + manager = getattr(self, "_cache_sync_manager", None) if manager is not None: - with self._delegation_sync_lock: + with self._cache_sync_lock: manager.sync(force=True, raise_on_error=raise_on_error) + def sync_skill_files(self) -> None: + """Refresh helper files before use, including inside the sync throttle. + + The manager still uploads only changed files. Serialize its state with + other projection work and propagate failure rather than executing an + old helper after an unsuccessful transfer. + """ + with self._cache_sync_lock: + self._sync_manager.sync(force=True, raise_on_error=True) + + def sync_projected_path(self, path: str) -> None: + """Refresh host-owned projections before a Toolbox file consumer.""" + path = _canonicalize_toolbox_path(path) + if path == "/skills" or path.startswith("/skills/"): + self.sync_skill_files() + elif _is_toolbox_cache_path(path): + self.sync_cache_files() + def _before_execute(self) -> None: - self._sync_manager.sync() - self.sync_delegation_artifacts(raise_on_error=False) + self.sync_skill_files() + self.sync_cache_files(raise_on_error=False) def _run_bash( self, @@ -710,9 +803,11 @@ def _files(self, payload: dict[str, Any]) -> dict[str, Any]: if isinstance(value, str) and value.startswith("~"): payload[key] = self._expand_path(value) try: + read_path = _canonicalize_toolbox_path(str(payload.get("path", ""))) if (payload.get("operation") in {"read", "readRaw", "search", "stat"} - and str(payload.get("path", "")).startswith(SPRITES_DELEGATION_ROOT + "/")): - self.env.sync_delegation_artifacts() + and (read_path == "/skills" or read_path.startswith("/skills/") + or _is_toolbox_cache_path(read_path))): + self.env.sync_projected_path(read_path) return self.env.file_request(payload) except SpritesToolboxError as error: operation = str(payload.get("operation", "unknown")) @@ -747,6 +842,8 @@ def read_file(self, path: str, offset: int = 1, limit: int = 500) -> ReadResult: {"operation": "read", "path": path, "offset": offset, "limit": limit} ) if error := response.get("error"): + if str(error).startswith("File too large"): + return self._read_large_text_page(path, offset, limit) return ReadResult(error=str(error), similar_files=response.get("similarFiles", [])) content = str(response.get("content", "")) if not response.get("lineNumbered", False): @@ -764,11 +861,111 @@ def read_file(self, path: str, offset: int = 1, limit: int = 500) -> ReadResult: def read_file_raw(self, path: str) -> ReadResult: response = self._files({"operation": "readRaw", "path": path}) if error := response.get("error"): + if str(error).startswith("File too large"): + # The JSON read caps at the Toolbox's 2 MiB request size. Text + # artifacts above it (a stdout recovery file, a long worker + # transcript) are still whole-readable through the bounded raw + # stream; beyond that ceiling the caller must page with + # read_file offset/limit. + return self._read_large_text(path) return ReadResult(error=str(error), similar_files=response.get("similarFiles", [])) content = str(response.get("content", "")) content, _ = _strip_bom(content) return ReadResult(content=content, file_size=int(response.get("fileSize", 0))) + def _read_large_text_page(self, path: str, offset: int, limit: int) -> ReadResult: + """Page a text file the Toolbox's JSON ``read`` refuses as too large. + + Same window semantics and hint text as the Toolbox's own paged read, + computed from the raw stream one chunk at a time: only the requested + window is held in memory, so a file of any size pages, and the + ``offset/limit`` recipe the whole-read error gives out always works. + """ + from tools.tool_output_limits import get_max_line_length + + expanded = self._expand_path(path) + start = max(1, offset) + end = start + limit - 1 + decoder = codecs.getincrementaldecoder("utf-8")(errors="replace") + window: list[str] = [] + pending = "" + # Retain only the displayable prefix, including room for a BOM and + # one over-limit character so _add_line_numbers marks truncation. + # A minified document must not grow this buffer to the file's size. + prefix_limit = get_max_line_length() + 2 + line_no = 0 + size = 0 + + def take(line: str) -> None: + nonlocal line_no + line_no += 1 + if start <= line_no <= end: + window.append(line.rstrip("\r")) + + try: + for chunk in self.env.stream_file_bytes(expanded): + size += len(chunk) + pieces = decoder.decode(chunk).split("\n") + for piece in pieces[:-1]: + take(pending + piece[:prefix_limit - len(pending)]) + pending = "" + pending += pieces[-1][:prefix_limit - len(pending)] + pending += decoder.decode(b"", final=True)[:prefix_limit - len(pending)] + if pending: + take(pending) + except SpritesToolboxError as error: + return ReadResult( + error=render_sprites_toolbox_error( + error, service="file tools", action="file read", + context=f"path {expanded!r}", + ) + ) + total = line_no + if start > total: + return ReadResult( + error=f"Offset {start} is beyond the end of the file ({total} lines): {expanded}", + total_lines=total, file_size=size, + ) + if start == 1 and window: + window[0], _ = _strip_bom(window[0]) + shown_end = min(end, total) + truncated = total > shown_end + return ReadResult( + content=self._add_line_numbers("\n".join(window), start), + total_lines=total, + file_size=size, + truncated=truncated, + hint=f"Use offset={shown_end + 1} to continue reading" if truncated else None, + ) + + def _read_large_text(self, path: str) -> ReadResult: + """Whole-file text read through the raw stream, bounded by + ``_MAX_RAW_TEXT_READ_BYTES``; larger files are paged instead.""" + expanded = self._expand_path(path) + chunks: list[bytes] = [] + size = 0 + try: + for chunk in self.env.stream_file_bytes(expanded): + size += len(chunk) + if size > _MAX_RAW_TEXT_READ_BYTES: + return ReadResult( + error=( + f"File exceeds {_MAX_RAW_TEXT_READ_BYTES} bytes: {expanded}. " + "Use read_file with offset/limit to page through it." + ) + ) + chunks.append(chunk) + except SpritesToolboxError as error: + return ReadResult( + error=render_sprites_toolbox_error( + error, service="file tools", action="file read", + context=f"path {expanded!r}", + ) + ) + data = b"".join(chunks) + content, _ = _strip_bom(data.decode("utf-8", errors="replace")) + return ReadResult(content=content, file_size=len(data)) + def write_file(self, path: str, content: str) -> WriteResult: path = self._expand_path(path) if _is_write_denied(path): diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index ab13be24bdc91..ef6031d36b4eb 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -661,6 +661,11 @@ def _find_skill(name: str) -> Optional[Dict[str, Any]]: continue if skill_md.parent.name == name: return {"path": skill_md.parent} + # Upstream #98099: accept the categorized identifier advertised + # by skill_view. Match against each owning root, including external + # catalogs, without resolving through a different skill's symlink. + if skill_md.parent.relative_to(skills_dir).as_posix() == name: + return {"path": skill_md.parent} return None diff --git a/tools/skills_tool.py b/tools/skills_tool.py index 236ccbe18550a..4481c8fbd028e 100644 --- a/tools/skills_tool.py +++ b/tools/skills_tool.py @@ -66,6 +66,7 @@ content = skill_view("axolotl", "references/dataset-formats.md") """ +import hashlib import json import logging import time @@ -1027,6 +1028,30 @@ def skill_view( return render_skill_result(payload) +def _deduplicate_same_root_skills(candidates, all_dirs): + """Upstream #113126: prefer an unambiguous shallow copy of ONE skill.""" + if len(candidates) < 2: + return candidates + roots = { + max((root for root in all_dirs if path.is_relative_to(root)), + key=lambda root: len(root.parts), default=None) + for _, path in candidates + } + if len(roots) != 1 or None in roots: + return candidates + try: + if len({hashlib.sha256(path.read_bytes()).digest() for _, path in candidates}) != 1: + return candidates + except OSError: + return candidates + root = roots.pop() + def rank(candidate): + path = candidate[1] + return path.name != "SKILL.md", len(path.relative_to(root).parts) + ranked = sorted(candidates, key=rank) + return [ranked[0]] if rank(ranked[0]) != rank(ranked[1]) else candidates + + def _load_skill_content( name: str, file_path: str = None, @@ -1248,6 +1273,7 @@ def _record(sd: Optional[Path], smd: Path) -> None: ): _record(None, found_md) + candidates = _deduplicate_same_root_skills(candidates, all_dirs) if len(candidates) > 1: paths = [str(smd) for _, smd in candidates] logging.getLogger(__name__).warning( @@ -1276,13 +1302,20 @@ def _record(sd: Optional[Path], smd: Path) -> None: skill_dir, skill_md = candidates[0] if not skill_md or not skill_md.exists(): - available = [s["name"] for s in _sort_skills(_find_all_skills())[:20]] + installed = _sort_skills(_find_all_skills()) + available = [skill["name"] for skill in installed[:20]] + # Recover stale category references without silently choosing a + # different skill or bypassing qualified-name collision checks. + bare_name = name.replace(":", "/").rsplit("/", 1)[-1] + matching = [{"name": skill["name"], "category": skill.get("category")} + for skill in installed if skill["name"] == bare_name] return json.dumps( { "success": False, "error": f"Skill '{name}' not found.", "available_skills": available, - "hint": "Use skills_list to see all available skills", + "matching_skills": matching, + "hint": "Use a matching installed name, or skills_list to see all available skills. Do not guess a category.", }, ensure_ascii=False, ) @@ -1386,7 +1419,7 @@ def _record(sd: Optional[Path], smd: Path) -> None: }, ensure_ascii=False, ) - if not target_file.exists(): + if not target_file.is_file(): # List available files in the skill directory, organized by type available_files = { "references": [], @@ -1630,6 +1663,9 @@ def _record(sd: Optional[Path], smd: Path) -> None: "Could not preprocess skill content for %s", skill_name, exc_info=True ) + from agent.skill_path_mapping import map_skill_dir_for_backend + + runtime_skill_dir = map_skill_dir_for_backend(skill_dir, task_id=task_id) if skill_dir else None result = { "success": True, "name": skill_name, @@ -1638,7 +1674,7 @@ def _record(sd: Optional[Path], smd: Path) -> None: "related_skills": related_skills, "content": rendered_content, "path": rel_path, - "skill_dir": str(skill_dir) if skill_dir else None, + "skill_dir": runtime_skill_dir if preprocess else (str(skill_dir) if skill_dir else None), "linked_files": linked_files if linked_files else None, "usage_hint": "To view linked files, call skill_view(name, file_path) where file_path is e.g. 'references/api.md' or 'assets/config.yaml'" if linked_files diff --git a/tools/web_tools.py b/tools/web_tools.py index 131c00303658f..12d8027dad181 100644 --- a/tools/web_tools.py +++ b/tools/web_tools.py @@ -545,6 +545,14 @@ def _truncate_with_footer( total = len(content) stored_path = _store_full_text(url, content) + if stored_path: + # The footer is read by the AGENT, whose read_file runs inside the + # active backend: name the path where the sandbox sees the projected + # cache, not the host path (upstream #72389, #81984), and make sure + # the file is already there before the path is handed out. + from tools.credential_files import publish_cache_path + + stored_path = publish_cache_path(stored_path) shown = len(head) + len(tail) footer_lines = [