diff --git a/apps/cli/src/backends/claude/sdk/metadataExtractor.test.ts b/apps/cli/src/backends/claude/sdk/metadataExtractor.test.ts new file mode 100644 index 000000000..a9a1f63ec --- /dev/null +++ b/apps/cli/src/backends/claude/sdk/metadataExtractor.test.ts @@ -0,0 +1,159 @@ +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +function withEnv(vars: Record, fn: () => Promise): Promise { + const prev: Record = {}; + for (const [key, value] of Object.entries(vars)) { + prev[key] = process.env[key]; + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + return fn().finally(() => { + for (const [key, value] of Object.entries(prev)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + }); +} + +async function waitForFile(path: string, timeoutMs: number): Promise { + const start = Date.now(); + for (;;) { + if (existsSync(path)) return; + if (Date.now() - start > timeoutMs) { + throw new Error(`Timed out waiting for file: ${path}`); + } + await new Promise((resolve) => setTimeout(resolve, 10)); + } +} + +async function waitForPidToExit(pid: number, timeoutMs: number): Promise { + const start = Date.now(); + for (;;) { + try { + process.kill(pid, 0); + } catch { + return; + } + if (Date.now() - start > timeoutMs) { + throw new Error(`Timed out waiting for PID ${pid} to exit`); + } + await new Promise((resolve) => setTimeout(resolve, 25)); + } +} + +async function withTimeout(promise: Promise, timeoutMs: number, label: string): Promise { + let timeoutId: NodeJS.Timeout | null = null; + const timeoutPromise = new Promise((_resolve, reject) => { + timeoutId = setTimeout(() => reject(new Error(`Timed out waiting for ${label} after ${timeoutMs}ms`)), timeoutMs); + }); + return Promise.race([promise, timeoutPromise]).finally(() => { + if (timeoutId) clearTimeout(timeoutId); + }); +} + +describe.sequential('claude sdk metadata extractor', () => { + let tmpRoot = ''; + + beforeEach(() => { + vi.resetModules(); + if (tmpRoot) { + rmSync(tmpRoot, { recursive: true, force: true }); + tmpRoot = ''; + } + }); + + it('aborts the claude process after capturing init metadata (no leak)', { timeout: 20_000 }, async () => { + tmpRoot = mkdtempSync(join(tmpdir(), 'happier-claude-metadata-extractor-init-')); + const pidFile = join(tmpRoot, 'pid.txt'); + const fakeClaude = join(tmpRoot, 'fake-claude.js'); + writeFileSync( + fakeClaude, + ` + const { writeFileSync } = require('node:fs'); + const pidFile = ${JSON.stringify(pidFile)}; + writeFileSync(pidFile, String(process.pid), 'utf8'); + + process.stdout.write(JSON.stringify({ + type: 'system', + subtype: 'init', + tools: ['tool-a'], + slash_commands: ['/cmd'], + }) + '\\n'); + + process.on('SIGTERM', () => process.exit(0)); + setInterval(() => {}, 1000); + `, + 'utf8', + ); + + await withEnv( + { + HAPPIER_CLAUDE_PATH: fakeClaude, + }, + async () => { + const { extractSDKMetadata } = await import('./metadataExtractor'); + const metadata = await withTimeout(extractSDKMetadata(), 2_000, 'extractSDKMetadata to resolve'); + expect(metadata).toEqual({ tools: ['tool-a'], slashCommands: ['/cmd'] }); + + await waitForFile(pidFile, 1_000); + const pid = Number.parseInt(readFileSync(pidFile, 'utf8').trim(), 10); + await waitForPidToExit(pid, 2_000); + }, + ); + }); + + it('respects a configurable extraction timeout and aborts on hang', { timeout: 20_000 }, async () => { + tmpRoot = mkdtempSync(join(tmpdir(), 'happier-claude-metadata-extractor-timeout-')); + const pidFile = join(tmpRoot, 'pid.txt'); + const fakeClaude = join(tmpRoot, 'fake-claude.js'); + writeFileSync( + fakeClaude, + ` + const { writeFileSync } = require('node:fs'); + const pidFile = ${JSON.stringify(pidFile)}; + writeFileSync(pidFile, String(process.pid), 'utf8'); + process.on('SIGTERM', () => process.exit(0)); + setInterval(() => {}, 1000); + `, + 'utf8', + ); + + await withEnv( + { + HAPPIER_CLAUDE_PATH: fakeClaude, + // Allow enough time for the subprocess to start under load, but keep the test fast. + HAPPIER_CLAUDE_SDK_METADATA_EXTRACTION_TIMEOUT_MS: '500', + }, + async () => { + const { extractSDKMetadata } = await import('./metadataExtractor'); + const resultPromise = extractSDKMetadata(); + + // Best-effort: if the child starts before the timeout triggers, ensure it exits. + let pid: number | null = null; + try { + await waitForFile(pidFile, 2_000); + pid = Number.parseInt(readFileSync(pidFile, 'utf8').trim(), 10); + } catch { + // ignore (abort may fire before the child starts) + } + + try { + await expect(withTimeout(resultPromise, 3_000, 'extractSDKMetadata to timeout')).resolves.toEqual({}); + } finally { + if (pid) { + try { + process.kill(pid, 'SIGTERM'); + } catch { + // ignore + } + await waitForPidToExit(pid, 2_000); + } + } + }, + ); + }); +}); diff --git a/apps/cli/src/backends/claude/sdk/metadataExtractor.ts b/apps/cli/src/backends/claude/sdk/metadataExtractor.ts index 5b4a2195f..0304578ec 100644 --- a/apps/cli/src/backends/claude/sdk/metadataExtractor.ts +++ b/apps/cli/src/backends/claude/sdk/metadataExtractor.ts @@ -12,16 +12,41 @@ export interface SDKMetadata { slashCommands?: string[] } +const DEFAULT_METADATA_EXTRACTION_TIMEOUT_MS = 10_000 +const MIN_METADATA_EXTRACTION_TIMEOUT_MS = 10 +const MAX_METADATA_EXTRACTION_TIMEOUT_MS = 120_000 + +function resolveMetadataExtractionTimeoutMs(): number { + const raw = typeof process.env.HAPPIER_CLAUDE_SDK_METADATA_EXTRACTION_TIMEOUT_MS === 'string' + ? process.env.HAPPIER_CLAUDE_SDK_METADATA_EXTRACTION_TIMEOUT_MS.trim() + : '' + const parsed = Number.parseInt(raw, 10) + if (!Number.isFinite(parsed)) return DEFAULT_METADATA_EXTRACTION_TIMEOUT_MS + return Math.max(MIN_METADATA_EXTRACTION_TIMEOUT_MS, Math.min(MAX_METADATA_EXTRACTION_TIMEOUT_MS, parsed)) +} + /** - * Extract SDK metadata by running a minimal query and capturing the init message + * Extract SDK metadata by running a minimal query and capturing the init message. + * + * Times out after the configured extraction timeout to prevent indefinite hangs + * when the spawned SDK process blocks (e.g. on slow or inaccessible filesystems). + * * @returns SDK metadata containing tools and slash commands */ export async function extractSDKMetadata(): Promise { const abortController = new AbortController() - + const timeoutMs = resolveMetadataExtractionTimeoutMs() + const timeoutId = setTimeout(() => { + logger.debug(`[metadataExtractor] Extraction timed out after ${timeoutMs}ms`) + abortController.abort() + }, timeoutMs) + if (typeof timeoutId.unref === 'function') { + timeoutId.unref() + } + try { logger.debug('[metadataExtractor] Starting SDK metadata extraction') - + // Run SDK with minimal tools allowed const sdkQuery = query({ prompt: 'hello', @@ -36,32 +61,34 @@ export async function extractSDKMetadata(): Promise { for await (const message of sdkQuery) { if (message.type === 'system' && message.subtype === 'init') { const systemMessage = message as SDKSystemMessage - + const metadata: SDKMetadata = { tools: systemMessage.tools, slashCommands: systemMessage.slash_commands } - + logger.debug('[metadataExtractor] Captured SDK metadata:', metadata) - + // Abort the query since we got what we need abortController.abort() - + return metadata } } - + logger.debug('[metadataExtractor] No init message received from SDK') return {} - + } catch (error) { - // Check if it's an abort error (expected) + // Check if it's an abort error (expected — either from timeout or after capture) if (error instanceof Error && error.name === 'AbortError') { - logger.debug('[metadataExtractor] SDK query aborted after capturing metadata') + logger.debug('[metadataExtractor] SDK query aborted (timeout or after capturing metadata)') return {} } logger.debug('[metadataExtractor] Error extracting SDK metadata:', error) return {} + } finally { + clearTimeout(timeoutId) } } @@ -79,4 +106,4 @@ export function extractSDKMetadataAsync(onComplete: (metadata: SDKMetadata) => v .catch(error => { logger.debug('[metadataExtractor] Async extraction failed:', error) }) -} \ No newline at end of file +}