fix(cliproxy): thread runtime platform as a parameter instead of re-reading os.platform()

extractZip(), installVersion(), and rollbackVersion() each independently called
os.platform() inline in their own module scope even after #10244 switched the
detection helpers to os.platform()/os.arch(). Each independent call site is its
own opportunity for a bundler to constant-fold that particular occurrence away.

Detect the runtime platform once per orchestrating call (installVersion,
downloadRelease, rollbackVersion) and thread the already-detected value down as
an explicit parameter into extractZip and the symlink/copy decisions, instead of
re-reading the global in every helper.
This commit is contained in:
adevwithpurpose
2026-08-17 22:40:54 -03:00
parent 060a906dee
commit ca4ba987b5
2 changed files with 109 additions and 8 deletions

View File

@@ -81,8 +81,21 @@ export function buildExtractZipCommand(
return { command: "unzip", args: ["-o", archivePath, "-d", destDir] };
}
async function extractZip(archivePath: string, destDir: string): Promise<void> {
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<void> {
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<string> {
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<string | null>
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);

View File

@@ -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");