From 6e367027e41736de91799da8f4dc6b574ac0f7a5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 05:32:53 +0000 Subject: [PATCH 1/6] feat(mcp)!: delete_record refuses a call without confirm: true The MCP delete_record tool takes run_action's confirmation gate: a call that does not carry the confirmation member as the boolean true is refused with ACTION_CONFIRMATION_REQUIRED (428) and the run_action refusal's details shape, before any bridge is reached, so nothing is deleted. The member is declared on the tool schema (optional, so the handler answers the coded refusal) and the description tells the agent to ask the human first and then retry. Both transports gate: they register their tools through wireBridgeTools. Tests that drove delete_record to reach the bridge now send confirm: true. Claude-Session: https://claude.ai/code/session_016WV4czsSxuhcKFSWS5tLdW Co-authored-by: Claude --- packages/mcp/README.md | 2 +- packages/mcp/src/mcp-coded-refusal.test.ts | 3 +- .../mcp-delete-record-confirmation.test.ts | 289 ++++++++++++++++++ packages/mcp/src/mcp-http-tools.ts | 87 +++++- ...p-http-tools.unknown-argument-keys.test.ts | 3 +- .../mcp/src/mcp-server-runtime.http.test.ts | 2 +- .../mcp-unique-violation-wire-code.test.ts | 2 +- ...update-delete-record-stale-summary.test.ts | 4 +- packages/mcp/src/skill-md.ts | 6 +- ...delete-record-confirmation.dogfood.test.ts | 137 +++++++++ 10 files changed, 523 insertions(+), 12 deletions(-) create mode 100644 packages/mcp/src/mcp-delete-record-confirmation.test.ts create mode 100644 packages/qa/dogfood/test/mcp-delete-record-confirmation.dogfood.test.ts diff --git a/packages/mcp/README.md b/packages/mcp/README.md index 9f5448148d0..b709783bc4b 100644 --- a/packages/mcp/README.md +++ b/packages/mcp/README.md @@ -117,7 +117,7 @@ apply): 'get_record' // Fetch one by id 'create_record' // Create 'update_record' // Update by id -'delete_record' // Delete by id (destructive) +'delete_record' // Delete by id (destructive; refused without confirm: true) // Business actions — operate the app, not just its rows 'list_actions' // Invokable business actions the caller may run diff --git a/packages/mcp/src/mcp-coded-refusal.test.ts b/packages/mcp/src/mcp-coded-refusal.test.ts index b44827928bb..8890ffc8742 100644 --- a/packages/mcp/src/mcp-coded-refusal.test.ts +++ b/packages/mcp/src/mcp-coded-refusal.test.ts @@ -100,7 +100,8 @@ const SITES: readonly Site[] = [ { tool: 'get_record', args: { objectName: 'task', recordId: 't9' }, member: 'get' }, { tool: 'create_record', args: { objectName: 'task', data: { title: 'x' } }, member: 'create' }, { tool: 'update_record', args: { objectName: 'task', recordId: 't9', data: { title: 'x' } }, member: 'update' }, - { tool: 'delete_record', args: { objectName: 'task', recordId: 't9' }, member: 'remove' }, + // `confirm: true` passes the confirmation gate, so the call reaches `remove`. + { tool: 'delete_record', args: { objectName: 'task', recordId: 't9', confirm: true }, member: 'remove' }, { 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-delete-record-confirmation.test.ts b/packages/mcp/src/mcp-delete-record-confirmation.test.ts new file mode 100644 index 00000000000..8001819ba8f --- /dev/null +++ b/packages/mcp/src/mcp-delete-record-confirmation.test.ts @@ -0,0 +1,289 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The MCP `delete_record` tool takes `run_action`'s confirmation gate: a call + * without `confirm: true` is REFUSED with `ACTION_CONFIRMATION_REQUIRED` and + * nothing is deleted. + * + * ## Why + * + * A delete over MCP is hard and permanent, and before this gate one call did + * it: `annotations.destructiveHint` is a hint a client may ignore, and the + * bridge's own checks (API exposure, credentials, the caller's permissions) + * say whether the caller MAY delete, never whether a human asked for it. An + * instruction planted in a record's text could lead an agent to delete rows + * its user may delete with no human asked. The maintainer chose option B: the + * server-side backstop `run_action` already has, widened to this one tool, + * with the same member, code, status and `details`. + * + * ## What is pinned, and where + * + * - The door, over the HTTP `/mcp` transport, against the REAL stdio data + * bridge on an engine double that holds the row: the member is advertised, + * an unconfirmed call (absent, `false`, the string `'true'`) is refused and + * the row is still there, a confirmed call deletes it. + * - The refusal's code, status and `details` keys are `run_action`'s, served + * through the same door from a bridge throwing what the runtime gate + * throws. The real runtime gate beside the real `delete_record` refusal, and + * REST `DELETE /data/:object/:id` (outside the gate), are driven together in + * `packages/qa/dogfood/test/mcp-delete-record-confirmation.dogfood.test.ts`. + * - The long-lived server the stdio transport serves gates too: both + * transports register their tools through `wireBridgeTools`. + * - Controls: `update_record` is not gated, and the bridge is never reached + * by a refused call. + */ + +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 type { McpActionBridge, McpDataBridge } from './mcp-http-tools.js'; + +const OBJECT = 'dlc_note'; + +const NOTE = { + name: OBJECT, + label: 'Note', + fields: { + id: { name: 'id', type: 'text' }, + title: { name: 'title', type: 'text' }, + }, + enable: {}, +}; + +/** A store of `dlc_note` rows behind an engine that applies every write it is handed. */ +function engineDouble(rows: Map>) { + return { + find: vi.fn(async (_o: string, query?: any) => { + const id = query?.where?.id; + const all = [...rows.values()].filter((r) => id === undefined || r.id === id); + 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); + const id = String(options?.where?.id); + rows.set(id, { ...rows.get(id), ...data }); + return rows.get(id); + }), + delete: vi.fn(async (_o: string, options?: any) => { + assertEngineDeleteDispatch(options); + rows.delete(String(options?.where?.id)); + return { deleted: 1 }; + }), + count: vi.fn(async () => rows.size), + }; +} + +function metadataDouble(): IMetadataService { + return { + listObjects: vi.fn(async () => [NOTE]), + getObject: vi.fn(async () => NOTE), + } as unknown as IMetadataService; +} + +function stdioBridge(engine: ReturnType): McpDataBridge { + return createStdioDataBridge({ + engine: engine as unknown as IDataEngine, + metadataService: metadataDouble(), + resolvePrincipal: async () => ({ userId: 'u1' }) as unknown as ExecutionContext, + }); +} + +/** What the runtime's `run_action` gate throws for an author-gated action. */ +function runtimeGateRefusal(): Error { + return Object.assign( + new Error( + "Action 'archive_note' on 'dlc_note' declares ai.requiresConfirmation: true — nothing was run. " + + "Confirm with the human in the loop, then retry this call with 'confirm': true on the request.", + ), + { + code: 'ACTION_CONFIRMATION_REQUIRED', + status: 428, + details: { actionName: 'archive_note', objectName: OBJECT, confirmationMember: AI_ACTION_CONFIRMATION_MEMBER }, + }, + ); +} + +let nextId = 1; +async function rpc(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); + const json = (await res.json()) as { result?: any; error?: unknown }; + expect(json.error, JSON.stringify(json.error)).toBeUndefined(); + return json.result; +} + +async function callTool(bridge: McpDataBridge, name: string, args: Record) { + return (await rpc(bridge, 'tools/call', { name, arguments: args })) as { + content: Array<{ text: string }>; + isError?: boolean; + }; +} + +const envelopeOf = (result: { content: Array<{ text: string }> }) => JSON.parse(result.content[0]!.text) as { error: any }; + +describe('MCP delete_record refuses a call without confirm: true (ACTION_CONFIRMATION_REQUIRED)', () => { + let rows: Map>; + let engine: ReturnType; + let bridge: McpDataBridge; + + beforeEach(() => { + rows = new Map([['n1', { id: 'n1', title: 'keep me' }]]); + engine = engineDouble(rows); + bridge = stdioBridge(engine); + }); + + it('advertises the confirmation member on the tool schema, so a model can discover the retry', async () => { + const listed = await rpc(bridge, 'tools/list'); + const tool = (listed.tools as any[]).find((t) => t.name === 'delete_record'); + expect(tool, 'delete_record is registered').toBeDefined(); + const props = tool.inputSchema.properties; + // Control: the members that were already there are still advertised. + expect(Object.keys(props)).toEqual(expect.arrayContaining(['objectName', 'recordId'])); + expect(props[AI_ACTION_CONFIRMATION_MEMBER]?.type).toBe('boolean'); + // Optional in the schema: a call without it must reach the handler, which + // answers the coded refusal. Required there, it would be refused as a bare + // input-validation error naming no code and no retry. + expect(tool.inputSchema.required ?? []).not.toContain(AI_ACTION_CONFIRMATION_MEMBER); + // The description tells the agent the order: ask the human, then retry. + expect(tool.description).toContain('confirm: true'); + expect(tool.description).toMatch(/ask the human/i); + expect(tool.annotations?.destructiveHint).toBe(true); + }); + + it('refuses a call without the member: code, status and the retry details; the record is intact', async () => { + const result = await callTool(bridge, 'delete_record', { objectName: OBJECT, recordId: 'n1' }); + + expect(result.isError).toBe(true); + const { error } = envelopeOf(result); + expect(error.code).toBe('ACTION_CONFIRMATION_REQUIRED'); + expect(error.status).toBe(428); + expect(error.details).toEqual({ + actionName: 'delete_record', + objectName: OBJECT, + confirmationMember: AI_ACTION_CONFIRMATION_MEMBER, + }); + expect(String(error.message)).toContain('nothing was deleted'); + + // Nothing was deleted: the engine was never asked, and the row is there. + expect(engine.delete).not.toHaveBeenCalled(); + expect(rows.get('n1')).toEqual({ id: 'n1', title: 'keep me' }); + }); + + it.each([ + ['false', false], + ['the string "true"', 'true'], + ['1', 1], + ])('refuses %s as the member: only the boolean true attests, and the record is intact', async (_label, value) => { + const result = await callTool(bridge, 'delete_record', { + objectName: OBJECT, + recordId: 'n1', + [AI_ACTION_CONFIRMATION_MEMBER]: value, + }); + + expect(result.isError).toBe(true); + expect(engine.delete).not.toHaveBeenCalled(); + expect(rows.has('n1')).toBe(true); + }); + + it('deletes on the retry with confirm: true, once', async () => { + const result = await callTool(bridge, 'delete_record', { + objectName: OBJECT, + recordId: 'n1', + [AI_ACTION_CONFIRMATION_MEMBER]: true, + }); + + expect(result.isError, result.content[0]?.text).toBeFalsy(); + expect(JSON.parse(result.content[0]!.text)).toEqual({ object: OBJECT, id: 'n1', success: true }); + expect(engine.delete).toHaveBeenCalledTimes(1); + expect(rows.has('n1')).toBe(false); + }); + + it('answers with run_action\'s refusal shape: the same code, status, envelope keys and details keys', async () => { + const actionBridge: McpDataBridge & McpActionBridge = { + ...bridge, + async listActions() { + return [{ name: 'archive_note', objectName: OBJECT, requiresRecord: true, requiresConfirmation: true }]; + }, + async runAction() { + throw runtimeGateRefusal(); + }, + }; + + const viaAction = envelopeOf(await callTool(actionBridge, 'run_action', { actionName: 'archive_note', objectName: OBJECT, recordId: 'n1' })).error; + const viaDelete = envelopeOf(await callTool(actionBridge, 'delete_record', { objectName: OBJECT, recordId: 'n1' })).error; + + // Lit control: the run_action half really is the gate's refusal. + expect(viaAction.code).toBe('ACTION_CONFIRMATION_REQUIRED'); + + expect(viaDelete.code).toBe(viaAction.code); + expect(viaDelete.status).toBe(viaAction.status); + expect(Object.keys(viaDelete)).toEqual(Object.keys(viaAction)); + expect(Object.keys(viaDelete.details).sort()).toEqual(Object.keys(viaAction.details).sort()); + expect(viaDelete.details.confirmationMember).toBe(viaAction.details.confirmationMember); + expect(viaDelete.details.objectName).toBe(viaAction.details.objectName); + expect(rows.has('n1')).toBe(true); + }); + + it('control: update_record is not gated, and writes without the member', async () => { + const result = await callTool(bridge, 'update_record', { objectName: OBJECT, recordId: 'n1', data: { title: 'changed' } }); + + expect(result.isError, result.content[0]?.text).toBeFalsy(); + expect(engine.update).toHaveBeenCalledTimes(1); + expect(rows.get('n1')).toMatchObject({ title: 'changed' }); + }); + + it('the long-lived server the stdio transport serves gates 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_record', arguments: { objectName: OBJECT, recordId: 'n1' } }); + expect(refused.isError).toBe(true); + const { error } = JSON.parse(refused.content[0].text); + expect(error.code).toBe('ACTION_CONFIRMATION_REQUIRED'); + expect(error.status).toBe(428); + expect(error.details).toEqual({ actionName: 'delete_record', objectName: OBJECT, confirmationMember: AI_ACTION_CONFIRMATION_MEMBER }); + expect(rows.has('n1')).toBe(true); + + const confirmed: any = await client.callTool({ + name: 'delete_record', + arguments: { objectName: OBJECT, recordId: 'n1', [AI_ACTION_CONFIRMATION_MEMBER]: true }, + }); + expect(confirmed.isError, confirmed.content?.[0]?.text).toBeFalsy(); + expect(rows.has('n1')).toBe(false); + } finally { + await client.close(); + } + }); +}); diff --git a/packages/mcp/src/mcp-http-tools.ts b/packages/mcp/src/mcp-http-tools.ts index 76899f768dc..755a5bc9018 100644 --- a/packages/mcp/src/mcp-http-tools.ts +++ b/packages/mcp/src/mcp-http-tools.ts @@ -46,7 +46,7 @@ import type { EngineAggregateOptions } from '@objectstack/spec/data'; import { AggregationNodeSchema } from '@objectstack/spec/data'; // [#23529] The catalogue's spelling of `PERMISSION_DENIED`, the code whose // `details` `errorResultFromThrown` withholds. Imported, never hand-spelled. -import { StandardErrorCode } from '@objectstack/spec/api'; +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. @@ -64,6 +64,7 @@ import { import { AI_ACTION_CONFIRMATION_MEMBER, type AIActionConfirmation, + type ActionConfirmationRequiredDetails, type ImportObjectResult, type RemoteTable, } from '@objectstack/spec/contracts'; @@ -515,6 +516,67 @@ function codedErrorResult(error: ThrownRefusalEnvelope & { details?: unknown }) }; } +/** + * The code and status the confirmation gate answers with, on `run_action`'s + * door (the runtime's `actionConfirmationRefusal`) and on `delete_record`'s + * (below). Typed as the registered vocabulary, so a misspelling fails to + * compile instead of reaching a client as a code nobody branches on. + */ +const ACTION_CONFIRMATION_REQUIRED: ErrorCode = 'ACTION_CONFIRMATION_REQUIRED'; +const ACTION_CONFIRMATION_REQUIRED_HTTP_STATUS = 428; + +/** The `details.actionName` a `delete_record` refusal names: the tool the refused call addressed. */ +const DELETE_RECORD_TOOL = 'delete_record'; + +/** + * The confirmation gate on the MCP `delete_record` tool: `run_action`'s gate, + * widened by the maintainer's decision to this one tool, for every object. + * + * A delete over MCP is permanent, and the bridge's checks say whether the + * caller MAY delete, never whether a human asked for it. So a call that does + * not carry the confirmation member as the boolean `true` is refused with the + * refusal `run_action` answers an author-gated action with: the same code, + * status, envelope keys and `details` shape (`ActionConfirmationRequiredDetails`), + * rendered by the same {@link codedErrorResult}. A client that already handles + * that refusal confirms with its human and retries this call unchanged but for + * the member. + * + * The runtime's builder is out of reach (`@objectstack/runtime` and this + * package do not depend on each other), so the contract types and the shared + * renderer are what hold the two refusals to one shape; the dogfood suite + * serves both side by side to one caller. + * + * What it is NOT, as on `run_action`: `confirm: true` is an unverifiable + * caller claim. The gate makes forgetting loud; it does not prove a human. + * + * Scope, as decided: this tool only. REST `DELETE /api/v1/data/...` (the human + * path) and `update_record` stay outside it, no object is exempt, and a future + * bulk-delete tool takes the same gate. + * + * Returns `undefined` when the call may proceed. + */ +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. ` + + '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.`, + status: ACTION_CONFIRMATION_REQUIRED_HTTP_STATUS, + details: { + actionName: DELETE_RECORD_TOOL, + objectName, + confirmationMember: AI_ACTION_CONFIRMATION_MEMBER, + }, + }; +} + /** What {@link thrownRefusalEnvelope} reads off a thrown coded refusal. */ export interface ThrownRefusalEnvelope { code: string; @@ -1515,19 +1577,38 @@ export function registerObjectTools( server.registerTool( note('delete_record'), { - description: 'Delete a record by id. This is destructive.', + description: + 'Delete a record by id. This is destructive and permanent. A call is REFUSED unless it also sends ' + + 'confirm: true — ask the human first, then retry; nothing is deleted on a refused call.', inputSchema: strictToolInput( { surface: 'this delete_record call', aliases: { ...OBJECT_NAME_ALIASES, ...RECORD_ID_ALIASES } }, { objectName: z.string().describe('The object/table name'), recordId: z.string().describe('The record id'), + // The confirmation member, keyed off the contract's constant as + // `run_action` declares it. OPTIONAL in the schema on purpose: a + // required member would refuse an unconfirmed call as a bare input + // validation error, with no code and no retry details, before the + // handler could answer the gate's refusal. + [AI_ACTION_CONFIRMATION_MEMBER]: z + .boolean() + .optional() + .describe( + 'Set to true only when the human in the loop has approved deleting THIS record. Without it ' + + 'the call is refused and nothing is deleted.', + ), }, ), annotations: { readOnlyHint: false, destructiveHint: true, openWorldHint: false }, }, - async ({ objectName, recordId }) => { + async (args) => { + const { objectName, recordId } = args; const bad = guard(objectName); if (bad) return errorResult(bad); + // The gate, before the bridge: a refused call reaches no bridge, so + // nothing is read or deleted whichever bridge serves this server. + const unconfirmed = recordDeleteConfirmationRefusal(objectName, recordId, args); + if (unconfirmed) return codedErrorResult(unconfirmed); try { return textResult(await bridge.remove(objectName, recordId)); } catch (err) { 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 8431c7bca98..4f95ca4b219 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 @@ -106,7 +106,8 @@ const VALID_ARGS: Record> = { get_record: { objectName: 'crm_opportunity', recordId: 'r1' }, create_record: { objectName: 'crm_opportunity', data: { name: 'x' } }, update_record: { objectName: 'crm_opportunity', recordId: 'r1', data: { name: 'x' } }, - delete_record: { objectName: 'crm_opportunity', recordId: 'r1' }, + // `confirm: true`: without it the confirmation gate refuses before the bridge. + delete_record: { objectName: 'crm_opportunity', recordId: 'r1', confirm: true }, list_actions: {}, run_action: { actionName: 'complete_task' }, resume_run: { runId: 'run_1' }, diff --git a/packages/mcp/src/mcp-server-runtime.http.test.ts b/packages/mcp/src/mcp-server-runtime.http.test.ts index 32c159d8535..a136024ab97 100644 --- a/packages/mcp/src/mcp-server-runtime.http.test.ts +++ b/packages/mcp/src/mcp-server-runtime.http.test.ts @@ -190,7 +190,7 @@ describe('MCPServerRuntime.handleHttpRequest (Streamable HTTP)', () => { jsonrpc: '2.0', id: 6, method: 'tools/call', - params: { name: 'delete_record', arguments: { objectName: 'task', recordId: '1' } }, + params: { name: 'delete_record', arguments: { objectName: 'task', recordId: '1', confirm: true } }, }, bridge, ); diff --git a/packages/mcp/src/mcp-unique-violation-wire-code.test.ts b/packages/mcp/src/mcp-unique-violation-wire-code.test.ts index 9865d16f0e1..4c0bd1a83b5 100644 --- a/packages/mcp/src/mcp-unique-violation-wire-code.test.ts +++ b/packages/mcp/src/mcp-unique-violation-wire-code.test.ts @@ -112,7 +112,7 @@ const UPDATE: Site = { args: { objectName: 'udoc', recordId: 'r2', data: { email: 'taken@example.com' } }, member: 'update', }; -const DELETE: Site = { tool: 'delete_record', args: { objectName: 'udoc', recordId: 'r2' }, member: 'remove' }; +const DELETE: Site = { tool: 'delete_record', args: { objectName: 'udoc', recordId: 'r2', confirm: true }, member: 'remove' }; const RUN_ACTION: Site = { tool: 'run_action', args: { actionName: 'add_duplicate', objectName: 'udoc' }, member: 'runAction' }; const RESUME_RUN: Site = { tool: 'resume_run', args: { runId: 'run_1', values: { email: 'taken@example.com' } }, member: 'resumeRun' }; diff --git a/packages/mcp/src/mcp-update-delete-record-stale-summary.test.ts b/packages/mcp/src/mcp-update-delete-record-stale-summary.test.ts index 55275d1b811..19c96b4ea24 100644 --- a/packages/mcp/src/mcp-update-delete-record-stale-summary.test.ts +++ b/packages/mcp/src/mcp-update-delete-record-stale-summary.test.ts @@ -167,7 +167,7 @@ describe('[#28844] MCP update_record / delete_record answer a write applied unde it('delete_record, stdio bridge: the success result, the delete applied once', async () => { const rows = new Map([['c2', { id: 'c2', name: 'c2', amount: 135 }]]); const writes: string[] = []; - const result = await callTool(stdioBridge(rows, writes), 'delete_record', { objectName: 'qsf_child', recordId: 'c2' }); + const result = await callTool(stdioBridge(rows, writes), 'delete_record', { objectName: 'qsf_child', recordId: 'c2', confirm: true }); expect(result.isError, result.content[0]?.text).toBeFalsy(); expect(JSON.parse(result.content[0]!.text)).toEqual({ object: 'qsf_child', id: 'c2', success: true }); expect(writes).toEqual(['delete:c2']); @@ -183,7 +183,7 @@ describe('[#28844] MCP update_record / delete_record answer a write applied unde const updated = await callTool(bridge, 'update_record', { objectName: 'qsf_child', recordId: 'r1', data: { amount: 6 } }); expect(updated.isError, updated.content[0]?.text).toBeFalsy(); expect(JSON.parse(updated.content[0]!.text)).toEqual(answers.update); - const removed = await callTool(bridge, 'delete_record', { objectName: 'qsf_child', recordId: 'r2' }); + const removed = await callTool(bridge, 'delete_record', { objectName: 'qsf_child', recordId: 'r2', confirm: true }); expect(removed.isError, removed.content[0]?.text).toBeFalsy(); expect(JSON.parse(removed.content[0]!.text)).toEqual(answers.remove); }); diff --git a/packages/mcp/src/skill-md.ts b/packages/mcp/src/skill-md.ts index bcefe9b5ec8..de39c4ee4c3 100644 --- a/packages/mcp/src/skill-md.ts +++ b/packages/mcp/src/skill-md.ts @@ -285,8 +285,10 @@ create/update payload. - **get_record({ objectName, recordId })** — fetch one record by id. - **create_record({ objectName, data })** — create a record. - **update_record({ objectName, recordId, data })** — change fields on a record. -- **delete_record({ objectName, recordId })** — delete a record (destructive — - confirm with the user first). +- **delete_record({ objectName, recordId, confirm })** — delete a record, + 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. - **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-record-confirmation.dogfood.test.ts b/packages/qa/dogfood/test/mcp-delete-record-confirmation.dogfood.test.ts new file mode 100644 index 00000000000..350e098226a --- /dev/null +++ b/packages/qa/dogfood/test/mcp-delete-record-confirmation.dogfood.test.ts @@ -0,0 +1,137 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// The MCP `delete_record` tool refuses a call without `confirm: true` with +// `run_action`'s confirmation refusal, and REST `DELETE /data/:object/:id` +// stays outside that gate — proven together on a booted showcase, through the +// live `/api/v1/mcp` transport and the live REST data door, for one caller. +// +// ## Why this file and not only the package's own pins +// +// `@objectstack/mcp` and `@objectstack/runtime` do not depend on each other, +// so the door's pins (`packages/mcp/src/mcp-delete-record-confirmation.test.ts`) +// compare the delete refusal with a DOUBLE of the runtime gate. Here the +// runtime gate is the real one: `run_action` on an action the configuration +// declares `ai.requiresConfirmation: true` reaches `actionConfirmationRefusal` +// in `@objectstack/runtime`, and its served refusal is held against the +// `delete_record` refusal served by the same transport to the same caller. +// +// The derived configuration flags the showcase's own AI-exposed, object-less +// action (`showcase_portfolio_snapshot`) as gated; nothing else changes. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import showcaseStack from '@objectstack/example-showcase'; +import { type VerifyStack } from '@objectstack/verify'; +import { MCPServerPlugin } from '@objectstack/mcp'; +import { AI_ACTION_CONFIRMATION_MEMBER } from '@objectstack/spec/contracts'; +import { bootShowcase } from './showcase-boot.js'; + +const OBJECT = 'showcase_private_note'; +const GATED_ACTION = 'showcase_portfolio_snapshot'; +const idOf = (b: any) => b?.id ?? b?.record?.id ?? b?.data?.id ?? b?.recordId; + +describe('MCP delete_record takes run_action\'s confirmation gate; REST DELETE stays outside it', () => { + 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 createNote = async (title: string): Promise => { + const res = await stack.apiAs(admin, 'POST', `/data/${OBJECT}`, { title }); + expect(res.status, await res.clone().text()).toBeLessThan(300); + const id = idOf(await res.json()); + expect(id, 'created note id').toBeTruthy(); + return String(id); + }; + const readStatus = async (id: string) => (await stack.apiAs(admin, 'GET', `/data/${OBJECT}/${id}`)).status; + + beforeAll(async () => { + const declared = showcaseStack as unknown as { actions?: any[] }; + const actions = (declared.actions ?? []).map((a) => + a?.name === GATED_ACTION ? { ...a, ai: { ...(a.ai ?? {}), requiresConfirmation: true } } : a, + ); + expect(actions.some((a) => a?.name === GATED_ACTION && a.ai?.exposed === true), 'the fixture action moved').toBe(true); + stack = await bootShowcase( + { extraPlugins: [new MCPServerPlugin()] }, + { ...(showcaseStack as unknown as Record), actions }, + ); + admin = await stack.signIn(); + }, 300_000); + + afterAll(async () => { + await stack?.stop?.(); + }); + + it('refuses an unconfirmed delete_record with ACTION_CONFIRMATION_REQUIRED, and the record is intact', async () => { + const id = await createNote('mcp delete unconfirmed'); + + const refused = await tool('delete_record', { objectName: OBJECT, recordId: id }); + + expect(refused.isError, refused.text).toBe(true); + expect(refused.payload.error).toMatchObject({ + code: 'ACTION_CONFIRMATION_REQUIRED', + status: 428, + details: { actionName: 'delete_record', objectName: OBJECT, confirmationMember: AI_ACTION_CONFIRMATION_MEMBER }, + }); + expect(await readStatus(id), 'the refused delete removed the record').toBe(200); + }, 60_000); + + it('serves the same code, status, envelope keys and confirmation member as run_action\'s real gate', async () => { + const id = await createNote('mcp delete shape'); + + const viaAction = await tool('run_action', { actionName: GATED_ACTION }); + const viaDelete = await tool('delete_record', { objectName: OBJECT, recordId: id }); + + // Lit control: the runtime gate really refused the action. + expect(viaAction.isError, viaAction.text).toBe(true); + expect(viaAction.payload.error?.code).toBe('ACTION_CONFIRMATION_REQUIRED'); + expect(viaAction.payload.error?.details?.actionName).toBe(GATED_ACTION); + + const a = viaAction.payload.error; + const d = viaDelete.payload.error; + expect(d.code).toBe(a.code); + expect(d.status).toBe(a.status); + expect(Object.keys(d)).toEqual(Object.keys(a)); + expect(d.details.confirmationMember).toBe(a.details.confirmationMember); + // The action is object-less, so its details carry no `objectName`; every + // key it does carry, the delete refusal carries too. + expect(Object.keys(d.details)).toEqual(expect.arrayContaining(Object.keys(a.details))); + expect(await readStatus(id)).toBe(200); + }, 60_000); + + it('deletes on the retry with confirm: true', async () => { + const id = await createNote('mcp delete confirmed'); + + const deleted = await tool('delete_record', { objectName: OBJECT, recordId: id, [AI_ACTION_CONFIRMATION_MEMBER]: true }); + + expect(deleted.isError, deleted.text).toBe(false); + expect(deleted.payload).toMatchObject({ object: OBJECT, success: true }); + expect(await readStatus(id), 'the confirmed delete left the record').toBe(404); + }, 60_000); + + it('control: REST DELETE /data/:object/:id takes no confirmation and deletes', async () => { + const id = await createNote('rest delete'); + + const res = await stack.apiAs(admin, 'DELETE', `/data/${OBJECT}/${id}`); + + expect(res.status, await res.clone().text()).toBeLessThan(300); + expect(await readStatus(id)).toBe(404); + }, 60_000); +}); From 0a2a5814c2b3b34e2238d3ea57e8b6e5f3a110f9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 05:37:24 +0000 Subject: [PATCH 2/6] wip: changeset, spec contract docblock, ledger row, docs Claude-Session: https://claude.ai/code/session_016WV4czsSxuhcKFSWS5tLdW Co-authored-by: Claude --- .../30813-mcp-delete-record-confirmation.md | 35 +++++++++++++++++++ content/docs/ai/connect-mcp.mdx | 7 ++++ .../spec/src/api/error-code-ledger.zod.ts | 12 +++++++ packages/spec/src/contracts/ai-service.ts | 27 ++++++++++++-- 4 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 .changeset/30813-mcp-delete-record-confirmation.md diff --git a/.changeset/30813-mcp-delete-record-confirmation.md b/.changeset/30813-mcp-delete-record-confirmation.md new file mode 100644 index 00000000000..35eb32f5ba0 --- /dev/null +++ b/.changeset/30813-mcp-delete-record-confirmation.md @@ -0,0 +1,35 @@ +--- +'@objectstack/mcp': patch +'@objectstack/spec': patch +--- + +feat(mcp)!: the MCP `delete_record` tool refuses a call without `confirm: true`, with `run_action`'s confirmation refusal + +Clause-②: yes (narrowing) — the MCP `delete_record` tool refuses every call that does not carry `confirm: true`, where it deleted the record. The confirmation contract in `@objectstack/spec/contracts` (`AIActionConfirmation`, `ActionConfirmationRequiredDetails`) gains its second door in its docblock, and the error-code ledger lists `ACTION_CONFIRMATION_REQUIRED` under `@objectstack/mcp` as a second emitter; no shape changes. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the MCP `delete_record` tool (`@objectstack/mcp`), over both transports (Streamable HTTP `/api/v1/mcp` and stdio): every MCP client that deletes is refused until it sends the confirmation member. + +- **Before.** `delete_record({ objectName, recordId })` deleted the record. A delete is hard and permanent, and the tool's only safety signal was `annotations.destructiveHint`, a hint a client may ignore. A call carrying `confirm` was refused as an unknown key. +- **Now.** A call whose `confirm` is not the boolean `true` (absent, `false`, `"true"`) is refused before the data bridge is reached, and nothing is read or deleted. The answer is the refusal `run_action` gives an action whose author declared `ai.requiresConfirmation: true`: an `isError` result whose text is `{ "error": { "code": "ACTION_CONFIRMATION_REQUIRED", "message", "status": 428, "details": { "actionName": "delete_record", "objectName", "confirmationMember": "confirm" } } }`. `delete_record({ objectName, recordId, confirm: true })` deletes as before, under the same exposure, credential and permission checks. The tool schema advertises `confirm` as an optional boolean, and the tool description tells the agent to ask the human first, then retry. +- **Who moves.** An MCP client or agent that calls `delete_record`. A client that already handles `run_action`'s `ACTION_CONFIRMATION_REQUIRED` (read `details.confirmationMember`, ask the human, resend the same call with that member set to `true`) needs no change. + +FROM → TO, for an MCP client that deletes: + +``` +delete_record({ objectName: 'crm_lead', recordId: 'lead_1' }) // was: deleted +delete_record({ objectName: 'crm_lead', recordId: 'lead_1', confirm: true }) // now: required +``` + +**The remedy.** Ask the human in the loop, then send the same `delete_record` call with `confirm: true` added beside `objectName` and `recordId`. The flag asserts an approval; it does not obtain one, and the gate makes forgetting loud without proving a human. + +**The bounds, as decided.** + +- REST `DELETE /api/v1/data/:object/:id` (the human path) is outside the gate and unchanged. +- `update_record` is not gated. +- No object is exempt: every object a caller may delete through `delete_record` takes the gate. +- Any future bulk-delete tool takes the same gate. + +The member, the code, the status and the `details` shape are the contract `run_action` already serves (`AI_ACTION_CONFIRMATION_MEMBER` and `ActionConfirmationRequiredDetails` in `@objectstack/spec/contracts`), so an agent learns one retry for both tools. The MCP server's own skill (`GET /api/v1/mcp/skill`) teaches the member on `delete_record`. diff --git a/content/docs/ai/connect-mcp.mdx b/content/docs/ai/connect-mcp.mdx index fd329a20729..01a7a361e12 100644 --- a/content/docs/ai/connect-mcp.mdx +++ b/content/docs/ai/connect-mcp.mdx @@ -375,6 +375,13 @@ drive this MCP server. Same word, different layer. an action as a code-review-worthy act — that's the real security boundary. - An action can also declare `ai.requiresConfirmation`; destructive-looking actions (`confirmText`, danger variants) default to requiring it. +- **`delete_record` needs `confirm: true` on every call.** A record delete over + MCP is permanent, so the tool refuses a call without it with `run_action`'s + refusal (`428` `ACTION_CONFIRMATION_REQUIRED`, its `details.actionName` + naming `delete_record`) and deletes nothing. The agent asks its human, then retries + 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. ## Verify the connection diff --git a/packages/spec/src/api/error-code-ledger.zod.ts b/packages/spec/src/api/error-code-ledger.zod.ts index 21e349e0da3..b09882d2e40 100644 --- a/packages/spec/src/api/error-code-ledger.zod.ts +++ b/packages/spec/src/api/error-code-ledger.zod.ts @@ -369,6 +369,10 @@ 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`) + // answers this code, 428, with the same `details` shape for every record + // delete without the member; listed under `@objectstack/mcp` too. 'ACTION_CONFIRMATION_REQUIRED', // [ADR-0126 §8 item 2] a packaged ACTION this installation switched off, // refused at dispatch — the activation-ledger consult in @@ -1440,6 +1444,14 @@ 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. + 'ACTION_CONFIRMATION_REQUIRED', ], '@objectstack/service-datasource': [ 'DATASOURCE_ADMIN_ERROR', // lifecycle/validation refusal from the datasource-admin service diff --git a/packages/spec/src/contracts/ai-service.ts b/packages/spec/src/contracts/ai-service.ts index a6ac55fec7a..1d950f680eb 100644 --- a/packages/spec/src/contracts/ai-service.ts +++ b/packages/spec/src/contracts/ai-service.ts @@ -213,8 +213,10 @@ export interface AIToolDefinition { * `invokeBusinessAction` (`objectName` / `recordId` / `params`, * `packages/runtime/src/action-execution.ts`), which the MCP bridge builds * that object from. Each grows the member in the change that enforces it, so - * neither ever accepts-and-ignores it. Two placements were rejected, and the - * reasons are the contract: + * neither ever accepts-and-ignores it. A third door carries the same member + * at the top level of its input: the MCP `delete_record` tool (see + * {@link AIActionConfirmation}, "The second gate"). Two placements were + * rejected, and the reasons are the contract: * * - NOT inside `params`. That bag is not a free surface: it is CLOSED against * the action author's own declared input vocabulary. `enforceActionParams` @@ -298,6 +300,22 @@ export const AI_ACTION_CONFIRMATION_MEMBER = 'confirm'; * names the action and the exact member to set. Nothing dispatches: the * action body does not run, no record is read or written, and the refusal * carries no `status`. + * + * ## The second gate: the MCP `delete_record` tool + * + * A record delete over MCP is permanent, so the same member gates it, by the + * maintainer's decision, for every object and with no declared flag to + * consult: `delete_record` (`packages/mcp/src/mcp-http-tools.ts`) refuses + * EVERY call that does not carry the member as the boolean `true`, before + * its data bridge is reached, so nothing is read or deleted. The refusal is + * the one above: the same code and HTTP status, and `error.details` shaped as + * {@link ActionConfirmationRequiredDetails}, whose `actionName` is the tool's + * name, `delete_record`. A client that handles the action refusal confirms + * with its human and retries the same call with the member set. + * + * 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. */ export interface AIActionConfirmation { /** @@ -317,7 +335,10 @@ export interface AIActionConfirmation { * client never has to hard-code the member's spelling from documentation. */ export interface ActionConfirmationRequiredDetails { - /** The declarative action name the refused call addressed. */ + /** + * The declarative action name the refused call addressed; on the MCP + * `delete_record` door, the tool's own name, `delete_record`. + */ actionName: string; /** The object the action operates on; omitted for object-less actions. */ objectName?: string; From da57b9713dd8f82a58a311605cb102cc221df0dd Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 05:37:57 +0000 Subject: [PATCH 3/6] wip: changeset migration as before/now/remedy prose Claude-Session: https://claude.ai/code/session_016WV4czsSxuhcKFSWS5tLdW Co-authored-by: Claude --- .changeset/30813-mcp-delete-record-confirmation.md | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.changeset/30813-mcp-delete-record-confirmation.md b/.changeset/30813-mcp-delete-record-confirmation.md index 35eb32f5ba0..a64432ebd26 100644 --- a/.changeset/30813-mcp-delete-record-confirmation.md +++ b/.changeset/30813-mcp-delete-record-confirmation.md @@ -16,13 +16,6 @@ Metadata-protocol: none - **Now.** A call whose `confirm` is not the boolean `true` (absent, `false`, `"true"`) is refused before the data bridge is reached, and nothing is read or deleted. The answer is the refusal `run_action` gives an action whose author declared `ai.requiresConfirmation: true`: an `isError` result whose text is `{ "error": { "code": "ACTION_CONFIRMATION_REQUIRED", "message", "status": 428, "details": { "actionName": "delete_record", "objectName", "confirmationMember": "confirm" } } }`. `delete_record({ objectName, recordId, confirm: true })` deletes as before, under the same exposure, credential and permission checks. The tool schema advertises `confirm` as an optional boolean, and the tool description tells the agent to ask the human first, then retry. - **Who moves.** An MCP client or agent that calls `delete_record`. A client that already handles `run_action`'s `ACTION_CONFIRMATION_REQUIRED` (read `details.confirmationMember`, ask the human, resend the same call with that member set to `true`) needs no change. -FROM → TO, for an MCP client that deletes: - -``` -delete_record({ objectName: 'crm_lead', recordId: 'lead_1' }) // was: deleted -delete_record({ objectName: 'crm_lead', recordId: 'lead_1', confirm: true }) // now: required -``` - **The remedy.** Ask the human in the loop, then send the same `delete_record` call with `confirm: true` added beside `objectName` and `recordId`. The flag asserts an approval; it does not obtain one, and the gate makes forgetting loud without proving a human. **The bounds, as decided.** From 7723700f040790f24c16f91a5ad4e8c8d74eecee Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 05:53:05 +0000 Subject: [PATCH 4/6] test(dogfood): delete_record calls that pin the gates behind it send confirm: true Claude-Session: https://claude.ai/code/session_016WV4czsSxuhcKFSWS5tLdW Co-authored-by: Claude --- .../qa/dogfood/test/mcp-stdio-package-object-discovery.test.ts | 3 ++- .../qa/dogfood/test/mcp-stdio-runtime-object-exposure.test.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/qa/dogfood/test/mcp-stdio-package-object-discovery.test.ts b/packages/qa/dogfood/test/mcp-stdio-package-object-discovery.test.ts index f5a7c6ca1ab..32edda5b740 100644 --- a/packages/qa/dogfood/test/mcp-stdio-package-object-discovery.test.ts +++ b/packages/qa/dogfood/test/mcp-stdio-package-object-discovery.test.ts @@ -383,7 +383,8 @@ const DATA_TOOLS: ReadonlyArray Record ({ objectName: o, aggregations: [{ function: 'count', alias: 'n' }] })], ['create_record', (o) => ({ objectName: o, data: { name: 'written' } })], ['update_record', (o) => ({ objectName: o, recordId: 'r_any', data: { name: 'written' } })], - ['delete_record', (o) => ({ objectName: o, recordId: 'r_any' })], + // `confirm: true` passes the delete confirmation gate, so the call meets the gates behind it. + ['delete_record', (o) => ({ objectName: o, recordId: 'r_any', confirm: true })], ]; /** Every engine call a data tool could reach `object`'s store through. */ diff --git a/packages/qa/dogfood/test/mcp-stdio-runtime-object-exposure.test.ts b/packages/qa/dogfood/test/mcp-stdio-runtime-object-exposure.test.ts index bc580224cc6..b560cc8288d 100644 --- a/packages/qa/dogfood/test/mcp-stdio-runtime-object-exposure.test.ts +++ b/packages/qa/dogfood/test/mcp-stdio-runtime-object-exposure.test.ts @@ -366,7 +366,8 @@ describe.each(SHAPES)('the MCP stdio door follows a runtime-authored object\'s c expect(refusal(await k.call('get_record', { objectName: GATED, recordId: 'any' }))).toEqual(refused); expect(refusal(await k.call('create_record', { objectName: GATED, data: { name: 'leak-probe' } }))).toEqual(refused); expect(refusal(await k.call('update_record', { objectName: GATED, recordId: 'any', data: { name: 'x' } }))).toEqual(refused); - expect(refusal(await k.call('delete_record', { objectName: GATED, recordId: 'any' }))).toEqual(refused); + // `confirm: true` passes the delete confirmation gate, so the exposure gate behind it answers. + expect(refusal(await k.call('delete_record', { objectName: GATED, recordId: 'any', confirm: true }))).toEqual(refused); // Refused before dispatch: the table holds exactly what it held. expect(stores.get(GATED)?.size).toBe(2); From 36030eabba1430e89bb9172865317568e8b8d1fd Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 06:35:28 +0000 Subject: [PATCH 5/6] test(mcp): record the delete_record confirmation test's engine doubles in the pinned ledger check:engine-double-contract asks for the new file's pinned delete/findOne/update doubles to be recorded (generated by --write; no seam lost). Claim-Session: session_016WV4czsSxuhcKFSWS5tLdW Claude-Session: https://claude.ai/code/session_016WV4czsSxuhcKFSWS5tLdW Co-authored-by: Claude --- scripts/engine-double-contract.pinned.json | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 7a7fbd3e301..39bc7b5733b 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -81,6 +81,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/mcp/src/mcp-delete-record-confirmation.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/mcp/src/mcp-delete-record-confirmation.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/mcp/src/mcp-delete-record-confirmation.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/mcp/src/mcp-stdio-tools.test.ts", "verb": "delete", From 273858232db5774419d8d4a267d5e0dc208ec2c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 07:23:23 +0000 Subject: [PATCH 6/6] fix(spec,mcp): the confirmation pin names its second emitter; a non-boolean confirm is refused by the input schema The action-confirmation contract pin asserted ACTION_CONFIRMATION_REQUIRED is listed under no package but @objectstack/runtime; the MCP delete_record gate is its second emitter (the provenance row this PR adds), so the pin now names exactly that one, and a further emitter still moves it (Test Core 2/8 on 36030eab). Contract review should-fix: the changeset's Now bullet and the contract's "second gate" paragraph said the string "true" (and 1) answer the 428 ACTION_CONFIRMATION_REQUIRED envelope. They do not: the tool's closed input schema refuses a non-boolean member first, with no code and no details, exactly as on run_action. Only an absent or false member reaches the gate. Claude-Session: https://claude.ai/code/session_016WV4czsSxuhcKFSWS5tLdW Co-authored-by: Claude --- .changeset/30813-mcp-delete-record-confirmation.md | 2 +- .../action-confirmation-contract.pin.test.ts | 14 +++++++++++--- packages/spec/src/contracts/ai-service.ts | 7 +++++-- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/.changeset/30813-mcp-delete-record-confirmation.md b/.changeset/30813-mcp-delete-record-confirmation.md index a64432ebd26..fde82a15ab4 100644 --- a/.changeset/30813-mcp-delete-record-confirmation.md +++ b/.changeset/30813-mcp-delete-record-confirmation.md @@ -13,7 +13,7 @@ Metadata-protocol: none **BREAKING** accept-set narrowing at the MCP `delete_record` tool (`@objectstack/mcp`), over both transports (Streamable HTTP `/api/v1/mcp` and stdio): every MCP client that deletes is refused until it sends the confirmation member. - **Before.** `delete_record({ objectName, recordId })` deleted the record. A delete is hard and permanent, and the tool's only safety signal was `annotations.destructiveHint`, a hint a client may ignore. A call carrying `confirm` was refused as an unknown key. -- **Now.** A call whose `confirm` is not the boolean `true` (absent, `false`, `"true"`) is refused before the data bridge is reached, and nothing is read or deleted. The answer is the refusal `run_action` gives an action whose author declared `ai.requiresConfirmation: true`: an `isError` result whose text is `{ "error": { "code": "ACTION_CONFIRMATION_REQUIRED", "message", "status": 428, "details": { "actionName": "delete_record", "objectName", "confirmationMember": "confirm" } } }`. `delete_record({ objectName, recordId, confirm: true })` deletes as before, under the same exposure, credential and permission checks. The tool schema advertises `confirm` as an optional boolean, and the tool description tells the agent to ask the human first, then retry. +- **Now.** A call without `confirm`, or with `confirm: false`, is refused before the data bridge is reached, and nothing is read or deleted. A non-boolean `confirm` (the string `"true"`, `1`) is refused before that by the tool's closed input schema, with no code and no `details`, exactly as on `run_action`. The answer to an absent or `false` member is the refusal `run_action` gives an action whose author declared `ai.requiresConfirmation: true`: an `isError` result whose text is `{ "error": { "code": "ACTION_CONFIRMATION_REQUIRED", "message", "status": 428, "details": { "actionName": "delete_record", "objectName", "confirmationMember": "confirm" } } }`. `delete_record({ objectName, recordId, confirm: true })` deletes as before, under the same exposure, credential and permission checks. The tool schema advertises `confirm` as an optional boolean, and the tool description tells the agent to ask the human first, then retry. - **Who moves.** An MCP client or agent that calls `delete_record`. A client that already handles `run_action`'s `ACTION_CONFIRMATION_REQUIRED` (read `details.confirmationMember`, ask the human, resend the same call with that member set to `true`) needs no change. **The remedy.** Ask the human in the loop, then send the same `delete_record` call with `confirm: true` added beside `objectName` and `recordId`. The flag asserts an approval; it does not obtain one, and the gate makes forgetting loud without proving a human. diff --git a/packages/spec/src/contracts/action-confirmation-contract.pin.test.ts b/packages/spec/src/contracts/action-confirmation-contract.pin.test.ts index 8cad375f1e9..8af09e5d5fb 100644 --- a/packages/spec/src/contracts/action-confirmation-contract.pin.test.ts +++ b/packages/spec/src/contracts/action-confirmation-contract.pin.test.ts @@ -14,7 +14,8 @@ * ## What is pinned, and what is deliberately NOT * * Pinned: that the refusal code is admitted by the closed `ErrorCode` union, - * that the ledger registers it under the package that owns the action doors, + * that the ledger registers it under the package that owns the action doors + * and under exactly one second emitter, the MCP `delete_record` tool's gate, * that it is not a synonym of a standard-catalog member (which is the whole * argument for registering it rather than answering `PRECONDITION_REQUIRED`), * and that `ActionConfirmationRequiredDetails.confirmationMember` is typed to @@ -55,6 +56,11 @@ import { const CODE = 'ACTION_CONFIRMATION_REQUIRED'; /** The package that owns both AI-facing action doors, and so owns the code. */ const OWNER = '@objectstack/runtime'; +/** + * The one second EMITTER (provenance, not identity): the MCP `delete_record` + * tool's gate stamps the same code on a record delete without the member. + */ +const SECOND_EMITTER = '@objectstack/mcp'; /** Identity helpers, so a rejected shape errors on the argument's own line. */ const asDetails = (d: ActionConfirmationRequiredDetails) => d; @@ -69,15 +75,17 @@ describe('action-confirmation contract — an unconfirmed gated action is refuse expect(ErrorCode.safeParse('ACTION_CONFIRMATION_REQUIRE').success).toBe(false); }); - it('the ledger registers it under the package that owns the action doors', () => { + it('the ledger registers it under the package that owns the action doors, and under its one second emitter', () => { expect(ERROR_CODE_LEDGER[OWNER]).toContain(CODE); // Control: the row is not merely somewhere in the ledger. If the code // moved owners, the assertion above would still pass under a `flat()`. + // The one other package listing it is the MCP `delete_record` gate's; a + // further emitter is a new door, and moving this pin is how it says so. const otherOwners = Object.entries(ERROR_CODE_LEDGER) .filter(([pkg]) => pkg !== OWNER) .filter(([, codes]) => (codes as readonly string[]).includes(CODE)) .map(([pkg]) => pkg); - expect(otherOwners).toEqual([]); + expect(otherOwners).toEqual([SECOND_EMITTER]); }); it('it is NOT a synonym of a standard-catalog member', () => { diff --git a/packages/spec/src/contracts/ai-service.ts b/packages/spec/src/contracts/ai-service.ts index 1d950f680eb..2c6291ce59c 100644 --- a/packages/spec/src/contracts/ai-service.ts +++ b/packages/spec/src/contracts/ai-service.ts @@ -307,8 +307,11 @@ export const AI_ACTION_CONFIRMATION_MEMBER = 'confirm'; * maintainer's decision, for every object and with no declared flag to * consult: `delete_record` (`packages/mcp/src/mcp-http-tools.ts`) refuses * EVERY call that does not carry the member as the boolean `true`, before - * its data bridge is reached, so nothing is read or deleted. The refusal is - * the one above: the same code and HTTP status, and `error.details` shaped as + * its data bridge is reached, so nothing is read or deleted. A non-boolean + * member (the string `"true"`, `1`) is refused first by the tool's closed + * input schema, with no code and no details, exactly as on `run_action`; an + * absent or `false` member reaches the gate, and its refusal is the one above: + * the same code and HTTP status, and `error.details` shaped as * {@link ActionConfirmationRequiredDetails}, whose `actionName` is the tool's * name, `delete_record`. A client that handles the action refusal confirms * with its human and retries the same call with the member set.