diff --git a/README.md b/README.md index d27812fbc..dcf664f3f 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,6 @@ dashboard — any provider, any deploy target, no lock-in. - **Human-in-the-loop** — flag a tool `needsApproval` and pause execution until a human approves or rejects it, then `resumeAfterApproval()` from any process - **Durable execution** — pass a `sessionId` + `checkpointStore` and a crash mid-conversation resumes instead of restarting - **Cancellation** — pass an `AbortSignal` (`agent.send(input, { signal })`) to stop a run; it resolves with `finishReason: 'aborted'` and the transcript so far, and the signal reaches the provider, tools and delegated agents -- **Skills** — `defineSkill()` / `loadSkills('./skills')`: only each skill's name and description sit in the system prompt; the model loads the full markdown on demand through an auto-registered `load_skill` tool - **Multi-agent delegation** — wrap a child agent as a tool with `createDelegateTool()`, with a `maxDepth` guard against delegation loops - **Pre/post hooks** — a `HookRegistry` of `AgentHook`s that can inspect or mutate a tool call/LLM generate step, or throw to abort it, run sandboxed by Agent Forge's hook editor - **Guardrails** — fail-closed, concurrently-run checks (secret scan, diff size, test/lint commands) that gate a fixer agent's patch before it's used @@ -197,7 +196,6 @@ console.log(result.usage.totalTokens, result.finishReason, result.steps); - [Deployment](docs/deployment.md) - `loushy build` targets: Node server, Docker, Cloudflare Workers - [API Overview](docs/api-overview.md) - the main exports; `npm run docs:build` generates the full TypeDoc reference - [Testing](docs/testing.md) - unit-test agents deterministically with the scripted `mockModel` -- [Skills](docs/skills.md) - on-demand instructions: `defineSkill()`, `loadSkills()`, how they save context - [Agent Forge](docs/agent-forge.md) - the visual dashboard (`loushy studio`): quickstart, first-agent walkthrough, hook authoring - Full guides site: [linuxdevil.github.io/agent-sdk-docs](https://linuxdevil.github.io/agent-sdk-docs/) diff --git a/docs/api-overview.md b/docs/api-overview.md index 287ce8d34..895ee2ba9 100644 --- a/docs/api-overview.md +++ b/docs/api-overview.md @@ -19,13 +19,6 @@ npm run docs:build # writes docs/api/index.html | `resumeAfterApproval()` | Resume an execution paused for human approval. | | `createDelegateTool()` | Wrap a child agent as a tool for multi-agent delegation. | -### Skills - -Pass `skills: [defineSkill({ name, description, content }), ...(await loadSkills(dir))]` to -`createAgent()` or `AgentExecutor.execute()`: only names and descriptions go in -the system prompt and the model loads bodies through an auto-registered -`load_skill` tool. See [Skills](./skills.md). - ### Cancellation Pass an `AbortSignal` to stop a run: `agent.send(input, { signal })`, @@ -132,6 +125,49 @@ import { loadMcpTools } from '@loushy/build-ai-agent/mcp'; const tools = await loadMcpTools(mcpClient, 'my-server'); ``` +**Schema support.** Each tool's JSON Schema `inputSchema` is converted to a +zod schema, and the model's arguments are validated against it before the +server is called. The converter handles `type` (including arrays such as +`["string", "null"]`), `enum` with mixed types, `const`, `anyOf` / `oneOf` +(a union; `anyOf: [X, { type: "null" }]` becomes `X.nullable()`), `allOf` +(merge / intersection), local `$ref` into `$defs` / `definitions`, +`properties` / `required`, `additionalProperties` (boolean or schema), `items`, +`default`, and the constraints `minimum`, `maximum`, `exclusiveMinimum`, +`exclusiveMaximum`, `minLength`, `maxLength`, `pattern`, `minItems` and +`maxItems`. Where JSON Schema is ambiguous the conversion is permissive: +unknown keywords, empty schemas, tuple `items` and unresolvable refs become +`z.any()`, a recursive `$ref` is expanded once and `z.any()` is used for the +inner occurrence, an invalid `pattern` regex is skipped, and objects keep +extra properties unless `additionalProperties` is `false`. The converter never +throws on schema content. + +**Skipped tools.** If one tool still cannot be converted, only that tool is +skipped; the rest of the server's tools load. Pass a `logger` to receive a +warning naming the server, the tool and the reason, and `onSkip` to collect +what was left out: + +```ts +import { loadMcpTools, type SkippedMcpTool } from '@loushy/build-ai-agent/mcp'; + +const skipped: SkippedMcpTool[] = []; +const tools = await loadMcpTools(mcpClient, 'my-server', { + logger: console, + onSkip: (tool) => skipped.push(tool), // { name, reason } +}); +``` + +**Results.** An MCP result with `isError: true` is a normal tool failure: the +model receives `{ "error": "McpToolError", "toolName": "...", "message": "..." }` +where `message` is the server's text content. Successful results are +JSON-serializable: if the server returns `structuredContent` it is the result +object; otherwise the result is `{ text, content }`, where `text` joins all +text parts and `content` keeps every part in order (`text`, `image`, `audio`, +`resource`, `resource_link`; an unrecognised part type is kept as +`{ type: 'unknown', raw }`). When `structuredContent` arrives together with +non-text parts the result is `{ structuredContent, text, content }` with +`content` holding only the non-text parts, so images, audio and resources are +never dropped. + ### Tool errors A tool call can fail in two ways. In both, the run continues and the model diff --git a/docs/skills.md b/docs/skills.md deleted file mode 100644 index a2f0cd8fc..000000000 --- a/docs/skills.md +++ /dev/null @@ -1,106 +0,0 @@ -# Skills - -A skill is a named chunk of instructions (a markdown document) that the model -loads **only when it needs it**. This is progressive disclosure: instead of -pasting every playbook into the system prompt, you list skills by name and -one-line description, and the model pulls in the full text on demand. - -How it works when an agent has skills: - -1. The system prompt gets a compact **Available skills** block: one line per - skill (`- name: description`) plus one sentence telling the model to call - `load_skill` before doing a task a skill covers. -2. A `load_skill` tool (input `{ name }`) is registered automatically. It - returns the skill's full content as the tool result. -3. Skill bodies are **not** in the prompt until loaded. An agent with twenty - long skills pays roughly one line per skill on every request, and the body - cost only on the runs that use it. - -An unknown name is a tool error that lists the valid skill names, so the model -can correct itself. - -## Skills in code - -```ts -import { createAgent, defineSkill } from '@loushy/build-ai-agent'; - -const agent = createAgent({ - prompt: 'You are a release engineer.', - provider, - skills: [ - defineSkill({ - name: 'changelog', - description: 'How to write a changelog entry', - content: '# Changelog entries\n\nUse the imperative mood and link the PR.', - }), - ], -}); -``` - -`defineSkill({ name, description, content })` validates its input. `name` must -match `^[a-z0-9][a-z0-9-_]{0,63}$`; `description` and `content` must be -non-empty. Errors say what is wrong and how to fix it. - -The full executor API takes the same option: - -```ts -import { AgentExecutor, defineSkill } from '@loushy/build-ai-agent'; - -const result = await AgentExecutor.execute({ - agent, - input: 'Write the changelog for 1.2.0', - provider, - skills: [defineSkill({ name: 'changelog', description: 'Changelog style', content: '...' })], -}); -``` - -If you already registered a tool named `load_skill`, passing `skills` throws: -rename your tool or drop the option. - -## Skills from disk - -`loadSkills(dir)` reads skill files, so non-developers can edit them without -touching code. Two layouts are supported and can be mixed in one directory: - -``` -skills/ - changelog/SKILL.md # /SKILL.md (Anthropic / open-harness convention) - code-review.md # .md (eve convention) -``` - -Each file starts with YAML frontmatter; the rest is the skill's content: - -```markdown ---- -description: How to write a changelog entry ---- - -# Changelog entries - -Use the imperative mood and link the PR. -``` - -- `description` is required. -- `name` is optional and defaults to the folder name (`SKILL.md` layout) or the - file name without `.md`. -- Folders without a `SKILL.md` are ignored. -- Skills come back sorted by name, so prompts are deterministic. -- Errors name the file and the problem: missing description, invalid YAML, - invalid name, an unreadable directory, or two skills with the same name - (both paths are listed). - -```ts -import { createAgent, loadSkills } from '@loushy/build-ai-agent'; - -const agent = createAgent({ - prompt: 'You are a release engineer.', - provider, - skills: [...(await loadSkills('./skills'))], -}); -``` - -## Testing skills - -Use [`mockModel`](./testing.md) to assert the model sees descriptions but not -bodies until it loads one: the first request's system prompt lists the skills, -and the request after a `load_skill` call contains the body in the tool result. diff --git a/src/createAgent.ts b/src/createAgent.ts index 6c74e856c..c7214fd70 100644 --- a/src/createAgent.ts +++ b/src/createAgent.ts @@ -22,7 +22,6 @@ import { LLMProvider } from './providers/llm'; import { ToolRegistry } from './tools/ToolRegistry'; import { ToolDescriptor } from './types'; import type { DefinedTool } from './tools/defineTool'; -import type { Skill } from './skills/defineSkill'; /** * Configuration for createAgent(). Tools are keyed by the name the agent @@ -42,15 +41,6 @@ export interface CreateAgentConfig { * createAgent({ prompt: '...', provider, tools: [sendEmail] }); */ tools?: readonly DefinedTool[] | Record; - /** - * Optional skills (LOU-Y2): only name + description go in the system - * prompt; the model loads a skill's full content with the auto-registered - * `load_skill` tool. Build them with `defineSkill()` or `loadSkills()`. - * - * @example - * createAgent({ prompt: '...', provider, skills: await loadSkills('./skills') }); - */ - skills?: readonly Skill[]; /** Optional agent name; defaults to 'agent'. */ name?: string; /** Optional maxSteps passed through to AgentExecutor.execute(). */ @@ -101,7 +91,6 @@ export function createAgent(config: CreateAgentConfig): SimpleAgent { input: message, provider: config.provider, toolRegistry, - skills: config.skills, maxSteps: config.maxSteps, signal: options.signal, }); diff --git a/src/execution/AgentExecutor.ts b/src/execution/AgentExecutor.ts index bf8a1406b..31be10e8d 100644 --- a/src/execution/AgentExecutor.ts +++ b/src/execution/AgentExecutor.ts @@ -3,8 +3,6 @@ * Executes agents with streaming support and tool calling */ -import type { Skill } from '../skills/defineSkill'; -import { withSkills } from '../skills/withSkills'; import { nanoid } from 'nanoid'; import { LLMProvider, Message, ToolCall, GenerateOptions, GenerateResult, ToolDefinition } from '../providers'; import { AgentConfig } from '../types'; @@ -105,16 +103,6 @@ export interface ExecuteOptions { input: string | Message[]; provider: LLMProvider; toolRegistry?: ToolRegistry; - /** - * Skills (LOU-Y2): instructions the model loads on demand. Their names and - * descriptions are appended to the system prompt and a `load_skill` tool is - * registered; bodies only enter the conversation when the model loads them. - * Throws if a tool named `load_skill` is already registered. - * - * @example - * AgentExecutor.execute({ agent, input, provider, skills: [defineSkill({ name, description, content })] }); - */ - skills?: readonly Skill[]; streaming?: boolean; maxSteps?: number; temperature?: number; @@ -343,11 +331,7 @@ export class AgentExecutor { exporter, 'agent.run', { input: typeof input === 'string' ? input : JSON.stringify(input) }, - async (agentSpan) => - this.runAgentLoop( - { ...options, ...withSkills(options.agent, options.toolRegistry, options.skills) }, - agentSpan.id - ), + async (agentSpan) => this.runAgentLoop(options, agentSpan.id), undefined ); } diff --git a/src/index.ts b/src/index.ts index 927a82d95..90c63fa01 100644 --- a/src/index.ts +++ b/src/index.ts @@ -53,9 +53,6 @@ export * from './utils'; // createAgent() convenience API (LOU-H1) export * from './createAgent'; -// Skills: progressive disclosure of instructions (LOU-Y2) -export * from './skills'; - // Declarative agent spec file format (LOU-H9) export * from './spec'; diff --git a/src/skills/defineSkill.ts b/src/skills/defineSkill.ts deleted file mode 100644 index fcb525840..000000000 --- a/src/skills/defineSkill.ts +++ /dev/null @@ -1,80 +0,0 @@ -/** Skill names are file/tool friendly: lowercase, digits, `-` and `_`, 1-64 characters. */ -const SKILL_NAME_PATTERN = /^[a-z0-9][a-z0-9-_]{0,63}$/; - -/** Options accepted by {@link defineSkill}. */ -export interface DefineSkillOptions { - /** Name the model loads the skill by. Must match `^[a-z0-9][a-z0-9-_]{0,63}$`. */ - name: string; - /** One line telling the model when the skill applies. Always in the system prompt. */ - description: string; - /** The full instructions (markdown). Only sent to the model once it calls `load_skill`. */ - content: string; -} - -/** - * A named bundle of instructions the model loads on demand. Only `name` and - * `description` sit in the system prompt; `content` is returned by the - * auto-registered `load_skill` tool. Create one with {@link defineSkill} or - * {@link loadSkills}. - */ -export interface Skill { - readonly name: string; - readonly description: string; - readonly content: string; -} - -function fail(problem: string, fix: string): never { - throw new Error(`defineSkill: ${problem}. ${fix}`); -} - -function assertNonEmpty(name: string, field: 'description' | 'content', value: unknown): void { - if (typeof value === 'string' && value.trim() !== '') return; - const hint = - field === 'description' - ? 'a one-line summary the model uses to decide when to load the skill' - : 'the markdown instructions the model receives when it loads the skill'; - fail( - `skill '${name}' is missing a non-empty '${field}' (${hint})`, - `Example: defineSkill({ name: '${name}', description: 'When to use it', content: '# Steps...' })` - ); -} - -function assertName(name: unknown): asserts name is string { - if (typeof name !== 'string' || name === '') { - fail("'name' is required", "Example: defineSkill({ name: 'changelog', ... })"); - } - if (!SKILL_NAME_PATTERN.test(name)) { - const suggestion = - name - .toLowerCase() - .replace(/[^a-z0-9-_]+/g, '-') - .replace(/^[-_]+/, '') - .slice(0, 64) || 'my-skill'; - fail( - `invalid skill name ${JSON.stringify(name)} (use 1-64 characters: lowercase a-z, 0-9, '-' and '_', starting with a letter or digit)`, - `Rename it, e.g. ${JSON.stringify(suggestion)}` - ); - } -} - -/** - * Define a skill: instructions the model pulls in only when it needs them - * (progressive disclosure). Pass skills to `createAgent({ skills })` or - * `AgentExecutor.execute({ skills })`. - * - * @example - * ```ts - * const changelog = defineSkill({ - * name: 'changelog', - * description: 'How to write a changelog entry', - * content: '# Changelog entries\n\nUse the imperative mood and link the PR.', - * }); - * ``` - */ -export function defineSkill(opts: DefineSkillOptions): Skill { - const { name, description, content } = opts ?? ({} as Partial); - assertName(name); - assertNonEmpty(name, 'description', description); - assertNonEmpty(name, 'content', content); - return Object.freeze({ name, description: description.trim(), content }); -} diff --git a/src/skills/index.ts b/src/skills/index.ts deleted file mode 100644 index f0119db9c..000000000 --- a/src/skills/index.ts +++ /dev/null @@ -1,3 +0,0 @@ -export { defineSkill } from './defineSkill'; -export type { DefineSkillOptions, Skill } from './defineSkill'; -export { loadSkills } from './loadSkills'; diff --git a/src/skills/loadSkills.ts b/src/skills/loadSkills.ts deleted file mode 100644 index b127c8eed..000000000 --- a/src/skills/loadSkills.ts +++ /dev/null @@ -1,125 +0,0 @@ -import { promises as fs, type Dirent } from 'node:fs'; -import path from 'node:path'; -import { parse as parseYaml } from 'yaml'; -import { defineSkill, type Skill } from './defineSkill'; - -const FRONTMATTER = /^?---[ \t]*\r?\n([\s\S]*?)\r?\n---[ \t]*(?:\r?\n|$)/; - -interface SkillSource { - /** Skill name implied by the folder / file name. */ - defaultName: string; - file: string; -} - -function problem(file: string, message: string): Error { - return new Error(`loadSkills: ${file}: ${message}`); -} - -function parseFrontmatter(file: string, raw: string): { meta: Record; body: string } { - const match = FRONTMATTER.exec(raw); - if (!match) return { meta: {}, body: raw }; - let meta: unknown; - try { - meta = parseYaml(match[1]); - } catch (error) { - throw problem( - file, - `invalid YAML frontmatter (${(error as Error).message}). Fix the YAML between the leading '---' lines.` - ); - } - if (meta !== null && meta !== undefined && (typeof meta !== 'object' || Array.isArray(meta))) { - throw problem(file, "frontmatter must be a YAML mapping such as 'description: ...'."); - } - return { meta: (meta ?? {}) as Record, body: raw.slice(match[0].length) }; -} - -async function readSkill(source: SkillSource): Promise { - let raw: string; - try { - raw = await fs.readFile(source.file, 'utf8'); - } catch (error) { - throw problem(source.file, `cannot read file (${(error as Error).message})`); - } - const { meta, body } = parseFrontmatter(source.file, raw); - if (typeof meta.description !== 'string' || meta.description.trim() === '') { - throw problem( - source.file, - "missing 'description' in the frontmatter. Add a block at the top of the file:\n---\ndescription: When the model should use this skill\n---" - ); - } - const name = typeof meta.name === 'string' && meta.name !== '' ? meta.name : source.defaultName; - try { - return defineSkill({ name, description: meta.description, content: body.trim() }); - } catch (error) { - throw problem(source.file, (error as Error).message.replace(/^defineSkill: /, '')); - } -} - -async function isFile(file: string): Promise { - return fs.stat(file).then( - (s) => s.isFile(), - () => false - ); -} - -async function sourceOf(dir: string, entry: Dirent): Promise { - if (entry.isDirectory()) { - const file = path.join(dir, entry.name, 'SKILL.md'); - return (await isFile(file)) ? { defaultName: entry.name, file } : undefined; - } - if (entry.isFile() && entry.name.toLowerCase().endsWith('.md')) { - return { defaultName: entry.name.slice(0, -'.md'.length), file: path.join(dir, entry.name) }; - } - return undefined; -} - -async function listSources(dir: string): Promise { - let entries: Dirent[]; - try { - entries = await fs.readdir(dir, { withFileTypes: true }); - } catch (error) { - throw new Error( - `loadSkills: cannot read skills directory '${dir}' (${(error as Error).message}). ` + - `Check the path, or create it with a skill inside, e.g. ${path.join(dir, 'my-skill', 'SKILL.md')}.` - ); - } - const sources = await Promise.all(entries.map((entry) => sourceOf(dir, entry))); - return sources.filter((s): s is SkillSource => s !== undefined); -} - -/** - * Load skills from a directory. Two layouts are understood and can be mixed: - * - * - `dir//SKILL.md` (Anthropic / open-harness convention) - * - `dir/.md` (eve convention) - * - * Each file has YAML frontmatter with a required `description` and an - * optional `name` (defaults to the folder / file name); the markdown body is - * the skill's content. Folders without a `SKILL.md` are ignored. The result is - * sorted by name. Errors name the offending file. - * - * @example - * ```ts - * const agent = createAgent({ - * prompt: 'You are a release engineer.', - * provider, - * skills: await loadSkills('./skills'), - * }); - * ``` - */ -export async function loadSkills(dir: string): Promise { - const sources = await listSources(dir); - const byName = new Map(); - for (const source of sources) { - const skill = await readSkill(source); - const earlier = byName.get(skill.name); - if (earlier) { - throw new Error( - `loadSkills: duplicate skill name '${skill.name}' in ${earlier.file} and ${source.file}. ` + - `Rename one file/folder or set a different 'name' in its frontmatter.` - ); - } - byName.set(skill.name, { skill, file: source.file }); - } - return [...byName.values()].map((v) => v.skill).sort((a, b) => (a.name < b.name ? -1 : 1)); -} diff --git a/src/skills/skills.test.ts b/src/skills/skills.test.ts deleted file mode 100644 index 40168171e..000000000 --- a/src/skills/skills.test.ts +++ /dev/null @@ -1,223 +0,0 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { promises as fs } from 'node:fs'; -import os from 'node:os'; -import path from 'node:path'; -import { z } from 'zod'; -import { createAgent } from '../createAgent'; -import { AgentExecutor } from '../execution/AgentExecutor'; -import { AgentBuilder } from '../core/AgentBuilder'; -import { AgentType } from '../types'; -import { defineTool } from '../tools/defineTool'; -import { ToolRegistry } from '../tools/ToolRegistry'; -import { mockModel } from '../testing'; -import { defineSkill, loadSkills } from './index'; - -const changelog = defineSkill({ - name: 'changelog', - description: 'How to write a changelog entry', - content: '# Changelog\nSECRET-CHANGELOG-BODY: use the imperative mood.', -}); -const review = defineSkill({ - name: 'code_review', - description: 'How to review a pull request', - content: 'SECRET-REVIEW-BODY', -}); - -const systemOf = (call: { messages: readonly { role: string; content: unknown }[] }): string => - String(call.messages.find((m) => m.role === 'system')?.content); - -describe('defineSkill', () => { - it('returns the skill', () => { - expect(changelog).toEqual({ - name: 'changelog', - description: 'How to write a changelog entry', - content: '# Changelog\nSECRET-CHANGELOG-BODY: use the imperative mood.', - }); - }); - - it('rejects bad names with a suggested fix', () => { - expect(() => defineSkill({ name: 'My Skill!', description: 'd', content: 'c' })).toThrow( - /invalid skill name "My Skill!".*e\.g\. "my-skill-"/ - ); - expect(() => defineSkill({ name: '', description: 'd', content: 'c' })).toThrow(/'name' is required/); - expect(() => defineSkill({ name: 'a'.repeat(65), description: 'd', content: 'c' })).toThrow(/invalid skill name/); - expect(() => defineSkill({ name: '-x', description: 'd', content: 'c' })).toThrow(/invalid skill name/); - }); - - it('rejects empty description and content', () => { - expect(() => defineSkill({ name: 'x', description: ' ', content: 'c' })).toThrow( - /skill 'x' is missing a non-empty 'description'.*Example/ - ); - expect(() => defineSkill({ name: 'x', description: 'd', content: '' })).toThrow(/non-empty 'content'/); - expect(() => defineSkill(undefined as never)).toThrow(/'name' is required/); - }); -}); - -describe('skills wiring (createAgent)', () => { - it('lists skills by description without bodies, then returns the body via load_skill', async () => { - const model = mockModel([ - { toolCalls: [{ name: 'load_skill', args: { name: 'changelog' } }] }, - 'Done.', - ]); - const agent = createAgent({ - prompt: 'You are a release engineer.', - provider: model, - skills: [changelog, review], - }); - - const result = await agent.send('write a changelog'); - - expect(result.text).toBe('Done.'); - const first = model.calls[0]; - const system = systemOf(first); - expect(system).toContain('You are a release engineer.'); - expect(system).toContain('- changelog: How to write a changelog entry'); - expect(system).toContain('- code_review: How to review a pull request'); - expect(system).toContain('`load_skill`'); - expect(system).not.toContain('SECRET-CHANGELOG-BODY'); - expect(system).not.toContain('SECRET-REVIEW-BODY'); - expect(first.tools?.map((t) => t.function.name)).toEqual(['load_skill']); - - const second = model.calls[1]; - const toolMessage = second.messages.find((m) => m.role === 'tool'); - expect(toolMessage?.content).toContain('SECRET-CHANGELOG-BODY'); - expect(systemOf(second)).not.toContain('SECRET-CHANGELOG-BODY'); - expect(JSON.stringify(second.messages)).not.toContain('SECRET-REVIEW-BODY'); - }); - - it('returns a tool error listing valid names for an unknown skill', async () => { - const model = mockModel([ - { toolCalls: [{ name: 'load_skill', args: { name: 'nope' } }] }, - 'ok', - ]); - await createAgent({ prompt: 'p', provider: model, skills: [changelog, review] }).send('go'); - - const toolMessage = model.calls[1].messages.find((m) => m.role === 'tool'); - expect(toolMessage).toMatchObject({ isError: true, toolName: 'load_skill' }); - expect(toolMessage?.content).toContain("Unknown skill 'nope'"); - expect(toolMessage?.content).toContain('changelog, code_review'); - }); - - it('keeps the user tools alongside load_skill', async () => { - const ping = defineTool({ name: 'ping', description: 'Ping', input: z.object({}), execute: () => 'pong' }); - const model = mockModel(['hi']); - await createAgent({ prompt: 'p', provider: model, tools: [ping], skills: [changelog] }).send('x'); - expect(model.calls[0].tools?.map((t) => t.function.name).sort()).toEqual(['load_skill', 'ping']); - }); - - it('adds no skills block or tool when no skills are given', async () => { - const model = mockModel(['hi']); - await createAgent({ prompt: 'plain', provider: model, skills: [] }).send('x'); - expect(systemOf(model.calls[0])).toBe('plain'); - expect(model.calls[0].tools ?? []).toEqual([]); - }); - - it('throws when the user already registered load_skill', () => { - const own = defineTool({ name: 'load_skill', description: 'mine', input: z.object({}), execute: () => 1 }); - const agent = createAgent({ prompt: 'p', provider: mockModel(['x']), tools: [own], skills: [changelog] }); - return expect(agent.send('x')).rejects.toThrow(/'load_skill' is already registered/); - }); - - it('throws on duplicate skill names', async () => { - const agent = createAgent({ prompt: 'p', provider: mockModel(['x']), skills: [changelog, changelog] }); - await expect(agent.send('x')).rejects.toThrow(/duplicate skill name 'changelog'/); - }); -}); - -describe('skills wiring (AgentExecutor.execute)', () => { - it('supports the skills option without mutating the caller registry or agent', async () => { - const model = mockModel(['hi']); - const agent = AgentBuilder.create().setType(AgentType.SmartAssistant).setName('a').setPrompt('base').build(); - const registry = new ToolRegistry(); - - await AgentExecutor.execute({ agent, input: 'x', provider: model, toolRegistry: registry, skills: [review] }); - - expect(systemOf(model.calls[0])).toContain('- code_review: How to review a pull request'); - expect(agent.prompt).toBe('base'); - expect(registry.has('load_skill')).toBe(false); - }); - - it('uses the block alone when the agent has no prompt', async () => { - const model = mockModel(['hi']); - const agent = AgentBuilder.create().setType(AgentType.SmartAssistant).setName('a').build(); - await AgentExecutor.execute({ agent, input: 'x', provider: model, skills: [review] }); - expect(systemOf(model.calls[0]).startsWith('## Available skills')).toBe(true); - }); -}); - -describe('loadSkills', () => { - let dir: string; - const write = async (rel: string, content: string) => { - const file = path.join(dir, rel); - await fs.mkdir(path.dirname(file), { recursive: true }); - await fs.writeFile(file, content); - return file; - }; - - beforeEach(async () => { - dir = await fs.mkdtemp(path.join(os.tmpdir(), 'skills-')); - }); - afterEach(async () => { - await fs.rm(dir, { recursive: true, force: true }); - }); - - it('loads both layouts sorted by name, defaulting names from folder/file', async () => { - await write('zeta/SKILL.md', '---\ndescription: Zeta skill\n---\n\nZeta body\n'); - await write('alpha.md', '---\ndescription: Alpha skill\n---\nAlpha body'); - await write('mid/SKILL.md', '---\r\nname: renamed\r\ndescription: "Mid: skill"\r\n---\r\nMid body'); - await write('notes/README.txt', 'not a skill'); - await write('empty-dir/.gitkeep', ''); - - const skills = await loadSkills(dir); - - expect(skills).toEqual([ - { name: 'alpha', description: 'Alpha skill', content: 'Alpha body' }, - { name: 'renamed', description: 'Mid: skill', content: 'Mid body' }, - { name: 'zeta', description: 'Zeta skill', content: 'Zeta body' }, - ]); - }); - - it('names the file when the description is missing', async () => { - const file = await write('a.md', '# no frontmatter at all'); - await expect(loadSkills(dir)).rejects.toThrow( - new RegExp(`${file.replace(/[\\^$.*+?()[\]{}|]/g, '\\$&')}: missing 'description'`) - ); - await write('a.md', '---\nname: a\n---\nbody'); - await expect(loadSkills(dir)).rejects.toThrow(/missing 'description'/); - }); - - it('names the file for invalid YAML, non-mapping frontmatter and invalid names', async () => { - await write('a.md', '---\ndescription: [unclosed\n---\nbody'); - await expect(loadSkills(dir)).rejects.toThrow(/a\.md: invalid YAML frontmatter/); - await write('a.md', '---\n- a\n- b\n---\nbody'); - await expect(loadSkills(dir)).rejects.toThrow(/a\.md: frontmatter must be a YAML mapping/); - await write('a.md', '---\nname: Bad Name\ndescription: d\n---\nbody'); - await expect(loadSkills(dir)).rejects.toThrow(/a\.md: invalid skill name "Bad Name"/); - await write('a.md', '---\ndescription: d\n---\n'); - await expect(loadSkills(dir)).rejects.toThrow(/a\.md: skill 'a' is missing a non-empty 'content'/); - }); - - it('reports duplicate names with both paths', async () => { - const a = await write('dup/SKILL.md', '---\ndescription: one\n---\nA'); - const b = await write('dup2.md', '---\nname: dup\ndescription: two\n---\nB'); - const error = await loadSkills(dir).then( - () => undefined, - (e: Error) => e - ); - expect(error?.message).toMatch(/duplicate skill name 'dup'/); - expect(error?.message).toContain(a); - expect(error?.message).toContain(b); - }); - - it('reports an unreadable directory', async () => { - await expect(loadSkills(path.join(dir, 'missing'))).rejects.toThrow(/cannot read skills directory/); - }); - - it('loaded skills work end to end', async () => { - await write('release/SKILL.md', '---\ndescription: Cut a release\n---\nRELEASE-BODY'); - const model = mockModel([{ toolCalls: [{ name: 'load_skill', args: { name: 'release' } }] }, 'ok']); - await createAgent({ prompt: 'p', provider: model, skills: await loadSkills(dir) }).send('go'); - expect(systemOf(model.calls[0])).toContain('- release: Cut a release'); - expect(model.calls[1].messages.find((m) => m.role === 'tool')?.content).toContain('RELEASE-BODY'); - }); -}); diff --git a/src/skills/withSkills.ts b/src/skills/withSkills.ts deleted file mode 100644 index 1b62e0b2d..000000000 --- a/src/skills/withSkills.ts +++ /dev/null @@ -1,82 +0,0 @@ -import { z } from 'zod'; -import type { AgentConfig } from '../types'; -import { defineTool } from '../tools/defineTool'; -import { ToolRegistry } from '../tools/ToolRegistry'; -import type { Skill } from './defineSkill'; - -/** Name of the tool the model uses to load a skill's full content. */ -const LOAD_SKILL_TOOL = 'load_skill'; - -function skillsPromptBlock(skills: readonly Skill[]): string { - const lines = skills.map((s) => `- ${s.name}: ${s.description.replace(/\s+/g, ' ').trim()}`); - return [ - '## Available skills', - '', - `Before doing a task that one of these skills covers, call the \`${LOAD_SKILL_TOOL}\` tool with its name to read the full instructions.`, - '', - ...lines, - ].join('\n'); -} - -function createLoadSkillTool(skills: readonly Skill[]) { - const byName = new Map(skills.map((s) => [s.name, s])); - return defineTool({ - name: LOAD_SKILL_TOOL, - description: - 'Load the full instructions of a skill listed under "Available skills". Call this before doing a task the skill covers.', - input: z.object({ name: z.string().describe('The skill name, exactly as listed') }), - execute: ({ name }) => { - const skill = byName.get(name); - if (!skill) { - throw new Error(`Unknown skill '${name}'. Valid skills: ${[...byName.keys()].join(', ')}.`); - } - return skill.content; - }, - }); -} - -function assertUsable(skills: readonly Skill[], agent: AgentConfig, registry?: ToolRegistry): void { - if (registry?.has(LOAD_SKILL_TOOL) || agent.tools?.[LOAD_SKILL_TOOL]) { - throw new Error( - `skills: a tool named '${LOAD_SKILL_TOOL}' is already registered, but agents with skills get one automatically. ` + - `Rename your tool, or remove the 'skills' option.` - ); - } - const seen = new Set(); - for (const { name } of skills) { - if (seen.has(name)) { - throw new Error(`skills: duplicate skill name '${name}'. Skill names must be unique; rename one.`); - } - seen.add(name); - } -} - -/** - * Applies skills to an agent run: appends the "Available skills" block to the - * system prompt and adds a `load_skill` tool. Inputs are not mutated; with no - * skills they are returned as is. - */ -export function withSkills( - agent: AgentConfig, - toolRegistry: ToolRegistry | undefined, - skills: readonly Skill[] | undefined -): { agent: AgentConfig; toolRegistry: ToolRegistry | undefined } { - if (!skills || skills.length === 0) return { agent, toolRegistry }; - assertUsable(skills, agent, toolRegistry); - - const registry = new ToolRegistry(); - for (const [name, descriptor] of Object.entries(toolRegistry?.getAll() ?? {})) { - registry.register(name, descriptor); - } - registry.register(createLoadSkillTool(skills)); - - const block = skillsPromptBlock(skills); - return { - agent: { - ...agent, - prompt: agent.prompt ? `${agent.prompt}\n\n${block}` : block, - tools: { ...agent.tools, [LOAD_SKILL_TOOL]: { tool: LOAD_SKILL_TOOL } }, - }, - toolRegistry: registry, - }; -} diff --git a/src/tools/mcp/McpToolLoader.ts b/src/tools/mcp/McpToolLoader.ts index 8a77bc449..5d973ce64 100644 --- a/src/tools/mcp/McpToolLoader.ts +++ b/src/tools/mcp/McpToolLoader.ts @@ -1,14 +1,16 @@ /** - * MCP Tool Loader (LOU-F2 / LOU-F3) + * MCP Tool Loader (LOU-F2 / LOU-F3 / LOU-Z1 / LOU-Z2) * * Loads tools advertised by a remote MCP server (via an already-connected * `Client` from `@modelcontextprotocol/sdk`) and turns them into raw MCP - * tool descriptions (listRemoteTools). + * tool descriptions (listRemoteTools) or SDK tool descriptors (loadMcpTools). */ import type { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { tool as aiTool } from 'ai'; import { ToolDescriptor } from '../../types'; +import { noopLogger, type Logger } from '../../execution/logger'; +import { handleCallToolResult } from './result'; import { jsonSchemaToZod } from './schema'; /** @@ -43,6 +45,25 @@ export async function listRemoteTools(client: Client): Promise { return response.tools as unknown as RawMcpTool[]; } +/** A tool that {@link loadMcpTools} could not load. */ +export interface SkippedMcpTool { + /** The bare MCP tool name (not namespaced). */ + name: string; + /** Why the tool was skipped. */ + reason: string; +} + +/** Options for {@link loadMcpTools}. */ +export interface LoadMcpToolsOptions { + /** + * Receives a warning for every tool that is skipped (naming the server, + * the tool and the reason). Defaults to a no-op logger. + */ + logger?: Logger; + /** Called once per skipped tool, so callers can surface what was left out. */ + onSkip?: (skipped: SkippedMcpTool) => void; +} + /** * Load a connected MCP client's tools and synthesize a ToolDescriptor for * each one, keyed by `${connectionName}__${tool.name}` so tools from @@ -53,29 +74,58 @@ export async function listRemoteTools(client: Client): Promise { * `client.callTool({ name: tool.name, arguments: args })` - the *raw* * MCP tool name, not the namespaced key - since that's what the remote * server actually knows about. + * + * A tool whose schema cannot be converted is skipped (warned through + * `options.logger`, reported to `options.onSkip`) and never prevents the + * server's other tools from loading. Results with `isError: true` throw an + * {@link McpToolError}; other results keep text, structured and media + * content (see {@link handleCallToolResult}). + * + * @example + * const skipped: SkippedMcpTool[] = []; + * const tools = await loadMcpTools(client, 'github', { + * logger: console, + * onSkip: (s) => skipped.push(s), + * }); */ export async function loadMcpTools( client: Client, - connectionName: string + connectionName: string, + options: LoadMcpToolsOptions = {} ): Promise> { + const { logger = noopLogger, onSkip } = options; const rawTools = await listRemoteTools(client); const descriptors: Record = {}; for (const rawTool of rawTools) { - const key = `${connectionName}__${rawTool.name}`; - const parameters = jsonSchemaToZod(rawTool.inputSchema); - - descriptors[key] = { - displayName: rawTool.description || rawTool.name, - tool: aiTool({ - description: rawTool.description || '', - parameters: parameters as any, - execute: async (args: Record) => { - return client.callTool({ name: rawTool.name, arguments: args }); - }, - }), - }; + try { + descriptors[`${connectionName}__${rawTool.name}`] = buildDescriptor(client, rawTool); + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + logger.warn(`MCP server '${connectionName}': skipping tool '${rawTool.name}': ${reason}`, { + server: connectionName, + tool: rawTool.name, + reason, + }); + onSkip?.({ name: rawTool.name, reason }); + } } return descriptors; } + +function buildDescriptor(client: Client, rawTool: RawMcpTool): ToolDescriptor { + const parameters = jsonSchemaToZod(rawTool.inputSchema); + return { + displayName: rawTool.description || rawTool.name, + tool: aiTool({ + description: rawTool.description || '', + parameters: parameters as any, + execute: async (args: Record) => + handleCallToolResult( + await client.callTool({ name: rawTool.name, arguments: args }), + rawTool.name + ), + }), + }; +} diff --git a/src/tools/mcp/index.ts b/src/tools/mcp/index.ts index 192b1f613..1e7cb1a7a 100644 --- a/src/tools/mcp/index.ts +++ b/src/tools/mcp/index.ts @@ -7,4 +7,5 @@ */ export * from './McpToolLoader'; +export * from './result'; export * from './schema'; diff --git a/src/tools/mcp/loadMcpTools.test.ts b/src/tools/mcp/loadMcpTools.test.ts index 1f7953513..74a0428ea 100644 --- a/src/tools/mcp/loadMcpTools.test.ts +++ b/src/tools/mcp/loadMcpTools.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi } from 'vitest'; +import type { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { loadMcpTools } from './McpToolLoader'; import { createConnectedClient } from './McpToolLoader.test'; @@ -23,6 +24,53 @@ describe('loadMcpTools', () => { expect(result).toMatchObject({ content: [{ type: 'text', text: '5' }] }); }); + it('skips a tool whose schema cannot be converted, warns, reports it and loads the rest', async () => { + const badTool = { + name: 'broken', + get inputSchema(): never { + throw new Error('boom'); + }, + }; + const goodTool = { + name: 'fine', + description: 'ok', + inputSchema: { type: 'object', properties: { a: { $ref: '#/$defs/A' } }, $defs: { A: { anyOf: [{ type: 'string' }, { type: 'null' }] } } }, + }; + const client = { + listTools: async () => ({ tools: [badTool, goodTool] }), + } as unknown as Client; + const logger = { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; + const onSkip = vi.fn(); + + const descriptors = await loadMcpTools(client, 'srv', { logger, onSkip }); + + expect(Object.keys(descriptors)).toEqual(['srv__fine']); + expect(onSkip).toHaveBeenCalledWith({ name: 'broken', reason: 'boom' }); + expect(logger.warn).toHaveBeenCalledTimes(1); + const message = logger.warn.mock.calls[0][0] as string; + expect(message).toContain('srv'); + expect(message).toContain('broken'); + expect(message).toContain('boom'); + }); + + it('loads a tool whose schema uses anyOf and $ref instead of failing the whole server', async () => { + const client = { + listTools: async () => ({ + tools: [ + { + name: 'create', + inputSchema: { + type: 'object', + properties: { labels: { anyOf: [{ type: 'array', items: { type: 'string' } }, { type: 'null' }] } }, + }, + }, + ], + }), + } as unknown as Client; + const descriptors = await loadMcpTools(client, 'srv'); + expect(Object.keys(descriptors)).toEqual(['srv__create']); + }); + it('produces distinctly-named descriptors with zero collision across two connections exposing a tool of the same name', async () => { const { client: linearClient } = await createConnectedClient(); const { client: githubClient } = await createConnectedClient(); diff --git a/src/tools/mcp/mcpResult.test.ts b/src/tools/mcp/mcpResult.test.ts new file mode 100644 index 000000000..b29581d3a --- /dev/null +++ b/src/tools/mcp/mcpResult.test.ts @@ -0,0 +1,131 @@ +import { describe, it, expect, vi } from 'vitest'; +import type { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { AgentExecutor } from '../../execution/AgentExecutor'; +import { AgentBuilder } from '../../core'; +import { AgentType } from '../../types'; +import { mockModel } from '../../testing'; +import { ToolRegistry } from '..'; +import { loadMcpTools } from './McpToolLoader'; +import { McpToolError } from './result'; + +/** A fake MCP client exposing one tool whose call returns `callResult`. */ +function fakeClient(callResult: unknown, inputSchema: unknown = { type: 'object' }) { + return { + listTools: async () => ({ tools: [{ name: 'do_it', description: 'Do it', inputSchema }] }), + callTool: vi.fn(async () => callResult), + } as unknown as Client; +} + +async function run(callResult: unknown) { + const tools = await loadMcpTools(fakeClient(callResult), 'srv'); + return tools['srv__do_it'].tool.execute!({}, {} as never); +} + +describe('MCP result handling (LOU-Z2)', () => { + it('joins text parts and keeps the typed parts', async () => { + const result = await run({ + content: [ + { type: 'text', text: 'line 1' }, + { type: 'text', text: 'line 2' }, + ], + }); + expect(result).toEqual({ + text: 'line 1\nline 2', + content: [ + { type: 'text', text: 'line 1' }, + { type: 'text', text: 'line 2' }, + ], + }); + }); + + it('prefers structuredContent as the result object', async () => { + const result = await run({ + content: [{ type: 'text', text: '{"n":1}' }], + structuredContent: { n: 1 }, + }); + expect(result).toEqual({ n: 1 }); + }); + + it('preserves image, audio and resource parts in a JSON-serializable form', async () => { + const result = await run({ + content: [ + { type: 'text', text: 'here' }, + { type: 'image', data: 'aGk=', mimeType: 'image/png' }, + { type: 'audio', data: 'YXVk', mimeType: 'audio/wav' }, + { type: 'resource', resource: { uri: 'file:///a.txt', mimeType: 'text/plain', text: 'hi' } }, + { type: 'resource', resource: { uri: 'file:///b.bin', blob: 'AAE=' } }, + { type: 'resource_link', uri: 'file:///c', name: 'c', description: 'link' }, + { type: 'hologram', depth: 3 }, + ], + }); + expect(JSON.parse(JSON.stringify(result))).toEqual({ + text: 'here', + content: [ + { type: 'text', text: 'here' }, + { type: 'image', data: 'aGk=', mimeType: 'image/png' }, + { type: 'audio', data: 'YXVk', mimeType: 'audio/wav' }, + { type: 'resource', uri: 'file:///a.txt', mimeType: 'text/plain', text: 'hi' }, + { type: 'resource', uri: 'file:///b.bin', blob: 'AAE=' }, + { type: 'resource_link', uri: 'file:///c', name: 'c', description: 'link' }, + { type: 'unknown', raw: { type: 'hologram', depth: 3 } }, + ], + }); + }); + + it('keeps media next to structuredContent instead of dropping it', async () => { + const result = await run({ + content: [ + { type: 'text', text: 'chart' }, + { type: 'image', data: 'aGk=', mimeType: 'image/png' }, + ], + structuredContent: { points: 3 }, + }); + expect(result).toEqual({ + structuredContent: { points: 3 }, + text: 'chart', + content: [{ type: 'image', data: 'aGk=', mimeType: 'image/png' }], + }); + }); + + it('throws McpToolError with the server text when isError is true', async () => { + const error = await run({ + isError: true, + content: [{ type: 'text', text: 'file not found' }], + }).catch((e: unknown) => e); + expect(error).toBeInstanceOf(McpToolError); + expect((error as Error).name).toBe('McpToolError'); + expect((error as Error).message).toBe('file not found'); + }); + + it('gives an isError result without text a descriptive message', async () => { + await expect(run({ isError: true, content: [] })).rejects.toThrow(/do_it.*error/); + }); + + it('returns a legacy result without a content array unchanged', async () => { + expect(await run({ toolResult: 42 })).toEqual({ toolResult: 42 }); + }); + + it('reaches the model as a structured { error, toolName, message } tool error', async () => { + const client = fakeClient({ isError: true, content: [{ type: 'text', text: 'rate limited' }] }); + const toolRegistry = new ToolRegistry(); + toolRegistry.registerMany(await loadMcpTools(client, 'srv')); + const agent = AgentBuilder.create() + .setType(AgentType.SmartAssistant) + .setName('Test Agent') + .addTool('srv__do_it', { tool: 'srv__do_it', options: {} }) + .build(); + const provider = mockModel([{ toolCalls: [{ name: 'srv__do_it' }] }, 'done']); + const onToolResult = vi.fn(); + + await AgentExecutor.execute({ agent, input: 'go', provider, toolRegistry, onToolResult }); + + const toolMessage = provider.calls[1].messages.find((m) => m.role === 'tool'); + expect(toolMessage?.isError).toBe(true); + expect(JSON.parse(toolMessage!.content as string)).toEqual({ + error: 'McpToolError', + toolName: 'srv__do_it', + message: 'rate limited', + }); + expect(onToolResult.mock.calls[0][1]?.error).toBeDefined(); + }); +}); diff --git a/src/tools/mcp/result.ts b/src/tools/mcp/result.ts new file mode 100644 index 000000000..ae2f9b898 --- /dev/null +++ b/src/tools/mcp/result.ts @@ -0,0 +1,145 @@ +/** + * MCP `CallToolResult` handling (LOU-Z2) + * + * Turns the raw result of `client.callTool()` into what the SDK hands the + * model: an `McpToolError` for `isError` results (so the normal tool-error + * path reports it), otherwise a JSON-serializable result that keeps every + * content part. + */ + +/** A content part of an MCP tool result, in a typed, JSON-serializable form. */ +export type McpContentPart = + | { type: 'text'; text: string } + | { type: 'image'; data: string; mimeType: string } + | { type: 'audio'; data: string; mimeType: string } + | { type: 'resource'; uri: string; mimeType?: string; text?: string; blob?: string } + | { + type: 'resource_link'; + uri: string; + name?: string; + mimeType?: string; + description?: string; + } + /** A part type this SDK version does not know; the raw part is preserved. */ + | { type: 'unknown'; raw: unknown }; + +/** + * Result of a successful MCP tool call that has no `structuredContent`. + * + * `text` is every text part joined with newlines; `content` keeps all parts + * (including image/audio/resource) in order. + */ +export interface McpToolResult { + text: string; + content: McpContentPart[]; +} + +/** + * Result when the server returned `structuredContent` alongside non-text + * parts (images, audio, resources). With only text parts the + * `structuredContent` object itself is returned. + */ +export interface McpStructuredToolResult { + structuredContent: Record; + text: string; + /** The non-text parts, so media is never dropped. */ + content: McpContentPart[]; +} + +/** + * Thrown from a loaded MCP tool when the server reports `isError: true`. + * The model sees `{ error: 'McpToolError', toolName, message }`, where + * `message` is the server's text content. + */ +export class McpToolError extends Error { + constructor(message: string) { + super(message); + this.name = 'McpToolError'; + } +} + +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === 'object' && !Array.isArray(value); +} + +const str = (value: unknown): string | undefined => + typeof value === 'string' ? value : undefined; + +function definedProps(obj: T): T { + return Object.fromEntries(Object.entries(obj).filter(([, v]) => v !== undefined)) as T; +} + +function convertResource(part: Record): McpContentPart { + const resource = isRecord(part.resource) ? part.resource : {}; + return definedProps({ + type: 'resource' as const, + uri: str(resource.uri) ?? '', + mimeType: str(resource.mimeType), + text: str(resource.text), + blob: str(resource.blob), + }); +} + +function convertPart(part: unknown): McpContentPart { + if (!isRecord(part)) return { type: 'unknown', raw: part }; + switch (part.type) { + case 'text': + return { type: 'text', text: str(part.text) ?? '' }; + case 'image': + case 'audio': + return { + type: part.type, + data: str(part.data) ?? '', + mimeType: str(part.mimeType) ?? '', + }; + case 'resource': + return convertResource(part); + case 'resource_link': + return definedProps({ + type: 'resource_link' as const, + uri: str(part.uri) ?? '', + name: str(part.name), + mimeType: str(part.mimeType), + description: str(part.description), + }); + default: + return { type: 'unknown', raw: part }; + } +} + +function joinText(parts: McpContentPart[]): string { + return parts + .filter((part): part is Extract => part.type === 'text') + .map((part) => part.text) + .join('\n'); +} + +/** + * Convert the raw value returned by `client.callTool()`. + * + * - `isError: true` throws an {@link McpToolError} carrying the joined text. + * - `structuredContent` is preferred as the result object. + * - Otherwise `{ text, content }` is returned with all parts preserved. + * - A result without a `content` array (legacy `toolResult` protocol shape) + * is returned unchanged. + */ +export function handleCallToolResult( + raw: unknown, + toolName: string +): McpToolResult | McpStructuredToolResult | Record { + if (!isRecord(raw) || !Array.isArray(raw.content)) { + return isRecord(raw) ? raw : { text: String(raw ?? ''), content: [] }; + } + const content = raw.content.map(convertPart); + const text = joinText(content); + + if (raw.isError === true) { + throw new McpToolError(text || `MCP tool '${toolName}' reported an error without a message`); + } + if (isRecord(raw.structuredContent)) { + const media = content.filter((part) => part.type !== 'text'); + if (media.length === 0) return raw.structuredContent; + return { structuredContent: raw.structuredContent, text, content: media }; + } + return { text, content }; +} diff --git a/src/tools/mcp/schema.test.ts b/src/tools/mcp/schema.test.ts index b0daa1d20..7cc5239de 100644 --- a/src/tools/mcp/schema.test.ts +++ b/src/tools/mcp/schema.test.ts @@ -87,27 +87,240 @@ describe('jsonSchemaToZod', () => { expect(result.success).toBe(true); }); - it('throws for a $ref schema', () => { - const schema = { $ref: '#/definitions/Node' }; - expect(() => jsonSchemaToZod(schema)).toThrow(/\$ref/); + // LOU-Z1: these used to throw; conversion is now permissive and never throws. + it('falls back to z.any() for an unresolvable $ref', () => { + const zodSchema = jsonSchemaToZod({ $ref: '#/definitions/Missing' }); + expect(zodSchema.safeParse({ anything: 1 }).success).toBe(true); }); - it('throws for a recursive schema containing a nested $ref', () => { + it('resolves a recursive $ref one level and uses z.any() for the inner occurrence', () => { + const zodSchema = jsonSchemaToZod({ + $ref: '#/definitions/Node', + definitions: { + Node: { + type: 'object', + properties: { + name: { type: 'string' }, + children: { type: 'array', items: { $ref: '#/definitions/Node' } }, + }, + required: ['name'], + }, + }, + }); + expect(zodSchema.safeParse({ name: 'a', children: [{ name: 'b', children: [{}] }] }).success).toBe( + true + ); + expect(zodSchema.safeParse({ children: [] }).success).toBe(false); + }); + + it('maps type "null" to z.null()', () => { + const zodSchema = jsonSchemaToZod({ type: 'null' }); + expect(zodSchema.safeParse(null).success).toBe(true); + expect(zodSchema.safeParse(1).success).toBe(false); + }); +}); + +describe('jsonSchemaToZod (LOU-Z1 features)', () => { + const accepts = (schema: unknown, value: unknown) => jsonSchemaToZod(schema).safeParse(value).success; + + it('treats unknown, empty and boolean schemas as z.any()', () => { + for (const schema of [{}, true, false, undefined, null, { type: 'weird' }, { format: 'uri', foo: 1 }]) { + expect(accepts(schema, { x: 1 })).toBe(true); + } + }); + + it('converts the nullable anyOf pattern with .nullable()', () => { + const schema = { anyOf: [{ type: 'string' }, { type: 'null' }] }; + expect(accepts(schema, 'a')).toBe(true); + expect(accepts(schema, null)).toBe(true); + expect(accepts(schema, 1)).toBe(false); + }); + + it('converts anyOf / oneOf to a union', () => { + for (const key of ['anyOf', 'oneOf']) { + const schema = { [key]: [{ type: 'string' }, { type: 'integer' }] }; + expect(accepts(schema, 'a')).toBe(true); + expect(accepts(schema, 2)).toBe(true); + expect(accepts(schema, true)).toBe(false); + } + }); + + it('converts type arrays, including null', () => { + const schema = { type: ['string', 'null'] }; + expect(accepts(schema, 'a')).toBe(true); + expect(accepts(schema, null)).toBe(true); + expect(accepts(schema, 1)).toBe(false); + expect(accepts({ type: ['string', 'number'] }, 1)).toBe(true); + }); + + it('merges allOf object schemas', () => { + const schema = { + allOf: [ + { type: 'object', properties: { a: { type: 'string' } }, required: ['a'] }, + { type: 'object', properties: { b: { type: 'number' } }, required: ['b'] }, + ], + }; + expect(accepts(schema, { a: 'x', b: 1 })).toBe(true); + expect(accepts(schema, { a: 'x' })).toBe(false); + }); + + it('intersects allOf schemas that are not both objects', () => { + const schema = { allOf: [{ type: 'string' }, { minLength: 2 }] }; + expect(accepts(schema, 'ab')).toBe(true); + expect(accepts(schema, 1)).toBe(false); + }); + + it('resolves $ref into $defs and decodes JSON pointer escapes', () => { + const schema = { + type: 'object', + properties: { who: { $ref: '#/$defs/a~1b' } }, + required: ['who'], + $defs: { 'a/b': { type: 'string', enum: ['x', 'y'] } }, + }; + expect(accepts(schema, { who: 'x' })).toBe(true); + expect(accepts(schema, { who: 'z' })).toBe(false); + }); + + it('falls back to z.any() for a non-local $ref and for self-reference', () => { + expect(accepts({ $ref: 'https://example.com/s.json' }, 1)).toBe(true); + expect(accepts({ $ref: '#' }, 1)).toBe(true); + }); + + it('supports enum with mixed types and const', () => { + expect(accepts({ enum: ['a', 1, null, true] }, null)).toBe(true); + expect(accepts({ enum: ['a', 1, null, true] }, 1)).toBe(true); + expect(accepts({ enum: ['a', 1, null, true] }, 2)).toBe(false); + expect(accepts({ const: 'fixed' }, 'fixed')).toBe(true); + expect(accepts({ const: 'fixed' }, 'other')).toBe(false); + expect(accepts({ const: { a: 1 } }, { a: 2 })).toBe(true); // object const: permissive + expect(accepts({ enum: [] }, 'anything')).toBe(true); + }); + + it('handles additionalProperties as boolean and as a schema', () => { + const base = { type: 'object', properties: { a: { type: 'string' } } }; + expect(accepts(base, { a: 'x', extra: 1 })).toBe(true); + expect(jsonSchemaToZod(base).parse({ a: 'x', extra: 1 })).toEqual({ a: 'x', extra: 1 }); + expect(accepts({ ...base, additionalProperties: true }, { extra: 1 })).toBe(true); + expect(accepts({ ...base, additionalProperties: false }, { extra: 1 })).toBe(false); + const typed = { ...base, additionalProperties: { type: 'number' } }; + expect(accepts(typed, { a: 'x', n: 1 })).toBe(true); + expect(accepts(typed, { a: 'x', n: 'no' })).toBe(false); + }); + + it('applies defaults so omitted values are filled in', () => { + const schema = jsonSchemaToZod({ + type: 'object', + properties: { limit: { type: 'integer', default: 10 } }, + }); + expect(schema.parse({})).toEqual({ limit: 10 }); + }); + + it('enforces the constraints zod can express', () => { + expect(accepts({ type: 'string', minLength: 2, maxLength: 3 }, 'abcd')).toBe(false); + expect(accepts({ type: 'string', minLength: 2, maxLength: 3 }, 'abc')).toBe(true); + expect(accepts({ type: 'string', pattern: '^[a-z]+$' }, 'abc')).toBe(true); + expect(accepts({ type: 'string', pattern: '^[a-z]+$' }, 'ABC')).toBe(false); + expect(accepts({ type: 'number', minimum: 1, maximum: 5 }, 6)).toBe(false); + expect(accepts({ type: 'number', exclusiveMinimum: 1 }, 1)).toBe(false); + expect(accepts({ type: 'number', exclusiveMaximum: 1 }, 0)).toBe(true); + expect(accepts({ type: 'array', items: { type: 'string' }, minItems: 1, maxItems: 2 }, [])).toBe(false); + expect(accepts({ type: 'array', maxItems: 1 }, [1, 2])).toBe(false); + }); + + it('skips an invalid regex pattern instead of throwing', () => { + expect(accepts({ type: 'string', pattern: '([unclosed' }, 'anything')).toBe(true); + expect(accepts({ type: 'string', pattern: 5 }, 'anything')).toBe(true); + }); + + it('is permissive for tuple items, missing items and untyped properties', () => { + expect(accepts({ type: 'array', items: [{ type: 'string' }] }, [1, 'a'])).toBe(true); + expect(accepts({ type: 'array' }, [1, 'a'])).toBe(true); + expect(accepts({ properties: { a: { type: 'string' } } }, { a: 'x' })).toBe(true); + expect(accepts({ items: { type: 'string' } }, ['x'])).toBe(true); + }); + + it('carries the description through', () => { + expect(jsonSchemaToZod({ type: 'string', description: 'A name' }).description).toBe('A name'); + }); + + it('stops at the depth limit instead of overflowing the stack', () => { + let schema: Record = { type: 'string' }; + for (let i = 0; i < 200; i++) schema = { type: 'array', items: schema }; + expect(() => jsonSchemaToZod(schema)).not.toThrow(); + }); +}); + +describe('jsonSchemaToZod accepts documented payloads of popular MCP servers', () => { + const accepts = (schema: unknown, value: unknown) => jsonSchemaToZod(schema).safeParse(value).success; + + it('filesystem read_file (path, optional head/tail, $schema keyword)', () => { const schema = { type: 'object', properties: { - name: { type: 'string' }, - children: { - type: 'array', - items: { $ref: '#/definitions/Node' }, - }, + path: { type: 'string' }, + tail: { type: 'number', description: 'last N lines' }, + head: { type: 'number', description: 'first N lines' }, }, - required: ['name'], + required: ['path'], + additionalProperties: false, + $schema: 'http://json-schema.org/draft-07/schema#', }; - expect(() => jsonSchemaToZod(schema)).toThrow(/\$ref/); + expect(accepts(schema, { path: '/tmp/a.txt' })).toBe(true); + expect(accepts(schema, { path: '/tmp/a.txt', head: 10 })).toBe(true); + expect(accepts(schema, { head: 10 })).toBe(false); }); - it('throws for an unsupported schema type', () => { - expect(() => jsonSchemaToZod({ type: 'null' })).toThrow(/unsupported/i); + it('GitHub create_issue (optional arrays, nullable fields, numeric milestone)', () => { + const schema = { + type: 'object', + properties: { + owner: { type: 'string' }, + repo: { type: 'string' }, + title: { type: 'string' }, + body: { type: ['string', 'null'] }, + assignees: { type: 'array', items: { type: 'string' } }, + labels: { anyOf: [{ type: 'array', items: { type: 'string' } }, { type: 'null' }] }, + milestone: { anyOf: [{ type: 'number' }, { type: 'null' }] }, + }, + required: ['owner', 'repo', 'title'], + }; + expect(accepts(schema, { owner: 'o', repo: 'r', title: 't' })).toBe(true); + expect( + accepts(schema, { + owner: 'o', + repo: 'r', + title: 't', + body: null, + assignees: ['me'], + labels: ['bug'], + milestone: null, + }) + ).toBe(true); + expect(accepts(schema, { owner: 'o', repo: 'r' })).toBe(false); + }); + + it('a $ref/$defs schema (search with shared filter definition)', () => { + const schema = { + type: 'object', + properties: { + query: { type: 'string' }, + filters: { type: 'array', items: { $ref: '#/$defs/Filter' } }, + sort: { $ref: '#/$defs/Sort' }, + }, + required: ['query'], + $defs: { + Filter: { + type: 'object', + properties: { field: { type: 'string' }, value: { type: ['string', 'number', 'boolean'] } }, + required: ['field', 'value'], + }, + Sort: { type: 'string', enum: ['asc', 'desc'], default: 'asc' }, + }, + }; + expect(accepts(schema, { query: 'q' })).toBe(true); + expect(accepts(schema, { query: 'q', filters: [{ field: 'state', value: 'open' }], sort: 'desc' })).toBe( + true + ); + expect(accepts(schema, { query: 'q', filters: [{ field: 'state' }] })).toBe(false); }); }); diff --git a/src/tools/mcp/schema.ts b/src/tools/mcp/schema.ts index 1df344b45..d4018fc34 100644 --- a/src/tools/mcp/schema.ts +++ b/src/tools/mcp/schema.ts @@ -1,124 +1,288 @@ /** - * JSON Schema -> Zod conversion utility (LOU-F1) + * JSON Schema -> Zod conversion utility (LOU-F1, LOU-Z1) * * Converts the plain-JSON-Schema `inputSchema` objects that MCP tools - * advertise into Zod schemas so they can be plugged straight into the - * 'ai' SDK's `tool({ parameters })` (which expects a ZodTypeAny). + * advertise into Zod schemas so the SDK can validate a model's tool + * arguments before calling the remote server. * - * Deliberately NOT supported in this ticket: `$ref` / recursive schemas. - * MCP tool schemas are typically flat, and properly resolving `$ref` - * (including cycles, which Zod can only express via z.lazy()) is a - * meaningfully bigger problem than the rest of this converter. Rather - * than silently produce a wrong/partial schema, we throw a clear error - * so callers know the schema needs manual handling. + * Since arguments are validated with the converted schema, a too-strict + * conversion would reject calls the server accepts. The converter is + * therefore permissive wherever JSON Schema is ambiguous or a keyword has no + * zod equivalent: it falls back to `z.any()` and never throws on an + * unrecognised keyword, type or shape. */ import { z, ZodTypeAny } from 'zod'; +type JsonSchema = Record; + +/** Resolution state shared by one conversion run. */ +interface Context { + /** The document root, used to resolve local `$ref` pointers. */ + root: unknown; + /** `$ref` pointers currently being expanded (cycle protection). */ + active: ReadonlySet; + depth: number; +} + +/** Nesting deeper than this falls back to `z.any()` (guards pathological input). */ +const MAX_DEPTH = 64; + /** * Convert a JSON Schema object into an equivalent Zod schema. * - * Supported keywords: `type` (object/array/string/number/integer/boolean), - * `enum`, `properties`/`required` (object), `items` (array), `description`. + * Supported: `type` (string or array of types, including `null`), `enum` + * (mixed types), `const`, `anyOf` / `oneOf` (union; `[X, {type:'null'}]` + * becomes `X.nullable()`), `allOf` (merge / intersection), local `$ref` into + * `$defs` / `definitions` (a recursive ref falls back to `z.any()` for the + * inner occurrence), `properties` / `required`, `additionalProperties` + * (boolean or schema), `items`, `default`, `description`, and the + * constraints `minimum` / `maximum` / `exclusiveMinimum` / `exclusiveMaximum`, + * `minLength` / `maxLength` / `pattern` and `minItems` / `maxItems`. An + * invalid `pattern` regex is skipped. * - * Throws for `$ref` (anywhere in the schema, since it signals either a - * genuine cross-reference or a recursive/self-referential schema, neither - * of which this converter attempts to resolve) and for any other - * unsupported/unrecognized schema shape. + * Anything else (unknown keywords, empty or boolean schemas, unresolvable + * refs) becomes permissive `z.any()` - this function does not throw for + * schema content. + * + * @example + * const schema = jsonSchemaToZod({ + * type: 'object', + * properties: { labels: { anyOf: [{ type: 'array', items: { type: 'string' } }, { type: 'null' }] } }, + * }); + * schema.parse({ labels: null }); // ok */ -export function jsonSchemaToZod(schema: any): ZodTypeAny { - return convert(schema); +export function jsonSchemaToZod(schema: unknown): ZodTypeAny { + return convert(schema, { root: schema, active: new Set(), depth: 0 }); } -/** - * Builders for each supported JSON Schema `type`. A Map (not an object - * literal) so that a `type` like 'toString' can't resolve to an inherited - * Object.prototype member. - */ -const TYPE_CONVERTERS = new Map([ - ['object', (schema) => convertObject(schema)], - ['array', (schema) => convertArray(schema)], - ['string', () => z.string()], - ['number', () => z.number()], - ['integer', () => z.number().int()], - ['boolean', () => z.boolean()], -]); - -function convert(schema: any): ZodTypeAny { - assertConvertible(schema); - - // enum takes precedence over `type`, since a schema may specify both. - if (Array.isArray(schema.enum)) { - return applyDescription(convertEnum(schema.enum), schema); - } - - const converter = TYPE_CONVERTERS.get(schema.type); - if (!converter) { - throw new Error( - `jsonSchemaToZod: unsupported schema type '${schema.type}' in ${JSON.stringify(schema)}` - ); - } - return applyDescription(converter(schema), schema); +function isRecord(value: unknown): value is JsonSchema { + return value !== null && typeof value === 'object' && !Array.isArray(value); } -/** Reject non-objects and `$ref` schemas up front. */ -function assertConvertible(schema: unknown): void { - if (schema === null || typeof schema !== 'object') { - throw new Error( - `jsonSchemaToZod: expected a JSON Schema object, got ${JSON.stringify(schema)}` - ); - } +function convert(schema: unknown, ctx: Context): ZodTypeAny { + if (!isRecord(schema) || ctx.depth > MAX_DEPTH) return z.any(); + const next: Context = { ...ctx, depth: ctx.depth + 1 }; + return applyDescription(convertKeywords(schema, next), schema); +} - if ('$ref' in schema) { - throw new Error( - 'jsonSchemaToZod: $ref (and recursive schemas) are not supported in this converter' - ); - } +function convertKeywords(schema: JsonSchema, ctx: Context): ZodTypeAny { + if (typeof schema.$ref === 'string') return convertRef(schema.$ref, ctx); + if ('const' in schema) return convertConst(schema.const); + if (Array.isArray(schema.enum)) return convertEnum(schema.enum); + if (Array.isArray(schema.allOf)) return convertAllOf(schema, ctx); + const variants = schema.anyOf ?? schema.oneOf; + if (Array.isArray(variants)) return convertUnion(variants, ctx); + return withDefault(convertTyped(schema, ctx), schema); } -function applyDescription(zodType: ZodTypeAny, schema: any): ZodTypeAny { +function applyDescription(zodType: ZodTypeAny, schema: JsonSchema): ZodTypeAny { if (typeof schema.description === 'string' && schema.description.length > 0) { return zodType.describe(schema.description); } return zodType; } -function convertEnum(values: unknown[]): ZodTypeAny { - if (values.length === 0) { - throw new Error('jsonSchemaToZod: enum must have at least one value'); +function withDefault(zodType: ZodTypeAny, schema: JsonSchema): ZodTypeAny { + return 'default' in schema && schema.default !== undefined + ? zodType.default(schema.default) + : zodType; +} + +// --- $ref ------------------------------------------------------------------ + +/** Resolve a local JSON pointer (`#/$defs/Foo`) against the document root. */ +function resolvePointer(root: unknown, ref: string): unknown { + if (ref === '#') return root; + if (!ref.startsWith('#/')) return undefined; + let node: unknown = root; + for (const raw of ref.slice(2).split('/')) { + let segment = raw; + try { + segment = decodeURIComponent(raw); + } catch { + // keep the raw segment + } + segment = segment.replace(/~1/g, '/').replace(/~0/g, '~'); + if (!isRecord(node) && !Array.isArray(node)) return undefined; + node = (node as Record)[segment]; } + return node; +} + +function convertRef(ref: string, ctx: Context): ZodTypeAny { + if (ctx.active.has(ref)) return z.any(); + const target = resolvePointer(ctx.root, ref); + if (target === undefined) return z.any(); + return convert(target, { ...ctx, active: new Set(ctx.active).add(ref) }); +} + +// --- const / enum ---------------------------------------------------------- + +function isPrimitive(value: unknown): value is string | number | boolean | null { + return value === null || ['string', 'number', 'boolean'].includes(typeof value); +} + +function convertConst(value: unknown): ZodTypeAny { + return isPrimitive(value) ? z.literal(value) : z.any(); +} + +function convertEnum(values: unknown[]): ZodTypeAny { + if (values.length === 0 || !values.every(isPrimitive)) return z.any(); if (values.every((v) => typeof v === 'string')) { return z.enum(values as [string, ...string[]]); } - // Non-string enums (numbers, booleans, mixed) fall back to a union of - // literals, which Zod's z.enum() cannot express directly. - const literals = values.map((v) => z.literal(v as any)); - if (literals.length === 1) { - return literals[0]; + return unionOf(values.map((v) => z.literal(v as string | number | boolean | null))); +} + +// --- unions / allOf -------------------------------------------------------- + +function unionOf(members: ZodTypeAny[]): ZodTypeAny { + if (members.length === 0) return z.any(); + if (members.length === 1) return members[0]; + return z.union(members as [ZodTypeAny, ZodTypeAny, ...ZodTypeAny[]]); +} + +function isNullSchema(schema: unknown): boolean { + return isRecord(schema) && schema.type === 'null' && Object.keys(schema).every( + (key) => key === 'type' || key === 'description' + ); +} + +function convertUnion(variants: unknown[], ctx: Context): ZodTypeAny { + const nullable = variants.some(isNullSchema); + const rest = variants.filter((variant) => !isNullSchema(variant)); + const union = unionOf(rest.map((variant) => convert(variant, ctx))); + if (!nullable) return union; + return rest.length === 0 ? z.null() : union.nullable(); +} + +function mergeTwo(a: ZodTypeAny, b: ZodTypeAny): ZodTypeAny { + if (a instanceof z.ZodObject && b instanceof z.ZodObject) return a.merge(b); + return z.intersection(a, b); +} + +function convertAllOf(schema: JsonSchema, ctx: Context): ZodTypeAny { + const { allOf, ...own } = schema; + const parts = (allOf as unknown[]).map((part) => convert(part, ctx)); + if (Object.keys(own).some((key) => key !== 'description')) { + parts.unshift(convert(own, ctx)); + } + return parts.length === 0 ? z.any() : parts.reduce(mergeTwo); +} + +// --- typed schemas --------------------------------------------------------- + +function convertTyped(schema: JsonSchema, ctx: Context): ZodTypeAny { + if (Array.isArray(schema.type)) return convertTypeArray(schema, schema.type, ctx); + return convertSingleType(schema, schema.type, ctx); +} + +function convertTypeArray(schema: JsonSchema, types: unknown[], ctx: Context): ZodTypeAny { + const nullable = types.includes('null'); + const members = types + .filter((type) => type !== 'null') + .map((type) => convertSingleType(schema, type, ctx)); + const union = unionOf(members); + if (!nullable) return union; + return members.length === 0 ? z.null() : union.nullable(); +} + +function convertSingleType(schema: JsonSchema, type: unknown, ctx: Context): ZodTypeAny { + switch (type) { + case 'object': + return convertObject(schema, ctx); + case 'array': + return convertArray(schema, ctx); + case 'string': + return convertString(schema); + case 'number': + return convertNumber(schema, false); + case 'integer': + return convertNumber(schema, true); + case 'boolean': + return z.boolean(); + case 'null': + return z.null(); + default: + return inferFromKeywords(schema, ctx); } - return z.union(literals as unknown as [ZodTypeAny, ZodTypeAny, ...ZodTypeAny[]]); } -function convertObject(schema: any): ZodTypeAny { - const properties = schema.properties ?? {}; - const required: string[] = Array.isArray(schema.required) ? schema.required : []; +/** No (or an unknown) `type`: infer from structural keywords, else `z.any()`. */ +function inferFromKeywords(schema: JsonSchema, ctx: Context): ZodTypeAny { + if (isRecord(schema.properties)) return convertObject(schema, ctx); + if (schema.items !== undefined) return convertArray(schema, ctx); + return z.any(); +} + +function convertObject(schema: JsonSchema, ctx: Context): ZodTypeAny { + const properties = isRecord(schema.properties) ? schema.properties : {}; + const required = Array.isArray(schema.required) ? schema.required : []; const shape: Record = {}; for (const [key, propSchema] of Object.entries(properties)) { - let propZod = convert(propSchema); - if (!required.includes(key)) { - propZod = propZod.optional(); - } - shape[key] = propZod; + const propZod = convert(propSchema, ctx); + // A ZodDefault already accepts `undefined`; wrapping it in optional() would skip the default. + const keepAsIs = required.includes(key) || propZod instanceof z.ZodDefault; + shape[key] = keepAsIs ? propZod : propZod.optional(); } + return applyAdditionalProperties(z.object(shape), schema.additionalProperties, ctx); +} - return z.object(shape); +/** + * JSON Schema allows extra properties unless `additionalProperties` says + * otherwise, so unknown keys pass through by default instead of being + * stripped from the arguments the server receives. + */ +function applyAdditionalProperties( + object: z.ZodObject>, + additional: unknown, + ctx: Context +): ZodTypeAny { + if (additional === false) return object.strict(); + if (isRecord(additional)) return object.catchall(convert(additional, ctx)); + return object.passthrough(); } -function convertArray(schema: any): ZodTypeAny { - if (schema.items === undefined) { - throw new Error('jsonSchemaToZod: array schema is missing "items"'); +function convertArray(schema: JsonSchema, ctx: Context): ZodTypeAny { + // Tuple-form `items` (an array) is ambiguous across drafts: stay permissive. + const item = isRecord(schema.items) ? convert(schema.items, ctx) : z.any(); + let array = z.array(item); + if (typeof schema.minItems === 'number') array = array.min(schema.minItems); + if (typeof schema.maxItems === 'number') array = array.max(schema.maxItems); + return array; +} + +function convertString(schema: JsonSchema): ZodTypeAny { + let str = z.string(); + if (typeof schema.minLength === 'number') str = str.min(schema.minLength); + if (typeof schema.maxLength === 'number') str = str.max(schema.maxLength); + const pattern = compilePattern(schema.pattern); + return pattern ? str.regex(pattern) : str; +} + +/** Compile a `pattern`; an invalid regex is skipped rather than thrown. */ +function compilePattern(pattern: unknown): RegExp | undefined { + if (typeof pattern !== 'string') return undefined; + try { + return new RegExp(pattern, 'u'); + } catch { + try { + return new RegExp(pattern); + } catch { + return undefined; + } } - const itemZod = convert(schema.items); - return z.array(itemZod); +} + +function convertNumber(schema: JsonSchema, integer: boolean): ZodTypeAny { + let num = z.number(); + if (integer) num = num.int(); + if (typeof schema.minimum === 'number') num = num.min(schema.minimum); + if (typeof schema.maximum === 'number') num = num.max(schema.maximum); + // Draft-6+ numeric form only; the draft-4 boolean form is ignored (permissive). + if (typeof schema.exclusiveMinimum === 'number') num = num.gt(schema.exclusiveMinimum); + if (typeof schema.exclusiveMaximum === 'number') num = num.lt(schema.exclusiveMaximum); + return num; }