lelii
/
humainflow-service
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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>
This commit is contained in:
Lucio Lelii 2026-09-14 12:34:42 +02:00
parent 4516105d74
commit 818f9c28e1
2 changed files with 39 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/main/java/it/cnr/isti/workflow/manager/mcp/MCPAgentService.java
View File

@ -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 -> {

32
src/test/java/it/cnr/isti/workflow/manager/mcp/MCPAgentServiceTest.java
View File

@ -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(),