|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#28844] The MCP `update_record` and `delete_record` tools answer a write the |
| 5 | + * engine STORED but whose parent roll-up it could not recompute with the |
| 6 | + * update or delete it is — never the refusal, which told an agent its write |
| 7 | + * failed and invited it to apply it again. |
| 8 | + * |
| 9 | + * ## The defect, measured on `origin/main` (`b95a856e3`) before this file |
| 10 | + * |
| 11 | + * The engine reports that case as `ERR_SUMMARY_RECOMPUTE` ("the triggering |
| 12 | + * records WERE written", carrying `written`). #28843 taught `create_record` |
| 13 | + * and the stdio bridge's `create` to read #28781's carrier; the update and |
| 14 | + * delete tools still rendered the refusal as an `isError` result on both |
| 15 | + * bridges: |
| 16 | + * |
| 17 | + * - the runtime bridge (`callData('update' | 'delete')` → the protocol's |
| 18 | + * `updateData` / `deleteData`, which attach the earned answer since #28781) |
| 19 | + * — the tools never read it; |
| 20 | + * - the stdio bridge (`createStdioDataBridge`, `engine.update` / |
| 21 | + * `engine.delete` direct) built no answer at all. |
| 22 | + * |
| 23 | + * ## What is pinned |
| 24 | + * |
| 25 | + * - over the HTTP `/mcp` transport, each tool answers the bridge's success |
| 26 | + * result for both bridges, and the stdio bridge applies the write once; |
| 27 | + * - a refusal BEFORE the write is still an `isError` result (ruling 4); |
| 28 | + * - the stale column is logged once, at `error`. |
| 29 | + * |
| 30 | + * The engine is a double whose `update` / `delete` apply the write and then |
| 31 | + * throw the refusal as objectql's `SummaryRecomputeError` does (this package |
| 32 | + * has no SQL driver); the real engine's throw is pinned by #28781's REST suite. |
| 33 | + */ |
| 34 | + |
| 35 | +import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; |
| 36 | +import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts'; |
| 37 | +import type { ExecutionContext } from '@objectstack/spec/kernel'; |
| 38 | +import { withEarnedAnswer } from '@objectstack/types'; |
| 39 | +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core'; |
| 40 | + |
| 41 | +import { MCPServerRuntime } from './mcp-server-runtime.js'; |
| 42 | +import { createStdioDataBridge } from './stdio-data-bridge.js'; |
| 43 | +import type { McpDataBridge } from './mcp-http-tools.js'; |
| 44 | + |
| 45 | +const CHILD = { |
| 46 | + name: 'qsf_child', |
| 47 | + label: 'Child', |
| 48 | + fields: { |
| 49 | + id: { name: 'id', type: 'text' }, |
| 50 | + name: { name: 'name', type: 'text' }, |
| 51 | + amount: { name: 'amount', type: 'number' }, |
| 52 | + }, |
| 53 | + enable: {}, |
| 54 | +}; |
| 55 | + |
| 56 | +/** objectql's `SummaryRecomputeError`, as a write's caller reads it: by `code`, carrying `written`. */ |
| 57 | +function staleRefusal(written: unknown): Error { |
| 58 | + return Object.assign(new Error('Roll-up summary recompute failed after retries for 1 parent record(s); the triggering records WERE written.'), { |
| 59 | + code: 'ERR_SUMMARY_RECOMPUTE', |
| 60 | + written, |
| 61 | + failures: [{ childObject: 'qsf_child', parentObject: 'qsf_parent', parentId: 'p1', field: 'probe_sf', error: new Error('sum over text') }], |
| 62 | + }); |
| 63 | +} |
| 64 | + |
| 65 | +const idOf = (options: any): string | undefined => options?.where?.id; |
| 66 | + |
| 67 | +/** |
| 68 | + * A store of `qsf_child` rows behind an engine whose `update` / `delete` apply |
| 69 | + * the write, then report the stale roll-up — or refuse before writing anything |
| 70 | + * when the patch names `refused`. |
| 71 | + */ |
| 72 | +function engineDouble(rows: Map<string, Record<string, unknown>>, writes: string[]) { |
| 73 | + return { |
| 74 | + find: vi.fn(async (_o: string, query?: any) => { |
| 75 | + const id = query?.where?.id; |
| 76 | + const all = [...rows.values()].filter((r) => id === undefined || r.id === id); |
| 77 | + return typeof query?.limit === 'number' ? all.slice(0, query.limit) : all; |
| 78 | + }), |
| 79 | + findOne: vi.fn(async (object: string, query?: EngineFindOneQueryInput) => { |
| 80 | + assertEngineFindOnePredicate(object, query); |
| 81 | + return rows.get(String((query as any)?.where?.id)) ?? null; |
| 82 | + }), |
| 83 | + insert: vi.fn(async () => { throw new Error('not used'); }), |
| 84 | + update: vi.fn(async (_o: string, data: Record<string, unknown>, options?: any) => { |
| 85 | + assertEngineUpdateDispatch(data, options); |
| 86 | + if (data.name === 'refused') { |
| 87 | + throw Object.assign(new Error('name is not allowed'), { code: 'VALIDATION_FAILED', status: 400 }); |
| 88 | + } |
| 89 | + const id = idOf(options)!; |
| 90 | + const next = { ...rows.get(id), ...data }; |
| 91 | + rows.set(id, next); |
| 92 | + writes.push(`update:${id}`); |
| 93 | + throw staleRefusal(next); |
| 94 | + }), |
| 95 | + delete: vi.fn(async (_o: string, options?: any) => { |
| 96 | + assertEngineDeleteDispatch(options); |
| 97 | + const id = idOf(options)!; |
| 98 | + rows.delete(id); |
| 99 | + writes.push(`delete:${id}`); |
| 100 | + throw staleRefusal(true); |
| 101 | + }), |
| 102 | + count: vi.fn(async () => rows.size), |
| 103 | + } as unknown as IDataEngine; |
| 104 | +} |
| 105 | + |
| 106 | +function metadataDouble(): IMetadataService { |
| 107 | + return { |
| 108 | + listObjects: vi.fn(async () => [CHILD]), |
| 109 | + getObject: vi.fn(async () => CHILD), |
| 110 | + } as unknown as IMetadataService; |
| 111 | +} |
| 112 | + |
| 113 | +function stdioBridge(rows: Map<string, Record<string, unknown>>, writes: string[]) { |
| 114 | + return createStdioDataBridge({ |
| 115 | + engine: engineDouble(rows, writes), |
| 116 | + metadataService: metadataDouble(), |
| 117 | + resolvePrincipal: async () => ({ userId: 'u1' }) as unknown as ExecutionContext, |
| 118 | + }); |
| 119 | +} |
| 120 | + |
| 121 | +let nextId = 1; |
| 122 | +async function callTool(bridge: McpDataBridge, name: string, args: Record<string, unknown>) { |
| 123 | + const body = { jsonrpc: '2.0', id: nextId++, method: 'tools/call', params: { name, arguments: args } }; |
| 124 | + const runtime = new MCPServerRuntime({ name: 'pin', version: '0.0.0' }); |
| 125 | + const res = await runtime.handleHttpRequest( |
| 126 | + new Request('http://localhost/api/v1/mcp', { |
| 127 | + method: 'POST', |
| 128 | + headers: { 'content-type': 'application/json', accept: 'application/json, text/event-stream' }, |
| 129 | + body: JSON.stringify(body), |
| 130 | + }), |
| 131 | + { bridge: bridge as any, parsedBody: body }, |
| 132 | + ); |
| 133 | + expect(res.status).toBe(200); |
| 134 | + const json = (await res.json()) as { result?: any; error?: unknown }; |
| 135 | + expect(json.error, JSON.stringify(json.error)).toBeUndefined(); |
| 136 | + return json.result as { content: Array<{ text: string }>; isError?: boolean }; |
| 137 | +} |
| 138 | + |
| 139 | +/** The runtime bridge's shape: `callData` hands back the protocol's refusal with the earned answer attached. */ |
| 140 | +function runtimeShapedBridge(answers: { update: Record<string, unknown>; remove: Record<string, unknown> }): McpDataBridge { |
| 141 | + return { |
| 142 | + async listObjects() { return [{ name: 'qsf_child', label: 'Child', fieldCount: 3 }]; }, |
| 143 | + async describeObject() { return { name: 'qsf_child', fields: [] }; }, |
| 144 | + async query(object: string) { return { object, records: [], total: 0 }; }, |
| 145 | + async get() { return null; }, |
| 146 | + async create() { return null; }, |
| 147 | + async update() { throw withEarnedAnswer(staleRefusal(answers.update.record) as any, answers.update); }, |
| 148 | + async remove() { throw withEarnedAnswer(staleRefusal(true) as any, answers.remove); }, |
| 149 | + } as unknown as McpDataBridge; |
| 150 | +} |
| 151 | + |
| 152 | +let errorLog: ReturnType<typeof vi.spyOn>; |
| 153 | +beforeAll(() => { errorLog = vi.spyOn(console, 'error').mockImplementation(() => {}); }); |
| 154 | +afterAll(() => { errorLog.mockRestore(); }); |
| 155 | + |
| 156 | +describe('[#28844] MCP update_record / delete_record answer a write applied under a failing roll-up as the write it is', () => { |
| 157 | + it('update_record, stdio bridge: the success result, the update applied once', async () => { |
| 158 | + const rows = new Map([['c1', { id: 'c1', name: 'c1', amount: 100 }]]); |
| 159 | + const writes: string[] = []; |
| 160 | + const result = await callTool(stdioBridge(rows, writes), 'update_record', { objectName: 'qsf_child', recordId: 'c1', data: { amount: 35 } }); |
| 161 | + expect(result.isError, result.content[0]?.text).toBeFalsy(); |
| 162 | + expect(JSON.parse(result.content[0]!.text)).toEqual({ object: 'qsf_child', id: 'c1', record: { id: 'c1', name: 'c1', amount: 35 } }); |
| 163 | + expect(writes).toEqual(['update:c1']); |
| 164 | + expect(rows.get('c1')).toMatchObject({ amount: 35 }); |
| 165 | + }); |
| 166 | + |
| 167 | + it('delete_record, stdio bridge: the success result, the delete applied once', async () => { |
| 168 | + const rows = new Map([['c2', { id: 'c2', name: 'c2', amount: 135 }]]); |
| 169 | + const writes: string[] = []; |
| 170 | + const result = await callTool(stdioBridge(rows, writes), 'delete_record', { objectName: 'qsf_child', recordId: 'c2' }); |
| 171 | + expect(result.isError, result.content[0]?.text).toBeFalsy(); |
| 172 | + expect(JSON.parse(result.content[0]!.text)).toEqual({ object: 'qsf_child', id: 'c2', success: true }); |
| 173 | + expect(writes).toEqual(['delete:c2']); |
| 174 | + expect(rows.has('c2')).toBe(false); |
| 175 | + }); |
| 176 | + |
| 177 | + it('both tools, runtime bridge: the answer the protocol attached to the refusal', async () => { |
| 178 | + const answers = { |
| 179 | + update: { object: 'qsf_child', id: 'r1', record: { id: 'r1', name: 'r1', amount: 6 } }, |
| 180 | + remove: { object: 'qsf_child', id: 'r2', success: true }, |
| 181 | + }; |
| 182 | + const bridge = runtimeShapedBridge(answers); |
| 183 | + const updated = await callTool(bridge, 'update_record', { objectName: 'qsf_child', recordId: 'r1', data: { amount: 6 } }); |
| 184 | + expect(updated.isError, updated.content[0]?.text).toBeFalsy(); |
| 185 | + expect(JSON.parse(updated.content[0]!.text)).toEqual(answers.update); |
| 186 | + const removed = await callTool(bridge, 'delete_record', { objectName: 'qsf_child', recordId: 'r2' }); |
| 187 | + expect(removed.isError, removed.content[0]?.text).toBeFalsy(); |
| 188 | + expect(JSON.parse(removed.content[0]!.text)).toEqual(answers.remove); |
| 189 | + }); |
| 190 | + |
| 191 | + it('a refusal before the write is still an error result, and nothing is written', async () => { |
| 192 | + const rows = new Map([['c3', { id: 'c3', name: 'c3', amount: 10 }]]); |
| 193 | + const writes: string[] = []; |
| 194 | + const result = await callTool(stdioBridge(rows, writes), 'update_record', { objectName: 'qsf_child', recordId: 'c3', data: { name: 'refused' } }); |
| 195 | + expect(result.isError).toBe(true); |
| 196 | + expect(result.content[0]!.text).toContain('VALIDATION_FAILED'); |
| 197 | + expect(writes).toEqual([]); |
| 198 | + expect(rows.get('c3')).toMatchObject({ name: 'c3' }); |
| 199 | + }); |
| 200 | + |
| 201 | + it('the stale column is logged once, at error', () => { |
| 202 | + const lines = errorLog.mock.calls.map((c: unknown[]) => String(c[0])).filter((l: string) => l.includes('could not be recomputed')); |
| 203 | + expect(lines).toHaveLength(1); |
| 204 | + expect(lines[0]).toContain('"qsf_parent.probe_sf"'); |
| 205 | + }); |
| 206 | +}); |
![objectstack-fleet[bot]](https://avatars.githubusercontent.com/u/254823548?v=4&size=40){kind=avatar}
0 commit comments