Skip to content

Commit bc8aebd

Browse files
MQ37Copilot
andauthored
fix: skyfire parameter injection and missing tool registration (#393)
* 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(-) * Centralize skyfire pay ID validation in tool dispatcher (#395) * Initial plan * Centralize skyfire validation in MCP server dispatcher Co-authored-by: MQ37 <29043708+MQ37@users.noreply.github.com> * fix: enable task mode support for internal tools with taskSupport Previously, task mode execution (executeToolAndUpdateTask) rejected all internal tools, only allowing actor tools. This was inconsistent with tools like call-actor that declare taskSupport: 'optional'. Changes: - Remove hard rejection of internal tools in task mode - Add execution path for internal tools in task mode - Pass userRentedActorIds to task execution context - Add integration test for call-actor in task mode This fixes the architectural issue identified in PR #396 review where centralized skyfire validation worked but internal tools couldn't reach that code path due to early rejection. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MQ37 <29043708+MQ37@users.noreply.github.com> Co-authored-by: MQ37 <themq37@gmail.com> * refactor: move createApifyClientWithSkyfireSupport to apify-clients.ts * fix compiler and lint issues after merge --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: MQ37 <29043708+MQ37@users.noreply.github.com>
1 parent e69f74f commit bc8aebd

16 files changed

Lines changed: 376 additions & 133 deletions

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.

package-lock.json

Lines changed: 0 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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/apify-client.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { ApifyClient as _ApifyClient } from 'apify-client';
33
import type { AxiosRequestConfig } from 'axios';
44

55
import { USER_AGENT_ORIGIN } from './const.js';
6+
import type { ActorsMcpServer } from './mcp/server.js';
7+
import type { ApifyToken } from './types.js';
68

79
type ExtendedApifyClientOptions = Omit<ApifyClientOptions, 'token'> & {
810
token?: string | null | undefined;
@@ -62,3 +64,22 @@ export class ApifyClient extends _ApifyClient {
6264
});
6365
}
6466
}
67+
68+
/**
69+
* Creates ApifyClient with appropriate credentials based on Skyfire mode.
70+
* In Skyfire mode, uses skyfire-pay-id from args; otherwise uses apifyToken.
71+
*
72+
* @param apifyMcpServer - The MCP server instance with configuration options
73+
* @param args - Tool arguments that may contain skyfire-pay-id
74+
* @param apifyToken - Standard Apify token for non-Skyfire mode
75+
* @returns ApifyClient instance configured for the appropriate mode
76+
*/
77+
export function createApifyClientWithSkyfireSupport(
78+
apifyMcpServer: ActorsMcpServer,
79+
args: Record<string, unknown>,
80+
apifyToken: ApifyToken,
81+
): ApifyClient {
82+
return apifyMcpServer.options.skyfireMode && typeof args['skyfire-pay-id'] === 'string'
83+
? new ApifyClient({ skyfirePayId: args['skyfire-pay-id'] })
84+
: new ApifyClient({ token: apifyToken });
85+
}

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

0 commit comments

Comments
 (0)