diff --git a/CHANGELOG.md b/CHANGELOG.md index 36c42777..b3f11414 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,19 @@ ## [Unreleased] +### Fixed +- A `GET /mcp` is now refused with 405 instead of answered with an empty + stream. That stream is how a server speaks to a client unprompted; VoLCA + never speaks first, so it was returned already closed, which a client reads + as a dropped connection and reconnects at once. Server and client then loop + for as long as both are up: one such pair sent 71 644 requests in 21 hours, + and on an engine holding several gigabytes of loaded data each wake cost a + full garbage collection, so the engine burned two and a half cores doing + nothing. A 405 says there is no stream to open, and the client stops + asking, and it is what the protocol asks of a server that offers no such + stream. This reads as a fix and not a removal: no working client relied + on the old answer, the one observed behaviour was the loop. + ## [0.10.0] - 2026-08-22 ### Added diff --git a/src/API/MCP.hs b/src/API/MCP.hs index 1a69da84..fb43d593 100644 --- a/src/API/MCP.hs +++ b/src/API/MCP.hs @@ -51,7 +51,7 @@ import qualified Method.Explain as Explain import Method.Mapping (LCIAOutcome (..), MappingStats (..), SimilarCF (..), SimilarReason (..), UncharacterizedFlow (..), applyLongTermMode, computeLCIAScoreAuto, computeLCIAScoreFromTables, computeMappingStats, defaultUncharacterizedOpts, inventoryContributions, longTermModeFromExclude) import qualified Method.Mapping as Mapping import Method.Types (FlowDirection (..), Method (..), MethodCF (..), MethodCollection (..), ScoringSet (..)) -import Network.HTTP.Types.Header (RequestHeaders, hAccept, hHost) +import Network.HTTP.Types.Header (RequestHeaders, hAccept, hAllow, hHost) import Numeric (showFFloat) import Progress (ProgressLevel (Warning), reportProgress) import qualified Service @@ -177,18 +177,6 @@ mcpApp dbManager presets hasFrontend mHosting mName markActivity = do wantsSse = "text/event-stream" `BS.isInfixOf` acceptHdr st <- readIORef stateRef case method of - -- GET: open SSE stream for server-initiated messages. - -- VoLCA is stateless so we return an empty stream immediately. - "GET" -> - respond $ - responseLBS - status200 - [ (hContentType, "text/event-stream; charset=utf-8") - , ("Cache-Control", "no-cache") - , ("Connection", "keep-alive") - , ("Mcp-Session-Id", TE.encodeUtf8 (mcpSessionId st)) - ] - "" "POST" -> do body <- strictRequestBody req case eitherDecode body of @@ -210,11 +198,18 @@ mcpApp dbManager presets hasFrontend mHosting mName markActivity = do if wantsSse then respond $ sseResponse (mcpSessionId st) val else respond $ jsonResponse (mcpSessionId st) val + -- Everything else, GET included. A GET opens the stream a server + -- uses to speak first; VoLCA never does, and answering it with a + -- stream that closes at once reads to a client as a dropped + -- connection, which it reconnects, forever. 405 says there is no + -- stream to open, and the client stops asking. _ -> - respond $ - responseLBS status405 [(hContentType, "application/json")] $ - encode $ - rpcError Null (-32700) "Method not allowed" + respond + $ responseLBS + status405 + [(hContentType, "application/json"), (hAllow, "POST")] + $ encode + $ rpcError Null (-32700) "Method not allowed" where jsonResponse sid v = responseLBS diff --git a/test/MCPStreamSpec.hs b/test/MCPStreamSpec.hs new file mode 100644 index 00000000..c346a36b --- /dev/null +++ b/test/MCPStreamSpec.hs @@ -0,0 +1,46 @@ +{-# LANGUAGE OverloadedStrings #-} + +{- | The @\/mcp@ endpoint answers POST and refuses everything else. + +A GET opens the stream a server uses to speak to a client unprompted. VoLCA +never speaks first, and an empty stream closed at once reads to a client as a +dropped connection: it reconnects, and the pair loops for as long as both are +up. One such pair sent 71 644 GETs in 21 hours. 405 says there is no stream, +and the client stops asking. +-} +module MCPStreamSpec (spec) where + +import Data.ByteString (ByteString) +import Data.IORef +import Network.HTTP.Types (Method) +import Network.HTTP.Types.Header (hAllow) +import Network.HTTP.Types.Status (statusCode) +import Network.Wai (defaultRequest, requestMethod, responseHeaders, responseStatus) +import Network.Wai.Internal (ResponseReceived (..)) +import Test.Hspec + +import API.MCP (mcpApp) +import Config (defaultConfig) +import Database.Manager (initDatabaseManager) + +{- | Drive one request of the given method through the endpoint, +report its status and the methods it says are allowed. +-} +answer :: Method -> IO (Int, Maybe ByteString) +answer m = do + manager <- initDatabaseManager defaultConfig True + app <- mcpApp manager [] False Nothing Nothing (pure ()) + ref <- newIORef Nothing + _ <- app defaultRequest{requestMethod = m} $ \resp -> do + writeIORef ref (Just resp) + pure ResponseReceived + let read' resp = (statusCode (responseStatus resp), lookup hAllow (responseHeaders resp)) + maybe (fail "no response") (pure . read') =<< readIORef ref + +spec :: Spec +spec = describe "the /mcp endpoint" $ do + it "refuses a GET rather than hand back a stream that closes at once" $ + answer "GET" `shouldReturn` (405, Just "POST") + + it "refuses a DELETE the same way" $ + answer "DELETE" `shouldReturn` (405, Just "POST") diff --git a/volca.cabal b/volca.cabal index 09066997..a2829a38 100644 --- a/volca.cabal +++ b/volca.cabal @@ -296,6 +296,7 @@ test-suite lca-tests , ILCDWriterSpec , MCPSchemaSpec , MCPDispatchSpec + , MCPStreamSpec , BatchImpactsSpec , MCPEnrichSpec , MCPColumnarSpec