Skip to content

fix: stop quit() from cancelling its own owning task through a gathered tool call - #300

Merged
frostming merged 2 commits into
bubbuild:mainfrom
AmirF194:fix/186-quit-recursion-error
Sep 11, 2026
Merged

frostming merged 2 commits into
bubbuild:mainfrom
AmirF194:fix/186-quit-recursion-error

Conversation

@AmirF194

@AmirF194 AmirF194 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

The "quit" tool's coroutine runs as a child task of ToolExecutor.execute_async's own asyncio.gather() (bub/tools.py), so asyncio.current_task() inside it is that child task, not the _run_message task registered in controller.active_tasks. quit()'s self-skip check compares against the wrong task, cancels its own ancestor, and awaits it, but that ancestor is itself awaiting the same gather() this code is a child of, so CPython recurses cancelling it without bound.

Fixed by tracking the owning task in a contextvar set once at the top of _run_message. Child tasks, including the ones execute_async's gather spawns, inherit a copy of that context, so quit() resolves the real owning task regardless of how many awaits deep it's invoked from.

Reproduced against the real ChannelManager/ToolExecutor classes: same RecursionError and wedged event loop on main (990 recursive _GatheringFuture.cancel() frames), clean exit with 0 tasks cancelled on this branch, both confirmed on Python 3.12 and 3.14. The regression test runs the scenario in a subprocess with an OS-level timeout, since the hang also defeats asyncio.wait_for's own cancellation and can't be bounded from inside the event loop itself. Full suite, ruff, and mypy pass (mypy has one pre-existing unrelated error in settings.py that's on main too).

…ed tool call

The quit tool runs as a child task of ToolExecutor.execute_async's own
asyncio.gather() call, so asyncio.current_task() inside it is that child
task, never the _run_message task registered in controller.active_tasks.
quit()'s self-skip check compares against the wrong task, cancels its own
ancestor, and awaits a task that is itself awaiting the gather() this code
is a child of. CPython's cancellation propagation then recurses without bound.

Track the owning task in a contextvar set once at the top of _run_message;
child tasks copy the active context, so it resolves correctly no matter
how many awaits deep quit() is invoked from.

Fixes bubbuild#186

@lodyai lodyai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ContextVar approach looks like a good minimal fix: the task to exclude is the owning turn, which may differ from asyncio.current_task() inside a gathered tool call.

A few suggestions:

Scope _owning_task with a token and try/finally: token = _owning_task.set(asyncio.current_task()), then _owning_task.reset(token) in finally. This is not necessary for the current fresh-task entrypoint, but keeps the context scoped correctly if _run_message is ever directly awaited or nested.

_run_message set the contextvar but never cleared it, so it would keep
leaking the finished turn's task into a nested or re-entrant call
instead of falling back to asyncio.current_task().
@AmirF194

Copy link
Copy Markdown
Contributor Author

Good catch, pushed the token/finally scoping so the contextvar clears itself after each turn instead of persisting past it.

@frostming
frostming merged commit 4139538 into bubbuild:main Sep 11, 2026
5 checks passed
@AmirF194
AmirF194 deleted the fix/186-quit-recursion-error branch September 11, 2026 11:12
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.

2 participants