diff --git a/packages/cli/src/cli/bin.ts b/packages/cli/src/cli/bin.ts index 01bd971..6b292a8 100755 --- a/packages/cli/src/cli/bin.ts +++ b/packages/cli/src/cli/bin.ts @@ -1,4 +1,5 @@ #!/usr/bin/env -S npx tsx +import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { AiderAdapter } from "../core/aider-adapter.server.js"; @@ -82,6 +83,22 @@ function printJson(value: unknown): void { console.log(JSON.stringify(value, null, 2)); } +/** + * Runs `fn` with a fresh, uniquely-named scratch directory, cleaned up afterward either way. Used for + * downloads that get replaced/installed into their real destination right after — a fixed, predictable + * filename directly under the shared OS temp dir would let another local user pre-place a symlink there + * and have the download silently overwrite whatever it points at (no O_EXCL); `mkdtemp`'s random suffix + * means there's nothing for an attacker to predict and pre-create in advance. + */ +async function withScratchDir(fn: (dir: string) => Promise): Promise { + const dir = await mkdtemp(join(tmpdir(), "sessionforge-")); + try { + return await fn(dir); + } finally { + await rm(dir, { recursive: true, force: true }); + } +} + async function cmdDiscover(): Promise { const store = new SessionStore(); try { @@ -279,27 +296,29 @@ async function cmdWirePaseo(args: ParsedArgs): Promise { return; } - const archivePath = join(tmpdir(), PLUGIN_ARCHIVE_NAME); - console.log(`Downloading the v${version} Paseo plugin release asset...`); - await downloadPluginArchive(version, archivePath); + await withScratchDir(async (scratchDir) => { + const archivePath = join(scratchDir, PLUGIN_ARCHIVE_NAME); + console.log(`Downloading the v${version} Paseo plugin release asset...`); + await downloadPluginArchive(version, archivePath); - const installDir = pluginInstallDir(); - console.log(`Extracting to ${installDir}...`); - await extractPluginArchive(archivePath, installDir); + const installDir = pluginInstallDir(); + console.log(`Extracting to ${installDir}...`); + await extractPluginArchive(archivePath, installDir); - const id = flagString(args.flags, "id") ?? DEFAULT_PLUGIN_ID; - console.log("Installing via `paseo plugin install`..."); - const result = await installPluginDirectory(installDir, id); + const id = flagString(args.flags, "id") ?? DEFAULT_PLUGIN_ID; + console.log("Installing via `paseo plugin install`..."); + const result = await installPluginDirectory(installDir, id); - if (result.status !== "running") { - console.error(`\nPlugin installed but is not running (status: ${result.status}).`); - if (result.error) console.error(result.error); - console.error(`Check \`paseo plugin logs ${id}\` for details.`); - process.exitCode = 1; - return; - } + if (result.status !== "running") { + console.error(`\nPlugin installed but is not running (status: ${result.status}).`); + if (result.error) console.error(result.error); + console.error(`Check \`paseo plugin logs ${id}\` for details.`); + process.exitCode = 1; + return; + } - console.log(`\nSessionForge is wired into Paseo (plugin id: ${id}, status: running).`); + console.log(`\nSessionForge is wired into Paseo (plugin id: ${id}, status: running).`); + }); } async function cmdPaseoStatus(): Promise { @@ -360,12 +379,14 @@ async function cmdUpdate(): Promise { } const isWindows = process.platform === "win32"; - const newBinaryPath = join(tmpdir(), `sessionforge-update-${latest}${isWindows ? ".exe" : ""}`); - console.log(`Downloading v${latest}...`); - await downloadCliBinary(latest, newBinaryPath); - - console.log("Installing..."); - await selfReplaceBinary(newBinaryPath, process.execPath); + await withScratchDir(async (scratchDir) => { + const newBinaryPath = join(scratchDir, `sessionforge-update-${latest}${isWindows ? ".exe" : ""}`); + console.log(`Downloading v${latest}...`); + await downloadCliBinary(latest, newBinaryPath); + + console.log("Installing..."); + await selfReplaceBinary(newBinaryPath, process.execPath); + }); console.log(`Updated to v${latest}. Run \`sessionforge wire-paseo\` too if you use the Paseo plugin, to keep it in sync.`); } diff --git a/packages/cli/src/core/download.server.test.ts b/packages/cli/src/core/download.server.test.ts new file mode 100644 index 0000000..ef55fa2 --- /dev/null +++ b/packages/cli/src/core/download.server.test.ts @@ -0,0 +1,70 @@ +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { downloadFile } from "./download.server.js"; + +describe("downloadFile", () => { + let root: string; + + beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), "sessionforge-download-")); + }); + + afterEach(async () => { + await rm(root, { recursive: true, force: true }); + vi.unstubAllGlobals(); + }); + + it("writes the response body to the destination path", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => new Response("hello world", { status: 200 })), + ); + + const destPath = join(root, "file.bin"); + await downloadFile("https://example.com/file.bin", destPath); + + expect(await readFile(destPath, "utf8")).toBe("hello world"); + }); + + it("throws with the status when the request itself fails", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => new Response(null, { status: 404, statusText: "Not Found" })), + ); + + await expect(downloadFile("https://example.com/file.bin", join(root, "file.bin"))).rejects.toThrow(/404/); + }); + + it("succeeds silently when the server sends no Content-Length to check against", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => new Response("no length header on this one", { status: 200 })), + ); + + await expect(downloadFile("https://example.com/file.bin", join(root, "file.bin"))).resolves.toBeUndefined(); + }); + + it("throws when fewer bytes arrive than the server's own Content-Length promised — a truncated transfer", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => new Response("short", { status: 200, headers: { "content-length": "9999" } })), + ); + + await expect(downloadFile("https://example.com/file.bin", join(root, "file.bin"))).rejects.toThrow(/incomplete/i); + }); + + it("succeeds when the transferred bytes match a real Content-Length", async () => { + const body = "exact content"; + vi.stubGlobal( + "fetch", + vi.fn(async () => new Response(body, { status: 200, headers: { "content-length": String(Buffer.byteLength(body)) } })), + ); + + const destPath = join(root, "file.bin"); + await downloadFile("https://example.com/file.bin", destPath); + + expect(await readFile(destPath, "utf8")).toBe(body); + }); +}); diff --git a/packages/cli/src/core/download.server.ts b/packages/cli/src/core/download.server.ts new file mode 100644 index 0000000..9b41df4 --- /dev/null +++ b/packages/cli/src/core/download.server.ts @@ -0,0 +1,33 @@ +import { createWriteStream } from "node:fs"; +import { pipeline } from "node:stream/promises"; +import { Readable, Transform } from "node:stream"; + +/** + * Fetches a URL and streams it to disk, verifying the transferred byte count against the response's own + * Content-Length (when the server sends one) before treating the write as successful — a connection reset + * or truncated proxy response can otherwise leave a corrupt, incomplete file on disk that still looks like + * a normal successful download to a caller that only checked `response.ok`. Shared by + * `paseo-wire.server.ts` (plugin archive) and `update.server.ts` (CLI binary) since both hand their result + * straight to something high-stakes: `paseo plugin install` or replacing the running executable. + */ +export async function downloadFile(url: string, destPath: string): Promise { + const response = await fetch(url); + if (!response.ok || !response.body) { + throw new Error(`Download failed (${response.status} ${response.statusText}): ${url}`); + } + + const expectedLength = response.headers.get("content-length"); + let receivedBytes = 0; + const countBytes = new Transform({ + transform(chunk: Buffer, _encoding, callback) { + receivedBytes += chunk.length; + callback(null, chunk); + }, + }); + + await pipeline(Readable.fromWeb(response.body as import("node:stream/web").ReadableStream), countBytes, createWriteStream(destPath)); + + if (expectedLength !== null && receivedBytes !== Number(expectedLength)) { + throw new Error(`Download incomplete: expected ${expectedLength} bytes, got ${receivedBytes} (${url})`); + } +} diff --git a/packages/cli/src/core/paseo-wire.server.test.ts b/packages/cli/src/core/paseo-wire.server.test.ts index 29b2388..d532f44 100644 --- a/packages/cli/src/core/paseo-wire.server.test.ts +++ b/packages/cli/src/core/paseo-wire.server.test.ts @@ -1,4 +1,5 @@ -import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { existsSync, writeFileSync } from "node:fs"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -94,7 +95,6 @@ describe("paseo-wire", () => { await downloadPluginArchive("1.2.3", destPath); expect(fetchMock).toHaveBeenCalledOnce(); - const { readFile } = await import("node:fs/promises"); expect(await readFile(destPath, "utf8")).toBe("fake tarball contents"); }); @@ -109,7 +109,7 @@ describe("paseo-wire", () => { }); describe("extractPluginArchive", () => { - it("wipes the destination and shells out to tar", async () => { + it("extracts into a staging directory alongside the destination, not straight into it", async () => { const destDir = join(root, "dest"); let sawTarArgs: string[] = []; execFileHandler = (cmd, args, callback) => { @@ -120,7 +120,37 @@ describe("paseo-wire", () => { await extractPluginArchive(join(root, "archive.tar.gz"), destDir); - expect(sawTarArgs).toEqual(["-xzf", join(root, "archive.tar.gz"), "-C", destDir]); + expect(sawTarArgs).toEqual(["-xzf", join(root, "archive.tar.gz"), "-C", `${destDir}.staging`]); + }); + + it("leaves a previous working install untouched when tar fails, instead of wiping it first", async () => { + const destDir = join(root, "dest"); + await mkdir(destDir, { recursive: true }); + await writeFile(join(destDir, "still-here.txt"), "previous working install"); + + execFileHandler = (_cmd, _args, callback) => callback(new Error("tar: unexpected end of file")); + + await expect(extractPluginArchive(join(root, "archive.tar.gz"), destDir)).rejects.toThrow(); + + expect(await readFile(join(destDir, "still-here.txt"), "utf8")).toBe("previous working install"); + }); + + it("replaces the destination with the newly-extracted content once tar actually succeeds", async () => { + const destDir = join(root, "dest"); + await mkdir(destDir, { recursive: true }); + await writeFile(join(destDir, "old.txt"), "old content"); + + execFileHandler = (_cmd, args, callback) => { + // Simulate tar really writing into the staging directory it was told to extract into. + const stagingDir = args[args.indexOf("-C") + 1]; + writeFileSync(join(stagingDir, "new.txt"), "new content"); + callback(null, { stdout: "", stderr: "" }); + }; + + await extractPluginArchive(join(root, "archive.tar.gz"), destDir); + + expect(existsSync(join(destDir, "old.txt"))).toBe(false); + expect(await readFile(join(destDir, "new.txt"), "utf8")).toBe("new content"); }); }); @@ -168,7 +198,6 @@ describe("paseo-wire", () => { describe("getInstalledPluginVersion", () => { it("reads the version marker scripts/package-plugin.mjs bakes into the packaged bundle", async () => { - const { writeFile } = await import("node:fs/promises"); await writeFile(join(root, ".sessionforge-version"), "0.3.0\n"); expect(await getInstalledPluginVersion(root)).toBe("0.3.0"); diff --git a/packages/cli/src/core/paseo-wire.server.ts b/packages/cli/src/core/paseo-wire.server.ts index e802e25..94c7287 100644 --- a/packages/cli/src/core/paseo-wire.server.ts +++ b/packages/cli/src/core/paseo-wire.server.ts @@ -1,11 +1,9 @@ import { execFile } from "node:child_process"; -import { createWriteStream, existsSync } from "node:fs"; -import { mkdir, readFile, rm } from "node:fs/promises"; +import { mkdir, readFile, rename, rm } from "node:fs/promises"; import { homedir } from "node:os"; import { join } from "node:path"; -import { pipeline } from "node:stream/promises"; -import { Readable } from "node:stream"; import { promisify } from "node:util"; +import { downloadFile } from "./download.server.js"; const execFileAsync = promisify(execFile); @@ -57,19 +55,26 @@ export async function arePluginsEnabled(): Promise { * always wires up the plugin build it actually shipped with, not a possibly-incompatible newer one. */ export async function downloadPluginArchive(version: string, destPath: string): Promise { const url = `https://github.com/4mGLn/sessionforge/releases/download/v${version}/${PLUGIN_ARCHIVE_NAME}`; - const response = await fetch(url); - if (!response.ok || !response.body) { - throw new Error(`Download failed (${response.status} ${response.statusText}): ${url}`); - } - await pipeline(Readable.fromWeb(response.body as import("node:stream/web").ReadableStream), createWriteStream(destPath)); + await downloadFile(url, destPath); } -/** tar ships on Linux/macOS by default and as bsdtar on Windows 10 1803+ / Windows 11 — the same - * assumption install.sh/install.ps1 and this project's other platform-support claims already make. */ +/** + * tar ships on Linux/macOS by default and as bsdtar on Windows 10 1803+ / Windows 11 — the same assumption + * install.sh/install.ps1 and this project's other platform-support claims already make. + * + * Extracts to a staging directory first and only replaces `destDir` after `tar` has actually succeeded — + * a corrupt or truncated archive (bad download, disk full mid-extract) then just fails cleanly, instead of + * first wiping out a previously-working plugin install and leaving Paseo pointed at a broken directory. + */ export async function extractPluginArchive(archivePath: string, destDir: string): Promise { + const stagingDir = `${destDir}.staging`; + await rm(stagingDir, { recursive: true, force: true }); + await mkdir(stagingDir, { recursive: true }); + await execFileAsync("tar", ["-xzf", archivePath, "-C", stagingDir]); + + // Only reached once tar has proven the archive is valid. await rm(destDir, { recursive: true, force: true }); - await mkdir(destDir, { recursive: true }); - await execFileAsync("tar", ["-xzf", archivePath, "-C", destDir]); + await rename(stagingDir, destDir); } interface PluginInstallResult { @@ -91,10 +96,6 @@ export async function getPluginStatus(id: string = DEFAULT_PLUGIN_ID): Promise

