fix(cli): #1295 follow-ups — semver ordering, manual-update policy, telemetry

packages/cli/src/commands/maintenance.ts

@@ -37,6 +37,9 @@ import { batchEntityQuads } from '../batching.js';
 import {
   runDaemon,
   checkForNpmVersionUpdate,
+  resolveLatestNpmVersion,
+  isValidSemver,
+  isPrerelease,
   performNpmUpdate,
   performNpmUpdateEdge,
   getCurrentCliVersion,
@@ -103,6 +106,51 @@ import {
   runForegroundSupervisor,
 } from '../cli-supervisor.js';
 
+/**
+ * Decide whether an EXPLICIT `dkg update <target>` is allowed under the node's
+ * stable-only policy. Resolves a dist-tag to its concrete version first, then:
+ *   - allowPre (config or --allow-prerelease) → always ok
+ *   - prerelease target under stable-only → refuse
+ *   - dist-tag that can't be resolved (registry error) → refuse (fail CLOSED,
+ *     matching the no-arg path; otherwise a transient blip could let a
+ *     stable-only node install an RC that `latest` happens to point at)
+ * Extracted + dep-injected so it is unit-testable. Returns ok, or a reason.
+ */
+export async function resolveExplicitUpdateTarget(
+  target: string,
+  allowPre: boolean,
+  deps: {
+    resolveLatestNpmVersion: (
+      log: (m: string) => void,
+      allowPrerelease: boolean,
+      channel?: string,
+    ) => Promise<{ version: string | null; error?: boolean }>;
+    log: (m: string) => void;
+  },
+): Promise<{ ok: true } | { ok: false; reason: string }> {
+  if (allowPre) return { ok: true };
+  let resolved = target;
+  if (!isValidSemver(resolved)) {
+    // Non-semver target → treat as a dist-tag and resolve it (allowPrerelease=true
+    // so a prerelease target is visible to be rejected).
+    const r = await deps.resolveLatestNpmVersion(deps.log, true, resolved);
+    if (r.error) {
+      return {
+        ok: false,
+        reason: `could not resolve dist-tag "${target}" against the npm registry (transient error). On a stable-only node a dist-tag must be confirmed non-prerelease before install — retry, pass an explicit version, or use --allow-prerelease`,
+      };
+    }
+    if (r.version) resolved = r.version;
+  }
+  if (isValidSemver(resolved) && isPrerelease(resolved)) {
+    return {
+      ok: false,
+      reason: `target "${resolved}" is a pre-release and this node has allowPrerelease=false — re-run with --allow-prerelease to override`,
+    };
+  }
+  return { ok: true };
+}
+
 export function registerMaintenanceCommands(program: Command): void {
 // ─── dkg update ──────────────────────────────────────────────────────
 //
@@ -197,6 +245,19 @@ program
       if (version) {
         version = version.replace(/^refs\/tags\/v?/, '').replace(/^v/, '');
       }
+      // An EXPLICIT target (version or dist-tag) must still honour the
+      // stable-only policy: `dkg update 10.1.0-rc.1` or `dkg update latest`
+      // (when latest points at an RC) must NOT bypass allowPrerelease:false.
+      if (version) {
+        const gate = await resolveExplicitUpdateTarget(version, allowPre, {
+          resolveLatestNpmVersion,
+          log: logFn,
+        });
+        if (!gate.ok) {
+          console.error(`[dkg update] Refusing to update: ${gate.reason}.`);
+          process.exit(1);
+        }
+      }
       if (!version) {
         console.log('Checking NPM registry for updates...');
         const check = await checkForNpmVersionUpdate(logFn, allowPre, au.channel);

packages/cli/src/daemon/auto-update.ts

@@ -372,9 +372,14 @@ export async function resolveLatestNpmVersion(
       return { version: null, error: false };
     }
 
-    const candidates = [stable, tags.dev, tags.beta, tags.next].filter(
+    // Filter to VALID semver BEFORE sorting: a single malformed tag (e.g.
+    // latest="garbage") must not sort ahead of and mask a valid candidate
+    // (e.g. beta="9.0.0-beta.4") — compareSemver on garbage returns NaN, which
+    // makes the sort order undefined. Filtering invalid values never changes
+    // selection among valid ones.
+    const candidates = ([stable, tags.dev, tags.beta, tags.next].filter(
       Boolean,
-    ) as string[];
+    ) as string[]).filter(isValidSemver);
     if (candidates.length === 0) return { version: null, error: false };
     candidates.sort((a, b) => compareSemver(b, a));
     return { version: candidates[0] };
@@ -387,19 +392,23 @@ export async function resolveLatestNpmVersion(
 }
 
 export function compareSemver(a: string, b: string): number {
-  const pa = a.replace(/^v/, "").split(/[-+]/)[0].split(".").map(Number);
-  const pb = b.replace(/^v/, "").split(/[-+]/)[0].split(".").map(Number);
+  // Build metadata (everything from `+`) is ignored for precedence (semver §10),
+  // so strip it FIRST. Detecting the prerelease via the raw string's `-` is
+  // wrong: `10.0.0+mainnet-build.1` is a STABLE version whose hyphen lives in
+  // build metadata — treating it as a prerelease made it sort BELOW
+  // `10.0.0-rc.19` and read as "not newer".
+  const core = (v: string) => v.replace(/^v/, "").split("+")[0];
+  const ca = core(a);
+  const cb = core(b);
+  const pa = ca.split("-")[0].split(".").map(Number);
+  const pb = cb.split("-")[0].split(".").map(Number);
   for (let i = 0; i < 3; i++) {
     if ((pa[i] ?? 0) !== (pb[i] ?? 0)) return (pa[i] ?? 0) - (pb[i] ?? 0);
   }
-  const stripBuild = (s: string) => s.replace(/\+.*$/, "");
-  const preA = a.includes("-")
-    ? stripBuild(a.split("-").slice(1).join("-"))
-    : "";
-  const preB = b.includes("-")
-    ? stripBuild(b.split("-").slice(1).join("-"))
-    : "";
-  if (!preA && preB) return 1;
+  const prerelease = (v: string) => (v.includes("-") ? v.slice(v.indexOf("-") + 1) : "");
+  const preA = prerelease(ca);
+  const preB = prerelease(cb);
+  if (!preA && preB) return 1; // stable outranks its own prerelease
   if (preA && !preB) return -1;
   return preA.localeCompare(preB, undefined, { numeric: true });
 }

packages/cli/src/daemon/lifecycle.ts

@@ -2155,6 +2155,10 @@ export async function runDaemonInner(
         if (!autoUpdateEnabled) return "disabled";
         if (daemonState.isUpdating) return "updating";
         if (daemonState.lastUpdateCheck.checkedAt === 0) return "unknown";
+        // A pinned channel with no acceptable target is NOT healthy "latest" —
+        // surface it distinctly so central monitoring can flag an unpublished /
+        // rejected channel instead of seeing a falsely-current node.
+        if (daemonState.lastUpdateCheck.channelTargetMissing) return "channel-missing";
         return daemonState.lastUpdateCheck.upToDate ? "latest" : "behind";
       },
     });

packages/cli/test/auto-update.test.ts

@@ -1174,6 +1174,27 @@ describe('checkForNpmVersionUpdate tag precedence', () => {
     const result = await checkForNpmVersionUpdate(() => {}, true);
     expect(result.status).toBe('up-to-date'); // not 'available' on garbage
   });
+
+  it('default path: a garbage `latest` must NOT mask a valid `beta` candidate', async () => {
+    const { checkForNpmVersionUpdate } = await import('../src/daemon.js');
+    fetchImpl = async () => makeRegistryResponse({ latest: 'garbage', beta: '9.0.0-beta.4' });
+    const result = await checkForNpmVersionUpdate(() => {}, true);
+    // current is 9.0.0-beta.3 → the valid beta.4 must be selected, not dropped.
+    expect(result.status).toBe('available');
+    expect(result.version).toBe('9.0.0-beta.4');
+  });
+
+  it('channel mainnet: a STABLE +build target is "available" over the current rc (not falsely up-to-date)', async () => {
+    const { checkForNpmVersionUpdate } = await import('../src/daemon.js');
+    readFileImpl = async (p: any) => {
+      if (String(p).endsWith('.current-version')) return '10.0.0-rc.19';
+      throw new Error('ENOENT');
+    };
+    fetchImpl = async () => makeRegistryResponse({ mainnet: '10.0.0+mainnet-build.1' });
+    const result = await checkForNpmVersionUpdate(() => {}, false, 'mainnet');
+    expect(result.status).toBe('available');
+    expect(result.version).toBe('10.0.0+mainnet-build.1');
+  });
 });
 
 describe('deriveUpdateCheckState (runCheck → /api/status mapping)', () => {
@@ -1617,6 +1638,14 @@ describe('compareSemver', () => {
     expect(compareSemver('9.0.0+build.1', '9.0.0+build.2')).toBe(0);
   });
 
+  it('treats HYPHENATED build metadata as stable, not prerelease (PR #1295 regression)', () => {
+    // The hyphen lives in +build metadata → 10.0.0+mainnet-build.1 is a STABLE
+    // 10.0.0 and must outrank 10.0.0-rc.19, not read as "not newer".
+    expect(compareSemver('10.0.0+mainnet-build.1', '10.0.0-rc.19')).toBeGreaterThan(0);
+    expect(compareSemver('10.0.0-rc.19', '10.0.0+mainnet-build.1')).toBeLessThan(0);
+    expect(compareSemver('10.0.0+mainnet-build.1', '10.0.0')).toBe(0);
+  });
+
   it('handles dev suffix with numeric comparison', () => {
     expect(compareSemver('9.0.0-beta.4-dev.200', '9.0.0-beta.4-dev.100')).toBeGreaterThan(0);
   });

packages/cli/test/maintenance-update-gate.test.ts

@@ -0,0 +1,57 @@
+import { describe, it, expect, vi } from 'vitest';
+import { resolveExplicitUpdateTarget } from '../src/commands/maintenance.js';
+
+// Guards the `dkg update <target>` stable-only gate: an explicit prerelease /
+// dist-tag-pointing-at-a-prerelease must not bypass allowPrerelease:false, and a
+// transient dist-tag resolution error must fail CLOSED (PR #1295 follow-up).
+describe('resolveExplicitUpdateTarget', () => {
+  const log = () => {};
+  const resolverReturning = (ret: { version: string | null; error?: boolean }) =>
+    vi.fn(async () => ret);
+
+  it('allowPrerelease=true short-circuits — any target allowed, no registry hit', async () => {
+    const resolve = resolverReturning({ version: null });
+    expect(
+      await resolveExplicitUpdateTarget('10.1.0-rc.1', true, { resolveLatestNpmVersion: resolve, log }),
+    ).toEqual({ ok: true });
+    expect(resolve).not.toHaveBeenCalled();
+  });
+
+  it('stable explicit version on a stable-only node → allowed', async () => {
+    const resolve = resolverReturning({ version: null });
+    expect(
+      await resolveExplicitUpdateTarget('10.1.0', false, { resolveLatestNpmVersion: resolve, log }),
+    ).toEqual({ ok: true });
+  });
+
+  it('prerelease explicit version on a stable-only node → refused', async () => {
+    const resolve = resolverReturning({ version: null });
+    const r = await resolveExplicitUpdateTarget('10.1.0-rc.1', false, { resolveLatestNpmVersion: resolve, log });
+    expect(r.ok).toBe(false);
+  });
+
+  it('dist-tag resolving to a STABLE version → allowed (resolved with allowPrerelease=true)', async () => {
+    const resolve = resolverReturning({ version: '10.0.0' });
+    const r = await resolveExplicitUpdateTarget('latest', false, { resolveLatestNpmVersion: resolve, log });
+    expect(r).toEqual({ ok: true });
+    expect(resolve).toHaveBeenCalledWith(log, true, 'latest');
+  });
+
+  it('dist-tag resolving to a PRERELEASE → refused', async () => {
+    const resolve = resolverReturning({ version: '10.1.0-rc.1' });
+    const r = await resolveExplicitUpdateTarget('latest', false, { resolveLatestNpmVersion: resolve, log });
+    expect(r.ok).toBe(false);
+  });
+
+  it('dist-tag resolution ERROR → refused (fail CLOSED, like the no-arg path)', async () => {
+    const resolve = resolverReturning({ version: null, error: true });
+    const r = await resolveExplicitUpdateTarget('latest', false, { resolveLatestNpmVersion: resolve, log });
+    expect(r.ok).toBe(false);
+  });
+
+  it('unknown dist-tag (no version, no error) → allowed to fall through (npm install fails naturally)', async () => {
+    const resolve = resolverReturning({ version: null });
+    const r = await resolveExplicitUpdateTarget('nonexistent-tag', false, { resolveLatestNpmVersion: resolve, log });
+    expect(r).toEqual({ ok: true });
+  });
+});