Merge pull request #1122 from thedotmack/claude/friendly-pascal

fix: resolve orphaned subprocesses and Chroma HTTP regressions
This commit is contained in:
Alex Newman
2026-02-15 23:31:10 -05:00
committed by GitHub
6 changed files with 299 additions and 257 deletions
+1 -1
View File
@@ -97,8 +97,8 @@
}, },
"dependencies": { "dependencies": {
"@anthropic-ai/claude-agent-sdk": "^0.1.76", "@anthropic-ai/claude-agent-sdk": "^0.1.76",
"@chroma-core/default-embed": "^0.1.9",
"@modelcontextprotocol/sdk": "^1.25.1", "@modelcontextprotocol/sdk": "^1.25.1",
"@chroma-core/default-embed": "^0.1.9",
"ansi-to-html": "^0.7.2", "ansi-to-html": "^0.7.2",
"chromadb": "^3.2.2", "chromadb": "^3.2.2",
"dompurify": "^3.3.1", "dompurify": "^3.3.1",
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
+39 -9
View File
@@ -11,7 +11,7 @@
import { spawn, ChildProcess, execSync } from 'child_process'; import { spawn, ChildProcess, execSync } from 'child_process';
import path from 'path'; import path from 'path';
import os from 'os'; import os from 'os';
import fs from 'fs'; import fs, { existsSync } from 'fs';
import { logger } from '../../utils/logger.js'; import { logger } from '../../utils/logger.js';
export interface ChromaServerConfig { export interface ChromaServerConfig {
@@ -108,14 +108,35 @@ export class ChromaServerManager {
// Cross-platform: use npx.cmd on Windows // Cross-platform: use npx.cmd on Windows
const isWindows = process.platform === 'win32'; const isWindows = process.platform === 'win32';
const command = isWindows ? 'npx.cmd' : 'npx';
const args = [ // Resolve chroma binary absolutely — npx fails when spawned from cache dirs (#1120)
'chroma', 'run', let command: string;
'--path', this.config.dataDir, let args: string[];
'--host', this.config.host, try {
'--port', String(this.config.port) // chromadb package installs a 'chroma' bin entry
]; const chromaBinDir = path.dirname(require.resolve('chromadb/package.json'));
// Check project-level .bin first (most common npm/bun installation layout)
const projectBin = path.join(chromaBinDir, '..', '.bin', isWindows ? 'chroma.cmd' : 'chroma');
// Fallback: nested node_modules .bin (rare — pnpm or workspace hoisting)
const nestedBin = path.join(chromaBinDir, 'node_modules', '.bin', isWindows ? 'chroma.cmd' : 'chroma');
if (existsSync(projectBin)) {
command = projectBin;
} else if (existsSync(nestedBin)) {
command = nestedBin;
} else {
// Last resort: npx with explicit cwd
command = isWindows ? 'npx.cmd' : 'npx';
}
} catch {
command = isWindows ? 'npx.cmd' : 'npx';
}
if (command.includes('npx')) {
args = ['chroma', 'run', '--path', this.config.dataDir, '--host', this.config.host, '--port', String(this.config.port)];
} else {
args = ['run', '--path', this.config.dataDir, '--host', this.config.host, '--port', String(this.config.port)];
}
logger.info('CHROMA_SERVER', 'Starting Chroma server', { logger.info('CHROMA_SERVER', 'Starting Chroma server', {
command, command,
@@ -125,11 +146,20 @@ export class ChromaServerManager {
const spawnEnv = this.getSpawnEnv(); const spawnEnv = this.getSpawnEnv();
// Resolve cwd for npx fallback — ensures node_modules is findable (#1120)
let spawnCwd: string | undefined;
try {
spawnCwd = path.dirname(require.resolve('chromadb/package.json'));
} catch {
// If chromadb isn't resolvable, omit cwd and let npx handle it
}
this.serverProcess = spawn(command, args, { this.serverProcess = spawn(command, args, {
stdio: ['ignore', 'pipe', 'pipe'], stdio: ['ignore', 'pipe', 'pipe'],
detached: !isWindows, // Don't detach on Windows (no process groups) detached: !isWindows, // Don't detach on Windows (no process groups)
windowsHide: true, // Hide console window on Windows windowsHide: true, // Hide console window on Windows
env: spawnEnv env: spawnEnv,
...(spawnCwd && { cwd: spawnCwd })
}); });
// Log server output for debugging // Log server output for debugging
+8 -5
View File
@@ -189,17 +189,20 @@ export class ChromaSync {
} }
try { try {
// getOrCreateCollection handles both cases // Use WASM backend to avoid native ONNX binary issues (#1104, #1105, #1110).
// Lazy-load DefaultEmbeddingFunction to avoid eagerly pulling in // Same model (all-MiniLM-L6-v2), same embeddings, but runs in WASM —
// @huggingface/transformers → sharp native binaries at bundle startup // no native binary loading, no segfaults, no ENOENT errors.
const { DefaultEmbeddingFunction } = await import('@chroma-core/default-embed'); const { DefaultEmbeddingFunction } = await import('@chroma-core/default-embed');
const embeddingFunction = new DefaultEmbeddingFunction(); const embeddingFunction = new DefaultEmbeddingFunction({ wasm: true });
this.collection = await this.chromaClient.getOrCreateCollection({ this.collection = await this.chromaClient.getOrCreateCollection({
name: this.collectionName, name: this.collectionName,
embeddingFunction embeddingFunction
}); });
logger.debug('CHROMA_SYNC', 'Collection ready', { collection: this.collectionName }); logger.debug('CHROMA_SYNC', 'Collection ready', {
collection: this.collectionName
});
} catch (error) { } catch (error) {
logger.error('CHROMA_SYNC', 'Failed to get/create collection', { collection: this.collectionName }, error as Error); logger.error('CHROMA_SYNC', 'Failed to get/create collection', { collection: this.collectionName }, error as Error);
throw new Error(`Collection setup failed: ${error instanceof Error ? error.message : String(error)}`); throw new Error(`Collection setup failed: ${error instanceof Error ? error.message : String(error)}`);
+10 -1
View File
@@ -141,7 +141,9 @@ export class SDKAgent {
} }
}); });
// Process SDK messages // Process SDK messages — cleanup in finally ensures subprocess termination
// even if the loop throws (e.g., context overflow, invalid API key)
try {
for await (const message of queryResult) { for await (const message of queryResult) {
// Capture or update memory session ID from SDK message // Capture or update memory session ID from SDK message
// IMPORTANT: The SDK may return a DIFFERENT session_id on resume than what we sent! // IMPORTANT: The SDK may return a DIFFERENT session_id on resume than what we sent!
@@ -271,6 +273,13 @@ export class SDKAgent {
// Usage telemetry is captured at SDK level // Usage telemetry is captured at SDK level
} }
} }
} finally {
// Ensure subprocess is terminated after query completes (or on error)
const tracked = getProcessBySession(session.sessionDbId);
if (tracked && !tracked.process.killed && tracked.process.exitCode === null) {
await ensureProcessExit(tracked, 5000);
}
}
// Mark session complete // Mark session complete
const sessionDuration = Date.now() - session.startTime; const sessionDuration = Date.now() - session.startTime;