Fix update file write

Author: emilwidlund

.github/workflows/release.yml

@@ -9,7 +9,22 @@ permissions:
   contents: write
 
 jobs:
+  test:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: oven-sh/setup-bun@v2
+        with:
+          bun-version: latest
+
+      - run: bun install
+
+      - run: bun test
+
   build:
+    needs: test
     strategy:
       matrix:
         include:

package.json

@@ -1,13 +1,13 @@
 {
 	"name": "polar-cli",
-	"version": "1.3.1",
+	"version": "1.3.2",
 	"description": "",
 	"bin": "bin/cli.js",
 	"type": "module",
 	"scripts": {
 		"build": "tsup ./src/cli.ts --format esm --outDir bin",
 		"dev": "tsc --watch",
-		"test": "echo \"Error: no test specified\" && exit 1",
+		"test": "bun test",
 		"check": "biome check --write ./src",
 		"build:binary": "bun build ./src/cli.ts --compile --outfile polar",
 		"build:binary:darwin-arm64": "bun build ./src/cli.ts --compile --target=bun-darwin-arm64 --outfile polar",

src/commands/update.test.ts

@@ -0,0 +1,115 @@
+import { describe, test, expect, beforeEach, afterEach, spyOn } from "bun:test";
+import { mkdtemp, rm, writeFile, readFile, stat } from "fs/promises";
+import { tmpdir } from "os";
+import { join } from "path";
+import { Effect } from "effect";
+import { replaceBinary } from "./update";
+
+async function makeTemp() {
+  return mkdtemp(join(tmpdir(), "polar-test-"));
+}
+
+describe("replaceBinary", () => {
+  let dir: string;
+  let newBinaryPath: string;
+  let binaryPath: string;
+
+  beforeEach(async () => {
+    dir = await makeTemp();
+    newBinaryPath = join(dir, "polar-new");
+    binaryPath = join(dir, "polar");
+    await writeFile(newBinaryPath, "#!/bin/sh\necho new");
+    await writeFile(binaryPath, "#!/bin/sh\necho old");
+  });
+
+  afterEach(async () => {
+    await rm(dir, { recursive: true, force: true });
+  });
+
+  test("replaces target binary with new binary content", async () => {
+    await Effect.runPromise(replaceBinary(newBinaryPath, binaryPath));
+
+    const content = await readFile(binaryPath, "utf8");
+    expect(content).toBe("#!/bin/sh\necho new");
+  });
+
+  test("sets executable permissions on target binary", async () => {
+    await Effect.runPromise(replaceBinary(newBinaryPath, binaryPath));
+
+    const s = await stat(binaryPath);
+    // check owner execute bit
+    expect(s.mode & 0o111).toBeGreaterThan(0);
+  });
+
+  test("leaves no temp file behind after success", async () => {
+    await Effect.runPromise(replaceBinary(newBinaryPath, binaryPath));
+
+    // list files in dir — only the replaced binary should remain
+    const { readdir } = await import("fs/promises");
+    const files = await readdir(dir);
+    const tempFiles = files.filter((f) => f.startsWith(".polar-update-"));
+    expect(tempFiles).toHaveLength(0);
+  });
+
+  test("throws and cleans up temp file on non-EACCES write error", async () => {
+    // Simulate a generic I/O error during Bun.write (not EACCES)
+    const bunSpy = spyOn(Bun, "write").mockImplementationOnce(() =>
+      Promise.reject(new Error("EIO: input/output error")),
+    );
+
+    await expect(
+      Effect.runPromise(replaceBinary(newBinaryPath, binaryPath)),
+    ).rejects.toThrow("EIO");
+
+    const { readdir } = await import("fs/promises");
+    const files = await readdir(dir);
+    const tempFiles = files.filter((f) => f.startsWith(".polar-update-"));
+    expect(tempFiles).toHaveLength(0);
+
+    bunSpy.mockRestore();
+  });
+
+  test("does not throw when EACCES triggers sudo fallback", async () => {
+    // Simulate EACCES on rename by mocking Bun.write to throw it
+    const bunSpy = spyOn(Bun, "write").mockImplementationOnce(() => {
+      const err: any = new Error("EACCES: permission denied");
+      err.code = "EACCES";
+      return Promise.reject(err);
+    });
+
+    // Mock Bun.spawn so sudo mv appears to succeed
+    const spawnSpy = spyOn(Bun, "spawn").mockImplementationOnce(() => ({
+      exited: Promise.resolve(0),
+    }));
+
+    await Effect.runPromise(replaceBinary(newBinaryPath, binaryPath));
+
+    // Verify sudo mv was called with the right args
+    expect(spawnSpy).toHaveBeenCalledWith(
+      ["sudo", "mv", newBinaryPath, binaryPath],
+      expect.objectContaining({ stdin: "inherit" }),
+    );
+
+    bunSpy.mockRestore();
+    spawnSpy.mockRestore();
+  });
+
+  test("throws when sudo mv exits non-zero", async () => {
+    const bunSpy = spyOn(Bun, "write").mockImplementationOnce(() => {
+      const err: any = new Error("EACCES: permission denied");
+      err.code = "EACCES";
+      return Promise.reject(err);
+    });
+
+    const spawnSpy = spyOn(Bun, "spawn").mockImplementationOnce(() => ({
+      exited: Promise.resolve(1),
+    }));
+
+    await expect(
+      Effect.runPromise(replaceBinary(newBinaryPath, binaryPath)),
+    ).rejects.toThrow("sudo mv failed");
+
+    bunSpy.mockRestore();
+    spawnSpy.mockRestore();
+  });
+});

src/commands/update.ts

@@ -1,11 +1,72 @@
 import { Command } from "@effect/cli";
 import { Console, Effect, Schema } from "effect";
 import { createHash } from "crypto";
-import { chmod, mkdtemp, rm } from "fs/promises";
+import { chmod, mkdtemp, rename, rm, unlink } from "fs/promises";
 import { tmpdir } from "os";
-import { join } from "path";
+import { dirname, join } from "path";
 import { VERSION } from "../version";
 
+const fsError = (e: unknown): Error =>
+  Object.assign(
+    new Error(e instanceof Error ? e.message : String(e)),
+    { code: (e as any)?.code },
+  );
+
+export const replaceBinary = (
+  newBinaryPath: string,
+  binaryPath: string,
+): Effect.Effect<void, Error> =>
+  Effect.gen(function* () {
+    yield* Effect.tryPromise({
+      try: () => chmod(newBinaryPath, 0o755),
+      catch: () => new Error("Failed to chmod new binary"),
+    });
+
+    const tempPath = join(dirname(binaryPath), `.polar-update-${Date.now()}`);
+
+    yield* Effect.gen(function* () {
+      const newBinary = yield* Effect.tryPromise({
+        try: () => Bun.file(newBinaryPath).arrayBuffer(),
+        catch: fsError,
+      });
+      yield* Effect.tryPromise({
+        try: () => Bun.write(tempPath, newBinary),
+        catch: fsError,
+      });
+      yield* Effect.tryPromise({
+        try: () => rename(tempPath, binaryPath),
+        catch: fsError,
+      });
+    }).pipe(
+      Effect.tapError(() =>
+        Effect.promise(() => unlink(tempPath).catch(() => {})),
+      ),
+      Effect.catchAll((e: Error) =>
+        (e as any)?.code === "EACCES"
+          ? Effect.gen(function* () {
+              const proc = Bun.spawn(["sudo", "mv", newBinaryPath, binaryPath], {
+                stdout: "inherit",
+                stderr: "inherit",
+                stdin: "inherit",
+              });
+              const exitCode = yield* Effect.tryPromise({
+                try: () => proc.exited,
+                catch: () => new Error("Failed to run sudo mv"),
+              });
+              if (exitCode !== 0) {
+                return yield* Effect.fail(new Error("sudo mv failed"));
+              }
+            })
+          : Effect.fail(e),
+      ),
+    );
+
+    yield* Effect.tryPromise({
+      try: () => chmod(binaryPath, 0o755),
+      catch: () => new Error("Failed to chmod binary"),
+    });
+  });
+
 const REPO = "polarsource/cli";
 
 const GitHubRelease = Schema.Struct({
@@ -137,7 +198,7 @@ const downloadAndUpdate = (
         }
 
         const archiveData = yield* Effect.tryPromise({
-          try: () => Bun.file(archivePath).arrayBuffer(),
+          try: () => Bun.file(archivePath).arrayBuffer() as Promise<ArrayBuffer>,
           catch: () => new Error("Failed to read archive for checksum"),
         });
 
@@ -180,17 +241,7 @@ const downloadAndUpdate = (
 
         yield* Console.log(`${dim}Replacing binary...${reset}`);
 
-        yield* Effect.tryPromise({
-          try: async () => {
-            const newBinary = await Bun.file(newBinaryPath).arrayBuffer();
-            await Bun.write(binaryPath, newBinary);
-            await chmod(binaryPath, 0o755);
-          },
-          catch: (e) =>
-            new Error(
-              `Failed to replace binary: ${e instanceof Error ? e.message : e}`,
-            ),
-        });
+        yield* replaceBinary(newBinaryPath, binaryPath);
 
         yield* Console.log("");
         yield* Console.log(

src/version.ts

@@ -1 +1 @@
-export const VERSION = "v1.3.1";
+export const VERSION = "v1.3.2";