Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -3207,6 +3207,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 @@ -323,6 +323,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