Expose database load/unload across all user-facing surfaces#153
Merged
Conversation
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.
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.
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.
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.
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.
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The engine already loads and unloads databases at runtime (
Database.Manager,REST
POST /api/v1/db/{name}/load|unload), but those endpoints lived only inRoutes.hs— invisible to MCP, the CLI, pyvolca and the OpenAPI spec.Loading/unloading changes which databases are in the working set, not the
read-only data itself, so it is a legitimate analyst action rather than an
infrastructure-only endpoint. This promotes both operations to the canonical
Resourceregistry, which drives every surface from one definition.Final state
load_database/unload_databasetools, each taking adatabaseargument. The load handler catches exceptions (a fresh load parses from disk
and can throw) and surfaces them as a tool error instead of crashing.
database load <db>/database unload <db>, in both local managerand HTTP-client modes.
load_database/unload_databasemethods now route throughthe generic operationId dispatcher instead of bespoke direct-HTTP wrappers.
The underlying load/unload logic (dependency auto-loading, refusal to unload a
database others still depend on) is unchanged — this is purely surface exposure.
A dispatch test pins the registry-vs-dispatch invariant that a tool-name typo
would otherwise hide until runtime.