Skip to content

Commit 60e20cf

Browse files
authored
fix: clone also the internal tools for skyfire mode (#308)
* fix: clone also the internal tools for skyfire mode * fix lint
1 parent e86923b commit 60e20cf

File tree

2 files changed

+45
-10
lines changed

2 files changed

+45
-10
lines changed

src/mcp/server.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import type { ActorMcpTool, ActorTool, HelperTool, ToolEntry } from '../types.js
4141
import { buildActorResponseContent } from '../utils/actor-response.js';
4242
import { buildMCPResponse } from '../utils/mcp.js';
4343
import { createProgressTracker } from '../utils/progress.js';
44-
import { getToolPublicFieldOnly } from '../utils/tools.js';
44+
import { cloneToolEntry, getToolPublicFieldOnly } from '../utils/tools.js';
4545
import { connectMCPClient } from './client.js';
4646
import { EXTERNAL_TOOL_CALL_TIMEOUT_MSEC, LOG_LEVEL_MAP } from './const.js';
4747
import { processParamsGetTools } from './utils.js';
@@ -262,31 +262,42 @@ export class ActorsMcpServer {
262262
* @returns Array of added/updated tool wrappers
263263
*/
264264
public upsertTools(tools: ToolEntry[], shouldNotifyToolsChangedHandler = false) {
265-
for (const wrap of tools) {
266-
this.tools.set(wrap.tool.name, wrap);
267-
}
268-
// Handle Skyfire mode modifications once per tool upsert
265+
// Handle Skyfire mode modifications before storing tools
269266
if (this.options.skyfireMode) {
270267
for (const wrap of tools) {
271268
if (wrap.type === 'actor'
272269
|| (wrap.type === 'internal' && wrap.tool.name === HelperTools.ACTOR_CALL)
273270
|| (wrap.type === 'internal' && wrap.tool.name === HelperTools.ACTOR_OUTPUT_GET)) {
271+
// Clone the tool before modifying it to avoid affecting shared objects
272+
const clonedWrap = cloneToolEntry(wrap);
273+
274274
// Add Skyfire instructions to description if not already present
275-
if (!wrap.tool.description.includes(SKYFIRE_TOOL_INSTRUCTIONS)) {
276-
wrap.tool.description += `\n\n${SKYFIRE_TOOL_INSTRUCTIONS}`;
275+
if (!clonedWrap.tool.description.includes(SKYFIRE_TOOL_INSTRUCTIONS)) {
276+
clonedWrap.tool.description += `\n\n${SKYFIRE_TOOL_INSTRUCTIONS}`;
277277
}
278278
// Add skyfire-pay-id property if not present
279-
if (wrap.tool.inputSchema && 'properties' in wrap.tool.inputSchema) {
280-
const props = wrap.tool.inputSchema.properties as Record<string, unknown>;
279+
if (clonedWrap.tool.inputSchema && 'properties' in clonedWrap.tool.inputSchema) {
280+
const props = clonedWrap.tool.inputSchema.properties as Record<string, unknown>;
281281
if (!props['skyfire-pay-id']) {
282282
props['skyfire-pay-id'] = {
283283
type: 'string',
284284
description: SKYFIRE_PAY_ID_PROPERTY_DESCRIPTION,
285285
};
286286
}
287287
}
288+
289+
// Store the cloned and modified tool
290+
this.tools.set(clonedWrap.tool.name, clonedWrap);
291+
} else {
292+
// Store unmodified tools as-is
293+
this.tools.set(wrap.tool.name, wrap);
288294
}
289295
}
296+
} else {
297+
// No skyfire mode - store tools as-is
298+
for (const wrap of tools) {
299+
this.tools.set(wrap.tool.name, wrap);
300+
}
290301
}
291302
if (shouldNotifyToolsChangedHandler) this.notifyToolsChangedHandler();
292303
return tools;

src/utils/tools.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { toolCategories } from '../tools/index.js';
2-
import type { ToolBase, ToolCategory, ToolEntry } from '../types.js';
2+
import type { InternalTool, ToolBase, ToolCategory, ToolEntry } from '../types.js';
33

44
/**
55
* Returns a public version of the tool containing only fields that should be exposed publicly.
@@ -27,3 +27,27 @@ export function getExpectedToolsByCategories(categories: ToolCategory[]): ToolEn
2727
export function getExpectedToolNamesByCategories(categories: ToolCategory[]): string[] {
2828
return getExpectedToolsByCategories(categories).map((tool) => tool.tool.name);
2929
}
30+
31+
/**
32+
* Creates a deep copy of a tool entry, preserving functions like ajvValidate and call
33+
* while cloning all other properties to avoid shared state mutations.
34+
*/
35+
export function cloneToolEntry(toolEntry: ToolEntry): ToolEntry {
36+
// Store the original functions
37+
const originalAjvValidate = toolEntry.tool.ajvValidate;
38+
const originalCall = toolEntry.type === 'internal' ? (toolEntry.tool as InternalTool).call : undefined;
39+
40+
// Create a deep copy using JSON serialization (excluding functions)
41+
const cloned = JSON.parse(JSON.stringify(toolEntry, (key, value) => {
42+
if (key === 'ajvValidate' || key === 'call') return undefined;
43+
return value;
44+
})) as ToolEntry;
45+
46+
// Restore the original functions
47+
cloned.tool.ajvValidate = originalAjvValidate;
48+
if (toolEntry.type === 'internal' && originalCall) {
49+
(cloned.tool as InternalTool).call = originalCall;
50+
}
51+
52+
return cloned;
53+
}

0 commit comments

Comments
 (0)