Skip to content

[BUG] validate_tool_attributes never checks keyword-only or positional-only __init__ parameters#2668

Description

@VANDRANKI

Describe the bug

validate_tool_attributes is supposed to enforce that every __init__ parameter of a custom Tool subclass has a default value, per its own docstring: "Any argument of __init__ should have a default. Args chosen at init are not traceable, so we cannot rebuild the source code for them." The check that enforces this, ClassLevelChecker._check_init_function_parameters in src/smolagents/tool_validation.py, only inspects node.args.args and node.args.defaults:

def _check_init_function_parameters(self, node):
    for arg, default in reversed(list(zip_longest(reversed(node.args.args), reversed(node.args.defaults)))):
        if default is None:
            if arg.arg != "self":
                self.non_defaults.add(arg.arg)
        elif not isinstance(default, (ast.Constant, ast.Dict, ast.List, ast.Set)):
            self.non_literal_defaults.add(arg.arg)

node.args.args covers only positional-or-keyword parameters. Python's ast.arguments node also carries posonlyargs (positional-only, before /) and kwonlyargs with kw_defaults (after *), and neither is examined here. A required parameter written in either form is invisible to this check.

Reproduction

import ast
from itertools import zip_longest

source = '''
class BadTool:
    def __init__(self, *, api_key):
        self.api_key = api_key
'''
tree = ast.parse(source)
init = tree.body[0].body[0]

non_defaults = set()
for arg, default in reversed(list(zip_longest(reversed(init.args.args), reversed(init.args.defaults)))):
    if default is None and arg.arg != "self":
        non_defaults.add(arg.arg)

print(init.args.args)        # [] -- only 'self' would appear here, and it's absent because
                              #        'self' before a bare '*' is a kwonly-only signature
print(init.args.kwonlyargs)  # [api_key] -- the actual required parameter, never inspected
print(non_defaults)          # set() -- validator reports no problem

The same happens for a positional-only required parameter (def __init__(self, endpoint, /):): both self and endpoint land in args.posonlyargs, args.args is empty, and non_defaults again comes back empty.

Why this reaches real usage

validate_tool_attributes is not a standalone lint some user has to opt into. It runs from two call sites in tools.py:

  • Tool.save(), right before instance_to_source(self, base_cls=Tool) serializes the tool
  • get_tools_definition_code(), used when exporting an agent's tool definitions

A tool author writing def __init__(self, *, api_key):, an entirely idiomatic way to require a mandatory keyword argument, passes validation silently. instance_to_source then has no way to recover the value passed for api_key at construction time (that's exactly the traceability problem this validator exists to catch), so the serialized tool's __init__ call is missing it, and reconstructing the tool from the saved source fails with a missing-argument error at the point the saved code is loaded back, not at the point the mistake was made.

Suggested fix

Extend the loop to also walk node.args.posonlyargs (paired with the same node.args.defaults list, since positional-only and positional-or-keyword defaults share one sequence in the AST) and node.args.kwonlyargs against node.args.kw_defaults (which is already parallel, one entry per kwonly arg, using None for "no default" rather than needing zip_longest):

def _check_init_function_parameters(self, node):
    positional = node.args.posonlyargs + node.args.args
    for arg, default in reversed(list(zip_longest(reversed(positional), reversed(node.args.defaults)))):
        if default is None:
            if arg.arg != "self":
                self.non_defaults.add(arg.arg)
        elif not isinstance(default, (ast.Constant, ast.Dict, ast.List, ast.Set)):
            self.non_literal_defaults.add(arg.arg)

    for arg, default in zip(node.args.kwonlyargs, node.args.kw_defaults):
        if default is None:
            self.non_defaults.add(arg.arg)
        elif not isinstance(default, (ast.Constant, ast.Dict, ast.List, ast.Set)):
            self.non_literal_defaults.add(arg.arg)

Happy to open a PR with this plus tests covering a required keyword-only arg, a required positional-only arg, and confirming defaulted versions of both still pass.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions