From 0e1e4673c504da147647c00506a522b6ca9ef8de Mon Sep 17 00:00:00 2001 From: aro-has00 <124628055+aro-has00@users.noreply.github.com> Date: Fri, 22 May 2026 11:32:20 -0700 Subject: [PATCH] fix: avoid forced exit after embedding analyze --- gitnexus/src/cli/analyze.ts | 34 ++++++++++++++----- .../unit/analyze-embeddings-limit.test.ts | 27 +++++++++++++++ 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index 32ceaca62..df278425f 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -5,7 +5,7 @@ * * Delegates core analysis to the shared runFullAnalysis orchestrator. * This CLI wrapper handles: heap management, progress bar, SIGINT, - * skill generation (--skills), summary output, and process.exit(). + * skill generation (--skills), summary output, and shutdown behavior. */ import path from 'path'; @@ -606,6 +606,20 @@ export const shouldGenerateCommunitySkillFiles = ( pipelineResult: unknown, ): boolean => Boolean(options?.skills && pipelineResult && !options?.indexOnly); +/** + * Force-exit only on successful non-embedding analyzes. + * + * The forced exit works around native LadybugDB handles that can otherwise keep + * short-lived CLI invocations alive. Local embedding runs initialize ONNX + * Runtime through transformers.js, and on macOS arm64 / Node 20 that native + * stack can abort during `process.exit(0)` with + * `std::__1::system_error: mutex lock failed`. Let embedding-enabled runs + * unwind naturally after runFullAnalysis has closed LadybugDB. + */ +export const shouldForceExitAfterAnalyzeSuccess = ( + options: Pick | undefined, +): boolean => !options?.embeddings; + export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOptions) => { if (await ensureHeap()) return; forceHeapOOMForTestIfEnabled(); @@ -617,10 +631,9 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption // Snapshot the GITNEXUS_* env vars that the impl writes for downstream // consumption, so they don't leak across `analyzeCommand` invocations in - // programmatic callers (tests, long-running hosts). `process.exit(0)` on - // the success path bypasses `finally` — intentional: when the process is - // exiting, restoration is moot. For early-return paths (validation - // errors) and the alreadyUpToDate fast path the finally restores the + // programmatic callers (tests, long-running hosts). A forced process exit on + // the non-embedding success path bypasses `finally` because the process is + // terminating. Natural-return paths, including embedding success, restore the // pre-call values. const envSnap = snapshotAnalyzeEnv(); try { @@ -1251,8 +1264,11 @@ const analyzeCommandImpl = async (inputPath?: string, options?: AnalyzeOptions): return; } - // LadybugDB's native module holds open handles that prevent Node from exiting. - // ONNX Runtime also registers native atexit hooks that segfault on some - // platforms (#38, #40). Force-exit to ensure clean termination. - process.exit(0); + if (shouldForceExitAfterAnalyzeSuccess(options)) { + // LadybugDB's native module can hold open handles that prevent Node from + // exiting after short-lived non-embedding analyzes. Embedding-enabled runs + // deliberately return naturally; forcing process.exit(0) after ONNX Runtime + // initialization can abort on macOS arm64 / Node 20 (#151). + process.exit(0); + } }; diff --git a/gitnexus/test/unit/analyze-embeddings-limit.test.ts b/gitnexus/test/unit/analyze-embeddings-limit.test.ts index 6fbc5af56..0b269a448 100644 --- a/gitnexus/test/unit/analyze-embeddings-limit.test.ts +++ b/gitnexus/test/unit/analyze-embeddings-limit.test.ts @@ -104,4 +104,31 @@ describe('analyzeCommand --embeddings [limit] parsing', () => { expect(opts.embeddings).toBe(false); expect(opts.embeddingsNodeLimit).toBeUndefined(); }); + + it('does not force process exit after a successful embedding analysis', async () => { + runFullAnalysisMock.mockResolvedValueOnce({ + repoName: 'repo', + repoPath: '/repo', + stats: { nodes: 1, edges: 0, communities: 0, processes: 0 }, + alreadyUpToDate: false, + }); + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: string | number | null) => { + throw new Error(`unexpected process.exit(${String(code)})`); + }) as never); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, { embeddings: true }); + + expect(exitSpy).not.toHaveBeenCalled(); + exitSpy.mockRestore(); + }); + + it('classifies successful analyze shutdown by embedding mode', async () => { + const { shouldForceExitAfterAnalyzeSuccess } = await import('../../src/cli/analyze.js'); + + expect(shouldForceExitAfterAnalyzeSuccess(undefined)).toBe(true); + expect(shouldForceExitAfterAnalyzeSuccess({})).toBe(true); + expect(shouldForceExitAfterAnalyzeSuccess({ embeddings: true })).toBe(false); + expect(shouldForceExitAfterAnalyzeSuccess({ embeddings: '0' })).toBe(false); + }); });