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
7 changes: 4 additions & 3 deletions src/mcp/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Two MCP protocol revisions are served, each by its own adapter:
- `tool_call_engine.ts` — shared `tools/call` orchestration. `prepareToolCall()` handles
preparation; `executeSyncToolCall()` runs synchronous calls.
- `client.ts` — `connectMCPClient(url, token)`: transport negotiation.
- `proxy.ts` — MCP-in-MCP: `getMCPServerID(url)`.
- `proxy.ts` — MCP-in-MCP: `getMCPServerID(url)`, `getProxyMCPServerToolName(url, toolName)`.
- `actors.ts` — `getActorMCPServerPath()`: parses an Actor's `webServerMcpPath`.
- `utils.ts` — `processParamsGetTools()`: turns `?actors=` URL params into tools.
- `tool_call_error_mapper.ts` — shared tool-call error classification.
Expand All @@ -61,8 +61,9 @@ Two MCP protocol revisions are served, each by its own adapter:
each other.
- **Tool names: capped + hash-deduped.** Names are capped at `MAX_TOOL_NAME_LENGTH`;
over-length or colliding names get a `TOOL_NAME_HASH_LENGTH` hash suffix so the
exposed set stays unique within the limit (the hashing is in `../tools/actor_tool_naming.ts`).
Never widen the cap — downstream clients depend on it.
exposed set stays unique within the limit (Actor tools: `../tools/actor_tool_naming.ts`;
proxied Actor-MCP tools: `proxy.ts` `getProxyMCPServerToolName`). Never widen the
cap — downstream clients depend on it.
- **Proxy server IDs are keyed by URL, not Actor ID.** `getMCPServerID(url)` is
`sha256(url)` sliced to `SERVER_ID_LENGTH`. One Actor can expose both an SSE and a
streamable endpoint; keying by URL keeps those distinct. Keying by Actor ID would
Expand Down
18 changes: 12 additions & 6 deletions src/mcp/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { fixedAjvCompile } from '../tools/actor_input_schema.js';
import type { ActorMcpTool, ToolEntry } from '../types.js';
import { TOOL_TYPE } from '../types.js';
import { ajv } from '../utils/ajv.js';
import { MAX_TOOL_NAME_LENGTH, SERVER_ID_LENGTH } from './const.js';
import { MAX_TOOL_NAME_LENGTH, SERVER_ID_LENGTH, TOOL_NAME_HASH_LENGTH } from './const.js';

