From 041695a9aceb795351e8cceb6ea5543ad2ac76b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 02:30:56 +0000 Subject: [PATCH] fix(mcp): a metadata outage stops being answered as "Agent X not found" (#6055) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `agent_prompt` read `metadataService.get('agent', name)` and answered its `undefined` with `Error: Agent "X" not found`. That `undefined` carries two opposite facts (#5840, ADR-0110 D3) — never declared, or every loader down — so an availability failure was reported to an MCP client as a declaration fact. The `objectstack://objects/{objectName}` resource had the same shape on `getObject()`. Both now separate the two, keeping the surface fail-closed: a degraded read answers SERVICE_UNAVAILABLE (the #5532/#5843 spelling), a genuine miss keeps its not-found answer. MCP's prompt/resource results carry no error envelope, so the classification travels in each surface's existing payload. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017uFVNMmTxLpmfQYiuKM1Yx --- .changeset/mcp-metadata-outage-vs-miss.md | 34 ++ ...mcp-server-runtime.metadata-outage.test.ts | 344 ++++++++++++++++ packages/mcp/src/mcp-server-runtime.ts | 388 ++++++++++++++---- 3 files changed, 688 insertions(+), 78 deletions(-) create mode 100644 .changeset/mcp-metadata-outage-vs-miss.md create mode 100644 packages/mcp/src/mcp-server-runtime.metadata-outage.test.ts diff --git a/.changeset/mcp-metadata-outage-vs-miss.md b/.changeset/mcp-metadata-outage-vs-miss.md new file mode 100644 index 0000000000..e982920b5f --- /dev/null +++ b/.changeset/mcp-metadata-outage-vs-miss.md @@ -0,0 +1,34 @@ +--- +'@objectstack/mcp': patch +--- + +mcp: a metadata outage stops being reported to MCP clients as `Agent "X" not found` + +The `agent_prompt` prompt resolved its body through `metadataService.get('agent', name)` +and answered the resulting `undefined` with `Error: Agent "X" not found`. That `undefined` +carries two opposite facts (#5840, ADR-0110 D3): the name was never declared, or every +loader behind the metadata service was down. So during a metadata outage an MCP client was +told, positively, what the author had declared — from a read that never happened. The same +shape sat one bridge over: the `objectstack://objects/{objectName}` resource answered +`getObject()`'s `undefined` with `Object "X" not found`. + +**Both surfaces now separate the two.** A degraded read answers `SERVICE_UNAVAILABLE` — +the same catalogued code and the same "whether it exists is unknown, retry once it is +reachable" sentence the `sys_metadata` half of this family already emits (#5532 / #5843) — +and a genuine miss keeps its not-found answer, byte for byte on the prompt surface. +MCP's `prompts/get` and `resources/read` results carry no error envelope, so the +classification travels in the payload each surface already had: the prompt's text, and the +resource's JSON body, which now names `code` and `status` on **both** answers +(`SERVICE_UNAVAILABLE`/503 vs `RESOURCE_NOT_FOUND`/404) so a client can tell them apart +without parsing prose. + +**This is a diagnosis fix, not an access change.** Both surfaces were already fail-closed: +no instructions and no schema were served during an outage before this, and none are now. +The defect was the description. + +Hosts whose `metadata` slot predates the optional `getDiagnosed` member report nothing +degraded — exactly what they could express before — so their behaviour is unchanged. The +object resource additionally keeps `getObject()` as its resolver and consults the +diagnosed read only as a verdict probe on the miss path, because `getObject` is its own +contract member with no documented equivalence to `get('object', name)` (and +`MetadataFacade.getObject` is not that). diff --git a/packages/mcp/src/mcp-server-runtime.metadata-outage.test.ts b/packages/mcp/src/mcp-server-runtime.metadata-outage.test.ts new file mode 100644 index 0000000000..0b588e309a --- /dev/null +++ b/packages/mcp/src/mcp-server-runtime.metadata-outage.test.ts @@ -0,0 +1,344 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#6055, ADR-0110 D3 — MCP side] A metadata plane that could not be READ is + * not an agent (or an object) that nobody declared. + * + * --------------------------------------------------------------------------- + * The defect + * --------------------------------------------------------------------------- + * `mcp-server-runtime.ts` resolved the `agent_prompt` body through + * `metadataService.get('agent', name)` and answered its `undefined` with + * `Error: Agent "X" not found`. `MetadataManager.get()` answers an unreachable + * loader chain with exactly the `undefined` a never-declared name produces + * (#5840), so during a metadata outage an MCP client was told, positively, what + * the author had declared — from a read that never happened. + * + * The same shape sat one bridge over in the same file: the + * `objectstack://objects/{objectName}` resource answered `getObject()`'s + * `undefined` with `Object "X" not found`. + * + * Both were **fail-closed** — no instructions and no schema were served either + * way — so this is a diagnosis defect, not a security one, and the fix must + * keep it that way. Every degraded case below therefore asserts BOTH halves: + * the answer is correctly classified, AND nothing was served. + * + * --------------------------------------------------------------------------- + * Why the assertions are not `toThrow()`, and what stands in for an envelope + * --------------------------------------------------------------------------- + * Neither surface throws, before or after: MCP answers `prompts/get` with a + * `GetPromptResult` and `resources/read` with a `ReadResourceResult`, and + * neither type carries an error envelope (only `CallToolResult` has `isError`). + * There is no ADR-0112 `code`+`status` on the wire to pin, so the strongest + * available discriminator is used instead, per surface: + * + * - the RESOURCE body is JSON, so it carries `code`/`status` structurally and + * both answers are pinned on them (`SERVICE_UNAVAILABLE`/503 vs + * `RESOURCE_NOT_FOUND`/404); + * - the PROMPT body is plain text, so the classification travels in the text + * and is pinned as: carries `SERVICE_UNAVAILABLE`, says "unknown", and does + * NOT say "not found". + * + * On top of that, every pair is pinned as a pair: the outage answer and the + * miss answer must not be equal. That is the fact the defect was — before the + * fix the two were byte-identical — and it is the one assertion that cannot be + * satisfied by a mis-worded improvement. + * + * --------------------------------------------------------------------------- + * Reverse verification, direction predicted BEFORE running + * --------------------------------------------------------------------------- + * Ordinary red, taken on this consumer. These doubles feed `getDiagnosed`'s + * return contract directly, so reverting the producer (`MetadataManager`) + * cannot move this file — only restoring the pre-#6055 reads here can. The + * reversion is defined as: `agent_prompt` back to + * `await metadataService.get('agent', name)` with the single `if (!raw)`, and + * the resource back to `getObject()` with the bare + * `{ error: 'Object "X" not found' }` body. + * + * Predicted, written down before running: **8 red / 9 green**, split + * 4 red / 6 green on the prompt and 4 red / 3 green on the resource. + * + * ⚠️ One case is predicted GREEN in BOTH directions, and that is the point of + * it rather than a gap: *"DEGRADED: access is still refused"*. The pre-fix code + * served no instructions during an outage either — it was fail-closed and + * merely mis-described — so an assertion that pins the affordance CANNOT go red + * on this fix's reversion. It is an invariant pin, not coverage of the change, + * and it is what would go red if a future "fix" here started serving a body. + * Reporting it as part of the red count would be a fabricated number; the + * measured result is recorded in the PR body as it came out. + * + * The doubles declare metadata reads only — no engine write verb — so there is + * no `delete`/`update` dispatch for `check:engine-double-contract` to scan and + * no guard to hand-mirror. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { IMetadataService } from '@objectstack/spec/contracts'; +import { + buildAgentPromptResult, + buildObjectSchemaResource, +} from './mcp-server-runtime.js'; + +type AnyRecord = Record; + +const LOADER_FAILURE = 'database: connect ECONNREFUSED 10.0.0.5:5432'; + +const AGENT = { name: 'data_chat', instructions: 'You answer questions about the data.' }; +const OBJECT = { name: 'acct', label: 'Account', fields: { title: { type: 'text' } } }; + +function makeLogger() { + return { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() } as AnyRecord; +} + +/** + * Build a metadata-service double. + * + * Every REQUIRED member of `IMetadataService` is present and throws, so a code + * path that reaches one this fix should not touch fails loudly instead of + * resolving `undefined` and looking like the very absence under test. + */ +function makeService(overrides: AnyRecord): IMetadataService { + const unexpected = (member: string) => async () => { + throw new Error(`double: ${member}() should not be called by this surface`); + }; + return { + register: unexpected('register'), + get: unexpected('get'), + list: unexpected('list'), + unregister: unexpected('unregister'), + exists: unexpected('exists'), + listNames: unexpected('listNames'), + getObject: unexpected('getObject'), + listObjects: unexpected('listObjects'), + ...overrides, + } as unknown as IMetadataService; +} + +/** Every loader behind the metadata service is down; nothing answered. */ +const inOutage = (extra: AnyRecord = {}) => + makeService({ + get: vi.fn(async () => undefined), + getObject: vi.fn(async () => undefined), + getDiagnosed: vi.fn(async () => ({ data: undefined, degraded: true, errors: [LOADER_FAILURE] })), + ...extra, + }); + +/** The read HAPPENED and nobody declared this name. */ +const withMiss = (extra: AnyRecord = {}) => + makeService({ + get: vi.fn(async () => undefined), + getObject: vi.fn(async () => undefined), + getDiagnosed: vi.fn(async () => ({ data: undefined, degraded: false, errors: [] })), + ...extra, + }); + +/** A healthy service holding `body`. */ +const holding = (body: unknown, extra: AnyRecord = {}) => + makeService({ + get: vi.fn(async () => body), + getObject: vi.fn(async () => body), + getDiagnosed: vi.fn(async () => ({ data: body, degraded: false, errors: [] })), + ...extra, + }); + +/** + * A service that predates `getDiagnosed` (#5840 declared it OPTIONAL). It + * cannot report the distinction, so this surface must degrade to exactly what + * it did before — never probe a member that is not there, never throw. + */ +const legacy = (body?: unknown) => + makeService({ + get: vi.fn(async () => body), + getObject: vi.fn(async () => body), + }); + +const promptText = (r: { messages: Array<{ content: { text: string } }> }) => r.messages[0].content.text; +const promptRole = (r: { messages: Array<{ role: string }> }) => r.messages[0].role; +const resourceBody = (r: { contents: Array<{ text: string }> }) => JSON.parse(r.contents[0].text); + +// ───────────────────────────────────────────────────────────────────────────── +// agent_prompt — the call site the issue names +// ───────────────────────────────────────────────────────────────────────────── + +describe('agent_prompt — a metadata outage is not "Agent not found" (#6055)', () => { + it('PRESENT: serves the agent instructions (unchanged)', async () => { + const svc = holding(AGENT); + const result = await buildAgentPromptResult(svc, { agentName: 'data_chat' }); + + expect(promptRole(result)).toBe('assistant'); + expect(promptText(result)).toContain('You answer questions about the data.'); + }); + + it('PRESENT: still folds the UI context hints in (unchanged)', async () => { + const result = await buildAgentPromptResult(holding(AGENT), { + agentName: 'data_chat', + objectName: 'acct', + recordId: 'r1', + viewName: 'all', + }); + + expect(promptText(result)).toContain('--- Current Context ---'); + expect(promptText(result)).toContain('Current object: acct'); + expect(promptText(result)).toContain('Selected record ID: r1'); + expect(promptText(result)).toContain('Current view: all'); + }); + + it('GENUINELY ABSENT: the not-found answer is preserved, byte for byte', async () => { + const result = await buildAgentPromptResult(withMiss(), { agentName: 'data_chat' }); + + // Verbatim: a miss is a real fact about what the author declared, and this + // surface was always right to state it. The fix must not reword it. + expect(promptText(result)).toBe('Error: Agent "data_chat" not found'); + }); + + it('DEGRADED: answers SERVICE_UNAVAILABLE, and never the not-found claim', async () => { + const result = await buildAgentPromptResult(inOutage(), { agentName: 'data_chat' }); + const text = promptText(result); + + expect(text).toContain('SERVICE_UNAVAILABLE'); + expect(text).toContain('whether agent "data_chat" exists is unknown'); + expect(text).not.toMatch(/not found/); + }); + + it('DEGRADED: access is still refused — no instructions are served', async () => { + const result = await buildAgentPromptResult(inOutage(), { agentName: 'data_chat' }); + + // Fail-closed, before and after. A body here would be a security + // regression, not a nicety — the defect was the DESCRIPTION, never the + // affordance. + expect(promptRole(result)).toBe('user'); + expect(promptText(result)).not.toContain(AGENT.instructions); + }); + + it('the outage and the miss no longer collapse to the same answer', async () => { + const outage = promptText(await buildAgentPromptResult(inOutage(), { agentName: 'data_chat' })); + const miss = promptText(await buildAgentPromptResult(withMiss(), { agentName: 'data_chat' })); + + // Same surface, same agent name, same (absent) result — only the health of + // the metadata plane differs. Before #6055 both produced + // `Error: Agent "data_chat" not found`. + expect(outage).not.toBe(miss); + expect(miss).toMatch(/not found/); + expect(outage).toMatch(/SERVICE_UNAVAILABLE/); + }); + + it('reads through getDiagnosed, not get, when the service offers it', async () => { + const svc = inOutage(); + await buildAgentPromptResult(svc, { agentName: 'data_chat' }); + + // If `get` were still the read, the verdict would be unreachable and the + // case above could only pass by accident. + expect((svc as AnyRecord).getDiagnosed).toHaveBeenCalledWith('agent', 'data_chat'); + expect((svc as AnyRecord).get).not.toHaveBeenCalled(); + }); + + it('logs the outage once, with the consequence and the fix', async () => { + const logger = makeLogger(); + await buildAgentPromptResult(inOutage(), { agentName: 'data_chat' }, logger as any); + + expect(logger.warn).toHaveBeenCalledTimes(1); + const [line, detail] = logger.warn.mock.calls[0]; + expect(String(line)).toContain('no instructions were served'); + expect(String(line)).toContain('Fix:'); + expect(detail).toMatchObject({ agentName: 'data_chat', errors: [LOADER_FAILURE] }); + // A miss is not a degradation and must not log at all. + const quiet = makeLogger(); + await buildAgentPromptResult(withMiss(), { agentName: 'data_chat' }, quiet as any); + expect(quiet.warn).not.toHaveBeenCalled(); + }); + + it('a service that predates getDiagnosed behaves exactly as it did', async () => { + const missing = legacy(); + expect(promptText(await buildAgentPromptResult(missing, { agentName: 'data_chat' }))) + .toBe('Error: Agent "data_chat" not found'); + expect((missing as AnyRecord).get).toHaveBeenCalledWith('agent', 'data_chat'); + + const present = legacy(AGENT); + expect(promptText(await buildAgentPromptResult(present, { agentName: 'data_chat' }))) + .toContain(AGENT.instructions); + }); + + it('a missing agentName argument is still refused before any read', async () => { + // `makeService`'s required members all throw, so reaching a read here fails + // the test rather than passing quietly. + const result = await buildAgentPromptResult(makeService({}), {}); + expect(promptText(result)).toBe('Error: agentName argument is required'); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// objectstack://objects/{objectName} — the same family, one bridge over +// ───────────────────────────────────────────────────────────────────────────── + +describe('object_schema resource — a metadata outage is not "Object not found" (#6055)', () => { + it('PRESENT: serves the object schema (unchanged)', async () => { + const body = resourceBody(await buildObjectSchemaResource(holding(OBJECT), 'acct')); + + expect(body).toMatchObject({ name: 'acct', label: 'Account' }); + expect(body.fields).toEqual([{ name: 'title', type: 'text', label: 'title', required: false }]); + }); + + it('PRESENT: the hit path costs no second read', async () => { + const svc = holding(OBJECT); + await buildObjectSchemaResource(svc, 'acct'); + + // `getObject` stays the resolver (it is its own contract member, and + // `MetadataFacade.getObject` is NOT `get('object', name)`); the diagnosed + // read is a verdict probe on the MISS path only. + expect((svc as AnyRecord).getObject).toHaveBeenCalledWith('acct'); + expect((svc as AnyRecord).getDiagnosed).not.toHaveBeenCalled(); + }); + + it('GENUINELY ABSENT: not-found, classified 404 / RESOURCE_NOT_FOUND', async () => { + const body = resourceBody(await buildObjectSchemaResource(withMiss(), 'acct')); + + expect(body).toEqual({ + error: 'Object "acct" not found', + code: 'RESOURCE_NOT_FOUND', + status: 404, + }); + }); + + it('DEGRADED: unavailable, classified 503 / SERVICE_UNAVAILABLE, no schema served', async () => { + const body = resourceBody(await buildObjectSchemaResource(inOutage(), 'acct')); + + expect(body.code).toBe('SERVICE_UNAVAILABLE'); + expect(body.status).toBe(503); + expect(body.error).toContain('whether object "acct" exists is unknown'); + expect(body.error).not.toMatch(/not found/); + // Fail-closed: still no schema. + expect(body.fields).toBeUndefined(); + expect(body.name).toBeUndefined(); + }); + + it('the outage and the miss no longer collapse to the same answer', async () => { + const outage = resourceBody(await buildObjectSchemaResource(inOutage(), 'acct')); + const miss = resourceBody(await buildObjectSchemaResource(withMiss(), 'acct')); + + expect(outage).not.toEqual(miss); + expect([outage.code, outage.status]).toEqual(['SERVICE_UNAVAILABLE', 503]); + expect([miss.code, miss.status]).toEqual(['RESOURCE_NOT_FOUND', 404]); + }); + + it('probes the object type by name when the resolver came back empty', async () => { + const svc = inOutage(); + await buildObjectSchemaResource(svc, 'acct'); + + expect((svc as AnyRecord).getObject).toHaveBeenCalledWith('acct'); + expect((svc as AnyRecord).getDiagnosed).toHaveBeenCalledWith('object', 'acct'); + }); + + it('a service that predates getDiagnosed behaves exactly as it did', async () => { + const missing = legacy(); + const body = resourceBody(await buildObjectSchemaResource(missing, 'acct')); + + expect(body.error).toBe('Object "acct" not found'); + expect((missing as AnyRecord).getObject).toHaveBeenCalledWith('acct'); + + const present = legacy(OBJECT); + expect(resourceBody(await buildObjectSchemaResource(present, 'acct'))).toMatchObject({ + name: 'acct', + label: 'Account', + }); + }); +}); diff --git a/packages/mcp/src/mcp-server-runtime.ts b/packages/mcp/src/mcp-server-runtime.ts index 852fb4b925..21280beb43 100644 --- a/packages/mcp/src/mcp-server-runtime.ts +++ b/packages/mcp/src/mcp-server-runtime.ts @@ -68,6 +68,308 @@ const DESTRUCTIVE_TOOLS = new Set([ 'delete_field', ]); +// ── Metadata outage vs. metadata miss (#6055, ADR-0110 D3) ─────────────────── + +/** + * [#6055] The classification this file gives "the metadata read did not + * happen", and the classification it gives "the read happened and found + * nothing". Both are the standard catalog's own codes for their status + * (`HttpStatusErrorCodeMap[503]` / `[404]`, ADR-0112) — the same spelling the + * `sys_metadata` half of this family already emits (#5532 / #5843 / #5705), not + * a vocabulary invented for MCP. + * + * There is no HTTP status on this surface: MCP answers `prompts/get` with a + * `GetPromptResult` and `resources/read` with a `ReadResourceResult`, and + * neither carries an error envelope (only `CallToolResult` has `isError`). So + * the code travels in the payload the surface already had — text for a prompt, + * the JSON body for a resource — and that is the strongest discriminator this + * transport offers. See the PR body for why the channel was not changed. + */ +const METADATA_UNAVAILABLE_CODE = 'SERVICE_UNAVAILABLE'; +const METADATA_MISS_CODE = 'RESOURCE_NOT_FOUND'; + +/** + * [#6055] The sentence for "a read that would have decided this did not + * happen", modelled on `METADATA_STORE_UNAVAILABLE_MESSAGE` (#5532) — + * *whether it exists is unknown*, plus the retry advice — with two deliberate + * differences: + * + * 1. It names the **metadata service**, not "the metadata store". The verdict + * behind it is `getDiagnosed`'s `degraded`, whose meaning is "at least one + * LOADER threw and nothing answered this item" — a loader-set fact, not a + * single-store one. PR #6051 records that distinction explicitly (it is why + * `degraded` did not copy #5897's `storeUnavailable` spelling), so echoing + * "store" here would import the narrower claim. + * 2. It states what the caller is NOT getting. This surface is fail-CLOSED + * both before and after this change — the defect was never that an outage + * widened access, only that it was **described** as an author's decision — + * and saying so keeps the next reader from "restoring" a body here. + */ +function metadataUnavailableSentence(subject: string, withheld: string): string { + return ( + `The metadata service could not be read, so whether ${subject} exists is unknown. ` + + `No ${withheld} is being served for this call. ` + + 'Retry once the metadata service is reachable.' + ); +} + +/** What {@link diagnosedGet} and {@link diagnoseEmptyRead} report. */ +interface DiagnosedRead { + data: unknown; + degraded: boolean; + errors: string[]; +} + +/** + * [#6055] Read one metadata item, keeping the ADR-0110 D3 verdict instead of + * flattening an outage into the same `undefined` a never-declared name + * produces. + * + * `IMetadataService.getDiagnosed` is **optional** (#5840): implementations that + * predate it cannot report the distinction at all, so a service without it is + * read exactly as before and reports nothing degraded — which is precisely what + * it could express. Same probe, same fallback, as the two consumers PR #6051 + * landed (`metadata-protocol/src/protocol.ts`, `objectql/src/plugin.ts`). + * + * `getDiagnosed` is the diagnosed twin of `get` (registry-first, and + * `metadata-manager-get-diagnosed.test.ts` pins that `get()` and + * `getDiagnosed().data` agree on every case), so swapping it in for a `get` + * call site changes what the caller LEARNS, never what it resolves. + */ +async function diagnosedGet( + metadataService: IMetadataService, + type: string, + name: string, +): Promise { + if (typeof metadataService.getDiagnosed === 'function') { + const diagnosed = await metadataService.getDiagnosed(type, name); + return { + data: diagnosed?.data, + degraded: diagnosed?.degraded === true, + errors: Array.isArray(diagnosed?.errors) ? diagnosed.errors : [], + }; + } + return { data: await metadataService.get(type, name), degraded: false, errors: [] }; +} + +/** + * [#6055] The verdict for a lookup that did NOT go through `get` — used by the + * `object_schema` resource, whose resolver is `getObject(name)`. + * + * Deliberately a **verdict-only probe run after the empty answer**, rather than + * swapping `getObject` out for `getDiagnosed('object', name)`. `getObject` is + * its own member of `IMetadataService` with no documented equivalence to + * `get('object', name)`, and the equivalence does not hold in general: + * `MetadataManager.getObject` delegates to `get('object', name)`, but + * `MetadataFacade.getObject` (objectql) returns `registry.getObject(name)` — a + * different shape from its own `get()`. Presuming the equivalence at a consumer + * is exactly the private dialect Prime Directive #12 forbids, so the resolver + * is left untouched and only the *question* "could this answer be trusted as + * complete?" is asked of the contract member that is declared to answer it. + * + * Consequences of that choice, both acceptable and both deliberate: + * - one extra read on the MISS path only (never on a hit, never on success); + * - on a host that implements `getDiagnosed` *and* a `getObject` resolving + * somewhere else, a degraded loader set makes this answer "unknown" for an + * object that is genuinely absent. That is the conservative direction — it + * withholds, it never admits — and no such host exists today + * (`MetadataManager` is the only `getDiagnosed` implementation on `main`). + */ +async function diagnoseEmptyRead( + metadataService: IMetadataService, + type: string, + name: string, +): Promise<{ degraded: boolean; errors: string[] }> { + if (typeof metadataService.getDiagnosed !== 'function') { + return { degraded: false, errors: [] }; + } + const diagnosed = await metadataService.getDiagnosed(type, name); + return { + degraded: diagnosed?.degraded === true, + errors: Array.isArray(diagnosed?.errors) ? diagnosed.errors : [], + }; +} + +/** + * What MCP's `prompts/get` answers with — the text-content subset of the SDK's + * `GetPromptResult` this surface produces. + * + * The index signature is not decoration: the SDK's own result types are + * `{ [x: string]: unknown; … }` (protocol passthrough plus `_meta`), and a + * named interface without it is rejected by `registerPrompt`'s callback + * signature. Narrower than `GetPromptResult` on the `content` union so the pin + * tests can read `.text` without narrowing at every assertion. + */ +export interface AgentPromptResult { + [key: string]: unknown; + messages: Array<{ role: 'user' | 'assistant'; content: { type: 'text'; text: string } }>; +} + +/** An `Error: …` answer on the prompt surface — this file's existing shape. */ +function promptError(text: string): AgentPromptResult { + return { messages: [{ role: 'user' as const, content: { type: 'text' as const, text } }] }; +} + +/** + * [#6055] Resolve the `agent_prompt` answer for one call. + * + * Exported so the three states below can be pinned directly: the handler is + * registered on a private `McpServer`, and driving it over a transport would + * test the SDK rather than this decision. + * + * The three states, and why each answers what it does: + * + * - **present** — the agent's instructions plus any UI context. Unchanged. + * - **genuinely absent** — `Error: Agent "X" not found`, byte-identical to + * before. A miss is a real fact about what the author declared and this + * surface was always right to state it. + * - **degraded** — an honest {@link METADATA_UNAVAILABLE_CODE} sentence. + * Before #6055 this case produced the *not found* text: an availability + * failure reported to an MCP client as a declaration fact, the ADR-0110 D3 + * shape. It never widened access (no instructions were served either way), + * so this is a diagnosis fix and MUST stay one — a body served here would be + * a security regression, not a nicety. + * + * Order matters and is the same narrowing PR #6051 applied to `getMetaItem`: + * `degraded` only decides the answer once the read has resolved NOTHING, i.e. + * only when the answer would otherwise have been the unfounded "not found". + * A read that answered a body is served as it always was. + */ +export async function buildAgentPromptResult( + metadataService: IMetadataService, + args: { agentName?: unknown; objectName?: unknown; recordId?: unknown; viewName?: unknown }, + logger?: Logger, +): Promise { + const agentName = String(args.agentName ?? ''); + if (!agentName) { + return promptError('Error: agentName argument is required'); + } + + const read = await diagnosedGet(metadataService, 'agent', agentName); + + if (read.data === undefined || read.data === null) { + if (read.degraded) { + logger?.warn( + '[MCP] agent prompt refused — the metadata service could not be read, so whether this agent ' + + 'exists is unknown. The caller was told SERVICE_UNAVAILABLE and no instructions were served ' + + '(unchanged: nothing is served on a miss either). ' + + 'Fix: check the loaders behind the metadata service (datasource connection, credentials, table).', + { agentName, errors: read.errors }, + ); + return promptError( + `Error: ${METADATA_UNAVAILABLE_CODE} — ` + + metadataUnavailableSentence(`agent "${agentName}"`, 'agent instruction'), + ); + } + return promptError(`Error: Agent "${agentName}" not found`); + } + + const agent = read.data as Agent; + + // Build system prompt from agent instructions + context + const parts: string[] = []; + parts.push(agent.instructions ?? ''); + + const contextHints: string[] = []; + if (args.objectName) contextHints.push(`Current object: ${args.objectName}`); + if (args.recordId) contextHints.push(`Selected record ID: ${args.recordId}`); + if (args.viewName) contextHints.push(`Current view: ${args.viewName}`); + if (contextHints.length > 0) { + parts.push('\n--- Current Context ---\n' + contextHints.join('\n')); + } + + return { + messages: [{ + role: 'assistant' as const, + content: { type: 'text' as const, text: parts.join('\n') }, + }], + }; +} + +/** + * What MCP's `resources/read` answers with — the text-content subset of the + * SDK's `ReadResourceResult`. Carries an index signature for the same reason + * {@link AgentPromptResult} does. + */ +export interface ObjectSchemaResourceResult { + [key: string]: unknown; + contents: Array<{ uri: string; mimeType: string; text: string }>; +} + +/** + * [#6055] Resolve the `objectstack://objects/{objectName}` answer for one call. + * + * The `agent_prompt` sibling of this fix, on the second occurrence of the same + * family in this package: `getObject()` returning `undefined` was read as + * `Object "X" not found` whether the object was never declared or every loader + * behind the metadata service was down. + * + * A resource body is JSON, so unlike the prompt surface it can carry the + * classification structurally — and both answers now do, so an MCP client can + * tell an outage from a miss without parsing prose. The `error` sentence of the + * miss is unchanged; `code`/`status` are additive. + * + * Why the resolver is still `getObject` and the verdict is a second, probe-only + * read: see {@link diagnoseEmptyRead}. + */ +export async function buildObjectSchemaResource( + metadataService: IMetadataService, + objectName: string, + logger?: Logger, +): Promise { + const uri = `objectstack://objects/${objectName}`; + const body = (payload: unknown): ObjectSchemaResourceResult => ({ + contents: [{ uri, mimeType: 'application/json', text: JSON.stringify(payload) }], + }); + + const objectDef = await metadataService.getObject(objectName); + + if (!objectDef) { + const { degraded, errors } = await diagnoseEmptyRead(metadataService, 'object', objectName); + if (degraded) { + logger?.warn( + '[MCP] object schema withheld — the metadata service could not be read, so whether this object ' + + 'exists is unknown. The caller was told SERVICE_UNAVAILABLE and no schema was served ' + + '(unchanged: nothing is served on a miss either). ' + + 'Fix: check the loaders behind the metadata service (datasource connection, credentials, table).', + { objectName, errors }, + ); + return body({ + error: metadataUnavailableSentence(`object "${objectName}"`, 'schema'), + code: METADATA_UNAVAILABLE_CODE, + status: 503, + }); + } + return body({ + error: `Object "${objectName}" not found`, + code: METADATA_MISS_CODE, + status: 404, + }); + } + + const def = objectDef as ObjectDef; + const fields = def.fields ?? {}; + const fieldSummary = Object.entries(fields).map(([key, f]) => ({ + name: key, + type: f.type, + label: f.label ?? key, + required: f.required ?? false, + })); + + return { + contents: [{ + uri, + mimeType: 'application/json', + text: JSON.stringify({ + name: def.name, + label: def.label ?? def.name, + fields: fieldSummary, + enableFeatures: def.enable ?? {}, + }, null, 2), + }], + }; +} + /** * MCPServerRuntime — Bridges ObjectStack kernel services to the Model Context Protocol. * @@ -291,42 +593,10 @@ export class MCPServerRuntime { description: 'Get the full schema of a specific data object including fields and features', mimeType: 'application/json', }, - async (_uri, variables) => { - const objectName = String(variables.objectName); - const objectDef = await metadataService.getObject(objectName); - - if (!objectDef) { - return { - contents: [{ - uri: `objectstack://objects/${objectName}`, - mimeType: 'application/json', - text: JSON.stringify({ error: `Object "${objectName}" not found` }), - }], - }; - } - - const def = objectDef as ObjectDef; - const fields = def.fields ?? {}; - const fieldSummary = Object.entries(fields).map(([key, f]) => ({ - name: key, - type: f.type, - label: f.label ?? key, - required: f.required ?? false, - })); - - return { - contents: [{ - uri: `objectstack://objects/${objectName}`, - mimeType: 'application/json', - text: JSON.stringify({ - name: def.name, - label: def.label ?? def.name, - fields: fieldSummary, - enableFeatures: def.enable ?? {}, - }, null, 2), - }], - }; - }, + async (_uri, variables) => + // [#6055] Outage vs. miss lives in the builder — see + // {@link buildObjectSchemaResource}. + buildObjectSchemaResource(metadataService, String(variables.objectName), logger), ); resourceCount++; @@ -448,48 +718,10 @@ export class MCPServerRuntime { viewName: z.string().optional().describe('Current view name'), }, }, - async (args) => { - const agentName = String(args.agentName ?? ''); - if (!agentName) { - return { - messages: [{ - role: 'user' as const, - content: { type: 'text' as const, text: 'Error: agentName argument is required' }, - }], - }; - } - - const raw = await metadataService.get('agent', agentName); - if (!raw) { - return { - messages: [{ - role: 'user' as const, - content: { type: 'text' as const, text: `Error: Agent "${agentName}" not found` }, - }], - }; - } - - const agent = raw as Agent; - - // Build system prompt from agent instructions + context - const parts: string[] = []; - parts.push(agent.instructions ?? ''); - - const contextHints: string[] = []; - if (args.objectName) contextHints.push(`Current object: ${args.objectName}`); - if (args.recordId) contextHints.push(`Selected record ID: ${args.recordId}`); - if (args.viewName) contextHints.push(`Current view: ${args.viewName}`); - if (contextHints.length > 0) { - parts.push('\n--- Current Context ---\n' + contextHints.join('\n')); - } - - return { - messages: [{ - role: 'assistant' as const, - content: { type: 'text' as const, text: parts.join('\n') }, - }], - }; - }, + async (args) => + // [#6055] Outage vs. miss lives in the builder — see + // {@link buildAgentPromptResult}. + buildAgentPromptResult(metadataService, args, logger), ); logger?.info('[MCP] Agent prompts bridged');