From cc5d7d5ad04b75cfa7405fbcc22199d0ff72a79d Mon Sep 17 00:00:00 2001 From: matevip Date: Tue, 12 May 2026 21:24:36 +0800 Subject: [PATCH] fix(agent): auto-include enabled MCP tools in effective allowlist (#108) --- .../binding/service/AgentBindingService.java | 45 +++++++++++++++++ .../binding/AgentBindingServiceTest.java | 48 +++++++++++++++++++ mateclaw-ui/src/i18n/locales/en-US.ts | 4 +- mateclaw-ui/src/i18n/locales/zh-CN.ts | 4 +- mateclaw-ui/src/views/Agents.vue | 30 +++++++++++- 5 files changed, 128 insertions(+), 3 deletions(-) diff --git a/mateclaw-server/src/main/java/vip/mate/agent/binding/service/AgentBindingService.java b/mateclaw-server/src/main/java/vip/mate/agent/binding/service/AgentBindingService.java index 22095376..ea5391b4 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/binding/service/AgentBindingService.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/binding/service/AgentBindingService.java @@ -301,6 +301,19 @@ public class AgentBindingService { * → contribute nothing through this path; legacy SKILL.md prompt * enhancement still runs separately. * + * + *

Auto-included on every non-null result, in addition to the bound + * tools and skill-expanded tools: + *

*/ public Set getEffectiveToolNames(Long agentId) { Set boundSkillIds = getBoundSkillIds(agentId); @@ -342,9 +355,41 @@ public class AgentBindingService { // (the LLM stops being able to write to LESSONS.md / MEMORY.md). merged.addAll(SYSTEM_LEVEL_TOOLS); + // Enabled MCP server tools auto-join the allowlist for the same + // reason SYSTEM_LEVEL_TOOLS does: MCP servers are an + // administrator-enabled capability, not a per-agent opt-in. Without + // this union, an agent with any skill or built-in tool bound would + // silently lose every MCP tool — users hit this when they bound one + // built-in tool, didn't tick the MCP rows, and observed "only + // built-in tools work". Operators who need to hide a specific MCP + // tool from a specific agent still have the tool-guard deny path + // (AgentGraphBuilder applies withDeniedToolsFiltered before this). + merged.addAll(getEnabledMcpToolNames()); + return merged; } + /** + * Names of every currently-bindable MCP tool, sourced from the same + * picker that the agent edit screen reads. Failures (picker outage, + * cache parse error) yield an empty set so the caller's allowlist is + * strictly narrower, never wider, than the picker — never throws. + */ + private Set getEnabledMcpToolNames() { + try { + return availableToolService.listAvailable().stream() + .filter(t -> "mcp".equals(t.getSource())) + .filter(AvailableToolDTO::isAvailable) + .map(AvailableToolDTO::getName) + .filter(n -> n != null && !n.isBlank()) + .collect(Collectors.toCollection(LinkedHashSet::new)); + } catch (Exception e) { + log.warn("AvailableToolService unavailable while computing effective tool allowlist; " + + "MCP tools will be excluded for this resolve cycle: {}", e.getMessage()); + return Collections.emptySet(); + } + } + /** * RFC-090 §11 — tools that exist outside the skill scope and must * survive any agent-level skill binding restriction. diff --git a/mateclaw-server/src/test/java/vip/mate/agent/binding/AgentBindingServiceTest.java b/mateclaw-server/src/test/java/vip/mate/agent/binding/AgentBindingServiceTest.java index 2a47afb5..d5136b1f 100644 --- a/mateclaw-server/src/test/java/vip/mate/agent/binding/AgentBindingServiceTest.java +++ b/mateclaw-server/src/test/java/vip/mate/agent/binding/AgentBindingServiceTest.java @@ -286,6 +286,54 @@ class AgentBindingServiceTest { "validation 必须在 delete 旧绑定之前完成，否则会留下空绑定状态"); } + /** + * Seed a connected MCP server with one cached tool so + * {@link vip.mate.tool.service.AvailableToolService#listAvailable()} + * returns at least one bindable MCP row. + */ + private void seedMcpServerWithOneTool(long id, String serverName, String rawToolName) { + String toolsCacheJson = "[{\"name\":\"" + rawToolName + "\",\"description\":\"fixture\"}]"; + jdbcTemplate.update( + "MERGE INTO mate_mcp_server (id, name, description, transport, enabled, " + + "connect_timeout_seconds, read_timeout_seconds, last_status, tool_count, " + + "builtin, tools_cache_json, create_time, update_time, deleted) " + + "KEY(id) VALUES (?, ?, '', 'stdio', TRUE, 30, 30, 'connected', 1, FALSE, ?, " + + "CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 0)", + id, serverName, toolsCacheJson); + } + + @Test + @DisplayName("Issue #108: 绑定任意 builtin tool 后，enabled MCP 工具仍自动出现在 effective allowlist") + void mcpToolsAutoIncludedWhenAnyBindingExists() { + // Reproduce the user-reported scenario: agent has one built-in tool + // bound (e.g. by template), no MCP tools ticked. Before the fix this + // returned a whitelist that excluded every MCP tool; after the fix + // MCP tools auto-join the allowlist. + seedBuiltinTool("builtin_probe"); + seedMcpServerWithOneTool(8_888_001L, "issue108-server", "search_web"); + bindingService.setToolBindings(agentId, List.of("builtin_probe")); + + Set effective = bindingService.getEffectiveToolNames(agentId); + assertNotNull(effective, "binding 非空时应返回 allowlist（非 null）"); + assertTrue(effective.contains("builtin_probe"), "用户显式勾选的工具必须在 allowlist 中"); + boolean hasMcpEntry = effective.stream().anyMatch(n -> n != null && n.startsWith("mcp_")); + assertTrue(hasMcpEntry, + "enabled MCP server 的工具必须自动并入 allowlist；缺失会让用户在 chat 时只见到 built-in 工具，" + + "即 issue #108 描述的现象。实际 allowlist: " + effective); + } + + @Test + @DisplayName("Issue #108: agent 完全没绑定时 effective allowlist 返回 null（不要意外改成 strict）") + void noBindingsStillReturnsNull() { + // The auto-union must not flip the three-state contract: an agent + // with zero bindings still means "no agent-level restriction". + seedMcpServerWithOneTool(8_888_002L, "issue108-no-binding-server", "search_web"); + + Set effective = bindingService.getEffectiveToolNames(agentId); + assertNull(effective, "完全没有 skill / tool 绑定时必须返回 null（= 不过滤），" + + "否则 AgentToolSet.withAllowedToolsOnly 会变成空集禁掉所有工具"); + } + @Test @DisplayName("unbindTool 后 DB 里真的没行（物理 delete，不是软删留 deleted=1）") void unbindPhysicallyRemovesRow() { diff --git a/mateclaw-ui/src/i18n/locales/en-US.ts b/mateclaw-ui/src/i18n/locales/en-US.ts index 4a20e2ed..46082e1a 100644 --- a/mateclaw-ui/src/i18n/locales/en-US.ts +++ b/mateclaw-ui/src/i18n/locales/en-US.ts @@ -1131,7 +1131,9 @@ export default { searchTools: 'Search tool name, description, source, or group', advancedToolsTitle: 'Advanced: Hand-picked atomic tools', advancedToolsHint: 'Skill bindings already auto-expand allowed tools. Use this only for built-in micro-utilities not packaged as a skill (e.g. datetime, delegate_agent).', - toolUnionHint: 'Tools selected here are unioned with tools from any bound skills. To restrict an employee to a subset of an MCP server\'s tools, leave the MCP skill unchecked and select only the tools you want here.', + toolUnionHint: 'Tools selected here are unioned with tools from any bound skills. MCP servers enabled at the system level are available to this agent by default — ticking MCP rows here only records an explicit binding, leaving them unticked does not block the agent. To deny a specific MCP tool, configure it under Security → Tool Guard.', + mcpAutoIncludedBadge: 'Auto-available', + mcpAutoIncludedTooltip: 'MCP servers enabled at the system level are available to every agent by default. Ticking only records an explicit binding; leaving rows unticked does not restrict this agent. To deny a specific MCP tool, configure it under Security → Tool Guard.', toolStaleBadge: 'Cached (offline)', toolStaleTooltip: 'The owning MCP server is not currently connected. This tool entry comes from the last successful snapshot; the LLM will not invoke it until the connection is restored.', toolUnavailableBadge: 'Unavailable', diff --git a/mateclaw-ui/src/i18n/locales/zh-CN.ts b/mateclaw-ui/src/i18n/locales/zh-CN.ts index b641c788..80266b66 100644 --- a/mateclaw-ui/src/i18n/locales/zh-CN.ts +++ b/mateclaw-ui/src/i18n/locales/zh-CN.ts @@ -1029,7 +1029,9 @@ export default { searchTools: '搜索工具名称、描述、来源或分组', advancedToolsTitle: '高级：手选原子工具', advancedToolsHint: 'Skill 绑定已自动展开 allowed-tools。此处仅用于未打包成 Skill 的内置微工具（如 datetime、delegate_agent）。', - toolUnionHint: '直选工具会与已绑定技能提供的工具合并生效。如果只想让员工使用某 MCP 服务的部分工具，请不要勾选对应的 MCP 技能，只在这里勾选具体工具。', + toolUnionHint: '直选工具会与已绑定技能提供的工具合并生效。MCP 服务在系统层启用后默认对此 Agent 可用，勾选 MCP 行只是显式记录绑定，不勾也能调用。需要禁用某个 MCP 工具请到「安全 → 工具防护」配置黑名单。', + mcpAutoIncludedBadge: '自动可用', + mcpAutoIncludedTooltip: 'MCP 服务在系统中启用后，其工具对所有 Agent 默认可用。勾选只用于记录显式绑定，不勾不会限制此 Agent 调用。如需禁用，请在「安全 → 工具防护」配置。', toolStaleBadge: '离线缓存', toolStaleTooltip: '该 MCP 服务当前未连接，工具列表来自上次连接时的快照；连接恢复前 LLM 不会调用这个工具。', toolUnavailableBadge: '不可用', diff --git a/mateclaw-ui/src/views/Agents.vue b/mateclaw-ui/src/views/Agents.vue index 92320515..6c24c0bf 100644 --- a/mateclaw-ui/src/views/Agents.vue +++ b/mateclaw-ui/src/views/Agents.vue @@ -382,7 +382,22 @@
{{ t('agents.binding.noMatchingTools') }}