Skip to content

fix(subagents): confirm teardown after cancellation drains - #4

Merged
Zsanz3 merged 1 commit into
Zsanz3:cursor/fix-batch-bookkeeping-overlap-adc2from
shoyann:fix/confirm-subagent-teardown-on-execution
Sep 21, 2026
Merged

Zsanz3 merged 1 commit into
Zsanz3:cursor/fix-batch-bookkeeping-overlap-adc2from
shoyann:fix/confirm-subagent-teardown-on-execution

Conversation

@shoyann

@shoyann shoyann commented Sep 21, 2026

Copy link
Copy Markdown

Follow-up to bytedance#5253, based on d42a748bd3ffacb338e1d1efb97932b6ba176085. This PR contains only the additional teardown-confirmation fix and its regressions.

Why

request_cancel_background_task() cancels the cross-thread Future returned by asyncio.run_coroutine_threadsafe(). That Future invokes its done callbacks before the isolated-loop task has finished cancellation. Currently forget_future() sets execution_teardown_event at this point, allowing the batch supervisor to accept teardown while the child is still draining cleanup.

The regression blocks the real SandboxLeaseManager.release_async() around a controlled synchronous release. On the base revision, teardown is already acknowledged while release is blocked and the result is still RUNNING.

What changed

  • Leave teardown confirmation to run_with_timeout()'s finally, after child cleanup. The Future callback still removes its submission handle.
  • Add regressions for cancellation during blocked cleanup and cancellation before the isolated loop dispatches the wrapper. The latter also verifies eventual terminalization.
  • Document the distinction between cancelling a submission Future and completing execution teardown.

This does not change ordinary FAILED-result retry handling or the bounded teardown wait / lease-expiry recovery policy.

Surface area

  • Agents / LangGraph
  • Default behavior change — cancelled submission handles no longer confirm execution teardown prematurely
  • Frontend UI
  • Backend API
  • Sandbox provider behavior
  • Skills
  • Dependencies
  • Docs / tests only

Bug fix verification

Both new cases in tests/test_subagent_executor.py::TestCooperativeCancellation fail on the current bytedance#5253 head and pass with this commit:

  • test_cancelled_future_does_not_confirm_execution_teardown
  • test_cancel_before_isolated_loop_dispatch_still_confirms_teardown

The model body is mocked; submission, cross-thread cancellation, and the sandbox-release shield/drain helper execute production code. This is not a red/green claim against upstream main, which does not yet contain bytedance#5253's teardown event. No live model or sandbox integration test was run.

Validation

Windows, Python 3.12.10, project dependencies installed with uv sync --locked:

Check Base Patched
New regressions 2 failed 2 passed
Full offline suite 16,931 passed / 106 failed / 350 skipped 16,933 passed / 106 failed / 350 skipped
Strict blocking-I/O suite 141 passed / 4 failed / 1 skipped 141 passed / 4 failed / 1 skipped
  • Executor, batch-service, capacity and direct-runtime tests: 185 passed on the patched revision.
  • Full backend ruff check . and ruff format --check .: passed.
  • Offline suites used pytest -m 'not live' --ignore=tests/blocking_io tests/; blocking suites used pytest tests/blocking_io.
  • There are 105 common offline failures, not identical failure lists. The patched-only failure was test_extension_task_lifecycle.py::test_notification_timeout_is_one_shared_budget (20 ms budget); it subsequently passed standalone and in 20 repeated runs on both revisions. The base-only failure was test_mcp_task_service.py::test_run_once_releases_claim_when_driver_is_missing_or_fails (strict time comparison); standalone it again failed on base and passed on the patched revision. Those tests and their implementations are unchanged, but the differing outcomes have not been fully diagnosed.
  • The 4 blocking-I/O failures are identical: three Windows POSIX-permission assertions and one Lark CLI test requiring an unavailable executable.

The full local suite is not green. These results document the baseline and observed differences, rather than claiming complete validation.

AI assistance

Tool(s) used: OpenAI Codex.

How it was used: Codex investigated the cancellation path, wrote the implementation and regression tests, and ran local validation.

  • I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output.

@Zsanz3
Zsanz3 merged commit f86b766 into Zsanz3:cursor/fix-batch-bookkeeping-overlap-adc2 Sep 21, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants