Skip to content

test(esc): pin async-subagent abort-controller isolation invariants#153

Merged
ericleepi314 merged 1 commit into
mainfrom
test/subagent-abort-isolation
May 16, 2026
Merged

test(esc): pin async-subagent abort-controller isolation invariants#153
ericleepi314 merged 1 commit into
mainfrom
test/subagent-abort-isolation

Conversation

@ericleepi314
Copy link
Copy Markdown
Collaborator

Summary

  • Critic on PR fix(esc): override cancelled tool_result with REJECT_MESSAGE in production path #150 flagged a defensive concern: when the user presses ESC and then types please resume, the engine's reset_abort_controller swaps in a fresh controller. If a subagent spawned in the previous turn held a stale (aborted) parent controller snapshot, the REJECT_MESSAGE override in _dispatch_single_tool would fire for every tool the subagent ran — spurious user-rejected results.
  • Verified: no bug exists in current Python. The code at src/agent/run_agent.py:281-287 already mirrors TS typescript/src/tools/AgentTool/runAgent.ts:533-541. Async agents get a fresh, unlinked AbortController(); sync agents share params.parent_context.abort_controller (a current reference, mutated in place by engine.reset_abort_controller — no snapshot staleness possible).
  • This PR is pure regression-test coverage. No production-code change. The 4 tests pin the invariants so a future refactor that switches the async path to a child-linked controller (which WOULD propagate parent abort and re-introduce the bug) is caught immediately.

Tests

  1. test_async_subagent_context_has_fresh_unlinked_controller — async path: explicit override IS the fresh instance, parent abort does NOT propagate.
  2. test_sync_subagent_shares_parent_controller_reference — sync path: child.abort_controller IS the parent's controller (same object).
  3. test_async_subagent_unaffected_by_parent_reset — end-to-end: parent ESC + parent controller swap (simulating engine reset), async subagent controller remains untouched.
  4. test_create_subagent_context_default_path_is_child_linked — default fallback: parent-abort propagates down (one-way); child-abort does NOT propagate up.

TS parity

Path TS reference Python mirror
Async agent runAgent.ts:533-541new AbortController() run_agent.py:281-284AbortController()
Sync agent runAgent.ts:537toolUseContext.abortController run_agent.py:287params.parent_context.abort_controller
Fallback forkedAgent.ts:354createChildAbortController(parent) subagent_context.py:86create_child_abort_controller(parent.abort_controller)

Test plan

  • 4/4 new tests pass.
  • 0 production-code changes; this is pure coverage.
  • Confirmed via reading source that the tested invariants reflect actual production behavior — the tests would fail if anyone broke the parity.

🤖 Generated with Claude Code

After the REJECT_MESSAGE override fix in PR #150, a critic flagged a
defensive concern: when the user presses ESC and then types resume,
the engine's reset_abort_controller swaps in a fresh controller. If a
subagent spawned in the previous turn held a stale (aborted) parent
controller snapshot, _dispatch_single_tool would emit REJECT_MESSAGE
for every tool the subagent ran — spurious user-rejected results.

Verification confirmed the existing Python code already mirrors TS at
typescript/src/tools/AgentTool/runAgent.ts:533-541:

* Async agents get a fresh, unlinked AbortController at
  src/agent/run_agent.py:281-284.
* Sync agents share params.parent_context.abort_controller at line 287
  (current reference, mutated in place by engine.reset_abort_controller
  so the field reflects the new controller without snapshot staleness).
* Engine.reset_abort_controller mutates the controller on the SAME
  tool_context object, so any holder of the context reference sees the
  new controller automatically.

This is pure regression-test coverage — no production-code change — to
lock down the invariants so a future refactor that switches the async
path to a child-linked controller (which WOULD propagate parent abort
and re-introduce the bug) is caught immediately.

The 4 tests cover:

1. async path: explicit override → child controller IS the fresh
   instance, NOT a child-linked wrapper; parent abort does NOT
   propagate to child.
2. sync path: shared controller → child.abort_controller IS the
   parent's controller (same object); parent ESC reaches child for free.
3. End-to-end: parent ESC fires, parent controller swapped for a fresh
   one (simulating engine.reset_abort_controller), async subagent's
   controller remains untouched throughout.
4. Default fallback: no override, no share → child-linked branch fires
   (parent-abort propagates down one-way; child-abort does NOT
   propagate up).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.

1 participant