Skip to content

bug fixes #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sajeerzeji merged 1 commit into from
Apr 9, 2026
Merged

bug fixes #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 43 additions & 46 deletions packages/toolpack-sdk/src/mcp/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ export class McpClient extends EventEmitter {
return this._connected && this.process !== null;
}

private async initializeServer(): Promise<void> {
await this.request('initialize', { client: 'toolpack-sdk' });
}

// ======================================================================
// Connection
// ======================================================================
Expand All @@ -88,62 +92,55 @@ export class McpClient extends EventEmitter {
throw new McpConnectionError('Client is shutting down');
}

return new Promise((resolve, reject) => {
try {
this.buffer = '';
this.process = spawn(this.config.command, this.config.args || [], {
env: { ...process.env, ...this.config.env },
stdio: ['pipe', 'pipe', 'pipe'], // stdin, stdout, stderr
});

if (!this.process.stdout || !this.process.stdin) {
throw new McpConnectionError('Failed to spawn MCP server: stdout/stdin unavailable');
}

this.process.stdout.on('data', (data: Buffer) => {
this.handleData(data);
});
this.buffer = '';
this.process = spawn(this.config.command, this.config.args || [], {
env: { ...process.env, ...this.config.env },
stdio: ['pipe', 'pipe', 'pipe'], // stdin, stdout, stderr
});

// Route child stderr through logger instead of inheriting
// (inherited stderr corrupts Ink TUI rendering)
if (this.process.stderr) {
this.process.stderr.on('data', (data: Buffer) => {
logWarn(`[MCP server stderr] ${data.toString().trim()}`);
});
}
if (!this.process.stdout || !this.process.stdin) {
throw new McpConnectionError('Failed to spawn MCP server: stdout/stdin unavailable');
}

this.process.on('error', (err) => {
this._connected = false;
this.emit('error', err);
});
this.process.stdout.on('data', (data: Buffer) => {
this.handleData(data);
});

this.process.on('exit', (code) => {
const wasConnected = this._connected;
this._connected = false;
this.process = null;
// Route child stderr through logger instead of inheriting
// (inherited stderr corrupts Ink TUI rendering)
if (this.process.stderr) {
this.process.stderr.on('data', (data: Buffer) => {
logWarn(`[MCP server stderr] ${data.toString().trim()}`);
});
}

// Reject all pending requests
this.rejectAllPending(
new McpConnectionError(`MCP server exited with code ${code}`, code)
);
this.process.on('error', (err) => {
this._connected = false;
this.emit('error', err);
});

this.emit('close', code);
this.process.on('exit', (code) => {
const wasConnected = this._connected;
this._connected = false;
this.process = null;

// Auto-reconnect on unexpected crash
if (wasConnected && !this._shuttingDown && this.autoReconnect) {
this.attemptReconnect();
}
});
// Reject all pending requests
this.rejectAllPending(
new McpConnectionError(`MCP server exited with code ${code}`, code)
);

this._connected = true;
this._reconnectAttempts = 0;
this.emit('close', code);

// Give it a moment to start
setTimeout(resolve, 500);
} catch (error) {
reject(error);
// Auto-reconnect on unexpected crash
if (wasConnected && !this._shuttingDown && this.autoReconnect) {
this.attemptReconnect();
}
});

this._reconnectAttempts = 0;

await this.initializeServer();
this._connected = true;
}

// ======================================================================
Expand Down
85 changes: 80 additions & 5 deletions packages/toolpack-sdk/src/tools/mcp-tools/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ vi.mock('../../mcp/client.js', async () => {
}

async request(method: string): Promise<any> {
if (method === 'initialize') {
return { initialized: true };
}

if (method === 'tools/list') {
return {
tools: [
Expand Down Expand Up @@ -291,6 +295,35 @@ describe('McpToolManager', () => {
const serverTools = toolsAfter.filter(t => t.name.includes('test-server'));
expect(serverTools).toHaveLength(0);
});

it('should only remove exact server tools when disconnecting', async () => {
const config: McpToolsConfig = {
servers: [],
};

manager = new McpToolManager(config);

await manager.connectServer({
name: 'test',
command: 'node',
args: ['test.js'],
});

await manager.connectServer({
name: 'test-server',
command: 'node',
args: ['test.js'],
});

await manager.disconnectServer('test');

const toolsAfter = manager.getToolDefinitions();
const exactServerTools = toolsAfter.filter(t => t.name.startsWith('mcp.test-server.'));
expect(exactServerTools.length).toBeGreaterThan(0);

const removedTools = toolsAfter.filter(t => t.name.startsWith('mcp.test.'));
expect(removedTools.every(t => t.name.startsWith('mcp.test-server.'))).toBe(true);
});

it('should disconnect from all servers', async () => {
const config: McpToolsConfig = {
Expand Down Expand Up @@ -355,6 +388,48 @@ describe('McpToolManager', () => {
expect(result).toBeDefined();
expect(typeof result).toBe('string');
});

it('should refresh tools after reconnecting', async () => {
const config: McpToolsConfig = {
servers: [
{
name: 'test-server',
command: 'node',
args: ['test.js'],
},
],
};

manager = new McpToolManager(config);
await manager.connectServer(config.servers[0]);

const client = (manager as any).clients.get('test-server');
client.request = async (method: string): Promise<any> => {
if (method === 'tools/list') {
return {
tools: [
{
name: 'updated_tool',
description: 'Updated tool after reconnect',
inputSchema: { type: 'object', properties: {}, required: [] },
},
],
};
}

if (method === 'initialize') {
return { initialized: true };
}
throw new Error(`Unknown method: ${method}`);
};

client.emit('reconnected', { attempt: 1 });
await new Promise(resolve => setTimeout(resolve, 0));

const tools = manager.getToolDefinitions();
expect(tools.some(t => t.name.includes('updated_tool'))).toBe(true);
expect(tools.some(t => t.name.includes('test_tool'))).toBe(false);
});
});
});

Expand Down Expand Up @@ -395,10 +470,10 @@ describe('createMcpToolProject', () => {
],
};

const project = await createMcpToolProject(config);
const project = await createMcpToolProject(config) as any;

expect((project as any)._mcpManager).toBeDefined();
expect((project as any)._mcpManager).toBeInstanceOf(McpToolManager);
expect(project.mcpManager).toBeDefined();
expect(project.mcpManager).toBeInstanceOf(McpToolManager);

// Cleanup
await disconnectMcpToolProject(project);
Expand Down Expand Up @@ -437,7 +512,7 @@ describe('createMcpToolProject', () => {

expect(project.tools.length).toBeGreaterThan(0);

const manager = (project as any)._mcpManager;
const manager = (project as any).mcpManager;
expect(manager.getConnectedServers()).toHaveLength(2);

// Cleanup
Expand All @@ -458,7 +533,7 @@ describe('disconnectMcpToolProject', () => {
};

const project = await createMcpToolProject(config);
const manager = (project as any)._mcpManager;
const manager = (project as any).mcpManager;

expect(manager.getConnectedServers()).toHaveLength(1);

Expand Down
Loading
Loading