Fix false update-available nags (#1819)

* Suppress false update-available nags on unstamped and unreleased-channel builds A 0.0.0 build-time fallback compared as older than any release, and a prerelease with no matching dist-tag (rc, alpha, ...) always lost the compare against latest. Both cases now short-circuit to "no update" before hitting the registry, via a shared resolveComparisonChannel / isUpdateAvailable used by both the CLI check and the sidebar card. * Add changeset for update-check false nags fix

Rhys Sullivan committed 2 months ago

Commit 1cd81d61 parent 6305b6d6 files · +269 −16Browse files

Unified · Split

6 files

.changeset/update-check-suppress-false-nags.md +14 −0
packages/core/api/src/index.ts +3 −0
packages/core/api/src/update-check.test.ts +103 −2
packages/core/api/src/update-check.ts +58 −3
packages/react/src/components/update-card.test.ts +58 −0
packages/react/src/components/update-card.tsx +33 −11

.changeset/update-check-suppress-false-nags.md+14−0

@@ -0,0 +1,14 @@
	1	+---
	2	+"executor": patch
	3	+---
	4	+
	5	+Fix: stop the update check from claiming a newer version is available on builds it cannot compare
	6	+
	7	+A build stamped with the placeholder 0.0.0 version always compared as older
	8	+than the latest release, and a prerelease on a channel with no matching
	9	+dist-tag (rc, alpha, and similar) always lost the comparison too. Both cases
	10	+now short-circuit to "no update available" before the check reaches the
	11	+registry.
	12	+
	13	+This applies wherever the update check runs, so the CLI check and the sidebar
	14	+update card both stop showing an update prompt that a user could never act on.

packages/core/api/src/index.ts+3−0

@@ -3,7 +3,10 @@ export { · 2 unmodified lines
3	3	checkForUpdate,
4	4	resolveDistTags,
5	5	resolveUpdateChannel,
	6	+ resolveComparisonChannel,
6	7	compareVersions,
	8	+ isUnstampedVersion,
	9	+ isUpdateAvailable,
7	10	EXECUTOR_PACKAGE_NAME,
8	11	type UpdateStatus,
9	12	type UpdateChannel,

packages/core/api/src/update-check.test.ts+103−2

@@ -10,6 +10,9 @@ import { · 9 unmodified lines
10	10	__resetDistTagsCache,
11	11	checkForUpdate,
12	12	compareVersions,
	13	+ isUnstampedVersion,
	14	+ isUpdateAvailable,
	15	+ resolveComparisonChannel,
13	16	resolveDistTags,
14	17	resolveUpdateChannel,
15	18	} from "./update-check";
@@ -53,6 +56,73 @@ describe("resolveUpdateChannel", () => { · 37 unmodified lines
53	56	});
54	57	});
55	58
	59	+describe("isUnstampedVersion", () => {
	60	+ it("flags 0.0.0, with or without a prerelease suffix", () => {
	61	+ expect(isUnstampedVersion("0.0.0")).toBe(true);
	62	+ expect(isUnstampedVersion("0.0.0-dev")).toBe(true);
	63	+ });
	64	+
	65	+ it("leaves any published version alone", () => {
	66	+ expect(isUnstampedVersion("1.5.22")).toBe(false);
	67	+ expect(isUnstampedVersion("0.0.1")).toBe(false);
	68	+ });
	69	+
	70	+ it("is false for unparseable input", () => {
	71	+ expect(isUnstampedVersion("not-a-version")).toBe(false);
	72	+ });
	73	+});
	74	+
	75	+describe("resolveComparisonChannel", () => {
	76	+ it("suppresses unstamped build-time fallback versions", () => {
	77	+ expect(resolveComparisonChannel("0.0.0")).toBeNull();
	78	+ expect(resolveComparisonChannel("0.0.0-dev")).toBeNull();
	79	+ });
	80	+
	81	+ it("suppresses unparseable input", () => {
	82	+ expect(resolveComparisonChannel("not-a-version")).toBeNull();
	83	+ });
	84	+
	85	+ it("routes a release version to the latest tag", () => {
	86	+ expect(resolveComparisonChannel("1.6.0")).toBe("latest");
	87	+ });
	88	+
	89	+ it("routes a beta prerelease to the beta tag", () => {
	90	+ expect(resolveComparisonChannel("1.6.0-beta.1")).toBe("beta");
	91	+ });
	92	+
	93	+ it("suppresses prereleases with no matching dist-tag", () => {
	94	+ // rc, alpha, next, dev, ... — none of these publish a dist-tag, so
	95	+ // comparing against `latest` would nag forever.
	96	+ expect(resolveComparisonChannel("1.6.0-rc.1")).toBeNull();
	97	+ expect(resolveComparisonChannel("1.6.0-alpha.1")).toBeNull();
	98	+ expect(resolveComparisonChannel("1.6.0-next.1")).toBeNull();
	99	+ });
	100	+});
	101	+
	102	+describe("isUpdateAvailable", () => {
	103	+ it("is false with no current version", () => {
	104	+ expect(isUpdateAvailable(undefined, "1.6.0")).toBe(false);
	105	+ });
	106	+
	107	+ it("is false with no comparison channel", () => {
	108	+ expect(isUpdateAvailable("0.0.0-dev", "1.6.0")).toBe(false);
	109	+ expect(isUpdateAvailable("1.6.0-rc.1", "1.6.0")).toBe(false);
	110	+ });
	111	+
	112	+ it("is false with no published tag", () => {
	113	+ expect(isUpdateAvailable("1.5.22", null)).toBe(false);
	114	+ });
	115	+
	116	+ it("is false when already current", () => {
	117	+ expect(isUpdateAvailable("1.6.0", "1.6.0")).toBe(false);
	118	+ });
	119	+
	120	+ it("is true when a newer version is published on the matching channel", () => {
	121	+ expect(isUpdateAvailable("1.6.0", "1.6.1")).toBe(true);
	122	+ expect(isUpdateAvailable("1.6.0-beta.1", "1.6.0-beta.2")).toBe(true);
	123	+ });
	124	+});
	125	+
