From 818f9c28e18cd4bebba216bdab1624b8cc5ff686 Mon Sep 17 00:00:00 2001
From: Lucio Lelii <lucio.lelii@isti.cnr.it>
Date: Mon, 14 Sep 2026 12:34:42 +0200
Subject: [PATCH] Stop re-deleting MCP sessions the bridge has already dropped

The bridge expires sessions on its own, so the idle sweep regularly asks
it to delete one that is already gone. That 404 was treated as a failure,
and the cost was not just the warning and stack trace every minute: the
removal from activeSessions sat after the call that threw, so the dead
session was never untracked and the sweep retried it forever - and each
attempt counted against the circuit breaker that guards real MCP calls,
where five of them open it.

A session the bridge no longer has is the outcome this method wants, so
404 now completes it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../workflow/manager/mcp/MCPAgentService.java |  7 ++++
 .../manager/mcp/MCPAgentServiceTest.java      | 32 +++++++++++++++++++
 2 files changed, 39 insertions(+)

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<Void> 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<String, Long> activeSessions =
+                    (Map<String, Long>) 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(),