diff --git a/plugins/plugin-skilled-openapi/README.md b/plugins/plugin-skilled-openapi/README.md index 62ca77919..bf90ec629 100644 --- a/plugins/plugin-skilled-openapi/README.md +++ b/plugins/plugin-skilled-openapi/README.md @@ -62,11 +62,11 @@ loading a skill, the model runs its operations with a script: ## The three tools -| Tool | Input | What it does | -| -------------- | ---------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `search_skill` | `query`, `limit?` (default 20), `tags?`, `notQuery?` | Searches the skills and returns `{ skills: [{ skillId, name, description, score }] }`. Its description lists the skills the caller may use, so the model sees what the server can do. | -| `load_skill` | `skillId` | Returns the skill's markdown instructions and its `actions`, with their input and output JSON Schemas: the `actionId`s a script calls. | -| `run_workflow` | `script` | Runs an AgentScript program (a subset of JavaScript) in the `@enclave-vm` sandbox. Each `await callTool(actionId, input)` calls an operation; the script's `return` value is the result. | +| Tool | Input | What it does | +| -------------- | ---------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `search_skill` | `query`, `limit?` (default 20), `tags?`, `notQuery?` | Searches the skills and returns `{ skills: [{ skillId, name, description, score, bundleVersion }] }`. Its description lists the skills the caller may use, so the model sees what the server can do, and loads a skill it names with `load_skill` directly, without searching. | +| `load_skill` | `skillId`, or `skillIds` (up to 10) | Returns the skill's markdown instructions and its `actions`, with their input and output JSON Schemas: the `actionId`s a script calls. `skillId` takes the name the catalog shows, or an id; `skillIds` loads several, under `skills`. Its description tells the model to load each skill once per conversation, and again only when its `bundleVersion` changes. | +| `run_workflow` | `script` | Runs an AgentScript program (a subset of JavaScript) in the `@enclave-vm` sandbox. Each `await callTool(actionId, input)` calls an operation; the script's `return` value is the result. | For each `callTool`, the plugin checks the caller against the operation's and skill's `requiredAuthorities`, validates the input against the operation's `inputSchema`, sends the request with the credential from the bundle's auth binding, diff --git a/plugins/plugin-skilled-openapi/src/__tests__/load-skill.guidance.spec.ts b/plugins/plugin-skilled-openapi/src/__tests__/load-skill.guidance.spec.ts new file mode 100644 index 000000000..8a7079efa --- /dev/null +++ b/plugins/plugin-skilled-openapi/src/__tests__/load-skill.guidance.spec.ts @@ -0,0 +1,100 @@ +/** + * `search_skill` and `load_skill` lead the model to the calls it needs (#1068): a skill the catalog names loads by + * that name with no search first, several load in one call, and the descriptions say so, and that a loaded skill stays + * in the model's context until its `bundleVersion` changes. + */ +import 'reflect-metadata'; + +import { compileSkilledBundleFromOpenApi } from '@frontmcp/adapters/skills'; +import { FrontMcpInstance, LogLevel, type DirectMcpServer } from '@frontmcp/sdk'; + +import { SkilledOpenApiPlugin } from '../index'; +import type { LoadSkillOutput } from '../tools/load-skill.schema'; + +const operation = (operationId: string, summary: string) => ({ + get: { operationId, summary, responses: { 200: { description: 'OK' } } }, +}); + +let server: DirectMcpServer; + +beforeAll(async () => { + const spec = { + openapi: '3.0.3', + info: { title: 'Pet shop', version: '1.0.0' }, + paths: { + '/pets': operation('findPets', 'Finds pets.'), + '/store/inventory': operation('getInventory', 'Returns pet inventories by status.'), + }, + }; + const bundle = await compileSkilledBundleFromOpenApi( + spec, + [ + { + id: 'pet-catalog', + name: 'Pet catalog', + description: 'Find dogs, cats and birds.', + instructions: '# Pet catalog', + operationIds: ['findPets'], + }, + { + id: 'store', + name: 'Store', + description: 'Count the pets in stock, and read an order.', + instructions: '# Store\n\nUse getInventory for the inventory.', + operationIds: ['getInventory'], + }, + ], + { bundleId: 'shop', version: '1.0.0', baseUrl: 'https://shop.example' }, + ); + server = await FrontMcpInstance.createDirect({ + info: { name: 'shop', version: '1.0.0' }, + apps: [], + logging: { level: LogLevel.Off }, + plugins: [SkilledOpenApiPlugin.init({ source: { type: 'inline', content: bundle }, requireSignature: false })], + }); +}); + +afterAll(async () => { + await server?.dispose(); +}); + +const load = async (args: Record) => + (await server.callTool('load_skill', args)).structuredContent as unknown as LoadSkillOutput; + +describe('load_skill (#1068)', () => { + it('loads a skill by the name the catalog shows, with no search first', async () => { + const { skill } = await load({ skillId: 'Store' }); + + expect(skill?.id).toBe('store'); + expect(skill?.actions?.map((a) => a.actionId)).toEqual(['getInventory']); + expect(skill?.bundleVersion).toBe('1.0.0'); + }); + + it('loads several skills in one call, each once', async () => { + const { skills, skill } = await load({ skillIds: ['Store', 'pet-catalog', 'store'] }); + + expect(skills?.map((r) => r.skill.id)).toEqual(['store', 'pet-catalog']); + expect(skills?.every((r) => r.isComplete)).toBe(true); + expect(skill).toBeUndefined(); + }); +}); + +describe('the tool descriptions (#1068)', () => { + const descriptionOf = async (name: string) => + (await server.listTools()).tools.find((tool) => tool.name === name)?.description ?? ''; + + it('tell the model to load a skill the catalog names directly', async () => { + const description = await descriptionOf('search_skill'); + + expect(description).toContain('call `load_skill` with that name directly: no\nsearch is needed'); + expect(description).not.toContain('Use\nthis tool first'); + }); + + it('tell the model to load each skill once, and again only when its bundleVersion changed', async () => { + const description = await descriptionOf('load_skill'); + + expect(description).toContain('Load each skill once per conversation'); + expect(description).toContain('only when its `bundleVersion` changed'); + expect(description).toContain('`skillIds`'); + }); +}); diff --git a/plugins/plugin-skilled-openapi/src/__tests__/meta-tools.spec.ts b/plugins/plugin-skilled-openapi/src/__tests__/meta-tools.spec.ts index 2a3f2d952..384f7f80f 100644 --- a/plugins/plugin-skilled-openapi/src/__tests__/meta-tools.spec.ts +++ b/plugins/plugin-skilled-openapi/src/__tests__/meta-tools.spec.ts @@ -204,11 +204,59 @@ describe('load_skill', () => { hiddenOps.set(buildEntry('s1', 'a1')); const ctx = makeToolThis({ scopeSkills: { loadSkill, hasAny: jest.fn(() => true) }, hiddenOps }); const result = await LoadSkillTool.prototype.execute.call(ctx, { skillId: 's1' }); - expect(result.skill.actions?.[0]?.actionId).toBe('a1'); - expect(result.skill.bundleVersion).toBe('v9'); + expect(result.skill?.actions?.[0]?.actionId).toBe('a1'); + expect(result.skill?.bundleVersion).toBe('v9'); expect(result.isComplete).toBe(true); }); + it('loads several skills at once with skillIds, each once, in the order asked for (#1068)', async () => { + const content = (id: string, name: string): SkillContent => ({ + id, + name, + description: 'd', + instructions: `# ${name}`, + tools: [], + }); + const byKey: Record = { + s1: content('s1', 'S1'), + S1: content('s1', 'S1'), + s2: content('s2', 'S2'), + }; + const loadSkill = jest.fn(async (key: string) => + byKey[key] ? { skill: byKey[key], availableTools: [], missingTools: [], isComplete: true } : undefined, + ); + const ctx = makeToolThis({ scopeSkills: { loadSkill, hasAny: jest.fn(() => true) } }); + const result = await LoadSkillTool.prototype.execute.call(ctx, { skillIds: ['s2', 'S1', 's1'] }); + + expect(result.skills?.map((r) => r.skill.id)).toEqual(['s2', 's1']); + expect(result.skill).toBeUndefined(); + }); + + it('refuses a call with neither skillId nor skillIds, or with both', async () => { + const ctx = makeToolThis({ scopeSkills: { loadSkill: jest.fn(), hasAny: jest.fn(() => true) } }); + await expect(LoadSkillTool.prototype.execute.call(ctx, {})).rejects.toThrow('Pass either skillId or skillIds'); + await expect(LoadSkillTool.prototype.execute.call(ctx, { skillId: 's1', skillIds: ['s1'] })).rejects.toThrow( + 'Pass either skillId or skillIds', + ); + }); + + it('answers not found for skillIds when one of them is not found', async () => { + const loadSkill = jest.fn(async (key: string) => + key === 's1' + ? { + skill: { id: 's1', name: 'S1', description: 'd', instructions: '', tools: [] }, + availableTools: [], + missingTools: [], + isComplete: true, + } + : undefined, + ); + const ctx = makeToolThis({ scopeSkills: { loadSkill, hasAny: jest.fn(() => true) } }); + await expect(LoadSkillTool.prototype.execute.call(ctx, { skillIds: ['s1', 'nope'] })).rejects.toThrow( + 'Skill "nope" not found', + ); + }); + it('throws when skill is not found', async () => { const loadSkill = jest.fn(async () => undefined); const ctx = makeToolThis({ scopeSkills: { loadSkill, hasAny: jest.fn(() => false) } }); diff --git a/plugins/plugin-skilled-openapi/src/tools/load-skill.schema.ts b/plugins/plugin-skilled-openapi/src/tools/load-skill.schema.ts index 5422ddd8c..294309cda 100644 --- a/plugins/plugin-skilled-openapi/src/tools/load-skill.schema.ts +++ b/plugins/plugin-skilled-openapi/src/tools/load-skill.schema.ts @@ -1,61 +1,90 @@ import { z } from '@frontmcp/lazy-zod'; -export const loadSkillDescription = `Load the full instructions and executable actions for a specific skill. +/** The most skills one \`load_skill\` call loads. */ +export const MAX_SKILLS_PER_LOAD = 10; -Use this AFTER \`search_skill\` once you've identified the right skill for the -user's task. The returned object contains: +export const loadSkillDescription = `Load the full instructions and executable actions of one or more skills. + +Pass the name the \`search_skill\` catalog shows, or a \`skillId\` from a search +result. Load several skills at once with \`skillIds\` (up to ${MAX_SKILLS_PER_LOAD}). + +Load each skill once per conversation: its instructions and actions stay in your +context, so use them from there for the next steps of the task. Load it again +only when its \`bundleVersion\` changed (\`search_skill\` results report the +current one), or when its content is no longer in your context. + +The returned object contains: - \`instructions\`: markdown the LLM should read carefully before invoking - \`actions\`: each action's input/output JSON Schema and required authorities - \`bundleVersion\`: changes when the bundle is hot-swapped (use it to detect drift) -INPUT: { skillId } -OUTPUT: { skill: { id, name, description, instructions, actions[] }, isComplete }`; +INPUT: { skillId } or { skillIds } +OUTPUT: { skill: { id, name, description, instructions, actions[] }, isComplete } for skillId, + { skills: Array<{ skill, isComplete }> } for skillIds`; + +const loadedSkillSchema = z.object({ + id: z.string(), + name: z.string(), + description: z.string(), + instructions: z.string(), + bundleVersion: z.string().optional(), + actions: z + .array( + z.object({ + actionId: z.string(), + summary: z.string(), + description: z.string().optional(), + inputJsonSchema: z.record(z.string(), z.unknown()), + outputJsonSchema: z.record(z.string(), z.unknown()), + requiredAuthorities: z.record(z.string(), z.unknown()).optional(), + }), + ) + .optional(), +}); export const loadSkillInputSchema = { - skillId: z.string().min(1).max(256).describe('Stable skill identifier (returned by search_skill)'), + skillId: z + .string() + .min(1) + .max(256) + .optional() + .describe('Name or id of the skill to load (the catalog shows names; search_skill returns ids)'), + skillIds: z + .array(z.string().min(1).max(256)) + .min(1) + .max(MAX_SKILLS_PER_LOAD) + .optional() + .describe(`Names or ids of several skills to load at once (up to ${MAX_SKILLS_PER_LOAD}), in place of skillId`), }; export const loadSkillOutputSchema = { - skill: z.object({ - id: z.string(), - name: z.string(), - description: z.string(), - instructions: z.string(), - bundleVersion: z.string().optional(), - actions: z - .array( - z.object({ - actionId: z.string(), - summary: z.string(), - description: z.string().optional(), - inputJsonSchema: z.record(z.string(), z.unknown()), - outputJsonSchema: z.record(z.string(), z.unknown()), - requiredAuthorities: z.record(z.string(), z.unknown()).optional(), - }), - ) - .optional(), - }), - isComplete: z.boolean(), + skill: loadedSkillSchema.optional(), + isComplete: z.boolean().optional(), warning: z.string().optional(), + skills: z + .array(z.object({ skill: loadedSkillSchema, isComplete: z.boolean(), warning: z.string().optional() })) + .optional(), }; -export type LoadSkillInput = { skillId: string }; -export type LoadSkillOutput = { - skill: { - id: string; - name: string; - description: string; - instructions: string; - bundleVersion?: string; - actions?: Array<{ - actionId: string; - summary: string; - description?: string; - inputJsonSchema: Record; - outputJsonSchema: Record; - requiredAuthorities?: Record; - }>; - }; - isComplete: boolean; - warning?: string; +export type LoadSkillInput = { skillId?: string; skillIds?: string[] }; + +export type LoadedSkill = { + id: string; + name: string; + description: string; + instructions: string; + bundleVersion?: string; + actions?: Array<{ + actionId: string; + summary: string; + description?: string; + inputJsonSchema: Record; + outputJsonSchema: Record; + requiredAuthorities?: Record; + }>; }; + +export type LoadSkillResult = { skill: LoadedSkill; isComplete: boolean; warning?: string }; + +/** One skill's answer for \`skillId\`, or every skill's under \`skills\` for \`skillIds\`. */ +export type LoadSkillOutput = Partial & { skills?: LoadSkillResult[] }; diff --git a/plugins/plugin-skilled-openapi/src/tools/load-skill.tool.ts b/plugins/plugin-skilled-openapi/src/tools/load-skill.tool.ts index 9eb148552..5332d90d6 100644 --- a/plugins/plugin-skilled-openapi/src/tools/load-skill.tool.ts +++ b/plugins/plugin-skilled-openapi/src/tools/load-skill.tool.ts @@ -14,6 +14,7 @@ import { loadSkillOutputSchema, type LoadSkillInput, type LoadSkillOutput, + type LoadSkillResult, } from './load-skill.schema'; @Tool({ @@ -28,6 +29,9 @@ import { }) export default class LoadSkillTool extends ToolContext { async execute(input: LoadSkillInput): Promise { + if ((input.skillId === undefined) === (input.skillIds === undefined)) { + throw new PublicMcpError('Pass either skillId or skillIds', 'INVALID_PARAMS', 400); + } // Await the first bundle apply so the skill registry is populated (stateless // workers have no background loop to finish a deferred sync). await this.get(BundleSyncService).ensureReady(); @@ -39,13 +43,6 @@ export default class LoadSkillTool extends ToolContext { // message instead of leaking implementation details. throw new InternalMcpError('SkillRegistry is not available on the active scope', 'SKILL_REGISTRY_UNAVAILABLE'); } - const notFound = () => - // Caller-visible: skill id was not registered, or the caller may not see it - // (the same answer, so a refused skill's existence isn't revealed). Map to a - // 404 with a stable code so MCP clients can branch on it. - new PublicMcpError(`Skill "${input.skillId}" not found`, 'SKILL_NOT_FOUND', 404); - const result = await skillRegistry.loadSkill(input.skillId); - if (!result) throw notFound(); // The skill's own rules (bundle `requiredAuthorities`, `@Skill` authorities, the // `skills:filter` flow) decide whether the caller may read it at all, and each @@ -58,23 +55,43 @@ export default class LoadSkillTool extends ToolContext { unprotectedOps: config.unprotectedOps, authInfo: this.authInfo, }); - const skill = result.skill; - if (!(await visibility.isVisible(input.skillId)) || !(await visibility.isVisible(skill.id))) throw notFound(); - const actions = skill.actions - ? await visibility.visibleActions(skill.id, skill.actions, this.get(HiddenOpRegistry)) - : undefined; + const hiddenOps = this.get(HiddenOpRegistry); - return { - skill: { - id: skill.id, - name: skill.name, - description: skill.description, - instructions: skill.instructions, - ...(skill.bundleVersion !== undefined && { bundleVersion: skill.bundleVersion }), - ...(actions ? { actions } : {}), - }, - isComplete: result.isComplete, - ...(result.warning !== undefined && { warning: result.warning }), + const load = async (skillId: string): Promise => { + // Caller-visible: skill id was not registered, or the caller may not see it + // (the same answer, so a refused skill's existence isn't revealed). Map to a + // 404 with a stable code so MCP clients can branch on it. + const notFound = () => new PublicMcpError(`Skill "${skillId}" not found`, 'SKILL_NOT_FOUND', 404); + const result = await skillRegistry.loadSkill(skillId); + if (!result) throw notFound(); + const skill = result.skill; + if (!(await visibility.isVisible(skillId)) || !(await visibility.isVisible(skill.id))) throw notFound(); + const actions = skill.actions ? await visibility.visibleActions(skill.id, skill.actions, hiddenOps) : undefined; + return { + skill: { + id: skill.id, + name: skill.name, + description: skill.description, + instructions: skill.instructions, + ...(skill.bundleVersion !== undefined && { bundleVersion: skill.bundleVersion }), + ...(actions ? { actions } : {}), + }, + isComplete: result.isComplete, + ...(result.warning !== undefined && { warning: result.warning }), + }; }; + + if (input.skillId !== undefined) return load(input.skillId); + + // Each skill once, in the order asked for; a name and its id are the same skill. + const skills: LoadSkillResult[] = []; + const loaded = new Set(); + for (const skillId of input.skillIds ?? []) { + const result = await load(skillId); + if (loaded.has(result.skill.id)) continue; + loaded.add(result.skill.id); + skills.push(result); + } + return { skills }; } } diff --git a/plugins/plugin-skilled-openapi/src/tools/search-skill.schema.ts b/plugins/plugin-skilled-openapi/src/tools/search-skill.schema.ts index e0f6b7727..80de3655e 100644 --- a/plugins/plugin-skilled-openapi/src/tools/search-skill.schema.ts +++ b/plugins/plugin-skilled-openapi/src/tools/search-skill.schema.ts @@ -5,11 +5,15 @@ export const searchSkillDescription = `Search the available skills by free-form A "skill" is a curated bundle of REST operations exposed to you behind a single named capability — instead of seeing each individual API endpoint, you see one -skill that knows how to do something useful (e.g. "billing", "customers"). Use -this tool first to discover what skills exist for the user's request, then call -\`load_skill\` to read its instructions + the actions it offers, and \`run_workflow\` -to execute — a short sandboxed script that calls the skill's actions via -\`callTool(actionId, input)\` (chaining several in one round-trip). +skill that knows how to do something useful (e.g. "billing", "customers"). + +The catalog below lists the skills you may use, by name. When it names the +skill the user's request needs, call \`load_skill\` with that name directly: no +search is needed. Search when the catalog shows no fitting skill, or is +truncated. Then call \`load_skill\` to read a skill's instructions and the +actions it offers, and \`run_workflow\` to execute — a short sandboxed script +that calls the skill's actions via \`callTool(actionId, input)\` (chaining +several in one round-trip). INPUT: - query: short natural-language description of what you want to do @@ -19,7 +23,7 @@ INPUT: demoted (e.g. query "rate limiting", notQuery "enforcement" to prefer guidance over enforcement skills) -OUTPUT: { skills: Array<{ skillId, name, description, score }> }`; +OUTPUT: { skills: Array<{ skillId, name, description, score, bundleVersion? }> }`; export const searchSkillInputSchema = { query: z.string().min(1).max(2048).describe('Natural-language search query'),