Require an LLM node to have used an MCP server, not to have named its tools

A node listed the tool names it had to have used successfully before its
answer was believed. Nobody could tell what to write there without
knowing each server's tools, and listing them from the editor would take
the server's credentials, which may only exist at run time.

Each bound server now says instead whether it must be used: the node
fails, with LLM_REQUIRED_MCP_SERVER_UNUSED, when the model finishes
without one successful call to a required server, and the message says
which one was never called or only ever failed. A made-up tool name
counts for no server. The per-tool list is gone; a flow that still
carries it reads as before, without the check. The fixture that used it
now requires the development server and the browser.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/JsonSchemaProducer.java

@@ -1449,9 +1449,6 @@ public class JsonSchemaProducer {
                     equalsAny.add(value);
                 }
             }
-            if (dependency.present()) {
-                visibleWhen.put("present", true);
-            }
         }
     }
 

src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/LLMBlockConfiguration.java

@@ -18,7 +18,6 @@ import it.cnr.isti.workflow.manager.configurations.annotations.UiDescription;
 import it.cnr.isti.workflow.manager.configurations.annotations.UiLabel;
 import it.cnr.isti.workflow.manager.configurations.annotations.UiOrder;
 import it.cnr.isti.workflow.manager.configurations.annotations.UiUniqueItemsBy;
-import it.cnr.isti.workflow.manager.configurations.annotations.UiVisibleWhen;
 import it.cnr.isti.workflow.manager.llms.LLMDescriptor;
 import it.cnr.isti.workflow.manager.mcp.MCPToolServerBinding;
 import it.cnr.isti.workflow.manager.skills.SkillBinding;
@@ -72,21 +71,6 @@ public class LLMBlockConfiguration extends BlockConfiguration<LLMBlockType> {
     @JsonProperty(required = false)
     List<MCPToolServerBinding> mcpServers = List.of();
 
-    /**
-     * Evidence a tool-using node must collect before its natural-language answer is accepted.
-     * This is deliberately opt-in: many ordinary MCP nodes only retrieve information and have no
-     * browser or service lifecycle to prove.
-     */
-    @Structural
-    @Size(max = 16)
-    @UiOrder(70)
-    // Checked only by the loop MCP servers start, so without one there is nothing it could apply to.
-    @UiVisibleWhen(field = "mcpServers", present = true)
-    @UiLabel("Required successful MCP tools")
-    @UiDescription("Tool names that must each complete successfully before this node can finish. Use this for agents that must prove work, for example start_service, browser_navigate and browser_snapshot.")
-    @JsonProperty(required = false)
-    List<String> requiredSuccessfulMcpTools = List.of();
-
     /**
      * Fields the answer is split into, each becoming a port of its own.
      *
@@ -125,7 +109,6 @@ public class LLMBlockConfiguration extends BlockConfiguration<LLMBlockType> {
             String prompt,
             List<SkillBinding> skills,
             List<MCPToolServerBinding> mcpServers,
-            List<String> requiredSuccessfulMcpTools,
             List<LLMOutputField> outputs,
             List<UploadInput> uploadInputs) {
         super(name);
@@ -133,7 +116,6 @@ public class LLMBlockConfiguration extends BlockConfiguration<LLMBlockType> {
         this.prompt = prompt;
         this.skills = skills == null ? List.of() : List.copyOf(skills);
         this.mcpServers = mcpServers == null ? List.of() : List.copyOf(mcpServers);
-        this.requiredSuccessfulMcpTools = requiredSuccessfulMcpTools == null ? List.of() : List.copyOf(requiredSuccessfulMcpTools);
         this.outputs = outputs == null ? List.of() : List.copyOf(outputs);
         this.uploadInputs = uploadInputs == null ? List.of() : List.copyOf(uploadInputs);
     }
@@ -148,7 +130,6 @@ public class LLMBlockConfiguration extends BlockConfiguration<LLMBlockType> {
         configuration.name = LLMBlockType.TYPE;
         configuration.skills = List.of();
         configuration.mcpServers = List.of();
-        configuration.requiredSuccessfulMcpTools = List.of();
         configuration.outputs = List.of();
         configuration.uploadInputs = List.of();
         return configuration;
@@ -215,17 +196,6 @@ public class LLMBlockConfiguration extends BlockConfiguration<LLMBlockType> {
         return names.stream().distinct().count() == names.size();
     }
 
-    @AssertTrue(message = "requiredSuccessfulMcpTools must contain distinct non-empty tool names")
-    @JsonIgnore
-    boolean areRequiredSuccessfulMcpToolsValid() {
-        if (requiredSuccessfulMcpTools == null || requiredSuccessfulMcpTools.isEmpty()) {
-            return true;
-        }
-        return requiredSuccessfulMcpTools.stream()
-                .allMatch(tool -> tool != null && !tool.isBlank())
-                && requiredSuccessfulMcpTools.stream().distinct().count() == requiredSuccessfulMcpTools.size();
-    }
-
     @AssertTrue(message = "structured output names must be distinct, and none may be called 'response'")
     @JsonIgnore
     boolean areOutputNamesUsable() {

src/main/java/it/cnr/isti/workflow/manager/configurations/annotations/UiVisibleWhen.java

@@ -26,6 +26,4 @@ public @interface UiVisibleWhen {
     String equals() default "";
 
     String[] equalsAny() default {};
-    /** Shown only once {@link #field()} holds something: a text that is not blank, a list with an item. */
-    boolean present() default false;
 }

