Skip to content

proposal: unexport ToolFor #401

@jba

Description

@jba

ToolFor splits out the creation of a ToolHandler from its registration. It also returns a copy of its input Tool with schemas populated.

As its doc says, there are two motivations for ToolFor: to allow the handler to be wrapped, and to modify the input and output schemas.

However, the first of these is unnecessary and the second is broken.

Wrapping can happen in middleware, as #400 demonstrates.

Modifying schemas in the returned Tool is buggy, thanks to our recent redesign of the AddTool mechanism. Server.AddTool does not validate schemas or touch them in any way. The validation, and the prerequisite schema resolution, happens in ToolFor, on the schema that ToolFor infers or is given. Modifying the schemas after ToolFor returns introduces a mismatch between the schema and its resolved form; it violates the condition on Schema.Resolve that the schema must not be touched after Resolve is called. That can result in a panic.

The right way to provide a custom schema is to create it before calling AddTool. The ToolFor function doesn't help with that.

So we should unexport ToolFor until we find a reason for it to exist.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions