From be6414384387cba99cda5b3c804f8034eca03943 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 16:38:27 +0000 Subject: [PATCH 1/5] feat(mcp): delete_records deletes many records of one object in one confirmed call (WIP) The tool selects by a filter or an id list, answers an unconfirmed call with delete_record's confirmation refusal plus a preview (count, sample ids), and on confirm deletes each row through the bridge's remove, so every row meets the caller's permissions, row-level security and the package's delete rules; refusals come back grouped by reason. 200 per call. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibx1NhZMrqCvW7FBxg5 --- packages/mcp/src/mcp-http-tools.ts | 414 ++++++++++++++++++++++++++++- 1 file changed, 406 insertions(+), 8 deletions(-) diff --git a/packages/mcp/src/mcp-http-tools.ts b/packages/mcp/src/mcp-http-tools.ts index 755a5bc9018..5c18c462f56 100644 --- a/packages/mcp/src/mcp-http-tools.ts +++ b/packages/mcp/src/mcp-http-tools.ts @@ -559,24 +559,301 @@ function recordDeleteConfirmationRefusal( objectName: string, recordId: string, request: AIActionConfirmation, -): (ThrownRefusalEnvelope & { details: ActionConfirmationRequiredDetails }) | undefined { - // Only the boolean `true` attests; absent, `false` and a truthy string do not. - if (request[AI_ACTION_CONFIRMATION_MEMBER] === true) return undefined; - return { - code: ACTION_CONFIRMATION_REQUIRED, - message: - `delete_record on '${objectName}' record '${recordId}' carries no confirmation — nothing was deleted. ` +): DeleteConfirmationRefusal | undefined { + if (isDeleteConfirmed(request)) return undefined; + return deleteConfirmationRefusal( + DELETE_RECORD_TOOL, + objectName, + `delete_record on '${objectName}' record '${recordId}' carries no confirmation — nothing was deleted. ` + 'A delete over MCP is permanent: confirm with the human in the loop, then retry this call with ' + `'${AI_ACTION_CONFIRMATION_MEMBER}': true on the request.`, + ); +} + +/** The refusal both delete tools answer an unconfirmed call with: `run_action`'s, addressed to the tool. */ +type DeleteConfirmationRefusal = ThrownRefusalEnvelope & { details: ActionConfirmationRequiredDetails }; + +/** + * Only the boolean `true` attests; absent, `false` and a truthy string do not. + * One predicate for `delete_record` and `delete_records`, so the one-row and + * the many-row delete cannot disagree about what a confirmation is. + */ +function isDeleteConfirmed(request: AIActionConfirmation): boolean { + return request[AI_ACTION_CONFIRMATION_MEMBER] === true; +} + +/** + * The ONE builder of the delete tools' confirmation refusal: the code, status + * and `details` shape (`ActionConfirmationRequiredDetails`) `run_action`'s gate + * answers, with `details.actionName` naming the tool the refused call + * addressed. `delete_record` and `delete_records` differ only in the sentence. + */ +function deleteConfirmationRefusal(actionName: string, objectName: string, message: string): DeleteConfirmationRefusal { + return { + code: ACTION_CONFIRMATION_REQUIRED, + message, status: ACTION_CONFIRMATION_REQUIRED_HTTP_STATUS, details: { - actionName: DELETE_RECORD_TOOL, + actionName, objectName, confirmationMember: AI_ACTION_CONFIRMATION_MEMBER, }, }; } +// ── delete_records: many rows of one object in one confirmed call (#30891) ── + +/** The `details.actionName` a `delete_records` refusal names: the tool the refused call addressed. */ +const DELETE_RECORDS_TOOL = 'delete_records'; + +/** + * [#30891] The most records one `delete_records` call deletes. + * + * 200 is the bound the platform's other many-row delete door already applies + * per request: REST `POST /data/:object/deleteMany` refuses more ids than + * `RestServerConfig.batch.maxBatchSize`, whose default is 200. Like that + * route, this tool deletes ONE ROW AT A TIME (each row its own engine delete, + * so each row meets its own delete rules and cascade), so the cap also bounds + * the length of one call. A selection that matches more answers how many + * remain, and the next call continues. + */ +export const DELETE_RECORDS_MAX_PER_CALL = 200; + +/** How many ids of the selection a preview names. */ +const DELETE_RECORDS_SAMPLE_SIZE = 10; + +/** The alias the selection's count is read under. */ +const DELETE_RECORDS_COUNT_ALIAS = 'matched'; + +/** What one `delete_records` call selects: a filter, or an id list. Exactly one. */ +interface DeleteRecordsSelection { + where?: Record; + recordIds?: string[]; +} + +/** The selection, resolved under the caller's permissions and row-level security. */ +interface ResolvedDeleteSelection { + /** The ids a confirmed call attempts NOW, at most the cap: by id for a filter, the caller's order for a list. */ + attempt: string[]; + /** How many records the selection matches for this caller, when that is known. */ + matched?: number; + /** True when more records match than one call attempts, whether or not `matched` is known. */ + moreThanCap: boolean; + /** The listed ids that name no record this caller can read (an id-list selection only). */ + notFound: string[]; +} + +/** One group of refused rows: the same code, status and reason, as the delete answered each row. */ +interface DeleteRefusalGroup { + code?: string; + status?: number; + reason: string; + count: number; + recordIds: string[]; +} + +/** The rows a bridge's `query` answered, read off the protocol's `{ records }` shape. */ +function queriedRows(result: unknown, objectName: string): Array> { + const records = (result as { records?: unknown } | null | undefined)?.records; + if (!Array.isArray(records)) { + // Loud, never "nothing matched": an answer this tool cannot read is not a + // selection of zero rows, and a delete planned from it would be a guess. + throw new Error(`delete_records could not read the records the query on '${objectName}' answered — nothing was deleted.`); + } + return records as Array>; +} + +function idOfRow(row: Record): string | undefined { + const id = row?.id; + return typeof id === 'string' && id.length > 0 ? id : typeof id === 'number' ? String(id) : undefined; +} + +/** + * Resolve a `delete_records` selection to the ids one call attempts, through + * the bridge's `query` (and `aggregate`, for the count), so the read runs as + * the caller: a record the caller cannot read is never selected, and so never + * deleted by this tool. + * + * A filter is read in id order, one row past the cap, which says whether more + * match than one call attempts; the exact count is then asked of `aggregate`, + * when the bridge carries it. An id list is at most the cap long, so one read + * of those ids answers both which exist and how many. + */ +async function resolveDeleteSelection( + bridge: McpDataBridge, + objectName: string, + selection: DeleteRecordsSelection, +): Promise { + const cap = DELETE_RECORDS_MAX_PER_CALL; + if (selection.recordIds) { + const listed = [...new Set(selection.recordIds)]; + const rows = queriedRows( + await bridge.query(objectName, { where: { id: { $in: listed } }, fields: ['id'], limit: listed.length }), + objectName, + ); + const found = new Set(rows.map(idOfRow).filter((id): id is string => id !== undefined)); + const attempt = listed.filter((id) => found.has(id)); + return { attempt, matched: attempt.length, moreThanCap: false, notFound: listed.filter((id) => !found.has(id)) }; + } + const where = selection.where ?? {}; + const rows = queriedRows( + await bridge.query(objectName, { + where, + fields: ['id'], + orderBy: [{ field: 'id', order: 'asc' }], + limit: cap + 1, + }), + objectName, + ); + const ids = rows.map(idOfRow).filter((id): id is string => id !== undefined); + const moreThanCap = ids.length > cap; + const matched = moreThanCap ? await countSelection(bridge, objectName, where) : ids.length; + return { attempt: ids.slice(0, cap), ...(matched !== undefined ? { matched } : {}), moreThanCap, notFound: [] }; +} + +/** The selection's count under the caller's row-level security, or `undefined` when the bridge cannot say. */ +async function countSelection( + bridge: McpDataBridge, + objectName: string, + where: Record, +): Promise { + if (typeof bridge.aggregate !== 'function') return undefined; + const rows = await bridge.aggregate(objectName, { + where, + aggregations: [{ function: 'count', alias: DELETE_RECORDS_COUNT_ALIAS }], + }); + const value = (rows?.[0] as Record | undefined)?.[DELETE_RECORDS_COUNT_ALIAS]; + if (typeof value === 'bigint') return Number(value); + return typeof value === 'number' && Number.isFinite(value) ? value : undefined; +} + +/** + * Delete each id of the attempt set through the bridge's `remove`, the member + * `delete_record` deletes with, so each row passes exactly the checks a + * one-row delete passes: the object's exposure, the caller's permissions and + * row-level security, and the package's delete rules (`beforeDelete` hooks, + * restricted relations). No row is deleted any other way, so no rule is + * bypassed, for an administrator either (the maintainer's ruling on the card). + * + * A refused row stays, and its refusal is read with the reader the one-row + * tool renders a throw with ({@link thrownRefusalEnvelope}: `code`, `message`, + * `status`, never `details`), then grouped: rows refused with the same code, + * status and reason are one group. A delete that landed under a parent + * roll-up the engine could not recompute counts as deleted, as `delete_record` + * answers it, with the stale column logged. + */ +async function deleteEach( + bridge: McpDataBridge, + objectName: string, + ids: readonly string[], +): Promise<{ deleted: number; refusals: DeleteRefusalGroup[]; notRemoved: string[] }> { + let deleted = 0; + const notRemoved: string[] = []; + const groups = new Map(); + for (const id of ids) { + try { + const answer = await bridge.remove(objectName, id); + // `success: false` is the protocol's "matched, and deliberately NOT + // removed" (a reset, not a delete); anything else is a deletion. + if ((answer as { success?: unknown } | null | undefined)?.success === false) notRemoved.push(id); + else deleted++; + } catch (err) { + if (earnedAnswerOf(err) !== undefined) { + for (const line of staleSummaryAnswerLines(err as SummaryRecomputeRefusal, objectName)) console.error(`[MCP] ${line}`); + deleted++; + continue; + } + const envelope = thrownRefusalEnvelope(err); + const reason = envelope?.message ?? messageOf(err); + const key = JSON.stringify([envelope?.code ?? null, envelope?.status ?? null, reason]); + const group = groups.get(key) ?? { + ...(envelope?.code !== undefined ? { code: envelope.code } : {}), + ...(envelope?.status !== undefined ? { status: envelope.status } : {}), + reason, + count: 0, + recordIds: [], + }; + group.count++; + group.recordIds.push(id); + groups.set(key, group); + } + } + // Largest group first; a tie keeps the order the rows were refused in. + const refusals = [...groups.values()].sort((a, b) => b.count - a.count); + return { deleted, refusals, notRemoved }; +} + +/** How many records one call names: the exact count, or that more than the cap match. */ +function selectionCount(resolved: ResolvedDeleteSelection): { matched: number } | { matchedMoreThan: number } { + return resolved.matched !== undefined ? { matched: resolved.matched } : { matchedMoreThan: DELETE_RECORDS_MAX_PER_CALL }; +} + +function deleteRecordsPreview(objectName: string, resolved: ResolvedDeleteSelection) { + return { + object: objectName, + ...selectionCount(resolved), + wouldAttempt: resolved.attempt.length, + cap: DELETE_RECORDS_MAX_PER_CALL, + sampleRecordIds: resolved.attempt.slice(0, DELETE_RECORDS_SAMPLE_SIZE), + ...(resolved.notFound.length > 0 ? { notFound: resolved.notFound } : {}), + note: + 'Nothing was deleted. The object\'s delete rules (locks, restricted relations, permissions) are judged per ' + + 'record when the delete runs: a confirmed call deletes the records they allow and answers the rest grouped ' + + 'by reason, with their ids.', + }; +} + +/** The unconfirmed `delete_records` call's refusal sentence. */ +function deleteRecordsRefusalMessage(objectName: string, resolved: ResolvedDeleteSelection): string { + const count = resolved.matched !== undefined + ? `${resolved.matched} record(s)` + : `more than ${DELETE_RECORDS_MAX_PER_CALL} records`; + return ( + `delete_records on '${objectName}' carries no confirmation — nothing was deleted. The selection matches ${count}; ` + + `a confirmed call deletes up to ${DELETE_RECORDS_MAX_PER_CALL} of them. A delete over MCP is permanent: show ` + + `the preview to the human in the loop, then retry this call with '${AI_ACTION_CONFIRMATION_MEMBER}': true on the request.` + ); +} + +/** What a confirmed `delete_records` call answers: the outcome per row, counted and grouped. */ +function deleteRecordsOutcome( + objectName: string, + resolved: ResolvedDeleteSelection, + done: { deleted: number; refusals: DeleteRefusalGroup[]; notRemoved: string[] }, +) { + const refused = done.refusals.reduce((n, g) => n + g.count, 0); + // Of the records the selection matched when this call began, how many still + // match: the refused, the not removed and the ones past the cap. + const remaining = resolved.matched !== undefined ? resolved.matched - done.deleted : undefined; + const beyondCap = resolved.matched !== undefined ? resolved.matched - resolved.attempt.length : undefined; + const next: string[] = []; + if (resolved.moreThanCap) { + next.push( + `${beyondCap !== undefined ? beyondCap : 'More'} matching record(s) were past this call's cap of ` + + `${DELETE_RECORDS_MAX_PER_CALL}: call again with the same selection and confirm: true to continue.`, + ); + } + if (refused > 0 && resolved.moreThanCap) { + next.push( + 'Refused records still match the selection and are attempted again first; to move past them, exclude their ids ' + + '(for example {"id": {"$nin": [...]}} in where).', + ); + } + return { + object: objectName, + ...selectionCount(resolved), + attempted: resolved.attempt.length, + deleted: done.deleted, + refused, + refusals: done.refusals, + ...(done.notRemoved.length > 0 ? { notRemoved: done.notRemoved } : {}), + ...(resolved.notFound.length > 0 ? { notFound: resolved.notFound } : {}), + ...(remaining !== undefined ? { remaining } : {}), + cap: DELETE_RECORDS_MAX_PER_CALL, + ...(next.length > 0 ? { next: next.join(' ') } : {}), + }; +} + /** What {@link thrownRefusalEnvelope} reads off a thrown coded refusal. */ export interface ThrownRefusalEnvelope { code: string; @@ -1616,6 +1893,127 @@ export function registerObjectTools( } }, ); + + // [#30891] Many records of one object in one confirmed call, where the + // agent used to plan one `delete_record` per row. It composes the bridge + // members the other data tools call — `query` (and `aggregate`) to select, + // `remove` to delete each row — so it adds no execution path: each row is + // deleted exactly as `delete_record` deletes it, under the caller's + // permissions, row-level security and the package's delete rules, and a + // rule's refusal holds for every caller, an administrator included + // (maintainer's ruling on the card: no bypass). + // + // The confirmation is `delete_record`'s, by the decision that gated it: an + // unconfirmed call is refused with the same code, status and `details` + // shape. What the refusal adds is a `preview` beside `error` — the count + // and a sample of what a confirmed call would delete — because a human + // asked to approve a bulk delete has to see its size. Reading the + // selection is the one thing an unconfirmed call does; it deletes nothing. + // + // ⛔ The preview names no per-record refusal, and claims none: the engine + // has no delete dry run (`BatchOptions.validateOnly` was retired for + // promising one), and running the delete inside a rolled-back transaction + // would still fire `afterDelete` hooks, which do not wait for a commit. + // So the delete rules are judged when the delete runs, and the confirmed + // call answers them grouped by reason. + server.registerTool( + note(DELETE_RECORDS_TOOL), + { + description: + 'Delete many records of ONE object in one call: those a filter (where) matches, or an id list (recordIds) — ' + + 'instead of one delete_record per row. Destructive and permanent. A call without confirm: true deletes ' + + 'nothing: it is REFUSED (ACTION_CONFIRMATION_REQUIRED) with a `preview` of what a confirmed call would ' + + 'delete (how many match, sample ids). Show that to the human, and retry with confirm: true only once they ' + + `approve. A confirmed call deletes at most ${DELETE_RECORDS_MAX_PER_CALL} records, each under your ` + + 'permissions, row-level security and the object\'s delete rules, which no caller can bypass. Records a ' + + 'rule refuses stay; the answer counts them grouped by reason with their ids, and says how many still match.', + inputSchema: strictToolInput( + { + surface: 'this delete_records call', + aliases: { + ...OBJECT_NAME_ALIASES, + filters: 'where', + filter: 'where', + conditions: 'where', + criteria: 'where', + ids: 'recordIds', + record_ids: 'recordIds', + recordId: 'recordIds', + }, + }, + { + objectName: z.string().describe('The object/table name'), + where: z + .record(z.string(), z.unknown()) + .optional() + .describe( + 'Filter conditions selecting the records to delete, as query_records takes them, e.g. ' + + '{"status":"closed"}. {} selects every record you can see. Send this or recordIds, not both.', + ), + recordIds: z + .array(z.string().min(1)) + .min(1) + .max(DELETE_RECORDS_MAX_PER_CALL) + .optional() + .describe( + `The ids of the records to delete (at most ${DELETE_RECORDS_MAX_PER_CALL}). Send this or where, not both.`, + ), + // The confirmation member, as `delete_record` declares it: optional + // in the schema so an unconfirmed call reaches the handler and is + // answered with the gate's refusal and the preview. + [AI_ACTION_CONFIRMATION_MEMBER]: z + .boolean() + .optional() + .describe( + 'Set to true only when the human in the loop has approved deleting the records this selection ' + + 'matches. Without it nothing is deleted: the call answers a preview.', + ), + }, + ), + annotations: { readOnlyHint: false, destructiveHint: true, openWorldHint: false }, + }, + async (args) => { + const { objectName, where, recordIds } = args; + const bad = guard(objectName); + if (bad) return errorResult(bad); + if ((where === undefined) === (recordIds === undefined)) { + return errorResult( + where === undefined + ? 'delete_records needs a selection: send where (a filter, {} for every record) or recordIds (an id list).' + : 'delete_records takes one selection: send where or recordIds, not both.', + ); + } + try { + const resolved = await resolveDeleteSelection(bridge, objectName, { where, recordIds }); + // [ADR-0090 D10] A count a delegated read narrowed is served with the + // narrowing stated, as `query_records` serves its rows. ⛔ The probe + // never fails the call it annotates. + const narrowing = typeof bridge.diagnoseDelegation === 'function' + ? await bridge.diagnoseDelegation(objectName).catch(() => undefined) + : undefined; + const stated = (value: Record): unknown => + narrowing?.narrowed && narrowing.statement ? withDelegationNotice(value, narrowing.statement) : value; + if (!isDeleteConfirmed(args)) { + const refusal = deleteConfirmationRefusal( + DELETE_RECORDS_TOOL, + objectName, + deleteRecordsRefusalMessage(objectName, resolved), + ); + return { + content: [{ + type: 'text' as const, + text: jsonText({ error: refusal, preview: stated(deleteRecordsPreview(objectName, resolved)) }), + }], + isError: true as const, + }; + } + const done = await deleteEach(bridge, objectName, resolved.attempt); + return textResult(stated(deleteRecordsOutcome(objectName, resolved, done))); + } catch (err) { + return errorResultWithCode(err); + } + }, + ); } // end canWrite (data:write) return registered; From df996a762f407b67401834b5354ff3103084c5e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 16:44:37 +0000 Subject: [PATCH 2/5] test(mcp): tool-surface pins and the skill line learn delete_records (WIP) Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibx1NhZMrqCvW7FBxg5 --- packages/mcp/README.md | 3 ++- packages/mcp/src/mcp-coded-refusal.test.ts | 3 +++ packages/mcp/src/mcp-http-tools.scopes.test.ts | 2 +- .../src/mcp-http-tools.unknown-argument-keys.test.ts | 10 +++++++--- .../src/mcp-server-runtime.authoring-scope.test.ts | 2 +- packages/mcp/src/mcp-server-runtime.http.test.ts | 1 + packages/mcp/src/mcp-stdio-tools.test.ts | 1 + packages/mcp/src/skill-md.test.ts | 1 + packages/mcp/src/skill-md.ts | 11 ++++++++++- .../stdio-data-bridge.stored-metadata-body.test.ts | 4 ++++ 10 files changed, 31 insertions(+), 7 deletions(-) diff --git a/packages/mcp/README.md b/packages/mcp/README.md index 7dcfa481bb0..719d1cd1a2c 100644 --- a/packages/mcp/README.md +++ b/packages/mcp/README.md @@ -118,6 +118,7 @@ apply): 'create_record' // Create 'update_record' // Update by id 'delete_record' // Delete by id (destructive; refused without confirm: true) +'delete_records' // Delete many by filter or id list, ≤200 per call (refused with a preview without confirm: true) // Business actions — operate the app, not just its rows 'list_actions' // Invokable business actions the caller may run @@ -130,7 +131,7 @@ and `resume_run` only when it implements `resumeRun`; a bridge without those seams serves the rest and advertises nothing it cannot do. OAuth scopes narrow the families at consent time: `data:read` covers -list/describe/query/aggregate/get, `data:write` covers create/update/delete, and +list/describe/query/aggregate/get, `data:write` covers create/update/delete (both delete tools), and `actions:execute` covers `list_actions` / `run_action` / `resume_run`. A tool outside the grant is **not registered at all**, so the SDK rejects it as an unknown tool — the grant doubles as dispatch-time enforcement. diff --git a/packages/mcp/src/mcp-coded-refusal.test.ts b/packages/mcp/src/mcp-coded-refusal.test.ts index 8890ffc8742..ad22c8127ae 100644 --- a/packages/mcp/src/mcp-coded-refusal.test.ts +++ b/packages/mcp/src/mcp-coded-refusal.test.ts @@ -102,6 +102,9 @@ const SITES: readonly Site[] = [ { tool: 'update_record', args: { objectName: 'task', recordId: 't9', data: { title: 'x' } }, member: 'update' }, // `confirm: true` passes the confirmation gate, so the call reaches `remove`. { tool: 'delete_record', args: { objectName: 'task', recordId: 't9', confirm: true }, member: 'remove' }, + // [#30891] The selection read is the member a bulk delete calls first; a + // refusal there refuses the whole call, rendered as the data tools render it. + { tool: 'delete_records', args: { objectName: 'task', where: {}, confirm: true }, member: 'query' }, { tool: 'list_actions', args: {}, member: 'listActions' }, { tool: 'run_action', args: { actionName: 'complete' }, member: 'runAction', actionDoor: true }, { tool: 'resume_run', args: { runId: 'r1' }, member: 'resumeRun', actionDoor: true }, diff --git a/packages/mcp/src/mcp-http-tools.scopes.test.ts b/packages/mcp/src/mcp-http-tools.scopes.test.ts index 9051adb34ef..5af1c392772 100644 --- a/packages/mcp/src/mcp-http-tools.scopes.test.ts +++ b/packages/mcp/src/mcp-http-tools.scopes.test.ts @@ -23,7 +23,7 @@ import { MCPServerRuntime } from './mcp-server-runtime.js'; import type { McpActionBridge, McpDataBridge } from './mcp-http-tools.js'; const READ_TOOLS = ['list_objects', 'describe_object', 'validate_expression', 'query_records', 'get_record', 'aggregate_records']; -const WRITE_TOOLS = ['create_record', 'update_record', 'delete_record']; +const WRITE_TOOLS = ['create_record', 'update_record', 'delete_record', 'delete_records']; const ACTION_TOOLS = ['list_actions', 'run_action']; function makeBridge(): McpDataBridge & McpActionBridge & { calls: any[] } { diff --git a/packages/mcp/src/mcp-http-tools.unknown-argument-keys.test.ts b/packages/mcp/src/mcp-http-tools.unknown-argument-keys.test.ts index 4f95ca4b219..9f7777b7f87 100644 --- a/packages/mcp/src/mcp-http-tools.unknown-argument-keys.test.ts +++ b/packages/mcp/src/mcp-http-tools.unknown-argument-keys.test.ts @@ -108,6 +108,9 @@ const VALID_ARGS: Record> = { update_record: { objectName: 'crm_opportunity', recordId: 'r1', data: { name: 'x' } }, // `confirm: true`: without it the confirmation gate refuses before the bridge. delete_record: { objectName: 'crm_opportunity', recordId: 'r1', confirm: true }, + // [#30891] A selection is required (`where` or `recordIds`); `confirm: true` + // deletes, where an unconfirmed call is answered with the refusal and a preview. + delete_records: { objectName: 'crm_opportunity', where: { stage: 'closed_lost' }, confirm: true }, list_actions: {}, run_action: { actionName: 'complete_task' }, resume_run: { runId: 'run_1' }, @@ -214,12 +217,13 @@ describe('MCP tool arguments — undeclared keys are refused (#16913)', () => { // ── The sweep: every advertised tool holds the same posture ──────────────── - it('advertises the twelve tools this sweep covers', async () => { + it('advertises the thirteen tools this sweep covers', async () => { const json = await rpc(runtime, bridge, 'tools/list'); const names = (json.result?.tools ?? []).map((t: any) => t.name).sort(); expect(names).toEqual(Object.keys(VALID_ARGS).sort()); - // [#15705] Twelve with `resume_run`, which this stub's bridge implements. - expect(names).toHaveLength(12); + // [#15705] Twelve with `resume_run`, which this stub's bridge implements; + // [#30891] thirteen with `delete_records`. + expect(names).toHaveLength(13); }); it('every advertised tool ACCEPTS its declared arguments (the sweep control)', async () => { diff --git a/packages/mcp/src/mcp-server-runtime.authoring-scope.test.ts b/packages/mcp/src/mcp-server-runtime.authoring-scope.test.ts index b54f5f97833..3f6ea9dfe95 100644 --- a/packages/mcp/src/mcp-server-runtime.authoring-scope.test.ts +++ b/packages/mcp/src/mcp-server-runtime.authoring-scope.test.ts @@ -53,7 +53,7 @@ import type { // --------------------------------------------------------------------------- const READ_TOOLS = ['list_objects', 'describe_object', 'validate_expression', 'query_records', 'get_record', 'aggregate_records']; -const WRITE_TOOLS = ['create_record', 'update_record', 'delete_record']; +const WRITE_TOOLS = ['create_record', 'update_record', 'delete_record', 'delete_records']; const ACTION_TOOLS = ['list_actions', 'run_action']; const NATIVE_TOOLS = [...READ_TOOLS, ...WRITE_TOOLS, ...ACTION_TOOLS].sort(); diff --git a/packages/mcp/src/mcp-server-runtime.http.test.ts b/packages/mcp/src/mcp-server-runtime.http.test.ts index a136024ab97..ffeade7f007 100644 --- a/packages/mcp/src/mcp-server-runtime.http.test.ts +++ b/packages/mcp/src/mcp-server-runtime.http.test.ts @@ -97,6 +97,7 @@ describe('MCPServerRuntime.handleHttpRequest (Streamable HTTP)', () => { [ 'create_record', 'delete_record', + 'delete_records', 'describe_object', 'get_record', 'list_objects', diff --git a/packages/mcp/src/mcp-stdio-tools.test.ts b/packages/mcp/src/mcp-stdio-tools.test.ts index 3b3c52f47a6..379c7260946 100644 --- a/packages/mcp/src/mcp-stdio-tools.test.ts +++ b/packages/mcp/src/mcp-stdio-tools.test.ts @@ -184,6 +184,7 @@ function makeBridge(): McpDataBridge & { calls: any[][] } { const OBJECT_TOOLS = [ 'create_record', 'delete_record', + 'delete_records', 'describe_object', 'get_record', 'list_objects', diff --git a/packages/mcp/src/skill-md.test.ts b/packages/mcp/src/skill-md.test.ts index d947c308250..aa8023640cd 100644 --- a/packages/mcp/src/skill-md.test.ts +++ b/packages/mcp/src/skill-md.test.ts @@ -72,6 +72,7 @@ describe('renderSkillMarkdown', () => { 'create_record', 'update_record', 'delete_record', + 'delete_records', 'list_actions', 'run_action', 'resume_run', diff --git a/packages/mcp/src/skill-md.ts b/packages/mcp/src/skill-md.ts index de39c4ee4c3..16a3f9abbb2 100644 --- a/packages/mcp/src/skill-md.ts +++ b/packages/mcp/src/skill-md.ts @@ -32,7 +32,7 @@ import type { AIToolDefinition } from '@objectstack/spec/contracts'; import { MCP_OAUTH_SCOPE_DATA_READ, MCP_OAUTH_SCOPE_METADATA_WRITE } from '@objectstack/spec/ai'; -import { grantsAuthoringTools } from './mcp-http-tools.js'; +import { DELETE_RECORDS_MAX_PER_CALL, grantsAuthoringTools } from './mcp-http-tools.js'; /** Skill identity (mirrors the `SKILL.md` YAML frontmatter). */ export const OBJECTSTACK_SKILL_NAME = 'objectstack'; @@ -289,6 +289,15 @@ create/update payload. permanently. The server REFUSES a call without \`confirm: true\` (\`ACTION_CONFIRMATION_REQUIRED\`) and nothing is deleted. Ask your human first; the flag asserts an approval, it does not obtain one. +- **delete_records({ objectName, where | recordIds, confirm })** — delete many + records of one object in one call, instead of one \`delete_record\` per row. + Without \`confirm: true\` it deletes nothing and is REFUSED + (\`ACTION_CONFIRMATION_REQUIRED\`) with a \`preview\`: how many records match + and sample ids. Show that to your human, then retry with \`confirm: true\`. + A confirmed call deletes up to ${DELETE_RECORDS_MAX_PER_CALL} records, each under your permissions and the + object's delete rules (no one bypasses them); refused records stay and come + back grouped by reason with their ids, and \`remaining\` says how many still + match. Call again to continue. - **list_actions()** — list the business actions available to you (only those the app author exposed to AI and you are permitted to run), with each action's declared parameters, whether it operates on a record, and whether diff --git a/packages/mcp/src/stdio-data-bridge.stored-metadata-body.test.ts b/packages/mcp/src/stdio-data-bridge.stored-metadata-body.test.ts index 0b9b089d3a8..3e30be9e62b 100644 --- a/packages/mcp/src/stdio-data-bridge.stored-metadata-body.test.ts +++ b/packages/mcp/src/stdio-data-bridge.stored-metadata-body.test.ts @@ -492,6 +492,10 @@ const ENGINE_READ_SITES: Record> = { 'stdio-data-bridge.ts': { 'find(object)': 2, 'aggregate(object)': 1 }, // The ADR-0101 record resource reader. 'plugin.ts': { 'find(objectName)': 1 }, + // [#30891] NOT an engine read: `delete_records` counting its selection + // through the BRIDGE's `aggregate` member, the one enumerated above for + // this transport, so the stored-body and hash refusals apply to it there. + 'mcp-http-tools.ts': { 'aggregate(objectName)': 1 }, }; const ENGINE_READ_VERBS: ReadonlySet = new Set(['find', 'findOne', 'aggregate', 'count']); From 5cc50f96731fd7a2bde3df6689795e25d1dcb3e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 16:50:11 +0000 Subject: [PATCH 3/5] test(mcp): pin delete_records preview, grouped refusals, caller context and the 200 cap (WIP) Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibx1NhZMrqCvW7FBxg5 --- packages/mcp/src/mcp-delete-records.test.ts | 509 ++++++++++++++++++++ scripts/engine-double-contract.pinned.json | 15 + 2 files changed, 524 insertions(+) create mode 100644 packages/mcp/src/mcp-delete-records.test.ts diff --git a/packages/mcp/src/mcp-delete-records.test.ts b/packages/mcp/src/mcp-delete-records.test.ts new file mode 100644 index 00000000000..7ba27cc36f0 --- /dev/null +++ b/packages/mcp/src/mcp-delete-records.test.ts @@ -0,0 +1,509 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#30891] The MCP `delete_records` tool deletes many records of ONE object in + * one confirmed call, selected by a filter (`where`) or an id list + * (`recordIds`). + * + * ## What is pinned, and where + * + * - The confirmation is `delete_record`'s (#30813): the same member, code, + * status, envelope keys and `details` keys. A call without `confirm: true` + * is REFUSED and deletes nothing; the refusal carries a `preview` beside + * `error` (how many records match, how many a confirmed call attempts, a + * sample of ids). + * - A confirmed call deletes each selected row through the bridge's + * `remove`, the member `delete_record` deletes with, under the CALLER's + * resolved context, handed to the engine unchanged: the tool has no other + * path to a delete, so it has no way around a package's delete rules, for + * an administrator either (the maintainer's ruling on the card). Rows a + * rule refuses stay, and are answered grouped by code, status and reason, + * with their ids. + * - The per-call cap (200) and the remainder: a selection over the cap + * deletes 200 and says how many still match; the next call continues. + * + * Driven over the HTTP `/mcp` transport against the REAL stdio data bridge on + * an engine double that holds the rows; the long-lived server the stdio + * transport serves is driven once too. A real stack, with a real package + * locking rule refusing an administrator, is + * `packages/qa/dogfood/test/mcp-delete-records.dogfood.test.ts`. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; +import { AI_ACTION_CONFIRMATION_MEMBER } from '@objectstack/spec/contracts'; +import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { + assertEngineDeleteDispatch, + assertEngineUpdateDispatch, + assertEngineFindOnePredicate, + type EngineFindOneQueryInput, +} from '@objectstack/metadata-core'; + +import { MCPServerRuntime } from './mcp-server-runtime.js'; +import { createStdioDataBridge } from './stdio-data-bridge.js'; +import { DELETE_RECORDS_MAX_PER_CALL, type McpDataBridge } from './mcp-http-tools.js'; + +const OBJECT = 'dlr_task'; + +const TASK = { + name: OBJECT, + label: 'Task', + fields: { + id: { name: 'id', type: 'text' }, + title: { name: 'title', type: 'text' }, + status: { name: 'status', type: 'text' }, + locked: { name: 'locked', type: 'boolean' }, + owner: { name: 'owner', type: 'text' }, + }, + enable: {}, +}; + +/** The resolved caller: an administrator. Every engine call must carry exactly this context. */ +const CALLER = { userId: 'u_admin', roles: ['admin'] } as unknown as ExecutionContext; + +const LOCKED_REASON = 'The period is closed: a task of a closed period cannot be deleted.'; +const OWNER_REASON = 'You may not delete a task you do not own.'; +/** A permission refusal's `details` names positions; the tools keep `details` off the wire. */ +const WITHHELD_POSITION = 'pos_finance_controller'; + +type Row = Record; + +/** The equality / `$in` / `$nin` subset the tool's selections use; anything else is loud. */ +function matches(row: Row, where: Record | undefined): boolean { + for (const [key, cond] of Object.entries(where ?? {})) { + const cell = row[key]; + if (cond !== null && typeof cond === 'object' && !Array.isArray(cond)) { + for (const [op, operand] of Object.entries(cond as Record)) { + if (op === '$in') { + if (!(operand as unknown[]).includes(cell)) return false; + } else if (op === '$nin') { + if ((operand as unknown[]).includes(cell)) return false; + } else { + throw new Error(`engine double: operator ${op} is not modelled`); + } + } + } else if (cell !== cond) { + return false; + } + } + return true; +} + +/** objectql's `SummaryRecomputeError`, as a write's caller reads it: by `code`, carrying `written`. */ +function staleRefusal(): Error { + return Object.assign(new Error('Failed to recompute 1 roll-up summary value(s) after write; the triggering records WERE written.'), { + code: 'ERR_SUMMARY_RECOMPUTE', + written: undefined, + failures: [{ childObject: OBJECT, parentObject: 'dlr_project', parentId: 'p1', field: 'open_tasks', error: new Error('count over text') }], + }); +} + +/** + * A store of `dlr_task` rows behind an engine that judges each delete the way + * a package's delete rules do: a `locked` row is refused with + * `RECORD_LOCKED` / 409 whoever asks, a row another user owns with + * `PERMISSION_DENIED` / 403 (carrying `details`), and a `stale` row is + * deleted under a roll-up the engine could not recompute. + */ +function engineDouble(rows: Map, opts: { aggregate?: boolean } = {}) { + const contexts: unknown[] = []; + const engine: Record = { + find: vi.fn(async (_o: string, query?: any, options?: any) => { + contexts.push(options?.context); + let all = [...rows.values()].filter((r) => matches(r, query?.where)); + const order = query?.orderBy?.[0]; + if (order) { + const dir = order.order === 'desc' ? -1 : 1; + all = all.sort((a, b) => (String(a[order.field]) < String(b[order.field]) ? -dir : String(a[order.field]) > String(b[order.field]) ? dir : 0)); + } + if (Array.isArray(query?.fields)) all = all.map((r) => Object.fromEntries(query.fields.map((f: string) => [f, r[f]]))); + return typeof query?.limit === 'number' ? all.slice(0, query.limit) : all; + }), + findOne: vi.fn(async (object: string, query?: EngineFindOneQueryInput) => { + assertEngineFindOnePredicate(object, query); + return rows.get(String((query as any)?.where?.id)) ?? null; + }), + insert: vi.fn(async () => { + throw new Error('not used'); + }), + update: vi.fn(async (_o: string, data: Record, options?: any) => { + assertEngineUpdateDispatch(data, options); + throw new Error('not used'); + }), + delete: vi.fn(async (_o: string, options?: any) => { + assertEngineDeleteDispatch(options); + contexts.push(options?.context); + const id = String(options?.where?.id); + const row = rows.get(id); + if (row?.locked === true) { + throw Object.assign(new Error(LOCKED_REASON), { code: 'RECORD_LOCKED', status: 409 }); + } + if (row?.owner === 'someone_else') { + throw Object.assign(new Error(OWNER_REASON), { + code: 'PERMISSION_DENIED', + status: 403, + details: { positions: [WITHHELD_POSITION] }, + }); + } + rows.delete(id); + if (row?.stale === true) throw staleRefusal(); + return { deleted: 1 }; + }), + count: vi.fn(async () => rows.size), + }; + if (opts.aggregate !== false) { + engine.aggregate = vi.fn(async (_o: string, query: any) => { + contexts.push(query?.context); + const count = [...rows.values()].filter((r) => matches(r, query?.where)).length; + return [Object.fromEntries((query.aggregations as Array<{ alias: string }>).map((a) => [a.alias, count]))]; + }); + } + return { engine: engine as any, contexts }; +} + +function metadataDouble(): IMetadataService { + return { + listObjects: vi.fn(async () => [TASK]), + getObject: vi.fn(async () => TASK), + } as unknown as IMetadataService; +} + +function stdioBridge(engine: unknown): McpDataBridge { + return createStdioDataBridge({ + engine: engine as IDataEngine, + metadataService: metadataDouble(), + resolvePrincipal: async () => CALLER, + }); +} + +let nextId = 1; +async function rpcRaw(bridge: McpDataBridge, method: string, params?: Record) { + const body = { jsonrpc: '2.0', id: nextId++, method, ...(params ? { params } : {}) }; + const runtime = new MCPServerRuntime({ name: 'pin', version: '0.0.0' }); + const res = await runtime.handleHttpRequest( + new Request('http://localhost/api/v1/mcp', { + method: 'POST', + headers: { 'content-type': 'application/json', accept: 'application/json, text/event-stream' }, + body: JSON.stringify(body), + }), + { bridge: bridge as any, parsedBody: body }, + ); + expect(res.status).toBe(200); + return (await res.json()) as { result?: any; error?: unknown }; +} + +async function rpc(bridge: McpDataBridge, method: string, params?: Record) { + const json = await rpcRaw(bridge, method, params); + expect(json.error, JSON.stringify(json.error)).toBeUndefined(); + return json.result; +} + +type ToolResult = { content: Array<{ text: string }>; isError?: boolean }; + +async function callTool(bridge: McpDataBridge, name: string, args: Record): Promise { + return (await rpc(bridge, 'tools/call', { name, arguments: args })) as ToolResult; +} + +const parsed = (result: ToolResult) => JSON.parse(result.content[0]!.text); + +const task = (id: string, extra: Row = {}): [string, Row] => [id, { id, title: `task ${id}`, status: 'done', ...extra }]; + +describe('MCP delete_records: many records of one object in one confirmed call (#30891)', () => { + let rows: Map; + let double: ReturnType; + let bridge: McpDataBridge; + + beforeEach(() => { + rows = new Map([ + task('t1'), + task('t2', { locked: true }), + task('t3'), + task('t4', { locked: true }), + task('t5', { owner: 'someone_else' }), + task('t6', { status: 'open' }), + ]); + double = engineDouble(rows); + bridge = stdioBridge(double.engine); + }); + + it('is advertised as destructive, with an optional boolean confirmation member and a capped id list', async () => { + const listed = await rpc(bridge, 'tools/list'); + const tool = (listed.tools as any[]).find((t) => t.name === 'delete_records'); + expect(tool, 'delete_records is registered').toBeDefined(); + const props = tool.inputSchema.properties; + expect(Object.keys(props)).toEqual(expect.arrayContaining(['objectName', 'where', 'recordIds', AI_ACTION_CONFIRMATION_MEMBER])); + expect(props[AI_ACTION_CONFIRMATION_MEMBER]?.type).toBe('boolean'); + // Optional, as on delete_record: a call without it must reach the handler, + // which answers the coded refusal and the preview. + expect(tool.inputSchema.required ?? []).toEqual(['objectName']); + expect(props.recordIds?.maxItems).toBe(DELETE_RECORDS_MAX_PER_CALL); + expect(tool.description).toContain('confirm: true'); + expect(tool.description).toContain(String(DELETE_RECORDS_MAX_PER_CALL)); + expect(tool.annotations?.destructiveHint).toBe(true); + expect(tool.annotations?.readOnlyHint).toBe(false); + }); + + it('an unconfirmed call is refused with a preview: count, sample ids, nothing deleted', async () => { + const result = await callTool(bridge, 'delete_records', { objectName: OBJECT, where: { status: 'done' } }); + + expect(result.isError).toBe(true); + const { error, preview } = parsed(result); + expect(error.code).toBe('ACTION_CONFIRMATION_REQUIRED'); + expect(error.status).toBe(428); + expect(error.details).toEqual({ actionName: 'delete_records', objectName: OBJECT, confirmationMember: AI_ACTION_CONFIRMATION_MEMBER }); + expect(String(error.message)).toContain('nothing was deleted'); + expect(String(error.message)).toContain('matches 5 record(s)'); + + expect(preview).toMatchObject({ + object: OBJECT, + matched: 5, + wouldAttempt: 5, + cap: DELETE_RECORDS_MAX_PER_CALL, + sampleRecordIds: ['t1', 't2', 't3', 't4', 't5'], + }); + expect(preview.note).toMatch(/^Nothing was deleted\./); + + // Nothing was deleted: the engine was never asked to, and every row is there. + expect(double.engine.delete).not.toHaveBeenCalled(); + expect([...rows.keys()]).toEqual(['t1', 't2', 't3', 't4', 't5', 't6']); + }); + + it.each([ + ['false', false], + ['the string "true"', 'true'], + ['1', 1], + ])('refuses %s as the member: only the boolean true attests, and nothing is deleted', async (_label, value) => { + const result = await callTool(bridge, 'delete_records', { + objectName: OBJECT, + where: {}, + [AI_ACTION_CONFIRMATION_MEMBER]: value, + }); + + expect(result.isError).toBe(true); + expect(double.engine.delete).not.toHaveBeenCalled(); + expect(rows.size).toBe(6); + }); + + it('answers with delete_record\'s refusal: the same code, status, envelope keys and details keys', async () => { + const one = parsed(await callTool(bridge, 'delete_record', { objectName: OBJECT, recordId: 't1' })).error; + const many = parsed(await callTool(bridge, 'delete_records', { objectName: OBJECT, recordIds: ['t1'] })).error; + + // Lit control: the delete_record half really is #30813's refusal. + expect(one.code).toBe('ACTION_CONFIRMATION_REQUIRED'); + + expect(many.code).toBe(one.code); + expect(many.status).toBe(one.status); + expect(Object.keys(many)).toEqual(Object.keys(one)); + expect(Object.keys(many.details)).toEqual(Object.keys(one.details)); + expect(many.details.confirmationMember).toBe(one.details.confirmationMember); + expect(many.details.objectName).toBe(one.details.objectName); + // The tool the refused call addressed, as run_action names its action. + expect(many.details.actionName).toBe('delete_records'); + expect(rows.has('t1')).toBe(true); + }); + + it('a confirmed call deletes the rows the rules allow and answers the rest grouped by reason, with ids', async () => { + const result = await callTool(bridge, 'delete_records', { + objectName: OBJECT, + where: { status: 'done' }, + [AI_ACTION_CONFIRMATION_MEMBER]: true, + }); + + expect(result.isError, result.content[0]?.text).toBeFalsy(); + const answer = parsed(result); + expect(answer).toEqual({ + object: OBJECT, + matched: 5, + attempted: 5, + deleted: 2, + refused: 3, + refusals: [ + { code: 'RECORD_LOCKED', status: 409, reason: LOCKED_REASON, count: 2, recordIds: ['t2', 't4'] }, + { code: 'PERMISSION_DENIED', status: 403, reason: OWNER_REASON, count: 1, recordIds: ['t5'] }, + ], + remaining: 3, + cap: DELETE_RECORDS_MAX_PER_CALL, + }); + // A refusal's `details` stays off the wire, as on every other tool. + expect(result.content[0]!.text).not.toContain(WITHHELD_POSITION); + + // Ground truth: the allowed rows are gone, the refused ones and the unselected one stay. + expect([...rows.keys()]).toEqual(['t2', 't4', 't5', 't6']); + // Each selected row was asked once, no retry, no second path. + expect(double.engine.delete).toHaveBeenCalledTimes(5); + }); + + it('runs as the caller: every engine read and delete carries the caller\'s resolved context, unchanged', async () => { + await callTool(bridge, 'delete_records', { objectName: OBJECT, where: { status: 'done' }, [AI_ACTION_CONFIRMATION_MEMBER]: true }); + + expect(double.contexts.length).toBeGreaterThan(5); + for (const context of double.contexts) { + // The same object the principal resolver answered: no elevated copy, no + // `isSystem`, no swapped user. A locking rule judges the caller. + expect(context).toBe(CALLER); + } + const deleteContexts = double.engine.delete.mock.calls.map((c: any[]) => c[1]?.context); + expect(deleteContexts).toHaveLength(5); + for (const context of deleteContexts) expect((context as any).isSystem).toBeUndefined(); + }); + + it('an id list: duplicates collapse, an id naming no readable record is answered, not attempted', async () => { + const preview = parsed(await callTool(bridge, 'delete_records', { objectName: OBJECT, recordIds: ['t3', 't1', 't3', 'gone', 't2'] })).preview; + expect(preview).toMatchObject({ matched: 3, wouldAttempt: 3, sampleRecordIds: ['t3', 't1', 't2'], notFound: ['gone'] }); + expect(double.engine.delete).not.toHaveBeenCalled(); + + const answer = parsed( + await callTool(bridge, 'delete_records', { + objectName: OBJECT, + recordIds: ['t3', 't1', 't3', 'gone', 't2'], + [AI_ACTION_CONFIRMATION_MEMBER]: true, + }), + ); + expect(answer).toMatchObject({ + matched: 3, + attempted: 3, + deleted: 2, + refused: 1, + refusals: [{ code: 'RECORD_LOCKED', status: 409, count: 1, recordIds: ['t2'] }], + notFound: ['gone'], + remaining: 1, + }); + expect([...rows.keys()].sort()).toEqual(['t2', 't4', 't5', 't6']); + }); + + it('caps one call at 200, says how many remain, and the next call continues', async () => { + rows.clear(); + for (let i = 0; i < 205; i++) { + const [id, row] = task(`r${String(i).padStart(3, '0')}`); + rows.set(id, row); + } + + const preview = parsed(await callTool(bridge, 'delete_records', { objectName: OBJECT, where: {} })).preview; + expect(preview).toMatchObject({ matched: 205, wouldAttempt: DELETE_RECORDS_MAX_PER_CALL, cap: DELETE_RECORDS_MAX_PER_CALL }); + expect(preview.sampleRecordIds).toHaveLength(10); + expect(rows.size).toBe(205); + + const first = parsed(await callTool(bridge, 'delete_records', { objectName: OBJECT, where: {}, [AI_ACTION_CONFIRMATION_MEMBER]: true })); + expect(first).toMatchObject({ matched: 205, attempted: 200, deleted: 200, refused: 0, remaining: 5 }); + expect(first.next).toContain('5 matching record(s) were past this call\'s cap of 200'); + expect(rows.size).toBe(5); + // In id order: the 200 lowest ids went first. + expect([...rows.keys()].sort()).toEqual(['r200', 'r201', 'r202', 'r203', 'r204']); + + const second = parsed(await callTool(bridge, 'delete_records', { objectName: OBJECT, where: {}, [AI_ACTION_CONFIRMATION_MEMBER]: true })); + expect(second).toMatchObject({ matched: 5, attempted: 5, deleted: 5, remaining: 0 }); + expect(second.next).toBeUndefined(); + expect(rows.size).toBe(0); + }); + + it('over the cap with refused rows, the answer tells the caller to exclude their ids to move past them', async () => { + rows.clear(); + for (let i = 0; i < 202; i++) { + const [id, row] = task(`r${String(i).padStart(3, '0')}`, i < 2 ? { locked: true } : {}); + rows.set(id, row); + } + const answer = parsed(await callTool(bridge, 'delete_records', { objectName: OBJECT, where: {}, [AI_ACTION_CONFIRMATION_MEMBER]: true })); + expect(answer).toMatchObject({ matched: 202, attempted: 200, deleted: 198, refused: 2, remaining: 4 }); + expect(answer.refusals).toEqual([{ code: 'RECORD_LOCKED', status: 409, reason: LOCKED_REASON, count: 2, recordIds: ['r000', 'r001'] }]); + expect(answer.next).toContain('$nin'); + }); + + it('with no aggregate on the bridge, a selection over the cap answers "more than 200", never a guessed count', async () => { + rows.clear(); + for (let i = 0; i < 203; i++) { + const [id, row] = task(`r${String(i).padStart(3, '0')}`); + rows.set(id, row); + } + const plain = engineDouble(rows, { aggregate: false }); + const noAggregate = stdioBridge(plain.engine); + expect(noAggregate.aggregate).toBeUndefined(); + + const preview = parsed(await callTool(noAggregate, 'delete_records', { objectName: OBJECT, where: {} })).preview; + expect(preview).toMatchObject({ matchedMoreThan: DELETE_RECORDS_MAX_PER_CALL, wouldAttempt: DELETE_RECORDS_MAX_PER_CALL }); + expect('matched' in preview).toBe(false); + + const answer = parsed(await callTool(noAggregate, 'delete_records', { objectName: OBJECT, where: {}, [AI_ACTION_CONFIRMATION_MEMBER]: true })); + expect(answer).toMatchObject({ matchedMoreThan: DELETE_RECORDS_MAX_PER_CALL, attempted: 200, deleted: 200 }); + expect('remaining' in answer).toBe(false); + expect(answer.next).toMatch(/^More matching record\(s\) were past this call's cap/); + expect(rows.size).toBe(3); + }); + + it('a delete applied under a stale roll-up counts as deleted; a remove answering success: false is named', async () => { + rows.set('t7', { id: 't7', title: 'task t7', status: 'done', stale: true }); + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + const partial: McpDataBridge = { + ...bridge, + async remove(object, id) { + if (id === 't3') return { object, id, success: false }; + return bridge.remove(object, id); + }, + }; + const answer = parsed( + await callTool(partial, 'delete_records', { objectName: OBJECT, recordIds: ['t1', 't3', 't7'], [AI_ACTION_CONFIRMATION_MEMBER]: true }), + ); + expect(answer).toMatchObject({ matched: 3, attempted: 3, deleted: 2, refused: 0, refusals: [], notRemoved: ['t3'], remaining: 1 }); + expect(rows.has('t7')).toBe(false); + expect(rows.has('t3')).toBe(true); + } finally { + errors.mockRestore(); + } + }); + + it.each([ + ['neither selection', {}, 'needs a selection'], + ['both selections', { where: {}, recordIds: ['t1'] }, 'takes one selection'], + ])('refuses %s before reading anything, and deletes nothing', async (_label, selection, words) => { + const result = await callTool(bridge, 'delete_records', { objectName: OBJECT, ...selection, [AI_ACTION_CONFIRMATION_MEMBER]: true }); + + expect(result.isError).toBe(true); + expect(result.content[0]!.text).toContain(words); + expect(double.engine.find).not.toHaveBeenCalled(); + expect(double.engine.delete).not.toHaveBeenCalled(); + expect(rows.size).toBe(6); + }); + + it('refuses an id list longer than the cap at the input, and deletes nothing', async () => { + const ids = Array.from({ length: DELETE_RECORDS_MAX_PER_CALL + 1 }, (_, i) => `x${i}`); + const json = await rpcRaw(bridge, 'tools/call', { + name: 'delete_records', + arguments: { objectName: OBJECT, recordIds: ids, [AI_ACTION_CONFIRMATION_MEMBER]: true }, + }); + + const refused = json.error !== undefined || json.result?.isError === true; + expect(refused, JSON.stringify(json).slice(0, 300)).toBe(true); + expect(double.engine.delete).not.toHaveBeenCalled(); + }); + + it('the long-lived server the stdio transport serves refuses, previews and deletes the same way', async () => { + const runtime = new MCPServerRuntime({ name: 'pin-stdio', version: '0.0.0' }); + runtime.bridgeDataTools(bridge); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await runtime.server.connect(serverTransport); + const client = new Client({ name: 'pin-client', version: '0.0.0' }); + await client.connect(clientTransport); + try { + const refused: any = await client.callTool({ name: 'delete_records', arguments: { objectName: OBJECT, where: { status: 'done' } } }); + expect(refused.isError).toBe(true); + const { error, preview } = JSON.parse(refused.content[0].text); + expect(error.code).toBe('ACTION_CONFIRMATION_REQUIRED'); + expect(error.details).toEqual({ actionName: 'delete_records', objectName: OBJECT, confirmationMember: AI_ACTION_CONFIRMATION_MEMBER }); + expect(preview).toMatchObject({ matched: 5, wouldAttempt: 5 }); + expect(rows.size).toBe(6); + + const confirmed: any = await client.callTool({ + name: 'delete_records', + arguments: { objectName: OBJECT, where: { status: 'done' }, [AI_ACTION_CONFIRMATION_MEMBER]: true }, + }); + expect(confirmed.isError, confirmed.content?.[0]?.text).toBeFalsy(); + expect(JSON.parse(confirmed.content[0].text)).toMatchObject({ deleted: 2, refused: 3, remaining: 3 }); + expect([...rows.keys()]).toEqual(['t2', 't4', 't5', 't6']); + } finally { + await client.close(); + } + }); +}); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 39bc7b5733b..2dbce8b9d40 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -96,6 +96,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/mcp/src/mcp-delete-records.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/mcp/src/mcp-delete-records.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/mcp/src/mcp-delete-records.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/mcp/src/mcp-stdio-tools.test.ts", "verb": "delete", From e223b85436eb1ad4eb748b2e3015528ab1a19b58 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 16:53:15 +0000 Subject: [PATCH 4/5] docs(mcp): delete_records in the MCP guide, the spec contract and a changeset; dogfood pins (WIP) Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibx1NhZMrqCvW7FBxg5 --- .changeset/30891-mcp-delete-records.md | 19 ++ content/docs/ai/connect-mcp.mdx | 17 +- .../test/mcp-delete-records.dogfood.test.ts | 213 ++++++++++++++++++ packages/spec/src/contracts/ai-service.ts | 14 +- 4 files changed, 260 insertions(+), 3 deletions(-) create mode 100644 .changeset/30891-mcp-delete-records.md create mode 100644 packages/qa/dogfood/test/mcp-delete-records.dogfood.test.ts diff --git a/.changeset/30891-mcp-delete-records.md b/.changeset/30891-mcp-delete-records.md new file mode 100644 index 00000000000..e1df86a7e56 --- /dev/null +++ b/.changeset/30891-mcp-delete-records.md @@ -0,0 +1,19 @@ +--- +'@objectstack/mcp': patch +'@objectstack/spec': patch +--- + +feat(mcp): `delete_records` deletes many records of one object in one confirmed call, by a filter or an id list (#30891) + +Clause-②: yes (widening) — the MCP surface (HTTP `/api/v1/mcp` and the stdio server) registers one more `data:write` tool, `delete_records`; no existing tool, input or answer changes. +Metadata-protocol: none + +An agent that had to clear a set of records called `delete_record` once per row, and confirmed each one. `delete_records` takes the selection as `where` (a filter, `{}` for every record) or `recordIds` (an id list), never both. + +- **The preview is the refusal.** A call without `confirm: true` deletes nothing. It answers `delete_record`'s refusal (`428` `ACTION_CONFIRMATION_REQUIRED`, the same `details` keys, `details.actionName` naming `delete_records`) with a `preview` member beside `error`: how many records the selection matches for this caller (`matched`), how many a confirmed call attempts (`wouldAttempt`), up to ten sample ids and, for an id list, the ids that name no readable record (`notFound`). The agent shows that to its human and retries with `confirm: true` once they approve. +- **A confirmed call deletes each record as `delete_record` does**, through the same bridge member, under the caller's permissions, row-level security and the object's delete rules (`beforeDelete` hooks, restricted relations). No caller bypasses a package's locking rule, an administrator included. Records a rule refuses stay, and the answer groups them by code, status and reason, with their ids: `{ matched, attempted, deleted, refused, refusals: [{ code, status, reason, count, recordIds }], remaining, cap }`. A refusal's `details` stays off the wire, as on every other tool. +- **At most 200 records per call**, the bound REST `POST /data/:object/deleteMany` applies by default (`batch.maxBatchSize`). A filter is worked in id order; when more than 200 match, the answer says how many remain and the next call with the same selection continues. When the deployment has no aggregate support, the answer says `matchedMoreThan: 200` instead of guessing a count. + +What a preview cannot say: which records a delete rule will refuse. The engine has no delete dry run, so the rules are judged when the confirmed call runs, and the preview says so. + +`@objectstack/spec` changes only a doc comment: `AIActionConfirmation` names `delete_records` as the third door of the confirmation member. diff --git a/content/docs/ai/connect-mcp.mdx b/content/docs/ai/connect-mcp.mdx index 4e8a8b3be32..102f586ff79 100644 --- a/content/docs/ai/connect-mcp.mdx +++ b/content/docs/ai/connect-mcp.mdx @@ -245,7 +245,7 @@ decision. ## What the agent gets -Twelve tools, generated from your metadata: +Thirteen tools, generated from your metadata: | Tool | What it does | |:---|:---| @@ -254,6 +254,7 @@ Twelve tools, generated from your metadata: | `query_records` / `get_record` | Read data (list queries are capped at 50 rows per page by default) | | `aggregate_records` | Grouped aggregation (registered when the active driver supports it) | | `create_record` / `update_record` / `delete_record` | Write data | +| `delete_records` | Delete many records of one object, by a filter or an id list, at most 200 per call | | `list_actions` / `run_action` | Discover and invoke your business actions by name | | `resume_run` | Finish a flow action that paused on a screen, by submitting the screen's values | @@ -383,6 +384,18 @@ drive this MCP server. Same word, different layer. with `confirm: true`; the flag asserts an approval, it does not obtain one. REST `DELETE /api/v1/data/:object/:id` and `update_record` take no confirmation. +- **`delete_records` takes the same gate, and its refusal is the preview.** A + call without `confirm: true` reads the selection as the caller and deletes + nothing: the same `428` refusal (`details.actionName` naming + `delete_records`) arrives with a `preview` beside it, saying how many records + match, how many a confirmed call attempts and a sample of their ids, for the + agent to show its human. A confirmed call deletes at most 200 records, each + exactly as `delete_record` would: under the caller's permissions and + row-level security, and through the object's delete rules (a package's + locking hook, a restricted relation), which no caller bypasses, an + administrator included. Records a rule refuses stay, and the answer counts + them grouped by reason with their ids; when more than 200 match it says how + many remain, and the next call continues. ## Verify the connection @@ -411,7 +424,7 @@ call '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion": # → {"result":{"serverInfo":{"name":"objectstack",...},"capabilities":{"tools":{...},"prompts":{}}},...} call '{"jsonrpc":"2.0","id":2,"method":"tools/list"}' | grep -o '"name":"[a-z_]*"' -# → the twelve tools, list_objects … resume_run +# → the thirteen tools, list_objects … resume_run call '{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"query_records","arguments":{"objectName":"crm_opportunity","limit":3}}}' # → {"result":{"content":[{"type":"text","text":"{ \"object\": \"crm_opportunity\", \"records\": [ ... ], \"total\": 23, \"hasMore\": true }"}]}} diff --git a/packages/qa/dogfood/test/mcp-delete-records.dogfood.test.ts b/packages/qa/dogfood/test/mcp-delete-records.dogfood.test.ts new file mode 100644 index 00000000000..3a473fc05fa --- /dev/null +++ b/packages/qa/dogfood/test/mcp-delete-records.dogfood.test.ts @@ -0,0 +1,213 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#30891] The MCP `delete_records` tool, end to end on a booted stack: the +// live `/api/v1/mcp` transport, the runtime's real data bridge, the real +// engine and a package's real locking rule, for an ADMINISTRATOR caller. +// +// ## What this file pins that the package's own pins cannot +// +// `packages/mcp/src/mcp-delete-records.test.ts` drives the tool on an engine +// double. Here the rule that refuses a row is a real `beforeDelete` hook, +// written the way a package writes one: HotCRM's +// `opportunity_line_item_freeze` refuses a USER's change of a closed deal's +// line with `RECORD_LOCKED` / 409 and lets a write with no acting user (a +// seed, a migration) through. So the lock refuses whoever calls as a user, +// administrators included, and only a delete that stopped running as the +// caller could get past it. The maintainer's ruling on the card is that no +// caller bypasses such a rule (「不绕过」); the admin pin below is that ruling's +// executable form. +// +// • preview: an unconfirmed call answers the count and a sample, and the +// records are all still readable over REST afterwards; +// • confirmed: the unlocked rows are deleted, the locked rows are answered +// as one group (code, status, the hook's own sentence, their ids) and stay; +// • control: REST `DELETE /data/:object/:id` refuses the same admin on a +// locked row, so the tool answers what the platform's other door answers; +// • the cap: 203 matching rows, one call deletes 200 and says 3 remain, the +// next call deletes the 3 (the count is the runtime's real aggregate). + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { MCPServerPlugin } from '@objectstack/mcp'; +import { AI_ACTION_CONFIRMATION_MEMBER } from '@objectstack/spec/contracts'; + +const LOCKED = 'dlr_close_task'; +const BULK = 'dlr_bulk_row'; +const LOCK_SENTENCE = 'This task belongs to a closed period, so it can no longer be deleted.'; + +const CloseTask = ObjectSchema.create({ + name: LOCKED, + label: 'Close task', + fields: { + name: Field.text({ label: 'Name', required: true }), + batch: Field.text({ label: 'Batch' }), + period_closed: Field.boolean({ label: 'Period closed' }), + }, +}); + +const BulkRow = ObjectSchema.create({ + name: BULK, + label: 'Bulk row', + fields: { + name: Field.text({ label: 'Name', required: true }), + batch: Field.text({ label: 'Batch' }), + }, +}); + +const fixture = defineStack({ + manifest: { + id: 'com.dogfood.mcp-delete-records', + namespace: 'dlr', + version: '0.0.0', + type: 'app', + name: 'MCP delete_records fixture', + description: 'A package locking rule that refuses a user delete of a closed-period row, for #30891.', + }, + objects: [CloseTask, BulkRow], + hooks: [ + { + // A package's locking rule, in HotCRM's shape: a USER's delete of a + // closed-period row is refused; a write with no acting user passes. + name: 'dlr_close_period_lock', + object: LOCKED, + events: ['beforeDelete'], + handler: async (ctx: any) => { + if (!ctx.user?.id) return; + if (ctx.previous?.period_closed !== true) return; + throw Object.assign(new Error(LOCK_SENTENCE), { code: 'RECORD_LOCKED', status: 409 }); + }, + }, + ], +}); + +const idOf = (b: any) => b?.id ?? b?.record?.id ?? b?.data?.id; + +describe('MCP delete_records on a real stack: preview, grouped refusals, an admin under a locking rule, the cap (#30891)', () => { + let stack: VerifyStack; + let admin: string; + + /** One `tools/call` over the live MCP transport; the tool's text content, parsed. */ + const tool = async (name: string, args: Record) => { + const res = await stack.api('/mcp', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Accept: 'application/json, text/event-stream', + Authorization: `Bearer ${admin}`, + }, + body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name, arguments: args } }), + }); + expect(res.status).toBe(200); + const rpc: any = await res.json(); + expect(rpc.error, JSON.stringify(rpc.error)).toBeUndefined(); + const text = String(rpc.result?.content?.[0]?.text ?? ''); + let payload: any; + try { + payload = JSON.parse(text); + } catch { + payload = { message: text }; + } + return { isError: rpc.result?.isError === true, payload, text }; + }; + + const create = async (object: string, data: Record): Promise => { + const res = await stack.apiAs(admin, 'POST', `/data/${object}`, data); + expect(res.status, await res.clone().text()).toBeLessThan(300); + const id = idOf(await res.json()); + expect(id, 'created record id').toBeTruthy(); + return String(id); + }; + const readStatus = async (object: string, id: string) => (await stack.apiAs(admin, 'GET', `/data/${object}/${id}`)).status; + + beforeAll(async () => { + stack = await bootStack(fixture, { extraPlugins: [new MCPServerPlugin()] }); + admin = await stack.signIn(); + }, 300_000); + + afterAll(async () => { + await stack?.stop?.(); + }); + + describe('a selection with locked rows', () => { + let open: string[]; + let closed: string[]; + let other: string; + + beforeAll(async () => { + open = [await create(LOCKED, { name: 'open 1', batch: 'q4' }), await create(LOCKED, { name: 'open 2', batch: 'q4' })]; + closed = [ + await create(LOCKED, { name: 'closed 1', batch: 'q4', period_closed: true }), + await create(LOCKED, { name: 'closed 2', batch: 'q4', period_closed: true }), + ]; + other = await create(LOCKED, { name: 'another batch', batch: 'q1' }); + }, 60_000); + + it('preview: an unconfirmed call answers the count and a sample, and deletes nothing', async () => { + const refused = await tool('delete_records', { objectName: LOCKED, where: { batch: 'q4' } }); + + expect(refused.isError, refused.text).toBe(true); + expect(refused.payload.error).toMatchObject({ + code: 'ACTION_CONFIRMATION_REQUIRED', + status: 428, + details: { actionName: 'delete_records', objectName: LOCKED, confirmationMember: AI_ACTION_CONFIRMATION_MEMBER }, + }); + expect(refused.payload.preview).toMatchObject({ object: LOCKED, matched: 4, wouldAttempt: 4, cap: 200 }); + expect([...refused.payload.preview.sampleRecordIds].sort()).toEqual([...open, ...closed].sort()); + + // Nothing was deleted: every record still reads over REST. + for (const id of [...open, ...closed, other]) expect(await readStatus(LOCKED, id), `record ${id}`).toBe(200); + }, 60_000); + + it('control: REST DELETE refuses the same administrator on a locked row', async () => { + const res = await stack.apiAs(admin, 'DELETE', `/data/${LOCKED}/${closed[0]}`); + expect(res.status, await res.clone().text()).toBe(409); + expect(await readStatus(LOCKED, closed[0]!)).toBe(200); + }, 60_000); + + it('confirmed, as an administrator: the unlocked rows go, the locked rows stay and are answered as one group', async () => { + const done = await tool('delete_records', { + objectName: LOCKED, + where: { batch: 'q4' }, + [AI_ACTION_CONFIRMATION_MEMBER]: true, + }); + + expect(done.isError, done.text).toBe(false); + expect(done.payload).toMatchObject({ object: LOCKED, matched: 4, attempted: 4, deleted: 2, refused: 2, remaining: 2 }); + expect(done.payload.refusals).toHaveLength(1); + const [group] = done.payload.refusals; + expect(group).toMatchObject({ code: 'RECORD_LOCKED', status: 409, reason: LOCK_SENTENCE, count: 2 }); + expect([...group.recordIds].sort()).toEqual([...closed].sort()); + + // Ground truth over REST: the administrator's delete did not get past the lock. + for (const id of closed) expect(await readStatus(LOCKED, id), `locked record ${id}`).toBe(200); + for (const id of open) expect(await readStatus(LOCKED, id), `open record ${id}`).toBe(404); + // Outside the selection: untouched. + expect(await readStatus(LOCKED, other)).toBe(200); + }, 60_000); + }); + + it('the cap: 203 matching rows, one call deletes 200 and says 3 remain, the next call deletes the 3', async () => { + const rows = Array.from({ length: 203 }, (_, i) => ({ name: `bulk ${i}`, batch: 'cap' })); + for (const slice of [rows.slice(0, 200), rows.slice(200)]) { + const res = await stack.apiAs(admin, 'POST', `/data/${BULK}/createMany`, slice); + expect(res.status, await res.clone().text()).toBeLessThan(300); + } + + const preview = await tool('delete_records', { objectName: BULK, where: { batch: 'cap' } }); + expect(preview.isError).toBe(true); + expect(preview.payload.preview).toMatchObject({ matched: 203, wouldAttempt: 200, cap: 200 }); + + const first = await tool('delete_records', { objectName: BULK, where: { batch: 'cap' }, [AI_ACTION_CONFIRMATION_MEMBER]: true }); + expect(first.isError, first.text).toBe(false); + expect(first.payload).toMatchObject({ matched: 203, attempted: 200, deleted: 200, refused: 0, remaining: 3 }); + expect(first.payload.next).toContain('3 matching record(s) were past this call\'s cap of 200'); + + const second = await tool('delete_records', { objectName: BULK, where: { batch: 'cap' }, [AI_ACTION_CONFIRMATION_MEMBER]: true }); + expect(second.payload).toMatchObject({ matched: 3, attempted: 3, deleted: 3, remaining: 0 }); + + const after = await tool('delete_records', { objectName: BULK, where: { batch: 'cap' } }); + expect(after.payload.preview).toMatchObject({ matched: 0, wouldAttempt: 0 }); + }, 180_000); +}); diff --git a/packages/spec/src/contracts/ai-service.ts b/packages/spec/src/contracts/ai-service.ts index 2c6291ce59c..11d9d740fa6 100644 --- a/packages/spec/src/contracts/ai-service.ts +++ b/packages/spec/src/contracts/ai-service.ts @@ -319,6 +319,18 @@ export const AI_ACTION_CONFIRMATION_MEMBER = 'confirm'; * The gate's bounds are part of the decision: REST `DELETE /api/v1/data/...` * (the human path) and the `update_record` tool stay outside it, no object is * exempt, and any future bulk-delete tool takes the same gate. + * + * ## The third gate: the MCP `delete_records` tool + * + * The bulk delete (#30891) takes that gate, with the same member, code, HTTP + * status and `details` shape, whose `actionName` is `delete_records`. One + * difference, and it is the tool's reason to exist: its unconfirmed call READS + * the selection (a filter or an id list), as the caller, and answers the + * refusal with a `preview` member beside `error` (how many records match, how + * many a confirmed call attempts, sample ids), so the human approves a count + * rather than a filter. Nothing is deleted. A confirmed call deletes each + * selected record as `delete_record` does, under the caller's permissions and + * the object's delete rules, which no caller bypasses. */ export interface AIActionConfirmation { /** @@ -340,7 +352,7 @@ export interface AIActionConfirmation { export interface ActionConfirmationRequiredDetails { /** * The declarative action name the refused call addressed; on the MCP - * `delete_record` door, the tool's own name, `delete_record`. + * `delete_record` and `delete_records` doors, the tool's own name. */ actionName: string; /** The object the action operates on; omitted for object-less actions. */ From a798923389e5c2efd6720875855de9886d2af13c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 17:41:01 +0000 Subject: [PATCH 5/5] test(mcp): the delete_records engine double refuses combinators it does not model Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibx1NhZMrqCvW7FBxg5 --- packages/mcp/src/mcp-delete-records.test.ts | 31 ++++++++++----------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/packages/mcp/src/mcp-delete-records.test.ts b/packages/mcp/src/mcp-delete-records.test.ts index 7ba27cc36f0..9380f2c6ee2 100644 --- a/packages/mcp/src/mcp-delete-records.test.ts +++ b/packages/mcp/src/mcp-delete-records.test.ts @@ -71,25 +71,22 @@ const WITHHELD_POSITION = 'pos_finance_controller'; type Row = Record; -/** The equality / `$in` / `$nin` subset the tool's selections use; anything else is loud. */ +/** + * The equality / `$in` / `$nin` subset the tool's selections use. Anything + * else is loud: a combinator (`$and`, `$or`, …) is refused rather than read as + * a field name, and so is an operator this double does not model. + */ function matches(row: Row, where: Record | undefined): boolean { - for (const [key, cond] of Object.entries(where ?? {})) { + return Object.entries(where ?? {}).every(([key, cond]) => { + if (key.startsWith('$')) throw new Error(`engine double: combinator ${key} is not modelled`); const cell = row[key]; - if (cond !== null && typeof cond === 'object' && !Array.isArray(cond)) { - for (const [op, operand] of Object.entries(cond as Record)) { - if (op === '$in') { - if (!(operand as unknown[]).includes(cell)) return false; - } else if (op === '$nin') { - if ((operand as unknown[]).includes(cell)) return false; - } else { - throw new Error(`engine double: operator ${op} is not modelled`); - } - } - } else if (cell !== cond) { - return false; - } - } - return true; + if (cond === null || typeof cond !== 'object' || Array.isArray(cond)) return cell === cond; + return Object.entries(cond as Record).every(([op, operand]) => { + if (op === '$in') return (operand as unknown[]).includes(cell); + if (op === '$nin') return !(operand as unknown[]).includes(cell); + throw new Error(`engine double: operator ${op} is not modelled`); + }); + }); } /** objectql's `SummaryRecomputeError`, as a write's caller reads it: by `code`, carrying `written`. */