-
Notifications
You must be signed in to change notification settings - Fork 14
Eliminate sync/async twin methods in ToolRunner #200
Copy link
Copy link
Open
Description
Context
From PR #192 architecture review. runner.py has 10 pairs of near-identical sync/async methods (e.g. _resolve_permission / _resolve_permission_async, _run_pre_tool_use_sync / _run_pre_tool_use_async). The sync path uses _run_awaitable_sync which spawns a daemon thread with a nested event loop — a known footgun in production async contexts.
Proposal
Wrap sync tool handlers at registration time, then delete all sync twins:
# registry.py — at register time
def register(self, entry: ToolEntry):
if not asyncio.iscoroutinefunction(entry.handler):
original = entry.handler
async def async_wrapper(**kwargs):
return await asyncio.to_thread(original, **kwargs)
entry = replace(entry, handler=async_wrapper)
self._tools[entry.name] = entryThen remove all 10 _xxx_sync methods in runner.py.
Impact
- ~400 lines removed
- Eliminates daemon-thread + nested-event-loop bridge
- Single code path = easier to reason about and test
Ref
- PR Progress: core runtime refactor checkpoints #192 review: Progress: core runtime refactor checkpoints #192 (review)
- Simplification suggestions: Progress: core runtime refactor checkpoints #192 (comment)
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels