From 9a49b3c984fc6ad62bf14d155d3a87d24c66552c Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:45:26 -0700 Subject: [PATCH] fix(execute_code): subagent kernels survive the LRU cap for the child's lifetime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A delegated child's execute_code kernel was keyed correctly (::child::) but counted against the process-wide max_session_kernels LRU cap (default 4) like any other kernel. In a fan-out wider than the cap every child's first cell spawned a kernel and evicted the oldest sibling's, so the sibling's next cell started a fresh interpreter and NameError'd on state its own previous cell had set — while the tool schema promised "variables, imports, and loaded data survive across execute_code calls". Finished children's kernels also squatted the cap for kernel_idle_timeout (1800 s) after the child was gone. 48 NameErrors across 28 subagent lanes in the Sep 10-14 retrospective. A live child's kernel (local and remote) is now pinned: exempt from LRU eviction while the child runs, disposed by the delegation cleanup path (shutdown_kernels_for_delegated_child) as soon as the child finishes. Top-level sessions keep the existing cap and idle reaping unchanged. --- tests/tools/test_code_kernel.py | 51 +++++++++++++++++++ tools/code_kernel.py | 50 ++++++++++++++---- tools/code_kernel_remote.py | 18 +++++-- tools/delegate_tool_child_run.py | 5 ++ .../user-guide/features/code-execution.md | 4 +- 5 files changed, 113 insertions(+), 15 deletions(-) diff --git a/tests/tools/test_code_kernel.py b/tests/tools/test_code_kernel.py index 1e48192bd3..c783ec6633 100644 --- a/tests/tools/test_code_kernel.py +++ b/tests/tools/test_code_kernel.py @@ -283,6 +283,57 @@ class TestKernelOwnershipAndLifecycle(unittest.TestCase): ) self.assertIn("ISOLATED", peek.get("output", ""), peek) + def test_live_children_keep_their_kernels_past_the_lru_cap(self): + """A fan-out wider than max_session_kernels used to evict LIVE children's kernels (each + child's execute_code spawned a kernel, the cap reaped the oldest sibling's), so a child's + second call hit NameError on state its first call had set — 48 NameErrors across 28 lanes, + while the schema promised persistence. A live child's kernel is pinned for the child's life.""" + import contextvars + + from agent.delegation_context import delegated_child_context + + with _kernel_config(max_session_kernels=2): + contexts = [] + for index in range(5): + def _set(index=index): + with delegated_child_context(f"child-{index}"): + self._run_as("conv", f"v = {index}", task_id=f"child-{index}") + ctx = contextvars.copy_context() + ctx.run(_set) + contexts.append(ctx) + outcomes = {} + for index, ctx in enumerate(contexts): + def _read(index=index): + with delegated_child_context(f"child-{index}"): + outcomes[index] = self._run_as("conv", "print(v)", task_id=f"child-{index}") + ctx.run(_read) + for index, outcome in outcomes.items(): + self.assertEqual(outcome["status"], "success", outcome) + self.assertTrue(outcome["kernel"]["reused"], outcome) + self.assertIn(str(index), outcome["output"]) + + def test_finished_children_release_their_kernels(self): + """The pin is not a leak: when the child is torn down (the delegate_task cleanup path calls + ``shutdown_kernels_for_delegated_child``) its kernels die and stop counting.""" + from agent.delegation_context import delegated_child_context + from tools.code_kernel import shutdown_kernels_for_delegated_child + + with _kernel_config(): + with delegated_child_context("child-done"): + self._run_as("conv", "v = 1", task_id="child-done") + with delegated_child_context("child-live"): + self._run_as("conv", "v = 2", task_id="child-live") + doomed = [k for k in _KERNELS.values() if k.owner.endswith("::child::child-done")] + self.assertEqual(len(doomed), 1) + shutdown_kernels_for_delegated_child("child-done") + self.assertEqual([k for k in _KERNELS.values() if k.owner.endswith("::child::child-done")], []) + doomed[0].proc.wait(timeout=10) + self.assertFalse(doomed[0].alive()) + # The sibling's kernel is untouched. + with delegated_child_context("child-live"): + still = self._run_as("conv", "print(v)", task_id="child-live") + self.assertIn("2", still["output"]) + def test_session_clear_disposes_the_owners_kernels(self): from tools.approval import clear_session diff --git a/tools/code_kernel.py b/tools/code_kernel.py index d2307d2873..2f05a1d07d 100644 --- a/tools/code_kernel.py +++ b/tools/code_kernel.py @@ -317,6 +317,8 @@ class SessionKernel: # cell settles). Reaping/cap-eviction skip attached kernels: tearing one down mid-spawn # rmtree'd the staging dir under the spawner and killed live cells. self.attached: int = 0 + # Owned by a live delegate_task child: exempt from LRU eviction (the child's teardown disposes it). + self.pinned: bool = False self.response_q: "queue.Queue[dict]" = queue.Queue() self.raw, self.stderr = _BoundedBuffer(), _BoundedBuffer() self.execution_count, self.last_used = 0, time.monotonic() @@ -363,11 +365,13 @@ class KernelRegistry: self.kernels: Dict[Tuple, Any] = {} self.lock, self._teardown = threading.Lock(), teardown - def shutdown(self, owner: Optional[str] = None) -> None: - """Tear down every kernel, or every kernel one owner (key[0]) holds.""" + def shutdown(self, owner: Optional[str] = None, *, owner_matches: Optional[Callable[[str], bool]] = None) -> None: + """Tear down every kernel, every kernel one owner (key[0]) holds, or every kernel whose owner + satisfies ``owner_matches``.""" with self.lock: doomed = [self.kernels.pop(key) for key in list(self.kernels) - if owner is None or key[0] == owner] + if (owner is None and owner_matches is None) or key[0] == owner + or (owner_matches is not None and owner_matches(key[0]))] for kernel in doomed: self._teardown(kernel) @@ -402,6 +406,9 @@ def _lifecycle_limits() -> Tuple[int, int]: return limit("max_session_kernels", DEFAULT_MAX_SESSION_KERNELS), limit("kernel_idle_timeout", DEFAULT_KERNEL_IDLE_TIMEOUT) +_CHILD_OWNER_QUALIFIER = "::child::" + + 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, distinct per session). ``run_agent`` mints a fresh @@ -423,7 +430,7 @@ def _resolve_owner(task_id: str) -> str: if is_delegated_child_context(): from gateway.session_context import get_session_env child_id = get_session_env("HERMES_SESSION_ID", "") or (task_id or "") - owner = f"{owner}::child::{child_id}" + owner = f"{owner}{_CHILD_OWNER_QUALIFIER}{child_id}" except Exception: pass return owner @@ -444,6 +451,26 @@ def shutdown_kernels_for_owner(owner: str) -> None: _REGISTRY.shutdown(owner) +def delegated_child_owner_matcher(child_session_id: str) -> Callable[[str], bool]: + """Predicate for the kernels a delegate_task child owns (``_resolve_owner`` qualifies a child's + owner with its delegation session id). Shared with the remote registry.""" + suffix = f"{_CHILD_OWNER_QUALIFIER}{child_session_id}" + return lambda owner: owner.endswith(suffix) + + +def shutdown_kernels_for_delegated_child(child_session_id: str) -> None: + """Dispose a finished child's kernels (local and remote). A child's kernel lives exactly as long as the + child: pinned against LRU eviction while it runs, torn down here — otherwise finished children's + kernels squatted the process-wide cap for ``kernel_idle_timeout`` and evicted LIVE children's kernels, + which then lost their state mid-task with no signal but ``reused: false``.""" + if not child_session_id: + return + matcher = delegated_child_owner_matcher(child_session_id) + _REGISTRY.shutdown(owner_matches=matcher) + from tools.code_kernel_remote import shutdown_remote_kernels_where + shutdown_remote_kernels_where(matcher) + + atexit.register(shutdown_all_kernels) @@ -625,10 +652,12 @@ def _spawn(kernel: SessionKernel, *, child_python: str, child_cwd: str, threading.Thread(target=target, args=args, daemon=True).start() -def _acquire_kernel(key: Tuple, reset: bool) -> Tuple[SessionKernel, bool]: +def _acquire_kernel(key: Tuple, reset: bool, *, pinned: bool = False) -> Tuple[SessionKernel, bool]: """Look up or register the kernel for *key*; returns (kernel, state_reset). Every entry also sweeps idle-expired kernels and enforces the process-wide LRU cap (doomed kernels are popped - under the lock, torn down outside it), so a long-lived host stays bounded.""" + under the lock, torn down outside it), so a long-lived host stays bounded. ``pinned`` kernels + (live delegate_task children) are exempt from the cap: their lifetime is the child's, ended by + ``shutdown_kernels_for_delegated_child``, so the cap has nothing to bound for them.""" cap, idle_timeout = _lifecycle_limits() with _REGISTRY.lock: now = time.monotonic() @@ -644,11 +673,13 @@ def _acquire_kernel(key: Tuple, reset: bool) -> Tuple[SessionKernel, bool]: kernel = None if kernel is None: kernel = _KERNELS[key] = SessionKernel(key) + kernel.pinned = pinned kernel.last_used = time.monotonic() kernel.attached += 1 - by_age = sorted((k for k in _KERNELS if k != key and _KERNELS[k].attached == 0), + unpinned = [k for k in _KERNELS if not _KERNELS[k].pinned] + by_age = sorted((k for k in unpinned if k != key and _KERNELS[k].attached == 0), key=lambda k: _KERNELS[k].last_used) - expired.extend(_KERNELS.pop(k) for k in by_age[: max(0, len(_KERNELS) - cap)]) + expired.extend(_KERNELS.pop(k) for k in by_age[: max(0, len(unpinned) - cap)]) for doomed in expired: doomed.teardown() return kernel, state_reset @@ -748,7 +779,8 @@ def execute_in_session_kernel( session key (``_resolve_owner``), not the per-turn task id, so state survives across turns.""" key = (_resolve_owner(task_id) or "", mode, child_python, child_cwd, tuple(sorted(sandbox_tools))) exec_start = time.monotonic() - kernel, state_reset = _acquire_kernel(key, reset) + from agent.delegation_context import is_delegated_child_context + kernel, state_reset = _acquire_kernel(key, reset, pinned=is_delegated_child_context()) try: return _run_cell(kernel, key, code, task_id=task_id, child_python=child_python, child_cwd=child_cwd, sandbox_tools=sandbox_tools, timeout=timeout, max_tool_calls=max_tool_calls, diff --git a/tools/code_kernel_remote.py b/tools/code_kernel_remote.py index 85c8b36c9e..cc35a2304a 100644 --- a/tools/code_kernel_remote.py +++ b/tools/code_kernel_remote.py @@ -26,7 +26,7 @@ import threading import time import uuid from dataclasses import dataclass, field -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Callable, Dict, List, Optional, Tuple from tools.code_kernel import RUNNER_CELL_SOURCE, KernelRegistry @@ -118,6 +118,8 @@ class RemoteKernel: # kernels: killing one mid-cell tears the runner out from under a live # poll loop (same guard as tools.code_kernel, hermes-agent#101861). attached: int = 0 + # Owned by a live delegate_task child: exempt from LRU eviction (the child's teardown disposes it). + pinned: bool = False def sh(self, cmd: str, timeout: int = 15) -> str: return _sh(self.env, cmd, timeout) @@ -168,6 +170,11 @@ def shutdown_remote_kernels_for_owner(owner: str) -> None: _REGISTRY.shutdown(owner) +def shutdown_remote_kernels_where(owner_matches: Callable[[str], bool]) -> None: + """Dispose every remote kernel whose owner satisfies the predicate (a finished child's kernels).""" + _REGISTRY.shutdown(owner_matches=owner_matches) + + def _reap_unlocked(idle_timeout: int) -> List["RemoteKernel"]: """Pop idle-expired, unattached remote kernels; caller tears them down outside the lock. The runner self-exits after the same idle window, so this clears the HOST-side entry — without it @@ -183,11 +190,12 @@ def _evict_over_cap_unlocked(keep: Tuple) -> List["RemoteKernel"]: ``max_session_kernels`` bound as local kernels, applied independently to this map).""" from tools.code_kernel import _lifecycle_limits cap, _ = _lifecycle_limits() - if len(_REMOTE_KERNELS) <= cap: + unpinned = [key for key in _REMOTE_KERNELS if not _REMOTE_KERNELS[key].pinned] + if len(unpinned) <= cap: return [] - by_age = sorted((key for key in _REMOTE_KERNELS if key != keep and _REMOTE_KERNELS[key].attached == 0), + by_age = sorted((key for key in unpinned if key != keep and _REMOTE_KERNELS[key].attached == 0), key=lambda key: _REMOTE_KERNELS[key].last_used) - return [_REMOTE_KERNELS.pop(key) for key in by_age[: len(_REMOTE_KERNELS) - cap]] + return [_REMOTE_KERNELS.pop(key) for key in by_age[: len(unpinned) - cap]] atexit.register(shutdown_all_remote_kernels) @@ -265,6 +273,8 @@ def _acquire_remote_kernel(env, env_type: str, owner: str, task_env_id: str, 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: + from agent.delegation_context import is_delegated_child_context + kernel.pinned = is_delegated_child_context() with _REGISTRY.lock: _REMOTE_KERNELS[key] = kernel return kernel, reused, state_reset, state_lost diff --git a/tools/delegate_tool_child_run.py b/tools/delegate_tool_child_run.py index 036e83f7bb..e21fa7c9c3 100644 --- a/tools/delegate_tool_child_run.py +++ b/tools/delegate_tool_child_run.py @@ -892,6 +892,11 @@ class _ChildRun: # processes, httpx clients) so subagent subprocesses don't outlive the delegation. if not close_deferred: _close_child(child, "Failed to close child agent after delegation") + # The child's execute_code kernels live exactly as long as the child (pinned against the LRU + # cap while it runs); dispose them here so they never squat the cap after the child is gone. + with _quiet("Failed to dispose child execute_code kernels: %s"): + from tools.code_kernel import shutdown_kernels_for_delegated_child + shutdown_kernels_for_delegated_child(str(getattr(child, "session_id", "") or "")) # The AIAgent turn boundary normally closes the child scope itself. This fallback covers failures before that # boundary starts, but must not pop a scope while a timed-out child worker is still unwinding. diff --git a/website/docs/user-guide/features/code-execution.md b/website/docs/user-guide/features/code-execution.md index 9cf28e5f48..0736de5500 100644 --- a/website/docs/user-guide/features/code-execution.md +++ b/website/docs/user-guide/features/code-execution.md @@ -176,13 +176,13 @@ code_execution: ## State Between Calls (the session kernel) -On the local terminal backend, `execute_code` does not start a fresh interpreter for every call. Each session owns a persistent Python kernel, so variables, imports, and loaded data from one call are available in the next. The agent can load a dataset once and query it across several turns instead of re-reading it every time. Subagents get their own kernel; kernels are never shared across sessions. +On the local terminal backend, `execute_code` does not start a fresh interpreter for every call. Each session owns a persistent Python kernel, so variables, imports, and loaded data from one call are available in the next. The agent can load a dataset once and query it across several turns instead of re-reading it every time. Subagents get their own kernel; kernels are never shared across sessions. A subagent's kernel lives exactly as long as the subagent: it is exempt from the live-kernel cap while the subagent runs (a wide fan-out no longer evicts a sibling's kernel mid-task) and is disposed when the subagent finishes. What ends a kernel: - **Timeout or interrupt.** A cell that hits the timeout (or is interrupted) kills the kernel process and its state is lost on purpose; the result says so and the next call starts a fresh kernel. - **`reset=true`.** The agent can pass `reset: true` to discard the kernel's state and start clean. This is also the way to pick up environment changes: a kernel's environment is frozen when it spawns, so a newly allowlisted passthrough variable is invisible until the kernel is reset. -- **Idle timeout and eviction.** Kernels die with the session, after `code_execution.kernel_idle_timeout` idle seconds (default 1800), or when more than `code_execution.max_session_kernels` (default 4) are alive and the oldest is evicted. +- **Idle timeout and eviction.** Kernels die with the session, after `code_execution.kernel_idle_timeout` idle seconds (default 1800), or when more than `code_execution.max_session_kernels` (default 4) top-level sessions' kernels are alive and the oldest is evicted (running subagents' kernels do not count against the cap). The security envelope is the same as a one-shot script: environment scrubbing, the tool whitelist, and the per-call tool budget all apply to every cell, and tool-call authority (approvals, session, allow-list) is rebound on each cell.