Skip to content

Commit 3ea2ca1

Browse files
committed
fix: skyfire parameter injection and missing tool registration
Fixed critical bug where 8 out of 11 skyfire-enabled tools were missing the required `skyfire-pay-id` parameter in their input schemas. Refactored Skyfire integration to eliminate ~180 lines of duplicated code. Critical fixes: - Schema injection now uses SKYFIRE_ENABLED_TOOLS Set instead of hardcoded list - Added missing abortActorRun tool to runs category Changes: - Created src/utils/skyfire.ts with reusable helper functions - Refactored 13 code locations to use helpers - Added skyfire mode detection via ?payment=skyfire query parameter - Added integration test for schema injection - Updated test helpers and documentation Tools affected: call-actor, get-actor-output, get-actor-run, get-actor-log, abort-actor-run, get-dataset, get-dataset-items, get-dataset-schema, get-key-value-store, get-key-value-store-keys, get-key-value-store-record Files: 13 files changed, 286 insertions(+), 92 deletions(-)
1 parent a89e957 commit 3ea2ca1

13 files changed

Lines changed: 286 additions & 92 deletions

File tree

AGENTS.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ After completing ANY code change (feature, fix, refactor), you MUST:
114114

115115
- `tests/unit/` - Unit tests for individual modules
116116
- `tests/integration/` - Integration tests for MCP server functionality
117+
- `tests/integration/suite.ts` - **Main integration test suite** where all test cases should be added
118+
- Other files in this directory set up different transport modes (stdio, SSE, streamable-http) that all use suite.ts
117119
- `tests/helpers.ts` - Shared test utilities
118120
- `tests/const.ts` - Test constants
119121

@@ -124,6 +126,33 @@ After completing ANY code change (feature, fix, refactor), you MUST:
124126
- Follow existing test patterns in the codebase
125127
- Ensure all tests pass before submitting a PR
126128

129+
### Adding integration tests
130+
131+
**IMPORTANT**: When adding integration test cases, add them to `tests/integration/suite.ts`, NOT as separate test files.
132+
133+
The `suite.ts` file contains a test suite factory function `createIntegrationTestsSuite()` that is used by all transport modes (stdio, SSE, streamable-http). Adding tests here ensures they run across all transport types.
134+
135+
**How to add a test case:**
136+
1. Open `tests/integration/suite.ts`
137+
2. Add your test case inside the `describe` block (before the closing braces at the end)
138+
3. Use `it()` or `it.runIf()` for conditional tests
139+
4. Follow the existing patterns for client creation and assertions
140+
5. Use `client = await createClientFn(options)` to create the test client
141+
6. Always call `await client.close()` when done
142+
143+
**Example:**
144+
```typescript
145+
it('should do something awesome', async () => {
146+
client = await createClientFn({ tools: ['actors'] });
147+
const result = await client.callTool({
148+
name: HelperTools.SOME_TOOL,
149+
arguments: { /* ... */ },
150+
});
151+
expect(result.content).toBeDefined();
152+
await client.close();
153+
});
154+
```
155+
127156
### Manual testing as an MCP client
128157

129158
To test the MCP server, a human must first configure the MCP server.

