Skip to content

fix(gateway): keep abandoned-turn reapers in the session profile - #112845

Closed
poijygfdyy wants to merge 3 commits into
NousResearch:mainfrom
poijygfdyy:fix/gateway-reaper-profile-scope
Closed

poijygfdyy wants to merge 3 commits into
NousResearch:mainfrom
poijygfdyy:fix/gateway-reaper-profile-scope

Conversation

@poijygfdyy

@poijygfdyy poijygfdyy commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Problem

Abandoned-turn cleanup can run on bare threading.Thread reapers after a /stop-style interrupt or API-server turn abandonment. In a multiplexed gateway, those threads start without the caller's contextvars.Context, so process-registry cleanup can resolve get_hermes_home() to the launch profile instead of the served session profile.

That breaks the profile-isolation invariant for process checkpoint/result/config reads and writes during cleanup.

Fixes #112968.

Invariant

Every abandoned-turn reaper must execute under the same profile scope as the turn it is cleaning up. A reaper must never fall back to the process launch HERMES_HOME merely because finalization crossed a thread boundary.

Deterministic RED

tests/gateway/test_reaper_profile_scope.py drives the real _interrupt_running_turn path with launch home A and a context-local served home B, then observes process_registry.kill_started_since() from the spawned reaper thread.

On the unpatched path the thread is bare, so it starts with an empty Context and observes A. The regression requires B.

Fix

Capture the caller context at the thread-spawn boundary and execute _reap_gateway_turn_processes(...) through copy_context().run(...).

This PR applies that mechanism to the two abandoned-turn reaper sites:

  • gateway/run_agent_cache.py/stop / /new / eviction interrupt reaper;
  • gateway/platforms/api_server.py — API turn-abandonment reaper.

Generation/epoch guards, process ownership, reaper timing, and _reap_gateway_turn_processes itself are unchanged.

The earlier unrelated aiohttp serveraiohttp web server wording was removed and folded out of the final history. Current head b39ea694d9ad3f06a4c452de2f8fcd75bc4174f1 is three commits (RED test + the two production spawn-site fixes) and the final diff is only the two Context propagation sites plus the focused regression.

Collision / consolidation alignment

Searched current code plus open/recent PRs, issues, and commits for _reap_gateway_turn_processes, process-registry profile scope, copy_context, and the affected files/root cause.

Validation

  • Deterministic regression included for the real gateway interrupt path.
  • @KeyArgo independently reproduced the regression on pristine upstream main: the reaper observed the launch home; the prior functional head passed the focused test.
  • fix: background thread hops keep the routed profile scope (salvage #112845 #112599 #112593 #112938) #113063 carries the same reaper mechanism in its combined validation; this PR does not claim that consolidation PR's hosted CI as its own.
  • The extra inactivity-watchdog test briefly added while auditing the wider thread-hop class was removed again because it did not belong to this focused reaper PR and had no production delta here.
  • Repository-wide/canonical tests are not claimed in this environment.
  • Exact-head fork workflows remain subject to the external-contributor approval gate; no hosted-CI pass or failure is claimed unless jobs actually execute.

This PR is ready for review.

@KeyArgo

KeyArgo commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Independently reproduced the boundary on this branch (Linux, CPython 3.11, pytest):

  • pristine upstream/main 948e9706618 + this PR's test file only → FAILED: the reaper resolved the launch home (assert [PosixPath('.../launch')] == [PosixPath('.../served')]), i.e. the clean-up really did run outside the served profile's scope.
  • this head 4f5e56b9f64 + the same file → 1 passed.

Both spawn sites now snapshot on the caller's thread, which is the correct place for copy_context() (before threading.Thread(...).start()); the API-server abandonment reaper and the /stop/eviction reaper therefore share one contract.

Two non-blocking notes:

  1. The gateway/platforms/api_server.py docstring edit (aiohttp serveraiohttp web server) is unrelated to this fix — a reviewer applying the one-logical-change-per-PR rule may ask for it to be dropped.
  2. If any part of the reaped clean-up path can reach tool dispatch, tools/thread_context.propagate_context_to_thread() is the canonical wrapper for a thread hop: it carries the profile scope and installs the per-thread approval/sudo/unlock prompt callbacks, where a bare copy_context().run carries only ContextVars. For a pure process_registry reap the bare form fully covers the invariant stated here, so this is a consistency question, not a defect.

Copy link
Copy Markdown
Contributor Author

Thanks for the independent check. Two scope decisions here:

  1. The aiohttp serveraiohttp web server docstring wording is unrelated to the fix; current main still has the original wording, so I agree it is review noise rather than part of the mechanism. It has no behavioral role and I’m not using it as evidence for this PR.
  2. For the reaper hop itself, copy_context().run(...) is deliberate. This path is pure process_registry cleanup and does not dispatch tools, so carrying the approval/sudo/unlock callbacks installed by tools.thread_context.propagate_context_to_thread() would broaden the thread contract without a consumer. If that cleanup path ever reaches tool dispatch, the canonical wrapper becomes the right boundary.

The functional scope remains only profile Context propagation at the two abandoned-turn reaper spawn sites.

@poijygfdyy
poijygfdyy force-pushed the fix/gateway-reaper-profile-scope branch from b909f45 to b39ea69 Compare September 16, 2026 17:17
@teknium1

Copy link
Copy Markdown
Collaborator

Landed on main in #113063 (26e9205653) with your commit cherry-picked and authorship intact — thank you, @poijygfdyy. Closing here since all four thread-scope fixes went through one carrier.

@teknium1 teknium1 closed this Sep 16, 2026
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.

Abandoned-turn reaper threads can fall back to the launch profile

3 participants