fix(agent-bus): establish a dispatched session task, and cancel a round cleanly - #17
Merged
Merged
Conversation
…nd cleanly ``_dispatch_session_task`` returned without a guaranteed suspension point: ``_emit_session_task_submitted`` returns before awaiting when no sink is installed, and a sink whose ``append`` never awaits does not yield either. So ``submit_task_to_session`` — fire-and-return by contract — could hand a job id back while its coroutine had not started. A host reporting job state right after dispatch then described a worker that had not begun as merely ``"submitted"``, and every cancellation in that window relied on ``_mark_job_aborted`` standing in for a ``finally`` the coroutine never reached. One trailing yield establishes the task instead. That exposed two defects in ``cancel_for_parent`` which its own test had been hiding — the test only passed because none of its three jobs had ever started: - the running job's cancellation runs its ``finally``, which drains the session queue, so the single interleaved pass dispatched the very tasks the call was tearing down. Each one reached the LLM before being cancelled again a few iterations later. Queued jobs are now cleared in a first pass, so the drain finds nothing to start. - the return count only counted jobs that still needed the stand-in. A job that had actually started absorbs its own cancellation, so a round that really did tear down every running sub-agent reported 0. Both outcomes are now counted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
Found while migrating FrontierAgent onto the shared agent-bus kernel (ApodexAI/FrontierAgent#103): the product's
_dispatch_session_taskended with anawait asyncio.sleep(0)that the shared implementation does not have. Restoring it turned out to expose two real defects incancel_for_parent.1. A dispatched session task was not guaranteed to have started
submit_task_to_sessionis fire-and-return by contract — a host tool hands back a job id and its invocation scope closes. But nothing on the dispatch path is guaranteed to suspend:_emit_session_task_submittedreturns before awaiting when no sink is installed, and a sink whoseappendnever awaits does not yield either. So the coroutine could still be unstarted when the caller moved on.Consequences:
"submitted";_mark_job_abortedstanding in for afinallythe coroutine never reached.One trailing yield establishes the task, so cancellations land inside the job wrapper on their own.
2.
cancel_for_parentstarted the tasks it was tearing downCancelling a running job runs its
finally, which drains that session's queue. In a single interleaved pass that dispatched the very tasks the call was cancelling — each reached the LLM before being cancelled again a few iterations later. Queued jobs are now cleared in a first pass, so the drain finds nothing to start.3.
cancel_for_parentunder-reported what it cancelledThe count only incremented for jobs that still needed the stand-in. A job that had actually started absorbs its own cancellation (
_run_and_finalizeflips the status itself), so a round that really did tear down every running sub-agent returned0.This is also why
test_cancel_for_parent_clears_running_and_queuedwas passing: all three of its jobs had never started, which is only true without the yield in (1). Reproducible onmainwithout any change to the bus — let the first job start with oneawait asyncio.sleep(0)before cancelling and the existingassert cancelled == 3fails while every job still ends up correctlyaborted.Tests
tests/test_agent_bus_session_dispatch_handoff.py, both verified red without the corresponding fix:test_dispatch_leaves_the_job_running_not_merely_submitted—'submitted' == 'running'without the yield;test_cancelling_a_round_never_lets_a_queued_task_reach_the_llm—['T1', 'T2', 'T3'] == ['T1']without the two-pass cancel, plus the count and final statuses.test_cancel_for_parent_clears_running_and_queuednow passes for the right reason rather than by accident.Validation
ruff check agent_core testsclean,pyright agent_core0 errors,pytest -q911 passed,uv buildOK.