fix(agent): respect explicit MCP tool selection with skills disabled

This commit is contained in:
matevip 2026-10-16 04:05:25 -04:00
parent da67c5e8b9
commit 614740459a
8 changed files with 577 additions and 52 deletions

View File

@ -453,9 +453,8 @@ public class AgentGraphBuilder {
builtinSearchEnabled = true; builtinSearchEnabled = true;
} }
if (builtinSearchEnabled) { if (builtinSearchEnabled) {
// Phase 2: 不再移除 search 工具,改为在 prompt 中设定优先级引导 // Provider-native search is independent of the scoped web_search callback.
// 内置搜索作为首选,search 工具作为补充/兜底 log.info("模型原生搜索已开启 (provider={}),web_search 指导由最终工具范围决定", provider.getProviderId());
log.info("内置搜索已开启 (provider={}),search 工具保留作为补充通道", provider.getProviderId());
} }
// Default 100 if DB row leaves max_iterations null. Negative or zero is an // Default 100 if DB row leaves max_iterations null. Negative or zero is an
// explicit opt-in to "no soft cap" — ObservationDispatcher already treats // explicit opt-in to "no soft cap" — ObservationDispatcher already treats
@ -492,7 +491,9 @@ public class AgentGraphBuilder {
entity.getId(), basePromptTokens, prefixBudgetPlan.effectiveMaxTokens()); entity.getId(), basePromptTokens, prefixBudgetPlan.effectiveMaxTokens());
} }
String enhancedPrompt = buildEnhancedPrompt(entity, builtinSearchEnabled, prefixBudgetPlan.memoryTokens()); String enhancedPrompt = buildEnhancedPrompt(entity, builtinSearchEnabled, prefixBudgetPlan.memoryTokens(),
toolSet.callbacks().stream().map(callback -> callback.getToolDefinition().name())
.collect(java.util.stream.Collectors.toSet()));
// Runtime skill-catalog renderer — captures this agent's bound skills, // Runtime skill-catalog renderer — captures this agent's bound skills,
// effective tool allowlist, model window and workspace; invoked each // effective tool allowlist, model window and workspace; invoked each
@ -1382,6 +1383,11 @@ public class AgentGraphBuilder {
} }
private String buildEnhancedPrompt(AgentEntity entity, boolean builtinSearchEnabled, int memoryBudgetTokens) { private String buildEnhancedPrompt(AgentEntity entity, boolean builtinSearchEnabled, int memoryBudgetTokens) {
return buildEnhancedPrompt(entity, builtinSearchEnabled, memoryBudgetTokens, Set.of());
}
private String buildEnhancedPrompt(AgentEntity entity, boolean builtinSearchEnabled, int memoryBudgetTokens,
Set<String> availableToolNames) {
// The agent's own systemPrompt encodes its identity (role / goal / // The agent's own systemPrompt encodes its identity (role / goal /
// backstory). The memory block from workspace files (AGENTS.md, SOUL.md, // backstory). The memory block from workspace files (AGENTS.md, SOUL.md,
// PROFILE.md, MEMORY.md, ...) augments that identity with durable // PROFILE.md, MEMORY.md, ...) augments that identity with durable
@ -1538,27 +1544,41 @@ public class AgentGraphBuilder {
- These notifications appear at most once per event; if you miss one, it will not be repeated, so act on it in the turn you see it. - These notifications appear at most once per event; if you miss one, it will not be repeated, so act on it in the turn you see it.
""".formatted(entity.getId()); """.formatted(entity.getId());
// Web-search vs browser_use priority guidance — emitted unconditionally so the rule // Only recommend callbacks surviving the runtime deny/allow filters. Provider-native
// also reaches OpenAI-compatible / Anthropic / Gemini / DeepSeek / Ollama agents that // search is a separate capability and does not imply either callback is available.
// do not have builtin search. Issue #40: without this rule the model treats boolean webSearchAvailable = availableToolNames.contains("web_search");
// browser_use as a search tool and gets stuck in a Playwright launch loop on Windows. boolean browserAvailable = availableToolNames.contains("browser_use");
String searchGuidance = """ StringBuilder searchGuidance = new StringBuilder();
if (webSearchAvailable || browserAvailable || builtinSearchEnabled) {
searchGuidance.append("\n\n## Web Search Capability\n");
}
if (webSearchAvailable) {
searchGuidance.append("""
## Web Search Capability - For plain web search or fetching public page content, call the `web_search` tool. It supports advanced parameters: `freshness` (day/week/month/year), `language` (zh-CN/en), `count` (1-10).
- When searching for news, use the standard format: `📰 [Category] Title — Source | Time + Summary`, up to 5 results per category.
""");
}
if (browserAvailable) {
// Issue #40: browser search can cause a Playwright launch loop on Windows.
searchGuidance.append("""
### Tool Priority - Call `browser_use` ONLY when you need to interact with a page (click, fill forms, screenshot, run JS, follow a logged-in flow). Do NOT use `browser_use` as a search alternative.
- For plain web search or fetching public page content, call the `search` tool. It supports advanced parameters: `freshness` (day/week/month/year), `language` (zh-CN/en), `count` (1-10). """);
- Call `browser_use` ONLY when you need to interact with a page (click, fill forms, screenshot, run JS, follow a logged-in flow). Do NOT use `browser_use` as a search alternative. }
- **NEVER** call both `browser_use` and `search` for the same query. if (webSearchAvailable && browserAvailable) {
- When searching for news, use the standard format: `📰 [Category] Title — Source | Time + Summary`, up to 5 results per category. searchGuidance.append("\n- **NEVER** call both `browser_use` and `web_search` for the same query.\n");
"""; }
if (builtinSearchEnabled) { if (builtinSearchEnabled) {
searchGuidance += """ searchGuidance.append("""
### Built-in Search (preferred when available) ### Provider-native Search
Your responses automatically incorporate live web search results from the model provider. For most queries, answer directly — your reply already includes real-time search data. Do NOT say you cannot search. Search capability is enabled for the model provider. It may supply live web results when used; do not assume every response includes search results. Base claims about current information on results actually returned.
Use the `search` tool ONLY when you need precise time filtering (e.g., "yesterday's news" → freshness=day), a specific language, or when built-in results feel insufficient. When provider results are available and sufficient, use them directly.
"""; """);
if (webSearchAvailable) {
searchGuidance.append("Use `web_search` only when additional results are needed.\n");
}
} }
// Wiki 知识库上下文注入。Share the same prefix budget as the memory // Wiki 知识库上下文注入。Share the same prefix budget as the memory

View File

@ -506,13 +506,16 @@ public class AgentBindingService implements AgentBindingResolver {
* <p>Auto-included on every non-null result, in addition to the bound * <p>Auto-included on every non-null result, in addition to the bound
* tools and skill-expanded tools: * tools and skill-expanded tools:
* <ul> * <ul>
* <li>{@link #MINIMAL_SYSTEM_TOOLS} for explicit tools opt-out; otherwise * <li>{@link #MINIMAL_SYSTEM_TOOLS} for explicit tools opt-out;
* {@link #SYSTEM_LEVEL_TOOLS} for compatibility with skill bindings.</li> * {@link #SCOPED_TOOL_INTERNAL_TOOLS} when skills are disabled and
* explicit tool bindings exist (including all-disabled rows);
* otherwise {@link #SYSTEM_LEVEL_TOOLS} for compatibility.</li>
* <li>Every currently-bindable MCP tool ({@code source="mcp"}, * <li>Every currently-bindable MCP tool ({@code source="mcp"},
* {@code available=true} in the picker) — but only when the agent * {@code available=true} in the picker) — but only when the agent
* has not ticked any MCP tool itself. MCP servers are * has not ticked any MCP tool itself and is not in the explicit
* administrator-level capabilities, so an agent that bound merely * skills-disabled tool scope described above. MCP servers are
* a skill or a built-in tool keeps full MCP access. Once the * administrator-level capabilities, so a skills-enabled agent that
* bound merely a skill or a built-in tool keeps full MCP access. Once the
* operator ticks specific MCP rows, that is read as a deliberate * operator ticks specific MCP rows, that is read as a deliberate
* per-agent scope: only the ticked MCP tools stay and the rest * per-agent scope: only the ticked MCP tools stay and the rest
* are not auto-joined, so a role can be limited to a fixed MCP * are not auto-joined, so a role can be limited to a fixed MCP
@ -548,6 +551,16 @@ public class AgentBindingService implements AgentBindingResolver {
return null; return null;
} }
// With skills disabled, explicit tool rows are the complete business
// capability selection. Do not infer scope from the live MCP catalog:
// stale/offline selections and an explicit empty set must stay scoped.
// tools_disabled retains its separate minimal-only opt-out below.
if (skillsDisabled && !toolsDisabled && directTools != null) {
Set<String> scoped = new LinkedHashSet<>(directTools);
scoped.addAll(SCOPED_TOOL_INTERNAL_TOOLS);
return scoped;
}
Set<String> merged = new LinkedHashSet<>(); Set<String> merged = new LinkedHashSet<>();
if (boundSkillIds != null && !boundSkillIds.isEmpty()) { if (boundSkillIds != null && !boundSkillIds.isEmpty()) {
@ -677,6 +690,16 @@ public class AgentBindingService implements AgentBindingResolver {
"enable_tool", "tool_search", "tool_describe", "tool_call" "enable_tool", "tool_search", "tool_describe", "tool_call"
); );
/**
* Internal tools for an explicit selection with skills disabled. read_file
* preserves access to large MCP results spilled into the workspace; normal
* sandbox and global tool-guard restrictions still apply at runtime.
* This is deliberately separate from the tools-disabled minimal set.
*/
private static final Set<String> SCOPED_TOOL_INTERNAL_TOOLS = java.util.stream.Stream.concat(
MINIMAL_SYSTEM_TOOLS.stream(), java.util.stream.Stream.of("read_file"))
.collect(Collectors.toUnmodifiableSet());
/** Compatibility defaults for skill-bound agents that have NOT disabled tools. */ /** Compatibility defaults for skill-bound agents that have NOT disabled tools. */
private static final Set<String> SYSTEM_LEVEL_TOOLS = Set.of( private static final Set<String> SYSTEM_LEVEL_TOOLS = Set.of(
// Structured memory primitives — used by every agent regardless // Structured memory primitives — used by every agent regardless

View File

@ -0,0 +1,87 @@
package vip.mate.agent;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.test.util.ReflectionTestUtils;
import vip.mate.agent.model.AgentEntity;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Set;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.*;
class AgentGraphBuilderSearchGuidanceTest {
private AgentGraphBuilder builder;
@BeforeEach
void setUp() {
builder = mock(AgentGraphBuilder.class, CALLS_REAL_METHODS);
for (var field : AgentGraphBuilder.class.getDeclaredFields()) {
if (!Modifier.isStatic(field.getModifiers()) && !field.getType().isPrimitive()) {
ReflectionTestUtils.setField(builder, field.getName(), mock(field.getType()));
}
}
}
@Test void mcpOnlyDoesNotRecommendUnselectedWebTools() throws Exception {
String prompt = prompt(false, Set.of("mcp_catalog_search"));
assertFalse(prompt.contains("## Web Search Capability"));
assertTrue(prompt.contains("exact-name"));
assertTrue(prompt.contains("serverId prefixes"));
}
@Test void noToolsHasNoWebGuidance() throws Exception {
assertFalse(prompt(false, Set.of()).contains("## Web Search Capability"));
}
@Test void searchOnlyUsesExactRuntimeName() throws Exception {
String prompt = prompt(false, Set.of("web_search"));
assertTrue(prompt.contains("call the `web_search` tool"));
assertFalse(prompt.contains("`browser_use`"));
assertFalse(prompt.contains("call the `search` tool"));
}
@Test void browserOnlyRecommendsOnlyBrowserInteraction() throws Exception {
String prompt = prompt(false, Set.of("browser_use"));
assertTrue(prompt.contains("Call `browser_use` ONLY"));
assertFalse(prompt.contains("`web_search`"));
assertFalse(prompt.contains("`search` for the same query"));
}
@Test void bothToolsReceivePriorityGuidance() throws Exception {
String prompt = prompt(false, Set.of("web_search", "browser_use"));
assertTrue(prompt.contains("call the `web_search` tool"));
assertTrue(prompt.contains("Call `browser_use` ONLY"));
assertTrue(prompt.contains("both `browser_use` and `web_search`"));
}
@Test void nativeSearchDoesNotClaimEveryReplyContainsSearchResults() throws Exception {
String prompt = prompt(true, Set.of());
assertTrue(prompt.contains("model provider"));
assertFalse(prompt.contains("automatically incorporate live web search results"));
assertFalse(prompt.contains("already includes real-time search data"));
assertFalse(prompt.contains("`web_search`"));
assertFalse(prompt.contains("`browser_use`"));
assertFalse(prompt.contains("`search` tool"));
}
@Test void nativeSearchUsesReturnedResultsBeforeSupplementaryScopedSearch() throws Exception {
String prompt = prompt(true, Set.of("web_search"));
assertTrue(prompt.contains("call the `web_search` tool"));
assertTrue(prompt.contains("When provider results are available and sufficient, use them directly"));
assertFalse(prompt.contains("`browser_use`"));
assertFalse(prompt.contains("already includes real-time search data"));
}
private String prompt(boolean nativeSearch, Set<String> names) throws Exception {
AgentEntity entity = new AgentEntity();
entity.setId(664L);
entity.setSystemPrompt("Scoped agent identity");
Method method = AgentGraphBuilder.class.getDeclaredMethod("buildEnhancedPrompt",
AgentEntity.class, boolean.class, int.class, Set.class);
method.setAccessible(true);
return (String) method.invoke(builder, entity, nativeSearch, 1024, names);
}
}

View File

@ -0,0 +1,303 @@
package vip.mate.agent;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.sun.net.httpserver.HttpServer;
import org.junit.jupiter.api.Test;
import org.springframework.ai.chat.messages.AssistantMessage;
import org.springframework.ai.chat.model.ToolContext;
import org.springframework.ai.support.ToolCallbacks;
import org.springframework.ai.tool.ToolCallback;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.test.util.ReflectionTestUtils;
import vip.mate.agent.binding.model.AgentToolBinding;
import vip.mate.agent.binding.repository.*;
import vip.mate.agent.binding.service.AgentBindingService;
import vip.mate.agent.context.PrefixBudgetPlan;
import vip.mate.agent.graph.executor.ToolExecutionExecutor;
import vip.mate.agent.model.AgentEntity;
import vip.mate.agent.repository.AgentMapper;
import vip.mate.llm.model.*;
import vip.mate.tool.ToolRegistry;
import vip.mate.tool.builtin.ProgressiveToolBridgeTool;
import vip.mate.tool.disclosure.*;
import vip.mate.tool.guard.ToolGuardResult;
import vip.mate.tool.guard.service.ToolGuardConfigService;
import vip.mate.tool.mcp.model.McpServerEntity;
import vip.mate.tool.mcp.runtime.*;
import vip.mate.tool.mcp.service.McpServerService;
import vip.mate.tool.service.ToolService;
import java.lang.reflect.Modifier;
import java.net.InetSocketAddress;
import java.util.*;
import java.util.concurrent.CopyOnWriteArrayList;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;
/** Real HTTP MCP transport and production build filtering; model decisions are scripted tool calls. */
class Issue664McpExecutionIntegrationTest {
static final ObjectMapper JSON = new ObjectMapper();
static final String A = McpToolNameResolver.prefixedName(66401L, "search");
static final String B = McpToolNameResolver.prefixedName(66402L, "search");
@Test void selectedServerExecutesThroughBridgeWithoutRecoveringUncheckedTools() throws Exception {
try (var a = new FakeMcp(); var b = new FakeMcp()) {
var manager = manager();
try {
manager.connect(a.entity(66401L)); manager.connect(b.entity(66402L));
var callbacks = manager.getAllToolCallbacks();
assertEquals(Set.of(A, B), names(callbacks));
var binding = bindings();
var bridge = new ProgressiveToolBridgeTool(mock(ToolRegistry.class), binding,
mock(ToolGuardConfigService.class));
var all = new ArrayList<>(callbacks);
all.addAll(List.of(ToolCallbacks.from(bridge)));
all.add(callback("web_search")); all.add(callback("browser_use"));
var scoped = AgentToolSet.fromCallbacks(List.of(), all)
.withAllowedToolsOnly(binding.getEffectiveToolNames(664L));
assertTrue(names(scoped.callbacks()).contains(A));
assertFalse(names(scoped.callbacks()).contains(B));
var context = new ToolContext(Map.of(ProgressiveToolBridgeTool.SCOPED_TOOL_CALLBACKS_CONTEXT_KEY,
scoped.callbackByName()));
assertTrue(bridge.search("search", 20, context).contains(A));
for (String excluded : List.of(B, "web_search", "browser_use", "search")) {
assertFalse(bridge.search("", 20, context).contains("\"name\":\"" + excluded + "\""));
assertTrue(bridge.describe(excluded, context).contains("not available"));
assertTrue(call(scoped, excluded).contains("not available"));
}
assertTrue(bridge.describe(A, context).contains("query"));
assertEquals("selected MCP result", call(scoped, A));
assertEquals(1, a.calls.size()); assertTrue(b.calls.isEmpty());
assertEquals("search", a.calls.getFirst().get("name"));
assertEquals(Map.of("query", "issue664"), a.calls.getFirst().get("arguments"));
// Hard schema budgeting hides a selected core schema, while execution retains its scoped callback.
var ts = mock(ToolService.class); when(ts.listTools()).thenReturn(List.of());
var ms = mock(McpServerService.class);
var server = a.entity(66401L); server.setDisclosureTier("core");
when(ms.listAll()).thenReturn(List.of(server)); when(ms.listEnabled()).thenReturn(List.of(server));
var registry = mock(ToolRegistry.class); when(registry.enabledToolBeanFunctionNameIndex()).thenReturn(Map.of());
var disclosure = new DefaultToolDisclosureService(ts, ms, registry, new ToolUsageRecencyTracker());
var demoted = disclosure.computeAutoDemotions(scoped, 1);
assertTrue(demoted.contains(A));
assertFalse(names(disclosure.split(scoped, Set.of(), demoted).activeCallbacks()).contains(A));
assertTrue(disclosure.renderExtensionCatalog(scoped, 8192, demoted).contains(A));
assertEquals("selected MCP result", call(scoped, A));
var denied = scoped.withDeniedToolsFiltered(Set.of(A)).withAllowedToolsOnly(binding.getEffectiveToolNames(664L));
assertTrue(call(denied, A).contains("not available"));
assertEquals(2, a.calls.size());
// Oversized remote text is preserved on disk, then retrieved by the actual read_file callback.
a.resultText = "fixture line\n".repeat(1000) + "TAIL_EVIDENCE_664\n";
var dir = java.nio.file.Files.createTempDirectory("issue664-spill-");
try {
var props = new vip.mate.agent.graph.executor.ToolResultProperties();
props.setStorageBaseDir(dir.toString());
var storage = new vip.mate.agent.graph.executor.ToolResultStorage(props);
var reader = new vip.mate.tool.builtin.ReadFileTool(mock(vip.mate.i18n.I18nService.class));
var withReader = new ArrayList<>(scoped.callbacks());
withReader.addAll(List.of(ToolCallbacks.from(reader)));
var readScope = AgentToolSet.fromCallbacks(List.of(), withReader)
.withAllowedToolsOnly(binding.getEffectiveToolNames(664L));
assertTrue(names(readScope.callbacks()).contains("read_file"));
var executor = new ToolExecutionExecutor(readScope, (n,args) -> ToolGuardResult.allow(), null, null);
ReflectionTestUtils.setField(executor, "resultStorage", storage);
var spilled = executor.execute(List.of(new AssistantMessage.ToolCall("large", "function", "tool_call",
"{\"toolName\":\"" + A + "\",\"arguments\":{\"query\":\"large\"}}")),
"issue664-large", "664", false, "user", null).responses().getFirst().responseData();
assertTrue(spilled.startsWith(vip.mate.agent.graph.executor.ToolResultStorage.SPILL_MARKER_PREFIX));
java.nio.file.Path file;
try (var files = java.nio.file.Files.walk(dir)) { file = files.filter(java.nio.file.Files::isRegularFile).findFirst().orElseThrow(); }
assertEquals(a.resultText, java.nio.file.Files.readString(file));
var readArgs = JSON.writeValueAsString(Map.of("filePath", file.toString(), "startLine", 1001));
var retrieved = executor.execute(List.of(new AssistantMessage.ToolCall("read", "function", "read_file", readArgs)),
"issue664-large", "664", false, "user", null).responses().getFirst().responseData();
assertTrue(retrieved.contains("TAIL_EVIDENCE_664"), retrieved);
} finally {
try (var files = java.nio.file.Files.walk(dir)) {
for (var file : files.sorted(Comparator.reverseOrder()).toList()) java.nio.file.Files.deleteIfExists(file);
}
}
manager.remove(66401L);
var offline = AgentToolSet.fromCallbacks(List.of(), manager.getAllToolCallbacks())
.withAllowedToolsOnly(binding.getEffectiveToolNames(664L));
assertTrue(offline.isEmpty());
assertTrue(call(offline, A).contains("not available"));
assertTrue(b.calls.isEmpty());
} finally { manager.closeAll(); }
}
}
@Test void scriptedModelRequestRunsSelectedMcpThroughProductionGraph() throws Exception {
var tracker = new vip.mate.channel.web.ChatStreamTracker(JSON);
try (var server = new FakeMcp()) {
var manager = manager();
try {
manager.connect(server.entity(66401L));
var bridge = new ProgressiveToolBridgeTool(mock(ToolRegistry.class), bindings(), mock(ToolGuardConfigService.class));
var callbacks = new ArrayList<>(manager.getAllToolCallbacks());
callbacks.addAll(List.of(ToolCallbacks.from(bridge)));
var scoped = AgentToolSet.fromCallbacks(List.of(), callbacks).withAllowedToolsOnly(bindings().getEffectiveToolNames(664L));
var builder = mock(AgentGraphBuilder.class, CALLS_REAL_METHODS);
for (var field : AgentGraphBuilder.class.getDeclaredFields()) {
if (!Modifier.isStatic(field.getModifiers()) && !field.getType().isPrimitive())
ReflectionTestUtils.setField(builder, field.getName(), mock(field.getType()));
}
ReflectionTestUtils.setField(builder, "streamTracker", tracker);
ReflectionTestUtils.setField(builder, "graphObservationProperties", new vip.mate.config.GraphObservationProperties());
ReflectionTestUtils.setField(builder, "toolTimeoutProperties", new vip.mate.config.ToolTimeoutProperties());
ReflectionTestUtils.setField(builder, "conversationWindowManager", null);
ReflectionTestUtils.setField(builder, "progressLedgerService", null);
ReflectionTestUtils.setField(builder, "executionCheckpoint", null);
ReflectionTestUtils.setField(builder, "executionEvidenceRecorder", null);
ReflectionTestUtils.setField(builder, "toolResultStorage", null);
ReflectionTestUtils.setField(builder, "generatedFileCache", null);
var guard = (vip.mate.tool.guard.service.ToolGuardService) ReflectionTestUtils.getField(builder, "toolGuardService");
when(guard.evaluate(any(), eq(true))).thenReturn(vip.mate.tool.guard.model.GuardEvaluation.allow(A));
var disclosure = disclosure(server.entity(66401L));
ReflectionTestUtils.setField(builder, "toolDisclosureService", disclosure);
var advertised = disclosure.split(scoped, Set.of(), Set.of()).activeCallbacks();
var prompts = new CopyOnWriteArrayList<org.springframework.ai.chat.prompt.Prompt>();
var model = mock(org.springframework.ai.chat.model.ChatModel.class);
var emitted = new AssistantMessage.ToolCall("remote", "function", "tool_call",
"{\"toolName\":\"" + A + "\",\"arguments\":{\"query\":\"model-query\"}}");
when(model.stream(any(org.springframework.ai.chat.prompt.Prompt.class))).thenAnswer(inv -> {
prompts.add(inv.getArgument(0));
AssistantMessage answer = prompts.size() == 1
? AssistantMessage.builder().content("").toolCalls(List.of(emitted)).build()
: new AssistantMessage(prompts.size() == 2 ? "selected MCP result" :
"{\"decision\":\"ACCEPT\",\"taskComplete\":true,\"remainingWork\":false,\"failedCallCount\":0,\"recoveryClaims\":[],\"reason\":\"Selected MCP output matches answer\"}");
return reactor.core.publisher.Flux.just(new org.springframework.ai.chat.model.ChatResponse(
List.of(new org.springframework.ai.chat.model.Generation(answer))));
});
var graph = builder.buildReActGraph(scoped, model, 5, null);
var states = graph.stream(Map.of(vip.mate.agent.graph.state.MateClawStateKeys.CONVERSATION_ID, "issue664-model",
vip.mate.agent.graph.state.MateClawStateKeys.SYSTEM_PROMPT, "Use the selected catalog tool " + A,
vip.mate.agent.graph.state.MateClawStateKeys.USER_MESSAGE, "Search fixture",
vip.mate.agent.graph.state.MateClawStateKeys.MESSAGES, List.of(new org.springframework.ai.chat.messages.UserMessage("Search fixture"))))
.collectList().block(java.time.Duration.ofSeconds(15));
assertNotNull(states); assertFalse(states.isEmpty());
assertEquals("selected MCP result", states.getLast().state().value(vip.mate.agent.graph.state.MateClawStateKeys.FINAL_ANSWER, ""));
assertFalse(prompts.isEmpty());
var options = (org.springframework.ai.model.tool.ToolCallingChatOptions) prompts.getFirst().getOptions();
assertEquals(names(advertised), names(options.getToolCallbacks()));
assertFalse(names(options.getToolCallbacks()).contains(A));
assertEquals(1, server.calls.size());
assertEquals(Map.of("query", "model-query"), server.calls.getFirst().get("arguments"));
assertTrue(prompts.stream().skip(1).anyMatch(prompt -> prompt.getContents().contains("selected MCP result")));
} finally { manager.closeAll(); }
} finally { tracker.onShutdown(); }
}
@Test void bothProductionBuildPathsReceiveFilteredToolsAndPrompt() throws Exception {
for (String type : List.of("react", "plan_execute")) {
for (boolean denySelected : List.of(false, true)) {
var builder = mock(AgentGraphBuilder.class, CALLS_REAL_METHODS);
for (var field : AgentGraphBuilder.class.getDeclaredFields()) {
if (!Modifier.isStatic(field.getModifiers()) && !field.getType().isPrimitive())
ReflectionTestUtils.setField(builder, field.getName(), mock(field.getType()));
}
var registry = (ToolRegistry) ReflectionTestUtils.getField(builder, "toolRegistry");
var enabled = AgentToolSet.fromCallbacks(List.of(),
List.of(callback(A), callback(B), callback("web_search"), callback("browser_use")));
when(registry.getEnabledToolSet()).thenReturn(enabled);
var guardConfig = (ToolGuardConfigService) ReflectionTestUtils.getField(builder, "toolGuardConfigService");
when(guardConfig.getDeniedTools()).thenReturn(denySelected ? Set.of(A) : Set.of());
ReflectionTestUtils.setField(builder, "agentBindingService", bindings());
var provider = new ModelProviderEntity(); provider.setProviderId("fixture"); provider.setChatModel("openai-compatible");
var model = new ModelConfigEntity(); model.setProvider("fixture"); model.setModelName("fixture");
var models = (vip.mate.llm.service.ModelConfigService) ReflectionTestUtils.getField(builder, "modelConfigService");
when(models.resolveModel(any())).thenReturn(model);
var providers = (vip.mate.llm.service.ModelProviderService) ReflectionTestUtils.getField(builder, "modelProviderService");
when(providers.getProviderConfig("fixture")).thenReturn(provider);
when(providers.isProviderConfigured("fixture")).thenReturn(true);
var planner = (vip.mate.agent.context.PrefixBudgetPlanner) ReflectionTestUtils.getField(builder, "prefixBudgetPlanner");
when(planner.plan(any(), anyInt(), anyInt())).thenReturn(PrefixBudgetPlan.unlimited(16000));
var captured = new java.util.concurrent.atomic.AtomicReference<AgentToolSet>();
if (type.equals("react")) doAnswer(inv -> { captured.set(inv.getArgument(0)); return mock(vip.mate.agent.graph.StateGraphReActAgent.class); })
.when(builder).buildReActAgent(any(), any(), anyInt(), any(), any(), any(), anySet());
else doAnswer(inv -> { captured.set(inv.getArgument(0)); return mock(vip.mate.agent.graph.plan.StateGraphPlanExecuteAgent.class); })
.when(builder).buildPlanExecuteAgent(any(), any(), anyInt(), any(), any(), anySet());
var entity = new AgentEntity(); entity.setId(664L); entity.setName("fixture"); entity.setAgentType(type);
var agent = builder.build(entity);
assertEquals(denySelected ? Set.of() : Set.of(A), names(captured.get().callbacks()));
String prompt = (String) ReflectionTestUtils.getField(agent, "systemPrompt");
assertFalse(prompt.contains("web_search")); assertFalse(prompt.contains("browser_use"));
assertFalse(prompt.contains("调用 search"));
}
}
}
static Set<String> names(List<ToolCallback> callbacks) {
return callbacks.stream().map(cb -> cb.getToolDefinition().name()).collect(java.util.stream.Collectors.toSet());
}
static ToolCallback callback(String name) {
var cb = mock(ToolCallback.class);
when(cb.getToolDefinition()).thenReturn(org.springframework.ai.tool.definition.ToolDefinition.builder()
.name(name).description(name).inputSchema("{\"type\":\"object\"}").build());
return cb;
}
static String call(AgentToolSet set, String target) {
var executor = new ToolExecutionExecutor(set, (n,a) -> ToolGuardResult.allow(), null, null);
return executor.execute(List.of(new AssistantMessage.ToolCall("call", "function", "tool_call",
"{\"toolName\":\"" + target + "\",\"arguments\":{\"query\":\"issue664\"}}")),
"issue664", "664", false, "user", null).responses().getFirst().responseData();
}
static AgentBindingService bindings() {
var agents = mock(AgentMapper.class); var agent = new AgentEntity(); agent.setId(664L); agent.setSkillsDisabled(true);
when(agents.selectById(664L)).thenReturn(agent);
var tools = mock(AgentToolBindingMapper.class); var row = new AgentToolBinding(); row.setToolName(A); row.setEnabled(true);
when(tools.selectList(any())).thenReturn(List.of(row));
return new AgentBindingService(mock(AgentSkillBindingMapper.class), tools,
mock(AgentProviderPreferenceMapper.class), mock(AgentWikiKbBindingMapper.class),
mock(vip.mate.wiki.repository.WikiKnowledgeBaseMapper.class), mock(vip.mate.skill.runtime.SkillRuntimeService.class),
mock(vip.mate.tool.service.AvailableToolService.class), agents,
mock(vip.mate.skill.repository.SkillMapper.class), mock(vip.mate.skill.acp.AcpSkillBridge.class));
}
static DefaultToolDisclosureService disclosure(McpServerEntity server) {
var tools = mock(ToolService.class);
when(tools.listTools()).thenReturn(List.of());
var servers = mock(McpServerService.class);
when(servers.listAll()).thenReturn(List.of(server));
when(servers.listEnabled()).thenReturn(List.of(server));
var registry = mock(ToolRegistry.class);
when(registry.enabledToolBeanFunctionNameIndex()).thenReturn(Map.of());
return new DefaultToolDisclosureService(tools, servers, registry, new ToolUsageRecencyTracker());
}
static McpClientManager manager() {
return new McpClientManager(mock(ApplicationEventPublisher.class),
new McpIdentityForwardService(new McpIdentityForwardProperties()), null, JSON);
}
static class FakeMcp implements AutoCloseable {
volatile String resultText = "selected MCP result";
final HttpServer http; final List<Map<String,Object>> calls = new CopyOnWriteArrayList<>();
FakeMcp() throws Exception {
http = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0);
http.createContext("/mcp", exchange -> {
if (!exchange.getRequestMethod().equals("POST")) { exchange.sendResponseHeaders(405,-1); exchange.close(); return; }
var request = JSON.readTree(exchange.getRequestBody());
if (!request.has("id")) { exchange.sendResponseHeaders(202,-1); exchange.close(); return; }
Object result = switch (request.path("method").asText()) {
case "initialize" -> Map.of("protocolVersion", request.path("params").path("protocolVersion").asText(),
"capabilities", Map.of("tools", Map.of()), "serverInfo", Map.of("name", "fixture", "version", "1"));
case "tools/list" -> Map.of("tools", List.of(Map.of("name", "search", "description", "Search fixture",
"inputSchema", Map.of("type", "object", "required", List.of("query"), "properties", Map.of("query", Map.of("type", "string"))))));
case "tools/call" -> { calls.add(JSON.convertValue(request.path("params"), Map.class));
yield Map.of("content", List.of(Map.of("type", "text", "text", resultText)), "isError", false); }
default -> Map.of();
};
byte[] body = JSON.writeValueAsBytes(Map.of("jsonrpc", "2.0", "id", request.get("id"), "result", result));
exchange.getResponseHeaders().set("Content-Type", "application/json");
exchange.sendResponseHeaders(200, body.length); exchange.getResponseBody().write(body); exchange.close();
}); http.start();
}
McpServerEntity entity(long id) {
var s = new McpServerEntity(); s.setId(id); s.setName("fixture-" + id); s.setTransport("streamable_http");
s.setToolsCacheJson("[{\"name\":\"search\",\"description\":\"Search fixture\"}]");
s.setUrl("http://127.0.0.1:" + http.getAddress().getPort() + "/mcp"); s.setConnectTimeoutSeconds(3); s.setReadTimeoutSeconds(3); return s;
}
public void close() { http.stop(0); }
}
}

View File

@ -387,6 +387,107 @@ class AgentBindingServiceTest {
+ "实际 allowlist: " + effective); + "实际 allowlist: " + effective);
} }
@Test
void skillsDisabledSelectedMcpExcludesUncheckedCapabilities() {
seedMcpServerWithOneTool(8_888_664L, "issue664-selected", "search");
seedMcpServerWithOneTool(8_888_665L, "issue664-other", "search");
String selected = mcpToolNameForServer(8_888_664L);
String other = mcpToolNameForServer(8_888_665L);
assertNotNull(selected);
assertNotNull(other);
setSkillsDisabledFlag(true);
bindingService.setToolBindings(agentId, List.of(selected));
Set<String> effective = bindingService.getEffectiveToolNames(agentId);
assertTrue(effective.contains(selected));
assertFalse(effective.contains(other));
assertScopedInternalTools(effective);
assertUncheckedBusinessToolsAbsent(effective);
}
@Test
void skillsDisabledStaleMcpBindingDoesNotAutoJoinAvailableMcp() {
seedMcpServerWithOneTool(8_888_666L, "issue664-offline", "search");
seedMcpServerWithOneTool(8_888_667L, "issue664-online", "search");
String selected = mcpToolNameForServer(8_888_666L);
String other = mcpToolNameForServer(8_888_667L);
assertNotNull(selected);
assertNotNull(other);
setSkillsDisabledFlag(true);
bindingService.setToolBindings(agentId, List.of(selected));
jdbcTemplate.update("UPDATE mate_mcp_server SET enabled = FALSE, last_status = 'disconnected' WHERE id = ?",
8_888_666L);
Set<String> effective = bindingService.getEffectiveToolNames(agentId);
assertTrue(effective.contains(selected), "Saved binding remains scoped even when its provider goes offline");
assertFalse(effective.contains(other), "Unavailable selection must not authorize another provider");
assertScopedInternalTools(effective);
assertUncheckedBusinessToolsAbsent(effective);
}
@Test
void skillsDisabledExplicitEmptyBindingDoesNotInheritBusinessToolsOrMcp() {
seedMcpServerWithOneTool(8_888_668L, "issue664-empty", "search");
setSkillsDisabledFlag(true);
bindingService.bindTool(agentId, "disabled_binding_probe");
jdbcTemplate.update("UPDATE mate_agent_tool SET enabled = FALSE WHERE agent_id = ?", agentId);
assertEquals(Set.of(), bindingService.getBoundToolNames(agentId));
Set<String> effective = bindingService.getEffectiveToolNames(agentId);
assertScopedInternalTools(effective);
assertUncheckedBusinessToolsAbsent(effective);
assertFalse(effective.contains("disabled_binding_probe"));
assertFalse(effective.stream().anyMatch(name -> name.startsWith("mcp_")));
}
@Test
void skillsDisabledExplicitBuiltinSelectionIsPreservedWithoutMcpAutoJoin() {
seedBuiltinTool("web_search");
seedMcpServerWithOneTool(8_888_669L, "issue664-mixed", "search");
seedMcpServerWithOneTool(8_888_670L, "issue664-unselected", "search");
String selected = mcpToolNameForServer(8_888_669L);
assertNotNull(selected);
setSkillsDisabledFlag(true);
bindingService.setToolBindings(agentId, List.of(selected, "web_search"));
Set<String> effective = bindingService.getEffectiveToolNames(agentId);
assertTrue(effective.containsAll(Set.of(selected, "web_search")));
assertFalse(effective.contains(mcpToolNameForServer(8_888_670L)));
assertFalse(effective.contains("browser_use"));
assertFalse(effective.contains("execute_shell_command"));
assertScopedInternalTools(effective);
}
@Test
void skillsDisabledBuiltinOnlySelectionDoesNotAutoJoinMcp() {
seedBuiltinTool("builtin_only_probe");
seedMcpServerWithOneTool(8_888_671L, "issue664-builtin-only", "search");
setSkillsDisabledFlag(true);
bindingService.setToolBindings(agentId, List.of("builtin_only_probe"));
Set<String> effective = bindingService.getEffectiveToolNames(agentId);
assertTrue(effective.contains("builtin_only_probe"));
assertScopedInternalTools(effective);
assertUncheckedBusinessToolsAbsent(effective);
assertFalse(effective.stream().anyMatch(name -> name.startsWith("mcp_")));
}
private void assertScopedInternalTools(Set<String> effective) {
assertNotNull(effective);
assertTrue(effective.containsAll(Set.of("read_file", "record_lesson", "get_progress",
"getManagedGoalJsonSlots", "publishManagedGoalJson", "checkManagedGoalJson",
"enable_tool", "tool_search", "tool_describe", "tool_call")));
}
private void assertUncheckedBusinessToolsAbsent(Set<String> effective) {
for (String name : Set.of("web_search", "browser_use", "execute_code", "execute_shell_command",
"delegateToAgent", "delegateParallel", "delegateAsync", "taskOutput", "listAvailableAgents",
"write_file", "append_file", "edit_file", "send_file", "renderDocx", "image_generate",
"wiki_create_page", "load_skill", "runSkillScript", "listAvailableSkills")) {
assertFalse(effective.contains(name), "Unchecked capability must not be exposed: " + name);
}
}
/** Picker name the UI would save for the (only) MCP tool of {@code serverId}. */ /** Picker name the UI would save for the (only) MCP tool of {@code serverId}. */
private String mcpToolNameForServer(long serverId) { private String mcpToolNameForServer(long serverId) {
return availableToolService.listAvailable().stream() return availableToolService.listAvailable().stream()
@ -507,7 +608,7 @@ class AgentBindingServiceTest {
} }
@Test @Test
@DisplayName("issue #184 matrix (T,F,*,>0): skillsDisabled + 显式 tool 绑定 → 仅这些 tool + SYSTEM + MCP-rule") @DisplayName("issue #184 matrix (T,F,*,>0): skillsDisabled + 显式 tool 绑定 → 仅这些 tool + minimal internal + read_file")
void skillsDisabledWithToolBindingsScopesToTools() { void skillsDisabledWithToolBindingsScopesToTools() {
seedBuiltinTool("scoped_tool_probe"); seedBuiltinTool("scoped_tool_probe");
setSkillsDisabledFlag(true); setSkillsDisabledFlag(true);

View File

@ -44,7 +44,8 @@ class DshSdkTransportTest {
read shutdown read shutdown
"""); """);
try (var sdk = new DshSdkProcess(process, new ObjectMapper(), "")) { try (var sdk = new DshSdkProcess(process, new ObjectMapper(), "")) {
assertEquals("0.0.1", sdk.initialize(Map.of(), Duration.ofSeconds(2)).path("serverInfo").path("version").asText()); // Allow process/reader scheduling under full-suite load; this verifies identity, not handshake latency.
assertEquals("0.0.1", sdk.initialize(Map.of(), Duration.ofSeconds(10)).path("serverInfo").path("version").asText());
} }
assertTrue(process.waitFor(2, TimeUnit.SECONDS)); assertTrue(process.waitFor(2, TimeUnit.SECONDS));
} }

View File

@ -48,6 +48,8 @@ class FeishuWebSocketTransportTest {
OkHttpClient http=(OkHttpClient)field(sdk,"httpClient"); OkHttpClient http=(OkHttpClient)field(sdk,"httpClient");
assertTrue(pool.awaitTermination(3,TimeUnit.SECONDS)); assertTrue(pool.awaitTermination(3,TimeUnit.SECONDS));
assertTrue(http.dispatcher().executorService().awaitTermination(3,TimeUnit.SECONDS)); assertTrue(http.dispatcher().executorService().awaitTermination(3,TimeUnit.SECONDS));
// Socket cancellation releases the WebSocket connection through asynchronous failure cleanup.
await(()->http.connectionPool().connectionCount()==0);
assertEquals(0,http.connectionPool().connectionCount()); assertEquals(0,http.connectionPool().connectionCount());
} }
@Test void delayedHandshakeStaysReconnectingThenUsesOnlyOneClient() throws Exception { @Test void delayedHandshakeStaysReconnectingThenUsesOnlyOneClient() throws Exception {

View File

@ -3,12 +3,15 @@ package vip.mate.channel.webchat;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.test.context.bean.override.mockito.MockitoBean;
import org.springframework.boot.test.web.server.LocalServerPort; import org.springframework.boot.test.web.server.LocalServerPort;
import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.TestPropertySource;
import org.springframework.test.context.DynamicPropertyRegistry;
import org.springframework.test.context.DynamicPropertySource;
import reactor.core.publisher.Flux; import reactor.core.publisher.Flux;
import vip.mate.MateClawApplication; import vip.mate.MateClawApplication;
import vip.mate.agent.AgentService; import vip.mate.agent.AgentService;
@ -24,11 +27,8 @@ import java.net.http.HttpResponse;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.Duration; import java.time.Duration;
import java.util.Comparator;
import java.util.Map; import java.util.Map;
import java.util.stream.Stream;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
@ -77,12 +77,14 @@ class WebChatAttachmentE2ETest {
private static final long AGENT_ID = 9_148_1011L; private static final long AGENT_ID = 9_148_1011L;
private static final Duration HTTP_TIMEOUT = Duration.ofSeconds(15); private static final Duration HTTP_TIMEOUT = Duration.ofSeconds(15);
/** /** Each test-class run owns its upload tree; JUnit removes it after the class. */
* Wipe the upload dir for our test conversation ids. The path is deterministic @TempDir
* from the API key prefix, so we can clean it precisely instead of nuking static Path uploadRoot;
* the whole {@code data/chat-uploads} tree.
*/ @DynamicPropertySource
private static final Path UPLOAD_ROOT = Paths.get("data", "chat-uploads"); static void uploadProperties(DynamicPropertyRegistry registry) {
registry.add("mateclaw.chat.upload.base-dir", () -> uploadRoot.toString());
}
@LocalServerPort private int port; @LocalServerPort private int port;
@Autowired private JdbcTemplate jdbc; @Autowired private JdbcTemplate jdbc;
@ -245,21 +247,6 @@ class WebChatAttachmentE2ETest {
return end < 0 ? null : json.substring(start, end); return end < 0 ? null : json.substring(start, end);
} }
// ==================== cleanup ====================
@org.junit.jupiter.api.AfterEach
void cleanUploadDirs() throws IOException {
// Only the convs under our key8 prefix are ours; safe to wipe.
Path root = UPLOAD_ROOT.resolve("webchat:testkey1");
if (Files.exists(root)) {
try (Stream<Path> walk = Files.walk(root)) {
walk.sorted(Comparator.reverseOrder()).forEach(p -> {
try { Files.deleteIfExists(p); } catch (IOException ignored) { }
});
}
}
}
// ==================== tests ==================== // ==================== tests ====================
@Test @Test
@ -288,6 +275,7 @@ class WebChatAttachmentE2ETest {
// on Windows), so assert against the sanitized segment. // on Windows), so assert against the sanitized segment.
String path = extractStringField(parts, "path"); String path = extractStringField(parts, "path");
assertThat(path).contains(ChatUploadLocationResolver.sanitizeSegment(cid)); assertThat(path).contains(ChatUploadLocationResolver.sanitizeSegment(cid));
assertThat(Path.of(path).toAbsolutePath().normalize()).startsWith(uploadRoot.toAbsolutePath().normalize());
assertThat(Files.isRegularFile(Path.of(path))).isTrue(); assertThat(Files.isRegularFile(Path.of(path))).isTrue();
// The bytes on disk match what we uploaded. // The bytes on disk match what we uploaded.
assertThat(Files.readString(Path.of(path))).isEqualTo(fileBody); assertThat(Files.readString(Path.of(path))).isEqualTo(fileBody);