Skip to content

Commit 3dcff1e

Browse files
MQ37MQ37
committed
refactor: simplify tool types to flat discriminated union pattern
- Remove wrapper object pattern (HelperToolEntry, ActorToolEntry, ActorMcpToolEntry) - Implement flat discriminated union: ToolEntry = HelperTool | ActorTool | ActorMcpTool - Add direct 'type' discriminator to tool interfaces (no nested .tool property) - Introduce McpInputSchema type for strict MCP SDK schema alignment - Replace all 'as any' type casts with proper McpInputSchema assertions - Update 16 tool files to use flat structure pattern - Update server.ts and proxy.ts to remove .tool property access - Fix tools-loader.ts and tools.ts utility functions - Update test files to reference flat structure (.name instead of .tool.name) - Fix runtime error: serverUrl access in actor MCP tool handling This refactoring improves: - Type safety: Flat union eliminates property access ambiguity - Code clarity: Direct tool properties instead of nested .tool wrapper - MCP alignment: Proper inputSchema type matching MCP SDK requirements - No 'any' types: Complete type safety throughout codebase
1 parent 9e22895 commit 3dcff1e

22 files changed

+771
-861
lines changed

‎src/mcp/proxy.ts‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export async function getMCPServerTools(
1717
const compiledTools: ToolEntry[] = [];
1818
for (const tool of tools) {
1919
const mcpTool: ActorMcpTool = {
20+
type: 'actor-mcp',
2021
actorId: actorID,
2122
serverId: getMCPServerID(serverUrl),
2223
serverUrl,
@@ -28,12 +29,7 @@ export async function getMCPServerTools(
2829
ajvValidate: fixedAjvCompile(ajv, tool.inputSchema),
2930
};
3031

31-
const wrap: ToolEntry = {
32-
type: 'actor-mcp',
33-
tool: mcpTool,
34-
};
35-
36-
compiledTools.push(wrap);
32+
compiledTools.push(mcpTool);
3733
}
3834

3935
return compiledTools;

‎src/mcp/server.ts‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ export class ActorsMcpServer {
142142
private listInternalToolNames(): string[] {
143143
return Array.from(this.tools.values())
144144
.filter((tool) => tool.type === 'internal')
145-
.map((tool) => (tool.tool as HelperTool).name);
145+
.map((tool) => (tool as HelperTool).name);
146146
}
147147

148148
/**
@@ -152,7 +152,7 @@ export class ActorsMcpServer {
152152
public listActorToolNames(): string[] {
153153
return Array.from(this.tools.values())
154154
.filter((tool) => tool.type === 'actor')
155-
.map((tool) => (tool.tool as ActorTool).actorFullName);
155+
.map((tool) => (tool as ActorTool).actorFullName);
156156
}
157157

158158
/**
@@ -162,7 +162,7 @@ export class ActorsMcpServer {
162162
private listActorMcpServerToolIds(): string[] {
163163
const ids = Array.from(this.tools.values())
164164
.filter((tool: ToolEntry) => tool.type === 'actor-mcp')
165-
.map((tool: ToolEntry) => (tool.tool as ActorMcpTool).actorId);
165+
.map((tool: ToolEntry) => (tool as ActorMcpTool).actorId);
166166
// Ensure uniqueness
167167
return Array.from(new Set(ids));
168168
}
@@ -188,7 +188,7 @@ export class ActorsMcpServer {
188188
const internalToolMap = new Map([
189189
...defaultTools,
190190
...Object.values(toolCategories).flat(),
191-
].map((tool) => [tool.tool.name, tool]));
191+
].map((tool) => [tool.name, tool]));
192192

193193
for (const tool of toolNames) {
194194
// Skip if the tool is already loaded
@@ -266,20 +266,20 @@ export class ActorsMcpServer {
266266
if (this.options.skyfireMode) {
267267
for (const wrap of tools) {
268268
if (wrap.type === 'actor'
269-
|| (wrap.type === 'internal' && wrap.tool.name === HelperTools.ACTOR_CALL)
270-
|| (wrap.type === 'internal' && wrap.tool.name === HelperTools.ACTOR_OUTPUT_GET)) {
269+
|| (wrap.type === 'internal' && wrap.name === HelperTools.ACTOR_CALL)
270+
|| (wrap.type === 'internal' && wrap.name === HelperTools.ACTOR_OUTPUT_GET)) {
271271
// Clone the tool before modifying it to avoid affecting shared objects
272272
const clonedWrap = cloneToolEntry(wrap);
273273

274274
// Add Skyfire instructions to description if not already present
275-
if (clonedWrap.tool.description && !clonedWrap.tool.description.includes(SKYFIRE_TOOL_INSTRUCTIONS)) {
276-
clonedWrap.tool.description += `\n\n${SKYFIRE_TOOL_INSTRUCTIONS}`;
277-
} else if (!clonedWrap.tool.description) {
278-
clonedWrap.tool.description = SKYFIRE_TOOL_INSTRUCTIONS;
275+
if (clonedWrap.description && !clonedWrap.description.includes(SKYFIRE_TOOL_INSTRUCTIONS)) {
276+
clonedWrap.description += `\n\n${SKYFIRE_TOOL_INSTRUCTIONS}`;
277+
} else if (!clonedWrap.description) {
278+
clonedWrap.description = SKYFIRE_TOOL_INSTRUCTIONS;
279279
}
280280
// Add skyfire-pay-id property if not present
281-
if (clonedWrap.tool.inputSchema && 'properties' in clonedWrap.tool.inputSchema) {
282-
const props = clonedWrap.tool.inputSchema.properties as Record<string, unknown>;
281+
if (clonedWrap.inputSchema && 'properties' in clonedWrap.inputSchema) {
282+
const props = clonedWrap.inputSchema.properties as Record<string, unknown>;
283283
if (!props['skyfire-pay-id']) {
284284
props['skyfire-pay-id'] = {
285285
type: 'string',
@@ -289,16 +289,16 @@ export class ActorsMcpServer {
289289
}
290290

291291
// Store the cloned and modified tool
292-
this.tools.set(clonedWrap.tool.name, clonedWrap);
292+
this.tools.set(clonedWrap.name, clonedWrap);
293293
} else {
294294
// Store unmodified tools as-is
295-
this.tools.set(wrap.tool.name, wrap);
295+
this.tools.set(wrap.name, wrap);
296296
}
297297
}
298298
} else {
299299
// No skyfire mode - store tools as-is
300300
for (const wrap of tools) {
301-
this.tools.set(wrap.tool.name, wrap);
301+
this.tools.set(wrap.name, wrap);
302302
}
303303
}
304304
if (shouldNotifyToolsChangedHandler) this.notifyToolsChangedHandler();
@@ -458,7 +458,7 @@ export class ActorsMcpServer {
458458
* @returns {object} - The response object containing the tools.
459459
*/
460460
this.server.setRequestHandler(ListToolsRequestSchema, async () => {
461-
const tools = Array.from(this.tools.values()).map((tool) => getToolPublicFieldOnly(tool.tool));
461+
const tools = Array.from(this.tools.values()).map((tool) => getToolPublicFieldOnly(tool));
462462
return { tools };
463463
});
464464

@@ -504,7 +504,7 @@ export class ActorsMcpServer {
504504
// TODO - if connection is /mcp client will not receive notification on tool change
505505
// Find tool by name or actor full name
506506
const tool = Array.from(this.tools.values())
507-
.find((t) => t.tool.name === name || (t.type === 'actor' && (t.tool as ActorTool).actorFullName === name));
507+
.find((t) => t.name === name || (t.type === 'actor' && (t as ActorTool).actorFullName === name));
508508
if (!tool) {
509509
const msg = `Tool ${name} not found. Available tools: ${this.listToolNames().join(', ')}`;
510510
log.error(msg);
@@ -526,9 +526,9 @@ export class ActorsMcpServer {
526526
// Decode dot property names in arguments before validation,
527527
// since validation expects the original, non-encoded property names.
528528
args = decodeDotPropertyNames(args);
529-
log.debug('Validate arguments for tool', { toolName: tool.tool.name, input: args });
530-
if (!tool.tool.ajvValidate(args)) {
531-
const msg = `Invalid arguments for tool ${tool.tool.name}: args: ${JSON.stringify(args)} error: ${JSON.stringify(tool?.tool.ajvValidate.errors)}`;
529+
log.debug('Validate arguments for tool', { toolName: tool.name, input: args });
530+
if (!tool.ajvValidate(args)) {
531+
const msg = `Invalid arguments for tool ${tool.name}: args: ${JSON.stringify(args)} error: ${JSON.stringify(tool?.ajvValidate.errors)}`;
532532
log.error(msg);
533533
await this.server.sendLoggingMessage({ level: 'error', data: msg });
534534
throw new McpError(
@@ -540,7 +540,7 @@ export class ActorsMcpServer {
540540
try {
541541
// Handle internal tool
542542
if (tool.type === 'internal') {
543-
const internalTool = tool.tool as HelperTool;
543+
const internalTool = tool as HelperTool;
544544

545545
// Only create progress tracker for call-actor tool
546546
const progressTracker = internalTool.name === 'call-actor'
@@ -566,7 +566,7 @@ export class ActorsMcpServer {
566566
}
567567

568568
if (tool.type === 'actor-mcp') {
569-
const serverTool = tool.tool as ActorMcpTool;
569+
const serverTool = tool as ActorMcpTool;
570570
let client: Client | null = null;
571571
try {
572572
client = await connectMCPClient(serverTool.serverUrl, apifyToken);
@@ -627,7 +627,7 @@ export class ActorsMcpServer {
627627
};
628628
}
629629

630-
const actorTool = tool.tool as ActorTool;
630+
const actorTool = tool as ActorTool;
631631

632632
// Create progress tracker if progressToken is available
633633
const progressTracker = createProgressTracker(progressToken, extra.sendNotification);
@@ -700,8 +700,8 @@ export class ActorsMcpServer {
700700
}
701701
// Clear all tools and their compiled schemas
702702
for (const tool of this.tools.values()) {
703-
if (tool.tool.ajvValidate && typeof tool.tool.ajvValidate === 'function') {
704-
(tool.tool as { ajvValidate: ValidateFunction<unknown> | null }).ajvValidate = null;
703+
if (tool.ajvValidate && typeof tool.ajvValidate === 'function') {
704+
(tool as { ajvValidate: ValidateFunction<unknown> | null }).ajvValidate = null;
705705
}
706706
}
707707
this.tools.clear();

0 commit comments

Comments
 (0)