Validate tool inputs against the tool schema in agent-tools

Open
#6,576 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
48/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Quiet
Tech stack
python
Domain
tooling

Research direction

Start with ToolDefinition.args_model in libs/agent-tools/agent_tools/registry.py and the build_sdk_server adapter that dispatches handlers. Review how agent-tools, hackbot-runtime, and lando-client tests cover malformed tool arguments and existing ToolError rendering. Done means schema type failures become structured feedback without changing semantic checks or omitted-argument behavior.

Written by the indexing model from the issue text.

Description

hackbot

Tool arguments coming from the model are never checked against the tool's own schema, so malformed input reaches handlers and fails in ways the agent cannot act on.

ToolDefinition.args_model (in libs/agent-tools/agent_tools/registry.py) already builds a pydantic model from each tool's signature and caches it, and input_schema is derived from it and handed to the SDK. But no adapter validates with it — build_sdk_server calls the handler directly:

@sdk_tool(mcp_name, defn.description, defn.input_schema)
async def run(args):
    try:
        result = await defn.handler(ctx, **args)
    except ToolError as e:
        ...

The schema therefore only guides the model; nothing enforces it.

Why it matters

Two concrete failure modes, using try_server.push's tests: dict[str, list[str]]:

  • A bare string ({"xpcshell": "dom/base/test"}) is iterated character by character, silently producing ['', 'a', 'b', 'd', 'e', 'm', 'o', 's', 't'] as "paths". Handlers currently need hand-rolled isinstance guards to avoid this.
  • A wrong element type ({"xpcshell": [42]}) gets past a container check and raises AttributeError inside the handler. run() only catches ToolError, so it escapes as an unhandled exception rather than becoming feedback the agent could correct.

Pydantic already rejects both, in default (lax) mode, with no configuration:

bare string  -> REJECTED list_type: Input should be a valid list
int in list  -> REJECTED string_type: Input should be a valid string
tuple        -> OK  (normalised to list)

Related symptoms of the same shape elsewhere: #6517 (an LLM returned a list where a string was expected and it crashed on .rfind) and possibly #6493 (unhelpful errors when the model malforms tool arguments).

Proposal

Validate with the existing args_model before dispatching, and turn failures into structured tool errors. Put the helper on ToolDefinition (e.g. defn.validate(args)) rather than inline in the claude-sdk adapter, so a future non-claude-sdk adapter reuses it — registry.py is already the framework-neutral home for args_model.

Handlers then drop their hand-rolled type guards and keep only the semantic checks a type cannot express.

Decisions to make
  • Error rendering. Pydantic's text is precise but jargon-y and includes a docs URL (Input should be a valid list [type=list_type, input_value='dom/base/test', input_type=str]). It should be rendered down to something like tests.xpcshell: Input should be a valid list. Pydantic should own type errors; hand-written ToolErrors keep owning semantic ones, which can teach the agent what to do instead.
  • Lax, not strict. Models routinely emit "123" for an int; lax coerces, strict rejects. Lax also accepts tuple→list, which is wanted.
  • Validate-only, or pass model_dump()? Dumping gives handlers coerced values, but also materialises defaults, so a handler can no longer distinguish "omitted" from "explicitly None". Validate-then-pass-original is the safer first step.
  • extra. create_model currently ignores unknown arguments, so a hallucinated parameter is silently dropped. extra="forbid" would surface it as feedback, and is the change most likely to break an existing caller.
Scope

This affects every tool across agent-tools and hackbot-runtime, so inputs that currently pass by luck would start failing. Worth its own PR, with the agent-tools, hackbot-runtime and lando-client suites run.

Came up in review of #6531 (https://github.com/mozilla/bugbug/pull/6531#discussion_r3763095103), where a handler needed a manual isinstance check for exactly this reason.

Dominant language
Python
Stars
570
Forks
351
Avg merge
2d 13h
Merged PRs (30d)
65

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from mozilla/bugbug

All issues in mozilla/bugbug

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.