mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(tui): scope session.interrupt pending-prompt release to the calling session (#12441)
session.interrupt on session A was blast-resolving pending clarify/sudo/secret prompts on ALL sessions sharing the same tui_gateway process. Other sessions' agent threads unblocked with empty-string answers as if the user had cancelled — silent cross-session corruption. Root cause: _pending and _answers were globals keyed by random rid with no record of the owning session. _clear_pending() iterated every entry, so the session.interrupt handler had no way to limit the release to its own sid. Fix: - tui_gateway/server.py: _pending now maps rid to (sid, Event) tuples. _clear_pending takes an optional sid argument and filters by owner_sid when provided. session.interrupt passes the calling sid so unrelated sessions are untouched. _clear_pending(None) remains the shutdown path for completeness. - _block and _respond updated to pack/unpack the new tuple format. Tests (tests/test_tui_gateway_server.py): 4 new cases. - test_interrupt_only_clears_own_session_pending: two sessions with pending prompts, interrupting one must not release the other. - test_interrupt_clears_multiple_own_pending: same-sid multi-prompt release works. - test_clear_pending_without_sid_clears_all: shutdown path preserved. - test_respond_unpacks_sid_tuple_correctly: _respond handles the tuple format. Also updated tests/tui_gateway/test_protocol.py to use the new tuple format for test_block_and_respond and test_clear_pending. Live E2E against the live Python environment confirmed cross-session isolation: interrupting sid_a released its own pending prompt without touching sid_b's. All 78 related tests pass.
This commit is contained in:
parent
ce410521b3
commit
dca439fe92
3 changed files with 144 additions and 11 deletions
|
|
@ -712,3 +712,119 @@ def test_prompt_submit_history_version_match_persists_normally(monkeypatch):
|
|||
finally:
|
||||
server._sessions.pop("sid", None)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# session.interrupt must only cancel pending prompts owned by the calling
|
||||
# session — it must not blast-resolve clarify/sudo/secret prompts on
|
||||
# unrelated sessions sharing the same tui_gateway process. Without
|
||||
# session scoping the other sessions' prompts silently resolve to empty
|
||||
# strings, unblocking their agent threads as if the user cancelled.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_interrupt_only_clears_own_session_pending():
|
||||
"""session.interrupt on session A must NOT release pending prompts
|
||||
that belong to session B."""
|
||||
import types
|
||||
|
||||
session_a = _session()
|
||||
session_a["agent"] = types.SimpleNamespace(interrupt=lambda: None)
|
||||
session_b = _session()
|
||||
session_b["agent"] = types.SimpleNamespace(interrupt=lambda: None)
|
||||
server._sessions["sid_a"] = session_a
|
||||
server._sessions["sid_b"] = session_b
|
||||
|
||||
try:
|
||||
# Simulate pending prompts on both sessions (what _block creates
|
||||
# while a clarify/sudo/secret request is outstanding).
|
||||
ev_a = threading.Event()
|
||||
ev_b = threading.Event()
|
||||
server._pending["rid-a"] = ("sid_a", ev_a)
|
||||
server._pending["rid-b"] = ("sid_b", ev_b)
|
||||
server._answers.clear()
|
||||
|
||||
# Interrupt session A.
|
||||
resp = server.handle_request(
|
||||
{"id": "1", "method": "session.interrupt", "params": {"session_id": "sid_a"}}
|
||||
)
|
||||
assert resp.get("result"), f"got error: {resp.get('error')}"
|
||||
|
||||
# Session A's pending must be released to empty.
|
||||
assert ev_a.is_set(), "sid_a pending Event should be set after interrupt"
|
||||
assert server._answers.get("rid-a") == ""
|
||||
|
||||
# Session B's pending MUST remain untouched — no cross-session blast.
|
||||
assert not ev_b.is_set(), (
|
||||
"CRITICAL: session.interrupt on sid_a released a pending prompt "
|
||||
"belonging to sid_b — other sessions' clarify/sudo/secret "
|
||||
"prompts are being silently cancelled"
|
||||
)
|
||||
assert "rid-b" not in server._answers
|
||||
finally:
|
||||
server._sessions.pop("sid_a", None)
|
||||
server._sessions.pop("sid_b", None)
|
||||
server._pending.pop("rid-a", None)
|
||||
server._pending.pop("rid-b", None)
|
||||
server._answers.pop("rid-a", None)
|
||||
server._answers.pop("rid-b", None)
|
||||
|
||||
|
||||
def test_interrupt_clears_multiple_own_pending():
|
||||
"""When a single session has multiple pending prompts (uncommon but
|
||||
possible via nested tool calls), interrupt must release all of them."""
|
||||
import types
|
||||
|
||||
sess = _session()
|
||||
sess["agent"] = types.SimpleNamespace(interrupt=lambda: None)
|
||||
server._sessions["sid"] = sess
|
||||
|
||||
try:
|
||||
ev1, ev2 = threading.Event(), threading.Event()
|
||||
server._pending["r1"] = ("sid", ev1)
|
||||
server._pending["r2"] = ("sid", ev2)
|
||||
|
||||
resp = server.handle_request(
|
||||
{"id": "1", "method": "session.interrupt", "params": {"session_id": "sid"}}
|
||||
)
|
||||
assert resp.get("result")
|
||||
assert ev1.is_set() and ev2.is_set()
|
||||
assert server._answers.get("r1") == "" and server._answers.get("r2") == ""
|
||||
finally:
|
||||
server._sessions.pop("sid", None)
|
||||
for key in ("r1", "r2"):
|
||||
server._pending.pop(key, None)
|
||||
server._answers.pop(key, None)
|
||||
|
||||
|
||||
def test_clear_pending_without_sid_clears_all():
|
||||
"""_clear_pending(None) is the shutdown path — must still release
|
||||
every pending prompt regardless of owning session."""
|
||||
ev1, ev2, ev3 = threading.Event(), threading.Event(), threading.Event()
|
||||
server._pending["a"] = ("sid_x", ev1)
|
||||
server._pending["b"] = ("sid_y", ev2)
|
||||
server._pending["c"] = ("sid_z", ev3)
|
||||
try:
|
||||
server._clear_pending(None)
|
||||
assert ev1.is_set() and ev2.is_set() and ev3.is_set()
|
||||
finally:
|
||||
for key in ("a", "b", "c"):
|
||||
server._pending.pop(key, None)
|
||||
server._answers.pop(key, None)
|
||||
|
||||
|
||||
def test_respond_unpacks_sid_tuple_correctly():
|
||||
"""After the (sid, Event) tuple change, _respond must still work."""
|
||||
ev = threading.Event()
|
||||
server._pending["rid-x"] = ("sid_x", ev)
|
||||
try:
|
||||
resp = server.handle_request(
|
||||
{"id": "1", "method": "clarify.respond",
|
||||
"params": {"request_id": "rid-x", "answer": "the answer"}}
|
||||
)
|
||||
assert resp.get("result")
|
||||
assert ev.is_set()
|
||||
assert server._answers.get("rid-x") == "the answer"
|
||||
finally:
|
||||
server._pending.pop("rid-x", None)
|
||||
server._answers.pop("rid-x", None)
|
||||
|
||||
|
|
|
|||
|
|
@ -120,7 +120,9 @@ def test_block_and_respond(capture):
|
|||
|
||||
rid = next(iter(server._pending))
|
||||
server._answers[rid] = "my_answer"
|
||||
server._pending[rid].set()
|
||||
# _pending values are (sid, Event) tuples — unpack to set the Event
|
||||
_, ev = server._pending[rid]
|
||||
ev.set()
|
||||
|
||||
threading.Event().wait(0.1)
|
||||
assert result[0] == "my_answer"
|
||||
|
|
@ -128,7 +130,8 @@ def test_block_and_respond(capture):
|
|||
|
||||
def test_clear_pending(server):
|
||||
ev = threading.Event()
|
||||
server._pending["r1"] = ev
|
||||
# _pending values are (sid, Event) tuples
|
||||
server._pending["r1"] = ("sid-x", ev)
|
||||
server._clear_pending()
|
||||
|
||||
assert ev.is_set()
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ from tui_gateway.render import make_stream_renderer, render_diff, render_message
|
|||
|
||||
_sessions: dict[str, dict] = {}
|
||||
_methods: dict[str, callable] = {}
|
||||
_pending: dict[str, threading.Event] = {}
|
||||
_pending: dict[str, tuple[str, threading.Event]] = {}
|
||||
_answers: dict[str, str] = {}
|
||||
_db = None
|
||||
_stdout_lock = threading.Lock()
|
||||
|
|
@ -296,7 +296,7 @@ def _enable_gateway_prompts() -> None:
|
|||
def _block(event: str, sid: str, payload: dict, timeout: int = 300) -> str:
|
||||
rid = uuid.uuid4().hex[:8]
|
||||
ev = threading.Event()
|
||||
_pending[rid] = ev
|
||||
_pending[rid] = (sid, ev)
|
||||
payload["request_id"] = rid
|
||||
_emit(event, sid, payload)
|
||||
ev.wait(timeout=timeout)
|
||||
|
|
@ -304,10 +304,19 @@ def _block(event: str, sid: str, payload: dict, timeout: int = 300) -> str:
|
|||
return _answers.pop(rid, "")
|
||||
|
||||
|
||||
def _clear_pending():
|
||||
for rid, ev in list(_pending.items()):
|
||||
_answers[rid] = ""
|
||||
ev.set()
|
||||
def _clear_pending(sid: str | None = None) -> None:
|
||||
"""Release pending prompts with an empty answer.
|
||||
|
||||
When *sid* is provided, only prompts owned by that session are
|
||||
released — critical for session.interrupt, which must not
|
||||
collaterally cancel clarify/sudo/secret prompts on unrelated
|
||||
sessions sharing the same tui_gateway process. When *sid* is
|
||||
None, every pending prompt is released (used during shutdown).
|
||||
"""
|
||||
for rid, (owner_sid, ev) in list(_pending.items()):
|
||||
if sid is None or owner_sid == sid:
|
||||
_answers[rid] = ""
|
||||
ev.set()
|
||||
|
||||
|
||||
# ── Agent factory ────────────────────────────────────────────────────
|
||||
|
|
@ -1345,7 +1354,11 @@ def _(rid, params: dict) -> dict:
|
|||
return err
|
||||
if hasattr(session["agent"], "interrupt"):
|
||||
session["agent"].interrupt()
|
||||
_clear_pending()
|
||||
# Scope the pending-prompt release to THIS session. A global
|
||||
# _clear_pending() would collaterally cancel clarify/sudo/secret
|
||||
# prompts on unrelated sessions sharing the same tui_gateway
|
||||
# process, silently resolving them to empty strings.
|
||||
_clear_pending(params.get("session_id", ""))
|
||||
try:
|
||||
from tools.approval import resolve_gateway_approval
|
||||
resolve_gateway_approval(session["session_key"], "deny", resolve_all=True)
|
||||
|
|
@ -1684,9 +1697,10 @@ def _(rid, params: dict) -> dict:
|
|||
|
||||
def _respond(rid, params, key):
|
||||
r = params.get("request_id", "")
|
||||
ev = _pending.get(r)
|
||||
if not ev:
|
||||
entry = _pending.get(r)
|
||||
if not entry:
|
||||
return _err(rid, 4009, f"no pending {key} request")
|
||||
_, ev = entry
|
||||
_answers[r] = params.get(key, "")
|
||||
ev.set()
|
||||
return _ok(rid, {"status": "ok"})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue