fix: [AI-198] prioritize managed engine venv over CWD .venv (#199)

* fix: [AI-198] prioritize managed engine venv over CWD `.venv` in `resolvePython()`

`ensureEngine()` installs altimate-engine into the managed venv at
`~/.opencode/data/engine/venv/`, but `resolvePython()` checked for a
`.venv` in the user's current working directory first. When running
from any Python project with its own `.venv`, the bridge would spawn
that project's Python — which doesn't have altimate-engine — and fail.

Swap the priority so the managed engine venv is checked before the
CWD `.venv`. Add a test covering the new ordering.

Closes #198

* fix: [AI-196] install `altimate-engine[warehouses]` extra by default

The managed engine install was missing the `[warehouses]` optional
dependency group, which includes `snowflake-connector-python`,
`psycopg2-binary`, `duckdb`, and other database connectors. This
caused FinOps tools and warehouse operations to fail with
"snowflake-connector-python not installed" errors.

Closes #196

* fix: harden engine bootstrap — validate venv, track extras, fix Windows paths, add startup mutex

- Validate Python binary exists on disk before trusting manifest
  (fixes: manifest exists but venv manually deleted → silent failure)
- Track installed extras in manifest so version match with different
  extras (e.g. upgrading from no-extras to `[warehouses]`) triggers reinstall
- Use platform-aware `venvPythonBin()` helper for dev/cwd venv paths
  (fixes: Windows uses `Scripts/python.exe`, not `bin/python`)
- Add `pendingStart` mutex to `Bridge.call()` preventing concurrent
  `start()` calls from spawning duplicate Python processes
- Add 25 new tests covering: priority ordering, env var edge cases,
  extras tracking, manifest validation, Windows paths, startup mutex,
  concurrent calls

* fix: clean up child process on ping failure + use platform-aware paths in tests

Address Sentry bot review comments on PR #199:

1. When `start()` spawns a child but `ping` verification fails, kill
   and clear the `child` handle so subsequent `call()` invocations
   trigger a restart instead of writing to a broken process and
   hanging for 30s timeout.

2. Use platform-aware `testVenvPythonBin()` helper in test file for
   dev/cwd venv detection, matching production `venvPythonBin()`
   behavior. Fixes incorrect test skip logic on Windows.

* fix: address multi-model code review feedback

- Remove dead else branch in `ENGINE_INSTALL_SPEC` conditional (constant
  is always truthy)
- Add `extras` field to `engine_started` telemetry event for debugging
- Update `ENGINE_INSTALL_SPEC` comment to reflect production usage
- Tighten concurrent startup test assertion to verify mutex coalescing
  (`ensureEngineCalls <= 2` instead of just `>= 1`)

* fix: handle spawn errors, venv recovery, and post-mutex child guard

Address Gemini review findings:

1. Add `error` event handler on spawned child process to prevent
   unhandled ENOENT/EACCES from crashing the host process when
   `resolvePython()` returns a nonexistent or non-executable path.

2. Recreate venv when Python binary is missing even if the venv
   directory still exists (e.g. user deleted just the binary).

3. Re-check `child` state after awaiting `pendingStart` mutex —
   the process may have died between startup completing and the
   secondary caller resuming.

* fix: don't increment `restartCount` when process is killed by signal

When `child.kill()` is called (e.g. after ping failure), the exit
handler fires with `code = null`. The check `code !== 0` treated
this as a crash, incrementing `restartCount`. After two ping
failures, the bridge would be permanently disabled.

Changed to `code !== null && code !== 0` so only actual non-zero
exit codes (real crashes) count toward the restart limit.

Author: anandgupta42

packages/opencode/src/altimate/bridge/client.ts

@@ -14,6 +14,13 @@ import type { BridgeMethod, BridgeMethods } from "./protocol"
 import { Telemetry } from "../telemetry"
 import { Log } from "../../util/log"
 
+/** Platform-aware path to the python binary inside a venv directory. */
+function venvPythonBin(venvDir: string): string {
+  return process.platform === "win32"
+    ? path.join(venvDir, "Scripts", "python.exe")
+    : path.join(venvDir, "bin", "python")
+}
+
 /** Resolve the Python interpreter to use for the engine sidecar.
  *  Exported for testing — not part of the public API. */
 export function resolvePython(): string {
@@ -22,17 +29,20 @@ export function resolvePython(): string {
 
   // 2. Check for .venv relative to altimate-engine package (local dev)
   const engineDir = path.resolve(__dirname, "..", "..", "..", "altimate-engine")
-  const venvPython = path.join(engineDir, ".venv", "bin", "python")
+  const venvPython = venvPythonBin(path.join(engineDir, ".venv"))
   if (existsSync(venvPython)) return venvPython
 
-  // 3. Check for .venv in cwd
-  const cwdVenv = path.join(process.cwd(), ".venv", "bin", "python")
-  if (existsSync(cwdVenv)) return cwdVenv
-
-  // 4. Check the managed engine venv (created by ensureEngine)
+  // 3. Check the managed engine venv (created by ensureEngine)
+  //    This must come before the CWD venv check — ensureEngine() installs
+  //    altimate-engine here, so an unrelated .venv in the user's project
+  //    directory must not shadow it.
   const managedPython = enginePythonPath()
   if (existsSync(managedPython)) return managedPython
 
+  // 4. Check for .venv in cwd
+  const cwdVenv = venvPythonBin(path.join(process.cwd(), ".venv"))
+  if (existsSync(cwdVenv)) return cwdVenv
+
   // 5. Fallback
   return "python3"
 }
@@ -45,6 +55,8 @@ export namespace Bridge {
   const CALL_TIMEOUT_MS = 30_000
   const pending = new Map<number, { resolve: (value: any) => void; reject: (reason: any) => void }>()
   let buffer = ""
+  // Mutex to prevent concurrent start() calls from spawning duplicate processes
+  let pendingStart: Promise<void> | null = null
 
   export async function call<M extends BridgeMethod>(
     method: M,
@@ -53,7 +65,20 @@ export namespace Bridge {
     const startTime = Date.now()
     if (!child || child.exitCode !== null) {
       if (restartCount >= MAX_RESTARTS) throw new Error("Python bridge failed after max restarts")
-      await start()
+      if (pendingStart) {
+        await pendingStart
+        // Re-check: the process may have died between startup and now
+        if (!child || child.exitCode !== null) {
+          throw new Error("Bridge process died during startup")
+        }
+      } else {
+        pendingStart = start()
+        try {
+          await pendingStart
+        } finally {
+          pendingStart = null
+        }
+      }
     }
     const id = ++requestId
     const request = JSON.stringify({ jsonrpc: "2.0", method, params, id })
@@ -141,8 +166,18 @@ export namespace Bridge {
       if (msg) Log.Default.error("altimate-engine stderr", { message: msg })
     })
 
+    child.on("error", (err) => {
+      Log.Default.error("altimate-engine spawn error", { error: String(err) })
+      restartCount++
+      for (const [id, p] of pending) {
+        p.reject(new Error(`Bridge process failed to spawn: ${err}`))
+        pending.delete(id)
+      }
+      child = undefined
+    })
+
     child.on("exit", (code) => {
-      if (code !== 0) restartCount++
+      if (code !== null && code !== 0) restartCount++
       for (const [id, p] of pending) {
         p.reject(new Error(`Bridge process exited (code ${code})`))
         pending.delete(id)
@@ -154,6 +189,11 @@ export namespace Bridge {
     try {
       await call("ping", {} as any)
     } catch (e) {
+      // Clean up the spawned process so subsequent call() invocations
+      // correctly detect !child and trigger a restart instead of writing
+      // to a non-functional process and hanging until timeout.
+      child?.kill()
+      child = undefined
       throw new Error(`Failed to start Python bridge: ${e}`)
     }
   }

packages/opencode/src/altimate/bridge/engine.ts

@@ -25,12 +25,18 @@ declare const OPENCODE_VERSION: string
 // Mutex to prevent concurrent ensureEngine/ensureUv calls from corrupting state
 let pendingEnsure: Promise<void> | null = null
 
+/** Pip extras spec for altimate-engine (e.g. "warehouses" → altimate-engine[warehouses]).
+ *  Used in ensureEngine install command and recorded in manifest for upgrade detection. */
+export const ENGINE_INSTALL_SPEC = "warehouses"
+
 interface Manifest {
   engine_version: string
   python_version: string
   uv_version: string
   cli_version: string
   installed_at: string
+  /** Comma-separated extras that were installed (e.g. "warehouses") */
+  extras?: string
 }
 
 /** Returns path to the engine directory */
@@ -158,7 +164,12 @@ export async function ensureEngine(): Promise<void> {
 async function ensureEngineImpl(): Promise<void> {
   const manifest = await readManifest()
   const isUpgrade = manifest !== null
-  if (manifest && manifest.engine_version === ALTIMATE_ENGINE_VERSION) return
+
+  // Validate both version AND filesystem state — a matching version in the
+  // manifest is not enough if the venv or Python binary was deleted.
+  const pythonExists = existsSync(enginePythonPath())
+  const extrasMatch = (manifest?.extras ?? "") === ENGINE_INSTALL_SPEC
+  if (manifest && manifest.engine_version === ALTIMATE_ENGINE_VERSION && pythonExists && extrasMatch) return
 
   const startTime = Date.now()
 
@@ -168,8 +179,9 @@ async function ensureEngineImpl(): Promise<void> {
   const dir = engineDir()
   const venvDir = path.join(dir, "venv")
 
-  // Create venv if it doesn't exist
-  if (!existsSync(venvDir)) {
+  // Create venv if it doesn't exist, or recreate if the Python binary is missing
+  // (e.g. user deleted the binary but left the venv directory intact)
+  if (!existsSync(venvDir) || !pythonExists) {
     Log.Default.info("creating python environment")
     try {
       execFileSync(uv, ["venv", "--python", "3.12", venvDir], { stdio: "pipe" })
@@ -189,7 +201,8 @@ async function ensureEngineImpl(): Promise<void> {
   const pythonPath = enginePythonPath()
   Log.Default.info("installing altimate-engine", { version: ALTIMATE_ENGINE_VERSION })
   try {
-    execFileSync(uv, ["pip", "install", "--python", pythonPath, `altimate-engine==${ALTIMATE_ENGINE_VERSION}`], { stdio: "pipe" })
+    const spec = `altimate-engine[${ENGINE_INSTALL_SPEC}]==${ALTIMATE_ENGINE_VERSION}`
+    execFileSync(uv, ["pip", "install", "--python", pythonPath, spec], { stdio: "pipe" })
   } catch (e: any) {
     Telemetry.track({
       type: "engine_error",
@@ -212,6 +225,7 @@ async function ensureEngineImpl(): Promise<void> {
     uv_version: uvVersion,
     cli_version: typeof OPENCODE_VERSION === "string" ? OPENCODE_VERSION : "local",
     installed_at: new Date().toISOString(),
+    extras: ENGINE_INSTALL_SPEC,
   })
 
   Telemetry.track({
@@ -220,6 +234,7 @@ async function ensureEngineImpl(): Promise<void> {
     session_id: Telemetry.getContext().sessionId,
     engine_version: ALTIMATE_ENGINE_VERSION,
     python_version: pyVersion,
+    extras: ENGINE_INSTALL_SPEC,
     status: isUpgrade ? "upgraded" : "started",
     duration_ms: Date.now() - startTime,
   })

packages/opencode/src/altimate/telemetry/index.ts

@@ -153,6 +153,7 @@ export namespace Telemetry {
         session_id: string
         engine_version: string
         python_version: string
+        extras?: string
         status: "started" | "restarted" | "upgraded"
         duration_ms: number
       }