Skip to content

Commit d9768ca

Browse files
authored
fix: Stop pointing non-Actor tools at fetch-actor-details (#1485)
## Why Closes #1214 Invalid- and missing-arguments errors told every tool to inspect its schema via `fetch-actor-details`. That tool only describes Actors and may not be served, so for `search-apify-docs` and other non-Actor tools the advice was dead. ## What changed The pointer stays only for an Actor tool in a session that serves `fetch-actor-details`, and that text is unchanged. Everything else now points at the schema in `tools/list`. Two additions beyond the issue: the missing-arguments message had the same pointer, and an Actor tool without `fetch-actor-details` served got the same dead advice. ## Notes for reviewers (human-written) - ## Proof it works Six new cases in `tool_call_engine.test.ts`; four failed before the fix. Dropping either half of the condition fails only its own two cases. ``` Test Files 115 passed (115) Tests 2118 passed | 1 skipped (2119) Start at 11:43:23 Duration 4.33s (transform 5.27s, setup 0ms, import 38.51s, tests 3.02s, environment 5ms) ``` co-written with Claude Code.
1 parent b5b29df commit d9768ca

2 files changed

Lines changed: 82 additions & 4 deletions

File tree

‎src/mcp/tool_call_engine.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,12 +175,17 @@ export async function prepareToolCall(params: {
175175

176176
const actorName = extractActorName(tool, args as Record<string, unknown>);
177177
const actorId = extractActorId(tool);
178+
// fetch-actor-details only describes Actors, and only helps if this session serves it.
179+
const schemaHint =
180+
tool.type === TOOL_TYPE.ACTOR && tools.has(HELPER_TOOLS.ACTOR_GET_DETAILS)
181+
? `using ${HELPER_TOOLS.ACTOR_GET_DETAILS} tool`
182+
: 'in tools/list';
178183

179184
if (!args) {
180185
return {
181186
message: dedent`
182187
Missing arguments for tool "${name}".
183-
Please provide the required arguments for this tool. Check the tool's input schema using ${HELPER_TOOLS.ACTOR_GET_DETAILS} tool to see what parameters are required.
188+
Please provide the required arguments for this tool. Check the tool's input schema ${schemaHint} to see what parameters are required.
184189
`,
185190
toolStatus: TOOL_STATUS.SOFT_FAIL,
186191
callDiagnostics: {
@@ -233,7 +238,7 @@ export async function prepareToolCall(params: {
233238
message: dedent`
234239
Invalid arguments for tool "${tool.name}".
235240
Validation errors: ${errorMessages}.
236-
Please check the tool's input schema using ${HELPER_TOOLS.ACTOR_GET_DETAILS} tool and ensure all required parameters are provided with correct types and values.
241+
Please check the tool's input schema ${schemaHint} and ensure all required parameters are provided with correct types and values.
237242
`,
238243
toolStatus: TOOL_STATUS.SOFT_FAIL,
239244
callDiagnostics: {

‎tests/unit/tool_call_engine.test.ts‎

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,15 @@ import { afterEach, describe, expect, it, vi } from 'vitest';
22

33
import log from '@apify/log';
44

5-
import { FAILURE_CATEGORY, TOOL_STATUS } from '../../src/const.js';
5+
import { FAILURE_CATEGORY, HELPER_TOOLS, TOOL_STATUS } from '../../src/const.js';
66
import type { ActorsMcpServer } from '../../src/mcp/server.js';
77
import type { InvalidToolCall, PreparedCall } from '../../src/mcp/tool_call_engine.js';
88
import { executeSyncToolCall, prepareToolCall } from '../../src/mcp/tool_call_engine.js';
9-
import type { ToolCallTelemetryProperties } from '../../src/types.js';
9+
import { fetchActorDetails } from '../../src/tools/actors/fetch_actor_details.js';
10+
import type { ToolCallTelemetryProperties, ToolEntry, ToolInputSchema } from '../../src/types.js';
11+
import { TOOL_TYPE } from '../../src/types.js';
12+
import { compileSchema } from '../../src/utils/ajv.js';
13+
import { respondRaw } from '../../src/utils/mcp.js';
1014
import { makePaymentRequiredError, makeRecorderTool, makeThrowingTool, withServer } from './helpers/mcp_server.js';
1115

1216
/** An abort signal for direct engine tests, optionally already aborted. */
@@ -126,6 +130,75 @@ describe('prepareToolCall()', () => {
126130
expect(telemetryData.tool_name).toBe('recorder-tool');
127131
});
128132
});
133+
134+
describe('argument failure hint', () => {
135+
const STRICT_SCHEMA = {
136+
type: 'object',
137+
properties: { query: { type: 'string', minLength: 1 } },
138+
required: ['query'],
139+
};
140+
141+
function makeStrictTool(type: typeof TOOL_TYPE.INTERNAL | typeof TOOL_TYPE.ACTOR): ToolEntry {
142+
const base = {
143+
name: 'strict-tool',
144+
description: 'requires a non-empty query',
145+
inputSchema: STRICT_SCHEMA as ToolInputSchema,
146+
ajvValidate: compileSchema(STRICT_SCHEMA),
147+
};
148+
return type === TOOL_TYPE.ACTOR
149+
? { ...base, type, actorId: 'actor-id-1', actorFullName: 'apify/strict-actor' }
150+
: { ...base, type, call: async () => respondRaw({ content: [] }) };
151+
}
152+
153+
// The map is built per case, so each one controls exactly whether fetch-actor-details is served.
154+
async function failureMessage(
155+
tool: ToolEntry,
156+
fetchActorDetailsServed: boolean,
157+
args: Record<string, unknown> | undefined,
158+
): Promise<string> {
159+
const tools = new Map<string, ToolEntry>([[tool.name, tool]]);
160+
if (fetchActorDetailsServed) tools.set(HELPER_TOOLS.ACTOR_GET_DETAILS, fetchActorDetails);
161+
const result = await prepareToolCall({
162+
tools,
163+
apifyToken: 'fake-token',
164+
name: tool.name,
165+
args,
166+
meta: undefined,
167+
requestHeaders: undefined,
168+
isTaskRequest: false,
169+
mcpSessionId: 's1',
170+
telemetryData: null,
171+
clientContext: undefined,
172+
});
173+
expect('message' in result).toBe(true);
174+
return (result as InvalidToolCall).message;
175+
}
176+
177+
describe.each([
178+
['invalid', { query: '' }],
179+
['missing', undefined],
180+
])('%s arguments', (_label, args) => {
181+
it('points a non-Actor tool at its tools/list schema, not fetch-actor-details', async () => {
182+
const message = await failureMessage(makeStrictTool(TOOL_TYPE.INTERNAL), true, args);
183+
184+
expect(message).not.toContain(HELPER_TOOLS.ACTOR_GET_DETAILS);
185+
expect(message).toContain('tools/list');
186+
});
187+
188+
it('points an Actor tool at fetch-actor-details when the session serves it', async () => {
189+
const message = await failureMessage(makeStrictTool(TOOL_TYPE.ACTOR), true, args);
190+
191+
expect(message).toContain(HELPER_TOOLS.ACTOR_GET_DETAILS);
192+
});
193+
194+
it('points an Actor tool at tools/list when fetch-actor-details is not served', async () => {
195+
const message = await failureMessage(makeStrictTool(TOOL_TYPE.ACTOR), false, args);
196+
197+
expect(message).not.toContain(HELPER_TOOLS.ACTOR_GET_DETAILS);
198+
expect(message).toContain('tools/list');
199+
});
200+
});
201+
});
129202
});
130203

131204
describe('executeSyncToolCall()', () => {

0 commit comments

Comments
 (0)