40daf8f3fa
* feat: replace WASM embeddings with persistent chroma-mcp MCP connection Replace ChromaServerManager (npx chroma run + chromadb npm + ONNX/WASM) with ChromaMcpManager, a singleton stdio MCP client that communicates with chroma-mcp via uvx. This eliminates native binary issues, segfaults, and WASM embedding failures that plagued cross-platform installs. Key changes: - Add ChromaMcpManager: singleton MCP client with lazy connect, auto-reconnect, connection lock, and Zscaler SSL cert support - Rewrite ChromaSync to use MCP tool calls instead of chromadb npm client - Handle chroma-mcp's non-JSON responses (plain text success/error messages) - Treat "collection already exists" as idempotent success - Wire ChromaMcpManager into GracefulShutdown for clean subprocess teardown - Delete ChromaServerManager (no longer needed) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review — connection guard leak, timer leak, async reset - Clear connecting guard in finally block to prevent permanent reconnection block - Clear timeout after successful connection to prevent timer leak - Make reset() async to await stop() before nullifying instance - Delete obsolete chroma-server-manager test (imports deleted class) - Update graceful-shutdown test to use chromaMcpManager property name Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: prevent chroma-mcp spawn storm — zombie cleanup, stale onclose guard, reconnect backoff Three bugs caused chroma-mcp processes to accumulate (92+ observed): 1. Zombie on timeout: failed connections left subprocess alive because only the timer was cleared, not the transport. Now catch block explicitly closes transport+client before rethrowing. 2. Stale onclose race: old transport's onclose handler captured `this` and overwrote the current connection reference after reconnect, orphaning the new subprocess. Now guarded with reference check. 3. No backoff: every failure triggered immediate reconnect. With backfill doing hundreds of MCP calls, this created rapid-fire spawning. Added 10s backoff on both connection failure and unexpected process death. Also includes ChromaSync fixes from PR review: - queryChroma deduplication now preserves index-aligned arrays - SQL injection guard on backfill ID exclusion lists Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
257 lines
7.7 KiB
TypeScript
257 lines
7.7 KiB
TypeScript
import { describe, it, expect, beforeEach, afterEach, mock, spyOn } from 'bun:test';
|
|
import { existsSync, readFileSync } from 'fs';
|
|
import { homedir } from 'os';
|
|
import path from 'path';
|
|
import http from 'http';
|
|
import {
|
|
performGracefulShutdown,
|
|
writePidFile,
|
|
readPidFile,
|
|
removePidFile,
|
|
type GracefulShutdownConfig,
|
|
type ShutdownableService,
|
|
type CloseableClient,
|
|
type CloseableDatabase,
|
|
type PidInfo
|
|
} from '../../src/services/infrastructure/index.js';
|
|
|
|
const DATA_DIR = path.join(homedir(), '.claude-mem');
|
|
const PID_FILE = path.join(DATA_DIR, 'worker.pid');
|
|
|
|
describe('GracefulShutdown', () => {
|
|
// Store original PID file content if it exists
|
|
let originalPidContent: string | null = null;
|
|
const originalPlatform = process.platform;
|
|
|
|
beforeEach(() => {
|
|
// Backup existing PID file if present
|
|
if (existsSync(PID_FILE)) {
|
|
originalPidContent = readFileSync(PID_FILE, 'utf-8');
|
|
}
|
|
|
|
// Ensure we're testing on non-Windows to avoid child process enumeration
|
|
Object.defineProperty(process, 'platform', {
|
|
value: 'darwin',
|
|
writable: true,
|
|
configurable: true
|
|
});
|
|
});
|
|
|
|
afterEach(() => {
|
|
// Restore original PID file or remove test one
|
|
if (originalPidContent !== null) {
|
|
const { writeFileSync } = require('fs');
|
|
writeFileSync(PID_FILE, originalPidContent);
|
|
originalPidContent = null;
|
|
} else {
|
|
removePidFile();
|
|
}
|
|
|
|
// Restore platform
|
|
Object.defineProperty(process, 'platform', {
|
|
value: originalPlatform,
|
|
writable: true,
|
|
configurable: true
|
|
});
|
|
});
|
|
|
|
describe('performGracefulShutdown', () => {
|
|
it('should call shutdown steps in correct order', async () => {
|
|
const callOrder: string[] = [];
|
|
|
|
const mockServer = {
|
|
closeAllConnections: mock(() => {
|
|
callOrder.push('closeAllConnections');
|
|
}),
|
|
close: mock((cb: (err?: Error) => void) => {
|
|
callOrder.push('serverClose');
|
|
cb();
|
|
})
|
|
} as unknown as http.Server;
|
|
|
|
const mockSessionManager: ShutdownableService = {
|
|
shutdownAll: mock(async () => {
|
|
callOrder.push('sessionManager.shutdownAll');
|
|
})
|
|
};
|
|
|
|
const mockMcpClient: CloseableClient = {
|
|
close: mock(async () => {
|
|
callOrder.push('mcpClient.close');
|
|
})
|
|
};
|
|
|
|
const mockDbManager: CloseableDatabase = {
|
|
close: mock(async () => {
|
|
callOrder.push('dbManager.close');
|
|
})
|
|
};
|
|
|
|
const mockChromaMcpManager = {
|
|
stop: mock(async () => {
|
|
callOrder.push('chromaMcpManager.stop');
|
|
})
|
|
};
|
|
|
|
// Create a PID file so we can verify it's removed
|
|
writePidFile({ pid: 12345, port: 37777, startedAt: new Date().toISOString() });
|
|
expect(existsSync(PID_FILE)).toBe(true);
|
|
|
|
const config: GracefulShutdownConfig = {
|
|
server: mockServer,
|
|
sessionManager: mockSessionManager,
|
|
mcpClient: mockMcpClient,
|
|
dbManager: mockDbManager,
|
|
chromaMcpManager: mockChromaMcpManager
|
|
};
|
|
|
|
await performGracefulShutdown(config);
|
|
|
|
// Verify order: PID removal happens first (synchronous), then server, then session, then MCP, then Chroma, then DB
|
|
expect(callOrder).toContain('closeAllConnections');
|
|
expect(callOrder).toContain('serverClose');
|
|
expect(callOrder).toContain('sessionManager.shutdownAll');
|
|
expect(callOrder).toContain('mcpClient.close');
|
|
expect(callOrder).toContain('chromaMcpManager.stop');
|
|
expect(callOrder).toContain('dbManager.close');
|
|
|
|
// Verify server closes before session manager
|
|
expect(callOrder.indexOf('serverClose')).toBeLessThan(callOrder.indexOf('sessionManager.shutdownAll'));
|
|
|
|
// Verify session manager shuts down before MCP client
|
|
expect(callOrder.indexOf('sessionManager.shutdownAll')).toBeLessThan(callOrder.indexOf('mcpClient.close'));
|
|
|
|
// Verify MCP closes before database
|
|
expect(callOrder.indexOf('mcpClient.close')).toBeLessThan(callOrder.indexOf('dbManager.close'));
|
|
|
|
// Verify Chroma stops before DB closes
|
|
expect(callOrder.indexOf('chromaMcpManager.stop')).toBeLessThan(callOrder.indexOf('dbManager.close'));
|
|
});
|
|
|
|
it('should remove PID file during shutdown', async () => {
|
|
const mockSessionManager: ShutdownableService = {
|
|
shutdownAll: mock(async () => {})
|
|
};
|
|
|
|
// Create PID file
|
|
writePidFile({ pid: 99999, port: 37777, startedAt: new Date().toISOString() });
|
|
expect(existsSync(PID_FILE)).toBe(true);
|
|
|
|
const config: GracefulShutdownConfig = {
|
|
server: null,
|
|
sessionManager: mockSessionManager
|
|
};
|
|
|
|
await performGracefulShutdown(config);
|
|
|
|
// PID file should be removed
|
|
expect(existsSync(PID_FILE)).toBe(false);
|
|
});
|
|
|
|
it('should handle missing optional services gracefully', async () => {
|
|
const mockSessionManager: ShutdownableService = {
|
|
shutdownAll: mock(async () => {})
|
|
};
|
|
|
|
const config: GracefulShutdownConfig = {
|
|
server: null,
|
|
sessionManager: mockSessionManager
|
|
// mcpClient and dbManager are undefined
|
|
};
|
|
|
|
// Should not throw
|
|
await expect(performGracefulShutdown(config)).resolves.toBeUndefined();
|
|
|
|
// Session manager should still be called
|
|
expect(mockSessionManager.shutdownAll).toHaveBeenCalled();
|
|
});
|
|
|
|
it('should handle null server gracefully', async () => {
|
|
const mockSessionManager: ShutdownableService = {
|
|
shutdownAll: mock(async () => {})
|
|
};
|
|
|
|
const config: GracefulShutdownConfig = {
|
|
server: null,
|
|
sessionManager: mockSessionManager
|
|
};
|
|
|
|
// Should not throw
|
|
await expect(performGracefulShutdown(config)).resolves.toBeUndefined();
|
|
});
|
|
|
|
it('should call sessionManager.shutdownAll even without server', async () => {
|
|
const mockSessionManager: ShutdownableService = {
|
|
shutdownAll: mock(async () => {})
|
|
};
|
|
|
|
const config: GracefulShutdownConfig = {
|
|
server: null,
|
|
sessionManager: mockSessionManager
|
|
};
|
|
|
|
await performGracefulShutdown(config);
|
|
|
|
expect(mockSessionManager.shutdownAll).toHaveBeenCalledTimes(1);
|
|
});
|
|
|
|
it('should stop chroma server before database close', async () => {
|
|
const callOrder: string[] = [];
|
|
|
|
const mockSessionManager: ShutdownableService = {
|
|
shutdownAll: mock(async () => {
|
|
callOrder.push('sessionManager');
|
|
})
|
|
};
|
|
|
|
const mockMcpClient: CloseableClient = {
|
|
close: mock(async () => {
|
|
callOrder.push('mcpClient');
|
|
})
|
|
};
|
|
|
|
const mockDbManager: CloseableDatabase = {
|
|
close: mock(async () => {
|
|
callOrder.push('dbManager');
|
|
})
|
|
};
|
|
|
|
const mockChromaMcpManager = {
|
|
stop: mock(async () => {
|
|
callOrder.push('chromaMcpManager');
|
|
})
|
|
};
|
|
|
|
const config: GracefulShutdownConfig = {
|
|
server: null,
|
|
sessionManager: mockSessionManager,
|
|
mcpClient: mockMcpClient,
|
|
dbManager: mockDbManager,
|
|
chromaMcpManager: mockChromaMcpManager
|
|
};
|
|
|
|
await performGracefulShutdown(config);
|
|
|
|
expect(callOrder).toEqual(['sessionManager', 'mcpClient', 'chromaMcpManager', 'dbManager']);
|
|
});
|
|
|
|
it('should handle shutdown when PID file does not exist', async () => {
|
|
// Ensure PID file doesn't exist
|
|
removePidFile();
|
|
expect(existsSync(PID_FILE)).toBe(false);
|
|
|
|
const mockSessionManager: ShutdownableService = {
|
|
shutdownAll: mock(async () => {})
|
|
};
|
|
|
|
const config: GracefulShutdownConfig = {
|
|
server: null,
|
|
sessionManager: mockSessionManager
|
|
};
|
|
|
|
// Should not throw
|
|
await expect(performGracefulShutdown(config)).resolves.toBeUndefined();
|
|
});
|
|
});
|
|
});
|