Skip to content

Commit 0d0a8d8

Browse files
authored
Merge pull request #113 from QuantGeekDev/fix/tool-schema
fix: http transport uses same tool loading
2 parents dbb679b + 7ff3c11 commit 0d0a8d8

File tree

3 files changed

+27
-113
lines changed

3 files changed

+27
-113
lines changed

src/core/MCPServer.ts

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -138,53 +138,6 @@ export class MCPServer {
138138
logger.debug(`Creating HttpStreamTransport. response mode: ${httpConfig.responseMode}`);
139139
transport = new HttpStreamTransport(httpConfig);
140140

141-
const serverConfig = {
142-
name: this.serverName,
143-
version: this.serverVersion,
144-
};
145-
146-
const setupCallback = async (server: any) => {
147-
for (const [name, tool] of this.toolsMap.entries()) {
148-
let zodSchema: any;
149-
if ((tool as any).schema && typeof (tool as any).schema === 'object') {
150-
zodSchema = (tool as any).schema;
151-
} else {
152-
zodSchema = {};
153-
}
154-
155-
server.tool(name, zodSchema, async (input: any) => {
156-
try {
157-
logger.debug(`Executing tool ${name} with input: ${JSON.stringify(input)}`);
158-
const result = await (tool as any).execute(input);
159-
160-
if (result && typeof result === 'object' && 'content' in result) {
161-
return result;
162-
} else {
163-
return {
164-
content: [
165-
{
166-
type: 'text',
167-
text: typeof result === 'string' ? result : JSON.stringify(result),
168-
},
169-
],
170-
};
171-
}
172-
} catch (error) {
173-
logger.error(`Tool execution failed for ${name}: ${error}`);
174-
return {
175-
content: [
176-
{
177-
type: 'text',
178-
text: `Error: ${error instanceof Error ? error.message : String(error)}`,
179-
},
180-
],
181-
};
182-
}
183-
});
184-
}
185-
};
186-
187-
(transport as HttpStreamTransport).setServerConfig(serverConfig, setupCallback);
188141
break;
189142
}
190143
case 'stdio':

src/transports/http/server.ts

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { IncomingMessage, ServerResponse, createServer, Server as HttpServer } f
33
import { AbstractTransport } from '../base.js';
44
import { JSONRPCMessage, isInitializeRequest } from '@modelcontextprotocol/sdk/types.js';
55
import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js';
6-
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
76
import { HttpStreamTransportConfig } from './types.js';
87
import { logger } from '../../core/Logger.js';
98

@@ -17,9 +16,6 @@ export class HttpStreamTransport extends AbstractTransport {
1716

1817
private _transports: { [sessionId: string]: StreamableHTTPServerTransport } = {};
1918

20-
private _serverConfig: any;
21-
private _serverSetupCallback?: (server: McpServer) => Promise<void>;
22-
2319
constructor(config: HttpStreamTransportConfig = {}) {
2420
super();
2521

@@ -40,11 +36,6 @@ export class HttpStreamTransport extends AbstractTransport {
4036
);
4137
}
4238

43-
setServerConfig(serverConfig: any, setupCallback: (server: McpServer) => Promise<void>): void {
44-
this._serverConfig = serverConfig;
45-
this._serverSetupCallback = setupCallback;
46-
}
47-
4839
async start(): Promise<void> {
4940
if (this._isRunning) {
5041
throw new Error('HttpStreamTransport already started');
@@ -126,17 +117,11 @@ export class HttpStreamTransport extends AbstractTransport {
126117
}
127118
};
128119

129-
if (this._serverSetupCallback && this._serverConfig) {
130-
const server = new McpServer(this._serverConfig);
131-
await this._serverSetupCallback(server);
132-
await server.connect(transport);
133-
} else {
134-
transport.onmessage = async (message: JSONRPCMessage) => {
135-
if (this._onmessage) {
136-
await this._onmessage(message);
137-
}
138-
};
139-
}
120+
transport.onmessage = async (message: JSONRPCMessage) => {
121+
if (this._onmessage) {
122+
await this._onmessage(message);
123+
}
124+
};
140125

141126
await transport.handleRequest(req, res, body);
142127
return;

tests/transports/http/server.test.ts

Lines changed: 22 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,6 @@ describe('HttpStreamTransport', () => {
4141
});
4242
});
4343

