fix: repair rb update on Windows and wire the background update check (#37)

* fix: extract rb update archive to temp dir, not over the running binary

rb update extracted the release archive straight into the binary's own
directory. The payload (rb / rb.exe) shares the running executable's name,
so Expand-Archive -Force tried to overwrite the live, locked exe. On Windows
its internal Remove-Item fails with "Access to the path is denied", aborting
the update. The extractedPath !== binPath guard was then false (both resolved
to the same rb.exe), so the .new staging rename was skipped and the run died
with "extracted binary not found at expected location".

Extract into an isolated .rb-update-<ts> temp dir instead, then stage the
result as .new and rename-swap into place. Renaming a running binary is
permitted on Windows; deleting it is not. The existing .bak rollback and
post-swap --version verification are unchanged. The temp dir is always
removed via finally.

Verified end-to-end on Windows: a pinned 1.2.0 standalone self-replaced to
the live 1.3.0 release, kept rb.exe.bak for rollback, left no temp dirs, and
re-ran as a clean no-op.

* fix: wire the background update check so the update notice works

The "Update available" notice on the welcome screen read a cache that nothing
ever populated: checkForUpdate() was only ever called from tests, so the cache
stayed empty and getPendingNotification() always returned null. The notice
could never fire on its own.

A naive inline call cannot fix this — short-lived commands (the welcome screen
exits immediately) terminate long before the GitHub round-trip completes, so
the fetch would be killed mid-flight. Instead spawn a detached, unref'd
`rb __update-check` process on launch that outlives the command and warms the
cache for the next run (the same approach gh and update-notifier use).

- add isRefreshDue(): cheap sync gate (honors skip rules + 24h TTL) so a fresh
  cache costs zero subprocesses
- add the hidden __update-check entrypoint in main.ts; gate it before routing
- spawn the detached refresh only in standalone builds (IS_BIN); dev mode's
  execPath is bun, not rb
- skip entirely in CI and under --quiet/--json/--url-only (existing shouldSkip)

Verified end-to-end on Windows: a pinned 1.2.0 standalone spawned the detached
check, the cache filled with 1.3.0, and the next run rendered
"Update available: 1.2.0 -> 1.3.0". --quiet produced no output and no spawn.

Author: a-essawy

src/__tests__/update-check.test.ts

@@ -151,6 +151,40 @@ describe("update-check", () => {
     expect(cache.latest).toBe("1.1.0");
   });
 
+  it("isRefreshDue: true when no cache exists", async () => {
+    const { isRefreshDue } = await import("../lib/update-check.js");
+    expect(isRefreshDue()).toBe(true);
+  });
+
+  it("isRefreshDue: false when cache is within TTL", async () => {
+    const cachePath = join(tempDir, "update-check.json");
+    mkdirSync(dirname(cachePath), { recursive: true });
+    writeFileSync(
+      cachePath,
+      JSON.stringify({ latest: "1.1.0", checkedAt: Date.now(), ttl: 24 * 60 * 60 * 1000 })
+    );
+    const { isRefreshDue } = await import("../lib/update-check.js");
+    expect(isRefreshDue()).toBe(false);
+  });
+
+  it("isRefreshDue: true when cache is older than TTL", async () => {
+    const cachePath = join(tempDir, "update-check.json");
+    mkdirSync(dirname(cachePath), { recursive: true });
+    const ttl = 24 * 60 * 60 * 1000;
+    writeFileSync(
+      cachePath,
+      JSON.stringify({ latest: "1.1.0", checkedAt: Date.now() - ttl - 1000, ttl })
+    );
+    const { isRefreshDue } = await import("../lib/update-check.js");
+    expect(isRefreshDue()).toBe(true);
+  });
+
+  it("isRefreshDue: false when a skip rule applies (CI)", async () => {
+    process.env.CI = "true";
+    const { isRefreshDue } = await import("../lib/update-check.js");
+    expect(isRefreshDue()).toBe(false);
+  });
+
   it("does not write cache when no CLI release found", async () => {
     const fetchMock = mock(async () =>
       new Response(JSON.stringify([

src/commands/update.ts

@@ -2,7 +2,7 @@
  * `rb update` — Self-replace for standalone binary, print install instructions for dev mode.
  * Detects install method via compile-time IS_STANDALONE define.
  */
-import { writeFileSync, mkdirSync, existsSync, chmodSync, renameSync, unlinkSync } from "node:fs";
+import { writeFileSync, mkdirSync, existsSync, chmodSync, renameSync, unlinkSync, rmSync } from "node:fs";
 import { join, dirname } from "node:path";
 import { homedir } from "node:os";
 import { spawnSync } from "node:child_process";
@@ -139,50 +139,50 @@ async function updateBinary(latest: string): Promise<void> {
   }
   console.log(`Checksum verified: ${actualHash}`);
 
-  // Extract archive to temp path alongside current binary
+  // Extract into an isolated temp dir, NEVER straight into binDir. The archive's
+  // payload (`rb` / `rb.exe`) shares the running binary's name, so extracting
+  // into binDir forces the archiver to overwrite the live executable — which
+  // Windows forbids (the running image is locked; `Expand-Archive -Force`'s
+  // internal Remove-Item fails with "Access to the path ... is denied"). Extract
+  // aside, then rename-swap below: renaming a running binary IS allowed on
+  // Windows, deleting it is not.
   const binDir = dirname(binPath);
   const tmpBinPath = `${binPath}.new`;
   const bakBinPath = `${binPath}.bak`;
+  const archiveBase = ext === ".tar.gz" ? "rb" : "rb.exe";
+  const extractDir = join(binDir, `.rb-update-${Date.now()}`);
+  mkdirSync(extractDir, { recursive: true });
 
-  if (ext === ".tar.gz") {
-    const tmpArchivePath = join(binDir, `update-${Date.now()}.tar.gz`);
-    writeFileSync(tmpArchivePath, archive);
-    const tar = spawnSync("tar", ["-xzf", tmpArchivePath, "-C", binDir], { stdio: "inherit" });
-    try {
-      unlinkSync(tmpArchivePath);
-    } catch {
-      // non-fatal
-    }
-    if (tar.status !== 0) throw new Error("tar extraction failed");
-    const extractedPath = join(binDir, "rb");
-    if (existsSync(extractedPath) && extractedPath !== binPath) {
-      renameSync(extractedPath, tmpBinPath);
-    }
-  } else {
-    const tmpArchivePath = join(binDir, `update-${Date.now()}.zip`);
+  try {
+    const tmpArchivePath = join(extractDir, `archive${ext}`);
     writeFileSync(tmpArchivePath, archive);
-    const expand = spawnSync(
-      "powershell",
-      [
-        "-Command",
-        `Expand-Archive -Path '${tmpArchivePath}' -DestinationPath '${binDir}' -Force`,
-      ],
-      { stdio: "inherit" }
-    );
-    try {
-      unlinkSync(tmpArchivePath);
-    } catch {
-      // non-fatal
-    }
-    if (expand.status !== 0) throw new Error("Expand-Archive failed");
-    const extractedPath = join(binDir, "rb.exe");
-    if (existsSync(extractedPath) && extractedPath !== binPath) {
-      renameSync(extractedPath, tmpBinPath);
+
+    if (ext === ".tar.gz") {
+      const tar = spawnSync("tar", ["-xzf", tmpArchivePath, "-C", extractDir], { stdio: "inherit" });
+      if (tar.status !== 0) throw new Error("tar extraction failed");
+    } else {
+      const expand = spawnSync(
+        "powershell",
+        [
+          "-NoProfile",
+          "-Command",
+          `Expand-Archive -Path '${tmpArchivePath}' -DestinationPath '${extractDir}' -Force`,
+        ],
+        { stdio: "inherit" }
+      );
+      if (expand.status !== 0) throw new Error("Expand-Archive failed");
     }
-  }
 
-  if (!existsSync(tmpBinPath)) {
-    throw new Error("extracted binary not found at expected location");
+    const extractedPath = join(extractDir, archiveBase);
+    if (!existsSync(extractedPath)) {
+      throw new Error("extracted binary not found at expected location");
+    }
+    // Stage the new binary next to the live one as `.new` (same volume → the
+    // swap below is atomic). Clear any stale `.new` from an aborted prior run.
+    if (existsSync(tmpBinPath)) unlinkSync(tmpBinPath);
+    renameSync(extractedPath, tmpBinPath);
+  } finally {
+    rmSync(extractDir, { recursive: true, force: true });
   }
 
   chmodSync(tmpBinPath, 0o755);

src/lib/update-check.ts

@@ -77,6 +77,20 @@ function versionGt(a: string, b: string): boolean {
   return false;
 }
 
+/**
+ * Cheap synchronous gate: is a background refresh warranted right now?
+ * Honors the same skip rules as the check itself, plus the 24h TTL. Used by the
+ * launcher to decide whether to spawn the detached refresh process at all — so a
+ * fresh cache costs zero subprocesses. No jitter here: jitter exists to spread
+ * the network call across the fleet on release day, not to gate a local spawn.
+ */
+export function isRefreshDue(): boolean {
+  if (shouldSkip()) return false;
+  const cache = readCache();
+  if (!cache) return true;
+  return Date.now() - cache.checkedAt >= cache.ttl;
+}
+
 /**
  * Non-blocking background check. Updates cache file if network succeeds.
  * Uses injected fetch so tests can mock it.

src/main.ts

@@ -4,16 +4,49 @@
  * Bare `rb` and `rb --help` render the welcome screen. Subcommands are derived
  * from the central registry. Unknown commands and stray ffmpeg flags are hinted.
  */
+import { spawn } from "node:child_process";
 import { defineCommand, runMain, renderUsage, type CommandDef } from "citty";
 import { VERSION } from "./generated/version.js";
 import { toSubCommands, commandNames, COMMANDS } from "./registry.js";
 import { buildState, renderWelcome, renderHelp, renderWelcomeJson } from "./lib/welcome.js";
+import { checkForUpdate, isRefreshDue } from "./lib/update-check.js";
+
+// Compile-time define from `bun build --compile --define`. Undefined in dev mode
+// (`bun run src/main.ts`), where process.execPath is the bun runtime, not `rb`.
+declare const IS_STANDALONE: boolean | undefined;
+const IS_BIN = typeof IS_STANDALONE !== "undefined" && IS_STANDALONE === true;
 
 // Never crash with an EPIPE stack trace when piped into a closed reader (`rb | head`).
 process.stdout.on("error", (err: NodeJS.ErrnoException) => {
   if (err.code === "EPIPE") process.exit(0);
 });
 
+// Hidden internal entrypoint. A prior invocation spawns `rb __update-check`
+// detached so the GitHub round-trip warms the version cache WITHOUT blocking the
+// user's command — short-lived commands (e.g. the welcome screen) exit long
+// before an inline fetch could finish, which is why the refresh must outlive
+// them in its own process. Not a registered command; handled before all routing.
+function isBackgroundCheck(): boolean {
+  return process.argv.slice(2)[0] === "__update-check";
+}
+
+// On launch, fire the detached refresh if one is due. Best-effort: a failed
+// spawn must never surface to the user or delay their command.
+function spawnBackgroundCheck(): void {
+  if (!IS_BIN) return; // dev mode: execPath is bun, not the rb binary
+  if (!isRefreshDue()) return; // fresh cache or a skip rule (CI, --quiet, ...)
+  try {
+    const child = spawn(process.execPath, ["__update-check"], {
+      detached: true,
+      stdio: "ignore",
+      windowsHide: true,
+    });
+    child.unref();
+  } catch {
+    // Never block the CLI on a failed background spawn.
+  }
+}
+
 const FFMPEG_FLAGS = ["-i", "-vf", "-c:v", "-c:a", "-f", "-filter_complex"];
 
 // A minimal parent stub passed to renderUsage so subcommand usage lines are
@@ -87,43 +120,58 @@ const main = defineCommand({
   },
 });
 
-// Pre-validate: intercept unknown commands before citty throws with exit 1.
-// citty dispatches to run() only when args parse cleanly, but it throws
-// CLIError("Unknown command") before run() for unrecognised positional args.
-// We catch those here so we can exit 2 instead of 1.
-const rawArgs = process.argv.slice(2);
+// `knownNames` is referenced by the main command's run() closure above, so it
+// stays at module scope.
 const knownNames = new Set([...commandNames(), "help"]);
 
-// Check for --help / --version / -h — citty owns these, don't intercept.
-const isCittyOwned =
-  rawArgs.includes("--help") ||
-  rawArgs.includes("-h") ||
-  (rawArgs.length === 1 && rawArgs[0] === "--version");
-
-if (!isCittyOwned) {
-  const firstPositional = rawArgs.find((a) => !a.startsWith("-"));
-  if (firstPositional !== undefined && !knownNames.has(firstPositional)) {
-    // Unknown first positional — but first check if any arg looks like an
-    // ffmpeg flag (e.g. `rb -i in.mp4 out.mp4` or `rb -vf scale=1:1`).
-    // The check must run even when the first positional isn't itself a flag,
-    // because the flag may appear after file arguments.
-    if (rawArgs.some((a) => FFMPEG_FLAGS.includes(a))) {
-      process.stderr.write(`Did you mean: rb ffmpeg ${rawArgs.join(" ")}?\n`);
+if (isBackgroundCheck()) {
+  // Detached refresh process: warm the cache, then exit. Never touches routing,
+  // stdout, or the user's terminal. Failures stay silent.
+  checkForUpdate(VERSION)
+    .catch(() => {})
+    .finally(() => process.exit(0));
+} else {
+  // Kick off the next refresh in the background before handing control to the
+  // CLI. The notice shown this run (if any) comes from a prior run's cache.
+  spawnBackgroundCheck();
+
+  // Pre-validate: intercept unknown commands before citty throws with exit 1.
+  // citty dispatches to run() only when args parse cleanly, but it throws
+  // CLIError("Unknown command") before run() for unrecognised positional args.
+  // We catch those here so we can exit 2 instead of 1.
+  const rawArgs = process.argv.slice(2);
+
+  // Check for --help / --version / -h — citty owns these, don't intercept.
+  const isCittyOwned =
+    rawArgs.includes("--help") ||
+    rawArgs.includes("-h") ||
+    (rawArgs.length === 1 && rawArgs[0] === "--version");
+
+  if (!isCittyOwned) {
+    const firstPositional = rawArgs.find((a) => !a.startsWith("-"));
+    if (firstPositional !== undefined && !knownNames.has(firstPositional)) {
+      // Unknown first positional — but first check if any arg looks like an
+      // ffmpeg flag (e.g. `rb -i in.mp4 out.mp4` or `rb -vf scale=1:1`).
+      // The check must run even when the first positional isn't itself a flag,
+      // because the flag may appear after file arguments.
+      if (rawArgs.some((a) => FFMPEG_FLAGS.includes(a))) {
+        process.stderr.write(`Did you mean: rb ffmpeg ${rawArgs.join(" ")}?\n`);
+        process.exit(2);
+      }
+      process.stderr.write(`unknown command '${firstPositional}' -- run \`rb\` to see commands\n`);
       process.exit(2);
     }
-    process.stderr.write(`unknown command '${firstPositional}' -- run \`rb\` to see commands\n`);
-    process.exit(2);
   }
-}
 
-runMain(main, {
-  async showUsage(cmd, parent) {
-    // Root help (`rb --help` / `rb -h`) → our welcome+flags screen.
-    if (!parent) {
-      process.stdout.write(renderHelp(buildState()) + "\n");
-      return;
-    }
-    // Subcommand help (`rb ffmpeg --help`) → citty's default usage.
-    process.stdout.write((await renderUsage(cmd, parent)) + "\n");
-  },
-});
+  runMain(main, {
+    async showUsage(cmd, parent) {
+      // Root help (`rb --help` / `rb -h`) → our welcome+flags screen.
+      if (!parent) {
+        process.stdout.write(renderHelp(buildState()) + "\n");
+        return;
+      }
+      // Subcommand help (`rb ffmpeg --help`) → citty's default usage.
+      process.stdout.write((await renderUsage(cmd, parent)) + "\n");
+    },
+  });
+}