Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/mcp/built-in-servers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ export function getBuiltInMcpServers(env: Env): Record<string, BuiltInMCPServerC
url: governanceUrl,
sendImsToken: true,
instructions: GOVERNANCE_AGENT_INSTRUCTIONS,
// evaluate_* tools produce reports the user should review before the agent
// continues (e.g. mid-skill, before preview/publish). Gate them with a
// post-execution continuation prompt instead of a pre-execution approval.
continuationApprovalPatterns: ['evaluate_*'],
},
};
}
31 changes: 30 additions & 1 deletion src/mcp/tool-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,20 @@ function getServerUrl(cfg: MCPServerConfig): string | null {
return null;
}

/**
* Match a name against a glob pattern where `*` means "any run of characters".
* All other regex metacharacters are escaped. Used for continuation-approval
* patterns (e.g. `evaluate_*` → matches `evaluate_page`).
*/
export function matchesGlob(name: string, pattern: string): boolean {
const escaped = pattern.replace(/[.*+?^${}()|[\]\\]/g, (ch) => (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.
Expand Down Expand Up @@ -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}`;
Expand All @@ -98,17 +113,27 @@ export function mcpToolToAITool(
mcpTool.inputSchema as Record<string, unknown> | undefined,
);

// Continuation-gated tools pause the agentic loop after they finish (see server.ts),
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 } } }
: {}),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vercel SDK provides natively the needsApproval method to gate before a tool executes.
It doesn't provide anything native for gating a tool after it finishes the execution, so this is the solution Claude came up with: found a freeform metadata, where we can mark tools that we want to gate after.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to point out that our tacked-on approvals system is the biggest source of crashes in the agent. We should be aware of this after merging and reconsider if we see that this is not behaving as expected.

// 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. This is independent of continuation gating: a tool
// can require both pre-execution approval and post-execution continuation
// approval.
needsApproval: async () => {
const { readOnlyHint, destructiveHint } = mcpTool.annotations ?? {};
return readOnlyHint !== true && destructiveHint !== false;
Expand Down Expand Up @@ -146,6 +171,8 @@ export async function connectAndRegisterMCPTools(
mcpConfig: {
mcpServers: Record<string, MCPServerConfig>;
toolAllowPatterns: string[];
/** Per-server glob patterns for post-execution continuation approval. */
continuationApprovalPatterns?: Record<string, string[]>;
},
options?: {
headers?: Record<string, string>;
Expand Down Expand Up @@ -194,13 +221,15 @@ 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 {
const { name: qualifiedName, tool: aiTool } = mcpToolToAITool(
serverId,
toolDef,
client,
serverContinuationPatterns,
);
tools[qualifiedName] = aiTool;
registeredCount += 1;
Expand Down
6 changes: 6 additions & 0 deletions src/mcp/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,18 @@ 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 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';
url: string;
sendImsToken?: boolean;
instructions?: string;
continuationApprovalPatterns?: string[];
}

export function isStdioConfig(config: MCPServerConfig): config is StdioMCPServerConfig {
Expand Down
37 changes: 37 additions & 0 deletions src/message-pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,3 +426,40 @@ export function expandLatestUserAttachmentsForModel(
}

/* eslint-enable @typescript-eslint/no-explicit-any */

/** 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;
}
25 changes: 25 additions & 0 deletions src/prompt-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,31 @@ 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.

**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 exit_plan_mode", "status": "running" }
:::
\`\`\`
\`\`\`
:::task-item
{ "label": "Same label as in exit_plan_mode", "status": "done" }
:::
\`\`\`

Rules:
- Always call \`enter_plan_mode\` first, then \`exit_plan_mode\` with ALL planned steps.
- 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), 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:

Expand Down
53 changes: 49 additions & 4 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
ensureOrphanedToolResults,
expandUserSelectionContextForModel,
expandLatestUserAttachmentsForModel,
buildContinuationParts,
} from './message-pipeline.js';
import { buildSystemPrompt } from './prompt-builder.js';
import { buildEarlyChatContext, resolveAsyncContext } from './chat-context.js';
Expand Down Expand Up @@ -72,7 +73,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);
Expand Down Expand Up @@ -206,6 +210,14 @@ async function handleChat(request: Request, env: Env): Promise<Response> {
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;
Expand Down Expand Up @@ -297,7 +309,7 @@ async function handleChat(request: Request, env: Env): Promise<Response> {
});

const stream = createUIMessageStream({
execute: ({ writer }) => {
execute: async ({ writer }) => {
// Stream results for tools the user approved this round so the client can
// move each approved card to its result state, before the model continues.
for (const o of executedOutputs) {
Expand Down Expand Up @@ -343,7 +355,15 @@ async function handleChat(request: Request, env: Env): Promise<Response> {
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',
Expand All @@ -357,7 +377,32 @@ async function handleChat(request: Request, env: Env): Promise<Response> {
},
});

writer.merge(result.toUIMessageStream());
// Merge the model stream manually so we can 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. (Outputs of tools
// approved this round were already streamed above from `executedOutputs`.)
const reader = result.toUIMessageStream().getReader();
let finishChunk: Awaited<ReturnType<typeof reader.read>>['value'] | null = null;

@anfibiacreativa anfibiacreativa Aug 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the reorder here (hold finish, emit data-continuation, then finish) is the load-bearing bit and it's the one part with no test. buildContinuationParts is well covered on its own, but if the ordering regresses the client shows Continue/Stop at the wrong time or after finish. can we add an integration test that drives a gated tool through the stream and asserts …tool-output, data-continuation, finish?

cc: @AlexRRR

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) => {
console.error('[da-agent] stream error:', formatErrorForLog(error));
Expand Down
5 changes: 5 additions & 0 deletions src/tool-assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export async function assembleTools(
}

const builtInServers = getBuiltInMcpServers(env);
const continuationApprovalPatterns: Record<string, string[]> = {};

for (const [id, builtIn] of Object.entries(builtInServers)) {
const headers: Record<string, string> = {};
Expand All @@ -111,13 +112,17 @@ export async function assembleTools(
url: builtIn.url,
...(Object.keys(headers).length > 0 ? { headers } : {}),
};
if (builtIn.continuationApprovalPatterns?.length) {
continuationApprovalPatterns[id] = builtIn.continuationApprovalPatterns;
}
}

const mcpConfig =
Object.keys(allMcpServers).length > 0
? {
mcpServers: allMcpServers,
toolAllowPatterns: Object.keys(allMcpServers).map((id) => `mcp__${id}__*`),
continuationApprovalPatterns,
}
: null;

Expand Down
42 changes: 41 additions & 1 deletion src/tools/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 enter_plan_mode then exit_plan_mode with your planned tasks so the user can review and approve.',
};
} catch (e) {
return { error: String(e) };
}
Expand Down Expand Up @@ -557,6 +562,41 @@ export function createDATools(
},
});

// 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:
'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)'),
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:
Expand Down
5 changes: 5 additions & 0 deletions test/mcp/built-in-servers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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_*']);
});
});
Loading
Loading