Skip to content

fix(api-server): cancel orphaned agent + true interrupt on SSE disconnect (salvage #3399) - #3427

Merged
teknium1 merged 2 commits into
mainfrom
hermes/hermes-a2b72b01
Mar 27, 2026
Merged

fix(api-server): cancel orphaned agent + true interrupt on SSE disconnect (salvage #3399)#3427
teknium1 merged 2 commits into
mainfrom
hermes/hermes-a2b72b01

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

Summary

Salvage of #3399 by @binhnt92 with true agent interruption added on top.

Problem: When a streaming /v1/chat/completions client disconnects mid-stream (network drop, browser tab close, Open WebUI navigation), response.write() raises ConnectionResetError but the agent task keeps running — making LLM API calls and consuming tokens with no one listening.

Original fix (#3399): Wrapped the SSE write loop in try/except to catch disconnect errors and cancel the asyncio task. However, agent_task.cancel() only marks the asyncio Future as cancelled — the underlying thread (via run_in_executor) continues running, still burning tokens.

Added on top: True agent interruption via agent.interrupt():

  • _run_agent() accepts an optional agent_ref mutable container and stores the AIAgent reference at agent_ref[0] before run_conversation() begins
  • On SSE disconnect, the except block calls agent.interrupt("SSE client disconnected") which sets _interrupt_requested and signals all tools to abort
  • The agent stops at the next loop iteration boundary — no more orphaned LLM API calls

Changes

  • gateway/platforms/api_server.py: disconnect handling + agent_ref wiring + interrupt call
  • tests/gateway/test_sse_agent_cancel.py: 6 tests (4 original + 2 new for interrupt behavior)

Test plan

python -m pytest tests/gateway/test_sse_agent_cancel.py -v  # 6 pass
python -m pytest tests/gateway/ -q                          # 1599 pass

Closes #3399. Original commit by @binhnt92 preserved via cherry-pick.

binhnt92 and others added 2 commits March 27, 2026 10:54
When a streaming /v1/chat/completions client disconnects mid-stream
(network drop, browser tab close), response.write raises
ConnectionResetError but the agent_task created via ensure_future
is never cancelled. The orphaned agent continues consuming API tokens
and memory with no reference held to it.

Wrap the SSE write loop in try/except to catch disconnect errors and
cancel the agent task in the except handler.
The original PR (#3399) caught disconnect errors and cancelled the
asyncio task, but run_in_executor tasks can't be interrupted by
asyncio cancellation — the agent thread keeps running and consuming
API tokens.

Wire agent.interrupt() via a mutable agent_ref container:
- _run_agent() stores the AIAgent at agent_ref[0] before run_conversation
- On SSE disconnect, the except block calls agent.interrupt() which sets
  _interrupt_requested and signals tools to abort
- The agent stops at the next loop iteration boundary

Added 2 tests: interrupt is called on disconnect, agent_ref=None still
handles disconnect gracefully.
@malaiwah

malaiwah commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

FYI: this PR's "true interrupt on SSE disconnect" path interacts with the process-wide singleton in tools/interrupt.py in a way that causes cross-session contamination on the API server when multiple completions run concurrently in the same process.

The session isolation at the agent_ref level holds — each request's agent.interrupt() only flips its own self._interrupt_requested for the LLM API-call polling loop. But interrupt() also fans out to _set_interrupt(True) (run_agent.py:2478), which sets a module-level threading.Event shared by every other agent in the process. The 11 long-running tools (terminal_tool, web_tools, vision_tools, browser_tool, all environment runners) check is_interrupted() from that singleton, so when client A disconnects, in-flight tool calls in concurrent request B return [interrupted] immediately and the LLM in B emits a truncated completion mid-task.

The reverse race exists too: any other request whose run_conversation() starts (e.g. an Open WebUI title-generation meta-request) calls clear_interrupt()_set_interrupt(False), which silently un-interrupts the agent this PR was trying to stop.

Verified on main at ff6a86cb. Filed as a follow-up in #4072 with full details. Not suggesting a revert — this PR does the right thing for single-session usage; the singleton in tools/interrupt.py is what needs to become per-AIAgent.

malaiwah pushed a commit to malaiwah/hermes-agent that referenced this pull request Apr 8, 2026
…ntamination

`tools/interrupt.py` exposed a single process-wide
`_interrupt_event = threading.Event()` shared by every concurrent
`AIAgent` in the process. When one agent's `interrupt()` was called
(e.g. an SSE client disconnecting on the API server), the global event
was set and every other concurrently-running agent's tools — terminal,
web_extract, browser, vision, all environment runners — observed
`is_interrupted() == True` and returned `[interrupted]` immediately,
producing truncated mid-task completions on unrelated requests.

The reverse race existed too: any other request whose
`run_conversation()` started would call `clear_interrupt()` →
`_set_interrupt(False)`, silently un-interrupting an agent that another
caller had legitimately stopped.

The session isolation at `self._interrupt_requested` only guarded the
LLM API-call polling loop; the 11 long-running tools that import
`is_interrupted` from `tools.interrupt` all observed the singleton.

Fix:

- Each `AIAgent` now owns its own `threading.Event`
  (`self._interrupt_event`).
- `tools/interrupt.py` adds a `contextvars.ContextVar` that holds the
  currently-active agent's event. `is_interrupted()` and
  `set_interrupt()` consult the bound event when present, falling back
  to the historical module-level singleton when none is bound (CLI
  single-agent usage and tests that import `_interrupt_event` directly
  keep working unchanged).
- `AIAgent.run_conversation()` becomes a thin wrapper that binds
  `self._interrupt_event` to the context variable, calls the renamed
  `_run_conversation_locked()` (the original body), and unbinds in a
  `finally`.
- `AIAgent.interrupt()` and `clear_interrupt()` write directly to
  `self._interrupt_event` rather than going through `set_interrupt`,
  because they are typically invoked from a different thread (gateway
  message handler, SSE disconnect handler) where the contextvar may be
  unbound or bound to a different agent.
- `_execute_tool_calls_concurrent` snapshots the current context with
  `contextvars.copy_context()` and hands each `ThreadPoolExecutor`
  worker a fresh `.copy()` to `run()` the tool in. Without this,
  worker threads would not inherit the binding, since neither
  `ThreadPoolExecutor.submit` nor `asyncio.loop.run_in_executor`
  propagates contextvars automatically.

Tests:

- New `tests/tools/test_interrupt_isolation.py` adds 8 regression
  tests covering the bind/unbind API, two-context independence,
  ThreadPoolExecutor inheritance, and end-to-end `AIAgent` cross-
  contamination scenarios.
- Existing `tests/run_agent/test_interrupt_propagation.py` updated:
  the previous `test_child_clear_interrupt_at_start_clears_global`
  asserted the bug-as-feature; it now asserts the new isolation
  contract under the name
  `test_child_clear_interrupt_does_not_affect_global`.
- All bare-constructed mock agents in `test_interrupt_propagation`,
  `test_real_interrupt_subagent`, and `test_cli_interrupt_subagent`
  receive a `_interrupt_event = threading.Event()`.
- `TestInterrupt` and `TestHydrateTodoStore` in `test_run_agent.py`
  drop the now-unused `patch("run_agent._set_interrupt")` context
  managers (the import alias is gone).
- `TestMemoryNudgeCounterPersistence::test_counters_not_reset_in_preamble`
  and `TestDeadRetryCode::test_no_unreachable_max_retries_after_backoff`
  now `inspect.getsource(AIAgent._run_conversation_locked)` since
  `run_conversation` is the binding wrapper and no longer holds the
  preamble or the retry loop.

Reported in NousResearch#4072 (thread safety) and NousResearch#3427 (the merged SSE-disconnect
PR that made this trigger reliable on multi-client API server
deployments).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@malaiwah

malaiwah commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Followed up with #6185 and a fix in #6186 that makes the interrupt event per-AIAgent (bound via contextvars.ContextVar for the duration of run_conversation). The SSE-disconnect path this PR added is correct in isolation; the real culprit was the process-wide singleton in tools/interrupt.py that turned a single-session interrupt into a fanout across every concurrent agent in the process.

angelburgosrosado pushed a commit to angelburgosrosado/hermes-agent that referenced this pull request Apr 27, 2026
…nect (salvage NousResearch#3399) (NousResearch#3427)

Salvage of NousResearch#3399 by @binhnt92 with true agent interruption added on top.

When a streaming /v1/chat/completions client disconnects mid-stream, the agent is now interrupted via agent.interrupt() so it stops making LLM API calls, and the asyncio task wrapper is cancelled.

Closes NousResearch#3399.
02356abc pushed a commit to 02356abc/hermes-agent that referenced this pull request May 14, 2026
…nect (salvage NousResearch#3399) (NousResearch#3427)

Salvage of NousResearch#3399 by @binhnt92 with true agent interruption added on top.

When a streaming /v1/chat/completions client disconnects mid-stream, the agent is now interrupted via agent.interrupt() so it stops making LLM API calls, and the asyncio task wrapper is cancelled.

Closes NousResearch#3399.
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
…nect (salvage NousResearch#3399) (NousResearch#3427)

Salvage of NousResearch#3399 by @binhnt92 with true agent interruption added on top.

When a streaming /v1/chat/completions client disconnects mid-stream, the agent is now interrupted via agent.interrupt() so it stops making LLM API calls, and the asyncio task wrapper is cancelled.

Closes NousResearch#3399.
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…nect (salvage NousResearch#3399) (NousResearch#3427)

Salvage of NousResearch#3399 by @binhnt92 with true agent interruption added on top.

When a streaming /v1/chat/completions client disconnects mid-stream, the agent is now interrupted via agent.interrupt() so it stops making LLM API calls, and the asyncio task wrapper is cancelled.

Closes NousResearch#3399.
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
…nect (salvage NousResearch#3399) (NousResearch#3427)

Salvage of NousResearch#3399 by @binhnt92 with true agent interruption added on top.

When a streaming /v1/chat/completions client disconnects mid-stream, the agent is now interrupted via agent.interrupt() so it stops making LLM API calls, and the asyncio task wrapper is cancelled.

Closes NousResearch#3399.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants