feat: support deriving tools from existing tools via inputSchema#1086
feat: support deriving tools from existing tools via inputSchema#1086pgrayy wants to merge 3 commits into
Conversation
Allow tool() to accept an existing InvokableTool as the inputSchema field. TypeScript infers the callback's input type from the source tool's generic parameters, enabling fully typed wrappers without re-declaring schemas. When the source is a ZodTool, the derived tool preserves Zod runtime validation. When the source is a FunctionTool or McpTool, the JSON schema is inherited and the derived tool is created as a FunctionTool.
| input: TInput, | ||
| context?: ToolContext | ||
| ) => AsyncGenerator<JSONValue, TReturn, never> | Promise<TReturn> | TReturn | ||
| } |
There was a problem hiding this comment.
Each tool type has a config defined like this (example).
| */ | ||
| interface DerivedToolConfig<TInput, TReturn extends JSONValue> { | ||
| /** The unique name of the tool */ | ||
| name: string |
There was a problem hiding this comment.
Any reason not to inherit name as well?
There was a problem hiding this comment.
I considered it but figured it would be good to force it at least for now since a derived tool is something different.
There was a problem hiding this comment.
Given that it's agent-visible my gut says that most would want to keep the name. Like I don't expect both tools being passed to an agent being a common use-case.
Not a blocker FWIW.
| * @param config - Tool configuration with a source tool as inputSchema | ||
| * @returns An InvokableTool with input typed from the source tool | ||
| */ | ||
| export function tool<TInput, TReturn extends JSONValue = JSONValue>( |
There was a problem hiding this comment.
Did we consider anything like a wrapTool for clarity?
There was a problem hiding this comment.
Oh I guess we can't have a helper method on tools because they're interfaces could we? otherwise this devx would be cool and fairly cool:
httpRequest.wrap({
....
})
There was a problem hiding this comment.
Just didn't want to give the users a separate export. I thought it might be simpler to overload. But definitely open to suggestions here.
There was a problem hiding this comment.
Of the options that fit our criteria,
tool.wrap(httpTool, { ... }Might be the most clear - it guides you towards thinking of it as "inheriting" or "wrapping".
But either way, I don't expect most folks to self-discover this, so I think it's going to be a docs thing; but inputSpec: tool seems a little weird IMHO
| }) | ||
|
|
||
| const result = await derived.invoke({ url: 'https://example.com', method: 'POST' }) | ||
| expect(result).toStrictEqual({ fetched: 'https://example.com', method: 'POST', wrapped: true }) |
There was a problem hiding this comment.
Issue: Missing test for Zod validation preservation in derived tools.
The PR description states "When the source tool is a ZodTool, the derived tool preserves Zod runtime validation," but there's no test that passes invalid input to a derived ZodTool and asserts that it throws a Zod validation error. This is the key differentiator from the FunctionTool path and should be explicitly tested.
Suggestion: Add a test like:
it('preserves Zod validation when derived from a ZodTool', async () => {
const derived = tool({
name: 'derived',
inputSchema: zodTool,
callback: async (input) => input.url,
})
await expect(derived.invoke({ url: 'not-a-url', method: 'GET' })).rejects.toThrow()
})| * Note: undefined is normalized to z.void() in constructor, so this is always defined. | ||
| */ | ||
| private readonly _inputSchema: z.ZodType | ||
| readonly inputSchema: TInput extends z.ZodType ? TInput : z.ZodVoid |
There was a problem hiding this comment.
Issue: Exposing inputSchema as a public property is a public API change that broadens ZodTool's surface area.
Previously _inputSchema was private, meaning consumers couldn't depend on it. Making it readonly inputSchema means it becomes part of the public contract that customers (and downstream libraries) can rely on. This warrants the needs-api-review label per the API bar-raising process.
Additionally, the property type (TInput extends z.ZodType ? TInput : z.ZodVoid) exposes Zod types directly in the public API, coupling the SDK's public surface to Zod's type system. Consider whether this should remain an internal detail accessed only by the factory, rather than a public property on the tool instance.
Suggestion: If the only consumer of this property is the tool() factory, consider keeping it private/internal (e.g., a package-internal access pattern or a symbol-keyed property) rather than making it fully public. If you do want it public, add the needs-api-review label to the PR.
There was a problem hiding this comment.
Users might want to access it for custom type inference, testing, or building their own composition utilities. Also, we already expose Zod through the public contract. This is the ZodTool after all.
| description: config.description ?? sourceTool.description, | ||
| inputSchema: sourceTool.inputSchema, | ||
| callback: config.callback, | ||
| } as ZodToolConfig<z.ZodType, JSONValue>) |
There was a problem hiding this comment.
Issue: The as ZodToolConfig<z.ZodType, JSONValue> and as FunctionToolConfig casts bypass type checking on the callback field.
When deriving from a source tool, the config.callback has the DerivedToolConfig signature (input: TInput, context?: ToolContext) => ..., which differs from ZodToolConfig's callback signature (input: ZodInferred<TInput>, context?: ToolContext) => .... The as cast silently reconciles this, but if the signatures ever diverge (e.g., a required context parameter), this would produce a runtime error with no compile-time warning.
Suggestion: Consider extracting the callback assignment explicitly so the type relationship is clear, or add a comment explaining why the cast is safe here.
| * @typeParam TInput - Input type inferred from the source tool | ||
| * @typeParam TReturn - Return type of the callback function | ||
| */ | ||
| interface DerivedToolConfig<TInput, TReturn extends JSONValue> { |
There was a problem hiding this comment.
Issue: The DerivedToolConfig interface is not exported, which means users cannot type their configuration objects or create utility functions that accept/return this config shape.
Suggestion: Export DerivedToolConfig from the module (and potentially from src/index.ts) so that users building helper functions around derived tools have access to the type. This aligns with the SDK tenet of being "extensible by design."
| * @param config - Tool configuration with a source tool as inputSchema | ||
| * @returns An InvokableTool with input typed from the source tool | ||
| */ | ||
| export function tool<TInput, TReturn extends JSONValue = JSONValue>( |
There was a problem hiding this comment.
Issue: The overload ordering places DerivedToolConfig after FunctionToolConfig. Because FunctionToolConfig.description is required (non-optional) while DerivedToolConfig.description is optional, there shouldn't be an ambiguity issue in practice. However, when a user omits description, TypeScript may surface confusing error messages pointing at the wrong overload.
Consider placing the DerivedToolConfig overload before the FunctionToolConfig overload, since the instanceof Tool check in the implementation already handles runtime disambiguation, and this would give users better error messages when they pass a tool as inputSchema but forget another required field.
|
Issue: The naming convention This is a minor design consideration - the ergonomics at the call site are good. Just flagging it as something to consider from an API clarity perspective. An alternative could be a dedicated field name like |
|
Assessment: Comment This PR introduces a useful ergonomic feature for tool composition. The type inference approach is well-thought-out, and the implementation cleanly handles both ZodTool and FunctionTool source paths. Review Categories
The implementation is clean and well-documented. The type-level test is a nice addition. |
| }) | ||
|
|
||
| const result = await derived.invoke({ url: 'https://example.com', method: 'POST' }) | ||
| expect(result).toStrictEqual({ fetched: 'https://example.com', method: 'POST', wrapped: true }) |
There was a problem hiding this comment.
Issue: Missing test for Zod validation preservation in derived tools.
The PR description states "When the source tool is a ZodTool, the derived tool preserves Zod runtime validation," but there's no test verifying that invalid input is rejected. This is the key behavioral claim of the ZodTool derivation path.
Suggestion: Add a test that passes invalid input and asserts the Zod error:
it('preserves Zod validation from source ZodTool', async () => {
const derived = tool({
name: 'derived',
inputSchema: zodTool,
callback: async (input) => input.url,
})
await expect(derived.invoke({ url: 'not-a-url', method: 'GET' })).rejects.toThrow()
})| * Note: undefined is normalized to z.void() in constructor, so this is always defined. | ||
| */ | ||
| private readonly _inputSchema: z.ZodType | ||
| readonly inputSchema: TInput extends z.ZodType ? TInput : z.ZodVoid |
There was a problem hiding this comment.
Issue: Exposing inputSchema as a public readonly property is a public API surface change.
Previously _inputSchema was private, meaning customers couldn't depend on it. Making it readonly inputSchema makes it part of the public contract. Per the API Bar Raising process, this should have the needs-api-review label since it's a new public property on a class customers use.
Additionally, the property type (TInput extends z.ZodType ? TInput : z.ZodVoid) couples the public API surface directly to Zod's type system.
Suggestion: If the only consumer is the tool() factory (internal to the SDK), consider keeping it package-internal rather than fully public. If you do want it public, add the needs-api-review label.
| description: config.description ?? sourceTool.description, | ||
| inputSchema: sourceTool.inputSchema, | ||
| callback: config.callback, | ||
| } as ZodToolConfig<z.ZodType, JSONValue>) |
There was a problem hiding this comment.
Issue: The as ZodToolConfig<z.ZodType, JSONValue> and as FunctionToolConfig casts suppress type checking at the implementation boundary.
The DerivedToolConfig.callback has signature (input: TInput, context?: ToolContext) => ... while FunctionToolCallback expects (input: unknown, toolContext: ToolContext) => .... The cast papers over this mismatch. While it works at runtime (JS doesn't enforce parameter arity), these casts would hide future type regressions.
Suggestion: Consider a thin wrapper function that explicitly adapts the callback signature instead of relying on as casts:
const wrappedCallback: FunctionToolCallback = (input, toolContext) =>
(config.callback as DerivedToolConfig<unknown, JSONValue>['callback'])(input, toolContext)This makes the adaptation explicit rather than relying on structural compatibility hidden behind a cast.
| description?: string | ||
|
|
||
| /** An existing tool whose input schema and types will be reused */ | ||
| inputSchema: InvokableTool<TInput, unknown> |
There was a problem hiding this comment.
Issue: The inputSchema field is overloaded to mean three different things across the overloads: a Zod schema, a JSON schema object, or an existing tool instance. This polymorphism on a single property name may confuse users.
The existing reviewers raised a similar point about whether a wrapTool function would be clearer. From an API design perspective, having inputSchema accept a Tool instance stretches the semantic meaning of "schema" — a tool is not a schema, it's something that has a schema.
Suggestion: Consider whether a separate field name (e.g., sourceTool or fromTool) or a separate factory function (e.g., tool.from(existingTool, { ... })) would make the intent clearer. This would avoid the semantic overload while still providing the typed derivation ergonomics.
| * @typeParam TInput - Input type inferred from the source tool | ||
| * @typeParam TReturn - Return type of the callback function | ||
| */ | ||
| interface DerivedToolConfig<TInput, TReturn extends JSONValue> { |
There was a problem hiding this comment.
Issue: DerivedToolConfig is not exported, which means customers who want to type-annotate a variable holding this config or pass it to helper functions can't reference the type.
Suggestion: Export DerivedToolConfig from the module (and src/index.ts) if it's intended as a public API pattern. If it's intentionally internal-only, document that decision.
|
Assessment: Request Changes The feature concept is solid — deriving typed tools from existing tools addresses a real ergonomic gap. However, there are a few concerns that should be addressed before merging. Review Categories
The type inference ergonomics in the type-test file look great. |
|
Assessment: Comment Most substantive issues have already been raised in prior review threads and discussed with reviewers. This is a clean, well-scoped feature addition (~192 lines across 4 files). Outstanding Items
The Zod validation preservation test (line 178) addresses the previously flagged testing gap. The type-level test is well-structured and follows the existing pattern in |
Description
Users building agents with tools like
httpRequestoften need to add restrictions (URL blocklists, auth headers, rate limiting) without rebuilding the tool from scratch. Today, reusing another tool's input schema requires manually copyingtoolSpec.inputSchema(losing type information) or re-declaring the Zod schema.This PR adds a new overload to
tool()that accepts an existing tool as theinputSchemafield. TypeScript infers the callback's input type from the source tool's generic parameters, giving users fully typed wrappers with zero casts.When the source tool is a ZodTool, the derived tool preserves Zod runtime validation. When the source is a FunctionTool or McpTool, the JSON schema is inherited and a FunctionTool is created.
This also exposes
inputSchemaas a public readonly property onZodTool(previously private as_inputSchema), enabling the factory to access the Zod schema for runtime validation preservation.Alternatives Considered
We explored adding a public
inputSchemaproperty to every tool type (ZodTool,FunctionTool,McpTool) and surfacing it via an intersection on thetool()return type (InvokableTool<...> & { readonly inputSchema: TSchema }). This would let users dotool({ inputSchema: existingTool.inputSchema, ... })directly.The approach worked but had downsides:
inputSchemato theToolabstract class is a breaking change (all subclasses must implement it).Tool, the property is invisible when a tool is passed through a function typed asInvokableTool. The intersection only exists at the factory call site and gets erased when assigned to the interface type.FunctionToolandMcpTool, the exposed schema isJSONSchema(opaque to TypeScript), so passing it totool()still results inunknowninput. Only ZodTool benefits from typed schema exposure.The chosen approach (passing the tool itself as
inputSchema) avoids these issues. Types are inferred fromInvokableTool<TInput, TReturn>'s generic parameters rather than from a schema property, so it works uniformly regardless of the underlying tool type or how the tool reference is passed around.Related Issues
N/A
Documentation PR
Will be created if this proposal is accepted.
Type of Change
New feature
Testing
How have you tested the change?
npm run checkChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.