From 75164711aae00c7f1ef8cdb449ec1657e81566a2 Mon Sep 17 00:00:00 2001 From: Zeeshan Adil Date: Wed, 15 Jul 2026 11:47:06 +0500 Subject: [PATCH] fix: show update notice on next run instead of racing 500ms window --- src/index.ts | 13 ++- src/lib/version-check.test.ts | 153 ++++++++++++++++++++++++++++++++++ src/lib/version-check.ts | 61 ++++++++++++-- 3 files changed, 212 insertions(+), 15 deletions(-) diff --git a/src/index.ts b/src/index.ts index 0ca5bed..e916dee 100644 --- a/src/index.ts +++ b/src/index.ts @@ -26,7 +26,7 @@ import { registerMcpUninstall } from "./commands/mcp-uninstall.js"; import { registerListMcp } from "./commands/list-mcp.js"; import { registerDoctor } from "./commands/doctor.js"; -import { maybeNotifyUpdate } from "./lib/version-check.js"; +import { showPendingUpdateNotice, scheduleUpdateCheck } from "./lib/version-check.js"; // Replaced at build time by tsup's `define` with the package.json version. // The fallback only fires under `npm run dev` (tsx) where define doesn't apply. @@ -81,15 +81,12 @@ async function main(): Promise { registerMcpUninstall(mcpCmd); registerListMcp(listCmd); - // Best-effort update notice — runs in parallel with the command, - // never blocks command output, never throws. - const notifyPromise = maybeNotifyUpdate(VERSION, process.argv[2]); + // Show any pending update notice from the previous check (appears before command output). + showPendingUpdateNotice(VERSION, process.argv[2]); + // Fire the registry check for the next run — no await, never blocks. + scheduleUpdateCheck(VERSION, process.argv[2]); await program.parseAsync(process.argv); - - // Brief window for the update notice to land before exit. It's - // background work; we don't wait forever. - await Promise.race([notifyPromise, new Promise((r) => setTimeout(r, 500))]); } main().catch((err: any) => { diff --git a/src/lib/version-check.test.ts b/src/lib/version-check.test.ts index 47d25a1..fd21b9b 100644 --- a/src/lib/version-check.test.ts +++ b/src/lib/version-check.test.ts @@ -9,9 +9,12 @@ import { isNewer, maybeNotifyUpdate, parseVersion, + scheduleUpdateCheck, + showPendingUpdateNotice, } from "./version-check.js"; const CACHE_FILE = path.join(os.tmpdir(), "olostep-cli-version-cache.json"); +const NOTICE_FILE = path.join(os.tmpdir(), "olostep-cli-update-notice.json"); function clearCache() { try { fs.unlinkSync(CACHE_FILE); } catch { /* ignore */ } @@ -211,3 +214,153 @@ describe("maybeNotifyUpdate", () => { await expect(maybeNotifyUpdate("1.0.0")).resolves.toBeUndefined(); }); }); + +describe("showPendingUpdateNotice", () => { + const ORIGINAL_TTY = (process.stderr as any).isTTY; + + function clearNotice() { + try { fs.unlinkSync(NOTICE_FILE); } catch { /* ignore */ } + } + + function writeNotice(data: object) { + fs.writeFileSync(NOTICE_FILE, JSON.stringify(data), "utf8"); + } + + beforeEach(() => { + clearNotice(); + delete process.env.OLOSTEP_NO_UPDATE_NOTICE; + delete process.env.OLOSTEP_NO_UPDATE_CHECK; + }); + + afterEach(() => { + (process.stderr as any).isTTY = ORIGINAL_TTY; + delete process.env.OLOSTEP_NO_UPDATE_NOTICE; + delete process.env.OLOSTEP_NO_UPDATE_CHECK; + clearNotice(); + }); + + it("prints to stderr when a newer notice file exists", () => { + (process.stderr as any).isTTY = true; + writeNotice({ latest: "9.9.9" }); + const chunks: string[] = []; + const orig = process.stderr.write.bind(process.stderr); + process.stderr.write = (chunk: any) => { chunks.push(String(chunk)); return true; }; + showPendingUpdateNotice("1.0.0"); + process.stderr.write = orig; + expect(chunks.join("")).toContain("9.9.9"); + expect(fs.existsSync(NOTICE_FILE)).toBe(false); + }); + + it("does nothing when no notice file exists", () => { + (process.stderr as any).isTTY = true; + const chunks: string[] = []; + const orig = process.stderr.write.bind(process.stderr); + process.stderr.write = (chunk: any) => { chunks.push(String(chunk)); return true; }; + showPendingUpdateNotice("1.0.0"); + process.stderr.write = orig; + expect(chunks).toHaveLength(0); + }); + + it("deletes the notice file even when version is not newer", () => { + (process.stderr as any).isTTY = true; + writeNotice({ latest: "0.0.1" }); + showPendingUpdateNotice("1.0.0"); + expect(fs.existsSync(NOTICE_FILE)).toBe(false); + }); + + it("skips when stderr is not a TTY", () => { + (process.stderr as any).isTTY = false; + writeNotice({ latest: "9.9.9" }); + const chunks: string[] = []; + const orig = process.stderr.write.bind(process.stderr); + process.stderr.write = (chunk: any) => { chunks.push(String(chunk)); return true; }; + showPendingUpdateNotice("1.0.0"); + process.stderr.write = orig; + expect(chunks).toHaveLength(0); + // Notice file should NOT be deleted when we skip (TTY check) + expect(fs.existsSync(NOTICE_FILE)).toBe(true); + }); + + it("skips when OLOSTEP_NO_UPDATE_CHECK is set", () => { + (process.stderr as any).isTTY = true; + process.env.OLOSTEP_NO_UPDATE_CHECK = "1"; + writeNotice({ latest: "9.9.9" }); + showPendingUpdateNotice("1.0.0"); + expect(fs.existsSync(NOTICE_FILE)).toBe(true); + }); + + it("skips when invokedSubcommand === 'update'", () => { + (process.stderr as any).isTTY = true; + writeNotice({ latest: "9.9.9" }); + showPendingUpdateNotice("1.0.0", "update"); + expect(fs.existsSync(NOTICE_FILE)).toBe(true); + }); + + it("never throws on malformed notice file", () => { + (process.stderr as any).isTTY = true; + fs.writeFileSync(NOTICE_FILE, "not-json", "utf8"); + expect(() => showPendingUpdateNotice("1.0.0")).not.toThrow(); + }); +}); + +describe("scheduleUpdateCheck", () => { + function clearNotice() { + try { fs.unlinkSync(NOTICE_FILE); } catch { /* ignore */ } + } + + beforeEach(() => { + clearCache(); + clearNotice(); + delete process.env.OLOSTEP_NO_UPDATE_NOTICE; + delete process.env.OLOSTEP_NO_UPDATE_CHECK; + }); + + afterEach(() => { + vi.unstubAllGlobals(); + delete process.env.OLOSTEP_NO_UPDATE_NOTICE; + delete process.env.OLOSTEP_NO_UPDATE_CHECK; + clearCache(); + clearNotice(); + }); + + it("writes a notice file when a newer version is found", async () => { + fs.writeFileSync( + CACHE_FILE, + JSON.stringify({ latest: "9.9.9", checkedAt: Date.now() }), + ); + scheduleUpdateCheck("1.0.0"); + // scheduleUpdateCheck is fire-and-forget; wait for the microtask to settle + await new Promise((r) => setTimeout(r, 50)); + expect(fs.existsSync(NOTICE_FILE)).toBe(true); + const data = JSON.parse(fs.readFileSync(NOTICE_FILE, "utf8")); + expect(data.latest).toBe("9.9.9"); + }); + + it("does not write a notice file when already on latest", async () => { + fs.writeFileSync( + CACHE_FILE, + JSON.stringify({ latest: "1.0.0", checkedAt: Date.now() }), + ); + scheduleUpdateCheck("1.0.0"); + await new Promise((r) => setTimeout(r, 50)); + expect(fs.existsSync(NOTICE_FILE)).toBe(false); + }); + + it("skips when OLOSTEP_NO_UPDATE_CHECK is set", async () => { + process.env.OLOSTEP_NO_UPDATE_CHECK = "1"; + const fetchSpy = vi.fn(); + vi.stubGlobal("fetch", fetchSpy); + scheduleUpdateCheck("1.0.0"); + await new Promise((r) => setTimeout(r, 50)); + expect(fetchSpy).not.toHaveBeenCalled(); + expect(fs.existsSync(NOTICE_FILE)).toBe(false); + }); + + it("skips when invokedSubcommand === 'update'", async () => { + const fetchSpy = vi.fn(); + vi.stubGlobal("fetch", fetchSpy); + scheduleUpdateCheck("1.0.0", "update"); + await new Promise((r) => setTimeout(r, 50)); + expect(fetchSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/version-check.ts b/src/lib/version-check.ts index f4129e6..ba3f665 100644 --- a/src/lib/version-check.ts +++ b/src/lib/version-check.ts @@ -5,6 +5,7 @@ import * as path from "node:path"; import { c } from "./colors.js"; const CACHE_FILE = path.join(os.tmpdir(), "olostep-cli-version-cache.json"); +const NOTICE_FILE = path.join(os.tmpdir(), "olostep-cli-update-notice.json"); const CACHE_TTL_MS = 24 * 60 * 60 * 1000; // 24h const REGISTRY_URL = "https://registry.npmjs.org/olostep-cli/latest"; @@ -79,15 +80,61 @@ export async function checkForUpdate(current: string): Promise { return isNewer(latest, current) ? latest : null; } +function isSuppressed(subcommand?: string): boolean { + if (subcommand === "update") return true; + if (process.env.OLOSTEP_NO_UPDATE_CHECK) return true; + if (process.env.OLOSTEP_NO_UPDATE_NOTICE) return true; + return false; +} + +/** + * Shows any pending update notice written by a previous scheduleUpdateCheck call. + * Call at process start (before the command) so the notice lands before output. + * Deletes the notice file after reading it. Fail-silent. + */ +export function showPendingUpdateNotice(current: string, invokedSubcommand?: string): void { + if (isSuppressed(invokedSubcommand)) return; + try { + if (!process.stderr.isTTY) return; + } catch { return; } + try { + if (!fs.existsSync(NOTICE_FILE)) return; + const raw = fs.readFileSync(NOTICE_FILE, "utf8"); + // Delete regardless of whether we can show it. + try { fs.unlinkSync(NOTICE_FILE); } catch { /* ignore */ } + const data = JSON.parse(raw); + const latest = (data?.latest || "").toString().trim(); + if (!latest || !isNewer(latest, current)) return; + process.stderr.write( + c.yellow(` ↑ olostep ${latest} available — run \`olostep update\`\n`), + ); + } catch { /* fail silent */ } +} + +/** + * Fires a background registry check with no await. If a newer version is found, + * writes a notice file so showPendingUpdateNotice displays it on the next run. + * Never blocks the command. Fail-silent. + */ +export function scheduleUpdateCheck(current: string, invokedSubcommand?: string): void { + if (isSuppressed(invokedSubcommand)) return; + checkForUpdate(current) + .then((latest) => { + if (latest) { + try { + fs.writeFileSync(NOTICE_FILE, JSON.stringify({ latest }), "utf8"); + } catch { /* ignore */ } + } + }) + .catch(() => { /* ignore */ }); +} + /** - * Prints a one-line "update available" notice to stderr — interactive use only. - * Suppressed in pipes/CI and when OLOSTEP_NO_UPDATE_NOTICE is set. - * Never throws. + * Legacy combined helper. Kept for tests that import it directly. + * New code should use showPendingUpdateNotice + scheduleUpdateCheck. */ export async function maybeNotifyUpdate(current: string, invokedSubcommand?: string): Promise { - if (invokedSubcommand === "update") return; - if (process.env.OLOSTEP_NO_UPDATE_CHECK) return; - if (process.env.OLOSTEP_NO_UPDATE_NOTICE) return; + if (isSuppressed(invokedSubcommand)) return; try { if (!process.stderr.isTTY) return; } catch { return; } @@ -95,7 +142,7 @@ export async function maybeNotifyUpdate(current: string, invokedSubcommand?: str const latest = await checkForUpdate(current); if (latest) { process.stderr.write( - c.yellow(` ↑ olostep ${latest} is available (you have ${current}) — run \`olostep update\`\n`), + c.yellow(` ↑ olostep ${latest} available — run \`olostep update\`\n`), ); } } catch { /* fail silent */ }