Skip to content

Commit 662aba1

Browse files
committed
fix tests and logic to adhere to the new spec
1 parent f326b73 commit 662aba1

File tree

7 files changed

+85
-69
lines changed

7 files changed

+85
-69
lines changed

src/input.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ export function processInput(originalInput: Partial<Input>): Input {
3333
log.warning('enableActorAutoLoading is deprecated, use enableAddingActors instead');
3434
input.enableAddingActors = input.enableActorAutoLoading === true || input.enableActorAutoLoading === 'true';
3535
} else {
36-
input.enableAddingActors = true;
36+
// Default: do NOT enable add-actor unless explicitly requested
37+
input.enableAddingActors = false;
3738
}
3839
} else {
3940
input.enableAddingActors = input.enableAddingActors === true || input.enableAddingActors === 'true';
@@ -50,14 +51,20 @@ export function processInput(originalInput: Partial<Input>): Input {
5051
input.tools = [] as unknown as ToolSelector[];
5152
}
5253

53-
// Backward compatibility: if tools is explicitly specified, merge also actors into tools selectors
54-
// This keeps previous semantics when tools is undefined (defaults categories apply).
55-
if (input.tools !== undefined && Array.isArray(input.actors) && input.actors.length > 0) {
56-
let currentTools: ToolSelector[] = [];
57-
if (input.tools !== undefined) {
58-
currentTools = Array.isArray(input.tools) ? input.tools : [input.tools as ToolSelector];
54+
// Merge actors into tools selectors so that specifying only actors disables
55+
// default internal tools/categories. If tools are not provided, treat actors
56+
// as the only tool selectors. If tools are provided, append actors to tools.
57+
if (Array.isArray(input.actors) && input.actors.length > 0) {
58+
if (input.tools === undefined) {
59+
input.tools = [...input.actors] as ToolSelector[];
60+
// Treat as if only tools were specified; clear actors to avoid duplicate semantics
61+
input.actors = undefined as unknown as string[];
62+
} else {
63+
const currentTools: ToolSelector[] = Array.isArray(input.tools)
64+
? input.tools
65+
: [input.tools as ToolSelector];
66+
input.tools = [...currentTools, ...input.actors] as ToolSelector[];
5967
}
60-
input.tools = [...currentTools, ...input.actors] as ToolSelector[];
6168
}
6269
return input;
6370
}

src/stdio.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,12 @@ const argv = yargs(hideBin(process.argv))
5555
})
5656
.option('enable-adding-actors', {
5757
type: 'boolean',
58-
default: true,
58+
default: false,
5959
describe: 'Enable dynamically adding Actors as tools based on user requests.',
6060
})
6161
.option('enableActorAutoLoading', {
6262
type: 'boolean',
63-
default: true,
63+
default: false,
6464
hidden: true,
6565
describe: 'Deprecated: use enable-adding-actors instead.',
6666
})
@@ -91,7 +91,8 @@ Note: Tools that enable you to search Actors from the Apify Store and get their
9191
.epilogue('For more information, visit https://mcp.apify.com or https://github.com/apify/actors-mcp-server')
9292
.parseSync() as CliArgs;
9393

94-
const enableAddingActors = argv.enableAddingActors && argv.enableActorAutoLoading;
94+
// Respect either the new flag or the deprecated one
95+
const enableAddingActors = Boolean(argv.enableAddingActors || argv.enableActorAutoLoading);
9596
// Split actors argument, trim whitespace, and filter out empty strings
9697
const actorList = argv.actors !== undefined
9798
? argv.actors.split(',').map((a: string) => a.trim()).filter((a: string) => a.length > 0)

src/utils/tools-loader.ts

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,23 @@ export async function loadToolsFromInput(
7171
}
7272

7373
let actorNamesToLoad: string[] = [];
74+
const toolSelectorsProvided = toolSelectors !== undefined;
75+
const toolSelectorsIsEmpty = Array.isArray(toolSelectors) && toolSelectors.length === 0;
76+
const addActorEnabled = input.enableAddingActors === true;
7477
if (actorsFromInputField !== undefined) {
7578
actorNamesToLoad = actorsFromInputField;
7679
} else if (actorSelectorsFromTools.length > 0) {
7780
// If no explicit `actors` were provided, but `tools` includes actor names,
7881
// load exactly those instead of defaults
7982
actorNamesToLoad = actorSelectorsFromTools;
83+
} else if (!toolSelectorsProvided) {
84+
// Tools not provided
85+
// If add-actor is enabled and nothing else specified, do not load default actors
86+
actorNamesToLoad = addActorEnabled ? [] : defaults.actors;
8087
} else {
81-
// Use default actors if nothing specified anywhere
82-
actorNamesToLoad = defaults.actors;
88+
// Tools are provided (non-empty) but do not specify any actor names
89+
// => do not load default actors
90+
actorNamesToLoad = [];
8391
}
8492

