Skip to content

Commit 799dd90

Browse files
eleftheriasclaude
andauthored
chore: remove --experimental-mcp flag and internal ToolHive MCP server (#2322)
Drops the experimental MCP backend that was spawned via `thv serve --experimental-mcp*` and surfaced in the playground as the internal `toolhive-mcp-internal` server, removing it end-to-end across the main process, IPC/preload, renderer, and docs. Note: existing users may have enabled-tool entries persisted under `toolhive-mcp-internal`. These self-heal — getEnabledMcpTools() no longer matches the internal name against running servers, so the orphaned key is cleaned up on next read (one-time "Cleaning up tools for stopped server" info log). No migration needed. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0be5645 commit 799dd90

21 files changed

Lines changed: 33 additions & 589 deletions

File tree

.claude/skills/devcontainer-dev/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ The Electron main window's `WM_CLASS` is `ToolHive`, **not** `Electron`. The `~/
233233

234234
### Readiness false-positive (transient processes)
235235

236-
`pgrep -x thv` would match the short-lived `thv version` / `thv --version` invocation that Electron runs during startup to verify the binary is present. The long-running backend is always `thv serve --openapi --experimental-mcp ... --port=N`, so use `pgrep -f "thv serve"` to gate on that specifically.
236+
`pgrep -x thv` would match the short-lived `thv version` / `thv --version` invocation that Electron runs during startup to verify the binary is present. The long-running backend is always `thv serve --openapi ... --port=N`, so use `pgrep -f "thv serve"` to gate on that specifically.
237237

238238
### Readiness false-positive (pgrep self-match)
239239

.codex/skills/devcontainer-dev/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ The Electron main window's `WM_CLASS` is `ToolHive`, **not** `Electron`. The `~/
233233

234234
### Readiness false-positive (transient processes)
235235

236-
`pgrep -x thv` would match the short-lived `thv version` / `thv --version` invocation that Electron runs during startup to verify the binary is present. The long-running backend is always `thv serve --openapi --experimental-mcp ... --port=N`, so use `pgrep -f "thv serve"` to gate on that specifically.
236+
`pgrep -x thv` would match the short-lived `thv version` / `thv --version` invocation that Electron runs during startup to verify the binary is present. The long-running backend is always `thv serve --openapi ... --port=N`, so use `pgrep -f "thv serve"` to gate on that specifically.
237237

238238
### Readiness false-positive (pgrep self-match)
239239

.cursor/skills/devcontainer-dev/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ The Electron main window's `WM_CLASS` is `ToolHive`, **not** `Electron`. The `~/
233233

234234
### Readiness false-positive (transient processes)
235235

236-
`pgrep -x thv` would match the short-lived `thv version` / `thv --version` invocation that Electron runs during startup to verify the binary is present. The long-running backend is always `thv serve --openapi --experimental-mcp ... --port=N`, so use `pgrep -f "thv serve"` to gate on that specifically.
236+
`pgrep -x thv` would match the short-lived `thv version` / `thv --version` invocation that Electron runs during startup to verify the binary is present. The long-running backend is always `thv serve --openapi ... --port=N`, so use `pgrep -f "thv serve"` to gate on that specifically.
237237

238238
### Readiness false-positive (pgrep self-match)
239239

docs/README.md

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -190,34 +190,27 @@ point the studio at it via `THV_SOCKET`:
190190
```bash
191191
thv serve \
192192
--openapi \
193-
--socket=/tmp/thv-dev.sock \
194-
--experimental-mcp \
195-
--experimental-mcp-host=127.0.0.1 \
196-
--experimental-mcp-port=50001
193+
--socket=/tmp/thv-dev.sock
197194
```
198195

199196
**Windows (PowerShell)**
200197

201198
```powershell
202199
thv.exe serve `
203200
--openapi `
204-
--socket='\\.\pipe\thv-dev' `
205-
--experimental-mcp `
206-
--experimental-mcp-host=127.0.0.1 `
207-
--experimental-mcp-port=50001
201+
--socket='\\.\pipe\thv-dev'
208202
```
209203

210-
2. Set `THV_SOCKET` (and `THV_MCP_PORT` if you also need the experimental MCP
211-
backend) and start the dev server:
204+
2. Set `THV_SOCKET` and start the dev server:
212205

213206
```bash
214-
THV_SOCKET=/tmp/thv-dev.sock THV_MCP_PORT=50001 pnpm start
207+
THV_SOCKET=/tmp/thv-dev.sock pnpm start
215208
```
216209

217210
On Windows:
218211

219212
```powershell
220-
$env:THV_SOCKET = '\\.\pipe\thv-dev'; $env:THV_MCP_PORT = '50001'; pnpm start
213+
$env:THV_SOCKET = '\\.\pipe\thv-dev'; pnpm start
221214
```
222215

223216
The UI displays a banner with the socket / pipe path when `THV_SOCKET` is set.

main/src/chat/__tests__/mcp-tools.test.ts

Lines changed: 9 additions & 156 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'
44
// Hoisted mock factories — must be defined before vi.mock() calls
55
// ---------------------------------------------------------------------------
66

7-
const mockGetToolhiveMcpPort = vi.hoisted(() => vi.fn().mockReturnValue(3001))
87
const mockCreateMainProcessApiClient = vi.hoisted(() =>
98
vi.fn().mockReturnValue({})
109
)
@@ -29,7 +28,7 @@ const mockSdkClient = vi.hoisted(() => ({
2928
close: vi.fn().mockResolvedValue(undefined),
3029
}))
3130

32-
// AI SDK MCP client mock (used by getToolhiveMcpInfo / createMcpTools)
31+
// AI SDK MCP client mock (used by createMcpTools)
3332
const mockAiMcpClient = vi.hoisted(() => ({
3433
tools: vi.fn().mockResolvedValue({}),
3534
close: vi.fn().mockResolvedValue(undefined),
@@ -48,10 +47,6 @@ const mockReplaceAllMcpAppUiMetadata = vi.hoisted(() => vi.fn())
4847
// Module mocks
4948
// ---------------------------------------------------------------------------
5049

51-
vi.mock('../../toolhive-manager', () => ({
52-
getToolhiveMcpPort: mockGetToolhiveMcpPort,
53-
}))
54-
5550
vi.mock('../../unix-socket-fetch', () => ({
5651
createMainProcessApiClient: mockCreateMainProcessApiClient,
5752
}))
@@ -104,10 +99,6 @@ vi.mock('@modelcontextprotocol/sdk/client/index.js', () => ({
10499
},
105100
}))
106101

107-
vi.mock('@modelcontextprotocol/sdk/client/streamableHttp.js', () => ({
108-
StreamableHTTPClientTransport: function StreamableHTTPTransportMock() {},
109-
}))
110-
111102
// The schemas are passed through to the mocked client.request(), so they
112103
// just need to be non-undefined objects.
113104
vi.mock('@modelcontextprotocol/sdk/types.js', () => ({
@@ -136,7 +127,6 @@ import {
136127
getCachedUiMetadata,
137128
fetchUiResource,
138129
proxyMcpToolCall,
139-
getToolhiveMcpInfo,
140130
getMcpServerTools,
141131
createMcpTools,
142132
} from '../mcp-tools'
@@ -146,8 +136,6 @@ import log from '../../logger'
146136
// Shared fixtures
147137
// ---------------------------------------------------------------------------
148138

149-
const THV_SERVER = 'toolhive-mcp-internal'
150-
151139
const makeWorkload = (overrides: Record<string, unknown> = {}) => ({
152140
name: 'test-server',
153141
port: 9999,
@@ -172,9 +160,6 @@ const makeToolDef = (overrides: Record<string, unknown> = {}) => ({
172160
beforeEach(() => {
173161
vi.clearAllMocks()
174162

175-
// MCP backend port (still TCP)
176-
mockGetToolhiveMcpPort.mockReturnValue(3001)
177-
178163
// API client chain (fetchWorkloads)
179164
mockCreateMainProcessApiClient.mockReturnValue({})
180165
mockGetApiV1BetaWorkloads.mockResolvedValue({ data: { workloads: [] } })
@@ -250,8 +235,11 @@ describe('getCachedUiMetadata', () => {
250235
ui: { resourceUri: 'res://cached-tool', visibility: ['model'] },
251236
},
252237
}
238+
mockGetApiV1BetaWorkloads.mockResolvedValue({
239+
data: { workloads: [makeWorkload()] },
240+
})
253241
mockGetEnabledMcpTools.mockResolvedValue({
254-
[THV_SERVER]: ['cached-tool'],
242+
'test-server': ['cached-tool'],
255243
})
256244
mockAiMcpClient.tools.mockResolvedValue({ 'cached-tool': toolWithUi })
257245

@@ -345,20 +333,6 @@ describe('fetchUiResource', () => {
345333
expect(mockSdkClient.close).toHaveBeenCalledOnce()
346334
})
347335

348-
it('uses the raw ToolHive MCP transport for the internal ToolHive server', async () => {
349-
mockSdkClient.request.mockResolvedValue({
350-
contents: [{ text: '<html>thv</html>' }],
351-
})
352-
353-
await fetchUiResource(THV_SERVER, 'res://thv')
354-
355-
// buildRawTransport should NOT be called for the internal server
356-
expect(mockBuildRawTransport).not.toHaveBeenCalled()
357-
// StreamableHTTPClientTransport is newed up inside
358-
// createToolhiveRawMcpTransport
359-
expect(mockSdkClient.connect).toHaveBeenCalledOnce()
360-
})
361-
362336
it('uses buildRawTransport for external servers', async () => {
363337
mockGetApiV1BetaWorkloads.mockResolvedValue({
364338
data: { workloads: [makeWorkload()] },
@@ -423,88 +397,6 @@ describe('proxyMcpToolCall', () => {
423397
})
424398
})
425399

426-
// ---------------------------------------------------------------------------
427-
// getToolhiveMcpInfo
428-
// ---------------------------------------------------------------------------
429-
430-
describe('getToolhiveMcpInfo', () => {
431-
it('returns isRunning: false when port is not available', async () => {
432-
mockGetToolhiveMcpPort.mockReturnValue(null)
433-
434-
const result = await getToolhiveMcpInfo()
435-
436-
expect(result.isRunning).toBe(false)
437-
expect(result.serverName).toBe(THV_SERVER)
438-
expect(mockCreateMCPClient).not.toHaveBeenCalled()
439-
})
440-
441-
it('returns tools with correct enabled flags', async () => {
442-
mockAiMcpClient.tools.mockResolvedValue({
443-
'tool-enabled': makeToolDef({ description: 'enabled' }),
444-
'tool-disabled': makeToolDef({ description: 'disabled' }),
445-
})
446-
447-
const result = await getToolhiveMcpInfo(['tool-enabled'])
448-
449-
expect(result.isRunning).toBe(true)
450-
expect(result.tools).toHaveLength(2)
451-
const enabled = result.tools.find((t) => t.name === 'tool-enabled')
452-
const disabled = result.tools.find((t) => t.name === 'tool-disabled')
453-
expect(enabled?.enabled).toBe(true)
454-
expect(disabled?.enabled).toBe(false)
455-
})
456-
457-
it('advertises MCP_UI_EXTENSION_CAPABILITY when creating the client', async () => {
458-
await getToolhiveMcpInfo()
459-
460-
expect(mockCreateMCPClient).toHaveBeenCalledWith(
461-
expect.objectContaining({
462-
capabilities: expect.objectContaining({
463-
extensions: expect.objectContaining({
464-
'io.modelcontextprotocol/ui': expect.anything(),
465-
}),
466-
}),
467-
})
468-
)
469-
})
470-
471-
it('closes the AI SDK client after listing tools', async () => {
472-
await getToolhiveMcpInfo()
473-
474-
expect(mockAiMcpClient.close).toHaveBeenCalledOnce()
475-
})
476-
477-
it('returns isRunning: false and logs when createMCPClient throws', async () => {
478-
mockCreateMCPClient.mockRejectedValue(new Error('connection refused'))
479-
480-
const result = await getToolhiveMcpInfo()
481-
482-
expect(result.isRunning).toBe(false)
483-
expect(log.error).toHaveBeenCalledWith(
484-
'Failed to get Toolhive MCP info:',
485-
expect.any(Error)
486-
)
487-
})
488-
489-
it('extracts tool parameters from inputSchema', async () => {
490-
mockAiMcpClient.tools.mockResolvedValue({
491-
'draw-tool': {
492-
description: 'Draw something',
493-
inputSchema: {
494-
type: 'object',
495-
properties: { canvas: { type: 'string' } },
496-
},
497-
},
498-
})
499-
500-
const result = await getToolhiveMcpInfo()
501-
502-
expect(result.tools[0]?.parameters).toEqual({
503-
canvas: { type: 'string' },
504-
})
505-
})
506-
})
507-
508400
// ---------------------------------------------------------------------------
509401
// getMcpServerTools
510402
// ---------------------------------------------------------------------------
@@ -557,19 +449,6 @@ describe('getMcpServerTools', () => {
557449
expect(mockGetWorkloadAvailableTools).not.toHaveBeenCalled()
558450
})
559451

560-
it('falls back to getToolhiveMcpInfo for the internal ToolHive server', async () => {
561-
mockGetApiV1BetaWorkloads.mockResolvedValue({ data: { workloads: [] } })
562-
mockGetEnabledMcpTools.mockResolvedValue({ [THV_SERVER]: ['thv-tool'] })
563-
mockAiMcpClient.tools.mockResolvedValue({
564-
'thv-tool': makeToolDef(),
565-
})
566-
567-
const result = await getMcpServerTools(THV_SERVER)
568-
569-
expect(result?.serverName).toBe(THV_SERVER)
570-
expect(mockCreateMCPClient).toHaveBeenCalled()
571-
})
572-
573452
it('throws when a regular server workload is not found', async () => {
574453
mockGetApiV1BetaWorkloads.mockResolvedValue({ data: { workloads: [] } })
575454

@@ -612,7 +491,10 @@ describe('createMcpTools', () => {
612491
...makeToolDef(),
613492
_meta: { ui: { resourceUri: 'res://reset-test', visibility: ['model'] } },
614493
}
615-
mockGetEnabledMcpTools.mockResolvedValue({ [THV_SERVER]: ['cached'] })
494+
mockGetApiV1BetaWorkloads.mockResolvedValue({
495+
data: { workloads: [makeWorkload()] },
496+
})
497+
mockGetEnabledMcpTools.mockResolvedValue({ 'test-server': ['cached'] })
616498
mockAiMcpClient.tools.mockResolvedValue({ cached: toolWithUi })
617499
await createMcpTools()
618500
expect(getCachedUiMetadata()).toHaveProperty('cached')
@@ -718,35 +600,6 @@ describe('createMcpTools', () => {
718600
expect(mockCreateMCPClient).not.toHaveBeenCalled()
719601
})
720602

721-
it('registers ToolHive MCP tools when port is available', async () => {
722-
mockGetEnabledMcpTools.mockResolvedValue({ [THV_SERVER]: ['thv-draw'] })
723-
mockAiMcpClient.tools.mockResolvedValue({ 'thv-draw': makeToolDef() })
724-
725-
const { tools, clients } = await createMcpTools()
726-
727-
expect(tools).toHaveProperty('thv-draw')
728-
expect(clients).toHaveLength(1)
729-
expect(mockCreateMCPClient).toHaveBeenCalledWith(
730-
expect.objectContaining({
731-
name: 'toolhive-mcp',
732-
capabilities: expect.objectContaining({
733-
extensions: expect.objectContaining({
734-
'io.modelcontextprotocol/ui': expect.anything(),
735-
}),
736-
}),
737-
})
738-
)
739-
})
740-
741-
it('skips ToolHive MCP tools when port is not available', async () => {
742-
mockGetToolhiveMcpPort.mockReturnValue(null)
743-
mockGetEnabledMcpTools.mockResolvedValue({ [THV_SERVER]: ['thv-draw'] })
744-
745-
const { tools } = await createMcpTools()
746-
747-
expect(tools).toEqual({})
748-
})
749-
750603
it('registers per-server tools using createTransport', async () => {
751604
const workload = makeWorkload()
752605
mockGetApiV1BetaWorkloads.mockResolvedValue({

0 commit comments

Comments
 (0)