From 4dac5f28af54001b899c9b6fc8ba81cb58da2f0e Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:20:41 -0700 Subject: [PATCH] fix(execute_code): parallel cells no longer orphan kernel processes; LSP stops treating package dirs as project roots Session kernels: a kernel mid-spawn (proc=None) read as dead, so every concurrent cell for one owner replaced the registry entry and the winner's process leaked outside the registry (110 live kernels, 1.1 GB, 330 threads under one 4-capped process). Reap/evict also tore down kernels with cells attached, rmtree-ing the staging dir under the spawner. Kernels now track attached cells: only settled kernels are reaped/evicted, a kernel dropped while busy is torn down by its last cell, and in-cell registry pops never remove a replacement. LSP: a directory holding __init__.py is a package, not a project root. hermes_cli/setup.py matched the python marker list and gave every worktree a second pyright rooted at hermes_cli/ (70 of 105 reaped clients in one session, ~40 servers / 8.7 GB live). --- agent/lsp/workspace.py | 18 ++++--- tests/agent/lsp/test_workspace.py | 13 +++++ tests/tools/test_code_kernel.py | 25 ++++++++++ tools/code_kernel.py | 81 +++++++++++++++++++++++++++---- 4 files changed, 121 insertions(+), 16 deletions(-) diff --git a/agent/lsp/workspace.py b/agent/lsp/workspace.py index 4f5beacfbb..b25f02f7e0 100644 --- a/agent/lsp/workspace.py +++ b/agent/lsp/workspace.py @@ -153,13 +153,17 @@ def nearest_root( return None except OSError: continue - # Then check markers. - for marker in markers_list: - try: - if (cur / marker).exists(): - return str(cur) - except OSError: - continue + # Then check markers. A directory holding __init__.py is a Python + # package, never a project root: hermes_cli/setup.py matched the + # python marker list and gave every package dir its own pyright, + # doubling servers per worktree (Sep 2026). + if not (cur / "__init__.py").exists(): + for marker in markers_list: + try: + if (cur / marker).exists(): + return str(cur) + except OSError: + continue # Stop conditions. if ceiling_path is not None and cur == ceiling_path: return None diff --git a/tests/agent/lsp/test_workspace.py b/tests/agent/lsp/test_workspace.py index 8d96f902d6..7a0a6abcd7 100644 --- a/tests/agent/lsp/test_workspace.py +++ b/tests/agent/lsp/test_workspace.py @@ -49,6 +49,19 @@ def test_nearest_root_finds_first_marker(tmp_path: Path): assert found == str(root) +def test_nearest_root_skips_package_dirs(tmp_path: Path): + # hermes_cli/setup.py is a module inside a package, not a project + # marker; treating it as one spawned a second pyright per worktree. + root = tmp_path / "p" + pkg = root / "hermes_cli" + pkg.mkdir(parents=True) + (root / "pyproject.toml").write_text("") + (pkg / "__init__.py").write_text("") + (pkg / "setup.py").write_text("") + found = nearest_root(str(pkg / "main.py"), ["pyproject.toml", "setup.py"]) + assert found == str(root) + + diff --git a/tests/tools/test_code_kernel.py b/tests/tools/test_code_kernel.py index 19f3179044..337de2dcfa 100644 --- a/tests/tools/test_code_kernel.py +++ b/tests/tools/test_code_kernel.py @@ -307,6 +307,31 @@ class TestKernelOwnershipAndLifecycle(unittest.TestCase): stale.proc.wait(timeout=10) self.assertFalse(stale.alive()) + def test_parallel_cells_share_one_kernel_process(self): + """Parallel cells for one owner race the first spawn. Each racer + used to see proc=None as 'dead', replace the registry entry, and + orphan the winner's process — 110 live kernels under a 4-capped + process (Sep 2026). Every kernel process must stay registry-owned.""" + import subprocess + import threading + + results = [] + with _kernel_config(): + def _cell(): + results.append(self._run_as("conv-a", "import time; time.sleep(0.3)", task_id="t")) + threads = [threading.Thread(target=_cell) for _ in range(6)] + for t in threads: + t.start() + for t in threads: + t.join() + self.assertEqual([r["status"] for r in results], ["success"] * 6) + self.assertEqual(len(_KERNELS), 1) + live = subprocess.run( + ["pgrep", "-fc", "-P", str(os.getpid()), "hermes_kernel_runner"], + capture_output=True, text=True, + ).stdout.strip() + self.assertEqual(live, "1") + class TestPerCellRpcAuthority(unittest.TestCase): """Interpreter state persists across cells; RPC authority must not.""" diff --git a/tools/code_kernel.py b/tools/code_kernel.py index 7805f636d1..784d7add41 100644 --- a/tools/code_kernel.py +++ b/tools/code_kernel.py @@ -261,6 +261,12 @@ class SessionKernel: self.sentinel: str = "" self.tool_call_log: List = [] self.tool_call_counter: List[int] = [0] + # Cells currently attached to this kernel (bumped under _KERNELS_LOCK + # when a caller selects it, dropped when its cell settles). Reaping + # and cap-eviction skip kernels with attached cells: tearing one down + # mid-spawn rmtree'd the staging dir under the spawner + # (FileNotFoundError) and killed live cells (Sep 2026). + self.attached: int = 0 self.response_q: "queue.Queue[dict]" = queue.Queue() self.raw_chunks: List[bytes] = [] self.raw_bytes = [0] @@ -273,6 +279,17 @@ class SessionKernel: def alive(self) -> bool: return self.proc is not None and self.proc.poll() is None + def dead(self) -> bool: + """True only once a spawned process has exited. + + A kernel whose ``proc`` is still ``None`` is mid-spawn, not dead: + parallel cells for one owner race the first cell's ``_spawn``, and + treating the pending kernel as dead made every racer replace it, + orphaning the winner's process outside the registry (110 live + kernels under one 4-capped process, Sep 2026). + """ + return self.proc is not None and self.proc.poll() is not None + _KERNELS: Dict[Tuple, SessionKernel] = {} _KERNELS_LOCK = threading.Lock() @@ -380,18 +397,18 @@ def _reap_unlocked() -> List[SessionKernel]: doomed = [ key for key, kernel in _KERNELS.items() - if now - kernel.last_used > idle_timeout + if kernel.attached == 0 and 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.""" + """Pop least-recently-used idle 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 for key in _KERNELS if key != keep and _KERNELS[key].attached == 0), key=lambda key: _KERNELS[key].last_used, ) doomed = by_age[: len(_KERNELS) - cap] @@ -664,18 +681,60 @@ def execute_in_session_kernel( with _KERNELS_LOCK: expired = _reap_unlocked() kernel = _KERNELS.get(key) - if kernel is not None and (reset or not kernel.alive()): + if kernel is not None and (reset or kernel.dead()): _KERNELS.pop(key, None) - expired.append(kernel) + if kernel.attached == 0: + expired.append(kernel) kernel = None state_reset = True if kernel is None: kernel = SessionKernel(key) _KERNELS[key] = kernel kernel.last_used = time.monotonic() + kernel.attached += 1 expired.extend(_evict_over_cap_unlocked(keep=key)) for doomed in expired: _teardown(doomed) + 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, is_interrupted=is_interrupted, + exec_start=exec_start, state_reset=state_reset, + ) + finally: + with _KERNELS_LOCK: + kernel.attached -= 1 + kernel.last_used = time.monotonic() + # Dropped from the registry (reset/dead/reaped) while cells were + # still attached: the last one out owns the teardown. + orphaned = kernel.attached == 0 and _KERNELS.get(key) is not kernel + if orphaned: + _teardown(kernel) + + +def _run_cell( + kernel: SessionKernel, + key: Tuple, + code: str, + *, + task_id: str, + child_python: str, + child_cwd: str, + sandbox_tools: frozenset, + timeout: int, + max_tool_calls: int, + is_interrupted, + exec_start: float, + state_reset: bool, +) -> str: + 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 + reused = kernel.proc is not None # Captured on the calling thread BEFORE the cell runs — the same @@ -731,7 +790,8 @@ def execute_in_session_kernel( # 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) + if _KERNELS.get(key) is kernel: + _KERNELS.pop(key, None) _teardown(kernel) duration = round(time.monotonic() - exec_start, 2) @@ -810,7 +870,8 @@ def execute_in_session_kernel( elif cell_status == "exit": # The cell called sys.exit(): honor it as end-of-kernel. with _KERNELS_LOCK: - _KERNELS.pop(key, None) + if _KERNELS.get(key) is kernel: + _KERNELS.pop(key, None) _teardown(kernel) result["kernel"]["ended"] = True if cell_stderr: @@ -822,7 +883,8 @@ def execute_in_session_kernel( + (": " + stderr_raw.strip() if stderr_raw.strip() else ".") ) with _KERNELS_LOCK: - _KERNELS.pop(key, None) + if _KERNELS.get(key) is kernel: + _KERNELS.pop(key, None) _teardown(kernel) elif cell_stderr: result["output"] = stdout_text + "\n--- stderr ---\n" + cell_stderr @@ -831,7 +893,8 @@ def execute_in_session_kernel( 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) + if _KERNELS.get(key) is kernel: + _KERNELS.pop(key, None) _teardown(kernel) return json.dumps({ "status": "error",