plugin.id === id) ?? null; } -export function pluginArchiveExists(path: string): boolean { - return existsSync(path); -} - const PLUGIN_VERSION_FILE = ".sessionforge-version"; /** diff --git a/packages/cli/src/core/update.server.test.ts b/packages/cli/src/core/update.server.test.ts index ffb37cb..5b7b90b 100644 --- a/packages/cli/src/core/update.server.test.ts +++ b/packages/cli/src/core/update.server.test.ts @@ -1,17 +1,36 @@ -import { statSync, writeFileSync } from "node:fs"; -import { mkdtemp as mkdtempAsync, readFile, rm } from "node:fs/promises"; +import { existsSync, statSync, writeFileSync } from "node:fs"; +import { mkdtemp as mkdtempAsync, readFile, rename as realRename, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { + +// Lets specific tests simulate a cross-filesystem rename (EXDEV) without needing two real filesystems — +// falls through to the real implementation for every test that doesn't set an override. +let renameOverride: ((source: string, dest: string) => Promise) | null = null; +vi.mock("node:fs/promises", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + rename: (source: string, dest: string) => (renameOverride ? renameOverride(source, dest) : actual.rename(source, dest)), + }; +}); + +const { checkForUpdateCached, compareVersions, downloadCliBinary, getLatestReleaseVersion, + InvalidVersionError, isUpdateAvailable, selfReplaceBinary, targetTriple, -} from "./update.server.js"; +} = await import("./update.server.js"); + +function exdevError(): NodeJS.ErrnoException { + const error = new Error("EXDEV: cross-device link not permitted") as NodeJS.ErrnoException; + error.code = "EXDEV"; + return error; +} function setPlatform(platform: NodeJS.Platform): void { Object.defineProperty(process, "platform", { value: platform, configurable: true }); @@ -28,6 +47,12 @@ describe("compareVersions", () => { expect(compareVersions("1.2", "1.2.0")).toBe(0); expect(compareVersions("1.3", "1.2.9")).toBeGreaterThan(0); }); + + it("throws InvalidVersionError instead of silently comparing as NaN for a non-numeric component", () => { + // A plain numeric split+compare would make "0.3.0-rc" parse to [0, 3, NaN] — NaN is never > 0, so a + // real update would go permanently invisible to isUpdateAvailable without this ever raising an error. + expect(() => compareVersions("0.3.0-rc", "0.2.0")).toThrow(InvalidVersionError); + }); }); describe("isUpdateAvailable", () => { @@ -46,6 +71,10 @@ describe("isUpdateAvailable", () => { it("is always true for a dev-main build — any real release counts as newer", () => { expect(isUpdateAvailable("dev-main", "0.0.1")).toBe(true); }); + + it("fails toward 'yes, update available' rather than silently hiding an unparseable version", () => { + expect(isUpdateAvailable("0.2.0", "0.3.0-rc.1")).toBe(true); + }); }); describe("targetTriple", () => { @@ -195,21 +224,69 @@ describe("selfReplaceBinary", () => { expect(await readFile(current, "utf8")).toBe("new"); }); + + afterEach(() => { + renameOverride = null; + }); + + it("falls back to copy+unlink on EXDEV — e.g. the download and install location are on different filesystems", async () => { + setPlatform("linux"); + const current = join(root, "sessionforge"); + const incoming = join(root, "sessionforge-new"); + writeFileSync(current, "old"); + writeFileSync(incoming, "new"); + + renameOverride = async () => { + throw exdevError(); + }; + + await selfReplaceBinary(incoming, current); + + expect(await readFile(current, "utf8")).toBe("new"); + expect(existsSync(incoming)).toBe(false); // source removed after the copy, same as a real rename would leave it + }); + + it("rolls the original binary back into place on windows if putting the new one there fails, instead of leaving nothing runnable", async () => { + setPlatform("win32"); + const current = join(root, "sessionforge.exe"); + const incoming = join(root, "sessionforge-new.exe"); + writeFileSync(current, "old"); + writeFileSync(incoming, "new"); + + let call = 0; + renameOverride = async (source, dest) => { + call += 1; + if (call === 2) throw new Error("simulated: antivirus lock"); + return realRename(source, dest); + }; + + await expect(selfReplaceBinary(incoming, current)).rejects.toThrow(/antivirus lock/); + + // A failed update must be a no-op, never a bricked install — the original binary has to still be + // runnable at its original path. + expect(await readFile(current, "utf8")).toBe("old"); + }); }); describe("checkForUpdateCached", () => { let root: string; let previousHome: string | undefined; let previousUserProfile: string | undefined; + let previousCi: string | undefined; beforeEach(async () => { root = await mkdtempAsync(join(tmpdir(), "sessionforge-update-cache-")); previousHome = process.env.HOME; previousUserProfile = process.env.USERPROFILE; + previousCi = process.env.CI; // node:os's homedir() reads USERPROFILE (not HOME) on Windows — setting both keeps the cache file // sandboxed to `root` regardless of which real OS runs this test. process.env.HOME = root; process.env.USERPROFILE = root; + // This suite itself normally runs inside GitHub Actions, which sets CI=true by default — unset it here + // so the non-CI-skip tests below actually exercise real behavior instead of hitting the new CI guard + // regardless of what they're testing. + delete process.env.CI; }); afterEach(async () => { @@ -217,6 +294,8 @@ describe("checkForUpdateCached", () => { else process.env.HOME = previousHome; if (previousUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = previousUserProfile; + if (previousCi === undefined) delete process.env.CI; + else process.env.CI = previousCi; await rm(root, { recursive: true, force: true }); vi.unstubAllGlobals(); }); @@ -229,6 +308,15 @@ describe("checkForUpdateCached", () => { expect(fetchMock).not.toHaveBeenCalled(); }); + it("returns null immediately in CI, without any network call — no one reads the notice in a pipeline", async () => { + process.env.CI = "true"; + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + + expect(await checkForUpdateCached("0.3.0")).toBeNull(); + expect(fetchMock).not.toHaveBeenCalled(); + }); + it("performs a real check and reports an available update", async () => { vi.stubGlobal( "fetch", diff --git a/packages/cli/src/core/update.server.ts b/packages/cli/src/core/update.server.ts index 83dd17b..ee6ef40 100644 --- a/packages/cli/src/core/update.server.ts +++ b/packages/cli/src/core/update.server.ts @@ -1,9 +1,8 @@ -import { chmodSync, createWriteStream } from "node:fs"; -import { mkdir, readFile, rename, rm, writeFile } from "node:fs/promises"; +import { chmodSync } from "node:fs"; +import { copyFile, mkdir, readFile, rename, rm, unlink, writeFile } from "node:fs/promises"; import { arch, homedir, platform } from "node:os"; import { join } from "node:path"; -import { pipeline } from "node:stream/promises"; -import { Readable } from "node:stream"; +import { downloadFile } from "./download.server.js"; const REPO = "4mGLn/sessionforge"; const UPDATE_CHECK_CACHE_TTL_MS = 24 * 60 * 60 * 1000; // 24 hours — one real network check per day at most @@ -27,11 +26,24 @@ export function targetTriple(): string { throw new Error(`Unsupported platform/arch combination for self-update: ${p}/${a}`); } +/** Thrown by compareVersions for anything that isn't a plain dotted-numeric version (e.g. a pre-release + * tag like "0.3.0-rc.1") — better than silently comparing as NaN, which is neither > 0 nor < 0 nor === 0 + * and would make a real update invisible to isUpdateAvailable without ever raising an error anywhere. */ +export class InvalidVersionError extends Error {} + +function parseVersionParts(version: string): number[] { + const parts = version.split(".").map(Number); + if (parts.some((part) => Number.isNaN(part))) { + throw new InvalidVersionError(`Not a plain dotted-numeric version: "${version}"`); + } + return parts; +} + /** Compares two dotted-numeric version strings (e.g. "0.10.2" vs "0.2.9") field by field, not * lexicographically — a plain string compare would wrongly rank "0.10.0" below "0.2.0". */ export function compareVersions(a: string, b: string): number { - const partsA = a.split(".").map(Number); - const partsB = b.split(".").map(Number); + const partsA = parseVersionParts(a); + const partsB = parseVersionParts(b); const length = Math.max(partsA.length, partsB.length); for (let i = 0; i < length; i += 1) { const diff = (partsA[i] ?? 0) - (partsB[i] ?? 0); @@ -40,10 +52,19 @@ export function compareVersions(a: string, b: string): number { return 0; } -/** "dev-main" (a local/non-release build) never numerically compares — any real release counts as newer. */ +/** + * "dev-main" (a local/non-release build) never numerically compares — any real release counts as newer. + * A version that fails to parse (see InvalidVersionError) fails toward "yes, an update might be available" + * rather than silently claiming everything's fine when the comparison itself couldn't actually be done. + */ export function isUpdateAvailable(current: string, latest: string): boolean { if (current === "dev-main") return true; - return compareVersions(latest, current) > 0; + try { + return compareVersions(latest, current) > 0; + } catch (error) { + if (error instanceof InvalidVersionError) return true; + throw error; + } } interface GitHubRelease { @@ -73,13 +94,16 @@ interface UpdateCheckCache { } /** - * Rate-limited (24h) background-style check — never throws, so it's safe to call unconditionally at the - * end of any command without risking that command's own output/exit code. Returns null on any failure - * (network down, GitHub unreachable, cache unreadable) or when running a dev-main build, so callers can - * just skip printing anything rather than needing their own error handling. + * Rate-limited (24h) update check appended to the end of most commands. Not a detached background task — + * it genuinely delays process exit by up to BACKGROUND_CHECK_TIMEOUT_MS on a cache-miss day, since the + * whole point is printing its notice before the command's own output is done. Never throws, so it's safe + * to call unconditionally without risking the calling command's own output/exit code. Returns null on any + * failure (network down, GitHub unreachable, cache unreadable), when running a dev-main build, or in CI + * (`CI` env var — a stalled network check has no one to read its notice and just wastes time in a + * pipeline), so callers can just skip printing anything rather than needing their own error handling. */ export async function checkForUpdateCached(currentVersion: string): Promise<{ latestVersion: string; updateAvailable: boolean } | null> { - if (currentVersion === "dev-main") return null; + if (currentVersion === "dev-main" || process.env.CI) return null; try { const cachePath = updateCheckCachePath(); @@ -111,36 +135,52 @@ export async function downloadCliBinary(version: string, destPath: string): Prom const isWindows = platform() === "win32"; const assetName = `sessionforge-${targetTriple()}${isWindows ? ".exe" : ""}`; const url = `https://github.com/${REPO}/releases/download/v${version}/${assetName}`; - const response = await fetch(url); - if (!response.ok || !response.body) { - throw new Error(`Download failed (${response.status} ${response.statusText}): ${url}`); - } - await pipeline(Readable.fromWeb(response.body as import("node:stream/web").ReadableStream), createWriteStream(destPath)); + await downloadFile(url, destPath); if (!isWindows) chmodSync(destPath, 0o755); } +/** rename() across filesystems throws EXDEV — falls back to copy+unlink, since the downloaded file + * (usually under the OS temp dir) and the install location aren't guaranteed to be on the same filesystem + * (e.g. tmpfs /tmp vs a separately-mounted /usr/local/bin). Same pattern trash.server.ts's own + * renameOrCopy already uses for the identical reason. */ +async function renameOrCopy(sourcePath: string, destPath: string): Promise { + try { + await rename(sourcePath, destPath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EXDEV") throw error; + await copyFile(sourcePath, destPath); + await unlink(sourcePath); + } +} + /** * Replaces the currently-running binary in place with a freshly-downloaded one — the standard * self-updating-CLI pattern (same one rustup/many Go tools use), because a running executable can't just * be overwritten directly on every OS: - * - Linux/macOS: renaming a file that's currently executing is fine — the OS keeps the old inode alive for - * the still-running process, and the path just starts pointing at the new file. A same-directory rename - * (not a cross-filesystem copy) keeps this atomic. + * - Linux/macOS: renaming (or, cross-filesystem, copying) over a file that's currently executing is fine — + * the OS keeps the old inode alive for the still-running process, and the path just starts pointing at + * the new file. * - Windows: a running .exe can't be deleted or overwritten, but it CAN be renamed. So the running binary - * is renamed to a `.old.exe` sibling first, then the downloaded file takes its original name. The - * `.old.exe` is best-effort deleted immediately after (already-renamed-away, so this rarely fails, but a - * file still flushing to disk on a slow machine could keep it locked a moment longer) — a leftover - * `.old.exe` is harmless clutter, not a correctness problem, so a failed cleanup isn't treated as fatal. + * is renamed to a `.old.exe` sibling first, then the downloaded file takes its original name. If that + * second step fails (cross-filesystem copy error, antivirus lock, disk full) the `.old.exe` is renamed + * straight back to the original name so the user isn't left with no runnable binary at all — a failed + * update should be a no-op, never a bricked install. The `.old.exe` is best-effort deleted once the swap + * actually succeeds; a leftover on a rare cleanup failure is harmless clutter, not a correctness problem. */ export async function selfReplaceBinary(newBinaryPath: string, currentExecPath: string): Promise { if (platform() === "win32") { const oldPath = `${currentExecPath}.old.exe`; await rm(oldPath, { force: true }); - await rename(currentExecPath, oldPath); - await rename(newBinaryPath, currentExecPath); + await renameOrCopy(currentExecPath, oldPath); + try { + await renameOrCopy(newBinaryPath, currentExecPath); + } catch (error) { + await renameOrCopy(oldPath, currentExecPath).catch(() => {}); + throw error; + } await rm(oldPath, { force: true }).catch(() => {}); return; } - await rename(newBinaryPath, currentExecPath); + await renameOrCopy(newBinaryPath, currentExecPath); }