fix(registry): tolerate plugin handlers without **kw#1
Merged
Conversation
The dispatcher injects context kwargs (`task_id`, `user_task`, `parent_agent`) into every tool handler call. All bundled handlers accept `**kw` to swallow them, but this convention is undocumented — third-party plugin authors who write the natural `def handler(args)` signature get a confusing `TypeError: got an unexpected keyword argument 'task_id'` at first dispatch. Make the dispatcher inspect the handler's signature and pass only kwargs the handler accepts. Backward-compatible: handlers with `**kw` still receive every kwarg; handlers that explicitly name `task_id` still get it. - Add `ToolRegistry._filter_kwargs_for_handler` (signature-aware). - Update `dispatch()` to filter via the helper. - Two new tests: a no-`**kw` handler dispatches without TypeError, and a handler that names `task_id` still receives it. Caught while authoring an external plugin (nostrkey for Hermes); fix helps any third-party plugin written outside this repo.
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
The dispatcher in
tools/registry.pyinjects context kwargs (task_id,user_task,parent_agent) into every tool handler. Every bundled handler —tools/file_tools.py:1093(_handle_read_file),plugins/spotify/tools.py:202(_handle_spotify_search), etc. — accepts**kwto swallow them, but this convention is undocumented.Third-party plugin authors who write the natural
def handler(args)signature (matching the JSON schema, no extra noise) get a bafflingTypeError: got an unexpected keyword argument 'task_id'on the very first dispatch. The error points at the handler line, but nothing in the schema or the docs hints that the dispatcher injects extra kwargs.This patch makes the dispatcher signature-aware: it passes only kwargs the handler actually accepts. Backward-compatible — handlers that already declare
**kwstill receive every kwarg; handlers that explicitly name a context kwarg (e.g.def handler(args, task_id=None)) still get it; handlers that want neither now just work.Repro
Before the patch:
```python
from tools.registry import ToolRegistry
reg = ToolRegistry()
reg.register(name="naive", toolset="core",
schema={"name":"naive","description":"","parameters":{"type":"object","properties":{}}},
handler=lambda args: '{"ok":true}')
reg.dispatch("naive", {}, task_id="t1")
{"error": "Tool execution failed: TypeError: () got an unexpected keyword argument 'task_id'"}
```
After the patch the same dispatch returns
{"ok": true}.I hit this while authoring an external Hermes plugin (
nostrkey) — the bug only surfaces for plugins written outside this repo, since every in-tree handler already accepts**kw.Changes
tools/registry.py: addToolRegistry._filter_kwargs_for_handler(usesinspect.signature); call it fromdispatch()before invoking the handler. Tolerates**kw-style handlers (passes everything), explicitly-named-kwarg handlers (passes the matching ones, drops the rest), and signature-less callables (falls back to passing all kwargs).tests/tools/test_registry.py: two new tests —test_dispatch_tolerates_handler_without_kwargsandtest_dispatch_passes_named_kwargs_when_handler_accepts_them.Test plan
pytest tests/tools/test_registry.py— 33/33 pass (was 31/31, +2 new tests)pytest tests/test_model_tools.py tests/tools/— 896 pass; 2 unrelated failures (test_custom_cwd_in_command_wrapperin daytona env,test_os_environ_still_wins_over_dotenvin credential pool) confirmed pre-existing onmain— neither touches the registrydef handler(args)(no**kw) registers and dispatches cleanly underhermes chat