Skip to content

Commit 3cd3357

Browse files
daniel-lxsdaniel-lxs
authored
fix: exclude access_mcp_resource tool when MCP has no resources (#9615)
1 parent a8a4451 commit 3cd3357

File tree

7 files changed

+324
-11
lines changed

7 files changed

+324
-11
lines changed

‎src/core/prompts/__tests__/add-custom-instructions.spec.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,16 @@ const mockContext = {
170170
// Instead of extending McpHub, create a mock that implements just what we need
171171
const createMockMcpHub = (withServers: boolean = false): McpHub =>
172172
({
173-
getServers: () => (withServers ? [{ name: "test-server", disabled: false }] : []),
173+
getServers: () =>
174+
withServers
175+
? [
176+
{
177+
name: "test-server",
178+
disabled: false,
179+
resources: [{ uri: "test://resource", name: "Test Resource" }],
180+
},
181+
]
182+
: [],
174183
getMcpServersPath: async () => "/mock/mcp/path",
175184
getMcpSettingsFilePath: async () => "/mock/settings/path",
176185
dispose: async () => {},

‎src/core/prompts/__tests__/system-prompt.spec.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,16 @@ const mockContext = {
173173
// Instead of extending McpHub, create a mock that implements just what we need
174174
const createMockMcpHub = (withServers: boolean = false): McpHub =>
175175
({
176-
getServers: () => (withServers ? [{ name: "test-server", disabled: false }] : []),
176+
getServers: () =>
177+
withServers
178+
? [
179+
{
180+
name: "test-server",
181+
disabled: false,
182+
resources: [{ uri: "test://resource", name: "Test Resource" }],
183+
},
184+
]
185+
: [],
177186
getMcpServersPath: async () => "/mock/mcp/path",
178187
getMcpSettingsFilePath: async () => "/mock/settings/path",
179188
dispose: async () => {},

‎src/core/prompts/tools/__tests__/access-mcp-resource.spec.ts‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import { getAccessMcpResourceDescription } from "../access-mcp-resource"
2+
import { ToolArgs } from "../types"
3+
import { McpHub } from "../../../../services/mcp/McpHub"
4+
5+
describe("getAccessMcpResourceDescription", () => {
6+
const baseArgs: Omit<ToolArgs, "mcpHub"> = {
7+
cwd: "/test",
8+
supportsComputerUse: false,
9+
}
10+
11+
it("should return undefined when mcpHub is not provided", () => {
12+
const args: ToolArgs = {
13+
...baseArgs,
14+
mcpHub: undefined,
15+
}
16+
17+
const result = getAccessMcpResourceDescription(args)
18+
expect(result).toBeUndefined()
19+
})
20+
21+
it("should return undefined when mcpHub has no servers with resources", () => {
22+
const mockMcpHub = {
23+
getServers: () => [
24+
{
25+
name: "test-server",
26+
resources: [],
27+
},
28+
],
29+
} as unknown as McpHub
30+
31+
const args: ToolArgs = {
32+
...baseArgs,
33+
mcpHub: mockMcpHub,
34+
}
35+
36+
const result = getAccessMcpResourceDescription(args)
37+
expect(result).toBeUndefined()
38+
})
39+
40+
it("should return undefined when mcpHub has servers with undefined resources", () => {
41+
const mockMcpHub = {
42+
getServers: () => [
43+
{
44+
name: "test-server",
45+
resources: undefined,
46+
},
47+
],
48+
} as unknown as McpHub
49+
50+
const args: ToolArgs = {
51+
...baseArgs,
52+
mcpHub: mockMcpHub,
53+
}
54+
55+
const result = getAccessMcpResourceDescription(args)
56+
expect(result).toBeUndefined()
57+
})
58+
59+
it("should return undefined when mcpHub has no servers", () => {
60+
const mockMcpHub = {
61+
getServers: () => [],
62+
} as unknown as McpHub
63+
64+
const args: ToolArgs = {
65+
...baseArgs,
66+
mcpHub: mockMcpHub,
67+
}
68+
69+
const result = getAccessMcpResourceDescription(args)
70+
expect(result).toBeUndefined()
71+
})
72+
73+
it("should return description when mcpHub has servers with resources", () => {
74+
const mockMcpHub = {
75+
getServers: () => [
76+
{
77+
name: "test-server",
78+
resources: [{ uri: "test://resource", name: "Test Resource" }],
79+
},
80+
],
81+
} as unknown as McpHub
82+
83+
const args: ToolArgs = {
84+
...baseArgs,
85+
mcpHub: mockMcpHub,
86+
}
87+
88+
const result = getAccessMcpResourceDescription(args)
89+
expect(result).toBeDefined()
90+
expect(result).toContain("## access_mcp_resource")
91+
expect(result).toContain("server_name")
92+
expect(result).toContain("uri")
93+
})
94+
95+
it("should return description when at least one server has resources", () => {
96+
const mockMcpHub = {
97+
getServers: () => [
98+
{
99+
name: "server-without-resources",
100+
resources: [],
101+
},
102+
{
103+
name: "server-with-resources",
104+
resources: [{ uri: "test://resource", name: "Test Resource" }],
105+
},
106+
],
107+
} as unknown as McpHub
108+
109+
const args: ToolArgs = {
110+
...baseArgs,
111+
mcpHub: mockMcpHub,
112+
}
113+
114+
const result = getAccessMcpResourceDescription(args)
115+
expect(result).toBeDefined()
116+
expect(result).toContain("## access_mcp_resource")
117+
})
118+
})

‎src/core/prompts/tools/__tests__/filter-tools-for-mode.spec.ts‎

Lines changed: 159 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,15 @@ describe("filterNativeToolsForMode", () => {
7171
groups: ["read", "browser", "mcp"] as const,
7272
}
7373

74-
const filtered = filterNativeToolsForMode(mockNativeTools, "architect", [architectMode], {}, undefined, {})
74+
const filtered = filterNativeToolsForMode(
75+
mockNativeTools,
76+
"architect",
77+
[architectMode],
78+
{},
79+
undefined,
80+
{},
81+
undefined,
82+
)
7583

7684
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
7785

@@ -101,7 +109,7 @@ describe("filterNativeToolsForMode", () => {
101109
groups: ["read", "edit", "browser", "command", "mcp"] as const,
102110
}
103111

104-
const filtered = filterNativeToolsForMode(mockNativeTools, "code", [codeMode], {}, undefined, {})
112+
const filtered = filterNativeToolsForMode(mockNativeTools, "code", [codeMode], {}, undefined, {}, undefined)
105113

106114
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
107115

@@ -123,7 +131,15 @@ describe("filterNativeToolsForMode", () => {
123131
groups: [] as const, // No groups
124132
}
125133

126-
const filtered = filterNativeToolsForMode(mockNativeTools, "restrictive", [restrictiveMode], {}, undefined, {})
134+
const filtered = filterNativeToolsForMode(
135+
mockNativeTools,
136+
"restrictive",
137+
[restrictiveMode],
138+
{},
139+
undefined,
140+
{},
141+
undefined,
142+
)
127143

128144
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
129145

@@ -138,7 +154,7 @@ describe("filterNativeToolsForMode", () => {
138154
})
139155

140156
it("should handle undefined mode by using default mode", () => {
141-
const filtered = filterNativeToolsForMode(mockNativeTools, undefined, undefined, {}, undefined, {})
157+
const filtered = filterNativeToolsForMode(mockNativeTools, undefined, undefined, {}, undefined, {}, undefined)
142158

143159
// Should return some tools (default mode is code which has all groups)
144160
expect(filtered.length).toBeGreaterThan(0)
@@ -168,11 +184,136 @@ describe("filterNativeToolsForMode", () => {
168184
const toolsWithCodebaseSearch = [...mockNativeTools, mockCodebaseSearchTool]
169185

170186
// Without codeIndexManager
171-
const filtered = filterNativeToolsForMode(toolsWithCodebaseSearch, "code", [codeMode], {}, undefined, {})
187+
const filtered = filterNativeToolsForMode(
188+
toolsWithCodebaseSearch,
189+
"code",
190+
[codeMode],
191+
{},
192+
undefined,
193+
{},
194+
undefined,
195+
)
172196
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
173197
expect(toolNames).not.toContain("codebase_search")
174198
})
175199

200+
it("should exclude access_mcp_resource when mcpHub is not provided", () => {
201+
const codeMode: ModeConfig = {
202+
slug: "code",
203+
name: "Code",
204+
roleDefinition: "Test",
205+
groups: ["read", "edit", "browser", "command", "mcp"] as const,
206+
}
207+
208+
const mockAccessMcpResourceTool: OpenAI.Chat.ChatCompletionTool = {
209+
type: "function",
210+
function: {
211+
name: "access_mcp_resource",
212+
description: "Access MCP resource",
213+
parameters: {},
214+
},
215+
}
216+
217+
const toolsWithAccessMcpResource = [...mockNativeTools, mockAccessMcpResourceTool]
218+
219+
// Without mcpHub
220+
const filtered = filterNativeToolsForMode(
221+
toolsWithAccessMcpResource,
222+
"code",
223+
[codeMode],
224+
{},
225+
undefined,
226+
{},
227+
undefined,
228+
)
229+
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
230+
expect(toolNames).not.toContain("access_mcp_resource")
231+
})
232+
233+
it("should exclude access_mcp_resource when mcpHub has no resources", () => {
234+
const codeMode: ModeConfig = {
235+
slug: "code",
236+
name: "Code",
237+
roleDefinition: "Test",
238+
groups: ["read", "edit", "browser", "command", "mcp"] as const,
239+
}
240+
241+
const mockAccessMcpResourceTool: OpenAI.Chat.ChatCompletionTool = {
242+
type: "function",
243+
function: {
244+
name: "access_mcp_resource",
245+
description: "Access MCP resource",
246+
parameters: {},
247+
},
248+
}
249+
250+
const toolsWithAccessMcpResource = [...mockNativeTools, mockAccessMcpResourceTool]
251+
252+
// Mock mcpHub with no resources
253+
const mockMcpHub = {
254+
getServers: () => [
255+
{
256+
name: "test-server",
257+
resources: [],
258+
},
259+
],
260+
} as any
261+
262+
const filtered = filterNativeToolsForMode(
263+
toolsWithAccessMcpResource,
264+
"code",
265+
[codeMode],
266+
{},
267+
undefined,
268+
{},
269+
mockMcpHub,
270+
)
271+
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
272+
expect(toolNames).not.toContain("access_mcp_resource")
273+
})
274+
275+
it("should include access_mcp_resource when mcpHub has resources", () => {
276+
const codeMode: ModeConfig = {
277+
slug: "code",
278+
name: "Code",
279+
roleDefinition: "Test",
280+
groups: ["read", "edit", "browser", "command", "mcp"] as const,
281+
}
282+
283+
const mockAccessMcpResourceTool: OpenAI.Chat.ChatCompletionTool = {
284+
type: "function",
285+
function: {
286+
name: "access_mcp_resource",
287+
description: "Access MCP resource",
288+
parameters: {},
289+
},
290+
}
291+
292+
const toolsWithAccessMcpResource = [...mockNativeTools, mockAccessMcpResourceTool]
293+
294+
// Mock mcpHub with resources
295+
const mockMcpHub = {
296+
getServers: () => [
297+
{
298+
name: "test-server",
299+
resources: [{ uri: "test://resource", name: "Test Resource" }],
300+
},
301+
],
302+
} as any
303+
304+
const filtered = filterNativeToolsForMode(
305+
toolsWithAccessMcpResource,
306+
"code",
307+
[codeMode],
308+
{},
309+
undefined,
310+
{},
311+
mockMcpHub,
312+
)
313+
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
314+
expect(toolNames).toContain("access_mcp_resource")
315+
})
316+
176317
it("should exclude update_todo_list when todoListEnabled is false", () => {
177318
const codeMode: ModeConfig = {
178319
slug: "code",
@@ -192,9 +333,17 @@ describe("filterNativeToolsForMode", () => {
192333

193334
const toolsWithTodo = [...mockNativeTools, mockTodoTool]
194335

195-
const filtered = filterNativeToolsForMode(toolsWithTodo, "code", [codeMode], {}, undefined, {
196-
todoListEnabled: false,
197-
})
336+
const filtered = filterNativeToolsForMode(
337+
toolsWithTodo,
338+
"code",
339+
[codeMode],
340+
{},
341+
undefined,
342+
{
343+
todoListEnabled: false,
344+
},
345+
undefined,
346+
)
198347
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
199348
expect(toolNames).not.toContain("update_todo_list")
200349
})
@@ -225,6 +374,7 @@ describe("filterNativeToolsForMode", () => {
225374
{ imageGeneration: false },
226375
undefined,
227376
{},
377+
undefined,
228378
)
229379
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
230380
expect(toolNames).not.toContain("generate_image")
@@ -256,6 +406,7 @@ describe("filterNativeToolsForMode", () => {
256406
{ runSlashCommand: false },
257407
undefined,
258408
{},
409+
undefined,
259410
)
260411
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
261412
expect(toolNames).not.toContain("run_slash_command")

‎src/core/prompts/tools/access-mcp-resource.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,16 @@
11
import { ToolArgs } from "./types"
2+
import { McpHub } from "../../../services/mcp/McpHub"
3+
4+
/**
5+
* Helper function to check if any MCP server has resources available
6+
*/
7+
function hasAnyMcpResources(mcpHub: McpHub): boolean {
8+
const servers = mcpHub.getServers()
9+
return servers.some((server) => server.resources && server.resources.length > 0)
10+
}
211

312
export function getAccessMcpResourceDescription(args: ToolArgs): string | undefined {
4-
if (!args.mcpHub) {
13+
if (!args.mcpHub || !hasAnyMcpResources(args.mcpHub)) {
514
return undefined
615
}
716
return `## access_mcp_resource

0 commit comments

Comments
 (0)