diff --git a/src/lib/versionManager/binaryManager.ts b/src/lib/versionManager/binaryManager.ts index 2cf06b582f..26f9e05bdb 100644 --- a/src/lib/versionManager/binaryManager.ts +++ b/src/lib/versionManager/binaryManager.ts @@ -81,8 +81,21 @@ export function buildExtractZipCommand( return { command: "unzip", args: ["-o", archivePath, "-d", destDir] }; } -async function extractZip(archivePath: string, destDir: string): Promise { - const { command, args } = buildExtractZipCommand(os.platform(), archivePath, destDir); +/** + * #10244/#10293: `platform` MUST be an explicit parameter threaded down from the + * caller's single runtime detection (see `installVersion`/`downloadRelease`), not + * an independent `os.platform()` read inside this function. Multiple, independently + * evaluated `os.platform()` call sites scattered across the module are each an + * opportunity for a bundler to constant-fold that particular occurrence away — a + * single detected value threaded as data through the call chain has no per-call-site + * literal for the bundler to fold. + */ +async function extractZip( + archivePath: string, + destDir: string, + platform: NodeJS.Platform +): Promise { + const { command, args } = buildExtractZipCommand(platform, archivePath, destDir); await execFileAsync(command, args); } @@ -110,12 +123,16 @@ function findBinaryInDir(dir: string): string | null { export async function downloadRelease( version: string, targetDir: string, - signal?: AbortSignal + signal?: AbortSignal, + // Optional pre-detected target: lets a top-level orchestrator (installVersion) + // read the runtime platform/arch exactly once and pass the value down instead of + // this function independently re-reading os.platform()/os.arch() (#10244/#10293). + target?: { platform: Platform; arch: Arch } ): Promise { const release = await getReleaseByVersion(version); if (!release) throw new Error(`Version ${version} not found`); - const { platform, arch } = getTargetPlatform(); + const { platform, arch } = target || getTargetPlatform(); const ext = platform === "windows" ? ".zip" : ".tar.gz"; const assetName = `CLIProxyAPI_${release.version}_${platform}_${arch}${ext}`; const asset = release.assets.find((a) => a.name === assetName); @@ -140,7 +157,10 @@ export async function downloadRelease( } if (platform === "windows") { - await extractZip(archivePath, versionDir); + // Already inside the `platform === "windows"` branch of the single value + // detected above (or threaded in via `target`) — pass the corresponding + // NodeJS.Platform literal directly rather than calling os.platform() again. + await extractZip(archivePath, versionDir, "win32"); } else { await extractTarGz(archivePath, versionDir); } @@ -159,13 +179,18 @@ export async function installVersion(version: string, dataDir?: string): Promise const binDir = path.join(dir, "bin"); await fs.mkdir(binDir, { recursive: true }); - const binary = await downloadRelease(version, binDir); + // Single runtime detection for this whole orchestration: read once here and + // thread the value into downloadRelease() and the symlink/copy decision below, + // instead of each step re-reading os.platform()/os.arch() independently + // (#10244/#10293 — redundant reads are each an independent build-folding risk). + const target = getTargetPlatform(); + const binary = await downloadRelease(version, binDir, undefined, target); const symlinkPath = path.join(binDir, "cliproxyapi"); try { await fs.unlink(symlinkPath); } catch {} - if (os.platform() === "win32") { + if (target.platform === "windows") { await fs.copyFile(binary, symlinkPath); } else { await fs.symlink(binary, symlinkPath); @@ -219,7 +244,11 @@ export async function rollbackVersion(dataDir?: string): Promise try { await fs.unlink(symlinkPath); } catch {} - if (os.platform() === "win32") { + // Single runtime detection for this orchestration, via the module's one + // canonical read point (getTargetPlatform -> detectPlatform -> os.platform()), + // rather than a separate ad hoc os.platform() call (#10244/#10293). + const { platform } = getTargetPlatform(); + if (platform === "windows") { await fs.copyFile(oldBinary, symlinkPath); } else { await fs.symlink(oldBinary, symlinkPath); diff --git a/tests/unit/binaryManager.test.ts b/tests/unit/binaryManager.test.ts index 1aa58738c6..72911712b2 100644 --- a/tests/unit/binaryManager.test.ts +++ b/tests/unit/binaryManager.test.ts @@ -26,6 +26,7 @@ describe("binaryManager", () => { mod = await import("../../src/lib/versionManager/binaryManager.ts"); assert.ok(mod.getAssetName); assert.ok(mod.getTargetPlatform); + assert.ok(mod.downloadRelease); assert.ok(mod.installVersion); assert.ok(mod.getCurrentBinaryPath); assert.ok(mod.getInstalledVersions); @@ -227,6 +228,77 @@ describe("binaryManager", () => { }); }); + describe("downloadRelease platform parameter threading (#10244/#10293)", () => { + it("uses an explicitly-passed Windows target without reading os.platform() at all", async () => { + // Closing-fix regression guard: unlike the os.platform()/os.arch() mock-based + // tests above (which prove the single top-level detection reaches the right + // place, but would still pass even if extractZip re-read os.platform() itself + // since the mock is global), this test proves the actual PARAMETER THREADING: + // downloadRelease() is called with an explicit `target` and os.platform()/ + // os.arch() are NOT mocked at all — the real test host is Linux/darwin/etc. + // If downloadRelease or extractZip ever regressed to independently re-reading + // os.platform() instead of using the threaded `platform` value, this would + // resolve to the host's real (non-Windows) platform, `unzip` would run against + // a fake zip body, and the test would fail. + const binDir = path.join(tmpDir, "bin-param-thread"); + const extractedDir = path.join(binDir, "cliproxyapi-1.0.0"); + const fakePowerShellDir = path.join(tmpDir, "fake-powershell-param-thread"); + const commandLog = path.join(tmpDir, "powershell-command-param-thread.txt"); + const originalPath = process.env.PATH; + const originalFetch = globalThis.fetch; + + fs.mkdirSync(fakePowerShellDir, { recursive: true }); + fs.writeFileSync( + path.join(fakePowerShellDir, "powershell"), + "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"$OMNI_TEST_COMMAND_LOG_PT\"\n" + + "mkdir -p \"$OMNI_TEST_EXTRACT_DIR_PT\"\nprintf 'installed-binary' > \"$OMNI_TEST_EXTRACT_DIR_PT/cli-proxy-api\"\n" + ); + fs.chmodSync(path.join(fakePowerShellDir, "powershell"), 0o755); + process.env.PATH = `${fakePowerShellDir}:${originalPath || ""}`; + process.env.OMNI_TEST_COMMAND_LOG_PT = commandLog; + process.env.OMNI_TEST_EXTRACT_DIR_PT = extractedDir; + + globalThis.fetch = async (input: string | URL | Request) => { + const url = String(input); + if (url.includes("/releases/tags/")) { + return new Response( + JSON.stringify({ + tag_name: "v1.0.0", + published_at: "2026-01-01T00:00:00Z", + assets: [ + { + name: "CLIProxyAPI_1.0.0_windows_amd64.zip", + browser_download_url: "https://example.test/cliproxy.zip", + size: 3, + }, + ], + }), + { status: 200, headers: { "content-type": "application/json" } } + ); + } + if (url.endsWith("checksums.txt")) return new Response("", { status: 404 }); + return new Response("zip", { status: 200 }); + }; + + try { + const binary = await mod.downloadRelease("1.0.0", binDir, undefined, { + platform: "windows", + arch: "amd64", + }); + assert.equal(fs.readFileSync(binary, "utf8"), "installed-binary"); + + const command = fs.readFileSync(commandLog, "utf8"); + assert.match(command, /Expand-Archive -LiteralPath/); + assert.doesNotMatch(command, /unzip/); + } finally { + globalThis.fetch = originalFetch; + process.env.PATH = originalPath; + delete process.env.OMNI_TEST_COMMAND_LOG_PT; + delete process.env.OMNI_TEST_EXTRACT_DIR_PT; + } + }); + }); + describe("removeVersion", () => { it("should remove version directory", async () => { const binDir = path.join(tmpDir, "bin");