44-
describe('Server Configuration', () => {
45-
it('should set server configuration and setup callback', () => {
46-
const serverConfig = { name: 'test-server', version: '1.0.0' };
47-
const setupCallback = async () => {};
48-
49-
// Should not throw when setting configuration
50-
expect(() => transport.setServerConfig(serverConfig, setupCallback)).not.toThrow();
51-
});
52-
});
53-
5444
describe('Transport Management', () => {
5545
it('should start and stop successfully', async () => {
5646
expect(transport.isRunning()).toBe(false);
@@ -138,20 +128,7 @@ describe('HttpStreamTransport', () => {
138128
expect(transport.isRunning()).toBe(false);
139129
});
140130

141-
it('should support proper session management structure', () => {
142-
const serverConfig = { name: 'multi-client-server', version: '1.0.0' };
143-
const setupCallback = async () => {};
144-
145-
// Should accept configuration for multi-client support
146-
expect(() => transport.setServerConfig(serverConfig, setupCallback)).not.toThrow();
147-
});
148-
149131
it('should handle server lifecycle correctly', async () => {
150-
// Set up server configuration first
151-
const serverConfig = { name: 'test-server', version: '1.0.0' };
152-
const setupCallback = async () => {};
153-
transport.setServerConfig(serverConfig, setupCallback);
154-
155132
// Start and verify state
156133
await transport.start();
157134
expect(transport.isRunning()).toBe(true);
@@ -193,33 +170,32 @@ describe('HttpStreamTransport', () => {
193170
});
194171
});
195172

196-
describe('Integration with Framework Pattern', () => {
197-
it('should follow the multi-session architecture', () => {
198-
const serverConfig = {
199-
name: 'http-stream-server',
200-
version: '1.0.0',
201-
};
202-
203-
const setupCallback = async () => {};
204-
205-
// Should accept the configuration pattern used by working examples
206-
expect(() => transport.setServerConfig(serverConfig, setupCallback)).not.toThrow();
207-
208-
// Should maintain the http-stream transport type
173+
describe('Unified Transport Architecture', () => {
174+
it('should use standard MCP protocol flow like other transports', () => {
175+
// HTTP transport now uses the same unified architecture as SSE and stdio
176+
expect(transport).toBeDefined();
209177
expect(transport.type).toBe('http-stream');
210-
});
211-
212-
it('should support the official MCP pattern for session management', () => {
213-
// Verify the transport supports the pattern:
214-
// 1. Each session gets its own transport instance (handled by our implementation)
215-
// 2. Each session gets its own McpServer instance (handled by setServerConfig)
216-
// 3. Server connects to transport (handled by our implementation)
217-
// 4. Transport stores sessions in a map (our _transports map)
218178

219-
expect(transport).toBeDefined();
220-
expect(typeof transport.setServerConfig).toBe('function');
179+
// Should have standard transport interface methods
221180
expect(typeof transport.send).toBe('function');
222181
expect(typeof transport.close).toBe('function');
182+
expect(typeof transport.start).toBe('function');
183+
expect(typeof transport.isRunning).toBe('function');
184+
});
185+
186+
it('should support message handling through onmessage handler', () => {
187+
// HTTP transport now uses standard onmessage handler like other transports
188+
let messageReceived: JSONRPCMessage | undefined;
189+
190+
// Setting onmessage should not throw
191+
expect(() => {
192+
transport.onmessage = async (message: JSONRPCMessage) => {
193+
messageReceived = message;
194+
};
195+
}).not.toThrow();
196+
197+
// Verify the transport has the onmessage property in its interface
198+
expect('onmessage' in transport).toBe(true);
223199
});
224200
});
225201
});

0 commit comments

Comments
 (0)