Skip to content

Commit 7516471

Browse files
fix: show update notice on next run instead of racing 500ms window
1 parent 82fe200 commit 7516471

3 files changed

Lines changed: 212 additions & 15 deletions

File tree

src/index.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { registerMcpUninstall } from "./commands/mcp-uninstall.js";
2626
import { registerListMcp } from "./commands/list-mcp.js";
2727
import { registerDoctor } from "./commands/doctor.js";
2828

29-
import { maybeNotifyUpdate } from "./lib/version-check.js";
29+
import { showPendingUpdateNotice, scheduleUpdateCheck } from "./lib/version-check.js";
3030

3131
// Replaced at build time by tsup's `define` with the package.json version.
3232
// The fallback only fires under `npm run dev` (tsx) where define doesn't apply.
@@ -81,15 +81,12 @@ async function main(): Promise<void> {
8181
registerMcpUninstall(mcpCmd);
8282
registerListMcp(listCmd);
8383

84-
// Best-effort update notice — runs in parallel with the command,
85-
// never blocks command output, never throws.
86-
const notifyPromise = maybeNotifyUpdate(VERSION, process.argv[2]);
84+
// Show any pending update notice from the previous check (appears before command output).
85+
showPendingUpdateNotice(VERSION, process.argv[2]);
86+
// Fire the registry check for the next run — no await, never blocks.
87+
scheduleUpdateCheck(VERSION, process.argv[2]);
8788

8889
await program.parseAsync(process.argv);
89-
90-
// Brief window for the update notice to land before exit. It's
91-
// background work; we don't wait forever.
92-
await Promise.race([notifyPromise, new Promise((r) => setTimeout(r, 500))]);
9390
}
9491

