|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#6504, ADR-0110 D3 — MCP side, plural read] A metadata plane that could not |
| 5 | + * be fully READ is not an environment that declares fewer things. |
| 6 | + * |
| 7 | + * --------------------------------------------------------------------------- |
| 8 | + * The defect |
| 9 | + * --------------------------------------------------------------------------- |
| 10 | + * `MetadataManager` has computed a `degraded` verdict for every `list()` since |
| 11 | + * #5184 and spent it entirely on a cache TTL. The two consumers in this file |
| 12 | + * therefore read a short array during a loader outage with no way to ask why it |
| 13 | + * was short — the plural instance of the #5840 / PR #6051 shape #6055 closed on |
| 14 | + * the singular read. |
| 15 | + * |
| 16 | + * The two are NOT the same case, and PR #6051's discipline is to classify each |
| 17 | + * consumer rather than apply one rule, so they are pinned separately: |
| 18 | + * |
| 19 | + * - `objectstack://objects` **MIS-DESCRIBES**. It renders the listing as |
| 20 | + * `{ objects, totalCount }`, and during an outage `totalCount` is a |
| 21 | + * positive, numeric claim that is simply false. This is the surface where a |
| 22 | + * count is the strongest thing a read can wrongly say, and the fix withholds |
| 23 | + * the count while still serving the objects. |
| 24 | + * - the `agent_prompt` sibling **skill bridge** produces a SNAPSHOT. It |
| 25 | + * publishes no count and makes no completeness claim to any client; a |
| 26 | + * degraded read costs it silently-unregistered prompts. There is nobody on |
| 27 | + * the wire to tell, so the verdict goes to the operator at `warn`. |
| 28 | + * |
| 29 | + * --------------------------------------------------------------------------- |
| 30 | + * Why these doubles, and where the REAL loader failure is pinned |
| 31 | + * --------------------------------------------------------------------------- |
| 32 | + * Stated plainly rather than papered over. `packages/mcp` depends on |
| 33 | + * `@objectstack/spec`, `core`, `types` and `formula` — deliberately NOT on |
| 34 | + * `@objectstack/metadata` — so a real `MetadataManager` over a broken |
| 35 | + * `DatabaseLoader` cannot be constructed here, and adding that dependency to |
| 36 | + * drive a test would be a larger architectural change than the fix. |
| 37 | + * |
| 38 | + * The real failure is therefore driven where the loader actually lives: |
| 39 | + * `packages/metadata/src/metadata-manager-list-diagnosed.test.ts` fails a |
| 40 | + * `DatabaseLoader`'s driver for real, so `readListUncached()`'s `catch` is what |
| 41 | + * produces `degraded`, and asserts the exact record shape the doubles below |
| 42 | + * return. This file pins the other half of that chain — what each consumer DOES |
| 43 | + * with such a record — which is the same split #6055 used for `getDiagnosed`. |
| 44 | + * |
| 45 | + * --------------------------------------------------------------------------- |
| 46 | + * Reverse verification, direction predicted BEFORE running |
| 47 | + * --------------------------------------------------------------------------- |
| 48 | + * Ordinary red, taken on this consumer. These doubles feed `listDiagnosed`'s |
| 49 | + * return contract directly, so reverting the producer cannot move this file — |
| 50 | + * only restoring the pre-#6504 reads here can. |
| 51 | + * |
| 52 | + * The reversion is defined **behaviourally, not textually**: the degraded |
| 53 | + * branch of `buildObjectListResource` is removed so it answers |
| 54 | + * `{ objects, totalCount }` unconditionally, and the skill bridge's |
| 55 | + * incompleteness `warn` is removed. The extraction of the builder is kept. |
| 56 | + * Reverting the whole FILE to `origin/main` instead would delete the builder's |
| 57 | + * export, fail the import, and turn all eleven cases red — a result that |
| 58 | + * measures the extraction rather than the decision, and so proves nothing about |
| 59 | + * either. |
| 60 | + * |
| 61 | + * Predicted, written down before running: **5 red / 6 green**. |
| 62 | + * |
| 63 | + * The six predicted GREEN are invariant pins rather than gaps, and each is |
| 64 | + * green in BOTH directions on purpose: |
| 65 | + * - the two HEALTHY cases (resource + skill bridge) — this fix deliberately |
| 66 | + * leaves the healthy answer byte-identical, so a red there would report a |
| 67 | + * regression, not the fix; |
| 68 | + * - *"a degraded listing still serves the objects it could read"* — the |
| 69 | + * pre-fix code served them too. It is what would go red if a future change |
| 70 | + * here started withholding data instead of withholding the claim; |
| 71 | + * - *"the readable skills are still bridged"* — same shape, on the snapshot; |
| 72 | + * - the two *"a service without listDiagnosed"* cases — the optional-member |
| 73 | + * fallback, which by construction resolves to the pre-fix behaviour. |
| 74 | + * The measured result is recorded in the PR body as it came out. |
| 75 | + * |
| 76 | + * The doubles declare metadata reads only — no engine write verb — so there is |
| 77 | + * no `delete`/`update` dispatch for `check:engine-double-contract` to scan and |
| 78 | + * no guard to hand-mirror. |
| 79 | + */ |
| 80 | + |
| 81 | +import { describe, it, expect, vi } from 'vitest'; |
| 82 | +import type { IMetadataService, Logger } from '@objectstack/spec/contracts'; |
| 83 | +import { MCPServerRuntime, buildObjectListResource } from './mcp-server-runtime.js'; |
| 84 | + |
| 85 | +type AnyRecord = Record<string, any>; |
| 86 | + |
| 87 | +/** |
| 88 | + * A logger double that is BOTH a real `Logger` and a set of vitest mocks. |
| 89 | + * |
| 90 | + * Typed rather than left as a bare record on purpose: this package's tsconfig |
| 91 | + * excludes `*.test.ts`, so `pnpm typecheck` never reads this file and only the |
| 92 | + * TEST_DEBT ratchet does. An untyped double compiles to eight TS2345s that the |
| 93 | + * ledger's surplus would have absorbed in silence — which is the exact shape |
| 94 | + * #6376 exists to stop. |
| 95 | + */ |
| 96 | +type MockLogger = Logger & { |
| 97 | + debug: ReturnType<typeof vi.fn>; |
| 98 | + info: ReturnType<typeof vi.fn>; |
| 99 | + warn: ReturnType<typeof vi.fn>; |
| 100 | + error: ReturnType<typeof vi.fn>; |
| 101 | +}; |
| 102 | + |
| 103 | +const LOADER_FAILURE = 'database: connect ECONNREFUSED 10.0.0.5:5432'; |
| 104 | + |
| 105 | +/** What the loader that IS reachable holds — one object, one skill. */ |
| 106 | +const REACHABLE_OBJECT = { name: 'acct', label: 'Account', fields: { title: { type: 'text' } } }; |
| 107 | +const REACHABLE_SKILL = { |
| 108 | + name: 'case_management', |
| 109 | + label: 'Case Management', |
| 110 | + instructions: 'Handle the support case lifecycle.', |
| 111 | +}; |
| 112 | + |
| 113 | +function makeLogger(): MockLogger { |
| 114 | + return { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() } as unknown as MockLogger; |
| 115 | +} |
| 116 | + |
| 117 | +/** Everything logged at `warn`, joined — the snapshot cases read this. */ |
| 118 | +const warnLines = (logger: MockLogger): string => |
| 119 | + logger.warn.mock.calls.map((c: unknown[]) => String(c[0])).join('\n'); |
| 120 | + |
| 121 | +/** |
| 122 | + * Build a metadata-service double. |
| 123 | + * |
| 124 | + * Every REQUIRED member of `IMetadataService` is present and throws, so a code |
| 125 | + * path that reaches one this fix should not touch fails loudly instead of |
| 126 | + * resolving an empty array and looking like the very shortness under test. |
| 127 | + */ |
| 128 | +function makeService(overrides: AnyRecord): IMetadataService { |
| 129 | + const unexpected = (member: string) => async (): Promise<never> => { |
| 130 | + throw new Error(`double: ${member}() should not be called by this surface`); |
| 131 | + }; |
| 132 | + return { |
| 133 | + register: unexpected('register'), |
| 134 | + get: unexpected('get'), |
| 135 | + list: unexpected('list'), |
| 136 | + unregister: unexpected('unregister'), |
| 137 | + exists: unexpected('exists'), |
| 138 | + listNames: unexpected('listNames'), |
| 139 | + getObject: unexpected('getObject'), |
| 140 | + listObjects: unexpected('listObjects'), |
| 141 | + ...overrides, |
| 142 | + } as unknown as IMetadataService; |
| 143 | +} |
| 144 | + |
| 145 | +/** |
| 146 | + * One loader is down: the reachable ones answered, so `items` is a real |
| 147 | + * best-effort set and `degraded` says it is short. This is the record |
| 148 | + * `MetadataManager.listDiagnosed()` returns from a live `ECONNREFUSED`, pinned |
| 149 | + * in the metadata package. |
| 150 | + */ |
| 151 | +const listInOutage = (items: unknown[]) => |
| 152 | + makeService({ |
| 153 | + listObjects: vi.fn(async () => items), |
| 154 | + list: vi.fn(async () => items), |
| 155 | + listDiagnosed: vi.fn(async () => ({ items, degraded: true, errors: [LOADER_FAILURE] })), |
| 156 | + }); |
| 157 | + |
| 158 | +/** Every loader answered — the same items, and nothing degraded. */ |
| 159 | +const listComplete = (items: unknown[]) => |
| 160 | + makeService({ |
| 161 | + listObjects: vi.fn(async () => items), |
| 162 | + list: vi.fn(async () => items), |
| 163 | + listDiagnosed: vi.fn(async () => ({ items, degraded: false, errors: [] })), |
| 164 | + }); |
| 165 | + |
| 166 | +/** A service predating #6504: no `listDiagnosed` to probe. */ |
| 167 | +const listUndiagnosable = (items: unknown[]) => |
| 168 | + makeService({ |
| 169 | + listObjects: vi.fn(async () => items), |
| 170 | + list: vi.fn(async () => items), |
| 171 | + }); |
| 172 | + |
| 173 | +/** The parsed JSON body of the one-content resource answer. */ |
| 174 | +const bodyOf = async (result: { contents: Array<{ text: string }> }): Promise<AnyRecord> => |
| 175 | + JSON.parse(result.contents[0]!.text) as AnyRecord; |
| 176 | + |
| 177 | +describe('#6504 — `objectstack://objects` MIS-DESCRIBES: the count is withheld, the objects are not', () => { |
| 178 | + it('HEALTHY: `totalCount` is served exactly as before', async () => { |
| 179 | + const body = await bodyOf( |
| 180 | + await buildObjectListResource(listComplete([REACHABLE_OBJECT]), makeLogger()), |
| 181 | + ); |
| 182 | + |
| 183 | + expect(body.totalCount).toBe(1); |
| 184 | + expect(body.objects).toHaveLength(1); |
| 185 | + expect(body.objects[0].name).toBe('acct'); |
| 186 | + // No degradation vocabulary on a complete read. |
| 187 | + expect(body.partial).toBeUndefined(); |
| 188 | + expect(body.code).toBeUndefined(); |
| 189 | + }); |
| 190 | + |
| 191 | + it('DEGRADED: the count claim is ABSENT — a client asking for a total gets nothing, not a wrong number', async () => { |
| 192 | + const body = await bodyOf( |
| 193 | + await buildObjectListResource(listInOutage([REACHABLE_OBJECT]), makeLogger()), |
| 194 | + ); |
| 195 | + |
| 196 | + // The whole point. Before #6504 this said `totalCount: 1` about an |
| 197 | + // environment that declares more, and a machine consumer believed it. |
| 198 | + expect(body.totalCount).toBeUndefined(); |
| 199 | + expect('totalCount' in body).toBe(false); |
| 200 | + |
| 201 | + // What replaces it says only what is known, in a name that cannot be read |
| 202 | + // as a total. |
| 203 | + expect(body.partial).toBe(true); |
| 204 | + expect(body.returnedCount).toBe(1); |
| 205 | + }); |
| 206 | + |
| 207 | + it('DEGRADED: carries the ADR-0112-shaped envelope the sibling resource already uses', async () => { |
| 208 | + const body = await bodyOf( |
| 209 | + await buildObjectListResource(listInOutage([REACHABLE_OBJECT]), makeLogger()), |
| 210 | + ); |
| 211 | + |
| 212 | + expect(body.code).toBe('SERVICE_UNAVAILABLE'); |
| 213 | + expect(body.status).toBe(503); |
| 214 | + // The prose states the DIRECTION of the error — a floor, never an exact |
| 215 | + // number — and does not describe the listing as complete. |
| 216 | + expect(body.warning).toMatch(/INCOMPLETE/); |
| 217 | + expect(body.warning).toMatch(/at least/i); |
| 218 | + expect(body.warning).not.toMatch(/not found/i); |
| 219 | + }); |
| 220 | + |
| 221 | + it('the two answers are not equal — which is the fact the defect WAS', async () => { |
| 222 | + const outage = await bodyOf( |
| 223 | + await buildObjectListResource(listInOutage([REACHABLE_OBJECT]), makeLogger()), |
| 224 | + ); |
| 225 | + const complete = await bodyOf( |
| 226 | + await buildObjectListResource(listComplete([REACHABLE_OBJECT]), makeLogger()), |
| 227 | + ); |
| 228 | + |
| 229 | + // Same objects, same length — so nothing but the completeness claim moved, |
| 230 | + // and yet the two bodies are now distinguishable. |
| 231 | + expect(outage.objects).toEqual(complete.objects); |
| 232 | + expect(outage).not.toEqual(complete); |
| 233 | + }); |
| 234 | + |
| 235 | + it('DEGRADED: still serves the objects it could read — this is a diagnosis fix, not a withholding one', async () => { |
| 236 | + const body = await bodyOf( |
| 237 | + await buildObjectListResource(listInOutage([REACHABLE_OBJECT]), makeLogger()), |
| 238 | + ); |
| 239 | + |
| 240 | + // Kept deliberately free of any degradation assertion: this case pins the |
| 241 | + // AFFORDANCE, which the pre-#6504 code had too. It cannot go red on this |
| 242 | + // fix's reversion, and it is what would go red if a future change here |
| 243 | + // started withholding data instead of withholding the claim. |
| 244 | + expect(body.objects).toHaveLength(1); |
| 245 | + expect(body.objects[0].name).toBe('acct'); |
| 246 | + }); |
| 247 | + |
| 248 | + it('DEGRADED: the operator is told once, naming the loader that was lost', async () => { |
| 249 | + const logger = makeLogger(); |
| 250 | + await buildObjectListResource(listInOutage([REACHABLE_OBJECT]), logger); |
| 251 | + |
| 252 | + expect(warnLines(logger)).toMatch(/known-partial/); |
| 253 | + expect(logger.warn).toHaveBeenCalledTimes(1); |
| 254 | + expect(logger.warn.mock.calls[0][1].errors).toEqual([LOADER_FAILURE]); |
| 255 | + }); |
| 256 | + |
| 257 | + it('a service without `listDiagnosed` behaves exactly as before — the member is optional', async () => { |
| 258 | + const logger = makeLogger(); |
| 259 | + const body = await bodyOf( |
| 260 | + await buildObjectListResource(listUndiagnosable([REACHABLE_OBJECT]), logger), |
| 261 | + ); |
| 262 | + |
| 263 | + // An implementation that cannot report the distinction reports nothing |
| 264 | + // degraded, which is precisely what it could express. |
| 265 | + expect(body.totalCount).toBe(1); |
| 266 | + expect(body.partial).toBeUndefined(); |
| 267 | + expect(logger.warn).not.toHaveBeenCalled(); |
| 268 | + }); |
| 269 | +}); |
| 270 | + |
| 271 | +describe('#6504 — the skill bridge is a SNAPSHOT: the verdict goes to the operator', () => { |
| 272 | + const bridgeSkills = async (service: IMetadataService, logger: MockLogger): Promise<void> => { |
| 273 | + const runtime = new MCPServerRuntime({ name: 'list-outage', version: '0.0.0', logger }); |
| 274 | + await runtime.bridgePrompts(service); |
| 275 | + }; |
| 276 | + |
| 277 | + it('DEGRADED: says the prompt list is incomplete, and that the skills are MISSING rather than undeclared', async () => { |
| 278 | + const logger = makeLogger(); |
| 279 | + await bridgeSkills(listInOutage([REACHABLE_SKILL]), logger); |
| 280 | + |
| 281 | + const lines = warnLines(logger); |
| 282 | + expect(lines).toMatch(/INCOMPLETE/); |
| 283 | + expect(lines).toMatch(/missing, NOT undeclared/); |
| 284 | + // The consequence that makes it worth saying: the stdio snapshot outlives |
| 285 | + // the outage, so a healed loader does not fix the surface by itself. |
| 286 | + expect(lines).toMatch(/restarted/); |
| 287 | + }); |
| 288 | + |
| 289 | + it('DEGRADED: the readable skills are still bridged — the short surface is served, not refused', async () => { |
| 290 | + const logger = makeLogger(); |
| 291 | + await bridgeSkills(listInOutage([REACHABLE_SKILL]), logger); |
| 292 | + |
| 293 | + expect(logger.info.mock.calls.map((c: unknown[]) => String(c[0])).join('\n')) |
| 294 | + .toMatch(/Bridged 1 skill prompts/); |
| 295 | + }); |
| 296 | + |
| 297 | + it('HEALTHY: no incompleteness is announced', async () => { |
| 298 | + const logger = makeLogger(); |
| 299 | + await bridgeSkills(listComplete([REACHABLE_SKILL]), logger); |
| 300 | + |
| 301 | + expect(warnLines(logger)).not.toMatch(/INCOMPLETE/); |
| 302 | + }); |
| 303 | + |
| 304 | + it('a service without `listDiagnosed` announces nothing either — same optionality', async () => { |
| 305 | + const logger = makeLogger(); |
| 306 | + await bridgeSkills(listUndiagnosable([REACHABLE_SKILL]), logger); |
| 307 | + |
| 308 | + expect(warnLines(logger)).not.toMatch(/INCOMPLETE/); |
| 309 | + }); |
| 310 | +}); |
0 commit comments