From 3220a672c5df818bf39e60769e6ae92a2ed793ea Mon Sep 17 00:00:00 2001 From: Abdelrahman Essawy Date: Sun, 7 Jun 2026 00:58:57 +0300 Subject: [PATCH 1/2] 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- 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. --- src/commands/update.ts | 76 +++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/src/commands/update.ts b/src/commands/update.ts index 3de8f8d..3286caf 100644 --- a/src/commands/update.ts +++ b/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 { } 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); From 6726f60eed568c9d77badc39a28f60691a1f3606 Mon Sep 17 00:00:00 2001 From: Abdelrahman Essawy Date: Sun, 7 Jun 2026 01:08:15 +0300 Subject: [PATCH 2/2] fix: wire the background update check so the update notice works MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/__tests__/update-check.test.ts | 34 +++++++++ src/lib/update-check.ts | 14 ++++ src/main.ts | 116 ++++++++++++++++++++--------- 3 files changed, 130 insertions(+), 34 deletions(-) diff --git a/src/__tests__/update-check.test.ts b/src/__tests__/update-check.test.ts index a0a4725..28c2de7 100644 --- a/src/__tests__/update-check.test.ts +++ b/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([ diff --git a/src/lib/update-check.ts b/src/lib/update-check.ts index d29f782..4ab252f 100644 --- a/src/lib/update-check.ts +++ b/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. diff --git a/src/main.ts b/src/main.ts index c250950..5d6a41c 100644 --- a/src/main.ts +++ b/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"); + }, + }); +}