fix(cli): write tool output to stdout via fd 1 instead of stderr (#324) (#346)

gitnexus/src/cli/eval-server.ts

@@ -25,6 +25,7 @@
  */
 
 import http from 'http';
+import { writeSync } from 'node:fs';
 import { LocalBackend } from '../mcp/local/local-backend.js';
 
 export interface EvalServerOptions {
@@ -142,7 +143,10 @@ export function formatContextResult(result: any): string {
 }
 
 export function formatImpactResult(result: any): string {
-  if (result.error) return `Error: ${result.error}`;
+  if (result.error) {
+    const suggestion = result.suggestion ? `\nSuggestion: ${result.suggestion}` : '';
+    return `Error: ${result.error}${suggestion}`;
+  }
 
   const target = result.target;
   const direction = result.direction;
@@ -155,7 +159,11 @@ export function formatImpactResult(result: any): string {
 
   const lines: string[] = [];
   const dirLabel = direction === 'upstream' ? 'depends on this (will break if changed)' : 'this depends on';
-  lines.push(`Blast radius for ${target?.kind || ''} ${target?.name} (${direction}): ${total} symbol(s) ${dirLabel}\n`);
+  lines.push(`Blast radius for ${target?.kind || ''} ${target?.name} (${direction}): ${total} symbol(s) ${dirLabel}`);
+  if (result.partial) {
+    lines.push('⚠️  Partial results — graph traversal was interrupted. Deeper impacts may exist.');
+  }
+  lines.push('');
 
   const depthLabels: Record<number, string> = {
     1: 'WILL BREAK (direct)',
@@ -401,9 +409,10 @@ export async function evalServerCommand(options?: EvalServerOptions): Promise<vo
       console.error(`  Auto-shutdown after ${idleTimeoutSec}s idle`);
     }
     try {
-      process.stdout.write(`GITNEXUS_EVAL_SERVER_READY:${port}\n`);
+      // Use fd 1 directly — LadybugDB captures process.stdout (#324)
+      writeSync(1, `GITNEXUS_EVAL_SERVER_READY:${port}\n`);
     } catch {
-      // stdout may not be available
+      // stdout may not be available (e.g., broken pipe)
     }
   });
 

gitnexus/src/cli/tool.ts

@@ -10,10 +10,12 @@
  *   gitnexus impact --target "AuthService" --direction upstream
  *   gitnexus cypher "MATCH (n:Function) RETURN n.name LIMIT 10"
  * 
- * Note: Output goes to stderr because LadybugDB's native module captures stdout
- * at the OS level during init. This is consistent with augment.ts.
+ * Note: Output goes to stdout via fs.writeSync(fd 1), bypassing LadybugDB's
+ * native module which captures the Node.js process.stdout stream during init.
+ * See the output() function for details (#324).
  */
 
+import { writeSync } from 'node:fs';
 import { LocalBackend } from '../mcp/local/local-backend.js';
 
 let _backend: LocalBackend | null = null;
@@ -29,10 +31,29 @@ async function getBackend(): Promise<LocalBackend> {
   return _backend;
 }
 
+/**
+ * Write tool output to stdout using low-level fd write.
+ *
+ * LadybugDB's native module captures Node.js process.stdout during init,
+ * but the underlying OS file descriptor 1 (stdout) remains intact.
+ * By using fs.writeSync(1, ...) we bypass the Node.js stream layer
+ * and write directly to the real stdout fd (#324).
+ *
+ * Falls back to stderr if the fd write fails (e.g., broken pipe).
+ */
 function output(data: any): void {
   const text = typeof data === 'string' ? data : JSON.stringify(data, null, 2);
-  // stderr because LadybugDB captures stdout at OS level
-  process.stderr.write(text + '\n');
+  try {
+    writeSync(1, text + '\n');
+  } catch (err: any) {
+    if (err?.code === 'EPIPE') {
+      // Consumer closed the pipe (e.g., `gitnexus cypher ... | head -1`)
+      // Exit cleanly per Unix convention
+      process.exit(0);
+    }
+    // Fallback: stderr (previous behavior, works on all platforms)
+    process.stderr.write(text + '\n');
+  }
 }
 
 export async function queryCommand(queryText: string, options?: {
@@ -93,14 +114,25 @@ export async function impactCommand(target: string, options?: {
   }
 
   const backend = await getBackend();
-  const result = await backend.callTool('impact', {
-    target,
-    direction: options?.direction || 'upstream',
-    maxDepth: options?.depth ? parseInt(options.depth) : undefined,
-    includeTests: options?.includeTests ?? false,
-    repo: options?.repo,
-  });
-  output(result);
+  try {
+    const result = await backend.callTool('impact', {
+      target,
+      direction: options?.direction || 'upstream',
+      maxDepth: options?.depth ? parseInt(options.depth) : undefined,
+      includeTests: options?.includeTests ?? false,
+      repo: options?.repo,
+    });
+    output(result);
+  } catch (err: any) {
+    // Return structured error JSON instead of crashing (#321)
+    output({
+      error: err?.message || 'Impact analysis failed unexpectedly',
+      target,
+      direction: options?.direction || 'upstream',
+      suggestion: 'Try reducing --depth or using gitnexus context <symbol> as a fallback',
+    });
+    process.exit(1);
+  }
 }
 
 export async function cypherCommand(query: string, options?: {

gitnexus/src/mcp/local/local-backend.ts

@@ -1329,6 +1329,30 @@ export class LocalBackend {
     relationTypes?: string[];
     includeTests?: boolean;
     minConfidence?: number;
+  }): Promise<any> {
+    try {
+      return await this._impactImpl(repo, params);
+    } catch (err: any) {
+      // Return structured error instead of crashing (#321)
+      return {
+        error: err?.message || 'Impact analysis failed',
+        target: params.target,
+        direction: params.direction,
+        impactedCount: 0,
+        risk: 'UNKNOWN',
+        partial: true,
+        suggestion: 'The graph query failed — try gitnexus context <symbol> as a fallback',
+      };
+    }
+  }
+
+  private async _impactImpl(repo: RepoHandle, params: {
+    target: string;
+    direction: 'upstream' | 'downstream';
+    maxDepth?: number;
+    relationTypes?: string[];
+    includeTests?: boolean;
+    minConfidence?: number;
   }): Promise<any> {
     await this.ensureInitialized(repo.id);
     
@@ -1358,6 +1382,7 @@ export class LocalBackend {
     const impacted: any[] = [];
     const visited = new Set<string>([symId]);
     let frontier = [symId];
+    let traversalComplete = true;
     
     for (let depth = 1; depth <= maxDepth && frontier.length > 0; depth++) {
       const nextFrontier: string[] = [];
@@ -1391,7 +1416,13 @@ export class LocalBackend {
             });
           }
         }
-      } catch (e) { logQueryError('impact:depth-traversal', e); }
+      } catch (e) {
+        logQueryError('impact:depth-traversal', e);
+        // Break out of depth loop on query failure but return partial results
+        // collected so far, rather than silently swallowing the error (#321)
+        traversalComplete = false;
+        break;
+      }
       
       frontier = nextFrontier;
     }
@@ -1474,6 +1505,7 @@ export class LocalBackend {
       direction,
       impactedCount: impacted.length,
       risk,
+      ...(!traversalComplete && { partial: true }),
       summary: {
         direct: directCount,
         processes_affected: processCount,

gitnexus/test/integration/cli-e2e.test.ts

@@ -9,7 +9,7 @@
  * Accepts status === null (timeout) as valid on slow CI runners.
  */
 import { describe, it, expect, beforeAll, afterAll } from 'vitest';
-import { spawnSync } from 'child_process';
+import { spawnSync, spawn } from 'child_process';
 import path from 'path';
 import fs from 'fs';
 import os from 'os';
@@ -231,4 +231,176 @@ describe('CLI end-to-end', () => {
     });
 
   });
+
+  // ─── stdout fd 1 tests (#324) ───────────────────────────────────────
+  // These tests verify that tool output goes to stdout (fd 1), not stderr.
+  // Requires analyze to have run first (the analyze test above populates .gitnexus/).
+
+  describe('tool output goes to stdout via fd 1 (#324)', () => {
+    it('cypher: JSON appears on stdout, not stderr', () => {
+      const result = runCliRaw(['cypher', 'MATCH (n) RETURN n.name LIMIT 3'], MINI_REPO);
+      if (result.status === null) return; // CI timeout tolerance
+
+      expect(result.status).toBe(0);
+
+      // stdout must contain valid JSON (array or object)
+      expect(() => JSON.parse(result.stdout.trim())).not.toThrow();
+
+      // stderr must NOT contain JSON — only human-readable diagnostics allowed
+      const stderrTrimmed = result.stderr.trim();
+      if (stderrTrimmed.length > 0) {
+        expect(() => JSON.parse(stderrTrimmed)).toThrow();
+      }
+    });
+
+    it('query: JSON appears on stdout, not stderr', () => {
+      // "handler" is a generic term likely to match something in mini-repo
+      const result = runCliRaw(['query', 'handler'], MINI_REPO);
+      if (result.status === null) return;
+
+      expect(result.status).toBe(0);
+      expect(() => JSON.parse(result.stdout.trim())).not.toThrow();
+    });
+
+    it('impact: JSON appears on stdout, not stderr', () => {
+      const result = runCliRaw(
+        ['impact', 'handleRequest', '--direction', 'upstream'],
+        MINI_REPO,
+      );
+      if (result.status === null) return;
+
+      expect(result.status).toBe(0);
+      // impact may return an error object (symbol not found) or a real result —
+      // either way it must be valid JSON on stdout
+      expect(() => JSON.parse(result.stdout.trim())).not.toThrow();
+    });
+
+    it('stdout is pipeable: cypher output parses as valid JSON', () => {
+      const result = runCliRaw(
+        ['cypher', 'MATCH (n:Function) RETURN n.name LIMIT 5'],
+        MINI_REPO,
+      );
+      if (result.status === null) return;
+
+      expect(result.status).toBe(0);
+
+      // Simulate what jq does: parse stdout as JSON
+      const parsed = JSON.parse(result.stdout.trim());
+      expect(Array.isArray(parsed) || typeof parsed === 'object').toBe(true);
+    });
+  });
+
+  // ─── EPIPE clean exit test (#324) ───────────────────────────────────
+
+  describe('EPIPE handling (#324)', () => {
+    it('cypher: EPIPE exits with code 0, not stderr dump', () => {
+      return new Promise<void>((resolve, reject) => {
+        const child = spawn(
+          process.execPath,
+          ['--import', 'tsx', cliEntry, 'cypher', 'MATCH (n) RETURN n LIMIT 500'],
+          {
+            cwd: MINI_REPO,
+            stdio: ['ignore', 'pipe', 'pipe'],
+            env: {
+              ...process.env,
+              NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --max-old-space-size=8192`.trim(),
+            },
+          },
+        );
+
+        let stderrOutput = '';
+        child.stderr.on('data', (chunk: Buffer) => { stderrOutput += chunk.toString(); });
+
+        // Destroy stdout immediately — simulates `| head -0` (consumer closes early)
+        child.stdout.once('data', () => {
+          child.stdout.destroy(); // triggers EPIPE on next write
+        });
+
+        const timer = setTimeout(() => {
+          child.kill('SIGTERM');
+          // Timeout is acceptable on CI — not a failure
+          resolve();
+        }, 20000);
+
+        child.on('close', (code) => {
+          clearTimeout(timer);
+          try {
+            // Clean EPIPE exit: code 0
+            expect(code).toBe(0);
+            // No JSON payload should appear on stderr
+            const trimmed = stderrOutput.trim();
+            if (trimmed.length > 0) {
+              expect(() => JSON.parse(trimmed)).toThrow();
+            }
+            resolve();
+          } catch (err) {
+            reject(err);
+          }
+        });
+      });
+    }, 25000);
+  });
+
+  // ─── eval-server READY signal test (#324) ───────────────────────────
+
+  describe('eval-server READY signal (#324)', () => {
+    it('READY signal appears on stdout, not stderr', () => {
+      return new Promise<void>((resolve, reject) => {
+        const child = spawn(
+          process.execPath,
+          ['--import', 'tsx', cliEntry, 'eval-server', '--port', '0', '--idle-timeout', '3'],
+          {
+            cwd: MINI_REPO,
+            stdio: ['ignore', 'pipe', 'pipe'],
+            env: {
+              ...process.env,
+              NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --max-old-space-size=8192`.trim(),
+            },
+          },
+        );
+
+        let stdoutBuffer = '';
+        let foundOnStdout = false;
+        let foundOnStderr = false;
+
+        child.stdout.on('data', (chunk: Buffer) => {
+          stdoutBuffer += chunk.toString();
+          if (stdoutBuffer.includes('GITNEXUS_EVAL_SERVER_READY:')) {
+            foundOnStdout = true;
+            child.kill('SIGTERM');
+          }
+        });
+
+        child.stderr.on('data', (chunk: Buffer) => {
+          const text = chunk.toString();
+          if (text.includes('GITNEXUS_EVAL_SERVER_READY:')) {
+            foundOnStderr = true;
+            child.kill('SIGTERM');
+          }
+        });
+
+        const timer = setTimeout(() => {
+          child.kill('SIGTERM');
+          // Timeout is acceptable on CI — not a failure
+          resolve();
+        }, 30000);
+
+        child.on('close', () => {
+          clearTimeout(timer);
+          try {
+            if (foundOnStderr) {
+              reject(new Error('READY signal appeared on stderr instead of stdout'));
+            } else if (foundOnStdout) {
+              resolve();
+            } else {
+              // eval-server may not start on all CI environments — don't fail
+              resolve();
+            }
+          } catch (err) {
+            reject(err);
+          }
+        });
+      });
+    }, 35000);
+  });
 });

gitnexus/test/unit/eval-formatters.test.ts

@@ -147,7 +147,34 @@ describe('formatContextResult', () => {
 
 describe('formatImpactResult', () => {
   it('returns error message for error input', () => {
-    expect(formatImpactResult({ error: 'bad request' })).toBe('Error: bad request');
+    expect(formatImpactResult({ error: 'bad request' })).toContain('Error: bad request');
+  });
+
+  it('returns error with suggestion when provided', () => {
+    const result = formatImpactResult({
+      error: 'Impact analysis failed',
+      suggestion: 'Try gitnexus context <symbol> as a fallback',
+    });
+    expect(result).toContain('Error: Impact analysis failed');
+    expect(result).toContain('Suggestion: Try gitnexus context');
+  });
+
+  it('shows partial warning when traversal was interrupted', () => {
+    const result = formatImpactResult({
+      target: { kind: 'Function', name: 'foo' },
+      direction: 'upstream',
+      impactedCount: 2,
+      partial: true,
+      byDepth: {
+        1: [
+          { type: 'Function', name: 'caller1', filePath: 'src/a.ts', relationType: 'CALLS', confidence: 1 },
+          { type: 'Function', name: 'caller2', filePath: 'src/b.ts', relationType: 'CALLS', confidence: 1 },
+        ],
+      },
+    });
+    expect(result).toContain('Partial results');
+    expect(result).toContain('caller1');
+    expect(result).toContain('caller2');
   });
 
   it('handles zero impact', () => {