From be6414384387cba99cda5b3c804f8034eca03943 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 16:38:27 +0000 Subject: [PATCH 1/8] 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/8] 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/8] 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/8] 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/8] 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`. */ From ad203a283726b7fecf7bf1b1eab4654a352aaf42 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 18:53:55 +0000 Subject: [PATCH 6/8] fix(mcp): delete_records reads at the cap, stops on a fault, needs both data scopes Contract-review fixes on the delete_records tool (#30891): - The filter selection is read at the cap, not one row past it; whether more match is the aggregate count's answer. The cap+1 probe stays only for a bridge without aggregate. The per-call cap follows the deployment's read window down (OS_DATA_MAX_PAGE_SIZE / OS_DATA_QUERY_ROW_BUDGET), since the data door refuses a larger read; an id list longer than that cap is refused before anything is read. - A row's throw is judged with the REST data door's own classifier (mapDataError): a 4xx verdict is a refusal and is grouped, a 5xx verdict is a fault and stops the call, answered isError with the fault, the record it stopped at and the call's progress. - delete_records registers only under both data:read and data:write. - The error-code ledger's ACTION_CONFIRMATION_REQUIRED rows name both delete tools. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibx1NhZMrqCvW7FBxg5 --- .changeset/30891-mcp-delete-records.md | 8 +- content/docs/ai/connect-mcp.mdx | 11 +- packages/mcp/README.md | 5 +- packages/mcp/src/mcp-delete-records.test.ts | 189 ++++++- .../mcp/src/mcp-http-tools.scopes.test.ts | 38 +- packages/mcp/src/mcp-http-tools.ts | 489 ++++++++++++------ ...mcp-server-runtime.authoring-scope.test.ts | 6 +- packages/mcp/src/skill-md.ts | 5 +- .../test/mcp-delete-records.dogfood.test.ts | 177 ++++++- .../spec/src/api/error-code-ledger.zod.ts | 20 +- 10 files changed, 735 insertions(+), 213 deletions(-) diff --git a/.changeset/30891-mcp-delete-records.md b/.changeset/30891-mcp-delete-records.md index e1df86a7e56..afc43881f6a 100644 --- a/.changeset/30891-mcp-delete-records.md +++ b/.changeset/30891-mcp-delete-records.md @@ -5,15 +5,17 @@ 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. +Clause-②: yes (widening) — the MCP surface (HTTP `/api/v1/mcp` and the stdio server) registers one more tool, `delete_records`, offered to an OAuth token that carries both `data:read` and `data:write`; 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. +- **A fault is not a refusal.** A rule's refusal and a fault are told apart the way the REST `/data` door tells them apart (its own classifier): a 4xx verdict (a coded rule refusal, a sandboxed hook body's `400 HOOK_ABORTED`) is grouped and the call goes on; a 5xx verdict (a code hook's bare `throw new Error`, a lost connection, a driver error, a driver's own string code) stops the call at that record. The tool error names the fault and the record, and carries the call's `progress` (`deleted`, `stoppedAt`, `notAttempted`): the records deleted before it stay deleted. +- **At most 200 records per call**, the bound REST `POST /data/:object/deleteMany` applies by default (`batch.maxBatchSize`). A filter is read in id order at the cap, and the count says whether more match; 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. On a deployment whose read page size or row budget (`OS_DATA_MAX_PAGE_SIZE`, `OS_DATA_QUERY_ROW_BUDGET`) is under 200, one call runs at that size, since the data door refuses a larger read, and the answer's `cap` names it; a longer id list is refused before anything is read. +- **Scopes.** Over OAuth the tool needs both `data:read` and `data:write`: its preview reads the selection's ids and count, a read a `data:write`-only token is not granted. 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. +`@objectstack/spec` changes only doc comments: `AIActionConfirmation` names `delete_records` as the third door of the confirmation member, and the error-code ledger's `ACTION_CONFIRMATION_REQUIRED` rows name both delete tools. diff --git a/content/docs/ai/connect-mcp.mdx b/content/docs/ai/connect-mcp.mdx index 102f586ff79..a2224039a45 100644 --- a/content/docs/ai/connect-mcp.mdx +++ b/content/docs/ai/connect-mcp.mdx @@ -254,7 +254,7 @@ Thirteen 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 | +| `delete_records` | Delete many records of one object, by a filter or an id list, at most 200 per call (an OAuth token needs both `data:read` and `data:write`) | | `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 | @@ -395,7 +395,14 @@ drive this MCP server. Same word, different layer. 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. + many remain, and the next call continues. On a deployment whose read page + size or row budget (`OS_DATA_MAX_PAGE_SIZE`, `OS_DATA_QUERY_ROW_BUDGET`) is + under 200, one call runs at that size instead, and the answer's `cap` says + so. A fault (a store error, a crash) + is not a refusal: it stops the call, and the error names the record it + stopped at and how many were already deleted. Because the preview reads ids + and a count, an OAuth token is offered the tool only when it carries both + `data:read` and `data:write`. ## Verify the connection diff --git a/packages/mcp/README.md b/packages/mcp/README.md index 719d1cd1a2c..7c00cda3601 100644 --- a/packages/mcp/README.md +++ b/packages/mcp/README.md @@ -131,8 +131,9 @@ 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 (both delete tools), and -`actions:execute` covers `list_actions` / `run_action` / `resume_run`. A tool outside the grant +list/describe/query/aggregate/get, `data:write` covers create/update/delete, and +`actions:execute` covers `list_actions` / `run_action` / `resume_run`. `delete_records` needs both +`data:read` and `data:write`: its preview reads the selection's ids and count. 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-delete-records.test.ts b/packages/mcp/src/mcp-delete-records.test.ts index 9380f2c6ee2..9708ef1b3d4 100644 --- a/packages/mcp/src/mcp-delete-records.test.ts +++ b/packages/mcp/src/mcp-delete-records.test.ts @@ -29,7 +29,7 @@ * `packages/qa/dogfood/test/mcp-delete-records.dogfood.test.ts`. */ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } 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'; @@ -41,10 +41,11 @@ import { assertEngineFindOnePredicate, type EngineFindOneQueryInput, } from '@objectstack/metadata-core'; +import { planQueryReadWindow, resolveQueryReadLimits } from '@objectstack/types'; 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'; +import { DELETE_RECORDS_MAX_PER_CALL, deleteRecordsCap, type McpDataBridge } from './mcp-http-tools.js'; const OBJECT = 'dlr_task'; @@ -103,9 +104,11 @@ function staleRefusal(): Error { * 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. + * deleted under a roll-up the engine could not recompute. A row carrying + * `throws` throws that value instead, whatever it is (a fault, a sandboxed + * body's refusal). */ -function engineDouble(rows: Map, opts: { aggregate?: boolean } = {}) { +function engineDouble(rows: Map, opts: { aggregate?: boolean; countAsString?: boolean } = {}) { const contexts: unknown[] = []; const engine: Record = { find: vi.fn(async (_o: string, query?: any, options?: any) => { @@ -135,6 +138,7 @@ function engineDouble(rows: Map, opts: { aggregate?: boolean } = {} contexts.push(options?.context); const id = String(options?.where?.id); const row = rows.get(id); + if (row?.throws !== undefined) throw row.throws; if (row?.locked === true) { throw Object.assign(new Error(LOCKED_REASON), { code: 'RECORD_LOCKED', status: 409 }); } @@ -155,7 +159,9 @@ function engineDouble(rows: Map, opts: { aggregate?: boolean } = {} 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]))]; + // A SQL driver may answer COUNT(*) as a decimal string (a 64-bit integer). + const value = opts.countAsString ? String(count) : count; + return [Object.fromEntries((query.aggregations as Array<{ alias: string }>).map((a) => [a.alias, value]))]; }); } return { engine: engine as any, contexts }; @@ -208,6 +214,34 @@ 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 }]; +/** + * The data door's read window in front of a bridge: every `query` is planned + * with the platform's own rule (`planQueryReadWindow` over the deployment's + * `OS_DATA_*` limits), which REFUSES a read over the window, never clamps it. + * The limits each read asked for are recorded. + */ +function windowedBridge(inner: McpDataBridge): McpDataBridge & { limits: Array } { + const limits: Array = []; + return { + ...inner, + limits, + async query(object, options) { + const limit = (options as { limit?: number } | undefined)?.limit; + limits.push(limit); + planQueryReadWindow({ surface: 'list', object, ...(limit !== undefined ? { limit } : {}) }, resolveQueryReadLimits()); + return inner.query(object, options); + }, + }; +} + +function fillRows(rows: Map, n: number, extra: (i: number) => Row = () => ({})): void { + rows.clear(); + for (let i = 0; i < n; i++) { + const [id, row] = task(`r${String(i).padStart(3, '0')}`, extra(i)); + rows.set(id, row); + } +} + describe('MCP delete_records: many records of one object in one confirmed call (#30891)', () => { let rows: Map; let double: ReturnType; @@ -476,6 +510,151 @@ describe('MCP delete_records: many records of one object in one confirmed call ( expect(double.engine.delete).not.toHaveBeenCalled(); }); + describe('the selection read stays inside the deployment\'s read window', () => { + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it('a host whose maximum page size is the cap: a filter over it is read at the cap and counted, never refused', async () => { + vi.stubEnv('OS_DATA_MAX_PAGE_SIZE', String(DELETE_RECORDS_MAX_PER_CALL)); + fillRows(rows, 205); + const host = windowedBridge(bridge); + + // Lit control: the window really refuses one row past the cap. + await expect(host.query(OBJECT, { where: {}, limit: DELETE_RECORDS_MAX_PER_CALL + 1 })).rejects.toMatchObject({ + code: 'MAX_RECORDS_EXCEEDED', + }); + host.limits.length = 0; + + const previewed = await callTool(host, 'delete_records', { objectName: OBJECT, where: {} }); + const { preview } = parsed(previewed); + expect(preview, previewed.content[0]!.text).toMatchObject({ matched: 205, wouldAttempt: 200, cap: 200 }); + + const answer = parsed(await callTool(host, 'delete_records', { objectName: OBJECT, where: {}, [AI_ACTION_CONFIRMATION_MEMBER]: true })); + expect(answer).toMatchObject({ matched: 205, attempted: 200, deleted: 200, remaining: 5, cap: 200 }); + expect(rows.size).toBe(5); + // Every selection read asked for at most the cap. + expect(host.limits.length).toBeGreaterThan(0); + for (const limit of host.limits) expect(limit).toBeLessThanOrEqual(DELETE_RECORDS_MAX_PER_CALL); + }); + + it('exactly the cap matching is answered exactly, with nothing left to continue', async () => { + vi.stubEnv('OS_DATA_MAX_PAGE_SIZE', String(DELETE_RECORDS_MAX_PER_CALL)); + fillRows(rows, DELETE_RECORDS_MAX_PER_CALL); + const host = windowedBridge(bridge); + + const answer = parsed(await callTool(host, 'delete_records', { objectName: OBJECT, where: {}, [AI_ACTION_CONFIRMATION_MEMBER]: true })); + expect(answer).toMatchObject({ matched: 200, attempted: 200, deleted: 200, remaining: 0 }); + expect(answer.next).toBeUndefined(); + expect(rows.size).toBe(0); + }); + + it('a count a SQL driver answers as a decimal string is read as the count', async () => { + fillRows(rows, 203); + const sql = stdioBridge(engineDouble(rows, { countAsString: true }).engine); + + const { preview } = parsed(await callTool(sql, 'delete_records', { objectName: OBJECT, where: {} })); + expect(preview).toMatchObject({ matched: 203, wouldAttempt: 200 }); + expect('matchedMoreThan' in preview).toBe(false); + }); + + it.each([ + ['maximum page size', 'OS_DATA_MAX_PAGE_SIZE'], + ['query row budget', 'OS_DATA_QUERY_ROW_BUDGET'], + ])('a %s under the cap lowers this call\'s cap to it, and the answer says so', async (_label, variable) => { + vi.stubEnv(variable, '50'); + expect(deleteRecordsCap()).toBe(50); + fillRows(rows, 60); + const host = windowedBridge(bridge); + + const { preview } = parsed(await callTool(host, 'delete_records', { objectName: OBJECT, where: {} })); + expect(preview).toMatchObject({ matched: 60, wouldAttempt: 50, cap: 50 }); + + const answer = parsed(await callTool(host, 'delete_records', { objectName: OBJECT, where: {}, [AI_ACTION_CONFIRMATION_MEMBER]: true })); + expect(answer).toMatchObject({ matched: 60, attempted: 50, deleted: 50, remaining: 10, cap: 50 }); + expect(answer.next).toContain('cap of 50'); + for (const limit of host.limits) expect(limit).toBeLessThanOrEqual(50); + }); + + it('an id list longer than a lowered cap is refused before anything is read, and nothing is deleted', async () => { + vi.stubEnv('OS_DATA_MAX_PAGE_SIZE', '2'); + const result = await callTool(bridge, 'delete_records', { + objectName: OBJECT, + recordIds: ['t1', 't3', 't6'], + [AI_ACTION_CONFIRMATION_MEMBER]: true, + }); + + expect(result.isError).toBe(true); + expect(result.content[0]!.text).toContain('at most 2 ids per call'); + expect(double.engine.find).not.toHaveBeenCalled(); + expect(double.engine.delete).not.toHaveBeenCalled(); + expect(rows.size).toBe(6); + }); + }); + + describe('a fault stops the call; a rule\'s refusal does not', () => { + it.each([ + ['an uncoded throw (a code hook\'s bare Error, a lost connection)', new Error('socket hang up'), undefined, undefined], + ['a coded 5xx', Object.assign(new Error('The store is unavailable.'), { code: 'INTERNAL_ERROR', status: 500 }), 'INTERNAL_ERROR', 500], + ['a driver\'s own string code', Object.assign(new Error('connect ECONNREFUSED 127.0.0.1:5432'), { code: 'ECONNREFUSED' }), 'ECONNREFUSED', undefined], + ])('%s ends the call at that row: an error naming it and what was already deleted', async (_label, thrown, code, status) => { + rows.get('t3')!.throws = thrown; + + const result = await callTool(bridge, 'delete_records', { + objectName: OBJECT, + where: { status: 'done' }, + [AI_ACTION_CONFIRMATION_MEMBER]: true, + }); + + expect(result.isError).toBe(true); + const { error, progress } = parsed(result); + expect(error.code).toBe(code); + expect(error.status).toBe(status); + expect(error.message).toContain("stopped at record 't3'"); + expect(error.message).toContain((thrown as Error).message); + expect(error.message).toContain('1 record(s) were deleted before it and stay deleted; 2 were not attempted'); + // t1 deleted, t2 refused by its lock, t3 the fault; t4 and t5 never asked. + expect(progress).toMatchObject({ + matched: 5, + attempted: 3, + deleted: 1, + refused: 1, + refusals: [{ code: 'RECORD_LOCKED', status: 409, count: 1, recordIds: ['t2'] }], + stoppedAt: 't3', + notAttempted: 2, + remaining: 4, + }); + expect(progress.next).toBeUndefined(); + expect(double.engine.delete).toHaveBeenCalledTimes(3); + expect([...rows.keys()]).toEqual(['t2', 't3', 't4', 't5', 't6']); + }); + + it('a sandboxed hook body\'s refusal (no code) is a refusal: grouped as the data door answers it, and the call goes on', async () => { + const sentence = 'This task is approved and cannot be deleted.'; + // The shape the sandbox runner relays a body's own throw in: the body's + // sentence on `innerMessage`, no `code`. + rows.get('t3')!.throws = Object.assign(new Error(`[sandbox] hook body threw: ${sentence}`), { + name: 'SandboxError', + innerMessage: sentence, + }); + + const result = await callTool(bridge, 'delete_records', { + objectName: OBJECT, + recordIds: ['t1', 't3', 't6'], + [AI_ACTION_CONFIRMATION_MEMBER]: true, + }); + + expect(result.isError, result.content[0]?.text).toBeFalsy(); + expect(parsed(result)).toMatchObject({ + attempted: 3, + deleted: 2, + refused: 1, + refusals: [{ code: 'HOOK_ABORTED', status: 400, reason: sentence, count: 1, recordIds: ['t3'] }], + }); + expect([...rows.keys()]).toEqual(['t2', 't3', 't4', 't5']); + }); + }); + 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); diff --git a/packages/mcp/src/mcp-http-tools.scopes.test.ts b/packages/mcp/src/mcp-http-tools.scopes.test.ts index 5af1c392772..7da0a768bdd 100644 --- a/packages/mcp/src/mcp-http-tools.scopes.test.ts +++ b/packages/mcp/src/mcp-http-tools.scopes.test.ts @@ -23,7 +23,10 @@ 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', 'delete_records']; +const WRITE_TOOLS = ['create_record', 'update_record', 'delete_record']; +// [#30891] `delete_records` reads its selection (ids and a count) before it +// deletes, so it registers only when the grant holds BOTH data scopes. +const READ_WRITE_TOOLS = ['delete_records']; const ACTION_TOOLS = ['list_actions', 'run_action']; function makeBridge(): McpDataBridge & McpActionBridge & { calls: any[] } { @@ -75,7 +78,7 @@ describe('MCP OAuth scope → tool-family gating', () => { it('undefined grantedScopes (API key / session) keeps the FULL tool surface', async () => { const names = await listTools(runtime, bridge, undefined); - expect(names).toEqual([...READ_TOOLS, ...WRITE_TOOLS, ...ACTION_TOOLS].sort()); + expect(names).toEqual([...READ_TOOLS, ...WRITE_TOOLS, ...READ_WRITE_TOOLS, ...ACTION_TOOLS].sort()); }); it('data:read alone exposes only the read family', async () => { @@ -83,9 +86,36 @@ describe('MCP OAuth scope → tool-family gating', () => { expect(names).toEqual([...READ_TOOLS].sort()); }); - it('data:write alone exposes only the write family', async () => { + it('data:write alone exposes only the write family, without delete_records', async () => { const names = await listTools(runtime, bridge, [MCP_OAUTH_SCOPE_DATA_WRITE]); expect(names).toEqual([...WRITE_TOOLS].sort()); + expect(names).not.toContain('delete_records'); + }); + + it('data:read with data:write exposes both families and delete_records', async () => { + const names = await listTools(runtime, bridge, [MCP_OAUTH_SCOPE_DATA_READ, MCP_OAUTH_SCOPE_DATA_WRITE]); + expect(names).toEqual([...READ_TOOLS, ...WRITE_TOOLS, ...READ_WRITE_TOOLS].sort()); + }); + + it('[#30891] delete_records with a data:write-only grant is refused WITHOUT reading or deleting', async () => { + const { json } = await call(runtime, bridge, [MCP_OAUTH_SCOPE_DATA_WRITE], { + jsonrpc: '2.0', + id: 5, + method: 'tools/call', + params: { name: 'delete_records', arguments: { objectName: 'task', where: { done: true } } }, + }); + const failed = Boolean(json.error) || json.result?.isError === true; + expect(failed).toBe(true); + // Neither the selection read (the preview's ids and count) nor a delete ran. + expect(bridge.calls.filter((c: any[]) => ['query', 'aggregate', 'remove'].includes(c[0]))).toEqual([]); + // The single-record delete stays callable under the same grant. + const single = await call(runtime, bridge, [MCP_OAUTH_SCOPE_DATA_WRITE], { + jsonrpc: '2.0', + id: 6, + method: 'tools/call', + params: { name: 'delete_record', arguments: { objectName: 'task', recordId: 't1', confirm: true } }, + }); + expect(single.json.result?.isError).toBeFalsy(); }); it('actions:execute alone exposes only the action family', async () => { @@ -100,7 +130,7 @@ describe('MCP OAuth scope → tool-family gating', () => { MCP_OAUTH_SCOPE_DATA_WRITE, MCP_OAUTH_SCOPE_ACTIONS, ]); - expect(names).toEqual([...READ_TOOLS, ...WRITE_TOOLS, ...ACTION_TOOLS].sort()); + expect(names).toEqual([...READ_TOOLS, ...WRITE_TOOLS, ...READ_WRITE_TOOLS, ...ACTION_TOOLS].sort()); }); it('an empty grant registers nothing (fail-closed)', async () => { diff --git a/packages/mcp/src/mcp-http-tools.ts b/packages/mcp/src/mcp-http-tools.ts index 4ca4f8e7818..d06c686d20c 100644 --- a/packages/mcp/src/mcp-http-tools.ts +++ b/packages/mcp/src/mcp-http-tools.ts @@ -50,7 +50,7 @@ import { StandardErrorCode, type ErrorCode } from '@objectstack/spec/api'; // [#23701] The one key every door that translates the engine's unique-violation // envelope asks (the registered code AND the class name, never message text), // and the one classification whose arm states the wire code for it. -import { earnedAnswerOf, isEngineDuplicateRecordEnvelope, staleSummaryAnswerLines, structuredCodeAnswer, type SummaryRecomputeRefusal } from '@objectstack/types'; +import { earnedAnswerOf, isEngineDuplicateRecordEnvelope, mapDataError, resolveQueryReadLimits, staleSummaryAnswerLines, structuredCodeAnswer, type SummaryRecomputeRefusal } from '@objectstack/types'; import { MCP_OAUTH_SCOPE_DATA_READ, MCP_OAUTH_SCOPE_DATA_WRITE, @@ -631,6 +631,23 @@ const DELETE_RECORDS_TOOL = 'delete_records'; */ export const DELETE_RECORDS_MAX_PER_CALL = 200; +/** + * The cap one `delete_records` call runs under: {@link DELETE_RECORDS_MAX_PER_CALL}, + * or the deployment's read window when that is smaller. + * + * The selection is READ before anything is deleted, and the data door's read + * window refuses (never clamps) a read asking for more rows than + * `OS_DATA_MAX_PAGE_SIZE` or walking past `OS_DATA_QUERY_ROW_BUDGET` + * (`@objectstack/types`' `query-read-limits.ts`). A cap above either would + * make every filter selection a refusal on that host, and the caller has no + * `limit` to lower. So the cap follows the window down, and the answer's + * `cap` names the number used. + */ +export function deleteRecordsCap(env?: Record): number { + const { maxPageSize, rowBudget } = resolveQueryReadLimits(env); + return Math.min(DELETE_RECORDS_MAX_PER_CALL, maxPageSize ?? Infinity, rowBudget ?? Infinity); +} + /** How many ids of the selection a preview names. */ const DELETE_RECORDS_SAMPLE_SIZE = 10; @@ -645,6 +662,8 @@ interface DeleteRecordsSelection { /** The selection, resolved under the caller's permissions and row-level security. */ interface ResolvedDeleteSelection { + /** The cap this call runs under ({@link deleteRecordsCap}). */ + cap: number; /** 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. */ @@ -686,17 +705,23 @@ function idOfRow(row: Record): string | undefined { * 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. + * A filter is read in id order, at most the cap. Whether more match than one + * call attempts is then the count's answer, asked of `aggregate` only when the + * read came back full. ⛔ Not a read of one row past the cap: the data door's + * read window REFUSES a read over the host's page size or row budget + * (`OS_DATA_MAX_PAGE_SIZE` / `OS_DATA_QUERY_ROW_BUDGET`, "refuse, never + * clamp"), so a 201-row probe would refuse every filter on a host that sets + * either at 200. A bridge without `aggregate` (or whose count cannot be read) + * keeps the probe, the one way it has to tell "exactly the cap" from "more". + * An id list is at most the cap long (the handler refuses a longer one before + * reading), so one read of those ids answers both which exist and how many. */ async function resolveDeleteSelection( bridge: McpDataBridge, objectName: string, selection: DeleteRecordsSelection, + cap: number, ): Promise { - const cap = DELETE_RECORDS_MAX_PER_CALL; if (selection.recordIds) { const listed = [...new Set(selection.recordIds)]; const rows = queriedRows( @@ -705,22 +730,37 @@ async function resolveDeleteSelection( ); 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)) }; + return { cap, 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: [] }; + const readIds = async (limit: number): Promise => + queriedRows( + await bridge.query(objectName, { where, fields: ['id'], orderBy: [{ field: 'id', order: 'asc' }], limit }), + objectName, + ) + .map(idOfRow) + .filter((id): id is string => id !== undefined); + + if (typeof bridge.aggregate === 'function') { + const ids = await readIds(cap); + if (ids.length < cap) return { cap, attempt: ids, matched: ids.length, moreThanCap: false, notFound: [] }; + const count = await countSelection(bridge, objectName, where); + if (count !== undefined) { + // A count below the rows just read is a write landing between the two + // reads; the read is the floor. + const matched = Math.max(count, ids.length); + return { cap, attempt: ids, matched, moreThanCap: matched > cap, notFound: [] }; + } + } + const probed = await readIds(cap + 1); + const moreThanCap = probed.length > cap; + return { + cap, + attempt: probed.slice(0, cap), + ...(moreThanCap ? {} : { matched: probed.length }), + moreThanCap, + notFound: [], + }; } /** The selection's count under the caller's row-level security, or `undefined` when the bridge cannot say. */ @@ -736,9 +776,65 @@ async function countSelection( }); const value = (rows?.[0] as Record | undefined)?.[DELETE_RECORDS_COUNT_ALIAS]; if (typeof value === 'bigint') return Number(value); + // A SQL driver may answer a COUNT as a decimal string (a 64-bit integer). + if (typeof value === 'string' && /^\d+$/.test(value)) return Number(value); return typeof value === 'number' && Number.isFinite(value) ? value : undefined; } +/** A row the delete stopped at: a fault, not a delete rule's refusal. */ +interface DeleteFault { + recordId: string; + code?: string; + status?: number; + message: string; +} + +/** What {@link deleteEach} did with the attempt set, in order. */ +interface DeleteEachOutcome { + /** Rows the bridge was asked to delete: every row before a fault, and the faulted one. */ + attempted: number; + deleted: number; + refusals: DeleteRefusalGroup[]; + notRemoved: string[]; + /** Set when a fault stopped the walk; the rows after it were not attempted. */ + fault?: DeleteFault; + notAttempted: number; +} + +/** + * A refused row's group key, or `undefined` when the throw is a FAULT. + * + * The line is the one the platform's data door already draws, read with its + * own classifier so the two doors cannot disagree on a row: `mapDataError` + * (`@objectstack/types`), the REST `/data` door's table, which the bulk-import + * runner adopts per row too. A 4xx verdict is a refusal; a 5xx verdict is a + * fault. That covers the coded and the uncoded throw alike: + * + * - a delete rule's coded refusal (`RECORD_LOCKED` / 409, `PERMISSION_DENIED` + * / 403, a package's own code at 422) is a 4xx refusal, grouped under the + * code and message {@link thrownRefusalEnvelope} reads, the spelling + * `delete_record` answers; + * - a sandboxed hook body's deliberate throw carries no code; the door + * answers it `400 HOOK_ABORTED` with the body's own sentence, and so is it + * grouped here; + * - a code hook's bare `throw new Error`, a lost connection, a driver error + * are faults, and so is a driver's OWN string code (`ECONNREFUSED`, a SQL + * state such as `57P01`, `SQLITE_BUSY`): a `code` is not a rule's verdict + * by being a string, and the door answers each of those `500`. + * + * Measured on a booted stack: REST `DELETE` answers the sandboxed refusal + * `400 HOOK_ABORTED` and the code hook's bare throw `500 INTERNAL_ERROR`. + */ +function rowRefusalOf(err: unknown, objectName: string): Omit | undefined { + const door = mapDataError(err, objectName); + if (door.status >= 500) return undefined; + const envelope = thrownRefusalEnvelope(err); + if (envelope) return { code: envelope.code, status: envelope.status ?? door.status, reason: envelope.message }; + const code = typeof door.body.code === 'string' ? door.body.code : undefined; + const reason = typeof door.body.error === 'string' && door.body.error !== '' ? door.body.error : messageOf(err); + return { ...(code !== undefined ? { code } : {}), status: door.status, reason }; +} + /** * 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 @@ -747,22 +843,30 @@ async function countSelection( * 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. + * A refused row stays, and its refusal is grouped: rows refused with the same + * code, status and reason are one group ({@link rowRefusalOf}; a refusal's + * `details` is never read, as the one-row tool renders a throw). 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. + * + * ⛔ A FAULT stops the walk. A driver error or a crash is not a rule's verdict + * on that row: grouped as a "refusal" it would read as the rules' answer, and + * the rows after it would meet the same broken store. So the first fault ends + * the call, and the answer names it, the row it stopped at, and what was + * already deleted (those deletes stay deleted). */ async function deleteEach( bridge: McpDataBridge, objectName: string, ids: readonly string[], -): Promise<{ deleted: number; refusals: DeleteRefusalGroup[]; notRemoved: string[] }> { +): Promise { let deleted = 0; + let attempted = 0; const notRemoved: string[] = []; const groups = new Map(); + let fault: DeleteFault | undefined; for (const id of ids) { + attempted++; try { const answer = await bridge.remove(objectName, id); // `success: false` is the protocol's "matched, and deliberately NOT @@ -775,16 +879,19 @@ async function deleteEach( 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: [], - }; + const refusal = rowRefusalOf(err, objectName); + if (!refusal) { + const envelope = thrownRefusalEnvelope(err); + fault = { + recordId: id, + ...(envelope ? { code: envelope.code } : {}), + ...(envelope?.status !== undefined ? { status: envelope.status } : {}), + message: envelope?.message ?? messageOf(err), + }; + break; + } + const key = JSON.stringify([refusal.code ?? null, refusal.status ?? null, refusal.reason]); + const group = groups.get(key) ?? { ...refusal, count: 0, recordIds: [] }; group.count++; group.recordIds.push(id); groups.set(key, group); @@ -792,12 +899,12 @@ async function deleteEach( } // 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 }; + return { attempted, deleted, refusals, notRemoved, ...(fault ? { fault } : {}), notAttempted: ids.length - attempted }; } /** 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 }; + return resolved.matched !== undefined ? { matched: resolved.matched } : { matchedMoreThan: resolved.cap }; } function deleteRecordsPreview(objectName: string, resolved: ResolvedDeleteSelection) { @@ -805,7 +912,7 @@ function deleteRecordsPreview(objectName: string, resolved: ResolvedDeleteSelect object: objectName, ...selectionCount(resolved), wouldAttempt: resolved.attempt.length, - cap: DELETE_RECORDS_MAX_PER_CALL, + cap: resolved.cap, sampleRecordIds: resolved.attempt.slice(0, DELETE_RECORDS_SAMPLE_SIZE), ...(resolved.notFound.length > 0 ? { notFound: resolved.notFound } : {}), note: @@ -819,33 +926,30 @@ function deleteRecordsPreview(objectName: string, resolved: ResolvedDeleteSelect function deleteRecordsRefusalMessage(objectName: string, resolved: ResolvedDeleteSelection): string { const count = resolved.matched !== undefined ? `${resolved.matched} record(s)` - : `more than ${DELETE_RECORDS_MAX_PER_CALL} records`; + : `more than ${resolved.cap} 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 ` + + `a confirmed call deletes up to ${resolved.cap} 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[] }, -) { +function deleteRecordsOutcome(objectName: string, resolved: ResolvedDeleteSelection, done: DeleteEachOutcome) { 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. + // match: the refused, the not removed, the faulted, the not attempted 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) { + if (resolved.moreThanCap && !done.fault) { 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.`, + + `${resolved.cap}: call again with the same selection and confirm: true to continue.`, ); } - if (refused > 0 && resolved.moreThanCap) { + if (refused > 0 && resolved.moreThanCap && !done.fault) { 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).', @@ -854,18 +958,34 @@ function deleteRecordsOutcome( return { object: objectName, ...selectionCount(resolved), - attempted: resolved.attempt.length, + attempted: done.attempted, deleted: done.deleted, refused, refusals: done.refusals, ...(done.notRemoved.length > 0 ? { notRemoved: done.notRemoved } : {}), ...(resolved.notFound.length > 0 ? { notFound: resolved.notFound } : {}), + ...(done.fault ? { stoppedAt: done.fault.recordId, notAttempted: done.notAttempted } : {}), ...(remaining !== undefined ? { remaining } : {}), - cap: DELETE_RECORDS_MAX_PER_CALL, + cap: resolved.cap, ...(next.length > 0 ? { next: next.join(' ') } : {}), }; } +/** + * The tool error a fault answers: the fault itself (its code and status when + * it declared them), the row it stopped at, and the call's progress, since + * the records deleted before it stay deleted and an agent that read a bare + * error would retry them as if nothing had happened. + */ +function deleteRecordsFaultMessage(objectName: string, done: DeleteEachOutcome & { fault: DeleteFault }): string { + return ( + `delete_records on '${objectName}' stopped at record '${done.fault.recordId}': ${done.fault.message} ` + + 'This is a fault, not a delete rule\'s refusal, so no record after it was attempted. ' + + `${done.deleted} record(s) were deleted before it and stay deleted; ${done.notAttempted} were not attempted. ` + + 'Retry the same call once the fault is resolved: it selects what still matches.' + ); +} + /** What {@link thrownRefusalEnvelope} reads off a thrown coded refusal. */ export interface ThrownRefusalEnvelope { code: string; @@ -1906,126 +2026,155 @@ 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', + // [#30891] Registered under BOTH data scopes. Its unconfirmed call reads + // the selection (ids and a count) through `query` / `aggregate`, a + // list-class read; under `data:write` alone that read would serve ids a + // token granted no `data:read` cannot list (seat decision on the review). + if (canRead) { + // [#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 (fewer when the ` + + 'deployment\'s read page size or row budget is smaller; the answer\'s cap says), 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), + { + 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.', ); - 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); - } - }, - ); + try { + const cap = deleteRecordsCap(); + if (recordIds !== undefined && new Set(recordIds).size > cap) { + // Before any read: the read of a longer list would be the read + // window's own refusal, naming a `limit` this caller never sent. + return errorResult( + `delete_records takes at most ${cap} ids per call on this deployment (its read page size or row ` + + `budget is ${cap}) — nothing was deleted. Send the list in parts of at most ${cap} ids.`, + ); + } + const resolved = await resolveDeleteSelection(bridge, objectName, { where, recordIds }, cap); + // [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); + const outcome = stated(deleteRecordsOutcome(objectName, resolved, done)); + if (done.fault) { + const { fault } = done; + const error = { + ...(fault.code !== undefined ? { code: fault.code } : {}), + message: deleteRecordsFaultMessage(objectName, { ...done, fault }), + ...(fault.status !== undefined ? { status: fault.status } : {}), + }; + return { + content: [{ type: 'text' as const, text: jsonText({ error, progress: outcome }) }], + isError: true as const, + }; + } + return textResult(outcome); + } catch (err) { + return errorResultWithCode(err); + } + }, + ); + } } // end canWrite (data:write) return registered; 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 3f6ea9dfe95..20b41b70031 100644 --- a/packages/mcp/src/mcp-server-runtime.authoring-scope.test.ts +++ b/packages/mcp/src/mcp-server-runtime.authoring-scope.test.ts @@ -53,9 +53,11 @@ 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', 'delete_records']; +const WRITE_TOOLS = ['create_record', 'update_record', 'delete_record']; +/** [#30891] Registered only when the grant holds both data:read and data:write. */ +const READ_WRITE_TOOLS = ['delete_records']; const ACTION_TOOLS = ['list_actions', 'run_action']; -const NATIVE_TOOLS = [...READ_TOOLS, ...WRITE_TOOLS, ...ACTION_TOOLS].sort(); +const NATIVE_TOOLS = [...READ_TOOLS, ...WRITE_TOOLS, ...READ_WRITE_TOOLS, ...ACTION_TOOLS].sort(); /** The shape a host injects: names the host chose, none of them native. */ const AUTHORING_TOOLS: AIToolDefinition[] = [ diff --git a/packages/mcp/src/skill-md.ts b/packages/mcp/src/skill-md.ts index 16a3f9abbb2..f4e630b2911 100644 --- a/packages/mcp/src/skill-md.ts +++ b/packages/mcp/src/skill-md.ts @@ -297,7 +297,10 @@ create/update payload. 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. + match. Call again to continue. The answer's \`cap\` is the call's real cap + (lower on a deployment with a smaller read page). A fault (not a rule's refusal) stops the call + and names the record it stopped at. Over OAuth it needs both \`data:read\` and + \`data:write\`. - **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/qa/dogfood/test/mcp-delete-records.dogfood.test.ts b/packages/qa/dogfood/test/mcp-delete-records.dogfood.test.ts index 3a473fc05fa..28cb06e5962 100644 --- a/packages/qa/dogfood/test/mcp-delete-records.dogfood.test.ts +++ b/packages/qa/dogfood/test/mcp-delete-records.dogfood.test.ts @@ -24,9 +24,15 @@ // • 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). +// next call deletes the 3 (the count is the runtime's real aggregate), on +// a host whose maximum page size IS the cap, so a read one row past it is +// refused; and a host whose page size is under the cap runs at that size; +// • a rule's refusal vs a fault: a sandboxed hook body's refusal (no code) +// is grouped as REST answers it, `400 HOOK_ABORTED`, and the call goes on; +// a code hook's bare `throw new Error` is REST's `500`, a fault, and ends +// the call at that row with what was already deleted. -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import { bootStack, type VerifyStack } from '@objectstack/verify'; import { defineStack } from '@objectstack/spec'; import { ObjectSchema, Field } from '@objectstack/spec/data'; @@ -35,7 +41,10 @@ import { AI_ACTION_CONFIRMATION_MEMBER } from '@objectstack/spec/contracts'; const LOCKED = 'dlr_close_task'; const BULK = 'dlr_bulk_row'; +const GUARDED = 'dlr_guarded_row'; const LOCK_SENTENCE = 'This task belongs to a closed period, so it can no longer be deleted.'; +const APPROVED_SENTENCE = 'This row is approved, so it can no longer be deleted.'; +const FAULT_SENTENCE = 'dlr ledger store went away mid-write'; const CloseTask = ObjectSchema.create({ name: LOCKED, @@ -56,6 +65,16 @@ const BulkRow = ObjectSchema.create({ }, }); +const GuardedRow = ObjectSchema.create({ + name: GUARDED, + label: 'Guarded row', + fields: { + name: Field.text({ label: 'Name', required: true }), + approved: Field.boolean({ label: 'Approved' }), + broken: Field.boolean({ label: 'Broken' }), + }, +}); + const fixture = defineStack({ manifest: { id: 'com.dogfood.mcp-delete-records', @@ -65,7 +84,7 @@ const fixture = defineStack({ 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], + objects: [CloseTask, BulkRow, GuardedRow], hooks: [ { // A package's locking rule, in HotCRM's shape: a USER's delete of a @@ -79,11 +98,44 @@ const fixture = defineStack({ throw Object.assign(new Error(LOCK_SENTENCE), { code: 'RECORD_LOCKED', status: 409 }); }, }, + { + // A package's rule written as a sandboxed body: its refusal declares no code. + name: 'dlr_approved_guard', + object: GUARDED, + events: ['beforeDelete'], + body: { + language: 'js', + source: `if (ctx.previous && ctx.previous.approved === true) throw new Error(${JSON.stringify(APPROVED_SENTENCE)});`, + }, + }, + { + // A code hook that crashes on one row: not a rule's verdict, a fault. + name: 'dlr_broken_store', + object: GUARDED, + events: ['beforeDelete'], + handler: async (ctx: any) => { + if (ctx.previous?.broken === true) throw new Error(FAULT_SENTENCE); + }, + }, ], }); const idOf = (b: any) => b?.id ?? b?.record?.id ?? b?.data?.id; +/** Run `body` with the deployment's read window set (read per request by the data door), then restore it. */ +async function withReadWindow(vars: Record, body: () => Promise): Promise { + const before = Object.fromEntries(Object.keys(vars).map((k) => [k, process.env[k]])); + Object.assign(process.env, vars); + try { + return await body(); + } finally { + for (const [k, v] of Object.entries(before)) { + if (v === undefined) delete process.env[k]; + else process.env[k] = v; + } + } +} + 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; @@ -188,26 +240,121 @@ describe('MCP delete_records on a real stack: preview, grouped refusals, an admi }, 60_000); }); - it('the cap: 203 matching rows, one call deletes 200 and says 3 remain, the next call deletes the 3', async () => { + it('the cap, on a host whose maximum page size IS the cap: 203 rows, one call deletes 200 and says 3 remain, the next 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 }); + await withReadWindow({ OS_DATA_MAX_PAGE_SIZE: '200' }, async () => { + // Lit control: this host refuses a read one row past the cap. + const over = await stack.apiAs(admin, 'GET', `/data/${BULK}?limit=201`); + expect(over.status, await over.clone().text()).toBe(400); + expect(await over.text()).toContain('MAX_RECORDS_EXCEEDED'); - 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 preview = await tool('delete_records', { objectName: BULK, where: { batch: 'cap' } }); + expect(preview.isError).toBe(true); + expect(preview.payload.preview, preview.text).toMatchObject({ matched: 203, wouldAttempt: 200, cap: 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 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 after = await tool('delete_records', { objectName: BULK, where: { batch: 'cap' } }); - expect(after.payload.preview).toMatchObject({ matched: 0, wouldAttempt: 0 }); + 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); + + it('a host whose maximum page size is under the cap: the call runs at that size and says so', async () => { + const ids = [ + await create(BULK, { name: 'small 1', batch: 'small' }), + await create(BULK, { name: 'small 2', batch: 'small' }), + await create(BULK, { name: 'small 3', batch: 'small' }), + ]; + + await withReadWindow({ OS_DATA_MAX_PAGE_SIZE: '2' }, async () => { + const preview = await tool('delete_records', { objectName: BULK, where: { batch: 'small' } }); + expect(preview.payload.preview, preview.text).toMatchObject({ matched: 3, wouldAttempt: 2, cap: 2 }); + + const done = await tool('delete_records', { objectName: BULK, where: { batch: 'small' }, [AI_ACTION_CONFIRMATION_MEMBER]: true }); + expect(done.isError, done.text).toBe(false); + expect(done.payload).toMatchObject({ matched: 3, attempted: 2, deleted: 2, remaining: 1, cap: 2 }); + + const tooMany = await tool('delete_records', { objectName: BULK, recordIds: ids, [AI_ACTION_CONFIRMATION_MEMBER]: true }); + expect(tooMany.isError).toBe(true); + expect(tooMany.text).toContain('at most 2 ids per call'); + }); + const left = (await Promise.all(ids.map((id) => readStatus(BULK, id)))).filter((s) => s === 200); + expect(left).toHaveLength(1); + }, 120_000); + + describe('a rule\'s refusal is grouped; a fault ends the call', () => { + let plain: string[]; + let approved: string; + let broken: string; + + beforeAll(async () => { + plain = [ + await create(GUARDED, { name: 'plain 1' }), + await create(GUARDED, { name: 'plain 2' }), + await create(GUARDED, { name: 'plain 3' }), + ]; + approved = await create(GUARDED, { name: 'approved', approved: true }); + broken = await create(GUARDED, { name: 'broken', broken: true }); + }, 60_000); + + it('control: REST DELETE answers the body\'s refusal 400 HOOK_ABORTED and the code hook\'s crash 500', async () => { + const refused = await stack.apiAs(admin, 'DELETE', `/data/${GUARDED}/${approved}`); + const refusedBody = await refused.text(); + expect(refused.status, refusedBody).toBe(400); + expect(refusedBody).toContain('HOOK_ABORTED'); + expect(refusedBody).toContain(APPROVED_SENTENCE); + + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + const crashed = await stack.apiAs(admin, 'DELETE', `/data/${GUARDED}/${broken}`); + expect(crashed.status, await crashed.clone().text()).toBe(500); + } finally { + errors.mockRestore(); + } + expect(await readStatus(GUARDED, approved)).toBe(200); + expect(await readStatus(GUARDED, broken)).toBe(200); + }, 60_000); + + it('confirmed: the refusal is grouped and the walk goes on; the fault stops it, naming the row and what was deleted', async () => { + const done = await tool('delete_records', { + objectName: GUARDED, + recordIds: [plain[0], approved, plain[1], broken, plain[2]], + [AI_ACTION_CONFIRMATION_MEMBER]: true, + }); + + expect(done.isError, done.text).toBe(true); + const { error, progress } = done.payload; + expect(error.message).toContain(`stopped at record '${broken}'`); + expect(error.message).toContain(FAULT_SENTENCE); + expect(error.message).toContain('2 record(s) were deleted before it and stay deleted; 1 were not attempted'); + expect(progress).toMatchObject({ + matched: 5, + attempted: 4, + deleted: 2, + refused: 1, + refusals: [{ code: 'HOOK_ABORTED', status: 400, reason: APPROVED_SENTENCE, count: 1, recordIds: [approved] }], + stoppedAt: broken, + notAttempted: 1, + }); + + // Ground truth over REST. + expect(await readStatus(GUARDED, plain[0]!)).toBe(404); + expect(await readStatus(GUARDED, plain[1]!)).toBe(404); + expect(await readStatus(GUARDED, approved)).toBe(200); + expect(await readStatus(GUARDED, broken)).toBe(200); + // Past the fault: never attempted. + expect(await readStatus(GUARDED, plain[2]!)).toBe(200); + }, 60_000); + }); }); diff --git a/packages/spec/src/api/error-code-ledger.zod.ts b/packages/spec/src/api/error-code-ledger.zod.ts index b09882d2e40..b9868da9c3c 100644 --- a/packages/spec/src/api/error-code-ledger.zod.ts +++ b/packages/spec/src/api/error-code-ledger.zod.ts @@ -369,8 +369,8 @@ export const ERROR_CODE_LEDGER = { // not a residue. Registered HERE and not under a producing service for the // same reason as its `FLOW_*` neighbours: the action door, not the // predicate, is where the wire vocabulary is named. - // Second emitter: the MCP `delete_record` tool's gate - // (`recordDeleteConfirmationRefusal`, `packages/mcp/src/mcp-http-tools.ts`) + // Second emitter: the MCP `delete_record` and `delete_records` tools' gate + // (`deleteConfirmationRefusal`, `packages/mcp/src/mcp-http-tools.ts`) // answers this code, 428, with the same `details` shape for every record // delete without the member; listed under `@objectstack/mcp` too. 'ACTION_CONFIRMATION_REQUIRED', @@ -1444,13 +1444,15 @@ export const ERROR_CODE_LEDGER = { // for the data door. 'OBJECT_API_DISABLED', 'OBJECT_API_METHOD_NOT_ALLOWED', - // The `delete_record` tool's confirmation gate (`mcp-http-tools.ts`, - // `recordDeleteConfirmationRefusal`): a record delete without the - // confirmation member as the boolean `true` is refused before any bridge - // is reached, 428, `details` shaped as `ActionConfirmationRequiredDetails` - // with `actionName: 'delete_record'`. Rendered by the tools' own coded - // result on both transports. Second EMITTER of the code `@objectstack/runtime` - // registers for `run_action`'s gate: provenance, not identity. + // The `delete_record` and `delete_records` tools' confirmation gate + // (`mcp-http-tools.ts`, `deleteConfirmationRefusal`): a record delete + // without the confirmation member as the boolean `true` is refused, 428, + // `details` shaped as `ActionConfirmationRequiredDetails` with `actionName` + // the tool's name. `delete_record` refuses before any bridge is reached; + // `delete_records` reads its selection first and answers a `preview` + // beside the refusal, deleting nothing. Rendered on both transports. + // Second EMITTER of the code `@objectstack/runtime` registers for + // `run_action`'s gate: provenance, not identity. 'ACTION_CONFIRMATION_REQUIRED', ], '@objectstack/service-datasource': [ From 47cce5368ff0304767f1d611158571390475639a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 18:58:18 +0000 Subject: [PATCH 7/8] fix(console): the downloaded SKILL.md mirrors the server's delete_records bullet #31015's drift pin compares the console's renderObjectStackSkill with the MCP server's renderSkillMarkdown byte for byte; the server's SKILL.md now teaches delete_records, so the console copy carries the same bullet and the tool-name pin lists it. The server-side bullet is reflowed to the file's line width; its words are unchanged. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibx1NhZMrqCvW7FBxg5 --- .changeset/30891-mcp-delete-records.md | 3 +++ packages/mcp/src/skill-md.ts | 14 +++++++------- .../src/pages/developer/objectstack-skill.test.ts | 1 + .../src/pages/developer/objectstack-skill.ts | 12 ++++++++++++ 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/.changeset/30891-mcp-delete-records.md b/.changeset/30891-mcp-delete-records.md index afc43881f6a..b123da229c6 100644 --- a/.changeset/30891-mcp-delete-records.md +++ b/.changeset/30891-mcp-delete-records.md @@ -1,6 +1,7 @@ --- '@objectstack/mcp': patch '@objectstack/spec': patch +'@object-ui/console': patch --- feat(mcp): `delete_records` deletes many records of one object in one confirmed call, by a filter or an id list (#30891) @@ -18,4 +19,6 @@ An agent that had to clear a set of records called `delete_record` once per row, 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. +`@object-ui/console`: the Integrations page's "Download skill" SKILL.md is a byte-for-byte mirror of the server's (#31015's drift pin), so it carries the same `delete_records` bullet. + `@objectstack/spec` changes only doc comments: `AIActionConfirmation` names `delete_records` as the third door of the confirmation member, and the error-code ledger's `ACTION_CONFIRMATION_REQUIRED` rows name both delete tools. diff --git a/packages/mcp/src/skill-md.ts b/packages/mcp/src/skill-md.ts index f4e630b2911..6b7cb2f4280 100644 --- a/packages/mcp/src/skill-md.ts +++ b/packages/mcp/src/skill-md.ts @@ -294,13 +294,13 @@ create/update payload. 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. The answer's \`cap\` is the call's real cap - (lower on a deployment with a smaller read page). A fault (not a rule's refusal) stops the call - and names the record it stopped at. Over OAuth it needs both \`data:read\` and - \`data:write\`. + A confirmed call deletes up to ${DELETE_RECORDS_MAX_PER_CALL} records (fewer on a deployment with + a smaller read page; the answer's \`cap\` says), 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. A fault (not a rule's refusal) + stops the call and names the record it stopped at. Over OAuth it needs both + \`data:read\` and \`data:write\`. - **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/ui/apps/console/src/pages/developer/objectstack-skill.test.ts b/ui/apps/console/src/pages/developer/objectstack-skill.test.ts index 3aa94585a25..883f7c9b92d 100644 --- a/ui/apps/console/src/pages/developer/objectstack-skill.test.ts +++ b/ui/apps/console/src/pages/developer/objectstack-skill.test.ts @@ -66,6 +66,7 @@ describe('renderObjectStackSkill', () => { 'create_record', 'update_record', 'delete_record', + 'delete_records', 'list_actions', 'run_action', 'resume_run', diff --git a/ui/apps/console/src/pages/developer/objectstack-skill.ts b/ui/apps/console/src/pages/developer/objectstack-skill.ts index 8612fdae097..062bde10821 100644 --- a/ui/apps/console/src/pages/developer/objectstack-skill.ts +++ b/ui/apps/console/src/pages/developer/objectstack-skill.ts @@ -144,6 +144,18 @@ 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 200 records (fewer on a deployment with + a smaller read page; the answer's \`cap\` says), 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. A fault (not a rule's refusal) + stops the call and names the record it stopped at. Over OAuth it needs both + \`data:read\` and \`data:write\`. - **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 From 4e5b0f2dc8e8b3e32eda6679524596fd1057b8dd Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 19:06:38 +0000 Subject: [PATCH 8/8] test(mcp): the write-only delete_records pin types the answers it reads The two new reads in the data:write-only refusal pin were of type unknown, which grew the package's test-typecheck debt; they are cast to the JSON-RPC shapes they read, as the rest of the file's new code should be. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L9qibx1NhZMrqCvW7FBxg5 --- packages/mcp/src/mcp-http-tools.scopes.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/mcp/src/mcp-http-tools.scopes.test.ts b/packages/mcp/src/mcp-http-tools.scopes.test.ts index 7da0a768bdd..440e27727bc 100644 --- a/packages/mcp/src/mcp-http-tools.scopes.test.ts +++ b/packages/mcp/src/mcp-http-tools.scopes.test.ts @@ -98,23 +98,23 @@ describe('MCP OAuth scope → tool-family gating', () => { }); it('[#30891] delete_records with a data:write-only grant is refused WITHOUT reading or deleting', async () => { - const { json } = await call(runtime, bridge, [MCP_OAUTH_SCOPE_DATA_WRITE], { + const { json } = (await call(runtime, bridge, [MCP_OAUTH_SCOPE_DATA_WRITE], { jsonrpc: '2.0', id: 5, method: 'tools/call', params: { name: 'delete_records', arguments: { objectName: 'task', where: { done: true } } }, - }); + })) as { json: { error?: unknown; result?: { isError?: boolean } } }; const failed = Boolean(json.error) || json.result?.isError === true; expect(failed).toBe(true); // Neither the selection read (the preview's ids and count) nor a delete ran. expect(bridge.calls.filter((c: any[]) => ['query', 'aggregate', 'remove'].includes(c[0]))).toEqual([]); // The single-record delete stays callable under the same grant. - const single = await call(runtime, bridge, [MCP_OAUTH_SCOPE_DATA_WRITE], { + const single = (await call(runtime, bridge, [MCP_OAUTH_SCOPE_DATA_WRITE], { jsonrpc: '2.0', id: 6, method: 'tools/call', params: { name: 'delete_record', arguments: { objectName: 'task', recordId: 't1', confirm: true } }, - }); + })) as { json: { result?: { isError?: boolean } } }; expect(single.json.result?.isError).toBeFalsy(); });