feat(tools): session-persistent kernels for execute_code (kernel_mode: session) - #94647
Conversation
…: session) execute_code spawns a fresh Python process per call, so every multi-step data task re-loads its inputs: a CSV parsed in call one is gone by call two, and scripts route state through temp files to survive. Hermes already rewards programmatic tool calling (execute_code-only turns refund the iteration budget), which makes the missing half — state that survives between calls — the bottleneck. Add opt-in `code_execution.kernel_mode: session`: one persistent kernel per (task, mode, interpreter, cwd, tool-set). Variables, imports, and loaded data persist across calls; `reset=true` discards state on demand. The default `per-call` keeps today's behavior byte-for-byte. Safety posture is unchanged by design: the child env comes from the same builder as the per-call path (extracted, not duplicated, so the secret scrubbing / PYTHONPATH hygiene cannot drift), the RPC server is the same `_rpc_server_loop` with the same token and a per-cell tool budget, and output passes the same ANSI strip + secret redaction. A timed-out or interrupted cell kills the whole kernel tree and the next call respawns — a wedged kernel can never hang the agent. The kernel env is frozen at spawn; the schema and config comment say so. Wire protocol: NDJSON requests on the kernel's stdin; responses framed on stdout behind a per-kernel random sentinel, with unframed bytes (fd-level output from user-spawned subprocesses) attributed to the serialized current cell. The generated RPC client reconnects once when HERMES_RPC_PERSISTENT=1, because a kernel legitimately outlives the RPC server's 300s idle window between cells. Tested on macOS 15 (Apple Silicon), Python 3.11: 13 new tests in tests/tools/test_code_kernel.py (persistence, reset, error-keeps-kernel, timeout-kills-kernel, sys.exit ends kernel, subprocess fd passthrough, schema surface, mode fallback) plus the existing test_code_execution.py / test_code_execution_modes.py suites (81 passed).
andrexibiza
left a comment
There was a problem hiding this comment.
Blocking review on exact head 0ff6dbc6b853bb7de61258eeaf0c842d40797891 against base 76e306c45843607e6dc135d23c13d3654417ebd5. I inspected the full changed-file set, execute-code dispatch path, current run_agent task ownership, thread_context propagation semantics, the earlier persistent-kernel implementation in #88637, and the pending execute-code approval hardening in #65592. Exact-head CI, Docker, and Nix are green. The kernel mechanism is plausible, but two authority/lifecycle boundaries are currently wrong and the existing tests do not exercise them.
1. The object called a “session” kernel is actually keyed to a top-level task/turn, and completed turns leak live kernels indefinitely.
_kernel_key() uses (task_id, mode, interpreter, cwd, tool-set). In the normal agent path, run_agent creates effective_task_id = task_id or str(uuid.uuid4()) for each top-level run_conversation invocation. So two user turns in the same Hermes conversation get different kernel keys unless an external caller happens to supply a stable task id. State therefore does not survive the conversation boundary the feature name/documentation implies.
More importantly, _KERNELS has no session/turn disposal, idle reap, or process-wide bound. Entries are removed only for explicit reset, dead kernels, timeout/interruption, sys.exit, protocol/runtime failure, or process-wide atexit. In a long-lived Gateway with kernel_mode: session, every completed user turn that used execute_code can leave a Python child, RPC listener thread/socket, and tempdir alive until Hermes exits. The new tests reuse one fixed task_id="kernel-test", so they prove intra-task persistence but miss both the next-user-turn discontinuity and the unbounded-lifetime case.
Required fix: give persistent code execution an explicit stable owner (conversation lineage + profile/subagent isolation, not the ephemeral turn task id) or intentionally scope this to one turn and dispose it at turn completion. Either design also needs bounded lifecycle semantics: disposal on owner teardown/reset plus an idle/LRU or equivalent process-wide cap. Add a regression that executes code in two separate top-level turns of one conversation and another that completes many owners and proves the registry/process count stays bounded.
This is also where #88637 is directly relevant. That earlier work by @z80dev is stale/conflicted, so I am not saying to merge it as-is, but it already carries the missing ownership/lifecycle shape: a separate stable code_execution_session_id, conversation/subagent scope, /new + agent-close disposal, idle reaping, and a max-live-kernel bound. #94647 is therefore an alternate/salvage implementation of the same feature class, not independent work. If this PR becomes the canonical path, please preserve that contributor credit and carry forward the applicable invariants rather than replacing them with per-task lifetime accidentally.
2. RPC authority is frozen at kernel spawn instead of being rebound per execute_code call.
_spawn() starts one long-lived _rpc_forever thread with target=propagate_context_to_thread(_rpc_forever). That helper explicitly snapshots contextvars.copy_context() plus the current approval/sudo callbacks at wrap time and installs them for the worker’s lifetime. That is correct for today’s per-call RPC thread; it is not correct for a thread that survives many later cells.
The consequence is that later cells invoking hermes_tools.* are dispatched under the first cell’s approval/session/platform/turn/tool-call/profile context and callbacks. Resetting tool_call_counter[0] per cell does not refresh that authority. At minimum this produces wrong approval/observability identity; across a genuinely session-persistent kernel it can route approval or scoped runtime state to an authority carrier that no longer owns the current call. This is the other side of making the interpreter persistent: interpreter state may persist, RPC authority must not.
Required fix: rebind RPC authority on every execute_code invocation. A fresh per-cell RPC endpoint/token/server context is the clearest design, but a host-side per-cell authority object is also fine if its handoff is atomic before the cell runs and cannot be used after the cell settles. Add a regression where call 1 and call 2 have different approval/turn/tool-call context and prove an RPC from call 2 observes only call 2’s context. Include profile/subagent isolation in that test surface.
Security interlock with #65592: that PR is currently turning check_execute_code_guard() into a much stronger per-script static policy boundary. A persistent namespace introduces cross-cell dataflow that a current-cell AST cannot reconstruct: capabilities/functions/objects created or dynamically resolved in cell N can be invoked by an opaque global name in cell N+1. #65592 explicitly documents runtime indirection as outside its static visibility. Once state persists, that residual stops being an exceptional one-script edge and becomes an expected execution mode. These two PRs therefore need an explicit merge-order/composition contract and cross-cell adversarial tests; “the same guard ran for this cell” is not enough to claim the same security envelope.
What I checked that does look good: the child-env builder is genuinely shared, the per-cell tool counter reset is explicit, timeout/interruption tears down the process group, and exact-head CI/Docker/Nix all pass. Those are useful pieces. The missing pieces are ownership, bounded lifetime, per-cell authority rebinding, and composition with the pending execute-code policy hardening.
… per-cell RPC authority Addresses the blocking review on the session-kernel design: two authority/lifecycle boundaries were wrong. 1. Ownership and bounded lifetime. The kernel key's first component is now the conversation's approval session key (_resolve_owner), not the per-turn task id run_agent mints per top-level invocation — so state genuinely survives across user turns of one conversation, and delegated subagent sessions isolate naturally under their own keys (the task id remains only the last-resort owner for embeds/tests with no session context). Lifetime is bounded on four edges: kernels are disposed at the same session boundary that clears the owner's approval/yolo state (tools.approval.clear_session -> shutdown_kernels_for_owner), reaped after code_execution.kernel_idle_timeout seconds idle (default 1800, swept on every entry), capped process-wide at code_execution.max_session_kernels live children (default 4, LRU evicted), and still torn down by reset/death/atexit as before. The ownership + disposal + idle-reap + cap shape deliberately carries forward the lifecycle invariants of the earlier session-persistent implementation in NousResearch#88637 by @z80dev. 2. Per-cell RPC authority. The serving thread no longer freezes the spawning cell's context/callbacks for the kernel's life. Each cell installs a CellAuthority — captured on the calling thread exactly as propagate_context_to_thread would for a per-call RPC thread — before its request is written, and retires it on every settle path; _rpc_server_loop gains a dispatch hook the kernel uses to route each tool call through the CURRENT cell's context, callbacks, and task id. A call arriving with no active cell is refused. Interpreter state persists; RPC authority does not. Composition with the per-script static guard (see the config note): a persistent namespace lets cell N+1 invoke objects cell N created, which a single-cell static scan cannot see — the runtime RPC boundary (allow-list by name, per-cell budget, per-cell authority) is the operative cross-cell enforcement in this mode, and the adversarial alias test pins exactly that. Tests (9 new): state survives across turns of one conversation; sessions isolate; clear_session disposes the owner's kernels (and the next turn starts fresh); the live-kernel cap LRU-evicts with evicted children proven dead; idle kernels are reaped; a later cell's RPC runs under that cell's approval callback; a cross-cell alias dispatches under the CURRENT cell's authority; a settled cell's authority refuses dispatch; each cell installs a fresh authority. 22/22 kernel tests, 81 code-execution tests, ruff clean. The 7 test-order failures in the tools/-k-approval selection reproduce identically on the clean branch base (pre-existing pollution, not this change).
|
Both blocking findings addressed in 98a5bee, point by point: 1. Ownership and bounded lifetime — fixed as the "explicit stable owner" design, with all four lifecycle edges. The kernel key's first component is now the conversation's approval session key ( Bounded lifecycle, all enforced now:
Both requested regressions exist and pass: 2. RPC authority — rebound per cell via a host-side authority object with atomic handoff and post-settle refusal (your second acceptable design). The serving thread no longer wraps Requested regression: #65592 composition: stated explicitly now, in the config note and in an adversarial test. A persistent namespace lets cell N+1 invoke objects cell N created through an opaque global — outside any single-cell static scan's visibility, as #65592 itself documents. The operative cross-cell enforcement in kernel mode is therefore the runtime RPC boundary: allow-list by tool name (an alias still crosses the socket under its real name), per-cell budget, per-cell authority. #88637 credit: the ownership + disposal + idle-reap + max-live shape deliberately carries forward the lifecycle invariants of @z80dev's earlier implementation, and the commit message and code comments say so explicitly. This PR should be read as a salvage/alternate implementation of that feature class, not independent work. Verification on the new head: 22/22 kernel tests (13 existing + 9 new), 81 code-execution/mode tests, ruff clean on every touched file. The 7 failures in the |
andrexibiza
left a comment
There was a problem hiding this comment.
Re-review of exact head 98a5bee54f8bfb0b91e7b66fefe38ae9e9d7e6ca.
The two original directions are materially improved: ownership now resolves to the stable approval-session identity, owner/session cleanup and registry bounds exist, and the server no longer freezes the first cell's callback/context forever. Both retained commits have successful core workflow receipts; the exact head is green in CI, Docker, and Nix.
This head is still blocked.
1. P0 — RPC origin and cell settlement are not generation-bound, so old work can execute under the next cell's authority
The implementation says a late background thread or raced client write from a settled cell will be refused, but that identity never reaches either wire protocol.
The generated RPC request contains only tool, args, and one kernel-lifetime token (client request). The server then dispatches through whatever happens to be in kernel.cell_authority at receive time (current-pointer dispatch). A thread created by cell N can therefore sleep until cell N settles, wake while cell N+1 is active, and have its RPC executed under N+1's approval callback, task id, budget, and session context. Retiring the old CellAuthority object does not help because the RPC request carries no reference to that object; the added test calls the retired object directly instead of traversing the real RPC path (test).
The response side has the same missing generation. The framing sentinel is exported into the untrusted child environment (spawn); fd-level output is parsed as a control frame and any JSON body is queued (reader); and the caller consumes the first queued payload without checking that payload["id"] matches the request it just minted (wait loop). User code can write a syntactically valid frame to fd 1, make the host settle/retire the cell while its code is still executing, and leave the real response to satisfy the next cell. The module docstring explicitly acknowledges frame forgery but the conclusion that it can only lie to its own caller is false once settlement controls authority handoff (claim).
Required repair: bind every RPC request and every settlement response to one immutable cell generation; reject stale, missing, duplicate, and mismatched generations; carry control replies on a channel that executed code cannot write; and do not install generation N+1 until N is authoritatively settled and late N work is unable to mutate. Add end-to-end adversarial tests for (a) a cell-N background thread calling hermes_tools during cell N+1 and (b) a forged/mismatched frame followed by a real frame. Both must fail closed without consuming N+1's authority or response slot.
2. P0 — the claimed #65592 composition covers RPC aliases, not the direct-Python effects #65592 exists to guard
check_execute_code_guard receives and scans only the current cell's source before the session branch runs (guard boundary). The new composition test persists hermes_tools.web_search and invokes that alias in the next cell (test); that call necessarily crosses RPC, so it proves only the new RPC dispatcher.
#65592 protects the opposite class: direct Python operations such as os/subprocess/pathlib/library writers that never cross terminal() or the tool RPC boundary. In a persistent GLOBALS, cell N can retain a callable or object capability and cell N+1 can invoke it through an opaque name—for example, bind retained_remove = os.remove in one cell and later execute retained_remove(target). The N+1 static scan cannot recover the earlier binding or exact target, and the runtime RPC boundary is not involved. Approval of cell N also cannot silently authorize an unrepresented future mutation in cell N+1.
Required repair: provide a cumulative, provenance-aware cross-cell policy for direct capabilities or an actual runtime enforcement boundary; otherwise session mode cannot claim composition with #65592. Add adversarial two-cell tests for retained direct filesystem, subprocess, process-kill, and library-writer capabilities, including the protected-target invariants from #65592. The “compatible in either merge order” claim is not established until those tests pass on the combined object.
3. P1 — lifecycle removal is neither busy-aware nor generation-fenced; it can kill active work or spawn an orphan after disposal
The registry stamps last_used before acquiring the per-kernel execution lock, then reaps and LRU-evicts entries without checking whether they are creating, queued, or actively executing (entry path, reap/evict). With max_session_kernels=1, owner B deterministically evicts owner A even if A's cell is running. An idle sweep can do the same once the age since cell start exceeds the threshold.
There is a second race: owner clear/reset/reap/cap removes the object from _KERNELS and tears it down outside kernel.lock, while the caller retains the object reference. That caller can subsequently enter the lock and _spawn() the already-removed, stop-marked object because _spawn() verifies neither registry membership nor generation (spawn path). The resulting child is no longer owned by the registry, so later session clear and atexit traversal cannot find it. The session-boundary hook also swallows every cleanup exception (clear hook), which converts a teardown failure into a silent live-interpreter leak.
Required repair: make registry ownership a generation/lease, mark entries creating/busy/idle/retiring/dead under one synchronization regime, update idle time at authoritative settlement, exclude creating/busy entries from idle/LRU disposal, and revalidate the lease immediately before spawn and mutation. If the cap is full of busy kernels, wait or return a typed capacity refusal rather than killing another request. Add concurrent tests for active cap pressure, active idle sweep, clear-before-spawn, reset-versus-spawn, and teardown failure; prove no killed active request, resurrected object, or unregistered child remains.
4. Structural gate — this expands three existing godfiles
This head adds session-kernel seams to tools/code_execution_tool.py, which is still over 2,300 lines (tail); hermes_cli/config_defaults.py, already beyond 2,800 lines (new config block); and tools/approval.py, already beyond 2,900 lines. tools/code_kernel.py is correctly below the 2K boundary; keep the implementation there or extract additional bounded support/config modules and leave only narrow imports/delegations in the existing godfiles. No further godfile growth.
Landing gate
Exact head 98a5bee5… is currently 2 ahead / 580 behind current main 5fc308a7…. tools/code_execution_tool.py has changed on main since this branch's merge base, including #95937. The existing green runs certify the old object, not the current landing edge. Rebase onto current main before the next push, repair the blockers on that object, perform the semantic same-path read against #65592/#95937/#88637, and regenerate CI, Docker, Nix, and required-check receipts on the resulting exact head. Every retained commit must remain green.
andrexibiza
left a comment
There was a problem hiding this comment.
Re-review on exact head 98a5bee54f8bfb0b91e7b66fefe38ae9e9d7e6ca. I inspected all six changed files, the added lifecycle/authority tests, the current #65592 head ead8c83cc22084e33d171452507bee9f028c3ce0, both PR commits' workflow history, and the current landing edge. The ownership correction is real: _resolve_owner(), owner teardown, idle reap/LRU, and ordinary later-cell callback rebinding materially fix the two defects from the first review. The #88637 lifecycle credit is also carried forward. Exact-head CI, Docker, and Nix are green.
I still cannot clear the object. Three security/lifecycle boundaries remain open.
1. The promised #65592 composition still bypasses hard-blocked native Python capabilities across cells
execute_code() runs check_execute_code_guard() against only the current code string before entering session mode (tools/code_execution_tool.py L1435-L1495). The kernel then executes every request in the same persistent GLOBALS namespace (tools/code_kernel.py L83-L124).
The current #65592 head explicitly documents function/lambda indirection as an XFAIL outside its static boundary, including def f(): return os.kill; f()(...) (its adversarial suite L677-L701). Session persistence turns that documented residual into an ordinary two-cell path:
# cell 1
import os
def f():
return os.kill# cell 2
f()(os.getpid(), 15)Cell 1 stores the capability without invoking it; cell 2 presents only an opaque persistent global to the per-script scanner. The eventual os.kill call is a direct child-process Python call. It never crosses the Hermes RPC socket, so the runtime tool allow-list, per-cell budget, and CellAuthority do not observe or stop it. The same shape applies to other native filesystem/process/network capabilities retained from an earlier cell.
test_cross_cell_alias_dispatches_under_the_current_cell proves only a hermes_tools.* alias, which is the one class that necessarily crosses RPC. It does not establish the claimed composition boundary for native Python capabilities, and therefore does not support “compatible whether #65592 lands first or second” or “same security envelope as per-call.”
Required fix: close the native cross-cell capability surface, or fail closed by disabling local session kernels wherever the per-script static guard is the security boundary. Add a real cross-branch regression against #65592's current policy object that splits its hard-block XFAIL shapes across two cells and proves they remain non-overridable, including under yolo/approvals-off. The RPC-alias test is useful but cannot substitute for that interlock.
2. A cell's RPC authority and settlement are not bound end-to-end to that cell
Stale work can inherit the next cell's authority
The generated client sends only tool, args, and the kernel-wide token—there is no immutable cell id/generation in the request (tools/code_execution_tool.py L548-L554). Server dispatch reads the single mutable kernel.cell_authority pointer at arrival time (tools/code_kernel.py L403-L410), and each later cell overwrites that pointer before writing its request (L640-L665).
So a background thread created by cell 1 can wait until cell 1 settles, then call hermes_tools.* while cell 2 is active. Its wire request is indistinguishable from cell 2's request and _dispatch() routes it through cell 2's task id, callbacks, context, tool budget, and approvals. Retiring authority 1 does not help after the kernel pointer has been replaced with authority 2.
The new post-settle test calls authority.retire(); authority.dispatch(...) directly. That proves the old object refuses when addressed directly; it does not exercise the live server dispatcher after the pointer is replaced. Add the adversarial lifecycle test the docstring itself promises: cell 1 starts a background RPC worker gated on an event, cell 2 installs a distinct authority and releases the event, and the stale call must be refused without reaching handle_function_call or consuming cell 2's budget. The wire/control design needs an origin-bound per-cell capability or generation that stale work cannot present for a later cell; a mutable “current authority” pointer is not that boundary.
User code can forge a host-accepted completion and desynchronize cells
The response sentinel is placed in os.environ inside the same interpreter that runs untrusted cell code (tools/code_kernel.py L70-L88). _stdout_reader() accepts any correctly framed JSON and puts it on response_q (L455-L491). The host generates a request id, but does not retain or match that id when it accepts the first queued payload (L663-L683).
A cell can therefore write a forged frame through sys.__stdout__.buffer/fd 1, then continue running. The host accepts the forged success, returns, retires the authority, and releases kernel.lock while the original cell is still executing. A later call can start logically; when the runner eventually emits the real reply for the old cell, that stale payload can be consumed as the later cell's result. This is not equivalent to “printing a forged success message” in the per-call path: it controls the host's settlement decision, defeats serialized-cell attribution/timeout assumptions, and corrupts the response queue across calls.
Required fix: untrusted cell code must be unable to manufacture a host-accepted settle record. Bind every response to the currently outstanding request/generation and put the control channel behind a boundary the executed namespace cannot write or introspect. Add a regression that emits a forged frame and then blocks: the host must wait for the authoritative reply or timeout, and the next call must never consume the prior cell's real response.
3. Owner teardown and LRU/idle eviction race with spawn and active execution
The registry operations pop kernels under _KERNELS_LOCK and call _teardown() later without coordinating with kernel.lock (shutdown_kernels_for_owner() L319-L335, _teardown() L367-L383). The execution path can register a new SessionKernel, release _KERNELS_LOCK, and only afterwards acquire kernel.lock and spawn it (L619-L650).
Concrete interleaving:
- A call registers a new kernel with
proc is Noneand releases_KERNELS_LOCK. clear_session()or cap eviction pops it and_teardown()setsstop_event.- The original call then acquires
kernel.lock, seesproc is None, and_spawn()starts a child anyway; there is no closed/tombstone check. - The RPC thread exits immediately because
stop_eventis already set, but the child/readers can remain alive and the kernel is no longer in_KERNELS, so later owner teardown and registry bounds no longer own it.
The same state model lets LRU/idle selection pop an in-flight kernel because neither _reap_unlocked() nor _evict_over_cap_unlocked() excludes a kernel whose cell holds kernel.lock (L338-L362). The sequential tests prove dead children after sequential eviction; they do not prove boundedness or ownership under concurrent session activity.
Required fix: make registry ownership, closed state/generation, spawn, and teardown one coherent state machine with a consistent lock order. A popped/closed kernel must be unable to spawn; active kernels must be pinned or new admission must fail explicitly rather than silently killing another owner's running cell. Add deterministic barrier tests for clear-between-register-and-spawn and cap pressure while another owner is executing, asserting zero untracked children and explicit outcomes.
Landing edge
At review time current main is 5fc308a70719a83cccdbba4c0e39c23f5a8239d5; this branch is 580 commits behind it, and both tools/code_execution_tool.py and tools/approval.py have moved on main since the branch point. The green runs above prove this submitted head, not current-main composition. After the blockers are fixed, this needs one current-main rebase/semantic collision read and fresh exact-head CI/Docker/Nix before clearance.
Status: the original ownership and first-cell callback defects are materially improved, but this head remains blocked on native-policy composition, generation-bound cell authority/settlement, and concurrency-safe lifecycle ownership.
andrexibiza
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 98a5bee54f8bfb0b91e7b66fefe38ae9e9d7e6ca after the ownership/lifecycle + per-cell-authority repair. The two original blockers are materially improved: the kernel owner is now the approval session key with owner teardown / idle reap / process-wide LRU cap, and the long-lived RPC thread no longer freezes the first cell's context. Both submitted commits also have green exact-SHA CI/Docker/Nix receipts (0ff6dbc…: 32837261784 / 32837260277 / 32837260217; 98a5bee…: 33065151167 / 33065150604 / 33065150577).
One P1 remains at the authority handoff, and it is specifically a cross-cell delayed-RPC race.
tools/code_execution_tool.py's generated persistent client authenticates every RPC with only the kernel-lifetime HERMES_RPC_TOKEN; the request carries {tool, args, token} and no cell/generation identity. tools/code_kernel.py::_rpc_forever() then chooses kernel.cell_authority at dispatch time. execute_in_session_kernel() replaces that slot with the next cell's CellAuthority before writing the next cell request.
That means a background thread created by cell N can outlive that cell, wait until cell N+1 is active, then call a retained hermes_tools.* function. Its RPC still has the valid kernel token, and the server will dispatch it through N+1's authority because that is what kernel.cell_authority points to at receipt time. Retiring N's CellAuthority does not help: the late request is never bound to N's authority object in the first place. It can also consume N+1's per-cell tool budget.
The current regressions do not exercise this shape. test_a_later_cells_rpc_runs_under_that_cells_authority and the cross-cell alias case are sequential calls intentionally originating in the current cell; test_a_settled_cells_authority_refuses_dispatch invokes the retired authority object directly. None sends a late RPC originating from the prior cell while a different cell authority is installed.
Required repair: bind every RPC request to the cell that issued it, not merely to the persistent kernel. A fresh per-cell endpoint/token is the simplest shape; a cell-generation/token carried in every request and validated against the installed CellAuthority is also fine. Rotation must make an N request unusable once N settles, even while N+1 is active.
Please lock it with the exact adversarial witness: cell 1 starts a background thread that blocks before hermes_tools.web_search; cell 1 settles; cell 2 installs a distinct approval/session/turn authority; release the cell-1 thread while cell 2 is active. The late call must be refused and must not invoke the tool or consume cell 2's budget. A genuine cell-2 RPC in the same window must still succeed under cell 2's authority.
There is also a real landing-edge reconciliation after this fix: live main is now f3cbb262c1d7014f1a4d225242d06983122c5ddf, 621 commits beyond this PR's recorded base, and GitHub reports the PR non-mergeable. At least tools/code_execution_tool.py and tools/approval.py changed on main since that base, so this needs a semantic rebase rather than a blind branch update.
The stable-owner/lifecycle repair is worth keeping, and the #88637 lineage is now explicitly preserved. Close the delayed-RPC generation boundary, then recompose on current main and rerun the exact object.
|
noice happy to see this moving forward |
…ntexts inherit the parent approval key, so qualify the owner with the delegation session id (live-verified leak, both directions)
|
Maintainer review (@teknium1's session): live-tested this branch end-to-end on Windows and pushed one isolation fix directly to the branch (maintainerCanModify). What was verified live (temp HERMES_HOME, kernel_mode: session)
The fix pushed to this branchDelegated children were sharing the parent's kernel. Fix: when Two regression tests added ( DirectionThis PR's posture is the one we want (opt-in |
…lity-leak detection Three-part response to andrexibiza's 2026-08-29 review (P1) plus the NousResearch#94647 cross-cell gap and fail-closed semantics: 1. Signature-aware exec*/spawn* argv extraction (P1) - node.args[-1] one-size-fits-all missed execve/spawnve/posix_spawn (last positional arg is env, not argv) — all three repros plus argv[0]-spoofing bypass were reproduced on c8e072d01c. - New _extract_exec_spawn_argv() resolves each family's real slots: execv* path@0/argv@1, spawnv* mode@0/path@1/argv@2, posix_spawn path@0/argv@1, *l families assembled from the positional tail (trailing env stripped for *le variants). - Path parameter is the authority: when statically resolvable it replaces argv[0] for package-manager classification, defeating os.execve('/usr/bin/pip', ['harmless', 'install', 'pkg'], env). 2. Fail-closed unresolvable process launch (NousResearch#97657 owner gate) - command-exec calls whose argv cannot be statically resolved now return _PACKAGE_UNRESOLVABLE instead of being skipped — owner approval required, not silently approved (was: 'static cannot resolve -> pass'; aligned with NousResearch#98138 bounded design). - Guard-level: unresolvable is still enforced before yolo/off. 3. Capability-leak detection (NousResearch#94647 cross-cell storage vector) - New _execute_code_has_capability_leak(): hard-blocked capabilities (os.kill family) referenced as VALUES (return/assignment/container/ argument/lambda body) are blocked before they can leave the cell — cell-1 'def f(): return os.kill' shapes that per-cell scanning cannot see at call sites. Four former Section-C XFAILs (fn/lambda indirection, fn-wrapper arg, container storage) are now resolved; only f-string path interpolation remains XFAIL. Tests: 566 passed + 1 xfailed (test_exec_code_guard + test_exec_code_guard_adversarial); exec/spawn 15 acquisition shapes + 6 benign controls, 8 leak shapes + 4 benign controls, yolo/off bypass regression for both new layers.
…lity-leak detection Three-part response to andrexibiza's 2026-08-29 review (P1) plus the NousResearch#94647 cross-cell gap and fail-closed semantics: 1. Signature-aware exec*/spawn* argv extraction (P1) - node.args[-1] one-size-fits-all missed execve/spawnve/posix_spawn (last positional arg is env, not argv) — all three repros plus argv[0]-spoofing bypass were reproduced on c8e072d01c. - New _extract_exec_spawn_argv() resolves each family's real slots: execv* path@0/argv@1, spawnv* mode@0/path@1/argv@2, posix_spawn path@0/argv@1, *l families assembled from the positional tail (trailing env stripped for *le variants). - Path parameter is the authority: when statically resolvable it replaces argv[0] for package-manager classification, defeating os.execve('/usr/bin/pip', ['harmless', 'install', 'pkg'], env). 2. Fail-closed unresolvable process launch (NousResearch#97657 owner gate) - command-exec calls whose argv cannot be statically resolved now return _PACKAGE_UNRESOLVABLE instead of being skipped — owner approval required, not silently approved (was: 'static cannot resolve -> pass'; aligned with NousResearch#98138 bounded design). - Guard-level: unresolvable is still enforced before yolo/off. 3. Capability-leak detection (NousResearch#94647 cross-cell storage vector) - New _execute_code_has_capability_leak(): hard-blocked capabilities (os.kill family) referenced as VALUES (return/assignment/container/ argument/lambda body) are blocked before they can leave the cell — cell-1 'def f(): return os.kill' shapes that per-cell scanning cannot see at call sites. Four former Section-C XFAILs (fn/lambda indirection, fn-wrapper arg, container storage) are now resolved; only f-string path interpolation remains XFAIL. Tests: 566 passed + 1 xfailed (test_exec_code_guard + test_exec_code_guard_adversarial); exec/spawn 15 acquisition shapes + 6 benign controls, 8 leak shapes + 4 benign controls, yolo/off bypass regression for both new layers. (cherry picked from commit 5da4362)
…lity-leak detection Three-part response to andrexibiza's 2026-08-29 review (P1) plus the NousResearch#94647 cross-cell gap and fail-closed semantics: 1. Signature-aware exec*/spawn* argv extraction (P1) - node.args[-1] one-size-fits-all missed execve/spawnve/posix_spawn (last positional arg is env, not argv) — all three repros plus argv[0]-spoofing bypass were reproduced on c8e072d01c. - New _extract_exec_spawn_argv() resolves each family's real slots: execv* path@0/argv@1, spawnv* mode@0/path@1/argv@2, posix_spawn path@0/argv@1, *l families assembled from the positional tail (trailing env stripped for *le variants). - Path parameter is the authority: when statically resolvable it replaces argv[0] for package-manager classification, defeating os.execve('/usr/bin/pip', ['harmless', 'install', 'pkg'], env). 2. Fail-closed unresolvable process launch (NousResearch#97657 owner gate) - command-exec calls whose argv cannot be statically resolved now return _PACKAGE_UNRESOLVABLE instead of being skipped — owner approval required, not silently approved (was: 'static cannot resolve -> pass'; aligned with NousResearch#98138 bounded design). - Guard-level: unresolvable is still enforced before yolo/off. 3. Capability-leak detection (NousResearch#94647 cross-cell storage vector) - New _execute_code_has_capability_leak(): hard-blocked capabilities (os.kill family) referenced as VALUES (return/assignment/container/ argument/lambda body) are blocked before they can leave the cell — cell-1 'def f(): return os.kill' shapes that per-cell scanning cannot see at call sites. Four former Section-C XFAILs (fn/lambda indirection, fn-wrapper arg, container storage) are now resolved; only f-string path interpolation remains XFAIL. Tests: 566 passed + 1 xfailed (test_exec_code_guard + test_exec_code_guard_adversarial); exec/spawn 15 acquisition shapes + 6 benign controls, 8 leak shapes + 4 benign controls, yolo/off bypass regression for both new layers. (cherry picked from commit 5da4362)
…sed semantics (NousResearch#65592 P0-3/P1-9 closeout) - _execute_code_has_package_acquisition: add os.exec* / os.posix_spawn* to the command-exec family scan (previously subprocess.*/os.system/ os.popen/os.spawn*/pty.spawn/asyncio.create_subprocess_* only) - Document fail-closed return semantics: _PACKAGE_UNRESOLVABLE when a command-exec call exists but argv is statically unresolvable — caller must owner-approve rather than allow - test_cross_cell_alias: narrow claim to the RPC-alias class only (native Python capabilities never cross RPC; cross-cell persistence is covered by the capability-leak detector, NousResearch#94647)
…: session) (NousResearch#94647) * feat(tools): session-persistent kernels for execute_code (kernel_mode: session) execute_code spawns a fresh Python process per call, so every multi-step data task re-loads its inputs: a CSV parsed in call one is gone by call two, and scripts route state through temp files to survive. Hermes already rewards programmatic tool calling (execute_code-only turns refund the iteration budget), which makes the missing half — state that survives between calls — the bottleneck. Add opt-in `code_execution.kernel_mode: session`: one persistent kernel per (task, mode, interpreter, cwd, tool-set). Variables, imports, and loaded data persist across calls; `reset=true` discards state on demand. The default `per-call` keeps today's behavior byte-for-byte. Safety posture is unchanged by design: the child env comes from the same builder as the per-call path (extracted, not duplicated, so the secret scrubbing / PYTHONPATH hygiene cannot drift), the RPC server is the same `_rpc_server_loop` with the same token and a per-cell tool budget, and output passes the same ANSI strip + secret redaction. A timed-out or interrupted cell kills the whole kernel tree and the next call respawns — a wedged kernel can never hang the agent. The kernel env is frozen at spawn; the schema and config comment say so. Wire protocol: NDJSON requests on the kernel's stdin; responses framed on stdout behind a per-kernel random sentinel, with unframed bytes (fd-level output from user-spawned subprocesses) attributed to the serialized current cell. The generated RPC client reconnects once when HERMES_RPC_PERSISTENT=1, because a kernel legitimately outlives the RPC server's 300s idle window between cells. Tested on macOS 15 (Apple Silicon), Python 3.11: 13 new tests in tests/tools/test_code_kernel.py (persistence, reset, error-keeps-kernel, timeout-kills-kernel, sys.exit ends kernel, subprocess fd passthrough, schema surface, mode fallback) plus the existing test_code_execution.py / test_code_execution_modes.py suites (81 passed). * fix(tools): session kernels get a stable owner, bounded lifetime, and per-cell RPC authority Addresses the blocking review on the session-kernel design: two authority/lifecycle boundaries were wrong. 1. Ownership and bounded lifetime. The kernel key's first component is now the conversation's approval session key (_resolve_owner), not the per-turn task id run_agent mints per top-level invocation — so state genuinely survives across user turns of one conversation, and delegated subagent sessions isolate naturally under their own keys (the task id remains only the last-resort owner for embeds/tests with no session context). Lifetime is bounded on four edges: kernels are disposed at the same session boundary that clears the owner's approval/yolo state (tools.approval.clear_session -> shutdown_kernels_for_owner), reaped after code_execution.kernel_idle_timeout seconds idle (default 1800, swept on every entry), capped process-wide at code_execution.max_session_kernels live children (default 4, LRU evicted), and still torn down by reset/death/atexit as before. The ownership + disposal + idle-reap + cap shape deliberately carries forward the lifecycle invariants of the earlier session-persistent implementation in NousResearch#88637 by @z80dev. 2. Per-cell RPC authority. The serving thread no longer freezes the spawning cell's context/callbacks for the kernel's life. Each cell installs a CellAuthority — captured on the calling thread exactly as propagate_context_to_thread would for a per-call RPC thread — before its request is written, and retires it on every settle path; _rpc_server_loop gains a dispatch hook the kernel uses to route each tool call through the CURRENT cell's context, callbacks, and task id. A call arriving with no active cell is refused. Interpreter state persists; RPC authority does not. Composition with the per-script static guard (see the config note): a persistent namespace lets cell N+1 invoke objects cell N created, which a single-cell static scan cannot see — the runtime RPC boundary (allow-list by name, per-cell budget, per-cell authority) is the operative cross-cell enforcement in this mode, and the adversarial alias test pins exactly that. Tests (9 new): state survives across turns of one conversation; sessions isolate; clear_session disposes the owner's kernels (and the next turn starts fresh); the live-kernel cap LRU-evicts with evicted children proven dead; idle kernels are reaped; a later cell's RPC runs under that cell's approval callback; a cross-cell alias dispatches under the CURRENT cell's authority; a settled cell's authority refuses dispatch; each cell installs a fresh authority. 22/22 kernel tests, 81 code-execution tests, ruff clean. The 7 test-order failures in the tools/-k-approval selection reproduce identically on the clean branch base (pre-existing pollution, not this change). * fix(code-kernel): delegated children get their own kernels — child contexts inherit the parent approval key, so qualify the owner with the delegation session id (live-verified leak, both directions) --------- Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
…: session) (NousResearch#94647) * feat(tools): session-persistent kernels for execute_code (kernel_mode: session) execute_code spawns a fresh Python process per call, so every multi-step data task re-loads its inputs: a CSV parsed in call one is gone by call two, and scripts route state through temp files to survive. Hermes already rewards programmatic tool calling (execute_code-only turns refund the iteration budget), which makes the missing half — state that survives between calls — the bottleneck. Add opt-in `code_execution.kernel_mode: session`: one persistent kernel per (task, mode, interpreter, cwd, tool-set). Variables, imports, and loaded data persist across calls; `reset=true` discards state on demand. The default `per-call` keeps today's behavior byte-for-byte. Safety posture is unchanged by design: the child env comes from the same builder as the per-call path (extracted, not duplicated, so the secret scrubbing / PYTHONPATH hygiene cannot drift), the RPC server is the same `_rpc_server_loop` with the same token and a per-cell tool budget, and output passes the same ANSI strip + secret redaction. A timed-out or interrupted cell kills the whole kernel tree and the next call respawns — a wedged kernel can never hang the agent. The kernel env is frozen at spawn; the schema and config comment say so. Wire protocol: NDJSON requests on the kernel's stdin; responses framed on stdout behind a per-kernel random sentinel, with unframed bytes (fd-level output from user-spawned subprocesses) attributed to the serialized current cell. The generated RPC client reconnects once when HERMES_RPC_PERSISTENT=1, because a kernel legitimately outlives the RPC server's 300s idle window between cells. Tested on macOS 15 (Apple Silicon), Python 3.11: 13 new tests in tests/tools/test_code_kernel.py (persistence, reset, error-keeps-kernel, timeout-kills-kernel, sys.exit ends kernel, subprocess fd passthrough, schema surface, mode fallback) plus the existing test_code_execution.py / test_code_execution_modes.py suites (81 passed). * fix(tools): session kernels get a stable owner, bounded lifetime, and per-cell RPC authority Addresses the blocking review on the session-kernel design: two authority/lifecycle boundaries were wrong. 1. Ownership and bounded lifetime. The kernel key's first component is now the conversation's approval session key (_resolve_owner), not the per-turn task id run_agent mints per top-level invocation — so state genuinely survives across user turns of one conversation, and delegated subagent sessions isolate naturally under their own keys (the task id remains only the last-resort owner for embeds/tests with no session context). Lifetime is bounded on four edges: kernels are disposed at the same session boundary that clears the owner's approval/yolo state (tools.approval.clear_session -> shutdown_kernels_for_owner), reaped after code_execution.kernel_idle_timeout seconds idle (default 1800, swept on every entry), capped process-wide at code_execution.max_session_kernels live children (default 4, LRU evicted), and still torn down by reset/death/atexit as before. The ownership + disposal + idle-reap + cap shape deliberately carries forward the lifecycle invariants of the earlier session-persistent implementation in NousResearch#88637 by @z80dev. 2. Per-cell RPC authority. The serving thread no longer freezes the spawning cell's context/callbacks for the kernel's life. Each cell installs a CellAuthority — captured on the calling thread exactly as propagate_context_to_thread would for a per-call RPC thread — before its request is written, and retires it on every settle path; _rpc_server_loop gains a dispatch hook the kernel uses to route each tool call through the CURRENT cell's context, callbacks, and task id. A call arriving with no active cell is refused. Interpreter state persists; RPC authority does not. Composition with the per-script static guard (see the config note): a persistent namespace lets cell N+1 invoke objects cell N created, which a single-cell static scan cannot see — the runtime RPC boundary (allow-list by name, per-cell budget, per-cell authority) is the operative cross-cell enforcement in this mode, and the adversarial alias test pins exactly that. Tests (9 new): state survives across turns of one conversation; sessions isolate; clear_session disposes the owner's kernels (and the next turn starts fresh); the live-kernel cap LRU-evicts with evicted children proven dead; idle kernels are reaped; a later cell's RPC runs under that cell's approval callback; a cross-cell alias dispatches under the CURRENT cell's authority; a settled cell's authority refuses dispatch; each cell installs a fresh authority. 22/22 kernel tests, 81 code-execution tests, ruff clean. The 7 test-order failures in the tools/-k-approval selection reproduce identically on the clean branch base (pre-existing pollution, not this change). * fix(code-kernel): delegated children get their own kernels — child contexts inherit the parent approval key, so qualify the owner with the delegation session id (live-verified leak, both directions) --------- Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
…sed semantics (NousResearch#65592 P0-3/P1-9 closeout) - _execute_code_has_package_acquisition: add os.exec* / os.posix_spawn* to the command-exec family scan (previously subprocess.*/os.system/ os.popen/os.spawn*/pty.spawn/asyncio.create_subprocess_* only) - Document fail-closed return semantics: _PACKAGE_UNRESOLVABLE when a command-exec call exists but argv is statically unresolvable — caller must owner-approve rather than allow - test_cross_cell_alias: narrow claim to the RPC-alias class only (native Python capabilities never cross RPC; cross-cell persistence is covered by the capability-leak detector, NousResearch#94647) (cherry picked from commit acec32f)
What does this PR do?
execute_codespawns a fresh Python process per call, so every multi-step data task re-loads its inputs: a CSV parsed in call one is gone by call two, and scripts end up routing state through temp files to survive. Hermes already rewards programmatic tool calling — execute_code-only turns refund the iteration budget (agent/conversation_loop.py) — which makes the missing half, state that survives between calls, the practical bottleneck.This adds an opt-in
code_execution.kernel_mode: session: one persistent kernel per (task, mode, interpreter, cwd, tool-set). Variables, imports, and loaded data persist across calls;reset=truediscards state on demand. The defaultper-callkeeps today's behavior byte-for-byte.Safety posture is unchanged by design:
_build_child_env()rather than duplicated, so the secret scrubbing / UTF-8 / TZ / PYTHONPATH-hygiene rules cannot drift between the two paths._rpc_server_loopwith the same per-kernel token; the tool-call budget applies per cell (counter resets each call).reset=true); the schema description and config comment both say so.Wire protocol: NDJSON requests on the kernel's stdin; responses framed on stdout behind a per-kernel random sentinel. Unframed bytes (fd-level output from subprocesses the user's code spawns) are attributed to the current cell — calls are serialized per kernel, so attribution is unambiguous. The generated RPC client reconnects once when
HERMES_RPC_PERSISTENT=1, because a kernel legitimately outlives the RPC server's 300s idle window between cells; the host re-accepts for the kernel's lifetime.Related Issue
No existing issue proposes persistent execute_code state (searched open+closed; #77367 covers other harness patterns — vibe-mode workers — but not kernel persistence). Happy to open a tracking issue first if preferred.
Type of Change
Changes Made
tools/code_kernel.py— new: kernel registry, runner source, frame reader, per-cell execution, teardown/atexittools/code_execution_tool.py—_build_child_env()extracted from the per-call spawn path (shared verbatim);KERNEL_MODES/_get_kernel_mode(); session branch inexecute_code()after all guards;resetschema param + session note; RPC client one-shot reconnect underHERMES_RPC_PERSISTENThermes_cli/config_defaults.py,cli-config.yaml.example—kernel_mode: per-calldefault with behavior commenttests/tools/test_code_kernel.py— 13 tests (see below)How to Test
python -m pytest tests/tools/test_code_kernel.py -q— 13 passed (persistence across cells, per-call default shares nothing, reset discards, an exception keeps the kernel alive, timeout kills the kernel and the next call respawns,sys.exit()ends it deliberately, subprocess fd output reaches the result, schema/mode-fallback surface)python -m pytest tests/tools/test_code_execution.py tests/tools/test_code_execution_modes.py -q— 81 passed (per-call regression)code_execution.kernel_mode: session, thenexecute_code(code="x=41")followed byexecute_code(code="print(x+1)")→42, result carries"kernel": {"mode": "session", "reused": true, "execution_count": 2}Checklist
Code
pytest tests/ -qnot run locally — missing optional heavy deps in my env)Documentation & Housekeeping
cli-config.yaml.examplefor the new config keypass_fds;CREATE_NO_WINDOWpreserved; frame protocol is platform-neutral — not run on a Windows box myself)resetparam)