RomanNum3ral
/
hermes-agent
mirror of https://github.com/NousResearch/hermes-agent.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(tui): spawn slash workers on demand instead of one per session 

Every slash_worker child runs its own MCP discovery (#61891), which
forks the full configured stdio MCP fleet — on a config with a handful
of stdio servers that is ~20 OS processes per worker once npx/cmd
wrappers are counted. The gateway pre-warmed a worker for every session
at create/build time, and sessions held by a live transport are (by
design) never reaped, so a desktop app left open for days accumulates
one fleet per retained session. On a real setup this reached ~120
processes across 6 sessions and pushed Windows commit charge to the
point where CreateProcess started failing system-wide ("Not enough
memory resources are available to process this command").

slash.exec already spawns a worker on demand when the session has none
and already recovers from a dead worker the same way, so the eager
pre-warm is pure pre-warming:

- drop the pre-warm in the deferred session-build path
- drop the pre-warm in _init_session
- make _restart_slash_worker a no-op for sessions that never spawned a
  worker (the next slash.exec builds one with the current session
  key/model, so no stale-key worker can exist)

Only sessions that actually run a worker-routed slash command now pay
for a fleet. Cost: the first such command in a session takes the CLI
build + MCP discovery hit that session.create used to absorb.

Tests: the two create/close-race guards now assert the build thread
never constructs a worker (the notify-unregister guarantees are kept);
the restart-orphan guard seeds a live worker so the close path is still
exercised; new test pins the restart no-op for workerless sessions.
This commit is contained in:
Ne0teric 2026-07-17 23:53:33 -07:00 committed by Teknium
parent 410877c7e1
commit 5aa3536b32
2 changed files with 82 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
tests/test_tui_gateway_server.py
View File

@ -8368,7 +8368,9 @@ def test_mirror_slash_compress_honors_here_argument(monkeypatch):
# ---------------------------------------------------------------------------
# session.create / session.close race: fast /new churn must not orphan the
# slash_worker subprocess or the global approval-notify registration.
# global approval-notify registration. (Slash workers are no longer pre-warmed
# by the build thread — slash.exec spawns them on demand — so the build thread
# must ALSO never construct one here.)
# ---------------------------------------------------------------------------
@ -8376,12 +8378,13 @@ def test_mirror_slash_compress_honors_here_argument(monkeypatch):
def test_session_create_close_race_does_not_orphan_worker(monkeypatch):
"""Regression guard: if session.close runs while session.create's
_build thread is still constructing the agent, the build thread
must detect the orphan and clean up the slash_worker + notify
registration it's about to install. Without the cleanup those
resources leak — the subprocess stays alive until atexit and the
notify callback lingers in the global registry."""
must detect the orphan and unregister the notify registration it's
about to install. It must also never pre-warm a slash worker (each
worker forks the full stdio MCP fleet; spawn is on-demand in
slash.exec) — a worker constructed here would be a regression."""
import threading
created_workers: list[str] = []
closed_workers: list[str] = []
unregistered_keys: list[str] = []
@ -8389,6 +8392,7 @@ def test_session_create_close_race_does_not_orphan_worker(monkeypatch):
def __init__(self, key, model, profile_home=None):
self.key = key
self._closed = False
created_workers.append(key)
def close(self):
self._closed = True
@ -8472,23 +8476,24 @@ def test_session_create_close_race_does_not_orphan_worker(monkeypatch):
)
assert close_resp.get("result", {}).get("closed") is True
# At this point session.close saw slash_worker=None (not yet
# installed) so it didn't close anything. Release the build thread
# and let it finish — it should detect the orphan and clean up the
# worker it just allocated + unregister the notify.
# At this point session.close saw slash_worker=None (never eagerly
# installed) so it had nothing to close. Release the build thread
# and let it finish — it should detect the orphan and unregister
# the notify, without ever having constructed a worker.
release_build.set()
# Give the build thread a moment to run through its finally.
for _ in range(100):
if closed_workers:
if unregistered_keys:
break
import time
time.sleep(0.02)
assert (
len(closed_workers) == 1
), f"orphan worker was not cleaned up — closed_workers={closed_workers}"
assert created_workers == [], (
f"build thread pre-warmed a slash worker (spawn must stay on-demand "
f"in slash.exec) — created_workers={created_workers}"
)
# Notify may be unregistered by both session.close (unconditional)
# and the orphan-cleanup path; the key guarantee is that the build
# thread does at least one unregister call (any prior close
@ -8502,8 +8507,9 @@ def test_session_create_close_race_does_not_orphan_worker(monkeypatch):
@pytest.mark.real_agent_prewarm
def test_session_create_no_race_keeps_worker_alive(monkeypatch):
"""Regression guard: when session.close does NOT race, the build
thread must install the worker + notify normally and leave them
alone (no over-eager cleanup)."""
thread must install the notify normally and leave it alone (no
over-eager cleanup) — and must not pre-warm a slash worker (spawn
is on-demand in slash.exec)."""
closed_workers: list[str] = []
unregistered_keys: list[str] = []
@ -8586,8 +8592,9 @@ def test_session_create_no_race_keeps_worker_alive(monkeypatch):
own_unregistered == []
), f"build thread unregistered its own notify despite no race: {own_unregistered}"
# Session should have the live worker installed.
assert session.get("slash_worker") is not None
# No pre-warmed worker: slash.exec spawns on demand, so a fresh
# session that hasn't run a worker-routed command carries None.
assert session.get("slash_worker") is None
finally:
# Cleanup + restore sibling sessions we snapshotted.
server._sessions.clear()
@ -11970,7 +11977,8 @@ def test_attach_worker_stores_worker_on_live_session():
def test_restart_slash_worker_closes_orphan_when_session_reaped(monkeypatch):
"""Post-turn restart of a session reaped mid-flight (e.g. close_on_disconnect
fired while `running` flipped false) must close the fresh worker, not orphan it."""
fired while `running` flipped false) must close both the stale worker and
the fresh replacement, not orphan either."""
closed = []
class _FakeWorker:
@ -11982,11 +11990,14 @@ def test_restart_slash_worker_closes_orphan_when_session_reaped(monkeypatch):
monkeypatch.setattr(server, "_SlashWorker", _FakeWorker)
server._sessions.pop("reaped", None)
reaped = {"session_key": "k"} # not in _sessions -> torn down concurrently
# not in _sessions -> torn down concurrently; carries a live worker so the
# restart path actually runs (a workerless session is a restart no-op now)
reaped = {"session_key": "k", "slash_worker": _FakeWorker()}
server._restart_slash_worker("reaped", reaped)
assert closed == [True]
assert reaped.get("slash_worker") is None
# stale worker closed by the restart, fresh worker closed by _attach_worker
# (sid no longer maps to this session)
assert closed == [True, True]
assert "reaped" not in server._sessions
@ -11999,15 +12010,41 @@ def test_restart_slash_worker_stores_on_live_session(monkeypatch):
pass
monkeypatch.setattr(server, "_SlashWorker", _FakeWorker)
live = {"session_key": "k", "slash_worker": None}
old_worker = _FakeWorker()
live = {"session_key": "k", "slash_worker": old_worker}
server._sessions["live-restart"] = live
try:
server._restart_slash_worker("live-restart", live)
assert isinstance(live["slash_worker"], _FakeWorker)
assert live["slash_worker"] is not old_worker
finally:
server._sessions.pop("live-restart", None)
def test_restart_slash_worker_noop_without_worker(monkeypatch):
"""A session that never spawned a worker (slash.exec not used yet) must
stay workerless across a restart — spawning here would fork the per-worker
stdio MCP fleet for sessions that never run worker-routed commands."""
spawned = []
class _FakeWorker:
def __init__(self, *a, **k):
spawned.append(True)
def close(self):
pass
monkeypatch.setattr(server, "_SlashWorker", _FakeWorker)
live = {"session_key": "k", "slash_worker": None}
server._sessions["lazy-noop"] = live
try:
server._restart_slash_worker("lazy-noop", live)
assert spawned == []
assert live["slash_worker"] is None
finally:
server._sessions.pop("lazy-noop", None)
def test_session_close_rpc_claims_then_tears_down(monkeypatch):
seen = []
claimed = {"session_key": "k"}

50
tui_gateway/server.py
View File

@ -1763,15 +1763,15 @@ def _start_agent_build(sid: str, session: dict) -> None:
# override is still active here.
current["config_model_seen"] = _config_model_target()
try:
worker = _SlashWorker(
key,
getattr(agent, "model", _resolve_model()),
profile_home=current.get("profile_home"),
)
_attach_worker(sid, current, worker)
except Exception:
pass
# No eager slash-worker pre-warm: slash.exec spawns one on demand
# (its error path already relies on that respawn to recover from a
# dead worker). Each worker child runs its own MCP discovery
# (#61891), so pre-warming one per session forks the full stdio
# MCP fleet — ~20 OS processes per retained session on a config
# with a few stdio servers — even for sessions that never run a
# worker-routed command. Sessions held by a live transport are
# never reaped, so with the desktop app open for days those
# fleets accumulate until the OS refuses new process spawns.
try:
from tools.approval import (
@ -3393,11 +3393,16 @@ def _tool_lifecycle_required_for_ui(name: str) -> bool:
def _restart_slash_worker(sid: str, session: dict):
worker = session.get("slash_worker")
if worker:
try:
worker.close()
except Exception:
pass
# A session that never spawned a worker has nothing stale to replace —
# the next slash.exec builds one with the current session key/model.
# Spawning here would fork the per-worker stdio MCP fleet for sessions
# that never use worker-routed commands.
if worker is None:
return
try:
worker.close()
except Exception:
pass
try:
new_worker = _SlashWorker(
session["session_key"],
@ -5631,19 +5636,10 @@ def _init_session(
except Exception:
pass
_register_session_cwd(_sessions[sid])
try:
_attach_worker(
sid,
_sessions[sid],
_SlashWorker(
key,
getattr(agent, "model", _resolve_model()),
profile_home=_sessions[sid].get("profile_home"),
),
)
except Exception:
# Defer hard-failure to slash.exec; chat still works without slash worker.
_sessions[sid]["slash_worker"] = None
# No eager slash-worker pre-warm — the session dict already carries
# slash_worker=None and slash.exec builds one on demand. See the
# deferred-build path in _start_agent_build for the full rationale
# (per-worker MCP fleets accumulating across retained sessions).
try:
from tools.approval import register_gateway_notify, load_permanent_allowlist