diff --git a/src/mcp/server.ts b/src/mcp/server.ts index ff3c60e..1a3e37a 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -12,7 +12,7 @@ import { ListToolsRequestSchema, McpError, } from '@modelcontextprotocol/sdk/types.js'; -import type { ActorCallOptions } from 'apify-client'; +import { type ActorCallOptions, ApifyApiError } from 'apify-client'; import log from '@apify/log'; @@ -453,6 +453,14 @@ export class ActorsMcpServer { return { content }; } } catch (error) { + if (error instanceof ApifyApiError) { + log.error(`Apify API error calling tool ${name}: ${error.message}`); + return { + content: [ + { type: 'text', text: `Apify API error calling tool ${name}: ${error.message}` }, + ], + }; + } log.error(`Error calling tool ${name}: ${error}`); throw new McpError( ErrorCode.InternalError, diff --git a/src/tools/actor.ts b/src/tools/actor.ts index 1c588da..1ba4466 100644 --- a/src/tools/actor.ts +++ b/src/tools/actor.ts @@ -225,7 +225,9 @@ export async function getActorsAsTools( } const getActorArgs = z.object({ - actorId: z.string().describe('Actor ID or a tilde-separated owner\'s username and Actor name.'), + actorId: z.string() + .min(1) + .describe('Actor ID or a tilde-separated owner\'s username and Actor name.'), }); /** @@ -245,10 +247,13 @@ export const getActor: ToolEntry = { ajvValidate: ajv.compile(zodToJsonSchema(getActorArgs)), call: async (toolArgs) => { const { args, apifyToken } = toolArgs; - const parsed = getActorArgs.parse(args); + const { actorId } = getActorArgs.parse(args); const client = new ApifyClient({ token: apifyToken }); // Get Actor - contains a lot of irrelevant information - const actor = await client.actor(parsed.actorId).get(); + const actor = await client.actor(actorId).get(); + if (!actor) { + return { content: [{ type: 'text', text: `Actor '${actorId}' not found.` }] }; + } return { content: [{ type: 'text', text: JSON.stringify(actor) }] }; }, } as InternalTool, diff --git a/src/tools/build.ts b/src/tools/build.ts index 948e1e0..91daec0 100644 --- a/src/tools/build.ts +++ b/src/tools/build.ts @@ -96,6 +96,7 @@ function truncateActorReadme(readme: string, limit = ACTOR_README_MAX_LENGTH): s const getActorDefinitionArgsSchema = z.object({ actorName: z.string() + .min(1) .describe('Retrieve input, readme, and other details for Actor ID or Actor full name. ' + 'Actor name is always composed from `username/name`'), limit: z.number() @@ -124,6 +125,9 @@ export const actorDefinitionTool: ToolEntry = { const parsed = getActorDefinitionArgsSchema.parse(args); const v = await getActorDefinition(parsed.actorName, apifyToken, parsed.limit); + if (!v) { + return { content: [{ type: 'text', text: `Actor '${parsed.actorName}' not found.` }] }; + } if (v && v.input && 'properties' in v.input && v.input) { const properties = filterSchemaProperties(v.input.properties as { [key: string]: ISchemaProperties }); v.input.properties = shortenProperties(properties); diff --git a/src/tools/dataset.ts b/src/tools/dataset.ts index 1c7bf36..cef33b4 100644 --- a/src/tools/dataset.ts +++ b/src/tools/dataset.ts @@ -9,11 +9,15 @@ import type { InternalTool, ToolEntry } from '../types.js'; const ajv = new Ajv({ coerceTypes: 'array', strict: false }); const getDatasetArgs = z.object({ - datasetId: z.string().describe('Dataset ID or username~dataset-name.'), + datasetId: z.string() + .min(1) + .describe('Dataset ID or username~dataset-name.'), }); const getDatasetItemsArgs = z.object({ - datasetId: z.string().describe('Dataset ID or username~dataset-name.'), + datasetId: z.string() + .min(1) + .describe('Dataset ID or username~dataset-name.'), clean: z.boolean().optional() .describe('If true, returns only non-empty items and skips hidden fields (starting with #). Shortcut for skipHidden=true and skipEmpty=true.'), offset: z.number().optional() @@ -54,6 +58,9 @@ export const getDataset: ToolEntry = { const parsed = getDatasetArgs.parse(args); const client = new ApifyClient({ token: apifyToken }); const v = await client.dataset(parsed.datasetId).get(); + if (!v) { + return { content: [{ type: 'text', text: `Dataset '${parsed.datasetId}' not found.` }] }; + } return { content: [{ type: 'text', text: JSON.stringify(v) }] }; }, } as InternalTool, @@ -98,6 +105,9 @@ export const getDatasetItems: ToolEntry = { desc: parsed.desc, flatten, }); + if (!v) { + return { content: [{ type: 'text', text: `Dataset '${parsed.datasetId}' not found.` }] }; + } return { content: [{ type: 'text', text: JSON.stringify(v) }] }; }, } as InternalTool, diff --git a/src/tools/helpers.ts b/src/tools/helpers.ts index fbbad2b..57e8718 100644 --- a/src/tools/helpers.ts +++ b/src/tools/helpers.ts @@ -62,6 +62,7 @@ In that case, the user should check the MCP client documentation to see if the c export const addToolArgsSchema = z.object({ actorName: z.string() + .min(1) .describe('Add a tool, Actor or MCP-Server to available tools by Actor ID or tool full name.' + 'Tool name is always composed from `username/name`'), }); @@ -79,6 +80,14 @@ export const addTool: ToolEntry = { call: async (toolArgs) => { const { apifyMcpServer, mcpServer, apifyToken, args } = toolArgs; const parsed = addToolArgsSchema.parse(args); + if (apifyMcpServer.listAllToolNames().includes(parsed.actorName)) { + return { + content: [{ + type: 'text', + text: `Actor ${parsed.actorName} is already available. No new tools were added.`, + }], + }; + } const tools = await getActorsAsTools([parsed.actorName], apifyToken); const toolsAdded = apifyMcpServer.upsertTools(tools, true); await mcpServer.notification({ method: 'notifications/tools/list_changed' }); @@ -98,6 +107,7 @@ export const addTool: ToolEntry = { }; export const removeToolArgsSchema = z.object({ toolName: z.string() + .min(1) .describe('Tool name to remove from available tools.') .transform((val) => actorNameToToolName(val)), }); @@ -112,8 +122,19 @@ export const removeTool: ToolEntry = { // TODO: I don't like that we are passing apifyMcpServer and mcpServer to the tool call: async (toolArgs) => { const { apifyMcpServer, mcpServer, args } = toolArgs; - const parsed = removeToolArgsSchema.parse(args); + // Check if tool exists before attempting removal + if (!apifyMcpServer.tools.has(parsed.toolName)) { + // Send notification so client can update its tool list + // just in case the client tool list is out of sync + await mcpServer.notification({ method: 'notifications/tools/list_changed' }); + return { + content: [{ + type: 'text', + text: `Tool '${parsed.toolName}' not found. No tools were removed.`, + }], + }; + } const removedTools = apifyMcpServer.removeToolsByName([parsed.toolName], true); await mcpServer.notification({ method: 'notifications/tools/list_changed' }); return { content: [{ type: 'text', text: `Tools removed: ${removedTools.join(', ')}` }] }; diff --git a/src/tools/key_value_store.ts b/src/tools/key_value_store.ts index 4caf248..9433089 100644 --- a/src/tools/key_value_store.ts +++ b/src/tools/key_value_store.ts @@ -10,6 +10,7 @@ const ajv = new Ajv({ coerceTypes: 'array', strict: false }); const getKeyValueStoreArgs = z.object({ storeId: z.string() + .min(1) .describe('Key-value store ID or username~store-name'), }); @@ -38,6 +39,7 @@ export const getKeyValueStore: ToolEntry = { const getKeyValueStoreKeysArgs = z.object({ storeId: z.string() + .min(1) .describe('Key-value store ID or username~store-name'), exclusiveStartKey: z.string() .optional() @@ -77,8 +79,10 @@ export const getKeyValueStoreKeys: ToolEntry = { const getKeyValueStoreRecordArgs = z.object({ storeId: z.string() + .min(1) .describe('Key-value store ID or username~store-name'), recordKey: z.string() + .min(1) .describe('Key of the record to retrieve.'), }); diff --git a/src/tools/run.ts b/src/tools/run.ts index c92a83b..6f3d580 100644 --- a/src/tools/run.ts +++ b/src/tools/run.ts @@ -9,11 +9,15 @@ import type { InternalTool, ToolEntry } from '../types.js'; const ajv = new Ajv({ coerceTypes: 'array', strict: false }); const getActorRunArgs = z.object({ - runId: z.string().describe('The ID of the Actor run.'), + runId: z.string() + .min(1) + .describe('The ID of the Actor run.'), }); const abortRunArgs = z.object({ - runId: z.string().describe('The ID of the Actor run to abort.'), + runId: z.string() + .min(1) + .describe('The ID of the Actor run to abort.'), gracefully: z.boolean().optional().describe('If true, the Actor run will abort gracefully with a 30-second timeout.'), }); @@ -35,6 +39,9 @@ export const getActorRun: ToolEntry = { const parsed = getActorRunArgs.parse(args); const client = new ApifyClient({ token: apifyToken }); const v = await client.run(parsed.runId).get(); + if (!v) { + return { content: [{ type: 'text', text: `Run with ID '${parsed.runId}' not found.` }] }; + } return { content: [{ type: 'text', text: JSON.stringify(v) }] }; }, } as InternalTool, diff --git a/tests/integration/actor.server-sse.test.ts b/tests/integration/actor.server-sse.test.ts index d84c1fe..c122051 100644 --- a/tests/integration/actor.server-sse.test.ts +++ b/tests/integration/actor.server-sse.test.ts @@ -21,7 +21,7 @@ createIntegrationTestsSuite({ getActorsMcpServer: () => mcpServer, createClientFn: async (options) => await createMcpSseClient(mcpUrl, options), beforeAllFn: async () => { - mcpServer = new ActorsMcpServer({ enableAddingActors: false }); + mcpServer = new ActorsMcpServer({ enableAddingActors: false, enableDefaultActors: false }); log.setLevel(log.LEVELS.OFF); // Create an express app using the proper server setup diff --git a/tests/integration/actor.server-streamable.test.ts b/tests/integration/actor.server-streamable.test.ts index 5fd417a..a42e230 100644 --- a/tests/integration/actor.server-streamable.test.ts +++ b/tests/integration/actor.server-streamable.test.ts @@ -23,7 +23,7 @@ createIntegrationTestsSuite({ beforeAllFn: async () => { log.setLevel(log.LEVELS.OFF); // Create an express app using the proper server setup - mcpServer = new ActorsMcpServer({ enableAddingActors: false }); + mcpServer = new ActorsMcpServer({ enableAddingActors: false, enableDefaultActors: false }); app = createExpressApp(httpServerHost, mcpServer); // Start a test server diff --git a/tests/integration/suite.ts b/tests/integration/suite.ts index a49c5ba..c1bf169 100644 --- a/tests/integration/suite.ts +++ b/tests/integration/suite.ts @@ -103,8 +103,7 @@ export function createIntegrationTestsSuite( await client.close(); }); - // TODO: This test is not working as there is a problem with server reset, which loads default Actors - it.runIf(false)('should list all default tools and two loaded Actors', async () => { + it('should list all default tools and two loaded Actors', async () => { const actors = ['apify/website-content-crawler', 'apify/instagram-scraper']; const client = await createClientFn({ actors, enableAddingActors: false }); const names = getToolNames(await client.listTools()); @@ -210,25 +209,27 @@ export function createIntegrationTestsSuite( }); it.runIf(getActorsMcpServer)('should reset and restore tool state with default tools', async () => { - const client = await createClientFn({ enableAddingActors: true }); + const firstClient = await createClientFn({ enableAddingActors: true }); const actorsMCPServer = getActorsMcpServer!(); const numberOfTools = defaultTools.length + addRemoveTools.length + defaults.actors.length; const toolList = actorsMCPServer.listAllToolNames(); expect(toolList.length).toEqual(numberOfTools); // Add a new Actor - await addActor(client, ACTOR_PYTHON_EXAMPLE); + await addActor(firstClient, ACTOR_PYTHON_EXAMPLE); // Store the tool name list const toolListWithActor = actorsMCPServer.listAllToolNames(); expect(toolListWithActor.length).toEqual(numberOfTools + 1); // + 1 for the added Actor + await firstClient.close(); // Remove all tools - // TODO: The reset functions sets the enableAddingActors to false, which is not expected - // await actorsMCPServer.reset(); - // const toolListAfterReset = actorsMCPServer.listAllToolNames(); - // expect(toolListAfterReset.length).toEqual(numberOfTools); - - await client.close(); + await actorsMCPServer.reset(); + // We connect second client so that the default tools are loaded + // if no specific list of Actors is provided + const secondClient = await createClientFn({ enableAddingActors: true }); + const toolListAfterReset = actorsMCPServer.listAllToolNames(); + expect(toolListAfterReset.length).toEqual(numberOfTools); + await secondClient.close(); }); it.runIf(getActorsMcpServer)('should notify tools changed handler on tool modifications', async () => {