fix(bifrost): send v-prefixed transport version, not bare semver (#8194)

* fix(bifrost): send v-prefixed transport version, not bare semver

bifrost.ts resolveSpawnArgs() set BIFROST_TRANSPORT_VERSION straight from
getInstalledVersionSync(), which reads the raw "version" field out of
node_modules/@maximhq/bifrost/package.json - always bare semver per npm
convention (e.g. "1.6.3").

@maximhq/bifrost's own bin.js validates that env var against
/^v\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?$/ or the literal "latest" and rejects
anything else with "Invalid transport version format", exiting immediately.
Every embedded Bifrost instance failed to start as a result.

Adds formatTransportVersion() to normalize at the call site that owns the
env var, so bifrost.ts and the upstream @maximhq/bifrost package both stay
exactly as designed - no changes needed to bifrost itself.

Strengthens the existing resolveSpawnArgs test to assert the actual v-prefix
format (it previously only checked for a non-empty string, the same gap
#6877 called out for cliproxy's pre-existing test), and adds a dedicated
regression test file covering the pure formatTransportVersion() helper plus
a real-filesystem resolveSpawnArgs() integration check.

Found and fixed while self-hosting OmniRoute and diagnosing why bifrost
kept crash-looping on startup.

* fix(bifrost): resolve install dir lazily so version read honors runtime DATA_DIR

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Co-authored-by: seanford <seanford@users.noreply.github.com>

src/lib/services/installers/bifrost.ts

@@ -25,12 +25,21 @@ export interface SpawnArgs {
 let latestVersionCache: { value: string; expiresAt: number } | null = null;
 const VERSION_CACHE_TTL_MS = 3_600_000;
 
+// Resolve the install dir lazily from the *current* DATA_DIR so a runtime
+// DATA_DIR override (operator env change, or a test's tmp-dir isolation) is
+// honored — the module-level BIFROST_INSTALL_DIR const is frozen at import.
+function getBifrostInstallDir(): string {
+  return process.env.DATA_DIR
+    ? path.join(process.env.DATA_DIR, "services", "bifrost")
+    : BIFROST_INSTALL_DIR;
+}
+
 function getInstalledPkgPath(): string {
-  return path.join(BIFROST_INSTALL_DIR, "node_modules", "@maximhq", "bifrost", "package.json");
+  return path.join(getBifrostInstallDir(), "node_modules", "@maximhq", "bifrost", "package.json");
 }
 
 function getBinPath(): string {
-  return path.join(BIFROST_INSTALL_DIR, "node_modules", "@maximhq", "bifrost", "bin.js");
+  return path.join(getBifrostInstallDir(), "node_modules", "@maximhq", "bifrost", "bin.js");
 }
 
 function getInstalledVersionSync(): string | null {
@@ -118,10 +127,28 @@ export async function update(): Promise<InstallResult> {
   return install("latest");
 }
 
+/**
+ * Bifrost's own bin.js validates BIFROST_TRANSPORT_VERSION against
+ * /^v\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?$/ or the literal "latest" - it does
+ * NOT accept a bare semver string like "1.6.3". getInstalledVersionSync()
+ * reads the raw "version" field straight out of package.json, which is
+ * always bare semver (npm/package.json convention never includes the "v"
+ * prefix), so passing it through unmodified made every embedded Bifrost
+ * instance fail on startup with "Invalid transport version format".
+ *
+ * Normalize here, at the call site that owns the env var, so both
+ * bifrost.ts and the upstream @maximhq/bifrost package can stay exactly as
+ * they are otherwise designed to be used.
+ */
+export function formatTransportVersion(version: string | null): string {
+  if (!version || version === "latest") return version ?? "latest";
+  return version.startsWith("v") ? version : `v${version}`;
+}
+
 export function resolveSpawnArgs(port: number): SpawnArgs {
   const binPath = getBinPath();
   // Pin transport version to the installed npm version for reproducibility (spec §2b)
-  const transportVersion = getInstalledVersionSync() ?? "latest";
+  const transportVersion = formatTransportVersion(getInstalledVersionSync());
 
   return {
     command: process.execPath,

tests/unit/services/installers/bifrost-transport-version-format.test.ts

@@ -0,0 +1,112 @@
+/**
+ * Regression test - bifrost resolveSpawnArgs() must pass a v-prefixed (or
+ * "latest") BIFROST_TRANSPORT_VERSION, never the bare semver string that
+ * getInstalledVersion() reads out of node_modules/@maximhq/bifrost/package.json.
+ *
+ * Bifrost's own bin.js validates the env var against
+ * /^v\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?$/ or the literal "latest" and exits
+ * immediately with "Invalid transport version format" on anything else.
+ * package.json's "version" field is always bare semver per npm convention
+ * (e.g. "1.6.3"), so every embedded Bifrost instance failed to start until
+ * this fix - unlike cliproxy's #6877 regression, this was silent at the
+ * resolveSpawnArgs() call site itself; the failure only surfaced once the
+ * spawned process actually ran.
+ *
+ * Covers both the pure formatTransportVersion() helper directly, and the
+ * real resolveSpawnArgs() integration path against a filesystem-backed
+ * fake npm, matching the pattern used by
+ * tests/unit/services/installers/cliproxy-resolve-spawn-args-6877.test.ts.
+ */
+
+import { describe, it, before, after } from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+describe("formatTransportVersion (pure)", () => {
+  it("prepends v to bare semver", async () => {
+    const { formatTransportVersion } = await import(
+      "../../../../src/lib/services/installers/bifrost.ts"
+    );
+    assert.equal(formatTransportVersion("1.6.3"), "v1.6.3");
+    assert.equal(formatTransportVersion("2.0.0-beta.1"), "v2.0.0-beta.1");
+  });
+
+  it("leaves an already-v-prefixed version untouched", async () => {
+    const { formatTransportVersion } = await import(
+      "../../../../src/lib/services/installers/bifrost.ts"
+    );
+    assert.equal(formatTransportVersion("v1.6.3"), "v1.6.3");
+  });
+
+  it('passes through "latest" untouched', async () => {
+    const { formatTransportVersion } = await import(
+      "../../../../src/lib/services/installers/bifrost.ts"
+    );
+    assert.equal(formatTransportVersion("latest"), "latest");
+  });
+
+  it('defaults null to "latest"', async () => {
+    const { formatTransportVersion } = await import(
+      "../../../../src/lib/services/installers/bifrost.ts"
+    );
+    assert.equal(formatTransportVersion(null), "latest");
+  });
+});
+
+describe("resolveSpawnArgs BIFROST_TRANSPORT_VERSION (real filesystem)", () => {
+  const ORIGINAL_DATA_DIR = process.env.DATA_DIR;
+  const ORIGINAL_PATH = process.env.PATH ?? "";
+  let dataDir: string;
+  let fakeBinDir: string;
+
+  before(() => {
+    dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-bifrost-vfix-"));
+    fakeBinDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-bifrost-vfix-bin-"));
+    process.env.DATA_DIR = dataDir;
+    process.env.PATH = `${fakeBinDir}:${ORIGINAL_PATH}`;
+
+    // Same package.json shape a real `npm install @maximhq/bifrost@1.6.3` would
+    // leave behind - bare semver "version" field, no "v" prefix.
+    const pkgDir = path.join(dataDir, "services", "bifrost", "node_modules", "@maximhq", "bifrost");
+    fs.mkdirSync(pkgDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(pkgDir, "package.json"),
+      JSON.stringify({ name: "@maximhq/bifrost", version: "1.6.3" }),
+      "utf8"
+    );
+    fs.writeFileSync(path.join(pkgDir, "bin.js"), "", "utf8");
+  });
+
+  after(() => {
+    if (ORIGINAL_DATA_DIR === undefined) {
+      delete process.env.DATA_DIR;
+    } else {
+      process.env.DATA_DIR = ORIGINAL_DATA_DIR;
+    }
+    process.env.PATH = ORIGINAL_PATH;
+    fs.rmSync(dataDir, { recursive: true, force: true });
+    fs.rmSync(fakeBinDir, { recursive: true, force: true });
+  });
+
+  it("BIFROST_TRANSPORT_VERSION is v-prefixed, matching what bin.js requires", async () => {
+    const { resolveSpawnArgs } = await import(
+      "../../../../src/lib/services/installers/bifrost.ts"
+    );
+
+    const args = resolveSpawnArgs(8080);
+
+    assert.equal(
+      args.env.BIFROST_TRANSPORT_VERSION,
+      "v1.6.3",
+      "must be v-prefixed - a bare '1.6.3' is rejected by bifrost's bin.js with " +
+        "'Invalid transport version format'"
+    );
+    assert.match(
+      args.env.BIFROST_TRANSPORT_VERSION as string,
+      /^(latest|v\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?)$/,
+      "must satisfy bifrost bin.js's own validateTransportVersion() regex"
+    );
+  });
+});

tests/unit/services/installers/bifrost.test.ts

@@ -127,12 +127,20 @@ test("resolveSpawnArgs shape: command is node, bin.js path, Go single-dash flags
   assert.ok(logLevelIdx !== -1, "must have -log-level flag");
   assert.equal(args.args[logLevelIdx + 1], "warn");
 
-  // BIFROST_TRANSPORT_VERSION must be set in env
+  // BIFROST_TRANSPORT_VERSION must be set in env AND match the format
+  // bifrost's own bin.js requires: /^v\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?$/ or "latest".
+  // (regression check for the "Invalid transport version format" startup crash -
+  // the fake npm helper above reports version "1.6.3", bare semver with no "v")
   assert.ok(
     typeof args.env.BIFROST_TRANSPORT_VERSION === "string" &&
       args.env.BIFROST_TRANSPORT_VERSION.length > 0,
     "BIFROST_TRANSPORT_VERSION must be set in env"
   );
+  assert.match(
+    args.env.BIFROST_TRANSPORT_VERSION,
+    /^(latest|v\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?)$/,
+    `BIFROST_TRANSPORT_VERSION must be "latest" or v-prefixed semver, got: ${args.env.BIFROST_TRANSPORT_VERSION}`
+  );
 });
 
 test("resolveSpawnArgs with different port passes correct -port value", () => {