56	126	describe("resolveDistTags", () => {
57	127	it("returns nothing when the check is disabled", async () => {
58	128	const tags = await resolveDistTags({
@@ -126,6 +196,14 @@ describe("checkForUpdate", () => { · 67 unmodified lines
126	196	expect(status.updateAvailable).toBe(false);
127	197	});
128	198
	199	+ it("flags a patch release on the latest channel", async () => {
	200	+ const status = await checkForUpdate("1.6.0", {
	201	+ env: { EXECUTOR_NPM_DIST_TAGS: JSON.stringify({ latest: "1.6.1" }) },
	202	+ });
	203	+ expect(status.updateAvailable).toBe(true);
	204	+ expect(status.latestVersion).toBe("1.6.1");
	205	+ });
	206	+
129	207	it("compares a beta build against the beta tag", async () => {
130	208	const status = await checkForUpdate("1.6.0-beta.1", {
131	209	env: { EXECUTOR_NPM_DIST_TAGS: JSON.stringify({ latest: "1.5.22", beta: "1.6.0-beta.2" }) },
@@ -136,10 +214,33 @@ describe("checkForUpdate", () => { · 4 unmodified lines
136	214	expect(status.command).toBe("npm i -g executor@beta");
137	215	});
138	216
139		− it("treats the dev build as upgradeable to any release", async () => {
	217	+ it("stays quiet on an unstamped dev build", async () => {
	218	+ // 0.0.0-dev is the build-time fallback, not a real release. It must never
	219	+ // claim an update is available, and — because there is no dist-tag it
	220	+ // could legitimately compare against — must not even hit the registry.
140	221	const status = await checkForUpdate("0.0.0-dev", {
141	222	env: { EXECUTOR_FORCE_LATEST_VERSION: "1.5.22" },
	223	+ fetchImpl: fetchThatFails(),
142	224	});
143		− expect(status.updateAvailable).toBe(true);
	225	+ expect(status.updateAvailable).toBe(false);
	226	+ expect(status.latestVersion).toBeNull();
	227	+ });
	228	+
	229	+ it("stays quiet on the desktop's unstamped fallback (plain 0.0.0)", async () => {
	230	+ const status = await checkForUpdate("0.0.0", {
	231	+ env: { EXECUTOR_FORCE_LATEST_VERSION: "1.5.22" },
	232	+ fetchImpl: fetchThatFails(),
	233	+ });
	234	+ expect(status.updateAvailable).toBe(false);
	235	+ expect(status.latestVersion).toBeNull();
	236	+ });
	237	+
	238	+ it("stays quiet on a prerelease channel with no matching dist-tag", async () => {
	239	+ const status = await checkForUpdate("1.6.0-rc.1", {
	240	+ env: { EXECUTOR_NPM_DIST_TAGS: JSON.stringify({ latest: "1.6.0" }) },
	241	+ fetchImpl: fetchThatFails(),
	242	+ });
	243	+ expect(status.updateAvailable).toBe(false);
	244	+ expect(status.latestVersion).toBeNull();
144	245	});
145	246	});

packages/core/api/src/update-check.ts+58−3

@@ -102,6 +102,53 @@ export const compareVersions = (left: string, right: string): number \| null => { · 101 unmodified lines
102	102	return comparePrereleaseIdentifiers(lv.prerelease, rv.prerelease);
103	103	};
104	104
	105	+/**
	106	+ * True when `version` parses as `0.0.0`, with or without a prerelease suffix
	107	+ * (e.g. `0.0.0-dev`). `0.0.0` is never a published release — it is the
	108	+ * build-time fallback baked in when the real version was unavailable at
	109	+ * build time — so a build carrying it must never claim an update.
	110	+ */
	111	+export const isUnstampedVersion = (version: string): boolean => {
	112	+ const parsed = parseVersion(version);
	113	+ return parsed !== null && parsed.major === 0 && parsed.minor === 0 && parsed.patch === 0;
	114	+};
	115	+
	116	+/**
	117	+ * The dist-tag channel `version` can legitimately be compared against, or
	118	+ * `null` when there is none (the check should be suppressed, not run):
	119	+ * - an unstamped build (0.0.0*) never shipped, so there is nothing to
	120	+ * compare it against
	121	+ * - unparseable input has no meaningful channel
	122	+ * - no prerelease -> the `latest` tag
	123	+ * - a prerelease whose first identifier is `beta` -> the `beta` tag
	124	+ * - any other prerelease (rc, alpha, next, dev, ...) has no matching
	125	+ * dist-tag, so suppress rather than nag forever against a channel it can
	126	+ * never catch up to
	127	+ */
	128	+export const resolveComparisonChannel = (version: string): UpdateChannel \| null => {
	129	+ if (isUnstampedVersion(version)) return null;
	130	+ const parsed = parseVersion(version);
	131	+ if (!parsed) return null;
	132	+ if (parsed.prerelease === null) return "latest";
	133	+ return parsed.prerelease[0] === "beta" ? "beta" : null;
	134	+};
	135	+
	136	+/**
	137	+ * The single "should we nag?" verdict both the CLI and the web UpdateCard
	138	+ * read. False whenever there is nothing to compare: no current version, no
	139	+ * dist-tag channel this version could match (see `resolveComparisonChannel`),
	140	+ * or no published tag for that channel.
	141	+ */
	142	+export const isUpdateAvailable = (
	143	+ currentVersion: string \| undefined,
	144	+ latestVersion: string \| null,
	145	+): boolean => {
	146	+ if (currentVersion === undefined) return false;
	147	+ if (resolveComparisonChannel(currentVersion) === null) return false;
	148	+ if (latestVersion === null) return false;
	149	+ return compareVersions(currentVersion, latestVersion) === -1;
	150	+};
	151	+
105	152	// ── dist-tags resolution ──────────────────────────────────────────────────
106	153
107	154	export type DistTags = Partial<Record<UpdateChannel, string>>;
@@ -223,9 +270,17 @@ export const checkForUpdate = async ( · 115 unmodified lines
223	270	): Promise<UpdateStatus> => {
224	271	const channel = resolveUpdateChannel(currentVersion);
225	272	const command = `npm i -g ${EXECUTOR_PACKAGE_NAME}@${channel}`;
	273	+
	274	+ // No dist-tag can legitimately be compared against this version (unstamped
	275	+ // dev build, or a prerelease channel that never gets published) — do not
	276	+ // even hit the registry, since the answer is already known.
	277	+ const comparisonChannel = resolveComparisonChannel(currentVersion);
	278	+ if (comparisonChannel === null) {
	279	+ return { updateAvailable: false, currentVersion, latestVersion: null, channel, command };
	280	+ }
	281	+
226	282	const tags = await resolveDistTags(options);
227		− const latestVersion = tags[channel] ?? null;
228		− const updateAvailable =
229		− latestVersion !== null && compareVersions(currentVersion, latestVersion) === -1;
	283	+ const latestVersion = tags[comparisonChannel] ?? null;
	284	+ const updateAvailable = isUpdateAvailable(currentVersion, latestVersion);
230	285	return { updateAvailable, currentVersion, latestVersion, channel, command };
231	286	};

packages/react/src/components/update-card.test.ts+58−0

@@ -0,0 +1,58 @@
	1	+import { describe, expect, it } from "@effect/vitest";
	2	+
	3	+import { isUpdateAvailable } from "@executor-js/api";
	4	+
	5	+import { updateFetchChannel } from "./update-card";
	6	+
	7	+/**
	8	+ * `updateFetchChannel` is what lets the hook skip the network call for a
	9	+ * build that can never legitimately claim an update — the dev build's
	10	+ * unstamped fallback, or a prerelease channel with no published dist-tag.
	11	+ * Pinning it here is what keeps that decision testable without rendering.
	12	+ */
	13	+describe("updateFetchChannel", () => {
	14	+ it("fetches nothing for the unstamped dev build", () => {
	15	+ expect(updateFetchChannel("0.0.0-dev")).toBeNull();
	16	+ });
	17	+
	18	+ it("fetches nothing for the desktop's plain unstamped fallback", () => {
	19	+ expect(updateFetchChannel("0.0.0")).toBeNull();
	20	+ });
	21	+
	22	+ it("fetches nothing when there is no version yet", () => {
	23	+ expect(updateFetchChannel(undefined)).toBeNull();
	24	+ });
	25	+
	26	+ it("fetches the latest tag for a release build", () => {
	27	+ expect(updateFetchChannel("1.6.0")).toBe("latest");
	28	+ });
	29	+
	30	+ it("fetches the beta tag for a beta prerelease", () => {
	31	+ expect(updateFetchChannel("1.6.0-beta.2")).toBe("beta");
	32	+ });
	33	+
	34	+ it("fetches nothing for a prerelease channel with no matching dist-tag", () => {
	35	+ expect(updateFetchChannel("1.6.0-rc.1")).toBeNull();
	36	+ });
	37	+});
	38	+
	39	+/**
	40	+ * The card's verdict is the shared `isUpdateAvailable`, so the fetch decision
	41	+ * above and the "should we show the card?" decision can never disagree.
	42	+ */
	43	+describe("isUpdateAvailable through the card's fetch channel", () => {
	44	+ it("stays quiet on the unstamped dev build even if a tag comes back", () => {
	45	+ expect(updateFetchChannel("0.0.0-dev")).toBeNull();
	46	+ expect(isUpdateAvailable("0.0.0-dev", "1.6.0")).toBe(false);
	47	+ });
	48	+
	49	+ it("flags a real update on the latest channel", () => {
	50	+ expect(updateFetchChannel("1.6.0")).toBe("latest");
	51	+ expect(isUpdateAvailable("1.6.0", "1.6.1")).toBe(true);
	52	+ });
	53	+
	54	+ it("flags a real update on the beta channel", () => {
	55	+ expect(updateFetchChannel("1.6.0-beta.1")).toBe("beta");
	56	+ expect(isUpdateAvailable("1.6.0-beta.1", "1.6.0-beta.2")).toBe(true);
	57	+ });
	58	+});

packages/react/src/components/update-card.tsx+33−11

@@ -13,11 +13,17 @@ · 12 unmodified lines
13	13	// - managed cloud (`"managed"`): nothing, it deploys itself.
14	14	//
15	15	// The "is a newer version published?" verdict comes from the same resolver as
16		−// the CLI notice (@executor-js/api) so the two can never disagree.
	16	+// the CLI notice (@executor-js/api) — `isUpdateAvailable` — so the two can
	17	+// never disagree.
17	18	import { useCallback, useEffect, useState } from "react";
18	19
19	20	import { Effect, Exit } from "effect";
20		−import { compareVersions, resolveUpdateChannel, type UpdateChannel } from "@executor-js/api";
	21	+import {
	22	+ isUpdateAvailable,
	23	+ resolveComparisonChannel,
	24	+ resolveUpdateChannel,
	25	+ type UpdateChannel,
	26	+} from "@executor-js/api";
21	27
22	28	import { Button } from "./button";
23	29	import { toast } from "./sonner";
@@ -47,12 +53,31 @@ const UPGRADE_DOCS_URL: Partial<Record<UpgradeHint, string>> = { · 23 unmodified lines
47	53
48	54	// ── useLatestVersion ────────────────────────────────────────────────────
49	55
	56	+/**
	57	+ * The dist-tag channel to fetch for `currentVersion`, or `null` when there is
	58	+ * nothing to check — no version baked in yet, or a build the check should
	59	+ * stay quiet on (an unstamped `0.0.0*` fallback, or a prerelease channel with
	60	+ * no published dist-tag; see `resolveComparisonChannel`). Returning `null`
	61	+ * here is what lets the hook skip the network call entirely, so a dev build
	62	+ * never even hits `/v1/app/npm/dist-tags`.
	63	+ */
	64	+export function updateFetchChannel(currentVersion: string \| undefined): UpdateChannel \| null {
	65	+ if (currentVersion === undefined) return null;
	66	+ return resolveComparisonChannel(currentVersion);
	67	+}
	68	+
50	69	function useLatestVersion(currentVersion: string \| undefined) {
51		− const channel: UpdateChannel = currentVersion ? resolveUpdateChannel(currentVersion) : "latest";
	70	+ // The channel shown in the upgrade command differs from the channel
	71	+ // fetched for comparison: the command always names a real channel, while
	72	+ // the fetch is skipped entirely for a version with nothing to compare.
	73	+ const displayChannel: UpdateChannel = currentVersion
	74	+ ? resolveUpdateChannel(currentVersion)
	75	+ : "latest";
	76	+ const fetchChannel = updateFetchChannel(currentVersion);
52	77	const [latestVersion, setLatestVersion] = useState<string \| null>(null);
53	78
54	79	useEffect(() => {
55		− if (!currentVersion) return;
	80	+ if (fetchChannel === null) return;
56	81	let cancelled = false;
57	82	void Effect.runPromiseExit(
58	83	Effect.tryPromise({
@@ -65,20 +90,17 @@ function useLatestVersion(currentVersion: string \| undefined) { · 6 unmodified lines
65	90	}),
66	91	).then((exit) => {
67	92	if (!cancelled && Exit.isSuccess(exit)) {
68		− setLatestVersion(exit.value?.[channel] ?? null);
	93	+ setLatestVersion(exit.value?.[fetchChannel] ?? null);
69	94	}
70	95	});
71	96	return () => {
72	97	cancelled = true;
73	98	};
74		− }, [channel, currentVersion]);
	99	+ }, [fetchChannel]);
75	100
76		− const updateAvailable =
77		− currentVersion !== undefined &&
78		− latestVersion !== null &&
79		− compareVersions(currentVersion, latestVersion) === -1;
	101	+ const updateAvailable = isUpdateAvailable(currentVersion, latestVersion);
80	102
81		− return { latestVersion, updateAvailable, channel };
	103	+ return { latestVersion, updateAvailable, channel: displayChannel };
82	104	}
83	105
84	106	// ── Card chrome ──────────────────────────────────────────────────────────