Skip to content
Open
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
5 changes: 5 additions & 0 deletions .changeset/approval-gate-final-response.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@openrouter/agent": patch
---

Run the approval check before executing pending tool calls on the `allowFinalResponse` path, and evaluate function-form `requireApproval` predicates against schema-normalized arguments so the approval decision matches the executed input.
47 changes: 37 additions & 10 deletions packages/agent/src/lib/conversation-state.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type * as models from '@openrouter/sdk/models';
import * as z4 from 'zod/v4';
import type {
ConversationState,
ParsedToolCall,
Expand Down Expand Up @@ -255,6 +256,21 @@ export function appendToMessages(
];
}

function normalizeToolCallArguments<
TTool extends Exclude<
Tool,
{
_brand: 'server-tool';
}
>,
>(toolCall: ParsedToolCall<TTool>, tool: TTool): boolean {
const parsed = z4.safeParse(tool.function.inputSchema, toolCall.arguments);
if (parsed.success) {
toolCall.arguments = parsed.data as ParsedToolCall<TTool>['arguments'];
}
return parsed.success;
}

/**
* Check if a tool call requires approval
* @param toolCall - The tool call to check
Expand All @@ -271,12 +287,6 @@ export async function toolRequiresApproval<TTools extends readonly Tool[]>(
context: TurnContext,
) => boolean | Promise<boolean>,
): Promise<boolean> {
// Call-level check takes precedence
if (callLevelCheck) {
return callLevelCheck(toolCall, context);
}

// Fall back to tool-level setting (server tools never require approval)
const tool = tools.find(
(
t,
Expand All @@ -287,17 +297,34 @@ export async function toolRequiresApproval<TTools extends readonly Tool[]>(
}
> => isClientTool(t) && t.function.name === toolCall.name,
);
if (tool) {
const argumentsAreValid = normalizeToolCallArguments(
toolCall as ParsedToolCall<typeof tool>,
tool,
);
if (!argumentsAreValid) {
// Invalid arguments cannot execute. Let executeTool surface its structured
// validation error instead of asking for approval or evaluating a
// predicate against unvalidated input.
return false;
}
}

// Call-level check takes precedence
if (callLevelCheck) {
return callLevelCheck(toolCall, context);
}

// Fall back to tool-level setting (server tools never require approval)
if (!tool) {
return false;
}
Comment thread
LukasParke marked this conversation as resolved.

const requireApproval = tool.function.requireApproval;

// If it's a function, call it with the tool's arguments and context.
// Arguments have already been parsed and validated against the tool's
// Zod inputSchema (a ZodObject), so the runtime shape is always a
// record here. A non-record value signals a real upstream bug — surface
// it rather than substituting an empty object.
// Arguments were schema-normalized above. The guard keeps this boundary
// explicit if an input schema ever produces a non-record value.
if (typeof requireApproval === 'function') {
const rawArgs: unknown = toolCall.arguments;
if (!isRecord(rawArgs)) {
Expand Down
4 changes: 4 additions & 0 deletions packages/agent/src/lib/model-result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3223,6 +3223,10 @@ export class ModelResult<
numberOfTurns: turnNumber,
};

if (await this.handleApprovalCheck(pendingToolCalls, turnNumber, currentResponse)) {
return;
}

await this.options.onTurnStart?.(turnContext);
await this.resolveAsyncFunctionsForTurn(turnContext);

Expand Down
55 changes: 55 additions & 0 deletions packages/agent/tests/unit/allow-final-response.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,61 @@ describe('allowFinalResponse', () => {
expect(fnCallOutput?.output).toContain('"temperature":99');
});

it('pauses for approval before executing final-response tool calls', async () => {
const executeSpy = vi.fn(async (_p: { location: string }) => ({
temperature: 99,
}));
const approvalTool = {
type: ToolType.Function,
function: {
name: 'get_weather',
description: 'Get the weather for a location.',
inputSchema: z.object({
location: z.string(),
}),
outputSchema: z.object({
temperature: z.number(),
}),
requireApproval: true,
execute: executeSpy,
},
} as const;
const saved: Array<{
status?: string;
pendingToolCalls?: unknown[];
}> = [];
const stateAccessor = {
load: async () => null,
save: async (state: { status?: string; pendingToolCalls?: unknown[] }) => {
saved.push(state);
},
};

mockBetaResponsesSend.mockResolvedValueOnce({
ok: true,
value: toolCallResponse(),
});

const result = callModel(client, {
model: 'test-model',
input: 'What is the weather?',
tools: [
approvalTool,
] as const,
stopWhen: stepCountIs(0),
allowFinalResponse: true,
state: stateAccessor as unknown as Parameters<typeof callModel>[1]['state'],
});

const pending = await result.getPendingToolCalls();

expect(pending).toHaveLength(1);
expect(pending[0]?.name).toBe('get_weather');
expect(executeSpy).not.toHaveBeenCalled();
expect(mockBetaResponsesSend).toHaveBeenCalledTimes(1);
expect(saved.some((state) => state.status === 'awaiting_approval')).toBe(true);
});

it('does not trigger when allowFinalResponse is false', async () => {
mockBetaResponsesSend.mockResolvedValueOnce({
ok: true,
Expand Down
103 changes: 103 additions & 0 deletions packages/agent/tests/unit/conversation-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
updateState,
} from '../../src/lib/conversation-state.js';
import { tool } from '../../src/lib/tool.js';
import { executeTool } from '../../src/lib/tool-executor.js';
import { hasApprovalRequiredTools, toolHasApprovalConfigured } from '../../src/lib/tool-types.js';

describe('Conversation State Utilities', () => {
Expand Down Expand Up @@ -308,6 +309,108 @@ describe('Conversation State Utilities', () => {
).toBe(true);
});

it('should apply schema defaults before function-based approval checks', async () => {
const toolWithDefaultedApproval = tool({
name: 'defaulted_action',
inputSchema: z.object({
destructive: z.boolean().default(true),
}),
requireApproval: (params) => params.destructive === true,
execute: async () => ({}),
});

const toolCall = {
id: '1',
name: 'defaulted_action',
arguments: {},
};

expect(
await toolRequiresApproval(
toolCall,
[
toolWithDefaultedApproval,
],
context,
),
).toBe(true);
expect(toolCall.arguments).toEqual({
destructive: true,
});
});

it('should apply schema defaults before call-level requireApproval', async () => {
const toolWithDefaultedInput = tool({
name: 'call_level_defaulted_action',
inputSchema: z.object({
destructive: z.boolean().default(true),
}),
execute: async () => ({}),
});
const toolCall = {
id: '1',
name: 'call_level_defaulted_action',
arguments: {},
};
let receivedArguments: unknown;

await toolRequiresApproval(
toolCall,
[
toolWithDefaultedInput,
],
context,
(normalizedToolCall) => {
receivedArguments = normalizedToolCall.arguments;
return false;
},
);

expect(receivedArguments).toEqual({
destructive: true,
});
});

it('should surface invalid arguments through execution without running approval or tool code', async () => {
let approvalChecked = false;
let toolExecuted = false;
const validatedTool = tool({
name: 'validated_action',
inputSchema: z.object({
count: z.number(),
}),
execute: async () => {
toolExecuted = true;
return {};
},
});
const toolCall = {
id: '1',
name: 'validated_action',
arguments: {
count: 'not a number',
},
};

const requiresApproval = await toolRequiresApproval(
toolCall,
[
validatedTool,
],
context,
() => {
approvalChecked = true;
return true;
},
);
const execution = await executeTool(validatedTool, toolCall, context);

expect(requiresApproval).toBe(false);
expect(approvalChecked).toBe(false);
expect(toolExecuted).toBe(false);
expect(execution?.error).toBeInstanceOf(z.ZodError);
});

it('should support async function-based tool-level requireApproval', async () => {
// Tool with async function-based approval
const toolWithAsyncApproval = tool({
Comment thread
LukasParke marked this conversation as resolved.
Expand Down