fix: exclude access_mcp_resource tool when MCP has no resources (#9615)

This commit is contained in:
Daniel 2025-11-26 15:18:15 -05:00 • committed by GitHub
parent a8a44510d5
commit 3cd3357808
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 324 additions and 11 deletions

View file

@ -170,7 +170,16 @@ const mockContext = {
// Instead of extending McpHub, create a mock that implements just what we need
const createMockMcpHub = (withServers: boolean = false): McpHub =>
({
getServers: () => (withServers ? [{ name: "test-server", disabled: false }] : []),
getServers: () =>
withServers
? [
{
name: "test-server",
disabled: false,
resources: [{ uri: "test://resource", name: "Test Resource" }],
},
]
: [],
getMcpServersPath: async () => "/mock/mcp/path",
getMcpSettingsFilePath: async () => "/mock/settings/path",
dispose: async () => {},

View file

@ -173,7 +173,16 @@ const mockContext = {
// Instead of extending McpHub, create a mock that implements just what we need
const createMockMcpHub = (withServers: boolean = false): McpHub =>
({
getServers: () => (withServers ? [{ name: "test-server", disabled: false }] : []),
getServers: () =>
withServers
? [
{
name: "test-server",
disabled: false,
resources: [{ uri: "test://resource", name: "Test Resource" }],
},
]
: [],
getMcpServersPath: async () => "/mock/mcp/path",
getMcpSettingsFilePath: async () => "/mock/settings/path",
dispose: async () => {},

View file

@ -0,0 +1,118 @@
import { getAccessMcpResourceDescription } from "../access-mcp-resource"
import { ToolArgs } from "../types"
import { McpHub } from "../../../../services/mcp/McpHub"
describe("getAccessMcpResourceDescription", () => {
const baseArgs: Omit<ToolArgs, "mcpHub"> = {
cwd: "/test",
supportsComputerUse: false,
}
it("should return undefined when mcpHub is not provided", () => {
const args: ToolArgs = {
...baseArgs,
mcpHub: undefined,
}
const result = getAccessMcpResourceDescription(args)
expect(result).toBeUndefined()
})
it("should return undefined when mcpHub has no servers with resources", () => {
const mockMcpHub = {
getServers: () => [
{
name: "test-server",
resources: [],
},
],
} as unknown as McpHub
const args: ToolArgs = {
...baseArgs,
mcpHub: mockMcpHub,
}
const result = getAccessMcpResourceDescription(args)
expect(result).toBeUndefined()
})
it("should return undefined when mcpHub has servers with undefined resources", () => {
const mockMcpHub = {
getServers: () => [
{
name: "test-server",
resources: undefined,
},
],
} as unknown as McpHub
const args: ToolArgs = {
...baseArgs,
mcpHub: mockMcpHub,
}
const result = getAccessMcpResourceDescription(args)
expect(result).toBeUndefined()
})
it("should return undefined when mcpHub has no servers", () => {
const mockMcpHub = {
getServers: () => [],
} as unknown as McpHub
const args: ToolArgs = {
...baseArgs,
mcpHub: mockMcpHub,
}
const result = getAccessMcpResourceDescription(args)
expect(result).toBeUndefined()
})
it("should return description when mcpHub has servers with resources", () => {
const mockMcpHub = {
getServers: () => [
{
name: "test-server",
resources: [{ uri: "test://resource", name: "Test Resource" }],
},
],
} as unknown as McpHub
const args: ToolArgs = {
...baseArgs,
mcpHub: mockMcpHub,
}
const result = getAccessMcpResourceDescription(args)
expect(result).toBeDefined()
expect(result).toContain("## access_mcp_resource")
expect(result).toContain("server_name")
expect(result).toContain("uri")
})
it("should return description when at least one server has resources", () => {
const mockMcpHub = {
getServers: () => [
{
name: "server-without-resources",
resources: [],
},
{
name: "server-with-resources",
resources: [{ uri: "test://resource", name: "Test Resource" }],
},
],
} as unknown as McpHub
const args: ToolArgs = {
...baseArgs,
mcpHub: mockMcpHub,
}
const result = getAccessMcpResourceDescription(args)
expect(result).toBeDefined()
expect(result).toContain("## access_mcp_resource")
})
})

View file

@ -71,7 +71,15 @@ describe("filterNativeToolsForMode", () => {
groups: ["read", "browser", "mcp"] as const,
}
const filtered = filterNativeToolsForMode(mockNativeTools, "architect", [architectMode], {}, undefined, {})
const filtered = filterNativeToolsForMode(
mockNativeTools,
"architect",
[architectMode],
{},
undefined,
{},
undefined,
)
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
@ -101,7 +109,7 @@ describe("filterNativeToolsForMode", () => {
groups: ["read", "edit", "browser", "command", "mcp"] as const,
}
const filtered = filterNativeToolsForMode(mockNativeTools, "code", [codeMode], {}, undefined, {})
const filtered = filterNativeToolsForMode(mockNativeTools, "code", [codeMode], {}, undefined, {}, undefined)
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
@ -123,7 +131,15 @@ describe("filterNativeToolsForMode", () => {
groups: [] as const, // No groups
}
const filtered = filterNativeToolsForMode(mockNativeTools, "restrictive", [restrictiveMode], {}, undefined, {})
const filtered = filterNativeToolsForMode(
mockNativeTools,
"restrictive",
[restrictiveMode],
{},
undefined,
{},
undefined,
)
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
@ -138,7 +154,7 @@ describe("filterNativeToolsForMode", () => {
})
it("should handle undefined mode by using default mode", () => {
const filtered = filterNativeToolsForMode(mockNativeTools, undefined, undefined, {}, undefined, {})
const filtered = filterNativeToolsForMode(mockNativeTools, undefined, undefined, {}, undefined, {}, undefined)
// Should return some tools (default mode is code which has all groups)
expect(filtered.length).toBeGreaterThan(0)
@ -168,11 +184,136 @@ describe("filterNativeToolsForMode", () => {
const toolsWithCodebaseSearch = [...mockNativeTools, mockCodebaseSearchTool]
// Without codeIndexManager
const filtered = filterNativeToolsForMode(toolsWithCodebaseSearch, "code", [codeMode], {}, undefined, {})
const filtered = filterNativeToolsForMode(
toolsWithCodebaseSearch,
"code",
[codeMode],
{},
undefined,
{},
undefined,
)
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
expect(toolNames).not.toContain("codebase_search")
})
it("should exclude access_mcp_resource when mcpHub is not provided", () => {
const codeMode: ModeConfig = {
slug: "code",
name: "Code",
roleDefinition: "Test",
groups: ["read", "edit", "browser", "command", "mcp"] as const,
}
const mockAccessMcpResourceTool: OpenAI.Chat.ChatCompletionTool = {
type: "function",
function: {
name: "access_mcp_resource",
description: "Access MCP resource",
parameters: {},
},
}
const toolsWithAccessMcpResource = [...mockNativeTools, mockAccessMcpResourceTool]
// Without mcpHub
const filtered = filterNativeToolsForMode(
toolsWithAccessMcpResource,
"code",
[codeMode],
{},
undefined,
{},
undefined,
)
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
expect(toolNames).not.toContain("access_mcp_resource")
})
it("should exclude access_mcp_resource when mcpHub has no resources", () => {
const codeMode: ModeConfig = {
slug: "code",
name: "Code",
roleDefinition: "Test",
groups: ["read", "edit", "browser", "command", "mcp"] as const,
}
const mockAccessMcpResourceTool: OpenAI.Chat.ChatCompletionTool = {
type: "function",
function: {
name: "access_mcp_resource",
description: "Access MCP resource",
parameters: {},
},
}
const toolsWithAccessMcpResource = [...mockNativeTools, mockAccessMcpResourceTool]
// Mock mcpHub with no resources
const mockMcpHub = {
getServers: () => [
{
name: "test-server",
resources: [],
},
],
} as any
const filtered = filterNativeToolsForMode(
toolsWithAccessMcpResource,
"code",
[codeMode],
{},
undefined,
{},
mockMcpHub,
)
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
expect(toolNames).not.toContain("access_mcp_resource")
})
it("should include access_mcp_resource when mcpHub has resources", () => {
const codeMode: ModeConfig = {
slug: "code",
name: "Code",
roleDefinition: "Test",
groups: ["read", "edit", "browser", "command", "mcp"] as const,
}
const mockAccessMcpResourceTool: OpenAI.Chat.ChatCompletionTool = {
type: "function",
function: {
name: "access_mcp_resource",
description: "Access MCP resource",
parameters: {},
},
}
const toolsWithAccessMcpResource = [...mockNativeTools, mockAccessMcpResourceTool]
// Mock mcpHub with resources
const mockMcpHub = {
getServers: () => [
{
name: "test-server",
resources: [{ uri: "test://resource", name: "Test Resource" }],
},
],
} as any
const filtered = filterNativeToolsForMode(
toolsWithAccessMcpResource,
"code",
[codeMode],
{},
undefined,
{},
mockMcpHub,
)
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
expect(toolNames).toContain("access_mcp_resource")
})
it("should exclude update_todo_list when todoListEnabled is false", () => {
const codeMode: ModeConfig = {
slug: "code",
@ -192,9 +333,17 @@ describe("filterNativeToolsForMode", () => {
const toolsWithTodo = [...mockNativeTools, mockTodoTool]
const filtered = filterNativeToolsForMode(toolsWithTodo, "code", [codeMode], {}, undefined, {
todoListEnabled: false,
})
const filtered = filterNativeToolsForMode(
toolsWithTodo,
"code",
[codeMode],
{},
undefined,
{
todoListEnabled: false,
},
undefined,
)
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
expect(toolNames).not.toContain("update_todo_list")
})
@ -225,6 +374,7 @@ describe("filterNativeToolsForMode", () => {
{ imageGeneration: false },
undefined,
{},
undefined,
)
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
expect(toolNames).not.toContain("generate_image")
@ -256,6 +406,7 @@ describe("filterNativeToolsForMode", () => {
{ runSlashCommand: false },
undefined,
{},
undefined,
)
const toolNames = filtered.map((t) => ("function" in t ? t.function.name : ""))
expect(toolNames).not.toContain("run_slash_command")

View file

@ -1,7 +1,16 @@
import { ToolArgs } from "./types"
import { McpHub } from "../../../services/mcp/McpHub"
/**
* Helper function to check if any MCP server has resources available
*/
function hasAnyMcpResources(mcpHub: McpHub): boolean {
const servers = mcpHub.getServers()
return servers.some((server) => server.resources && server.resources.length > 0)
}
export function getAccessMcpResourceDescription(args: ToolArgs): string | undefined {
if (!args.mcpHub) {
if (!args.mcpHub || !hasAnyMcpResources(args.mcpHub)) {
return undefined
}
return `## access_mcp_resource

View file

@ -4,6 +4,7 @@ import { getModeBySlug, getToolsForMode, isToolAllowedForMode } from "../../../s
import { TOOL_GROUPS, ALWAYS_AVAILABLE_TOOLS } from "../../../shared/tools"
import { defaultModeSlug } from "../../../shared/modes"
import type { CodeIndexManager } from "../../../services/code-index/manager"
import type { McpHub } from "../../../services/mcp/McpHub"
/**
* Filters native tools based on mode restrictions.
@ -15,6 +16,7 @@ import type { CodeIndexManager } from "../../../services/code-index/manager"
* @param experiments - Experiment flags
* @param codeIndexManager - Code index manager for codebase_search feature check
* @param settings - Additional settings for tool filtering
* @param mcpHub - MCP hub for checking available resources
* @returns Filtered array of tools allowed for the mode
*/
export function filterNativeToolsForMode(
@ -24,6 +26,7 @@ export function filterNativeToolsForMode(
experiments: Record<string, boolean> | undefined,
codeIndexManager?: CodeIndexManager,
settings?: Record<string, any>,
mcpHub?: McpHub,
): OpenAI.Chat.ChatCompletionTool[] {
// Get mode configuration and all tools for this mode
const modeSlug = mode ?? defaultModeSlug
@ -81,6 +84,11 @@ export function filterNativeToolsForMode(
allowedToolNames.delete("browser_action")
}
// Conditionally exclude access_mcp_resource if MCP is not enabled or there are no resources
if (!mcpHub || !hasAnyMcpResources(mcpHub)) {
allowedToolNames.delete("access_mcp_resource")
}
// Filter native tools based on allowed tool names
return nativeTools.filter((tool) => {
// Handle both ChatCompletionTool and ChatCompletionCustomTool
@ -91,6 +99,14 @@ export function filterNativeToolsForMode(
})
}
/**
* Helper function to check if any MCP server has resources available
*/
function hasAnyMcpResources(mcpHub: McpHub): boolean {
const servers = mcpHub.getServers()
return servers.some((server) => server.resources && server.resources.length > 0)
}
/**
* Checks if a specific tool is allowed in the current mode.
* This is useful for dynamically filtering system prompt content.

View file

@ -52,6 +52,7 @@ export async function buildNativeToolsArray(options: BuildToolsOptions): Promise
experiments,
codeIndexManager,
filterSettings,
mcpHub,
)
// Filter MCP tools based on mode restrictions