fix(agent): auto-include enabled MCP tools in effective allowlist (#108)

This commit is contained in:
matevip 2026-05-12 21:24:36 +08:00
parent bcf194c35e
commit cc5d7d5ad0
5 changed files with 128 additions and 3 deletions

View File

@ -301,6 +301,19 @@ public class AgentBindingService {
* contribute nothing through this path; legacy SKILL.md prompt
* enhancement still runs separately.</li>
* </ul>
*
* <p>Auto-included on every non-null result, in addition to the bound
* tools and skill-expanded tools:
* <ul>
* <li>{@link #SYSTEM_LEVEL_TOOLS} agent-wide primitives.</li>
* <li>Every currently-bindable MCP tool (any tool with
* {@code source="mcp"} and {@code available=true} in the picker).
* MCP servers are administrator-level capabilities; once enabled
* globally they should not be silently hidden from an agent that
* happens to have any other binding. To deny a specific MCP tool
* to a specific agent, use the tool-guard deny path applied
* upstream in {@code AgentGraphBuilder}.</li>
* </ul>
*/
public Set<String> getEffectiveToolNames(Long agentId) {
Set<Long> 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<String> 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.

View File

@ -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<String> 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<String> effective = bindingService.getEffectiveToolNames(agentId);
assertNull(effective, "完全没有 skill / tool 绑定时必须返回 null= 不过滤),"
+ "否则 AgentToolSet.withAllowedToolsOnly 会变成空集禁掉所有工具");
}
@Test
@DisplayName("unbindTool 后 DB 里真的没行(物理 delete不是软删留 deleted=1")
void unbindPhysicallyRemovesRow() {

View File

@ -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',

View File

@ -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: '不可用',

View File

@ -382,7 +382,22 @@
<div v-else-if="filteredAvailableToolGroups.length === 0" class="binding-empty binding-empty--compact">{{ t('agents.binding.noMatchingTools') }}</div>
<div v-else class="binding-list">
<template v-for="group in filteredAvailableToolGroups" :key="group.groupId">
<div class="binding-group-header">{{ group.label }}</div>
<div class="binding-group-header">
<span>{{ group.label }}</span>
<!-- MCP groups: ticking is now record-only enabled MCP
tools auto-join the agent's effective allowlist
(server is admin-enabled at the system level). Tell
the user that here so they don't think unchecked
MCP rows are disabled. To deny a specific MCP tool,
users still have Security Tool Guard. -->
<span
v-if="group.groupId && group.groupId.startsWith('mcp:')"
class="binding-group-note"
:title="t('agents.binding.mcpAutoIncludedTooltip')"
>
{{ t('agents.binding.mcpAutoIncludedBadge') }}
</span>
</div>
<label
v-for="tool in group.tools"
:key="tool.rowId || `${group.groupId}#${tool.rawName}#${tool.name}`"
@ -1458,6 +1473,19 @@ html.dark .live-pill {
.advanced-tools-chevron { color: var(--mc-text-tertiary); font-size: 12px; }
.advanced-tools-note { font-style: italic; color: var(--mc-text-tertiary); margin-top: 4px; }
/* MCP group header: inline "auto-available" tag next to the label so users
know enabled MCP tools work without being ticked. */
.binding-group-header { display: flex; align-items: center; gap: 8px; }
.binding-group-note {
font-size: 11px;
font-weight: 500;
padding: 2px 8px;
border-radius: 10px;
background: color-mix(in srgb, var(--mc-primary) 12%, transparent);
color: var(--mc-primary);
cursor: help;
}
/* Icon picker trigger replaces the old free-text icon input. Tile shape
* mirrors SkillMarket's identity-icon-row so the create/edit affordance
* is consistent across the app. */