fix(audit): correct semverCompare + drift/probe robustness + scaffold finalize safety

src/audit-cd.ts

@@ -1,7 +1,7 @@
 import { execFileSync } from "node:child_process";
 import { existsSync, statSync } from "node:fs";
 import { isAbsolute, resolve } from "node:path";
-import { parseGitHubRemote, semverCompare, tryGit } from "./audit-helpers.js";
+import { extractSemver, parseGitHubRemote, semverCompare, tryGit } from "./audit-helpers.js";
 import { extractStarterSignals } from "./starter-detect.js";
 
 export type DestinationName =
@@ -55,6 +55,65 @@ function classifyDrift(local: string | null, published: string | null): CdStatus
   return "local-stale";
 }
 
+/**
+ * Like {@link classifyDrift} but for a published *git tag* that may carry a
+ * monorepo / scoped prefix (`@scope/x@1.2.3`, `release-1.2.3`). Extract the
+ * trailing SemVer before comparing; if the tag has no extractable SemVer we
+ * can't meaningfully compare, so we treat it as in-sync (no phantom drift)
+ * rather than fabricating a stale/needs-publish verdict from prefix noise.
+ */
+function classifyDriftTag(local: string | null, tag: string | null): CdStatus {
+  if (!local || !tag) return "not-found";
+  const publishedVer = extractSemver(tag);
+  if (publishedVer === null) return "in-sync";
+  return classifyDrift(local, publishedVer);
+}
+
+/**
+ * Template placeholder identifiers that ship inside the Starter Series
+ * scaffolds. A repo still carrying one of these has not been configured for a
+ * real destination yet — reporting `not-found` / `needs-publish` against the
+ * literal placeholder is misleading. We surface a distinct `unsupported`
+ * ("template not configured") status instead so the auditor nudges the user to
+ * set a real name rather than to "publish my-mcp-server".
+ */
+const TEMPLATE_PLACEHOLDERS = new Set([
+  "my-mcp-server",
+  "my-package",
+  "my-discord-bot",
+  "my-telegram-bot",
+  "my-extension",
+  "my-vscode-extension",
+  "my-electron-app",
+  "my-app",
+  "my-site",
+  "my-service",
+]);
+
+function isPlaceholderIdentifier(id: string): boolean {
+  const lower = id.toLowerCase();
+  // Bare or scoped placeholder package names (e.g. "my-mcp-server",
+  // "@you/my-mcp-server"), and the AMO template gecko id.
+  const bare = lower.includes("/") ? lower.slice(lower.lastIndexOf("/") + 1) : lower;
+  if (TEMPLATE_PLACEHOLDERS.has(bare)) return true;
+  // AMO template id from the browser-extension starter manifest.
+  if (lower.includes("{your-extension-id}") || lower === "{your-extension-id}@example.com") {
+    return true;
+  }
+  return false;
+}
+
+function placeholderReport(name: DestinationName, identifier: string): DestinationReport {
+  return {
+    name,
+    identifier,
+    publishedVersion: null,
+    publishedAt: null,
+    status: "unsupported",
+    detail: "template not configured — set a real package/extension identifier before publishing",
+  };
+}
+
 // ---- fetch with timeout ----
 
 async function fetchJson(
@@ -83,12 +142,22 @@ async function fetchJson(
 
 // ---- destination probes ----
 
+/** Pick the highest SemVer key from an npm `versions` map (or any tag map). */
+function highestVersion(versions: string[]): string | null {
+  let best: string | null = null;
+  for (const v of versions) {
+    if (best === null || semverCompare(v, best) > 0) best = v;
+  }
+  return best;
+}
+
 async function probeNpm(
   pkgName: string,
   localVersion: string | null,
   f: typeof fetch,
   timeout: number,
 ): Promise<DestinationReport> {
+  if (isPlaceholderIdentifier(pkgName)) return placeholderReport("npm", pkgName);
   const url = `https://registry.npmjs.org/${encodeURIComponent(pkgName)}`;
   const res = await fetchJson(url, f, timeout);
   if (!res.ok) {
@@ -114,13 +183,46 @@ async function probeNpm(
   const data = res.data as Record<string, unknown>;
   const distTags = (data["dist-tags"] ?? {}) as Record<string, string>;
   const time = (data.time ?? {}) as Record<string, string>;
-  const latest = distTags.latest ?? null;
+  const versionsMap = (data.versions ?? {}) as Record<string, unknown>;
+
+  // Prefer dist-tags.latest. A package published only under prerelease tags
+  // (e.g. `next`, `beta`) has NO `dist-tags.latest` even though it IS
+  // published — falling through to "not-found" there is wrong (the old bug).
+  // Fall back to the highest version across all dist-tags, then the highest
+  // key in the full `versions` map, so a prerelease-only package reports its
+  // real highest published version instead of a phantom 404.
+  let published: string | null = distTags.latest ?? null;
+  let detail: string | undefined;
+  if (!published) {
+    const tagVersions = Object.values(distTags).filter((v): v is string => typeof v === "string");
+    const versionKeys = Object.keys(versionsMap);
+    published = highestVersion(tagVersions) ?? highestVersion(versionKeys);
+    if (published) {
+      const tagNames = Object.keys(distTags).join(", ") || "none";
+      detail = `no dist-tags.latest (published only as: ${tagNames}); comparing against highest published version ${published}`;
+    }
+  }
+
+  if (!published) {
+    // Reachable on the registry but with zero versions — genuinely nothing
+    // published (e.g. an unpublished/security-holding placeholder doc).
+    return {
+      name: "npm",
+      identifier: pkgName,
+      publishedVersion: null,
+      publishedAt: null,
+      status: "not-found",
+      detail: "package exists on npm but has no published versions",
+    };
+  }
+
   return {
     name: "npm",
     identifier: pkgName,
-    publishedVersion: latest,
-    publishedAt: latest && time[latest] ? time[latest] : null,
-    status: classifyDrift(localVersion, latest),
+    publishedVersion: published,
+    publishedAt: published && time[published] ? time[published] : null,
+    status: classifyDrift(localVersion, published),
+    detail,
   };
 }
 
@@ -292,6 +394,7 @@ async function probeAmo(
   f: typeof fetch,
   timeout: number,
 ): Promise<DestinationReport> {
+  if (isPlaceholderIdentifier(geckoId)) return placeholderReport("amo", geckoId);
   const url = `https://addons.mozilla.org/api/v5/addons/addon/${encodeURIComponent(geckoId)}/`;
   const res = await fetchJson(url, f, timeout);
   if (!res.ok) {
@@ -347,7 +450,10 @@ function probeGithubReleases(
       identifier: id,
       publishedVersion: tag,
       publishedAt: data.publishedAt ?? null,
-      status: classifyDrift(localVersion, tag),
+      // Tags may be monorepo/scoped (`@scope/x@1.2.3`, `release-1.2.3`); extract
+      // the trailing SemVer before comparing so a prefixed tag at the same
+      // version doesn't read as phantom drift.
+      status: classifyDriftTag(localVersion, tag),
     };
   } catch (e) {
     type ExecErr = Error & { stderr?: Buffer | string; stdout?: Buffer | string };
@@ -414,22 +520,56 @@ export async function auditCd(
   const destinations: DestinationReport[] = [];
   const probes: Promise<DestinationReport>[] = [];
 
+  // Wrap every probe so a single malformed payload can't abort the whole
+  // multi-destination audit. A probe that throws synchronously (e.g. a version
+  // returned as a JSON number, so `.replace` throws inside the probe) or
+  // rejects is converted into a per-destination `{status:'error'}` report; the
+  // other destinations still resolve. `name`/`identifier` are captured here so
+  // the error report is still attributable even though the probe never got far
+  // enough to build its own.
+  const safeProbe = (
+    name: DestinationName,
+    identifier: string,
+    run: () => Promise<DestinationReport>,
+  ): Promise<DestinationReport> =>
+    run().catch((e: unknown) => ({
+      name,
+      identifier,
+      publishedVersion: null,
+      publishedAt: null,
+      status: "error" as const,
+      detail: `probe crashed: ${(e as Error)?.message ?? String(e)}`,
+    }));
+
   // Per-starter destination plan
   switch (sig.id) {
     case "mcp-server":
     case "npm-package":
-      if (sig.npmName) probes.push(probeNpm(sig.npmName, sig.localVersion, f, timeout));
+      if (sig.npmName) {
+        probes.push(
+          safeProbe("npm", sig.npmName, () => probeNpm(sig.npmName!, sig.localVersion, f, timeout)),
+        );
+      }
       break;
     case "mcp-server-python":
-      if (sig.pyName) probes.push(probePyPI(sig.pyName, sig.localVersion, f, timeout));
+      if (sig.pyName) {
+        probes.push(
+          safeProbe("pypi", sig.pyName, () => probePyPI(sig.pyName!, sig.localVersion, f, timeout)),
+        );
+      }
       break;
     case "vscode-extension":
       if (sig.vscodePublisher && sig.vscodeName) {
+        const ovId = `${sig.vscodePublisher}.${sig.vscodeName}`;
         probes.push(
-          probeOpenVsx(sig.vscodePublisher, sig.vscodeName, sig.localVersion, f, timeout),
+          safeProbe("open-vsx", ovId, () =>
+            probeOpenVsx(sig.vscodePublisher!, sig.vscodeName!, sig.localVersion, f, timeout),
+          ),
         );
         probes.push(
-          probeVsMarketplace(sig.vscodePublisher, sig.vscodeName, sig.localVersion, f, timeout),
+          safeProbe("vs-marketplace", ovId, () =>
+            probeVsMarketplace(sig.vscodePublisher!, sig.vscodeName!, sig.localVersion, f, timeout),
+          ),
         );
       } else {
         destinations.push({
@@ -444,7 +584,9 @@ export async function auditCd(
       break;
     case "browser-extension":
       if (sig.geckoId) {
-        probes.push(probeAmo(sig.geckoId, sig.localVersion, f, timeout));
+        probes.push(
+          safeProbe("amo", sig.geckoId, () => probeAmo(sig.geckoId!, sig.localVersion, f, timeout)),
+        );
       } else {
         destinations.push({
           name: "amo",

src/audit-helpers.ts

@@ -33,17 +33,115 @@ export function tryGit(repoPath: string, args: string[]): string | null {
 }
 
 /**
- * Compare two semver-ish strings. Returns -1 if a < b, 1 if a > b, 0 if equal.
- * Tolerant: strips leading `v`, splits on `.`/`-`/`+`, falls back to string
- * compare for non-numeric segments. Good enough for "is local ahead of tag?"
- * checks; not a full semver parser.
+ * Split a semver string into its three precedence-bearing parts per SemVer 2.0:
+ *   <major.minor.patch>[-prerelease][+build]
+ * Build metadata (everything after the first `+`) is dropped entirely — SemVer
+ * §10: "Build metadata MUST be ignored when determining version precedence."
+ * The leading `v` (common in git tags) is tolerated.
+ *
+ * Returns null if the core (major.minor.patch) is not three dot-separated
+ * numeric identifiers, so callers can treat unparseable strings explicitly
+ * instead of getting a bogus ordering.
+ */
+function parseSemver(
+  s: string,
+): { core: number[]; prerelease: string[] } | null {
+  // Strip build metadata (§10) first, then a single leading `v`.
+  const noBuild = s.split("+", 1)[0];
+  const withoutV = noBuild.replace(/^v/, "");
+  const dash = withoutV.indexOf("-");
+  const coreStr = dash === -1 ? withoutV : withoutV.slice(0, dash);
+  const preStr = dash === -1 ? "" : withoutV.slice(dash + 1);
+
+  const coreParts = coreStr.split(".");
+  if (coreParts.length !== 3) return null;
+  const core: number[] = [];
+  for (const p of coreParts) {
+    if (!/^\d+$/.test(p)) return null;
+    core.push(Number(p));
+  }
+  const prerelease = preStr === "" ? [] : preStr.split(".");
+  return { core, prerelease };
+}
+
+/**
+ * Compare two prerelease identifier lists per SemVer §11.4:
+ *   - numeric identifiers compare numerically;
+ *   - alphanumeric identifiers compare lexically (ASCII);
+ *   - a numeric identifier always has LOWER precedence than an alphanumeric one;
+ *   - a larger set of fields has higher precedence when all preceding are equal.
+ * Both inputs are non-empty (the §11.3 "no prerelease > has prerelease" rule is
+ * handled by the caller).
+ */
+function comparePrerelease(a: string[], b: string[]): number {
+  const len = Math.max(a.length, b.length);
+  for (let i = 0; i < len; i++) {
+    // §11.4.4: a larger set of pre-release fields has higher precedence,
+    // provided all preceding identifiers are equal.
+    if (i >= a.length) return -1;
+    if (i >= b.length) return 1;
+    const ai = a[i];
+    const bi = b[i];
+    const aNum = /^\d+$/.test(ai);
+    const bNum = /^\d+$/.test(bi);
+    if (aNum && bNum) {
+      const an = Number(ai);
+      const bn = Number(bi);
+      if (an !== bn) return an < bn ? -1 : 1;
+    } else if (aNum && !bNum) {
+      // §11.4.3: numeric identifiers always have lower precedence.
+      return -1;
+    } else if (!aNum && bNum) {
+      return 1;
+    } else if (ai !== bi) {
+      return ai < bi ? -1 : 1;
+    }
+  }
+  return 0;
+}
+
+/**
+ * Compare two semver strings per SemVer 2.0 precedence. Returns -1 if a < b,
+ * 1 if a > b, 0 if equal. Implements §10 (build metadata ignored) and §11
+ * (prerelease ordering, including that a normal release outranks its own
+ * prerelease: 1.0.0 > 1.0.0-rc.3).
+ *
+ * Tolerant of a leading `v` on either side. If either side is not a valid
+ * `major.minor.patch[-pre]` string, falls back to a deterministic dot/dash
+ * split comparison so the auditor still produces *some* ordering rather than
+ * throwing — but well-formed semver always takes the spec path above.
  */
 export function semverCompare(a: string, b: string): number {
+  const pa = parseSemver(a);
+  const pb = parseSemver(b);
+
+  if (pa && pb) {
+    // Compare major.minor.patch numerically (§11.2).
+    for (let i = 0; i < 3; i++) {
+      if (pa.core[i] !== pb.core[i]) return pa.core[i] < pb.core[i] ? -1 : 1;
+    }
+    // §11.3: a version WITHOUT a prerelease has higher precedence than one
+    // WITH a prerelease at the same core (1.0.0 > 1.0.0-rc.3).
+    const aHasPre = pa.prerelease.length > 0;
+    const bHasPre = pb.prerelease.length > 0;
+    if (!aHasPre && !bHasPre) return 0;
+    if (!aHasPre && bHasPre) return 1;
+    if (aHasPre && !bHasPre) return -1;
+    return comparePrerelease(pa.prerelease, pb.prerelease);
+  }
+
+  // Fallback for non-semver strings: strip a leading `v` and the build
+  // metadata (still honoring §10), then compare segment-by-segment with the
+  // same numeric-vs-string rule. Deterministic, never throws.
   const norm = (s: string) =>
-    s.replace(/^v/, "").split(/[.\-+]/).map((p) => {
-      const n = Number(p);
-      return Number.isFinite(n) ? n : p;
-    });
+    s
+      .split("+", 1)[0]
+      .replace(/^v/, "")
+      .split(/[.\-]/)
+      .map((p) => {
+        const n = Number(p);
+        return /^\d+$/.test(p) && Number.isFinite(n) ? n : p;
+      });
   const aa = norm(a);
   const bb = norm(b);
   for (let i = 0; i < Math.max(aa.length, bb.length); i++) {
@@ -58,6 +156,30 @@ export function semverCompare(a: string, b: string): number {
   return 0;
 }
 
+/**
+ * Extract the trailing SemVer (`major.minor.patch[-pre][+build]`) substring
+ * from a git tag that may carry a monorepo / scoped prefix.
+ *
+ * GitHub release tags are not always bare `v1.2.3`. Monorepos and scoped
+ * packages tag releases as `@scope/pkg@1.2.3`, `pkg-name-v1.2.3`,
+ * `release-1.2.3`, `2026.01.0`, etc. Stripping only a leading `v` (the old
+ * behavior) left the prefix attached, so `semverCompare("1.2.3",
+ * "@scope/x@1.2.3")` reported phantom drift. We anchor on the LAST semver-shaped
+ * run in the string so the common `name@1.2.3` / `name-v1.2.3` shapes resolve to
+ * `1.2.3`.
+ *
+ * Returns the matched version (without a leading `v`) or null if the tag has no
+ * extractable SemVer core — callers should treat null as "can't compare".
+ */
+export function extractSemver(tag: string): string | null {
+  // Match all semver cores; keep the last one (handles `1.0-shipped-2.3.4`,
+  // and the `@scope/x@1.2.3` case where the leading part has no dotted core).
+  const re = /(\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?)/g;
+  let last: string | null = null;
+  for (const m of tag.matchAll(re)) last = m[1];
+  return last;
+}
+
 /**
  * Parse a GitHub remote URL (https or ssh form) into owner/repo.
  * Returns `${owner}/${repo}` or null if not a github.com remote.