Skip to content

Commit 938b255

Browse files
tianzhouclaude
andauthored
fix: display only enabled tools in startup table (#171) (#176)
* fix: display only enabled tools in startup table (#171) This commit fixes issue #171 and consolidates tool management into a single registry, eliminating code duplication. ## Changes 1. **Fixed Issue #171**: Startup table now correctly shows only enabled tools - Renamed ToolRegistry.getToolsForSource() → getEnabledToolConfigs() - Updated getToolsForSource() in tool-metadata.ts to respect registry 2. **Unified tool handling**: Eliminated separate code paths for built-in vs custom tools - Refactored getToolsForSource() to use uniform iteration pattern - Single loop processes all tool types consistently 3. **Consolidated validation**: Removed CustomToolRegistry entirely - Moved all validation logic into ToolRegistry - Deleted custom-tool-registry.ts (220 lines) - Simplified server initialization ## Impact - Net reduction: 77 lines removed - Single source of truth for all tool management - All 357 unit tests pass - Backward compatible 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: initialize ToolRegistry in sources.integration.test.ts The sources API integration test was failing because it didn't initialize the ToolRegistry before making API calls. The test now properly initializes the registry with the fixture config in beforeAll. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a8474d9 commit 938b255

File tree

6 files changed

+208
-280
lines changed

6 files changed

+208
-280
lines changed

src/api/__tests__/sources.integration.test.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
22
import express, { Application } from 'express';
3-
import { setupManagerWithFixture, FIXTURES } from '../../__fixtures__/helpers.js';
3+
import { setupManagerWithFixture, FIXTURES, loadFixtureConfig } from '../../__fixtures__/helpers.js';
44
import type { ConnectorManager } from '../../connectors/manager.js';
55
import { listSources, getSource } from '../sources.js';
66
import type { components } from '../openapi.js';
77
import { Server } from 'http';
8+
import { initializeToolRegistry } from '../../tools/registry.js';
89

910
// Import SQLite connector to ensure it's registered
1011
import '../../connectors/sqlite/index.js';
@@ -27,6 +28,10 @@ describe('Data Sources API Integration Tests', () => {
2728
// - writable_unlimited: readonly=false, no max_rows
2829
manager = await setupManagerWithFixture(FIXTURES.READONLY_MAXROWS);
2930

31+
// Initialize ToolRegistry with fixture config
32+
const sources = loadFixtureConfig(FIXTURES.READONLY_MAXROWS);
33+
initializeToolRegistry({ sources, tools: [] });
34+
3035
// Set up Express app with API routes
3136
app = express();
3237
app.use(express.json());

src/server.ts

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -107,25 +107,6 @@ See documentation for more details on configuring database connections.
107107
});
108108
console.error("Tool registry initialized");
109109

110-
// Initialize custom tool registry early (needed for API routes)
111-
// Only initialize once (idempotent - safe for multiple MCP client connections)
112-
if (sourceConfigsData.tools && sourceConfigsData.tools.length > 0) {
113-
const { customToolRegistry } = await import("./tools/custom-tool-registry.js");
114-
const { BUILTIN_TOOLS } = await import("./tools/builtin-tools.js");
115-
116-
if (!customToolRegistry.isInitialized()) {
117-
// Filter out built-in tools - custom tool registry only handles custom tools
118-
const customTools = sourceConfigsData.tools.filter(
119-
(tool) => !(BUILTIN_TOOLS as readonly string[]).includes(tool.name)
120-
);
121-
122-
if (customTools.length > 0) {
123-
customToolRegistry.initialize(customTools);
124-
console.error(`Custom tool registry initialized with ${customTools.length} tool(s)`);
125-
}
126-
}
127-
}
128-
129110
// Create MCP server factory function for HTTP transport
130111
// Note: This must be created AFTER ConnectorManager is initialized
131112
const createServer = () => {
@@ -135,7 +116,7 @@ See documentation for more details on configuring database connections.
135116
});
136117

137118
// Register tools (both built-in and custom)
138-
// Custom tools are already initialized in customToolRegistry by the code above
119+
// All tools are validated and managed by the ToolRegistry
139120
registerTools(server);
140121

141122
return server;

src/tools/custom-tool-registry.ts

Lines changed: 0 additions & 220 deletions
This file was deleted.

src/tools/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export function registerTools(server: McpServer): void {
2525

2626
// Register all enabled tools (both built-in and custom) for each source
2727
for (const sourceId of sourceIds) {
28-
const enabledTools = registry.getToolsForSource(sourceId);
28+
const enabledTools = registry.getEnabledToolConfigs(sourceId);
2929
const sourceConfig = ConnectorManager.getSourceConfig(sourceId)!;
3030
const dbType = sourceConfig.type;
3131
const isDefault = sourceIds[0] === sourceId;

0 commit comments

Comments
 (0)