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

Round 2 of Claude Code Review feedback on PR #1645:

Build guardrail (most important — protects the regression this PR fixes):

- scripts/build-hooks.js: post-build check that fails the build if
  mcp-server.cjs ever contains a `bun:sqlite` reference. This is the
  exact regression PR #1645 fixed; future contributors will get an
  immediate, actionable error if a transitive import re-introduces it.
  Verified the check trips when violated.

Code clarity:

- src/servers/mcp-server.ts: drop dead `_originalLog` capture — it was
  never restored. Less code is fewer bugs.

- src/servers/mcp-server.ts: elevate `cwd()` fallback log from WARN to
  ERROR. Per reviewer: a wrong WORKER_SCRIPT_PATH means worker auto-start
  silently fails, so the breadcrumb should be loud and searchable.

- src/services/worker-service.ts: extended doc comment on the
  `ensureWorkerStartedShared(port, __filename)` wrapper explaining why
  `__filename` is the correct script path here (CJS bundle = compiled
  worker-service.cjs) and why mcp-server.ts can't use the same trick.

- src/services/infrastructure/ProcessManager.ts: inline comment on the
  `env.BUN === 'bun'` bare-command guard explaining why it's reachable
  even though `isBunExecutablePath('bun')` is true (pathExists returns
  false for relative names, so the second branch is what fires).

Coverage:

- src/services/infrastructure/ProcessManager.ts: add `/usr/bin/bun` to
  the Linux candidate paths so apt-installed Bun on Debian/Ubuntu is
  found without falling through to the PATH lookup.

Out-of-scope items (deferred with rationale in PR replies):

- Unit tests for ensureWorkerStarted / Windows cooldown helpers — needs
  injectable-I/O refactor unsuitable for a hotfix.
- Sentinel object for Windows spawnDaemon `0` — broader API change.
- Windows Scoop install path — follow-up for a future PR.
- runOneTimeChromaMigration placement, aggressiveStartupCleanup,
  console.log redirect timing, platform timeout multiplier — all
  pre-existing and unrelated to this regression.

Verified: build clean, guardrail trips on simulated violation,
mcp-server.cjs still 0 bun:sqlite refs, ProcessManager tests 43/43.

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

scripts/build-hooks.js

@@ -244,6 +244,18 @@ async function buildHooks() {
     const mcpServerStats = fs.statSync(`${hooksDir}/${MCP_SERVER.name}.cjs`);
     console.log(`✓ mcp-server built (${(mcpServerStats.size / 1024).toFixed(2)} KB)`);
 
+    // GUARDRAIL (#1645): The MCP server runs under Node, but `bun:sqlite` is
+    // a Bun-only module. If any transitive import in mcp-server.ts ever pulls
+    // it back in, the bundle will crash on first require under Node — which
+    // is exactly the regression PR #1645 fixed. Fail the build instead of
+    // shipping a broken bundle so future contributors get an immediate signal.
+    const mcpBundleContent = fs.readFileSync(`${hooksDir}/${MCP_SERVER.name}.cjs`, 'utf-8');
+    if (mcpBundleContent.includes('bun:sqlite')) {
+      throw new Error(
+        `mcp-server.cjs contains a 'bun:sqlite' reference. This means a transitive import in src/servers/mcp-server.ts pulled in code from worker-service.ts (or another module that touches DatabaseManager/ChromaSync). The MCP server runs under Node and cannot load bun:sqlite. Audit recent imports in src/servers/mcp-server.ts and src/services/worker-spawner.ts — the spawner module is intentionally lightweight and MUST NOT import anything that touches SQLite. See PR #1645 for context.`
+      );
+    }
+
     // Build context generator
     console.log(`\n🔧 Building context generator...`);
     await build({

src/servers/mcp-server.ts

@@ -16,7 +16,6 @@ import { logger } from '../utils/logger.js';
 // CRITICAL: Redirect console to stderr BEFORE other imports
 // MCP uses stdio transport where stdout is reserved for JSON-RPC protocol messages.
 // Any logs to stdout break the protocol (Claude Desktop parses "[2025..." as JSON array).
-const _originalLog = console['log'];
 console['log'] = (...args: any[]) => {
   logger.error('CONSOLE', 'Intercepted console output (MCP protocol protection)', undefined, { args });
 };
@@ -47,10 +46,11 @@ const mcpServerDir = (() => {
   } catch {
     // Last-ditch fallback: cwd is almost certainly wrong, but throwing here
     // would crash the MCP server before it can serve a single request. Log
-    // loud so the existence check below has a useful breadcrumb to point at.
-    logger.warn(
+    // at ERROR so the existence check below has a loud, searchable breadcrumb
+    // — a wrong WORKER_SCRIPT_PATH means worker auto-start will silently fail.
+    logger.error(
       'SYSTEM',
-      'mcp-server: unable to resolve __dirname or import.meta.url; falling back to process.cwd() — WORKER_SCRIPT_PATH will likely be wrong'
+      'mcp-server: unable to resolve __dirname or import.meta.url; falling back to process.cwd() — WORKER_SCRIPT_PATH will almost certainly be wrong and worker auto-start will fail'
     );
     return process.cwd();
   }

src/services/infrastructure/ProcessManager.ts

@@ -112,6 +112,7 @@ export function resolveWorkerRuntimePath(options: RuntimeResolverOptions = {}):
         '/usr/local/bin/bun',
         '/opt/homebrew/bin/bun',
         '/home/linuxbrew/.linuxbrew/bin/bun',
+        '/usr/bin/bun', // Debian/Ubuntu apt install path
       ];
 
   for (const candidate of candidatePaths) {
@@ -122,7 +123,11 @@ export function resolveWorkerRuntimePath(options: RuntimeResolverOptions = {}):
       return normalized;
     }
 
-    // Allow command-style values from env (e.g. BUN=bun)
+    // Allow command-style values from env (e.g. BUN=bun). The previous branch
+    // would also match this candidate via isBunExecutablePath('bun') === true,
+    // but pathExists('bun') is false because it's a relative name — so this
+    // branch is what actually fires for the bare-command case. We return the
+    // bare name unchanged so child_process.spawn() resolves it via PATH.
     if (normalized.toLowerCase() === 'bun') {
       return normalized;
     }

src/services/worker-service.ts

@@ -992,8 +992,14 @@ export class WorkerService {
  * Ensures the worker is started and healthy.
  *
  * Thin wrapper around the canonical implementation in ./worker-spawner.ts.
- * When called from worker-service.ts itself, `__filename` resolves to the
- * worker-service bundle, which is the correct target for spawnDaemon.
+ *
+ * `__filename` is forwarded as the worker script path because, in the CJS
+ * bundle that ships to users, `__filename` always resolves to the compiled
+ * `worker-service.cjs` itself — which is exactly the script the spawner
+ * needs to relaunch as a detached daemon. The MCP server (a separate Node
+ * bundle) cannot rely on its own `__filename` because that would point at
+ * `mcp-server.cjs`, so it computes the worker path explicitly via
+ * `dirname(__filename) + 'worker-service.cjs'` instead.
  *
  * @param port - The TCP port (used for port-in-use checks and daemon spawn)
  * @returns true if worker is healthy (existing or newly started), false on failure