Skip to content

Commit a74d721

Browse files
committed
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
1 parent 6de0160 commit a74d721

3 files changed

Lines changed: 469 additions & 21 deletions

File tree

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

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ import type { BridgeMethod, BridgeMethods } from "./protocol"
1414
import { Telemetry } from "../telemetry"
1515
import { Log } from "../../util/log"
1616

17+
/** Platform-aware path to the python binary inside a venv directory. */
18+
function venvPythonBin(venvDir: string): string {
19+
return process.platform === "win32"
20+
? path.join(venvDir, "Scripts", "python.exe")
21+
: path.join(venvDir, "bin", "python")
22+
}
23+
1724
/** Resolve the Python interpreter to use for the engine sidecar.
1825
* Exported for testing — not part of the public API. */
1926
export function resolvePython(): string {
@@ -22,7 +29,7 @@ export function resolvePython(): string {
2229

2330
// 2. Check for .venv relative to altimate-engine package (local dev)
2431
const engineDir = path.resolve(__dirname, "..", "..", "..", "altimate-engine")
25-
const venvPython = path.join(engineDir, ".venv", "bin", "python")
32+
const venvPython = venvPythonBin(path.join(engineDir, ".venv"))
2633
if (existsSync(venvPython)) return venvPython
2734

2835
// 3. Check the managed engine venv (created by ensureEngine)
@@ -33,7 +40,7 @@ export function resolvePython(): string {
3340
if (existsSync(managedPython)) return managedPython
3441

3542
// 4. Check for .venv in cwd
36-
const cwdVenv = path.join(process.cwd(), ".venv", "bin", "python")
43+
const cwdVenv = venvPythonBin(path.join(process.cwd(), ".venv"))
3744
if (existsSync(cwdVenv)) return cwdVenv
3845

3946
// 5. Fallback
@@ -48,6 +55,8 @@ export namespace Bridge {
4855
const CALL_TIMEOUT_MS = 30_000
4956
const pending = new Map<number, { resolve: (value: any) => void; reject: (reason: any) => void }>()
5057
let buffer = ""
58+
// Mutex to prevent concurrent start() calls from spawning duplicate processes
59+
let pendingStart: Promise<void> | null = null
5160

5261
export async function call<M extends BridgeMethod>(
5362
method: M,
@@ -56,7 +65,16 @@ export namespace Bridge {
5665
const startTime = Date.now()
5766
if (!child || child.exitCode !== null) {
5867
if (restartCount >= MAX_RESTARTS) throw new Error("Python bridge failed after max restarts")
59-
await start()
68+
if (pendingStart) {
69+
await pendingStart
70+
} else {
71+
pendingStart = start()
72+
try {
73+
await pendingStart
74+
} finally {
75+
pendingStart = null
76+
}
77+
}
6078
}
6179
const id = ++requestId
6280
const request = JSON.stringify({ jsonrpc: "2.0", method, params, id })

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,17 @@ declare const OPENCODE_VERSION: string
2525
// Mutex to prevent concurrent ensureEngine/ensureUv calls from corrupting state
2626
let pendingEnsure: Promise<void> | null = null
2727

28+
/** The pip install spec used by ensureEngine — exported for tests. */
29+
export const ENGINE_INSTALL_SPEC = "warehouses"
30+
2831
interface Manifest {
2932
engine_version: string
3033
python_version: string
3134
uv_version: string
3235
cli_version: string
3336
installed_at: string
37+
/** Comma-separated extras that were installed (e.g. "warehouses") */
38+
extras?: string
3439
}
3540

3641
/** Returns path to the engine directory */
@@ -158,7 +163,12 @@ export async function ensureEngine(): Promise<void> {
158163
async function ensureEngineImpl(): Promise<void> {
159164
const manifest = await readManifest()
160165
const isUpgrade = manifest !== null
161-
if (manifest && manifest.engine_version === ALTIMATE_ENGINE_VERSION) return
166+
167+
// Validate both version AND filesystem state — a matching version in the
168+
// manifest is not enough if the venv or Python binary was deleted.
169+
const pythonExists = existsSync(enginePythonPath())
170+
const extrasMatch = (manifest?.extras ?? "") === ENGINE_INSTALL_SPEC
171+
if (manifest && manifest.engine_version === ALTIMATE_ENGINE_VERSION && pythonExists && extrasMatch) return
162172

163173
const startTime = Date.now()
164174

@@ -189,7 +199,10 @@ async function ensureEngineImpl(): Promise<void> {
189199
const pythonPath = enginePythonPath()
190200
Log.Default.info("installing altimate-engine", { version: ALTIMATE_ENGINE_VERSION })
191201
try {
192-
execFileSync(uv, ["pip", "install", "--python", pythonPath, `altimate-engine[warehouses]==${ALTIMATE_ENGINE_VERSION}`], { stdio: "pipe" })
202+
const spec = ENGINE_INSTALL_SPEC
203+
? `altimate-engine[${ENGINE_INSTALL_SPEC}]==${ALTIMATE_ENGINE_VERSION}`
204+
: `altimate-engine==${ALTIMATE_ENGINE_VERSION}`
205+
execFileSync(uv, ["pip", "install", "--python", pythonPath, spec], { stdio: "pipe" })
193206
} catch (e: any) {
194207
Telemetry.track({
195208
type: "engine_error",
@@ -212,6 +225,7 @@ async function ensureEngineImpl(): Promise<void> {
212225
uv_version: uvVersion,
213226
cli_version: typeof OPENCODE_VERSION === "string" ? OPENCODE_VERSION : "local",
214227
installed_at: new Date().toISOString(),
228+
extras: ENGINE_INSTALL_SPEC,
215229
})
216230

217231
Telemetry.track({

0 commit comments

Comments
 (0)