diff --git a/.changeset/28940-mcp-agent-prompt-advertised.md b/.changeset/28940-mcp-agent-prompt-advertised.md new file mode 100644 index 00000000000..3f4c379165c --- /dev/null +++ b/.changeset/28940-mcp-agent-prompt-advertised.md @@ -0,0 +1,8 @@ +--- +'@objectstack/mcp': patch +--- + +Clause-②: no +Metadata-protocol: none + +The stdio MCP server lists `agent_prompt` only on a boot that registers at least one agent. A boot with no agent, which is every open-framework boot, used to list it anyway. Its `agentName` argument suggested `"ask"` and `"build"`, and every `prompts/get` came back as a successful prompt whose one message read `Error: Agent "ask" not found`, which an MCP client passes to its model as instructions. On such a boot the prompt is now left out of `prompts/list`, a get on it is the protocol's own unknown-prompt error, and the boot log says once why. On a boot with agents nothing changes for a client: the prompt is listed, its argument suggests names the boot actually holds, and a miss still answers `Error: Agent "X" not found`. If the agent metadata cannot be fully read at boot, the prompt stays listed, because an outage is not proof that no agent exists. Its argument then suggests no names. diff --git a/packages/mcp/README.md b/packages/mcp/README.md index 35f08a0da0f..9f5448148d0 100644 --- a/packages/mcp/README.md +++ b/packages/mcp/README.md @@ -201,7 +201,9 @@ Two prompt families are bridged — there is no per-item prompt registration cal 1. **`agent_prompt`** — one dynamic prompt that loads a registered agent's system prompt by name, with optional UI context (`objectName`, `recordId`, - `viewName`). + `viewName`). It is listed only when the boot registers at least one agent, + and its `agentName` argument names agents that boot holds; the open + framework registers none, so there it is not listed. 2. **One prompt per authored skill** that carries `instructions` (#3905). Write a `*.skill.ts`, and its instructions become a prompt any connected MCP client can list and fetch. diff --git a/packages/mcp/src/__tests__/mcp-server-runtime.test.ts b/packages/mcp/src/__tests__/mcp-server-runtime.test.ts index 67e89b19726..f8de20284cf 100644 --- a/packages/mcp/src/__tests__/mcp-server-runtime.test.ts +++ b/packages/mcp/src/__tests__/mcp-server-runtime.test.ts @@ -293,8 +293,12 @@ describe('MCPServerRuntime', () => { it('survives a metadata service that cannot list skills', async () => { const metadataService = createMockMetadataService(); + const agents = await metadataService.list('agent'); metadataService.list = vi.fn(async (type: string) => { if (type === 'skill') throw new Error('unknown metadata type'); + // [#28940] The agents stay listed: `agent_prompt` is advertised only on + // a boot that holds one, and this case is about the skill read alone. + if (type === 'agent') return agents; return []; }) as any; diff --git a/packages/mcp/src/mcp-server-runtime.agent-prompt-advertised.test.ts b/packages/mcp/src/mcp-server-runtime.agent-prompt-advertised.test.ts new file mode 100644 index 00000000000..01fcf027adf --- /dev/null +++ b/packages/mcp/src/mcp-server-runtime.agent-prompt-advertised.test.ts @@ -0,0 +1,234 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `agent_prompt` is advertised only where an agent can answer it. + * + * The long-lived server (the one the stdio transport serves) used to register + * `agent_prompt` unconditionally, with an `agentName` argument described as + * `e.g. "ask", "build"`. On an open-framework boot that registers no agent at + * all, `prompts/list` therefore advertised a prompt nothing could serve, its + * description taught two names that resolve to nothing, and `prompts/get` + * answered each of them with a SUCCESSFUL prompt result whose one message is + * `Error: Agent "ask" not found` — text an MCP client hands its model as + * instructions. + * + * The skill projection in the same file already follows "declared IS served": + * a skill that cannot be served is not listed, and a get on it is a JSON-RPC + * -32602. These pins hold `agent_prompt` to the same rule: + * + * - no agent registered, and the read says so ⇒ not listed; a get is the + * protocol's own unknown-prompt error, not a fabricated success; + * - agents registered ⇒ listed, the argument names only registered agents, + * and a miss answers exactly what it answered before (#6055 kept that text + * byte-stable on purpose); + * - the read could not say (degraded, or it threw) ⇒ still listed: an outage + * is not absence, and the prompt then answers the honest #6055 outage text + * at get time. The argument names no agent, since none is known. + * + * Driven over the real wire (an in-memory transport into the same `McpServer` + * the stdio transport attaches to): the decision under test is what a client + * can list and fetch, so the client's view is the assertion. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; +import { MCPServerRuntime } from './mcp-server-runtime.js'; + +const SKILL = { + name: 'case_management', + label: 'Case Management', + description: 'Handles support case lifecycle', + instructions: 'Triage the case, then resolve it once the caller confirms.', + active: true, +}; + +const AGENT = { + name: 'support_triage', + label: 'Support Triage', + instructions: 'You triage support cases.', + active: true, +}; + +interface ServiceShape { + agents?: unknown[]; + skills?: unknown[]; + agentList?: () => Promise<{ items: unknown[]; degraded: boolean; errors: string[] }>; +} + +function metadataService({ agents = [], skills = [SKILL], agentList }: ServiceShape = {}) { + const byName = (items: unknown[], name: string) => + (items as Array<{ name: string }>).find((i) => i.name === name) ?? undefined; + return { + listObjects: vi.fn(async () => []), + getObject: vi.fn(async () => undefined), + get: vi.fn(async (type: string, name: string) => { + if (type === 'agent') return byName(agents, name); + if (type === 'skill') return byName(skills, name); + return undefined; + }), + list: vi.fn(async (type: string) => { + if (type === 'agent') return agents; + if (type === 'skill') return skills; + return []; + }), + listDiagnosed: vi.fn(async (type: string) => { + if (type === 'agent' && agentList) return agentList(); + const items = type === 'agent' ? agents : type === 'skill' ? skills : []; + return { items, degraded: false, errors: [] }; + }), + exists: vi.fn(async () => false), + register: vi.fn(), + unregister: vi.fn(), + }; +} + +function logger() { + return { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; +} + +async function connect(service: ReturnType, log = logger()) { + const runtime = new MCPServerRuntime({ name: 'objectstack-test', version: '9.9.9', logger: log as never }); + await runtime.bridgePrompts(service as never); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: 'test-client', version: '0.0.0' }); + await Promise.all([runtime.server.connect(serverTransport), client.connect(clientTransport)]); + return { + client, + log, + close: async () => { + await client.close(); + await runtime.stop().catch(() => {}); + }, + }; +} + +function agentNameDescription(prompts: Array<{ name: string; arguments?: Array<{ name: string; description?: string }> }>) { + const prompt = prompts.find((p) => p.name === 'agent_prompt'); + return prompt?.arguments?.find((a) => a.name === 'agentName')?.description; +} + +describe('agent_prompt — advertised only where an agent can answer it', () => { + it('a boot that registers no agent does not list agent_prompt', async () => { + const session = await connect(metadataService({ agents: [] })); + try { + const listed = await session.client.listPrompts(); + const names = listed.prompts.map((p) => p.name); + // The skill is still projected, so the absence below is a decision about + // this one prompt, not an empty or missing prompt surface. + expect(names).toContain('case_management'); + expect(names).not.toContain('agent_prompt'); + } finally { + await session.close(); + } + }); + + it('a get of agent_prompt on that boot is the protocol error, not a successful "not found" message', async () => { + const session = await connect(metadataService({ agents: [] })); + try { + const outcome = await session.client + .getPrompt({ name: 'agent_prompt', arguments: { agentName: 'ask' } }) + .then((result) => ({ result, error: undefined }), (error: { code?: number }) => ({ result: undefined, error })); + expect(outcome.result).toBeUndefined(); + expect(outcome.error?.code).toBe(-32602); + } finally { + await session.close(); + } + }); + + it('says once at boot why the prompt is not advertised', async () => { + const session = await connect(metadataService({ agents: [] })); + try { + const lines = session.log.info.mock.calls.map((c) => String(c[0])); + expect(lines.some((l) => l.includes('agent_prompt') && l.includes('no agent'))).toBe(true); + expect(lines).not.toContain('[MCP] Agent prompts bridged'); + } finally { + await session.close(); + } + }); + + it('a boot with agents lists it, and the argument names only agents the boot holds', async () => { + const session = await connect(metadataService({ agents: [AGENT] })); + try { + const listed = await session.client.listPrompts(); + expect(listed.prompts.map((p) => p.name)).toContain('agent_prompt'); + const description = agentNameDescription(listed.prompts); + expect(description).toContain('"support_triage"'); + expect(description).not.toContain('"ask"'); + expect(description).not.toContain('"build"'); + expect(session.log.info).toHaveBeenCalledWith('[MCP] Agent prompts bridged'); + } finally { + await session.close(); + } + }); + + it('on that boot a registered agent is served and a miss answers exactly as before (#6055)', async () => { + const session = await connect(metadataService({ agents: [AGENT] })); + try { + const served = await session.client.getPrompt({ name: 'agent_prompt', arguments: { agentName: 'support_triage' } }); + expect(served.messages[0].content).toEqual({ type: 'text', text: 'You triage support cases.' }); + + const miss = await session.client.getPrompt({ name: 'agent_prompt', arguments: { agentName: 'nope' } }); + expect(miss.messages).toEqual([ + { role: 'user', content: { type: 'text', text: 'Error: Agent "nope" not found' } }, + ]); + } finally { + await session.close(); + } + }); + + it('an agent read that reports itself degraded keeps the prompt listed, naming no agent', async () => { + const session = await connect( + metadataService({ + agentList: async () => ({ items: [], degraded: true, errors: ['loader sys_metadata: connection refused'] }), + }), + ); + try { + const listed = await session.client.listPrompts(); + expect(listed.prompts.map((p) => p.name)).toContain('agent_prompt'); + const description = agentNameDescription(listed.prompts) ?? ''; + expect(description).not.toContain('"ask"'); + expect(description).not.toContain('e.g.'); + expect(session.log.warn).toHaveBeenCalledWith( + expect.stringContaining('agent_prompt'), + expect.objectContaining({ errors: ['loader sys_metadata: connection refused'] }), + ); + } finally { + await session.close(); + } + }); + + it('an agent read that throws keeps the prompt listed', async () => { + const session = await connect( + metadataService({ + agentList: async () => { + throw new Error('unknown metadata type'); + }, + }), + ); + try { + const listed = await session.client.listPrompts(); + expect(listed.prompts.map((p) => p.name)).toContain('agent_prompt'); + expect(session.log.warn).toHaveBeenCalledWith( + expect.stringContaining('agent_prompt'), + expect.objectContaining({ errors: ['unknown metadata type'] }), + ); + } finally { + await session.close(); + } + }); + + it('with no agent and no skill the prompts capability is not advertised at all (#8034)', async () => { + const runtime = new MCPServerRuntime({ name: 'objectstack-test', version: '9.9.9' }); + await runtime.bridgePrompts(metadataService({ agents: [], skills: [] }) as never); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: 'test-client', version: '0.0.0' }); + await Promise.all([runtime.server.connect(serverTransport), client.connect(clientTransport)]); + try { + expect(client.getServerCapabilities()?.prompts).toBeUndefined(); + } finally { + await client.close(); + await runtime.stop().catch(() => {}); + } + }); +}); diff --git a/packages/mcp/src/mcp-server-runtime.ts b/packages/mcp/src/mcp-server-runtime.ts index cd9b926a77a..09f63b58993 100644 --- a/packages/mcp/src/mcp-server-runtime.ts +++ b/packages/mcp/src/mcp-server-runtime.ts @@ -777,6 +777,63 @@ export async function buildAgentPromptResult( }; } +/** + * [#28940] What the agent roster read at bridge time lets `agent_prompt` claim. + * + * - `agents` — the read named at least one agent: the prompt is advertised and + * its `agentName` argument names those agents. + * - `none` — the read answered completely and named no agent: nothing on this + * boot can answer the prompt, so it is not advertised. + * - `unknown` — the read was degraded or threw: whether an agent exists is not + * known, so the prompt is advertised as before (an outage is not absence, + * the #6055 discipline) and its argument names no agent. + */ +type AgentRoster = + | { state: 'agents'; names: string[] } + | { state: 'none' } + | { state: 'unknown'; errors: string[] }; + +/** + * [#28940] Read which agents this boot holds, for the `agent_prompt` + * advertisement. + * + * The roster is read from the metadata service — the same layer + * {@link buildAgentPromptResult} resolves a name from — so the names the + * argument teaches are names a `prompts/get` can answer. `agent` is a + * code-only type (ADR-0063 §2 closes `*.agent.ts` to third parties, and the + * save door refuses a runtime create), so no runtime write grows the set after + * boot, and the merged overlay read the skill snapshot uses has nothing to add. + */ +async function readAgentRoster(metadataService: IMetadataService): Promise { + let read: DiagnosedListRead; + try { + read = await diagnosedList(metadataService, 'agent'); + } catch (err) { + return { state: 'unknown', errors: [err instanceof Error ? err.message : String(err)] }; + } + const names = read.items + .map((item) => (item as { name?: unknown } | null | undefined)?.name) + .filter((name): name is string => typeof name === 'string' && name.length > 0); + if (names.length > 0) return { state: 'agents', names }; + if (read.degraded) return { state: 'unknown', errors: read.errors }; + return { state: 'none' }; +} + +/** + * [#28940] The `agentName` argument description: example names drawn from the + * agents this boot holds, never a fixed pair. + * + * It used to read `e.g. "ask", "build"` on every boot. On the open framework, + * which registers no agent, that taught two names that resolve to nothing. + * [#14461]'s concern — that the suggestion an author copies is a canonical id, + * never a retired alias — holds by construction: a registered agent's name is + * its canonical id. + */ +function agentNameArgumentDescription(names: readonly string[]): string { + if (names.length === 0) return 'Name of the agent to load'; + return `Name of the agent to load (e.g. ${names.slice(0, 2).map((n) => `"${n}"`).join(', ')})`; +} + /** * What MCP's `resources/read` answers with — the text-content subset of the * SDK's `ReadResourceResult`. Carries an index signature for the same reason @@ -1505,6 +1562,7 @@ export class MCPServerRuntime { * * 1. `agent_prompt` — one dynamic prompt that loads an agent's system prompt * by name, with optional UI context (objectName, recordId, viewName). + * Advertised only on a boot that holds an agent (#28940). * 2. One prompt per authored `skill` that carries `instructions` (#3905). * This is the open distribution's consumer for skill metadata: a tenant * writes `*.skill.ts`, and its instructions become a prompt any MCP client @@ -1539,33 +1597,58 @@ export class MCPServerRuntime { ): Promise { const logger = this.config.logger; - // Register a dynamic prompt that loads agents at call time - this.mcpServer.registerPrompt( - 'agent_prompt', - { - description: 'Load an agent\'s system prompt with optional UI context. ' + - 'Use the agentName argument to select which agent\'s instructions to use.', - argsSchema: { - // [#14461] The example names the two CANONICAL platform agent ids. - // It used to read `"data_chat", "metadata_assistant"` — both retired - // aliases, neither canonical id present — so every MCP client asking - // what to pass was taught the exact two spellings - // `skills/objectstack-ai` forbids. The aliases still resolve (cloud's - // one-way alias registry), so nothing broke; what was wrong is that - // this is the suggestion an author copies. - agentName: z.string().describe('Name of the agent to load (e.g. "ask", "build")'), - objectName: z.string().optional().describe('Current object the user is viewing'), - recordId: z.string().optional().describe('Currently selected record ID'), - viewName: z.string().optional().describe('Current view name'), + // [#28940] Declared IS served, the rule the skill projection below already + // follows: `agent_prompt` is advertised only when this boot holds an agent + // that can answer it. Before this it was registered unconditionally, so a + // boot with no agent (the open framework registers none) listed a prompt + // whose every get was a SUCCESSFUL result reading `Error: Agent "X" not + // found`, text an MCP client hands its model as instructions. The roster is + // a snapshot taken here, at the same moment as the skill list and for the + // same reason: this server's `prompts/list` belongs to the SDK and is fixed + // once the transport attaches. A roster read that cannot say (degraded, or + // it threw) keeps the prompt, which then answers the honest #6055 outage + // text at get time. + const roster = await readAgentRoster(metadataService); + if (roster.state === 'none') { + logger?.info( + '[MCP] agent_prompt not advertised — this boot registers no agent, so nothing could answer it. ' + + 'It is listed on a boot whose metadata service holds at least one agent.', + ); + } else { + if (roster.state === 'unknown') { + logger?.warn( + '[MCP] agent_prompt advertised without knowing whether any agent exists — the agent metadata ' + + 'could not be fully read, so its argument names no agent and a get answers ' + + 'SERVICE_UNAVAILABLE while the read stays down. ' + + 'Fix: check the loaders behind the metadata service (datasource connection, credentials, table).', + { errors: roster.errors }, + ); + } + // A dynamic prompt that loads the named agent at call time. + this.mcpServer.registerPrompt( + 'agent_prompt', + { + description: 'Load an agent\'s system prompt with optional UI context. ' + + 'Use the agentName argument to select which agent\'s instructions to use.', + argsSchema: { + // [#28940] Examples come from the roster, never a fixed pair: see + // {@link agentNameArgumentDescription}. + agentName: z.string().describe( + agentNameArgumentDescription(roster.state === 'agents' ? roster.names : []), + ), + objectName: z.string().optional().describe('Current object the user is viewing'), + recordId: z.string().optional().describe('Currently selected record ID'), + viewName: z.string().optional().describe('Current view name'), + }, }, - }, - async (args) => - // [#6055] Outage vs. miss lives in the builder — see - // {@link buildAgentPromptResult}. - buildAgentPromptResult(metadataService, args, logger), - ); + async (args) => + // [#6055] Outage vs. miss lives in the builder — see + // {@link buildAgentPromptResult}. + buildAgentPromptResult(metadataService, args, logger), + ); - logger?.info('[MCP] Agent prompts bridged'); + logger?.info('[MCP] Agent prompts bridged'); + } // ── Skill metadata → MCP prompts (#3905) ── // [#6504] This consumer is a SNAPSHOT, not a mis-describing surface, and is diff --git a/packages/mcp/src/mcp-stdio-tools.test.ts b/packages/mcp/src/mcp-stdio-tools.test.ts index 251a93c3f3c..67521932bd3 100644 --- a/packages/mcp/src/mcp-stdio-tools.test.ts +++ b/packages/mcp/src/mcp-stdio-tools.test.ts @@ -317,7 +317,9 @@ describe('#8034 stdio transport: capability ↔ served surface agreement', () => listObjects: vi.fn(async () => [{ name: 'task', label: 'Task', fields: { title: {} } }]), getObject: vi.fn(async () => null), get: vi.fn(async () => null), - list: vi.fn(async () => []), + // [#28940] One agent, so the bridged assembly really serves `prompts`: + // `agent_prompt` is advertised only on a boot that holds an agent. + list: vi.fn(async (type: string) => (type === 'agent' ? [{ name: 'support_triage' }] : [])), exists: vi.fn(async () => false), getRegisteredTypes: vi.fn(async () => ['object']), register: vi.fn(),