diff --git a/apps/e2e/demo-e2e-dts-consumer/e2e/dts-consumer.e2e.spec.ts b/apps/e2e/demo-e2e-dts-consumer/e2e/dts-consumer.e2e.spec.ts index fcbfb5a3f..3b4432b97 100644 --- a/apps/e2e/demo-e2e-dts-consumer/e2e/dts-consumer.e2e.spec.ts +++ b/apps/e2e/demo-e2e-dts-consumer/e2e/dts-consumer.e2e.spec.ts @@ -32,6 +32,7 @@ const fixtureFiles = [ 'host-must-compile.ts', 'skills-must-compile.ts', 'output-schemas-must-compile.ts', + 'providers-must-compile.ts', ].map((fileName) => path.join(fixtureDir, fileName)); function compileFixture(withoutExpectErrors?: string): { diagnostics: FixtureDiagnostic[]; sourceFiles: string[] } { @@ -129,7 +130,7 @@ describe('@frontmcp/sdk declarations in a strict consumer project', () => { expect(describeEach(inFixtureFile('host-must-compile.ts'))).toEqual([]); }); - it('types this.scope.skills.setExternalProvider() and skillsConfig.externalProvider', () => { + it('types this.scope.skills.setExternalProvider(), skillsConfig.externalProvider and its instructions and annotation policies', () => { expect(describeEach(inFixtureFile('skills-must-compile.ts'))).toEqual([]); }); @@ -137,6 +138,10 @@ describe('@frontmcp/sdk declarations in a strict consumer project', () => { expect(describeEach(inFixtureFile('output-schemas-must-compile.ts'))).toEqual([]); }); + it('types providers without a name, AsyncProvider with and without inject, useValue and symbol or string tokens in every entry point', () => { + expect(describeEach(inFixtureFile('providers-must-compile.ts'))).toEqual([]); + }); + it('ships declaration files that type-check with skipLibCheck disabled', () => { const sdkDeclarationDiagnostics = diagnostics.filter((diagnostic) => diagnostic.fileName.startsWith(sdkDeclarationsDir + path.sep), diff --git a/apps/e2e/demo-e2e-dts-consumer/fixture/providers-must-compile.ts b/apps/e2e/demo-e2e-dts-consumer/fixture/providers-must-compile.ts new file mode 100644 index 000000000..5f90b0d70 --- /dev/null +++ b/apps/e2e/demo-e2e-dts-consumer/fixture/providers-must-compile.ts @@ -0,0 +1,79 @@ +import 'reflect-metadata'; + +import { + App, + AsyncProvider, + FrontMcp, + FrontMcpInstance, + Provider, + ProviderScope, + type FrontMcpConfigInput, + type ProviderType, + type Token, +} from '@frontmcp/sdk'; + +interface Settings { + region: string; +} + +interface Store { + read(key: string): string | undefined; +} + +const SETTINGS: Token = Symbol('Settings'); +const STORE: Token = Symbol('Store'); +const GREETING: Token = 'greeting'; + +@Provider() +class Clock { + now(): number { + return Date.now(); + } +} + +@Provider({ scope: ProviderScope.CONTEXT }) +class RequestCounter { + count = 0; +} + +const settingsValue = { provide: SETTINGS, useValue: { region: 'eu' } }; + +const greetingValue = { provide: GREETING, useValue: 'hello', name: 'greeting' }; + +const storeWithInject = AsyncProvider({ + provide: STORE, + inject: () => [Clock] as const, + useFactory: async (clock: Clock): Promise => ({ read: (key) => `${clock.now()}:${key}` }), +}); + +const storeWithoutInject = AsyncProvider({ + provide: STORE, + useFactory: (): Store => ({ read: () => undefined }), +}); + +const providers: ProviderType[] = [Clock, RequestCounter, settingsValue, greetingValue, storeWithInject]; + +@App({ name: 'providers', providers: [settingsValue, storeWithInject, storeWithoutInject, Clock] }) +class ProvidersApp {} + +@FrontMcp({ + info: { name: 'providers', version: '1.0.0' }, + apps: [ProvidersApp], + providers: [settingsValue, greetingValue, storeWithInject, storeWithoutInject, RequestCounter], +}) +export class ProvidersServer {} + +const config: FrontMcpConfigInput = { + info: { name: 'providers', version: '1.0.0' }, + apps: [ProvidersApp], + providers, +}; + +export async function startEveryWay(): Promise { + await FrontMcpInstance.runStdio({ ...config, providers: [storeWithoutInject, settingsValue] }); + await FrontMcpInstance.bootstrap({ ...config, providers: [storeWithInject, settingsValue] }); + const direct = await FrontMcpInstance.createDirect({ ...config, providers: [storeWithInject, greetingValue] }); + await direct.dispose(); + const socket = await FrontMcpInstance.runUnixSocket({ ...config, socketPath: '/tmp/providers.sock' }); + await socket.close(); +} diff --git a/apps/e2e/demo-e2e-dts-consumer/fixture/skills-must-compile.ts b/apps/e2e/demo-e2e-dts-consumer/fixture/skills-must-compile.ts index bf45feb51..1adef9988 100644 --- a/apps/e2e/demo-e2e-dts-consumer/fixture/skills-must-compile.ts +++ b/apps/e2e/demo-e2e-dts-consumer/fixture/skills-must-compile.ts @@ -5,6 +5,7 @@ import { FrontMcp, type ScopeEntry, type SkillContent, + type SkillsConfigOptions, type SkillSearchResult, } from '@frontmcp/sdk'; @@ -41,6 +42,13 @@ export function useCatalog(scope: ScopeEntry): void { info: { name: 'dts-skills', version: '1.0.0' }, apps: [], serve: false, - skillsConfig: { externalProvider: new CatalogSkillProvider({ mode: 'read-only' }) }, + skillsConfig: { + externalProvider: new CatalogSkillProvider({ mode: 'read-only' }), + injectInstructions: 'compact', + resourceAnnotations: 'full', + }, }) export class SkillsServer {} + +// @ts-expect-error resourceAnnotations accepts 'full' | 'no-priority' | 'off' only +export const unknownAnnotations: SkillsConfigOptions = { resourceAnnotations: 'audience' }; diff --git a/apps/e2e/demo-e2e-dts-consumer/fixture/tsconfig.json b/apps/e2e/demo-e2e-dts-consumer/fixture/tsconfig.json index 23db0788c..366629440 100644 --- a/apps/e2e/demo-e2e-dts-consumer/fixture/tsconfig.json +++ b/apps/e2e/demo-e2e-dts-consumer/fixture/tsconfig.json @@ -17,6 +17,7 @@ "hooks-must-fail.ts", "host-must-compile.ts", "skills-must-compile.ts", - "output-schemas-must-compile.ts" + "output-schemas-must-compile.ts", + "providers-must-compile.ts" ] } diff --git a/apps/e2e/demo-e2e-feature-flags/e2e/feature-flags-instructions.e2e.spec.ts b/apps/e2e/demo-e2e-feature-flags/e2e/feature-flags-instructions.e2e.spec.ts index 229fed190..3b31c62f5 100644 --- a/apps/e2e/demo-e2e-feature-flags/e2e/feature-flags-instructions.e2e.spec.ts +++ b/apps/e2e/demo-e2e-feature-flags/e2e/feature-flags-instructions.e2e.spec.ts @@ -1,17 +1,37 @@ /** - * E2E: the skill catalog in the `initialize` instructions runs the `skills:filter` flow for the - * caller, like `skills/list`, so a flag-disabled skill's name and description never reach a client (#603). + * E2E: the skills in the `initialize` instructions run the `skills:filter` flow for the caller, like + * `skills/list`, so a flag-disabled skill's name and description never reach a client (#603). * * - `enabled-workflow` (skill) sits behind `flag-for-skill`, which is on * - `hidden-workflow` (skill) sits behind `flag-for-hidden-skill`, which is off */ import { expect, test } from '@frontmcp/testing'; +const SERVER = 'apps/e2e/demo-e2e-feature-flags/src/main.ts'; + test.describe('Feature Flags in the initialize instructions E2E (#603)', () => { + test.use({ server: SERVER, project: 'demo-e2e-feature-flags', publicMode: true }); + + test('sends one compact line that counts only the flag-enabled skill', async ({ mcp }) => { + expect(mcp.instructions).toBe( + "This server has 1 skill: the `skill://index.json` resource lists each skill's `SKILL.md` URI.", + ); + }); + + test('leaves the same skill out of the SKILL.md entries in resources/list', async ({ mcp }) => { + const resources = await mcp.resources.list(); + + expect(resources).toContainResource('skill://enabled-workflow/SKILL.md'); + expect(resources).not.toContainResource('skill://hidden-workflow/SKILL.md'); + }); +}); + +test.describe("Feature Flags in the initialize instructions E2E with injectInstructions: 'append' (#603)", () => { test.use({ - server: 'apps/e2e/demo-e2e-feature-flags/src/main.ts', + server: SERVER, project: 'demo-e2e-feature-flags', publicMode: true, + env: { SKILLS_INJECT_INSTRUCTIONS: 'append' }, }); test('lists a flag-enabled skill in the skill catalog', async ({ mcp }) => { @@ -22,11 +42,4 @@ test.describe('Feature Flags in the initialize instructions E2E (#603)', () => { expect(mcp.instructions).not.toContain('hidden-workflow'); expect(mcp.instructions).not.toContain('(disabled)'); }); - - test('leaves the same skill out of the SKILL.md entries in resources/list', async ({ mcp }) => { - const resources = await mcp.resources.list(); - - expect(resources).toContainResource('skill://enabled-workflow/SKILL.md'); - expect(resources).not.toContainResource('skill://hidden-workflow/SKILL.md'); - }); }); diff --git a/apps/e2e/demo-e2e-feature-flags/src/main.ts b/apps/e2e/demo-e2e-feature-flags/src/main.ts index 8d415128e..d9aef5e06 100644 --- a/apps/e2e/demo-e2e-feature-flags/src/main.ts +++ b/apps/e2e/demo-e2e-feature-flags/src/main.ts @@ -4,6 +4,7 @@ import { FlaggedApp } from './apps/flagged'; import { GateDefaultsApp } from './apps/gate-defaults'; const port = parseInt(process.env['PORT'] ?? '3115', 10); +const injectInstructions = process.env['SKILLS_INJECT_INSTRUCTIONS'] === 'append' ? 'append' : undefined; @FrontMcp({ info: { name: 'Demo E2E Feature Flags', version: '0.1.0' }, @@ -18,5 +19,6 @@ const port = parseInt(process.env['PORT'] ?? '3115', 10); transport: { protocol: { json: true, legacy: true, strictSession: false }, }, + skillsConfig: { injectInstructions }, }) export default class Server {} diff --git a/apps/e2e/demo-e2e-skills/e2e/skills-only-mode.e2e.spec.ts b/apps/e2e/demo-e2e-skills/e2e/skills-only-mode.e2e.spec.ts index 817dfc79d..d745c7039 100644 --- a/apps/e2e/demo-e2e-skills/e2e/skills-only-mode.e2e.spec.ts +++ b/apps/e2e/demo-e2e-skills/e2e/skills-only-mode.e2e.spec.ts @@ -5,6 +5,7 @@ * - tools/list returns an empty array * - tools/call is answered as for an unknown tool (`Tool "" not found`) * - skills/search and skills/load work normally + * - the initialize instructions still point at the skill index * * This mode is useful for planner agents that: * - Fetch skills to create execution plans @@ -129,6 +130,21 @@ test.describe('MCP Skills-Only Mode E2E', () => { } }); + test('should point the client at the skill index in its instructions', async ({ server }) => { + const client = await server + .createClientBuilder() + .withTransport('streamable-http') + .withQueryParams({ mode: 'skills_only' }) + .buildAndConnect(); + + try { + expect(client.instructions).toMatch(/^This server has \d+ skills: the `skill:\/\/index\.json` resource/); + expect(client.instructions).not.toContain('**review-pr**'); + } finally { + await client.disconnect(); + } + }); + test('should refuse tool execution in skills-only mode', async ({ server }) => { const builder = server.createClientBuilder(); const client = await builder diff --git a/apps/e2e/demo-e2e-skills/e2e/skills-resources.e2e.spec.ts b/apps/e2e/demo-e2e-skills/e2e/skills-resources.e2e.spec.ts index 3b4878027..33094b04a 100644 --- a/apps/e2e/demo-e2e-skills/e2e/skills-resources.e2e.spec.ts +++ b/apps/e2e/demo-e2e-skills/e2e/skills-resources.e2e.spec.ts @@ -6,8 +6,9 @@ * - skill://{skillPath}/SKILL.md — raw SKILL.md (frontmatter + body) * - skill://{skillPath}/{filePath} — any file inside the skill directory * - Auto-complete for skill paths + * - The annotations of each SKILL.md resource in resources/list (`skillsConfig.resourceAnnotations`) */ -import { expect, test } from '@frontmcp/testing'; +import { expect, test, type Resource } from '@frontmcp/testing'; import type { JsonRpcRequest, JsonRpcResponse } from './helpers/skills-protocol'; @@ -66,6 +67,12 @@ interface SkillIndexDocument { skills: SkillIndexEntry[]; } +async function listSkillMdResources(mcp: { resources: { list(): Promise } }): Promise { + const skillMds = (await mcp.resources.list()).filter((resource) => resource.uri.endsWith('/SKILL.md')); + expect(skillMds.length).toBeGreaterThan(0); + return skillMds; +} + test.describe('SEP-2640 Skills Resources E2E', () => { test.use({ server: 'apps/e2e/demo-e2e-skills/src/main.ts', @@ -97,6 +104,24 @@ test.describe('SEP-2640 Skills Resources E2E', () => { const result = await mcp.resources.read(pointedAt ?? ''); expect(result).toBeSuccessful(); }); + + test('default to one line that counts the skills instead of listing them', async ({ mcp }) => { + const index = extractResourceJson(await mcp.resources.read('skill://index.json')); + const skillMdCount = index.skills.filter((entry) => entry.type === 'skill-md').length; + + expect(mcp.instructions).toBe( + `This server has ${skillMdCount} skills: the \`skill://index.json\` resource lists each skill's \`SKILL.md\` URI.`, + ); + }); + }); + + test.describe('SKILL.md annotations in resources/list', () => { + test('carry the assistant audience and no numeric annotation by default', async ({ mcp }) => { + for (const resource of await listSkillMdResources(mcp)) { + expect(resource.annotations?.audience).toEqual(['assistant']); + expect(Object.values(resource.annotations ?? {}).filter((value) => typeof value === 'number')).toEqual([]); + } + }); }); test.describe('skill://index.json', () => { @@ -263,3 +288,18 @@ test.describe('SEP-2640 Skills Resources E2E', () => { }); }); }); + +test.describe("SEP-2640 Skills Resources E2E with resourceAnnotations: 'full'", () => { + test.use({ + server: 'apps/e2e/demo-e2e-skills/src/main.ts', + project: 'demo-e2e-skills', + publicMode: true, + env: { SKILLS_RESOURCE_ANNOTATIONS: 'full' }, + }); + + test('bring back priority 0.8 on every SKILL.md resource', async ({ mcp }) => { + for (const resource of await listSkillMdResources(mcp)) { + expect(resource.annotations).toMatchObject({ audience: ['assistant'], priority: 0.8 }); + } + }); +}); diff --git a/apps/e2e/demo-e2e-skills/src/main.ts b/apps/e2e/demo-e2e-skills/src/main.ts index 46105da3c..207870878 100644 --- a/apps/e2e/demo-e2e-skills/src/main.ts +++ b/apps/e2e/demo-e2e-skills/src/main.ts @@ -9,6 +9,7 @@ const port = parseInt(process.env['PORT'] ?? '3107', 10); // the long-standing public-mode server (no authorities) so all existing skills // e2es behave exactly as before. const authoritiesMode = process.env['AUTHORITIES_MODE'] === '1'; +const resourceAnnotations = process.env['SKILLS_RESOURCE_ANNOTATIONS'] === 'full' ? 'full' : undefined; if (authoritiesMode) { const idpProviderUrl = process.env['IDP_PROVIDER_URL'] || 'https://sample-app.frontegg.com'; @@ -65,6 +66,7 @@ if (authoritiesMode) { enabled: true, auth: 'public', // Single auth config for all HTTP endpoints mcpResources: true, // Enable SEP-2640 skill:// MCP resource templates + resourceAnnotations, }, }) class Server {} diff --git a/apps/e2e/demo-e2e-testing/e2e/jest-preset-unit.e2e.spec.ts b/apps/e2e/demo-e2e-testing/e2e/jest-preset-unit.e2e.spec.ts new file mode 100644 index 000000000..5388ed9b6 --- /dev/null +++ b/apps/e2e/demo-e2e-testing/e2e/jest-preset-unit.e2e.spec.ts @@ -0,0 +1,35 @@ +import { spawnSync } from 'node:child_process'; +import { join } from 'node:path'; +import { stripVTControlCharacters } from 'node:util'; + +const root = join(__dirname, '../../../..'); +const fixtureDir = join(__dirname, '../fixture/ts-jest-cjs'); +const jestBin = join(root, 'node_modules/jest/bin/jest.js'); + +function runFixtureJest(config: string): { status: number | null; output: string } { + const result = spawnSync(process.execPath, [jestBin, '--config', config, '--ci', '--no-cache'], { + cwd: fixtureDir, + env: { ...process.env, CI: '1' }, + encoding: 'utf8', + timeout: 110_000, + }); + return { status: result.status, output: stripVTControlCharacters(`${result.stdout}${result.stderr}`) }; +} + +describe('@frontmcp/testing/jest-preset-unit in a ts-jest CommonJS project', () => { + it('runs a spec that verifies a token with jose and runs a plugin hook in process', () => { + const { status, output } = runFixtureJest('jest.config.js'); + + expect(output).not.toMatch(/Unexpected token 'export'/); + expect(output).toMatch(/Tests:\s+2 passed, 2 total/); + expect(status).toBe(0); + }); + + it('is what that project needs: without the preset, Jest cannot load jose', () => { + const { status, output } = runFixtureJest('jest.without-preset.config.js'); + + expect(output).toMatch(/node_modules[/\\]jose[/\\]/); + expect(output).toMatch(/SyntaxError: Unexpected token 'export'/); + expect(status).not.toBe(0); + }); +}); diff --git a/apps/e2e/demo-e2e-testing/fixture/ts-jest-cjs/jest.config.js b/apps/e2e/demo-e2e-testing/fixture/ts-jest-cjs/jest.config.js new file mode 100644 index 000000000..947f30617 --- /dev/null +++ b/apps/e2e/demo-e2e-testing/fixture/ts-jest-cjs/jest.config.js @@ -0,0 +1,4 @@ +module.exports = { + preset: '@frontmcp/testing/jest-preset-unit', + transform: { '^.+\\.tsx?$': ['ts-jest', { tsconfig: '/tsconfig.json' }] }, +}; diff --git a/apps/e2e/demo-e2e-testing/fixture/ts-jest-cjs/jest.without-preset.config.js b/apps/e2e/demo-e2e-testing/fixture/ts-jest-cjs/jest.without-preset.config.js new file mode 100644 index 000000000..e1f9570b6 --- /dev/null +++ b/apps/e2e/demo-e2e-testing/fixture/ts-jest-cjs/jest.without-preset.config.js @@ -0,0 +1,4 @@ +module.exports = { + testEnvironment: 'node', + transform: { '^.+\\.tsx?$': ['ts-jest', { tsconfig: '/tsconfig.json' }] }, +}; diff --git a/apps/e2e/demo-e2e-testing/fixture/ts-jest-cjs/src/auth-server.spec.ts b/apps/e2e/demo-e2e-testing/fixture/ts-jest-cjs/src/auth-server.spec.ts new file mode 100644 index 000000000..7717a2178 --- /dev/null +++ b/apps/e2e/demo-e2e-testing/fixture/ts-jest-cjs/src/auth-server.spec.ts @@ -0,0 +1,109 @@ +import 'reflect-metadata'; + +import { MCP_20260728_META, PROTOCOL_2026_07_28 } from '@frontmcp/protocol'; +import { + App, + AsyncProvider, + FrontMcpInstance, + LogLevel, + Plugin, + Tool, + ToolContext, + ToolHook, + type FlowCtxOf, +} from '@frontmcp/sdk'; +import { TestTokenFactory } from '@frontmcp/testing'; + +const ISSUER = 'https://idp.example.com'; +const GREETING = Symbol('greeting'); +const toolsSeenByHook: string[] = []; + +@Plugin({ name: 'call-recorder' }) +class CallRecorderPlugin { + @ToolHook.Will('execute') + recordCall(ctx: FlowCtxOf<'tools:call-tool'>): void { + toolsSeenByHook.push(ctx.toolName); + } +} + +@Tool({ name: 'greet', inputSchema: {} }) +class GreetTool extends ToolContext { + async execute() { + return { greeting: `${this.get(GREETING)} ${this.auth.user.sub}` }; + } +} + +@App({ + name: 'greeter', + tools: [GreetTool], + plugins: [CallRecorderPlugin], + providers: [AsyncProvider({ provide: GREETING, useFactory: async () => 'hello' })], +}) +class GreeterApp {} + +const tokens = new TestTokenFactory({ issuer: ISSUER, audience: ISSUER }); +let handler: (request: Request) => Promise; + +beforeAll(async () => { + handler = await FrontMcpInstance.createFetchHandler({ + info: { name: 'ts-jest-cjs', version: '1.0.0' }, + apps: [GreeterApp], + logging: { level: LogLevel.Off }, + auth: { + mode: 'transparent', + provider: ISSUER, + providerConfig: { jwks: await tokens.getPublicJwks() }, + expectedAudience: ISSUER, + }, + }); +}); + +function callGreet(token: string): Promise { + return handler( + new Request('http://localhost/', { + method: 'POST', + headers: { + 'content-type': 'application/json', + accept: 'application/json, text/event-stream', + 'mcp-protocol-version': PROTOCOL_2026_07_28, + 'mcp-method': 'tools/call', + 'mcp-name': 'greet', + authorization: `Bearer ${token}`, + }, + body: JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { + name: 'greet', + arguments: {}, + _meta: { + [MCP_20260728_META.protocolVersion]: PROTOCOL_2026_07_28, + [MCP_20260728_META.clientInfo]: { name: 'ts-jest-cjs', version: '1.0.0' }, + [MCP_20260728_META.clientCapabilities]: {}, + }, + }, + }), + }), + ); +} + +describe('a FrontMCP server in a ts-jest CommonJS project', () => { + it('verifies the bearer token with jose and runs the plugin hook in process', async () => { + const response = await callGreet(await tokens.createTestToken({ sub: 'ada' })); + const body = (await response.json()) as { result?: { structuredContent?: unknown } }; + + expect(response.status).toBe(200); + expect(body.result?.structuredContent).toEqual({ greeting: 'hello ada' }); + expect(toolsSeenByHook).toEqual(['greet']); + }); + + it('refuses a token signed with another key', async () => { + const forged = await new TestTokenFactory({ issuer: ISSUER, audience: ISSUER }).createTestToken({ sub: 'eve' }); + + const response = await callGreet(forged); + + expect(response.status).toBe(401); + expect(toolsSeenByHook).toEqual(['greet']); + }); +}); diff --git a/apps/e2e/demo-e2e-testing/fixture/ts-jest-cjs/tsconfig.json b/apps/e2e/demo-e2e-testing/fixture/ts-jest-cjs/tsconfig.json new file mode 100644 index 000000000..c63b7be09 --- /dev/null +++ b/apps/e2e/demo-e2e-testing/fixture/ts-jest-cjs/tsconfig.json @@ -0,0 +1,13 @@ +{ + "compilerOptions": { + "target": "es2022", + "module": "commonjs", + "strict": true, + "esModuleInterop": true, + "experimentalDecorators": true, + "emitDecoratorMetadata": true, + "skipLibCheck": true, + "types": ["jest", "node"] + }, + "include": ["src/**/*.ts"] +} diff --git a/libs/di/src/__tests__/provider.metadata.spec.ts b/libs/di/src/__tests__/provider.metadata.spec.ts index 488cb91ef..ad76d4c59 100644 --- a/libs/di/src/__tests__/provider.metadata.spec.ts +++ b/libs/di/src/__tests__/provider.metadata.spec.ts @@ -31,6 +31,11 @@ describe('ProviderScope', () => { describe('providerMetadataSchema', () => { describe('valid inputs', () => { + it('should accept metadata without a name, which the token then provides', () => { + const result = providerMetadataSchema.safeParse({}); + expect(result.success).toBe(true); + }); + it('should accept minimal metadata with just name', () => { const result = providerMetadataSchema.safeParse({ name: 'TestProvider' }); expect(result.success).toBe(true); @@ -82,11 +87,6 @@ describe('providerMetadataSchema', () => { }); describe('invalid inputs', () => { - it('should reject missing name', () => { - const result = providerMetadataSchema.safeParse({}); - expect(result.success).toBe(false); - }); - it('should reject empty name', () => { const result = providerMetadataSchema.safeParse({ name: '' }); expect(result.success).toBe(false); diff --git a/libs/di/src/__tests__/token.utils.spec.ts b/libs/di/src/__tests__/token.utils.spec.ts index 72cce911a..d04ee496e 100644 --- a/libs/di/src/__tests__/token.utils.spec.ts +++ b/libs/di/src/__tests__/token.utils.spec.ts @@ -3,16 +3,17 @@ */ import 'reflect-metadata'; + +import { DESIGN_PARAMTYPES, META_ASYNC_WITH, META_ASYNC_WITH_TOKENS } from '../tokens/di.constants.js'; import { - tokenName, + depsOfClass, + depsOfFunc, + getAsyncWithTokens, isClass, isPromise, - getAsyncWithTokens, readWithParamTypes, - depsOfClass, - depsOfFunc, + tokenName, } from '../utils/token.utils.js'; -import { DESIGN_PARAMTYPES, META_ASYNC_WITH, META_ASYNC_WITH_TOKENS } from '../tokens/di.constants.js'; describe('token utils', () => { describe('tokenName', () => { @@ -35,6 +36,24 @@ describe('token utils', () => { expect(tokenName(null as any)).toBe('[ref]'); }); + it('should return the description of a symbol token', () => { + expect(tokenName(Symbol('api-url'))).toBe('api-url'); + }); + + it('should return [ref] for a symbol without a description', () => { + expect(tokenName(Symbol())).toBe('[ref]'); + expect(tokenName(Symbol(''))).toBe('[ref]'); + }); + + it('should return the function name for an arrow function token', () => { + const makeStore = () => ({}); + expect(tokenName(makeStore)).toBe('makeStore'); + }); + + it('should return a string token itself', () => { + expect(tokenName('greeting')).toBe('greeting'); + }); + it('should return function name for named functions', () => { function myFunction() {} expect(tokenName(myFunction)).toBe('myFunction'); diff --git a/libs/di/src/metadata/provider.metadata.ts b/libs/di/src/metadata/provider.metadata.ts index 5a2dff553..65041e309 100644 --- a/libs/di/src/metadata/provider.metadata.ts +++ b/libs/di/src/metadata/provider.metadata.ts @@ -34,10 +34,8 @@ export interface ProviderMetadata { */ id?: string; - /** - * Human-readable name for the provider. - */ - name: string; + /** Name shown in logs and errors; `@frontmcp/sdk` names a provider without one after its token. */ + name?: string; /** * Optional description of what the provider does. diff --git a/libs/di/src/metadata/provider.schema.ts b/libs/di/src/metadata/provider.schema.ts index ed785bbb6..36a2b01d0 100644 --- a/libs/di/src/metadata/provider.schema.ts +++ b/libs/di/src/metadata/provider.schema.ts @@ -27,7 +27,7 @@ export type RawZodShape = { export const providerMetadataSchema = z .object({ id: z.string().optional(), - name: z.string().min(1), + name: z.string().min(1).optional(), description: z.string().optional(), scope: z.nativeEnum(ProviderScope).optional().default(ProviderScope.GLOBAL), } satisfies RawZodShape) diff --git a/libs/di/src/utils/token.utils.ts b/libs/di/src/utils/token.utils.ts index 3d855a5ec..f1265b52a 100644 --- a/libs/di/src/utils/token.utils.ts +++ b/libs/di/src/utils/token.utils.ts @@ -13,10 +13,13 @@ import { getMetadata, hasAsyncWith } from './metadata.utils.js'; * Get a human-readable name for a token. * * @param t - The token to get the name for - * @returns A string representation of the token + * @returns The class name, the symbol's description, the string token itself, or `[ref]` */ -export const tokenName = (t: Token): string => - typeof t === 'function' && !!(t as any).prototype ? (t as any).name : ((t as any)?.name ?? '[ref]'); +export const tokenName = (t: Token): string => { + if (typeof t === 'string') return t; + if (typeof t === 'symbol') return t.description || '[ref]'; + return t?.name ?? '[ref]'; +}; /** * Check if a value is a class constructor. diff --git a/libs/sdk/src/common/decorators/front-mcp.decorator.ts b/libs/sdk/src/common/decorators/front-mcp.decorator.ts index c3316cecd..5bc2fd226 100644 --- a/libs/sdk/src/common/decorators/front-mcp.decorator.ts +++ b/libs/sdk/src/common/decorators/front-mcp.decorator.ts @@ -74,6 +74,13 @@ export function FrontMcp(providedMetadata: FrontMcpMetadata): ClassDecorator { if (formatted.providers) { throw new InvalidDecoratorMetadataError('FrontMcp', 'providers', JSON.stringify(formatted.providers, null, 2)); } + if (formatted.skillsConfig) { + throw new InvalidDecoratorMetadataError( + 'FrontMcp', + 'skillsConfig', + JSON.stringify(formatted.skillsConfig, null, 2), + ); + } const loggingFormat = formatted['logging'] as Record | undefined; if (loggingFormat?.['transports']) { throw new InvalidDecoratorMetadataError( diff --git a/libs/sdk/src/common/decorators/provider.decorator.ts b/libs/sdk/src/common/decorators/provider.decorator.ts index 7a10f5407..0839b37d2 100644 --- a/libs/sdk/src/common/decorators/provider.decorator.ts +++ b/libs/sdk/src/common/decorators/provider.decorator.ts @@ -6,7 +6,7 @@ import { FrontMcpProviderTokens } from '../tokens'; /** * Decorator that marks a class as a McpProvider module and provides metadata */ -function FrontMcpProvider(providedMetadata: ProviderMetadata): ClassDecorator { +function FrontMcpProvider(providedMetadata: ProviderMetadata = {}): ClassDecorator { return (target: Function) => { const metadata = frontMcpProviderMetadataSchema.parse(providedMetadata); diff --git a/libs/sdk/src/common/interfaces/__tests__/provider-type.spec.ts b/libs/sdk/src/common/interfaces/__tests__/provider-type.spec.ts index b088185ac..03ee2c310 100644 --- a/libs/sdk/src/common/interfaces/__tests__/provider-type.spec.ts +++ b/libs/sdk/src/common/interfaces/__tests__/provider-type.spec.ts @@ -2,9 +2,12 @@ import 'reflect-metadata'; import { ProviderKind } from '@frontmcp/di'; -import { InvalidDecoratorMetadataError } from '../../../errors'; +import { DependencyCycleError } from '../../../errors'; +import ProviderRegistry from '../../../provider/provider.registry'; import { normalizeProvider } from '../../../provider/provider.utils'; import { Plugin } from '../../decorators/plugin.decorator'; +import { Provider } from '../../decorators/provider.decorator'; +import { ProviderScope } from '../../metadata'; import { annotatedFrontMcpProvidersSchema } from '../../schemas/annotated-class.schema'; import { AsyncProvider, type ProviderType } from '../provider.interface'; @@ -36,15 +39,69 @@ describe('ProviderType factory providers', () => { expect(provider.inject?.()).toEqual([Punctuation]); }); - it('require a name, as the decorators do', () => { + it('take a factory without a name, as the decorators do', () => { expect(() => { - @Plugin({ - name: 'unnamed-factory', - // @ts-expect-error -- a factory provider without `name` is refused at runtime - providers: [{ provide: GREETING, inject: () => [], useFactory: () => 'hello' }], - }) + @Plugin({ name: 'unnamed-factory', providers: [{ provide: GREETING, useFactory: () => 'hello' }] }) class UnnamedFactoryPlugin {} return UnnamedFactoryPlugin; - }).toThrow(InvalidDecoratorMetadataError); + }).not.toThrow(); + }); +}); + +describe('a provider without a name', () => { + @Provider() + class Clock {} + + @Provider({ scope: ProviderScope.CONTEXT }) + class Counter {} + + @Provider({ name: 'named-clock' }) + class NamedClock {} + + @Provider() + class WallClock extends NamedClock {} + + it.each([ + [ + 'a symbol token, from its description', + { provide: Symbol('api-url'), useValue: 'https://api.example.com' }, + 'api-url', + ], + ['a string token, from the string', { provide: 'greeting', useFactory: () => 'hello' }, 'greeting'], + ['a class token, from the class name', { provide: Punctuation, useClass: Punctuation }, 'Punctuation'], + ])('is named from %s', (_case, provider: ProviderType, name) => { + expect(annotatedFrontMcpProvidersSchema.safeParse(provider).success).toBe(true); + expect(normalizeProvider(provider).metadata.name).toBe(name); + }); + + it('is named from its class when @Provider() names none', () => { + expect(normalizeProvider(Clock).metadata.name).toBe('Clock'); + expect(normalizeProvider(Counter).metadata).toMatchObject({ name: 'Counter', scope: ProviderScope.CONTEXT }); + expect(normalizeProvider(WallClock).metadata.name).toBe('WallClock'); + }); + + it('keeps the name it is given', () => { + expect(normalizeProvider(NamedClock).metadata.name).toBe('named-clock'); + expect(normalizeProvider({ name: 'api', provide: Symbol('api-url'), useValue: 'x' }).metadata.name).toBe('api'); + }); + + it('names its symbol token by its description in a dependency cycle', () => { + const alpha = Symbol('alpha'); + const beta = Symbol('beta'); + const building = () => + new ProviderRegistry([ + { provide: alpha, inject: () => [beta], useFactory: (dependency: unknown) => dependency }, + { provide: beta, inject: () => [alpha], useFactory: (dependency: unknown) => dependency }, + ]); + + expect(building).toThrow(new DependencyCycleError('alpha -> beta -> alpha')); + }); + + it('is still refused with an empty name', () => { + expect(() => { + @Provider({ name: '' }) + class EmptyName {} + return EmptyName; + }).toThrow(); }); }); diff --git a/libs/sdk/src/common/metadata/provider.metadata.ts b/libs/sdk/src/common/metadata/provider.metadata.ts index 11b4cc1c8..5a1b1b21d 100644 --- a/libs/sdk/src/common/metadata/provider.metadata.ts +++ b/libs/sdk/src/common/metadata/provider.metadata.ts @@ -10,7 +10,7 @@ import { type RawZodShape } from '../types'; export const frontMcpProviderMetadataSchema = z .object({ id: z.string().optional(), - name: z.string().min(1), + name: z.string().min(1).optional(), description: z.string().optional(), scope: z.nativeEnum(ProviderScope).optional().default(ProviderScope.GLOBAL), } satisfies RawZodShape) diff --git a/libs/sdk/src/common/schemas/annotated-class.schema.ts b/libs/sdk/src/common/schemas/annotated-class.schema.ts index 06eb65341..a48f99019 100644 --- a/libs/sdk/src/common/schemas/annotated-class.schema.ts +++ b/libs/sdk/src/common/schemas/annotated-class.schema.ts @@ -105,7 +105,7 @@ export const annotatedFrontMcpProvidersSchema = z.custom( { message: 'providers items must be classes annotated with @Provider() | @FrontMcpProvider(), ' + - 'or { provide, name, useValue | useClass | useFactory } objects.', + 'or { provide, useValue | useClass | useFactory } objects.', }, ); diff --git a/libs/sdk/src/common/types/options/__tests__/skills-http.options.spec.ts b/libs/sdk/src/common/types/options/__tests__/skills-http.options.spec.ts index 070c16bc3..ea587bf70 100644 --- a/libs/sdk/src/common/types/options/__tests__/skills-http.options.spec.ts +++ b/libs/sdk/src/common/types/options/__tests__/skills-http.options.spec.ts @@ -1,5 +1,9 @@ // common/types/options/__tests__/skills-http.options.spec.ts +import 'reflect-metadata'; + +import { InvalidDecoratorMetadataError } from '../../../../errors'; +import { FrontMcp } from '../../../decorators/front-mcp.decorator'; import { normalizeEndpointConfig, normalizeSkillsConfigOptions, @@ -98,6 +102,42 @@ describe('skillsConfigOptionsSchema', () => { const result = skillsConfigOptionsSchema.parse({}); expect(result.api).toBe(true); }); + + it("should apply default injectInstructions='compact'", () => { + const result = skillsConfigOptionsSchema.parse({}); + expect(result.injectInstructions).toBe('compact'); + }); + + it("should apply default resourceAnnotations='no-priority'", () => { + const result = skillsConfigOptionsSchema.parse({}); + expect(result.resourceAnnotations).toBe('no-priority'); + }); + }); + + describe('instructions and annotation policies', () => { + it.each(['off', 'compact', 'append', 'prepend', 'replace'])('should accept injectInstructions=%s', (policy) => { + expect(skillsConfigOptionsSchema.safeParse({ injectInstructions: policy }).success).toBe(true); + }); + + it.each(['full', 'no-priority', 'off'])('should accept resourceAnnotations=%s', (policy) => { + expect(skillsConfigOptionsSchema.safeParse({ resourceAnnotations: policy }).success).toBe(true); + }); + + it.each([ + ['injectInstructions', 'everything'], + ['resourceAnnotations', 'audience'], + ])('should refuse an unknown %s at boot', (option, value) => { + const boot = () => + FrontMcp({ + info: { name: 'invalid-skills-config', version: '1.0.0' }, + apps: [], + serve: false, + skillsConfig: { [option]: value }, + } as never)(class InvalidSkillsConfigServer {}); + + expect(boot).toThrow(InvalidDecoratorMetadataError); + expect(boot).toThrow(new RegExp(`skillsConfig[\\s\\S]*${option}`)); + }); }); describe('validation', () => { diff --git a/libs/sdk/src/common/types/options/skills-http/index.ts b/libs/sdk/src/common/types/options/skills-http/index.ts index 52a32ad9b..3e0b9cab0 100644 --- a/libs/sdk/src/common/types/options/skills-http/index.ts +++ b/libs/sdk/src/common/types/options/skills-http/index.ts @@ -5,6 +5,7 @@ export type { SkillsConfigOptions, SkillsConfigEndpointConfig, SkillsConfigAuthMode, + SkillResourceAnnotations, SkillsConfigAuditOptions, SkillsConfigJwtOptions, SkillsConfigCacheOptions, diff --git a/libs/sdk/src/common/types/options/skills-http/interfaces.ts b/libs/sdk/src/common/types/options/skills-http/interfaces.ts index 520f8ebeb..d0550d70a 100644 --- a/libs/sdk/src/common/types/options/skills-http/interfaces.ts +++ b/libs/sdk/src/common/types/options/skills-http/interfaces.ts @@ -78,6 +78,9 @@ import type { ExternalSkillProviderBase } from '../../../../skill/providers/exte */ export type SkillsConfigAuthMode = 'inherit' | 'public' | 'api-key' | 'bearer'; +/** Which annotations each `SKILL.md` resource carries in `resources/list`; see `SkillsConfigOptions.resourceAnnotations`. */ +export type SkillResourceAnnotations = 'full' | 'no-priority' | 'off'; + /** * JWT validation configuration for bearer auth mode. */ @@ -287,6 +290,19 @@ export interface SkillsConfigOptions { */ mcpResources?: boolean; + /** + * Which annotations each `skill:///SKILL.md` resource carries in `resources/list`. + * + * - `'no-priority'` (default): `audience: ['assistant']`, plus `lastModified` for file-backed + * skills. No numeric annotation is sent: some clients fail the whole `resources/list` when an + * annotation is a number. + * - `'full'`: also `priority: 0.8`, the SEP-2640 recommendation (the default up to 1.10.15). + * - `'off'`: no annotations. + * + * @default 'no-priority' + */ + resourceAnnotations?: SkillResourceAnnotations; + /** * SEP-2640 §Discovery — opt-in inclusion of skill URIs in the server's * `instructions` field. When `true`, the transport adapter adds a @@ -363,16 +379,24 @@ export interface SkillsConfigOptions { * `instructions` (from `@FrontMcp({ instructions })`) and any framework * hints (e.g. channel reply-tool guidance). * - * - `'append'` (default): `instructions` is sent as-is, followed by a + * - `'compact'` (default): `instructions` and channel hints, then one line + * with the number of skills the caller may see and where to read them + * (`skill://index.json`, or the `skills/search` and `skills/load` + * methods when `mcpResources` is false). No line when the caller sees no + * skill; no count when a read-only `externalProvider` serves skills. + * Some clients copy the instructions into every tool description, + * so the full catalog can cost many tokens per tool. + * - `'append'`: `instructions` is sent as-is, followed by a * `\n\n---\n\n` separator and the per-skill summary (`**Name**: …` - * blocks separated by `---`). Channel hints sit between them. + * blocks separated by `---`). Channel hints sit between them. The + * default up to 1.10.15. * - `'prepend'`: the skill catalog summary is sent first, then channel * hints, then the server `instructions`. Useful when bundle guidance * must appear in the model's prefix. * - `'replace'`: surface ONLY the server `instructions` field — the * skill catalog summary AND channel hints are dropped. Use when the * server config is the canonical prompt and skills should be - * discovered via `searchSkills` instead. **When `instructions` is + * discovered via `skills/search` instead. **When `instructions` is * empty/undefined this falls back to `'append'` semantics** so a * misconfigured server doesn't silently drop everything. * - `'off'`: never inject the bundle skill catalog into `initialize`. @@ -380,9 +404,9 @@ export interface SkillsConfigOptions { * the per-skill summary is suppressed. Lowest-cost option for very * large catalogs (catalog summaries can otherwise blow context). * - * @default 'append' + * @default 'compact' */ - injectInstructions?: 'off' | 'append' | 'prepend' | 'replace'; + injectInstructions?: 'off' | 'compact' | 'append' | 'prepend' | 'replace'; /** * Ranking function for skill semantic search (the in-memory vector index). diff --git a/libs/sdk/src/common/types/options/skills-http/schema.ts b/libs/sdk/src/common/types/options/skills-http/schema.ts index 228c6fb44..6a649cb30 100644 --- a/libs/sdk/src/common/types/options/skills-http/schema.ts +++ b/libs/sdk/src/common/types/options/skills-http/schema.ts @@ -116,10 +116,11 @@ export const skillsConfigOptionsSchema = z.object({ llmFullTxt: skillsConfigEndpointInputSchema.optional().default(true), api: skillsConfigEndpointInputSchema.optional().default(true), mcpResources: z.boolean().optional().default(true), + resourceAnnotations: z.enum(['full', 'no-priority', 'off']).optional().default('no-priority'), sep2640InInstructions: z.boolean().optional().default(false), cache: skillsConfigCacheOptionsSchema.optional(), audit: skillsConfigAuditOptionsSchema.optional(), - injectInstructions: z.enum(['off', 'append', 'prepend', 'replace']).optional().default('append'), + injectInstructions: z.enum(['off', 'compact', 'append', 'prepend', 'replace']).optional().default('compact'), scoring: z.enum(['cosine', 'bm25']).optional(), failOnInvalidSkills: z.boolean().optional(), externalProvider: z diff --git a/libs/sdk/src/provider/__tests__/decorator-object-providers.spec.ts b/libs/sdk/src/provider/__tests__/decorator-object-providers.spec.ts index 0a17b2182..ab1427ca3 100644 --- a/libs/sdk/src/provider/__tests__/decorator-object-providers.spec.ts +++ b/libs/sdk/src/provider/__tests__/decorator-object-providers.spec.ts @@ -1,7 +1,7 @@ /** * `@FrontMcp`, `@App`, `@Plugin` and `@Agent` take the value and class providers `ProviderType` - * describes (`{ provide, name, useValue }`, `{ provide, name, useClass }`), as `dynamicProviders()` - * and the provider registry already do, and still refuse one without a `name`. + * describes (`{ provide, useValue }`, `{ provide, useClass }`), as `dynamicProviders()` and the + * provider registry already do. `name` is optional. */ import 'reflect-metadata'; @@ -87,24 +87,24 @@ describe('value and class providers in decorator metadata', () => { }).not.toThrow(); }); - it('still require a name', () => { + it('take value and class providers without a name', () => { expect(() => { - @App({ - name: 'unnamed-value', - // @ts-expect-error -- a value provider without `name` is refused at runtime - providers: [{ provide: API_URL, useValue: 'https://api.example.com' }], - }) + @App({ name: 'unnamed-value', providers: [{ provide: API_URL, useValue: 'https://api.example.com' }] }) class UnnamedValueApp {} return UnnamedValueApp; - }).toThrow(InvalidDecoratorMetadataError); + }).not.toThrow(); expect(() => { - @Plugin({ - name: 'unnamed-class', - // @ts-expect-error -- a class provider without `name` is refused at runtime - providers: [{ provide: GREETER, useClass: Greeter }], - }) + @Plugin({ name: 'unnamed-class', providers: [{ provide: GREETER, useClass: Greeter }] }) class UnnamedClassPlugin {} return UnnamedClassPlugin; + }).not.toThrow(); + }); + + it('refuse an empty name', () => { + expect(() => { + @App({ name: 'empty-name', providers: [{ name: '', provide: API_URL, useValue: 'https://api.example.com' }] }) + class EmptyNameApp {} + return EmptyNameApp; }).toThrow(InvalidDecoratorMetadataError); }); diff --git a/libs/sdk/src/provider/provider.utils.ts b/libs/sdk/src/provider/provider.utils.ts index 97ac4bb1c..dadb9e0a2 100644 --- a/libs/sdk/src/provider/provider.utils.ts +++ b/libs/sdk/src/provider/provider.utils.ts @@ -22,11 +22,14 @@ interface ProviderLike { } function collectProviderMetadata(cls: Type): ProviderMetadata { - return Object.entries(FrontMcpProviderTokens).reduce((metadata, [key, token]) => { - return Object.assign(metadata, { + const metadata = Object.entries(FrontMcpProviderTokens).reduce((collected, [key, token]) => { + return Object.assign(collected, { [key]: getMetadata(token, cls), }); }, {} as ProviderMetadata); + const ownName: string | undefined = Reflect.getOwnMetadata(FrontMcpProviderTokens.name, cls); + metadata.name = ownName ?? tokenName(cls); + return metadata; } /** diff --git a/libs/sdk/src/skill/__tests__/skill-context.spec.ts b/libs/sdk/src/skill/__tests__/skill-context.spec.ts index 8bc823df7..b80067053 100644 --- a/libs/sdk/src/skill/__tests__/skill-context.spec.ts +++ b/libs/sdk/src/skill/__tests__/skill-context.spec.ts @@ -101,7 +101,7 @@ describe('the description an overridden build() returns, wherever skills are lis info: { name: 'skill-context-listings', version: '1.0.0' }, apps: [DeskApp], logging: { level: LogLevel.Off }, - skillsConfig: { enabled: true, sep2640InInstructions: true }, + skillsConfig: { enabled: true, sep2640InInstructions: true, injectInstructions: 'append' as const }, }; client = await connect(config); server = await createTestFetchServer(config); diff --git a/libs/sdk/src/skill/__tests__/skill-external-provider.spec.ts b/libs/sdk/src/skill/__tests__/skill-external-provider.spec.ts index 87c2a15cc..69fddfe19 100644 --- a/libs/sdk/src/skill/__tests__/skill-external-provider.spec.ts +++ b/libs/sdk/src/skill/__tests__/skill-external-provider.spec.ts @@ -8,7 +8,9 @@ import 'reflect-metadata'; import { App, LogLevel, ScopeEntry, skill, Tool, ToolContext } from '../../common'; import type { SkillContent } from '../../common/interfaces'; import { connect, type DirectClient } from '../../direct'; +import { FrontMcpInstance } from '../../front-mcp/front-mcp'; import { ExternalSkillProviderBase } from '../providers/external-skill.provider'; +import { composeCallerInstructions } from '../skill-instructions.helper'; import type { SkillSearchResult } from '../skill-storage.interface'; const remoteSkill: SkillContent = { @@ -186,3 +188,27 @@ describe('skillsConfig.externalProvider', () => { expect((skillMd.contents[0] as { text?: string }).text).toContain('name: remote-skill'); }); }); + +describe('skillsConfig.externalProvider in the default initialize instructions', () => { + const indexLine = "This server has skills: the `skill://index.json` resource lists each skill's `SKILL.md` URI."; + + async function instructionsWith(apps: Array): Promise { + const instance = await FrontMcpInstance.createForGraph({ + info: { name: 'external-skills-instructions', version: '1.0.0' }, + apps, + skillsConfig: { externalProvider: new InMemoryExternalProvider({ mode: 'read-only' }) }, + logging: { level: LogLevel.Off }, + }); + const [scope] = instance.getScopes(); + if (!scope) throw new Error('the config produced no scope'); + return composeCallerInstructions(scope, { ctx: { authInfo: {} } }); + } + + it("counts no skill, as skill://index.json also lists the provider's", async () => { + await expect(instructionsWith([DeskApp])).resolves.toBe(indexLine); + }); + + it('points at skill://index.json on a server that declares no skill itself', async () => { + await expect(instructionsWith([BareApp])).resolves.toBe(indexLine); + }); +}); diff --git a/libs/sdk/src/skill/__tests__/skill-filter.flow.spec.ts b/libs/sdk/src/skill/__tests__/skill-filter.flow.spec.ts index fd773431f..19fc08518 100644 --- a/libs/sdk/src/skill/__tests__/skill-filter.flow.spec.ts +++ b/libs/sdk/src/skill/__tests__/skill-filter.flow.spec.ts @@ -63,7 +63,7 @@ const serverConfig = { info: { name: 'skills-filter-flow', version: '1.0.0' }, apps: [GuidesApp], logging: { level: LogLevel.Off }, - skillsConfig: { enabled: true }, + skillsConfig: { enabled: true, injectInstructions: 'append' as const }, }; function textOf(result: ReadResourceResult): string { @@ -326,6 +326,7 @@ describe('skills:filter flow', () => { apps: [RecordedGuidesApp], logging: { level: LogLevel.Off }, instructions: 'Guides server.', + skillsConfig: { injectInstructions: 'append' }, }); expect(await initializeInstructions(scope)).toBe( diff --git a/libs/sdk/src/skill/__tests__/skill-instructions.helper.spec.ts b/libs/sdk/src/skill/__tests__/skill-instructions.helper.spec.ts index 60ea45409..6d7c75dd3 100644 --- a/libs/sdk/src/skill/__tests__/skill-instructions.helper.spec.ts +++ b/libs/sdk/src/skill/__tests__/skill-instructions.helper.spec.ts @@ -1,8 +1,11 @@ +import type { SkillEntry } from '../../common'; import { buildChannelInstructions, buildSkillsCatalogSummary, + composeCallerInstructions, composeInitializeInstructions, type InjectInstructionsPolicy, + type InstructionsScope, } from '../skill-instructions.helper'; import type { SkillRegistryInterface } from '../skill.registry'; @@ -22,7 +25,7 @@ function asEntry(skill: FakeSkill) { function makeRegistry(fakeSkills: FakeSkill[]): SkillRegistryInterface { const skills = fakeSkills.map(asEntry); - // Only `getSkills` is exercised by the helper; everything else can throw. + // Only `getSkills` and `getExternalProvider` are exercised by the helper; everything else can throw. const registry: Partial = { getSkills: ((opts?: unknown) => { const o = (opts ?? {}) as { visibility?: string }; @@ -33,6 +36,7 @@ function makeRegistry(fakeSkills: FakeSkill[]): SkillRegistryInterface { return v === o.visibility; }) as never; }) as SkillRegistryInterface['getSkills'], + getExternalProvider: () => undefined, }; return registry as SkillRegistryInterface; } @@ -53,6 +57,7 @@ function makeMutableRegistry(initial: FakeSkill[]): { return v === o.visibility; }) as never; }) as SkillRegistryInterface['getSkills'], + getExternalProvider: () => undefined, }; return { registry: registry as SkillRegistryInterface, @@ -206,7 +211,7 @@ describe('skill-instructions.helper', () => { const registry = makeRegistry([{ metadata: { name: 'demo', description: 'A demo skill' } }]); it('forwards mcpResources so a server without skill:// resources never advertises them', () => { - const out = composeInitializeInstructions({ skillRegistry: registry, mcpResources: false }); + const out = composeInitializeInstructions({ skillRegistry: registry, mcpResources: false, policy: 'append' }); expect(out).toContain('- **demo**: A demo skill'); expect(out).toContain('skills/load'); expect(out).not.toContain('skill://'); @@ -268,11 +273,12 @@ describe('skill-instructions.helper', () => { expect(out).not.toContain('demo'); }); - it("'append' (default) joins user, channel, then skill catalog", () => { + it("'append' joins user, channel, then skill catalog", () => { const out = composeInitializeInstructions({ userInstructions: 'User.', channelInstructions: 'Channel.', skillRegistry: registry, + policy: 'append', }); const userIdx = out.indexOf('User.'); const channelIdx = out.indexOf('Channel.'); @@ -300,6 +306,7 @@ describe('skill-instructions.helper', () => { userInstructions: '', channelInstructions: '', skillRegistry: registry, + policy: 'append', }); expect(out).not.toMatch(/---\n\n---/); expect(out).toContain('Available skills'); @@ -314,18 +321,18 @@ describe('skill-instructions.helper', () => { const { registry: dyn, add } = makeMutableRegistry([ { metadata: { name: 'first_skill', description: 'present at boot' } }, ]); - const before = composeInitializeInstructions({ skillRegistry: dyn }); + const before = composeInitializeInstructions({ skillRegistry: dyn, policy: 'append' }); expect(before).toContain('first_skill'); expect(before).not.toContain('second_skill'); add({ metadata: { name: 'second_skill', description: 'registered later' } }); - const after = composeInitializeInstructions({ skillRegistry: dyn }); + const after = composeInitializeInstructions({ skillRegistry: dyn, policy: 'append' }); expect(after).toContain('first_skill'); expect(after).toContain('second_skill'); }); - it.each(['off', 'append', 'prepend', 'replace'] satisfies InjectInstructionsPolicy[])( + it.each(['off', 'compact', 'append', 'prepend', 'replace'] satisfies InjectInstructionsPolicy[])( "policy '%s' never inserts trailing separator", (policy) => { const out = composeInitializeInstructions({ @@ -339,6 +346,94 @@ describe('skill-instructions.helper', () => { ); }); + describe("'compact' policy", () => { + const resourceLine = (skills: string) => + `This server has ${skills}: the \`skill://index.json\` resource lists each skill's \`SKILL.md\` URI.`; + const twoSkills = makeRegistry([ + { metadata: { name: 'review_pr', description: 'Review a pull request' } }, + { metadata: { name: 'deploy', description: 'Deploy to production' } }, + ]); + + it('is the default: one line with the skill count, naming no skill', () => { + expect(composeInitializeInstructions({ skillRegistry: twoSkills })).toBe(resourceLine('2 skills')); + }); + + it('counts a single skill in the singular', () => { + const registry = makeRegistry([{ metadata: { name: 'demo', description: 'A demo skill' } }]); + expect(composeInitializeInstructions({ skillRegistry: registry, policy: 'compact' })).toBe( + resourceLine('1 skill'), + ); + }); + + it('points at the skills/* methods when mcpResources is disabled', () => { + const out = composeInitializeInstructions({ skillRegistry: twoSkills, mcpResources: false }); + expect(out).toBe( + 'This server has 2 skills: find one with the `skills/search` request and load it with `skills/load`.', + ); + }); + + it('follows the server instructions and channel hints', () => { + const out = composeInitializeInstructions({ + userInstructions: 'User.', + channelInstructions: 'Channel.', + skillRegistry: twoSkills, + }); + expect(out).toBe(`User.\n\n---\n\nChannel.\n\n---\n\n${resourceLine('2 skills')}`); + }); + + it('adds no line when no skill is MCP-visible', () => { + const httpOnly = makeRegistry([ + { metadata: { name: 'http_only', description: 'HTTP only', visibility: 'http' } }, + ]); + expect(composeInitializeInstructions({ userInstructions: 'User.', skillRegistry: httpOnly })).toBe('User.'); + }); + + it('adds no line without a skill registry', () => { + expect(composeInitializeInstructions({ userInstructions: 'User.' })).toBe('User.'); + }); + + it.each([ + ['beside registered skills', twoSkills], + ['on its own', makeRegistry([])], + ])('counts no skill when a read-only external provider serves skills %s', (_case, registry) => { + const withProvider = { ...registry, getExternalProvider: () => ({ isReadOnly: () => true }) }; + expect(composeInitializeInstructions({ skillRegistry: withProvider as SkillRegistryInterface })).toBe( + resourceLine('skills'), + ); + }); + + it('keeps the count beside a persistent external provider, which serves the local skills', () => { + const withProvider = { ...twoSkills, getExternalProvider: () => ({ isReadOnly: () => false }) }; + expect(composeInitializeInstructions({ skillRegistry: withProvider as SkillRegistryInterface })).toBe( + resourceLine('2 skills'), + ); + }); + + it('counts only the skills the skills:filter flow keeps for the caller', async () => { + const scope = { + metadata: {}, + skills: twoSkills, + logger: { warn: jest.fn() }, + runFlowForOutput: async (_flow: string, input: { skills: SkillEntry[] }) => ({ + skills: input.skills.filter((skill) => skill.metadata.name === 'deploy'), + }), + } as unknown as InstructionsScope; + + expect(await composeCallerInstructions(scope)).toBe(resourceLine('1 skill')); + }); + + it('adds no line when the caller may see no skill', async () => { + const scope = { + metadata: { instructions: 'Server prompt.' }, + skills: twoSkills, + logger: { warn: jest.fn() }, + runFlowForOutput: async () => ({ skills: [] }), + } as unknown as InstructionsScope; + + expect(await composeCallerInstructions(scope)).toBe('Server prompt.'); + }); + }); + describe('buildChannelInstructions', () => { it('returns empty string when channels are absent', () => { expect(buildChannelInstructions(undefined)).toBe(''); diff --git a/libs/sdk/src/skill/__tests__/skill-surface.spec.ts b/libs/sdk/src/skill/__tests__/skill-surface.spec.ts index a33e1cd7d..75b19752e 100644 --- a/libs/sdk/src/skill/__tests__/skill-surface.spec.ts +++ b/libs/sdk/src/skill/__tests__/skill-surface.spec.ts @@ -35,7 +35,7 @@ const serverConfig = { info: { name: 'skills-surface', version: '1.0.0' }, apps: [GuidesApp], logging: { level: LogLevel.Off }, - skillsConfig: { enabled: true }, + skillsConfig: { enabled: true, injectInstructions: 'append' as const }, }; function textOf(result: ReadResourceResult): string { diff --git a/libs/sdk/src/skill/sep-2640/__tests__/sep-2640.per-skill.spec.ts b/libs/sdk/src/skill/sep-2640/__tests__/sep-2640.per-skill.spec.ts index 4f6058274..d2b7ddc66 100644 --- a/libs/sdk/src/skill/sep-2640/__tests__/sep-2640.per-skill.spec.ts +++ b/libs/sdk/src/skill/sep-2640/__tests__/sep-2640.per-skill.spec.ts @@ -3,8 +3,8 @@ * * SEP-2640 §Resource Metadata says each `skill:///SKILL.md` * resource SHOULD carry frontmatter-derived `name`/`description` plus - * `audience`/`priority` annotations. The per-skill registration emits - * those — verified here. + * annotations. The per-skill registration emits those, per + * `skillsConfig.resourceAnnotations` — verified here. */ import 'reflect-metadata'; @@ -46,19 +46,34 @@ describe('buildPerSkillResourceRecord', () => { expect(rec.metadata.mimeType).toBe(SKILL_MD_MIME_TYPE); }); - it('attaches audience: ["assistant"] and priority 0.8 annotations', () => { + it('attaches audience: ["assistant"] and no numeric priority by default', () => { const skill = makeSkill({ name: 'a', description: 'A' }); const rec = buildPerSkillResourceRecord(fakeScope, skill); - expect(rec.metadata.annotations).toEqual({ - audience: ['assistant'], - priority: SKILL_MD_PRIORITY, + expect(rec.metadata.annotations).toEqual({ audience: ['assistant'] }); + }); + + it("attaches priority 0.8 under resourceAnnotations: 'full'", () => { + const skill = makeSkill({ name: 'a', description: 'A' }); + const rec = buildPerSkillResourceRecord(fakeScope, skill, { resourceAnnotations: 'full' }); + + expect(rec.metadata.annotations).toEqual({ audience: ['assistant'], priority: SKILL_MD_PRIORITY }); + }); + + it("attaches no annotations under resourceAnnotations: 'off'", () => { + const skill = makeSkill({ name: 'a', description: 'A' }); + const rec = buildPerSkillResourceRecord(fakeScope, skill, { + resourceAnnotations: 'off', + lastModified: '2026-05-05T12:00:00Z', }); + + expect('annotations' in rec.metadata).toBe(false); }); - it('includes lastModified when supplied', () => { + it.each(['no-priority', 'full'] as const)('includes lastModified when supplied under %s', (resourceAnnotations) => { const skill = makeSkill({ name: 'a', description: 'A' }); const rec = buildPerSkillResourceRecord(fakeScope, skill, { + resourceAnnotations, lastModified: '2026-05-05T12:00:00Z', }); expect(rec.metadata.annotations?.lastModified).toBe('2026-05-05T12:00:00Z'); @@ -130,43 +145,81 @@ describe('buildPerSkillResourceRecord', () => { }); }); -describe('registerPerSkillResources when a skill is replaced at the same path (#606)', () => { - function setup(initial: SkillEntry) { - let served: SkillEntry[] = [initial]; - const listeners: Array<() => void> = []; - const records: ResourceFunctionRecord[] = []; - const disposers: Array<() => void> = []; - const scope = { - onDispose: (callback: () => void) => disposers.push(callback), - skills: { - hasAny: () => true, - getSkills: () => served, - subscribe: (_options: unknown, listener: () => void) => { - listeners.push(listener); - return () => listeners.splice(listeners.indexOf(listener), 1); - }, +type RegisterOptions = Pick< + Parameters[0], + 'resourceAnnotations' | 'resolveLastModified' +>; + +function setup(initial: SkillEntry, options: RegisterOptions = {}) { + let served: SkillEntry[] = [initial]; + const listeners: Array<() => void> = []; + const records: ResourceFunctionRecord[] = []; + const disposers: Array<() => void> = []; + const scope = { + onDispose: (callback: () => void) => disposers.push(callback), + skills: { + hasAny: () => true, + getSkills: () => served, + subscribe: (_options: unknown, listener: () => void) => { + listeners.push(listener); + return () => listeners.splice(listeners.indexOf(listener), 1); }, - } as unknown as ScopeEntry; - const unregistered: unknown[] = []; - const resourceRegistry = { - registerDynamicResource: (record: ResourceFunctionRecord) => records.push(record), - unregisterResourceInstance: (token: unknown) => unregistered.push(token), - } as unknown as ResourceRegistry; - const logger = { verbose: jest.fn(), warn: jest.fn() } as unknown as FrontMcpLogger; - - return { - register: () => registerPerSkillResources({ scope, skillRegistry: scope.skills, resourceRegistry, logger }), - metadata: () => (records[0]?.metadata ?? {}) as unknown as Record, - records, - unregistered, - dispose: () => disposers.forEach((callback) => callback()), - replaceWith: (skill?: SkillEntry) => { - served = skill ? [skill] : []; - for (const listener of [...listeners]) listener(); - }, - }; - } + }, + } as unknown as ScopeEntry; + const unregistered: unknown[] = []; + const resourceRegistry = { + registerDynamicResource: (record: ResourceFunctionRecord) => records.push(record), + unregisterResourceInstance: (token: unknown) => unregistered.push(token), + } as unknown as ResourceRegistry; + const logger = { verbose: jest.fn(), warn: jest.fn() } as unknown as FrontMcpLogger; + + return { + register: () => + registerPerSkillResources({ scope, skillRegistry: scope.skills, resourceRegistry, logger, ...options }), + metadata: () => (records[0]?.metadata ?? {}) as unknown as Record, + records, + unregistered, + dispose: () => disposers.forEach((callback) => callback()), + replaceWith: (skill?: SkillEntry) => { + served = skill ? [skill] : []; + for (const listener of [...listeners]) listener(); + }, + }; +} +describe('registerPerSkillResources annotations', () => { + const skill = makeSkill({ name: 'report', description: 'Open report' }); + const lastModified = '2026-05-05T12:00:00Z'; + + it('adds lastModified next to the audience by default', async () => { + const harness = setup(skill, { resolveLastModified: async () => lastModified }); + await harness.register(); + + expect(harness.metadata()['annotations']).toEqual({ audience: ['assistant'], lastModified }); + }); + + it("brings back priority 0.8 under resourceAnnotations: 'full'", async () => { + const harness = setup(skill, { resourceAnnotations: 'full', resolveLastModified: async () => lastModified }); + await harness.register(); + + expect(harness.metadata()['annotations']).toEqual({ + audience: ['assistant'], + priority: SKILL_MD_PRIORITY, + lastModified, + }); + }); + + it("neither annotates nor resolves lastModified under resourceAnnotations: 'off'", async () => { + const resolveLastModified = jest.fn(async () => lastModified); + const harness = setup(skill, { resourceAnnotations: 'off', resolveLastModified }); + await harness.register(); + + expect('annotations' in harness.metadata()).toBe(false); + expect(resolveLastModified).not.toHaveBeenCalled(); + }); +}); + +describe('registerPerSkillResources when a skill is replaced at the same path (#606)', () => { const plain = makeSkill({ name: 'report', description: 'Open report' }); const gated = makeSkill({ name: 'report', diff --git a/libs/sdk/src/skill/sep-2640/sep-2640.per-skill.ts b/libs/sdk/src/skill/sep-2640/sep-2640.per-skill.ts index 09041f3f7..43320e35f 100644 --- a/libs/sdk/src/skill/sep-2640/sep-2640.per-skill.ts +++ b/libs/sdk/src/skill/sep-2640/sep-2640.per-skill.ts @@ -13,7 +13,7 @@ * To satisfy that, we additionally register one **concrete** `Resource` per * MCP-visible skill, at scope startup and whenever skills are added or removed. * These show up in `resources/list` with frontmatter-derived metadata plus - * `audience`/`priority` annotations. + * the annotations `skillsConfig.resourceAnnotations` selects. * * When the skill served at a path is replaced, the resource takes the new * skill's description and policy metadata, so `resources/list` gates it by @@ -30,6 +30,7 @@ import { type ResourceMetadata, type ScopeEntry, type SkillEntry, + type SkillResourceAnnotations, } from '../../common'; import { ResourceKind } from '../../common/records/resource.record'; import type ResourceRegistry from '../../resource/resource.registry'; @@ -63,9 +64,9 @@ function skillExtensionMetadata(skill: SkillEntry): Record { * - `name` = skill's frontmatter name * - `description` = skill's frontmatter description (what a `build()` override returns, once loaded) * - `mimeType` = `text/markdown` - * - `annotations.audience` = `["assistant"]` - * - `annotations.priority` = 0.8 (the SEP-recommended SKILL.md priority) - * - `annotations.lastModified` (when supplied — populated for file-backed skills) + * - `annotations.audience` = `["assistant"]`, unless `resourceAnnotations` is `'off'` + * - `annotations.priority` = 0.8 (the SEP-recommended SKILL.md priority), only when `resourceAnnotations` is `'full'` + * - `annotations.lastModified` (when supplied — populated for file-backed skills), unless `resourceAnnotations` is `'off'` * - `_meta.io.modelcontextprotocol.skills/path` = the skill's `` * * The function is bound to the scope so `resources/read` invocation @@ -74,19 +75,12 @@ function skillExtensionMetadata(skill: SkillEntry): Record { export function buildPerSkillResourceRecord( scope: ScopeEntry, skill: SkillEntry, - opts: { lastModified?: string } = {}, + opts: { lastModified?: string; resourceAnnotations?: SkillResourceAnnotations } = {}, ): ResourceFunctionRecord { const skillPathSegments = skill.getSkillPathSegments(); const skillPath = skillPathSegments.join('/'); const uri = buildSkillUri(skillPathSegments, 'SKILL.md'); - - const annotations: ResourceMetadata['annotations'] = { - audience: ['assistant'], - priority: SKILL_MD_PRIORITY, - }; - if (opts.lastModified) { - annotations.lastModified = opts.lastModified; - } + const annotations = skillMdAnnotations(opts.resourceAnnotations ?? 'no-priority', opts.lastModified); const meta: Record = { [`${SEP_2640_META_NAMESPACE}path`]: skillPath, @@ -98,7 +92,7 @@ export function buildPerSkillResourceRecord( name: skill.metadata.name, description: skill.getDescription(), mimeType: SKILL_MD_MIME_TYPE, - annotations, + ...(annotations && { annotations }), _meta: meta, // The skill's own `availableWhen`, so `resources/list` and `resources/read` offer its SKILL.md // on the surfaces that offer the skill, and answer others as for an unknown resource. @@ -129,10 +123,22 @@ export function buildPerSkillResourceRecord( }; } +function skillMdAnnotations( + policy: SkillResourceAnnotations, + lastModified: string | undefined, +): ResourceMetadata['annotations'] { + if (policy === 'off') return undefined; + return { + audience: ['assistant'], + ...(policy === 'full' && { priority: SKILL_MD_PRIORITY }), + ...(lastModified && { lastModified }), + }; +} + /** * Register one concrete `Resource` per MCP-visible skill so * `resources/list` returns SEP-conformant entries (frontmatter-derived - * `name`/`description`, audience/priority annotations). + * `name`/`description`, and the annotations `resourceAnnotations` selects). * * This is in addition to — not a replacement for — the SKILL.md template * (`Sep2640SkillMdResource`). Hosts may discover skills either via @@ -147,13 +153,14 @@ export async function registerPerSkillResources(options: { resourceRegistry: ResourceRegistry; logger: FrontMcpLogger; resolveLastModified?: (skill: SkillEntry) => Promise; + resourceAnnotations?: SkillResourceAnnotations; }): Promise { - const { scope, skillRegistry, resourceRegistry, logger, resolveLastModified } = options; + const { scope, skillRegistry, resourceRegistry, logger, resolveLastModified, resourceAnnotations } = options; const registered = new Map(); const register = (skill: SkillEntry): Promise => { try { - const record = buildPerSkillResourceRecord(scope, skill); + const record = buildPerSkillResourceRecord(scope, skill, { resourceAnnotations }); // The resource registry's dynamic-register path accepts a normalised // record directly. resourceRegistry.registerDynamicResource(record); @@ -163,7 +170,7 @@ export async function registerPerSkillResources(options: { extensionKeys: Object.keys(skillExtensionMetadata(skill)), }); logger.verbose(`Registered SEP-2640 per-skill resource: ${record.metadata.uri}`); - return resolveLastModified + return resolveLastModified && record.metadata.annotations ? addLastModified(record.metadata, skill, resolveLastModified, logger) : Promise.resolve(); } catch (err) { diff --git a/libs/sdk/src/skill/skill-instructions.helper.ts b/libs/sdk/src/skill/skill-instructions.helper.ts index 5af567f75..5a60e9b31 100644 --- a/libs/sdk/src/skill/skill-instructions.helper.ts +++ b/libs/sdk/src/skill/skill-instructions.helper.ts @@ -6,11 +6,13 @@ * - The `initialize` response's `instructions` field is server-side prompt * text that clients usually inject verbatim into the model context. Pushing * every skill's full SKILL.md body would balloon every initialize response - * and burn token budget on tool calls. Instead, we ship a bounded catalog - * summary (`**name**: description`) plus a pointer to where the full content - * is served: the SEP-2640 `skill://index.json` resource, or the - * `skills/search` and `skills/load` MCP extension methods when - * `skillsConfig.mcpResources` is false and no `skill://` resource exists. + * and burn token budget on tool calls. Instead, the default `'compact'` + * policy ships one line with the skill count, and `'append'`, `'prepend'` + * and `'replace'` a bounded catalog summary (`**name**: description`). Both + * point to where the full content is served: the SEP-2640 + * `skill://index.json` resource, or the `skills/search` and `skills/load` + * MCP extension methods when `skillsConfig.mcpResources` is false and no + * `skill://` resource exists. * - Resolution is sync — the registry already holds metadata at boot. The * per-skill `instructions` markdown body stays lazy. * - Off / replace policies short-circuit before we touch the registry, so a @@ -21,8 +23,9 @@ * after the server boots are reflected in subsequent reconnects without a * restart. * - Transports compose it per caller with {@link composeCallerInstructions}: - * the catalog and the SEP-2640 hints only name skills the skill authorities - * and the `skills:filter` flow let that caller see, like `skills/list`. + * the compact line, the catalog and the SEP-2640 hints only count or name + * skills the skill authorities and the `skills:filter` flow let that caller + * see, like `skills/list`. */ import type { ScopeEntry, SkillEntry } from '../common'; @@ -33,9 +36,9 @@ import { filterSkillsByAuthorities } from './skill-authorities.helper'; import { filterServableSkills } from './skill-filter.helper'; import type { SkillRegistryInterface } from './skill.registry'; -export type InjectInstructionsPolicy = 'off' | 'append' | 'prepend' | 'replace'; +export type InjectInstructionsPolicy = 'off' | 'compact' | 'append' | 'prepend' | 'replace'; -const DEFAULT_POLICY: InjectInstructionsPolicy = 'append'; +const DEFAULT_POLICY: InjectInstructionsPolicy = 'compact'; /** * Hard ceiling on the catalog block. Keeps the initialize payload bounded * regardless of how many skills are registered. ~16 KB fits comfortably in a @@ -77,18 +80,29 @@ export interface SkillsCatalogSummaryOptions { interface CatalogPointers { header: string; truncationHint: string; + compact: string; } const RESOURCE_POINTERS: CatalogPointers = { header: `Available skills (read the \`${SKILL_INDEX_URI}\` resource for each skill's \`SKILL.md\` URI):`, truncationHint: `read \`${SKILL_INDEX_URI}\` to browse`, + compact: `the \`${SKILL_INDEX_URI}\` resource lists each skill's \`SKILL.md\` URI.`, }; const METHOD_POINTERS: CatalogPointers = { header: 'Available skills (load full content with the `skills/load` request, or find others with `skills/search`):', truncationHint: 'use `skills/search` to find the rest', + compact: 'find one with the `skills/search` request and load it with `skills/load`.', }; +/** The skills the instructions count or list; `visibility: 'mcp'` matches both `'mcp'` and `'both'` (see `getSkills`). */ +function listedSkills( + skillRegistry: SkillRegistryInterface | undefined, + options: SkillsCatalogSummaryOptions, +): readonly SkillEntry[] { + return options.skills ?? skillRegistry?.getSkills({ visibility: 'mcp' }) ?? []; +} + /** * Sanitize a skill description so it can be embedded as a single bullet in a * markdown catalog without breaking out of its line context. @@ -169,8 +183,7 @@ export function buildSkillsCatalogSummary( skillRegistry: SkillRegistryInterface | undefined, options: SkillsCatalogSummaryOptions = {}, ): string { - // `visibility: 'mcp'` matches both `'mcp'` and `'both'` (see `getSkills`). - const skills = options.skills ?? skillRegistry?.getSkills({ visibility: 'mcp' }) ?? []; + const skills = listedSkills(skillRegistry, options); if (skills.length === 0) return ''; const pointers = options.mcpResources === false ? METHOD_POINTERS : RESOURCE_POINTERS; @@ -205,6 +218,21 @@ export function buildSkillsCatalogSummary( return lines.join('\n'); } +/** The `'compact'` line: how many skills there are and where to read them. Empty when there are none. */ +function buildCompactSkillsLine( + skillRegistry: SkillRegistryInterface | undefined, + options: SkillsCatalogSummaryOptions, +): string { + const count = listedSkills(skillRegistry, options).length; + // A read-only external provider serves skills the registry does not hold, so none can be counted. + const servesExternalSkills = skillRegistry?.getExternalProvider()?.isReadOnly() === true; + if (count === 0 && !servesExternalSkills) return ''; + let skills = count === 1 ? '1 skill' : `${count} skills`; + if (servesExternalSkills) skills = 'skills'; + const pointers = options.mcpResources === false ? METHOD_POINTERS : RESOURCE_POINTERS; + return `This server has ${skills}: ${pointers.compact}`; +} + /** * Compose the final `instructions` string for the MCP `initialize` response * by merging server instructions, channel hints, and the skill catalog @@ -215,7 +243,8 @@ export function buildSkillsCatalogSummary( * * Policy semantics: * - `'off'`: skips the skill catalog; user + channel hints are still emitted. - * - `'append'` (default): user + channel + catalog, in that order. + * - `'compact'` (default): user + channel + one line counting the skills, no line without skills. + * - `'append'`: user + channel + catalog, in that order. * - `'prepend'`: catalog + channel + user. * - `'replace'`: only the user-supplied instructions are emitted. **Falls * back to `'append'` semantics when `userInstructions` is empty/undefined** @@ -242,14 +271,16 @@ export function composeInitializeInstructions(options: ComposeOptions): string { return joinSections([user, channel]); } - const catalog = buildSkillsCatalogSummary(options.skillRegistry, { - mcpResources: options.mcpResources, - skills: options.skills, - }); + const summaryOptions: SkillsCatalogSummaryOptions = { mcpResources: options.mcpResources, skills: options.skills }; + if (policy === 'compact') { + return joinSections([user, channel, buildCompactSkillsLine(options.skillRegistry, summaryOptions)]); + } + + const catalog = buildSkillsCatalogSummary(options.skillRegistry, summaryOptions); if (policy === 'prepend') { return joinSections([catalog, channel, user]); } - // 'append' (default and 'replace'-with-empty-user fallback) + // 'append' (and the 'replace'-with-empty-user fallback) return joinSections([user, channel, catalog]); } @@ -299,9 +330,9 @@ export interface CallerInstructionsOptions { /** * Compose the `initialize` instructions for one caller: server instructions, channel hints and the - * skill catalog (plus the SEP-2640 hints when asked), per `skillsConfig.injectInstructions`. + * compact skills line or skill catalog (plus the SEP-2640 hints when asked), per `skillsConfig.injectInstructions`. * - * The catalog and hints only name the MCP-visible skills the skill authorities and the hookable + * The line, catalog and hints only count or name the MCP-visible skills the skill authorities and the hookable * `skills:filter` flow let this caller see, the same skills `skills/list` returns. The filter only * runs when the instructions would list a skill. If it fails, the skills are left out rather than * failing the handshake. With nothing filtering, the result equals {@link composeInitializeInstructions}. @@ -366,7 +397,7 @@ function skillUriHintsEnabled(scope: InstructionsScope): boolean { return !(skillsConfig.injectInstructions === 'replace' && serverInstructions.length > 0); } -/** Whether {@link composeInitializeInstructions} emits the skill catalog under these options. */ +/** Whether {@link composeInitializeInstructions} counts or lists skills under these options. */ function includesSkillCatalog(options: ComposeOptions): boolean { const policy = options.policy ?? DEFAULT_POLICY; if (policy === 'off') return false; diff --git a/libs/sdk/src/skill/skill-scope.helper.ts b/libs/sdk/src/skill/skill-scope.helper.ts index e1c16ef06..83fe5c5e2 100644 --- a/libs/sdk/src/skill/skill-scope.helper.ts +++ b/libs/sdk/src/skill/skill-scope.helper.ts @@ -146,6 +146,7 @@ export async function registerSkillCapabilities(options: SkillScopeRegistrationO resourceRegistry, logger, resolveLastModified: resolveLastModifiedForSkill, + resourceAnnotations: skillsConfig?.resourceAnnotations, }); } else { logger.verbose('SEP-2640 skill:// resources disabled via skillsConfig.mcpResources=false'); diff --git a/libs/sdk/src/transport/adapters/__tests__/transport.local.skill-hints.spec.ts b/libs/sdk/src/transport/adapters/__tests__/transport.local.skill-hints.spec.ts index 3a2f9d72a..24591a0f2 100644 --- a/libs/sdk/src/transport/adapters/__tests__/transport.local.skill-hints.spec.ts +++ b/libs/sdk/src/transport/adapters/__tests__/transport.local.skill-hints.spec.ts @@ -14,7 +14,7 @@ import { composeCallerInstructions, type InstructionsScope } from '../../../skil type SkillsConfig = { sep2640InInstructions?: boolean; mcpResources?: boolean; - injectInstructions?: 'off' | 'append' | 'prepend' | 'replace'; + injectInstructions?: 'off' | 'compact' | 'append' | 'prepend' | 'replace'; }; const DEPLOY_HINT = 'skill://ops/deploy/SKILL.md — Deploy to production'; @@ -31,7 +31,12 @@ function makeScope( }; return { metadata: { skillsConfig, instructions }, - skills: { hasAny: () => true, getSkills: () => [skill], getSep2640InstructionUris: () => [] }, + skills: { + hasAny: () => true, + getSkills: () => [skill], + getSep2640InstructionUris: () => [], + getExternalProvider: () => undefined, + }, logger: { warn: jest.fn() }, runFlowForOutput: async (_flow: string, input: { skills: SkillEntry[] }) => ({ skills: servable(input.skills) }), } as unknown as InstructionsScope; @@ -57,6 +62,13 @@ describe('initialize instructions: SEP-2640 skill URI hints', () => { expect(await hintsFor(scope)).toBe('Server prompt.'); }); + it("keeps the hints under 'off', which drops only the skills line and catalog", async () => { + const scope = makeScope({ sep2640InInstructions: true, injectInstructions: 'off' }, 'Server prompt.'); + expect(await hintsFor(scope)).toBe( + `Server prompt.\n\n---\n\nAvailable skills (load via resources/read):\n- ${DEPLOY_HINT}`, + ); + }); + it("keeps the hints when 'replace' falls back to 'append' for empty instructions", async () => { const scope = makeScope({ sep2640InInstructions: true, injectInstructions: 'replace' }, ' '); expect(await hintsFor(scope)).toContain('skill://ops/deploy/SKILL.md'); @@ -80,7 +92,9 @@ describe('initialize instructions: SEP-2640 skill URI hints', () => { }); it('sends no hints unless the transport asks for them', async () => { - const instructions = await composeCallerInstructions(makeScope({ sep2640InInstructions: true })); + const instructions = await composeCallerInstructions( + makeScope({ sep2640InInstructions: true, injectInstructions: 'append' }), + ); expect(instructions).toContain('**deploy**: Deploy to production'); expect(instructions).not.toContain(DEPLOY_HINT); diff --git a/libs/skills/catalog/frontmcp-config/examples/configure-skills-http/inject-instructions.md b/libs/skills/catalog/frontmcp-config/examples/configure-skills-http/inject-instructions.md index 4a9ec1e80..34dc861c8 100644 --- a/libs/skills/catalog/frontmcp-config/examples/configure-skills-http/inject-instructions.md +++ b/libs/skills/catalog/frontmcp-config/examples/configure-skills-http/inject-instructions.md @@ -2,11 +2,12 @@ name: inject-instructions reference: configure-skills-http level: basic -description: Set a server-level instructions string and append the skill catalog summary on every initialize response. +description: Set a server-level instructions string and append the full skill catalog summary on every initialize response. tags: [config, skills, instructions, injection, initialize] features: - 'Top-level `instructions` on `@FrontMcp` exposes a global system prompt to MCP clients' - "`skillsConfig.injectInstructions: 'append'` adds the skill catalog summary after the user prompt" + - "The default, `'compact'`, adds one line with the skill count and the `skill://index.json` pointer instead" - 'Dynamic skills are picked up because the composer runs on every initialize request' - 'The catalog only names skills the initializing caller may see (skill authorities and the `skills:filter` flow, e.g. feature flags)' - 'Catalog summary is bounded at 16 KB with a truncation footer pointing at skill://index.json' @@ -14,7 +15,7 @@ features: # Inject Instructions on Initialize -Set a server-level instructions string and append the skill catalog summary on every initialize response. +Set a server-level instructions string and append the full skill catalog summary on every initialize response. ## Code @@ -37,7 +38,8 @@ import { MainApp } from './main.app'; skillsConfig: { enabled: true, mcpResources: true, - // 'append' (default) — the skill catalog summary is appended after instructions + // 'compact' (default) — one line: the skill count and where to read the skills + // 'append' — the skill catalog summary is appended after instructions // 'prepend' — summary first, then instructions // 'replace' — instructions only; summary and channel hints dropped (falls back to 'append' if instructions is empty) // 'off' — no summary; instructions and channel hints still sent @@ -51,6 +53,7 @@ export default class FlightBotServer {} - Top-level `instructions` on `@FrontMcp` exposes a global system prompt to MCP clients - `skillsConfig.injectInstructions: 'append'` adds the skill catalog summary after the user prompt +- The default, `'compact'`, adds one line with the skill count and the `skill://index.json` pointer instead - Dynamic skills are picked up because the composer runs on every initialize request - The catalog only names skills the initializing caller may see (skill authorities and the `skills:filter` flow, e.g. feature flags) - Catalog summary is bounded at 16 KB with a truncation footer pointing at skill://index.json diff --git a/libs/skills/catalog/frontmcp-config/references/configure-skills-http.md b/libs/skills/catalog/frontmcp-config/references/configure-skills-http.md index d35bd4766..b433a250c 100644 --- a/libs/skills/catalog/frontmcp-config/references/configure-skills-http.md +++ b/libs/skills/catalog/frontmcp-config/references/configure-skills-http.md @@ -1,7 +1,7 @@ --- name: configure-skills-http -description: Full reference for skillsConfig — HTTP catalog endpoints, auth, caching, instructions injection, and tamper-evident audit log. -tags: [config, skills, skills-http, llm-txt, instructions, audit, injection] +description: Full reference for skillsConfig — HTTP catalog endpoints, auth, caching, instructions injection, SKILL.md resource annotations, and tamper-evident audit log. +tags: [config, skills, skills-http, llm-txt, instructions, audit, injection, annotations] --- # Configure `skillsConfig` @@ -16,12 +16,13 @@ tags: [config, skills, skills-http, llm-txt, instructions, audit, injection] apps: [MainApp], // Server-level instructions, exposed as `instructions` on initialize. - // Combined with the skill catalog summary per skillsConfig.injectInstructions. + // Combined with the skills line or catalog summary per skillsConfig.injectInstructions. instructions: 'You are a helpful assistant for booking flights.', skillsConfig: { enabled: true, // turn on /skills, /llm.txt, skill:// resources mcpResources: true, // expose skills as MCP resources (skill://index.json, skill:///SKILL.md) + resourceAnnotations: 'no-priority', // 'no-priority' (default) | 'full' | 'off' llmTxt: true, // serve /llm.txt llmFullTxt: false, // serve /llm_full.txt (full SKILL.md bodies) auth: 'api-key', // 'inherit' (default) | 'public' | 'api-key' | 'bearer' @@ -32,7 +33,7 @@ tags: [config, skills, skills-http, llm-txt, instructions, audit, injection] redis: { provider: 'redis', host: 'localhost', port: 6379 }, ttlMs: 60_000, }, - injectInstructions: 'append', // 'off' | 'append' | 'prepend' | 'replace' + injectInstructions: 'compact', // 'compact' (default) | 'off' | 'append' | 'prepend' | 'replace' failOnInvalidSkills: true, // false: a 'strict' skill with a missing tool is logged, not fatal externalProvider: new RestSkillProvider({ mode: 'read-only' }), // external skill storage, see below audit: { @@ -57,22 +58,53 @@ The new top-level `instructions?: string` field on `@FrontMcp` is forwarded verb | -------------- | -------- | ------- | ----------------------------------------------------------------------- | | `instructions` | `string` | `''` | High-level prompt the server gives to the LLM client at connection time | -`skillsConfig.injectInstructions` controls whether (and how) FrontMCP appends a generated **skill catalog summary** to those instructions on every `initialize` request. +`skillsConfig.injectInstructions` controls what FrontMCP adds about the server's skills to those instructions on every `initialize` request: one short line by default, or a generated **skill catalog summary**. ## Skill Catalog Injection Policy | Mode | Behavior | | --------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `off` | The catalog summary is suppressed. Server `instructions` and channel hints are still surfaced. | -| `append` | Server `instructions`, then channel hints, then the catalog summary, joined by `\n\n---\n\n`. **(Default.)** | +| `compact` | Server `instructions`, then channel hints, then one line with the number of skills and where to read them, joined by `\n\n---\n\n`. No line when the caller sees no skill. **(Default.)** | +| `off` | No skills line or catalog. Server `instructions`, channel hints and the `sep2640InInstructions` hints are still surfaced. | +| `append` | Server `instructions`, then channel hints, then the catalog summary (every skill's name and description). The default up to 1.10.15. | | `prepend` | Catalog summary first, then channel hints, then server `instructions`. | | `replace` | Surface ONLY the server `instructions`; the catalog AND channel hints are dropped. When `instructions` is empty/undefined this falls back to `'append'` so a misconfig doesn't drop everything. | +The `compact` line reads: + +```text +This server has 3 skills: the `skill://index.json` resource lists each skill's `SKILL.md` URI. +``` + +With `mcpResources: false` it points at the `skills/*` methods instead: + +```text +This server has 3 skills: find one with the `skills/search` request and load it with `skills/load`. +``` + +With a read-only `externalProvider` the line names no number (`This server has skills: …`): the provider's skills are listed by `skill://index.json` and found by `skills/search`, not held by the registry, so they can't be counted. The line is sent even when the server declares no skill itself. + +**Default change (1.10.16):** up to 1.10.15 the default was `'append'`, which lists every skill's name and description (up to 16 KB). Some clients copy the server's instructions into every tool's description, so a large catalog was repeated once per tool. Set `injectInstructions: 'append'` to get the full catalog back. + The catalog summary is built by `composeInitializeInstructions(...)` and `buildSkillsCatalogSummary(...)` (exported from `@frontmcp/sdk`). It is bounded at **16 KB** with a truncation footer. Its header and footer point clients at `skill://index.json` (SEP-2640 — singular scheme), which lists each skill's `skill:///SKILL.md` URI. With `mcpResources: false` no `skill://` resource is served, so they point at the `skills/load` and `skills/search` methods instead, and `sep2640InInstructions` is ignored. > **Dynamic skills:** because the composer recomputes the summary on every `initialize` request, skills registered after server boot **are** picked up automatically. Their `skill:///SKILL.md` resources follow too: a skill registered or removed after boot (`this.scope.skills.registerSkillContent()` / `unregisterSkill()`, a skill-bundle plugin) is added to or dropped from `resources/list`, and clients get `notifications/resources/list_changed`. Up to 1.9.4 `resources/list` named only the skills present at startup. -> **Per caller:** the summary (and the SEP-2640 `skill://` hints under `sep2640InInstructions`) is composed for the client that initializes, like `skills/list`: it only names skills whose `authorities` that caller satisfies and that the hookable `skills:filter` flow keeps, so a flag-disabled skill's name and description are left out. With nothing gating a skill the instructions are unchanged. Transports use `composeCallerInstructions(scope, { ctx })`, exported from `@frontmcp/sdk` for custom transports. +> **Per caller:** the compact line or the summary (and the SEP-2640 `skill://` hints under `sep2640InInstructions`) is composed for the client that initializes, like `skills/list`: it only counts or names skills whose `authorities` that caller satisfies and that the hookable `skills:filter` flow keeps, so a flag-disabled skill's name and description are left out. With nothing gating a skill the instructions are unchanged. Transports use `composeCallerInstructions(scope, { ctx })`, exported from `@frontmcp/sdk` for custom transports. + +## SKILL.md Resource Annotations + +Each `skill:///SKILL.md` resource in `resources/list` carries the annotations `skillsConfig.resourceAnnotations` selects: + +| Mode | Annotations | +| ------------- | -------------------------------------------------------------------------------------------- | +| `no-priority` | `audience: ['assistant']`, plus `lastModified` for a file-backed skill. **(Default.)** | +| `full` | `audience: ['assistant']`, `priority: 0.8` (the SEP-2640 recommendation) and `lastModified`. | +| `off` | No `annotations` field. | + +**Default change (1.10.16):** up to 1.10.15 every `SKILL.md` resource carried `priority: 0.8`, and nothing turned it off. Some clients (for example a Rust MCP client whose JSON parser keeps arbitrary-precision numbers) fail the whole `resources/list` when an annotation is a number, so the default sends no numeric annotation. Set `resourceAnnotations: 'full'` to send `priority` again; a hook that deleted `priority` from the list is no longer needed. + +An unknown `injectInstructions` or `resourceAnnotations` value stops the server at boot: `@FrontMcp` throws `InvalidDecoratorMetadataError` for `skillsConfig`, naming the field and the accepted values. ## Skills-Only Connections (`?mode=skills_only`) @@ -185,17 +217,19 @@ See [`skill-audit-log`](../../frontmcp-extensibility/references/skill-audit-log. | ----------------------------------------- | ----------------------------------------------------------------------------------------------------- | | Local dev, no skills | `skillsConfig` unset | | Public server, hand-curated server prompt | `instructions: '...'`, `injectInstructions: 'off'` | -| Server with many dynamic skills | `injectInstructions: 'append'` (default) or `'prepend'` if skill guidance must lead the prompt | +| Server with many dynamic skills | `injectInstructions: 'compact'` (default): clients read `skill://index.json` when they need a skill | +| Clients must see every skill up front | `injectInstructions: 'append'`, or `'prepend'` if skill guidance must lead the prompt | +| Client that relies on resource `priority` | `resourceAnnotations: 'full'` | | Multi-pod production | `cache: { enabled: true, redis: {...} }`, `audit: { signer: Rs256, store: StorageAdapterAuditStore }` | | Compliance / forensic requirements | RS256 signer + persistent store + scheduled `verifyChain(...)` in CI | ## Examples -| Example | Level | Description | -| --------------------------------------------------------------------------------- | -------- | ----------------------------------------------------------------------------------------------------------- | -| [`inject-instructions`](../examples/configure-skills-http/inject-instructions.md) | Basic | Set a server-level instructions string and append the skill catalog summary on every initialize response. | -| [`audit-log-basic`](../examples/configure-skills-http/audit-log-basic.md) | Basic | Enable the skill audit log with the in-memory store and HS256 signer for development and tests. | -| [`audit-log-redis`](../examples/configure-skills-http/audit-log-redis.md) | Advanced | Production-grade audit log with the Redis-backed StorageAdapterAuditStore and the RS256 bundle-signing key. | +| Example | Level | Description | +| --------------------------------------------------------------------------------- | -------- | -------------------------------------------------------------------------------------------------------------- | +| [`inject-instructions`](../examples/configure-skills-http/inject-instructions.md) | Basic | Set a server-level instructions string and append the full skill catalog summary on every initialize response. | +| [`audit-log-basic`](../examples/configure-skills-http/audit-log-basic.md) | Basic | Enable the skill audit log with the in-memory store and HS256 signer for development and tests. | +| [`audit-log-redis`](../examples/configure-skills-http/audit-log-redis.md) | Advanced | Production-grade audit log with the Redis-backed StorageAdapterAuditStore and the RS256 bundle-signing key. | > See all examples in [`examples/configure-skills-http/`](../examples/configure-skills-http/) diff --git a/libs/skills/catalog/frontmcp-deployment/references/protocol-versions.md b/libs/skills/catalog/frontmcp-deployment/references/protocol-versions.md index 817fd4294..b63a3d86e 100644 --- a/libs/skills/catalog/frontmcp-deployment/references/protocol-versions.md +++ b/libs/skills/catalog/frontmcp-deployment/references/protocol-versions.md @@ -115,7 +115,7 @@ contract. | Tasks | core protocol | `io.modelcontextprotocol/tasks` extension | | Not found | `-32002` | `-32602` | -`server/discover` returns the capabilities and `instructions` `initialize` would: `experimental['claude/channel']` when the server has channels, and the server's `instructions` with the channel hint and the skill catalog (`skillsConfig.injectInstructions`) for that caller. Up to 1.9.2 it sent neither the channel capability nor the hint and catalog. +`server/discover` returns the capabilities and `instructions` `initialize` would: `experimental['claude/channel']` when the server has channels, and the server's `instructions` with the channel hint and the skills line or catalog (`skillsConfig.injectInstructions`) for that caller. Up to 1.9.2 it sent neither the channel capability nor the hint and catalog. ## Mirrored request headers diff --git a/libs/skills/catalog/frontmcp-development/examples/create-plugin/configurable-dynamic-plugin.md b/libs/skills/catalog/frontmcp-development/examples/create-plugin/configurable-dynamic-plugin.md index 00b6e6734..c07cdbc38 100644 --- a/libs/skills/catalog/frontmcp-development/examples/create-plugin/configurable-dynamic-plugin.md +++ b/libs/skills/catalog/frontmcp-development/examples/create-plugin/configurable-dynamic-plugin.md @@ -176,7 +176,7 @@ class MyApp {} class MyServer {} ``` -Providers that `dynamicProviders()` returns reach the apps the plugin is installed for without an `exports` entry, which is why `this.myService` resolves in `DeleteUserTool`. Each provider needs a `name`; `inject` is optional and only needed when the factory takes dependencies. Inside the plugin, `this.get(token)` resolves from the plugin's own providers. +Providers that `dynamicProviders()` returns reach the apps the plugin is installed for without an `exports` entry, which is why `this.myService` resolves in `DeleteUserTool`. `name` is optional and defaults to the token's name; `inject` is optional too, and only needed when the factory takes dependencies. Inside the plugin, `this.get(token)` resolves from the plugin's own providers. ## What This Demonstrates diff --git a/libs/skills/catalog/frontmcp-development/examples/create-plugin/plugin-with-context-extension.md b/libs/skills/catalog/frontmcp-development/examples/create-plugin/plugin-with-context-extension.md index d58d0a4c3..a4aa9fe79 100644 --- a/libs/skills/catalog/frontmcp-development/examples/create-plugin/plugin-with-context-extension.md +++ b/libs/skills/catalog/frontmcp-development/examples/create-plugin/plugin-with-context-extension.md @@ -127,7 +127,7 @@ class RecordsApp {} export default class Server {} ``` -`providers` and `exports` take provider definitions, not bare tokens: `exports: [AuditLoggerToken]` throws when `@Plugin` runs (`providers items must be annotated with @Provider()`). Every provider object needs a `name`, whether it binds the token with `useFactory` (as here), `useClass` or `useValue`. The tool reaches `AuditLoggerToken` (and so `this.auditLog`) only because the plugin exports that definition to the app. The caller's identity comes from `this.auth.user.sub`; `this.scope` is the server scope, not the user. +`providers` and `exports` take provider definitions, not bare tokens: `exports: [AuditLoggerToken]` throws when `@Plugin` runs (`providers items must be annotated with @Provider()`). A provider object binds the token with `useFactory` (as here), `useClass` or `useValue`, and its `name` is optional: it defaults to the token's name. The tool reaches `AuditLoggerToken` (and so `this.auditLog`) only because the plugin exports that definition to the app. The caller's identity comes from `this.auth.user.sub`; `this.scope` is the server scope, not the user. ## What This Demonstrates diff --git a/libs/skills/catalog/frontmcp-development/references/create-plugin.md b/libs/skills/catalog/frontmcp-development/references/create-plugin.md index e44975c4a..4873320d5 100644 --- a/libs/skills/catalog/frontmcp-development/references/create-plugin.md +++ b/libs/skills/catalog/frontmcp-development/references/create-plugin.md @@ -140,7 +140,7 @@ class MainApp {} class MyServer {} ``` -A provider that `dynamicProviders()` returns reaches the apps the plugin is installed for without an `exports` entry, so `GreetTool` resolves `GreeterService` directly. Every provider object needs a `name`; `GreeterService` is built by the factory, so it needs no `@Provider` decorator. +A provider that `dynamicProviders()` returns reaches the apps the plugin is installed for without an `exports` entry, so `GreetTool` resolves `GreeterService` directly. A provider object's `name` is optional and defaults to its token's name; `GreeterService` is built by the factory, so it needs no `@Provider` decorator. ## Step 1: Create a Simple Plugin @@ -216,9 +216,9 @@ export default class AuditLogPlugin {} A tool in the host app then resolves `this.get(AuditLoggerToken)`. -- `@Plugin({ providers, exports })` accepts a class decorated with `@Provider({ name })` (for example `providers: [AuditLogger], exports: [AuditLogger]`), or a provider object with a `name`: `{ name, provide, useFactory, inject? }`, `{ name, provide, useClass }` or `{ name, provide, useValue }`. `inject: () => [Dep] as const` is only needed when the factory takes dependencies. -- `exports` lists provider definitions from `providers`, never a bare token: `exports: [AuditLoggerToken]` throws when `@Plugin` runs (`providers items must be annotated with @Provider()`). A provider object without `name` is rejected the same way. -- `@Provider` metadata is strict: `name` is required, and `id`, `description` and `scope` (`ProviderScope.GLOBAL`, the default, or `ProviderScope.CONTEXT` for one instance per request) are the only other keys. A bare `@Provider()` throws. +- `@Plugin({ providers, exports })` accepts a class decorated with `@Provider()` (for example `providers: [AuditLogger], exports: [AuditLogger]`), or a provider object: `{ provide, useFactory, inject? }`, `{ provide, useClass }` or `{ provide, useValue }`, each with an optional `name`. `inject: () => [Dep] as const` is only needed when the factory takes dependencies. +- `exports` lists provider definitions from `providers`, never a bare token: `exports: [AuditLoggerToken]` throws when `@Plugin` runs (`providers items must be annotated with @Provider()`). +- `@Provider` metadata is strict: `name`, `id`, `description` and `scope` (`ProviderScope.GLOBAL`, the default, or `ProviderScope.CONTEXT` for one instance per request) are the only keys, and all are optional. `name` defaults to the class name, and a provider object's to its token's name (a symbol's description or a string token). Up to 1.10.15, every provider needed a `name`, and a bare `@Provider()` threw. - Without `exports`, a provider from `@Plugin({ providers })` is visible only inside the plugin (its hooks, tools and `this.get()`). Providers returned by `dynamicProviders()` reach the host app without it. ### A provider that needs the scope @@ -618,7 +618,7 @@ plugins/ | Context extension registration | `contextExtensions: [{ property: 'auditLog', token: AuditLoggerToken }]` in metadata | `Object.defineProperty(ExecutionContextBase.prototype, ...)` manually | SDK handles runtime installation; manual modification causes ordering issues | | Type augmentation | `declare module '@frontmcp/sdk' { interface ExecutionContextBase { ... } }` in a separate file | Skipping the augmentation and casting `this` in tools | Without augmentation, TypeScript cannot type-check `this.auditLog` | | Provider types | `Token = Symbol('AuditLogger')` with typed token | `provide: Symbol('AuditLogger')` without type annotation | Typed tokens enable compile-time DI resolution checking | -| Exporting a provider | `providers: [auditLoggerProvider], exports: [auditLoggerProvider]` | `exports: [AuditLoggerToken]`, or a provider object without `name` | `exports` takes named provider definitions; the other forms throw at decoration | +| Exporting a provider | `providers: [auditLoggerProvider], exports: [auditLoggerProvider]` | `exports: [AuditLoggerToken]` (a bare token) | `exports` takes the provider definitions from `providers`; a bare token throws at decoration | | Caller identity in a tool | `this.auth.user.sub` | `this.scope.userId` | `this.scope` is the server scope; the user is on `this.auth` | | Plugin scope | `scope: 'app'` (default) for app-scoped behavior | `scope: 'server'` when hooks should only apply to one app | Server scope fires hooks for every app's entries, even when declared on one `@App`; default to app | | Dynamic options | Extend `DynamicPlugin` with `static dynamicProviders()` | Constructing providers in the constructor body | `dynamicProviders` runs before instantiation, enabling proper DI wiring | @@ -631,7 +631,7 @@ plugins/ - [ ] `@Plugin` decorator has `name` and `description` - [ ] Providers are listed in `providers` array with typed tokens - [ ] Exported providers are listed in `exports` array as the same definitions, not bare tokens -- [ ] Every provider object has a `name`, and every `@Provider` decorator passes `{ name }` +- [ ] A provider sets `name` only where its default (the class name, or a provider object's token name) is not the name you want in logs and errors - [ ] Context extensions have `property`, `token`, and `errorMessage` fields ### Type Safety @@ -651,16 +651,16 @@ plugins/ ## Troubleshooting -| Problem | Cause | Solution | -| ------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------- | -| `this.auditLog` has type `any` or is unrecognized | Module augmentation file not imported | Add side-effect import: `import './audit-log.context-extension'` in plugin file | -| Circular dependency error at startup | Calling `installExtension()` at module top level | Remove manual installation; use `contextExtensions` metadata array instead | -| Provider not found in tool context | Provider not listed in plugin `exports` | List the same provider definition in both `providers` and `exports` (the definition, not its token), or return it from `dynamicProviders()` | -| Hooks fire for unrelated apps in gateway | Plugin `scope` set to `'server'` | Change to `scope: 'app'` (default) unless server-wide behavior is intended | -| `DynamicPlugin.init()` options ignored | Overriding constructor without calling `super()` | Ensure constructor calls `super()` and merges defaults properly | -| `this.auditLog` throws your `errorMessage` although the plugin is installed | The token is registered in `@Plugin({ providers })` but not exported, so the app cannot resolve it | Add the provider definition to `exports`, or return it from `dynamicProviders()` | -| `@Plugin invalid metadata for "providers"` or `providers items must be annotated with @Provider()` when the module loads | A bare token in `exports`, a provider object (`useFactory`, `useClass` or `useValue`) without `name`, or an undecorated class | Use a `@Provider({ name })` class or a provider object with a `name` (`{ name, provide, useFactory }`), and export that same definition | -| Error thrown where a provider class is declared | Bare `@Provider()`: `name` is required | `@Provider({ name: 'my-plugin:service' })`, optionally with `description` and `scope: ProviderScope.CONTEXT` | +| Problem | Cause | Solution | +| ------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------- | +| `this.auditLog` has type `any` or is unrecognized | Module augmentation file not imported | Add side-effect import: `import './audit-log.context-extension'` in plugin file | +| Circular dependency error at startup | Calling `installExtension()` at module top level | Remove manual installation; use `contextExtensions` metadata array instead | +| Provider not found in tool context | Provider not listed in plugin `exports` | List the same provider definition in both `providers` and `exports` (the definition, not its token), or return it from `dynamicProviders()` | +| Hooks fire for unrelated apps in gateway | Plugin `scope` set to `'server'` | Change to `scope: 'app'` (default) unless server-wide behavior is intended | +| `DynamicPlugin.init()` options ignored | Overriding constructor without calling `super()` | Ensure constructor calls `super()` and merges defaults properly | +| `this.auditLog` throws your `errorMessage` although the plugin is installed | The token is registered in `@Plugin({ providers })` but not exported, so the app cannot resolve it | Add the provider definition to `exports`, or return it from `dynamicProviders()` | +| `@Plugin invalid metadata for "providers"` or `providers items must be annotated with @Provider()` when the module loads | A bare token in `exports`, or an undecorated class | Use a `@Provider()` class or a provider object (`{ provide, useFactory }`), and export that same definition | +| Error thrown where a provider class is declared | `@Provider({...})` got an empty `name` or a key it does not take (`provide`, `useFactory`, ...) | Pass only `name`, `id`, `description` and `scope`, and bind a token with `AsyncProvider({ provide, useFactory })` | ## Examples diff --git a/libs/skills/catalog/frontmcp-development/references/create-provider.md b/libs/skills/catalog/frontmcp-development/references/create-provider.md index 84fab7608..ccd005b38 100644 --- a/libs/skills/catalog/frontmcp-development/references/create-provider.md +++ b/libs/skills/catalog/frontmcp-development/references/create-provider.md @@ -130,7 +130,9 @@ class MyApp {} class Server {} ``` -Besides a `@Provider` class and a factory, `providers` takes a value or class binding, `{ name, provide, useValue }` or `{ name, provide, useClass }`. Every provider object needs a `name`. Up to 1.9.0, `@FrontMcp`, `@App`, `@Plugin` and `@Agent` refused `useClass` objects, and `useValue` objects unless the value was an instance of a `@Provider` class. +A config object takes the same `providers`, `AsyncProvider` factories included, wherever it goes: `FrontMcpInstance.runStdio(config)`, `bootstrap(config)`, `createDirect(config)` and `runUnixSocket({ ...config, socketPath })`. + +Besides a `@Provider` class and a factory, `providers` takes a value or class binding, `{ provide, useValue }` or `{ provide, useClass }`. `name` is optional everywhere and shows in logs and errors. It defaults to the class name for a `@Provider()` class, and to the token's name for a provider object: the class name, a symbol's description or a string token. Up to 1.10.15, every provider needed a `name`. Up to 1.9.0, `@FrontMcp`, `@App`, `@Plugin` and `@Agent` refused `useClass` objects, and `useValue` objects unless the value was an instance of a `@Provider` class. ## Step 4: Use in Tools @@ -362,7 +364,7 @@ frontmcp dev ### Configuration -- [ ] Provider class has `@Provider` decorator with `name` (or factory uses `AsyncProvider`) +- [ ] Provider class has the `@Provider()` decorator (or factory uses `AsyncProvider`) - [ ] Token is defined with `Token` using a `Symbol` and typed interface - [ ] Provider (class or `AsyncProvider` factory) is registered in `providers` array of `@App` or `@FrontMcp` - [ ] Sync setup happens in the constructor (throws fast on missing config) diff --git a/libs/skills/catalog/frontmcp-development/references/decorators-guide.md b/libs/skills/catalog/frontmcp-development/references/decorators-guide.md index 99edf8aa3..b444630b0 100644 --- a/libs/skills/catalog/frontmcp-development/references/decorators-guide.md +++ b/libs/skills/catalog/frontmcp-development/references/decorators-guide.md @@ -64,7 +64,7 @@ FrontMCP uses a hierarchical decorator system. The nesting order is: | Field | Description | | --------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | `info` | `serverInfo` of every `initialize`: `name`, `version`, optional `title` (else the name), `websiteUrl`, `icons` | -| `instructions?` | Top-level server-level instructions (system prompt) surfaced in the MCP `initialize` response and the 2026-07-28 `server/discover` result. Combined with the channel hint and the skill catalog summary per `skillsConfig.injectInstructions`; sent by every transport, `createFetchHandler()` and `connect()` included | +| `instructions?` | Top-level server-level instructions (system prompt) surfaced in the MCP `initialize` response and the 2026-07-28 `server/discover` result. Combined with the channel hint and the skills line or catalog per `skillsConfig.injectInstructions`; sent by every transport, `createFetchHandler()` and `connect()` included | | `apps` | Array of `@App` classes to mount | | `serve?` | Auto-start HTTP server (default: `true`). Set `false` for programmatic usage | | `splitByApp?` | If `true`, each app gets its own scope and basePath. Default: `false` | @@ -74,7 +74,7 @@ FrontMCP uses a hierarchical decorator system. The nesting order is: | `tools?` | Standalone tools (outside apps, merged with app tools) | | `resources?` | Standalone resources (merged with app resources) | | `skills?` | Standalone skills (merged with app skills) | -| `skillsConfig?` | Skills HTTP endpoints (`/llm.txt`, `/skills`), MCP tool config, `injectInstructions` policy, and tamper-evident `audit` log — see `configure-skills-http` | +| `skillsConfig?` | Skills HTTP endpoints (`/llm.txt`, `/skills`), MCP tool config, `injectInstructions` policy, `SKILL.md` resource annotations, and tamper-evident `audit` log — see `configure-skills-http` | | `transport?` | Transport preset (`'modern'`, `'legacy'`, `'stateless-api'`, `'full'`) or object | | `auth?` | Authentication mode: `'public'`, `'transparent'`, `'local'`, `'remote'` | | `http?` | HTTP server options (port, host, cors, socketPath) | @@ -469,7 +469,7 @@ class GitHubAdapter { | Field | Description | | -------------- | ------------------------------------------------------------------------------------------------------------------------- | -| `name` | Provider name (required) | +| `name?` | Provider name, shown in logs and errors; defaults to the class name | | `id?` | Optional stable identifier | | `description?` | Human-readable description | | `scope?` | `ProviderScope.GLOBAL` (default) or `ProviderScope.CONTEXT` (`SESSION` and `REQUEST` are deprecated aliases of `CONTEXT`) | diff --git a/libs/skills/catalog/frontmcp-production-readiness/references/common-checklist.md b/libs/skills/catalog/frontmcp-production-readiness/references/common-checklist.md index 029ea7c65..a03426fce 100644 --- a/libs/skills/catalog/frontmcp-production-readiness/references/common-checklist.md +++ b/libs/skills/catalog/frontmcp-production-readiness/references/common-checklist.md @@ -162,7 +162,7 @@ These checks apply to ALL deployment targets. Run them first, then proceed to yo - [ ] Audit chain verifier (`verifyChain`) runs in CI on the latest tail - [ ] Single-writer constraint respected (v1.2.0): writes routed to a single elected leader pod - [ ] `instructions` is set or `skillsConfig.injectInstructions` tuned for clients -- [ ] Auto-injected skill catalog summary stays within the 16 KB ceiling (review on each catalog change) +- [ ] With `injectInstructions: 'append'`, `'prepend'` or `'replace'`, the skill catalog summary stays within the 16 KB ceiling (the default `'compact'` sends one line) ## ExtApps / Widgets (if enabled) diff --git a/libs/skills/catalog/frontmcp-testing/SKILL.md b/libs/skills/catalog/frontmcp-testing/SKILL.md index 2d26aa7cd..02f61bbc7 100644 --- a/libs/skills/catalog/frontmcp-testing/SKILL.md +++ b/libs/skills/catalog/frontmcp-testing/SKILL.md @@ -95,18 +95,18 @@ This is a router skill. Follow this order to pick a testing approach, then move ## Cross-Cutting Testing Patterns -| Pattern | Rule | -| ------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| File naming | Always `.spec.ts` (not `.test.ts`); E2E uses `.e2e.spec.ts` | -| File organization | Split E2E tests by app/feature: `e2e/calc.e2e.spec.ts`, `e2e/ecommerce.e2e.spec.ts`. Never put all tests in a single `server.e2e.spec.ts` | -| Environment | `frontmcp test` loads `.env` / `.env.local` into the Jest child, same precedence as `dev` (real environment wins over the files, config `env.shared` + `env.test` underneath). `--no-env` skips it | -| Test runner | Standalone projects: use `frontmcp test` (auto-generates Jest/SWC config; discovers `src/**/*.spec.ts(x)`, `__tests__/**/*.spec.ts(x)`, and `e2e/**/*.e2e.spec.ts(x)`; transforms both `.ts` and `.tsx` with the automatic JSX runtime; transpiles ESM-only deps such as `jose`, `@noble/hashes` and `@noble/ciphers` (CodeCall) under npm, yarn AND pnpm's `node_modules/.pnpm/` store — add your own via `test.esmPackages` in `frontmcp.config.ts`; does not force `NODE_OPTIONS=--experimental-vm-modules` (it would bypass the ESM transforms; scaffolded projects use Jest 30); delegates to a user-provided `jest.config.{ts,js,mjs,cjs,json}` if present, which drops the injected config unless it starts from `preset: '@frontmcp/testing'`, which carries the same transform, discovery, timeout and ESM transforms). Nx monorepos: use `nx test ` (resolves the project's `jest.config.ts`). Never invoke `jest --config ...` directly | -| Coverage threshold | 95%+ across statements, branches, functions, lines | -| Test descriptions | Plain English, no prefixes like "PT-001"; describe behavior not implementation | -| Mocking | Mock providers via DI token replacement, never mock the framework | -| httpMock scope | `httpMock` intercepts HTTP in the **test process** only, NOT in the MCP server subprocess. Do not use httpMock to intercept server-to-API calls — those happen in the child process. Use httpMock for verifying client-to-server request shapes or mocking external APIs called from the test itself | -| Error testing | Assert `instanceof` specific error class AND MCP error code | -| Async | Always `await` async operations; use `expect(...).rejects.toThrow()` for async errors | +| Pattern | Rule | +| ------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| File naming | Always `.spec.ts` (not `.test.ts`); E2E uses `.e2e.spec.ts` | +| File organization | Split E2E tests by app/feature: `e2e/calc.e2e.spec.ts`, `e2e/ecommerce.e2e.spec.ts`. Never put all tests in a single `server.e2e.spec.ts` | +| Environment | `frontmcp test` loads `.env` / `.env.local` into the Jest child, same precedence as `dev` (real environment wins over the files, config `env.shared` + `env.test` underneath). `--no-env` skips it | +| Test runner | Standalone projects: use `frontmcp test` (auto-generates Jest/SWC config; discovers `src/**/*.spec.ts(x)`, `__tests__/**/*.spec.ts(x)`, and `e2e/**/*.e2e.spec.ts(x)`; transforms both `.ts` and `.tsx` with the automatic JSX runtime; transpiles ESM-only deps such as `jose`, `@noble/hashes` and `@noble/ciphers` (CodeCall) under npm, yarn AND pnpm's `node_modules/.pnpm/` store — add your own via `test.esmPackages` in `frontmcp.config.ts`; does not force `NODE_OPTIONS=--experimental-vm-modules` (it would bypass the ESM transforms; scaffolded projects use Jest 30); delegates to a user-provided `jest.config.{ts,js,mjs,cjs,json}` if present, which drops the injected config unless it starts from `preset: '@frontmcp/testing'`, which carries the same transform, discovery, timeout and ESM transforms; for unit tests, a CommonJS or ts-jest project included, start from `preset: '@frontmcp/testing/jest-preset-unit'`: the same transforms, Jest's 5 s timeout, no setup file, `*.spec.ts(x)` other than `.e2e`/`.perf`/`.pw` specs). Nx monorepos: use `nx test ` (resolves the project's `jest.config.ts`). Never invoke `jest --config ...` directly | +| Coverage threshold | 95%+ across statements, branches, functions, lines | +| Test descriptions | Plain English, no prefixes like "PT-001"; describe behavior not implementation | +| Mocking | Mock providers via DI token replacement, never mock the framework | +| httpMock scope | `httpMock` intercepts HTTP in the **test process** only, NOT in the MCP server subprocess. Do not use httpMock to intercept server-to-API calls — those happen in the child process. Use httpMock for verifying client-to-server request shapes or mocking external APIs called from the test itself | +| Error testing | Assert `instanceof` specific error class AND MCP error code | +| Async | Always `await` async operations; use `expect(...).rejects.toThrow()` for async errors | ## Common Patterns @@ -146,22 +146,22 @@ This is a router skill. Follow this order to pick a testing approach, then move ## Troubleshooting -| Problem | Cause | Solution | -| --------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Jest not finding test files | Wrong file extension (`.test.ts` instead of `.spec.ts`) | Rename to `.spec.ts`; check `testMatch` in jest.config | -| `SyntaxError: Unexpected token 'export'` | An ESM-only dependency is being ignored instead of transpiled | Add it to `test.esmPackages` in `frontmcp.config.ts`. With a hand-written `jest.config.ts`, start from `preset: '@frontmcp/testing'` or use the pnpm-safe `transformIgnorePatterns` in [`setup-testing`](./references/setup-testing.md#jest-configuration) AND make sure `transform` matches `.js` (`^.+\.[tj]sx?$` + `allowJs`) — un-ignoring a file does nothing if no transform matches it | -| Coverage below 95% | Untested error paths or conditional branches | Run `frontmcp test --coverage` and inspect uncovered lines in the report | -| E2E test timeout | Server startup too slow or port conflict | Increase Jest timeout; use random port allocation | -| DI resolution fails in tests | Provider not registered in test scope | Register mock providers before creating the test context | -| Istanbul shows 0% on async methods | TypeScript source-map mismatch with Istanbul | Known issue with some TS compilation settings; verify coverage with actual test output | -| Specs see no `.env` values | An older CLI did not load `.env` for `frontmcp test`, only for `dev` | Upgrade the CLI — `frontmcp test` now loads `.env` / `.env.local` with the same precedence as `dev` (real environment wins, so CI secrets still override). Pass `--no-env` for a hermetic run | -| `Invalid first argument, true` at collection time | `test.skip(condition, reason)` reached Jest's `skip(name, fn)` | Upgrade the CLI — the Playwright signature is supported: `test.skip(!hasCredentials, 'credentials not set')` skips every test registered after it in the enclosing block | -| `test.use()` in one `describe` leaks into another, or two files fight over a port | Older `@frontmcp/testing` kept one global config and per-process ports | Upgrade — `test.use()` is scoped per `describe` and ports are locked across Jest workers; use `port: 0` | -| Every spec fails with `HTTP 404` after setting `http.entryPath` | The test client always connected to the server root | Upgrade the CLI — the client now follows the `entryPaths` a 404 reports, and `test.use({ entryPath: '/mcp' })` sets it explicitly | -| `test.beforeEach(async ({ mcp }) => …)` times out waiting for `done` | Older `@frontmcp/testing` handed the hook to Jest, which read the parameter as `done` | Upgrade — hooks that take a parameter now receive the test's fixtures (same `mcp` as the test); `beforeAll` / `afterAll` never get fixtures | -| `test.use({ transport: 'sse' })` throws "SSE transport not yet implemented" | Older `@frontmcp/testing` had no legacy SSE client | Upgrade — `'sse'` is the legacy HTTP+SSE transport; the server must enable it with `transport: { protocol: { legacy: true } }` | -| `Module @swc/jest in the transform option was not found` from `frontmcp test` | Older `@frontmcp/testing` did not install the transformer the injected config uses | Upgrade — `@frontmcp/testing` depends on `@swc/jest` / `@swc/core`, and `frontmcp test` resolves the transformer through it | -| `ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG` from a server started inside a spec | Older SDKs loaded the optional `vectoriadb` with `import()` at boot | Upgrade the SDK — it loads `vectoriadb` and `jose` on first use and falls back to `require` where Jest cannot run `import()`; no `--experimental-vm-modules` needed | +| Problem | Cause | Solution | +| --------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Jest not finding test files | Wrong file extension (`.test.ts` instead of `.spec.ts`) | Rename to `.spec.ts`; check `testMatch` in jest.config | +| `SyntaxError: Unexpected token 'export'` | An ESM-only dependency is being ignored instead of transpiled | Add it to `test.esmPackages` in `frontmcp.config.ts`. With a hand-written `jest.config.ts`, start from `preset: '@frontmcp/testing'` (or `'@frontmcp/testing/jest-preset-unit'`; with ts-jest, set its transform for `^.+\.tsx?$` only) or use the pnpm-safe `transformIgnorePatterns` in [`setup-testing`](./references/setup-testing.md#jest-configuration) AND make sure `transform` matches `.js` (`^.+\.[tj]sx?$` + `allowJs`) — un-ignoring a file does nothing if no transform matches it | +| Coverage below 95% | Untested error paths or conditional branches | Run `frontmcp test --coverage` and inspect uncovered lines in the report | +| E2E test timeout | Server startup too slow or port conflict | Increase Jest timeout; use random port allocation | +| DI resolution fails in tests | Provider not registered in test scope | Register mock providers before creating the test context | +| Istanbul shows 0% on async methods | TypeScript source-map mismatch with Istanbul | Known issue with some TS compilation settings; verify coverage with actual test output | +| Specs see no `.env` values | An older CLI did not load `.env` for `frontmcp test`, only for `dev` | Upgrade the CLI — `frontmcp test` now loads `.env` / `.env.local` with the same precedence as `dev` (real environment wins, so CI secrets still override). Pass `--no-env` for a hermetic run | +| `Invalid first argument, true` at collection time | `test.skip(condition, reason)` reached Jest's `skip(name, fn)` | Upgrade the CLI — the Playwright signature is supported: `test.skip(!hasCredentials, 'credentials not set')` skips every test registered after it in the enclosing block | +| `test.use()` in one `describe` leaks into another, or two files fight over a port | Older `@frontmcp/testing` kept one global config and per-process ports | Upgrade — `test.use()` is scoped per `describe` and ports are locked across Jest workers; use `port: 0` | +| Every spec fails with `HTTP 404` after setting `http.entryPath` | The test client always connected to the server root | Upgrade the CLI — the client now follows the `entryPaths` a 404 reports, and `test.use({ entryPath: '/mcp' })` sets it explicitly | +| `test.beforeEach(async ({ mcp }) => …)` times out waiting for `done` | Older `@frontmcp/testing` handed the hook to Jest, which read the parameter as `done` | Upgrade — hooks that take a parameter now receive the test's fixtures (same `mcp` as the test); `beforeAll` / `afterAll` never get fixtures | +| `test.use({ transport: 'sse' })` throws "SSE transport not yet implemented" | Older `@frontmcp/testing` had no legacy SSE client | Upgrade — `'sse'` is the legacy HTTP+SSE transport; the server must enable it with `transport: { protocol: { legacy: true } }` | +| `Module @swc/jest in the transform option was not found` from `frontmcp test` | Older `@frontmcp/testing` did not install the transformer the injected config uses | Upgrade — `@frontmcp/testing` depends on `@swc/jest` / `@swc/core`, and `frontmcp test` resolves the transformer through it | +| `ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG` from a server started inside a spec | Older SDKs loaded the optional `vectoriadb` with `import()` at boot | Upgrade the SDK — it loads `vectoriadb` and `jose` on first use and falls back to `require` where Jest cannot run `import()`; no `--experimental-vm-modules` needed | ## Examples diff --git a/libs/skills/catalog/frontmcp-testing/references/setup-testing.md b/libs/skills/catalog/frontmcp-testing/references/setup-testing.md index 806426572..91b914077 100644 --- a/libs/skills/catalog/frontmcp-testing/references/setup-testing.md +++ b/libs/skills/catalog/frontmcp-testing/references/setup-testing.md @@ -552,8 +552,23 @@ defaults: the same `@swc/jest` transform (no `ts-jest` needed), the same discove `__tests__/**/*.spec.ts(x)`, `e2e/**/*.e2e.spec.ts(x)`), the same 60-second timeout, and the same `transformIgnorePatterns` for `jose`, `@noble/hashes` and `@noble/ciphers`. Setting `testMatch` or `transformIgnorePatterns` replaces the preset's list rather than extending it, so copy its entries into yours. -Name the package, not `'@frontmcp/testing/jest-preset'`: Jest looks for `jest-preset.js` at the root of the -package it is given. +Name the package, `'@frontmcp/testing'`, not `'@frontmcp/testing/jest-preset'`: Jest looks for `jest-preset.js` +under the name it is given. + +For unit tests, start from `preset: '@frontmcp/testing/jest-preset-unit'`. It shares the transform and the +`jose` / `@noble/*` patterns above, keeps Jest's default 5-second timeout, registers no setup file (add +`@frontmcp/testing/setup` to `setupFilesAfterEnv` to use the MCP matchers), and runs every `*.spec.ts(x)` except +`.e2e.spec`, `.perf.spec` and `.pw.spec` files. It also fits a CommonJS project that compiles its TypeScript with +`ts-jest`: without it, the first auth path that loads `jose` fails with `SyntaxError: Unexpected token 'export'`. +Set the ts-jest transform for `.ts` only, so the preset's transform still compiles those packages: + +```javascript +// jest.config.js +module.exports = { + preset: '@frontmcp/testing/jest-preset-unit', + transform: { '^.+\\.tsx?$': ['ts-jest', { tsconfig: '/tsconfig.json' }] }, +}; +``` ## Manual Testing with frontmcp dev diff --git a/libs/skills/catalog/skills-manifest.json b/libs/skills/catalog/skills-manifest.json index af0215455..99f53ef36 100644 --- a/libs/skills/catalog/skills-manifest.json +++ b/libs/skills/catalog/skills-manifest.json @@ -1066,7 +1066,7 @@ }, { "name": "configure-skills-http", - "description": "Full reference for skillsConfig — HTTP catalog endpoints, auth, caching, instructions injection, and tamper-evident audit log.", + "description": "Full reference for skillsConfig — HTTP catalog endpoints, auth, caching, instructions injection, SKILL.md resource annotations, and tamper-evident audit log.", "examples": [ { "name": "audit-log-basic", @@ -1095,12 +1095,13 @@ }, { "name": "inject-instructions", - "description": "Set a server-level instructions string and append the skill catalog summary on every initialize response.", + "description": "Set a server-level instructions string and append the full skill catalog summary on every initialize response.", "level": "basic", "tags": ["config", "skills", "instructions", "injection", "initialize"], "features": [ "Top-level `instructions` on `@FrontMcp` exposes a global system prompt to MCP clients", "`skillsConfig.injectInstructions: 'append'` adds the skill catalog summary after the user prompt", + "The default, `'compact'`, adds one line with the skill count and the `skill://index.json` pointer instead", "Dynamic skills are picked up because the composer runs on every initialize request", "The catalog only names skills the initializing caller may see (skill authorities and the `skills:filter` flow, e.g. feature flags)", "Catalog summary is bounded at 16 KB with a truncation footer pointing at skill://index.json" diff --git a/libs/testing/ARCHITECTURE.md b/libs/testing/ARCHITECTURE.md index 455afc2b6..de214eba7 100644 --- a/libs/testing/ARCHITECTURE.md +++ b/libs/testing/ARCHITECTURE.md @@ -1267,7 +1267,9 @@ libs/testing/ │ └── errors/ │ └── index.ts # Custom error classes │ -├── jest-preset.js # Jest preset configuration +├── jest-preset.js # Jest preset configuration (`frontmcp test`'s settings) +├── jest-preset-unit.js # Jest preset for unit tests, CommonJS projects included +├── jest-preset.base.js # What both presets share: transform, ESM-only packages ├── package.json # Package manifest └── ARCHITECTURE.md # This file ``` diff --git a/libs/testing/README.md b/libs/testing/README.md index 9daf048e5..389f53f50 100644 --- a/libs/testing/README.md +++ b/libs/testing/README.md @@ -36,7 +36,26 @@ test('tool execution works', async ({ mcp }) => { npx frontmcp test ``` -`frontmcp test` (from the [`frontmcp`](https://www.npmjs.com/package/frontmcp) CLI) runs `src/**/*.spec.ts`, `__tests__/**/*.spec.ts` and `e2e/**/*.e2e.spec.ts` with a Jest configuration it generates. With a `jest.config.*` of your own, use `preset: '@frontmcp/testing'` for the same settings. The server starts when the first test that needs it runs, and tests with the same `test.use()` options share it. +`frontmcp test` (from the [`frontmcp`](https://www.npmjs.com/package/frontmcp) CLI) runs `src/**/*.spec.ts`, `__tests__/**/*.spec.ts` and `e2e/**/*.e2e.spec.ts` with a Jest configuration it generates. With a `jest.config.*` of your own, start from one of the [Jest presets](#jest-presets). The server starts when the first test that needs it runs, and tests with the same `test.use()` options share it. + +## Jest presets + +| Preset | Use it for | +| ---------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `preset: '@frontmcp/testing'` | The configuration `frontmcp test` generates: unit and e2e specs, a 60-second timeout, and the matchers registered through `@frontmcp/testing/setup` | +| `preset: '@frontmcp/testing/jest-preset-unit'` | Unit tests, in a CommonJS project too: Jest's default 5-second timeout, no setup file, and every `*.spec.ts(x)` except `*.e2e.spec`, `*.perf.spec` and `*.pw.spec` files. Add `@frontmcp/testing/setup` to `setupFilesAfterEnv` to use the matchers | + +Both compile TypeScript and TSX with `@swc/jest`, and both compile `jose`, `@noble/hashes` and `@noble/ciphers`, ESM-only packages the SDK loads, which Jest's CommonJS runtime otherwise fails on with `SyntaxError: Unexpected token 'export'`. To compile your TypeScript with `ts-jest`, set a transform for `.ts` only, so the preset still compiles those packages: + +```js +// jest.config.js +module.exports = { + preset: '@frontmcp/testing/jest-preset-unit', + transform: { '^.+\\.tsx?$': 'ts-jest' }, +}; +``` + +`testMatch` and `transformIgnorePatterns` of your own replace the preset's lists, so copy its entries into yours. ## Fixtures @@ -60,7 +79,7 @@ npx frontmcp test **Tool UI**: `toHaveRenderedHtml`, `toContainHtmlElement`, `toContainBoundValue`, `toBeXssSafe`, `toHaveWidgetMetadata`, `toHaveCssClass`, `toNotContainRawContent`, `toHaveProperHtmlStructure` -Use them through the `expect` this package exports, which is Jest's `expect` with these matchers typed. `frontmcp test` and the preset also register them on Jest's global `expect`, through `@frontmcp/testing/setup`, which you can add to `setupFilesAfterEnv` yourself. +Use them through the `expect` this package exports, which is Jest's `expect` with these matchers typed. `frontmcp test` and the `@frontmcp/testing` preset register them, through `@frontmcp/testing/setup`, which you can add to `setupFilesAfterEnv` yourself. ## Configuration diff --git a/libs/testing/jest-preset-unit.js b/libs/testing/jest-preset-unit.js new file mode 100644 index 000000000..00d74dfb3 --- /dev/null +++ b/libs/testing/jest-preset-unit.js @@ -0,0 +1,11 @@ +/** Jest preset for unit tests, CommonJS projects included; the @frontmcp/testing README shows how to use it. */ + +const base = require('./jest-preset.base.js'); + +module.exports = { + ...base, + + testMatch: ['/**/*.spec.ts', '/**/*.spec.tsx'], + + testPathIgnorePatterns: [...base.testPathIgnorePatterns, '\\.(e2e|perf|pw)\\.spec\\.tsx?$'], +}; diff --git a/libs/testing/jest-preset.base.js b/libs/testing/jest-preset.base.js new file mode 100644 index 000000000..a2ce7b92f --- /dev/null +++ b/libs/testing/jest-preset.base.js @@ -0,0 +1,35 @@ +/** What both @frontmcp/testing Jest presets share, so they cannot drift. */ + +module.exports = { + testEnvironment: 'node', + + // Covers `.js` too: ESM-only deps ship `.js`, and un-ignoring them in `transformIgnorePatterns` needs a matching transform. + transform: { + '^.+\\.[tj]sx?$': [ + // The transform `frontmcp test` injects, resolved from here because @frontmcp/testing installs it + require.resolve('@swc/jest'), + { + jsc: { + target: 'es2022', + parser: { syntax: 'typescript', tsx: true, decorators: true, dynamicImport: true }, + transform: { decoratorMetadata: true, legacyDecorator: true, react: { runtime: 'automatic' } }, + keepClassNames: true, + externalHelpers: false, + loose: true, + }, + module: { type: 'es6' }, + sourceMaps: true, + swcrc: false, + }, + ], + }, + + moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'], + + // The SDK's ESM-only runtime deps: `jose` (auth) and `@noble/*` (`@frontmcp/utils` crypto). The `.pnpm` skip keeps this correct under pnpm (issue #519). + transformIgnorePatterns: [ + 'node_modules[/\\\\](?!\\.pnpm[/\\\\])(?!(jose|@noble[/\\\\]hashes|@noble[/\\\\]ciphers)[/\\\\])', + ], + + testPathIgnorePatterns: ['/node_modules/', '/dist/'], +}; diff --git a/libs/testing/jest-preset.js b/libs/testing/jest-preset.js index 2092426f7..12ee02f21 100644 --- a/libs/testing/jest-preset.js +++ b/libs/testing/jest-preset.js @@ -3,6 +3,7 @@ * @description Jest preset for @frontmcp/testing * * Mirrors the config `frontmcp test` injects (libs/cli/src/commands/dev/test.ts); a CLI test fails when they differ. + * For unit tests in a CommonJS project, use `@frontmcp/testing/jest-preset-unit`. * * Usage in jest.config.ts: * ```typescript @@ -13,33 +14,10 @@ * ``` */ -module.exports = { - // Use Node.js environment for E2E tests - testEnvironment: 'node', - - // Covers `.js` too: ESM-only deps ship `.js`, and un-ignoring them in `transformIgnorePatterns` needs a matching transform. - transform: { - '^.+\\.[tj]sx?$': [ - // The transform `frontmcp test` injects, resolved from here because @frontmcp/testing installs it - require.resolve('@swc/jest'), - { - jsc: { - target: 'es2022', - parser: { syntax: 'typescript', tsx: true, decorators: true, dynamicImport: true }, - transform: { decoratorMetadata: true, legacyDecorator: true, react: { runtime: 'automatic' } }, - keepClassNames: true, - externalHelpers: false, - loose: true, - }, - module: { type: 'es6' }, - sourceMaps: true, - swcrc: false, - }, - ], - }, +const base = require('./jest-preset.base.js'); - // File extensions to consider - moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'], +module.exports = { + ...base, testTimeout: 60000, @@ -56,14 +34,6 @@ module.exports = { '/e2e/**/*.e2e.spec.tsx', ], - // Transpile ESM-only deps (`@noble/*` for CodeCall); the `.pnpm` skip keeps this correct under pnpm (issue #519). - transformIgnorePatterns: [ - 'node_modules[/\\\\](?!\\.pnpm[/\\\\])(?!(jose|@noble[/\\\\]hashes|@noble[/\\\\]ciphers)[/\\\\])', - ], - - // Ignore patterns - testPathIgnorePatterns: ['/node_modules/', '/dist/'], - // Coverage settings (optional, disabled by default for E2E) collectCoverage: false, diff --git a/libs/testing/package.json b/libs/testing/package.json index 281118029..c4234b64d 100644 --- a/libs/testing/package.json +++ b/libs/testing/package.json @@ -56,6 +56,12 @@ "./jest-preset": { "default": "./jest-preset.js" }, + "./jest-preset-unit": { + "default": "./jest-preset-unit.js" + }, + "./jest-preset-unit/jest-preset": { + "default": "./jest-preset-unit.js" + }, "./fixtures": { "development": "./src/fixtures/index.ts", "require": { diff --git a/libs/testing/project.json b/libs/testing/project.json index 804b17b1e..d5b2f682d 100644 --- a/libs/testing/project.json +++ b/libs/testing/project.json @@ -19,7 +19,14 @@ "bundle": true, "thirdParty": false, "platform": "node", - "assets": ["libs/testing/README.md", "LICENSE", "libs/testing/jest-preset.js", "libs/testing/package.json"], + "assets": [ + "libs/testing/README.md", + "LICENSE", + "libs/testing/jest-preset.js", + "libs/testing/jest-preset.base.js", + "libs/testing/jest-preset-unit.js", + "libs/testing/package.json" + ], "additionalEntryPoints": [ "libs/testing/src/setup.ts", "libs/testing/src/fixtures/index.ts", diff --git a/libs/testing/src/__tests__/jest-preset-unit.spec.ts b/libs/testing/src/__tests__/jest-preset-unit.spec.ts new file mode 100644 index 000000000..2d17ec263 --- /dev/null +++ b/libs/testing/src/__tests__/jest-preset-unit.spec.ts @@ -0,0 +1,114 @@ +import * as path from 'path'; + +interface JestPreset { + transform: Record; + moduleFileExtensions: string[]; + transformIgnorePatterns: string[]; + testMatch: string[]; + testPathIgnorePatterns: string[]; + testTimeout?: number; + setupFilesAfterEnv?: string[]; +} + +interface ProjectConfig { + transform: [pattern: string, transformerPath: string, options: unknown][]; + transformIgnorePatterns: string[]; +} + +const packageRoot = path.resolve(__dirname, '../..'); +const unitPreset = require('../../jest-preset-unit.js') as JestPreset; +const e2ePreset = require('../../jest-preset.js') as JestPreset; +const { globsToMatcher } = require('jest-util') as { globsToMatcher(globs: string[]): (filePath: string) => boolean }; +const { readConfig } = require('jest-config') as { + readConfig( + argv: object, + config: object, + skipArgvConfigOption: boolean, + configDir: string, + ): Promise<{ projectConfig: ProjectConfig }>; +}; + +async function resolveConfig(config: object): Promise { + const { projectConfig } = await readConfig({}, { rootDir: packageRoot, ...config }, false, packageRoot); + return projectConfig; +} + +function transformerFor(config: ProjectConfig, filePath: string): string | undefined { + return config.transform.find(([pattern]) => new RegExp(pattern).test(filePath))?.[1]; +} + +const isIgnored = (patterns: string[], filePath: string) => + patterns.some((pattern) => new RegExp(pattern).test(filePath)); + +describe('@frontmcp/testing/jest-preset-unit', () => { + it('is what `preset: "@frontmcp/testing/jest-preset-unit"` and the package export load', async () => { + const config = await resolveConfig({ preset: '@frontmcp/testing/jest-preset-unit' }); + + expect(require('@frontmcp/testing/jest-preset-unit')).toBe(unitPreset); + expect(config.transformIgnorePatterns).toEqual(unitPreset.transformIgnorePatterns); + }); + + it('shares the transform of @frontmcp/testing/jest-preset', () => { + expect(unitPreset.transform).toBe(e2ePreset.transform); + expect(unitPreset.transformIgnorePatterns).toBe(e2ePreset.transformIgnorePatterns); + expect(unitPreset.moduleFileExtensions).toBe(e2ePreset.moduleFileExtensions); + }); + + it.each([ + '/proj/node_modules/jose/dist/webapi/index.js', + '/proj/node_modules/.pnpm/jose@6.2.3/node_modules/jose/dist/webapi/index.js', + '/proj/node_modules/@noble/hashes/sha2.js', + '/proj/node_modules/.pnpm/@noble+ciphers@2.1.1/node_modules/@noble/ciphers/aes.js', + ])('transpiles the ESM-only dependency at %s', (filePath) => { + expect(isIgnored(unitPreset.transformIgnorePatterns, filePath)).toBe(false); + }); + + it('still compiles those dependencies with its own transform when ts-jest compiles the TypeScript', async () => { + const config = await resolveConfig({ + preset: '@frontmcp/testing/jest-preset-unit', + transform: { '^.+\\.tsx?$': 'ts-jest' }, + }); + + expect(transformerFor(config, '/proj/src/server.spec.ts')).toBe(require.resolve('ts-jest')); + expect(transformerFor(config, '/proj/node_modules/jose/dist/webapi/index.js')).toBe(require.resolve('@swc/jest')); + }); + + it("keeps Jest's default timeout and registers no setup file", () => { + expect(unitPreset.testTimeout).toBeUndefined(); + expect(unitPreset.setupFilesAfterEnv).toBeUndefined(); + }); + + it.each(['/proj/src/tools/add.tool.spec.ts', '/proj/lib/__tests__/badge.spec.tsx'])('runs %s', (filePath) => { + const matches = globsToMatcher(unitPreset.testMatch.map((glob) => glob.replace('', '/proj'))); + + expect(matches(filePath)).toBe(true); + expect(isIgnored(unitPreset.testPathIgnorePatterns, filePath)).toBe(false); + }); + + it.each([ + '/proj/e2e/server.e2e.spec.ts', + '/proj/src/load.perf.spec.ts', + '/proj/src/ui.pw.spec.tsx', + '/proj/node_modules/pkg/a.spec.ts', + '/proj/dist/a.spec.ts', + ])('leaves out %s', (filePath) => { + expect(isIgnored(unitPreset.testPathIgnorePatterns, filePath)).toBe(true); + }); +}); + +describe('the @frontmcp/testing build', () => { + it('ships every file the package exports outside dist', () => { + const { exports } = require('../../package.json') as { exports: Record }; + const project = require('../../project.json') as { + targets: { 'build-cjs': { options: { assets: (string | { input: string })[] } } }; + }; + const assets = project.targets['build-cjs'].options.assets; + const shipped = Object.values(exports) + .map((target) => target?.default) + .filter((file): file is string => typeof file === 'string' && !file.startsWith('./dist/')) + .map((file) => path.posix.join('libs/testing', file)); + const requiredByPresets = ['libs/testing/jest-preset.base.js']; + + expect(assets).toEqual(expect.arrayContaining([...shipped, ...requiredByPresets])); + }); +}); diff --git a/plugins/plugin-feature-flags/src/__tests__/feature-flag.flows.spec.ts b/plugins/plugin-feature-flags/src/__tests__/feature-flag.flows.spec.ts index dd5a755d2..edc3aaf1d 100644 --- a/plugins/plugin-feature-flags/src/__tests__/feature-flag.flows.spec.ts +++ b/plugins/plugin-feature-flags/src/__tests__/feature-flag.flows.spec.ts @@ -555,7 +555,10 @@ describe('FeatureFlagPlugin and the skill catalog in the initialize instructions } it('lists an enabled skill and leaves a disabled one out of the catalog', async () => { - const instructions = await instructionsFor(serverConfig); + const instructions = await instructionsFor({ + ...serverConfig, + skillsConfig: { enabled: true, injectInstructions: 'append' }, + }); expect(instructions).toContain('**enabled-skill**: Enabled flagged workflow'); expect(instructions).not.toContain('disabled-skill');