9592
main().catch((err: any) => {

src/lib/version-check.test.ts

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,12 @@ import {
99
isNewer,
1010
maybeNotifyUpdate,
1111
parseVersion,
12+
scheduleUpdateCheck,
13+
showPendingUpdateNotice,
1214
} from "./version-check.js";
1315

1416
const CACHE_FILE = path.join(os.tmpdir(), "olostep-cli-version-cache.json");
17+
const NOTICE_FILE = path.join(os.tmpdir(), "olostep-cli-update-notice.json");
1518

1619
function clearCache() {
1720
try { fs.unlinkSync(CACHE_FILE); } catch { /* ignore */ }
@@ -211,3 +214,153 @@ describe("maybeNotifyUpdate", () => {
211214
await expect(maybeNotifyUpdate("1.0.0")).resolves.toBeUndefined();
212215
});
213216
});
217+
218+
describe("showPendingUpdateNotice", () => {
219+
const ORIGINAL_TTY = (process.stderr as any).isTTY;
220+
221+
function clearNotice() {
222+
try { fs.unlinkSync(NOTICE_FILE); } catch { /* ignore */ }
223+
}
224+
225+
function writeNotice(data: object) {
226+
fs.writeFileSync(NOTICE_FILE, JSON.stringify(data), "utf8");
227+
}
228+
229+
beforeEach(() => {
230+
clearNotice();
231+
delete process.env.OLOSTEP_NO_UPDATE_NOTICE;
232+
delete process.env.OLOSTEP_NO_UPDATE_CHECK;
233+
});
234+
235+
afterEach(() => {
236+
(process.stderr as any).isTTY = ORIGINAL_TTY;
237+
delete process.env.OLOSTEP_NO_UPDATE_NOTICE;
238+
delete process.env.OLOSTEP_NO_UPDATE_CHECK;
239+
clearNotice();
240+
});
241+
242+
it("prints to stderr when a newer notice file exists", () => {
243+
(process.stderr as any).isTTY = true;
244+
writeNotice({ latest: "9.9.9" });
245+
const chunks: string[] = [];
246+
const orig = process.stderr.write.bind(process.stderr);
247+
process.stderr.write = (chunk: any) => { chunks.push(String(chunk)); return true; };
248+
showPendingUpdateNotice("1.0.0");
249+
process.stderr.write = orig;
250+
expect(chunks.join("")).toContain("9.9.9");
251+
expect(fs.existsSync(NOTICE_FILE)).toBe(false);
252+
});
253+
254+
it("does nothing when no notice file exists", () => {
255+
(process.stderr as any).isTTY = true;
256+
const chunks: string[] = [];
257+
const orig = process.stderr.write.bind(process.stderr);
258+
process.stderr.write = (chunk: any) => { chunks.push(String(chunk)); return true; };
259+
showPendingUpdateNotice("1.0.0");
260+
process.stderr.write = orig;
261+
expect(chunks).toHaveLength(0);
262+
});
263+
264+
it("deletes the notice file even when version is not newer", () => {
265+
(process.stderr as any).isTTY = true;
266+
writeNotice({ latest: "0.0.1" });
267+
showPendingUpdateNotice("1.0.0");
268+
expect(fs.existsSync(NOTICE_FILE)).toBe(false);
269+
});
270+
271+
it("skips when stderr is not a TTY", () => {
272+
(process.stderr as any).isTTY = false;
273+
writeNotice({ latest: "9.9.9" });
274+
const chunks: string[] = [];
275+
const orig = process.stderr.write.bind(process.stderr);
276+
process.stderr.write = (chunk: any) => { chunks.push(String(chunk)); return true; };
277+
showPendingUpdateNotice("1.0.0");
278+
process.stderr.write = orig;
279+
expect(chunks).toHaveLength(0);
280+
// Notice file should NOT be deleted when we skip (TTY check)
281+
expect(fs.existsSync(NOTICE_FILE)).toBe(true);
282+
});
283+
284+
it("skips when OLOSTEP_NO_UPDATE_CHECK is set", () => {
285+
(process.stderr as any).isTTY = true;
286+
process.env.OLOSTEP_NO_UPDATE_CHECK = "1";
287+
writeNotice({ latest: "9.9.9" });
288+
showPendingUpdateNotice("1.0.0");
289+
expect(fs.existsSync(NOTICE_FILE)).toBe(true);
290+
});
291+
292+
it("skips when invokedSubcommand === 'update'", () => {
293+
(process.stderr as any).isTTY = true;
294+
writeNotice({ latest: "9.9.9" });
295+
showPendingUpdateNotice("1.0.0", "update");
296+
expect(fs.existsSync(NOTICE_FILE)).toBe(true);
297+
});
298+
299+
it("never throws on malformed notice file", () => {
300+
(process.stderr as any).isTTY = true;
301+
fs.writeFileSync(NOTICE_FILE, "not-json", "utf8");
302+
expect(() => showPendingUpdateNotice("1.0.0")).not.toThrow();
303+
});
304+
});
305+
306+
describe("scheduleUpdateCheck", () => {
307+
function clearNotice() {
308+
try { fs.unlinkSync(NOTICE_FILE); } catch { /* ignore */ }
309+
}
310+
311+
beforeEach(() => {
312+
clearCache();
313+
clearNotice();
314+
delete process.env.OLOSTEP_NO_UPDATE_NOTICE;
315+
delete process.env.OLOSTEP_NO_UPDATE_CHECK;
316+
});
317+
318+
afterEach(() => {
319+
vi.unstubAllGlobals();
320+
delete process.env.OLOSTEP_NO_UPDATE_NOTICE;
321+
delete process.env.OLOSTEP_NO_UPDATE_CHECK;
322+
clearCache();
323+
clearNotice();
324+
});
325+
326+
it("writes a notice file when a newer version is found", async () => {
327+
fs.writeFileSync(
328+
CACHE_FILE,
329+
JSON.stringify({ latest: "9.9.9", checkedAt: Date.now() }),
330+
);
331+
scheduleUpdateCheck("1.0.0");
332+
// scheduleUpdateCheck is fire-and-forget; wait for the microtask to settle
333+
await new Promise((r) => setTimeout(r, 50));
334+
expect(fs.existsSync(NOTICE_FILE)).toBe(true);
335+
const data = JSON.parse(fs.readFileSync(NOTICE_FILE, "utf8"));
336+
expect(data.latest).toBe("9.9.9");
337+
});
338+
339+
it("does not write a notice file when already on latest", async () => {
340+
fs.writeFileSync(
341+
CACHE_FILE,
342+
JSON.stringify({ latest: "1.0.0", checkedAt: Date.now() }),
343+
);
344+
scheduleUpdateCheck("1.0.0");
345+
await new Promise((r) => setTimeout(r, 50));
346+
expect(fs.existsSync(NOTICE_FILE)).toBe(false);
347+
});
348+
349+
it("skips when OLOSTEP_NO_UPDATE_CHECK is set", async () => {
350+
process.env.OLOSTEP_NO_UPDATE_CHECK = "1";
351+
const fetchSpy = vi.fn();
352+
vi.stubGlobal("fetch", fetchSpy);
353+
scheduleUpdateCheck("1.0.0");
354+
await new Promise((r) => setTimeout(r, 50));
355+
expect(fetchSpy).not.toHaveBeenCalled();
356+
expect(fs.existsSync(NOTICE_FILE)).toBe(false);
357+
});
358+
359+
it("skips when invokedSubcommand === 'update'", async () => {
360+
const fetchSpy = vi.fn();
361+
vi.stubGlobal("fetch", fetchSpy);
362+
scheduleUpdateCheck("1.0.0", "update");
363+
await new Promise((r) => setTimeout(r, 50));
364+
expect(fetchSpy).not.toHaveBeenCalled();
365+
});
366+
});

src/lib/version-check.ts

Lines changed: 54 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import * as path from "node:path";
55
import { c } from "./colors.js";
66

77
const CACHE_FILE = path.join(os.tmpdir(), "olostep-cli-version-cache.json");
8+
const NOTICE_FILE = path.join(os.tmpdir(), "olostep-cli-update-notice.json");
89
const CACHE_TTL_MS = 24 * 60 * 60 * 1000; // 24h
910
const REGISTRY_URL = "https://registry.npmjs.org/olostep-cli/latest";
1011

@@ -79,23 +80,69 @@ export async function checkForUpdate(current: string): Promise<string | null> {
7980
return isNewer(latest, current) ? latest : null;
8081
}
8182

83+
function isSuppressed(subcommand?: string): boolean {
84+
if (subcommand === "update") return true;
85+
if (process.env.OLOSTEP_NO_UPDATE_CHECK) return true;
86+
if (process.env.OLOSTEP_NO_UPDATE_NOTICE) return true;
87+
return false;
88+
}
89+
90+
/**
91+
* Shows any pending update notice written by a previous scheduleUpdateCheck call.
92+
* Call at process start (before the command) so the notice lands before output.
93+
* Deletes the notice file after reading it. Fail-silent.
94+
*/
95+
export function showPendingUpdateNotice(current: string, invokedSubcommand?: string): void {
96+
if (isSuppressed(invokedSubcommand)) return;
97+
try {
98+
if (!process.stderr.isTTY) return;
99+
} catch { return; }
100+
try {
101+
if (!fs.existsSync(NOTICE_FILE)) return;
102+
const raw = fs.readFileSync(NOTICE_FILE, "utf8");
103+
// Delete regardless of whether we can show it.
104+
try { fs.unlinkSync(NOTICE_FILE); } catch { /* ignore */ }
105+
const data = JSON.parse(raw);
106+
const latest = (data?.latest || "").toString().trim();
107+
if (!latest || !isNewer(latest, current)) return;
108+
process.stderr.write(
109+
c.yellow(` ↑ olostep ${latest} available — run \`olostep update\`\n`),
110+
);
111+
} catch { /* fail silent */ }
112+
}
113+
114+
/**
115+
* Fires a background registry check with no await. If a newer version is found,
116+
* writes a notice file so showPendingUpdateNotice displays it on the next run.
117+
* Never blocks the command. Fail-silent.
118+
*/
119+
export function scheduleUpdateCheck(current: string, invokedSubcommand?: string): void {
120+
if (isSuppressed(invokedSubcommand)) return;
121+
checkForUpdate(current)
122+
.then((latest) => {
123+
if (latest) {
124+
try {
125+
fs.writeFileSync(NOTICE_FILE, JSON.stringify({ latest }), "utf8");
126+
} catch { /* ignore */ }
127+
}
128+
})
129+
.catch(() => { /* ignore */ });
130+
}
131+
82132
/**
83-
* Prints a one-line "update available" notice to stderr — interactive use only.
84-
* Suppressed in pipes/CI and when OLOSTEP_NO_UPDATE_NOTICE is set.
85-
* Never throws.
133+
* Legacy combined helper. Kept for tests that import it directly.
134+
* New code should use showPendingUpdateNotice + scheduleUpdateCheck.
86135
*/
87136
export async function maybeNotifyUpdate(current: string, invokedSubcommand?: string): Promise<void> {
88-
if (invokedSubcommand === "update") return;
89-
if (process.env.OLOSTEP_NO_UPDATE_CHECK) return;
90-
if (process.env.OLOSTEP_NO_UPDATE_NOTICE) return;
137+
if (isSuppressed(invokedSubcommand)) return;
91138
try {
92139
if (!process.stderr.isTTY) return;
93140
} catch { return; }
94141
try {
95142
const latest = await checkForUpdate(current);
96143
if (latest) {
97144
process.stderr.write(
98-
c.yellow(` ↑ olostep ${latest} is available (you have ${current}) — run \`olostep update\`\n`),
145+
c.yellow(` ↑ olostep ${latest} available — run \`olostep update\`\n`),
99146
);
100147
}
101148
} catch { /* fail silent */ }

0 commit comments

Comments
 (0)