fix(delegation): bind subagent.interrupt to the caller's session, like subagent.steer - #80988
Open
pierrenode wants to merge 1 commit into
Open
Conversation
…e subagent.steer steer_subagent() (this week's 60e1f75/a94ebf5f5/9d4ef04ed series) was hardened to require exact owner_session_id + owner_transport (ContextVar- bound, unforgeable) + owner_session_record identity before a steer is accepted -- closing a cross-session authority hole. interrupt_subagent(), the function steer_subagent()'s own docstring says it mirrors, was never given the same treatment: it takes only subagent_id and performs zero ownership validation. The subagent.interrupt RPC handler didn't even resolve a caller identity. _active_subagents is a bare module-level dict, not session- or profile-scoped. delegation.status (list_active_subagents()) returns every active subagent's id system-wide with no session filter, so any session could enumerate ids and then hard-stop a child it does not own via subagent.interrupt. Fix mirrors steer_subagent()'s exact ownership contract: - tools/delegate_tool.py::interrupt_subagent() gains owner_session_id/ owner_transport/owner_session_record kwargs, checked the same way steer_subagent() checks them. owner_session_id=None (the default) deliberately preserves the internal in-process helper contract (test harnesses, same-process callers) -- the one existing caller (tests/agent/test_interrupt_compat.py) needs no changes. - tui_gateway/methods_session.py's subagent.interrupt handler now resolves caller authority via _current_session_steer_authority(), identical to subagent.steer's handler. - The only production client, ui-tui's agentsOverlay.tsx, never sent a session_id at all -- without threading it through, the ownership check would have silently broken the interrupt button for every legitimate caller (invoking_transport/invoking_session would always resolve to None). Added a sessionId prop (AgentsOverlayPane -> ui.sid, mirroring ModelPicker's existing sessionId threading) and pass session_id in the RPC params. Testing: - Added registry-level ownership tests for interrupt_subagent() (owned succeeds, foreign session denied, foreign transport with correct session_id denied) and a TestSubagentInterruptRPC class mirroring the existing TestSubagentSteerRPC coverage (missing subagent_id, owned child interrupted, unknown child not-found, foreign session/transport denied, no invoking identity denied, recycled id does not inherit old authority) to tests/tools/test_subagent_steer.py. - Mutation-verified: stashed the backend fix, confirmed all 7 new authority-check tests fail on pre-fix code (assert True is False / KeyError: 'error'). - tests/tools/test_subagent_steer.py (41), tests/agent/test_interrupt_compat.py, tests/tools/test_delegate.py + 5 neighboring delegate/subagent suites (96), and the full tests/test_tui_gateway_server.py (517/518, the one failure is a pre-existing threading-timing flake in an unrelated write_json test, confirmed by re-running in isolation 5x clean and against pre-fix code) all pass. - ui-tui: npm run typecheck clean; full vitest suite 138/138 files, 1528 passed after building the hermes-ink package dependency (pre-existing missing-build-artifact issue, unrelated to this change). - ruff check clean on all changed Python files. Note: PR NousResearch#70899 ("Mission Control" async-delegation panel, branched 2026-07-24 before the steer_subagent hardening landed) adds a parallel send_to_subagent() whose docstring explicitly states it matches interrupt_subagent()'s current no-ownership-check posture, and touches the same four files this fix touches. The maintainer has commented it will be salvaged/expanded. This fix does not depend on or block that PR -- it hardens a real, currently-exploitable gap in main today -- but whichever lands second will likely need a rebase.
28 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This week's
60e1f7517/a94ebf5f5/9d4ef04edseries hardenedsteer_subagent()to require exactowner_session_id+owner_transport(ContextVar-bound, unforgeable) +owner_session_recordidentity before a steer is accepted, closing a cross-session authority hole.interrupt_subagent()— whichsteer_subagent()'s own docstring says it mirrors ("The redirection-side mirror ofinterrupt_subagent()") — never got the same treatment: it takes onlysubagent_idand performs zero ownership validation. Thesubagent.interruptRPC handler doesn't even resolve a caller identity before calling it._active_subagentsis a bare module-level dict, not session- or profile-scoped.delegation.status(list_active_subagents()) returns every active subagent's id system-wide with no session filter, so any session can enumerate ids and then hard-stop a child it does not own viasubagent.interrupt.Fix
Mirrors
steer_subagent()'s exact ownership contract:tools/delegate_tool.py::interrupt_subagent()gainsowner_session_id/owner_transport/owner_session_recordkwargs, checked identically tosteer_subagent().owner_session_id=None(the default) deliberately preserves the internal in-process helper contract — the one existing caller (tests/agent/test_interrupt_compat.py) needs no changes.tui_gateway/methods_session.py'ssubagent.interrupthandler now resolves caller authority via_current_session_steer_authority(), identical tosubagent.steer's handler.ui-tui'sagentsOverlay.tsx, never sent asession_idat all — without threading it through, the ownership check would have silently broken the interrupt button for every legitimate caller. Added asessionIdprop (AgentsOverlayPane→ui.sid, mirroringModelPicker's existingsessionIdthreading pattern) and passsession_idin the RPC params.Testing
interrupt_subagent()(owned succeeds, foreign session denied, foreign transport with correct session_id denied) and aTestSubagentInterruptRPCclass mirroring the existingTestSubagentSteerRPCcoverage totests/tools/test_subagent_steer.py.tests/tools/test_subagent_steer.py(41),tests/agent/test_interrupt_compat.py,tests/tools/test_delegate.py+ 5 neighboring delegate/subagent suites (96 tests), and the fulltests/test_tui_gateway_server.py(517/518 — the one failure is a pre-existing threading-timing flake in an unrelatedwrite_jsontest, confirmed by isolated re-runs and against pre-fix code) all pass.ui-tui:npm run typecheckclean; full vitest suite 138/138 files, 1528 passed.ruff checkclean on all changed Python files.Note on a related open PR
#70899 ("Mission Control" async-delegation panel, branched 2026-07-24 before the
steer_subagenthardening landed) adds a parallelsend_to_subagent()whose docstring explicitly states it matchesinterrupt_subagent()'s current no-ownership-check posture, and touches the same four files this PR touches. The maintainer has commented it will be salvaged/expanded. This fix doesn't depend on or block that PR — it hardens a real, currently-exploitable gap inmaintoday — but whichever lands second will likely need a rebase against the other.