mirror of
https://github.com/czlonkowski/n8n-mcp.git
synced 2026-03-23 02:43:08 +00:00
fix: return 202 for notifications with stale sessions, warn on updateTable columns (#654)
**Issue #654 (critical):** handleRequest() returned HTTP 400 for JSON-RPC notifications with stale/expired session IDs. Per JSON-RPC 2.0 spec, notifications don't expect responses. Returning 400 caused Claude's proxy to interpret the connection as broken, triggering reconnection storms (930 errors/day, 216 users affected). Fix: Added isJsonRpcNotification() helper that detects single and batch notifications (messages with method but no id field). Path C in handleRequest() now returns 202 Accepted for notifications with stale sessions instead of 400. Also added TOCTOU null guard in Path B. **Issue 1 (medium):** updateTable silently accepted columns parameter via the shared MCP tool schema. Zod stripped it but gave no feedback. Fix: handleUpdateTable now detects columns in raw args and appends a warning to the response. Tool schema descriptions clarified that columns are for createTable only and schema is immutable after creation. Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -219,6 +219,7 @@ describe('HTTP Server Session Management', () => {
|
||||
status: vi.fn().mockReturnThis(),
|
||||
json: vi.fn().mockReturnThis(),
|
||||
send: vi.fn().mockReturnThis(),
|
||||
end: vi.fn().mockReturnThis(),
|
||||
setHeader: vi.fn((key: string, value: string) => {
|
||||
headers[key.toLowerCase()] = value;
|
||||
}),
|
||||
@@ -1170,7 +1171,7 @@ describe('HTTP Server Session Management', () => {
|
||||
|
||||
it('should show legacy SSE session when present', async () => {
|
||||
server = new SingleSessionHTTPServer();
|
||||
|
||||
|
||||
// Mock legacy session
|
||||
const mockSession = {
|
||||
sessionId: 'sse-session-123',
|
||||
@@ -1180,10 +1181,111 @@ describe('HTTP Server Session Management', () => {
|
||||
(server as any).session = mockSession;
|
||||
|
||||
const sessionInfo = server.getSessionInfo();
|
||||
|
||||
|
||||
expect(sessionInfo.active).toBe(true);
|
||||
expect(sessionInfo.sessionId).toBe('sse-session-123');
|
||||
expect(sessionInfo.age).toBeGreaterThanOrEqual(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Notification handling for stale sessions (#654)', () => {
|
||||
beforeEach(() => {
|
||||
// Re-apply mockImplementation after vi.clearAllMocks() resets it
|
||||
mockConsoleManager.wrapOperation.mockImplementation(async (fn: () => Promise<any>) => {
|
||||
return await fn();
|
||||
});
|
||||
});
|
||||
|
||||
it('should return 202 for notification with stale session ID', async () => {
|
||||
server = new SingleSessionHTTPServer();
|
||||
|
||||
const { req, res } = createMockReqRes();
|
||||
|
||||
req.headers = { 'mcp-session-id': 'stale-session-that-does-not-exist' };
|
||||
req.method = 'POST';
|
||||
req.body = {
|
||||
jsonrpc: '2.0',
|
||||
method: 'notifications/initialized',
|
||||
};
|
||||
|
||||
await server.handleRequest(req as any, res as any);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(202);
|
||||
expect(res.end).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return 202 for notification batch with stale session ID', async () => {
|
||||
server = new SingleSessionHTTPServer();
|
||||
|
||||
const { req, res } = createMockReqRes();
|
||||
|
||||
req.headers = { 'mcp-session-id': 'stale-session-that-does-not-exist' };
|
||||
req.method = 'POST';
|
||||
req.body = [
|
||||
{ jsonrpc: '2.0', method: 'notifications/initialized' },
|
||||
{ jsonrpc: '2.0', method: 'notifications/cancelled' },
|
||||
];
|
||||
|
||||
await server.handleRequest(req as any, res as any);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(202);
|
||||
expect(res.end).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return 400 for request (with id) with stale session ID', async () => {
|
||||
server = new SingleSessionHTTPServer();
|
||||
|
||||
const { req, res } = createMockReqRes();
|
||||
req.headers = { 'mcp-session-id': 'stale-session-that-does-not-exist' };
|
||||
req.method = 'POST';
|
||||
req.body = {
|
||||
jsonrpc: '2.0',
|
||||
method: 'tools/call',
|
||||
params: { name: 'search_nodes', arguments: { query: 'http' } },
|
||||
id: 42,
|
||||
};
|
||||
|
||||
await server.handleRequest(req as any, res as any);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(400);
|
||||
expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
|
||||
error: expect.objectContaining({
|
||||
message: 'Bad Request: Session not found or expired',
|
||||
}),
|
||||
}));
|
||||
});
|
||||
|
||||
it('should return 202 for notification with no session ID', async () => {
|
||||
server = new SingleSessionHTTPServer();
|
||||
|
||||
const { req, res } = createMockReqRes();
|
||||
|
||||
req.method = 'POST';
|
||||
req.body = {
|
||||
jsonrpc: '2.0',
|
||||
method: 'notifications/cancelled',
|
||||
};
|
||||
|
||||
await server.handleRequest(req as any, res as any);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(202);
|
||||
expect(res.end).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return 400 for request with no session ID and not initialize', async () => {
|
||||
server = new SingleSessionHTTPServer();
|
||||
|
||||
const { req, res } = createMockReqRes();
|
||||
req.method = 'POST';
|
||||
req.body = {
|
||||
jsonrpc: '2.0',
|
||||
method: 'tools/list',
|
||||
id: 1,
|
||||
};
|
||||
|
||||
await server.handleRequest(req as any, res as any);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(400);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user