src/actor/server.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@ export function createExpressApp(
9393
const uiModeParam = urlParams.get('ui') as UiMode | undefined;
9494
const uiMode = uiModeParam ?? process.env.UI_MODE as UiMode | undefined;
9595

96+
// Extract payment mode parameter - if payment=skyfire, enable skyfire mode
97+
const paymentParam = urlParams.get('payment');
98+
const skyfireMode = paymentParam === 'skyfire';
99+
96100
const mcpServer = new ActorsMcpServer({
97101
taskStore,
98102
setupSigintHandler: false,
@@ -101,6 +105,7 @@ export function createExpressApp(
101105
enabled: telemetryEnabled,
102106
},
103107
uiMode,
108+
skyfireMode,
104109
});
105110
const transport = new SSEServerTransport(Routes.MESSAGE, res);
106111

@@ -221,6 +226,10 @@ export function createExpressApp(
221226
const uiModeParam = urlParams.get('ui') as UiMode | undefined;
222227
const uiMode = uiModeParam ?? process.env.UI_MODE as UiMode | undefined;
223228

229+
// Extract payment mode parameter - if payment=skyfire, enable skyfire mode
230+
const paymentParam = urlParams.get('payment');
231+
const skyfireMode = paymentParam === 'skyfire';
232+
224233
const mcpServer = new ActorsMcpServer({
225234
taskStore,
226235
setupSigintHandler: false,
@@ -230,6 +239,7 @@ export function createExpressApp(
230239
enabled: telemetryEnabled,
231240
},
232241
uiMode,
242+
skyfireMode,
233243
});
234244

235245
// Load MCP server tools

src/const.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,24 @@ export const SKYFIRE_PAY_ID_PROPERTY_DESCRIPTION = `Skyfire PAY token ID used fo
7373

7474
export const SKYFIRE_README_CONTENT = `The Apify MCP Server allows clients to interact with Apify Actors, storage, and documentation. Before running any Apify Actor, you need to set up a Skyfire PAY token for billing purposes. To set up the PAY token you need to use the create-pay-token tool from the Skyfire MCP server. The PAY token must be charged with a minimum of ${SKYFIRE_MIN_CHARGE_USD} USD to be valid.${SKYFIRE_SELLER_ID ? ` The Skyfire seller ID for Apify MCP server is "${SKYFIRE_SELLER_ID}".` : ''} When running an Actor using the call-actor tool, you must provide the Skyfire PAY token ID in the \`skyfire-pay-id\` input property. Similarly, when retrieving Actor output using the get-actor-output tool, you must also provide the same Skyfire PAY token ID in the \`skyfire-pay-id\` input property.`;
7575

76+
/**
77+
* Set of internal tool names that require Skyfire PAY token ID in Skyfire mode.
78+
* These tools interact with Actor runs, datasets, or key-value stores and need billing support.
79+
*/
80+
export const SKYFIRE_ENABLED_TOOLS = new Set([
81+
HelperTools.ACTOR_CALL,
82+
HelperTools.ACTOR_OUTPUT_GET,
83+
HelperTools.ACTOR_RUNS_GET,
84+
HelperTools.ACTOR_RUNS_LOG,
85+
HelperTools.ACTOR_RUNS_ABORT,
86+
HelperTools.DATASET_GET,
87+
HelperTools.DATASET_GET_ITEMS,
88+
HelperTools.DATASET_SCHEMA_GET,
89+
HelperTools.KEY_VALUE_STORE_GET,
90+
HelperTools.KEY_VALUE_STORE_KEYS_GET,
91+
HelperTools.KEY_VALUE_STORE_RECORD_GET,
92+
]);
93+
7694
export const CALL_ACTOR_MCP_MISSING_TOOL_NAME_MSG = `When calling an MCP server Actor, you must specify the tool name in the actor parameter as "{actorName}:{toolName}" in the "actor" input property.`;
7795

7896
// Cache

src/mcp/server.ts

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import {
4444
SERVER_INSTRUCTIONS,
4545
SERVER_NAME,
4646
SERVER_VERSION,
47+
SKYFIRE_ENABLED_TOOLS,
4748
SKYFIRE_PAY_ID_PROPERTY_DESCRIPTION,
4849
SKYFIRE_README_CONTENT,
4950
SKYFIRE_TOOL_INSTRUCTIONS,
@@ -69,6 +70,7 @@ import { parseBooleanFromString } from '../utils/generic.js';
6970
import { logHttpError } from '../utils/logging.js';
7071
import { buildMCPResponse } from '../utils/mcp.js';
7172
import { createProgressTracker } from '../utils/progress.js';
73+
import { createApifyClientWithSkyfireSupport, validateSkyfirePayId } from '../utils/skyfire.js';
7274
import { getToolStatusFromError } from '../utils/tool-status.js';
7375
import { cloneToolEntry, getToolPublicFieldOnly } from '../utils/tools.js';
7476
import { getUserIdFromTokenCached } from '../utils/userid-cache.js';
@@ -346,8 +348,7 @@ export class ActorsMcpServer {
346348

347349
// Handle Skyfire mode modifications
348350
if (this.options.skyfireMode && (wrap.type === 'actor'
349-
|| (wrap.type === 'internal' && wrap.name === HelperTools.ACTOR_CALL)
350-
|| (wrap.type === 'internal' && wrap.name === HelperTools.ACTOR_OUTPUT_GET))) {
351+
|| (wrap.type === 'internal' && SKYFIRE_ENABLED_TOOLS.has(wrap.name as HelperTools)))) {
351352
// Add Skyfire instructions to description if not already present
352353
if (clonedWrap.description && !clonedWrap.description.includes(SKYFIRE_TOOL_INSTRUCTIONS)) {
353354
clonedWrap.description += `\n\n${SKYFIRE_TOOL_INSTRUCTIONS}`;
@@ -916,22 +917,16 @@ Please verify the server URL is correct and accessible, and ensure you have a va
916917

917918
// Handle actor tool
918919
if (tool.type === 'actor') {
919-
if (this.options.skyfireMode && args['skyfire-pay-id'] === undefined) {
920-
return buildMCPResponse({ texts: [SKYFIRE_TOOL_INSTRUCTIONS] });
921-
}
920+
const skyfireError = validateSkyfirePayId(this, args);
921+
if (skyfireError) return skyfireError;
922922

923923
// Create progress tracker if progressToken is available
924924
const progressTracker = createProgressTracker(progressToken, extra.sendNotification);
925925

926926
const callOptions: ActorCallOptions = { memory: tool.memoryMbytes };
927927

928-
/**
929-
* Create Apify token, for Skyfire mode use `skyfire-pay-id` and for normal mode use `apifyToken`.
930-
*/
931-
const { 'skyfire-pay-id': skyfirePayId, ...actorArgs } = args as Record<string, unknown>;
932-
const apifyClient = this.options.skyfireMode && typeof skyfirePayId === 'string'
933-
? new ApifyClient({ skyfirePayId })
934-
: new ApifyClient({ token: apifyToken });
928+
const { 'skyfire-pay-id': _skyfirePayId, ...actorArgs } = args as Record<string, unknown>;
929+
const apifyClient = createApifyClientWithSkyfireSupport(this, args, apifyToken);
935930

936931
try {
937932
log.info('Calling Actor', { actorName: tool.actorFullName, input: actorArgs });
@@ -1097,16 +1092,15 @@ Please verify the tool name and ensure the tool is properly registered.`;
10971092
let result: Record<string, unknown> = {};
10981093

10991094
// Handle actor tool
1100-
if (this.options.skyfireMode && args['skyfire-pay-id'] === undefined) {
1101-
result = buildMCPResponse({ texts: [SKYFIRE_TOOL_INSTRUCTIONS], isError: true });
1095+
const skyfireError = validateSkyfirePayId(this, args);
1096+
if (skyfireError) {
1097+
result = skyfireError;
11021098
toolStatus = TOOL_STATUS.SOFT_FAIL;
11031099
} else {
11041100
const progressTracker = createProgressTracker(progressToken, extra.sendNotification, taskId);
11051101
const callOptions: ActorCallOptions = { memory: tool.memoryMbytes };
1106-
const { 'skyfire-pay-id': skyfirePayId, ...actorArgs } = args as Record<string, unknown>;
1107-
const apifyClient = this.options.skyfireMode && typeof skyfirePayId === 'string'
1108-
? new ApifyClient({ skyfirePayId })
1109-
: new ApifyClient({ token: apifyToken });
1102+
const { 'skyfire-pay-id': _skyfirePayId, ...actorArgs } = args as Record<string, unknown>;
1103+
const apifyClient = createApifyClientWithSkyfireSupport(this, args, apifyToken);
11101104

11111105
log.info('Calling Actor for task', { taskId, actorName: tool.actorFullName, input: actorArgs });
11121106
const callResult = await callActorGetDataset(

src/tools/actor.ts

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
HelperTools,
1212
RAG_WEB_BROWSER,
1313
RAG_WEB_BROWSER_ADDITIONAL_DESC,
14-
SKYFIRE_TOOL_INSTRUCTIONS,
1514
TOOL_MAX_OUTPUT_CHARS,
1615
TOOL_STATUS,
1716
} from '../const.js';
@@ -28,6 +27,7 @@ import { buildMCPResponse } from '../utils/mcp.js';
2827
import type { ProgressTracker } from '../utils/progress.js';
2928
import type { JsonSchemaProperty } from '../utils/schema-generation.js';
3029
import { generateSchemaFromItems } from '../utils/schema-generation.js';
30+
import { createApifyClientWithSkyfireSupport, validateSkyfirePayId } from '../utils/skyfire.js';
3131
import { getWidgetConfig, WIDGET_URIS } from '../utils/widgets.js';
3232
import { getActorDefinition } from './build.js';
3333
import { actorNameToToolName, buildActorInputSchema, fixedAjvCompile, isActorInfoMcpServer } from './utils.js';
@@ -442,25 +442,10 @@ EXAMPLES:
442442
}
443443

444444
try {
445-
/**
446-
* In Skyfire mode, we check for the presence of `skyfire-pay-id`.
447-
* If it is missing, we return instructions to the LLM on how to create it and pass it to the tool.
448-
*/
449-
if (apifyMcpServer.options.skyfireMode && args['skyfire-pay-id'] === undefined) {
450-
return {
451-
content: [{
452-
type: 'text',
453-
text: SKYFIRE_TOOL_INSTRUCTIONS,
454-
}],
455-
};
456-
}
445+
const skyfireError = validateSkyfirePayId(apifyMcpServer, args);
446+
if (skyfireError) return skyfireError;
457447

458-
/**
459-
* Create Apify token, for Skyfire mode use `skyfire-pay-id` and for normal mode use `apifyToken`.
460-
*/
461-
const apifyClient = apifyMcpServer.options.skyfireMode && typeof args['skyfire-pay-id'] === 'string'
462-
? new ApifyClient({ skyfirePayId: args['skyfire-pay-id'] })
463-
: new ApifyClient({ token: apifyToken });
448+
const apifyClient = createApifyClientWithSkyfireSupport(apifyMcpServer, args, apifyToken);
464449

465450
// Determine execution mode: always async when UI mode is enabled, otherwise respect the parameter
466451
const isAsync = apifyMcpServer.options.uiMode === 'openai'

src/tools/categories.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { getHtmlSkeleton } from './get-html-skeleton.js';
1313
import { addTool } from './helpers.js';
1414
import { getKeyValueStore, getKeyValueStoreKeys, getKeyValueStoreRecord } from './key_value_store.js';
1515
import { getUserKeyValueStoresList } from './key_value_store_collection.js';
16-
import { getActorRun, getActorRunLog } from './run.js';
16+
import { abortActorRun, getActorRun, getActorRunLog } from './run.js';
1717
import { getUserRunsList } from './run_collection.js';
1818
import { searchApifyDocsTool } from './search-apify-docs.js';
1919
import { searchActors } from './store_collection.js';
@@ -35,6 +35,7 @@ export const toolCategories = {
3535
getActorRun,
3636
getUserRunsList,
3737
getActorRunLog,
38+
abortActorRun,
3839
],
3940
storage: [
4041
getDataset,

src/tools/dataset.ts

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { z } from 'zod';
22

3-
import { ApifyClient } from '../apify-client.js';
43
import { HelperTools, TOOL_STATUS } from '../const.js';
54
import type { InternalToolArgs, ToolEntry, ToolInputSchema } from '../types.js';
65
import { compileSchema } from '../utils/ajv.js';
76
import { parseCommaSeparatedList } from '../utils/generic.js';
87
import { buildMCPResponse } from '../utils/mcp.js';
98
import { generateSchemaFromItems } from '../utils/schema-generation.js';
9+
import { createApifyClientWithSkyfireSupport, validateSkyfirePayId } from '../utils/skyfire.js';
1010

1111
const getDatasetArgs = z.object({
1212
datasetId: z.string()
@@ -56,17 +56,24 @@ USAGE EXAMPLES:
5656
- user_input: Show info for dataset xyz123
5757
- user_input: What fields does username~my-dataset have?`,
5858
inputSchema: z.toJSONSchema(getDatasetArgs) as ToolInputSchema,
59-
ajvValidate: compileSchema(z.toJSONSchema(getDatasetArgs)),
59+
/**
60+
* Allow additional properties for Skyfire mode to pass `skyfire-pay-id`.
61+
*/
62+
ajvValidate: compileSchema({ ...z.toJSONSchema(getDatasetArgs), additionalProperties: true }),
6063
annotations: {
6164
title: 'Get dataset',
6265
readOnlyHint: true,
6366
idempotentHint: true,
6467
openWorldHint: false,
6568
},
6669
call: async (toolArgs: InternalToolArgs) => {
67-
const { args, apifyToken } = toolArgs;
70+
const { args, apifyToken, apifyMcpServer } = toolArgs;
6871
const parsed = getDatasetArgs.parse(args);
69-
const client = new ApifyClient({ token: apifyToken });
72+
73+
const skyfireError = validateSkyfirePayId(apifyMcpServer, args);
74+
if (skyfireError) return skyfireError;
75+
76+
const client = createApifyClientWithSkyfireSupport(apifyMcpServer, args, apifyToken);
7077
const v = await client.dataset(parsed.datasetId).get();
7178
if (!v) {
7279
return buildMCPResponse({
@@ -98,17 +105,24 @@ USAGE EXAMPLES:
98105
- user_input: Get first 100 items from dataset abd123
99106
- user_input: Get only metadata.url and title from dataset username~my-dataset (flatten metadata)`,
100107
inputSchema: z.toJSONSchema(getDatasetItemsArgs) as ToolInputSchema,
101-
ajvValidate: compileSchema(z.toJSONSchema(getDatasetItemsArgs)),
108+
/**
109+
* Allow additional properties for Skyfire mode to pass `skyfire-pay-id`.
110+
*/
111+
ajvValidate: compileSchema({ ...z.toJSONSchema(getDatasetItemsArgs), additionalProperties: true }),
102112
annotations: {
103113
title: 'Get dataset items',
104114
readOnlyHint: true,
105115
idempotentHint: true,
106116
openWorldHint: false,
107117
},
108118
call: async (toolArgs: InternalToolArgs) => {
109-
const { args, apifyToken } = toolArgs;
119+
const { args, apifyToken, apifyMcpServer } = toolArgs;
110120
const parsed = getDatasetItemsArgs.parse(args);
111-
const client = new ApifyClient({ token: apifyToken });
121+
122+
const skyfireError = validateSkyfirePayId(apifyMcpServer, args);
123+
if (skyfireError) return skyfireError;
124+
125+
const client = createApifyClientWithSkyfireSupport(apifyMcpServer, args, apifyToken);
112126

113127
// Convert comma-separated strings to arrays
114128
const fields = parseCommaSeparatedList(parsed.fields);
@@ -167,17 +181,24 @@ USAGE EXAMPLES:
167181
- user_input: Generate schema for dataset 34das2 using 10 items
168182
- user_input: Show schema of username~my-dataset (clean items only)`,
169183
inputSchema: z.toJSONSchema(getDatasetSchemaArgs) as ToolInputSchema,
170-
ajvValidate: compileSchema(z.toJSONSchema(getDatasetSchemaArgs)),
184+
/**
185+
* Allow additional properties for Skyfire mode to pass `skyfire-pay-id`.
186+
*/
187+
ajvValidate: compileSchema({ ...z.toJSONSchema(getDatasetSchemaArgs), additionalProperties: true }),
171188
annotations: {
172189
title: 'Get dataset schema',
173190
readOnlyHint: true,
174191
idempotentHint: true,
175192
openWorldHint: false,
176193
},
177194
call: async (toolArgs: InternalToolArgs) => {
178-
const { args, apifyToken } = toolArgs;
195+
const { args, apifyToken, apifyMcpServer } = toolArgs;
179196
const parsed = getDatasetSchemaArgs.parse(args);
180-
const client = new ApifyClient({ token: apifyToken });
197+
198+
const skyfireError = validateSkyfirePayId(apifyMcpServer, args);
199+
if (skyfireError) return skyfireError;
200+
201+
const client = createApifyClientWithSkyfireSupport(apifyMcpServer, args, apifyToken);
181202

182203
// Get dataset items
183204
const datasetResponse = await client.dataset(parsed.datasetId).listItems({

src/tools/get-actor-output.ts

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { z } from 'zod';
22

3-
import { ApifyClient } from '../apify-client.js';
4-
import { HelperTools, SKYFIRE_TOOL_INSTRUCTIONS, TOOL_MAX_OUTPUT_CHARS, TOOL_STATUS } from '../const.js';
3+
import { HelperTools, TOOL_MAX_OUTPUT_CHARS, TOOL_STATUS } from '../const.js';
54
import type { InternalToolArgs, ToolEntry, ToolInputSchema } from '../types.js';
65
import { compileSchema } from '../utils/ajv.js';
76
import { getValuesByDotKeys, parseCommaSeparatedList } from '../utils/generic.js';
87
import { buildMCPResponse } from '../utils/mcp.js';
8+
import { createApifyClientWithSkyfireSupport, validateSkyfirePayId } from '../utils/skyfire.js';
99

1010
/**
1111
* Zod schema for get-actor-output tool arguments
@@ -97,27 +97,10 @@ Note: This tool is automatically included if the Apify MCP Server is configured
9797
call: async (toolArgs: InternalToolArgs) => {
9898
const { args, apifyToken, apifyMcpServer } = toolArgs;
9999

100-
/**
101-
* In Skyfire mode, we check for the presence of `skyfire-pay-id`.
102-
* If it is missing, we return instructions to the LLM on how to create it and pass it to the tool.
103-
*/
104-
if (apifyMcpServer.options.skyfireMode
105-
&& args['skyfire-pay-id'] === undefined
106-
) {
107-
return {
108-
content: [{
109-
type: 'text',
110-
text: SKYFIRE_TOOL_INSTRUCTIONS,
111-
}],
112-
};
113-
}
100+
const skyfireError = validateSkyfirePayId(apifyMcpServer, args);
101+
if (skyfireError) return skyfireError;
114102

115-
/**
116-
* Create Apify token, for Skyfire mode use `skyfire-pay-id` and for normal mode use `apifyToken`.
117-
*/
118-
const apifyClient = apifyMcpServer.options.skyfireMode && typeof args['skyfire-pay-id'] === 'string'
119-
? new ApifyClient({ skyfirePayId: args['skyfire-pay-id'] })
120-
: new ApifyClient({ token: apifyToken });
103+
const apifyClient = createApifyClientWithSkyfireSupport(apifyMcpServer, args, apifyToken);
121104
const parsed = getActorOutputArgs.parse(args);
122105

123106
// Parse fields into array

0 commit comments

Comments
 (0)