From 38bdc0ad936558ed1c5f3504a4f5a39002f4a75c Mon Sep 17 00:00:00 2001 From: olaservo Date: Wed, 22 Apr 2026 08:59:23 -0700 Subject: [PATCH 1/2] feat(core): implement skills-over-MCP SEP extension MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Host support for io.modelcontextprotocol/skills: MCP servers can publish skills at skill://index.json (or any :// the server chooses), which gemini-cli discovers, surfaces through SkillManager, and lazy-loads on activation via resources/read. Design highlights - SkillDefinition gains optional source + mcp fields; MCP skills have an empty body until first activation. - SkillManager tracks filesystem (localSkills) and sourced (sourcedSkills) skills separately, so MCP skills stay visible in the merged view throughout async filesystem rescans. setSkillsForSource(tag, entries) gives dynamic providers an atomic register/replace/clear path keyed by a source tag; filesystem skills win on name collision (warning emitted), MCP-vs-MCP collisions are deduped first-source-wins. - McpClient.discoverSkills runs after tool/prompt/resource discovery, gated on isTrustedFolder() AND (capabilities.resources || the skills extension being declared). Tools-only servers no longer receive a stray readResource('skill://index.json') probe. - ActivateSkillTool branches on source === 'mcp': fetches SKILL.md via resources/read, validates frontmatter.name against the index-advertised name (rejects on drift), caches the body, renders / provenance, and enumerates sibling skill-root URIs from the ResourceRegistry. Workspace context is NOT expanded for MCP skills. - Cascade resources/list_changed to skill re-discovery so servers that publish skills post-connect get re-indexed. - read_mcp_resource resolves the serving MCP server from the URI via the registry fallback, so the model only needs the bare URI. Trust boundaries Server-supplied content is treated as untrusted model input, never first-party host text. Specifically: - Skill names must match /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/ so server-supplied names cannot smuggle XML / z.enum / UI metacharacters into activated_skill XML, tool enum values, or /mcp output. - Descriptions capped at 500 chars, ASCII control bytes stripped; descriptions flow into model prompts verbatim. - Activated body is XML-escaped and wrapped as so the model cannot be tricked by a in the body to break out of the untrusted fence and impersonate host content. Same escape applied to serverName, skillUri, and sibling entries in the llmContent; markdown-escape applied to the activation-confirmation UI prompt so spoofed trust cues can't reach the user's decision point. - 256 KiB cap on fetched SKILL.md body before caching or inlining — the body lands in the model's context, so an unbounded server response is a memory and context-budget DoS vector. - Empty includeSkills == deny-all (matches the includeTools contract); cross-config intersection in mergeMcpConfigs is case-insensitive so two configs with intersecting allowlists don't produce [] that accidentally reads as "no allowlist configured." - discoverSkills short-circuits clear the source slot via setSkillsForSource(tag, []) so mid-session admin toggles, folder trust changes, and skillsEnabled=false flips don't leave already- registered skills in place until disconnect. Config / UI - MCPServerConfig gains skillsEnabled / includeSkills / excludeSkills (appended positionally to preserve source compat). includeSkills / excludeSkills matching is case-insensitive on both sides so 'Pull-Requests' in config matches a skill named 'pull-requests'. - /skills list tags MCP skills as [mcp:]. - /mcp list gains a Skills section. Tests - mcpSkillDiscovery, activate-skill, skillManager, mcp-client, read-mcp-resource, and mcp-client-manager coverage (~1,077 test lines), including MCP-vs-MCP name collision dedup, setDisabledSkills applying to MCP-sourced skills, case-insensitive include/exclude filtering, admin gate, per-server skillsEnabled=false, capability gate, and disconnect cleanup. - Verified end-to-end against olaservo/github-mcp-server @ add-agent-skills (pull-requests skill's SKILL.md containing submit_pending + add_comment_to_pending_review). Reproduction harness lives at skills-over-mcp-ig/clients/gemini-cli-harness/. SDK compatibility note discoverSkills today accepts servers that declare the extension via capabilities.experimental, because the TS SDK strips unknown capabilities keys. When SEP-2133 lands in the SDK, declaresSkillsExtension() already checks both the extensions and experimental slots, so no host-side change will be needed. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/cli/src/config/settingsSchema.ts | 17 + .../cli/src/ui/commands/mcpCommand.test.ts | 4 + packages/cli/src/ui/commands/mcpCommand.ts | 16 + .../cli/src/ui/components/views/McpStatus.tsx | 38 ++- .../src/ui/components/views/SkillsList.tsx | 5 + packages/cli/src/ui/types.ts | 8 + packages/core/src/config/config.ts | 9 + .../core/src/skills/mcpSkillDiscovery.test.ts | 300 +++++++++++++++++ packages/core/src/skills/mcpSkillDiscovery.ts | 248 ++++++++++++++ packages/core/src/skills/skillLoader.ts | 22 +- packages/core/src/skills/skillManager.test.ts | 161 +++++++++ packages/core/src/skills/skillManager.ts | 98 ++++-- .../core/src/tools/activate-skill.test.ts | 310 +++++++++++++++++- packages/core/src/tools/activate-skill.ts | 181 +++++++++- .../core/src/tools/mcp-client-manager.test.ts | 41 +++ packages/core/src/tools/mcp-client-manager.ts | 27 ++ packages/core/src/tools/mcp-client.test.ts | 227 ++++++++++++- packages/core/src/tools/mcp-client.ts | 148 ++++++++- .../core/src/tools/read-mcp-resource.test.ts | 41 +++ .../src/test-mcp-server-template.mjs | 99 +++++- packages/test-utils/src/test-mcp-server.ts | 49 +++ 21 files changed, 2005 insertions(+), 44 deletions(-) create mode 100644 packages/core/src/skills/mcpSkillDiscovery.test.ts create mode 100644 packages/core/src/skills/mcpSkillDiscovery.ts diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 93ac53ada3a..000072d768d 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -2883,6 +2883,23 @@ export const SETTINGS_SCHEMA_DEFINITIONS: Record< 'Tools that should be disabled for this server even if exposed.', items: { type: 'string' }, }, + skillsEnabled: { + type: 'boolean', + description: + 'Whether to discover Agent Skills published by this server per the skills-over-MCP SEP (io.modelcontextprotocol/skills). Defaults to true.', + }, + includeSkills: { + type: 'array', + description: + 'Allowlist of skill names to surface from this server. When omitted or empty all skills are surfaced. Matching is case-insensitive.', + items: { type: 'string' }, + }, + excludeSkills: { + type: 'array', + description: + 'Blocklist of skill names to hide from this server even when published. When omitted or empty no skills are blocked. Matching is case-insensitive.', + items: { type: 'string' }, + }, extension: { type: 'object', description: diff --git a/packages/cli/src/ui/commands/mcpCommand.test.ts b/packages/cli/src/ui/commands/mcpCommand.test.ts index d082c4ed09c..6f4711e3c14 100644 --- a/packages/cli/src/ui/commands/mcpCommand.test.ts +++ b/packages/cli/src/ui/commands/mcpCommand.test.ts @@ -77,6 +77,7 @@ describe('mcpCommand', () => { getGeminiClient: ReturnType; getMcpClientManager: ReturnType; getResourceRegistry: ReturnType; + getSkillManager: ReturnType; setUserInteractedWithMcp: ReturnType; getLastMcpError: ReturnType; }; @@ -113,6 +114,9 @@ describe('mcpCommand', () => { getResourceRegistry: vi.fn().mockReturnValue({ getAllResources: vi.fn().mockReturnValue([]), }), + getSkillManager: vi.fn().mockReturnValue({ + getAllSkills: vi.fn().mockReturnValue([]), + }), setUserInteractedWithMcp: vi.fn(), getLastMcpError: vi.fn().mockReturnValue(undefined), }; diff --git a/packages/cli/src/ui/commands/mcpCommand.ts b/packages/cli/src/ui/commands/mcpCommand.ts index 3fd214152e4..19bc421ae42 100644 --- a/packages/cli/src/ui/commands/mcpCommand.ts +++ b/packages/cli/src/ui/commands/mcpCommand.ts @@ -244,6 +244,16 @@ const listAction = async ( .getAllResources() .filter((entry) => serverNames.includes(entry.serverName)); + const skillManager = config.getSkillManager(); + const mcpSkills = skillManager + .getAllSkills() + .filter( + (skill) => + skill.source === 'mcp' && + !!skill.mcp && + serverNames.includes(skill.mcp.serverName), + ); + const authStatus: HistoryItemMcpStatus['authStatus'] = {}; const tokenStorage = new MCPOAuthTokenStorage(); for (const serverName of serverNames) { @@ -301,6 +311,12 @@ const listAction = async ( mimeType: resource.mimeType, description: resource.description, })), + skills: mcpSkills.map((skill) => ({ + serverName: skill.mcp!.serverName, + name: skill.name, + description: skill.description, + uri: skill.mcp!.skillUri, + })), authStatus, enablementState, errors, diff --git a/packages/cli/src/ui/components/views/McpStatus.tsx b/packages/cli/src/ui/components/views/McpStatus.tsx index 1f14c0b5c5e..015c2be791a 100644 --- a/packages/cli/src/ui/components/views/McpStatus.tsx +++ b/packages/cli/src/ui/components/views/McpStatus.tsx @@ -13,6 +13,7 @@ import type { HistoryItemMcpStatus, JsonMcpPrompt, JsonMcpResource, + JsonMcpSkill, JsonMcpTool, } from '../../types.js'; @@ -21,6 +22,7 @@ interface McpStatusProps { tools: JsonMcpTool[]; prompts: JsonMcpPrompt[]; resources: JsonMcpResource[]; + skills?: JsonMcpSkill[]; blockedServers: Array<{ name: string; extensionName: string }>; serverStatus: (serverName: string) => MCPServerStatus; authStatus: HistoryItemMcpStatus['authStatus']; @@ -37,6 +39,7 @@ export const McpStatus: React.FC = ({ tools, prompts, resources, + skills = [], blockedServers, serverStatus, authStatus, @@ -97,11 +100,15 @@ export const McpStatus: React.FC = ({ const serverResources = resources.filter( (resource) => resource.serverName === serverName, ); + const serverSkills = skills.filter( + (skill) => skill.serverName === serverName, + ); const originalStatus = serverStatus(serverName); const hasCachedItems = serverTools.length > 0 || serverPrompts.length > 0 || - serverResources.length > 0; + serverResources.length > 0 || + serverSkills.length > 0; const status = originalStatus === MCPServerStatus.DISCONNECTED && hasCachedItems ? MCPServerStatus.CONNECTED @@ -164,6 +171,10 @@ export const McpStatus: React.FC = ({ `${resourceCount} ${resourceCount === 1 ? 'resource' : 'resources'}`, ); } + const skillCount = serverSkills.length; + if (skillCount > 0) { + parts.push(`${skillCount} ${skillCount === 1 ? 'skill' : 'skills'}`); + } const serverAuthStatus = authStatus[serverName]; let authStatusNode: React.ReactNode = null; @@ -315,6 +326,31 @@ export const McpStatus: React.FC = ({ )} )} + + {serverSkills.length > 0 && ( + + + Skills (io.modelcontextprotocol/skills): + + {serverSkills.map((skill) => ( + + + - {skill.name} + {` (${skill.uri})`} + + {showDescriptions && skill.description && ( + + + {skill.description.trim()} + + + )} + + ))} + + )} ); })} diff --git a/packages/cli/src/ui/components/views/SkillsList.tsx b/packages/cli/src/ui/components/views/SkillsList.tsx index d6b681a94e6..c6c257d37c2 100644 --- a/packages/cli/src/ui/components/views/SkillsList.tsx +++ b/packages/cli/src/ui/components/views/SkillsList.tsx @@ -44,6 +44,11 @@ export const SkillsList: React.FC = ({ {skill.isBuiltin && ( {' [Built-in]'} )} + {skill.source === 'mcp' && skill.mcp && ( + + {` [mcp:${skill.mcp.serverName}]`} + + )} {showDescriptions && skill.description && ( diff --git a/packages/cli/src/ui/types.ts b/packages/cli/src/ui/types.ts index 1ded2ae643e..b77ee15cd0d 100644 --- a/packages/cli/src/ui/types.ts +++ b/packages/cli/src/ui/types.ts @@ -355,12 +355,20 @@ export interface JsonMcpResource { description?: string; } +export interface JsonMcpSkill { + serverName: string; + name: string; + description: string; + uri: string; +} + export type HistoryItemMcpStatus = HistoryItemBase & { type: 'mcp_status'; servers: Record; tools: JsonMcpTool[]; prompts: JsonMcpPrompt[]; resources: JsonMcpResource[]; + skills?: JsonMcpSkill[]; authStatus: Record< string, 'authenticated' | 'expired' | 'unauthenticated' | 'not-configured' diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 01c6fd7bfd6..af67ec5c58d 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -498,6 +498,15 @@ export class MCPServerConfig { readonly targetAudience?: string, /* targetServiceAccount format: @.iam.gserviceaccount.com */ readonly targetServiceAccount?: string, + // Skills-over-MCP (io.modelcontextprotocol/skills) extension controls. + // Appended at the end so existing positional callers remain source-compatible. + // When false, skill discovery is suppressed for this server even if it + // declares the extension capability. Defaults to true. + readonly skillsEnabled?: boolean, + // Allowlist of skill names to surface from this server (exact match). + readonly includeSkills?: string[], + // Blocklist of skill names to hide from this server (exact match). + readonly excludeSkills?: string[], ) {} } diff --git a/packages/core/src/skills/mcpSkillDiscovery.test.ts b/packages/core/src/skills/mcpSkillDiscovery.test.ts new file mode 100644 index 00000000000..a964f72b35e --- /dev/null +++ b/packages/core/src/skills/mcpSkillDiscovery.test.ts @@ -0,0 +1,300 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi } from 'vitest'; +import { + declaresSkillsExtension, + discoverMcpSkills, + skillSourceTag, + SKILLS_EXTENSION_ID, + SKILL_INDEX_URI, +} from './mcpSkillDiscovery.js'; +import type { Client } from '@modelcontextprotocol/sdk/client/index.js'; + +function makeClient({ + capabilities, + readResourceImpl, +}: { + capabilities?: unknown; + readResourceImpl?: (args: { uri: string }) => Promise; +}): Client { + return { + getServerCapabilities: vi.fn().mockReturnValue(capabilities), + readResource: vi + .fn() + .mockImplementation((args: { uri: string }) => + readResourceImpl + ? readResourceImpl(args) + : Promise.reject(new Error('not configured')), + ), + } as unknown as Client; +} + +function textContents(obj: unknown): { + contents: Array<{ uri?: string; mimeType?: string; text: string }>; +} { + return { + contents: [ + { + uri: SKILL_INDEX_URI, + mimeType: 'application/json', + text: typeof obj === 'string' ? obj : JSON.stringify(obj), + }, + ], + }; +} + +describe('declaresSkillsExtension', () => { + it('returns true when extensions slot carries the SEP id', () => { + const client = makeClient({ + capabilities: { + extensions: { [SKILLS_EXTENSION_ID]: {} }, + }, + }); + expect(declaresSkillsExtension(client)).toBe(true); + }); + + it('also accepts the experimental slot as a fallback', () => { + const client = makeClient({ + capabilities: { + experimental: { [SKILLS_EXTENSION_ID]: {} }, + }, + }); + expect(declaresSkillsExtension(client)).toBe(true); + }); + + it('returns false when capability is absent', () => { + const client = makeClient({ + capabilities: { tools: {}, resources: {} }, + }); + expect(declaresSkillsExtension(client)).toBe(false); + }); + + it('returns false when capabilities are undefined', () => { + const client = makeClient({ capabilities: undefined }); + expect(declaresSkillsExtension(client)).toBe(false); + }); +}); + +describe('discoverMcpSkills', () => { + it('materializes skill-md entries as SkillDefinitions with empty body', async () => { + const client = makeClient({ + readResourceImpl: async () => + textContents({ + $schema: 'https://schemas.agentskills.io/discovery/0.2.0/schema.json', + skills: [ + { + name: 'pull-requests', + type: 'skill-md', + description: 'PR workflow', + url: 'skill://pull-requests/SKILL.md', + }, + ], + }), + }); + const result = await discoverMcpSkills(client, 'github-skills'); + expect(result).toHaveLength(1); + expect(result[0]).toMatchObject({ + name: 'pull-requests', + description: 'PR workflow', + source: 'mcp', + body: '', + location: 'skill://pull-requests/SKILL.md', + mcp: { + serverName: 'github-skills', + skillUri: 'skill://pull-requests/SKILL.md', + }, + }); + }); + + it('skips mcp-resource-template entries and logs them', async () => { + const client = makeClient({ + readResourceImpl: async () => + textContents({ + skills: [ + { + type: 'mcp-resource-template', + description: 'Per-product docs', + url: 'skill://docs/{product}/SKILL.md', + }, + { + name: 'ready', + type: 'skill-md', + description: 'A concrete skill', + url: 'skill://ready/SKILL.md', + }, + ], + }), + }); + const result = await discoverMcpSkills(client, 'srv'); + expect(result.map((s) => s.name)).toEqual(['ready']); + }); + + it('returns empty array when the server has no index', async () => { + const client = makeClient({ + readResourceImpl: async () => { + throw new Error('Resource not found'); + }, + }); + const result = await discoverMcpSkills(client, 'srv'); + expect(result).toEqual([]); + }); + + it('returns empty array when the index is not valid JSON', async () => { + const client = makeClient({ + readResourceImpl: async () => textContents('not-json'), + }); + const result = await discoverMcpSkills(client, 'srv'); + expect(result).toEqual([]); + }); + + it('returns empty array when the index is shaped wrong', async () => { + const client = makeClient({ + readResourceImpl: async () => textContents({ notSkills: true }), + }); + const result = await discoverMcpSkills(client, 'srv'); + expect(result).toEqual([]); + }); + + it('derives name from URI when the entry has no name field', async () => { + const client = makeClient({ + readResourceImpl: async () => + textContents({ + skills: [ + { + type: 'skill-md', + description: 'Nested skill', + url: 'skill://acme/billing/refunds/SKILL.md', + }, + ], + }), + }); + const result = await discoverMcpSkills(client, 'srv'); + expect(result).toHaveLength(1); + expect(result[0].name).toBe('refunds'); + }); + + it('accepts non-skill:// URIs (the SEP SHOULDs skill:// but permits any scheme)', async () => { + const client = makeClient({ + readResourceImpl: async () => + textContents({ + skills: [ + { + name: 'custom-scheme', + type: 'skill-md', + description: 'Server-native scheme', + url: 'github://skills/custom-scheme/SKILL.md', + }, + { + name: 'classic', + type: 'skill-md', + description: 'Classic entry', + url: 'skill://classic/SKILL.md', + }, + ], + }), + }); + const result = await discoverMcpSkills(client, 'srv'); + expect(result.map((s) => s.name).sort()).toEqual([ + 'classic', + 'custom-scheme', + ]); + const github = result.find((s) => s.name === 'custom-scheme'); + expect(github?.location).toBe('github://skills/custom-scheme/SKILL.md'); + expect(github?.mcp?.skillUri).toBe( + 'github://skills/custom-scheme/SKILL.md', + ); + }); + + it('rejects skill names that do not match the allowlist', async () => { + const client = makeClient({ + readResourceImpl: async () => + textContents({ + skills: [ + { + name: 'bad">