src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/LLMExecutor.java

@@ -113,7 +113,7 @@ public class LLMExecutor implements BlockExecutor<LLMBlockType> {
         try {
             if (config.getMcpServers() != null && !config.getMcpServers().isEmpty()) {
                 response = llmToolLoop.run(llmProvider, llmDescriptor, credential, prompt, attached.attachments(),
-                        config.getMcpServers(), executionVariables, eventLogger, config.getRequiredSuccessfulMcpTools());
+                        config.getMcpServers(), executionVariables, eventLogger);
                 return StructuredLLMOutputs.split(response, config.getOutputs(), LLMBlockFactory.OUTPUT_NAME);
             }
             // Files need a message to ride on; without them this stays the single prompt it always was.

src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/LLMToolLoop.java

@@ -18,6 +18,7 @@ import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Component;
 
+import it.cnr.isti.workflow.manager.executions.NodeExecutionException;
 import it.cnr.isti.workflow.manager.executions.ExecutionEventLogger;
 import it.cnr.isti.workflow.manager.executions.ExecutionEventType;
 import it.cnr.isti.workflow.manager.llms.ChatAttachment;
@@ -64,27 +65,20 @@ public class LLMToolLoop {
     public String run(LLMProvider provider, LLMDescriptor descriptor, ProviderCredential credential, String prompt,
             List<MCPToolServerBinding> bindings, Map<String, Object> executionVariables,
             ExecutionEventLogger eventLogger) {
-        return run(provider, descriptor, credential, prompt, bindings, executionVariables, eventLogger, List.of());
-    }
-
-    /**
-     * Runs an agent with optional evidence requirements. A tool can fail transiently and be retried,
-     * so requirements are checked only when the model tries to end the conversation.
-     */
-    public String run(LLMProvider provider, LLMDescriptor descriptor, ProviderCredential credential, String prompt,
-            List<MCPToolServerBinding> bindings, Map<String, Object> executionVariables,
-            ExecutionEventLogger eventLogger, List<String> requiredSuccessfulTools) {
-        return run(provider, descriptor, credential, prompt, List.of(), bindings, executionVariables, eventLogger,
-                requiredSuccessfulTools);
+        return run(provider, descriptor, credential, prompt, List.of(), bindings, executionVariables, eventLogger);
     }
 
     /**
+     * Runs an agent. A server bound as required must have answered at least one call successfully
+     * before the model's answer is accepted; a call can fail transiently and be retried, so that is
+     * checked only when the model tries to end the conversation.
+     *
      * @param attachments files sent with the opening message - the one message pruning never
      *                    touches, so the model can look at them again on every turn
      */
     public String run(LLMProvider provider, LLMDescriptor descriptor, ProviderCredential credential, String prompt,
             List<ChatAttachment> attachments, List<MCPToolServerBinding> bindings, Map<String, Object> executionVariables,
-            ExecutionEventLogger eventLogger, List<String> requiredSuccessfulTools) {
+            ExecutionEventLogger eventLogger) {
         if (!provider.supportsTools()) {
             // Checked here and not only when the flow is saved: the provider can arrive from an input
             // port, and then no save-time check ever saw it.
@@ -93,9 +87,8 @@ public class LLMToolLoop {
         }
 
         Instant deadline = Instant.now().plus(maxDuration);
-        Set<String> successfulTools = new HashSet<>();
-        Set<String> failedTools = new HashSet<>();
-        Set<String> attemptedTools = new HashSet<>();
+        Set<String> serversAnswered = new HashSet<>();
+        Set<String> serversFailed = new HashSet<>();
         try (MCPToolbox toolbox = MCPToolbox.open(bindings, executionVariables, sessionFactory)) {
             logEvent(eventLogger, ExecutionEventType.MCP_SESSION_OPENED,
                     "Opened MCP sessions for " + String.join(", ", toolbox.serverIds()),
@@ -134,7 +127,7 @@ public class LLMToolLoop {
                     if (looksLikeAWrittenToolCall(turn.content())) {
                         throw new IllegalStateException(describeWrittenToolCall(descriptor.model()));
                     }
-                    assertCompletionEvidence(requiredSuccessfulTools, successfulTools, failedTools, attemptedTools);
+                    assertRequiredServersUsed(bindings, serversAnswered, serversFailed);
                     return turn.content();
                 }
 
@@ -146,11 +139,9 @@ public class LLMToolLoop {
                 for (ToolCall toolCall : turn.toolCalls()) {
                     ToolRun toolRun = runTool(toolbox, toolCall, iteration, eventLogger);
                     messages.add(toolRun.message());
-                    attemptedTools.add(toolRun.name());
-                    if (toolRun.error()) {
-                        failedTools.add(toolRun.name());
-                    } else {
-                        successfulTools.add(toolRun.name());
+                    // A made-up tool name has no server: it counts for none of them.
+                    if (toolRun.serverId() != null) {
+                        (toolRun.error() ? serversFailed : serversAnswered).add(toolRun.serverId());
                     }
                 }
 
@@ -199,36 +190,35 @@ public class LLMToolLoop {
         logger.debug("MCP tool {} on {} finished in {}ms (error={})", toolCall.name(), result.serverId(), elapsed,
                 result.error());
 
-        return new ToolRun(ChatMessage.toolResult(toolCall.id(), toolCall.name(), result.text()), toolCall.name(), result.error());
+        return new ToolRun(ChatMessage.toolResult(toolCall.id(), toolCall.name(), result.text()), toolCall.name(),
+                result.serverId(), result.error());
     }
 
-    private record ToolRun(ChatMessage message, String name, boolean error) {
+    private record ToolRun(ChatMessage message, String name, String serverId, boolean error) {
     }
 
     /**
-     * A prose answer is not evidence that an agent built or tested anything. A flow opting into
-     * these requirements gets a clear, actionable failure instead of a false SUCCESS when the
-     * model gives up after an unsuccessful tool sequence.
+     * A prose answer is not evidence that an agent built or tested anything. A server bound as
+     * required that never answered a call means the model finished without doing what the flow
+     * relies on, and the step says which server, rather than passing on a report nobody checked.
      */
-    private static void assertCompletionEvidence(List<String> requiredSuccessfulTools, Set<String> successfulTools,
-            Set<String> failedTools, Set<String> attemptedTools) {
-        if (requiredSuccessfulTools == null || requiredSuccessfulTools.isEmpty()) {
-            return;
-        }
-        List<String> missing = requiredSuccessfulTools.stream()
-                .filter(required -> !successfulTools.contains(required))
-                .map(required -> failedTools.contains(required)
-                        ? required + " (attempted but failed)"
-                        : attemptedTools.contains(required)
-                                ? required + " (attempted without a successful result)"
-                                : required + " (not attempted)")
+    private static void assertRequiredServersUsed(List<MCPToolServerBinding> bindings, Set<String> serversAnswered,
+            Set<String> serversFailed) {
+        List<String> unused = bindings.stream()
+                .filter(MCPToolServerBinding::required)
+                .map(MCPToolServerBinding::serverName)
+                .filter(server -> !serversAnswered.contains(server))
+                .map(server -> server + (serversFailed.contains(server) ? " (every call to it failed)" : " (never called)"))
                 .toList();
-        if (!missing.isEmpty()) {
-            throw new IllegalStateException("LLM node produced an answer without the required successful MCP evidence: "
-                    + String.join(", ", missing));
+        if (!unused.isEmpty()) {
+            throw new NodeExecutionException(REQUIRED_SERVER_UNUSED, "The model finished without a successful call to"
+                    + " the MCP servers this node requires: " + String.join(", ", unused)
+                    + ". Its answer is not accepted without them");
         }
     }
 
+    static final String REQUIRED_SERVER_UNUSED = "LLM_REQUIRED_MCP_SERVER_UNUSED";
+
     /**
      * Below this, a placeholder costs about as much as the result it replaces - pruning it would move
      * the count without moving the needle, and would only make the reported "reclaimed" figure noise.

src/main/java/it/cnr/isti/workflow/manager/mcp/MCPToolServerBinding.java

@@ -42,8 +42,26 @@ public record MCPToolServerBinding(
         @NotNull
         @JsonProperty(required = false)
         @DynamicSchema(url = "/retriever/MCPServers/definitions/schema", dependsOn = { "serverName" })
-        JsonNode configuration) {
+        JsonNode configuration,
+
+        /**
+         * Whether the node must have used this server before its answer is believed. An agent can
+         * report building and testing something without having called a single tool, and the
+         * report reads the same either way.
+         */
+        @JsonProperty(required = false)
+        @UiLabel("Must be used")
+        @UiDescription("The node fails if the model finishes without at least one successful call to this server.")
+        Boolean required) {
 
     /** Kept low on purpose: every bound server is a session opened for the life of the block. */
     public static final int MAX_PER_BLOCK = 4;
+
+    public MCPToolServerBinding {
+        required = required != null && required;
+    }
+
+    public MCPToolServerBinding(String serverName, JsonNode configuration) {
+        this(serverName, configuration, false);
+    }
 }

src/test/java/it/cnr/isti/workflow/manager/blocks/configurations/LLMBlockSchemaTest.java

@@ -21,16 +21,6 @@ class LLMBlockSchemaTest {
     @Autowired
     private JsonSchemaProducer schemaProducer;
 
-    @Test
-    void theRequiredToolsAreOfferedOnlyToANodeThatHasMcpServersToCall() {
-        JsonNode schema = schemaProducer.generateSchemaNode(LLMBlockConfiguration.class);
-
-        JsonNode visibleWhen = schema.get("properties").get("requiredSuccessfulMcpTools").get("x-ui-visible-when");
-
-        assertEquals("mcpServers", visibleWhen.get("field").asText());
-        assertEquals(true, visibleWhen.get("present").asBoolean());
-    }
-
     @Test
     void theFieldsReadInTheOrderANodeIsSetUp() {
         // The model; what it is asked and given; what it may use; what it hands back.
@@ -40,6 +30,6 @@ class LLMBlockSchemaTest {
         schema.get("x-ui-property-order").forEach(name -> order.add(name.asText()));
 
         assertEquals(java.util.List.of("name", "llmDescriptor", "prompt", "uploadInputs", "skills", "mcpServers",
-                "requiredSuccessfulMcpTools", "outputs"), order.stream().filter(name -> !name.equals("type")).toList());
+                "outputs"), order.stream().filter(name -> !name.equals("type")).toList());
     }
 }

src/test/java/it/cnr/isti/workflow/manager/executions/executors/blocks/LLMToolLoopTest.java

@@ -29,6 +29,7 @@ import org.springframework.web.reactive.function.client.WebClient;
 import com.sun.net.httpserver.HttpExchange;
 import com.sun.net.httpserver.HttpServer;
 
+import it.cnr.isti.workflow.manager.executions.NodeExecutionException;
 import it.cnr.isti.workflow.manager.executions.ExecutionEventLogger;
 import it.cnr.isti.workflow.manager.executions.ExecutionEventType;
 import it.cnr.isti.workflow.manager.llms.ChatAttachment;
@@ -159,6 +160,10 @@ class LLMToolLoopTest {
                 .build());
     }
 
+    private List<MCPToolServerBinding> requiredBindings() {
+        return List.of(new MCPToolServerBinding("coding", JsonNodeFactory.instance.objectNode(), true));
+    }
+
     private static LLMDescriptor descriptor() {
         return new LLMDescriptor("Scripted", "test-model", null);
     }
@@ -209,7 +214,7 @@ class LLMToolLoopTest {
         ChatAttachment sketch = new ChatAttachment(ChatAttachment.Kind.IMAGE, "image/png", "sketch.png", new byte[] { 1 });
         try {
             loopFor(server, 10, 60).run(provider, descriptor(), null, "build what the sketch shows", List.of(sketch),
-                    bindings(), Map.of(), null, List.of());
+                    bindings(), Map.of(), null);
 
             // The model can look at the sketch again after each tool result, not only on the first turn.
             for (List<ChatMessage> conversation : provider.seenConversations) {
@@ -221,19 +226,50 @@ class LLMToolLoopTest {
     }
 
     @Test
-    void refusesAnAnswerWhenTheConfiguredVerificationToolNeverSucceeded() throws Exception {
+    void refusesAnAnswerWhenARequiredServerNeverAnsweredACall() throws Exception {
+        HttpServer server = startServer();
+        ScriptedProvider provider = new ScriptedProvider(true);
+        provider.turns.add(ToolChatResult.text("the page is ready"));
+        try {
+            NodeExecutionException failure = assertThrows(NodeExecutionException.class,
+                    () -> loopFor(server, 10, 60).run(provider, descriptor(), null, "do it", requiredBindings(), Map.of(),
+                            null));
+
+            assertEquals(LLMToolLoop.REQUIRED_SERVER_UNUSED, failure.getErrorCode());
+            assertTrue(failure.getMessage().contains("coding (never called)"), failure.getMessage());
+        } finally {
+            server.stop(0);
+        }
+    }
+
+    @Test
+    void acceptsTheAnswerOnceARequiredServerHasAnsweredACall() throws Exception {
         HttpServer server = startServer();
         ScriptedProvider provider = new ScriptedProvider(true);
         provider.turns.add(ToolChatResult.toolCalls("writing it",
                 List.of(ToolCall.of(0, "write_file", argumentsWithJsonLookingContent()))));
-        provider.turns.add(ToolChatResult.text("the page is ready"));
+        provider.turns.add(ToolChatResult.text("DONE"));
         try {
-            IllegalStateException failure = assertThrows(IllegalStateException.class,
-                    () -> loopFor(server, 10, 60).run(provider, descriptor(), null, "do it", bindings(), Map.of(),
-                            null, List.of("start_service", "browser_snapshot")));
+            assertEquals("DONE", loopFor(server, 10, 60).run(provider, descriptor(), null, "do it", requiredBindings(),
+                    Map.of(), null));
+        } finally {
+            server.stop(0);
+        }
+    }
 
-            assertTrue(failure.getMessage().contains("start_service (not attempted)"), failure.getMessage());
-            assertTrue(failure.getMessage().contains("browser_snapshot (not attempted)"), failure.getMessage());
+    @Test
+    void aMadeUpToolNameDoesNotCountAsUsingARequiredServer() throws Exception {
+        HttpServer server = startServer();
+        ScriptedProvider provider = new ScriptedProvider(true);
+        provider.turns.add(ToolChatResult.toolCalls("",
+                List.of(ToolCall.of(0, "no_such_tool", JsonNodeFactory.instance.objectNode()))));
+        provider.turns.add(ToolChatResult.text("DONE"));
+        try {
+            NodeExecutionException failure = assertThrows(NodeExecutionException.class,
+                    () -> loopFor(server, 10, 60).run(provider, descriptor(), null, "do it", requiredBindings(), Map.of(),
+                            null));
+
+            assertTrue(failure.getMessage().contains("coding (never called)"), failure.getMessage());
         } finally {
             server.stop(0);
         }

src/test/java/it/cnr/isti/workflow/manager/flows/AgentBuiltHumanJudgedFlowTest.java

@@ -94,9 +94,10 @@ class AgentBuiltHumanJudgedFlowTest {
                 .filter(block -> "build-and-check".equals(block.getName()))
                 .findFirst().orElseThrow().getSpecificConfiguration();
 
-        assertTrue(agent.getRequiredSuccessfulMcpTools().containsAll(
-                List.of("start_service", "browser_navigate", "browser_snapshot")),
-                agent.getRequiredSuccessfulMcpTools().toString());
+        assertEquals(List.of("dev-server-mcp", "browser-mcp"), agent.getMcpServers().stream()
+                .filter(it.cnr.isti.workflow.manager.mcp.MCPToolServerBinding::required)
+                .map(it.cnr.isti.workflow.manager.mcp.MCPToolServerBinding::serverName)
+                .toList(), "it must have started the service and looked at the page");
     }
 
     @Test

src/test/resources/flows/agent-built-human-judged.json

@@ -63,18 +63,15 @@
             },
             {
               "serverName": "dev-server-mcp",
-              "configuration": {}
+              "configuration": {},
+              "required": true
             },
             {
               "serverName": "browser-mcp",
-              "configuration": {}
+              "configuration": {},
+              "required": true
             }
           ],
-          "requiredSuccessfulMcpTools": [
-            "start_service",
-            "browser_navigate",
-            "browser_snapshot"
-          ],
           "outputs": [
             {
               "name": "previewUrl",
@@ -351,4 +348,4 @@
       }
     ]
   }
-}
+}