From 606cb2de92bf706a739cae003f566b3888d27f65 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 12:11:59 -0700 Subject: [PATCH] refactor(tools/code_exec): unify code_kernel local/remote helpers, split checkpoint_manager god methods, compact spill helpers --- tests/tools/test_checkpoint_manager.py | 8 +- tests/tools/test_code_execution.py | 2 +- tests/tools/test_code_execution_modes.py | 8 +- .../tools/test_code_execution_windows_env.py | 6 +- tests/tools/test_local_env_blocklist.py | 2 +- tests/tools/test_windows_compat.py | 2 +- tests/tools/test_windows_native_support.py | 7 +- tools/ansi_strip.py | 2 + tools/binary_extensions.py | 2 + tools/checkpoint_manager.py | 2143 ++++++----------- tools/code_execution_tool.py | 1620 ++----------- tools/code_kernel.py | 813 +++---- tools/code_kernel_remote.py | 466 ++-- tools/file_state.py | 1 + tools/hook_output_spill.py | 18 +- tools/tool_output_limits.py | 6 + tools/tool_result_storage.py | 5 +- 17 files changed, 1518 insertions(+), 3593 deletions(-) diff --git a/tests/tools/test_checkpoint_manager.py b/tests/tools/test_checkpoint_manager.py index 67cdaac9f6..bf041605e3 100644 --- a/tests/tools/test_checkpoint_manager.py +++ b/tests/tools/test_checkpoint_manager.py @@ -13,7 +13,6 @@ from unittest.mock import patch from tools.checkpoint_manager import ( CheckpointManager, - _shadow_repo_path, _init_store, _run_git, _git_env, @@ -80,10 +79,9 @@ def disabled_mgr(checkpoint_base, monkeypatch): class TestStorePath: def test_store_is_single_shared_path(self, work_dir, checkpoint_base, monkeypatch): monkeypatch.setattr("tools.checkpoint_manager.CHECKPOINT_BASE", checkpoint_base) - # All projects resolve to the same store. - p1 = _shadow_repo_path(str(work_dir)) - p2 = _shadow_repo_path(str(work_dir.parent / "other")) - assert p1 == p2 == _store_path(checkpoint_base) + # All projects resolve to the same store (only refs/indexes are per-project). + assert _store_path() == _store_path(checkpoint_base) + assert _project_hash(str(work_dir)) != _project_hash(str(work_dir.parent / "other")) def test_project_hash_identifies_dir_and_expands_tilde(self, fake_home): project = fake_home / "project" diff --git a/tests/tools/test_code_execution.py b/tests/tools/test_code_execution.py index 66ed46a686..c67ce0247c 100644 --- a/tests/tools/test_code_execution.py +++ b/tests/tools/test_code_execution.py @@ -441,7 +441,7 @@ class TestStubSchemaDrift(unittest.TestCase): import tools.file_tools # noqa: F401 - registers read_file, write_file, patch, search_files import tools.web_tools # noqa: F401 - registers web_search, web_extract - for tool_name, (func_name, sig, doc, args_expr) in _TOOL_STUBS.items(): + for tool_name, (sig, doc, args_expr) in _TOOL_STUBS.items(): entry = registry._tools.get(tool_name) if not entry: # Tool might not be registered yet (e.g., terminal uses a diff --git a/tests/tools/test_code_execution_modes.py b/tests/tools/test_code_execution_modes.py index fe9b4d7911..01ed0ff8cb 100644 --- a/tests/tools/test_code_execution_modes.py +++ b/tests/tools/test_code_execution_modes.py @@ -522,19 +522,19 @@ class TestUsesHermesPythonEnvironment(unittest.TestCase): def test_false_for_different_prefix(self): """An interpreter reporting a different prefix is external.""" - with patch("tools.code_execution_tool._python_environment_prefix", + with patch("tools.code_execution_env._python_environment_prefix", return_value="/some/other/venv"): self.assertFalse(_uses_hermes_python_environment("/other/python")) def test_false_when_prefix_is_empty(self): """If prefix cannot be determined (error path), treat as external.""" - with patch("tools.code_execution_tool._python_environment_prefix", + with patch("tools.code_execution_env._python_environment_prefix", return_value=""): self.assertFalse(_uses_hermes_python_environment("/bad/python")) def test_true_when_prefix_matches_sys_prefix(self): hermes_prefix = os.path.realpath(sys.prefix) - with patch("tools.code_execution_tool._python_environment_prefix", + with patch("tools.code_execution_env._python_environment_prefix", return_value=hermes_prefix): self.assertTrue(_uses_hermes_python_environment("/same/env/python")) @@ -575,7 +575,7 @@ class TestPythonPathComposition(unittest.TestCase): with patch("tools.code_execution_tool._load_config", return_value={"mode": "strict"}), \ patch("model_tools.handle_function_call", side_effect=_mock_handle_function_call), \ - patch("tools.code_execution_tool._uses_hermes_python_environment", + patch("tools.code_execution_env._uses_hermes_python_environment", return_value=same_env), \ patch("subprocess.Popen", side_effect=_fake_popen): try: diff --git a/tests/tools/test_code_execution_windows_env.py b/tests/tools/test_code_execution_windows_env.py index 45d0058f4b..a80d93f6f3 100644 --- a/tests/tools/test_code_execution_windows_env.py +++ b/tests/tools/test_code_execution_windows_env.py @@ -562,10 +562,10 @@ class TestChildStdioIsUtf8: so LLM scripts can print non-ASCII without crashing on Windows.""" def test_popen_env_sets_pythonioencoding_utf8(self): - """Source-level check: the Popen call site must set + """Source-level check: the child env builder must set PYTHONIOENCODING=utf-8 in child_env.""" - import tools.code_execution_tool as cet - src = open(cet.__file__, encoding="utf-8").read() + import tools.code_execution_env as cee + src = open(cee.__file__, encoding="utf-8").read() assert 'child_env["PYTHONIOENCODING"] = "utf-8"' in src, ( "PYTHONIOENCODING=utf-8 missing from child env — Windows " "scripts that print non-ASCII will crash with " diff --git a/tests/tools/test_local_env_blocklist.py b/tests/tools/test_local_env_blocklist.py index 5cae08330b..bed6e78ea0 100644 --- a/tests/tools/test_local_env_blocklist.py +++ b/tests/tools/test_local_env_blocklist.py @@ -988,7 +988,7 @@ class TestPythonpathSelectiveStrip: return_value={"mode": "strict"}), \ patch("model_tools.handle_function_call", side_effect=_mock_handle_function_call), \ - patch("tools.code_execution_tool._uses_hermes_python_environment", + patch("tools.code_execution_env._uses_hermes_python_environment", return_value=same_env), \ patch("subprocess.Popen", side_effect=_fake_popen), \ patch.dict(os.environ, { diff --git a/tests/tools/test_windows_compat.py b/tests/tools/test_windows_compat.py index 478778131b..109d5bd297 100644 --- a/tests/tools/test_windows_compat.py +++ b/tests/tools/test_windows_compat.py @@ -12,7 +12,7 @@ from pathlib import Path GUARDED_FILES = [ "tools/environments/local.py", "tools/process_registry.py", - "tools/code_execution_tool.py", + "tools/code_kernel.py", # execute_code's child Popen lives in the session kernel "gateway/platforms/whatsapp.py", ] diff --git a/tests/tools/test_windows_native_support.py b/tests/tools/test_windows_native_support.py index 1a04a28f55..9800d796eb 100644 --- a/tests/tools/test_windows_native_support.py +++ b/tests/tools/test_windows_native_support.py @@ -598,10 +598,11 @@ class TestCodeExecutionTransportTcpFallback: ) def test_server_side_branches_on_use_tcp_rpc(self): + # The local RPC listener lives in the session kernel (tools/code_kernel.py). root = Path(__file__).resolve().parents[2] - source = (root / "tools" / "code_execution_tool.py").read_text(encoding="utf-8") - assert "_use_tcp_rpc = _IS_WINDOWS" in source - assert 'rpc_endpoint = f"tcp://{_host}:{_port}"' in source + source = (root / "tools" / "code_kernel.py").read_text(encoding="utf-8") + assert "if _IS_WINDOWS:" in source + assert 'rpc_endpoint = f"tcp://{host}:{port}"' in source # --------------------------------------------------------------------------- diff --git a/tools/ansi_strip.py b/tools/ansi_strip.py index 3e880363f3..577cb79ef4 100644 --- a/tools/ansi_strip.py +++ b/tools/ansi_strip.py @@ -44,6 +44,7 @@ _HAS_CONTROL = re.compile(r"[\x00-\x08\x0b-\x1f\x7f-\x9f]") # channel. The only legitimate modern use is emoji tag sequences (TR51: U+1F3F4 # base + tag spec + U+E007F CANCEL TAG, e.g. the Scotland/Wales flags); those # are preserved, same rationale as keeping ZWJ inside emoji sequences. +# Ported from block/goose#10746 (which strips flags too). _UNICODE_TAG_SUB_RE = re.compile( r"(\U0001F3F4[\U000E0020-\U000E007E]+\U000E007F)" # valid emoji tag seq (kept) r"|[\U000E0000-\U000E007F]" # any other tag char (stripped) @@ -68,6 +69,7 @@ def sanitize_display_text(text: str) -> str: hide content). Use when re-rendering persisted text (e.g. the ``/resume`` recap): Rich's ``Text()`` does NOT neutralize raw escape bytes, so a replayed message must not be able to clear the screen, retitle the window, or restyle UI. + Mirrors openai/codex#31494 (``sanitize_user_text``). """ if not text or not _HAS_CONTROL.search(text): return text diff --git a/tools/binary_extensions.py b/tools/binary_extensions.py index d38a6f48da..307cb4b54f 100644 --- a/tools/binary_extensions.py +++ b/tools/binary_extensions.py @@ -55,6 +55,7 @@ def _has_extension_in(path: str, extensions: frozenset) -> bool: def has_binary_extension(path: str) -> bool: + """True when the path has a binary extension. Pure string check, no I/O.""" return _has_extension_in(path, BINARY_EXTENSIONS) @@ -64,4 +65,5 @@ def has_opaque_document_extension(path: str) -> bool: def is_pdf_path(path: str) -> bool: + """True when the path has a .pdf extension. Pure string check, no I/O.""" return path.lower().endswith(".pdf") diff --git a/tools/checkpoint_manager.py b/tools/checkpoint_manager.py index 1061874b41..489e44f681 100644 --- a/tools/checkpoint_manager.py +++ b/tools/checkpoint_manager.py @@ -1,51 +1,18 @@ -""" -Checkpoint Manager — Transparent filesystem snapshots via a single shared -shadow git store. +"""Checkpoint Manager — transparent filesystem snapshots via one shared shadow git store. -Creates automatic snapshots of working directories before file-mutating -operations (``write_file``, ``patch``, ``terminal`` with destructive flags), -triggered once per conversation turn. Provides rollback to any previous -checkpoint. +Snapshots a working directory before file-mutating tool calls, at most once per +directory per turn, and restores any previous checkpoint. Not a model tool — the +LLM never sees it; controlled by the ``checkpoints`` config / ``--checkpoints`` flag. -This is NOT a tool — the LLM never sees it. It's transparent infrastructure -controlled by the ``checkpoints`` config flag or ``--checkpoints`` CLI flag. - -Storage layout (single shared store, git objects deduplicated across projects) ------------------------------------------------------------------------------ - - ~/.hermes/checkpoints/ - store/ — single bare-ish git repo - HEAD, config, objects/ — standard git internals (shared) - refs/hermes/ — per-project branch tip - indexes/ — per-project git index - projects/.json — {workdir, created_at, last_touch} - info/exclude — default excludes (shared) - .last_prune — auto-prune idempotency marker - legacy-/ — archived pre-v2 per-project shadow - repos (auto-migrated on first init) - -Why a single store? -------------------- - -The pre-v2 design kept a full shadow repo per working directory. Each one -re-stored most of the project's files under its own ``objects/`` tree, with -zero sharing across worktrees of the same project. A single user with a -dozen worktrees of the same repo burned ~40 MB each (~500 MB total) storing -the same blobs over and over. A single shared store lets git's content- -addressable object DB deduplicate across projects and across turns, so adding -a new worktree costs near-zero. - -The shadow store uses ``GIT_DIR`` + ``GIT_WORK_TREE`` + ``GIT_INDEX_FILE`` -so no git state leaks into the user's project directory. - -Auto-maintenance ----------------- - -Shadow state accumulates over time. ``prune_checkpoints`` deletes refs whose -recorded working directory no longer exists (orphan) or whose last touch is -older than ``retention_days`` (stale), then runs ``git gc --prune=now`` to -reclaim object storage. A size-cap pass drops the oldest checkpoints per -project until total store size is under ``max_total_size_mb``. +Layout under ``~/.hermes/checkpoints/`` (one store so git's object DB dedupes blobs +across projects/worktrees/turns; the pre-v2 one-repo-per-workdir design re-stored +the same blobs ~40 MB per worktree): ``store/`` is a bare repo with per-project +``refs/hermes/``, ``indexes/`` (no shared-index races), +``projects/.json`` (workdir, timestamps, parent identity), +``ledgers/.json`` (agent-write ledger for safe restore) and a shared +``info/exclude``; ``.last_prune`` is the auto-prune marker; ``legacy-/`` holds +archived pre-v2 repos. Git runs with GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE so no +state leaks into the user's project. """ import hashlib @@ -56,93 +23,50 @@ import re import shutil import subprocess import time +from dataclasses import dataclass, field from pathlib import Path +from typing import Dict, List, NamedTuple, Optional, Set, Tuple + from hermes_constants import get_hermes_home from hermes_cli._subprocess_compat import windows_hide_flags -from typing import Dict, List, Optional, Set, Tuple - from utils import env_int logger = logging.getLogger(__name__) -# --------------------------------------------------------------------------- # Constants -# --------------------------------------------------------------------------- CHECKPOINT_BASE = get_hermes_home() / "checkpoints" -# Single shared store directory under CHECKPOINT_BASE. _STORE_DIRNAME = "store" _REFS_PREFIX = "refs/hermes" _INDEXES_DIRNAME = "indexes" _PROJECTS_DIRNAME = "projects" _LEDGERS_DIRNAME = "ledgers" _LEGACY_PREFIX = "legacy-" +_PRUNE_MARKER_NAME = ".last_prune" # Agent-write ledger cap: newest entries retained per project. _LEDGER_MAX_ENTRIES = 2000 DEFAULT_EXCLUDES = [ # Dependency / build output - "node_modules/", - "dist/", - "build/", - "target/", - "out/", - ".next/", - ".nuxt/", + "node_modules/", "dist/", "build/", "target/", "out/", ".next/", ".nuxt/", # Caches - "__pycache__/", - "*.pyc", - "*.pyo", - ".cache/", - ".pytest_cache/", - ".mypy_cache/", - ".ruff_cache/", - "coverage/", - ".coverage", + "__pycache__/", "*.pyc", "*.pyo", ".cache/", ".pytest_cache/", ".mypy_cache/", + ".ruff_cache/", "coverage/", ".coverage", # Virtualenvs - ".venv/", - "venv/", - "env/", - # VCS - ".git/", - ".hg/", - ".svn/", - # Worktrees (Hermes convention — don't recursively snapshot siblings) - ".worktrees/", + ".venv/", "venv/", "env/", + # VCS + worktrees (Hermes convention — don't recursively snapshot siblings) + ".git/", ".hg/", ".svn/", ".worktrees/", # Native / compiled binaries - "*.so", - "*.dylib", - "*.dll", - "*.o", - "*.a", - "*.jar", - "*.class", - "*.exe", - "*.obj", + "*.so", "*.dylib", "*.dll", "*.o", "*.a", "*.jar", "*.class", "*.exe", "*.obj", # Media / large binaries - "*.mp4", - "*.mov", - "*.mkv", - "*.webm", - "*.zip", - "*.tar", - "*.tar.gz", - "*.tgz", - "*.7z", - "*.rar", - "*.iso", + "*.mp4", "*.mov", "*.mkv", "*.webm", "*.zip", "*.tar", "*.tar.gz", "*.tgz", + "*.7z", "*.rar", "*.iso", # Secrets - ".env", - ".env.*", - ".env.local", - ".env.*.local", - # OS junk - ".DS_Store", - "Thumbs.db", - # Logs - "*.log", + ".env", ".env.*", ".env.local", ".env.*.local", + # OS junk / logs + ".DS_Store", "Thumbs.db", "*.log", ] # Git subprocess timeout (seconds). @@ -151,20 +75,42 @@ _GIT_TIMEOUT: int = max(10, min(60, env_int("HERMES_CHECKPOINT_TIMEOUT", 30))) # Max files to snapshot — skip huge directories to avoid slowdowns. _MAX_FILES = 50_000 -# Valid git commit hash pattern: 4–40 hex chars (short or full SHA-1/SHA-256). +# Valid git commit hash: 4–64 hex chars (short or full SHA-1/SHA-256). _COMMIT_HASH_RE = re.compile(r'^[0-9a-fA-F]{4,64}$') +_MB = 1024 * 1024 + +# Inherited GIT_* vars that would redirect the shadow store's git calls. +_GIT_LEAK_VARS = ("GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GIT_NAMESPACE", + "GIT_ALTERNATE_OBJECT_DIRECTORIES") + +# Per-store config: isolated by env vars already, but belt-and-suspenders. +_STORE_GIT_CONFIG = (("user.email", "hermes@local"), ("user.name", "Hermes Checkpoint"), + ("commit.gpgsign", "false"), ("tag.gpgSign", "false"), ("gc.auto", "0")) + +_PROJECT_MARKERS = {".git", "pyproject.toml", "package.json", "Cargo.toml", + "go.mod", "Makefile", "pom.xml", ".hg", "Gemfile"} + +_SHORTSTAT_FIELDS = (("files_changed", r'(\d+) file'), ("insertions", r'(\d+) insertion'), + ("deletions", r'(\d+) deletion')) + +_PRUNE_RESULT_KEYS = ("scanned", "deleted_orphan", "deleted_stale", "errors", "bytes_freed") + + +def _no_store_result() -> Dict: + return {"success": False, "error": "No checkpoints exist for this directory"} + + +def _empty_prune_result() -> Dict[str, int]: + return {key: 0 for key in _PRUNE_RESULT_KEYS} + -# --------------------------------------------------------------------------- # Input validation helpers -# --------------------------------------------------------------------------- def _validate_commit_hash(commit_hash: str) -> Optional[str]: - """Validate a commit hash to prevent git argument injection. + """Error string if ``commit_hash`` is unsafe as a git revision, else None. - Returns an error string if invalid, None if valid. - Values starting with '-' would be interpreted as git flags - (e.g., '--patch', '-p') instead of revision specifiers. + A leading '-' would be parsed as a git flag (``--patch``), not a revision. """ if not commit_hash or not commit_hash.strip(): return "Empty commit hash" @@ -176,26 +122,20 @@ def _validate_commit_hash(commit_hash: str) -> Optional[str]: def _validate_file_path(file_path: str, working_dir: str) -> Optional[str]: - """Validate a file path to prevent path traversal outside the working directory. - - Returns an error string if invalid, None if valid. - """ + """Error string if ``file_path`` is absolute or escapes ``working_dir``, else None.""" if not file_path or not file_path.strip(): return "Empty file path" if os.path.isabs(file_path): return f"File path must be relative, got absolute path: {file_path!r}" abs_workdir = _normalize_path(working_dir) - resolved = (abs_workdir / file_path).resolve() try: - resolved.relative_to(abs_workdir) + (abs_workdir / file_path).resolve().relative_to(abs_workdir) except ValueError: return f"File path escapes the working directory via traversal: {file_path!r}" return None -# --------------------------------------------------------------------------- -# Path / hash helpers -# --------------------------------------------------------------------------- +# Path / hash / JSON helpers def _normalize_path(path_value: str) -> Path: """Return a canonical absolute path for checkpoint operations.""" @@ -213,15 +153,8 @@ def _store_path(base: Optional[Path] = None) -> Path: return (base or CHECKPOINT_BASE) / _STORE_DIRNAME -def _shadow_repo_path(working_dir: str) -> Path: # pragma: no cover — kept for BC - """Return the shared store path. - - Retained for backward-compatibility with callers / tests that imported - this helper. Under v2 the shadow git storage is shared across all - projects — per-project isolation lives in refs and indexes, not in - separate repo directories. - """ - return _store_path() +def _store_has_head(store: Path) -> bool: + return (store / "HEAD").exists() def _index_path(store: Path, dir_hash: str) -> Path: @@ -232,6 +165,31 @@ def _ledger_path(store: Path, dir_hash: str) -> Path: return store / _LEDGERS_DIRNAME / f"{dir_hash}.json" +def _ref_name(dir_hash: str) -> str: + return f"{_REFS_PREFIX}/{dir_hash}" + + +def _project_meta_path(store: Path, dir_hash: str) -> Path: + return store / _PROJECTS_DIRNAME / f"{dir_hash}.json" + + +def _read_json_dict(path: Path) -> Optional[Dict]: + """Parse ``path`` as a JSON object; None when missing, unreadable or not a dict.""" + try: + data = json.loads(path.read_text(encoding="utf-8")) + except (OSError, ValueError): + return None + return data if isinstance(data, dict) else None + + +def _unlink_quiet(path: Path) -> None: + try: + if path.exists(): + path.unlink() + except OSError: + pass + + def _hash_file(path: Path) -> Optional[str]: """Streaming sha256 of a file's bytes. None if unreadable/missing.""" try: @@ -245,18 +203,13 @@ def _hash_file(path: Path) -> Optional[str]: def _load_ledger(store: Path, dir_hash: str) -> Dict[str, Dict]: - """Load the agent-write ledger: {relpath: {"sha256": ..., "ts": ...}}. + """Load the agent-write ledger ``{abs_path: {"sha256", "ts"}}``. - The ledger records the content hash of every file the last successful - ``write_file`` / ``patch`` produced, so restores can tell "Hermes wrote - this" apart from "the user hand-edited this afterwards". + Records the content hash of every file the last successful ``write_file`` / + ``patch`` produced, so restores can tell "Hermes wrote this" apart from + "the user hand-edited this afterwards". """ - try: - raw = _ledger_path(store, dir_hash).read_text(encoding="utf-8") - data = json.loads(raw) - return data if isinstance(data, dict) else {} - except (OSError, ValueError): - return {} + return _read_json_dict(_ledger_path(store, dir_hash)) or {} def _save_ledger(store: Path, dir_hash: str, ledger: Dict[str, Dict]) -> None: @@ -278,67 +231,51 @@ def _save_ledger(store: Path, dir_hash: str, ledger: Dict[str, Dict]) -> None: logger.debug("Failed to save agent-write ledger for %s", dir_hash, exc_info=True) -def _ref_name(dir_hash: str) -> str: - return f"{_REFS_PREFIX}/{dir_hash}" +# Git env + invocation +def _isolated_git_env() -> dict: + """Subprocess env with the user's global/system git config neutralised. -def _project_meta_path(store: Path, dir_hash: str) -> Path: - return store / _PROJECTS_DIRNAME / f"{dir_hash}.json" - - -# --------------------------------------------------------------------------- -# Git env -# --------------------------------------------------------------------------- - -def _git_env( - store: Path, - working_dir: str, - index_file: Optional[Path] = None, -) -> dict: - """Build env dict that redirects git to the shared store. - - The shared store is internal Hermes infrastructure — it must NOT inherit - the user's global or system git config. User-level settings like - ``commit.gpgsign = true``, signing hooks, or credential helpers would - either break background snapshots or, worse, spawn interactive prompts - (pinentry GUI windows) mid-session every time a file is written. - - Isolation strategy: - * ``GIT_CONFIG_GLOBAL=`` — ignore ``~/.gitconfig`` (git 2.32+). - * ``GIT_CONFIG_SYSTEM=`` — ignore ``/etc/gitconfig`` (git 2.32+). - * ``GIT_CONFIG_NOSYSTEM=1`` — legacy belt-and-suspenders for older git. - - ``index_file``, if given, forces git to use a per-project index under - ``store/indexes/`` so projects don't race on a shared index. + User settings (``commit.gpgsign``, hooks, credential helpers) would break + background snapshots or spawn pinentry prompts mid-session. GLOBAL/SYSTEM + need git 2.32+; NOSYSTEM covers older git. HOME is kept exactly — rewriting + it would change which ~/.gitconfig is being hidden. """ - normalized_working_dir = _normalize_path(working_dir) - # git child with hand-isolated config env; exact preservation — a HOME - # rewrite would change which ~/.gitconfig the isolation vars are hiding. from tools.environments.local import build_subprocess_env env = build_subprocess_env(scrub_secrets=False, inherit_profile_home=False) - env["GIT_DIR"] = str(store) - env["GIT_WORK_TREE"] = str(normalized_working_dir) - env.pop("GIT_NAMESPACE", None) - env.pop("GIT_ALTERNATE_OBJECT_DIRECTORIES", None) - if index_file is not None: - env["GIT_INDEX_FILE"] = str(index_file) - else: - env.pop("GIT_INDEX_FILE", None) env["GIT_CONFIG_GLOBAL"] = os.devnull env["GIT_CONFIG_SYSTEM"] = os.devnull env["GIT_CONFIG_NOSYSTEM"] = "1" return env -def _repair_bare_repo_dirs(store: Path) -> None: - """Recreate refs/ and branches/ dirs that ``git gc`` may have removed. +def _git_env(store: Path, working_dir: str, index_file: Optional[Path] = None) -> dict: + """Env that redirects git to the shared store (+ a per-project index if given).""" + env = _isolated_git_env() + for key in _GIT_LEAK_VARS: + env.pop(key, None) + env["GIT_DIR"] = str(store) + env["GIT_WORK_TREE"] = str(_normalize_path(working_dir)) + if index_file is not None: + env["GIT_INDEX_FILE"] = str(index_file) + return env - ``git gc --prune=now`` on a bare repo with only packed refs can remove - the empty ``refs/heads/`` directory. Git 2.34+ requires ``refs/`` (and - some versions require ``branches/``) to exist even when all refs are - packed in ``packed-refs``. Without them, ``git add -A`` returns - ``fatal: not a git repository`` and all checkpoint operations fail - silently. + +def _git_subprocess(cmd: List[str], env: dict, timeout: int, cwd: Optional[str] = None): + # creationflags suppresses the per-call conhost flash on Windows (no-op on POSIX). + return subprocess.run( + cmd, capture_output=True, text=True, encoding='utf-8', errors='replace', + timeout=timeout, env=env, cwd=cwd, stdin=subprocess.DEVNULL, + creationflags=windows_hide_flags(), + ) + + +def _repair_bare_repo_dirs(store: Path) -> None: + """Recreate ``refs/heads`` and ``branches`` after ``git gc``. + + gc on a bare repo with only packed refs can remove the empty dirs, yet git + 2.34+ requires them — without them ``git add -A`` fails with "not a git + repository" and every checkpoint operation silently fails. """ for subdir in ("refs/heads", "branches"): path = store / subdir @@ -347,9 +284,7 @@ def _repair_bare_repo_dirs(store: Path) -> None: path.mkdir(parents=True, exist_ok=True) logger.debug("Repaired missing %s in checkpoint store", subdir) except OSError as exc: - logger.warning( - "Cannot create %s in checkpoint store: %s", subdir, exc, - ) + logger.warning("Cannot create %s in checkpoint store: %s", subdir, exc) def _run_git( @@ -360,205 +295,232 @@ def _run_git( allowed_returncodes: Optional[Set[int]] = None, index_file: Optional[Path] = None, ) -> Tuple[bool, str, str]: - """Run a git command against the shared store. Returns (ok, stdout, stderr). + """Run git against the shared store. Returns (ok, stdout, stderr). - ``allowed_returncodes`` suppresses error logging for known/expected non-zero - exits while preserving the normal ``ok = (returncode == 0)`` contract. - Example: ``git diff --cached --quiet`` returns 1 when changes exist. + ``allowed_returncodes`` suppresses error logging for expected non-zero exits + (e.g. ``diff --cached --quiet`` returns 1 when changes exist) while keeping + ``ok = (returncode == 0)``. """ - normalized_working_dir = _normalize_path(working_dir) - if not normalized_working_dir.exists(): - msg = f"working directory not found: {normalized_working_dir}" - logger.error("Git command skipped: %s (%s)", " ".join(["git"] + list(args)), msg) - return False, "", msg - if not normalized_working_dir.is_dir(): - msg = f"working directory is not a directory: {normalized_working_dir}" - logger.error("Git command skipped: %s (%s)", " ".join(["git"] + list(args)), msg) - return False, "", msg - - env = _git_env(store, str(normalized_working_dir), index_file=index_file) + wd = _normalize_path(working_dir) cmd = ["git"] + list(args) - allowed_returncodes = allowed_returncodes or set() + if not wd.exists(): + msg = f"working directory not found: {wd}" + elif not wd.is_dir(): + msg = f"working directory is not a directory: {wd}" + else: + msg = None + if msg: + logger.error("Git command skipped: %s (%s)", " ".join(cmd), msg) + return False, "", msg + env = _git_env(store, str(wd), index_file=index_file) try: - result = subprocess.run( - cmd, - capture_output=True, - text=True, encoding='utf-8', errors='replace', - timeout=timeout, - env=env, - cwd=str(normalized_working_dir), - stdin=subprocess.DEVNULL, - # Checkpoints fire several bare git calls per turn from the - # console-less desktop/gateway backend; suppress the per-call - # conhost flash on Windows (no-op on POSIX). - creationflags=windows_hide_flags(), - ) - ok = result.returncode == 0 - stdout = result.stdout.strip() - stderr = result.stderr.strip() - if not ok and result.returncode not in allowed_returncodes: - logger.error( - "Git command failed: %s (rc=%d) stderr=%s", - " ".join(cmd), result.returncode, stderr, - ) - return ok, stdout, stderr + result = _git_subprocess(cmd, env, timeout, cwd=str(wd)) except subprocess.TimeoutExpired: msg = f"git timed out after {timeout}s: {' '.join(cmd)}" logger.error(msg, exc_info=True) return False, "", msg except FileNotFoundError as exc: - missing_target = getattr(exc, "filename", None) - if missing_target == "git": + if getattr(exc, "filename", None) == "git": logger.error("Git executable not found: %s", " ".join(cmd), exc_info=True) return False, "", "git not found" - msg = f"working directory not found: {normalized_working_dir}" + msg = f"working directory not found: {wd}" logger.error("Git command failed before execution: %s (%s)", " ".join(cmd), msg, exc_info=True) return False, "", msg except Exception as exc: logger.error("Unexpected git error running %s: %s", " ".join(cmd), exc, exc_info=True) return False, "", str(exc) + ok = result.returncode == 0 + stdout, stderr = result.stdout.strip(), result.stderr.strip() + if not ok and result.returncode not in (allowed_returncodes or set()): + logger.error("Git command failed: %s (rc=%d) stderr=%s", + " ".join(cmd), result.returncode, stderr) + return ok, stdout, stderr + + +# --- ref-level git helpers (shared by snapshot, prune and size-cap paths) --- + +def _ref_tip(store: Path, working_dir: str, ref: str) -> Optional[str]: + """Commit sha at ``ref``, or None when the ref does not exist yet.""" + ok, sha, _ = _run_git(["rev-parse", "--verify", ref + "^{commit}"], store, working_dir, + allowed_returncodes={128}) + return sha if ok and sha else None + + +def _ref_commit_count(store: Path, working_dir: str, ref: str) -> int: + ok, out, _ = _run_git(["rev-list", "--count", ref], store, working_dir, allowed_returncodes={128}) + try: + return int(out) if ok else 0 + except ValueError: + return 0 + + +def _ref_commits_oldest_first(store: Path, working_dir: str, ref: str) -> List[str]: + ok, out, _ = _run_git(["rev-list", "--reverse", ref], store, working_dir) + return out.splitlines() if ok and out else [] + + +def _list_project_refs(store: Path, working_dir: str) -> List[str]: + ok, out, _ = _run_git(["for-each-ref", "--format=%(refname)", _REFS_PREFIX], store, working_dir, + allowed_returncodes={128}) + return [r for r in out.splitlines() if r.strip()] if ok else [] + + +def _commit_tree_args(tree_sha: str, message: str, parent: Optional[str]) -> List[str]: + args = ["commit-tree", tree_sha] + if parent is not None: + args += ["-p", parent] + return args + ["-m", message, "--no-gpg-sign"] + + +def _rebuild_linear_chain(store: Path, working_dir: str, shas: List[str]) -> Optional[str]: + """Re-commit each sha's tree (same message) as a fresh linear chain. + + Returns the new tip, or None on any failure (caller leaves the ref untouched). + """ + new_parent: Optional[str] = None + for sha in shas: + ok_tree, tree_sha, _ = _run_git(["rev-parse", f"{sha}^{{tree}}"], store, working_dir) + if not ok_tree or not tree_sha: + return None + ok_msg, msg, _ = _run_git(["log", "--format=%s", "-1", sha], store, working_dir) + ok_commit, new_sha, _ = _run_git( + _commit_tree_args(tree_sha, msg if ok_msg and msg else "checkpoint", new_parent), + store, working_dir) + if not ok_commit or not new_sha: + return None + new_parent = new_sha + return new_parent + + +def _gc_store(store: Path, working_dir: str) -> None: + """Reclaim objects unreachable from the (rewritten/deleted) refs.""" + _run_git(["reflog", "expire", "--expire=now", "--all"], store, working_dir) + _run_git(["gc", "--prune=now", "--quiet"], store, working_dir, timeout=_GIT_TIMEOUT * 3) + _repair_bare_repo_dirs(store) + + +def _drop_oldest_commit(store: Path, working_dir: str, ref: str) -> bool: + """Rewrite ``ref`` without its oldest commit; never below one snapshot.""" + if _ref_commit_count(store, working_dir, ref) <= 1: + return False + commits = _ref_commits_oldest_first(store, working_dir, ref) + if not commits: + return False + tip = _rebuild_linear_chain(store, working_dir, commits[1:]) + if tip is None: + return False + _run_git(["update-ref", ref, tip], store, working_dir) + return True + + +def _shrink_store_to_cap(store: Path, working_dir: str, cap_bytes: int) -> bool: + """Round-robin-drop the oldest commit per project ref until the store fits. + + Bounded to 20 rounds against pathological loops. Returns False when there + are no project refs to work on. + """ + for _ in range(20): + if _dir_size_bytes(store) <= cap_bytes: + break + refs = _list_project_refs(store, working_dir) + if not refs: + return False + dropped = [_drop_oldest_commit(store, working_dir, ref) for ref in refs] + if not any(dropped): + break + return True + + +def _delete_ref(store: Path, ref: str) -> bool: + """Delete a ref from the store. Returns True on success.""" + ok, _, _ = _run_git(["update-ref", "-d", ref], store, str(store.parent), allowed_returncodes={128}) + return ok + -# --------------------------------------------------------------------------- # Store initialisation + legacy migration -# --------------------------------------------------------------------------- def _migrate_legacy_store(base: Path) -> Optional[Path]: - """Move pre-v2 per-project shadow repos into a ``legacy-/`` dir. + """Archive pre-v2 per-project shadow repos into ``legacy-/``. - The pre-v2 layout had one shadow git repo per working directory directly - under ``CHECKPOINT_BASE``. The v2 layout wants a single ``store/`` dir. - Rather than delete the old data (users might want to recover), rename - everything except our own v2 entries into ``legacy-/``. The - legacy dir is subject to the same retention sweep and can be manually - cleared with ``hermes checkpoints clear-legacy``. - - Returns the legacy-archive path, or None if nothing to migrate. + Everything under ``base`` that isn't a v2 entry is moved (not deleted — users + may want to recover); the archive falls under the retention sweep and + ``hermes checkpoints clear-legacy``. Returns the archive path or None. """ if not base.exists(): return None - store = _store_path(base) legacy_root: Optional[Path] = None - # Reserved top-level entries managed by v2. reserved = {_STORE_DIRNAME, _PRUNE_MARKER_NAME} for child in list(base.iterdir()): name = child.name if name in reserved or name.startswith(_LEGACY_PREFIX): continue - # Candidate: pre-v2 shadow repo (has HEAD) OR stray dir. Either way - # we archive it so v2 starts clean. if legacy_root is None: - stamp = time.strftime("%Y%m%d-%H%M%S") - legacy_root = base / f"{_LEGACY_PREFIX}{stamp}" + legacy_root = base / f"{_LEGACY_PREFIX}{time.strftime('%Y%m%d-%H%M%S')}" try: legacy_root.mkdir(parents=True, exist_ok=True) except OSError as exc: logger.warning("Could not create legacy archive dir: %s", exc) return None - dest = legacy_root / name try: - shutil.move(str(child), str(dest)) + shutil.move(str(child), str(legacy_root / name)) except OSError as exc: logger.warning("Could not archive legacy checkpoint %s: %s", child, exc) - # If the store still hasn't been created, create it here. - _ = store if legacy_root is not None: - logger.info( - "Migrated pre-v2 checkpoint repos to %s. " - "Clear with `hermes checkpoints clear-legacy` when safe.", - legacy_root, - ) + logger.info("Migrated pre-v2 checkpoint repos to %s. " + "Clear with `hermes checkpoints clear-legacy` when safe.", legacy_root) return legacy_root def _init_store(store: Path, working_dir: str) -> Optional[str]: - """Initialise the shared shadow store if needed. Returns error or None. - - Also performs one-time migration of pre-v2 per-directory shadow repos - into ``legacy-/``. - """ + """Initialise the shared store if needed (migrating pre-v2 repos first). Returns error or None.""" base = store.parent - # One-time legacy migration before we create the store. if not store.exists(): try: base.mkdir(parents=True, exist_ok=True) except OSError as exc: return f"Could not create checkpoint base: {exc}" - # Only migrate if the base dir has pre-existing content that isn't - # our own v2 layout. _migrate_legacy_store(base) - if (store / "HEAD").exists(): + if _store_has_head(store): return None store.mkdir(parents=True, exist_ok=True) (store / _INDEXES_DIRNAME).mkdir(exist_ok=True) (store / _PROJECTS_DIRNAME).mkdir(exist_ok=True) - # ``git init --bare`` rejects GIT_WORK_TREE, so we can't use _run_git - # here (which always sets GIT_DIR + GIT_WORK_TREE). Use a raw - # subprocess with just the config-isolation env vars. - from tools.environments.local import build_subprocess_env - init_env = build_subprocess_env(scrub_secrets=False, inherit_profile_home=False) - init_env["GIT_CONFIG_GLOBAL"] = os.devnull - init_env["GIT_CONFIG_SYSTEM"] = os.devnull - init_env["GIT_CONFIG_NOSYSTEM"] = "1" - # Drop any inherited GIT_* that would interfere. - for k in ("GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GIT_NAMESPACE", - "GIT_ALTERNATE_OBJECT_DIRECTORIES"): - init_env.pop(k, None) + # ``git init --bare`` rejects GIT_WORK_TREE, so bypass _run_git and use only + # the config-isolation env. + init_env = _isolated_git_env() + for key in _GIT_LEAK_VARS: + init_env.pop(key, None) try: - result = subprocess.run( - ["git", "init", "--bare", str(store)], - capture_output=True, text=True, encoding='utf-8', errors='replace', - env=init_env, timeout=_GIT_TIMEOUT, - stdin=subprocess.DEVNULL, - creationflags=windows_hide_flags(), - ) + result = _git_subprocess(["git", "init", "--bare", str(store)], init_env, _GIT_TIMEOUT) if result.returncode != 0: return f"Shadow store init failed: {result.stderr.strip()}" except (subprocess.TimeoutExpired, FileNotFoundError) as exc: return f"Shadow store init failed: {exc}" - # Per-store config (isolated by env vars above, but belt-and-suspenders). - # Use the base dir as the working_dir for config commands — it always - # exists since we just created the store inside it. - cfg_wd = str(base) - _run_git(["config", "user.email", "hermes@local"], store, cfg_wd) - _run_git(["config", "user.name", "Hermes Checkpoint"], store, cfg_wd) - _run_git(["config", "commit.gpgsign", "false"], store, cfg_wd) - _run_git(["config", "tag.gpgSign", "false"], store, cfg_wd) - _run_git(["config", "gc.auto", "0"], store, cfg_wd) + # The base dir always exists (we just created the store inside it). + for key, value in _STORE_GIT_CONFIG: + _run_git(["config", key, value], store, str(base)) info_dir = store / "info" info_dir.mkdir(exist_ok=True) - (info_dir / "exclude").write_text( - "\n".join(DEFAULT_EXCLUDES) + "\n", encoding="utf-8" - ) + (info_dir / "exclude").write_text("\n".join(DEFAULT_EXCLUDES) + "\n", encoding="utf-8") logger.debug("Initialised checkpoint store at %s", store) return None def _volume_evidence(workdir: Path) -> Dict: - """Record the identity of ``workdir``'s parent while the project is live. + """``(st_dev, st_ino)`` of ``workdir``'s parent, captured while the workdir is reachable. - ``(st_dev, st_ino)`` of the parent directory, captured at a moment when - the workdir itself is reachable, identifies the *directory* — not just - the path. A mount point resolves to the mounted filesystem's root while - the volume is attached and to the underlying (underlay) directory after - unmount: same path, different directory, different ``(st_dev, st_ino)``. - Orphan pruning uses this to distinguish "the project was deleted out of - the directory we knew" from "a different directory is now visible at - that path because the volume is detached". - - Returns ``{}`` when the workdir is not currently reachable, when the - filesystem does not provide a usable directory identity (a zero - ``st_dev`` or ``st_ino`` — e.g. Windows filesystems without file IDs and - some network shares), or when the probe fails — callers treat all of - these as "no evidence recorded" and orphan pruning stays conservative - for the project (never classified as orphan; retention still applies). + Identifies the *directory*, not the path (a mount point resolves to the underlay + after unmount), so orphan pruning can tell "deleted" from "volume detached". + ``{}`` when unreachable, on error, or with no usable identity (zero dev/ino: + Windows without file IDs, some shares) — pruning then stays conservative. """ try: if not workdir.exists(): @@ -566,39 +528,26 @@ def _volume_evidence(workdir: Path) -> Dict: st = workdir.parent.stat() if not st.st_dev or not st.st_ino: return {} - return { - "workdir_parent_dev": st.st_dev, - "workdir_parent_ino": st.st_ino, - } + return {"workdir_parent_dev": st.st_dev, "workdir_parent_ino": st.st_ino} except OSError: return {} def _register_project(store: Path, working_dir: str) -> None: - """Create or update ``projects/.json`` with workdir + timestamps.""" - dir_hash = _project_hash(working_dir) - meta_path = _project_meta_path(store, dir_hash) + """Create or update ``projects/.json``: workdir, last_touch, created_at. + + ``created_at`` survives re-registration. The parent identity is refreshed + while the project is observably live (a remount can legitimately change it); + on a failed probe the recorded identity is kept — stale evidence only makes + pruning MORE conservative (mismatch => not orphan). Never raises. + """ + meta_path = _project_meta_path(store, _project_hash(working_dir)) + existing = _read_json_dict(meta_path) or {} now = time.time() - meta: Dict = {"workdir": str(_normalize_path(working_dir)), - "created_at": now, "last_touch": now} - evidence = _volume_evidence(_normalize_path(working_dir)) - if evidence: - meta.update(evidence) - if meta_path.exists(): - try: - existing = json.loads(meta_path.read_text(encoding="utf-8")) - if isinstance(existing, dict): - meta["created_at"] = existing.get("created_at", now) - if not evidence: - # Fresh probe failed — keep the previously recorded - # parent identity rather than dropping it. Stale evidence - # only makes pruning MORE conservative (mismatch => not - # an orphan). - for key in ("workdir_parent_dev", "workdir_parent_ino"): - if key in existing: - meta[key] = existing[key] - except (OSError, ValueError): - pass + meta: Dict = dict(existing) + meta.update({"workdir": str(_normalize_path(working_dir)), "last_touch": now}) + meta.setdefault("created_at", now) + meta.update(_volume_evidence(_normalize_path(working_dir))) try: meta_path.parent.mkdir(parents=True, exist_ok=True) meta_path.write_text(json.dumps(meta), encoding="utf-8") @@ -606,71 +555,36 @@ def _register_project(store: Path, working_dir: str) -> None: logger.debug("Could not write project metadata %s: %s", meta_path, exc) -def _touch_project(store: Path, working_dir: str) -> None: - """Update last_touch for a project, preserving created_at.""" - dir_hash = _project_hash(working_dir) - meta_path = _project_meta_path(store, dir_hash) - if not meta_path.exists(): - _register_project(store, working_dir) - return - try: - meta = json.loads(meta_path.read_text(encoding="utf-8")) - except (OSError, ValueError): - meta = {} - if not isinstance(meta, dict): - meta = {} - meta["workdir"] = str(_normalize_path(working_dir)) - meta["last_touch"] = time.time() - meta.setdefault("created_at", meta["last_touch"]) - # Refresh the parent-directory identity while the project is observably - # live — a remount can legitimately change it (new device, new inode). - # On probe failure the previous evidence is kept: stale evidence can only - # make pruning MORE conservative (mismatch => not an orphan). - evidence = _volume_evidence(_normalize_path(working_dir)) - if evidence: - meta.update(evidence) - try: - meta_path.write_text(json.dumps(meta), encoding="utf-8") - except OSError as exc: - logger.debug("Could not update project metadata %s: %s", meta_path, exc) +_touch_project = _register_project # per-turn touch == re-register (same upsert) def _list_projects(store: Path) -> List[Dict]: - """Return all registered projects under the store.""" + """Return all registered projects under the store (each tagged with ``_hash``).""" projects_dir = store / _PROJECTS_DIRNAME if not projects_dir.exists(): return [] out: List[Dict] = [] for meta_path in projects_dir.glob("*.json"): - dir_hash = meta_path.stem - try: - meta = json.loads(meta_path.read_text(encoding="utf-8")) - except (OSError, ValueError): + meta = _read_json_dict(meta_path) + if meta is None: continue - if not isinstance(meta, dict): - continue - meta["_hash"] = dir_hash + meta["_hash"] = meta_path.stem out.append(meta) return out def _pre_v2_shadow_repos(base: Path) -> List[Dict]: - """Return pre-v2 per-project shadow repos still directly under ``base``. + """Pre-v2 per-project shadow repos (``base//HEAD``) still under ``base``. - Pre-v2 layout kept one shadow git repo per working directory directly - under ``CHECKPOINT_BASE`` (identified by a ``HEAD`` file). This is the - single source of truth for that scan so a preview built from it (e.g. - ``store_status``) always matches what ``prune_checkpoints`` deletes. + Single source of truth for that scan so a ``store_status`` preview always + matches what ``prune_checkpoints`` deletes. """ out: List[Dict] = [] if not base.exists(): return out for child in base.iterdir(): - if not child.is_dir(): - continue - if child.name == _STORE_DIRNAME or child.name.startswith(_LEGACY_PREFIX): - continue - if not (child / "HEAD").exists(): + if (not child.is_dir() or child.name == _STORE_DIRNAME + or child.name.startswith(_LEGACY_PREFIX) or not (child / "HEAD").exists()): continue workdir: Optional[str] = None marker_unreadable = False @@ -679,19 +593,19 @@ def _pre_v2_shadow_repos(base: Path) -> List[Dict]: try: workdir = wd_marker.read_text(encoding="utf-8").strip() except (OSError, UnicodeDecodeError): - # The marker is there, we just could not read it. That is - # not evidence the project is gone — never delete on it. - workdir = None + # Marker present but unreadable: no evidence the project is gone. marker_unreadable = True - out.append({ - "path": child, - "workdir": workdir, - "exists": bool(workdir) and Path(workdir).exists(), - "marker_unreadable": marker_unreadable, - }) + out.append({"path": child, "workdir": workdir, "marker_unreadable": marker_unreadable, + "exists": bool(workdir) and Path(workdir).exists()}) return out +def _legacy_archives(base: Path) -> List[Path]: + """``legacy-*`` archive dirs directly under ``base``.""" + return [c for c in list(base.iterdir()) + if c.is_dir() and c.name.startswith(_LEGACY_PREFIX)] + + def _dir_file_count(path: str) -> int: """Quick file count estimate (stops early if over _MAX_FILES).""" count = 0 @@ -720,67 +634,89 @@ def _dir_size_bytes(path: Path) -> int: return total -# Backwards-compatibility shim — some tests import ``_init_shadow_repo`` and -# look for ``HEAD``/``info/exclude``/``HERMES_WORKDIR``. In v2 we also write -# those markers, but inside the shared store + under ``projects/.json``. -# The shim initialises the store and registers the project so the old -# surface keeps roughly the same shape. -def _init_shadow_repo(shadow_repo: Path, working_dir: str) -> Optional[str]: - """Backwards-compatible initialiser. - - In v1 ``shadow_repo`` was a per-project dir; in v2 it's the shared - ``store/`` path (or a test path that we respect). We initialise the - store at ``shadow_repo``, create per-project markers, and return None - on success. - """ - err = _init_store(shadow_repo, working_dir) - if err: - return err - _register_project(shadow_repo, working_dir) - # Compat marker for tests that look at HERMES_WORKDIR - # (write in addition to the JSON metadata). +def _newest_mtime(path: Path) -> float: + """Newest mtime under ``path`` (0.0 when nothing is statable).""" + newest = 0.0 try: - (shadow_repo / "HERMES_WORKDIR").write_text( - str(_normalize_path(working_dir)) + "\n", encoding="utf-8" - ) + for p in path.rglob("*"): + try: + newest = max(newest, p.stat().st_mtime) + except OSError: + continue except OSError: pass - return None + return newest -# --------------------------------------------------------------------------- # CheckpointManager -# --------------------------------------------------------------------------- + +class _ProjectRefs(NamedTuple): + """Store coordinates for one working directory (resolved at call time).""" + abs_dir: str + store: Path + dir_hash: str + index_file: Path + ref: str + + @property + def store_ready(self) -> bool: + return _store_has_head(self.store) + + +def _project_refs(working_dir: str) -> _ProjectRefs: + abs_dir = str(_normalize_path(working_dir)) + store, dir_hash = _store_path(CHECKPOINT_BASE), _project_hash(abs_dir) + return _ProjectRefs(abs_dir, store, dir_hash, _index_path(store, dir_hash), _ref_name(dir_hash)) + + +def _stage_all(p: _ProjectRefs) -> Tuple[bool, str, str]: + """``git add -A`` into the per-project index.""" + return _run_git(["add", "-A"], p.store, p.abs_dir, + timeout=_GIT_TIMEOUT * 2, index_file=p.index_file) + + +def _diff_staged_tree(p: _ProjectRefs, *diff_args: List[str]) -> List[Tuple[bool, str, str]]: + """Stage the working tree (so new files show), run each ``git diff`` variant, + then point the index back at the ref so it doesn't drift.""" + _stage_all(p) + results = [_run_git(args, p.store, p.abs_dir, index_file=p.index_file) for args in diff_args] + _run_git(["read-tree", p.ref], p.store, p.abs_dir, + index_file=p.index_file, allowed_returncodes={128}) + return results + + +def _commit_exists(p: _ProjectRefs, commit_hash: str) -> Tuple[bool, str]: + ok, _, err = _run_git(["cat-file", "-t", commit_hash], p.store, p.abs_dir) + return ok, err + + +def _restore_ok(commit_hash: str, reason: str, abs_dir: str, **extra) -> Dict: + return {"success": True, "restored_to": commit_hash[:8], "reason": reason, + "directory": abs_dir, **extra} + + +@dataclass +class _SafeRestoreTargets: + checkout: List[str] = field(default_factory=list) + kept_oversize: List[str] = field(default_factory=list) + failed_deletes: List[str] = field(default_factory=list) + class CheckpointManager: """Manages automatic filesystem checkpoints. - Designed to be owned by AIAgent. Call ``new_turn()`` at the start of - each conversation turn and ``ensure_checkpoint(dir, reason)`` before - any file-mutating tool call. The manager deduplicates so at most one - snapshot is taken per directory per turn. + Owned by AIAgent: call ``new_turn()`` at the start of each turn and + ``ensure_checkpoint(dir, reason)`` before any file-mutating tool call; at most + one snapshot is taken per directory per turn. - Parameters - ---------- - enabled : bool - Master switch (from config / CLI flag). - max_snapshots : int - Keep at most this many checkpoints per directory. - max_total_size_mb : int - Hard ceiling on total store size. Oldest checkpoints per project - are dropped when the store exceeds this after a commit. - max_file_size_mb : int - Skip adding any single file larger than this to a checkpoint. - (Implemented via ``.gitignore`` excludes + a post-stage size check.) + ``max_snapshots`` caps checkpoints per directory; ``max_total_size_mb`` is a + hard ceiling on store size (oldest checkpoints per project dropped after a + commit); ``max_file_size_mb`` keeps any larger single file out of checkpoints + (excludes + a post-stage size check). """ - def __init__( - self, - enabled: bool = False, - max_snapshots: int = 20, - max_total_size_mb: int = 500, - max_file_size_mb: int = 10, - ): + def __init__(self, enabled: bool = False, max_snapshots: int = 20, + max_total_size_mb: int = 500, max_file_size_mb: int = 10): self.enabled = enabled self.max_snapshots = max(1, int(max_snapshots)) self.max_total_size_mb = max(0, int(max_total_size_mb)) @@ -788,27 +724,17 @@ class CheckpointManager: self._checkpointed_dirs: Set[str] = set() self._git_available: Optional[bool] = None # lazy probe - # ------------------------------------------------------------------ - # Turn lifecycle - # ------------------------------------------------------------------ - def new_turn(self) -> None: """Reset per-turn dedup. Call at the start of each agent iteration.""" self._checkpointed_dirs.clear() - # ------------------------------------------------------------------ - # Public API - # ------------------------------------------------------------------ + # --- public API --- def record_agent_write(self, file_path: str) -> None: - """Record the content hash of a file Hermes just successfully wrote. + """Record the content hash of a file Hermes just wrote (agent-write ledger). - Feeds the agent-write ledger used by :meth:`restore` in safe mode: - at restore time, a file whose current content no longer matches the - recorded hash was hand-edited by the user after Hermes last touched - it, and is skipped instead of clobbered. - - Never raises — the ledger is best-effort bookkeeping. + Safe-mode :meth:`restore` skips files whose current content no longer + matches, i.e. the user hand-edited them afterwards. Never raises. """ if not self.enabled: return @@ -829,76 +755,47 @@ class CheckpointManager: def safe_restore_plan(self, working_dir: str, commit_hash: str) -> Dict: """Classify files changed since ``commit_hash`` for a safe restore. - Returns ``{"success", "restore": [rel...], "skipped": [rel...], - "error"?}`` where ``restore`` lists files whose current content - still matches what Hermes last wrote (per the agent-write ledger) - and ``skipped`` lists files the user hand-edited after Hermes' - last write or that Hermes never wrote at all. + ``restore`` lists files still matching what Hermes last wrote (or deleted + since — their last content was Hermes-authored); ``skipped`` lists files + the user hand-edited afterwards or Hermes never wrote. ``ledger_empty`` + signals no ledger exists, so callers fall back to a full restore. """ hash_err = _validate_commit_hash(commit_hash) if hash_err: return {"success": False, "error": hash_err} - abs_dir = str(_normalize_path(working_dir)) - store = _store_path(CHECKPOINT_BASE) - if not (store / "HEAD").exists(): - return {"success": False, "error": "No checkpoints exist for this directory"} + p = _project_refs(working_dir) + if not p.store_ready: + return _no_store_result() - dir_hash = _project_hash(abs_dir) - index_file = _index_path(store, dir_hash) - - # Stage the current tree so the name-only diff sees new files too. - _run_git(["add", "-A"], store, abs_dir, - timeout=_GIT_TIMEOUT * 2, index_file=index_file) - ok, names_out, err = _run_git( - ["diff", "--name-only", commit_hash, "--cached"], - store, abs_dir, index_file=index_file, - ) - # Reset the index back to the project ref so it doesn't drift. - _run_git(["read-tree", _ref_name(dir_hash)], store, abs_dir, - index_file=index_file, allowed_returncodes={128}) + (ok, names_out, err), = _diff_staged_tree(p, ["diff", "--name-only", commit_hash, "--cached"]) if not ok: return {"success": False, "error": f"Could not compute changed files: {err}"} - ledger = _load_ledger(store, dir_hash) + ledger = _load_ledger(p.store, p.dir_hash) if not ledger: - # No agent-write ledger yet (pre-existing store, or Hermes has - # not written any files here since the ledger was introduced). - # Signal callers to fall back to a full restore rather than - # skipping every file. - return {"success": True, "restore": [], "skipped": [], - "ledger_empty": True} + return {"success": True, "restore": [], "skipped": [], "ledger_empty": True} restore: List[str] = [] skipped: List[str] = [] for rel in names_out.splitlines(): rel = rel.strip() if not rel: continue - abs_path = Path(abs_dir) / rel + abs_path = Path(p.abs_dir) / rel entry = ledger.get(str(abs_path)) recorded = entry.get("sha256") if isinstance(entry, dict) else None if recorded is None: - # Hermes never wrote this file (or the ledger predates it) — - # do not touch it in safe mode. skipped.append(rel) continue current = _hash_file(abs_path) - if current is None: - # File deleted since Hermes wrote it: restoring it back is - # safe — its last content was Hermes-authored. - restore.append(rel) - elif current == recorded: + if current is None or current == recorded: restore.append(rel) else: skipped.append(rel) return {"success": True, "restore": restore, "skipped": skipped} def ensure_checkpoint(self, working_dir: str, reason: str = "auto") -> bool: - """Take a checkpoint if enabled and not already done this turn. - - Returns True if a checkpoint was taken, False otherwise. - Never raises — all errors are silently logged. - """ + """Take a checkpoint if enabled and not already done this turn. Never raises.""" if not self.enabled: return False @@ -910,15 +807,11 @@ class CheckpointManager: return False abs_dir = str(_normalize_path(working_dir)) - - # Skip root, home, and other overly broad directories - if abs_dir in {"/", str(Path.home())}: + if abs_dir in {"/", str(Path.home())}: # never snapshot root/home logger.debug("Checkpoint skipped: directory too broad (%s)", abs_dir) return False - if abs_dir in self._checkpointed_dirs: return False - self._checkpointed_dirs.add(abs_dir) try: @@ -929,56 +822,39 @@ class CheckpointManager: def list_checkpoints(self, working_dir: str) -> List[Dict]: """List available checkpoints for a directory (most recent first).""" - abs_dir = str(_normalize_path(working_dir)) - store = _store_path(CHECKPOINT_BASE) - - if not (store / "HEAD").exists(): + p = _project_refs(working_dir) + if not p.store_ready: return [] - ref = _ref_name(_project_hash(abs_dir)) ok, stdout, _ = _run_git( - ["log", ref, "--format=%H|%h|%aI|%s", "-n", str(self.max_snapshots)], - store, abs_dir, - allowed_returncodes={128, 129}, - ) - + ["log", p.ref, "--format=%H|%h|%aI|%s", "-n", str(self.max_snapshots)], + p.store, p.abs_dir, allowed_returncodes={128, 129}) if not ok or not stdout: return [] results: List[Dict] = [] for line in stdout.splitlines(): parts = line.split("|", 3) - if len(parts) == 4: - entry = { - "hash": parts[0], - "short_hash": parts[1], - "timestamp": parts[2], - "reason": parts[3], - "files_changed": 0, - "insertions": 0, - "deletions": 0, - } - stat_ok, stat_out, _ = _run_git( - ["diff", "--shortstat", f"{parts[0]}~1", parts[0]], - store, abs_dir, - allowed_returncodes={128, 129}, - ) - if stat_ok and stat_out: - self._parse_shortstat(stat_out, entry) - results.append(entry) + if len(parts) != 4: + continue + entry = { + "hash": parts[0], "short_hash": parts[1], "timestamp": parts[2], + "reason": parts[3], "files_changed": 0, "insertions": 0, "deletions": 0, + } + stat_ok, stat_out, _ = _run_git(["diff", "--shortstat", f"{parts[0]}~1", parts[0]], + p.store, p.abs_dir, allowed_returncodes={128, 129}) + if stat_ok and stat_out: + self._parse_shortstat(stat_out, entry) + results.append(entry) return results def list_all_checkpoints(self) -> List[Dict]: """List checkpoints across every registered project (most recent first). - Surgical reapply of PR #10633 by @nightq (#10505) onto the v2 - single-store layout: iterate ``projects/.json`` metadata via - ``_list_projects`` instead of the pre-v2 per-shadow-dir scan. Each - entry carries the extra ``workdir`` key so callers can label which - project a checkpoint belongs to. + Each entry carries a ``workdir`` key so callers can label its project. """ store = _store_path(CHECKPOINT_BASE) - if not (store / "HEAD").exists(): + if not _store_has_head(store): return [] results: List[Dict] = [] for meta in _list_projects(store): @@ -994,15 +870,10 @@ class CheckpointManager: @staticmethod def _parse_shortstat(stat_line: str, entry: Dict) -> None: """Parse git --shortstat output into entry dict.""" - m = re.search(r'(\d+) file', stat_line) - if m: - entry["files_changed"] = int(m.group(1)) - m = re.search(r'(\d+) insertion', stat_line) - if m: - entry["insertions"] = int(m.group(1)) - m = re.search(r'(\d+) deletion', stat_line) - if m: - entry["deletions"] = int(m.group(1)) + for key, pattern in _SHORTSTAT_FIELDS: + m = re.search(pattern, stat_line) + if m: + entry[key] = int(m.group(1)) def diff(self, working_dir: str, commit_hash: str) -> Dict: """Show diff between a checkpoint and the current working tree.""" @@ -1010,69 +881,29 @@ class CheckpointManager: if hash_err: return {"success": False, "error": hash_err} - abs_dir = str(_normalize_path(working_dir)) - store = _store_path(CHECKPOINT_BASE) - - if not (store / "HEAD").exists(): - return {"success": False, "error": "No checkpoints exist for this directory"} - - ok, _, err = _run_git( - ["cat-file", "-t", commit_hash], store, abs_dir, - ) + p = _project_refs(working_dir) + if not p.store_ready: + return _no_store_result() + ok, _ = _commit_exists(p, commit_hash) if not ok: return {"success": False, "error": f"Checkpoint '{commit_hash}' not found"} - dir_hash = _project_hash(abs_dir) - index_file = _index_path(store, dir_hash) - - # Stage current state into the per-project index to compare. - _run_git(["add", "-A"], store, abs_dir, - timeout=_GIT_TIMEOUT * 2, index_file=index_file) - - ok_stat, stat_out, _ = _run_git( - ["diff", "--stat", commit_hash, "--cached"], - store, abs_dir, index_file=index_file, - ) - ok_diff, diff_out, _ = _run_git( - ["diff", commit_hash, "--cached", "--no-color"], - store, abs_dir, index_file=index_file, - ) - - # Reset staged tree back to the project's last checkpoint so the - # index doesn't drift out of sync with the ref. - ref = _ref_name(dir_hash) - _run_git(["read-tree", ref], store, abs_dir, - index_file=index_file, - allowed_returncodes={128}) + (ok_stat, stat_out, _), (ok_diff, diff_out, _) = _diff_staged_tree( + p, ["diff", "--stat", commit_hash, "--cached"], + ["diff", commit_hash, "--cached", "--no-color"]) if not ok_stat and not ok_diff: return {"success": False, "error": "Could not generate diff"} - - return { - "success": True, - "stat": stat_out if ok_stat else "", - "diff": diff_out if ok_diff else "", - } + return {"success": True, "stat": stat_out if ok_stat else "", + "diff": diff_out if ok_diff else ""} def session_diff(self, working_dir: str) -> Dict: - """Show the cumulative diff of everything changed in this directory. + """Cumulative diff of everything changed here (powers ``/diff session``). - This powers ``/diff session``. It answers "what has Hermes changed - here?" by diffing the *earliest retained checkpoint* — the snapshot - taken before the first recorded edit — against the current working - tree. Because checkpoints are captured just before each file-mutating - tool call, that baseline is the pre-edit state, so the diff covers the - first edit and everything after it. - - Note: checkpoints are a persistent per-project ref, so the earliest - *retained* checkpoint may predate the current session (or, after - pruning, postdate its true start). It is an approximation of "what - Hermes changed", not an exact per-session ledger. - - Returns the same shape as :meth:`diff` (``{"success", "stat", - "diff"}``). When no checkpoints exist yet — nothing has been edited — - the call still *succeeds* with empty output and ``"empty": True`` so - callers can show a friendly "no changes" message rather than an error. + Diffs the *earliest retained checkpoint* (pre-first-edit state) against the + working tree; the ref persists per project, so the baseline may predate the + session or postdate it after pruning — an approximation. Same shape as + :meth:`diff`; with no checkpoints it *succeeds* with ``"empty": True``. """ checkpoints = self.list_checkpoints(working_dir) if not checkpoints: @@ -1086,325 +917,184 @@ class CheckpointManager: result["empty"] = True return result - def restore( - self, - working_dir: str, - commit_hash: str, - file_path: str = None, - safe: bool = False, - ) -> Dict: + def restore(self, working_dir: str, commit_hash: str, file_path: str = None, + safe: bool = False) -> Dict: """Restore files to a checkpoint state. - With ``safe=True`` (full-directory restores only), files the user - hand-edited after Hermes' last write — per the agent-write ledger — - are left untouched, and only Hermes-authored changes are reverted. - The result gains ``skipped_user_edits`` listing the preserved paths, - ``skipped_oversize`` listing paths kept because the size cap excluded - them from every checkpoint, and — only when a delete failed — - ``failed_deletes`` listing paths that could not be removed. + ``safe=True`` (full-directory only) leaves files the user hand-edited after + Hermes' last write untouched (per the agent-write ledger); the result then + gains ``skipped_user_edits``, ``skipped_oversize`` (size cap kept them out + of every checkpoint) and, only when a delete failed, ``failed_deletes``. """ hash_err = _validate_commit_hash(commit_hash) if hash_err: return {"success": False, "error": hash_err} - abs_dir = str(_normalize_path(working_dir)) - + p = _project_refs(working_dir) + abs_dir = p.abs_dir if file_path: path_err = _validate_file_path(file_path, abs_dir) if path_err: return {"success": False, "error": path_err} - - store = _store_path(CHECKPOINT_BASE) - - if not (store / "HEAD").exists(): - return {"success": False, "error": "No checkpoints exist for this directory"} - - ok, _, err = _run_git( - ["cat-file", "-t", commit_hash], store, abs_dir, - ) + if not p.store_ready: + return _no_store_result() + ok, err = _commit_exists(p, commit_hash) if not ok: return {"success": False, "error": f"Checkpoint '{commit_hash}' not found", "debug": err or None} skipped_user_edits: List[str] = [] - kept_oversize: List[str] = [] - failed_deletes: List[str] = [] restore_paths: Optional[List[str]] = None if safe and not file_path: plan = self.safe_restore_plan(abs_dir, commit_hash) if not plan.get("success"): return {"success": False, "error": plan.get("error", "Safe-restore plan failed")} - if plan.get("ledger_empty"): - # No agent-write history to compare against — fall back to - # the classic full restore rather than restoring nothing. - restore_paths = None - else: + # No agent-write history to compare against => classic full restore. + if not plan.get("ledger_empty"): restore_paths = plan["restore"] skipped_user_edits = plan["skipped"] if not restore_paths: - return { - "success": True, - "restored_to": commit_hash[:8], - "reason": "nothing to restore (all changed files were user-edited)", - "directory": abs_dir, - "restored_files": [], - "skipped_user_edits": skipped_user_edits, - "skipped_oversize": [], - } + return _restore_ok( + commit_hash, "nothing to restore (all changed files were user-edited)", + abs_dir, restored_files=[], skipped_user_edits=skipped_user_edits, + skipped_oversize=[], + ) # Take a pre-rollback snapshot so you can undo the undo. self._take(abs_dir, f"pre-rollback snapshot (restoring to {commit_hash[:8]})") - dir_hash = _project_hash(abs_dir) - index_file = _index_path(store, dir_hash) - + targets = _SafeRestoreTargets() + checkout = [file_path or "."] if restore_paths is not None: - # Split into files present in the checkpoint (checkout) and - # Hermes-created files absent from it (delete to restore state). - checkout_targets: List[str] = [] - delete_targets: List[str] = [] - for rel in restore_paths: - ok_in_commit, _, _ = _run_git( - ["cat-file", "-e", f"{commit_hash}:{rel}"], - store, abs_dir, allowed_returncodes={1, 128}, - ) - if ok_in_commit: - checkout_targets.append(rel) - elif self._exceeds_size_cap(Path(abs_dir) / rel): - # Absent from the checkpoint because ``max_file_size_mb`` - # kept it out (_drop_oversize_from_index), not because - # Hermes created it. Deleting it would not restore a prior - # state — no checkpoint holds one — it would destroy the - # only copy. The ledger records a content hash, not whether - # a write created or modified the file, so an oversize path - # cannot be proven agent-created; leaving it costs a stale - # file, deleting it costs the file. - kept_oversize.append(rel) - else: - delete_targets.append(rel) - for rel in delete_targets: - try: - target = Path(abs_dir) / rel - if target.is_file() or target.is_symlink(): - target.unlink() - except OSError as exc: - logger.warning( - "Safe restore: could not remove %s: %s", rel, exc, - ) - failed_deletes.append(rel) - if not checkout_targets: - ok, stdout, err = True, "", "" - else: - ok, stdout, err = _run_git( - ["checkout", commit_hash, "--", *checkout_targets], - store, abs_dir, timeout=_GIT_TIMEOUT * 2, - index_file=index_file, - ) - else: - ok, stdout, err = _run_git( - ["checkout", commit_hash, "--", file_path if file_path else "."], - store, abs_dir, timeout=_GIT_TIMEOUT * 2, - index_file=index_file, - ) - + targets = self._apply_safe_restore_deletes(p, commit_hash, restore_paths) + checkout = targets.checkout + ok, err = True, "" + if checkout: + ok, _, err = _run_git(["checkout", commit_hash, "--", *checkout], p.store, abs_dir, + timeout=_GIT_TIMEOUT * 2, index_file=p.index_file) if not ok: - return {"success": False, "error": f"Restore failed: {err}", - "debug": err or None} + return {"success": False, "error": f"Restore failed: {err}", "debug": err or None} - ok2, reason_out, _ = _run_git( - ["log", "--format=%s", "-1", commit_hash], store, abs_dir, - ) - reason = reason_out if ok2 else "unknown" - - result = { - "success": True, - "restored_to": commit_hash[:8], - "reason": reason, - "directory": abs_dir, - } + ok2, reason_out, _ = _run_git(["log", "--format=%s", "-1", commit_hash], p.store, abs_dir) + result = _restore_ok(commit_hash, reason_out if ok2 else "unknown", abs_dir) if file_path: result["file"] = file_path if restore_paths is not None: - # Only what was actually acted on. A kept oversize path was not - # restored (and a failed unlink left the file in place), and - # reporting either as restored is how the data loss above stayed - # silent: the user was told "Restored" for a file that had just - # been unlinked. - not_restored = set(kept_oversize) | set(failed_deletes) - result["restored_files"] = [ - rel for rel in restore_paths if rel not in not_restored - ] + # Report only what was actually acted on: a kept oversize path or a + # failed unlink left the file in place and must not read as "Restored". + not_restored = set(targets.kept_oversize) | set(targets.failed_deletes) + result["restored_files"] = [rel for rel in restore_paths if rel not in not_restored] result["skipped_user_edits"] = skipped_user_edits - result["skipped_oversize"] = kept_oversize - if failed_deletes: - result["failed_deletes"] = failed_deletes + result["skipped_oversize"] = targets.kept_oversize + if targets.failed_deletes: + result["failed_deletes"] = targets.failed_deletes return result - def get_working_dir_for_path(self, file_path: str) -> str: - """Resolve a file path to its working directory for checkpointing.""" - path = _normalize_path(file_path) - if path.is_dir(): - candidate = path - else: - candidate = path.parent + def _apply_safe_restore_deletes(self, p: _ProjectRefs, commit_hash: str, + restore_paths: List[str]) -> _SafeRestoreTargets: + """Split ledger-approved paths into checkout targets and delete the rest. - markers = {".git", "pyproject.toml", "package.json", "Cargo.toml", - "go.mod", "Makefile", "pom.xml", ".hg", "Gemfile"} + A path absent from the checkpoint is Hermes-created (delete to restore) — + unless ``max_file_size_mb`` kept it out of every checkpoint: no prior copy + exists and the ledger can't prove it agent-created (it records hashes, not + create-vs-modify), so leaving it costs a stale file, deleting costs the file. + """ + targets = _SafeRestoreTargets() + delete_targets: List[str] = [] + for rel in restore_paths: + ok_in_commit, _, _ = _run_git(["cat-file", "-e", f"{commit_hash}:{rel}"], + p.store, p.abs_dir, allowed_returncodes={1, 128}) + if ok_in_commit: + targets.checkout.append(rel) + elif self._exceeds_size_cap(Path(p.abs_dir) / rel): + targets.kept_oversize.append(rel) + else: + delete_targets.append(rel) + for rel in delete_targets: + try: + target = Path(p.abs_dir) / rel + if target.is_file() or target.is_symlink(): + target.unlink() + except OSError as exc: + logger.warning("Safe restore: could not remove %s: %s", rel, exc) + targets.failed_deletes.append(rel) + return targets + + def get_working_dir_for_path(self, file_path: str) -> str: + """Resolve a file path to its working directory (nearest project-marker ancestor).""" + path = _normalize_path(file_path) + candidate = path if path.is_dir() else path.parent check = candidate while check != check.parent: - if any((check / m).exists() for m in markers): + if any((check / m).exists() for m in _PROJECT_MARKERS): return str(check) check = check.parent - return str(candidate) - # ------------------------------------------------------------------ - # Internal - # ------------------------------------------------------------------ + # --- internal --- def _take(self, working_dir: str, reason: str) -> bool: """Take a snapshot. Returns True on success.""" - store = _store_path(CHECKPOINT_BASE) - - err = _init_store(store, working_dir) + p = _project_refs(working_dir) + err = _init_store(p.store, working_dir) if err: logger.debug("Checkpoint store init failed: %s", err) return False - _touch_project(store, working_dir) + _touch_project(p.store, working_dir) # Quick size guard — don't try to snapshot enormous directories if _dir_file_count(working_dir) > _MAX_FILES: logger.debug("Checkpoint skipped: >%d files in %s", _MAX_FILES, working_dir) return False - dir_hash = _project_hash(working_dir) - index_file = _index_path(store, dir_hash) - ref = _ref_name(dir_hash) + ref_commit = _ref_tip(p.store, working_dir, p.ref) + _seed_project_index(p, ref_commit) - # Seed the per-project index from the last checkpoint, if any, so the - # diff/commit machinery sees only changes since then. On first call, - # clear the index so ``git add -A`` produces a clean tree. - if index_file.exists(): - # Reset index to current ref tip to avoid accumulating stale paths. - ok_ref, ref_commit, _ = _run_git( - ["rev-parse", "--verify", ref + "^{commit}"], - store, working_dir, - allowed_returncodes={128}, - ) - if ok_ref and ref_commit: - _run_git( - ["read-tree", ref_commit], - store, working_dir, - index_file=index_file, - allowed_returncodes={128}, - ) - else: - try: - index_file.unlink() - except OSError: - pass - else: - # First snapshot for this project. - index_file.parent.mkdir(parents=True, exist_ok=True) - - # Stage with per-project index. Include a per-stage file-size filter - # via ``core.bigFileThreshold`` is not what we want — instead, we - # rely on the exclude file for broad patterns and post-stage prune - # any path whose size exceeds max_file_size_mb. - ok, _, err = _run_git( - ["add", "-A"], store, working_dir, - timeout=_GIT_TIMEOUT * 2, index_file=index_file, - ) + # Broad patterns come from the exclude file; oversize paths are dropped post-stage. + ok, _, err = _stage_all(p) if not ok: logger.debug("Checkpoint git-add failed: %s", err) return False - if self.max_file_size_mb > 0: - self._drop_oversize_from_index(store, working_dir, index_file) + self._drop_oversize_from_index(p.store, working_dir, p.index_file) - # Compare against the current ref tip (not HEAD — HEAD points to a - # branch that doesn't exist on a bare store, so ``diff --cached`` - # against HEAD would always show "new file" for every staged path). - ok_ref, ref_commit, _ = _run_git( - ["rev-parse", "--verify", ref + "^{commit}"], - store, working_dir, - allowed_returncodes={128}, - ) - has_ref = ok_ref and bool(ref_commit) + skip = _index_unchanged_reason(p, ref_commit) + if skip: + logger.debug("Checkpoint skipped: %s in %s", skip, working_dir) + return False - if has_ref: - ok_diff, _, _ = _run_git( - ["diff-index", "--cached", "--quiet", ref_commit], - store, working_dir, - allowed_returncodes={1}, - index_file=index_file, - ) - if ok_diff: - logger.debug("Checkpoint skipped: no changes in %s", working_dir) - return False - else: - # No ref yet — skip only if the index is empty. - ok_ls, ls_out, _ = _run_git( - ["ls-files", "--cached"], - store, working_dir, - index_file=index_file, - ) - if ok_ls and not ls_out.strip(): - logger.debug("Checkpoint skipped: empty tree in %s", working_dir) - return False - - # Write tree from per-project index. - ok_tree, tree_sha, err = _run_git( - ["write-tree"], store, working_dir, - index_file=index_file, - ) + ok_tree, tree_sha, err = _run_git(["write-tree"], p.store, working_dir, index_file=p.index_file) if not ok_tree or not tree_sha: logger.debug("Checkpoint write-tree failed: %s", err) return False - # Build commit (parent = current ref tip, if any). - commit_args = ["commit-tree", tree_sha, "-m", reason, "--no-gpg-sign"] - if has_ref: - commit_args = ["commit-tree", tree_sha, "-p", ref_commit, "-m", reason, "--no-gpg-sign"] ok_commit, new_sha, err = _run_git( - commit_args, store, working_dir, - index_file=index_file, + _commit_tree_args(tree_sha, reason, ref_commit), + p.store, working_dir, index_file=p.index_file, ) if not ok_commit or not new_sha: logger.debug("Checkpoint commit-tree failed: %s", err) return False - # Update the per-project ref. - update_args = ["update-ref", ref, new_sha] - if has_ref: - update_args = ["update-ref", ref, new_sha, ref_commit] - ok_update, _, err = _run_git( - update_args, store, working_dir, - ) + update_args = ["update-ref", p.ref, new_sha] + ([ref_commit] if ref_commit else []) + ok_update, _, err = _run_git(update_args, p.store, working_dir) if not ok_update: logger.debug("Checkpoint update-ref failed: %s", err) return False logger.debug("Checkpoint taken in %s: %s (%s)", working_dir, reason, new_sha[:8]) - - # Real pruning — drop old commits beyond max_snapshots. - self._prune(store, working_dir, ref) - - # Enforce global size cap. - self._enforce_size_cap(store) - + self._prune(p.store, working_dir, p.ref) + self._enforce_size_cap(p.store) return True def _exceeds_size_cap(self, path: Path) -> bool: - """Whether *path* is larger than ``max_file_size_mb``. + """Whether *path* is larger than ``max_file_size_mb`` (0 disables; unstattable => False). - The same test :meth:`_drop_oversize_from_index` applies when building a - checkpoint, so "excluded from the checkpoint" and "refused deletion at - restore" agree on one definition. A cap of 0 disables it, and an - unstattable path is not claimed to be oversize. + The ONE predicate for both "excluded from the checkpoint" and "refused + deletion at restore" — a drifted threshold would delete a file with no copy. """ - cap = self.max_file_size_mb * 1024 * 1024 + cap = self.max_file_size_mb * _MB if cap <= 0: return False try: @@ -1415,201 +1105,90 @@ class CheckpointManager: def _drop_oversize_from_index( self, store: Path, working_dir: str, index_file: Path, ) -> None: - """Remove any staged file larger than ``max_file_size_mb`` from the index. - - Lets the agent keep snapshotting source code while refusing to - swallow generated assets (datasets, model weights, logs, videos). - """ + """Unstage files larger than ``max_file_size_mb`` (datasets, weights, videos).""" if self.max_file_size_mb <= 0: return ok, stdout, _ = _run_git( - ["ls-files", "--cached", "-z"], - store, working_dir, index_file=index_file, + ["ls-files", "--cached", "-z"], store, working_dir, index_file=index_file, ) if not ok or not stdout: return - # ls-files -z output is NUL-separated. _run_git strips trailing - # whitespace but that leaves NULs alone; rebuild list. + # NUL-separated; _run_git's strip() leaves NULs alone. paths = [p for p in stdout.split("\x00") if p] abs_workdir = _normalize_path(working_dir) - # Same predicate safe restore consults, called rather than restated: - # a threshold that drifted between the two would make a file both - # absent from the checkpoint and not recognised as capped at restore, - # which is precisely the deletion this change exists to prevent. - oversize = [ - rel for rel in paths if self._exceeds_size_cap(abs_workdir / rel) - ] + oversize = [rel for rel in paths if self._exceeds_size_cap(abs_workdir / rel)] if not oversize: return logger.debug( "Checkpoint: dropping %d oversize file(s) (>%d MB) from index", len(oversize), self.max_file_size_mb, ) - # Use --pathspec-from-file for safety with many paths. - # Chunk into manageable batches. - BATCH = 200 - for i in range(0, len(oversize), BATCH): - chunk = oversize[i:i + BATCH] - _run_git( - ["rm", "--cached", "--quiet", "--"] + chunk, - store, working_dir, index_file=index_file, - allowed_returncodes={128}, - ) + for i in range(0, len(oversize), 200): # chunk: never overflow argv + _run_git(["rm", "--cached", "--quiet", "--"] + oversize[i:i + 200], + store, working_dir, index_file=index_file, allowed_returncodes={128}) def _prune(self, store: Path, working_dir: str, ref: str) -> None: - """Keep only the last ``max_snapshots`` commits on the per-project ref. + """Rewrite the ref to its last ``max_snapshots`` commits and gc. - v1's ``_prune`` was documented as a no-op (``git``'s pack mechanism - was supposed to handle it, but only the log view was limited — loose - objects accumulated forever). v2 actually rewrites the ref to drop - commits older than ``max_snapshots`` and then runs ``git gc`` on the - store so unreachable objects are reclaimed. + Only limiting the log view (v1) let loose objects accumulate forever. """ - ok, stdout, _ = _run_git( - ["rev-list", "--count", ref], store, working_dir, - allowed_returncodes={128}, - ) - if not ok: + if _ref_commit_count(store, working_dir, ref) <= self.max_snapshots: return - try: - count = int(stdout) - except ValueError: + commits = _ref_commits_oldest_first(store, working_dir, ref) + if not commits: return - if count <= self.max_snapshots: + tip = _rebuild_linear_chain(store, working_dir, commits[-self.max_snapshots:]) + if tip is None: return - - # Collect commits oldest → newest, take last N. - ok_list, list_out, _ = _run_git( - ["rev-list", "--reverse", ref], store, working_dir, - ) - if not ok_list or not list_out: - return - commits = list_out.splitlines() - keep = commits[-self.max_snapshots:] - - # Rebuild a linear chain off keep[0]'s tree. - new_parent: Optional[str] = None - for sha in keep: - ok_tree, tree_sha, _ = _run_git( - ["rev-parse", f"{sha}^{{tree}}"], store, working_dir, - ) - if not ok_tree or not tree_sha: - return - ok_msg, msg, _ = _run_git( - ["log", "--format=%s", "-1", sha], store, working_dir, - ) - commit_msg = msg if ok_msg and msg else "checkpoint" - args = ["commit-tree", tree_sha, "-m", commit_msg, "--no-gpg-sign"] - if new_parent is not None: - args = ["commit-tree", tree_sha, "-p", new_parent, - "-m", commit_msg, "--no-gpg-sign"] - ok_commit, new_sha, _ = _run_git(args, store, working_dir) - if not ok_commit or not new_sha: - return - new_parent = new_sha - - if new_parent is None: - return - _run_git(["update-ref", ref, new_parent], store, working_dir) - - # Reclaim objects from the dropped commits. - _run_git( - ["reflog", "expire", "--expire=now", "--all"], - store, working_dir, - ) - _run_git( - ["gc", "--prune=now", "--quiet"], - store, working_dir, timeout=_GIT_TIMEOUT * 3, - ) - _repair_bare_repo_dirs(store) + _run_git(["update-ref", ref, tip], store, working_dir) + _gc_store(store, working_dir) def _enforce_size_cap(self, store: Path) -> None: - """If total store size exceeds ``max_total_size_mb``, drop oldest - checkpoints across ALL projects until under the cap. - """ + """Drop oldest checkpoints across ALL projects until under ``max_total_size_mb``.""" if self.max_total_size_mb <= 0: return - cap_bytes = self.max_total_size_mb * 1024 * 1024 + cap_bytes = self.max_total_size_mb * _MB size = _dir_size_bytes(store) if size <= cap_bytes: return logger.info( "Checkpoint store exceeded %d MB (actual %d MB) — pruning oldest", - self.max_total_size_mb, size // (1024 * 1024), + self.max_total_size_mb, size // _MB, ) + working_dir = str(store.parent) + if _shrink_store_to_cap(store, working_dir, cap_bytes): + _gc_store(store, working_dir) - # Collect (commit_time, ref, sha) across all per-project refs. - ok, stdout, _ = _run_git( - ["for-each-ref", "--format=%(refname)", _REFS_PREFIX], - store, str(store.parent), - allowed_returncodes={128}, - ) - if not ok or not stdout: - return - refs = [r for r in stdout.splitlines() if r.strip()] - any_dropped = False - # Round-robin-drop oldest commit per ref until under cap. - for _ in range(20): # hard upper bound to avoid pathological loops - size = _dir_size_bytes(store) - if size <= cap_bytes: - break - for ref in refs: - ok_count, count_out, _ = _run_git( - ["rev-list", "--count", ref], store, str(store.parent), - allowed_returncodes={128}, - ) - try: - count = int(count_out) if ok_count else 0 - except ValueError: - count = 0 - if count <= 1: - continue # keep at least one snapshot per project - ok_list, list_out, _ = _run_git( - ["rev-list", "--reverse", ref], store, str(store.parent), - ) - if not ok_list or not list_out: - continue - commits = list_out.splitlines() - keep = commits[1:] # drop oldest - new_parent: Optional[str] = None - fail = False - for sha in keep: - ok_tree, tree_sha, _ = _run_git( - ["rev-parse", f"{sha}^{{tree}}"], store, str(store.parent), - ) - if not ok_tree or not tree_sha: - fail = True - break - ok_msg, msg, _ = _run_git( - ["log", "--format=%s", "-1", sha], store, str(store.parent), - ) - commit_msg = msg if ok_msg and msg else "checkpoint" - args = ["commit-tree", tree_sha, "-m", commit_msg, "--no-gpg-sign"] - if new_parent is not None: - args = ["commit-tree", tree_sha, "-p", new_parent, - "-m", commit_msg, "--no-gpg-sign"] - ok_commit, new_sha, _ = _run_git(args, store, str(store.parent)) - if not ok_commit or not new_sha: - fail = True - break - new_parent = new_sha - if fail or new_parent is None: - continue - _run_git(["update-ref", ref, new_parent], store, str(store.parent)) - any_dropped = True - if not any_dropped: - break +def _seed_project_index(p: _ProjectRefs, ref_commit: Optional[str]) -> None: + """Reset the per-project index to the ref tip so ``add -A`` sees only changes since. - _run_git( - ["reflog", "expire", "--expire=now", "--all"], - store, str(store.parent), + First snapshot: just create the indexes dir. Existing index with no ref: + discard it so ``add -A`` produces a clean tree. + """ + if not p.index_file.exists(): + p.index_file.parent.mkdir(parents=True, exist_ok=True) + elif ref_commit: + _run_git(["read-tree", ref_commit], p.store, p.abs_dir, + index_file=p.index_file, allowed_returncodes={128}) + else: + _unlink_quiet(p.index_file) + + +def _index_unchanged_reason(p: _ProjectRefs, ref_commit: Optional[str]) -> Optional[str]: + """Why a snapshot would be redundant ("no changes" / "empty tree"), else None. + + Compares against the ref tip, not HEAD — HEAD on the bare store points at a + branch that doesn't exist, so every staged path would look like a new file. + """ + if ref_commit: + ok_diff, _, _ = _run_git( + ["diff-index", "--cached", "--quiet", ref_commit], p.store, p.abs_dir, + allowed_returncodes={1}, index_file=p.index_file, ) - _run_git( - ["gc", "--prune=now", "--quiet"], - store, str(store.parent), timeout=_GIT_TIMEOUT * 3, - ) - _repair_bare_repo_dirs(store) + return "no changes" if ok_diff else None + ok_ls, ls_out, _ = _run_git(["ls-files", "--cached"], p.store, p.abs_dir, index_file=p.index_file) + return "empty tree" if ok_ls and not ls_out.strip() else None def format_checkpoint_list(checkpoints: List[Dict], directory: str) -> str: @@ -1625,16 +1204,10 @@ def format_checkpoint_list(checkpoints: List[Dict], directory: str) -> str: date = cp["timestamp"].split("T")[0] ts = f"{date} {ts}" - files = cp.get("files_changed", 0) - ins = cp.get("insertions", 0) - dele = cp.get("deletions", 0) - if files: - stat = f" ({files} file{'s' if files != 1 else ''}, +{ins}/-{dele})" - else: - stat = "" + files, ins, dele = (cp.get(k, 0) for k in ("files_changed", "insertions", "deletions")) + stat = f" ({files} file{'s' if files != 1 else ''}, +{ins}/-{dele})" if files else "" - # Label per-project entries when showing the cross-project view - # (workdir key only present on list_all_checkpoints results). + # workdir key only present on list_all_checkpoints results. workdir = cp.get("workdir", "") if workdir and directory == "all directories": workdir_short = Path(workdir).name or workdir @@ -1650,25 +1223,7 @@ def format_checkpoint_list(checkpoints: List[Dict], directory: str) -> str: return "\n".join(lines) -# --------------------------------------------------------------------------- -# Auto-maintenance -# --------------------------------------------------------------------------- -# -# v2 rewrite. The sweep now operates on per-project refs inside the shared -# store rather than per-project shadow repos. Legacy-archive dirs -# (``legacy-/``) are swept with the same retention policy. - -_PRUNE_MARKER_NAME = ".last_prune" - - -def _delete_ref(store: Path, ref: str) -> bool: - """Delete a ref from the store. Returns True on success.""" - ok, _, _ = _run_git( - ["update-ref", "-d", ref], store, str(store.parent), - allowed_returncodes={128}, - ) - return ok - +# Auto-maintenance (per-project refs in the shared store + legacy archives) def _workdir_is_observably_gone( workdir: str, @@ -1678,48 +1233,17 @@ def _workdir_is_observably_gone( ) -> bool: """True only when we can positively observe that ``workdir`` was removed. - ``Path.exists()`` returns False for a deleted directory AND for one whose - storage simply is not attached right now — an unplugged external drive, a - network share behind a downed VPN, a bind-mount absent from this - container, an offline Windows mapped drive. Orphan pruning deletes the - project's entire checkpoint history, so treating that ambiguity as - "deleted" throws away the user's restore points over a transient mount - state, unattended, at startup. - - Require corroboration, in three steps. - - First, the parent directory must be present, so the absence of the project - inside it is something we actually observed. When the parent is missing - too, the volume is not there and we know nothing. - - Second, the present parent must be the directory we knew — not merely a - directory at the same path. Unmounting swaps the directory visible at a - mount point: while the volume is attached the path resolves to the - mounted filesystem's root; after detach it resolves to the *underlying* - (underlay) directory, which may carry entries of its own (a ``.keep`` - placeholder, sibling mount points). Those entries were never next to the - project and prove nothing about the volume being attached. So the - parent's ``(st_dev, st_ino)`` must match the identity recorded in the - project's metadata while the project was observably live - (``parent_dev``/``parent_ino``). A mismatch means a different directory - is visible at that path — a detached volume, not an observed deletion. - When no identity was ever recorded (metadata written by an older - version) and ``require_parent_identity`` is True, stay conservative and - do not classify as orphan. Callers that have no identity channel at all - (the frozen pre-v2 layout) pass ``require_parent_identity=False`` to - keep the structural checks only. - - Third, the (identity-confirmed) parent must actually carry information. - Unmounting leaves classic static mount points (``/mnt/volume/proj``, an - fstab entry, a container bind-mount) behind as *empty* directories, so an - empty parent is the signature of a detached volume just as much as of a - deleted project. Prune only when the parent holds something else (we - observed a populated directory that does not contain the project) or is - itself a live mount point (the volume is demonstrably attached and the - project is demonstrably not on it). - - Genuinely abandoned projects are still reclaimed by the retention/stale - rule, which runs off ``last_touch`` rather than a filesystem probe. + ``Path.exists()`` is False for a deleted dir AND for one whose storage is not + attached (unplugged drive, share behind a downed VPN, absent bind-mount). + Orphan pruning deletes the whole history, so ambiguity never counts as + "deleted". Three corroborations: (1) the parent is present (a filesystem root + has none to check); (2) the parent's ``(st_dev, st_ino)`` matches the identity + recorded while the project was live — an unmount exposes the underlay dir, + whose entries prove nothing; with no recorded identity stay conservative unless + ``require_parent_identity=False`` (frozen pre-v2 layout, structural checks + only); (3) the parent is non-empty or a live mount point — unmounting leaves + static mount points behind as *empty* dirs. Abandoned projects are still + reclaimed by the ``last_touch`` retention rule. """ if not workdir: return False @@ -1728,134 +1252,69 @@ def _workdir_is_observably_gone( if path.exists(): return False parent = path.parent - # A path whose parent is itself (a filesystem root) gives us nothing - # to corroborate against. - if parent == path: - return False - if not parent.is_dir(): + if parent == path or not parent.is_dir(): return False if parent_dev is not None and parent_ino is not None: st = parent.stat() if (st.st_dev, st.st_ino) != (parent_dev, parent_ino): - # A different directory is visible at the parent's path than - # the one the project lived in — the volume is detached (its - # underlay showing through) or was swapped. Not a deletion. return False elif require_parent_identity: - # No recorded identity to check against — we cannot tell the - # project's real parent from an underlay directory exposed by an - # unmount. Unsure never deletes; retention still reclaims. return False - if _dir_has_any_entry(parent): - return True - # Empty parent: only evidence if that directory is a mount point, i.e. - # the volume is attached right now and simply does not hold the - # project. An empty plain directory is an unmounted mount point as - # readily as an emptied project root. - return os.path.ismount(parent) + return _dir_has_any_entry(parent) or os.path.ismount(parent) except OSError: # Probe failed (permission, I/O error) — not evidence of deletion. return False def _dir_has_any_entry(directory: Path) -> bool: - """True when ``directory`` contains at least one entry. - - Stops after the first entry rather than materializing the listing; a - project root can hold a large tree. - """ + """True when ``directory`` has at least one entry (stops at the first).""" with os.scandir(directory) as entries: for _ in entries: return True return False -def prune_checkpoints( - retention_days: int = 7, - delete_orphans: bool = True, - checkpoint_base: Optional[Path] = None, - max_total_size_mb: int = 0, - orphan_allowlist: Optional[set] = None, -) -> Dict[str, int]: - """Delete stale/orphan checkpoints and reclaim store space. +def _int_or_none(value) -> Optional[int]: + return value if isinstance(value, int) and not isinstance(value, bool) else None - A project entry is deleted when either: - * ``delete_orphans=True`` and its ``workdir`` no longer exists on disk - (the original project was deleted / moved); OR - * its ``last_touch`` is older than ``retention_days`` days. +def _rmtree_counted(child: Path, result: Dict[str, int], key: str, fail_fmt: str, label) -> None: + """rmtree ``child``, crediting bytes + ``result[key]``; failures count as errors.""" + try: + size = _dir_size_bytes(child) + shutil.rmtree(child) + result["bytes_freed"] += size + result[key] += 1 + except OSError as exc: + result["errors"] += 1 + logger.warning(fail_fmt, label, exc) - ``orphan_allowlist``, when not ``None``, restricts orphan deletion to - the given identities (v2 project ``_hash`` strings and/or pre-v2 shadow - repo paths as ``str``). This lets a caller that showed the user a - confirmation preview (built from ``store_status()``) bind the resulting - deletion to exactly what was displayed — a project that only becomes - orphaned *after* the preview (e.g. its workdir vanishes while the human - is answering the prompt) is skipped rather than swept up under the - earlier confirmation. Pass ``None`` (the default) to delete every - currently-orphaned project, e.g. for ``--force`` or unattended callers - that never show a preview. - Additionally, if ``max_total_size_mb > 0`` and the store exceeds that - after orphan/stale pruning, the oldest commit per remaining project is - dropped until the store is under the cap. +def _prune_legacy_archives(base: Path, cutoff: float, result: Dict[str, int]) -> None: + """Delete ``legacy-*`` archives whose mtime predates ``cutoff`` (skipped when retention is off).""" + if cutoff <= 0: + return + for child in _legacy_archives(base): + try: + mtime = child.stat().st_mtime + except OSError: + continue + if mtime >= cutoff: + continue + _rmtree_counted(child, result, "deleted_stale", + "Failed to delete legacy archive %s: %s", child) - Legacy-archive dirs (``legacy-*``) older than ``retention_days`` are - also deleted. - Returns a dict with counts ``{"scanned", "deleted_orphan", - "deleted_stale", "errors", "bytes_freed"}``. +def _prune_pre_v2_repos( + base: Path, cutoff: float, delete_orphans: bool, + orphan_allowlist: Optional[set], result: Dict[str, int], +) -> None: + """Sweep pre-v2 per-project shadow repos exactly as the v1 pruner did. - Never raises — maintenance must never block interactive startup. + Scanned via ``_pre_v2_shadow_repos`` so a ``store_status`` preview matches + what is deleted. The frozen layout has no recorded parent identity, so + orphan detection uses the structural checks only. """ - base = checkpoint_base or CHECKPOINT_BASE - result = { - "scanned": 0, - "deleted_orphan": 0, - "deleted_stale": 0, - "errors": 0, - "bytes_freed": 0, - } - if not base.exists(): - return result - - size_before = _dir_size_bytes(base) - - # --- Legacy pre-v2 per-project shadow repos (kept directly under base) --- - # Pre-v2 layout: ``base//HEAD`` etc. We treat these exactly as the - # v1 pruner did so behaviour is unchanged for anyone still on that layout - # or sitting on a mid-migration system. - cutoff = 0.0 - if retention_days > 0: - cutoff = time.time() - retention_days * 86400 - - for child in base.iterdir(): - if not child.is_dir(): - continue - if child.name == _STORE_DIRNAME: - continue - if child.name.startswith(_LEGACY_PREFIX): - # Legacy archive: prune by dir mtime using same retention rule. - if retention_days <= 0: - continue - try: - m = child.stat().st_mtime - except OSError: - continue - if m >= cutoff: - continue - try: - size = _dir_size_bytes(child) - shutil.rmtree(child) - result["bytes_freed"] += size - result["deleted_stale"] += 1 - except OSError as exc: - result["errors"] += 1 - logger.warning("Failed to delete legacy archive %s: %s", child, exc) - - # Pre-v2 per-project shadow repos. Scanned via the same helper - # `store_status()` uses for its orphan preview, so a confirmation prompt - # built from that preview always matches what gets deleted here. for repo in _pre_v2_shadow_repos(base): child = repo["path"] result["scanned"] += 1 @@ -1865,185 +1324,92 @@ def prune_checkpoints( and not repo["marker_unreadable"] and ( repo["workdir"] is None - # The frozen pre-v2 layout has no metadata channel to carry a - # recorded parent identity, so only the structural checks - # (parent present + populated / live mount point) apply here. - or _workdir_is_observably_gone( - repo["workdir"], require_parent_identity=False, - ) + or _workdir_is_observably_gone(repo["workdir"], require_parent_identity=False) ) and (orphan_allowlist is None or str(child) in orphan_allowlist) ): reason = "orphan" - if reason is None and retention_days > 0: - newest = 0.0 - try: - for p in child.rglob("*"): - try: - mt = p.stat().st_mtime - newest = max(newest, mt) - except OSError: - continue - except OSError: - pass - if newest > 0 and newest < cutoff: - reason = "stale" + elif cutoff > 0 and 0 < _newest_mtime(child) < cutoff: + reason = "stale" + if reason is not None: + _rmtree_counted(child, result, f"deleted_{reason}", + "Failed to prune checkpoint repo %s: %s", child.name) + + +def _prune_v2_projects( + store: Path, cutoff: float, delete_orphans: bool, + orphan_allowlist: Optional[set], result: Dict[str, int], +) -> None: + """Drop the ref, index and metadata of orphan/stale projects in the shared store.""" + for meta in _list_projects(store): + dir_hash = meta.get("_hash") or "" + workdir = meta.get("workdir") or "" + if not dir_hash: + continue + result["scanned"] += 1 + reason: Optional[str] = None + if ( + delete_orphans + and ( + not workdir + or _workdir_is_observably_gone( + workdir, + parent_dev=_int_or_none(meta.get("workdir_parent_dev")), + parent_ino=_int_or_none(meta.get("workdir_parent_ino")), + ) + ) + and (orphan_allowlist is None or dir_hash in orphan_allowlist) + ): + reason = "orphan" + elif cutoff > 0 and 0 < float(meta.get("last_touch", 0) or 0) < cutoff: + reason = "stale" if reason is None: continue - try: - size = _dir_size_bytes(child) - shutil.rmtree(child) - result["bytes_freed"] += size - if reason == "orphan": - result["deleted_orphan"] += 1 - else: - result["deleted_stale"] += 1 - except OSError as exc: - result["errors"] += 1 - logger.warning("Failed to prune checkpoint repo %s: %s", child.name, exc) + _delete_ref(store, _ref_name(dir_hash)) + _unlink_quiet(_index_path(store, dir_hash)) + _unlink_quiet(_project_meta_path(store, dir_hash)) + result[f"deleted_{reason}"] += 1 + + +def prune_checkpoints( + retention_days: int = 7, + delete_orphans: bool = True, + checkpoint_base: Optional[Path] = None, + max_total_size_mb: int = 0, + orphan_allowlist: Optional[set] = None, +) -> Dict[str, int]: + """Delete stale/orphan checkpoints and reclaim store space. Never raises. + + A project (or legacy archive) is deleted when ``delete_orphans`` and its + workdir is observably gone, OR its last touch predates ``retention_days`` + (``<= 0`` disables retention). ``orphan_allowlist`` (v2 ``_hash`` strings + and/or pre-v2 repo paths as ``str``) binds orphan deletion to exactly what a + ``store_status()`` confirmation preview showed — a project orphaned after the + preview is skipped; ``None`` deletes every current orphan (``--force``, + unattended). With ``max_total_size_mb > 0`` the oldest commit per project is + dropped until the store fits. Returns ``{"scanned", "deleted_orphan", + "deleted_stale", "errors", "bytes_freed"}``. + """ + base = checkpoint_base or CHECKPOINT_BASE + result = _empty_prune_result() + if not base.exists(): + return result + + size_before = _dir_size_bytes(base) + cutoff = time.time() - retention_days * 86400 if retention_days > 0 else 0.0 + + _prune_legacy_archives(base, cutoff, result) + _prune_pre_v2_repos(base, cutoff, delete_orphans, orphan_allowlist, result) - # --- v2 shared store: per-project ref pruning via metadata --- store = _store_path(base) - if (store / "HEAD").exists(): - for meta in _list_projects(store): - dir_hash = meta.get("_hash") or "" - workdir = meta.get("workdir") or "" - if not dir_hash: - continue - result["scanned"] += 1 - reason = None - parent_dev = meta.get("workdir_parent_dev") - parent_ino = meta.get("workdir_parent_ino") - if not isinstance(parent_dev, int) or isinstance(parent_dev, bool): - parent_dev = None - if not isinstance(parent_ino, int) or isinstance(parent_ino, bool): - parent_ino = None - if ( - delete_orphans - and ( - not workdir - or _workdir_is_observably_gone( - workdir, - parent_dev=parent_dev, - parent_ino=parent_ino, - ) - ) - and (orphan_allowlist is None or dir_hash in orphan_allowlist) - ): - reason = "orphan" - elif retention_days > 0: - last_touch = float(meta.get("last_touch", 0) or 0) - if last_touch > 0 and last_touch < cutoff: - reason = "stale" - if reason is None: - continue - ref = _ref_name(dir_hash) - _delete_ref(store, ref) - # Drop per-project index and metadata. - try: - idx = _index_path(store, dir_hash) - if idx.exists(): - idx.unlink() - except OSError: - pass - try: - mp = _project_meta_path(store, dir_hash) - if mp.exists(): - mp.unlink() - except OSError: - pass - if reason == "orphan": - result["deleted_orphan"] += 1 - else: - result["deleted_stale"] += 1 - - # GC the store to reclaim unreachable objects from dropped refs. - _run_git( - ["reflog", "expire", "--expire=now", "--all"], - store, str(base), - ) - _run_git( - ["gc", "--prune=now", "--quiet"], - store, str(base), timeout=_GIT_TIMEOUT * 3, - ) - _repair_bare_repo_dirs(store) - - # Size-cap pass across remaining projects. + if _store_has_head(store): + _prune_v2_projects(store, cutoff, delete_orphans, orphan_allowlist, result) + _gc_store(store, str(base)) if max_total_size_mb > 0: - cap_bytes = max_total_size_mb * 1024 * 1024 - for _i in range(20): - size = _dir_size_bytes(store) - if size <= cap_bytes: - break - ok, stdout, _ = _run_git( - ["for-each-ref", "--format=%(refname)", _REFS_PREFIX], - store, str(base), - allowed_returncodes={128}, - ) - refs = [r for r in stdout.splitlines() if r.strip()] if ok else [] - if not refs: - break - any_drop = False - for ref in refs: - ok_c, count_out, _ = _run_git( - ["rev-list", "--count", ref], store, str(base), - allowed_returncodes={128}, - ) - try: - count = int(count_out) if ok_c else 0 - except ValueError: - count = 0 - if count <= 1: - continue - ok_l, lo, _ = _run_git( - ["rev-list", "--reverse", ref], store, str(base), - ) - if not ok_l or not lo: - continue - commits = lo.splitlines() - keep = commits[1:] - new_parent: Optional[str] = None - fail = False - for sha in keep: - ok_t, tsha, _ = _run_git( - ["rev-parse", f"{sha}^{{tree}}"], store, str(base), - ) - if not ok_t or not tsha: - fail = True - break - ok_m, m, _ = _run_git( - ["log", "--format=%s", "-1", sha], store, str(base), - ) - msg = m if ok_m and m else "checkpoint" - args = ["commit-tree", tsha, "-m", msg, "--no-gpg-sign"] - if new_parent is not None: - args = ["commit-tree", tsha, "-p", new_parent, - "-m", msg, "--no-gpg-sign"] - ok_cm, new_sha, _ = _run_git(args, store, str(base)) - if not ok_cm or not new_sha: - fail = True - break - new_parent = new_sha - if fail or new_parent is None: - continue - _run_git(["update-ref", ref, new_parent], store, str(base)) - any_drop = True - if not any_drop: - break - _run_git( - ["reflog", "expire", "--expire=now", "--all"], - store, str(base), - ) - _run_git( - ["gc", "--prune=now", "--quiet"], - store, str(base), timeout=_GIT_TIMEOUT * 3, - ) - _repair_bare_repo_dirs(store) - - size_after = _dir_size_bytes(base) - delta = size_before - size_after - result["bytes_freed"] = max(result["bytes_freed"], delta) + _shrink_store_to_cap(store, str(base), max_total_size_mb * _MB) + _gc_store(store, str(base)) + result["bytes_freed"] = max(result["bytes_freed"], size_before - _dir_size_bytes(base)) return result @@ -2056,21 +1422,16 @@ def maybe_auto_prune_checkpoints( ) -> Dict[str, object]: """Idempotent wrapper around ``prune_checkpoints`` for startup hooks. - Writes ``CHECKPOINT_BASE/.last_prune`` on completion so subsequent - calls within ``min_interval_hours`` short-circuit. - - Returns ``{"skipped": bool, "result": prune_checkpoints-dict, - "error": optional str}``. + Writes ``CHECKPOINT_BASE/.last_prune`` on completion so calls within + ``min_interval_hours`` short-circuit. Returns ``{"skipped": bool, + "result": prune dict, "error": optional str}``. """ base = checkpoint_base or CHECKPOINT_BASE out: Dict[str, object] = {"skipped": False} try: if not base.exists(): - out["result"] = { - "scanned": 0, "deleted_orphan": 0, "deleted_stale": 0, - "errors": 0, "bytes_freed": 0, - } + out["result"] = _empty_prune_result() return out marker = base / _PRUNE_MARKER_NAME @@ -2085,11 +1446,8 @@ def maybe_auto_prune_checkpoints( pass # corrupt marker — treat as no prior run result = prune_checkpoints( - retention_days=retention_days, - delete_orphans=delete_orphans, - checkpoint_base=base, - max_total_size_mb=max_total_size_mb, - ) + retention_days=retention_days, delete_orphans=delete_orphans, + checkpoint_base=base, max_total_size_mb=max_total_size_mb) out["result"] = result try: @@ -2102,10 +1460,8 @@ def maybe_auto_prune_checkpoints( logger.info( "checkpoint auto-maintenance: pruned %d entry(ies) " "(%d orphan, %d stale), reclaimed %.1f MB", - total, - result["deleted_orphan"], - result["deleted_stale"], - result["bytes_freed"] / (1024 * 1024), + total, result["deleted_orphan"], result["deleted_stale"], + result["bytes_freed"] / _MB, ) except Exception as exc: logger.warning("checkpoint auto-maintenance failed: %s", exc) @@ -2114,34 +1470,22 @@ def maybe_auto_prune_checkpoints( return out -# --------------------------------------------------------------------------- # Public helpers for `hermes checkpoints` CLI -# --------------------------------------------------------------------------- def store_status(checkpoint_base: Optional[Path] = None) -> Dict: - """Return a summary of the shadow store. + """Summarise the shadow store. - ``{"base": path, "store_size_bytes": N, "legacy_size_bytes": N, - "total_size_bytes": N, "project_count": N, "projects": [...], - "pre_v2_projects": [...], "legacy_archives": [...]}`` - - ``pre_v2_projects`` covers shadow repos still on the pre-v2 per-project - layout (``base//HEAD``) — distinct from ``legacy_archives``, which - are already-migrated ``legacy-/`` dirs. Callers that preview an - orphan-deletion sweep must include both ``projects`` and - ``pre_v2_projects``, since ``prune_checkpoints`` deletes orphans from - both layouts. + ``{"base", "store_size_bytes", "legacy_size_bytes", "total_size_bytes", + "project_count", "projects", "pre_v2_projects", "legacy_archives"}``. + ``pre_v2_projects`` are repos still on the pre-v2 layout, distinct from the + already-migrated ``legacy_archives``; an orphan-deletion preview must include + both ``projects`` and ``pre_v2_projects`` since ``prune_checkpoints`` sweeps both. """ base = checkpoint_base or CHECKPOINT_BASE out: Dict = { - "base": str(base), - "store_size_bytes": 0, - "legacy_size_bytes": 0, - "total_size_bytes": 0, - "project_count": 0, - "projects": [], - "pre_v2_projects": [], - "legacy_archives": [], + "base": str(base), "store_size_bytes": 0, "legacy_size_bytes": 0, + "total_size_bytes": 0, "project_count": 0, + "projects": [], "pre_v2_projects": [], "legacy_archives": [], } if not base.exists(): return out @@ -2149,54 +1493,33 @@ def store_status(checkpoint_base: Optional[Path] = None) -> Dict: store = _store_path(base) if store.exists(): out["store_size_bytes"] = _dir_size_bytes(store) - if (store / "HEAD").exists(): + if _store_has_head(store): for meta in _list_projects(store): dir_hash = meta.get("_hash") or "" workdir = meta.get("workdir") or "" - ref = _ref_name(dir_hash) - ok, count_out, _ = _run_git( - ["rev-list", "--count", ref], store, str(base), - allowed_returncodes={128}, - ) - try: - commits = int(count_out) if ok else 0 - except ValueError: - commits = 0 out["projects"].append({ "hash": dir_hash, "workdir": workdir, "exists": bool(workdir) and Path(workdir).exists(), "created_at": meta.get("created_at"), "last_touch": meta.get("last_touch"), - "commits": commits, + "commits": _ref_commit_count(store, str(base), _ref_name(dir_hash)), }) out["project_count"] = len(out["projects"]) out["pre_v2_projects"] = [ - { - "path": str(r["path"]), - "workdir": r["workdir"], - "exists": r["exists"], - } + {"path": str(r["path"]), "workdir": r["workdir"], "exists": r["exists"]} for r in _pre_v2_shadow_repos(base) ] - for child in base.iterdir(): - if child.is_dir() and child.name.startswith(_LEGACY_PREFIX): - try: - size = _dir_size_bytes(child) - except OSError: - size = 0 - out["legacy_size_bytes"] += size - try: - mt = child.stat().st_mtime - except OSError: - mt = 0 - out["legacy_archives"].append({ - "name": child.name, - "size_bytes": size, - "mtime": mt, - }) + for child in _legacy_archives(base): + size = _dir_size_bytes(child) + try: + mt = child.stat().st_mtime + except OSError: + mt = 0 + out["legacy_size_bytes"] += size + out["legacy_archives"].append({"name": child.name, "size_bytes": size, "mtime": mt}) out["total_size_bytes"] = _dir_size_bytes(base) return out @@ -2230,9 +1553,7 @@ def clear_legacy(checkpoint_base: Optional[Path] = None) -> Dict[str, int]: out = {"bytes_freed": 0, "deleted": 0} if not base.exists(): return out - for child in list(base.iterdir()): - if not child.is_dir() or not child.name.startswith(_LEGACY_PREFIX): - continue + for child in _legacy_archives(base): try: size = _dir_size_bytes(child) shutil.rmtree(child) diff --git a/tools/code_execution_tool.py b/tools/code_execution_tool.py index 470f956732..ce2a708ba2 100644 --- a/tools/code_execution_tool.py +++ b/tools/code_execution_tool.py @@ -3,29 +3,22 @@ Code Execution Tool -- Programmatic Tool Calling (PTC) Lets the LLM write a Python script that calls Hermes tools via RPC, -collapsing multi-step tool chains into a single inference turn. +collapsing multi-step tool chains into a single inference turn. Only the +script's stdout is returned to the LLM; intermediate tool results never +enter the context window. -Architecture (two transports): +Two transports: + * Local backend: a persistent per-conversation session kernel + (tools/code_kernel.py) talks to the parent's RPC thread over a Unix + domain socket (loopback TCP on Windows, where AF_UNIX is unreliable). + * Remote backends (Docker/SSH/Modal/Daytona/...): a remote session kernel + (tools/code_kernel_remote.py), falling open to a per-call script ship; + tool calls are request files that a polling thread on the parent reads + via env.execute(), dispatches, and answers with response files. - **Local backend (UDS):** - 1. Parent generates a `hermes_tools.py` stub module with UDS RPC functions - 2. Parent opens a Unix domain socket and starts an RPC listener thread - 3. Parent spawns a child process that runs the LLM's script - 4. Tool calls travel over the UDS back to the parent for dispatch - - **Remote backends (file-based RPC):** - 1. Parent generates `hermes_tools.py` with file-based RPC stubs - 2. Parent ships both files to the remote environment - 3. Script runs inside the terminal backend (Docker/SSH/Modal/Daytona/etc.) - 4. Tool calls are written as request files; a polling thread on the parent - reads them via env.execute(), dispatches, and writes response files - 5. The script polls for response files and continues - -In both cases, only the script's stdout is returned to the LLM; intermediate -tool results never enter the context window. - -Platform: Linux / macOS only (Unix domain sockets for local). Disabled on Windows. -Remote execution additionally requires Python 3 in the terminal backend. +Sibling modules: tools/code_execution_env.py (env scrubbing, interpreter/cwd +resolution) and tools/code_execution_rpc.py (RPC servers). Remote execution +requires Python 3 in the terminal backend. """ import base64 @@ -36,30 +29,52 @@ import platform import re import secrets import shlex -import socket import subprocess -import sys import tempfile import threading import time import uuid - -_IS_WINDOWS = platform.system() == "Windows" from typing import Any, Dict, List, Optional, Tuple from tools.thread_context import propagate_context_to_thread -from agent.thread_scoped_output import thread_scoped_silence +from tools.registry import registry, tool_error -# Availability gate. On Windows we fall back to loopback TCP for the -# sandbox RPC transport (AF_UNIX is unreliable on Windows Python) — see -# ``_use_tcp_rpc`` in ``_execute_local`` below. That makes execute_code -# available on every platform Hermes itself runs on. +# Env/interpreter resolution and RPC servers live in sibling modules; re-exported +# here so `from tools.code_execution_tool import X` / patch() targets keep working. +from tools.code_execution_env import ( # noqa: F401 + _SAFE_ENV_PREFIXES, + _SECRET_SUBSTRINGS, + _HERMES_CHILD_ALLOWED, + _WINDOWS_ESSENTIAL_ENV_VARS, + _scrub_child_env, + _build_child_env, + _PROBE_CACHE_MAX, + _usable_python_cache, + _python_prefix_cache, + _external_env_logged, + _cache_probe_result, + _is_usable_python, + _probe_python, + _python_environment_prefix, + _uses_hermes_python_environment, + _resolve_child_python, + _resolve_child_cwd, +) +from tools.code_execution_rpc import ( # noqa: F401 + _TERMINAL_BLOCKED_PARAMS, + _rpc_server_loop, + _rpc_poll_loop, +) + +_IS_WINDOWS = platform.system() == "Windows" logger = logging.getLogger(__name__) +# Loopback TCP replaces AF_UNIX on Windows, so execute_code is available on +# every platform Hermes itself runs on. SANDBOX_AVAILABLE = True -# The 7 tools allowed inside the sandbox. The intersection of this list -# and the session's enabled tools determines which stubs are generated. +# Tools allowed inside the sandbox; the intersection with the session's +# enabled tools determines which stubs are generated. SANDBOX_ALLOWED_TOOLS = frozenset([ "web_search", "web_extract", @@ -187,174 +202,9 @@ def _spill_full_stdout(stdout_text: str) -> Optional[str]: logger.debug("Failed to spill execute_code stdout: %s", exc) return None -# Environment variable scrubbing rules (shared between the local + remote -# backends). Secret-substring block is applied first; anything left must -# match a safe prefix, the operational HERMES_ allowlist, or (on Windows) an -# OS-essential name. Delegate-task child context is also an exact-name -# operational marker: without it, a sandbox script that spawns/imports Hermes -# code can lose the DB-layer Kanban mutation guard while still inheriting -# HERMES_HOME. -# -# NB: the broad "HERMES_" prefix was deliberately removed (#27303) — it leaked -# HERMES_*-named config that lacks a secret substring (e.g. HERMES_BASE_URL, -# HERMES_KANBAN_DB, HERMES_*_WEBHOOK). The child only needs the few -# location/profile vars in _HERMES_CHILD_ALLOWED below; HERMES_RPC_SOCKET / -# HERMES_RPC_DIR / TZ / HOME are injected explicitly after scrubbing. -_SAFE_ENV_PREFIXES = ("PATH", "HOME", "USER", "LANG", "LC_", "TERM", - "TMPDIR", "TMP", "TEMP", "SHELL", "LOGNAME", - "XDG_", "PYTHONPATH", "VIRTUAL_ENV", "CONDA") -_SECRET_SUBSTRINGS = ("KEY", "TOKEN", "SECRET", "PASSWORD", "CREDENTIAL", - "PASSWD", "AUTH", "DSN", "WEBHOOK", - # Abbreviations that appear in real-world credential - # variable names but were previously undetected: - # CREDS (CREDENTIALS abbreviated), BEARER - # (Authorization: Bearer tokens), APIKEY (written - # without an underscore). "PASS" is intentionally NOT - # added — it false-positives on legitimate non-secret - # vars (BYPASS_CACHE, COMPASS_DIR, PASSENGER_HOST) while - # PASSWORD/PASSWD already cover the credential cases. - "CREDS", "BEARER", "APIKEY") - -# Operational HERMES_* vars the child legitimately needs by exact name — these -# are non-secret runtime-location flags (the same set hermes_cli treats as the -# runtime location) that repo-root modules a sandbox script imports may read at -# import time. None match _SECRET_SUBSTRINGS. -_HERMES_CHILD_ALLOWED = frozenset({ - "HERMES_HOME", - "HERMES_PROFILE", - "HERMES_CONFIG", - "HERMES_ENV", - "HERMES_DELEGATED_CHILD_CONTEXT", -}) - -# Windows-only: a handful of variables are required by the OS/CRT itself. -# Without them, even stdlib calls like ``socket.socket()`` fail with -# WinError 10106 (Winsock can't locate mswsock.dll) and ``subprocess`` -# can't resolve cmd.exe. These are well-known OS paths, not secrets, so -# we allow them through by exact name. The _SECRET_SUBSTRINGS block -# still runs as a safety net (none of these names match those substrings). -_WINDOWS_ESSENTIAL_ENV_VARS = frozenset({ - "SYSTEMROOT", # %SYSTEMROOT%\System32 — Winsock needs this - "SYSTEMDRIVE", # C: (or wherever Windows lives) - "WINDIR", # usually same as SYSTEMROOT - "COMSPEC", # cmd.exe path — subprocess shell=True needs it - "PATHEXT", # .COM;.EXE;.BAT;... — shell lookup - "OS", # "Windows_NT" — some tools gate on this - "PROCESSOR_ARCHITECTURE", - "NUMBER_OF_PROCESSORS", - "PUBLIC", # C:\Users\Public - "ALLUSERSPROFILE", # C:\ProgramData — some stdlib paths use it - "PROGRAMDATA", # C:\ProgramData - "PROGRAMFILES", - "PROGRAMFILES(X86)", - "PROGRAMW6432", - "APPDATA", # %USERPROFILE%\AppData\Roaming — Python uses it - "LOCALAPPDATA", # %USERPROFILE%\AppData\Local - "USERPROFILE", # C:\Users\ — Python's expanduser uses it - "USERDOMAIN", - "USERNAME", - "HOMEDRIVE", # C: - "HOMEPATH", # \Users\ - "COMPUTERNAME", -}) - - -def _scrub_child_env(source_env, is_passthrough=None, is_windows=None): - """Produce the scrubbed child-process env for execute_code. - - Rules (order matters): - 1. Passthrough vars (skill- or config-declared) pass through the active - profile secret scope; an absent scoped value is omitted and an - unscoped multiplex read fails closed. - 2. Secret-substring names (KEY/TOKEN/DSN/WEBHOOK/etc.) are blocked. - 3. Names matching a safe prefix pass. - 4. Operational HERMES_* vars (_HERMES_CHILD_ALLOWED) pass by exact name. - 5. On Windows, a small OS-essential allowlist passes by exact name - — without these the child can't even create a socket or spawn a - subprocess. - - Extracted into a helper so tests can exercise the logic without - spawning a subprocess. - """ - resolve_passthrough_value = None - if is_passthrough is None: - try: - from tools.env_passthrough import ( - is_env_passthrough as _ep, - resolve_passthrough_value, - ) - except Exception: - _ep = lambda _: False # noqa: E731 - resolve_passthrough_value = lambda _name, _fallback: None # noqa: E731 - is_passthrough = _ep - else: - try: - from tools.env_passthrough import resolve_passthrough_value - except Exception: - resolve_passthrough_value = lambda _name, _fallback: None # noqa: E731 - if is_windows is None: - is_windows = _IS_WINDOWS - - scrubbed = {} - # Non-secret HERMES_* vars dropped by the tightened allowlist (#27303). The - # broad "HERMES_" prefix used to pass these through; now only the - # operational set does. The drop is intentional (those vars can carry - # config like HERMES_KANBAN_DB / HERMES_BASE_URL), but a sandbox script - # that imports a repo module reading one at import time would otherwise see - # it silently unset. Surface the drop once so the behavior change is - # diagnosable and points at the env_passthrough opt-in escape hatch. - _dropped_hermes = [] - for k, v in source_env.items(): - if is_passthrough(k): - resolved = resolve_passthrough_value(k, v) - if resolved is not None: - scrubbed[k] = resolved - continue - if any(s in k.upper() for s in _SECRET_SUBSTRINGS): - continue - if any(k.startswith(p) for p in _SAFE_ENV_PREFIXES): - scrubbed[k] = v - continue - if k in _HERMES_CHILD_ALLOWED: - scrubbed[k] = v - continue - if is_windows and k.upper() in _WINDOWS_ESSENTIAL_ENV_VARS: - scrubbed[k] = v - continue - if k.startswith("HERMES_"): - # Non-secret (secrets were already dropped above) and not in any - # allowlist — a deliberately-dropped HERMES_* var. - _dropped_hermes.append(k) - if _dropped_hermes: - logger.debug( - "execute_code: dropped %d non-allowlisted HERMES_* var(s) from the " - "sandbox child env (%s). This is intentional hardening (#27303); if " - "a sandbox script legitimately needs one, declare it via " - "env_passthrough in the skill/config so it passes by explicit opt-in.", - len(_dropped_hermes), - ", ".join(sorted(_dropped_hermes)), - ) - - # delegate_task children are marked with a ContextVar, not os.environ, while - # the execute_code sandbox crosses a process boundary. Bridge that context - # into the child env and strip dispatcher-owned Kanban variables after the - # normal secret/passthrough scrub so an explicit passthrough cannot re-grant - # a delegated child the parent's board mutation capability. - try: - from agent.delegation_context import ( - is_delegated_child_process_context, - scrub_kanban_env, - ) - - if is_delegated_child_process_context(): - scrubbed = scrub_kanban_env(scrubbed) - except Exception: - pass - return scrubbed - def check_sandbox_requirements() -> bool: - """Code execution sandbox requires a POSIX OS for Unix domain sockets.""" + """check_fn: available unless the vercel_sandbox backend fails its own checks.""" if not SANDBOX_AVAILABLE: return False @@ -379,47 +229,40 @@ def check_sandbox_requirements() -> bool: # hermes_tools.py code generator # --------------------------------------------------------------------------- -# Per-tool stub templates: (function_name, signature, docstring, args_dict_expr) -# The args_dict_expr builds the JSON payload sent over the RPC socket. +# Per-tool stub templates: (signature, docstring, args_dict_expr); the +# args_dict_expr builds the JSON payload sent over the RPC channel. _TOOL_STUBS = { "web_search": ( - "web_search", "query: str, limit: int = 5", '"""Search the web. Returns dict with data.web list of {url, title, description}."""', '{"query": query, "limit": limit}', ), "web_extract": ( - "web_extract", "urls: list, char_limit: int = None", '"""Extract content from URLs (no LLM summarization). Returns dict with results list of {url, title, content, error}. Pages over char_limit (default 15000) are head+tail truncated with the full text stored on disk; the content footer gives the path. content is markdown."""', '{"urls": urls, "char_limit": char_limit}', ), "read_file": ( - "read_file", "path: str, offset: int = 1, limit: int = 2000", '"""Read a file (1-indexed lines). Returns dict with "content" and "total_lines"."""', '{"path": path, "offset": offset, "limit": limit}', ), "write_file": ( - "write_file", "path: str, content: str, cross_profile: bool = False", '"""Write content to a file (always overwrites). Returns dict with status."""', '{"path": path, "content": content, "cross_profile": cross_profile}', ), "search_files": ( - "search_files", 'pattern: str, target: str = "content", path: str = ".", file_glob: str = None, limit: int = 50, offset: int = 0, output_mode: str = "content", context: int = 0', '"""Search file contents (target="content") or find files by name (target="files"). Returns dict with "matches"."""', '{"pattern": pattern, "target": target, "path": path, "file_glob": file_glob, "limit": limit, "offset": offset, "output_mode": output_mode, "context": context}', ), "patch": ( - "patch", 'path: str = None, old_string: str = None, new_string: str = None, replace_all: bool = False, mode: str = "replace", patch: str = None, cross_profile: bool = False', '"""Targeted find-and-replace (mode="replace") or V4A multi-file patches (mode="patch"). Returns dict with status."""', '{"path": path, "old_string": old_string, "new_string": new_string, "replace_all": replace_all, "mode": mode, "patch": patch, "cross_profile": cross_profile}', ), "terminal": ( - "terminal", "command: str, timeout: int = None, workdir: str = None", '"""Run a shell command (foreground only). Returns dict with "output" and "exit_code"."""', '{"command": command, "timeout": timeout, "workdir": workdir}', @@ -484,37 +327,19 @@ def _sandbox_failure_hint(stderr_text: str, enabled_tools=None) -> Optional[str] def generate_hermes_tools_module(enabled_tools: List[str], transport: str = "uds") -> str: - """ - Build the source code for the hermes_tools.py stub module. - - Only tools in both SANDBOX_ALLOWED_TOOLS and enabled_tools get stubs. - - Args: - enabled_tools: Tool names enabled in the current session. - transport: ``"uds"`` for Unix domain socket (local backend) or - ``"file"`` for file-based RPC (remote backends). - """ - tools_to_generate = sorted(SANDBOX_ALLOWED_TOOLS & set(enabled_tools)) - - stub_functions = [] - export_names = [] - for tool_name in tools_to_generate: - if tool_name not in _TOOL_STUBS: - continue - func_name, sig, doc, args_expr = _TOOL_STUBS[tool_name] - stub_functions.append( - f"def {func_name}({sig}):\n" + """Source of the hermes_tools.py stub module for tools in both + SANDBOX_ALLOWED_TOOLS and *enabled_tools*. ``transport``: ``"uds"`` (local + socket client) or ``"file"`` (file-based RPC client for remote backends).""" + header = _FILE_TRANSPORT_HEADER if transport == "file" else _UDS_TRANSPORT_HEADER + stubs = [] + for name in sorted(SANDBOX_ALLOWED_TOOLS & set(enabled_tools)): + sig, doc, args_expr = _TOOL_STUBS[name] + stubs.append( + f"def {name}({sig}):\n" f" {doc}\n" - f" return _call({func_name!r}, {args_expr})\n" + f" return _call({name!r}, {args_expr})\n" ) - export_names.append(func_name) - - if transport == "file": - header = _FILE_TRANSPORT_HEADER - else: - header = _UDS_TRANSPORT_HEADER - - return header + "\n".join(stub_functions) + return header + "\n".join(stubs) # ---- Shared helpers section (embedded in both transport headers) ---------- @@ -712,163 +537,19 @@ def _call(tool_name, args): ''' -# --------------------------------------------------------------------------- -# RPC server (runs in a thread inside the parent process) -# --------------------------------------------------------------------------- - -# Terminal parameters that must not be used from ephemeral sandbox scripts -_TERMINAL_BLOCKED_PARAMS = {"background", "pty", "notify", "notify_on_complete", "watch_patterns"} - - -def _rpc_server_loop( - server_sock: socket.socket, - task_id: str, - tool_call_log: list, - tool_call_counter: list, # mutable [int] so the thread can increment - max_tool_calls: int, - allowed_tools: frozenset, - stop_event: threading.Event, - rpc_token: str, - dispatch=None, -): - """ - Accept one client connection and dispatch tool-call requests until - the client disconnects or the call limit is reached. - - ``dispatch`` overrides how an allowed, budgeted call is executed: - per-call sandboxes use the default (this thread already carries the - cell's context via propagate_context_to_thread), while session kernels - pass a dispatcher that rebinds each call to the CURRENT cell's - authority — the serving thread outlives many cells there and must not - freeze the first cell's context. - """ - from model_tools import handle_function_call - - if dispatch is None: - def dispatch(tool_name, tool_args): - return handle_function_call(tool_name, tool_args, task_id=task_id) - - conn = None - try: - server_sock.settimeout(0.05) - while not stop_event.is_set(): - try: - conn, _ = server_sock.accept() - break - except socket.timeout: - continue - if conn is None: - return - conn.settimeout(300) - - buf = b"" - while True: - try: - chunk = conn.recv(65536) - except socket.timeout: - break - if not chunk: - break - buf += chunk - - # Process all complete newline-delimited messages in the buffer - while b"\n" in buf: - line, buf = buf.split(b"\n", 1) - line = line.strip() - if not line: - continue - - call_start = time.monotonic() - try: - request = json.loads(line.decode()) - except (json.JSONDecodeError, UnicodeDecodeError) as exc: - resp = tool_error(f"Invalid RPC request: {exc}") - conn.sendall((resp + "\n").encode()) - continue - - if not rpc_token or not secrets.compare_digest( - # Compare as bytes: compare_digest raises TypeError on a - # str with non-ASCII characters, and the token comes from - # sandbox-script-supplied JSON. - str(request.get("token") or "").encode(), rpc_token.encode() - ): - resp = tool_error("Unauthorized RPC request") - conn.sendall((resp + "\n").encode()) - continue - - tool_name = request.get("tool", "") - tool_args = request.get("args", {}) - - # Enforce the allow-list - if tool_name not in allowed_tools: - available = ", ".join(sorted(allowed_tools)) - resp = tool_error( - f"Tool '{tool_name}' is not available in execute_code. " - f"Available: {available}" - ) - conn.sendall((resp + "\n").encode()) - continue - - # Enforce tool call limit - if tool_call_counter[0] >= max_tool_calls: - resp = tool_error( - f"Tool call limit reached ({max_tool_calls}). " - "No more tool calls allowed in this execution." - ) - conn.sendall((resp + "\n").encode()) - continue - - # Strip forbidden terminal parameters - if tool_name == "terminal" and isinstance(tool_args, dict): - for param in _TERMINAL_BLOCKED_PARAMS: - tool_args.pop(param, None) - - # Dispatch through the standard tool handler. - # Suppress stdout/stderr from internal tool handlers so - # their status prints don't leak into the CLI spinner. - try: - with thread_scoped_silence(): - result = dispatch(tool_name, tool_args) - except Exception as exc: - logger.error("Tool call failed in sandbox: %s", exc, exc_info=True) - result = tool_error(str(exc)) - - tool_call_counter[0] += 1 - call_duration = time.monotonic() - call_start - - # Log for observability - args_preview = str(tool_args)[:80] - tool_call_log.append({ - "tool": tool_name, - "args_preview": args_preview, - "duration": round(call_duration, 2), - }) - - conn.sendall((result + "\n").encode()) - - except socket.timeout: - logger.debug("RPC listener socket timeout") - except OSError as e: - logger.debug("RPC listener socket error: %s", e, exc_info=True) - finally: - if conn: - try: - conn.close() - except OSError as e: - logger.debug("RPC conn close error: %s", e) - - # --------------------------------------------------------------------------- # Remote execution support (file-based RPC via terminal backend) # --------------------------------------------------------------------------- -def _get_or_create_env(task_id: str): - """Get or create the terminal environment for *task_id*. +# Backends whose environment takes an image; the config/override key is +# f"{env_type}_image". +_IMAGE_BACKENDS = frozenset({"docker", "singularity", "modal", "daytona"}) - Reuses the same environment (container/sandbox/SSH session) that the - terminal and file tools use, creating one if it doesn't exist yet. - Returns ``(env, env_type)`` tuple. - """ + +def _get_or_create_env(task_id: str): + """Return ``(env, env_type)`` — the same environment the terminal and file + tools use for *task_id*, created on first use (same double-checked + per-task lock pattern as file_tools._get_file_ops).""" from tools.terminal_tool import ( _active_environments, _env_lock, _create_environment, _get_env_config, _last_activity, _start_cleanup_thread, @@ -878,13 +559,11 @@ def _get_or_create_env(task_id: str): effective_task_id = _resolve_container_task_id(task_id) - # Fast path: environment already exists with _env_lock: if effective_task_id in _active_environments: _last_activity[effective_task_id] = time.time() return _active_environments[effective_task_id], _get_env_config()["env_type"] - # Slow path: create environment (same pattern as file_tools._get_file_ops) with _creation_locks_lock: if effective_task_id not in _creation_locks: _creation_locks[effective_task_id] = threading.Lock() @@ -900,16 +579,10 @@ def _get_or_create_env(task_id: str): env_type = config["env_type"] overrides = _task_env_overrides.get(effective_task_id, {}) - if env_type == "docker": - image = overrides.get("docker_image") or config["docker_image"] - elif env_type == "singularity": - image = overrides.get("singularity_image") or config["singularity_image"] - elif env_type == "modal": - image = overrides.get("modal_image") or config["modal_image"] - elif env_type == "daytona": - image = overrides.get("daytona_image") or config["daytona_image"] - else: - image = "" + image = "" + if env_type in _IMAGE_BACKENDS: + image_key = f"{env_type}_image" + image = overrides.get(image_key) or config[image_key] cwd = overrides.get("cwd") or config["cwd"] @@ -969,13 +642,9 @@ def _get_or_create_env(task_id: str): def _ship_file_to_remote(env, remote_path: str, content: str) -> None: - """Write *content* to *remote_path* on the remote environment. - - Uses ``echo … | base64 -d`` rather than stdin piping because some - backends (Modal) don't reliably deliver stdin_data to chained - commands. Base64 output is shell-safe ([A-Za-z0-9+/=]) so single - quotes are fine. - """ + """Write *content* to *remote_path* via ``echo … | base64 -d`` — some + backends (Modal) don't reliably deliver stdin_data to chained commands, and + base64 is shell-safe inside single quotes.""" encoded = base64.b64encode(content.encode("utf-8")).decode("ascii") quoted_remote_path = shlex.quote(remote_path) env.execute( @@ -1001,148 +670,6 @@ def _env_temp_dir(env: Any) -> str: return "/tmp" -def _rpc_poll_loop( - env, - rpc_dir: str, - task_id: str, - tool_call_log: list, - tool_call_counter: list, - max_tool_calls: int, - allowed_tools: frozenset, - stop_event: threading.Event, - rpc_token: str, -): - """Poll the remote filesystem for tool call requests and dispatch them. - - Runs in a background thread. Each ``env.execute()`` spawns an - independent process, so these calls run safely concurrent with the - script-execution thread. - """ - from model_tools import handle_function_call - - poll_interval = 0.1 # 100 ms - - quoted_rpc_dir = shlex.quote(rpc_dir) - while not stop_event.is_set(): - try: - # List pending request files (skip .tmp partials) - ls_result = env.execute( - f"ls -1 {quoted_rpc_dir}/req_* 2>/dev/null || true", - cwd="/", - timeout=10, - ) - output = ls_result.get("output", "").strip() - if not output: - stop_event.wait(poll_interval) - continue - - req_files = sorted([ - f.strip() for f in output.split("\n") - if f.strip() - and not f.strip().endswith(".tmp") - and "/req_" in f.strip() - ]) - - for req_file in req_files: - if stop_event.is_set(): - break - - call_start = time.monotonic() - - quoted_req_file = shlex.quote(req_file) - # Read request - read_result = env.execute( - f"cat {quoted_req_file}", - cwd="/", - timeout=10, - ) - try: - request = json.loads(read_result.get("output", "")) - except (json.JSONDecodeError, ValueError): - logger.debug("Malformed RPC request in %s", req_file) - # Remove bad request to avoid infinite retry - env.execute(f"rm -f {quoted_req_file}", cwd="/", timeout=5) - continue - - if not rpc_token or not secrets.compare_digest( - # Compare as bytes: compare_digest raises TypeError on a - # str with non-ASCII characters, and the token comes from - # sandbox-script-supplied JSON. - str(request.get("token") or "").encode(), rpc_token.encode() - ): - logger.debug("Unauthorized RPC request in %s", req_file) - env.execute(f"rm -f {quoted_req_file}", cwd="/", timeout=5) - continue - - tool_name = request.get("tool", "") - tool_args = request.get("args", {}) - seq = request.get("seq", 0) - seq_str = f"{seq:06d}" - res_file = f"{rpc_dir}/res_{seq_str}" - quoted_res_file = shlex.quote(res_file) - - # Enforce allow-list - if tool_name not in allowed_tools: - available = ", ".join(sorted(allowed_tools)) - tool_result = tool_error( - f"Tool '{tool_name}' is not available in execute_code. " - f"Available: {available}" - ) - # Enforce tool call limit - elif tool_call_counter[0] >= max_tool_calls: - tool_result = tool_error( - f"Tool call limit reached ({max_tool_calls}). " - "No more tool calls allowed in this execution." - ) - else: - # Strip forbidden terminal parameters - if tool_name == "terminal" and isinstance(tool_args, dict): - for param in _TERMINAL_BLOCKED_PARAMS: - tool_args.pop(param, None) - - # Dispatch through the standard tool handler - try: - with thread_scoped_silence(): - tool_result = handle_function_call( - tool_name, tool_args, task_id=task_id - ) - except Exception as exc: - logger.error("Tool call failed in remote sandbox: %s", - exc, exc_info=True) - tool_result = tool_error(str(exc)) - - tool_call_counter[0] += 1 - call_duration = time.monotonic() - call_start - tool_call_log.append({ - "tool": tool_name, - "args_preview": str(tool_args)[:80], - "duration": round(call_duration, 2), - }) - - # Write response atomically (tmp + rename). - # Use echo piping (not stdin_data) because Modal doesn't - # reliably deliver stdin to chained commands. - encoded_result = base64.b64encode( - tool_result.encode("utf-8") - ).decode("ascii") - env.execute( - f"echo '{encoded_result}' | base64 -d > {quoted_res_file}.tmp" - f" && mv {quoted_res_file}.tmp {quoted_res_file}", - cwd="/", - timeout=60, - ) - - # Remove the request file - env.execute(f"rm -f {quoted_req_file}", cwd="/", timeout=5) - - except Exception as e: - if not stop_event.is_set(): - logger.debug("RPC poll error: %s", e, exc_info=True) - - if not stop_event.is_set(): - stop_event.wait(poll_interval) - - def _format_interrupted_output(stdout_text: str) -> str: """Append an interruption marker without guessing who caused it.""" from tools.interrupt import get_interrupt_reason @@ -1156,38 +683,62 @@ def _format_interrupted_output(stdout_text: str) -> str: return f"{stdout_text}\n{marker}" if stdout_text else marker -def _finish_remote_kernel_result(kernel_result: Dict[str, Any], *, - timeout: int, exec_start: float) -> str: - """Post-process a remote-kernel cell result into the tool's JSON reply. +def _clean_output(stdout_text: str) -> Tuple[str, Dict[str, Any]]: + """Shared output pipeline: byte-cap (with spill), ANSI strip, secret redaction. - Same output pipeline as the per-call paths: truncation, ANSI strip, - secret redaction; timeout messaging mirrors the local kernel contract - (kernel killed, state lost, next call fresh). + code_file=True: execution output often echoes source/config — skip the + ENV/JSON/f-string-template false positives while still masking real + credentials. """ from tools.ansi_strip import strip_ansi from agent.redact import redact_sensitive_text + stdout_text, metadata = _truncate_stdout_text(stdout_text) + return redact_sensitive_text(strip_ansi(stdout_text), code_file=True), metadata + + +def _with_timeout_notice(stdout_text: str, timeout_msg: str) -> str: + """Put the timeout message in the output too — an empty result makes models + answer as if nothing happened, and the gateway drops empty replies.""" + return stdout_text + f"\n\n⏰ {timeout_msg}" if stdout_text else f"⏰ {timeout_msg}" + + +def _error_result(error: str, *, tool_calls_made: int = 0, duration: float = 0) -> str: + return json.dumps({ + "status": "error", + "error": error, + "tool_calls_made": tool_calls_made, + "duration_seconds": duration, + }, ensure_ascii=False) + + +_REMOTE_EXIT_STATUS = {124: "timeout", 130: "interrupted"} + + +def _finish_remote_kernel_result(kernel_result: Dict[str, Any], *, + timeout: int, exec_start: float) -> str: + """Post-process a remote-kernel cell result into the tool's JSON reply. + + Timeout messaging mirrors the local kernel contract (kernel killed, state + lost, next call fresh). + """ stdout_text = kernel_result.get("stdout", "") or "" stderr_text = kernel_result.get("stderr", "") or "" traceback_text = kernel_result.get("traceback", "") or "" if stderr_text or traceback_text: - # Same joining shape as the local kernel path (code_kernel result - # assembly): stderr and traceback ride in the output under one - # marker so the model always sees the failure inline. + # Same joining shape as the local kernel: stderr and traceback ride in + # the output under one marker so the model sees the failure inline. stdout_text = ( stdout_text + "\n--- stderr ---\n" + stderr_text + traceback_text ) - stdout_text, stdout_metadata = _truncate_stdout_text(stdout_text) - stdout_text = strip_ansi(stdout_text) - stdout_text = redact_sensitive_text(stdout_text, code_file=True) + stdout_text, stdout_metadata = _clean_output(stdout_text) - duration = round(time.monotonic() - exec_start, 2) result: Dict[str, Any] = { "status": kernel_result.get("status", "error"), "output": stdout_text, "tool_calls_made": kernel_result.get("tool_calls_made", 0), - "duration_seconds": duration, + "duration_seconds": round(time.monotonic() - exec_start, 2), "kernel": kernel_result.get("kernel", {"remote": True}), } result.update(stdout_metadata) @@ -1198,16 +749,19 @@ def _finish_remote_kernel_result(kernel_result: Dict[str, Any], *, "killed and its state was lost. The next call starts fresh." ) result["error"] = timeout_msg - result["output"] = ( - (stdout_text + f"\n\n⏰ {timeout_msg}") if stdout_text - else f"⏰ {timeout_msg}" - ) + result["output"] = _with_timeout_notice(stdout_text, timeout_msg) elif result["status"] == "error" and kernel_result.get("error"): result["error"] = kernel_result["error"] return json.dumps(result, ensure_ascii=False) +def _sandbox_tools_for(enabled_tools: Optional[List[str]]) -> frozenset: + """Tools the sandbox may call: the session's enabled set ∩ SANDBOX_ALLOWED_TOOLS, + or every sandbox tool when the intersection is empty.""" + return frozenset(SANDBOX_ALLOWED_TOOLS & set(enabled_tools or ())) or SANDBOX_ALLOWED_TOOLS + + def _execute_remote( code: str, task_id: Optional[str], @@ -1218,26 +772,19 @@ def _execute_remote( Preferred path: the owner's persistent remote session kernel (tools/code_kernel_remote.py — detached runner + file cell protocol). - Fallback path: the original per-call script ship (kept both as the - fail-open route when a kernel cannot be spawned and as the only route - for hosts that cannot sustain a background process). + Fallback: the per-call script ship — the fail-open route when a kernel + cannot be spawned, and the only route for hosts that cannot sustain a + background process. """ - _cfg = _load_config() timeout = _cfg.get("timeout", DEFAULT_TIMEOUT) max_tool_calls = _cfg.get("max_tool_calls", DEFAULT_MAX_TOOL_CALLS) - - session_tools = set(enabled_tools) if enabled_tools else set() - sandbox_tools = frozenset(SANDBOX_ALLOWED_TOOLS & session_tools) - if not sandbox_tools: - sandbox_tools = SANDBOX_ALLOWED_TOOLS + sandbox_tools = _sandbox_tools_for(enabled_tools) effective_task_id = task_id or "default" env, env_type = _get_or_create_env(effective_task_id) - sandbox_id = uuid.uuid4().hex[:12] - temp_dir = _env_temp_dir(env) - sandbox_dir = f"{temp_dir}/hermes_exec_{sandbox_id}" + sandbox_dir = f"{_env_temp_dir(env)}/hermes_exec_{uuid.uuid4().hex[:12]}" quoted_sandbox_dir = shlex.quote(sandbox_dir) quoted_rpc_dir = shlex.quote(f"{sandbox_dir}/rpc") @@ -1248,7 +795,6 @@ def _execute_remote( rpc_thread = None try: - # Verify Python is available on the remote py_check = env.execute( "command -v python3 >/dev/null 2>&1 && echo OK", cwd="/", timeout=15, @@ -1265,10 +811,8 @@ def _execute_remote( "duration_seconds": 0, }) - # --- Session-kernel path (hermes-agent#96873) ------------------- - # Same always-on model as local: one persistent kernel per owner, - # rebuilt on the run-to-completion transport (detached runner + - # file cell protocol). Spawn failure falls OPEN to the per-call + # Session-kernel path: one persistent kernel per owner on the + # run-to-completion transport. Spawn failure falls OPEN to the per-call # path below so a degraded remote host never blocks execution. try: from tools.code_kernel_remote import execute_in_remote_kernel @@ -1300,23 +844,16 @@ def _execute_remote( env_type, ) - # Create sandbox directory on remote - env.execute( - f"mkdir -p {quoted_rpc_dir}", cwd="/", timeout=10, - ) - + env.execute(f"mkdir -p {quoted_rpc_dir}", cwd="/", timeout=10) rpc_token = secrets.token_urlsafe(32) - - # Generate and ship files - tools_src = generate_hermes_tools_module( - list(sandbox_tools), transport="file", + _ship_file_to_remote( + env, f"{sandbox_dir}/hermes_tools.py", + generate_hermes_tools_module(list(sandbox_tools), transport="file"), ) - _ship_file_to_remote(env, f"{sandbox_dir}/hermes_tools.py", tools_src) _ship_file_to_remote(env, f"{sandbox_dir}/script.py", code) # Wrapped so the thread inherits the turn's approval context + callbacks - # (see tools.thread_context) — else sandbox RPC tool calls lose approval - # routing (#33057). + # (tools.thread_context) — else sandbox RPC tool calls lose approval routing. rpc_thread = threading.Thread( target=propagate_context_to_thread(_rpc_poll_loop), args=( @@ -1328,7 +865,6 @@ def _execute_remote( ) rpc_thread.start() - # Build environment variable prefix for the script env_prefix = ( f"HERMES_RPC_DIR={shlex.quote(f'{sandbox_dir}/rpc')} " f"HERMES_RPC_TOKEN={shlex.quote(rpc_token)} " @@ -1338,7 +874,6 @@ def _execute_remote( if tz: env_prefix += f" TZ={shlex.quote(tz)}" - # Execute the script on the remote backend logger.info("Executing code on %s backend (task %s)...", env_type, effective_task_id[:8]) script_result = env.execute( @@ -1348,13 +883,8 @@ def _execute_remote( stdout_text = script_result.get("output", "") or "" exit_code = script_result.get("returncode", -1) - status = "success" - - # Check for timeout/interrupt from the backend - if exit_code == 124: - status = "timeout" - elif exit_code == 130: - status = "interrupted" + # Backend exit codes: 124 = timeout wrapper, 130 = SIGINT. + status = _REMOTE_EXIT_STATUS.get(exit_code, "success") except Exception as exc: duration = round(time.monotonic() - exec_start, 2) @@ -1363,44 +893,20 @@ def _execute_remote( duration, tool_call_counter[0], type(exc).__name__, exc, exc_info=True, ) - return json.dumps({ - "status": "error", - "error": str(exc), - "tool_calls_made": tool_call_counter[0], - "duration_seconds": duration, - }, ensure_ascii=False) + return _error_result(str(exc), tool_calls_made=tool_call_counter[0], duration=duration) finally: - # Stop the polling thread stop_event.set() if rpc_thread is not None: rpc_thread.join(timeout=5) - - # Clean up remote sandbox dir try: - env.execute( - f"rm -rf {quoted_sandbox_dir}", cwd="/", timeout=15, - ) + env.execute(f"rm -rf {quoted_sandbox_dir}", cwd="/", timeout=15) except Exception: logger.debug("Failed to clean up remote sandbox %s", sandbox_dir) duration = round(time.monotonic() - exec_start, 2) + stdout_text, stdout_metadata = _clean_output(stdout_text) - # --- Post-process output (same as local path) --- - - stdout_text, stdout_metadata = _truncate_stdout_text(stdout_text) - - # Strip ANSI escape sequences - from tools.ansi_strip import strip_ansi - stdout_text = strip_ansi(stdout_text) - - # Redact secrets. code_file=True: execute_code output is code-execution - # output that often echoes source/config — skip false-positive ENV/JSON/ - # f-string-template redaction while still masking real credentials. - from agent.redact import redact_sensitive_text - stdout_text = redact_sensitive_text(stdout_text, code_file=True) - - # Build response result: Dict[str, Any] = { "status": status, "output": stdout_text, @@ -1413,12 +919,7 @@ def _execute_remote( if status == "timeout": timeout_msg = f"Script timed out after {timeout}s and was killed." result["error"] = timeout_msg - # Include timeout message in output so the LLM always surfaces it - # to the user (see local path comment — same reasoning, #10807). - if stdout_text: - result["output"] = stdout_text + f"\n\n⏰ {timeout_msg}" - else: - result["output"] = f"⏰ {timeout_msg}" + result["output"] = _with_timeout_notice(stdout_text, timeout_msg) logger.warning( "execute_code (remote) timed out after %ss (limit %ss) with %d tool calls", duration, timeout, tool_call_counter[0], @@ -1436,85 +937,6 @@ def _execute_remote( # Main entry point # --------------------------------------------------------------------------- -def _build_child_env(*, rpc_endpoint: str, rpc_token: str, tmpdir: str, - child_python: str) -> Dict[str, str]: - """Build the scrubbed child environment both execution paths share. - - Extracted verbatim from the per-call spawn path so the session-kernel - path (tools/code_kernel.py) cannot drift from the security rules here: - secret scrubbing, UTF-8 forcing, TZ handling, subprocess HOME, and the - PYTHONPATH hygiene for external interpreters. - """ - from hermes_constants import apply_subprocess_home_env - child_env = _scrub_child_env(os.environ) - child_env["HERMES_RPC_SOCKET"] = rpc_endpoint - child_env["HERMES_RPC_TOKEN"] = rpc_token - child_env["PYTHONDONTWRITEBYTECODE"] = "1" - # Force UTF-8 for the child's stdio and default file encoding. - # - # Without this, on Windows sys.stdout is bound to the console code - # page (cp1252 on US-locale installs), and any script that does - # ``print("café")`` or ``print("→")`` crashes with: - # - # UnicodeEncodeError: 'charmap' codec can't encode character - # '\u2192' in position N: character maps to - # - # PYTHONIOENCODING fixes sys.stdin/stdout/stderr. - # PYTHONUTF8=1 enables "UTF-8 mode" (PEP 540) which additionally - # makes ``open()``'s default encoding UTF-8, so user scripts that - # write files without specifying encoding= also work correctly. - # - # On POSIX both values usually match the locale default already, - # so setting them is harmless belt-and-suspenders for environments - # with a C/POSIX locale (containers, minimal base images). - child_env["PYTHONIOENCODING"] = "utf-8" - child_env["PYTHONUTF8"] = "1" - # Inject user's configured timezone so datetime.now() in sandboxed - # code reflects the correct wall-clock time. Only TZ is set — - # HERMES_TIMEZONE is an internal Hermes setting and must not leak - # into child processes. - _tz_name = os.getenv("HERMES_TIMEZONE", "").strip() - if _tz_name: - child_env["TZ"] = _tz_name - child_env.pop("HERMES_TIMEZONE", None) - - apply_subprocess_home_env(child_env) - # ``hermes_tools.py`` always lives in the staging directory, so that - # directory must be importable even when project mode changes CWD. - # Hermes's own package root is useful too, but only when the child - # uses the same Python environment. Project mode can select an - # external venv; exposing Hermes's site-packages to that interpreter - # can mix incompatible compiled extensions (for example, Python 3.12 - # NumPy with a Python 3.9 project interpreter). - # - # Before re-injecting PYTHONPATH, strip Hermes-owned entries that - # leaked through _scrub_child_env (PYTHONPATH is in _SAFE_ENV_PREFIXES - # so it passes the scrub). They are redundant for same-Hermes- - # environment children and may be incompatible with external - # interpreters (project mode can select a different venv), so they - # must not shadow or poison the child's sys.path (#74817). - from tools.environments.local import _strip_hermes_owned_pythonpath - _strip_hermes_owned_pythonpath(child_env) - _hermes_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) - _existing_pp = child_env.get("PYTHONPATH", "") - _pp_parts = [tmpdir] - if _uses_hermes_python_environment(child_python): - _pp_parts.append(_hermes_root) - elif child_python not in _external_env_logged: - # Import behavior changes silently otherwise — surface it (once - # per interpreter path) so "import hermes_constants suddenly - # fails" reports are diagnosable without log spam. - _external_env_logged.add(child_python) - logger.info( - "execute_code: child interpreter %s is outside the Hermes " - "environment; hermes root omitted from PYTHONPATH", - child_python, - ) - if _existing_pp: - _pp_parts.append(_existing_pp) - child_env["PYTHONPATH"] = os.pathsep.join(_pp_parts) - return child_env - def execute_code( code: str, @@ -1522,30 +944,15 @@ def execute_code( enabled_tools: Optional[List[str]] = None, reset: bool = False, ) -> str: - """ - Run Python in the session's persistent kernel (local) or a per-call - child process (remote backends), with RPC access to a subset of - Hermes tools. + """Run Python in the session's persistent kernel (local) or on the remote + terminal backend, with RPC access to a subset of Hermes tools; returns the + JSON result string. - "Sandbox" in names below refers to the security envelope (env - scrubbing, tool whitelist + call budget, output redaction) — not an - isolation jail: in the default `project` mode, code runs in the - session's cwd with the project venv's interpreter. - - Dispatches to the local (UDS) or remote (file-based RPC) path - depending on the configured terminal backend. - - Args: - code: Python source code to execute. - task_id: Session task ID for tool isolation (terminal env, etc.). - enabled_tools: Tool names enabled in the current session. The sandbox - gets the intersection with SANDBOX_ALLOWED_TOOLS. - reset: Session-kernel mode only: kill the existing kernel and - start fresh before running this code. Ignored in - per-call mode, where every call is already fresh. - - Returns: - JSON string with execution results. + "Sandbox" here means the security envelope (env scrubbing, tool whitelist + + call budget, output redaction) — not an isolation jail: in the default + `project` mode, code runs in the session's cwd with the project venv. + ``enabled_tools`` is intersected with SANDBOX_ALLOWED_TOOLS; ``reset`` kills + the existing kernel first (ignored on per-call paths). """ if not SANDBOX_AVAILABLE: return tool_error( @@ -1553,10 +960,9 @@ def execute_code( "Use normal tool calls (terminal, read_file, write_file, ...) instead." ) - # Fail closed under a terminal-policy refusal scope (#68559): the routed - # profile's terminal policy could not be resolved and execute_code runs on - # the configured terminal backend — refuse rather than inheriting the - # launch process's ambient policy. + # Fail closed under a terminal-policy refusal scope: the routed profile's + # terminal policy is unresolved, so refuse rather than inherit the launch + # process's ambient policy. try: from tools.terminal_scope import enforce_no_refusal @@ -1575,12 +981,10 @@ def execute_code( "terminal(command=...) instead." ) - # Hard-block gateway-lifecycle commands, mirroring the terminal_tool - # guard (#68289): without this, execute_code is a straight bypass — the - # terminal() path refuses `launchctl bootout ai.hermes.gateway`, but the - # identical command inside `os.system(...)` / `subprocess.run([...])` - # here sailed through and SIGTERM'd the gateway mid-task. Gated on - # PID-file ownership, not the inherited env marker (#92560). + # Hard-block gateway-lifecycle commands (mirrors the terminal_tool guard — + # otherwise `os.system("launchctl bootout ...")` here bypasses it and + # SIGTERMs the gateway mid-task). Gated on PID-file ownership, not the + # inherited env marker. from tools.process_registry import _is_supervised_gateway_process if _is_supervised_gateway_process(): from cron.lifecycle_guard import contains_gateway_lifecycle_command @@ -1592,36 +996,25 @@ def execute_code( "Run the lifecycle command from a shell outside the gateway." ) - # Dispatch: remote backends use file-based RPC, local uses UDS from tools.terminal_tool import _get_env_config, _docker_has_host_access _env_config = _get_env_config() env_type = _env_config["env_type"] - # execute_code runs arbitrary Python (subprocess/os.system/...) that never - # passes through terminal()/DANGEROUS_PATTERNS, so guard the whole script - # here before either dispatch path spawns it. Runs synchronously in the - # caller (tool-executor) thread, which holds the session context (#30882). - # A Docker sandbox with host bind mounts is no longer isolated, so its - # script does not get the container fast-path. + # Arbitrary Python never passes through terminal()/DANGEROUS_PATTERNS, so + # guard the whole script before either dispatch path spawns it — in this + # (tool-executor) thread, which holds the session context. A Docker sandbox + # with host bind mounts is not isolated, so it gets no container fast-path. from tools.approval import check_execute_code_guard _guard = check_execute_code_guard( code, env_type, has_host_access=_docker_has_host_access(_env_config), ) if not _guard.get("approved", False): - return json.dumps({ - "status": "error", - "error": _guard.get("message") or "execute_code blocked by approval guard.", - "tool_calls_made": 0, - "duration_seconds": 0, - }, ensure_ascii=False) + return _error_result(_guard.get("message") or "execute_code blocked by approval guard.") - # Clean interrupt slate for a user-approved script before EITHER dispatch - # path spawns it: drop a stale bit that landed on this thread during the - # blocking approval-wait so it can't kill the just-approved run on the first - # poll (local _wait_for_process loop, or remote/ssh env.execute which routes - # through the same poll loop). A genuine post-clear interrupt re-sets the - # bit and is still caught downstream. + # Clear a stale interrupt bit that landed on this thread during the blocking + # approval-wait so it can't kill the just-approved run on the first poll + # (either dispatch path). A genuine post-clear interrupt re-sets the bit. if _guard.get("user_approved"): from tools.interrupt import clear_current_thread_interrupt clear_current_thread_interrupt() @@ -1629,397 +1022,39 @@ def execute_code( if env_type != "local": return _execute_remote(code, task_id, enabled_tools, reset=bool(reset)) - # --- Local execution path (UDS) --- below this line is unchanged --- - - # Import per-thread interrupt check (cooperative cancellation) from tools.interrupt import is_interrupted as _is_interrupted - # Resolve config _cfg = _load_config() timeout = _cfg.get("timeout", DEFAULT_TIMEOUT) max_tool_calls = _cfg.get("max_tool_calls", DEFAULT_MAX_TOOL_CALLS) + sandbox_tools = _sandbox_tools_for(enabled_tools) - # Determine which tools the sandbox can call - session_tools = set(enabled_tools) if enabled_tools else set() - sandbox_tools = frozenset(SANDBOX_ALLOWED_TOOLS & session_tools) + # Session kernels are always on locally: one interpreter per conversation; + # the guards above already ran for this cell, and the kernel path reuses the + # same env builder, RPC server, and output redaction as the remote path. + from tools.code_kernel import execute_in_session_kernel - if not sandbox_tools: - sandbox_tools = SANDBOX_ALLOWED_TOOLS - - if _get_kernel_mode() == "session": - # Session kernels keep one interpreter alive across calls; the guards - # above already ran for this cell, and the kernel path reuses the - # same env builder, RPC server, and output redaction as below. - from tools.code_kernel import execute_in_session_kernel - - _mode = _get_execution_mode() - return execute_in_session_kernel( - code, - task_id=task_id or "", - mode=_mode, - child_python=_resolve_child_python(_mode), - child_cwd=_resolve_child_cwd(_mode, "", task_id=task_id or ""), - sandbox_tools=frozenset(sandbox_tools), - timeout=timeout, - max_tool_calls=max_tool_calls, - reset=bool(reset), - is_interrupted=_is_interrupted, - ) - - # --- Set up temp directory with hermes_tools.py and script.py --- - tmpdir = tempfile.mkdtemp(prefix="hermes_sandbox_") - # Use /tmp on macOS to avoid the long /var/folders/... path that pushes - # Unix domain socket paths past the 104-byte macOS AF_UNIX limit. - # On Linux, tempfile.gettempdir() already returns /tmp. - # - # Windows: Python 3.9+ added partial AF_UNIX support but the file-backed - # variant is flaky across Windows builds (requires Windows 10 1803+, - # still fails under some configurations, and the socket file can't live - # on the same temp drive as the script). Fall back to loopback TCP — - # same ephemeral port, same 1-connection listen queue, same serialized - # request/response framing. The generated client reads the transport - # selector from HERMES_RPC_SOCKET (path vs. ``tcp://host:port``). - _sock_tmpdir = "/tmp" if sys.platform == "darwin" else tempfile.gettempdir() - _use_tcp_rpc = _IS_WINDOWS - if _use_tcp_rpc: - sock_path = None # not used on Windows; TCP endpoint stored below - rpc_endpoint = None # set after bind() - else: - sock_path = os.path.join(_sock_tmpdir, f"hermes_rpc_{uuid.uuid4().hex}.sock") - rpc_endpoint = sock_path - - tool_call_log: list = [] - tool_call_counter = [0] # mutable so the RPC thread can increment - exec_start = time.monotonic() - server_sock = None - stop_event = threading.Event() - - try: - # Write the auto-generated hermes_tools module. - # encoding="utf-8" is required on Windows — the stub and user code - # both contain non-ASCII characters (em-dashes in docstrings, plus - # whatever the user script carries). Python's default open() uses - # the system locale on Windows (cp1252 typically), which corrupts - # those bytes; the child then fails to import with a SyntaxError - # ("'utf-8' codec can't decode byte 0x97 in position ...") because - # Python source files are decoded as UTF-8 by default (PEP 3120). - # sandbox_tools is already the correct set (intersection with session - # tools, or SANDBOX_ALLOWED_TOOLS as fallback — see lines above). - tools_src = generate_hermes_tools_module(list(sandbox_tools)) - with open(os.path.join(tmpdir, "hermes_tools.py"), "w", encoding="utf-8") as f: - f.write(tools_src) - - # Write the user's script - with open(os.path.join(tmpdir, "script.py"), "w", encoding="utf-8") as f: - f.write(code) - - # --- Start RPC server --- - rpc_token = secrets.token_urlsafe(32) - # Two transports: - # POSIX: AF_UNIX stream socket on sock_path, chmod 0600 for - # owner-only access. Filesystem permissions gate the socket. - # Windows: AF_INET stream socket on 127.0.0.1 with an ephemeral - # port. No filesystem permission story, but loopback-only bind - # means only the current user's processes (not remote) can - # connect. HERMES_RPC_SOCKET is set to ``tcp://127.0.0.1:`` - # which the generated client parses to pick AF_INET. - if _use_tcp_rpc: - server_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - server_sock.bind(("127.0.0.1", 0)) # ephemeral port - _host, _port = server_sock.getsockname()[:2] - rpc_endpoint = f"tcp://{_host}:{_port}" - else: - server_sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) - server_sock.bind(sock_path) - os.chmod(sock_path, 0o600) - server_sock.listen(1) - - # Wrapped so the thread inherits the turn's approval context + callbacks - # (see tools.thread_context) — else gateway sandbox tool calls silently - # auto-approve dangerous commands (#33057, #30882). - rpc_thread = threading.Thread( - target=propagate_context_to_thread(_rpc_server_loop), - args=( - server_sock, task_id, tool_call_log, - tool_call_counter, max_tool_calls, sandbox_tools, stop_event, rpc_token, - ), - daemon=True, - ) - rpc_thread.start() - - # --- Spawn child process --- - # Build a minimal environment for the child. We intentionally exclude - # API keys and tokens to prevent credential exfiltration from LLM- - # generated scripts. The child accesses tools via RPC, not direct API. - # Exception: env vars declared by loaded skills (via env_passthrough - # registry) or explicitly allowed by the user in config.yaml - # (terminal.env_passthrough) are passed through. On Windows, a small - # OS-essential allowlist (SYSTEMROOT, WINDIR, COMSPEC, ...) is also - # passed through — without those, the child can't create a socket - # or spawn a subprocess. See ``_scrub_child_env`` for the rules. - - # Resolve interpreter + CWD based on execute_code mode. - # - strict : today's behavior (sys.executable + tmpdir CWD). - # - project: user's venv python + session's working directory, so - # project deps like pandas and user files resolve. - # Env scrubbing and tool whitelist apply identically in both modes. - _mode = _get_execution_mode() - _child_python = _resolve_child_python(_mode) - _child_cwd = _resolve_child_cwd(_mode, tmpdir, task_id=task_id or "") - _script_path = os.path.join(tmpdir, "script.py") - - child_env = _build_child_env( - rpc_endpoint=rpc_endpoint, - rpc_token=rpc_token, - tmpdir=tmpdir, - child_python=_child_python, - ) - - proc = subprocess.Popen( - [_child_python, _script_path], - cwd=_child_cwd, - env=child_env, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - stdin=subprocess.DEVNULL, - start_new_session=True, - creationflags=subprocess.CREATE_NO_WINDOW if _IS_WINDOWS else 0, - ) - - # --- Poll loop: watch for exit, timeout, and interrupt --- - deadline = time.monotonic() + timeout - stderr_chunks: list = [] - - # Background readers to avoid pipe buffer deadlocks. - # For stdout we use a head+tail strategy: keep the first HEAD_BYTES - # and a rolling window of the last TAIL_BYTES so the final print() - # output is never lost. Stderr keeps head-only (errors appear early). - _STDOUT_HEAD_BYTES = int(MAX_STDOUT_BYTES * 0.4) # 40% head - _STDOUT_TAIL_BYTES = MAX_STDOUT_BYTES - _STDOUT_HEAD_BYTES # 60% tail - - def _drain(pipe, chunks, max_bytes): - """Simple head-only drain (used for stderr).""" - total = 0 - try: - while True: - data = pipe.read(4096) - if not data: - break - if total < max_bytes: - keep = max_bytes - total - chunks.append(data[:keep]) - total += len(data) - except (ValueError, OSError) as e: - logger.debug("Error reading process output: %s", e, exc_info=True) - - stdout_total_bytes = [0] # mutable ref for total bytes seen - - def _drain_head_tail(pipe, head_chunks, tail_chunks, head_bytes, tail_bytes, total_ref): - """Drain stdout keeping both head and tail data.""" - head_collected = 0 - from collections import deque - tail_buf = deque() - tail_collected = 0 - try: - while True: - data = pipe.read(4096) - if not data: - break - total_ref[0] += len(data) - # Fill head buffer first - if head_collected < head_bytes: - keep = min(len(data), head_bytes - head_collected) - head_chunks.append(data[:keep]) - head_collected += keep - data = data[keep:] # remaining goes to tail - if not data: - continue - # Everything past head goes into rolling tail buffer - tail_buf.append(data) - tail_collected += len(data) - # Evict old tail data to stay within tail_bytes budget - while tail_collected > tail_bytes and tail_buf: - oldest = tail_buf.popleft() - tail_collected -= len(oldest) - except (ValueError, OSError): - pass - # Transfer final tail to output list - tail_chunks.extend(tail_buf) - - stdout_head_chunks: list = [] - stdout_tail_chunks: list = [] - - stdout_reader = threading.Thread( - target=_drain_head_tail, - args=(proc.stdout, stdout_head_chunks, stdout_tail_chunks, - _STDOUT_HEAD_BYTES, _STDOUT_TAIL_BYTES, stdout_total_bytes), - daemon=True - ) - stderr_reader = threading.Thread( - target=_drain, args=(proc.stderr, stderr_chunks, MAX_STDERR_BYTES), daemon=True - ) - stdout_reader.start() - stderr_reader.start() - - status = "success" - _activity_state = { - "last_touch": time.monotonic(), - "start": exec_start, - } - try: - from tools.environments.base import touch_activity_if_due - except Exception: - touch_activity_if_due = None - poll_interval = 0.005 - while proc.poll() is None: - if _is_interrupted(): - _kill_process_group(proc) - status = "interrupted" - break - now = time.monotonic() - if now > deadline: - _kill_process_group(proc, escalate=True) - status = "timeout" - break - # Periodic activity touch so the gateway's inactivity timeout - # doesn't kill the agent during long code execution (#10807). - if touch_activity_if_due is not None: - try: - touch_activity_if_due(_activity_state, "execute_code running") - except Exception: - pass - try: - proc.wait(timeout=min(poll_interval, max(0.0, deadline - now))) - except subprocess.TimeoutExpired: - pass - poll_interval = min(0.2, poll_interval * 1.5) - - # Wait for readers to finish draining - stdout_reader.join(timeout=3) - stderr_reader.join(timeout=3) - - stderr_text = b"".join(stderr_chunks).decode("utf-8", errors="replace") - - stdout_text, stdout_metadata = _assemble_stdout_result( - b"".join(stdout_head_chunks), - b"".join(stdout_tail_chunks), - 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) - - # Wait for RPC thread to finish - stop_event.set() - server_sock.close() # break accept() so thread exits promptly - server_sock = None # prevent double close in finally - rpc_thread.join(timeout=3) - - # Strip ANSI escape sequences so the model never sees terminal - # formatting — prevents it from copying escapes into file writes. - from tools.ansi_strip import strip_ansi - stdout_text = strip_ansi(stdout_text) - stderr_text = strip_ansi(stderr_text) - - # Redact secrets (API keys, tokens, etc.) from sandbox output. - # The sandbox env-var filter (lines 434-454) blocks os.environ access, - # but scripts can still read secrets from disk (e.g. open('~/.hermes/.env')). - # This ensures leaked secrets never enter the model context. - # code_file=True: this is code-execution output — skip false-positive - # ENV/JSON/f-string-template redaction; real credentials still masked. - from agent.redact import redact_sensitive_text - stdout_text = redact_sensitive_text(stdout_text, code_file=True) - stderr_text = redact_sensitive_text(stderr_text, code_file=True) - - # Build response - result: Dict[str, Any] = { - "status": status, - "output": stdout_text, - "exit_code": exit_code, - "tool_calls_made": tool_call_counter[0], - "duration_seconds": duration, - } - result.update(stdout_metadata) - - if status == "timeout": - timeout_msg = f"Script timed out after {timeout}s and was killed." - result["error"] = timeout_msg - # Include timeout message in output so the LLM always surfaces it - # to the user. When output is empty, models often treat the result - # as "nothing happened" and produce an empty response, which the - # gateway stream consumer silently drops (#10807). - if stdout_text: - result["output"] = stdout_text + f"\n\n⏰ {timeout_msg}" - else: - result["output"] = f"⏰ {timeout_msg}" - logger.warning( - "execute_code timed out after %ss (limit %ss) with %d tool calls", - duration, timeout, tool_call_counter[0], - ) - elif status == "interrupted": - result["output"] = _format_interrupted_output(stdout_text) - elif exit_code != 0: - result["status"] = "error" - result["error"] = stderr_text or f"Script exited with code {exit_code}" - # Include stderr in output so the LLM sees the traceback - if stderr_text: - result["output"] = stdout_text + "\n--- stderr ---\n" + stderr_text - # Known-failure-class recovery hint (import misuse, missing - # module, dict-vs-string result handling) so the model fixes - # the script on the next attempt instead of re-diagnosing. - hint = _sandbox_failure_hint(stderr_text, enabled_tools=sandbox_tools) - if hint: - result["hint"] = hint - - return json.dumps(result, ensure_ascii=False) - - except Exception as exc: - duration = round(time.monotonic() - exec_start, 2) - logger.error( - "execute_code failed after %ss with %d tool calls: %s: %s", - duration, - tool_call_counter[0], - type(exc).__name__, - exc, - exc_info=True, - ) - return json.dumps({ - "status": "error", - "error": str(exc), - "tool_calls_made": tool_call_counter[0], - "duration_seconds": duration, - }, ensure_ascii=False) - - finally: - # Cleanup temp dir and socket - if server_sock is not None: - try: - server_sock.close() - except OSError as e: - logger.debug("Server socket close error: %s", e) - import shutil - shutil.rmtree(tmpdir, ignore_errors=True) - try: - # Only UDS has a filesystem socket to unlink; TCP sockets are - # freed by server_sock.close() above. - if sock_path: - os.unlink(sock_path) - except OSError: - pass # already cleaned up or never created + _mode = _get_execution_mode() + return execute_in_session_kernel( + code, + task_id=task_id or "", + mode=_mode, + child_python=_resolve_child_python(_mode), + child_cwd=_resolve_child_cwd(_mode, "", task_id=task_id or ""), + sandbox_tools=frozenset(sandbox_tools), + timeout=timeout, + max_tool_calls=max_tool_calls, + reset=bool(reset), + is_interrupted=_is_interrupted, + ) def _kill_process_group(proc, escalate: bool = False): - """Kill the child and its entire process tree (cross-platform). - - Delegates to :func:`agent.deadline.kill_process_tree` (#85125 4d): - SIGTERM to the whole tree first (killpg when the child leads its own - group — it does, ``start_new_session=True`` — plus a psutil descendant - sweep for setsid'd grandchildren; ``taskkill /T /F`` on Windows). - With ``escalate=True`` the child gets 5s to exit after SIGTERM, then the - surviving tree is SIGKILLed — same escalation the old psutil-local body - implemented. Never raises; a delegation failure degrades to a plain - ``proc.kill()`` like the old psutil-failure fallback. - """ + """Kill the child and its whole process tree (cross-platform) via + agent.deadline.kill_process_tree: SIGTERM the tree (killpg + psutil + descendant sweep for setsid'd grandchildren; ``taskkill /T /F`` on Windows); + with ``escalate=True`` wait 5s then SIGKILL survivors. Never raises — a + delegation failure degrades to a plain ``proc.kill()``.""" import signal as _signal def _tree_signal(sig) -> None: @@ -2046,14 +1081,10 @@ def _kill_process_group(proc, escalate: bool = False): def _load_config() -> dict: - """Load code_execution config without importing the interactive CLI. + """Load the ``code_execution`` config section via the lightweight raw reader. - This helper is called while building the module-level execute_code schema - during tool discovery. Importing ``cli`` here pulls prompt_toolkit/Rich and - a large chunk of the classic REPL onto every agent startup path, including - ``hermes --tui`` where it is never used. Read the lightweight raw config - instead; the config layer already caches by (mtime, size), and an absent - key cleanly falls back to DEFAULT_EXECUTION_MODE. + Runs while the module-level schema is built at tool discovery, so it must + not import ``cli`` (prompt_toolkit/Rich on every startup path). """ try: from hermes_cli.config import read_raw_config @@ -2068,20 +1099,14 @@ def _load_config() -> dict: # Execution mode resolution (strict vs project) # --------------------------------------------------------------------------- -# Valid values for code_execution.mode. Kept as a module constant so tests -# and the config layer can reference the canonical set. +# Canonical code_execution.mode values (referenced by tests and the config layer). EXECUTION_MODES = ("project", "strict") DEFAULT_EXECUTION_MODE = "project" -# Session kernels are the only local execution model: one persistent kernel -# per conversation (see tools/code_kernel.py). The former -# code_execution.kernel_mode knob ("per-call" | "session") is retired — the -# config key is silently ignored if present, and _get_kernel_mode() remains -# only as a compat symbol for external callers. Remote terminal backends -# still run per-call: their file-based RPC path has no kernel host YET (a -# long-lived remote runner + cell protocol is tracked follow-up work, not a -# design limit). -KERNEL_MODES = ("per-call", "session") # legacy compat constant +# Session kernels are the only local execution model (tools/code_kernel.py). +# The former code_execution.kernel_mode knob is retired: a leftover config key +# is silently ignored; these symbols remain only for external callers. +KERNEL_MODES = ("per-call", "session") DEFAULT_KERNEL_MODE = "session" @@ -2091,22 +1116,10 @@ def _get_kernel_mode() -> str: def _get_execution_mode() -> str: - """Return the active execute_code mode — 'project' or 'strict'. - - Reads ``code_execution.mode`` from config.yaml; invalid values fall back - to ``DEFAULT_EXECUTION_MODE`` ('project') with a log warning. - - Mode semantics: - - ``project`` (default): scripts run in the session's working directory - with the active virtual environment's python, so project dependencies - (pandas, torch, project packages) and files resolve naturally. - - ``strict``: scripts run in an isolated temp directory with - ``sys.executable`` (hermes-agent's python). Reproducible and the - interpreter is guaranteed to work, but project deps and relative paths - won't resolve. - - Env scrubbing and tool whitelist apply identically in both modes. - """ + """Active execute_code mode from ``code_execution.mode`` (invalid → default + with a warning). ``project``: session cwd + active venv python so project + deps/files resolve; ``strict``: isolated temp dir + ``sys.executable``. + Env scrubbing and the tool whitelist apply identically in both.""" cfg_value = str(_load_config().get("mode", DEFAULT_EXECUTION_MODE)).strip().lower() if cfg_value in EXECUTION_MODES: return cfg_value @@ -2117,199 +1130,6 @@ def _get_execution_mode() -> str: return DEFAULT_EXECUTION_MODE -# Shared budget for the two interpreter-probe caches below. Success-only -# dict caches (FIFO-evicted at the cap) rather than lru_cache: a transient -# probe failure (fork pressure, 5s timeout on a loaded host) must not stick -# for the process lifetime. -_PROBE_CACHE_MAX = 32 -_usable_python_cache: dict = {} -_python_prefix_cache: dict = {} - -# Interpreter paths already reported as outside the Hermes environment — -# dedupes the exclusion log to once per path per process. -_external_env_logged: set = set() - - -def _cache_probe_result(cache: dict, key: str, value): - """Insert into a bounded probe cache, FIFO-evicting at the cap.""" - if len(cache) >= _PROBE_CACHE_MAX: - cache.pop(next(iter(cache))) - cache[key] = value - - -def _is_usable_python(python_path: str) -> bool: - """Check whether a candidate Python interpreter is usable for execute_code. - - Requires Python 3.8+ (f-strings and stdlib modules the RPC stubs need). - Successful probes are cached per interpreter path; failures are retried - (a sticky False would silently pin project mode to sys.executable). - """ - cached = _usable_python_cache.get(python_path) - if cached is not None: - return cached - result = _probe_python( - python_path, - "import sys; sys.exit(0 if sys.version_info >= (3, 8) else 1)", - ) - if result is None: - return False - usable = result.returncode == 0 - _cache_probe_result(_usable_python_cache, python_path, usable) - return usable - - -def _probe_python(python_path: str, code: str, *, text: bool = False): - """Run ``python_path -c code`` with the standard interpreter-probe guards. - - Returns the ``CompletedProcess``, or ``None`` when the interpreter is - missing, can't be spawned, or hangs past the 5s timeout. - """ - try: - from agent.delegation_context import delegated_child_subprocess_env - - return subprocess.run( - [python_path, "-c", code], - timeout=5, - capture_output=True, - text=text, - creationflags=subprocess.CREATE_NO_WINDOW if _IS_WINDOWS else 0, - stdin=subprocess.DEVNULL, - env=delegated_child_subprocess_env(), - ) - except (OSError, subprocess.TimeoutExpired, subprocess.SubprocessError): - return None - - -def _python_environment_prefix(python_path: str) -> str: - """Return the resolved ``sys.prefix`` reported by *python_path*, if any. - - Successful probes are cached per interpreter path (bounded, FIFO-evicted). - Failures are NOT cached: a transient probe failure (fork pressure, 5s - timeout on a loaded host) must not stick for the process lifetime — a - sticky empty result would silently drop the hermes root from every - subsequent execute_code call's PYTHONPATH. - """ - cached = _python_prefix_cache.get(python_path) - if cached is not None: - return cached - result = _probe_python(python_path, "import sys; print(sys.prefix)", text=True) - if result is not None and result.returncode == 0 and result.stdout.strip(): - prefix = os.path.realpath(result.stdout.strip()) - _cache_probe_result(_python_prefix_cache, python_path, prefix) - return prefix - return "" - - -def _uses_hermes_python_environment(python_path: str) -> bool: - """Whether *python_path* belongs to Hermes's active Python environment. - - Short-circuits when *python_path* IS the running interpreter (by path or - realpath) — no subprocess probe on the default strict-mode path, and no - way for a flaky probe of ``sys.executable`` itself to break the invariant - that repo-root modules are importable in strict mode. The realpath leg - also covers venvs whose bin/python resolves to the same binary (e.g. - ``uv run`` setting VIRTUAL_ENV without changing sys.prefix). - """ - if python_path == sys.executable or ( - os.path.realpath(python_path) == os.path.realpath(sys.executable) - ): - return True - return _python_environment_prefix(python_path) == os.path.realpath(sys.prefix) - - -def _resolve_child_python(mode: str) -> str: - """Pick the Python interpreter for the execute_code subprocess. - - In ``strict`` mode, always ``sys.executable`` — guaranteed to work and - keeps behavior fully reproducible across sessions. - - In ``project`` mode, prefer the user's active virtualenv/conda env's - python so ``import pandas`` etc. work. Falls back to ``sys.executable`` - if no venv is detected, the candidate binary is missing/not executable, - or it fails a Python 3.8+ version check. - """ - if mode != "project": - return sys.executable - - if _IS_WINDOWS: - exe_names = ("python.exe", "python3.exe") - subdirs = ("Scripts",) - else: - exe_names = ("python", "python3") - subdirs = ("bin",) - - for var in ("VIRTUAL_ENV", "CONDA_PREFIX"): - root = os.environ.get(var, "").strip() - if not root: - continue - for subdir in subdirs: - for exe in exe_names: - candidate = os.path.join(root, subdir, exe) - if not (os.path.isfile(candidate) and os.access(candidate, os.X_OK)): - continue - if _is_usable_python(candidate): - return candidate - # Found the interpreter but it failed the version check — - # log once and fall through to sys.executable. - logger.info( - "execute_code: skipping %s=%s (Python version < 3.8 or broken). " - "Using sys.executable instead.", var, candidate, - ) - return sys.executable - - return sys.executable - - -def _resolve_child_cwd(mode: str, staging_dir: str, task_id: str = "") -> str: - """Resolve the working directory for the execute_code subprocess. - - - ``strict``: the staging tmpdir (today's behavior). - - ``project``: the session's own cwd — its per-session cwd record - (written after every completed terminal command), then the raw - per-session cwd override registered via ``session.cwd.set`` / - ``register_task_env_overrides``, then the session's TERMINAL_CWD - (same as the terminal tool), or ``os.getcwd()`` if none points at a - real dir. Falls back to the staging tmpdir as a last resort so we - never invoke Popen with a nonexistent cwd. - - This mirrors the resolution ladder file tools and the terminal use - (record → registered override → TERMINAL_CWD), so all file-writing - paths within a session agree on the working directory. (#56047) - """ - if mode != "project": - return staging_dir - if task_id: - # 1. The session's cwd record — IS the session's `cd` state. - try: - from tools.terminal_tool import get_session_cwd - - recorded = get_session_cwd(task_id) - except Exception: - recorded = None - if recorded and os.path.isdir(recorded): - return recorded - # 2. Registered workspace override (session.cwd.set → gateway/TUI/ACP). - try: - from tools.file_tools import _registered_task_cwd_override - - session_cwd = _registered_task_cwd_override(task_id) - except Exception: - session_cwd = None - if session_cwd and os.path.isdir(session_cwd): - return session_cwd - from agent.runtime_cwd import scope_terminal_cwd - - raw = scope_terminal_cwd().strip() - if raw: - expanded = os.path.expanduser(raw) - if os.path.isdir(expanded): - return expanded - here = os.getcwd() - if os.path.isdir(here): - return here - return staging_dir - - # --------------------------------------------------------------------------- # OpenAI Function-Calling Schema # --------------------------------------------------------------------------- @@ -2344,40 +1164,24 @@ _TOOL_DOC_LINES = [ def build_execute_code_schema(enabled_sandbox_tools: set = None, mode: str = None) -> dict: - """Build the execute_code schema with description listing only enabled tools. - - When tools are disabled via ``hermes tools`` (e.g. web is turned off), - the schema description should NOT mention web_search / web_extract — - otherwise the model thinks they are available and keeps trying to use them. - - ``mode`` controls the working-directory sentence in the description: - - ``'strict'``: scripts run in a temp dir (not the session's CWD) - - ``'project'`` (default): scripts run in the session's CWD with the - active venv's python - If ``mode`` is None, the current ``code_execution.mode`` config is read. + """Build the execute_code schema listing only *enabled_sandbox_tools* — a + disabled tool (e.g. web off) must not appear or the model keeps trying it. + ``mode`` (None → current config) selects the working-directory sentence. """ if enabled_sandbox_tools is None: enabled_sandbox_tools = SANDBOX_ALLOWED_TOOLS if mode is None: mode = _get_execution_mode() - # Build tool documentation lines for only the enabled tools tool_lines = "\n".join( doc for name, doc in _TOOL_DOC_LINES if name in enabled_sandbox_tools ) - # Build example import list from enabled tools import_examples = [n for n in ("web_search", "terminal") if n in enabled_sandbox_tools] if not import_examples: import_examples = sorted(enabled_sandbox_tools)[:2] - if import_examples: - import_str = ", ".join(import_examples) + ", ..." - else: - import_str = "..." + import_str = ", ".join(import_examples) + ", ..." if import_examples else "..." - # Mode-specific CWD guidance. Project mode is the default and matches - # terminal()'s filesystem/interpreter; strict mode retains the isolated - # temp-dir staging and hermes-agent's own python. if mode == "strict": cwd_note = ( "Scripts run in their own temp dir, not the session's CWD — use absolute paths " @@ -2392,11 +1196,8 @@ def build_execute_code_schema(enabled_sandbox_tools: set = None, "deps; check `import x` before relying on project packages)." ) - # Session kernels are always on (kernel_mode retired in #96787): - # persistence is part of the tool's one description, not a bolt-on - # paragraph behind a dead conditional. Remote hosts that cannot sustain - # a kernel fail open to per-call silently — not worth schema words; - # the result's `kernel` field tells the truth per call. + # Remote hosts that fail open to per-call are not worth schema words; the + # result's `kernel` field tells the truth per call. description = ( "Run Python that calls Hermes tools programmatically. Use when you " "need 3+ tool calls with logic between them: filtering/reducing " @@ -2452,18 +1253,9 @@ def build_execute_code_schema(enabled_sandbox_tools: set = None, EXECUTE_CODE_SCHEMA = build_execute_code_schema() -# --- Registry --- -from tools.registry import registry, tool_error - - def _execute_code_handler(args: dict, **kwargs) -> str: - """Recover misdirected calls before dispatching to ``execute_code``. - - Models sometimes reuse terminal's ``command`` argument or send a - non-string ``code`` payload; both get an actionable redirect instead - of a generic failure. - """ - # Help models recover when they reuse terminal's ``command`` argument. + """Redirect misdirected calls (terminal's ``command`` arg, non-string + ``code``) with an actionable error before dispatching to ``execute_code``.""" if "code" not in args and "command" in args: logger.warning( "execute_code received 'command' instead of the required 'code' argument" @@ -2476,8 +1268,6 @@ def _execute_code_handler(args: dict, **kwargs) -> str: code = args.get("code", "") if code is not None and not isinstance(code, str): - # A non-string payload (int, dict, list) would otherwise surface as - # a generic AttributeError from code.strip() — redirect instead. return tool_error( f"execute_code received a {type(code).__name__} in 'code', but it " "requires Python source as a string. Retry as " diff --git a/tools/code_kernel.py b/tools/code_kernel.py index 7805f636d1..c3b9420103 100644 --- a/tools/code_kernel.py +++ b/tools/code_kernel.py @@ -1,49 +1,34 @@ """Session-persistent Python kernels for execute_code. -With ``code_execution.kernel_mode: session``, execute_code keeps one Python -child process alive per (task, mode, interpreter, cwd, tool-set) and feeds it -one code cell per call, so variables, imports, and loaded data survive across -calls:: - - execute_code(code="df = load_big_csv()") # cell 1 - execute_code(code="print(df.describe())") # cell 2 — df still exists - -The default mode, ``per-call``, keeps today's behavior exactly: a fresh -process per call, no state carried over. +One Python child stays alive per (owner, mode, interpreter, cwd, tool-set) and +runs one code cell per call, so variables/imports/data survive across calls. Design constraints, in order: -- **Same security envelope as per-call.** The child env is built by the same - ``_build_child_env`` the per-call path uses (secret scrubbing, tool - whitelist, PYTHONPATH rules); the RPC server is the same - ``_rpc_server_loop`` with the same token and per-cell tool budget; output - passes through the same ANSI strip + secret redaction. Nothing here widens - what a script can reach — it only widens how long one interpreter lives. -- **A wedged kernel dies, never hangs the agent.** A cell that exceeds the - timeout (or an interrupt) kills the whole kernel process tree and drops the - registry entry; the next call spawns a fresh kernel. Losing kernel state on - timeout is deliberate: there is no reliable way to interrupt one cell - in-place without leaving the interpreter in an unknown state. -- **The env is frozen at spawn.** Skills that register env passthrough after - the kernel started are not visible until ``reset=true`` (or the kernel is - otherwise replaced). The result payload names the kernel so this is - diagnosable. +- **Same security envelope as per-call**: same ``_build_child_env`` (secret + scrubbing, tool whitelist, PYTHONPATH rules), same ``_rpc_server_loop`` with + the same token and per-cell tool budget, same ANSI strip + secret redaction. + Nothing here widens what a script can reach — only how long it lives. +- **A wedged kernel dies, never hangs the agent.** Timeout or interrupt kills + the whole kernel process tree and drops the registry entry; the next call + spawns fresh. Losing state is deliberate: there is no reliable way to + interrupt one cell in place without leaving the interpreter unknown. +- **The env is frozen at spawn.** Env passthrough registered after the kernel + started is invisible until ``reset=true``; the result payload names the + kernel so this is diagnosable. -Wire protocol (host <-> kernel child): +Wire protocol (host <-> child): requests are one JSON object per stdin line +``{"id", "code"}``; responses are framed on stdout as +`` \\n`` with a per-kernel random SENTINEL from +the environment. Bytes outside frames are raw fd-level output (subprocesses +inherit the real stdout) and are attributed to the running cell — calls are +serialized per kernel, so attribution is unambiguous. Python-level +stdout/stderr are captured via ``contextlib.redirect_*`` into the payload; a +script forging a frame can only fake its own cell result (same trust position +as a per-call script printing a forged success message). -- Requests: one JSON object per line on the child's stdin: - ``{"id": , "code": }``. -- Responses: framed on the child's stdout as - `` \\n`` where SENTINEL carries a - per-kernel random token from the environment. Bytes outside frames are - raw fd-level output (subprocesses spawned by user code inherit the real - stdout) and are attributed to the cell that was running when they arrived — - calls are serialized per kernel, so attribution is unambiguous. -- Python-level stdout/stderr inside a cell are captured by the runner via - ``contextlib.redirect_*`` and returned inside the JSON payload. A script - that deliberately prints a forged frame can fake its own cell result; that - is the same trust position as a per-call script printing a forged success - message, and it gains nothing beyond lying to its own caller. +Also hosts what ``tools.code_kernel_remote`` shares: owner resolution, the +registry lifecycle, and the runner's cell-exec core. """ from __future__ import annotations @@ -61,16 +46,48 @@ import tempfile import threading import time import uuid -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Callable, Dict, List, Optional, Tuple logger = logging.getLogger(__name__) _IS_WINDOWS = sys.platform == "win32" -# Runner-side caps: bound captured python-level output before it ever reaches -# the host (the host applies its own MAX_STDOUT truncation again). +# Runner-side cap on captured python-level output; the host applies its own +# MAX_STDOUT truncation again. _RUNNER_CAPTURE_BYTES = 1_000_000 +# Shared by both generated runners (which define _CAPTURE_LIMIT first): exec one +# request in the persistent GLOBALS namespace and build the response payload. +# `__name__` is `__main__` so scripts behave like the per-call path. +RUNNER_CELL_SOURCE = '''\ +GLOBALS = {"__name__": "__main__", "__builtins__": __builtins__} + + +def _clip(text): + return (text, False) if len(text) <= _CAPTURE_LIMIT else (text[:_CAPTURE_LIMIT], True) + + +def run_cell(request, execution_count): + """Exec one cell; returns (response payload, FULL stdout text).""" + out, err = io.StringIO(), io.StringIO() + status, trace = "ok", "" + try: + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + exec(compile(request["code"], "", "exec"), GLOBALS) + except SystemExit as exc: + status, trace = "exit", "SystemExit: " + repr(exc.code) + except BaseException: + status, trace = "error", traceback.format_exc() + stdout_text, stdout_clipped = _clip(out.getvalue()) + stderr_text, stderr_clipped = _clip(err.getvalue()) + return { + "id": request.get("id", ""), "status": status, + "stdout": stdout_text, "stderr": stderr_text, + "stdout_clipped": stdout_clipped, "stderr_clipped": stderr_clipped, + "traceback": trace, "execution_count": execution_count, + }, out.getvalue() +''' + KERNEL_RUNNER_SOURCE = '''\ """Auto-generated Hermes session-kernel runner. One exec cell per request.""" import contextlib @@ -84,40 +101,28 @@ _SENTINEL = os.environ["HERMES_KERNEL_SENTINEL"] _CAPTURE_LIMIT = {capture_limit} _SPILL_DIR = os.environ.get("HERMES_KERNEL_SPILL_DIR", "") _SPILL_CAP = {spill_cap} - -# The persistent cell namespace. `__name__` is `__main__` so scripts behave -# like the per-call path; builtins resolve normally through exec. -GLOBALS = {{"__name__": "__main__", "__builtins__": __builtins__}} - _real_stdout = sys.stdout +{cell_source} -def _bounded(text, spill_name=None): - """Clip to the inline cap; spill the FULL text to disk when clipping. - - Returns (clipped_text, clipped?, spill_path_or_empty). Spill is - best-effort — a failed write degrades to plain clipping. - """ - if len(text) <= _CAPTURE_LIMIT: - return text, False, "" - spill_path = "" - if _SPILL_DIR and spill_name: - try: - spill_path = os.path.join(_SPILL_DIR, spill_name) - with open(spill_path, "w", encoding="utf-8", errors="replace") as f: - f.write(text[:_SPILL_CAP]) - if len(text) > _SPILL_CAP: - f.write("\\n\\n[... spill capped ...]") - except Exception: - spill_path = "" - return text[: _CAPTURE_LIMIT], True, spill_path +def _spill(text, spill_name): + """Best-effort: write the FULL clipped stdout to disk, return its path or "".""" + if not _SPILL_DIR: + return "" + try: + spill_path = os.path.join(_SPILL_DIR, spill_name) + with open(spill_path, "w", encoding="utf-8", errors="replace") as f: + f.write(text[:_SPILL_CAP]) + if len(text) > _SPILL_CAP: + f.write("\\n\\n[... spill capped ...]") + return spill_path + except Exception: + return "" def _reply(payload): body = json.dumps(payload, ensure_ascii=False).encode("utf-8") - _real_stdout.buffer.write( - ("\\n" + _SENTINEL + " " + str(len(body)) + "\\n").encode("utf-8") - ) + _real_stdout.buffer.write(("\\n" + _SENTINEL + " " + str(len(body)) + "\\n").encode("utf-8")) _real_stdout.buffer.write(body) _real_stdout.buffer.flush() @@ -133,55 +138,30 @@ def main(): except ValueError: continue execution_count += 1 - out, err = io.StringIO(), io.StringIO() - status = "ok" - trace = "" - try: - with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): - exec(compile(request["code"], "", "exec"), GLOBALS) - except SystemExit as exc: - status = "exit" - trace = "SystemExit: " + repr(exc.code) - except BaseException: - status = "error" - trace = traceback.format_exc() - stdout_text, stdout_clipped, stdout_spill = _bounded( - out.getvalue(), "cell_%06d_stdout.txt" % execution_count + payload, full_stdout = run_cell(request, execution_count) + payload["stdout_spill_path"] = ( + _spill(full_stdout, "cell_%06d_stdout.txt" % execution_count) + if payload["stdout_clipped"] else "" ) - stderr_text, stderr_clipped, _ = _bounded(err.getvalue()) - _reply( - {{ - "id": request.get("id", ""), - "status": status, - "stdout": stdout_text, - "stderr": stderr_text, - "stdout_clipped": stdout_clipped, - "stderr_clipped": stderr_clipped, - "stdout_spill_path": stdout_spill, - "traceback": trace, - "execution_count": execution_count, - }} - ) - if status == "exit": + _reply(payload) + if payload["status"] == "exit": break if __name__ == "__main__": main() -'''.format(capture_limit=_RUNNER_CAPTURE_BYTES, - spill_cap=5_000_000) +'''.format(cell_source=RUNNER_CELL_SOURCE, capture_limit=_RUNNER_CAPTURE_BYTES, spill_cap=5_000_000) class CellAuthority: """The approval/context identity of exactly one execute_code cell. - Interpreter state persists across cells; RPC authority must not. Each - cell installs a fresh authority — captured from the CALLING thread at - cell start, exactly what ``propagate_context_to_thread`` would have - captured for a per-call RPC thread — and retires it when the cell - settles, so a tool call arriving later (a background thread the cell - left behind, a raced client write) is refused instead of running under - a stale approval/session/turn identity. + Interpreter state persists across cells; RPC authority must not. Each cell + installs a fresh authority — captured from the CALLING thread at cell + start, exactly what ``propagate_context_to_thread`` would capture for a + per-call RPC thread — and retires it when the cell settles, so a late tool + call (a background thread the cell left behind, a raced client write) is + refused instead of running under a stale approval/session/turn identity. """ def __init__(self, task_id: str): @@ -190,20 +170,17 @@ class CellAuthority: self.task_id = task_id self.ctx = contextvars.copy_context() self.active = True - self._approval_cb = None - self._sudo_cb = None - self._callback_setters = None + self._api = None # (get_approval, get_sudo, set_approval, set_sudo) + self._callbacks = (None, None) try: from tools.thread_context import _callback_api - get_approval, get_sudo, set_approval, set_sudo = _callback_api() - self._approval_cb = get_approval() - self._sudo_cb = get_sudo() - self._callback_setters = (set_approval, set_sudo) + self._api = _callback_api() + self._callbacks = (self._api[0](), self._api[1]()) except Exception: # Fail-closed, mirroring propagate_context_to_thread: with no # callbacks installed, dangerous approvals deny. - self._callback_setters = None + self._api = None def retire(self) -> None: self.active = False @@ -223,21 +200,18 @@ class CellAuthority: from model_tools import handle_function_call previous = None - if self._callback_setters is not None: + if self._api is not None: + get_approval, get_sudo, set_approval, set_sudo = self._api try: - from tools.thread_context import _callback_api - - get_approval, get_sudo, set_approval, set_sudo = _callback_api() previous = (get_approval(), get_sudo()) - set_approval(self._approval_cb) - set_sudo(self._sudo_cb) + set_approval(self._callbacks[0]) + set_sudo(self._callbacks[1]) except Exception: previous = None try: return handle_function_call(tool_name, tool_args, task_id=self.task_id) finally: - if previous is not None and self._callback_setters is not None: - set_approval, set_sudo = self._callback_setters + if previous is not None: try: set_approval(previous[0]) set_sudo(previous[1]) @@ -245,6 +219,25 @@ class CellAuthority: pass +class _BoundedBuffer: + """Byte chunks capped at a total size; ``drain`` returns text and resets.""" + + def __init__(self): + self.chunks: List[bytes] = [] + self.total = 0 + + def append(self, data: bytes, cap: int) -> None: + if self.total >= cap: + return + keep = data[: cap - self.total] + self.chunks.append(keep) + self.total += len(keep) + + def drain(self) -> str: + chunks, self.chunks, self.total = self.chunks, [], 0 + return b"".join(chunks).decode("utf-8", errors="replace") + + class SessionKernel: """One live kernel process plus its RPC server and reader threads.""" @@ -253,19 +246,14 @@ class SessionKernel: self.owner: str = key[0] self.lock = threading.Lock() self.proc: Optional[subprocess.Popen] = None - self.tmpdir: str = "" + self.tmpdir = self.rpc_token = self.sentinel = "" self.sock_path: Optional[str] = None self.server_sock: Optional[socket.socket] = None self.stop_event = threading.Event() - self.rpc_token: str = "" - self.sentinel: str = "" self.tool_call_log: List = [] self.tool_call_counter: List[int] = [0] self.response_q: "queue.Queue[dict]" = queue.Queue() - self.raw_chunks: List[bytes] = [] - self.raw_bytes = [0] - self.stderr_chunks: List[bytes] = [] - self.stderr_bytes = [0] + self.raw, self.stderr = _BoundedBuffer(), _BoundedBuffer() self.execution_count = 0 self.last_used: float = time.monotonic() self.cell_authority: Optional[CellAuthority] = None @@ -274,16 +262,43 @@ class SessionKernel: return self.proc is not None and self.proc.poll() is None -_KERNELS: Dict[Tuple, SessionKernel] = {} -_KERNELS_LOCK = threading.Lock() +class KernelRegistry: + """Key -> kernel map plus its lock (shared with the remote registry). + + Kernels are popped under the lock and torn down outside it — teardown + may block on the child process or the remote transport. + """ + + def __init__(self, teardown: Callable[[Any], None]): + self.kernels: Dict[Tuple, Any] = {} + self.lock = threading.Lock() + self._teardown = teardown + + def pop_all(self, owner: Optional[str] = None) -> list: + """Pop every kernel, or every kernel one owner (key[0]) holds.""" + with self.lock: + doomed = [key for key in self.kernels if owner is None or key[0] == owner] + return [self.kernels.pop(key) for key in doomed] + + def shutdown(self, owner: Optional[str] = None) -> None: + for kernel in self.pop_all(owner): + self._teardown(kernel) + + def discard(self, key: Tuple, kernel: Any) -> None: + """Drop one registry entry and tear the kernel down.""" + with self.lock: + self.kernels.pop(key, None) + self._teardown(kernel) + + +_REGISTRY = KernelRegistry(lambda kernel: _teardown(kernel)) +_KERNELS: Dict[Tuple, SessionKernel] = _REGISTRY.kernels +_KERNELS_LOCK = _REGISTRY.lock # Bounded lifecycle defaults (config: code_execution.max_session_kernels / # code_execution.kernel_idle_timeout). A long-lived gateway must never -# accumulate one live child per finished conversation — the ownership, -# disposal, idle-reap, and cap shape here deliberately carries forward the -# lifecycle invariants of the earlier session-persistent implementation in -# hermes-agent#88637 by @z80dev (stable owner id, owner-teardown disposal, -# idle reaping, max-live bound). +# accumulate one live child per finished conversation: stable owner id, +# owner-teardown disposal, idle reaping, max-live bound. DEFAULT_MAX_SESSION_KERNELS = 4 DEFAULT_KERNEL_IDLE_TIMEOUT = 1800 @@ -292,33 +307,31 @@ def _lifecycle_limits() -> Tuple[int, int]: from tools.code_execution_tool import _load_config config = _load_config() - try: - cap = int(config.get("max_session_kernels", DEFAULT_MAX_SESSION_KERNELS)) - except (TypeError, ValueError): - cap = DEFAULT_MAX_SESSION_KERNELS - try: - idle = int(config.get("kernel_idle_timeout", DEFAULT_KERNEL_IDLE_TIMEOUT)) - except (TypeError, ValueError): - idle = DEFAULT_KERNEL_IDLE_TIMEOUT - return max(1, cap), max(1, idle) + + def limit(key: str, default: int) -> int: + try: + return max(1, int(config.get(key, default))) + except (TypeError, ValueError): + return default + + return (limit("max_session_kernels", DEFAULT_MAX_SESSION_KERNELS), + limit("kernel_idle_timeout", DEFAULT_KERNEL_IDLE_TIMEOUT)) def _resolve_owner(task_id: str) -> str: """The stable identity a session kernel belongs to. - The conversation's approval session key — context-propagated, stable - across turns of one conversation, and distinct per session. ``run_agent`` - mints a fresh task id per top-level turn, so a task-keyed kernel would - neither survive the next user turn nor ever be torn down with anything; - the task id is only the last-resort owner for embeds and tests that run - with no session context at all. + The conversation's approval session key: context-propagated, stable across + turns, distinct per session. ``run_agent`` mints a fresh task id per + top-level turn, so a task-keyed kernel would neither survive the next turn + nor ever be torn down with anything; the task id is only the last-resort + owner for embeds and tests with no session context. - Delegated children run in a copy of the parent's context and therefore - INHERIT the parent's approval session key — without the qualifier below, - a child's execute_code would attach to the parent's kernel and read its - in-memory state (verified live: parent-planted globals were readable - from a delegated_child_context, both directions). Children get their own - kernels, keyed by their delegation session id. + Delegated children run in a copy of the parent's context and INHERIT its + approval session key — without the ``::child::`` qualifier a child's + execute_code would attach to the parent's kernel and read its in-memory + state (verified live, both directions). Children get their own kernels, + keyed by their delegation session id. """ try: from tools.approval import get_current_session_key @@ -326,9 +339,7 @@ def _resolve_owner(task_id: str) -> str: session_key = get_current_session_key(default="") except Exception: session_key = "" - owner = session_key or (task_id or "") - try: from agent.delegation_context import is_delegated_child_context @@ -339,7 +350,6 @@ def _resolve_owner(task_id: str) -> str: owner = f"{owner}::child::{child_id}" except Exception: pass - return owner @@ -350,11 +360,7 @@ def _kernel_key(owner: str, mode: str, child_python: str, child_cwd: str, def shutdown_all_kernels() -> None: """Kill every session kernel. Registered via atexit; also used by tests.""" - with _KERNELS_LOCK: - kernels = list(_KERNELS.values()) - _KERNELS.clear() - for kernel in kernels: - _teardown(kernel) + _REGISTRY.shutdown() def shutdown_kernels_for_owner(owner: str) -> None: @@ -362,40 +368,25 @@ def shutdown_kernels_for_owner(owner: str) -> None: Wired into ``tools.approval.clear_session`` so kernels die at the same session boundary that clears the owner's approval and yolo state - (the /new + session-close disposal shape from hermes-agent#88637). + (/new and session close). """ - if not owner: - return - with _KERNELS_LOCK: - doomed = [key for key in _KERNELS if key[0] == owner] - kernels = [_KERNELS.pop(key) for key in doomed] - for kernel in kernels: - _teardown(kernel) + if owner: + _REGISTRY.shutdown(owner) def _reap_unlocked() -> List[SessionKernel]: """Pop idle-expired kernels; caller tears them down outside the lock.""" _, idle_timeout = _lifecycle_limits() now = time.monotonic() - doomed = [ - key - for key, kernel in _KERNELS.items() - if now - kernel.last_used > idle_timeout - ] + doomed = [key for key, kernel in _KERNELS.items() if now - kernel.last_used > idle_timeout] return [_KERNELS.pop(key) for key in doomed] def _evict_over_cap_unlocked(keep: Tuple) -> List[SessionKernel]: """Pop least-recently-used kernels beyond the process-wide cap.""" cap, _ = _lifecycle_limits() - if len(_KERNELS) <= cap: - return [] - by_age = sorted( - (key for key in _KERNELS if key != keep), - key=lambda key: _KERNELS[key].last_used, - ) - doomed = by_age[: len(_KERNELS) - cap] - return [_KERNELS.pop(key) for key in doomed] + by_age = sorted((key for key in _KERNELS if key != keep), key=lambda key: _KERNELS[key].last_used) + return [_KERNELS.pop(key) for key in by_age[: max(0, len(_KERNELS) - cap)]] atexit.register(shutdown_all_kernels) @@ -407,17 +398,14 @@ def _teardown(kernel: SessionKernel) -> None: from tools.code_execution_tool import _kill_process_group _kill_process_group(kernel.proc, escalate=True) - if kernel.server_sock is not None: - try: - kernel.server_sock.close() - except OSError: - pass - kernel.server_sock = None - if kernel.sock_path: - try: + sock, kernel.server_sock = kernel.server_sock, None + try: + if sock is not None: + sock.close() + if kernel.sock_path: os.unlink(kernel.sock_path) - except OSError: - pass + except OSError: + pass if kernel.tmpdir: import shutil @@ -428,16 +416,13 @@ def _rpc_forever(kernel: SessionKernel, max_tool_calls: int, sandbox_tools: frozenset) -> None: """Serve tool RPC for the kernel's whole life. - ``_rpc_server_loop`` serves one connection and returns on disconnect or - on its 300s idle timeout; a kernel legitimately sits idle longer than - that between cells, so re-accept until the kernel is torn down. The - client stub reconnects on its side (HERMES_RPC_PERSISTENT). + ``_rpc_server_loop`` serves one connection and returns on disconnect or its + 300s idle timeout; a kernel legitimately idles longer between cells, so + re-accept until teardown (the client stub reconnects: HERMES_RPC_PERSISTENT). - The serving thread carries NO frozen authority of its own: every - dispatch is routed through the CURRENT cell's ``CellAuthority``, so a - later cell's tool calls run under that cell's approval/session/turn - context instead of whatever the first cell happened to capture. - Interpreter state persists; RPC authority does not. + The serving thread carries NO frozen authority: every dispatch routes + through the CURRENT cell's ``CellAuthority``, so a later cell's tool calls + run under that cell's context, not whatever the first cell captured. """ from tools.code_execution_tool import _rpc_server_loop, tool_error @@ -451,25 +436,9 @@ def _rpc_forever(kernel: SessionKernel, max_tool_calls: int, return authority.dispatch(tool_name, tool_args) while not kernel.stop_event.is_set(): - _rpc_server_loop( - kernel.server_sock, - "", - kernel.tool_call_log, - kernel.tool_call_counter, - max_tool_calls, - sandbox_tools, - kernel.stop_event, - kernel.rpc_token, - dispatch=_dispatch, - ) - - -def _append_bounded(chunks: List[bytes], total: List[int], data: bytes, cap: int) -> None: - if total[0] >= cap: - return - keep = data[: cap - total[0]] - chunks.append(keep) - total[0] += len(keep) + _rpc_server_loop(kernel.server_sock, "", kernel.tool_call_log, kernel.tool_call_counter, + max_tool_calls, sandbox_tools, kernel.stop_event, kernel.rpc_token, + dispatch=_dispatch) def _stdout_reader(kernel: SessionKernel) -> None: @@ -479,6 +448,10 @@ def _stdout_reader(kernel: SessionKernel) -> None: assert kernel.proc is not None and kernel.proc.stdout is not None stream = kernel.proc.stdout marker = ("\n" + kernel.sentinel + " ").encode("utf-8") + + def raw(data: bytes) -> None: + kernel.raw.append(data, MAX_STDOUT_BYTES) + buf = b"" while True: # read1: return as soon as any bytes arrive. A plain read(n) on a @@ -487,7 +460,7 @@ def _stdout_reader(kernel: SessionKernel) -> None: chunk = stream.read1(4096) if not chunk: if buf: - _append_bounded(kernel.raw_chunks, kernel.raw_bytes, buf, MAX_STDOUT_BYTES) + raw(buf) kernel.response_q.put({"status": "kernel-eof"}) return buf += chunk @@ -498,11 +471,11 @@ def _stdout_reader(kernel: SessionKernel) -> None: # across reads; everything before it is raw output. spill = buf[: -len(marker)] if len(buf) > len(marker) else b"" if spill: - _append_bounded(kernel.raw_chunks, kernel.raw_bytes, spill, MAX_STDOUT_BYTES) + raw(spill) buf = buf[len(spill):] break if index: - _append_bounded(kernel.raw_chunks, kernel.raw_bytes, buf[:index], MAX_STDOUT_BYTES) + raw(buf[:index]) rest = buf[index + len(marker):] newline = rest.find(b"\n") if newline < 0: @@ -513,18 +486,16 @@ def _stdout_reader(kernel: SessionKernel) -> None: except ValueError: # Not a real frame header (user output that happens to # contain the marker bytes); treat the marker as raw. - _append_bounded(kernel.raw_chunks, kernel.raw_bytes, marker, MAX_STDOUT_BYTES) + raw(marker) buf = rest continue body = rest[newline + 1:] - missing = length - len(body) - while missing > 0: - more = stream.read1(missing) + while len(body) < length: + more = stream.read1(length - len(body)) if not more: kernel.response_q.put({"status": "kernel-eof"}) return body += more - missing -= len(more) try: kernel.response_q.put(json.loads(body[:length].decode("utf-8", errors="replace"))) except ValueError: @@ -540,22 +511,11 @@ def _stderr_reader(kernel: SessionKernel) -> None: chunk = kernel.proc.stderr.read1(4096) if not chunk: return - _append_bounded(kernel.stderr_chunks, kernel.stderr_bytes, chunk, MAX_STDERR_BYTES) + kernel.stderr.append(chunk, MAX_STDERR_BYTES) -def _spawn(kernel: SessionKernel, *, task_id: str, child_python: str, - child_cwd: str, sandbox_tools: frozenset, max_tool_calls: int) -> None: - from tools.code_execution_tool import ( - _build_child_env, - generate_hermes_tools_module, - ) - - kernel.tmpdir = tempfile.mkdtemp(prefix="hermes_kernel_") - _sock_tmpdir = "/tmp" if sys.platform == "darwin" else tempfile.gettempdir() - - kernel.rpc_token = secrets.token_urlsafe(32) - kernel.sentinel = "@@HERMES-KERNEL-" + secrets.token_urlsafe(16) + "@@" - +def _bind_rpc_socket(kernel: SessionKernel) -> str: + """Bind the tool-RPC listener: loopback TCP on Windows, 0600 UDS elsewhere.""" if _IS_WINDOWS: kernel.sock_path = None server_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) @@ -563,27 +523,33 @@ def _spawn(kernel: SessionKernel, *, task_id: str, child_python: str, host, port = server_sock.getsockname()[:2] rpc_endpoint = f"tcp://{host}:{port}" else: - kernel.sock_path = os.path.join(_sock_tmpdir, f"hermes_rpc_{uuid.uuid4().hex}.sock") + sock_tmpdir = "/tmp" if sys.platform == "darwin" else tempfile.gettempdir() + kernel.sock_path = os.path.join(sock_tmpdir, f"hermes_rpc_{uuid.uuid4().hex}.sock") server_sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) server_sock.bind(kernel.sock_path) os.chmod(kernel.sock_path, 0o600) rpc_endpoint = kernel.sock_path server_sock.listen(1) kernel.server_sock = server_sock + return rpc_endpoint - tools_src = generate_hermes_tools_module(list(sandbox_tools)) - with open(os.path.join(kernel.tmpdir, "hermes_tools.py"), "w", encoding="utf-8") as f: - f.write(tools_src) - runner_path = os.path.join(kernel.tmpdir, "hermes_kernel_runner.py") - with open(runner_path, "w", encoding="utf-8") as f: - f.write(KERNEL_RUNNER_SOURCE) - child_env = _build_child_env( - rpc_endpoint=rpc_endpoint, - rpc_token=kernel.rpc_token, - tmpdir=kernel.tmpdir, - child_python=child_python, - ) +def _spawn(kernel: SessionKernel, *, child_python: str, child_cwd: str, + sandbox_tools: frozenset, max_tool_calls: int) -> None: + from tools.code_execution_tool import _build_child_env, generate_hermes_tools_module + + kernel.tmpdir = tempfile.mkdtemp(prefix="hermes_kernel_") + kernel.rpc_token = secrets.token_urlsafe(32) + kernel.sentinel = "@@HERMES-KERNEL-" + secrets.token_urlsafe(16) + "@@" + rpc_endpoint = _bind_rpc_socket(kernel) + + for name, src in (("hermes_tools.py", generate_hermes_tools_module(list(sandbox_tools))), + ("hermes_kernel_runner.py", KERNEL_RUNNER_SOURCE)): + with open(os.path.join(kernel.tmpdir, name), "w", encoding="utf-8") as f: + f.write(src) + + child_env = _build_child_env(rpc_endpoint=rpc_endpoint, rpc_token=kernel.rpc_token, + tmpdir=kernel.tmpdir, child_python=child_python) child_env["HERMES_KERNEL_SENTINEL"] = kernel.sentinel # Cells clip stdout to the inline cap; the full text spills to the # kernel's own tmpdir so the agent can read_file the middle instead of @@ -594,14 +560,11 @@ def _spawn(kernel: SessionKernel, *, task_id: str, child_python: str, child_env["HERMES_RPC_PERSISTENT"] = "1" kernel.proc = subprocess.Popen( - [child_python, runner_path], + [child_python, os.path.join(kernel.tmpdir, "hermes_kernel_runner.py")], # Strict mode resolves an empty cwd: the kernel's own staging dir # then plays the per-call tmpdir's role. - cwd=child_cwd or kernel.tmpdir, - env=child_env, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - stdin=subprocess.PIPE, + cwd=child_cwd or kernel.tmpdir, env=child_env, + stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE, start_new_session=True, creationflags=subprocess.CREATE_NO_WINDOW if _IS_WINDOWS else 0, ) @@ -609,66 +572,26 @@ def _spawn(kernel: SessionKernel, *, task_id: str, child_python: str, # Deliberately NOT propagate_context_to_thread: that would freeze the # spawning cell's context/callbacks into the server thread for the # kernel's whole life. Authority is rebound per cell via CellAuthority. - threading.Thread( - target=_rpc_forever, - args=(kernel, max_tool_calls, sandbox_tools), - daemon=True, - ).start() - threading.Thread(target=_stdout_reader, args=(kernel,), daemon=True).start() - threading.Thread(target=_stderr_reader, args=(kernel,), daemon=True).start() + for target, args in ((_rpc_forever, (kernel, max_tool_calls, sandbox_tools)), + (_stdout_reader, (kernel,)), (_stderr_reader, (kernel,))): + threading.Thread(target=target, args=args, daemon=True).start() -def _drain_raw(kernel: SessionKernel) -> str: - chunks, kernel.raw_chunks, kernel.raw_bytes = kernel.raw_chunks, [], [0] - return b"".join(chunks).decode("utf-8", errors="replace") +def _acquire_kernel(key: Tuple, reset: bool) -> Tuple[SessionKernel, bool]: + """Look up or register the kernel for *key*; returns (kernel, state_reset). - -def _drain_stderr(kernel: SessionKernel) -> str: - chunks, kernel.stderr_chunks, kernel.stderr_bytes = kernel.stderr_chunks, [], [0] - return b"".join(chunks).decode("utf-8", errors="replace") - - -def execute_in_session_kernel( - code: str, - *, - task_id: str, - mode: str, - child_python: str, - child_cwd: str, - sandbox_tools: frozenset, - timeout: int, - max_tool_calls: int, - reset: bool, - is_interrupted, -) -> str: - """Run one cell in the (owner, mode, python, cwd, tools) session kernel. - - The owner is the conversation's session key (``_resolve_owner``), not - the per-turn task id, so state genuinely survives across user turns of - one conversation and dies with the session. Every entry also sweeps - idle-expired kernels and enforces the process-wide cap, so a long-lived - host stays bounded even for owners that never toggle or reset. + Every entry also sweeps idle-expired kernels and enforces the + process-wide cap, so a long-lived host stays bounded even for owners + that never toggle or reset. """ - from tools.code_execution_tool import ( - _sandbox_failure_hint, - _truncate_stdout_text, - ) - from agent.redact import redact_sensitive_text - from tools.ansi_strip import strip_ansi - - owner = _resolve_owner(task_id) - key = _kernel_key(owner, mode, child_python, child_cwd, sandbox_tools) - exec_start = time.monotonic() - state_reset = False - with _KERNELS_LOCK: expired = _reap_unlocked() kernel = _KERNELS.get(key) - if kernel is not None and (reset or not kernel.alive()): + state_reset = kernel is not None and (reset or not kernel.alive()) + if state_reset: _KERNELS.pop(key, None) expired.append(kernel) kernel = None - state_reset = True if kernel is None: kernel = SessionKernel(key) _KERNELS[key] = kernel @@ -676,6 +599,119 @@ def execute_in_session_kernel( expired.extend(_evict_over_cap_unlocked(keep=key)) for doomed in expired: _teardown(doomed) + return kernel, state_reset + + +def _await_cell(kernel: SessionKernel, timeout: int, is_interrupted) -> Tuple[str, Dict[str, Any]]: + """Wait for the cell's reply; returns (host status, payload).""" + deadline = time.monotonic() + timeout if timeout else None + while True: + if is_interrupted(): + return "interrupted", {} + if deadline is not None and time.monotonic() > deadline: + return "timeout", {} + try: + payload = kernel.response_q.get(timeout=0.05) + except queue.Empty: + continue + if payload.get("status") in ("kernel-eof", "protocol-error"): + return "error", payload + return "success", payload + + +def _with_stderr(stdout_text: str, stderr_text: str) -> str: + return stdout_text + "\n--- stderr ---\n" + stderr_text + + +def _cell_result(kernel: SessionKernel, key: Tuple, status: str, payload: Dict[str, Any], *, + timeout: int, sandbox_tools: frozenset, reused: bool, + state_reset: bool, exec_start: float) -> Dict[str, Any]: + """Assemble the tool result for one settled cell (disposing the kernel where the contract says so).""" + from tools.code_execution_tool import _sandbox_failure_hint, _truncate_stdout_text + from agent.redact import redact_sensitive_text + from tools.ansi_strip import strip_ansi + + def clean(text: str) -> str: + return redact_sensitive_text(strip_ansi(text), code_file=True) + + if status in ("timeout", "interrupted"): + # No safe way to interrupt one cell in place: kill the kernel, + # report the state loss, let the next call respawn. + _REGISTRY.discard(key, kernel) + + duration = round(time.monotonic() - exec_start, 2) + kernel.execution_count = int(payload.get("execution_count", kernel.execution_count + 1)) + + stderr_raw = kernel.stderr.drain() + stdout_text = clean(str(payload.get("stdout", "")) + kernel.raw.drain()) + cell_stderr = clean(str(payload.get("stderr", "")) + stderr_raw) + stdout_text, stdout_metadata = _truncate_stdout_text(stdout_text) + + cell_status = payload.get("status", "") + result: Dict[str, Any] = { + "status": status, "output": stdout_text, "exit_code": 0, + "tool_calls_made": kernel.tool_call_counter[0], "duration_seconds": duration, + "kernel": {"mode": "session", "reused": reused, + "execution_count": kernel.execution_count, "state_reset": state_reset}, + } + result.update(stdout_metadata) + + # Cell-side spill (runner clipped before replying): surface the full-output + # path with the same read_file recipe as the host-side spill. + cell_spill = str(payload.get("stdout_spill_path", "") or "") + if cell_spill and payload.get("stdout_clipped"): + result["stdout_spill_path"] = cell_spill + result["warning"] = ( + f"Cell stdout exceeded the inline cap; head shown. FULL output saved to {cell_spill} " + f'— page it with read_file(path="{cell_spill}", offset=...) instead of re-running. ' + "(Kernel state persists: printing a narrower slice next call is often cheaper.)" + ) + + if status == "timeout": + message = (f"Cell timed out after {timeout}s; the session kernel was killed and its " + "state was lost. The next execute_code call starts a fresh kernel.") + result.update(exit_code=-1, error=message, + output=(stdout_text + "\n\n⏰ " + message) if stdout_text else ("⏰ " + message)) + elif status == "interrupted": + from tools.code_execution_tool import _format_interrupted_output + + result.update(exit_code=-1, output=_format_interrupted_output(stdout_text), + error="Interrupted; the session kernel was killed and its state was lost.") + elif cell_status == "error": + trace = clean(str(payload.get("traceback", ""))) + result.update(status="error", exit_code=1, error=trace or "Cell raised an exception.", + output=_with_stderr(stdout_text, cell_stderr + trace) if (cell_stderr or trace) else stdout_text) + hint = _sandbox_failure_hint(trace, enabled_tools=sandbox_tools) + if hint: + result["hint"] = hint + elif cell_status == "exit": + # The cell called sys.exit(): honor it as end-of-kernel. + _REGISTRY.discard(key, kernel) + result["kernel"]["ended"] = True + if cell_stderr: + result["output"] = _with_stderr(stdout_text, cell_stderr) + elif status == "error": + _REGISTRY.discard(key, kernel) + result.update(exit_code=-1, error="The session kernel died while running the cell" + + (": " + stderr_raw.strip() if stderr_raw.strip() else ".")) + elif cell_stderr: + result["output"] = _with_stderr(stdout_text, cell_stderr) + return result + + +def execute_in_session_kernel( + code: str, *, task_id: str, mode: str, child_python: str, child_cwd: str, + sandbox_tools: frozenset, timeout: int, max_tool_calls: int, reset: bool, is_interrupted, +) -> str: + """Run one cell in the (owner, mode, python, cwd, tools) session kernel. + + The owner is the conversation's session key (``_resolve_owner``), not + the per-turn task id, so state genuinely survives across user turns of + one conversation and dies with the session. + """ + key = _kernel_key(_resolve_owner(task_id), mode, child_python, child_cwd, sandbox_tools) + exec_start = time.monotonic() + kernel, state_reset = _acquire_kernel(key, reset) reused = kernel.proc is not None # Captured on the calling thread BEFORE the cell runs — the same @@ -687,155 +723,34 @@ def execute_in_session_kernel( with kernel.lock: try: if kernel.proc is None: - _spawn( - kernel, - task_id=task_id, - child_python=child_python, - child_cwd=child_cwd, - sandbox_tools=sandbox_tools, - max_tool_calls=max_tool_calls, - ) + _spawn(kernel, child_python=child_python, child_cwd=child_cwd, + sandbox_tools=sandbox_tools, max_tool_calls=max_tool_calls) assert kernel.proc is not None and kernel.proc.stdin is not None # Per-cell tool budget: the RPC loop enforces counter < max, so a # fresh cell starts from zero without restarting the server. kernel.tool_call_counter[0] = 0 # Anything raw that leaked between cells belongs to no cell. - _drain_raw(kernel) - _drain_stderr(kernel) + kernel.raw.drain() + kernel.stderr.drain() kernel.cell_authority = authority request = json.dumps({"id": uuid.uuid4().hex, "code": code}) + "\n" kernel.proc.stdin.write(request.encode("utf-8")) kernel.proc.stdin.flush() - deadline = time.monotonic() + timeout if timeout else None - status = "success" - payload: Dict[str, Any] = {} - while True: - if is_interrupted(): - status = "interrupted" - break - if deadline is not None and time.monotonic() > deadline: - status = "timeout" - break - try: - payload = kernel.response_q.get(timeout=0.05) - except queue.Empty: - continue - if payload.get("status") in ("kernel-eof", "protocol-error"): - status = "error" - break - - if status in ("timeout", "interrupted"): - # No safe way to interrupt one cell in place: kill the kernel, - # report the state loss, let the next call respawn. - with _KERNELS_LOCK: - _KERNELS.pop(key, None) - _teardown(kernel) - - duration = round(time.monotonic() - exec_start, 2) - kernel.execution_count = int(payload.get("execution_count", kernel.execution_count + 1)) - - raw_text = _drain_raw(kernel) - stderr_raw = _drain_stderr(kernel) - stdout_text = str(payload.get("stdout", "")) - if raw_text: - stdout_text = stdout_text + raw_text - cell_stderr = str(payload.get("stderr", "")) - if stderr_raw: - cell_stderr = cell_stderr + stderr_raw - - stdout_text = redact_sensitive_text(strip_ansi(stdout_text), code_file=True) - cell_stderr = redact_sensitive_text(strip_ansi(cell_stderr), code_file=True) - stdout_text, stdout_metadata = _truncate_stdout_text(stdout_text) - - cell_status = payload.get("status", "") - result: Dict[str, Any] = { - "status": status, - "output": stdout_text, - "exit_code": 0, - "tool_calls_made": kernel.tool_call_counter[0], - "duration_seconds": duration, - "kernel": { - "mode": "session", - "reused": reused, - "execution_count": kernel.execution_count, - "state_reset": state_reset, - }, - } - result.update(stdout_metadata) - - # Cell-side spill (runner clipped before replying): surface the - # full-output path with the same read_file recipe as the - # host-side spill in _truncate_stdout_text. - cell_spill = str(payload.get("stdout_spill_path", "") or "") - if cell_spill and payload.get("stdout_clipped"): - result["stdout_spill_path"] = cell_spill - result["warning"] = ( - "Cell stdout exceeded the inline cap; head shown. FULL " - f"output saved to {cell_spill} — page it with " - f'read_file(path="{cell_spill}", offset=...) instead of ' - "re-running. (Kernel state persists: printing a narrower " - "slice next call is often cheaper.)" - ) - - if status == "timeout": - message = ( - f"Cell timed out after {timeout}s; the session kernel was " - "killed and its state was lost. The next execute_code call " - "starts a fresh kernel." - ) - result["exit_code"] = -1 - result["error"] = message - result["output"] = (stdout_text + "\n\n⏰ " + message) if stdout_text else ("⏰ " + message) - elif status == "interrupted": - from tools.code_execution_tool import _format_interrupted_output - - result["exit_code"] = -1 - result["output"] = _format_interrupted_output(stdout_text) - result["error"] = "Interrupted; the session kernel was killed and its state was lost." - elif cell_status == "error": - trace = redact_sensitive_text(strip_ansi(str(payload.get("traceback", ""))), code_file=True) - result["status"] = "error" - result["exit_code"] = 1 - result["error"] = trace or "Cell raised an exception." - joined = stdout_text - if cell_stderr or trace: - joined = joined + "\n--- stderr ---\n" + cell_stderr + trace - result["output"] = joined - hint = _sandbox_failure_hint(trace, enabled_tools=sandbox_tools) - if hint: - result["hint"] = hint - elif cell_status == "exit": - # The cell called sys.exit(): honor it as end-of-kernel. - with _KERNELS_LOCK: - _KERNELS.pop(key, None) - _teardown(kernel) - result["kernel"]["ended"] = True - if cell_stderr: - result["output"] = stdout_text + "\n--- stderr ---\n" + cell_stderr - elif status == "error": - result["exit_code"] = -1 - result["error"] = ( - "The session kernel died while running the cell" - + (": " + stderr_raw.strip() if stderr_raw.strip() else ".") - ) - with _KERNELS_LOCK: - _KERNELS.pop(key, None) - _teardown(kernel) - elif cell_stderr: - result["output"] = stdout_text + "\n--- stderr ---\n" + cell_stderr - + status, payload = _await_cell(kernel, timeout, is_interrupted) + result = _cell_result( + kernel, key, status, payload, + timeout=timeout, sandbox_tools=sandbox_tools, reused=reused, + state_reset=state_reset, exec_start=exec_start, + ) return json.dumps(result, ensure_ascii=False) except Exception as exc: # pragma: no cover - defensive parity with per-call logger.error("session kernel failed: %s: %s", type(exc).__name__, exc, exc_info=True) - with _KERNELS_LOCK: - _KERNELS.pop(key, None) - _teardown(kernel) + _REGISTRY.discard(key, kernel) return json.dumps({ - "status": "error", - "error": str(exc), + "status": "error", "error": str(exc), "tool_calls_made": kernel.tool_call_counter[0], "duration_seconds": round(time.monotonic() - exec_start, 2), }, ensure_ascii=False) diff --git a/tools/code_kernel_remote.py b/tools/code_kernel_remote.py index 3b3e9a2293..3af6cae732 100644 --- a/tools/code_kernel_remote.py +++ b/tools/code_kernel_remote.py @@ -1,37 +1,28 @@ """Session-persistent kernels for REMOTE terminal backends (docker/ssh/modal). -Closes the gap tracked in hermes-agent#96873: local execute_code holds a -persistent kernel child (tools/code_kernel.py); remote backends previously -re-shipped and re-ran a fresh script per call, losing all interpreter state. +Remote backends offer one primitive — ``env.execute(cmd)``, run-to-completion +— so the three things the local kernel gets from owning a child are rebuilt: -The remote transport offers exactly one primitive — ``env.execute(cmd)``, -run-to-completion — so the three things the local kernel gets from owning a -child process are rebuilt on top of it: - -1. **A process that outlives one env.execute():** the kernel runner is - started detached (``nohup ... &``) and its PID recorded; each later cell - first probes liveness with ``kill -0``. +1. **A process outliving one env.execute():** the runner starts detached + (``nohup ... &``) with its PID recorded; each cell first probes ``kill -0``. 2. **A conversation channel:** a file-based CELL protocol in the kernel dir (``cell_req_NNNNNN.json`` / ``cell_res_NNNNNN.json``), sibling to the - existing file-based TOOL-RPC protocol (req_/res_ files) which is reused - unchanged — the host-side ``_rpc_poll_loop`` is started per cell with the - calling thread's context, which is what gives per-cell tool authority. + file-based TOOL-RPC protocol (req_/res_) reused unchanged — the host-side + ``_rpc_poll_loop`` starts per cell with the calling thread's context, which + is what gives per-cell tool authority. 3. **Death detection:** a failed liveness probe (transport drop, container restart, OOM-killed runner) reads as *kernel died: state lost*; the next - call respawns fresh and says so — never a hung poll loop, because every - wait is bounded by the cell timeout. + call respawns and says so — never a hung poll, every wait is bounded by + the cell timeout. -Same invariants as local: owner = approval session key with the -``::child::{id}`` qualifier for delegated children (imported from -tools.code_kernel — one resolver, cannot drift), same generated tool stubs, -same output post-processing in the caller. ``reset=true`` kills and -respawns. Spawn failure fails OPEN to the per-call path with a note, so a -degraded remote host never blocks execution entirely. +Same invariants as local: owner = approval session key with the ``::child::`` +qualifier (one resolver in tools.code_kernel, cannot drift), same generated +tool stubs, same output post-processing in the caller. ``reset=true`` kills +and respawns. Spawn failure fails OPEN to the per-call path with a note. """ from __future__ import annotations import atexit -import base64 import json import logging import shlex @@ -39,14 +30,12 @@ import threading import time import uuid from dataclasses import dataclass, field -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Dict, Optional, Tuple + +from tools.code_kernel import RUNNER_CELL_SOURCE, KernelRegistry logger = logging.getLogger(__name__) -# One lock guards the registry; teardown runs outside it (mirrors code_kernel). -_REMOTE_KERNELS: Dict[Tuple, "RemoteKernel"] = {} -_REMOTE_KERNELS_LOCK = threading.Lock() - # How often the host polls the remote for a cell result file. Each poll is # one env.execute round-trip (typically 0.1-0.4s on ssh/docker), so this is # a floor, not a rate. @@ -68,17 +57,10 @@ import traceback KDIR = os.environ["HERMES_KERNEL_DIR"] CELLS = os.path.join(KDIR, "cells") -CAPTURE_LIMIT = {capture_limit} +_CAPTURE_LIMIT = {capture_limit} IDLE_EXIT_SECONDS = {idle_exit} -GLOBALS = {{"__name__": "__main__", "__builtins__": __builtins__}} - - -def _bounded(text): - if len(text) <= CAPTURE_LIMIT: - return text, False - return text[:CAPTURE_LIMIT], True - +{cell_source} def main(): execution_count = 0 @@ -104,36 +86,13 @@ def main(): os.remove(req_path) last_activity = time.time() execution_count += 1 - out, err = io.StringIO(), io.StringIO() - status = "ok" - trace = "" - try: - with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): - exec(compile(request["code"], "", "exec"), GLOBALS) - except SystemExit as exc: - status = "exit" - trace = "SystemExit: " + repr(exc.code) - except BaseException: - status = "error" - trace = traceback.format_exc() - stdout_text, stdout_clipped = _bounded(out.getvalue()) - stderr_text, stderr_clipped = _bounded(err.getvalue()) - payload = {{ - "id": request.get("id", ""), - "status": status, - "stdout": stdout_text, - "stderr": stderr_text, - "stdout_clipped": stdout_clipped, - "stderr_clipped": stderr_clipped, - "traceback": trace, - "execution_count": execution_count, - }} + payload, _ = run_cell(request, execution_count) res_name = name.replace("cell_req_", "cell_res_") tmp = os.path.join(CELLS, res_name + ".tmp") with open(tmp, "w", encoding="utf-8") as f: json.dump(payload, f, ensure_ascii=False) os.replace(tmp, os.path.join(CELLS, res_name)) - if status == "exit": + if payload["status"] == "exit": return @@ -157,6 +116,15 @@ class RemoteKernel: execution_count: int = 0 cell_seq: int = 0 + def sh(self, cmd: str, timeout: int = 15) -> str: + return _sh(self.env, cmd, timeout) + + +def _sh(env, cmd: str, timeout: int = 15) -> str: + """Run *cmd* on the remote from ``/`` and return its output text.""" + result = env.execute(cmd, cwd="/", timeout=timeout) + return (result.get("output", "") if isinstance(result, dict) else "") or "" + def _kernel_key(owner: str, env_type: str, task_env_id: str) -> Tuple: return (owner, "remote", env_type, task_env_id) @@ -165,90 +133,75 @@ def _kernel_key(owner: str, env_type: str, task_env_id: str) -> Tuple: def _is_alive(kernel: RemoteKernel) -> bool: """Bounded liveness probe: kill -0 through the transport. - Any transport failure counts as dead — the caller respawns. This is the - "death detection" leg: a dropped ssh connection and a dead runner are - indistinguishable from here, and both have the same correct answer. + Any transport failure counts as dead — the caller respawns. A dropped ssh + connection and a dead runner are indistinguishable from here, and both + have the same correct answer. """ try: - probe = kernel.env.execute( - f"kill -0 {shlex.quote(kernel.pid)} 2>/dev/null && echo ALIVE", - cwd="/", timeout=15, - ) - return "ALIVE" in (probe.get("output", "") or "") + return "ALIVE" in kernel.sh(f"kill -0 {shlex.quote(kernel.pid)} 2>/dev/null && echo ALIVE") except Exception: return False def _kill(kernel: RemoteKernel) -> None: """Best-effort kill of the runner and its subprocesses, then rm -rf.""" - try: - kernel.env.execute( - # Kill the runner's process group if the shell gave it one, - # falling back to the single PID. - f"pkill -TERM -P {shlex.quote(kernel.pid)} 2>/dev/null; " - f"kill {shlex.quote(kernel.pid)} 2>/dev/null; true", - cwd="/", timeout=15, - ) - except Exception: - logger.debug("remote kernel kill failed (transport?)", exc_info=True) - try: - kernel.env.execute( - f"rm -rf {shlex.quote(kernel.kernel_dir)}", cwd="/", timeout=15, - ) - except Exception: - logger.debug("remote kernel dir cleanup failed", exc_info=True) + q_pid = shlex.quote(kernel.pid) + steps = ( + # Kill the runner's children if the shell gave it a group, then the PID itself. + (f"pkill -TERM -P {q_pid} 2>/dev/null; kill {q_pid} 2>/dev/null; true", + "remote kernel kill failed (transport?)"), + (f"rm -rf {shlex.quote(kernel.kernel_dir)}", "remote kernel dir cleanup failed"), + ) + for cmd, failure in steps: + try: + kernel.sh(cmd) + except Exception: + logger.debug(failure, exc_info=True) + + +# Registry + lock shared-shape with code_kernel; teardown runs outside the lock. +_REGISTRY = KernelRegistry(lambda kernel: _kill(kernel)) +_REMOTE_KERNELS: Dict[Tuple, RemoteKernel] = _REGISTRY.kernels +_REMOTE_KERNELS_LOCK = _REGISTRY.lock def shutdown_all_remote_kernels() -> None: - with _REMOTE_KERNELS_LOCK: - kernels = list(_REMOTE_KERNELS.values()) - _REMOTE_KERNELS.clear() - for kernel in kernels: - _kill(kernel) + _REGISTRY.shutdown() def shutdown_remote_kernels_for_owner(owner: str) -> None: """Session-boundary disposal — wired to the same clear_session hook as local kernels, so /new and session close reap both kinds.""" - if not owner: - return - with _REMOTE_KERNELS_LOCK: - doomed = [k for k in _REMOTE_KERNELS if k[0] == owner] - kernels = [_REMOTE_KERNELS.pop(k) for k in doomed] - for kernel in kernels: - _kill(kernel) + if owner: + _REGISTRY.shutdown(owner) atexit.register(shutdown_all_remote_kernels) def _spawn_remote_kernel(env, env_type: str, owner: str, task_env_id: str, - sandbox_tools: frozenset, *, - idle_exit: int) -> Optional[RemoteKernel]: + sandbox_tools: frozenset, *, idle_exit: int) -> Optional[RemoteKernel]: """Start a detached kernel runner on the remote. None on failure.""" from tools.code_execution_tool import ( - MAX_STDOUT_BYTES, - _ship_file_to_remote, - _env_temp_dir, - generate_hermes_tools_module, + MAX_STDOUT_BYTES, _ship_file_to_remote, _env_temp_dir, generate_hermes_tools_module, ) import secrets as _secrets kernel_dir = f"{_env_temp_dir(env)}/hermes_rkernel_{uuid.uuid4().hex[:12]}" q_dir = shlex.quote(kernel_dir) - try: - env.execute(f"mkdir -p {q_dir}/cells {q_dir}/rpc", cwd="/", timeout=15) + + def sh(cmd: str, timeout: int = 15) -> str: + return _sh(env, cmd, timeout) + + def start() -> Optional[RemoteKernel]: + sh(f"mkdir -p {q_dir}/cells {q_dir}/rpc") rpc_token = _secrets.token_urlsafe(32) runner_src = REMOTE_KERNEL_RUNNER_SOURCE.format( - capture_limit=MAX_STDOUT_BYTES, - idle_exit=idle_exit, - ) + cell_source=RUNNER_CELL_SOURCE, capture_limit=MAX_STDOUT_BYTES, idle_exit=idle_exit) _ship_file_to_remote(env, f"{kernel_dir}/kernel_runner.py", runner_src) - tools_src = generate_hermes_tools_module( - list(sandbox_tools), transport="file", - ) - _ship_file_to_remote(env, f"{kernel_dir}/hermes_tools.py", tools_src) + _ship_file_to_remote(env, f"{kernel_dir}/hermes_tools.py", + generate_hermes_tools_module(list(sandbox_tools), transport="file")) env_prefix = ( f"HERMES_KERNEL_DIR={q_dir} " @@ -256,58 +209,104 @@ def _spawn_remote_kernel(env, env_type: str, owner: str, task_env_id: str, f"HERMES_RPC_TOKEN={shlex.quote(rpc_token)} " f"PYTHONDONTWRITEBYTECODE=1 PYTHONPATH={q_dir}" ) - started = env.execute( - f"cd {q_dir} && nohup env {env_prefix} python3 kernel_runner.py " - f"> {q_dir}/runner.log 2>&1 & echo PID:$!", - cwd="/", timeout=20, - ) - pid = "" - for line in (started.get("output", "") or "").splitlines(): - if line.strip().startswith("PID:"): - pid = line.strip()[4:].strip() - break + started = sh(f"cd {q_dir} && nohup env {env_prefix} python3 kernel_runner.py " + f"> {q_dir}/runner.log 2>&1 & echo PID:$!", timeout=20) + pid = next((line.strip()[4:].strip() for line in started.splitlines() + if line.strip().startswith("PID:")), "") if not pid.isdigit(): - logger.warning("remote kernel spawn returned no PID: %r", - started.get("output", "")) - env.execute(f"rm -rf {q_dir}", cwd="/", timeout=15) + logger.warning("remote kernel spawn returned no PID: %r", started) return None - - kernel = RemoteKernel( - env=env, env_type=env_type, kernel_dir=kernel_dir, - pid=pid, rpc_token=rpc_token, owner=owner, - ) + kernel = RemoteKernel(env=env, env_type=env_type, kernel_dir=kernel_dir, + pid=pid, rpc_token=rpc_token, owner=owner) if not _is_alive(kernel): # Died instantly (missing python3 was pre-checked by the caller, - # so this is unexpected) — surface the runner log at debug. + # so this is unexpected) — surface the runner log. try: - log = env.execute(f"cat {q_dir}/runner.log", cwd="/", timeout=10) logger.warning("remote kernel died at spawn: %s", - (log.get("output", "") or "")[:500]) + sh(f"cat {q_dir}/runner.log", timeout=10)[:500]) except Exception: pass - env.execute(f"rm -rf {q_dir}", cwd="/", timeout=15) return None return kernel + + kernel = None + try: + kernel = start() except Exception: logger.warning("remote kernel spawn failed", exc_info=True) + if kernel is None: try: - env.execute(f"rm -rf {q_dir}", cwd="/", timeout=15) + sh(f"rm -rf {q_dir}") except Exception: pass - return None + return kernel + + +def _acquire_remote_kernel(env, env_type: str, owner: str, task_env_id: str, + sandbox_tools: frozenset, *, reset: bool, + idle_exit: int) -> Tuple[Optional[RemoteKernel], bool, bool, bool]: + """Find/respawn the owner's kernel: (kernel|None, reused, state_reset, state_lost).""" + key = _kernel_key(owner, env_type, task_env_id) + state_lost = state_reset = False + + with _REMOTE_KERNELS_LOCK: + kernel = _REMOTE_KERNELS.get(key) + + if kernel is not None and reset: + _REGISTRY.discard(key, kernel) + kernel, state_reset = None, True + if kernel is not None and not _is_alive(kernel): + # Transport drop, container restart, self-reaped on idle, OOM — all + # the same answer: report the loss, respawn fresh (_kill is then only + # best-effort dir cleanup; the process is already gone). + _REGISTRY.discard(key, kernel) + kernel, state_lost = None, True + + reused = kernel is not None + if kernel is None: + kernel = _spawn_remote_kernel(env, env_type, owner, task_env_id, sandbox_tools, + idle_exit=idle_exit) + if kernel is not None: + with _REMOTE_KERNELS_LOCK: + _REMOTE_KERNELS[key] = kernel + return kernel, reused, state_reset, state_lost + + +def _run_remote_cell(kernel: RemoteKernel, code: str, timeout: int) -> Tuple[str, Dict[str, Any]]: + """Ship one cell request and poll for its result: (cell status, payload).""" + from tools.code_execution_tool import _ship_file_to_remote + + kernel.cell_seq += 1 + seq = f"{kernel.cell_seq:06d}" + q_cells = shlex.quote(f"{kernel.kernel_dir}/cells") + res_name = f"cell_res_{seq}.json" + request = json.dumps({"id": seq, "code": code}, ensure_ascii=False) + _ship_file_to_remote(kernel.env, f"{kernel.kernel_dir}/cells/cell_req_{seq}.json.tmp", request) + kernel.sh(f"mv {q_cells}/cell_req_{seq}.json.tmp {q_cells}/cell_req_{seq}.json", timeout=10) + + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + try: + body = kernel.sh(f"cat {q_cells}/{shlex.quote(res_name)} 2>/dev/null", timeout=20).strip() + except Exception: + # One flaky round-trip is not kernel death; liveness decides. + time.sleep(_CELL_POLL_INTERVAL) + continue + if body: + try: + payload = json.loads(body) + status = payload.get("status", "error") + except ValueError: + payload, status = {}, "protocol-error" + kernel.sh(f"rm -f {q_cells}/{shlex.quote(res_name)}", timeout=10) + return status, payload + time.sleep(_CELL_POLL_INTERVAL) + return "timeout", {} def execute_in_remote_kernel( - code: str, - *, - env, - env_type: str, - task_env_id: str, - sandbox_tools: frozenset, - timeout: int, - max_tool_calls: int, - reset: bool, - idle_exit: int = 1800, + code: str, *, env, env_type: str, task_env_id: str, sandbox_tools: frozenset, + timeout: int, max_tool_calls: int, reset: bool, idle_exit: int = 1800, ) -> Optional[Dict[str, Any]]: """Run one cell in the owner's remote kernel. @@ -317,182 +316,81 @@ def execute_in_remote_kernel( the ``kernel`` sub-dict, matching the local kernel's result shape. """ from tools.code_kernel import _resolve_owner - from tools.code_execution_tool import ( - _rpc_poll_loop, - _ship_file_to_remote, - ) + from tools.code_execution_tool import _rpc_poll_loop from tools.thread_context import propagate_context_to_thread owner = _resolve_owner(task_env_id) - key = _kernel_key(owner, env_type, task_env_id) - state_lost = False - state_reset = False - - with _REMOTE_KERNELS_LOCK: - kernel = _REMOTE_KERNELS.get(key) - - if kernel is not None and reset: - with _REMOTE_KERNELS_LOCK: - _REMOTE_KERNELS.pop(key, None) - _kill(kernel) - kernel = None - state_reset = True - - if kernel is not None and not _is_alive(kernel): - # Transport drop, container restart, self-reaped on idle, OOM — all - # the same answer: report the loss, respawn fresh. - with _REMOTE_KERNELS_LOCK: - _REMOTE_KERNELS.pop(key, None) - _kill(kernel) # best-effort dir cleanup; process is already gone - kernel = None - state_lost = True - - reused = kernel is not None + kernel, reused, state_reset, state_lost = _acquire_remote_kernel( + env, env_type, owner, task_env_id, sandbox_tools, reset=reset, idle_exit=idle_exit) if kernel is None: - kernel = _spawn_remote_kernel( - env, env_type, owner, task_env_id, sandbox_tools, - idle_exit=idle_exit, - ) - if kernel is None: - return None # fail open to per-call - with _REMOTE_KERNELS_LOCK: - _REMOTE_KERNELS[key] = kernel - + return None # fail open to per-call + key = _kernel_key(owner, env_type, task_env_id) kernel.last_used = time.monotonic() - kernel.cell_seq += 1 - seq = f"{kernel.cell_seq:06d}" - q_cells = shlex.quote(f"{kernel.kernel_dir}/cells") # Clean stale tool-RPC requests from a previous cell before arming this # cell's poll loop, so a background thread the last cell leaked cannot # smuggle a call into this cell's authority window. + q_rpc = shlex.quote(kernel.kernel_dir + '/rpc') try: - env.execute( - f"rm -f {shlex.quote(kernel.kernel_dir + '/rpc')}/req_* " - f"{shlex.quote(kernel.kernel_dir + '/rpc')}/res_*", - cwd="/", timeout=10, - ) + kernel.sh(f"rm -f {q_rpc}/req_* {q_rpc}/res_*", timeout=10) except Exception: pass tool_call_log: list = [] - tool_call_counter = [0] - stop_event = threading.Event() + tool_call_counter, stop_event = [0], threading.Event() # Per-cell RPC thread carrying THIS call's approval/session context — # the remote analogue of CellAuthority: authority lives exactly as long # as the cell's poll loop. rpc_thread = threading.Thread( target=propagate_context_to_thread(_rpc_poll_loop), - args=( - env, f"{kernel.kernel_dir}/rpc", task_env_id, - tool_call_log, tool_call_counter, max_tool_calls, - sandbox_tools, stop_event, kernel.rpc_token, - ), + args=(env, f"{kernel.kernel_dir}/rpc", task_env_id, tool_call_log, tool_call_counter, + max_tool_calls, sandbox_tools, stop_event, kernel.rpc_token), daemon=True, ) rpc_thread.start() - cell_status = "no-result" - cell_payload: Dict[str, Any] = {} + cell_status, cell_payload = "no-result", {} try: - request = json.dumps({"id": seq, "code": code}, ensure_ascii=False) - _ship_file_to_remote( - env, f"{kernel.kernel_dir}/cells/cell_req_{seq}.json.tmp", request, - ) - env.execute( - f"mv {q_cells}/cell_req_{seq}.json.tmp {q_cells}/cell_req_{seq}.json", - cwd="/", timeout=10, - ) - - deadline = time.monotonic() + timeout - res_name = f"cell_res_{seq}.json" - while time.monotonic() < deadline: - try: - probe = env.execute( - f"cat {q_cells}/{shlex.quote(res_name)} 2>/dev/null", - cwd="/", timeout=20, - ) - except Exception: - # One flaky round-trip is not kernel death; liveness decides. - time.sleep(_CELL_POLL_INTERVAL) - continue - body = (probe.get("output", "") or "").strip() - if body: - try: - cell_payload = json.loads(body) - cell_status = cell_payload.get("status", "error") - except ValueError: - cell_status = "protocol-error" - env.execute( - f"rm -f {q_cells}/{shlex.quote(res_name)}", - cwd="/", timeout=10, - ) - break - time.sleep(_CELL_POLL_INTERVAL) - else: - cell_status = "timeout" + cell_status, cell_payload = _run_remote_cell(kernel, code, timeout) finally: stop_event.set() rpc_thread.join(timeout=5) + kernel_info: Dict[str, Any] = {"reused": reused, "remote": True} + result: Dict[str, Any] = { + "status": "error", "stdout": cell_payload.get("stdout", ""), + "stderr": cell_payload.get("stderr", ""), "traceback": cell_payload.get("traceback", ""), + "tool_calls_made": tool_call_counter[0], "kernel": kernel_info, + } if cell_status in ("timeout", "protocol-error", "no-result"): # No safe way to interrupt one cell in place (same contract as # local): kill the kernel, report the loss, respawn next call. - with _REMOTE_KERNELS_LOCK: - _REMOTE_KERNELS.pop(key, None) - _kill(kernel) - return { - "status": "timeout" if cell_status == "timeout" else "error", - "stdout": "", - "stderr": "", - "traceback": "", - "tool_calls_made": tool_call_counter[0], - "kernel": { - "reused": reused, - "remote": True, - "ended": True, - "state_lost": True, - "note": ( - "Cell timed out; the remote session kernel was killed and " - "its state was lost. The next call starts a fresh kernel." - if cell_status == "timeout" else - "Remote kernel protocol failure; kernel killed, state lost." - ), - }, - } + _REGISTRY.discard(key, kernel) + if cell_status == "timeout": + result["status"] = "timeout" + note = ("Cell timed out; the remote session kernel was killed and " + "its state was lost. The next call starts a fresh kernel.") + else: + note = "Remote kernel protocol failure; kernel killed, state lost." + kernel_info.update(ended=True, state_lost=True, note=note) + return result if cell_status == "exit": - with _REMOTE_KERNELS_LOCK: - _REMOTE_KERNELS.pop(key, None) - _kill(kernel) - + _REGISTRY.discard(key, kernel) + kernel_info["ended"] = True kernel.execution_count = int(cell_payload.get("execution_count", 0) or 0) - - result: Dict[str, Any] = { - "status": "success" if cell_status in ("ok", "exit") else "error", - "stdout": cell_payload.get("stdout", ""), - "stderr": cell_payload.get("stderr", ""), - "traceback": cell_payload.get("traceback", ""), - "stdout_clipped": bool(cell_payload.get("stdout_clipped")), - "stderr_clipped": bool(cell_payload.get("stderr_clipped")), - "tool_calls_made": tool_call_counter[0], - "kernel": { - "reused": reused, - "remote": True, - "execution_count": kernel.execution_count, - }, - } - if cell_status == "exit": - result["kernel"]["ended"] = True + kernel_info["execution_count"] = kernel.execution_count + if cell_status in ("ok", "exit"): + result["status"] = "success" + result["stdout_clipped"] = bool(cell_payload.get("stdout_clipped")) + result["stderr_clipped"] = bool(cell_payload.get("stderr_clipped")) if state_reset: - result["kernel"]["state_reset"] = True + kernel_info["state_reset"] = True if state_lost: - result["kernel"]["state_lost"] = True - result["kernel"]["note"] = ( + kernel_info.update(state_lost=True, note=( "The previous remote kernel was gone (transport drop, container " "restart, or idle self-exit); state from earlier calls was lost " - "and a fresh kernel was started." - ) + "and a fresh kernel was started.")) if cell_status == "error" and result["traceback"]: result["error"] = result["traceback"].strip().splitlines()[-1] return result diff --git a/tools/file_state.py b/tools/file_state.py index 893a1f1a95..77dd0577e0 100644 --- a/tools/file_state.py +++ b/tools/file_state.py @@ -226,6 +226,7 @@ def _mtime_or_none(resolved: str) -> Optional[float]: def _fmt_ts(ts: float) -> str: + # Short wall-clock for warnings; avoids datetime formatting on the hot path. return time.strftime("%H:%M:%S", time.localtime(ts)) diff --git a/tools/hook_output_spill.py b/tools/hook_output_spill.py index c518281f03..14ca516c73 100644 --- a/tools/hook_output_spill.py +++ b/tools/hook_output_spill.py @@ -17,7 +17,9 @@ Config (``config.yaml``):: directory: null # default: /hook_outputs Invariants: unchanged input when disabled or under the cap; never raises — -an I/O failure still returns a bounded preview with an in-prompt notice. +an I/O failure still returns a bounded preview with an in-prompt notice. Spill +files are grouped per session so ``/new`` sessions don't pile into one directory. +Ported from openai/codex PR #21069. """ from __future__ import annotations @@ -39,11 +41,6 @@ DEFAULT_PREVIEW_TAIL = 500 DEFAULT_ENABLED = True -def _coerce_non_negative_int(value: Any, default: int) -> int: - """Like ``_coerce_positive_int`` but allows zero (e.g. empty tail).""" - return _coerce_int(value, default, 0) - - def get_spill_config() -> Dict[str, Any]: """Return resolved hook output-spill config. Never raises.""" section: Dict[str, Any] = {} @@ -68,12 +65,9 @@ def get_spill_config() -> Dict[str, Any]: return { "enabled": enabled, "max_chars": _coerce_positive_int(section.get("max_chars"), DEFAULT_MAX_CHARS), - "preview_head": _coerce_non_negative_int( - section.get("preview_head"), DEFAULT_PREVIEW_HEAD - ), - "preview_tail": _coerce_non_negative_int( - section.get("preview_tail"), DEFAULT_PREVIEW_TAIL - ), + # head/tail allow zero (empty tail), max_chars must be positive. + "preview_head": _coerce_int(section.get("preview_head"), DEFAULT_PREVIEW_HEAD, 0), + "preview_tail": _coerce_int(section.get("preview_tail"), DEFAULT_PREVIEW_TAIL, 0), "directory": directory, } diff --git a/tools/tool_output_limits.py b/tools/tool_output_limits.py index 88f6d9d109..d091a4de61 100644 --- a/tools/tool_output_limits.py +++ b/tools/tool_output_limits.py @@ -4,6 +4,12 @@ Centralises the caps previously hardcoded in ``terminal_tool`` (``max_bytes``) and ``file_operations`` (``max_lines`` / ``max_line_length``). Defaults equal the old constants, so behaviour is unchanged when the section is absent, and the reader never raises — any config error falls back to the defaults. +Ported from anomalyco/opencode PR #23770. Example:: + + tool_output: + max_bytes: 100000 # terminal output cap (chars) + max_lines: 5000 # read_file pagination + truncation cap + max_line_length: 2000 # per-line cap before '... [truncated]' """ from __future__ import annotations diff --git a/tools/tool_result_storage.py b/tools/tool_result_storage.py index f291aba75d..8dfa8f6afb 100644 --- a/tools/tool_result_storage.py +++ b/tools/tool_result_storage.py @@ -220,10 +220,7 @@ def _build_persisted_message( ) -> str: """Build the replacement block.""" size_kb = original_size / 1024 - if size_kb >= 1024: - size_str = f"{size_kb / 1024:.1f} MB" - else: - size_str = f"{size_kb:.1f} KB" + size_str = f"{size_kb / 1024:.1f} MB" if size_kb >= 1024 else f"{size_kb:.1f} KB" msg = f"{PERSISTED_OUTPUT_TAG}\n" msg += f"This tool result was too large ({original_size:,} characters, {size_str}).\n"