|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#28782] The MCP `create_record` tool answers a record the engine STORED but |
| 5 | + * whose parent roll-up it could not recompute with the create it is — never |
| 6 | + * the refusal, which told an agent its create failed and invited it to create |
| 7 | + * the record again. |
| 8 | + * |
| 9 | + * ## The defect, measured on `origin/main` (`c9a9981da`) before this file |
| 10 | + * |
| 11 | + * The engine reports that case as `ERR_SUMMARY_RECOMPUTE` ("the triggering |
| 12 | + * records WERE written", carrying `written`). #28781 taught the REST and |
| 13 | + * dispatcher single-record doors to answer the write's success; both MCP |
| 14 | + * bridges still let the refusal reach the tool, which rendered it as an |
| 15 | + * `isError` result: |
| 16 | + * |
| 17 | + * - the runtime bridge (`callData('create')` → the protocol's `createData`, |
| 18 | + * which attaches the earned answer since #28781) — the tool never read it; |
| 19 | + * - the stdio bridge (`createStdioDataBridge`, `engine.insert` direct) built |
| 20 | + * no answer at all. |
| 21 | + * |
| 22 | + * ## What is pinned |
| 23 | + * |
| 24 | + * - over the HTTP `/mcp` transport, `create_record` answers the bridge's |
| 25 | + * success body for both bridges, and the stdio bridge stores the row once; |
| 26 | + * - a refusal BEFORE the write is still an `isError` result (ruling 3); |
| 27 | + * - the stale column is logged once, at `error`. |
| 28 | + * |
| 29 | + * The engine is a double whose `insert` stores the row and then throws the |
| 30 | + * refusal as objectql's `SummaryRecomputeError` does (this package has no SQL |
| 31 | + * driver); the real engine's throw is pinned by #28781's REST suite. |
| 32 | + */ |
| 33 | + |
| 34 | +import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; |
| 35 | +import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts'; |
| 36 | +import type { ExecutionContext } from '@objectstack/spec/kernel'; |
| 37 | +import { withEarnedAnswer } from '@objectstack/types'; |
| 38 | +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core'; |
| 39 | + |
| 40 | +import { MCPServerRuntime } from './mcp-server-runtime.js'; |
| 41 | +import { createStdioDataBridge } from './stdio-data-bridge.js'; |
| 42 | +import type { McpDataBridge } from './mcp-http-tools.js'; |
| 43 | + |
| 44 | +const CHILD = { |
| 45 | + name: 'qsf_child', |
| 46 | + label: 'Child', |
| 47 | + fields: { |
| 48 | + id: { name: 'id', type: 'text' }, |
| 49 | + name: { name: 'name', type: 'text' }, |
| 50 | + amount: { name: 'amount', type: 'number' }, |
| 51 | + }, |
| 52 | + enable: {}, |
| 53 | +}; |
| 54 | + |
| 55 | +/** objectql's `SummaryRecomputeError`, as a write's caller reads it: by `code`, carrying `written`. */ |
| 56 | +function staleRefusal(written: unknown): Error { |
| 57 | + return Object.assign(new Error('Failed to recompute 1 roll-up summary value(s) after write; the triggering records WERE written.'), { |
| 58 | + code: 'ERR_SUMMARY_RECOMPUTE', |
| 59 | + written, |
| 60 | + failures: [{ childObject: 'qsf_child', parentObject: 'qsf_parent', parentId: 'p1', field: 'probe_sf', error: new Error('sum over text') }], |
| 61 | + }); |
| 62 | +} |
| 63 | + |
| 64 | +/** An engine whose `insert` stores the row, then reports the stale roll-up — or refuses before storing anything. */ |
| 65 | +function engineDouble(stored: Record<string, unknown>[]) { |
| 66 | + return { |
| 67 | + find: vi.fn(async () => []), |
| 68 | + findOne: vi.fn(async (object: string, query?: EngineFindOneQueryInput) => { assertEngineFindOnePredicate(object, query); return null; }), |
| 69 | + insert: vi.fn(async (_o: string, data: Record<string, unknown>) => { |
| 70 | + if (data.name === 'refused') { |
| 71 | + throw Object.assign(new Error('name is not allowed'), { code: 'VALIDATION_FAILED', status: 400 }); |
| 72 | + } |
| 73 | + const row = { id: `n${stored.length + 1}`, ...data }; |
| 74 | + stored.push(row); |
| 75 | + throw staleRefusal(row); |
| 76 | + }), |
| 77 | + update: vi.fn(async (_o: string, data: Record<string, unknown>, options?: any) => { assertEngineUpdateDispatch(data, options); return null; }), |
| 78 | + delete: vi.fn(async (_o: string, options?: any) => { assertEngineDeleteDispatch(options); return true; }), |
| 79 | + count: vi.fn(async () => 0), |
| 80 | + } as unknown as IDataEngine; |
| 81 | +} |
| 82 | + |
| 83 | +function metadataDouble(): IMetadataService { |
| 84 | + return { |
| 85 | + listObjects: vi.fn(async () => [CHILD]), |
| 86 | + getObject: vi.fn(async () => CHILD), |
| 87 | + } as unknown as IMetadataService; |
| 88 | +} |
| 89 | + |
| 90 | +let nextId = 1; |
| 91 | +async function createRecord(bridge: McpDataBridge, data: Record<string, unknown>) { |
| 92 | + const body = { jsonrpc: '2.0', id: nextId++, method: 'tools/call', params: { name: 'create_record', arguments: { objectName: 'qsf_child', data } } }; |
| 93 | + const runtime = new MCPServerRuntime({ name: 'pin', version: '0.0.0' }); |
| 94 | + const res = await runtime.handleHttpRequest( |
| 95 | + new Request('http://localhost/api/v1/mcp', { |
| 96 | + method: 'POST', |
| 97 | + headers: { 'content-type': 'application/json', accept: 'application/json, text/event-stream' }, |
| 98 | + body: JSON.stringify(body), |
| 99 | + }), |
| 100 | + { bridge: bridge as any, parsedBody: body }, |
| 101 | + ); |
| 102 | + expect(res.status).toBe(200); |
| 103 | + const json = (await res.json()) as { result?: any; error?: unknown }; |
| 104 | + expect(json.error, JSON.stringify(json.error)).toBeUndefined(); |
| 105 | + return json.result as { content: Array<{ text: string }>; isError?: boolean }; |
| 106 | +} |
| 107 | + |
| 108 | +let errorLog: ReturnType<typeof vi.spyOn>; |
| 109 | +beforeAll(() => { errorLog = vi.spyOn(console, 'error').mockImplementation(() => {}); }); |
| 110 | +afterAll(() => { errorLog.mockRestore(); }); |
| 111 | + |
| 112 | +describe('[#28782] MCP create_record answers a record stored under a failing roll-up as the create it is', () => { |
| 113 | + it('the stdio bridge: the success body, the row stored once', async () => { |
| 114 | + const stored: Record<string, unknown>[] = []; |
| 115 | + const engine = engineDouble(stored); |
| 116 | + const bridge = createStdioDataBridge({ |
| 117 | + engine, |
| 118 | + metadataService: metadataDouble(), |
| 119 | + resolvePrincipal: async () => ({ userId: 'u1' }) as unknown as ExecutionContext, |
| 120 | + }); |
| 121 | + const result = await createRecord(bridge, { name: 'mcp1', amount: 5 }); |
| 122 | + expect(result.isError, result.content[0]?.text).toBeFalsy(); |
| 123 | + expect(JSON.parse(result.content[0]!.text)).toEqual({ object: 'qsf_child', id: 'n1', record: { id: 'n1', name: 'mcp1', amount: 5 } }); |
| 124 | + expect(engine.insert).toHaveBeenCalledTimes(1); |
| 125 | + expect(stored).toHaveLength(1); |
| 126 | + }); |
| 127 | + |
| 128 | + it('the runtime bridge: the answer the protocol attached to the refusal', async () => { |
| 129 | + const answer = { object: 'qsf_child', id: 'r1', record: { id: 'r1', name: 'mcp2', amount: 6 } }; |
| 130 | + const bridge = { |
| 131 | + async listObjects() { return [{ name: 'qsf_child', label: 'Child', fieldCount: 3 }]; }, |
| 132 | + async describeObject() { return { name: 'qsf_child', fields: [] }; }, |
| 133 | + async query(object: string) { return { object, records: [], total: 0 }; }, |
| 134 | + async get() { return null; }, |
| 135 | + async create() { throw withEarnedAnswer(staleRefusal(answer.record) as any, answer); }, |
| 136 | + async update() { return null; }, |
| 137 | + async remove() { return null; }, |
| 138 | + } as unknown as McpDataBridge; |
| 139 | + const result = await createRecord(bridge, { name: 'mcp2', amount: 6 }); |
| 140 | + expect(result.isError, result.content[0]?.text).toBeFalsy(); |
| 141 | + expect(JSON.parse(result.content[0]!.text)).toEqual(answer); |
| 142 | + }); |
| 143 | + |
| 144 | + it('a refusal before the write is still an error result, and nothing is stored', async () => { |
| 145 | + const stored: Record<string, unknown>[] = []; |
| 146 | + const bridge = createStdioDataBridge({ |
| 147 | + engine: engineDouble(stored), |
| 148 | + metadataService: metadataDouble(), |
| 149 | + resolvePrincipal: async () => ({ userId: 'u1' }) as unknown as ExecutionContext, |
| 150 | + }); |
| 151 | + const result = await createRecord(bridge, { name: 'refused' }); |
| 152 | + expect(result.isError).toBe(true); |
| 153 | + expect(result.content[0]!.text).toContain('VALIDATION_FAILED'); |
| 154 | + expect(stored).toEqual([]); |
| 155 | + }); |
| 156 | + |
| 157 | + it('the stale column is logged once, at error', () => { |
| 158 | + const lines = errorLog.mock.calls.map((c: unknown[]) => String(c[0])).filter((l: string) => l.includes('could not be recomputed')); |
| 159 | + expect(lines).toHaveLength(1); |
| 160 | + expect(lines[0]).toContain('"qsf_parent.probe_sf"'); |
| 161 | + }); |
| 162 | +}); |
0 commit comments