From 8fb5d7ab43c9b15533d36ab55e395de5f543722d Mon Sep 17 00:00:00 2001 From: Christophe Combelles Date: Thu, 25 Jun 2026 18:50:06 +0200 Subject: [PATCH 1/7] Expose database load/unload on the resource registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Loading and unloading a database change which databases are in the working set, not the (read-only) data — an analyst action, not infrastructure. Add LoadDatabase/UnloadDatabase to the canonical Resource registry so they reach MCP, CLI and OpenAPI from one definition instead of living only as REST routes. The MCP load handler catches exceptions the way the REST handler does (a fresh load parses from disk and can throw) and surfaces them as a tool error. A dispatch test pins the registry-vs-dispatch invariant that a tool-name typo would otherwise hide until runtime. --- src/API/MCP.hs | 44 ++++++++++++++++++++++- src/API/Resources.hs | 30 +++++++++++++--- test/MCPDispatchSpec.hs | 77 +++++++++++++++++++++++++++++++++++++++++ volca.cabal | 1 + 4 files changed, 147 insertions(+), 5 deletions(-) create mode 100644 test/MCPDispatchSpec.hs diff --git a/src/API/MCP.hs b/src/API/MCP.hs index 004fd20c..3a0fd586 100644 --- a/src/API/MCP.hs +++ b/src/API/MCP.hs @@ -1,13 +1,15 @@ {-# LANGUAGE OverloadedStrings #-} +{-# LANGUAGE ScopedTypeVariables #-} {- | MCP (Model Context Protocol) server endpoint. Implements Streamable HTTP transport (MCP spec 2025-03-26). POST /mcp handles initialize, tools/list, tools/call (JSON or SSE response). GET /mcp opens an SSE stream for server-initiated messages (stateless: closes immediately). -} -module API.MCP (mcpApp, toolDefinitions, selectMethod) where +module API.MCP (mcpApp, toolDefinitions, callTool, selectMethod) where import Control.Concurrent.STM (readTVarIO) +import Control.Exception (SomeException, try) import Data.Aeson import Data.Aeson.Key (fromText) import Data.Aeson.KeyMap (KeyMap) @@ -337,6 +339,8 @@ parseCallParams _ = Nothing callTool :: DatabaseManager -> [ClassificationPreset] -> Maybe Text -> Value -> Text -> KeyMap Value -> IO Value callTool dbManager presets mBaseUrl rid name args = case name of "list_databases" -> callListDatabases dbManager rid + "load_database" -> callLoadDatabase dbManager rid args + "unload_database" -> callUnloadDatabase dbManager rid args "list_presets" -> callListPresets presets rid "search_activities" -> withDb dbManager rid args $ callSearchActivities presets rid args "search_flows" -> withDb dbManager rid args $ callSearchFlows rid args @@ -490,6 +494,44 @@ callListDatabases dbManager rid = do entries = map mkDbEntry (M.elems loaded) return $ toolSuccessJson rid $ object ["databases" .= entries] +{- | Load a configured database into the working set. Wraps +'DM.loadDatabase', which also auto-loads declared dependencies. Any +dependency that fails to load is surfaced in the 'dependencies' array +(as a DepLoadFailed entry) rather than swallowed. +-} +callLoadDatabase :: DatabaseManager -> Value -> KeyMap Value -> IO Value +callLoadDatabase dbManager rid args = runTool rid $ do + dbName <- except (requireText "database" args) + -- A fresh load parses/reads from disk and can throw, so catch like the + -- REST handler does and surface it as a tool error rather than crashing. + res <- liftIO $ try (DM.loadDatabase dbManager dbName) + (_loaded, deps) <- case res of + Left (e :: SomeException) -> throwE ("Load failed: " <> T.pack (show e)) + Right (Left err) -> throwE err + Right (Right ok) -> pure ok + pure $ + toolSuccessJson rid $ + object + [ "status" .= ("loaded" :: Text) + , "database" .= dbName + , "dependencies" .= deps + ] + +{- | Unload a database from the working set. Wraps 'DM.unloadDatabase', +which refuses (returns 'Left') when another loaded database still +depends on it. +-} +callUnloadDatabase :: DatabaseManager -> Value -> KeyMap Value -> IO Value +callUnloadDatabase dbManager rid args = runTool rid $ do + dbName <- except (requireText "database" args) + ExceptT (DM.unloadDatabase dbManager dbName) + pure $ + toolSuccessJson rid $ + object + [ "status" .= ("unloaded" :: Text) + , "database" .= dbName + ] + callListPresets :: [ClassificationPreset] -> Value -> IO Value callListPresets presets rid = return $ diff --git a/src/API/Resources.hs b/src/API/Resources.hs index ebaab6be..614057ca 100644 --- a/src/API/Resources.hs +++ b/src/API/Resources.hs @@ -38,13 +38,17 @@ import Network.HTTP.Types.Method (StdMethod (..)) {- | Every operation VoLCA exposes through its user-facing surfaces. -Note: this covers the *queryable* operations. Infrastructure endpoints -(database load/unload, method-collection management, auth, version) are -not represented here — they live in Routes.hs only, because they have -no analyst-facing equivalent across all surfaces. +This covers the analyst-facing operations. Loading and unloading a database +belong here too: they change which databases are in the working set, not the +(read-only) data itself, so they are a legitimate analyst action and are +exposed on every surface. Genuinely state-mutating or infrastructure endpoints +(method-collection management, upload/delete/relink/copy, auth, version) stay +in Routes.hs only — they have no analyst-facing equivalent across all surfaces. -} data Resource = ListDatabases + | LoadDatabase + | UnloadDatabase | ListPresets | SearchActivities | SearchFlows @@ -124,6 +128,8 @@ spec. A build-time drift test (ResourcesDriftSpec) asserts that every apiPath :: Resource -> Maybe (StdMethod, [Text]) apiPath r = case r of ListDatabases -> Just (GET, ["db"]) + LoadDatabase -> Just (POST, ["db", "{dbName}", "load"]) + UnloadDatabase -> Just (POST, ["db", "{dbName}", "unload"]) ListPresets -> Just (GET, ["classification-presets"]) SearchActivities -> Just (GET, ["db", "{dbName}", "activities"]) SearchFlows -> Just (GET, ["db", "{dbName}", "flows"]) @@ -164,6 +170,8 @@ apiPathText r = do mcpName :: Resource -> Text mcpName r = case r of ListDatabases -> "list_databases" + LoadDatabase -> "load_database" + UnloadDatabase -> "unload_database" ListPresets -> "list_presets" SearchActivities -> "search_activities" SearchFlows -> "search_flows" @@ -200,6 +208,8 @@ not a top-level command). For those, 'cliName' returns the nested form. cliName :: Resource -> Text cliName r = case r of ListDatabases -> "database list" + LoadDatabase -> "database load" + UnloadDatabase -> "database unload" ListPresets -> "presets" SearchActivities -> "activities" SearchFlows -> "flows" @@ -249,6 +259,16 @@ description r = case r of ListDatabases -> "LCA / ACV — list all loaded LCA databases (Agribalyse, ecoinvent, …). \ \Call this first to discover which databases are available before searching." + LoadDatabase -> + "LCA / ACV — load a configured database into memory so it can be queried. \ + \Its declared dependencies are loaded first (needed for cross-database flow \ + \linking). A database must be loaded before search/score/impact tools can \ + \target it; use list_databases to see which are configured. No effect if it \ + \is already loaded." + UnloadDatabase -> + "LCA / ACV — unload a database from memory to free RAM. The on-disk data is \ + \kept and the database can be reloaded later with load_database. Refuses if \ + \another loaded database still depends on it — unload the dependents first." ListPresets -> "LCA / ACV — list named classification filter presets configured in this \ \instance. Each preset bundles multiple (system, value, mode) classification \ @@ -516,6 +536,8 @@ pSummaryOnly = params :: Resource -> [Param] params r = case r of ListDatabases -> [] + LoadDatabase -> [pDatabase] + UnloadDatabase -> [pDatabase] ListPresets -> [] SearchActivities -> [ pDatabase diff --git a/test/MCPDispatchSpec.hs b/test/MCPDispatchSpec.hs new file mode 100644 index 00000000..57f33e28 --- /dev/null +++ b/test/MCPDispatchSpec.hs @@ -0,0 +1,77 @@ +{-# LANGUAGE OverloadedStrings #-} + +{- | The database load/unload MCP tools are advertised in 'toolDefinitions' +*and* routed by 'callTool'. The two live in different places (the resource +registry vs. the dispatch case), so a name typo would compile yet strand a +tool at runtime with an "Unknown tool" reply. These tests pin both ends. +-} +module MCPDispatchSpec (spec) where + +import Data.Aeson (Value (..)) +import qualified Data.Aeson.KeyMap as KM +import Data.Foldable (toList) +import Data.Maybe (listToMaybe) +import Data.Text (Text) +import qualified Data.Text as T +import Test.Hspec + +import API.MCP (callTool, toolDefinitions) +import Config (defaultConfig) +import Database.Manager (initDatabaseManager) + +-- | The tool definition advertised under a given MCP name. +toolByName :: Text -> Maybe Value +toolByName name = + listToMaybe + [t | t@(Object o) <- toolDefinitions, KM.lookup "name" o == Just (String name)] + +-- | The 'required' parameter names declared in a tool's input schema. +requiredOf :: Value -> [Text] +requiredOf (Object o) = case KM.lookup "inputSchema" o of + Just (Object s) -> case KM.lookup "required" s of + Just (Array arr) -> [t | String t <- toList arr] + _ -> [] + _ -> [] +requiredOf _ = [] + +-- | The text payload of a tool reply (@result.content[0].text@). +resultText :: Value -> Text +resultText (Object o) = case KM.lookup "result" o of + Just (Object r) -> case KM.lookup "content" r of + Just (Array arr) -> case toList arr of + (Object c : _) -> case KM.lookup "text" c of + Just (String t) -> t + _ -> "" + _ -> "" + _ -> "" + _ -> "" +resultText _ = "" + +-- | Whether a tool reply is flagged as an error. +isError :: Value -> Bool +isError (Object o) = case KM.lookup "result" o of + Just (Object r) -> KM.lookup "isError" r == Just (Bool True) + _ -> False +isError _ = False + +call :: Text -> IO Value +call name = do + manager <- initDatabaseManager defaultConfig True Nothing + callTool manager [] Nothing Null name (KM.singleton "database" (String "no-such-db")) + +spec :: Spec +spec = describe "MCP database load/unload tools" $ do + it "are advertised with a required 'database' parameter" $ do + fmap requiredOf (toolByName "load_database") `shouldBe` Just ["database"] + fmap requiredOf (toolByName "unload_database") `shouldBe` Just ["database"] + + it "are routed by callTool (no 'Unknown tool' gap)" $ do + loadResp <- call "load_database" + unloadResp <- call "unload_database" + resultText loadResp `shouldNotSatisfy` ("Unknown tool:" `T.isPrefixOf`) + resultText unloadResp `shouldNotSatisfy` ("Unknown tool:" `T.isPrefixOf`) + + it "surface the engine error when unloading a database that is not loaded" $ do + resp <- call "unload_database" + isError resp `shouldBe` True + resultText resp `shouldSatisfy` ("Database not loaded:" `T.isInfixOf`) diff --git a/volca.cabal b/volca.cabal index f89ceee1..cff81ac3 100644 --- a/volca.cabal +++ b/volca.cabal @@ -271,6 +271,7 @@ test-suite lca-tests , SimaProWriterSpec , ILCDWriterSpec , MCPSchemaSpec + , MCPDispatchSpec , BatchImpactsSpec , MCPEnrichSpec , MCPColumnarSpec From a99b8582286e36ed5289b9a10084076a28e24feb Mon Sep 17 00:00:00 2001 From: Christophe Combelles Date: Thu, 25 Jun 2026 18:50:16 +0200 Subject: [PATCH 2/7] Add CLI database load/unload subcommands Wire the two new resources through the CLI on the model of `database copy`: local manager mode loads/unloads in place; HTTP client mode posts to the running engine. --- src/CLI/Client.hs | 7 +++++++ src/CLI/Command.hs | 32 ++++++++++++++++++++++++++++++++ src/CLI/Parser.hs | 2 ++ src/CLI/Types.hs | 4 ++++ 4 files changed, 45 insertions(+) diff --git a/src/CLI/Client.hs b/src/CLI/Client.hs index 70500e1c..d23e6308 100644 --- a/src/CLI/Client.hs +++ b/src/CLI/Client.hs @@ -82,6 +82,13 @@ executeRemoteCommand mgr rc globalOpts cmd = do case cmd of Database DbList -> apiGet mgr rc "/api/v1/db" >>= output fmt jp + Database (DbLoad name) -> + -- Load endpoint takes no body; the empty object is ignored server-side. + apiPost mgr rc ("/api/v1/db/" ++ T.unpack name ++ "/load") (object []) + >>= output fmt jp + Database (DbUnload name) -> + apiPost mgr rc ("/api/v1/db/" ++ T.unpack name ++ "/unload") (object []) + >>= outputStatus fmt jp "unload" Database (DbUpload args) -> executeUpload mgr rc fmt jp "/api/v1/db/upload" args Database (DbDelete name) -> diff --git a/src/CLI/Command.hs b/src/CLI/Command.hs index 661aad0b..e9e89d2b 100644 --- a/src/CLI/Command.hs +++ b/src/CLI/Command.hs @@ -97,6 +97,10 @@ executeCommand (CLIConfig globalOpts _) cmd manager = do exitFailure Database DbList -> DM.listDatabases manager >>= out . toJSON + Database (DbLoad name) -> + executeDbLoad registry outputFormat manager name + Database (DbUnload name) -> + executeDbUnload registry outputFormat manager name Database (DbUpload args) -> executeDbUpload registry outputFormat manager args Database (DbDelete name) -> @@ -513,6 +517,34 @@ executeDbDeleteActivities registry fmt manager args = do reportProgress Info $ "Deleted " ++ show deleted ++ " activities from " ++ T.unpack (ddaDb args) outputResult registry fmt $ object ["database" .= ddaDb args, "deleted" .= deleted] +{- | Execute database load: bring a configured database (and its declared +dependencies) into memory. Any failed dependency is surfaced in the output. +-} +executeDbLoad :: PluginRegistry -> OutputFormat -> DatabaseManager -> Text -> IO () +executeDbLoad registry fmt manager name = do + result <- DM.loadDatabase manager name + case result of + Left err -> do + reportError $ "Load failed: " ++ T.unpack err + exitFailure + Right (_loaded, deps) -> do + reportProgress Info $ "Loaded database: " ++ T.unpack name + outputResult registry fmt $ object ["loaded" .= name, "dependencies" .= deps] + +{- | Execute database unload: drop a database from memory (refused while another +loaded database still depends on it). +-} +executeDbUnload :: PluginRegistry -> OutputFormat -> DatabaseManager -> Text -> IO () +executeDbUnload registry fmt manager name = do + result <- DM.unloadDatabase manager name + case result of + Left err -> do + reportError $ "Unload failed: " ++ T.unpack err + exitFailure + Right () -> do + reportProgress Info $ "Unloaded database: " ++ T.unpack name + outputResult registry fmt $ object ["unloaded" .= name] + -- | Execute database copy command executeDbCopy :: PluginRegistry -> OutputFormat -> DatabaseManager -> Text -> Text -> IO () executeDbCopy registry fmt manager srcName newName = do diff --git a/src/CLI/Parser.hs b/src/CLI/Parser.hs index a6745e3e..619910c4 100644 --- a/src/CLI/Parser.hs +++ b/src/CLI/Parser.hs @@ -115,6 +115,8 @@ databaseParser = <$> optional ( subparser ( OA.command "list" (info (pure DbList) (progDesc "List databases")) + <> OA.command "load" (info (DbLoad <$> textArg "DB" "Name of the configured database to load" <**> helper) (progDesc "Load a configured database into memory")) + <> OA.command "unload" (info (DbUnload <$> textArg "DB" "Name of the loaded database to unload" <**> helper) (progDesc "Unload a database from memory")) <> OA.command "upload" (info (DbUpload <$> uploadArgsParser) (progDesc "Upload a database from a local file")) <> OA.command "delete" (info (DbDelete <$> deleteNameParser) (progDesc "Delete a database")) <> OA.command "delete-activities" (info (DbDeleteActivities <$> deleteActivitiesArgsParser <**> helper) (progDesc "Delete the whole filtered set of activities from a loaded database")) diff --git a/src/CLI/Types.hs b/src/CLI/Types.hs index 2ea0e234..209c131d 100644 --- a/src/CLI/Types.hs +++ b/src/CLI/Types.hs @@ -62,6 +62,10 @@ data Command -- | Database management actions data DatabaseAction = DbList + | -- | Load a configured database into memory (auto-loads its dependencies) + DbLoad Text + | -- | Unload a database from memory (refused if a loaded database depends on it) + DbUnload Text | DbUpload UploadArgs | DbDelete Text | {- | Delete the activities matched by a filter (the whole matching set, From 9c9d0fed1bd7fcc85e4aca88d9136662df20aad6 Mon Sep 17 00:00:00 2001 From: Christophe Combelles Date: Thu, 25 Jun 2026 18:50:16 +0200 Subject: [PATCH 3/7] pyvolca: route load/unload through the operationId dispatcher load_database / unload_database now carry an operationId, so replace their bespoke direct-HTTP wrappers with the generic _call path and guard them in the drift test. README api-reference block regenerated. --- pyvolca/README.md | 5 ++++- pyvolca/src/volca/client.py | 13 ++++++++----- pyvolca/tests/test_drift.py | 11 +++++++---- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/pyvolca/README.md b/pyvolca/README.md index ccfd8d18..315b7810 100644 --- a/pyvolca/README.md +++ b/pyvolca/README.md @@ -638,7 +638,8 @@ to filtering endpoints. Load a database into memory so it answers queries. -Has no effect if the database is already loaded. +Declared dependencies are loaded first; has no effect if the +database is already loaded. ##### `Client.refresh_stubs()` @@ -719,6 +720,8 @@ Args: Unload a database from memory to free RAM. The disk copy is kept. +Refused if another loaded database still depends on it. + ##### `Client.use(db_name: str) -> 'Client'` Return a new client targeting a different database. diff --git a/pyvolca/src/volca/client.py b/pyvolca/src/volca/client.py index 87351995..b45eed14 100644 --- a/pyvolca/src/volca/client.py +++ b/pyvolca/src/volca/client.py @@ -626,14 +626,17 @@ def list_databases(self) -> list[DatabaseInfo]: def load_database(self, db_name: str) -> dict: """Load a database into memory so it answers queries. - Has no effect if the database is already loaded. + Declared dependencies are loaded first; has no effect if the + database is already loaded. """ - # No operationId (infrastructure endpoint). Direct HTTP. - return self._json(self._session.post(f"{self.base_url}/api/v1/db/{db_name}/load")) + return self._call("load_database", db_name=db_name) def unload_database(self, db_name: str) -> dict: - """Unload a database from memory to free RAM. The disk copy is kept.""" - return self._json(self._session.post(f"{self.base_url}/api/v1/db/{db_name}/unload")) + """Unload a database from memory to free RAM. The disk copy is kept. + + Refused if another loaded database still depends on it. + """ + return self._call("unload_database", db_name=db_name) # -- Database write operations -- # diff --git a/pyvolca/tests/test_drift.py b/pyvolca/tests/test_drift.py index b5878629..073f9954 100644 --- a/pyvolca/tests/test_drift.py +++ b/pyvolca/tests/test_drift.py @@ -21,6 +21,8 @@ # and engine-side renames. WRAPPER_OPERATIONS = [ "list_databases", + "load_database", + "unload_database", "list_presets", "search_activities", "search_flows", @@ -68,9 +70,10 @@ def test_all_resources_have_operationid(live_spec): if live_spec is None: pytest.skip("No built engine binary found.") live_ops = _parse_spec(live_spec) - # We expect at least the 17 Resources that have an apiPath entry - # (list_geographies is MCP-only and correctly has no operationId). - assert len(live_ops) >= 17, ( + # We expect at least the 19 Resources that have an apiPath entry + # (list_geographies / compare_impacts / list_scoring_sets are MCP-only and + # correctly have no operationId). + assert len(live_ops) >= 19, ( f"Only {len(live_ops)} operations have operationIds in the live spec. " - f"Expected ≥17. Check API/OpenApi.hs enrichWithResources." + f"Expected ≥19. Check API/OpenApi.hs enrichWithResources." ) From fa275edd08b47fc9e2591fa989783561937a2c89 Mon Sep 17 00:00:00 2001 From: Christophe Combelles Date: Thu, 25 Jun 2026 20:50:49 +0200 Subject: [PATCH 4/7] Catch load-time exceptions inside loadDatabase A fresh load parses from disk and can throw, so the REST and MCP handlers each wrapped the call in their own `try`; the CLI handler didn't, so a throwing load crashed the process instead of reporting a clean error. Fold the exception into the `Left` that `loadDatabase` already returns, so every surface gets one total `Either` and the two duplicated catches collapse to a plain two-arm match. --- src/API/DatabaseHandlers.hs | 13 +++++-------- src/API/MCP.hs | 10 +--------- src/Database/Manager.hs | 34 +++++++++++++++++++++------------- 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/src/API/DatabaseHandlers.hs b/src/API/DatabaseHandlers.hs index bcf5e82a..fc69b491 100644 --- a/src/API/DatabaseHandlers.hs +++ b/src/API/DatabaseHandlers.hs @@ -170,14 +170,11 @@ getDatabases = do loadDatabaseHandler :: Text -> AppM LoadDatabaseResponse loadDatabaseHandler dbName = do dbManager <- asks aeDbManager - eitherResult <- liftIO $ try $ loadDatabase dbManager dbName - case eitherResult of - Left (ex :: SomeException) -> - return $ LoadFailed ("Server exception: " <> T.pack (show ex)) - Right (Left err) -> return $ LoadFailed err - Right (Right (loadedDb, depResults)) -> do - let status = makeStatusFromLoadedDb loadedDb - return $ LoadSucceeded status depResults + result <- liftIO $ loadDatabase dbManager dbName + case result of + Left err -> return $ LoadFailed err + Right (loadedDb, depResults) -> + return $ LoadSucceeded (makeStatusFromLoadedDb loadedDb) depResults -- | Unload a database from memory unloadDatabaseHandler :: Text -> AppM ActivateResponse diff --git a/src/API/MCP.hs b/src/API/MCP.hs index 3a0fd586..e795832a 100644 --- a/src/API/MCP.hs +++ b/src/API/MCP.hs @@ -1,5 +1,4 @@ {-# LANGUAGE OverloadedStrings #-} -{-# LANGUAGE ScopedTypeVariables #-} {- | MCP (Model Context Protocol) server endpoint. Implements Streamable HTTP transport (MCP spec 2025-03-26). @@ -9,7 +8,6 @@ GET /mcp opens an SSE stream for server-initiated messages (stateless: closes i module API.MCP (mcpApp, toolDefinitions, callTool, selectMethod) where import Control.Concurrent.STM (readTVarIO) -import Control.Exception (SomeException, try) import Data.Aeson import Data.Aeson.Key (fromText) import Data.Aeson.KeyMap (KeyMap) @@ -502,13 +500,7 @@ dependency that fails to load is surfaced in the 'dependencies' array callLoadDatabase :: DatabaseManager -> Value -> KeyMap Value -> IO Value callLoadDatabase dbManager rid args = runTool rid $ do dbName <- except (requireText "database" args) - -- A fresh load parses/reads from disk and can throw, so catch like the - -- REST handler does and surface it as a tool error rather than crashing. - res <- liftIO $ try (DM.loadDatabase dbManager dbName) - (_loaded, deps) <- case res of - Left (e :: SomeException) -> throwE ("Load failed: " <> T.pack (show e)) - Right (Left err) -> throwE err - Right (Right ok) -> pure ok + (_loaded, deps) <- ExceptT (DM.loadDatabase dbManager dbName) pure $ toolSuccessJson rid $ object diff --git a/src/Database/Manager.hs b/src/Database/Manager.hs index 7294eb08..504307bf 100644 --- a/src/Database/Manager.hs +++ b/src/Database/Manager.hs @@ -1880,19 +1880,27 @@ Pre-loads declared dependencies (from TOML config) so cross-DB linking works, then loads the target database. -} loadDatabase :: DatabaseManager -> Text -> IO (Either Text (LoadedDatabase, [DepLoadResult])) -loadDatabase manager dbName = do - -- Pre-load declared dependencies so they're available for cross-DB linking - availableDbs <- readTVarIO (dmAvailableDbs manager) - let configDeps = maybe [] dcDepends (M.lookup dbName availableDbs) - depResults1 <- autoLoadDeps manager configDeps - - result <- loadDatabaseSingle manager dbName - case result of - Left err -> return (Left err) - Right loaded -> do - -- Also auto-load any runtime-discovered dependencies - depResults2 <- autoLoadDeps manager (dbDependsOn (ldDatabase loaded)) - return (Right (loaded, depResults1 ++ depResults2)) +loadDatabase manager dbName = fmap flattenLoad (try go) + where + -- A fresh load parses/reads from disk and can throw. Fold any exception + -- into the Left this function already returns, so every surface (REST, + -- MCP, CLI) gets one total Either to handle instead of each having to + -- remember its own catch. + flattenLoad :: Either SomeException (Either Text a) -> Either Text a + flattenLoad = either (Left . T.pack . show) id + go = do + -- Pre-load declared dependencies so they're available for cross-DB linking + availableDbs <- readTVarIO (dmAvailableDbs manager) + let configDeps = maybe [] dcDepends (M.lookup dbName availableDbs) + depResults1 <- autoLoadDeps manager configDeps + + result <- loadDatabaseSingle manager dbName + case result of + Left err -> return (Left err) + Right loaded -> do + -- Also auto-load any runtime-discovered dependencies + depResults2 <- autoLoadDeps manager (dbDependsOn (ldDatabase loaded)) + return (Right (loaded, depResults1 ++ depResults2)) {- | Stage an uploaded database (parse + cross-DB link, no matrices yet) When a valid cache exists, reconstructs staged state from the cached Database From 1f24db55c3be02424006e3dcdf92619d9ccd35c7 Mon Sep 17 00:00:00 2001 From: Christophe Combelles Date: Thu, 25 Jun 2026 20:51:03 +0200 Subject: [PATCH 5/7] Fail loudly on a failed remote `database load` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The load endpoint returns HTTP 200 even on failure, with a bare `{"error": …}` body and no `success` field. The remote CLI piped that through plain `output`, which printed the error and exited 0 — unlike local mode and remote `unload`, which both exit non-zero. Add `outputLoad`, mirroring `outputStatus`: fail loudly when the error key is present so a failed remote load exits non-zero. --- src/CLI/Client.hs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/CLI/Client.hs b/src/CLI/Client.hs index d23e6308..445a33ed 100644 --- a/src/CLI/Client.hs +++ b/src/CLI/Client.hs @@ -85,7 +85,7 @@ executeRemoteCommand mgr rc globalOpts cmd = do Database (DbLoad name) -> -- Load endpoint takes no body; the empty object is ignored server-side. apiPost mgr rc ("/api/v1/db/" ++ T.unpack name ++ "/load") (object []) - >>= output fmt jp + >>= outputLoad fmt jp Database (DbUnload name) -> apiPost mgr rc ("/api/v1/db/" ++ T.unpack name ++ "/unload") (object []) >>= outputStatus fmt jp "unload" @@ -390,6 +390,20 @@ outputStatus fmt jp action (Right val) = case parseMaybe parseStatus val of parseStatus :: Value -> Parser (Bool, Text) parseStatus = withObject "StatusResponse" $ \o -> (,) <$> o .: "success" <*> o .: "message" +{- | @database load@ returns HTTP 200 even on failure, with a bare +@{"error": …}@ body ('LoadDatabaseResponse'\'s @LoadFailed@) and no @success@ +field for 'outputStatus' to check. Mirror 'outputStatus': fail loudly when that +key is present, so a failed remote load exits non-zero instead of printing the +error and returning success. +-} +outputLoad :: OutputFormat -> Maybe Text -> Either String Value -> IO () +outputLoad _ _ (Left err) = reportError err >> exitFailure +outputLoad fmt jp (Right val) = case val of + Object o + | Just (String err) <- KM.lookup "error" o -> + reportError ("load failed: " ++ T.unpack err) >> exitFailure + _ -> output fmt jp (Right val) + -- | Format and output a result output :: OutputFormat -> Maybe Text -> Either String Value -> IO () output _ _ (Left err) = reportError err >> exitFailure From 20e70dd12ea75db83fdd227bf04d42342b4f3533 Mon Sep 17 00:00:00 2001 From: Christophe Combelles Date: Thu, 25 Jun 2026 20:51:09 +0200 Subject: [PATCH 6/7] Make MCP dispatch resultText total MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resultText returned "" on any unexpected reply shape, so the "no Unknown tool gap" assertion (shouldNotSatisfy … isPrefixOf) passed even on a malformed reply — the exact stranded-tool regression the test exists to catch. Return `Maybe Text` and assert presence so a broken envelope fails the test. --- test/MCPDispatchSpec.hs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/test/MCPDispatchSpec.hs b/test/MCPDispatchSpec.hs index 57f33e28..b449cfa4 100644 --- a/test/MCPDispatchSpec.hs +++ b/test/MCPDispatchSpec.hs @@ -34,18 +34,18 @@ requiredOf (Object o) = case KM.lookup "inputSchema" o of _ -> [] requiredOf _ = [] --- | The text payload of a tool reply (@result.content[0].text@). -resultText :: Value -> Text -resultText (Object o) = case KM.lookup "result" o of - Just (Object r) -> case KM.lookup "content" r of - Just (Array arr) -> case toList arr of - (Object c : _) -> case KM.lookup "text" c of - Just (String t) -> t - _ -> "" - _ -> "" - _ -> "" - _ -> "" -resultText _ = "" +{- | The text payload of a tool reply (@result.content[0].text@), or 'Nothing' +when the reply doesn't have that shape — so a malformed reply fails a test +instead of silently passing a @""@ that satisfies any "doesn't contain X". +-} +resultText :: Value -> Maybe Text +resultText v = do + Object o <- Just v + Object r <- KM.lookup "result" o + Array arr <- KM.lookup "content" r + Object c <- listToMaybe (toList arr) + String t <- KM.lookup "text" c + pure t -- | Whether a tool reply is flagged as an error. isError :: Value -> Bool @@ -68,10 +68,10 @@ spec = describe "MCP database load/unload tools" $ do it "are routed by callTool (no 'Unknown tool' gap)" $ do loadResp <- call "load_database" unloadResp <- call "unload_database" - resultText loadResp `shouldNotSatisfy` ("Unknown tool:" `T.isPrefixOf`) - resultText unloadResp `shouldNotSatisfy` ("Unknown tool:" `T.isPrefixOf`) + resultText loadResp `shouldSatisfy` maybe False (not . T.isPrefixOf "Unknown tool:") + resultText unloadResp `shouldSatisfy` maybe False (not . T.isPrefixOf "Unknown tool:") it "surface the engine error when unloading a database that is not loaded" $ do resp <- call "unload_database" isError resp `shouldBe` True - resultText resp `shouldSatisfy` ("Database not loaded:" `T.isInfixOf`) + resultText resp `shouldSatisfy` maybe False ("Database not loaded:" `T.isInfixOf`) From a976512a9a520c0a25ceab399219aae957a290bb Mon Sep 17 00:00:00 2001 From: Christophe Combelles Date: Thu, 25 Jun 2026 20:51:16 +0200 Subject: [PATCH 7/7] Pin the operationId drift bound to the real count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The live spec stamps 22 operationIds (22 of 25 Resources carry an apiPath; 3 are MCP-only), but the backstop asserted only `>= 19` and its comment miscounted — it would silently tolerate up to 3 dropped operationIds from a path-template mismatch. Assert `>= 22`. Also drop the stale test_db_write docstring clause: load/unload now route through the operationId dispatcher, not direct URLs. --- pyvolca/tests/test_db_write.py | 8 ++++---- pyvolca/tests/test_drift.py | 11 ++++++----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/pyvolca/tests/test_db_write.py b/pyvolca/tests/test_db_write.py index 45ab03a8..4065b1e4 100644 --- a/pyvolca/tests/test_db_write.py +++ b/pyvolca/tests/test_db_write.py @@ -2,10 +2,10 @@ These endpoints (copy / delete / relink / export / add-/remove-dependency) carry no operationId — they bypass the OpenAPI dispatcher and build their -URLs directly, like ``load_database`` / ``unload_database``. They also do -not exist in any released engine binary, so these tests never touch a live -engine: they mock ``Client._session`` and assert on the wire shape -(URL, JSON body, base64 decoding, format validation, error surfacing). +URLs directly. They also do not exist in any released engine binary, so these +tests never touch a live engine: they mock ``Client._session`` and assert on +the wire shape (URL, JSON body, base64 decoding, format validation, error +surfacing). """ from __future__ import annotations diff --git a/pyvolca/tests/test_drift.py b/pyvolca/tests/test_drift.py index 073f9954..b8a1f055 100644 --- a/pyvolca/tests/test_drift.py +++ b/pyvolca/tests/test_drift.py @@ -70,10 +70,11 @@ def test_all_resources_have_operationid(live_spec): if live_spec is None: pytest.skip("No built engine binary found.") live_ops = _parse_spec(live_spec) - # We expect at least the 19 Resources that have an apiPath entry - # (list_geographies / compare_impacts / list_scoring_sets are MCP-only and - # correctly have no operationId). - assert len(live_ops) >= 19, ( + # 22 of the 25 Resource constructors carry an apiPath, so each stamps an + # operationId; the 3 MCP-only ones (list_geographies / compare_impacts / + # list_scoring_sets) correctly have none. A looser bound would silently + # tolerate a path-template mismatch dropping an operationId. + assert len(live_ops) >= 22, ( f"Only {len(live_ops)} operations have operationIds in the live spec. " - f"Expected ≥19. Check API/OpenApi.hs enrichWithResources." + f"Expected ≥22. Check API/OpenApi.hs enrichWithResources." )