From 5613c8a7433c26a16c4a5edc2191edd234deba41 Mon Sep 17 00:00:00 2001 From: Natalia Venditto Date: Fri, 19 Jun 2026 16:17:04 +0200 Subject: [PATCH 01/10] feat(tools): add submit_plan tool for user-reviewed multi-step execution - add submit_plan tool requiring user approval before agent executes any multi-step operation; execute returns {approved: true} so the agent continues after Run is clicked - add post-skill _hint in da_read_skill result nudging the agent to call submit_plan before proceeding, mirroring AO's post-skill hook behaviour - update system prompt: replace :::plan directive instructions with submit_plan call instructions; keep :::task-item for per-step progress Co-Authored-By: Claude Sonnet 4.6 --- src/prompt-builder.ts | 20 ++++++++++++++++++++ src/tools/tools.ts | 30 +++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/prompt-builder.ts b/src/prompt-builder.ts index 89dcf12..0c78f81 100644 --- a/src/prompt-builder.ts +++ b/src/prompt-builder.ts @@ -190,6 +190,26 @@ This is a critical issue. Use these blocks when they improve readability — for example, checklists for audits, alerts for important notes, toggle lists for detailed breakdowns. Do NOT overuse them for simple responses. +**Plan tool** — call \`submit_plan\` ONCE before starting any operation that involves 2 or more distinct steps or tool calls (multiple API calls, tasks involving different phases such as search → validate → create). The user will review the plan and click Run to approve execution. Do NOT call any other tools until the user approves. + +**Task item** — after the user approves and you begin execution, emit \`:::task-item\` before and after each step: +\`\`\` +:::task-item +{ "label": "Same label as in submit_plan", "status": "running" } +::: +\`\`\` +\`\`\` +:::task-item +{ "label": "Same label as in submit_plan", "status": "done" } +::: +\`\`\` + +Rules: +- Call \`submit_plan\` with ALL planned steps before executing any of them. +- Use the same \`label\` string in \`submit_plan\` tasks and \`:::task-item\` directives. +- Do NOT use these for single-step or trivial responses — only for operations with 2+ distinct steps. +- After the user approves (clicks Run), execute all steps in order, emitting \`:::task-item\` running → tool call → \`:::task-item\` done for each step. + ## EDS HTML Content Rules ALL content you create or update via tools MUST be valid Edge Delivery Services (EDS) semantic HTML. Follow these rules strictly: diff --git a/src/tools/tools.ts b/src/tools/tools.ts index 4444869..b7a3da4 100644 --- a/src/tools/tools.ts +++ b/src/tools/tools.ts @@ -461,7 +461,12 @@ export function createDATools( try { const content = await loadSkillBodyFromFolder(client, ctxOrg, ctxRepo, skillId); if (!content) return { error: `Skill "${skillId}" not found` }; - return { skillId, content }; + return { + skillId, + content, + _hint: + 'Skill loaded. Before executing any steps, call submit_plan with your planned tasks so the user can review and approve.', + }; } catch (e) { return { error: String(e) }; } @@ -557,6 +562,29 @@ export function createDATools( }, }); + // Plan submission — always needs approval so the user reviews before execution. + tools.submit_plan = tool({ + description: + 'Submit a multi-step execution plan for the user to review before any actions are taken. ' + + 'Call this ONCE at the start of any operation involving 2 or more distinct steps or tool calls. ' + + 'The user will see the plan and click Run to begin execution. ' + + 'Use the same task labels later in :::task-item directives to report progress.', + inputSchema: z.object({ + title: z.string().describe('Short plan title (≤ 8 words)'), + description: z.string().optional().describe('One-line summary of what you are about to do'), + tasks: z + .array( + z.object({ + id: z.string().describe('Unique step identifier, e.g. "1", "2"'), + label: z.string().describe('Human-readable step description'), + }), + ) + .describe('Ordered list of steps to execute'), + }), + needsApproval: async () => true, + execute: async () => ({ approved: true }), + }); + // Memory tools write to internal agent metadata paths — no user approval needed. tools.write_project_memory = tool({ description: From 1a1df01c973723e339d325e767320eb40563cd6d Mon Sep 17 00:00:00 2001 From: Natalia Venditto Date: Fri, 19 Jun 2026 17:29:17 +0200 Subject: [PATCH 02/10] refactor(tools): align plan tools with AO enter_plan_mode / exit_plan_mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace submit_plan with the two-tool bracket pattern used by AO's built-in planning tools so skills work consistently across both platforms. - rename submit_plan → exit_plan_mode (approval-gated, carries plan data) - add enter_plan_mode (no-op signal, no approval, mirrors AO semantics) - update system prompt: enter_plan_mode → reason → exit_plan_mode bracket - update da_read_skill _hint to reference the new tool names Co-Authored-By: Claude Sonnet 4.6 --- src/prompt-builder.ts | 14 +++++++++----- src/tools/tools.ts | 24 ++++++++++++++++++------ 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/prompt-builder.ts b/src/prompt-builder.ts index 0c78f81..990c249 100644 --- a/src/prompt-builder.ts +++ b/src/prompt-builder.ts @@ -190,23 +190,27 @@ This is a critical issue. Use these blocks when they improve readability — for example, checklists for audits, alerts for important notes, toggle lists for detailed breakdowns. Do NOT overuse them for simple responses. -**Plan tool** — call \`submit_plan\` ONCE before starting any operation that involves 2 or more distinct steps or tool calls (multiple API calls, tasks involving different phases such as search → validate → create). The user will review the plan and click Run to approve execution. Do NOT call any other tools until the user approves. +**Planning bracket** — for any operation involving 2 or more distinct steps or tool calls, use the planning bracket before executing anything: +1. Call \`enter_plan_mode\` — signals the start of planning (no side effects). +2. Reason about the steps needed. +3. Call \`exit_plan_mode\` with the full plan — the user reviews and clicks Run to approve. +4. After approval, execute all steps in order. **Task item** — after the user approves and you begin execution, emit \`:::task-item\` before and after each step: \`\`\` :::task-item -{ "label": "Same label as in submit_plan", "status": "running" } +{ "label": "Same label as in exit_plan_mode", "status": "running" } ::: \`\`\` \`\`\` :::task-item -{ "label": "Same label as in submit_plan", "status": "done" } +{ "label": "Same label as in exit_plan_mode", "status": "done" } ::: \`\`\` Rules: -- Call \`submit_plan\` with ALL planned steps before executing any of them. -- Use the same \`label\` string in \`submit_plan\` tasks and \`:::task-item\` directives. +- Always call \`enter_plan_mode\` first, then \`exit_plan_mode\` with ALL planned steps. +- Use the same \`label\` string in \`exit_plan_mode\` tasks and \`:::task-item\` directives. - Do NOT use these for single-step or trivial responses — only for operations with 2+ distinct steps. - After the user approves (clicks Run), execute all steps in order, emitting \`:::task-item\` running → tool call → \`:::task-item\` done for each step. diff --git a/src/tools/tools.ts b/src/tools/tools.ts index b7a3da4..e07a254 100644 --- a/src/tools/tools.ts +++ b/src/tools/tools.ts @@ -465,7 +465,7 @@ export function createDATools( skillId, content, _hint: - 'Skill loaded. Before executing any steps, call submit_plan with your planned tasks so the user can review and approve.', + 'Skill loaded. Before executing any steps, call enter_plan_mode then exit_plan_mode with your planned tasks so the user can review and approve.', }; } catch (e) { return { error: String(e) }; @@ -562,12 +562,24 @@ export function createDATools( }, }); - // Plan submission — always needs approval so the user reviews before execution. - tools.submit_plan = tool({ + // Planning bracket — mirrors AO's enter_plan_mode / exit_plan_mode built-in tools. + // enter_plan_mode: signals start of planning phase; no approval, no side effects. + tools.enter_plan_mode = tool({ description: - 'Submit a multi-step execution plan for the user to review before any actions are taken. ' + - 'Call this ONCE at the start of any operation involving 2 or more distinct steps or tool calls. ' + - 'The user will see the plan and click Run to begin execution. ' + + 'Signal the start of a planning phase. Call this before reasoning about what steps to take ' + + 'for any operation involving 2 or more distinct steps or tool calls. ' + + 'No action is taken — this is a signal only. Follow it by calling exit_plan_mode with the full plan.', + inputSchema: z.object({}), + needsApproval: async () => false, + execute: async () => ({ planning: true }), + }); + + // exit_plan_mode: submits the plan for user review; requires approval before execution proceeds. + tools.exit_plan_mode = tool({ + description: + 'Submit the completed plan for the user to review before any actions are taken. ' + + 'Call this after enter_plan_mode, once you have determined all the steps. ' + + 'The user will see the plan card and click Run to approve execution. ' + 'Use the same task labels later in :::task-item directives to report progress.', inputSchema: z.object({ title: z.string().describe('Short plan title (≤ 8 words)'), From 696533d90e041b8b2cb5063fb6fa15fc17521cfc Mon Sep 17 00:00:00 2001 From: Natalia Venditto Date: Mon, 22 Jun 2026 11:53:38 +0200 Subject: [PATCH 03/10] fix(prompt): harden task-item directive rules for plan execution - require character-for-character identical labels between exit_plan_mode tasks and :::task-item directives to prevent status merge mismatches - clarify that `done` must be emitted immediately after each tool result, before any commentary, and must never be skipped - add Content-Length: 0 header to HEAD /chat response to signal end of response faster Co-Authored-By: Claude Sonnet 4.6 --- src/prompt-builder.ts | 5 +++-- src/server.ts | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/prompt-builder.ts b/src/prompt-builder.ts index 990c249..01c4723 100644 --- a/src/prompt-builder.ts +++ b/src/prompt-builder.ts @@ -210,9 +210,10 @@ Use these blocks when they improve readability — for example, checklists for a Rules: - Always call \`enter_plan_mode\` first, then \`exit_plan_mode\` with ALL planned steps. -- Use the same \`label\` string in \`exit_plan_mode\` tasks and \`:::task-item\` directives. +- Use the **exact same** \`label\` string in \`exit_plan_mode\` tasks and \`:::task-item\` directives — character-for-character identical. - Do NOT use these for single-step or trivial responses — only for operations with 2+ distinct steps. -- After the user approves (clicks Run), execute all steps in order, emitting \`:::task-item\` running → tool call → \`:::task-item\` done for each step. +- After the user approves (clicks Run), for EVERY step: emit \`running\`, make the tool call, then emit \`done\` as the very first text after the tool result — before any commentary or prose. +- Never skip the \`done\` directive. Every step that started with \`running\` must end with \`done\`. ## EDS HTML Content Rules ALL content you create or update via tools MUST be valid Edge Delivery Services (EDS) semantic HTML. Follow these rules strictly: diff --git a/src/server.ts b/src/server.ts index b44f6ae..097c4ac 100644 --- a/src/server.ts +++ b/src/server.ts @@ -69,7 +69,10 @@ export default { const url = new URL(request.url); if (url.pathname === '/chat') { if (request.method === 'HEAD') { - return new Response(null, { status: 200, headers: CORS_HEADERS }); + return new Response(null, { + status: 200, + headers: { ...CORS_HEADERS, 'Content-Length': '0' }, + }); } if (request.method === 'POST') { return handleChat(request, env); From 6692f12e66589da4ab0d0f27e06b8c3c81852cd8 Mon Sep 17 00:00:00 2001 From: Natalia Venditto Date: Thu, 25 Jun 2026 10:13:56 +0200 Subject: [PATCH 04/10] feat(preflight): add run_preflight tool and agent instructions - add run_preflight tool with needsApproval: true and zod schema validating title, readiness (0-100 int), categories/checks, summary - instruct agent to auto-include preflight as final plan step when creating/updating HTML page documents (.html files only) - instruct agent not to re-enter plan mode or re-read the document when executing the preflight step - add tests for tool schema, needsApproval, execute, and prompt instructions Co-Authored-By: Claude Sonnet 4.6 --- src/prompt-builder.ts | 6 ++ src/tools/tools.ts | 38 +++++++++++ test/prompt-builder.test.ts | 31 +++++++++ test/tools/run-preflight.test.ts | 105 +++++++++++++++++++++++++++++++ 4 files changed, 180 insertions(+) create mode 100644 test/tools/run-preflight.test.ts diff --git a/src/prompt-builder.ts b/src/prompt-builder.ts index 01c4723..99d01e4 100644 --- a/src/prompt-builder.ts +++ b/src/prompt-builder.ts @@ -215,6 +215,12 @@ Rules: - After the user approves (clicks Run), for EVERY step: emit \`running\`, make the tool call, then emit \`done\` as the very first text after the tool result — before any commentary or prose. - Never skip the \`done\` directive. Every step that started with \`running\` must end with \`done\`. +**Preflight** — when a plan creates or updates one or more **HTML page documents** (\`.html\` files intended for publishing), always include a \`run_preflight\` step as the final step before any publish step. Do NOT add a preflight step for image uploads, config or metadata sheets, skills, fragments, or file operations (copy/move/delete). +When executing the preflight step: +- Call \`run_preflight\` directly — do NOT call \`enter_plan_mode\` or \`exit_plan_mode\` again. +- Evaluate the content using what is already in your context (from prior \`content_read\` or \`content_create\` results). Do NOT re-read the document. +- Score each category honestly based on the content you generated. + ## EDS HTML Content Rules ALL content you create or update via tools MUST be valid Edge Delivery Services (EDS) semantic HTML. Follow these rules strictly: diff --git a/src/tools/tools.ts b/src/tools/tools.ts index e07a254..1774f57 100644 --- a/src/tools/tools.ts +++ b/src/tools/tools.ts @@ -597,6 +597,44 @@ export function createDATools( execute: async () => ({ approved: true }), }); + // run_preflight: LLM-evaluated readiness check. Agent calls this after generating content, + // scoring it across categories (context, SEO, accessibility, etc.) before the user approves + // publication. needsApproval surfaces the structured result as a preflight card in the UI. + tools.run_preflight = tool({ + description: + 'Run a preflight readiness check on the generated content before publishing. ' + + 'Evaluate the content across the provided categories and score each check as passed or failed. ' + + 'Call this after content generation is complete and before any publish step. ' + + 'The user will see the preflight card and must approve before proceeding.', + inputSchema: z.object({ + title: z.string().describe('Document or page title being checked (≤ 10 words)'), + readiness: z + .number() + .int() + .min(0) + .max(100) + .describe('Overall readiness percentage (0–100)'), + categories: z + .array( + z.object({ + name: z.string().describe('Category name, e.g. "Context", "SEO", "Accessibility"'), + checks: z + .array( + z.object({ + label: z.string().describe('Short check description'), + passed: z.boolean().describe('Whether this check passed'), + }), + ) + .describe('Individual checks within this category'), + }), + ) + .describe('Ordered list of categories with their checks and results'), + summary: z.string().optional().describe('One-sentence overall assessment'), + }), + needsApproval: async () => true, + execute: async () => ({ approved: true }), + }); + // Memory tools write to internal agent metadata paths — no user approval needed. tools.write_project_memory = tool({ description: diff --git a/test/prompt-builder.test.ts b/test/prompt-builder.test.ts index 5116cb3..1a77277 100644 --- a/test/prompt-builder.test.ts +++ b/test/prompt-builder.test.ts @@ -251,3 +251,34 @@ describe('buildSystemPrompt with project memory', () => { expect(prompt).not.toContain('Project Memory'); }); }); + +describe('buildSystemPrompt preflight instructions', () => { + it('includes run_preflight in planning instructions', () => { + const prompt = buildSystemPrompt(); + expect(prompt).toContain('run_preflight'); + }); + + it('scopes preflight to HTML page documents only', () => { + const prompt = buildSystemPrompt(); + expect(prompt).toContain('.html'); + }); + + it('explicitly excludes image uploads from preflight', () => { + const prompt = buildSystemPrompt(); + expect(prompt).toContain('image uploads'); + }); + + it('instructs agent not to re-enter plan mode for preflight', () => { + const prompt = buildSystemPrompt(); + expect(prompt).toContain('enter_plan_mode'); + // The instruction must say NOT to call enter_plan_mode for preflight + const preflightSection = prompt.slice(prompt.indexOf('Preflight')); + expect(preflightSection).toContain('do NOT call'); + }); + + it('instructs agent to use content already in context, not re-read', () => { + const prompt = buildSystemPrompt(); + const preflightSection = prompt.slice(prompt.indexOf('Preflight')); + expect(preflightSection).toContain('Do NOT re-read'); + }); +}); diff --git a/test/tools/run-preflight.test.ts b/test/tools/run-preflight.test.ts new file mode 100644 index 0000000..9500269 --- /dev/null +++ b/test/tools/run-preflight.test.ts @@ -0,0 +1,105 @@ +import { describe, it, expect } from 'vitest'; +import { createDATools } from '../../src/tools/tools.js'; +import type { DAAdminClient } from '../../src/da-admin/client.js'; + +function mockClient(): DAAdminClient { + return { getSiteConfig: async () => ({}) } as unknown as DAAdminClient; +} + +function getPreflightTool() { + const tools = createDATools(mockClient(), { org: 'org', repo: 'site' }); + return tools.run_preflight; +} + +// ─── tool exists and is wired ────────────────────────────────────────────── + +describe('run_preflight tool definition', () => { + it('is registered in the tools registry', () => { + expect(getPreflightTool()).toBeDefined(); + }); + + it('requires user approval', async () => { + const tool = getPreflightTool(); + const needs = await tool.needsApproval?.({}); + expect(needs).toBe(true); + }); + + it('execute returns approved: true', async () => { + const tool = getPreflightTool(); + const result = await tool.execute({ + title: 'Test Page', + readiness: 90, + categories: [], + }); + expect(result).toEqual({ approved: true }); + }); +}); + +// ─── input schema validation ─────────────────────────────────────────────── + +describe('run_preflight input schema', () => { + it('accepts a valid full payload', () => { + const { inputSchema } = getPreflightTool(); + const result = inputSchema.safeParse({ + title: 'Cold Coffee Campaign', + readiness: 94, + categories: [ + { + name: 'Context', + checks: [ + { label: 'Tone of voice', passed: true }, + { label: 'Logo Usage', passed: false }, + ], + }, + ], + summary: '94% readiness.', + }); + expect(result.success).toBe(true); + }); + + it('accepts payload without optional summary', () => { + const { inputSchema } = getPreflightTool(); + const result = inputSchema.safeParse({ + title: 'No Summary Page', + readiness: 75, + categories: [], + }); + expect(result.success).toBe(true); + }); + + it('rejects readiness above 100', () => { + const { inputSchema } = getPreflightTool(); + expect(inputSchema.safeParse({ title: 'Over', readiness: 101, categories: [] }).success).toBe( + false, + ); + }); + + it('rejects readiness below 0', () => { + const { inputSchema } = getPreflightTool(); + expect(inputSchema.safeParse({ title: 'Under', readiness: -1, categories: [] }).success).toBe( + false, + ); + }); + + it('rejects non-integer readiness', () => { + const { inputSchema } = getPreflightTool(); + expect(inputSchema.safeParse({ title: 'Float', readiness: 94.5, categories: [] }).success).toBe( + false, + ); + }); + + it('rejects missing required title', () => { + const { inputSchema } = getPreflightTool(); + expect(inputSchema.safeParse({ readiness: 80, categories: [] }).success).toBe(false); + }); + + it('rejects a check missing the passed field', () => { + const { inputSchema } = getPreflightTool(); + const result = inputSchema.safeParse({ + title: 'Bad Check', + readiness: 50, + categories: [{ name: 'SEO', checks: [{ label: 'Title' }] }], + }); + expect(result.success).toBe(false); + }); +}); From 0f472477c7f38be573394340d55d2a7abf613d2b Mon Sep 17 00:00:00 2001 From: Natalia Venditto Date: Tue, 30 Jun 2026 16:29:30 +0200 Subject: [PATCH 05/10] feat(preflight): wire evaluate_page governance REST call in run_preflight - add url field to run_preflight input schema (Live Preview URL) - execute calls POST {GOVERNANCE_AGENT_URL}/api/v0/evaluate/page with IMS token - maps text_evaluation.evaluations (check_title, alignment YES/NO/NA) to card schema - falls back to approved:true when governance is unavailable or unconfigured - add imsToken + governanceUrl to DAToolsOptions; pass from tool-assembly - update prompt to make preflight conditional on a configured skill (not auto) - update tests: add url to schema fixtures, add rejects-missing-url case Co-Authored-By: Claude Sonnet 4.6 --- src/prompt-builder.ts | 5 +-- src/tool-assembly.ts | 2 + src/tools/tools.ts | 74 ++++++++++++++++++++++++++++---- test/prompt-builder.test.ts | 5 ++- test/tools/run-preflight.test.ts | 50 ++++++++++++++++----- 5 files changed, 112 insertions(+), 24 deletions(-) diff --git a/src/prompt-builder.ts b/src/prompt-builder.ts index 99d01e4..eb857a2 100644 --- a/src/prompt-builder.ts +++ b/src/prompt-builder.ts @@ -215,11 +215,10 @@ Rules: - After the user approves (clicks Run), for EVERY step: emit \`running\`, make the tool call, then emit \`done\` as the very first text after the tool result — before any commentary or prose. - Never skip the \`done\` directive. Every step that started with \`running\` must end with \`done\`. -**Preflight** — when a plan creates or updates one or more **HTML page documents** (\`.html\` files intended for publishing), always include a \`run_preflight\` step as the final step before any publish step. Do NOT add a preflight step for image uploads, config or metadata sheets, skills, fragments, or file operations (copy/move/delete). +**Preflight** — only include a \`run_preflight\` step in a plan if a preflight skill is explicitly configured for this project. Do NOT add preflight automatically. Do NOT add a preflight step for image uploads, config or metadata sheets, skills, fragments, or file operations (copy/move/delete). When executing the preflight step: - Call \`run_preflight\` directly — do NOT call \`enter_plan_mode\` or \`exit_plan_mode\` again. -- Evaluate the content using what is already in your context (from prior \`content_read\` or \`content_create\` results). Do NOT re-read the document. -- Score each category honestly based on the content you generated. +- Use the results provided by the preflight skill. Do NOT re-read the document or self-evaluate. ## EDS HTML Content Rules ALL content you create or update via tools MUST be valid Edge Delivery Services (EDS) semantic HTML. Follow these rules strictly: diff --git a/src/tool-assembly.ts b/src/tool-assembly.ts index 9ed9d94..10cfd7e 100644 --- a/src/tool-assembly.ts +++ b/src/tool-assembly.ts @@ -63,6 +63,8 @@ export async function assembleTools( getCollab: () => collabRef.promise, org: pageContext?.org, repo: pageContext?.site, + imsToken: imsToken ?? undefined, + governanceUrl: env.GOVERNANCE_AGENT_URL ?? undefined, resolveAttachmentByRef: (attachmentRef: string) => { const hit = attachmentMap.get(attachmentRef); if (!hit?.dataBase64) return null; diff --git a/src/tools/tools.ts b/src/tools/tools.ts index 1774f57..e5dcae4 100644 --- a/src/tools/tools.ts +++ b/src/tools/tools.ts @@ -49,6 +49,8 @@ export type DAToolsOptions = { mimeType: string; fileName: string; } | null; + imsToken?: string; + governanceUrl?: string; }; async function resolveCollab(options?: DAToolsOptions): Promise { @@ -597,27 +599,33 @@ export function createDATools( execute: async () => ({ approved: true }), }); - // run_preflight: LLM-evaluated readiness check. Agent calls this after generating content, - // scoring it across categories (context, SEO, accessibility, etc.) before the user approves - // publication. needsApproval surfaces the structured result as a preflight card in the UI. + // run_preflight: governance-evaluated readiness check. Calls the Brand Governance Agent's + // evaluate_page REST endpoint using the Live Preview URL, maps results to card schema, and + // surfaces the approval card. Falls back to LLM-provided payload if governance is unavailable. tools.run_preflight = tool({ description: 'Run a preflight readiness check on the generated content before publishing. ' + - 'Evaluate the content across the provided categories and score each check as passed or failed. ' + + 'Pass the Live Preview URL so the governance agent can evaluate it. ' + 'Call this after content generation is complete and before any publish step. ' + 'The user will see the preflight card and must approve before proceeding.', inputSchema: z.object({ title: z.string().describe('Document or page title being checked (≤ 10 words)'), + url: z + .string() + .url() + .describe('Live Preview URL of the document — always reflects current state'), readiness: z .number() .int() .min(0) .max(100) - .describe('Overall readiness percentage (0–100)'), + .describe( + 'Overall readiness percentage (0–100) — computed from governance results or estimated by the agent when governance is unavailable', + ), categories: z .array( z.object({ - name: z.string().describe('Category name, e.g. "Context", "SEO", "Accessibility"'), + name: z.string().describe('Category name'), checks: z .array( z.object({ @@ -628,11 +636,61 @@ export function createDATools( .describe('Individual checks within this category'), }), ) - .describe('Ordered list of categories with their checks and results'), + .describe('Populated from governance results when available; otherwise agent-estimated'), summary: z.string().optional().describe('One-sentence overall assessment'), }), needsApproval: async () => true, - execute: async () => ({ approved: true }), + execute: async ({ url, title, summary }) => { + const { governanceUrl, imsToken } = opts ?? {}; + if (!governanceUrl || !imsToken) return { approved: true }; + + try { + const resp = await fetch(`${governanceUrl}/api/v0/evaluate/page`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${imsToken}`, + }, + body: JSON.stringify({ url }), + }); + + if (!resp.ok) return { approved: true }; + + const data = (await resp.json()) as { + success: boolean; + text_evaluation?: { + overall_aligned: boolean; + successful_checks: number; + failed_checks: number; + not_applicable_checks: number; + evaluations?: Array<{ check_title: string; alignment: string; reasoning?: string }>; + }; + }; + + if (!data.success || !data.text_evaluation?.evaluations?.length) { + return { approved: true }; + } + + const evals = data.text_evaluation.evaluations; + const total = evals.length; + const passed = evals.filter((e) => e.alignment === 'YES' || e.alignment === 'NA').length; + const readiness = total > 0 ? Math.round((passed / total) * 100) : 0; + + const categories = [ + { + name: 'Brand Governance', + checks: evals.map((e) => ({ + label: e.check_title, + passed: e.alignment === 'YES' || e.alignment === 'NA', + })), + }, + ]; + + return { approved: true, title, url, readiness, categories, summary }; + } catch { + return { approved: true }; + } + }, }); // Memory tools write to internal agent metadata paths — no user approval needed. diff --git a/test/prompt-builder.test.ts b/test/prompt-builder.test.ts index 1a77277..f7d55d1 100644 --- a/test/prompt-builder.test.ts +++ b/test/prompt-builder.test.ts @@ -258,9 +258,10 @@ describe('buildSystemPrompt preflight instructions', () => { expect(prompt).toContain('run_preflight'); }); - it('scopes preflight to HTML page documents only', () => { + it('makes preflight conditional on a configured skill', () => { const prompt = buildSystemPrompt(); - expect(prompt).toContain('.html'); + const preflightSection = prompt.slice(prompt.indexOf('Preflight')); + expect(preflightSection).toContain('preflight skill'); }); it('explicitly excludes image uploads from preflight', () => { diff --git a/test/tools/run-preflight.test.ts b/test/tools/run-preflight.test.ts index 9500269..1468ce5 100644 --- a/test/tools/run-preflight.test.ts +++ b/test/tools/run-preflight.test.ts @@ -24,10 +24,11 @@ describe('run_preflight tool definition', () => { expect(needs).toBe(true); }); - it('execute returns approved: true', async () => { + it('execute returns approved: true (no governance config)', async () => { const tool = getPreflightTool(); const result = await tool.execute({ title: 'Test Page', + url: 'https://main--site--org.preview.da.live/index', readiness: 90, categories: [], }); @@ -42,6 +43,7 @@ describe('run_preflight input schema', () => { const { inputSchema } = getPreflightTool(); const result = inputSchema.safeParse({ title: 'Cold Coffee Campaign', + url: 'https://main--site--org.preview.da.live/index', readiness: 94, categories: [ { @@ -61,6 +63,7 @@ describe('run_preflight input schema', () => { const { inputSchema } = getPreflightTool(); const result = inputSchema.safeParse({ title: 'No Summary Page', + url: 'https://main--site--org.preview.da.live/index', readiness: 75, categories: [], }); @@ -69,34 +72,59 @@ describe('run_preflight input schema', () => { it('rejects readiness above 100', () => { const { inputSchema } = getPreflightTool(); - expect(inputSchema.safeParse({ title: 'Over', readiness: 101, categories: [] }).success).toBe( - false, - ); + expect( + inputSchema.safeParse({ + title: 'Over', + url: 'https://example.com', + readiness: 101, + categories: [], + }).success, + ).toBe(false); }); it('rejects readiness below 0', () => { const { inputSchema } = getPreflightTool(); - expect(inputSchema.safeParse({ title: 'Under', readiness: -1, categories: [] }).success).toBe( - false, - ); + expect( + inputSchema.safeParse({ + title: 'Under', + url: 'https://example.com', + readiness: -1, + categories: [], + }).success, + ).toBe(false); }); it('rejects non-integer readiness', () => { const { inputSchema } = getPreflightTool(); - expect(inputSchema.safeParse({ title: 'Float', readiness: 94.5, categories: [] }).success).toBe( - false, - ); + expect( + inputSchema.safeParse({ + title: 'Float', + url: 'https://example.com', + readiness: 94.5, + categories: [], + }).success, + ).toBe(false); }); it('rejects missing required title', () => { const { inputSchema } = getPreflightTool(); - expect(inputSchema.safeParse({ readiness: 80, categories: [] }).success).toBe(false); + expect( + inputSchema.safeParse({ url: 'https://example.com', readiness: 80, categories: [] }).success, + ).toBe(false); + }); + + it('rejects missing required url', () => { + const { inputSchema } = getPreflightTool(); + expect(inputSchema.safeParse({ title: 'No URL', readiness: 80, categories: [] }).success).toBe( + false, + ); }); it('rejects a check missing the passed field', () => { const { inputSchema } = getPreflightTool(); const result = inputSchema.safeParse({ title: 'Bad Check', + url: 'https://example.com', readiness: 50, categories: [{ name: 'SEO', checks: [{ label: 'Title' }] }], }); From 41d7d95383796aa12c88b15c4d3c46068e95507b Mon Sep 17 00:00:00 2001 From: Natalia Venditto Date: Tue, 30 Jun 2026 16:37:41 +0200 Subject: [PATCH 06/10] refactor(preflight): delegate evaluation to governance MCP, keep run_preflight as dumb gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - remove REST call from execute — agent calls mcp__governance-agent__evaluate_page directly - remove imsToken/governanceUrl from DAToolsOptions and tool-assembly (no longer needed) - update prompt: instruct agent to call evaluate_page, map results, then call run_preflight - update tests to match new prompt wording Co-Authored-By: Claude Sonnet 4.6 --- src/prompt-builder.ts | 6 ++- src/tool-assembly.ts | 2 - src/tools/tools.ts | 77 +++++-------------------------------- test/prompt-builder.test.ts | 8 ++-- 4 files changed, 17 insertions(+), 76 deletions(-) diff --git a/src/prompt-builder.ts b/src/prompt-builder.ts index eb857a2..7fb7568 100644 --- a/src/prompt-builder.ts +++ b/src/prompt-builder.ts @@ -217,8 +217,10 @@ Rules: **Preflight** — only include a \`run_preflight\` step in a plan if a preflight skill is explicitly configured for this project. Do NOT add preflight automatically. Do NOT add a preflight step for image uploads, config or metadata sheets, skills, fragments, or file operations (copy/move/delete). When executing the preflight step: -- Call \`run_preflight\` directly — do NOT call \`enter_plan_mode\` or \`exit_plan_mode\` again. -- Use the results provided by the preflight skill. Do NOT re-read the document or self-evaluate. +- Do NOT call \`enter_plan_mode\` or \`exit_plan_mode\` again. +- Call \`mcp__governance-agent__evaluate_page\` with the Live Preview URL from the current page context. +- Map the returned evaluations into \`categories\` and compute \`readiness\` as the percentage of YES/NA checks. +- Then call \`run_preflight\` with the mapped payload to surface the card and wait for user approval. ## EDS HTML Content Rules ALL content you create or update via tools MUST be valid Edge Delivery Services (EDS) semantic HTML. Follow these rules strictly: diff --git a/src/tool-assembly.ts b/src/tool-assembly.ts index 10cfd7e..9ed9d94 100644 --- a/src/tool-assembly.ts +++ b/src/tool-assembly.ts @@ -63,8 +63,6 @@ export async function assembleTools( getCollab: () => collabRef.promise, org: pageContext?.org, repo: pageContext?.site, - imsToken: imsToken ?? undefined, - governanceUrl: env.GOVERNANCE_AGENT_URL ?? undefined, resolveAttachmentByRef: (attachmentRef: string) => { const hit = attachmentMap.get(attachmentRef); if (!hit?.dataBase64) return null; diff --git a/src/tools/tools.ts b/src/tools/tools.ts index e5dcae4..15bf2cc 100644 --- a/src/tools/tools.ts +++ b/src/tools/tools.ts @@ -49,8 +49,6 @@ export type DAToolsOptions = { mimeType: string; fileName: string; } | null; - imsToken?: string; - governanceUrl?: string; }; async function resolveCollab(options?: DAToolsOptions): Promise { @@ -599,29 +597,24 @@ export function createDATools( execute: async () => ({ approved: true }), }); - // run_preflight: governance-evaluated readiness check. Calls the Brand Governance Agent's - // evaluate_page REST endpoint using the Live Preview URL, maps results to card schema, and - // surfaces the approval card. Falls back to LLM-provided payload if governance is unavailable. + // run_preflight: dumb approval gate. The agent calls mcp__governance-agent__evaluate_page + // first, maps the results into the card schema, then calls this tool to surface the card + // and wait for user approval. No evaluation logic lives here. tools.run_preflight = tool({ description: - 'Run a preflight readiness check on the generated content before publishing. ' + - 'Pass the Live Preview URL so the governance agent can evaluate it. ' + - 'Call this after content generation is complete and before any publish step. ' + + 'Surface a preflight readiness card and wait for user approval before publishing. ' + + 'Call mcp__governance-agent__evaluate_page with the Live Preview URL first, map the results ' + + 'into categories and checks, then call this tool with the structured payload. ' + 'The user will see the preflight card and must approve before proceeding.', inputSchema: z.object({ title: z.string().describe('Document or page title being checked (≤ 10 words)'), - url: z - .string() - .url() - .describe('Live Preview URL of the document — always reflects current state'), + url: z.string().url().describe('Live Preview URL of the document'), readiness: z .number() .int() .min(0) .max(100) - .describe( - 'Overall readiness percentage (0–100) — computed from governance results or estimated by the agent when governance is unavailable', - ), + .describe('Overall readiness percentage (0–100) computed from governance results'), categories: z .array( z.object({ @@ -636,61 +629,11 @@ export function createDATools( .describe('Individual checks within this category'), }), ) - .describe('Populated from governance results when available; otherwise agent-estimated'), + .describe('Mapped from governance agent evaluate_page results'), summary: z.string().optional().describe('One-sentence overall assessment'), }), needsApproval: async () => true, - execute: async ({ url, title, summary }) => { - const { governanceUrl, imsToken } = opts ?? {}; - if (!governanceUrl || !imsToken) return { approved: true }; - - try { - const resp = await fetch(`${governanceUrl}/api/v0/evaluate/page`, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - Authorization: `Bearer ${imsToken}`, - }, - body: JSON.stringify({ url }), - }); - - if (!resp.ok) return { approved: true }; - - const data = (await resp.json()) as { - success: boolean; - text_evaluation?: { - overall_aligned: boolean; - successful_checks: number; - failed_checks: number; - not_applicable_checks: number; - evaluations?: Array<{ check_title: string; alignment: string; reasoning?: string }>; - }; - }; - - if (!data.success || !data.text_evaluation?.evaluations?.length) { - return { approved: true }; - } - - const evals = data.text_evaluation.evaluations; - const total = evals.length; - const passed = evals.filter((e) => e.alignment === 'YES' || e.alignment === 'NA').length; - const readiness = total > 0 ? Math.round((passed / total) * 100) : 0; - - const categories = [ - { - name: 'Brand Governance', - checks: evals.map((e) => ({ - label: e.check_title, - passed: e.alignment === 'YES' || e.alignment === 'NA', - })), - }, - ]; - - return { approved: true, title, url, readiness, categories, summary }; - } catch { - return { approved: true }; - } - }, + execute: async () => ({ approved: true }), }); // Memory tools write to internal agent metadata paths — no user approval needed. diff --git a/test/prompt-builder.test.ts b/test/prompt-builder.test.ts index f7d55d1..e67af79 100644 --- a/test/prompt-builder.test.ts +++ b/test/prompt-builder.test.ts @@ -271,15 +271,13 @@ describe('buildSystemPrompt preflight instructions', () => { it('instructs agent not to re-enter plan mode for preflight', () => { const prompt = buildSystemPrompt(); - expect(prompt).toContain('enter_plan_mode'); - // The instruction must say NOT to call enter_plan_mode for preflight const preflightSection = prompt.slice(prompt.indexOf('Preflight')); - expect(preflightSection).toContain('do NOT call'); + expect(preflightSection).toContain('Do NOT call'); }); - it('instructs agent to use content already in context, not re-read', () => { + it('instructs agent to call evaluate_page before run_preflight', () => { const prompt = buildSystemPrompt(); const preflightSection = prompt.slice(prompt.indexOf('Preflight')); - expect(preflightSection).toContain('Do NOT re-read'); + expect(preflightSection).toContain('mcp__governance-agent__evaluate_page'); }); }); From a0a6ad95590de03af05143941d9273d00bd17dde Mon Sep 17 00:00:00 2001 From: Andrei Tuicu Date: Tue, 28 Jul 2026 10:08:53 +0200 Subject: [PATCH 07/10] fix: Tools requiring approval were not being returned to the browser --- src/server.ts | 35 +++++++++++++++++++-- src/tool-approval.ts | 24 ++++++++++++++- test/tool-approval.test.ts | 63 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 3 deletions(-) diff --git a/src/server.ts b/src/server.ts index a6b337d..4342a28 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,11 +1,18 @@ import { createAmazonBedrock } from '@ai-sdk/amazon-bedrock'; -import { streamText, stepCountIs, type ModelMessage } from 'ai'; +import { + streamText, + stepCountIs, + createUIMessageStream, + createUIMessageStreamResponse, + type ModelMessage, +} from 'ai'; import { initTelemetry, flushTelemetry } from './telemetry.js'; import { MCPClient } from './mcp/client.js'; import { buildApprovalContinuationResponse, getNewlyResolvedToolOutputs, hasPendingApprovals, + unwrapToolOutput, } from './tool-approval.js'; import { detectSessionUserPattern, @@ -232,6 +239,11 @@ async function handleChat(request: Request, env: Env): Promise { return buildApprovalContinuationResponse(toolOutputs, CORS_HEADERS); } + // Tools approved this request are executed in resolveApprovals and injected as tool-results, + // so streamText treats them as prior context and never re-emits their output. Surface those + // outputs to the client (for tool cards) by merging them into the streamText UI stream below. + const newlyResolvedOutputs = getNewlyResolvedToolOutputs(messages, processedMessages); + const withOrphanResults = ensureOrphanedToolResults(processedMessages); const strippedForModel = stripClientOnlyToolInputs(withOrphanResults); const sessionPattern = trailingAssistantAlreadySuggestedSkill(strippedForModel) @@ -344,7 +356,26 @@ async function handleChat(request: Request, env: Env): Promise { }, }); - const streamResponse = result.toUIMessageStreamResponse(); + // When tools were approved this request, prepend their outputs as tool-output-available + // events (unwrapped to the raw MCP shape the client renders) then merge the model stream. + // Otherwise emit the streamText response directly so the common path is unchanged. + const streamResponse = newlyResolvedOutputs.length + ? createUIMessageStreamResponse({ + stream: createUIMessageStream({ + execute: ({ writer }) => { + for (const { toolCallId, output } of newlyResolvedOutputs) { + writer.write({ + type: 'tool-output-available', + toolCallId, + output: unwrapToolOutput(output), + }); + } + writer.merge(result.toUIMessageStream()); + }, + onError: (error) => formatErrorForLog(error), + }), + }) + : result.toUIMessageStreamResponse(); const headers = new Headers(streamResponse.headers); for (const [key, value] of Object.entries(CORS_HEADERS)) { diff --git a/src/tool-approval.ts b/src/tool-approval.ts index 827b20c..1279dbf 100644 --- a/src/tool-approval.ts +++ b/src/tool-approval.ts @@ -60,6 +60,28 @@ export function getNewlyResolvedToolOutputs( return outputs; } +const TOOL_OUTPUT_ENVELOPE_TYPES = new Set(['text', 'json', 'error-text']); + +/** + * resolveApprovals stores tool outputs as `{ type: 'json' | 'text' | 'error-text', value }` + * envelopes so the model sees a well-formed tool-result. The client, however, renders cards + * from the raw MCP output (a JSON string or object), matching what the AI SDK streams for + * inline tool executions. Unwrap the envelope back to that raw value before emitting it. + * Checking `type` against the known envelope tags (not just its presence) avoids mistaking + * genuine tool output that happens to have `type`/`value` keys for our own envelope. + */ +export function unwrapToolOutput(output: unknown): unknown { + if ( + output && + typeof output === 'object' && + 'value' in output && + TOOL_OUTPUT_ENVELOPE_TYPES.has((output as Record).type as string) + ) { + return (output as { value: unknown }).value; + } + return output; +} + export function buildApprovalContinuationResponse( toolOutputs: Array<{ toolCallId: string; output: unknown }>, corsHeaders: Record, @@ -70,7 +92,7 @@ export function buildApprovalContinuationResponse( writer.write({ type: 'tool-output-available', toolCallId, - output, + output: unwrapToolOutput(output), }); } writer.write({ type: 'finish', finishReason: 'stop' }); diff --git a/test/tool-approval.test.ts b/test/tool-approval.test.ts index 2971038..8139011 100644 --- a/test/tool-approval.test.ts +++ b/test/tool-approval.test.ts @@ -1,10 +1,23 @@ import { describe, expect, it } from 'vitest'; import { + buildApprovalContinuationResponse, getNewlyResolvedToolOutputs, hasPendingApprovals, resolvedToolCallIds, + unwrapToolOutput, } from '../src/tool-approval.js'; +/** Collect the parsed `data:` payloads from a UI-message-stream SSE Response. */ +async function readSseEvents(response: Response): Promise { + const text = await response.text(); + return text + .split('\n') + .filter((line) => line.startsWith('data: ')) + .map((line) => line.slice(6).trim()) + .filter((raw) => raw && raw !== '[DONE]') + .map((raw) => JSON.parse(raw)); +} + describe('tool approval helpers', () => { const assistantWithTwoApprovals = { role: 'assistant', @@ -120,4 +133,54 @@ describe('tool approval helpers', () => { { toolCallId: 'call-b', output: { type: 'text', value: 'second' } }, ]); }); + + describe('unwrapToolOutput', () => { + it('unwraps a text envelope to its raw string value', () => { + expect(unwrapToolOutput({ type: 'text', value: '{"a":1}' })).toBe('{"a":1}'); + }); + + it('unwraps a json envelope to its raw object value', () => { + expect(unwrapToolOutput({ type: 'json', value: { a: 1 } })).toEqual({ a: 1 }); + }); + + it('unwraps an error-text envelope to its raw value', () => { + expect(unwrapToolOutput({ type: 'error-text', value: 'boom' })).toBe('boom'); + }); + + it('passes through values that are not {type,value} envelopes', () => { + expect(unwrapToolOutput('plain string')).toBe('plain string'); + expect(unwrapToolOutput({ text_evaluation: {} })).toEqual({ text_evaluation: {} }); + expect(unwrapToolOutput(undefined)).toBe(undefined); + expect(unwrapToolOutput(null)).toBe(null); + }); + + it('passes through genuine tool output that coincidentally has type/value keys', () => { + const geoPoint = { type: 'Point', value: [1, 2] }; + expect(unwrapToolOutput(geoPoint)).toEqual(geoPoint); + }); + }); + + describe('buildApprovalContinuationResponse', () => { + it('emits tool-output-available with the unwrapped raw output', async () => { + const evaluation = { brand_name: 'Frescopa', text_evaluation: { successful_checks: 2 } }; + const response = buildApprovalContinuationResponse( + [ + { + toolCallId: 'call-a', + output: { type: 'text', value: JSON.stringify(evaluation) }, + }, + ], + { 'access-control-allow-origin': '*' }, + ); + + const events = await readSseEvents(response); + const outputEvent = events.find((e) => e.type === 'tool-output-available'); + + expect(outputEvent).toBeDefined(); + expect(outputEvent.toolCallId).toBe('call-a'); + // Raw MCP shape (JSON string), not the { type, value } envelope. + expect(outputEvent.output).toBe(JSON.stringify(evaluation)); + expect(events.some((e) => e.type === 'finish')).toBe(true); + }); + }); }); From 23ad3fe27a5195c9de6b16d5d3ac7c092b541235 Mon Sep 17 00:00:00 2001 From: Andrei Tuicu Date: Tue, 28 Jul 2026 22:09:23 +0200 Subject: [PATCH 08/10] fix: Remove preflight tool -> LLM should not decide whether show continuation pop-up. It should be code driven --- src/prompt-builder.ts | 7 -- src/tools/tools.ts | 39 --------- test/prompt-builder.test.ts | 30 ------- test/tools/run-preflight.test.ts | 133 ------------------------------- 4 files changed, 209 deletions(-) delete mode 100644 test/tools/run-preflight.test.ts diff --git a/src/prompt-builder.ts b/src/prompt-builder.ts index a5fc8a1..d407bfd 100644 --- a/src/prompt-builder.ts +++ b/src/prompt-builder.ts @@ -218,13 +218,6 @@ Rules: - After the user approves (clicks Run), for EVERY step: emit \`running\`, make the tool call, then emit \`done\` as the very first text after the tool result — before any commentary or prose. - Never skip the \`done\` directive. Every step that started with \`running\` must end with \`done\`. -**Preflight** — only include a \`run_preflight\` step in a plan if a preflight skill is explicitly configured for this project. Do NOT add preflight automatically. Do NOT add a preflight step for image uploads, config or metadata sheets, skills, fragments, or file operations (copy/move/delete). -When executing the preflight step: -- Do NOT call \`enter_plan_mode\` or \`exit_plan_mode\` again. -- Call \`mcp__governance-agent__evaluate_page\` with the Live Preview URL from the current page context. -- Map the returned evaluations into \`categories\` and compute \`readiness\` as the percentage of YES/NA checks. -- Then call \`run_preflight\` with the mapped payload to surface the card and wait for user approval. - ## EDS HTML Content Rules ALL content you create or update via tools MUST be valid Edge Delivery Services (EDS) semantic HTML. Follow these rules strictly: diff --git a/src/tools/tools.ts b/src/tools/tools.ts index 15bf2cc..e07a254 100644 --- a/src/tools/tools.ts +++ b/src/tools/tools.ts @@ -597,45 +597,6 @@ export function createDATools( execute: async () => ({ approved: true }), }); - // run_preflight: dumb approval gate. The agent calls mcp__governance-agent__evaluate_page - // first, maps the results into the card schema, then calls this tool to surface the card - // and wait for user approval. No evaluation logic lives here. - tools.run_preflight = tool({ - description: - 'Surface a preflight readiness card and wait for user approval before publishing. ' + - 'Call mcp__governance-agent__evaluate_page with the Live Preview URL first, map the results ' + - 'into categories and checks, then call this tool with the structured payload. ' + - 'The user will see the preflight card and must approve before proceeding.', - inputSchema: z.object({ - title: z.string().describe('Document or page title being checked (≤ 10 words)'), - url: z.string().url().describe('Live Preview URL of the document'), - readiness: z - .number() - .int() - .min(0) - .max(100) - .describe('Overall readiness percentage (0–100) computed from governance results'), - categories: z - .array( - z.object({ - name: z.string().describe('Category name'), - checks: z - .array( - z.object({ - label: z.string().describe('Short check description'), - passed: z.boolean().describe('Whether this check passed'), - }), - ) - .describe('Individual checks within this category'), - }), - ) - .describe('Mapped from governance agent evaluate_page results'), - summary: z.string().optional().describe('One-sentence overall assessment'), - }), - needsApproval: async () => true, - execute: async () => ({ approved: true }), - }); - // Memory tools write to internal agent metadata paths — no user approval needed. tools.write_project_memory = tool({ description: diff --git a/test/prompt-builder.test.ts b/test/prompt-builder.test.ts index 99ffc0d..3e7e7d5 100644 --- a/test/prompt-builder.test.ts +++ b/test/prompt-builder.test.ts @@ -270,33 +270,3 @@ describe('buildSystemPrompt with project memory', () => { expect(prompt).not.toContain('Project Memory'); }); }); - -describe('buildSystemPrompt preflight instructions', () => { - it('includes run_preflight in planning instructions', () => { - const prompt = buildSystemPrompt(); - expect(prompt).toContain('run_preflight'); - }); - - it('makes preflight conditional on a configured skill', () => { - const prompt = buildSystemPrompt(); - const preflightSection = prompt.slice(prompt.indexOf('Preflight')); - expect(preflightSection).toContain('preflight skill'); - }); - - it('explicitly excludes image uploads from preflight', () => { - const prompt = buildSystemPrompt(); - expect(prompt).toContain('image uploads'); - }); - - it('instructs agent not to re-enter plan mode for preflight', () => { - const prompt = buildSystemPrompt(); - const preflightSection = prompt.slice(prompt.indexOf('Preflight')); - expect(preflightSection).toContain('Do NOT call'); - }); - - it('instructs agent to call evaluate_page before run_preflight', () => { - const prompt = buildSystemPrompt(); - const preflightSection = prompt.slice(prompt.indexOf('Preflight')); - expect(preflightSection).toContain('mcp__governance-agent__evaluate_page'); - }); -}); diff --git a/test/tools/run-preflight.test.ts b/test/tools/run-preflight.test.ts deleted file mode 100644 index 1468ce5..0000000 --- a/test/tools/run-preflight.test.ts +++ /dev/null @@ -1,133 +0,0 @@ -import { describe, it, expect } from 'vitest'; -import { createDATools } from '../../src/tools/tools.js'; -import type { DAAdminClient } from '../../src/da-admin/client.js'; - -function mockClient(): DAAdminClient { - return { getSiteConfig: async () => ({}) } as unknown as DAAdminClient; -} - -function getPreflightTool() { - const tools = createDATools(mockClient(), { org: 'org', repo: 'site' }); - return tools.run_preflight; -} - -// ─── tool exists and is wired ────────────────────────────────────────────── - -describe('run_preflight tool definition', () => { - it('is registered in the tools registry', () => { - expect(getPreflightTool()).toBeDefined(); - }); - - it('requires user approval', async () => { - const tool = getPreflightTool(); - const needs = await tool.needsApproval?.({}); - expect(needs).toBe(true); - }); - - it('execute returns approved: true (no governance config)', async () => { - const tool = getPreflightTool(); - const result = await tool.execute({ - title: 'Test Page', - url: 'https://main--site--org.preview.da.live/index', - readiness: 90, - categories: [], - }); - expect(result).toEqual({ approved: true }); - }); -}); - -// ─── input schema validation ─────────────────────────────────────────────── - -describe('run_preflight input schema', () => { - it('accepts a valid full payload', () => { - const { inputSchema } = getPreflightTool(); - const result = inputSchema.safeParse({ - title: 'Cold Coffee Campaign', - url: 'https://main--site--org.preview.da.live/index', - readiness: 94, - categories: [ - { - name: 'Context', - checks: [ - { label: 'Tone of voice', passed: true }, - { label: 'Logo Usage', passed: false }, - ], - }, - ], - summary: '94% readiness.', - }); - expect(result.success).toBe(true); - }); - - it('accepts payload without optional summary', () => { - const { inputSchema } = getPreflightTool(); - const result = inputSchema.safeParse({ - title: 'No Summary Page', - url: 'https://main--site--org.preview.da.live/index', - readiness: 75, - categories: [], - }); - expect(result.success).toBe(true); - }); - - it('rejects readiness above 100', () => { - const { inputSchema } = getPreflightTool(); - expect( - inputSchema.safeParse({ - title: 'Over', - url: 'https://example.com', - readiness: 101, - categories: [], - }).success, - ).toBe(false); - }); - - it('rejects readiness below 0', () => { - const { inputSchema } = getPreflightTool(); - expect( - inputSchema.safeParse({ - title: 'Under', - url: 'https://example.com', - readiness: -1, - categories: [], - }).success, - ).toBe(false); - }); - - it('rejects non-integer readiness', () => { - const { inputSchema } = getPreflightTool(); - expect( - inputSchema.safeParse({ - title: 'Float', - url: 'https://example.com', - readiness: 94.5, - categories: [], - }).success, - ).toBe(false); - }); - - it('rejects missing required title', () => { - const { inputSchema } = getPreflightTool(); - expect( - inputSchema.safeParse({ url: 'https://example.com', readiness: 80, categories: [] }).success, - ).toBe(false); - }); - - it('rejects missing required url', () => { - const { inputSchema } = getPreflightTool(); - expect(inputSchema.safeParse({ title: 'No URL', readiness: 80, categories: [] }).success).toBe( - false, - ); - }); - - it('rejects a check missing the passed field', () => { - const { inputSchema } = getPreflightTool(); - const result = inputSchema.safeParse({ - title: 'Bad Check', - url: 'https://example.com', - readiness: 50, - categories: [{ name: 'SEO', checks: [{ label: 'Title' }] }], - }); - expect(result.success).toBe(false); - }); -}); From ca4ff9a49b2c0b1b8bbcefbcbb33bf0fd2a91db1 Mon Sep 17 00:00:00 2001 From: Andrei Tuicu Date: Wed, 29 Jul 2026 16:39:48 +0200 Subject: [PATCH 09/10] feat: Tools that require approval to continue --- src/mcp/built-in-servers.ts | 4 ++ src/mcp/tool-adapter.ts | 32 ++++++++++++- src/mcp/types.ts | 5 ++ src/server.ts | 80 +++++++++++++++++++++++-------- src/tool-approval.ts | 37 ++++++++++++++ src/tool-assembly.ts | 5 ++ test/mcp/built-in-servers.test.ts | 5 ++ test/mcp/tool-adapter.test.ts | 63 +++++++++++++++++++++++- test/tool-approval.test.ts | 63 ++++++++++++++++++++++++ 9 files changed, 271 insertions(+), 23 deletions(-) diff --git a/src/mcp/built-in-servers.ts b/src/mcp/built-in-servers.ts index 91f99da..edef70e 100644 --- a/src/mcp/built-in-servers.ts +++ b/src/mcp/built-in-servers.ts @@ -24,6 +24,10 @@ export function getBuiltInMcpServers(env: Env): Record (ch === '*' ? '.*' : `\\${ch}`)); + return new RegExp(`^${escaped}$`).test(name); +} + +function matchesAnyGlob(name: string, patterns: string[] | undefined): boolean { + return !!patterns && patterns.some((p) => matchesGlob(name, p)); +} + /** * Convert a single JSON Schema property definition to a Zod type. * Handles the common scalar types returned by MCP servers. @@ -89,6 +103,7 @@ export function mcpToolToAITool( serverId: string, mcpTool: MCPToolDefinition, mcpClient: MCPClient, + continuationApprovalPatterns?: string[], ) { const toolName = `mcp__${serverId}__${mcpTool.name}`; const description = mcpTool.description ?? `MCP tool ${mcpTool.name} from server ${serverId}`; @@ -98,18 +113,29 @@ export function mcpToolToAITool( mcpTool.inputSchema as Record | undefined, ); + // Continuation-gated tools run WITHOUT pre-execution approval and instead pause + // the agentic loop after they finish (see server.ts). Skipping the pre-exec gate + // here avoids a double prompt (approve-to-run AND approve-to-continue). + const isContinuationGated = matchesAnyGlob(mcpTool.name, continuationApprovalPatterns); + return { name: toolName, tool: tool({ description, inputSchema, + // Flag read by the stopWhen predicate in server.ts. `daAgent` is our own + // provider namespace and is ignored by the Bedrock provider. + ...(isContinuationGated + ? { providerOptions: { daAgent: { continuationApproval: true } } } + : {}), // Fail-closed gating for untrusted external MCP servers. Per the MCP spec // annotation defaults (readOnlyHint=false, destructiveHint=true) and the // fact that annotations are optional, we gate unless the tool tells us it // is safe: skip approval only when it is read-only OR explicitly // non-destructive. Everything else — including unannotated tools — - // requires approval. + // requires approval. Continuation-gated tools opt out (post-exec gate instead). needsApproval: async () => { + if (isContinuationGated) return false; const { readOnlyHint, destructiveHint } = mcpTool.annotations ?? {}; return readOnlyHint !== true && destructiveHint !== false; }, @@ -146,6 +172,8 @@ export async function connectAndRegisterMCPTools( mcpConfig: { mcpServers: Record; toolAllowPatterns: string[]; + /** Per-server glob patterns for post-execution continuation approval. */ + continuationApprovalPatterns?: Record; }, options?: { headers?: Record; @@ -194,6 +222,7 @@ export async function connectAndRegisterMCPTools( const discoveredTools = await client.listTools(); clients.push(client); + const serverContinuationPatterns = mcpConfig.continuationApprovalPatterns?.[serverId]; let registeredCount = 0; for (const toolDef of discoveredTools) { try { @@ -201,6 +230,7 @@ export async function connectAndRegisterMCPTools( serverId, toolDef, client, + serverContinuationPatterns, ); tools[qualifiedName] = aiTool; registeredCount += 1; diff --git a/src/mcp/types.ts b/src/mcp/types.ts index 56552a2..510fc4d 100644 --- a/src/mcp/types.ts +++ b/src/mcp/types.ts @@ -25,12 +25,17 @@ export type MCPServerConfig = StdioMCPServerConfig | RemoteMCPServerConfig; * `sendImsToken` — forward the user's IMS Bearer token in the Authorization header. * `apiKey` — static API key to send as x-api-key (omitted when undefined). * `instructions` — additional prompt instructions appended to the system prompt. + * `continuationApprovalPatterns` — glob patterns (e.g. `evaluate_*`) matched against + * the bare MCP tool name. Matching tools run without pre-execution approval, then + * pause the agentic loop after they finish so the user can review results and decide + * whether to continue (post-execution "continuation approval" gate). */ export interface BuiltInMCPServerConfig { type: 'http' | 'sse'; url: string; sendImsToken?: boolean; instructions?: string; + continuationApprovalPatterns?: string[]; } export function isStdioConfig(config: MCPServerConfig): config is StdioMCPServerConfig { diff --git a/src/server.ts b/src/server.ts index 4342a28..5396eb4 100644 --- a/src/server.ts +++ b/src/server.ts @@ -10,6 +10,7 @@ import { initTelemetry, flushTelemetry } from './telemetry.js'; import { MCPClient } from './mcp/client.js'; import { buildApprovalContinuationResponse, + buildContinuationParts, getNewlyResolvedToolOutputs, hasPendingApprovals, unwrapToolOutput, @@ -215,6 +216,14 @@ async function handleChat(request: Request, env: Env): Promise { const { allTools, mcpClients, mcpConfig, mcpErrors, generatedToolsIndex, builtInServers } = assembled; + // A tool declares a post-execution "continuation approval" gate via + // providerOptions.daAgent.continuationApproval (built-in tools set it inline; + // MCP tools get it during adaptation when they match a server pattern). When such + // a tool runs we halt the agentic loop after its result and prompt the user to + // continue — the LLM never decides whether to pause. + const requiresContinuationApproval = (toolName: string): boolean => + allTools[toolName]?.providerOptions?.daAgent?.continuationApproval === true; + console.log(`[da-agent:perf] early=${t1 - t0}ms parallel=${t2 - t1}ms pre-stream=${t2 - t0}ms`); const { messages, requestedSkills, imsToken, attachments = [], sessionId } = parsed.data; @@ -342,7 +351,15 @@ async function handleChat(request: Request, env: Env): Promise { system: systemPrompt, messages: modelMessages as ModelMessage[], tools: allTools, - stopWhen: stepCountIs(5), + // Halt after the normal step budget OR immediately after a step that ran a + // continuation-gated tool, so the user can review results before continuing. + stopWhen: [ + stepCountIs(5), + ({ steps }) => { + const last = steps.at(-1); + return !!last?.toolCalls?.some((tc) => requiresContinuationApproval(tc.toolName)); + }, + ], experimental_telemetry: { isEnabled: true, functionId: 'da-agent-chat', @@ -356,26 +373,47 @@ async function handleChat(request: Request, env: Env): Promise { }, }); - // When tools were approved this request, prepend their outputs as tool-output-available - // events (unwrapped to the raw MCP shape the client renders) then merge the model stream. - // Otherwise emit the streamText response directly so the common path is unchanged. - const streamResponse = newlyResolvedOutputs.length - ? createUIMessageStreamResponse({ - stream: createUIMessageStream({ - execute: ({ writer }) => { - for (const { toolCallId, output } of newlyResolvedOutputs) { - writer.write({ - type: 'tool-output-available', - toolCallId, - output: unwrapToolOutput(output), - }); - } - writer.merge(result.toUIMessageStream()); - }, - onError: (error) => formatErrorForLog(error), - }), - }) - : result.toUIMessageStreamResponse(); + // Build the UI stream manually so we can (a) prepend outputs of tools approved this + // request as tool-output-available events (unwrapped to the raw MCP shape the client + // renders) and (b) emit a transient `data-continuation` part after the model stream for + // any continuation-gated tool that just ran, while holding the terminal `finish` chunk + // so the ordering is `…tool-output-available, data-continuation, finish`. The transient + // part is delivered to the client but never merged into message history. + const streamResponse = createUIMessageStreamResponse({ + stream: createUIMessageStream({ + execute: async ({ writer }) => { + for (const { toolCallId, output } of newlyResolvedOutputs) { + writer.write({ + type: 'tool-output-available', + toolCallId, + output: unwrapToolOutput(output), + }); + } + + const reader = result.toUIMessageStream().getReader(); + let finishChunk: Awaited>['value'] | null = null; + for (;;) { + // eslint-disable-next-line no-await-in-loop -- sequential stream consumption + const { done, value } = await reader.read(); + if (done) break; + if (value.type === 'finish') { + finishChunk = value; + } else { + writer.write(value); + } + } + + const continuationParts = buildContinuationParts( + (await result.steps).at(-1), + requiresContinuationApproval, + ); + for (const part of continuationParts) writer.write(part); + + if (finishChunk) writer.write(finishChunk); + }, + onError: (error) => formatErrorForLog(error), + }), + }); const headers = new Headers(streamResponse.headers); for (const [key, value] of Object.entries(CORS_HEADERS)) { diff --git a/src/tool-approval.ts b/src/tool-approval.ts index 1279dbf..56b2233 100644 --- a/src/tool-approval.ts +++ b/src/tool-approval.ts @@ -82,6 +82,43 @@ export function unwrapToolOutput(output: unknown): unknown { return output; } +/** A `data-continuation` transient stream part driving the client's Continue/Stop prompt. */ +export interface ContinuationPart { + type: 'data-continuation'; + transient: true; + data: { toolCallId: string; toolName: string }; +} + +interface StepLike { + toolCalls: Array<{ toolCallId: string; toolName: string }>; + toolResults: Array<{ toolCallId: string }>; +} + +/** + * The transient continuation parts to emit for the given (final) step: one per + * continuation-gated tool that actually produced a result in this step. A gated tool + * with no result (e.g. it was itself pre-execution-paused and never ran) is skipped so + * we never prompt "continue?" for a tool that hasn't finished. + */ +export function buildContinuationParts( + lastStep: StepLike | undefined, + requiresContinuationApproval: (toolName: string) => boolean, +): ContinuationPart[] { + if (!lastStep) return []; + const resultIds = new Set(lastStep.toolResults.map((r) => r.toolCallId)); + const parts: ContinuationPart[] = []; + for (const tc of lastStep.toolCalls) { + if (requiresContinuationApproval(tc.toolName) && resultIds.has(tc.toolCallId)) { + parts.push({ + type: 'data-continuation', + transient: true, + data: { toolCallId: tc.toolCallId, toolName: tc.toolName }, + }); + } + } + return parts; +} + export function buildApprovalContinuationResponse( toolOutputs: Array<{ toolCallId: string; output: unknown }>, corsHeaders: Record, diff --git a/src/tool-assembly.ts b/src/tool-assembly.ts index c4d3c2a..cb7a9d3 100644 --- a/src/tool-assembly.ts +++ b/src/tool-assembly.ts @@ -99,6 +99,7 @@ export async function assembleTools( } const builtInServers = getBuiltInMcpServers(env); + const continuationApprovalPatterns: Record = {}; for (const [id, builtIn] of Object.entries(builtInServers)) { const headers: Record = {}; @@ -111,6 +112,9 @@ export async function assembleTools( url: builtIn.url, ...(Object.keys(headers).length > 0 ? { headers } : {}), }; + if (builtIn.continuationApprovalPatterns?.length) { + continuationApprovalPatterns[id] = builtIn.continuationApprovalPatterns; + } } const mcpConfig = @@ -118,6 +122,7 @@ export async function assembleTools( ? { mcpServers: allMcpServers, toolAllowPatterns: Object.keys(allMcpServers).map((id) => `mcp__${id}__*`), + continuationApprovalPatterns, } : null; diff --git a/test/mcp/built-in-servers.test.ts b/test/mcp/built-in-servers.test.ts index 5dcf292..4716d05 100644 --- a/test/mcp/built-in-servers.test.ts +++ b/test/mcp/built-in-servers.test.ts @@ -33,4 +33,9 @@ describe('getBuiltInMcpServers', () => { ); expect(servers['governance-agent'].url).toBe('http://localhost:8000/mcp/'); }); + + it('gates evaluate_* tools behind a post-execution continuation approval', () => { + const servers = getBuiltInMcpServers(envWith()); + expect(servers['governance-agent'].continuationApprovalPatterns).toEqual(['evaluate_*']); + }); }); diff --git a/test/mcp/tool-adapter.test.ts b/test/mcp/tool-adapter.test.ts index d9e7fb7..8595461 100644 --- a/test/mcp/tool-adapter.test.ts +++ b/test/mcp/tool-adapter.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { mcpToolToAITool } from '../../src/mcp/tool-adapter.js'; +import { mcpToolToAITool, matchesGlob } from '../../src/mcp/tool-adapter.js'; import type { MCPClient, MCPToolDefinition } from '../../src/mcp/client.js'; const fakeClient = {} as MCPClient; @@ -9,6 +9,11 @@ function needsApproval(mcpTool: MCPToolDefinition): Promise return Promise.resolve(aiTool.needsApproval?.({}, { toolCallId: 'x', messages: [] })); } +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function continuationFlag(aiTool: any): boolean { + return aiTool.providerOptions?.daAgent?.continuationApproval === true; +} + describe('mcpToolToAITool', () => { it('gates the tool behind approval when the server sets destructiveHint', async () => { expect( @@ -32,3 +37,59 @@ describe('mcpToolToAITool', () => { ).toBe(false); }); }); + +describe('matchesGlob', () => { + it('matches a trailing wildcard', () => { + expect(matchesGlob('evaluate_page', 'evaluate_*')).toBe(true); + expect(matchesGlob('evaluate_image', 'evaluate_*')).toBe(true); + }); + + it('does not match a different prefix', () => { + expect(matchesGlob('retrieve_page', 'evaluate_*')).toBe(false); + }); + + it('requires a full-string match (anchored)', () => { + expect(matchesGlob('pre_evaluate_page', 'evaluate_*')).toBe(false); + expect(matchesGlob('evaluate', 'evaluate_*')).toBe(false); + }); + + it('escapes regex metacharacters in the pattern', () => { + expect(matchesGlob('a.b', 'a.b')).toBe(true); + expect(matchesGlob('axb', 'a.b')).toBe(false); + }); +}); + +describe('mcpToolToAITool continuation approval', () => { + it('flags a matching tool for continuation approval and skips pre-exec approval', async () => { + const { tool: aiTool } = mcpToolToAITool( + 'governance-agent', + { name: 'evaluate_page' }, + fakeClient, + ['evaluate_*'], + ); + expect(continuationFlag(aiTool)).toBe(true); + // Even though it is unannotated (would otherwise fail-closed to true), the + // continuation gate suppresses the pre-execution approval to avoid a double prompt. + expect(await aiTool.needsApproval?.({}, { toolCallId: 'x', messages: [] })).toBe(false); + }); + + it('does not flag a non-matching tool and keeps annotation-based gating', async () => { + const { tool: aiTool } = mcpToolToAITool( + 'governance-agent', + { name: 'retrieve_brand_rules' }, + fakeClient, + ['evaluate_*'], + ); + expect(continuationFlag(aiTool)).toBe(false); + expect(await aiTool.needsApproval?.({}, { toolCallId: 'x', messages: [] })).toBe(true); + }); + + it('does not flag anything when no patterns are configured', () => { + const { tool: aiTool } = mcpToolToAITool( + 'governance-agent', + { name: 'evaluate_page' }, + fakeClient, + ); + expect(continuationFlag(aiTool)).toBe(false); + }); +}); diff --git a/test/tool-approval.test.ts b/test/tool-approval.test.ts index 8139011..9043f79 100644 --- a/test/tool-approval.test.ts +++ b/test/tool-approval.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest'; import { buildApprovalContinuationResponse, + buildContinuationParts, getNewlyResolvedToolOutputs, hasPendingApprovals, resolvedToolCallIds, @@ -183,4 +184,66 @@ describe('tool approval helpers', () => { expect(events.some((e) => e.type === 'finish')).toBe(true); }); }); + + describe('buildContinuationParts', () => { + const gates = (name: string) => name === 'mcp__governance-agent__evaluate_page'; + + it('emits one transient part for a gated tool that produced a result', () => { + const parts = buildContinuationParts( + { + toolCalls: [{ toolCallId: 'call-a', toolName: 'mcp__governance-agent__evaluate_page' }], + toolResults: [{ toolCallId: 'call-a' }], + }, + gates, + ); + expect(parts).toEqual([ + { + type: 'data-continuation', + transient: true, + data: { toolCallId: 'call-a', toolName: 'mcp__governance-agent__evaluate_page' }, + }, + ]); + }); + + it('does not emit for a non-gated tool', () => { + const parts = buildContinuationParts( + { + toolCalls: [{ toolCallId: 'call-a', toolName: 'content_read' }], + toolResults: [{ toolCallId: 'call-a' }], + }, + gates, + ); + expect(parts).toEqual([]); + }); + + it('does not emit for a gated tool that produced no result', () => { + const parts = buildContinuationParts( + { + toolCalls: [{ toolCallId: 'call-a', toolName: 'mcp__governance-agent__evaluate_page' }], + toolResults: [], + }, + gates, + ); + expect(parts).toEqual([]); + }); + + it('emits one part per gated tool in a multi-tool step', () => { + const parts = buildContinuationParts( + { + toolCalls: [ + { toolCallId: 'call-a', toolName: 'mcp__governance-agent__evaluate_page' }, + { toolCallId: 'call-b', toolName: 'content_read' }, + { toolCallId: 'call-c', toolName: 'mcp__governance-agent__evaluate_page' }, + ], + toolResults: [{ toolCallId: 'call-a' }, { toolCallId: 'call-c' }], + }, + gates, + ); + expect(parts.map((p) => p.data.toolCallId)).toEqual(['call-a', 'call-c']); + }); + + it('returns nothing when there is no final step', () => { + expect(buildContinuationParts(undefined, gates)).toEqual([]); + }); + }); }); From 0adb183578b00791c9c8edcfebb0467bb6c3e26f Mon Sep 17 00:00:00 2001 From: Andrei Tuicu Date: Wed, 29 Jul 2026 17:27:44 +0200 Subject: [PATCH 10/10] fix(cleanup): remove complexity. tools can have both pre and continuation approval --- src/mcp/tool-adapter.ts | 9 ++++----- src/mcp/types.ts | 7 ++++--- test/mcp/tool-adapter.test.ts | 9 +++++---- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/mcp/tool-adapter.ts b/src/mcp/tool-adapter.ts index 37f292f..89575ca 100644 --- a/src/mcp/tool-adapter.ts +++ b/src/mcp/tool-adapter.ts @@ -113,9 +113,7 @@ export function mcpToolToAITool( mcpTool.inputSchema as Record | undefined, ); - // Continuation-gated tools run WITHOUT pre-execution approval and instead pause - // the agentic loop after they finish (see server.ts). Skipping the pre-exec gate - // here avoids a double prompt (approve-to-run AND approve-to-continue). + // Continuation-gated tools pause the agentic loop after they finish (see server.ts), const isContinuationGated = matchesAnyGlob(mcpTool.name, continuationApprovalPatterns); return { @@ -133,9 +131,10 @@ export function mcpToolToAITool( // fact that annotations are optional, we gate unless the tool tells us it // is safe: skip approval only when it is read-only OR explicitly // non-destructive. Everything else — including unannotated tools — - // requires approval. Continuation-gated tools opt out (post-exec gate instead). + // requires approval. This is independent of continuation gating: a tool + // can require both pre-execution approval and post-execution continuation + // approval. needsApproval: async () => { - if (isContinuationGated) return false; const { readOnlyHint, destructiveHint } = mcpTool.annotations ?? {}; return readOnlyHint !== true && destructiveHint !== false; }, diff --git a/src/mcp/types.ts b/src/mcp/types.ts index 510fc4d..6bd6d55 100644 --- a/src/mcp/types.ts +++ b/src/mcp/types.ts @@ -26,9 +26,10 @@ export type MCPServerConfig = StdioMCPServerConfig | RemoteMCPServerConfig; * `apiKey` — static API key to send as x-api-key (omitted when undefined). * `instructions` — additional prompt instructions appended to the system prompt. * `continuationApprovalPatterns` — glob patterns (e.g. `evaluate_*`) matched against - * the bare MCP tool name. Matching tools run without pre-execution approval, then - * pause the agentic loop after they finish so the user can review results and decide - * whether to continue (post-execution "continuation approval" gate). + * the bare MCP tool name. Matching tools pause the agentic loop after they finish so + * the user can review results and decide whether to continue (post-execution + * "continuation approval" gate). This is independent of pre-execution approval — + * a matching tool may still require approval to run in the first place. */ export interface BuiltInMCPServerConfig { type: 'http' | 'sse'; diff --git a/test/mcp/tool-adapter.test.ts b/test/mcp/tool-adapter.test.ts index 8595461..248f928 100644 --- a/test/mcp/tool-adapter.test.ts +++ b/test/mcp/tool-adapter.test.ts @@ -60,7 +60,7 @@ describe('matchesGlob', () => { }); describe('mcpToolToAITool continuation approval', () => { - it('flags a matching tool for continuation approval and skips pre-exec approval', async () => { + it('flags a matching tool for continuation approval independently of pre-exec approval', async () => { const { tool: aiTool } = mcpToolToAITool( 'governance-agent', { name: 'evaluate_page' }, @@ -68,9 +68,10 @@ describe('mcpToolToAITool continuation approval', () => { ['evaluate_*'], ); expect(continuationFlag(aiTool)).toBe(true); - // Even though it is unannotated (would otherwise fail-closed to true), the - // continuation gate suppresses the pre-execution approval to avoid a double prompt. - expect(await aiTool.needsApproval?.({}, { toolCallId: 'x', messages: [] })).toBe(false); + // Continuation gating and pre-execution approval are independent: this + // unannotated tool still fails closed on the pre-exec gate while also being + // continuation-gated, so it can be both pre-gated and post-gated. + expect(await aiTool.needsApproval?.({}, { toolCallId: 'x', messages: [] })).toBe(true); }); it('does not flag a non-matching tool and keeps annotation-based gating', async () => {