/**
* Generates a unique server ID by hashing the URL.
Expand All @@ -20,14 +20,20 @@ export function getMCPServerID(url: string): string {
}

/**
* Prefixes the tool name with the server ID hash and truncates to MAX_TOOL_NAME_LENGTH.
* Truncation can in theory collide two different origin tool names.
* Prefixes the tool name with the server ID hash. Over-length names get a hash suffix
* (same pattern as actor tool names) so bare truncation cannot collide two different
* origin tool names into one exposed name.
*/
function getProxyMCPServerToolName(url: string, toolName: string): string {
export function getProxyMCPServerToolName(url: string, toolName: string): string {
const prefix = getMCPServerID(url);

const fullName = `${prefix}-${toolName}`;
return fullName.slice(0, MAX_TOOL_NAME_LENGTH);

if (fullName.length <= MAX_TOOL_NAME_LENGTH) {
return fullName;
}

const hash = createHash('sha256').update(fullName).digest('hex').slice(0, TOOL_NAME_HASH_LENGTH);
return `${fullName.slice(0, MAX_TOOL_NAME_LENGTH - TOOL_NAME_HASH_LENGTH - 1)}-${hash}`;
}

export async function getMCPServerTools(actorID: string, client: Client, serverUrl: string): Promise<ToolEntry[]> {
Expand Down
6 changes: 6 additions & 0 deletions src/mcp/tool_dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ export async function dispatchToolCall(params: {
CallToolResultSchema,
{
timeout: EXTERNAL_TOOL_CALL_TIMEOUT_MSEC,
signal,
Comment thread
vojtechj-apify marked this conversation as resolved.
},
);

Expand All @@ -198,6 +199,11 @@ export async function dispatchToolCall(params: {

result = { ...res };
} catch (error) {
if (signal.aborted) {
// Yield a macrotask first: the SDK sends notifications/cancelled fire-and-forget on
// the transport's AbortController, which the finally's close() would abort.
await new Promise((resolve) => setImmediate(resolve));
}
({ toolStatus, callDiagnostics } = buildExecutionDiagnostics({
error,
isAborted: Boolean(signal.aborted),
Expand Down
32 changes: 27 additions & 5 deletions src/tools/actors/call_actor.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { Client } from '@modelcontextprotocol/sdk/client/index.js';
import type { ContentBlock } from '@modelcontextprotocol/sdk/types.js';
import { CallToolResultSchema } from '@modelcontextprotocol/sdk/types.js';
import dedent from 'dedent';
import { z } from 'zod';

Expand All @@ -15,6 +16,7 @@ import {
} from '../../const.js';
import { ACTOR_LOAD_ERROR_KIND, ActorLoadError } from '../../errors.js';
import { connectMCPClient } from '../../mcp/client.js';
import { EXTERNAL_TOOL_CALL_TIMEOUT_MSEC } from '../../mcp/const.js';
import type { PaymentProvider } from '../../payments/types.js';
import type { ActorInfo, ApifyToken, InternalToolArgs, ToolEntry, ToolInputSchema } from '../../types.js';
import { TOOL_TYPE } from '../../types.js';
Expand Down Expand Up @@ -324,8 +326,10 @@ export async function handleMcpToolCall(params: {
mcpServerUrl: string | false;
apifyToken: string;
mcpSessionId?: string;
signal: AbortSignal;
}): Promise<ToolResponse | null> {
const { baseActorName, mcpToolName, input, isActorMcpServer, mcpServerUrl, apifyToken, mcpSessionId } = params;
const { baseActorName, mcpToolName, input, isActorMcpServer, mcpServerUrl, apifyToken, mcpSessionId, signal } =
params;

if (!isActorMcpServer) {
return respondServerError(`Actor '${baseActorName}' is not an MCP server.`);
Expand All @@ -337,17 +341,28 @@ export async function handleMcpToolCall(params: {
);
}

if (signal.aborted) {
return respondAborted();
}

let client: Client | null = null;
try {
client = await connectMCPClient(mcpServerUrl as string, apifyToken, mcpSessionId);
if (!client) {
return respondServerError(`Failed to connect to MCP server ${mcpServerUrl}`);
}

const result = await client.callTool({
name: mcpToolName,
arguments: input,
});
const result = await client.callTool(
{
name: mcpToolName,
arguments: input,
},
CallToolResultSchema,
{
timeout: EXTERNAL_TOOL_CALL_TIMEOUT_MSEC,
signal,
},
);

// `call-actor` declares `actorRunOutputSchema`, so MCP SDK ≥ 1.11.4 rejects any response
// without `structuredContent` (unless `isError: true`) with -32600. The pass-through has no
Expand All @@ -369,6 +384,12 @@ export async function handleMcpToolCall(params: {
},
});
} catch (error) {
if (signal.aborted) {
// Yield a macrotask first: the SDK sends notifications/cancelled fire-and-forget on the
// transport's AbortController, which the finally's close() would abort before it flushes.
await new Promise((resolve) => setImmediate(resolve));
return respondAborted();
}
Comment thread
vojtechj-apify marked this conversation as resolved.
logHttpError(error, `Failed to call MCP tool '${mcpToolName}' on Actor '${baseActorName}'`, {
actorName: baseActorName,
toolName: mcpToolName,
Expand Down Expand Up @@ -529,6 +550,7 @@ export async function callActorPreExecute(
mcpServerUrl: mcpServerUrlOrFalse,
apifyToken,
mcpSessionId,
signal: toolArgs.signal,
});
if (mcpResult) {
return { earlyResponse: mcpResult };
Expand Down
51 changes: 51 additions & 0 deletions tests/unit/mcp.proxy.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { createHash } from 'node:crypto';

import { describe, expect, it } from 'vitest';

import { MAX_TOOL_NAME_LENGTH, SERVER_ID_LENGTH, TOOL_NAME_HASH_LENGTH } from '../../src/mcp/const.js';
import { getMCPServerID, getProxyMCPServerToolName } from '../../src/mcp/proxy.js';

describe('getMCPServerID()', () => {
it('returns a stable SERVER_ID_LENGTH hex prefix of sha256(url)', () => {
const url = 'https://example.com/mcp';
const expected = createHash('sha256').update(url).digest('hex').slice(0, SERVER_ID_LENGTH);
expect(getMCPServerID(url)).toBe(expected);
expect(getMCPServerID(url)).toBe(getMCPServerID(url));
});

it('keys by URL so SSE and streamable endpoints stay distinct', () => {
expect(getMCPServerID('https://actor.example/sse')).not.toBe(getMCPServerID('https://actor.example/mcp'));
});
});

describe('getProxyMCPServerToolName()', () => {
const url = 'https://example.com/mcp';

it('returns prefix-toolName when under the length cap', () => {
const name = getProxyMCPServerToolName(url, 'list-items');
expect(name).toBe(`${getMCPServerID(url)}-list-items`);
expect(name.length).toBeLessThanOrEqual(MAX_TOOL_NAME_LENGTH);
});

it('hash-suffixes over-length names instead of bare truncation', () => {
const longTool = `very-long-origin-tool-name-${'x'.repeat(80)}`;
const fullName = `${getMCPServerID(url)}-${longTool}`;
const hash = createHash('sha256').update(fullName).digest('hex').slice(0, TOOL_NAME_HASH_LENGTH);
const name = getProxyMCPServerToolName(url, longTool);

expect(name.length).toBe(MAX_TOOL_NAME_LENGTH);
expect(name.endsWith(`-${hash}`)).toBe(true);
// Bare slice would drop the distinguishing suffix and collide; hash must survive.
expect(name).not.toBe(fullName.slice(0, MAX_TOOL_NAME_LENGTH));
});

it('keeps two over-length origin names distinct after capping', () => {
const sharedPrefix = `shared-prefix-${'y'.repeat(80)}`;
const a = getProxyMCPServerToolName(url, `${sharedPrefix}-alpha`);
const b = getProxyMCPServerToolName(url, `${sharedPrefix}-beta`);

expect(a).not.toBe(b);
expect(a.length).toBe(MAX_TOOL_NAME_LENGTH);
expect(b.length).toBe(MAX_TOOL_NAME_LENGTH);
});
});
31 changes: 31 additions & 0 deletions tests/unit/mcp.server.tool_call_contracts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,37 @@ describe('ACTOR_MCP remote-McpError containment (sync tools/call catch)', () =>
expect(properties.failure_category).toBe(FAILURE_CATEGORY.INVALID_INPUT);
});

it('forwards the abort signal into the remote Actor-MCP callTool options', async () => {
// Without `signal` in callTool options, cancel/disconnect cannot stop the remote call before
// EXTERNAL_TOOL_CALL_TIMEOUT_MSEC — ACTOR/INTERNAL already abort; ACTOR_MCP must match.
await withServer(async (server) => {
silenceLogs();
const callTool = vi.fn().mockResolvedValue({ content: [{ type: 'text', text: 'ok' }] });
const stubClient = {
callTool,
close: vi.fn().mockResolvedValue(undefined),
setNotificationHandler: vi.fn(),
} as unknown as Client;
vi.spyOn(mcpClient, 'connectMCPClient').mockResolvedValue(stubClient);

const controller = new AbortController();
server.upsertTools([makeActorMcpTool()]);
const handler = getRequestHandler(server, 'tools/call');
await handler(
{
method: 'tools/call',
params: { name: 'test-actor-mcp-tool', arguments: {}, _meta: { mcpSessionId: 's1' } },
},
{ signal: controller.signal, sendNotification: vi.fn() },
);

expect(callTool).toHaveBeenCalledTimes(1);
const options = callTool.mock.calls[0][2] as { signal?: AbortSignal; timeout?: number };
expect(options.signal).toBe(controller.signal);
expect(options.timeout).toBeGreaterThan(0);
});
});

it('re-throws an escaped McpError as a JSON-RPC error, not an isError tool result', async () => {
// Escaped McpErrors must remain JSON-RPC errors, including 402-coded errors.
await withServer(async (server) => {
Expand Down
80 changes: 80 additions & 0 deletions tests/unit/tools.call_actor_common.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { Client } from '@modelcontextprotocol/sdk/client/index.js';
import { ApifyApiError } from 'apify-client';
import type { AxiosResponse } from 'axios';
import { beforeEach, describe, expect, it, vi } from 'vitest';
Expand All @@ -8,11 +9,14 @@ import {
HELPER_TOOLS,
TOOL_STATUS,
} from '../../src/const.js';
import * as mcpClient from '../../src/mcp/client.js';
import { EXTERNAL_TOOL_CALL_TIMEOUT_MSEC } from '../../src/mcp/const.js';
import {
buildCallActorAppsDescription,
buildCallActorDescription,
buildCallActorErrorResponse,
callActorArgs,
handleMcpToolCall,
resolveAndValidateActor,
} from '../../src/tools/actors/call_actor.js';
import type { InternalToolArgs, ToolEntry } from '../../src/types.js';
Expand Down Expand Up @@ -331,4 +335,80 @@ describe('call_actor_common', () => {
});
});
});

describe('handleMcpToolCall()', () => {
beforeEach(() => {
vi.restoreAllMocks();
});

it('forwards signal and timeout into client.callTool options', async () => {
const callTool = vi.fn().mockResolvedValue({
content: [{ type: 'text', text: 'remote ok' }],
isError: false,
});
vi.spyOn(mcpClient, 'connectMCPClient').mockResolvedValue({
callTool,
close: vi.fn().mockResolvedValue(undefined),
} as unknown as Client);

const controller = new AbortController();
const result = await handleMcpToolCall({
baseActorName: 'apify/mcp-demo',
mcpToolName: 'search',
input: { q: 'x' },
isActorMcpServer: true,
mcpServerUrl: 'https://example.invalid/mcp',
apifyToken: 'token',
signal: controller.signal,
});

expect(result?.isError).not.toBe(true);
expect(callTool).toHaveBeenCalledTimes(1);
const options = callTool.mock.calls[0][2] as { signal?: AbortSignal; timeout?: number };
expect(options.signal).toBe(controller.signal);
expect(options.timeout).toBe(EXTERNAL_TOOL_CALL_TIMEOUT_MSEC);
});

it('returns aborted when the signal is already aborted before the remote call', async () => {
const connectSpy = vi.spyOn(mcpClient, 'connectMCPClient').mockResolvedValue(null);
const controller = new AbortController();
controller.abort();

const result = await handleMcpToolCall({
Comment thread
vojtechj-apify marked this conversation as resolved.
baseActorName: 'apify/mcp-demo',
mcpToolName: 'search',
input: { q: 'x' },
isActorMcpServer: true,
mcpServerUrl: 'https://example.invalid/mcp',
apifyToken: 'token',
signal: controller.signal,
});

expect(result).toEqual({});
expect(connectSpy).not.toHaveBeenCalled();
});

it('returns aborted when the remote call rejects after the signal aborts', async () => {
const controller = new AbortController();
vi.spyOn(mcpClient, 'connectMCPClient').mockResolvedValue({
callTool: vi.fn().mockImplementation(async () => {
controller.abort();
throw new Error('aborted');
}),
close: vi.fn().mockResolvedValue(undefined),
} as unknown as Client);

const result = await handleMcpToolCall({
baseActorName: 'apify/mcp-demo',
mcpToolName: 'search',
input: { q: 'x' },
isActorMcpServer: true,
mcpServerUrl: 'https://example.invalid/mcp',
apifyToken: 'token',
signal: controller.signal,
});

expect(result).toEqual({});
});
});
});
Loading