Files
claude-mem/tests/infrastructure/graceful-shutdown.test.ts
T
Alex Newman 40daf8f3fa feat: replace WASM embeddings with persistent chroma-mcp MCP connection (#1176)
* 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>
2026-02-18 18:32:38 -05:00

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();
});
});
});