Skip to content

fix(gateway): set session contextvars in api_server _run_agent so notify_on_complete works - #19402

Closed
Bartok9 wants to merge 1 commit into
NousResearch:mainfrom
Bartok9:bartok9/tea-17-fix-notify-on-complete-api-server
Closed

fix(gateway): set session contextvars in api_server _run_agent so notify_on_complete works#19402
Bartok9 wants to merge 1 commit into
NousResearch:mainfrom
Bartok9:bartok9/tea-17-fix-notify-on-complete-api-server

Conversation

@Bartok9

@Bartok9 Bartok9 commented May 3, 2026

Copy link
Copy Markdown
Contributor

Summary

terminal(background=True, notify_on_complete=True) is silently a no-op when the agent runs through the API server path (WebUI / Dashboard / any OpenAI-compatible client). The terminal tool's watcher-registration block in tools/terminal_tool.py:1932-1944 is gated on HERMES_SESSION_PLATFORM being non-empty (read via gateway.session_context.get_session_env), but APIServerAdapter._run_agent never seeded those contextvars before scheduling the agent into the executor — and even if it had, plain loop.run_in_executor doesn't propagate contextvars into the worker thread. Result: the watcher gate evaluates False, the entire registration block is skipped, no error, no warning. Works fine on Telegram / Discord / Slack because GatewayRunner calls _set_session_env at gateway/run.py:5722 before each turn.

What changed

gateway/platforms/api_server.py::_run_agent now wraps the body in set_session_vars / clear_session_vars (from gateway.session_context) and threads a captured copy_context() into loop.run_in_executor(None, ctx.run, _run) so the contextvars are visible to terminal_tool running in the executor thread. This mirrors the GatewayRunner._set_session_env pattern at gateway/run.py:10677.

Watcher registration on the API server path now succeeds. Watcher delivery (routing the completion notification back to an HTTP client without a persistent message channel) is a separate, larger problem and is intentionally out of scope here — maintainers' call on whether to wire it through dependency injection from GatewayRunner or via a polling/SSE endpoint.

Test coverage

Two regression tests added in tests/gateway/test_api_server.py::TestAgentExecution:

  • test_run_agent_sets_session_contextvars_for_executor reads HERMES_SESSION_PLATFORM, HERMES_SESSION_CHAT_ID, and HERMES_SESSION_KEY from inside the agent's `run_conversation` callback (which executes in the executor thread, where terminal_tool's lookup actually happens) and asserts they're populated to \"api_server\" and the session id.
  • test_run_agent_clears_session_contextvars_after_run asserts the vars are cleared after the run so a subsequent unrelated request on the same asyncio task can't observe stale routing data.

Targeted run: `pytest tests/gateway/test_api_server.py -o addopts=''` → 126 passed, no regressions. Existing test_run_agent_uses_session_id_as_task_id still passes unchanged.

Fixes #10760

Made with Cursor

…ify_on_complete works

When the agent runs through the API server path (WebUI, Dashboard, any
OpenAI-compatible client), `terminal(background=True, notify_on_complete=True)`
was silently a no-op. The terminal tool's watcher-registration block in
`tools/terminal_tool.py` is gated on `HERMES_SESSION_PLATFORM` being non-empty
(read via `gateway.session_context.get_session_env`), but
`APIServerAdapter._run_agent` never seeded those contextvars before scheduling
the agent into the executor — and even if it had, plain `loop.run_in_executor`
doesn't propagate contextvars into the worker thread. Result: the watcher
gate evaluated False, the entire registration block was skipped, no error,
no warning. Works fine on Telegram/Discord/Slack because `GatewayRunner`
calls `_set_session_env` at `gateway/run.py:5722` before each turn.

This change wraps `_run_agent` with `set_session_vars` / `clear_session_vars`
and threads the captured `copy_context()` into `loop.run_in_executor` so the
contextvars are visible to `terminal_tool` running in the executor thread.
Mirrors the `GatewayRunner._set_session_env` pattern. Watcher *registration*
on the API server path now succeeds; watcher *delivery* (routing the
completion notification back to an HTTP client without a persistent message
channel) is a separate, larger problem and is intentionally out of scope —
maintainers' call on whether to wire that through dependency injection from
GatewayRunner or via a polling/SSE endpoint.

Two regression tests added in `TestAgentExecution`:
- `test_run_agent_sets_session_contextvars_for_executor` reads
  `HERMES_SESSION_PLATFORM`, `HERMES_SESSION_CHAT_ID`, and
  `HERMES_SESSION_KEY` from inside the agent's `run_conversation` callback
  (which executes in the executor thread, where `terminal_tool`'s lookup
  actually happens) and asserts they're populated.
- `test_run_agent_clears_session_contextvars_after_run` asserts the vars
  are cleared after the run so a subsequent unrelated request on the same
  asyncio task can't observe stale routing data.

Targeted run: `pytest tests/gateway/test_api_server.py -o addopts=''` →
126 passed, no regressions. Existing
`test_run_agent_uses_session_id_as_task_id` still passes unchanged.

Fixes NousResearch#10760

Co-authored-by: Cursor <cursoragent@cursor.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery tool/terminal Terminal execution and process management labels May 3, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #10760 (original bug report) and #10914 (earlier fix attempt). This PR addresses the same contextvars gap in the API server path.

1 similar comment
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #10760 (original bug report) and #10914 (earlier fix attempt). This PR addresses the same contextvars gap in the API server path.

@Bartok9

Bartok9 commented May 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @alt-glitch — yes, this directly addresses #10760 and also closes the gap left open by the earlier attempt in #10914. The key difference from #10914 is that this fix sets the ContextVar HERMES_SESSION_PLATFORM before the executor runs (inside _run_agent), rather than trying to patch the watcher registration itself. This ensures the entire execution context — including the terminal tool's watcher block — sees a non-empty platform value. Happy to clarify any implementation details if useful for review.

@Bartok9

Bartok9 commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

Changes present in upstream main after rebase. Closing as resolved.

@teknium1

Copy link
Copy Markdown
Contributor

For the record: #10760 is now fixed on main via #50319 (#50319).

Your PR called the split exactly right — the contextvar-registration fix landed, and you flagged watcher delivery as "a separate, larger problem ... maintainers' call." That call: there is no spec-compliant surface to wake the agent on a stateless HTTP client (every route closes its channel at turn end), so #50319 makes the no-op honest instead — a per-adapter supports_async_delivery flag (API server = off) so terminal tells the agent to poll and delegate_task falls back to synchronous execution. Thanks for the clean scoping here.

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

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: notify_on_complete silently fails on API Server (WebUI/Dashboard) path

3 participants