diff --git a/src/main/java/it/cnr/isti/workflow/manager/mcp/MCPAgentService.java b/src/main/java/it/cnr/isti/workflow/manager/mcp/MCPAgentService.java index 2a8e7a2..7d730f6 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/mcp/MCPAgentService.java +++ b/src/main/java/it/cnr/isti/workflow/manager/mcp/MCPAgentService.java @@ -595,6 +595,13 @@ public class MCPAgentService { Mono requestMono = client().delete() .uri("/sessions/{sessionId}", sessionId) .retrieve() + // A session the bridge no longer has is the outcome this method wants, not a + // failure to report: the bridge expires sessions on its own, so 404 is the + // ordinary answer for anything the idle sweep reaches second. Treated as an + // error it cost far more than noise - the removal below never ran, so the sweep + // retried the same dead session every minute forever, and each attempt counted + // against the circuit breaker that guards real MCP calls. + .onStatus(status -> status.value() == 404, clientResponse -> Mono.empty()) .onStatus(status -> status.isError(), clientResponse -> clientResponse.bodyToMono(String.class) .defaultIfEmpty("MCP bridge returned an error without body") .flatMap(body -> { diff --git a/src/test/java/it/cnr/isti/workflow/manager/mcp/MCPAgentServiceTest.java b/src/test/java/it/cnr/isti/workflow/manager/mcp/MCPAgentServiceTest.java index ea4c58b..4d0d9ed 100644 --- a/src/test/java/it/cnr/isti/workflow/manager/mcp/MCPAgentServiceTest.java +++ b/src/test/java/it/cnr/isti/workflow/manager/mcp/MCPAgentServiceTest.java @@ -185,6 +185,38 @@ class MCPAgentServiceTest { assertTrue(documentError.getMessage().contains("must be PDF")); } + @Test + void closingASessionTheBridgeNoLongerHasStopsTrackingIt() throws Exception { + // The bridge expires sessions on its own, so the idle sweep regularly finds one already + // gone. Treating that 404 as a failure left the session in activeSessions, so the sweep + // retried the same dead session every minute forever - and every attempt counted against + // the circuit breaker that guards real MCP calls. + AtomicInteger deleteCalls = new AtomicInteger(); + HttpServer bridge = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + bridge.createContext("/sessions/gone-session", exchange -> { + deleteCalls.incrementAndGet(); + assertEquals("DELETE", exchange.getRequestMethod()); + writeJson(exchange, 404, "{\"detail\":\"Session gone-session not found\"}"); + }); + bridge.start(); + try { + MCPAgentService service = serviceForBridge(bridge); + ReflectionTestUtils.setField(service, "closeTimeoutSeconds", 2L); + @SuppressWarnings("unchecked") + Map activeSessions = + (Map) ReflectionTestUtils.getField(service, "activeSessions"); + activeSessions.put("gone-session", System.currentTimeMillis()); + + service.closeSessionQuietly("gone-session"); + + assertEquals(1, deleteCalls.get()); + assertFalse(activeSessions.containsKey("gone-session"), + "a session the bridge has already dropped must not be swept again"); + } finally { + bridge.stop(0); + } + } + private MCPAgentService serviceForBridge(HttpServer bridge) throws Exception { MCPAgentService service = new MCPAgentService( WebClient.builder(),