From dca439fe9213f86c83fdd43f70bf6e1750902b54 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 19 Apr 2026 00:03:58 -0700 Subject: [PATCH] fix(tui): scope session.interrupt pending-prompt release to the calling session (#12441) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tests/test_tui_gateway_server.py | 116 +++++++++++++++++++++++++++++ tests/tui_gateway/test_protocol.py | 7 +- tui_gateway/server.py | 32 +++++--- 3 files changed, 144 insertions(+), 11 deletions(-) diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 8831efb89..07a68ac9e 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -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) + diff --git a/tests/tui_gateway/test_protocol.py b/tests/tui_gateway/test_protocol.py index eb51cccfe..926dfadf1 100644 --- a/tests/tui_gateway/test_protocol.py +++ b/tests/tui_gateway/test_protocol.py @@ -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() diff --git a/tui_gateway/server.py b/tui_gateway/server.py index c58c65763..921f868a3 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -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"})