|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#6055, ADR-0110 D3 — MCP side] A metadata plane that could not be READ is |
| 5 | + * not an agent (or an object) that nobody declared. |
| 6 | + * |
| 7 | + * --------------------------------------------------------------------------- |
| 8 | + * The defect |
| 9 | + * --------------------------------------------------------------------------- |
| 10 | + * `mcp-server-runtime.ts` resolved the `agent_prompt` body through |
| 11 | + * `metadataService.get('agent', name)` and answered its `undefined` with |
| 12 | + * `Error: Agent "X" not found`. `MetadataManager.get()` answers an unreachable |
| 13 | + * loader chain with exactly the `undefined` a never-declared name produces |
| 14 | + * (#5840), so during a metadata outage an MCP client was told, positively, what |
| 15 | + * the author had declared — from a read that never happened. |
| 16 | + * |
| 17 | + * The same shape sat one bridge over in the same file: the |
| 18 | + * `objectstack://objects/{objectName}` resource answered `getObject()`'s |
| 19 | + * `undefined` with `Object "X" not found`. |
| 20 | + * |
| 21 | + * Both were **fail-closed** — no instructions and no schema were served either |
| 22 | + * way — so this is a diagnosis defect, not a security one, and the fix must |
| 23 | + * keep it that way. Every degraded case below therefore asserts BOTH halves: |
| 24 | + * the answer is correctly classified, AND nothing was served. |
| 25 | + * |
| 26 | + * --------------------------------------------------------------------------- |
| 27 | + * Why the assertions are not `toThrow()`, and what stands in for an envelope |
| 28 | + * --------------------------------------------------------------------------- |
| 29 | + * Neither surface throws, before or after: MCP answers `prompts/get` with a |
| 30 | + * `GetPromptResult` and `resources/read` with a `ReadResourceResult`, and |
| 31 | + * neither type carries an error envelope (only `CallToolResult` has `isError`). |
| 32 | + * There is no ADR-0112 `code`+`status` on the wire to pin, so the strongest |
| 33 | + * available discriminator is used instead, per surface: |
| 34 | + * |
| 35 | + * - the RESOURCE body is JSON, so it carries `code`/`status` structurally and |
| 36 | + * both answers are pinned on them (`SERVICE_UNAVAILABLE`/503 vs |
| 37 | + * `RESOURCE_NOT_FOUND`/404); |
| 38 | + * - the PROMPT body is plain text, so the classification travels in the text |
| 39 | + * and is pinned as: carries `SERVICE_UNAVAILABLE`, says "unknown", and does |
| 40 | + * NOT say "not found". |
| 41 | + * |
| 42 | + * On top of that, every pair is pinned as a pair: the outage answer and the |
| 43 | + * miss answer must not be equal. That is the fact the defect was — before the |
| 44 | + * fix the two were byte-identical — and it is the one assertion that cannot be |
| 45 | + * satisfied by a mis-worded improvement. |
| 46 | + * |
| 47 | + * --------------------------------------------------------------------------- |
| 48 | + * Reverse verification, direction predicted BEFORE running |
| 49 | + * --------------------------------------------------------------------------- |
| 50 | + * Ordinary red, taken on this consumer. These doubles feed `getDiagnosed`'s |
| 51 | + * return contract directly, so reverting the producer (`MetadataManager`) |
| 52 | + * cannot move this file — only restoring the pre-#6055 reads here can. The |
| 53 | + * reversion is defined as: `agent_prompt` back to |
| 54 | + * `await metadataService.get('agent', name)` with the single `if (!raw)`, and |
| 55 | + * the resource back to `getObject()` with the bare |
| 56 | + * `{ error: 'Object "X" not found' }` body. |
| 57 | + * |
| 58 | + * Predicted, written down before running: **8 red / 9 green**, split |
| 59 | + * 4 red / 6 green on the prompt and 4 red / 3 green on the resource. |
| 60 | + * |
| 61 | + * ⚠️ One case is predicted GREEN in BOTH directions, and that is the point of |
| 62 | + * it rather than a gap: *"DEGRADED: access is still refused"*. The pre-fix code |
| 63 | + * served no instructions during an outage either — it was fail-closed and |
| 64 | + * merely mis-described — so an assertion that pins the affordance CANNOT go red |
| 65 | + * on this fix's reversion. It is an invariant pin, not coverage of the change, |
| 66 | + * and it is what would go red if a future "fix" here started serving a body. |
| 67 | + * Reporting it as part of the red count would be a fabricated number; the |
| 68 | + * measured result is recorded in the PR body as it came out. |
| 69 | + * |
| 70 | + * The doubles declare metadata reads only — no engine write verb — so there is |
| 71 | + * no `delete`/`update` dispatch for `check:engine-double-contract` to scan and |
| 72 | + * no guard to hand-mirror. |
| 73 | + */ |
| 74 | + |
| 75 | +import { describe, it, expect, vi } from 'vitest'; |
| 76 | +import type { IMetadataService } from '@objectstack/spec/contracts'; |
| 77 | +import { |
| 78 | + buildAgentPromptResult, |
| 79 | + buildObjectSchemaResource, |
| 80 | +} from './mcp-server-runtime.js'; |
| 81 | + |
| 82 | +type AnyRecord = Record<string, any>; |
| 83 | + |
| 84 | +const LOADER_FAILURE = 'database: connect ECONNREFUSED 10.0.0.5:5432'; |
| 85 | + |
| 86 | +const AGENT = { name: 'data_chat', instructions: 'You answer questions about the data.' }; |
| 87 | +const OBJECT = { name: 'acct', label: 'Account', fields: { title: { type: 'text' } } }; |
| 88 | + |
| 89 | +function makeLogger() { |
| 90 | + return { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() } as AnyRecord; |
| 91 | +} |
| 92 | + |
| 93 | +/** |
| 94 | + * Build a metadata-service double. |
| 95 | + * |
| 96 | + * Every REQUIRED member of `IMetadataService` is present and throws, so a code |
| 97 | + * path that reaches one this fix should not touch fails loudly instead of |
| 98 | + * resolving `undefined` and looking like the very absence under test. |
| 99 | + */ |
| 100 | +function makeService(overrides: AnyRecord): IMetadataService { |
| 101 | + const unexpected = (member: string) => async () => { |
| 102 | + throw new Error(`double: ${member}() should not be called by this surface`); |
| 103 | + }; |
| 104 | + return { |
| 105 | + register: unexpected('register'), |
| 106 | + get: unexpected('get'), |
| 107 | + list: unexpected('list'), |
| 108 | + unregister: unexpected('unregister'), |
| 109 | + exists: unexpected('exists'), |
| 110 | + listNames: unexpected('listNames'), |
| 111 | + getObject: unexpected('getObject'), |
| 112 | + listObjects: unexpected('listObjects'), |
| 113 | + ...overrides, |
| 114 | + } as unknown as IMetadataService; |
| 115 | +} |
| 116 | + |
| 117 | +/** Every loader behind the metadata service is down; nothing answered. */ |
| 118 | +const inOutage = (extra: AnyRecord = {}) => |
| 119 | + makeService({ |
| 120 | + get: vi.fn(async () => undefined), |
| 121 | + getObject: vi.fn(async () => undefined), |
| 122 | + getDiagnosed: vi.fn(async () => ({ data: undefined, degraded: true, errors: [LOADER_FAILURE] })), |
| 123 | + ...extra, |
| 124 | + }); |
| 125 | + |
| 126 | +/** The read HAPPENED and nobody declared this name. */ |
| 127 | +const withMiss = (extra: AnyRecord = {}) => |
| 128 | + makeService({ |
| 129 | + get: vi.fn(async () => undefined), |
| 130 | + getObject: vi.fn(async () => undefined), |
| 131 | + getDiagnosed: vi.fn(async () => ({ data: undefined, degraded: false, errors: [] })), |
| 132 | + ...extra, |
| 133 | + }); |
| 134 | + |
| 135 | +/** A healthy service holding `body`. */ |
| 136 | +const holding = (body: unknown, extra: AnyRecord = {}) => |
| 137 | + makeService({ |
| 138 | + get: vi.fn(async () => body), |
| 139 | + getObject: vi.fn(async () => body), |
| 140 | + getDiagnosed: vi.fn(async () => ({ data: body, degraded: false, errors: [] })), |
| 141 | + ...extra, |
| 142 | + }); |
| 143 | + |
| 144 | +/** |
| 145 | + * A service that predates `getDiagnosed` (#5840 declared it OPTIONAL). It |
| 146 | + * cannot report the distinction, so this surface must degrade to exactly what |
| 147 | + * it did before — never probe a member that is not there, never throw. |
| 148 | + */ |
| 149 | +const legacy = (body?: unknown) => |
| 150 | + makeService({ |
| 151 | + get: vi.fn(async () => body), |
| 152 | + getObject: vi.fn(async () => body), |
| 153 | + }); |
| 154 | + |
| 155 | +const promptText = (r: { messages: Array<{ content: { text: string } }> }) => r.messages[0].content.text; |
| 156 | +const promptRole = (r: { messages: Array<{ role: string }> }) => r.messages[0].role; |
| 157 | +const resourceBody = (r: { contents: Array<{ text: string }> }) => JSON.parse(r.contents[0].text); |
| 158 | + |
| 159 | +// ───────────────────────────────────────────────────────────────────────────── |
| 160 | +// agent_prompt — the call site the issue names |
| 161 | +// ───────────────────────────────────────────────────────────────────────────── |
| 162 | + |
| 163 | +describe('agent_prompt — a metadata outage is not "Agent not found" (#6055)', () => { |
| 164 | + it('PRESENT: serves the agent instructions (unchanged)', async () => { |
| 165 | + const svc = holding(AGENT); |
| 166 | + const result = await buildAgentPromptResult(svc, { agentName: 'data_chat' }); |
| 167 | + |
| 168 | + expect(promptRole(result)).toBe('assistant'); |
| 169 | + expect(promptText(result)).toContain('You answer questions about the data.'); |
| 170 | + }); |
| 171 | + |
| 172 | + it('PRESENT: still folds the UI context hints in (unchanged)', async () => { |
| 173 | + const result = await buildAgentPromptResult(holding(AGENT), { |
| 174 | + agentName: 'data_chat', |
| 175 | + objectName: 'acct', |
| 176 | + recordId: 'r1', |
| 177 | + viewName: 'all', |
| 178 | + }); |
| 179 | + |
| 180 | + expect(promptText(result)).toContain('--- Current Context ---'); |
| 181 | + expect(promptText(result)).toContain('Current object: acct'); |
| 182 | + expect(promptText(result)).toContain('Selected record ID: r1'); |
| 183 | + expect(promptText(result)).toContain('Current view: all'); |
| 184 | + }); |
| 185 | + |
| 186 | + it('GENUINELY ABSENT: the not-found answer is preserved, byte for byte', async () => { |
| 187 | + const result = await buildAgentPromptResult(withMiss(), { agentName: 'data_chat' }); |
| 188 | + |
| 189 | + // Verbatim: a miss is a real fact about what the author declared, and this |
| 190 | + // surface was always right to state it. The fix must not reword it. |
| 191 | + expect(promptText(result)).toBe('Error: Agent "data_chat" not found'); |
| 192 | + }); |
| 193 | + |
| 194 | + it('DEGRADED: answers SERVICE_UNAVAILABLE, and never the not-found claim', async () => { |
| 195 | + const result = await buildAgentPromptResult(inOutage(), { agentName: 'data_chat' }); |
| 196 | + const text = promptText(result); |
| 197 | + |
| 198 | + expect(text).toContain('SERVICE_UNAVAILABLE'); |
| 199 | + expect(text).toContain('whether agent "data_chat" exists is unknown'); |
| 200 | + expect(text).not.toMatch(/not found/); |
| 201 | + }); |
| 202 | + |
| 203 | + it('DEGRADED: access is still refused — no instructions are served', async () => { |
| 204 | + const result = await buildAgentPromptResult(inOutage(), { agentName: 'data_chat' }); |
| 205 | + |
| 206 | + // Fail-closed, before and after. A body here would be a security |
| 207 | + // regression, not a nicety — the defect was the DESCRIPTION, never the |
| 208 | + // affordance. |
| 209 | + expect(promptRole(result)).toBe('user'); |
| 210 | + expect(promptText(result)).not.toContain(AGENT.instructions); |
| 211 | + }); |
| 212 | + |
| 213 | + it('the outage and the miss no longer collapse to the same answer', async () => { |
| 214 | + const outage = promptText(await buildAgentPromptResult(inOutage(), { agentName: 'data_chat' })); |
| 215 | + const miss = promptText(await buildAgentPromptResult(withMiss(), { agentName: 'data_chat' })); |
| 216 | + |
| 217 | + // Same surface, same agent name, same (absent) result — only the health of |
| 218 | + // the metadata plane differs. Before #6055 both produced |
| 219 | + // `Error: Agent "data_chat" not found`. |
| 220 | + expect(outage).not.toBe(miss); |
| 221 | + expect(miss).toMatch(/not found/); |
| 222 | + expect(outage).toMatch(/SERVICE_UNAVAILABLE/); |
| 223 | + }); |
| 224 | + |
| 225 | + it('reads through getDiagnosed, not get, when the service offers it', async () => { |
| 226 | + const svc = inOutage(); |
| 227 | + await buildAgentPromptResult(svc, { agentName: 'data_chat' }); |
| 228 | + |
| 229 | + // If `get` were still the read, the verdict would be unreachable and the |
| 230 | + // case above could only pass by accident. |
| 231 | + expect((svc as AnyRecord).getDiagnosed).toHaveBeenCalledWith('agent', 'data_chat'); |
| 232 | + expect((svc as AnyRecord).get).not.toHaveBeenCalled(); |
| 233 | + }); |
| 234 | + |
| 235 | + it('logs the outage once, with the consequence and the fix', async () => { |
| 236 | + const logger = makeLogger(); |
| 237 | + await buildAgentPromptResult(inOutage(), { agentName: 'data_chat' }, logger as any); |
| 238 | + |
| 239 | + expect(logger.warn).toHaveBeenCalledTimes(1); |
| 240 | + const [line, detail] = logger.warn.mock.calls[0]; |
| 241 | + expect(String(line)).toContain('no instructions were served'); |
| 242 | + expect(String(line)).toContain('Fix:'); |
| 243 | + expect(detail).toMatchObject({ agentName: 'data_chat', errors: [LOADER_FAILURE] }); |
| 244 | + // A miss is not a degradation and must not log at all. |
| 245 | + const quiet = makeLogger(); |
| 246 | + await buildAgentPromptResult(withMiss(), { agentName: 'data_chat' }, quiet as any); |
| 247 | + expect(quiet.warn).not.toHaveBeenCalled(); |
| 248 | + }); |
| 249 | + |
| 250 | + it('a service that predates getDiagnosed behaves exactly as it did', async () => { |
| 251 | + const missing = legacy(); |
| 252 | + expect(promptText(await buildAgentPromptResult(missing, { agentName: 'data_chat' }))) |
| 253 | + .toBe('Error: Agent "data_chat" not found'); |
| 254 | + expect((missing as AnyRecord).get).toHaveBeenCalledWith('agent', 'data_chat'); |
| 255 | + |
| 256 | + const present = legacy(AGENT); |
| 257 | + expect(promptText(await buildAgentPromptResult(present, { agentName: 'data_chat' }))) |
| 258 | + .toContain(AGENT.instructions); |
| 259 | + }); |
| 260 | + |
| 261 | + it('a missing agentName argument is still refused before any read', async () => { |
| 262 | + // `makeService`'s required members all throw, so reaching a read here fails |
| 263 | + // the test rather than passing quietly. |
| 264 | + const result = await buildAgentPromptResult(makeService({}), {}); |
| 265 | + expect(promptText(result)).toBe('Error: agentName argument is required'); |
| 266 | + }); |
| 267 | +}); |
| 268 | + |
| 269 | +// ───────────────────────────────────────────────────────────────────────────── |
| 270 | +// objectstack://objects/{objectName} — the same family, one bridge over |
| 271 | +// ───────────────────────────────────────────────────────────────────────────── |
| 272 | + |
| 273 | +describe('object_schema resource — a metadata outage is not "Object not found" (#6055)', () => { |
| 274 | + it('PRESENT: serves the object schema (unchanged)', async () => { |
| 275 | + const body = resourceBody(await buildObjectSchemaResource(holding(OBJECT), 'acct')); |
| 276 | + |
| 277 | + expect(body).toMatchObject({ name: 'acct', label: 'Account' }); |
| 278 | + expect(body.fields).toEqual([{ name: 'title', type: 'text', label: 'title', required: false }]); |
| 279 | + }); |
| 280 | + |
| 281 | + it('PRESENT: the hit path costs no second read', async () => { |
| 282 | + const svc = holding(OBJECT); |
| 283 | + await buildObjectSchemaResource(svc, 'acct'); |
| 284 | + |
| 285 | + // `getObject` stays the resolver (it is its own contract member, and |
| 286 | + // `MetadataFacade.getObject` is NOT `get('object', name)`); the diagnosed |
| 287 | + // read is a verdict probe on the MISS path only. |
| 288 | + expect((svc as AnyRecord).getObject).toHaveBeenCalledWith('acct'); |
| 289 | + expect((svc as AnyRecord).getDiagnosed).not.toHaveBeenCalled(); |
| 290 | + }); |
| 291 | + |
| 292 | + it('GENUINELY ABSENT: not-found, classified 404 / RESOURCE_NOT_FOUND', async () => { |
| 293 | + const body = resourceBody(await buildObjectSchemaResource(withMiss(), 'acct')); |
| 294 | + |
| 295 | + expect(body).toEqual({ |
| 296 | + error: 'Object "acct" not found', |
| 297 | + code: 'RESOURCE_NOT_FOUND', |
| 298 | + status: 404, |
| 299 | + }); |
| 300 | + }); |
| 301 | + |
| 302 | + it('DEGRADED: unavailable, classified 503 / SERVICE_UNAVAILABLE, no schema served', async () => { |
| 303 | + const body = resourceBody(await buildObjectSchemaResource(inOutage(), 'acct')); |
| 304 | + |
| 305 | + expect(body.code).toBe('SERVICE_UNAVAILABLE'); |
| 306 | + expect(body.status).toBe(503); |
| 307 | + expect(body.error).toContain('whether object "acct" exists is unknown'); |
| 308 | + expect(body.error).not.toMatch(/not found/); |
| 309 | + // Fail-closed: still no schema. |
| 310 | + expect(body.fields).toBeUndefined(); |
| 311 | + expect(body.name).toBeUndefined(); |
| 312 | + }); |
| 313 | + |
| 314 | + it('the outage and the miss no longer collapse to the same answer', async () => { |
| 315 | + const outage = resourceBody(await buildObjectSchemaResource(inOutage(), 'acct')); |
| 316 | + const miss = resourceBody(await buildObjectSchemaResource(withMiss(), 'acct')); |
| 317 | + |
| 318 | + expect(outage).not.toEqual(miss); |
| 319 | + expect([outage.code, outage.status]).toEqual(['SERVICE_UNAVAILABLE', 503]); |
| 320 | + expect([miss.code, miss.status]).toEqual(['RESOURCE_NOT_FOUND', 404]); |
| 321 | + }); |
| 322 | + |
| 323 | + it('probes the object type by name when the resolver came back empty', async () => { |
| 324 | + const svc = inOutage(); |
| 325 | + await buildObjectSchemaResource(svc, 'acct'); |
| 326 | + |
| 327 | + expect((svc as AnyRecord).getObject).toHaveBeenCalledWith('acct'); |
| 328 | + expect((svc as AnyRecord).getDiagnosed).toHaveBeenCalledWith('object', 'acct'); |
| 329 | + }); |
| 330 | + |
| 331 | + it('a service that predates getDiagnosed behaves exactly as it did', async () => { |
| 332 | + const missing = legacy(); |
| 333 | + const body = resourceBody(await buildObjectSchemaResource(missing, 'acct')); |
| 334 | + |
| 335 | + expect(body.error).toBe('Object "acct" not found'); |
| 336 | + expect((missing as AnyRecord).getObject).toHaveBeenCalledWith('acct'); |
| 337 | + |
| 338 | + const present = legacy(OBJECT); |
| 339 | + expect(resourceBody(await buildObjectSchemaResource(present, 'acct'))).toMatchObject({ |
| 340 | + name: 'acct', |
| 341 | + label: 'Account', |
| 342 | + }); |
| 343 | + }); |
| 344 | +}); |
0 commit comments