feat(agent,mcp): validate tool bindings on save and keep returnDirect raw-name config working

mateclaw-server/src/main/java/vip/mate/agent/binding/service/AgentBindingService.java

@@ -12,8 +12,11 @@ import vip.mate.agent.binding.model.AgentToolBinding;
 import vip.mate.agent.binding.repository.AgentProviderPreferenceMapper;
 import vip.mate.agent.binding.repository.AgentSkillBindingMapper;
 import vip.mate.agent.binding.repository.AgentToolBindingMapper;
+import vip.mate.exception.MateClawException;
 import vip.mate.skill.runtime.SkillRuntimeService;
 import vip.mate.skill.runtime.model.ResolvedSkill;
+import vip.mate.tool.model.AvailableToolDTO;
+import vip.mate.tool.service.AvailableToolService;
 
 import java.util.Collections;
 import java.util.LinkedHashSet;
@@ -43,16 +46,25 @@ public class AgentBindingService {
      * graph when SkillRuntimeService initializes after binding.
      */
     private final SkillRuntimeService skillRuntimeService;
+    /**
+     * Source of truth for what the picker can offer (built-in + MCP). Used
+     * by {@link #setToolBindings} to refuse new tool names that the runtime
+     * couldn't resolve anyway — closes the gap where a UI-disabled row
+     * could still be saved by hitting the API directly.
+     */
+    private final AvailableToolService availableToolService;
 
     @Autowired
     public AgentBindingService(AgentSkillBindingMapper skillBindingMapper,
                                AgentToolBindingMapper toolBindingMapper,
                                AgentProviderPreferenceMapper providerPreferenceMapper,
-                               @Lazy SkillRuntimeService skillRuntimeService) {
+                               @Lazy SkillRuntimeService skillRuntimeService,
+                               AvailableToolService availableToolService) {
         this.skillBindingMapper = skillBindingMapper;
         this.toolBindingMapper = toolBindingMapper;
         this.providerPreferenceMapper = providerPreferenceMapper;
         this.skillRuntimeService = skillRuntimeService;
+        this.availableToolService = availableToolService;
     }
 
     // ==================== Skill Bindings ====================
@@ -335,9 +347,27 @@ public class AgentBindingService {
     }
 
     /**
-     * 批量设置 Agent 的 tool 绑定（替换模式）
+     * Replace the agent's tool binding set.
+     *
+     * <p>Validation rule for each incoming name:
+     * <ul>
+     *   <li><b>Already in the existing binding</b> → always allowed (so the
+     *       user can keep a previously-bound tool whose upstream MCP server
+     *       is currently stale or even removed; the client just keeps what
+     *       it already had).</li>
+     *   <li><b>New addition (not in existing binding)</b> → must appear in
+     *       {@link AvailableToolService#listAvailable()} with
+     *       {@code available == true}. Names that are unknown
+     *       (typos / legacy unprefixed MCP names / hand-crafted strings) or
+     *       that the picker marked unavailable (hash collision, etc.) are
+     *       rejected — saving them would put a {@code mate_agent_tool} row
+     *       in the database that the runtime can never resolve, which then
+     *       silently drops the tool when the agent runs.</li>
+     * </ul>
      */
     public void setToolBindings(Long agentId, List<String> toolNames) {
+        validateNewToolBindings(agentId, toolNames);
+
         toolBindingMapper.delete(
                 new LambdaQueryWrapper<AgentToolBinding>()
                         .eq(AgentToolBinding::getAgentId, agentId));
@@ -352,6 +382,56 @@ public class AgentBindingService {
         }
     }
 
+    /**
+     * Refuse the save when any *newly-added* tool name doesn't resolve to
+     * an {@code available=true} row in the picker. Names already in the
+     * existing binding are exempt so that subsequent edits (especially
+     * "remove this stale tool") still succeed even if upstream state has
+     * drifted.
+     */
+    private void validateNewToolBindings(Long agentId, List<String> incoming) {
+        if (incoming == null || incoming.isEmpty()) {
+            return;
+        }
+        Set<String> existing = listToolBindings(agentId).stream()
+                .map(AgentToolBinding::getToolName)
+                .collect(Collectors.toSet());
+        Set<String> bindable;
+        try {
+            bindable = availableToolService.listAvailable().stream()
+                    .filter(AvailableToolDTO::isAvailable)
+                    .map(AvailableToolDTO::getName)
+                    .collect(Collectors.toSet());
+        } catch (Exception e) {
+            // The picker source briefly failing must not block the user
+            // from saving a binding that's still in their existing set.
+            // Re-validate everything against just the existing set —
+            // strictly conservative: only allow keeps, refuse adds.
+            log.warn("AvailableToolService unavailable during binding validation, falling back to existing-only: {}",
+                    e.getMessage());
+            bindable = Set.of();
+        }
+
+        List<String> rejected = new java.util.ArrayList<>();
+        for (String name : incoming) {
+            if (name == null || name.isBlank()) {
+                rejected.add("<blank>");
+                continue;
+            }
+            if (existing.contains(name)) continue; // keeps are always allowed
+            if (!bindable.contains(name)) rejected.add(name);
+        }
+        if (!rejected.isEmpty()) {
+            String preview = rejected.size() <= 5
+                    ? String.join(", ", rejected)
+                    : String.join(", ", rejected.subList(0, 5)) + " (+" + (rejected.size() - 5) + " more)";
+            throw new MateClawException("err.agent.tool_binding_unbindable",
+                    "Tool name(s) cannot be bound: " + preview
+                            + ". Either the name is unknown or the picker marked it unavailable "
+                            + "(e.g. hash collision, upstream server removed).");
+        }
+    }
+
     // ==================== Provider Preferences (RFC-009 PR-3) ====================
 
     /** Raw rows for the agent edit form. Sorted by sort_order ascending. */

mateclaw-server/src/main/java/vip/mate/tool/mcp/runtime/McpReturnDirectProperties.java

@@ -8,7 +8,7 @@ import java.util.LinkedHashSet;
 import java.util.Set;
 
 /**
- * RFC-052 §3.4 / PR-4: MCP tool return-direct opt-in list.
+ * MCP tool return-direct opt-in list.
  *
  * <p>Tools listed here are wrapped in {@link ReturnDirectMcpToolCallback} so
  * their results bypass the LLM context (see {@code ToolExecutionExecutor} and
@@ -20,14 +20,27 @@ import java.util.Set;
  *   mcp:
  *     return-direct:
  *       tools:
- *         - query_employee_salary
- *         - read_medical_record
+ *         - query_employee_salary           # raw upstream name (legacy form, still supported)
+ *         - mcp_42_query_employee_salary_aB3xYz  # full prefixed callback name (server-scoped, precise)
  * </pre>
  *
- * <p>Match is by tool name only (matching the upstream {@code ToolDefinition.name()}).
- * Per-server scoping is intentionally out of scope for the first iteration; if
- * the same tool name comes from two servers and only one should be direct, give
- * one of them a name prefix at the MCP server config layer.
+ * <p><b>Two accepted name forms</b>:
+ * <ul>
+ *   <li><b>Raw upstream name</b> ({@code query_employee_salary}) —
+ *       matches the wrapped callback's underlying delegate name. This is
+ *       the form that existed before the runtime started prefixing
+ *       callback names; existing deployments keep working unchanged.
+ *       A raw name matches every server that exposes that tool, so use
+ *       this form when a sensitive name should be direct on every
+ *       server it appears.</li>
+ *   <li><b>Prefixed callback name</b>
+ *       ({@code mcp_<serverId>_<slug>_<hash6>}) — server-scoped, precise.
+ *       Use this form when only one of several MCP servers exposing the
+ *       same raw name should be treated as direct.</li>
+ * </ul>
+ * Matching happens via {@link #matches(String, String)} from the consumer
+ * side; see {@link McpToolCallbackProvider#getToolCallbacks} for the call
+ * site.
  *
  * @author MateClaw Team
  */
@@ -35,7 +48,7 @@ import java.util.Set;
 @ConfigurationProperties(prefix = "mateclaw.mcp.return-direct")
 public class McpReturnDirectProperties {
 
-    /** Tool names that should be treated as returnDirect. */
+    /** Tool names (raw or prefixed) that should be treated as returnDirect. */
     private Set<String> tools = Collections.emptySet();
 
     public Set<String> getTools() {
@@ -46,7 +59,27 @@ public class McpReturnDirectProperties {
         this.tools = tools != null ? new LinkedHashSet<>(tools) : Collections.emptySet();
     }
 
+    /**
+     * Single-string check kept for back-compat with any caller that has
+     * only one form of the name. Prefer {@link #matches(String, String)}
+     * from the wrapping path so both prefixed and raw forms get a chance
+     * to match.
+     */
     public boolean isReturnDirect(String toolName) {
         return toolName != null && tools.contains(toolName);
     }
+
+    /**
+     * @return {@code true} iff the configured set contains either the
+     *         prefixed callback name OR the raw upstream tool name. Either
+     *         argument may be {@code null} (e.g. when a callback isn't a
+     *         {@link PrefixedNameToolCallback} so no raw form is
+     *         available); the other is checked on its own.
+     */
+    public boolean matches(String prefixedName, String rawName) {
+        if (tools.isEmpty()) return false;
+        if (prefixedName != null && tools.contains(prefixedName)) return true;
+        if (rawName != null && tools.contains(rawName)) return true;
+        return false;
+    }
 }

mateclaw-server/src/main/java/vip/mate/tool/mcp/runtime/McpToolCallbackProvider.java

@@ -40,14 +40,30 @@ public class McpToolCallbackProvider implements ToolCallbackProvider {
                         callbacks.size(), mcpClientManager.getActiveCount());
             }
 
-            // RFC-052: opt-in returnDirect wrapping. The decorator only changes
+            // Opt-in returnDirect wrapping. The decorator only changes
             // ToolMetadata.returnDirect(); guard/approval/observability still
             // see the original callback through the wrapper.
+            //
+            // Names registered by the manager are now prefixed
+            // (mcp_<serverId>_<slug>_<hash6>) — but operators have been
+            // configuring the return-direct list with raw upstream names
+            // (e.g. `query_employee_salary`) since long before the prefix
+            // existed. Match on EITHER form so an existing deployment's
+            // sensitive-tool isolation doesn't silently regress when this
+            // change rolls out: a tool counts as return-direct if its
+            // configured token equals (a) the prefixed callback name OR
+            // (b) the underlying raw tool name visible through the
+            // PrefixedNameToolCallback wrapper.
             List<ToolCallback> wrapped = new ArrayList<>(callbacks.size());
             for (ToolCallback cb : callbacks) {
-                String name = cb.getToolDefinition() != null ? cb.getToolDefinition().name() : null;
-                if (returnDirectProperties.isReturnDirect(name)) {
-                    log.info("[McpToolCallbackProvider] wrapping MCP tool '{}' as returnDirect (RFC-052)", name);
+                String prefixed = cb.getToolDefinition() != null ? cb.getToolDefinition().name() : null;
+                String raw = (cb instanceof PrefixedNameToolCallback w && w.getDelegate() != null
+                        && w.getDelegate().getToolDefinition() != null)
+                        ? w.getDelegate().getToolDefinition().name()
+                        : null;
+                if (returnDirectProperties.matches(prefixed, raw)) {
+                    log.info("[McpToolCallbackProvider] wrapping MCP tool as returnDirect (prefixed='{}', raw='{}')",
+                            prefixed, raw);
                     wrapped.add(new ReturnDirectMcpToolCallback(cb));
                 } else {
                     wrapped.add(cb);