fix(mcp): address PR #1645 review feedback (round 4)

Round 4 of Claude Code Review feedback on PR #1645 (review of round-3
commit 193286f9):

tests/infrastructure/process-manager.test.ts: replace require('fs')
with the already-imported statSync. Reviewer correctly flagged that
the file uses ESM-style named imports everywhere else and the inline
require() calls would break under strict ESM. Two callsites updated
in the touchPidFile test.

src/services/infrastructure/ProcessManager.ts: hoist
resolveWorkerRuntimePath() and the `Bun runtime not found` error
handling out of both branches in spawnDaemon. Both Windows and Unix
branches need the same Bun lookup, and resolving once before the OS
branch split avoids a duplicate execSync('which bun')/where bun in the
no-well-known-path fallback. The error message is also DRY now —
single source of truth instead of two near-identical strings.

CodeRabbit confirmed in its previous reply that "All actionable items
across all four review rounds are fully resolved" — these two minor
items from claude-review of round 3 are the only remaining cleanup.

Verified: build clean, ProcessManager tests still 44/44.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

src/services/infrastructure/ProcessManager.ts

@@ -661,19 +661,24 @@ export function spawnDaemon(
     ...extraEnv
   });
 
+  // worker-service.cjs imports `bun:sqlite`, so the spawned runtime MUST be
+  // Bun on every platform — never the current process.execPath, which may be
+  // Node when the caller is the MCP server. Resolve once before the OS branch
+  // split so we don't pay for a duplicate PATH lookup if Bun isn't found at a
+  // well-known path. See resolveWorkerRuntimePath() for the candidate list.
+  const runtimePath = resolveWorkerRuntimePath();
+  if (!runtimePath) {
+    logger.error(
+      'SYSTEM',
+      'Bun runtime not found — install from https://bun.sh and ensure it is on PATH or set BUN env var. The worker daemon requires Bun because it uses bun:sqlite.'
+    );
+    return undefined;
+  }
+
   if (isWindows) {
     // Use PowerShell Start-Process to spawn a hidden, independent process
     // Unlike WMIC, PowerShell inherits environment variables from parent
     // -WindowStyle Hidden prevents console popup
-    const runtimePath = resolveWorkerRuntimePath();
-
-    if (!runtimePath) {
-      logger.error(
-        'SYSTEM',
-        'Bun runtime not found — install from https://bun.sh and ensure it is on PATH or set BUN env var. The worker daemon requires Bun because it uses bun:sqlite.'
-      );
-      return undefined;
-    }
 
     // Use -EncodedCommand to avoid all shell quoting issues with spaces in paths
     const psScript = `Start-Process -FilePath '${runtimePath.replace(/'/g, "''")}' -ArgumentList @('${scriptPath.replace(/'/g, "''")}','--daemon') -WindowStyle Hidden`;
@@ -704,22 +709,10 @@ export function spawnDaemon(
   // controlling terminal. This prevents SIGHUP from reaching the daemon
   // even if the in-process SIGHUP handler somehow fails (belt-and-suspenders).
   // Fall back to standard detached spawn if setsid is not available.
-  //
-  // IMPORTANT: worker-service.cjs imports `bun:sqlite`, so the spawned runtime
-  // MUST be Bun — never the current process.execPath, which may be Node when
-  // the caller is the MCP server. See resolveWorkerRuntimePath() for lookup.
-  const unixRuntimePath = resolveWorkerRuntimePath();
-  if (!unixRuntimePath) {
-    logger.error(
-      'SYSTEM',
-      'Bun runtime not found — install from https://bun.sh and ensure it is on PATH or set BUN env var. The worker daemon requires Bun because it uses bun:sqlite.'
-    );
-    return undefined;
-  }
-
+  // `runtimePath` was resolved at the top of this function (see comment there).
   const setsidPath = '/usr/bin/setsid';
   if (existsSync(setsidPath)) {
-    const child = spawn(setsidPath, [unixRuntimePath, scriptPath, '--daemon'], {
+    const child = spawn(setsidPath, [runtimePath, scriptPath, '--daemon'], {
       detached: true,
       stdio: 'ignore',
       env
@@ -734,7 +727,7 @@ export function spawnDaemon(
   }
 
   // Fallback: standard detached spawn (macOS, systems without setsid)
-  const child = spawn(unixRuntimePath, [scriptPath, '--daemon'], {
+  const child = spawn(runtimePath, [scriptPath, '--daemon'], {
     detached: true,
     stdio: 'ignore',
     env

tests/infrastructure/process-manager.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect, beforeEach, afterEach } from 'bun:test';
-import { existsSync, readFileSync, mkdirSync, writeFileSync, rmSync } from 'fs';
+import { existsSync, readFileSync, mkdirSync, writeFileSync, rmSync, statSync } from 'fs';
 import { homedir } from 'os';
 import { tmpdir } from 'os';
 import path from 'path';
@@ -419,7 +419,7 @@ describe('ProcessManager', () => {
       // Wait a bit to ensure measurable mtime difference
       await new Promise(r => setTimeout(r, 50));
 
-      const statsBefore = require('fs').statSync(PID_FILE);
+      const statsBefore = statSync(PID_FILE);
       const mtimeBefore = statsBefore.mtimeMs;
 
       // Wait again to ensure mtime advances
@@ -427,7 +427,7 @@ describe('ProcessManager', () => {
 
       touchPidFile();
 
-      const statsAfter = require('fs').statSync(PID_FILE);
+      const statsAfter = statSync(PID_FILE);
       const mtimeAfter = statsAfter.mtimeMs;
 
       expect(mtimeAfter).toBeGreaterThanOrEqual(mtimeBefore);