feat: reduce context complexity multi db#277
feat: reduce context complexity multi db#277MarlzRana wants to merge 9 commits intobytebase:mainfrom
Conversation
| import LockIcon from '../icons/LockIcon'; | ||
| import CopyIcon from '../icons/CopyIcon'; | ||
| import CheckIcon from '../icons/CheckIcon'; | ||
| import { useEffect, useState, useCallback, useRef } from "react"; |
There was a problem hiding this comment.
The linter ran on this file when I edited via VSCode - perhaps it is worth adding a Claude Code hook, to also do this after an edit.
There was a problem hiding this comment.
Most of the file change is the linter, but there are also functional changes.
| </Link> | ||
| <div className="flex-1 overflow-auto"> | ||
| {currentSource.tools | ||
| .filter((tool) => tool.name !== 'search_objects') |
There was a problem hiding this comment.
There is already a WIP notice in the tool detail - let's still present it in the sidebar.
|
Ready when you are @tianzhou! 😄 Let's first review the functional and test changes, then I'll get Claude to update the docs as well. |
| export function getExecuteSqlMetadata(sourceId: string): ToolMetadata { | ||
| const sourceIds = ConnectorManager.getAvailableSourceIds(); | ||
| const sourceConfig = ConnectorManager.getSourceConfig(sourceId)!; | ||
| export function getExecuteSqlMetadata(boundSourceId?: string): ToolMetadata { |
There was a problem hiding this comment.
it's strange that boundSourceId can be nil. Introducing list_source shouldn't change this behavior.
There was a problem hiding this comment.
Pull request overview
This PR reduces MCP tool “context bloat” in multi-source configurations by switching from per-source tool instances to a small set of generic tools that accept a source_id, plus a new list_sources meta-tool for discovery.
Changes:
- Add
list_sourcesmeta-tool and register only genericexecute_sql/search_objectsin multi-source mode. - Introduce multi-source Zod schemas that require
source_id, and adjust tool metadata generation accordingly. - Update backend/frontend tests and UI to handle generic tool names and
source_idbinding.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/tool-metadata.ts | Generates generic tool metadata + schemas for multi-source mode; improves Zod-to-parameter mapping. |
| src/utils/tests/tool-metadata.test.ts | Adds unit tests for new metadata + zodToParameters behavior. |
| src/tools/execute-sql.ts | Adds multi-source schema + source_id argument support in handler. |
| src/tools/search-objects.ts | Adds multi-source schema + source_id argument support in handler. |
| src/tools/list-sources.ts | New list_sources meta-tool implementation. |
| src/tools/index.ts | Registers generic tools + list_sources in multi-source mode; custom tools remain per-source. |
| src/tools/builtin-tools.ts | Introduces meta-tool constant and combined builtin/meta list. |
| src/tools/registry.ts | Updates builtin name handling and adds getCustomToolsForSource. |
| src/tools/tests/execute-sql.test.ts | Adds tests for source_id resolution precedence behavior. |
| src/tools/tests/registry.test.ts | Tests meta-tool naming conflict protection + custom tool filtering. |
| src/tools/tests/list-sources.test.ts | Tests list_sources registration and response shape. |
| src/config/toml-loader.ts | Updates builtin detection to include meta-tools. |
| src/api/tests/sources.integration.test.ts | Updates expectations for generic tool names + source_id parameter exposure. |
| frontend/src/components/views/ToolDetailView.tsx | Binds source_id automatically for generic tools; hides source_id from editable params. |
| frontend/src/components/views/SourceDetailView.tsx | Displays bound source_id parameter distinctly in tool parameter lists. |
| frontend/src/components/Sidebar/Sidebar.tsx | Shows search_objects in the sidebar tool list (no longer filtered out). |
Comments suppressed due to low confidence (1)
src/tools/execute-sql.ts:58
- In multi-source mode this handler can execute against a source where
execute_sqlis disabled in the ToolRegistry:getBuiltinToolConfig(...)will returnundefined, but the code still executes with no readonly/max_rows constraints. Please explicitly reject execution whentoolConfigis missing foractualSourceId(i.e., tool not enabled for that source).
// Get tool-specific configuration (tool is already registered, so it's enabled)
const registry = getToolRegistry();
const toolConfig = registry.getBuiltinToolConfig(BUILTIN_TOOL_EXECUTE_SQL, actualSourceId);
| @@ -499,25 +509,28 @@ export function createSearchDatabaseObjectsToolHandler(sourceId?: string) { | |||
| table, | |||
| detail_level = "names", | |||
| limit = 100, | |||
| source_id: argSourceId, | |||
| } = args as { | |||
| object_type: DatabaseObjectType; | |||
| pattern?: string; | |||
| schema?: string; | |||
| table?: string; | |||
| detail_level: DetailLevel; | |||
| limit: number; | |||
| source_id?: string; | |||
| }; | |||
|
|
|||
| const resolvedSourceId = boundSourceId ?? argSourceId; | |||
| const startTime = Date.now(); | |||
| const effectiveSourceId = getEffectiveSourceId(sourceId); | |||
| const effectiveSourceId = getEffectiveSourceId(resolvedSourceId); | |||
| let success = true; | |||
| let errorMessage: string | undefined; | |||
|
|
|||
| try { | |||
| // Ensure source is connected (handles lazy connections) | |||
| await ConnectorManager.ensureConnected(sourceId); | |||
| await ConnectorManager.ensureConnected(resolvedSourceId); | |||
|
|
|||
| const connector = ConnectorManager.getCurrentConnector(sourceId); | |||
| const connector = ConnectorManager.getCurrentConnector(resolvedSourceId); | |||
There was a problem hiding this comment.
resolvedSourceId may be undefined if the caller omits source_id, which will default to the first configured source via ensureConnected(undefined)/getCurrentConnector(undefined). In multi-source mode this is ambiguous and can lead to querying the wrong DB. Consider returning a validation error when multiple sources are configured and source_id is missing.
| // Unwrap ZodDefault/ZodOptional | ||
| if (innerType instanceof z.ZodDefault) { | ||
| required = false; | ||
| innerType = (innerType as z.ZodDefault<any>)._def.innerType; | ||
| } | ||
|
|
||
| if (innerType instanceof z.ZodOptional) { | ||
| required = false; | ||
| innerType = (innerType as z.ZodOptional<any>).unwrap(); | ||
| } |
There was a problem hiding this comment.
zodToParameters unwraps ZodDefault via the private ._def.innerType, which is not part of Zod’s public API and can break on upgrades. Prefer the public removeDefault() method (and then continue unwrapping optionals) to avoid relying on internals.
| // Parameters the user can actually fill in (excludes bound source_id) | ||
| const editableParameters = tool?.parameters.filter((p) => p.name !== "source_id") ?? []; |
There was a problem hiding this comment.
The UI treats any parameter named source_id as a bound, non-editable field by filtering it out of editableParameters. This will break any custom tool that legitimately defines a source_id parameter (it becomes impossible to set and will never be sent). Consider scoping this behavior to the built-ins (execute_sql/search_objects) in multi-source mode, or reserving/disallowing source_id as a custom-tool parameter name at config validation time.
| // Parameters the user can actually fill in (excludes bound source_id) | |
| const editableParameters = tool?.parameters.filter((p) => p.name !== "source_id") ?? []; | |
| // Parameters the user can actually fill in. | |
| // For non-custom tools in multi-source mode, source_id is treated as a bound param | |
| // and therefore excluded from the editable set. Custom tools may freely use source_id. | |
| const editableParameters = | |
| tool?.parameters.filter((p) => !(toolType !== "custom" && p.name === "source_id")) ?? []; |
| * Create an execute_sql tool handler for a specific source | ||
| * @param sourceId - The source ID this handler is bound to (undefined for single-source mode) | ||
| * @param boundSourceId - The source ID this handler is bound to (undefined for single-source mode) | ||
| * @returns A handler function bound to the specified source | ||
| */ |
There was a problem hiding this comment.
JSDoc says boundSourceId is undefined for single-source mode, but registerTools() passes a concrete source ID in single-source mode and undefined in multi-source mode (generic tool). Please update the comment to match actual usage to avoid confusion.
| * @param boundSourceId - The source ID this handler is bound to (undefined for single-source mode) | ||
| * @returns A handler function bound to the specified source |
There was a problem hiding this comment.
JSDoc says boundSourceId is undefined for single-source mode, but registerTools() passes a concrete source ID in single-source mode and undefined in multi-source mode (generic tool). Please update the comment to reflect the actual binding behavior.
| * @param boundSourceId - The source ID this handler is bound to (undefined for single-source mode) | |
| * @returns A handler function bound to the specified source | |
| * @param boundSourceId - The source ID this handler is bound to in single-source mode. If undefined, the handler | |
| * is generic and will use the source_id argument provided at call time (multi-source mode). | |
| * @returns A handler function bound to the specified source (or generic if no source is bound) |
| const { sql, source_id: argSourceId } = args as { sql: string; source_id?: string }; | ||
| const resolvedSourceId = boundSourceId ?? argSourceId; | ||
| const startTime = Date.now(); | ||
| const effectiveSourceId = getEffectiveSourceId(sourceId); | ||
| const effectiveSourceId = getEffectiveSourceId(resolvedSourceId); | ||
| let success = true; | ||
| let errorMessage: string | undefined; | ||
| let result: any; | ||
|
|
||
| try { | ||
| // Ensure source is connected (handles lazy connections) | ||
| await ConnectorManager.ensureConnected(sourceId); | ||
| await ConnectorManager.ensureConnected(resolvedSourceId); | ||
|
|
||
| // Get connector for the specified source (or default) | ||
| const connector = ConnectorManager.getCurrentConnector(sourceId); | ||
| const connector = ConnectorManager.getCurrentConnector(resolvedSourceId); |
There was a problem hiding this comment.
resolvedSourceId can be undefined (e.g., if the caller omits source_id), which will silently fall back to the default source via ensureConnected(undefined)/getCurrentConnector(undefined). In multi-source mode this is risky and contradicts the schema comment that source_id is required. Consider failing fast when multiple sources are configured and no source_id is provided (return a clear tool error).
| const resolvedSourceId = boundSourceId ?? argSourceId; | ||
| const startTime = Date.now(); | ||
| const effectiveSourceId = getEffectiveSourceId(sourceId); | ||
| const effectiveSourceId = getEffectiveSourceId(resolvedSourceId); | ||
| let success = true; | ||
| let errorMessage: string | undefined; | ||
|
|
||
| try { | ||
| // Ensure source is connected (handles lazy connections) | ||
| await ConnectorManager.ensureConnected(sourceId); | ||
| await ConnectorManager.ensureConnected(resolvedSourceId); | ||
|
|
||
| const connector = ConnectorManager.getCurrentConnector(sourceId); | ||
| const connector = ConnectorManager.getCurrentConnector(resolvedSourceId); | ||
|
|
There was a problem hiding this comment.
In multi-source mode this handler can run against a source where search_objects is disabled because it never checks tool enablement per resolvedSourceId/connector ID, and registerTools() registers the generic search_objects unconditionally. Please consult the ToolRegistry for the selected source and return an error when search_objects is not enabled for that source.
At moment, the context complexity of this MCP server is
O(n), wherenis the number of sources. As you can imagine in an organization with multiple databases, this causes context bloat.Let's bring down the context complexity of the MCP server, down to O(1), in multi-source mode, by introducing a
list_sourcesmeta tool, that is used to get asource_idper source, that agents pass as an argument to the other tools (i.e.execute_sql/describe_objects), to execute a particular operation against a particular source.Really awesome MCP server btw! 😄