diff --git a/apps/e2e/demo-e2e-cloudflare/e2e/helpers/managed-bundles.ts b/apps/e2e/demo-e2e-cloudflare/e2e/helpers/managed-bundles.ts index a4252d322..f006374d0 100644 --- a/apps/e2e/demo-e2e-cloudflare/e2e/helpers/managed-bundles.ts +++ b/apps/e2e/demo-e2e-cloudflare/e2e/helpers/managed-bundles.ts @@ -99,16 +99,33 @@ const createInvoiceOperation: OperationDescriptor = { authBindingRef: 'default', }; +const voidInvoiceOperation: OperationDescriptor = { + operationId: 'voidInvoice', + serviceId: 'billing', + httpMethod: 'DELETE', + pathTemplate: '/v1/invoices/{id}', + summary: 'Void an invoice', + public: true, + inputSchema: { type: 'object', properties: { id: { type: 'string' } }, required: ['id'] }, + outputSchema: { + type: 'object', + properties: { id: { type: 'string' }, status: { type: 'string' } }, + required: ['id', 'status'], + }, + mapper: [{ inputKey: 'id', type: 'path', key: 'id', required: true }], + authBindingRef: 'default', +}; + export type BillingSkillId = 'invoices' | 'reports' | 'refunds'; const billingSkills: Record = { invoices: { id: 'invoices', name: 'Invoices', - description: 'Create billing invoices for customers.', - instructions: '# Invoices\n\nCreate an invoice with createInvoice.', + description: 'Create and void billing invoices for customers.', + instructions: '# Invoices\n\nCreate an invoice with createInvoice, and void one with voidInvoice.', tags: ['billing'], - operationIds: ['createInvoice'], + operationIds: ['createInvoice', 'voidInvoice'], }, reports: { id: 'reports', @@ -139,6 +156,6 @@ export function billingBundle(version: string, skillIds: readonly BillingSkillId services: [{ id: 'billing', baseUrl: BILLING_API_URL }], authBindings: { default: { kind: 'bearer', vaultRef: 'billing-token' } }, skills: skillIds.map((skillId) => billingSkills[skillId]), - operations: { createInvoice: createInvoiceOperation }, + operations: { createInvoice: createInvoiceOperation, voidInvoice: voidInvoiceOperation }, }; } diff --git a/apps/e2e/demo-e2e-cloudflare/e2e/helpers/managed-internet.ts b/apps/e2e/demo-e2e-cloudflare/e2e/helpers/managed-internet.ts index 28a56880b..e828a1523 100644 --- a/apps/e2e/demo-e2e-cloudflare/e2e/helpers/managed-internet.ts +++ b/apps/e2e/demo-e2e-cloudflare/e2e/helpers/managed-internet.ts @@ -58,6 +58,10 @@ export class ManagedInternet { const invoice = JSON.parse(body) as { customerId: string; amount: number }; return Response.json({ id: `inv_${invoice.customerId}`, status: 'open' }, { status: 201 }); } + if (method === 'DELETE' && url.startsWith(`${BILLING_API_URL}/v1/invoices/`)) { + if (authorization !== `Bearer ${this.options.billingToken}`) return new Response('unauthorized', { status: 401 }); + return Response.json({ id: url.slice(`${BILLING_API_URL}/v1/invoices/`.length), status: 'void' }); + } return new Response(`no route for ${method} ${url}`, { status: 404 }); } } diff --git a/apps/e2e/demo-e2e-cloudflare/e2e/helpers/managed-worker.ts b/apps/e2e/demo-e2e-cloudflare/e2e/helpers/managed-worker.ts index 0f3206ce7..e8b0f8c8a 100644 --- a/apps/e2e/demo-e2e-cloudflare/e2e/helpers/managed-worker.ts +++ b/apps/e2e/demo-e2e-cloudflare/e2e/helpers/managed-worker.ts @@ -20,10 +20,27 @@ const SECOND_REPLICA = 'replica-2'; const THIRD_REPLICA = 'replica-3'; interface JsonRpcResponse { - result?: { isError?: boolean; content?: Array<{ type: string; text?: string }> }; + result?: AskingToolResult; error?: { code: number; message: string }; } +/** A protocol 2026-07-28 `tools/call` result: complete, or asking for input the client sends back on its retry. */ +export interface AskingToolResult { + resultType?: 'complete' | 'input_required'; + isError?: boolean; + content?: Array<{ type: string; text?: string }>; + inputRequests?: Record; + requestState?: string; +} + +/** What a retry of a 2026-07-28 call carries: the answers to the input requests, and the state to echo. */ +export interface AskingToolRetry { + inputResponses: Record; + requestState?: string; +} + +const PROTOCOL_20260728 = '2026-07-28'; + export interface ToolCallResult { isError: boolean; text: string; @@ -129,6 +146,41 @@ export class ManagedWorker { return { isError: message.result.isError === true, text, json: () => JSON.parse(text) as T }; } + /** A protocol 2026-07-28 `tools/call` from a client that declares elicitation, so the server may ask it. */ + async callToolAsking( + name: string, + args: Record, + retry?: AskingToolRetry, + ): Promise { + const response = await this.fetch(MCP_URL, { + method: 'POST', + headers: { + 'content-type': 'application/json', + accept: 'application/json, text/event-stream', + 'mcp-protocol-version': PROTOCOL_20260728, + 'mcp-method': 'tools/call', + 'mcp-name': name, + }, + body: JSON.stringify({ + jsonrpc: '2.0', + id: ++this.requestId, + method: 'tools/call', + params: { + name, + arguments: args, + ...retry, + _meta: { + 'io.modelcontextprotocol/protocolVersion': PROTOCOL_20260728, + 'io.modelcontextprotocol/clientCapabilities': { elicitation: { form: {} } }, + }, + }, + }), + }); + const message = await readJsonRpc(response); + if (!message.result) throw new Error(`tools/call ${name} failed: ${JSON.stringify(message.error)}\n${this.log}`); + return message.result; + } + /** Run the Cron Trigger handler, as the platform does on schedule. */ async scheduled(): Promise<{ outcome: string }> { const worker = await this.miniflare.getWorker(this.replicaName); diff --git a/apps/e2e/demo-e2e-cloudflare/e2e/managed-edge-autoupdate.e2e.spec.ts b/apps/e2e/demo-e2e-cloudflare/e2e/managed-edge-autoupdate.e2e.spec.ts index fa1619c8d..7c9595078 100644 --- a/apps/e2e/demo-e2e-cloudflare/e2e/managed-edge-autoupdate.e2e.spec.ts +++ b/apps/e2e/demo-e2e-cloudflare/e2e/managed-edge-autoupdate.e2e.spec.ts @@ -21,7 +21,7 @@ interface SearchSkillResponse { } interface LoadSkillResponse { - skill: { id: string; bundleVersion?: string; actions?: Array<{ actionId: string }> }; + skill: { id: string; bundleVersion?: string; actions?: Array<{ actionId: string; asksForConfirmation?: true }> }; } interface RunWorkflowResponse { @@ -75,7 +75,7 @@ describe('a managed edge Worker on workerd', () => { expect(skills.map((skill) => skill.skillId).sort()).toEqual(['invoices', 'reports']); expect(skills.every((skill) => skill.bundleVersion === '1.0.0')).toBe(true); expect(loaded.json().skill).toMatchObject({ id: 'invoices', bundleVersion: '1.0.0' }); - expect(loaded.json().skill.actions?.map((action) => action.actionId)).toEqual(['createInvoice']); + expect(loaded.json().skill.actions?.map((action) => action.actionId)).toEqual(['createInvoice', 'voidInvoice']); expect(internet.requestsTo(BUNDLE_URL)).toHaveLength(1); expect(await worker.cachedBundleVersion()).toBe('1.0.0'); }); @@ -95,6 +95,53 @@ describe('a managed edge Worker on workerd', () => { expect(result.text).not.toContain(billingToken); }); + describe("managed.confirm: { methods: ['DELETE'] }, on a Worker that can't enable elicitation", () => { + const VOID_INVOICE = { actionId: 'voidInvoice', input: { id: 'inv_edge' } }; + const CANNOT_ASK = + "the user's confirmation is needed for voidInvoice, and this server can't ask for it: elicitation is disabled; nothing was sent"; + const voidRequests = () => internet.requests.filter((request) => request.method === 'DELETE'); + + it('refuses the DELETE action in execute_action and run_workflow, sending nothing', async () => { + const action = await worker.callTool('execute_action', VOID_INVOICE); + const workflow = await worker.callTool('run_workflow', { + script: "return await callTool('voidInvoice', { id: 'inv_edge' });", + }); + + expect(action.isError).toBe(true); + expect(action.text).toBe(CANNOT_ASK); + expect(workflow.json()).toEqual({ success: false, error: CANNOT_ASK }); + expect(voidRequests()).toEqual([]); + }); + + it('refuses it to a client that declares elicitation too, without asking', async () => { + const answer = await worker.callToolAsking('execute_action', VOID_INVOICE); + + expect(answer.resultType).not.toBe('input_required'); + expect(answer.isError).toBe(true); + expect(answer.content?.[0]?.text).toBe(CANNOT_ASK); + expect(voidRequests()).toEqual([]); + }); + + it('still runs an action the rule does not name', async () => { + const result = await worker.callTool<{ id: string }>('execute_action', { + actionId: 'createInvoice', + input: { customerId: 'cus_confirm', amount: 3 }, + }); + + expect(result.isError).toBe(false); + expect(result.json()).toMatchObject({ id: 'inv_cus_confirm' }); + }); + + it('marks the DELETE action in load_skill as one that asks for confirmation', async () => { + const loaded = await worker.callTool('load_skill', { skillId: 'invoices' }); + + expect(loaded.json().skill.actions).toEqual([ + expect.not.objectContaining({ asksForConfirmation: true }), + expect.objectContaining({ actionId: 'voidInvoice', asksForConfirmation: true }), + ]); + }); + }); + it('picks up a new bundle version on a Cron run, applied and kept in KV by the time the run ends', async () => { publish('2.0.0', ['invoices', 'refunds']); diff --git a/apps/e2e/demo-e2e-cloudflare/fixture-managed/worker.ts b/apps/e2e/demo-e2e-cloudflare/fixture-managed/worker.ts index 1182adbbb..1959d3abe 100644 --- a/apps/e2e/demo-e2e-cloudflare/fixture-managed/worker.ts +++ b/apps/e2e/demo-e2e-cloudflare/fixture-managed/worker.ts @@ -28,6 +28,7 @@ const mcp = createEdgeMcp({ trustedKeys: [JSON.parse(bindings.BUNDLE_TRUSTED_KEY)], credentials: { 'billing-token': bindings.BILLING_API_TOKEN }, enableWebhook: true, + confirm: { methods: ['DELETE'] }, cache: kvBundleCacheFromEnv('BUNDLE_CACHE'), }, }); diff --git a/apps/e2e/demo-e2e-skilled-openapi/e2e/confirm-destructive.e2e.spec.ts b/apps/e2e/demo-e2e-skilled-openapi/e2e/confirm-destructive.e2e.spec.ts index 36683b656..2c4908551 100644 --- a/apps/e2e/demo-e2e-skilled-openapi/e2e/confirm-destructive.e2e.spec.ts +++ b/apps/e2e/demo-e2e-skilled-openapi/e2e/confirm-destructive.e2e.spec.ts @@ -2,7 +2,7 @@ import { expect, test, type McpTestClient, type TestFixtures } from '@frontmcp/testing'; import { ProductionEnvironment } from './helpers/production-environment'; -import { asAdmin, executeAction, runWorkflow } from './helpers/skilled-client'; +import { asAdmin, executeAction, loadSkill, runWorkflow } from './helpers/skilled-client'; const environment = new ProductionEnvironment(); @@ -115,6 +115,19 @@ test.describe('a DELETE action on a server that confirms them', () => { expect(requestsSent()).toEqual(['POST /v1/invoices']); }); + test('is marked in load_skill as an action that asks for confirmation; the others are not', async ({ + server, + auth, + }) => { + const client = await asAdmin({ server, auth }); + + const skill = await loadSkill(client, 'billing-admin'); + + const asking = (skill?.actions ?? []).filter((action) => action.asksForConfirmation === true); + expect(asking.map((action) => action.actionId)).toEqual(['voidInvoice']); + expect(skill?.actions?.length).toBeGreaterThan(1); + }); + test('does not ask for an action the option does not name', async ({ server, auth }) => { const { client, asked } = await adminAnswering({ server, auth }, 'decline'); diff --git a/apps/e2e/demo-e2e-skilled-openapi/e2e/helpers/skilled-client.ts b/apps/e2e/demo-e2e-skilled-openapi/e2e/helpers/skilled-client.ts index 0d7ca2bf3..cb68336a6 100644 --- a/apps/e2e/demo-e2e-skilled-openapi/e2e/helpers/skilled-client.ts +++ b/apps/e2e/demo-e2e-skilled-openapi/e2e/helpers/skilled-client.ts @@ -22,6 +22,7 @@ export interface LoadedSkill { inputJsonSchema: Record; outputJsonSchema: Record; requiredAuthorities?: Record; + asksForConfirmation?: true; }>; } diff --git a/libs/adapters/src/skills/audit/__tests__/audit-writer.spec.ts b/libs/adapters/src/skills/audit/__tests__/audit-writer.spec.ts index 47161824c..40da451b1 100644 --- a/libs/adapters/src/skills/audit/__tests__/audit-writer.spec.ts +++ b/libs/adapters/src/skills/audit/__tests__/audit-writer.spec.ts @@ -242,6 +242,21 @@ describe('SkillAuditWriter', () => { expect(result.ok).toBe(true); }); + it('records the confirmation a call passes, signed and chained with the rest of the record', async () => { + const { writer, store } = makeWriter(); + + await writer.writeAuthorityPass(ctx); + await writer.writeHttpCallSuccess({ ...ctx, confirmation: 'confirmed' }, { status: 200, output: { id: 'inv-1' } }); + await writer.writeHttpCallFailure({ ...ctx, confirmation: 'declined' }, { status: 0, error: 'the user declined' }); + const all = await store.read(); + + expect(all.map((record) => record.confirmation)).toEqual([undefined, 'confirmed', 'declined']); + expect('confirmation' in all[0]).toBe(false); + expect(verifyChain(all, TRUSTED, defaultAuditSignatureVerifier)).toEqual({ ok: true, verified: 3 }); + const edited = all.map((record, index) => (index === 1 ? { ...record, confirmation: 'skipped' as const } : record)); + expect(verifyChain(edited, TRUSTED, defaultAuditSignatureVerifier)).toMatchObject({ ok: false, breakAt: 2 }); + }); + it('does not produce a sequence gap when signing fails (M4 regression)', async () => { // Backing adapter that exposes a real `decr` for sequence rollback. const seqMap = new Map(); diff --git a/libs/adapters/src/skills/audit/audit-record.types.ts b/libs/adapters/src/skills/audit/audit-record.types.ts index 15299ffdd..b46d0e4f4 100644 --- a/libs/adapters/src/skills/audit/audit-record.types.ts +++ b/libs/adapters/src/skills/audit/audit-record.types.ts @@ -29,6 +29,18 @@ export type SkillAuditPhase = | 'http-call-success' | 'http-call-failure'; +/** + * Whether the user confirmed the call, as `@frontmcp/plugin-skilled-openapi`'s `confirm` option decided it. + * + * - `not_required`: no `confirm` rule names the action, so nobody was asked. + * - `confirmed`: the user said yes. A generated CLI's `--yes` says yes for them, so it is recorded the same. + * - `skipped`: the client can't ask, and `whenUnsupported: 'allow'` sent the request without asking. + * - `declined`: the user said no, or cancelled. Nothing was sent. + * - `unanswered`: no answer came, so nothing was sent: the client can't ask (`whenUnsupported: 'refuse'`), the + * question timed out, or a `run_workflow` script reached the action by an id it didn't name. + */ +export type SkillAuditConfirmation = 'not_required' | 'confirmed' | 'skipped' | 'declined' | 'unanswered'; + /** * Signature algorithm used to sign the audit record. * @@ -90,6 +102,11 @@ export interface SkillAuditRecord { outputHash?: string; /** Redacted, truncated error description for http-call-failure. */ errorMessage?: string; + /** + * Whether the user confirmed the call — see {@link SkillAuditConfirmation}. Set on the records written once that is + * decided, after the input check; left out of the records before it, and of records written up to 1.11.0. + */ + confirmation?: SkillAuditConfirmation; /** * sha256Hex of the canonicalized previous record (without its own signature). * The genesis record uses `'0'.repeat(64)`. Tamper detection works because diff --git a/libs/adapters/src/skills/audit/audit-writer.ts b/libs/adapters/src/skills/audit/audit-writer.ts index 825fc8d3c..2a14d170a 100644 --- a/libs/adapters/src/skills/audit/audit-writer.ts +++ b/libs/adapters/src/skills/audit/audit-writer.ts @@ -37,7 +37,12 @@ import { bytesToHex, hmacSha256, randomUUID, sha256Hex } from '@frontmcp/utils'; import { canonicalize } from '../security/bundle-signature'; import { linkRecord } from './audit-chain'; import type { SkillAuditSubjectMode } from './audit-config'; -import { SKILL_AUDIT_GENESIS_PREV_HASH, type SkillAuditPhase, type SkillAuditRecord } from './audit-record.types'; +import { + SKILL_AUDIT_GENESIS_PREV_HASH, + type SkillAuditConfirmation, + type SkillAuditPhase, + type SkillAuditRecord, +} from './audit-record.types'; import type { SkillAuditSigner } from './audit-signer'; import type { SkillAuditStore } from './audit-store'; @@ -97,6 +102,8 @@ export interface SkillAuditWriteContext { bundleVersion: string; /** Raw input — hashed via JCS before storage. Never persisted verbatim. */ input: unknown; + /** Whether the user confirmed the call, once that is decided; recorded as it is. */ + confirmation?: SkillAuditConfirmation; } /** Extra fields for the http-call-success path. */ @@ -365,6 +372,7 @@ export class SkillAuditWriter { ...(phase === 'http-call-failure' || phase === 'authority-check-fail' ? { errorMessage: this.formatError(error) } : {}), + ...(ctx.confirmation ? { confirmation: ctx.confirmation } : {}), }; // Stamp prevHash from `prev` (or the genesis sentinel for the first write). diff --git a/libs/adapters/src/skills/audit/index.ts b/libs/adapters/src/skills/audit/index.ts index 33d1e3a55..53eb96319 100644 --- a/libs/adapters/src/skills/audit/index.ts +++ b/libs/adapters/src/skills/audit/index.ts @@ -7,6 +7,7 @@ export { SKILL_AUDIT_GENESIS_PREV_HASH, SKILL_AUDIT_KEYS, + type SkillAuditConfirmation, type SkillAuditPhase, type SkillAuditRecord, type SkillAuditSignatureAlg, diff --git a/libs/adapters/src/skills/index.ts b/libs/adapters/src/skills/index.ts index 664bd8f4e..db724e6ab 100644 --- a/libs/adapters/src/skills/index.ts +++ b/libs/adapters/src/skills/index.ts @@ -150,6 +150,7 @@ export { type AuditTrustedKey, type SkillAuditAuthorityFailExtras, type SkillAuditConfig, + type SkillAuditConfirmation, type SkillAuditFailureExtras, type SkillAuditLogger, type SkillAuditMetrics, diff --git a/libs/edge/README.md b/libs/edge/README.md index 86a607790..04432e8bd 100644 --- a/libs/edge/README.md +++ b/libs/edge/README.md @@ -142,6 +142,7 @@ crons = ["*/5 * * * *"] | `pushReplay` | Where pushed tokens' `jti`s are kept: `{ binding }` (default `FRONTMCP_PUSH_REPLAY`), as below | | `allowRollback` | Apply a pulled or pushed bundle older than the one being served, as below. Default `false` | | `requireSignature`, `trustedKeys`, `dev`, `credentials`, `outbound` | Passed to the Skilled OpenAPI plugin. `dev` skips signature checks: never use it in production | +| `confirm` | Actions that ask the user before their request, as the plugin's `confirm`; see below | | `pollIntervalMs`, `bundleCacheDir` | Ignored on a Worker, which has no timers or file system | A Cron run ends once the bundle it pulled is applied and kept in `cache`, also when it is the first event an isolate gets: it then starts the bundle sync, whose first pull is the run's pull. A bundle that fails its signature or its other checks is not applied and not kept, so a cold start that can't reach the bundle server serves the last bundle the Worker applied. @@ -159,6 +160,8 @@ Up to 1.11.0, such a run reported success. A run whose pull applies a bundle, or Each operation call checks the API's host first: on a Worker, `node:dns` resolves it over DNS over HTTPS, so the host must resolve publicly. +`confirm` names the actions that ask the user before their request, by HTTP method, tag or operationId, as the Skilled OpenAPI plugin's [`confirm` option](https://frontmcp.dev/reference/plugins/skilled-openapi) does: `confirm: { methods: ['DELETE'] }`. The plugin checks it, so an invalid value refuses to start (`500 CONFIG_INVALID`, and the log names the field). The question is an MCP elicitation, which the server asks only with `elicitation: { enabled: true }`. On a Cloudflare Worker that setting refuses to start (`Elicitation requires distributed storage when running on Edge runtime`), so there an action `confirm` names is refused, `the user's confirmation is needed for voidInvoice, and this server can't ask for it: elicitation is disabled; nothing was sent`, or with `whenUnsupported: 'allow'` runs without asking. `load_skill` marks it with `asksForConfirmation: true`, and the audit log records what the confirmation came to. Up to 1.11.0 managed mode had no `confirm`, so every action ran without asking. + ### Pushed bundles With `enableWebhook: true`, the bundle server can push a new bundle instead of waiting for the next Cron run: diff --git a/libs/edge/src/__tests__/managed.spec.ts b/libs/edge/src/__tests__/managed.spec.ts index 7d53163b6..780c6be1a 100644 --- a/libs/edge/src/__tests__/managed.spec.ts +++ b/libs/edge/src/__tests__/managed.spec.ts @@ -7,6 +7,8 @@ */ import 'reflect-metadata'; +import { skilledOpenApiPluginOptionsSchema } from '@frontmcp/plugin-skilled-openapi'; + import { buildManagedOpenApiPluginOptions, createEdgeMcp, type ManagedEdgeOptions } from '../index'; const MANAGED: ManagedEdgeOptions = { @@ -51,9 +53,29 @@ describe('buildManagedOpenApiPluginOptions', () => { expect(opts['credentials']).toEqual({ 'acme-token': 'xyz' }); }); + it("passes confirm to the plugin as it is, for the plugin's schema to check", () => { + const confirm = { methods: ['DELETE' as const], tags: ['destructive'], whenUnsupported: 'allow' as const }; + + const opts = buildManagedOpenApiPluginOptions({ ...MANAGED, confirm }); + + expect(opts['confirm']).toEqual(confirm); + expect(skilledOpenApiPluginOptionsSchema.parse(opts).confirm).toEqual(confirm); + }); + + it("leaves whenUnsupported to the plugin's default, refuse", () => { + const opts = buildManagedOpenApiPluginOptions({ ...MANAGED, confirm: { operations: ['voidInvoice'] } }); + + expect(opts['confirm']).toEqual({ operations: ['voidInvoice'] }); + expect(skilledOpenApiPluginOptionsSchema.parse(opts).confirm).toEqual({ + operations: ['voidInvoice'], + whenUnsupported: 'refuse', + }); + }); + it('omits unset optional fields (no undefined leakage)', () => { const opts = buildManagedOpenApiPluginOptions(MANAGED); expect('requireSignature' in opts).toBe(false); + expect('confirm' in opts).toBe(false); expect('dev' in opts).toBe(false); expect('pollIntervalMs' in (opts['source'] as object)).toBe(false); expect('allowRollback' in (opts['source'] as object)).toBe(false); diff --git a/libs/edge/src/index.spec.ts b/libs/edge/src/index.spec.ts index cd4d84da2..313736a86 100644 --- a/libs/edge/src/index.spec.ts +++ b/libs/edge/src/index.spec.ts @@ -11,7 +11,7 @@ import { exportJWK, generateKeyPair, SignJWT } from 'jose'; import { App, Tool, ToolContext, z } from '@frontmcp/sdk'; -import { createEdgeMcp } from './index'; +import { createEdgeMcp, type ManagedEdgeOptions } from './index'; const echoInput = { message: z.string() }; @@ -434,6 +434,247 @@ describe('a managed edge Cron run in an isolate that has served no request', () }); }); +describe('a managed edge whose confirm option names an action', () => { + const ISSUER = 'https://cloud.example.dev'; + const ENDPOINT = `${ISSUER}/v1/bundles/billing`; + const JWKS_URL = `${ISSUER}/.well-known/jwks.json`; + const AUDIENCE = 'billing:prod'; + const KEY_ID = 'edge-confirm'; + const BILLING_API = 'https://billing.example.dev'; + const PROTOCOL_20260728 = '2026-07-28'; + const sent: string[] = []; + let fetchMock: jest.SpyInstance; + let pullToken: string; + + const invoiceOperation = (operationId: string, httpMethod: string, summary: string) => ({ + operationId, + serviceId: 'billing', + httpMethod, + pathTemplate: '/invoices/{id}', + summary, + public: true, + inputSchema: { type: 'object', properties: { id: { type: 'string' } }, required: ['id'] }, + outputSchema: {}, + mapper: [{ inputKey: 'id', type: 'path', key: 'id', required: true }], + authBindingRef: 'none', + }); + + const bundle = { + schemaVersion: 1, + bundleId: 'edge:billing', + version: '1.0.0', + generatedAt: '2026-11-05T12:00:00.000Z', + sourceDigest: '0'.repeat(64), + services: [{ id: 'billing', baseUrl: BILLING_API }], + authBindings: { none: { kind: 'none' } }, + skills: [ + { + id: 'invoices', + name: 'Invoices', + description: 'Look up and void invoices.', + instructions: '# Invoices', + operationIds: ['getInvoice', 'voidInvoice'], + }, + ], + operations: { + getInvoice: invoiceOperation('getInvoice', 'GET', 'Fetch an invoice'), + voidInvoice: invoiceOperation('voidInvoice', 'DELETE', 'Void an invoice'), + }, + }; + + beforeAll(async () => { + const { publicKey, privateKey } = await generateKeyPair('ES256'); + const jwks = { keys: [{ ...(await exportJWK(publicKey)), kid: KEY_ID, alg: 'ES256', use: 'sig' }] }; + pullToken = await new SignJWT({ iss: ISSUER, aud: AUDIENCE }) + .setProtectedHeader({ alg: 'ES256', kid: KEY_ID }) + .setIssuedAt() + .setExpirationTime('5m') + .sign(privateKey); + fetchMock = jest.spyOn(globalThis, 'fetch').mockImplementation(async (input, init) => { + const url = String(input); + if (url === JWKS_URL) return Response.json(jwks); + if (url === ENDPOINT) return Response.json(bundle); + if (!url.startsWith(BILLING_API)) return new Response('not found', { status: 404 }); + sent.push(`${init?.method ?? 'GET'} ${new URL(url).pathname}`); + return Response.json({ id: 'inv_1', status: init?.method === 'DELETE' ? 'void' : 'open' }); + }); + }); + + afterAll(() => { + fetchMock.mockRestore(); + }); + + beforeEach(() => { + sent.length = 0; + }); + + const managedEdge = (confirm: ManagedEdgeOptions['confirm'], elicitation = true) => + createEdgeMcp({ + info: { name: 'managed-confirm', version: '1.0.0' }, + apps: [], + tasks: { enabled: false }, + http: { entryPath: '/mcp' }, + ...(elicitation ? { elicitation: { enabled: true } } : {}), + managed: { + endpoint: ENDPOINT, + authToken: pullToken, + expectedAudience: AUDIENCE, + jwksUrl: JWKS_URL, + expectedIssuer: ISSUER, + requireSignature: false, + outbound: { allowPrivateNetworks: true }, + confirm, + }, + }); + + type ToolAnswer = { + result?: { + isError?: boolean; + resultType?: string; + content?: Array<{ text?: string }>; + structuredContent?: { skill?: { actions?: Array<{ actionId: string; asksForConfirmation?: true }> } }; + inputRequests?: Record; + requestState?: string; + }; + error?: { code: number; message: string }; + }; + + /** A `tools/call` from a client that sends no protocol version, as one without elicitation does. */ + const callTool = async (edge: ReturnType, name: string, args: Record) => + readMcp( + await edge.fetch( + new Request('https://worker.example.com/mcp', { + method: 'POST', + headers: MCP_HEADERS, + body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name, arguments: args } }), + }), + ), + ); + + /** A 2026-07-28 `tools/call` from a client that declares elicitation, with the answers of an earlier round. */ + const callToolAsking = async ( + edge: ReturnType, + name: string, + args: Record, + retry: Record = {}, + ) => + readMcp( + await edge.fetch( + new Request('https://worker.example.com/mcp', { + method: 'POST', + headers: { + ...MCP_HEADERS, + 'mcp-protocol-version': PROTOCOL_20260728, + 'mcp-method': 'tools/call', + 'mcp-name': name, + }, + body: JSON.stringify({ + jsonrpc: '2.0', + id: 2, + method: 'tools/call', + params: { + name, + arguments: args, + ...retry, + _meta: { + 'io.modelcontextprotocol/protocolVersion': PROTOCOL_20260728, + 'io.modelcontextprotocol/clientCapabilities': { elicitation: { form: {} } }, + }, + }, + }), + }), + ), + ); + + const textOf = (answer: ToolAnswer) => answer.result?.content?.[0]?.text ?? ''; + const VOID_INVOICE = { actionId: 'voidInvoice', input: { id: 'inv_1' } }; + + it("refuses the action for a client that can't ask, sending nothing, and runs one it doesn't name", async () => { + const edge = managedEdge({ methods: ['DELETE'] }); + + const refused = await callTool(edge, 'execute_action', VOID_INVOICE); + const read = await callTool(edge, 'execute_action', { actionId: 'getInvoice', input: { id: 'inv_1' } }); + + expect(refused.result?.isError).toBe(true); + expect(textOf(refused)).toBe( + "the user's confirmation is needed for voidInvoice, and this client can't ask for it: it doesn't support elicitation; nothing was sent", + ); + expect(read.result?.isError).not.toBe(true); + expect(sent).toEqual(['GET /invoices/inv_1']); + }); + + it("runs the action unasked with whenUnsupported: 'allow'", async () => { + const edge = managedEdge({ methods: ['DELETE'], whenUnsupported: 'allow' }); + + const answer = await callTool(edge, 'execute_action', VOID_INVOICE); + + expect(answer.result?.isError).not.toBe(true); + expect(sent).toEqual(['DELETE /invoices/inv_1']); + }); + + it('refuses the action on a server with elicitation disabled, which says so', async () => { + const edge = managedEdge({ methods: ['DELETE'] }, false); + + const answer = await callTool(edge, 'execute_action', VOID_INVOICE); + + expect(textOf(answer)).toContain("this server can't ask for it: elicitation is disabled"); + expect(sent).toEqual([]); + }); + + it('asks a client that declares elicitation, and sends the request once the user confirms', async () => { + const edge = managedEdge({ methods: ['DELETE'] }); + + const question = await callToolAsking(edge, 'execute_action', VOID_INVOICE); + const [[key, request]] = Object.entries(question.result?.inputRequests ?? {}); + const sentBeforeAnswer = [...sent]; + const answer = await callToolAsking(edge, 'execute_action', VOID_INVOICE, { + inputResponses: { [key]: { action: 'accept', content: { confirmed: true } } }, + requestState: question.result?.requestState, + }); + + expect(question.result?.resultType).toBe('input_required'); + expect(request.method).toBe('elicitation/create'); + expect(request.params.message).toBe( + 'Run voidInvoice: Void an invoice (DELETE /invoices/{id}) with {"id":"inv_1"}?', + ); + expect(sentBeforeAnswer).toEqual([]); + expect(answer.result?.isError).not.toBe(true); + expect(sent).toEqual(['DELETE /invoices/inv_1']); + }); + + it('marks the action in load_skill', async () => { + const edge = managedEdge({ operations: ['voidInvoice'] }); + + const loaded = await callTool(edge, 'load_skill', { skillId: 'invoices' }); + + expect(loaded.result?.structuredContent?.skill?.actions).toEqual([ + expect.not.objectContaining({ asksForConfirmation: true }), + expect.objectContaining({ actionId: 'voidInvoice', asksForConfirmation: true }), + ]); + }); + + it("refuses to start with a confirm option the plugin's schema rejects, and the log names the field", async () => { + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); + const edge = managedEdge({ methods: ['FETCH'] } as unknown as ManagedEdgeOptions['confirm']); + + try { + const response = await edge.fetch( + new Request('https://worker.example.com/mcp', { + method: 'POST', + headers: MCP_HEADERS, + body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/list', params: {} }), + }), + ); + + expect(response.status).toBe(500); + expect(await response.json()).toMatchObject({ code: 'CONFIG_INVALID' }); + expect(consoleError).toHaveBeenCalledWith(expect.stringContaining('confirm.methods.0'), expect.anything()); + } finally { + consoleError.mockRestore(); + } + }); +}); + /** * The server is built on its first request, but a config whose entries ask for protection nothing * gives them (an `approval` or `featureFlag` field no plugin enforces, `authorities` without the diff --git a/libs/edge/src/managed.ts b/libs/edge/src/managed.ts index 3a9b9dd7e..38cef09e1 100644 --- a/libs/edge/src/managed.ts +++ b/libs/edge/src/managed.ts @@ -10,6 +10,8 @@ * (which handles signature verification, replay/SSRF guards, and last-good * cache fallback). */ +import type { SkilledOpenApiPluginOptionsInput } from '@frontmcp/plugin-skilled-openapi'; + import type { EdgeBundleCacheFactory, EdgeBundleCacheStore } from './kv-cache'; import type { EdgePushReplayStoreFactory } from './push-replay'; @@ -59,6 +61,12 @@ export interface ManagedEdgeOptions { credentials?: Record; /** Outbound HTTP / SSRF options for the executor. */ outbound?: Record; + /** + * Actions that ask the user before their request, as the plugin's `confirm` option: `{ methods?, tags?, operations?, + * whenUnsupported? }`. The question is an MCP elicitation: without `elicitation: { enabled: true }`, which a + * Cloudflare Worker can't start with, such an action is refused, or runs unasked with `whenUnsupported: 'allow'`. + */ + confirm?: SkilledOpenApiPluginOptionsInput['confirm']; /** * Cache dir for the last-good bundle (boot fallback when a fresh pull fails). * Filesystem-only — a no-op on a Worker (there is no filesystem); use {@link @@ -103,6 +111,7 @@ export function buildManagedOpenApiPluginOptions(managed: ManagedEdgeOptions): R if (managed.dev !== undefined) options['dev'] = managed.dev; if (managed.credentials !== undefined) options['credentials'] = managed.credentials; if (managed.outbound !== undefined) options['outbound'] = managed.outbound; + if (managed.confirm !== undefined) options['confirm'] = managed.confirm; if (managed.bundleCacheDir !== undefined) options['bundleCacheDir'] = managed.bundleCacheDir; return options; } diff --git a/libs/skills/catalog/frontmcp-config/references/configure-elicitation.md b/libs/skills/catalog/frontmcp-config/references/configure-elicitation.md index 18e8f6775..4992cd14a 100644 --- a/libs/skills/catalog/frontmcp-config/references/configure-elicitation.md +++ b/libs/skills/catalog/frontmcp-config/references/configure-elicitation.md @@ -251,6 +251,12 @@ class Server {} elicitation fallback, where the model would answer for the user. - The generated CLI (`frontmcp build --target cli`) asks in the terminal (`y/N`), in-process and through its daemon; `--yes` answers yes, and with no terminal and no `--yes` it declines. See `build-for-cli`. +- `load_skill` marks each action that asks with `asksForConfirmation: true`. +- The skill audit log records what the confirmation came to on each call's `http-call-*` record, as `confirmation`: + `not_required`, `confirmed`, `skipped`, `declined` or `unanswered`. See `skill-audit-log`. +- On a Cloudflare Worker, `elicitation: { enabled: true }` refuses to start without a distributed store, so a managed + Worker's `confirm` (`createEdgeMcp({ managed: { confirm } })`) refuses the actions it names, or runs them unasked + with `'allow'`. See `deploy-to-cloudflare-skills-only`. ## Notes diff --git a/libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare-skills-only.md b/libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare-skills-only.md index 9966cf8b6..77c74ba7a 100644 --- a/libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare-skills-only.md +++ b/libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare-skills-only.md @@ -42,9 +42,23 @@ answer came. A workflow that fails is an `isError: true` result with the same result had no `isError`. A failed `execute_action` is an `isError: true` result whose text is that same error and whose `structuredContent` is the `{ ok: false, status, data?, error? }` outcome; on success its `structuredContent` is -the action's data. Managed mode doesn't set the plugin's `confirm` option, so -no action asks the user first; a server built with `SkilledOpenApiPlugin.init({ -confirm })` and `elicitation: { enabled: true }` can make actions ask. +the action's data. + +`managed.confirm` is the plugin's `confirm` option, checked by the plugin's +schema (an invalid value answers `500 CONFIG_INVALID`, and the log names the +field): `confirm: { methods: ['DELETE'], tags: ['destructive'], operations: +['refundInvoice'], whenUnsupported: 'refuse' }` names the actions that ask the +user before their request. The question is an MCP elicitation, and on a +Cloudflare Worker `elicitation: { enabled: true }` refuses to start +(`Elicitation requires distributed storage when running on Edge runtime`). So a +Worker can't ask: an action `confirm` names is refused, `the user's +confirmation is needed for voidInvoice, and this server can't ask for it: +elicitation is disabled; nothing was sent` (`isError: true`, for a client that +declares elicitation too), and `run_workflow` fails with the same error before +its script starts. With `whenUnsupported: 'allow'` it runs without asking. +`load_skill` marks such an action with `asksForConfirmation: true`, and the +audit log's `confirmation` field records `unanswered` or `skipped` for it. Up +to 1.11.0 managed mode had no `confirm`, so every action ran without asking. The meta-tools only show what the caller may use: a skill whose `requiredAuthorities` the caller doesn't satisfy is left out of `search_skill` diff --git a/libs/skills/catalog/frontmcp-extensibility/references/skill-audit-log.md b/libs/skills/catalog/frontmcp-extensibility/references/skill-audit-log.md index bebcd7cf2..6780fe905 100644 --- a/libs/skills/catalog/frontmcp-extensibility/references/skill-audit-log.md +++ b/libs/skills/catalog/frontmcp-extensibility/references/skill-audit-log.md @@ -31,7 +31,7 @@ run_workflow → callTool(action) | execute_action | this.callTool('.< The `write*` methods queue the record and never reject: a failed sign or append is logged as a `[skill-audit]` warning and the tool call carries on. -Each record matches the outcome the caller got. An action whose input fails its schema is recorded as `http-call-failure` with `status: 0` (no request is sent), and an answer that fails the operation's `outputSchema` as `http-call-failure` with the upstream status, both with the reason in `errorMessage`. A request aborted because its workflow ended first (the script hit its time limit, or the `run_workflow` call was cancelled) is recorded as `http-call-failure` with `status: 0` and an `errorMessage` such as `request aborted: the workflow ended before the request finished`. An action the plugin's `confirm` option names and the user declines, or that a client without elicitation is refused, is recorded as `http-call-failure` with `status: 0` and the refusal, such as `the user declined: voidInvoice`, after its `authority-check-pass`; a confirmed call writes the records of any call. A workflow the user declines before its script starts runs no action, so it writes none. `http-call-success` is written only for an answer the caller receives. +Each record matches the outcome the caller got. An action whose input fails its schema is recorded as `http-call-failure` with `status: 0` (no request is sent), and an answer that fails the operation's `outputSchema` as `http-call-failure` with the upstream status, both with the reason in `errorMessage`. A request aborted because its workflow ended first (the script hit its time limit, or the `run_workflow` call was cancelled) is recorded as `http-call-failure` with `status: 0` and an `errorMessage` such as `request aborted: the workflow ended before the request finished`. An action the plugin's `confirm` option names and the user declines, or that a client without elicitation is refused, is recorded as `http-call-failure` with `status: 0` and the refusal, such as `the user declined: voidInvoice`, after its `authority-check-pass`. A workflow the user declines before its script starts runs no action, so it writes none. `http-call-success` is written only for an answer the caller receives. Up to 1.11.0 a call of an operation tool, `this.callTool('.')`, wrote no records. @@ -58,6 +58,21 @@ Each `SkillAuditRecord` carries: | `inputHash` | SHA-256 hex of the canonical input — the input itself is never stored | | `outputHash` | SHA-256 hex of the canonical response body, on `http-call-success` | | `errorMessage` | Truncated error (or denial reason), on `http-call-failure` and `authority-check-fail` | +| `confirmation` | Whether the user confirmed the call, on `http-call-*` once its input passed; see below | + +### Whether the user confirmed the call + +A record written once a call's input passed its schema check, its `http-call-success` or `http-call-failure`, says what the plugin's `confirm` option came to, in `confirmation`. The `authority-check-*` records, and the `http-call-failure` of an input that fails its schema, come before it and leave it out, as do records written up to 1.11.0. + +| `confirmation` | When | +| -------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `not_required` | No `confirm` rule names the action, so nobody was asked | +| `confirmed` | The user said yes. A generated CLI's `--yes` answers yes for the user, so the server can't tell it apart and records it the same | +| `skipped` | The client can't ask (or the server has elicitation disabled), and `whenUnsupported: 'allow'` sent the request without asking | +| `declined` | The user said no, or cancelled (a generated CLI with no terminal declines). Nothing was sent | +| `unanswered` | No answer came, so nothing was sent: the client can't ask (`whenUnsupported: 'refuse'`), the question timed out, or a script reached the action by an id it didn't name | + +In a `run_workflow`, each action call carries the workflow's answer: the calls of the actions its question named get `confirmed` (or `skipped`), and the calls of actions no rule names get `not_required`. The field is signed with the rest of the record, so a verifier from 1.11.0 or before still verifies a record that has it. `SkillAuditConfirmation` is the field's type, and a plugin that writes its own records may set `confirmation` on the `SkillAuditWriteContext` it passes. ## Configuration diff --git a/plugins/plugin-skilled-openapi/README.md b/plugins/plugin-skilled-openapi/README.md index 5d7b350cb..eb66ed83f 100644 --- a/plugins/plugin-skilled-openapi/README.md +++ b/plugins/plugin-skilled-openapi/README.md @@ -212,11 +212,29 @@ yes/no form: with an error that tells the model to write the id as a string. - **A decline** sends nothing. The call fails with `the user declined: voidInvoice` (`isError: true`), and a declined workflow runs none of its script, reads included. The audit log records a declined call as `http-call-failure` with - status 0 and that error; a confirmed call's records are those of any call. + status 0 and that error. - **A client that can't show the question**, because it declared no elicitation capability or the server has elicitation disabled, is refused: `the user's confirmation is needed for voidInvoice, and this client can't ask for it: it doesn't support elicitation; nothing was sent`. With `whenUnsupported: 'allow'` the action runs unasked there instead. The plugin doesn't use the SDK's elicitation fallback, in which the model would answer for the user. +- **`load_skill`** marks each action that asks with `asksForConfirmation: true`, so the model knows the user will be + asked before it calls it. + +The audit records of a call say what the confirmation came to, in a `confirmation` field. It is set on the call's +`http-call-success` or `http-call-failure` once its input passed the schema check. The `authority-check-*` records, +and the failure of an input that fails the schema, come before the confirmation and have none: + +| `confirmation` | When | +| -------------- | ----------------------------------------------------------------------------------------------------------------------------------------- | +| `not_required` | No `confirm` rule names the action, so nobody was asked | +| `confirmed` | The user said yes. The CLI's `--yes` says yes for the user, so the server records it the same | +| `skipped` | The client can't ask, and `whenUnsupported: 'allow'` sent the request without asking | +| `declined` | The user said no, or cancelled. Nothing was sent | +| `unanswered` | No answer came, so nothing was sent: the client can't ask, the question timed out, or a script reached the action by an id it didn't name | + +A workflow's action calls carry the workflow's answer: `confirmed` (or `skipped`) for the actions it asked about, and +`not_required` for the others. A workflow the user declines runs no action, so it writes no records. Up to 1.11.0 the +records had no `confirmation`, so a confirmed call's records were those of a call no rule names. The generated CLI (`frontmcp build --target cli`) asks the question in the terminal, in-process and through its daemon: it reads `y/N` when standard input is a terminal, and `--yes` answers yes without asking. With no terminal and no diff --git a/plugins/plugin-skilled-openapi/src/__tests__/audit-and-startup.spec.ts b/plugins/plugin-skilled-openapi/src/__tests__/audit-and-startup.spec.ts index e946a99fe..d779ff460 100644 --- a/plugins/plugin-skilled-openapi/src/__tests__/audit-and-startup.spec.ts +++ b/plugins/plugin-skilled-openapi/src/__tests__/audit-and-startup.spec.ts @@ -52,7 +52,7 @@ function auditedDeps() { }, unprotectedOps: 'allow', }, - confirmAction: jest.fn(async () => undefined), + confirmAction: jest.fn(async () => ({ confirmation: 'confirmed' as const })), resolver: { resolve: jest.fn(async () => undefined) }, guard: { check: jest.fn(async () => ({ granted: true })) } as never, visibleListings: scopeHidingSkills().visibleListings, diff --git a/plugins/plugin-skilled-openapi/src/__tests__/confirm-actions.integration.spec.ts b/plugins/plugin-skilled-openapi/src/__tests__/confirm-actions.integration.spec.ts index 3673031db..b8ee91a5b 100644 --- a/plugins/plugin-skilled-openapi/src/__tests__/confirm-actions.integration.spec.ts +++ b/plugins/plugin-skilled-openapi/src/__tests__/confirm-actions.integration.spec.ts @@ -160,19 +160,28 @@ function textOf(result: CallToolResult): string { return result.content.map((block) => (block.type === 'text' ? block.text : '')).join(''); } -async function auditedPhases(auditStore: MemoryAuditStore, count: number): Promise { +async function auditRecords(auditStore: MemoryAuditStore, count: number): Promise { const deadline = Date.now() + 5_000; let records: SkillAuditRecord[] = []; while (Date.now() < deadline) { records = await auditStore.read(); - if (records.length >= count) { - return records.map((record) => [record.phase, record.actionId, record.errorMessage].filter(Boolean).join(':')); - } + if (records.length >= count) return records; await new Promise((resolve) => setTimeout(resolve, 10)); } throw new Error(`expected ${count} audit records, got ${records.length}`); } +async function auditedPhases(auditStore: MemoryAuditStore, count: number): Promise { + const records = await auditRecords(auditStore, count); + return records.map((record) => [record.phase, record.actionId, record.errorMessage].filter(Boolean).join(':')); +} + +/** Each record as `phase:actionId:confirmation`, the confirmation left out where the record has none. */ +async function auditedConfirmations(auditStore: MemoryAuditStore, count: number): Promise { + const records = await auditRecords(auditStore, count); + return records.map((record) => [record.phase, record.actionId, record.confirmation].filter(Boolean).join(':')); +} + const originalFetch = global.fetch; beforeAll(() => { @@ -366,3 +375,144 @@ describe('a server with no confirm option', () => { expect(sent).toEqual(['DELETE /v1/invoices/inv_1', 'DELETE /v1/invoices/inv_2']); }); }); + +describe('the audit records of a call say whether the user confirmed it', () => { + it('records confirmed for a call the user confirmed, and declined for one the user declined', async () => { + const confirmedStore = new MemoryAuditStore(); + const declinedStore = new MemoryAuditStore(); + const { client: confirmingClient } = await connectClient({ answer: 'accept', auditStore: confirmedStore }); + const { client: decliningClient } = await connectClient({ answer: 'decline', auditStore: declinedStore }); + + await call(confirmingClient, 'execute_action', { actionId: 'voidInvoice', input: { id: 'inv_1' } }); + await call(decliningClient, 'execute_action', { actionId: 'voidInvoice', input: { id: 'inv_1' } }); + + expect(await auditedConfirmations(confirmedStore, 2)).toEqual([ + 'authority-check-pass:voidInvoice', + 'http-call-success:voidInvoice:confirmed', + ]); + expect(await auditedConfirmations(declinedStore, 2)).toEqual([ + 'authority-check-pass:voidInvoice', + 'http-call-failure:voidInvoice:declined', + ]); + }); + + it('records not_required for an action no rule names', async () => { + const auditStore = new MemoryAuditStore(); + const { client, asked } = await connectClient({ answer: 'decline', auditStore }); + + await call(client, 'execute_action', { actionId: 'getInvoice', input: { id: 'inv_1' } }); + + expect(asked).toEqual([]); + expect(await auditedConfirmations(auditStore, 2)).toEqual([ + 'authority-check-pass:getInvoice', + 'http-call-success:getInvoice:not_required', + ]); + }); + + it("records unanswered for a client that can't ask, and skipped when whenUnsupported is 'allow'", async () => { + const refusedStore = new MemoryAuditStore(); + const allowedStore = new MemoryAuditStore(); + const { client: refusedClient } = await connectClient({ auditStore: refusedStore }); + const { client: allowedClient } = await connectClient({ + auditStore: allowedStore, + plugin: { confirm: { ...CONFIRM_DELETES_AND_TAGGED, whenUnsupported: 'allow' } }, + }); + + await call(refusedClient, 'execute_action', { actionId: 'voidInvoice', input: { id: 'inv_1' } }); + await call(allowedClient, 'execute_action', { actionId: 'voidInvoice', input: { id: 'inv_1' } }); + + expect(await auditedConfirmations(refusedStore, 2)).toEqual([ + 'authority-check-pass:voidInvoice', + 'http-call-failure:voidInvoice:unanswered', + ]); + expect(await auditedConfirmations(allowedStore, 2)).toEqual([ + 'authority-check-pass:voidInvoice', + 'http-call-success:voidInvoice:skipped', + ]); + }); + + it("gives each action call of a workflow the workflow's confirmation, and not_required to those no rule names", async () => { + const auditStore = new MemoryAuditStore(); + const { client, asked } = await connectClient({ answer: 'accept', auditStore }); + const script = [ + "const invoice = await callTool('getInvoice', { id: 'inv_1' });", + "await callTool('voidInvoice', { id: invoice.id });", + "await callTool('voidInvoice', { id: 'inv_2' });", + 'return invoice.id;', + ].join('\n'); + + await call(client, 'run_workflow', { script }); + + expect(asked).toHaveLength(1); + expect(await auditedConfirmations(auditStore, 6)).toEqual([ + 'authority-check-pass:getInvoice', + 'http-call-success:getInvoice:not_required', + 'authority-check-pass:voidInvoice', + 'http-call-success:voidInvoice:confirmed', + 'authority-check-pass:voidInvoice', + 'http-call-success:voidInvoice:confirmed', + ]); + }); + + it('records skipped on the action calls of a workflow a client without elicitation runs under allow', async () => { + const auditStore = new MemoryAuditStore(); + const { client } = await connectClient({ + auditStore, + plugin: { confirm: { ...CONFIRM_DELETES_AND_TAGGED, whenUnsupported: 'allow' } }, + }); + + await call(client, 'run_workflow', { script: "return await callTool('voidInvoice', { id: 'inv_1' });" }); + + expect(await auditedConfirmations(auditStore, 2)).toEqual([ + 'authority-check-pass:voidInvoice', + 'http-call-success:voidInvoice:skipped', + ]); + }); + + it('records unanswered for a workflow call of an action it reached by a computed id', async () => { + const auditStore = new MemoryAuditStore(); + const { client } = await connectClient({ answer: 'accept', auditStore }); + + await call(client, 'run_workflow', { + script: "const verb = 'void'; return await callTool(verb + 'Invoice', { id: 'inv_1' });", + }); + + expect(await auditedConfirmations(auditStore, 2)).toEqual([ + 'authority-check-pass:voidInvoice', + 'http-call-failure:voidInvoice:unanswered', + ]); + }); +}); + +describe('load_skill', () => { + interface LoadedActions { + skill: { actions: Array<{ actionId: string; asksForConfirmation?: true }> }; + } + + it('marks the actions that ask for confirmation', async () => { + const { client } = await connectClient({ answer: 'accept' }); + + const invoices = await call(client, 'load_skill', { skillId: 'invoices' }); + const cleanup = await call(client, 'load_skill', { skillId: 'cleanup' }); + + expect((invoices.structuredContent as unknown as LoadedActions).skill.actions).toEqual([ + expect.not.objectContaining({ asksForConfirmation: expect.anything() }), + expect.objectContaining({ actionId: 'voidInvoice', asksForConfirmation: true }), + expect.objectContaining({ actionId: 'refundInvoice', asksForConfirmation: true }), + ]); + expect((invoices.structuredContent as unknown as LoadedActions).skill.actions[0].actionId).toBe('getInvoice'); + expect((cleanup.structuredContent as unknown as LoadedActions).skill.actions).toEqual([ + expect.objectContaining({ actionId: 'purgeDraft', asksForConfirmation: true }), + ]); + }); + + it('marks no action on a server with no confirm option', async () => { + const { client } = await connectClient({ plugin: { confirm: undefined } }); + + const invoices = await call(client, 'load_skill', { skillId: 'invoices' }); + + const { actions } = (invoices.structuredContent as unknown as LoadedActions).skill; + expect(actions.map((action) => action.actionId)).toEqual(['getInvoice', 'voidInvoice', 'refundInvoice']); + expect(actions.some((action) => 'asksForConfirmation' in action)).toBe(false); + }); +}); diff --git a/plugins/plugin-skilled-openapi/src/executor/__tests__/action-confirmation.spec.ts b/plugins/plugin-skilled-openapi/src/executor/__tests__/action-confirmation.spec.ts index d6c3ae4da..9d602581f 100644 --- a/plugins/plugin-skilled-openapi/src/executor/__tests__/action-confirmation.spec.ts +++ b/plugins/plugin-skilled-openapi/src/executor/__tests__/action-confirmation.spec.ts @@ -91,7 +91,9 @@ describe('askingBeforeEachCall', () => { it('asks about the action, its summary, method, path and input, and lets it run when the user confirms', async () => { const askUser = jest.fn(accepted); - await expect(asking(askUser, { entry: voidInvoice, input: { id: 'inv_1' } })).resolves.toBeUndefined(); + await expect(asking(askUser, { entry: voidInvoice, input: { id: 'inv_1' } })).resolves.toEqual({ + confirmation: 'confirmed', + }); expect(askUser).toHaveBeenCalledWith( 'Run voidInvoice: Void an invoice (DELETE /invoices/{id}) with {"id":"inv_1"}?', ); @@ -111,48 +113,53 @@ describe('askingBeforeEachCall', () => { ['declines', { status: 'decline' as const }], ['cancels', { status: 'cancel' as const }], ['accepts without confirming', { status: 'accept' as const, content: { confirmed: false } }], - ])('refuses the call when the user %s', async (_answer, answer) => { - const refusal = await asking(async () => answer, { entry: voidInvoice, input: { id: 'inv_1' } }); + ])('refuses the call, as declined, when the user %s', async (_answer, answer) => { + const decision = await asking(async () => answer, { entry: voidInvoice, input: { id: 'inv_1' } }); - expect(refusal).toBe('the user declined: voidInvoice'); + expect(decision).toEqual({ confirmation: 'declined', refusal: 'the user declined: voidInvoice' }); }); - it('refuses the call when the user does not answer in time', async () => { - const refusal = await asking( + it('refuses the call, as unanswered, when the user does not answer in time', async () => { + const decision = await asking( async () => { throw new ElicitationTimeoutError('elicit-1', 300_000); }, { entry: voidInvoice, input: {} }, ); - expect(refusal).toBe('the user did not answer in time: voidInvoice'); + expect(decision).toEqual({ confirmation: 'unanswered', refusal: 'the user did not answer in time: voidInvoice' }); }); it.each([ ['takes the elicitation fallback', new ElicitationFallbackRequired('elicit-1', 'Run?', {}, 'tool', {}, 1000)], ['has no elicitation support', new ElicitationNotSupportedError()], ['declares no elicitation capability', new MissingClientCapabilityError({ elicitation: {} }, 'no elicitation')], - ])('refuses, or with allow runs, a call from a client that %s', async (_client, error) => { + ])('refuses as unanswered, or with allow runs as skipped, a call from a client that %s', async (_client, error) => { const askUser: AskUser = async () => { throw error; }; const call = { entry: voidInvoice, input: { id: 'inv_1' } }; - await expect(asking(askUser, call)).resolves.toBe( - "the user's confirmation is needed for voidInvoice, and this client can't ask for it: it doesn't support elicitation; nothing was sent", - ); - await expect(asking(askUser, call, 'allow')).resolves.toBeUndefined(); + await expect(asking(askUser, call)).resolves.toEqual({ + confirmation: 'unanswered', + refusal: + "the user's confirmation is needed for voidInvoice, and this client can't ask for it: it doesn't support elicitation; nothing was sent", + }); + await expect(asking(askUser, call, 'allow')).resolves.toEqual({ confirmation: 'skipped' }); }); it('refuses a call on a server with elicitation disabled', async () => { - const refusal = await asking( + const decision = await asking( async () => { throw new ElicitationDisabledError(); }, { entry: voidInvoice, input: {} }, ); - expect(refusal).toContain("this server can't ask for it: elicitation is disabled"); + expect(decision).toEqual({ + confirmation: 'unanswered', + refusal: expect.stringContaining("this server can't ask for it: elicitation is disabled"), + }); }); it('rethrows what is not an answer, such as a request for input the client sends on its retry', async () => { @@ -233,12 +240,45 @@ describe('confirmWorkflow', () => { const confirmation = await confirming(CLEANUP, accepted, { methods: ['DELETE'] }); if (!('confirmAction' in confirmation)) throw new Error('the workflow was refused'); - await expect(confirmation.confirmAction({ entry: voidInvoice, input: { id: 'inv_2' } })).resolves.toBeUndefined(); + await expect(confirmation.confirmAction({ entry: voidInvoice, input: { id: 'inv_2' } })).resolves.toEqual({ + confirmation: 'confirmed', + }); await expect( confirmation.confirmAction({ entry: entry('dropTable', { httpMethod: 'DELETE' }), input: {} }), - ).resolves.toContain( - `write the actionId as a string, as in callTool('dropTable', …), or run it with execute_action`, - ); + ).resolves.toEqual({ + confirmation: 'unanswered', + refusal: expect.stringContaining( + `write the actionId as a string, as in callTool('dropTable', …), or run it with execute_action`, + ), + }); + }); + + it("carries the workflow's answer, skipped, to the calls of the actions it would have asked about", async () => { + const clientWithoutElicitation: AskUser = async () => { + throw new ElicitationNotSupportedError(); + }; + + const confirmation = await confirming(CLEANUP, clientWithoutElicitation, { + methods: ['DELETE'], + whenUnsupported: 'allow', + }); + if (!('confirmAction' in confirmation)) throw new Error('the workflow was refused'); + + await expect(confirmation.confirmAction({ entry: voidInvoice, input: { id: 'inv_2' } })).resolves.toEqual({ + confirmation: 'skipped', + }); + }); + + it('refuses, as unanswered, an action that needs confirmation in a workflow that asked about none', async () => { + const askUser = jest.fn(accepted); + + const confirmation = await confirming("return await callTool('getInvoice', {});", askUser, { methods: ['DELETE'] }); + if (!('confirmAction' in confirmation)) throw new Error('the workflow was refused'); + + await expect(confirmation.confirmAction({ entry: voidInvoice, input: { id: 'inv_2' } })).resolves.toMatchObject({ + confirmation: 'unanswered', + }); + expect(askUser).not.toHaveBeenCalled(); }); it('does not ask about an action whose every listing skill the skills:filter flow hides from the caller', async () => { diff --git a/plugins/plugin-skilled-openapi/src/executor/__tests__/execute-skill-action.spec.ts b/plugins/plugin-skilled-openapi/src/executor/__tests__/execute-skill-action.spec.ts index 27e91cb30..ca47748d1 100644 --- a/plugins/plugin-skilled-openapi/src/executor/__tests__/execute-skill-action.spec.ts +++ b/plugins/plugin-skilled-openapi/src/executor/__tests__/execute-skill-action.spec.ts @@ -62,7 +62,7 @@ function makeDeps( }, ...(over.confirm ? { confirm: over.confirm } : {}), } as SkillActionDeps['config'], - confirmAction: over.confirmAction ?? jest.fn(async () => undefined), + confirmAction: over.confirmAction ?? jest.fn(async () => ({ confirmation: 'confirmed' as const })), resolver: { resolve: jest.fn(async () => undefined) as never }, guard: { check: jest.fn(async () => ({ granted: over.granted ?? true, deniedBy: over.deniedBy })) } as never, visibleListings: over.visibleListings ?? scopeHidingSkills().visibleListings, @@ -297,7 +297,7 @@ describe('executeSkillAction: an action the confirm option names', () => { }); it('asks with the listing and the validated input after authorization, and sends the request once confirmed', async () => { - const confirmAction = jest.fn(async () => undefined); + const confirmAction = jest.fn(async () => ({ confirmation: 'confirmed' as const })); const entry = deleteThing(); const res = await executeSkillAction({ @@ -321,7 +321,10 @@ describe('executeSkillAction: an action the confirm option names', () => { }; const deps = makeDeps({ confirm: DELETES, - confirmAction: jest.fn(async () => 'the user declined: deleteThing'), + confirmAction: jest.fn(async () => ({ + confirmation: 'declined' as const, + refusal: 'the user declined: deleteThing', + })), audit: { writer: writer as never, subject: 'u' }, }); @@ -330,10 +333,10 @@ describe('executeSkillAction: an action the confirm option names', () => { expect(res).toEqual({ ok: false, status: 0, error: 'the user declined: deleteThing' }); expect(mockExecuteOperation).not.toHaveBeenCalled(); expect(writer.writeAuthorityPass).toHaveBeenCalledTimes(1); - expect(writer.writeHttpCallFailure).toHaveBeenCalledWith(expect.objectContaining({ actionId: 'deleteThing' }), { - status: 0, - error: 'the user declined: deleteThing', - }); + expect(writer.writeHttpCallFailure).toHaveBeenCalledWith( + expect.objectContaining({ actionId: 'deleteThing', confirmation: 'declined' }), + { status: 0, error: 'the user declined: deleteThing' }, + ); expect(writer.writeHttpCallSuccess).not.toHaveBeenCalled(); }); @@ -341,7 +344,7 @@ describe('executeSkillAction: an action the confirm option names', () => { ['the caller is denied', { granted: false, deniedBy: 'role:admin' }, { id: 7 }], ['the input is invalid', {}, { id: 'seven' }], ])('does not ask when %s', async (_case, guard, input) => { - const confirmAction = jest.fn(async () => undefined); + const confirmAction = jest.fn(async () => ({ confirmation: 'confirmed' as const })); const res = await executeSkillAction({ listings: [deleteThing()], @@ -356,7 +359,7 @@ describe('executeSkillAction: an action the confirm option names', () => { }); it('does not ask for an action no rule names, nor when there is no confirm option', async () => { - const confirmAction = jest.fn(async () => undefined); + const confirmAction = jest.fn(async () => ({ confirmation: 'confirmed' as const })); await executeSkillAction({ listings: [makeEntry()], @@ -376,6 +379,122 @@ describe('executeSkillAction: an action the confirm option names', () => { }); }); +describe('executeSkillAction: whether the audit records say the call was confirmed', () => { + const DELETES = { methods: ['DELETE' as const], whenUnsupported: 'refuse' as const }; + const deleteThing = () => makeEntry({ operationId: 'deleteThing', httpMethod: 'DELETE' }); + + function recordingWriter() { + const records: Array<{ phase: string; confirmation?: string }> = []; + const record = + (phase: string) => + async (ctx: { confirmation?: string }): Promise => { + records.push({ phase, ...(ctx.confirmation ? { confirmation: ctx.confirmation } : {}) }); + }; + const writer = { + writeAuthorityPass: jest.fn(record('authority-check-pass')), + writeAuthorityFail: jest.fn(record('authority-check-fail')), + writeHttpCallSuccess: jest.fn(record('http-call-success')), + writeHttpCallFailure: jest.fn(record('http-call-failure')), + }; + return { audit: { writer: writer as never, subject: 'u' }, records }; + } + + beforeEach(() => { + mockExecuteOperation.mockReset(); + mockExecuteOperation.mockResolvedValue({ ok: true, status: 204 }); + }); + + it('records not_required for an action no rule names, from the records written after the input check', async () => { + const { audit, records } = recordingWriter(); + + await executeSkillAction({ + listings: [makeEntry()], + input: { id: 1 }, + authInfo: {}, + deps: makeDeps({ confirm: DELETES, audit }), + }); + + expect(records).toEqual([ + { phase: 'authority-check-pass' }, + { phase: 'http-call-success', confirmation: 'not_required' }, + ]); + }); + + it.each([['confirmed' as const], ['skipped' as const]])( + "records the decision, %s, of a call that ran, on its outcome's record", + async (confirmation) => { + const { audit, records } = recordingWriter(); + + await executeSkillAction({ + listings: [deleteThing()], + input: { id: 7 }, + authInfo: {}, + deps: makeDeps({ confirm: DELETES, confirmAction: async () => ({ confirmation }), audit }), + }); + + expect(records).toEqual([{ phase: 'authority-check-pass' }, { phase: 'http-call-success', confirmation }]); + }, + ); + + it('records the decision of a call that ran and then failed', async () => { + mockExecuteOperation.mockResolvedValue({ ok: false, status: 500, error: 'boom' }); + const { audit, records } = recordingWriter(); + + await executeSkillAction({ + listings: [deleteThing()], + input: { id: 7 }, + authInfo: {}, + deps: makeDeps({ confirm: DELETES, audit }), + }); + + expect(records).toEqual([ + { phase: 'authority-check-pass' }, + { phase: 'http-call-failure', confirmation: 'confirmed' }, + ]); + }); + + it('records unanswered for a call refused because no answer came', async () => { + const { audit, records } = recordingWriter(); + const refusal = 'the user did not answer in time: deleteThing'; + + const res = await executeSkillAction({ + listings: [deleteThing()], + input: { id: 7 }, + authInfo: {}, + deps: makeDeps({ confirm: DELETES, confirmAction: async () => ({ confirmation: 'unanswered', refusal }), audit }), + }); + + expect(res).toEqual({ ok: false, status: 0, error: refusal }); + expect(records).toEqual([ + { phase: 'authority-check-pass' }, + { phase: 'http-call-failure', confirmation: 'unanswered' }, + ]); + }); + + it('records no confirmation on a call that ends before it is decided', async () => { + const { audit, records } = recordingWriter(); + + await executeSkillAction({ + listings: [deleteThing()], + input: { id: 'seven' }, + authInfo: {}, + deps: makeDeps({ confirm: DELETES, audit }), + }); + await executeSkillAction({ + listings: [deleteThing()], + input: { id: 7 }, + authInfo: {}, + deps: makeDeps({ granted: false, deniedBy: 'role:admin', confirm: DELETES, audit }), + }); + + expect(records).toEqual([ + { phase: 'authority-check-pass' }, + { phase: 'http-call-failure' }, + { phase: 'authority-check-fail' }, + ]); + }); +}); + describe('authorizeAction: an operation several skills list', () => { const ADMIN = { roles: { any: ['admin'] } }; const VIEWER = { user: { iss: 'test', sub: 'viewer', roles: ['viewer'] } }; diff --git a/plugins/plugin-skilled-openapi/src/executor/action-confirmation.ts b/plugins/plugin-skilled-openapi/src/executor/action-confirmation.ts index e31c7c2f2..6c3f619d9 100644 --- a/plugins/plugin-skilled-openapi/src/executor/action-confirmation.ts +++ b/plugins/plugin-skilled-openapi/src/executor/action-confirmation.ts @@ -1,3 +1,4 @@ +import { type SkillAuditConfirmation } from '@frontmcp/adapters/skills'; import { z } from '@frontmcp/lazy-zod'; import { ElicitationDisabledError, @@ -26,8 +27,22 @@ export interface ConfirmationSubject { input?: Record; } -/** Whether an action call that needs the user's confirmation may send its request: undefined when it may, else why not. */ -export type ConfirmAction = (call: Required) => Promise; +/** A call that may send its request: the user said yes, or no one could be asked and `whenUnsupported` allows it. */ +export interface AllowedCall { + confirmation: Extract; +} + +/** A call that may not send its request, and why. */ +export interface RefusedCall { + confirmation: Extract; + refusal: string; +} + +/** What the confirmation of a call decided, as the audit log records it. */ +export type ConfirmationDecision = AllowedCall | RefusedCall; + +/** Whether an action call that needs the user's confirmation may send its request. */ +export type ConfirmAction = (call: Required) => Promise; const MAX_INPUT_TEXT_LENGTH = 1000; @@ -59,10 +74,8 @@ export async function confirmWorkflow(args: { askUser: AskUser; }): Promise<{ refusal: string } | { confirmAction: ConfirmAction }> { const { script, config, guard, visibleListings, hiddenOps, authInfo, askUser } = args; - const confirmedActionIds = new Set(); - const confirmAction: ConfirmAction = async ({ entry }) => - confirmedActionIds.has(entry.op.operationId) ? undefined : notAskedByWorkflow(entry.op.operationId); - if (!config.confirm) return { confirmAction }; + const askedActionIds = new Set(); + if (!config.confirm) return { confirmAction: confirmingAskedActions(askedActionIds) }; const mayRun = async (listings: OperationListings): Promise => { for (const entry of await visibleListings(listings)) { @@ -82,33 +95,47 @@ export async function confirmWorkflow(args: { const listings = hiddenOps.listingsOf(actionId); if (!listings || !needsConfirmation(config.confirm, listings) || !(await mayRun(listings))) continue; subjects.push({ entry: listings[0] }); - confirmedActionIds.add(actionId); + askedActionIds.add(actionId); } - if (subjects.length === 0) return { confirmAction }; + if (subjects.length === 0) return { confirmAction: confirmingAskedActions(askedActionIds) }; - const refusal = await confirmWithUser(askUser, subjects, config.confirm.whenUnsupported); - return refusal ? { refusal } : { confirmAction }; + const decision = await confirmWithUser(askUser, subjects, config.confirm.whenUnsupported); + if ('refusal' in decision) return { refusal: decision.refusal }; + return { confirmAction: confirmingAskedActions(askedActionIds, decision) }; } -/** Ask the user to confirm the actions: undefined when they may run, else why they may not. */ +/** A workflow's calls of the actions it asked about carry its answer; other actions that need confirmation are refused. */ +function confirmingAskedActions(askedActionIds: ReadonlySet, answer?: AllowedCall): ConfirmAction { + return async ({ entry }) => { + const actionId = entry.op.operationId; + return answer && askedActionIds.has(actionId) ? answer : notAskedByWorkflow(actionId); + }; +} + +/** Ask the user to confirm the actions: whether they may run, and why not when they may not. */ async function confirmWithUser( askUser: AskUser, subjects: readonly ConfirmationSubject[], whenUnsupported: WhenConfirmationUnsupported, -): Promise { +): Promise { const actionIds = subjects.map(({ entry }) => entry.op.operationId).join(', '); let answer: ElicitResult<{ confirmed: boolean }>; try { answer = await askUser(confirmationMessage(subjects)); } catch (error) { - if (error instanceof ElicitationTimeoutError) return `the user did not answer in time: ${actionIds}`; + if (error instanceof ElicitationTimeoutError) { + return { confirmation: 'unanswered', refusal: `the user did not answer in time: ${actionIds}` }; + } const unavailable = whyUserCannotBeAsked(error); if (!unavailable) throw error; - if (whenUnsupported === 'allow') return undefined; - return `the user's confirmation is needed for ${actionIds}, and ${unavailable}; nothing was sent`; + if (whenUnsupported === 'allow') return { confirmation: 'skipped' }; + const refusal = `the user's confirmation is needed for ${actionIds}, and ${unavailable}; nothing was sent`; + return { confirmation: 'unanswered', refusal }; } const confirmed = answer.status === 'accept' && answer.content?.confirmed === true; - return confirmed ? undefined : `the user declined: ${actionIds}`; + return confirmed + ? { confirmation: 'confirmed' } + : { confirmation: 'declined', refusal: `the user declined: ${actionIds}` }; } function whyUserCannotBeAsked(error: unknown): string | undefined { @@ -135,9 +162,9 @@ function actionText({ entry: { op }, input }: ConfirmationSubject): string { return `${action} with ${shownInput}`; } -function notAskedByWorkflow(actionId: string): string { - return ( +function notAskedByWorkflow(actionId: string): RefusedCall { + const refusal = `action "${actionId}" needs the user's confirmation, which run_workflow asks for before the script starts, ` + - `for the actions it names: write the actionId as a string, as in callTool('${actionId}', …), or run it with execute_action` - ); + `for the actions it names: write the actionId as a string, as in callTool('${actionId}', …), or run it with execute_action`; + return { confirmation: 'unanswered', refusal }; } diff --git a/plugins/plugin-skilled-openapi/src/executor/execute-skill-action.ts b/plugins/plugin-skilled-openapi/src/executor/execute-skill-action.ts index c230badbc..277f37618 100644 --- a/plugins/plugin-skilled-openapi/src/executor/execute-skill-action.ts +++ b/plugins/plugin-skilled-openapi/src/executor/execute-skill-action.ts @@ -69,6 +69,9 @@ export interface ActionAuthorization { hidden?: true; } +/** The decision of a call no `confirm` rule names: it runs without asking. */ +const NO_CONFIRMATION_REQUIRED = { confirmation: 'not_required' } as const; + /** Why the audit log records a call denied because `skills:filter` hides every skill listing its operation. */ const HIDDEN_SKILLS_REASON = 'skills:filter hides every skill that lists the action'; @@ -178,12 +181,14 @@ export async function executeSkillAction(args: { const validInput = inputParse.data as Record; // 3) An action the `confirm` option names asks the user before its request; a refusal sends nothing. - if (needsConfirmation(config.confirm, listings)) { - const refusal = await deps.confirmAction({ entry, input: validInput }); - if (refusal) { - detach(audit?.writer.writeHttpCallFailure(auditCtx, { status: 0, error: refusal }), 'http-call-failure'); - return { ok: false, status: 0, error: refusal }; - } + const decision = needsConfirmation(config.confirm, listings) + ? await deps.confirmAction({ entry, input: validInput }) + : NO_CONFIRMATION_REQUIRED; + const decidedAuditCtx = { ...auditCtx, confirmation: decision.confirmation }; + if ('refusal' in decision) { + const error = decision.refusal; + detach(audit?.writer.writeHttpCallFailure(decidedAuditCtx, { status: 0, error }), 'http-call-failure'); + return { ok: false, status: 0, error }; } // 4) Allowed hosts: ONLY the single service this op is bound to (B3). @@ -218,12 +223,12 @@ export async function executeSkillAction(args: { deps: runtimeDeps, }); } catch (e) { - detach(audit?.writer.writeHttpCallFailure(auditCtx, { status: 0, error: e }), 'http-call-failure'); + detach(audit?.writer.writeHttpCallFailure(decidedAuditCtx, { status: 0, error: e }), 'http-call-failure'); throw e; } if (!result.ok) { detach( - audit?.writer.writeHttpCallFailure(auditCtx, { + audit?.writer.writeHttpCallFailure(decidedAuditCtx, { status: result.status, error: result.error ?? `http call failed with status ${result.status}`, }), @@ -237,7 +242,10 @@ export async function executeSkillAction(args: { const outputParse = schemas.output.safeParse(result.data); if (!outputParse.success) { const error = `upstream response failed output schema: ${formatZodIssues(outputParse.error.issues)}`; - detach(audit?.writer.writeHttpCallFailure(auditCtx, { status: result.status, error }), 'http-call-failure'); + detach( + audit?.writer.writeHttpCallFailure(decidedAuditCtx, { status: result.status, error }), + 'http-call-failure', + ); return { ok: false, status: result.status, @@ -248,7 +256,7 @@ export async function executeSkillAction(args: { } if (result.ok) { detach( - audit?.writer.writeHttpCallSuccess(auditCtx, { status: result.status, output: result.data ?? null }), + audit?.writer.writeHttpCallSuccess(decidedAuditCtx, { status: result.status, output: result.data ?? null }), 'http-call-success', ); } diff --git a/plugins/plugin-skilled-openapi/src/tools/load-skill.schema.ts b/plugins/plugin-skilled-openapi/src/tools/load-skill.schema.ts index 0fd243f13..cc8b2b060 100644 --- a/plugins/plugin-skilled-openapi/src/tools/load-skill.schema.ts +++ b/plugins/plugin-skilled-openapi/src/tools/load-skill.schema.ts @@ -38,6 +38,7 @@ const loadedSkillSchema = z.object({ inputJsonSchema: z.record(z.string(), z.unknown()), outputJsonSchema: z.record(z.string(), z.unknown()), requiredAuthorities: z.record(z.string(), z.unknown()).optional(), + asksForConfirmation: z.literal(true).optional(), }), ) .optional(), @@ -82,6 +83,8 @@ export type LoadedSkill = { inputJsonSchema: Record; outputJsonSchema: Record; requiredAuthorities?: Record; + /** Set when the plugin's `confirm` option makes the action ask the user before its request. */ + asksForConfirmation?: true; }>; }; diff --git a/plugins/plugin-skilled-openapi/src/tools/load-skill.tool.ts b/plugins/plugin-skilled-openapi/src/tools/load-skill.tool.ts index e22dd0ff9..f96dffc71 100644 --- a/plugins/plugin-skilled-openapi/src/tools/load-skill.tool.ts +++ b/plugins/plugin-skilled-openapi/src/tools/load-skill.tool.ts @@ -3,6 +3,7 @@ import { BundleStore } from '@frontmcp/adapters/skills'; import { InternalMcpError, PublicMcpError, ScopeEntry, Tool, ToolContext } from '@frontmcp/sdk'; +import { needsConfirmation } from '../executor/action-confirmation'; import { HiddenOpRegistry } from '../registry/hidden-op.registry'; import { AuthorityGuard } from '../security/authority-guard'; import { SkillVisibility } from '../security/skill-visibility'; @@ -17,6 +18,16 @@ import { type LoadSkillResult, } from './load-skill.schema'; +/** The action as `load_skill` shows it: marked when the `confirm` option makes it ask the user before its request. */ +function withConfirmationMarker( + action: Action, + config: SkilledOpenApiConfig, + hiddenOps: HiddenOpRegistry, +): Action & { asksForConfirmation?: true } { + const listings = hiddenOps.listingsOf(action.actionId); + return listings && needsConfirmation(config.confirm, listings) ? { ...action, asksForConfirmation: true } : action; +} + @Tool({ name: 'load_skill', description: loadSkillDescription, @@ -74,7 +85,7 @@ export default class LoadSkillTool extends ToolContext { description: skill.description, instructions: skill.instructions, ...(skill.bundleVersion !== undefined && { bundleVersion: skill.bundleVersion }), - ...(actions ? { actions } : {}), + ...(actions ? { actions: actions.map((action) => withConfirmationMarker(action, config, hiddenOps)) } : {}), }, isComplete: result.isComplete, ...(result.warning !== undefined && { warning: result.warning }),