8593
// If both fields specify actors, merge them
@@ -93,20 +101,34 @@ export async function loadToolsFromInput(
93101
tools = await getActorsAsTools(actorNamesToLoad, apifyToken);
94102
}
95103

96-
// Add tool for dynamically adding actors if enabled
97-
if (input.enableAddingActors) {
104+
// Add tool for dynamically adding actors if enabled.
105+
// Respect explicit empty tools array or explicitly empty actors list
106+
const actorsExplicitlyEmpty = (Array.isArray(input.actors) && input.actors.length === 0) || input.actors === '';
107+
if (addActorEnabled && !toolSelectorsIsEmpty && !actorsExplicitlyEmpty) {
98108
tools.push(addTool);
99109
}
100110

101111
// Add internal tools from categories/tool names or defaults when `tools` unspecified
102112
if (toolSelectors !== undefined) {
103-
// Respect disable flag: do not include add-actor even if explicitly requested
104-
const filteredInternal = input.enableAddingActors
113+
// Respect disable flag, but if 'experimental' category is explicitly requested,
114+
// or the add-actor tool is selected directly, include add-actor even when enableAddingActors=false
115+
const list = toolSelectors as (string | ToolCategory)[];
116+
const experimentalExplicitlySelected = Array.isArray(list)
117+
&& list.includes('experimental');
118+
const addActorSelectedDirectly = Array.isArray(list)
119+
&& list.includes(addTool.tool.name);
120+
const allowAddActor = addActorEnabled || experimentalExplicitlySelected || addActorSelectedDirectly;
121+
const filteredInternal = allowAddActor
105122
? internalCategoryEntries
106123
: internalCategoryEntries.filter((entry) => entry.tool.name !== addTool.tool.name);
107124
tools.push(...filteredInternal);
108125
} else {
109-
tools.push(...getExpectedToolsByCategories(toolCategoriesEnabledByDefault));
126+
// When tools are not provided:
127+
// - If add-actor is enabled: do not include default internal categories
128+
// - If actors are explicitly empty: do not include defaults either
129+
if (!addActorEnabled && !actorsExplicitlyEmpty) {
130+
tools.push(...getExpectedToolsByCategories(toolCategoriesEnabledByDefault));
131+
}
110132
}
111133

112134
// De-duplicate by tool name

tests/integration/internals.test.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,9 @@ describe('MCP server internals integration tests', () => {
3030

3131
// Store the tool name list
3232
const names = actorsMcpServer.listAllToolNames();
33+
// With enableAddingActors=true and no tools/actors, we should only have add-actor initially
3334
const expectedToolNames = [
34-
...defaults.actors,
3535
addTool.tool.name,
36-
...defaultTools.map((tool) => tool.tool.name),
3736
ACTOR_PYTHON_EXAMPLE,
3837
];
3938
expectArrayWeakEquals(expectedToolNames, names);
@@ -51,7 +50,8 @@ describe('MCP server internals integration tests', () => {
5150

5251
it('should notify tools changed handler on tool modifications', async () => {
5352
let latestTools: string[] = [];
54-
const numberOfTools = 1 + defaults.actors.length + defaultTools.length;
53+
// With enableAddingActors=true and no tools/actors, seeded set contains only add-actor
54+
const numberOfTools = 1;
5555

5656
let toolNotificationCount = 0;
5757
const onToolsChanged = (tools: string[]) => {
@@ -74,9 +74,7 @@ describe('MCP server internals integration tests', () => {
7474
expect(latestTools.length).toBe(numberOfTools + 1);
7575
expect(latestTools).toContain(actor);
7676
expect(latestTools).toContain(addTool.tool.name);
77-
for (const tool of defaults.actors) {
78-
expect(latestTools).toContain(tool);
79-
}
77+
// No default actors are present when only add-actor is enabled by default
8078

8179
// Remove the Actor
8280
actorsMCPServer.removeToolsByName([actorNameToToolName(actor)], true);
@@ -86,15 +84,13 @@ describe('MCP server internals integration tests', () => {
8684
expect(latestTools.length).toBe(numberOfTools);
8785
expect(latestTools).not.toContain(actor);
8886
expect(latestTools).toContain(addTool.tool.name);
89-
for (const tool of defaults.actors) {
90-
expect(latestTools).toContain(tool);
91-
}
87+
// No default actors are present by default in this mode
9288
});
9389

9490
it('should stop notifying after unregistering tools changed handler', async () => {
9591
let latestTools: string[] = [];
9692
let notificationCount = 0;
97-
const numberOfTools = 1 + defaults.actors.length + defaultTools.length;
93+
const numberOfTools = 1;
9894
const onToolsChanged = (tools: string[]) => {
9995
latestTools = tools;
10096
notificationCount++;

tests/integration/suite.ts

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,11 @@ export function createIntegrationTestsSuite(
9797
await client.close();
9898
});
9999

100-
it('should list all default tools and Actors, with add/remove tools', async () => {
100+
it('should list only add-actor when add/remove tools are enabled and no tools/actors specified', async () => {
101101
const client = await createClientFn({ enableAddingActors: true });
102102
const names = getToolNames(await client.listTools());
103-
expect(names.length).toEqual(defaultTools.length + defaults.actors.length + 1);
104-
105-
expectToolNamesToContain(names, DEFAULT_TOOL_NAMES);
106-
expectToolNamesToContain(names, DEFAULT_ACTOR_NAMES);
107-
expectToolNamesToContain(names, [addTool.tool.name]);
103+
expect(names.length).toEqual(1);
104+
expect(names).toContain(addTool.tool.name);
108105
await client.close();
109106
});
110107

@@ -157,11 +154,10 @@ export function createIntegrationTestsSuite(
157154
await client.close();
158155
});
159156

160-
it.only('should load only specified Actors via tools selectors when actors param omitted', async () => {
157+
it('should load only specified Actors via tools selectors when actors param omitted', async () => {
161158
const actors = ['apify/python-example'];
162159
const client = await createClientFn({ tools: actors });
163160
const names = getToolNames(await client.listTools());
164-
console.log(names);
165161
// Only the Actor should be loaded
166162
expect(names).toHaveLength(actors.length);
167163
expect(names).toContain(actorNameToToolName(actors[0]));
@@ -198,11 +194,10 @@ export function createIntegrationTestsSuite(
198194
await client.close();
199195
});
200196

201-
it('should not add any internal tools when tools param is empty', async () => {
197+
it('should not add any tools when tools param is empty and actors omitted', async () => {
202198
const client = await createClientFn({ tools: [] });
203199
const names = getToolNames(await client.listTools());
204-
expect(names.length).toEqual(defaults.actors.length);
205-
expectToolNamesToContain(names, defaults.actors.map((actor) => actorNameToToolName(actor)));
200+
expect(names.length).toEqual(0);
206201
await client.close();
207202
});
208203

@@ -453,7 +448,7 @@ export function createIntegrationTestsSuite(
453448
}
454449
});
455450

456-
it('should NOT include add-actor when disabled even if experimental category is selected', async () => {
451+
it('should include add-actor when experimental category is selected even if add/remove tools are disabled', async () => {
457452
const client = await createClientFn({
458453
enableAddingActors: false,
459454
tools: ['experimental'],
@@ -462,18 +457,12 @@ export function createIntegrationTestsSuite(
462457
const loadedTools = await client.listTools();
463458
const toolNames = getToolNames(loadedTools);
464459

465-
// Default actors still load when actors are omitted
466-
const expectedActorNames = defaults.actors.map((actor) => actorNameToToolName(actor));
467-
for (const n of expectedActorNames) {
468-
expect(toolNames).toContain(n);
469-
}
470-
// Must not include add-actor
471-
expect(toolNames).not.toContain(addTool.tool.name);
460+
expect(toolNames).toContain(addTool.tool.name);
472461

473462
await client.close();
474463
});
475464

476-
it('should NOT include add-actor when disabled even if tool is selected directly', async () => {
465+
it('should include add-actor when enableAddingActors disabled and tool add-actor selected directly', async () => {
477466
const client = await createClientFn({
478467
enableAddingActors: false,
479468
tools: [addTool.tool.name],
@@ -482,13 +471,8 @@ export function createIntegrationTestsSuite(
482471
const loadedTools = await client.listTools();
483472
const toolNames = getToolNames(loadedTools);
484473

485-
// Default actors still load when actors are omitted
486-
const expectedActorNames = defaults.actors.map((actor) => actorNameToToolName(actor));
487-
for (const n of expectedActorNames) {
488-
expect(toolNames).toContain(n);
489-
}
490-
// Must not include add-actor
491-
expect(toolNames).not.toContain(addTool.tool.name);
474+
// Must include add-actor since it was selected directly
475+
expect(toolNames).toContain(addTool.tool.name);
492476

493477
await client.close();
494478
});

tests/unit/input.test.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,22 @@ import { processInput } from '../../src/input.js';
44
import type { Input } from '../../src/types.js';
55

66
describe('processInput', () => {
7-
it('should handle string actors input and convert to array', async () => {
7+
it('should handle string actors input and convert to tools selectors', async () => {
88
const input: Partial<Input> = {
99
actors: 'actor1, actor2,actor3',
1010
};
1111
const processed = processInput(input);
12-
expect(processed.actors).toEqual(['actor1', 'actor2', 'actor3']);
12+
expect(processed.tools).toEqual(['actor1', 'actor2', 'actor3']);
13+
expect(processed.actors).toBeUndefined();
1314
});
1415

15-
it('should keep array actors input unchanged', async () => {
16+
it('should move array actors input into tools selectors', async () => {
1617
const input: Partial<Input> = {
1718
actors: ['actor1', 'actor2', 'actor3'],
1819
};
1920
const processed = processInput(input);
20-
expect(processed.actors).toEqual(['actor1', 'actor2', 'actor3']);
21+
expect(processed.tools).toEqual(['actor1', 'actor2', 'actor3']);
22+
expect(processed.actors).toBeUndefined();
2123
});
2224

2325
it('should handle enableActorAutoLoading to set enableAddingActors', async () => {
@@ -39,12 +41,12 @@ describe('processInput', () => {
3941
expect(processed.enableAddingActors).toBe(false);
4042
});
4143

42-
it('should default enableAddingActors to true when not provided', async () => {
44+
it('should default enableAddingActors to false when not provided', async () => {
4345
const input: Partial<Input> = {
4446
actors: ['actor1'],
4547
};
4648
const processed = processInput(input);
47-
expect(processed.enableAddingActors).toBe(true);
49+
expect(processed.enableAddingActors).toBe(false);
4850
});
4951

5052
it('should keep tools as array of valid featureTools keys', async () => {
@@ -63,12 +65,13 @@ describe('processInput', () => {
6365
expect(processed.tools).toEqual([]);
6466
});
6567

66-
it('should handle missing tools field (undefined)', async () => {
68+
it('should handle missing tools field (undefined) by moving actors into tools', async () => {
6769
const input: Partial<Input> = {
6870
actors: ['actor1'],
6971
};
7072
const processed = processInput(input);
71-
expect(processed.tools).toBeUndefined();
73+
expect(processed.tools).toEqual(['actor1']);
74+
expect(processed.actors).toBeUndefined();
7275
});
7376

7477
it('should include all keys, even invalid ones', async () => {

tests/unit/mcp.utils.test.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,18 @@ import { describe, expect, it } from 'vitest';
33
import { parseInputParamsFromUrl } from '../../src/mcp/utils.js';
44

55
describe('parseInputParamsFromUrl', () => {
6-
it('should parse Actors from URL query params', () => {
6+
it('should parse Actors from URL query params (as tools selectors)', () => {
77
const url = 'https://actors-mcp-server.apify.actor?token=123&actors=apify/web-scraper';
88
const result = parseInputParamsFromUrl(url);
9-
expect(result.actors).toEqual(['apify/web-scraper']);
9+
expect(result.tools).toEqual(['apify/web-scraper']);
10+
expect(result.actors).toBeUndefined();
1011
});
1112

12-
it('should parse multiple Actors from URL', () => {
13+
it('should parse multiple Actors from URL (as tools selectors)', () => {
1314
const url = 'https://actors-mcp-server.apify.actor?actors=apify/instagram-scraper,lukaskrivka/google-maps';
1415
const result = parseInputParamsFromUrl(url);
15-
expect(result.actors).toEqual(['apify/instagram-scraper', 'lukaskrivka/google-maps']);
16+
expect(result.tools).toEqual(['apify/instagram-scraper', 'lukaskrivka/google-maps']);
17+
expect(result.actors).toBeUndefined();
1618
});
1719

1820
it('should handle URL without query params', () => {
@@ -39,9 +41,10 @@ describe('parseInputParamsFromUrl', () => {
3941
expect(result.enableAddingActors).toBe(false);
4042
});
4143

42-
it('should handle Actors as string parameter', () => {
44+
it('should handle Actors as string parameter (as tools selectors)', () => {
4345
const url = 'https://actors-mcp-server.apify.actor?actors=apify/rag-web-browser';
4446
const result = parseInputParamsFromUrl(url);
45-
expect(result.actors).toEqual(['apify/rag-web-browser']);
47+
expect(result.tools).toEqual(['apify/rag-web-browser']);
48+
expect(result.actors).toBeUndefined();
4649
});
4750
});

0 commit comments

Comments
 (0)