fix(release): validate explicit versions and drop duplicate comparators

Review findings on #7586: - validate an explicit release version before comparing; the shared comparator never throws, so 999.bad previously reached every manifest - ci-release-notes imports the comparator by relative path: the release_github job runs without bun install - route Bun cache pruning through compareVersions and delete compareSemverLikeVersions - regression test for the release-version guard
2026-08-05 01:09:31 +09:00
parent 954894f1d4
commit 726698e2b0
6 changed files with 87 additions and 55 deletions
@@ -7,6 +7,13 @@
 ### Changed

 - Upgraded the bundled omptype schema engine: intersection and pipe operators, bigint and RegExp literals in the string DSL, Standard Schema V1 interop, JSON Schema import via fromJsonSchema(), and richer union/collection error reporting.
+### Breaking Changes
+
+- Renamed `compareVersions` to `compareChangelogEntries` in `@oh-my-pi/pi-coding-agent/utils/changelog`. The function signature and behavior are unchanged; update imports to use the new name.
+
+### Changed
+
+- Routed Bun install-cache pruning in `update-cli` through the shared `compareVersions` utility (`@oh-my-pi/pi-utils`), removing a duplicate local comparator that rounded large numeric version identifiers via `Number`.

 ## [17.2.7] - 2026-08-03

@@ -514,42 +514,6 @@ function stripBunCacheVersionSuffix(name: string): string {
 	return metadataIndex === -1 ? name : name.slice(0, metadataIndex);
 }

-function compareSemverIdentifier(a: string, b: string): number {
-	const aNumber = /^\d+$/.test(a);
-	const bNumber = /^\d+$/.test(b);
-	if (aNumber && bNumber) return Number(a) - Number(b);
-	if (aNumber) return -1;
-	if (bNumber) return 1;
-	return a.localeCompare(b);
-}
-
-function compareSemverLikeVersions(a: string, b: string): number {
-	const [aCoreWithPrerelease] = a.split("+", 1);
-	const [bCoreWithPrerelease] = b.split("+", 1);
-	const [aCore, aPrerelease] = aCoreWithPrerelease.split("-", 2);
-	const [bCore, bPrerelease] = bCoreWithPrerelease.split("-", 2);
-	const aParts = aCore.split(".");
-	const bParts = bCore.split(".");
-	for (let i = 0; i < Math.max(aParts.length, bParts.length); i++) {
-		const diff = Number(aParts[i] ?? 0) - Number(bParts[i] ?? 0);
-		if (diff !== 0 && Number.isFinite(diff)) return diff;
-	}
-	if (!aPrerelease && !bPrerelease) return 0;
-	if (!aPrerelease) return 1;
-	if (!bPrerelease) return -1;
-	const aPrereleaseParts = aPrerelease.split(".");
-	const bPrereleaseParts = bPrerelease.split(".");
-	for (let i = 0; i < Math.max(aPrereleaseParts.length, bPrereleaseParts.length); i++) {
-		const aPart = aPrereleaseParts[i];
-		const bPart = bPrereleaseParts[i];
-		if (aPart === undefined) return -1;
-		if (bPart === undefined) return 1;
-		const diff = compareSemverIdentifier(aPart, bPart);
-		if (diff !== 0) return diff;
-	}
-	return 0;
-}
-
 async function readdirIfExists(dir: string): Promise<fs.Dirent[]> {
 	try {
 		return await fs.promises.readdir(dir, { withFileTypes: true });
@@ -671,7 +635,7 @@ export async function pruneBunInstallCache(
 		scannedPackages++;
 		let latestVersion: string | undefined;
 		for (const version of group.actualDirs.keys()) {
-			if (!latestVersion || compareSemverLikeVersions(version, latestVersion) > 0) latestVersion = version;
+			if (!latestVersion || compareVersions(version, latestVersion) > 0) latestVersion = version;
 		}
 		if (!latestVersion) continue;
 		for (const [version, paths] of group.actualDirs) {
@@ -2,6 +2,10 @@

 ## [Unreleased]

+### Added
+
+- Added a public `compareVersions` utility (`@oh-my-pi/pi-utils`) that compares two version strings with SemVer-2.0 prerelease ordering, build-metadata stripping, and numeric segment comparison without float overflow; never throws.
+
 ## [17.2.6] - 2026-08-03

 ### Added
@@ -29,8 +29,8 @@
 * underneath; this only adds curated context.
 */

-import { compareVersions } from "@oh-my-pi/pi-utils";
 import { $, Glob } from "bun";
+import { compareVersions } from "../packages/utils/src/version";

 const changelogGlob = new Glob("packages/*/CHANGELOG.md");
 const REPO = process.env.OMP_REPO ?? process.env.GITHUB_REPOSITORY ?? "can1357/oh-my-pi";
@@ -0,0 +1,36 @@
+import { describe, expect, test } from "bun:test";
+import { isValidExplicitVersion } from "./release";
+
+describe("isValidExplicitVersion", () => {
+	test("rejects malformed versions", () => {
+		expect(isValidExplicitVersion("999.bad")).toBe(false);
+		expect(isValidExplicitVersion("17")).toBe(false);
+		expect(isValidExplicitVersion("17.2")).toBe(false);
+		expect(isValidExplicitVersion("17.2.8.9")).toBe(false);
+		expect(isValidExplicitVersion("v17.2.8.9")).toBe(false);
+		expect(isValidExplicitVersion("abc")).toBe(false);
+		expect(isValidExplicitVersion("")).toBe(false);
+		expect(isValidExplicitVersion("v")).toBe(false);
+		expect(isValidExplicitVersion("17.2.8-")).toBe(false);
+	});
+
+	test("accepts valid three-segment numeric versions", () => {
+		expect(isValidExplicitVersion("17.2.8")).toBe(true);
+		expect(isValidExplicitVersion("0.0.0")).toBe(true);
+		expect(isValidExplicitVersion("1.0.0")).toBe(true);
+	});
+
+	test("accepts leading v prefix", () => {
+		expect(isValidExplicitVersion("v17.2.8")).toBe(true);
+		expect(isValidExplicitVersion("V17.2.8")).toBe(false);
+	});
+
+	test("accepts prerelease suffixes", () => {
+		expect(isValidExplicitVersion("17.2.8-rc.1")).toBe(true);
+		expect(isValidExplicitVersion("v17.2.8-rc.1")).toBe(true);
+		expect(isValidExplicitVersion("1.0.0-beta")).toBe(true);
+		expect(isValidExplicitVersion("1.0.0-alpha.1.2")).toBe(true);
+		expect(isValidExplicitVersion("1.0.0-0.3.7")).toBe(true);
+		expect(isValidExplicitVersion("1.0.0-x.7.z.92")).toBe(true);
+	});
+});
@@ -15,6 +15,14 @@ import { runChangelogFixer } from "./fix-changelogs";
 const changelogGlob = new Glob("packages/*/CHANGELOG.md");
 const packageJsonGlob = new Glob("packages/*/package.json");
 const cargoTomlGlob = new Glob("crates/*/Cargo.toml");
+/**
+ * Strict explicit-version guard: three numeric dot-segments, optional leading
+ * `v`, optional SemVer-2.0 prerelease suffix. Bump keywords (major/minor/patch)
+ * are handled separately and must not be routed through this check.
+ */
+export function isValidExplicitVersion(version: string): boolean {
+	return /^v?\d+\.\d+\.\d+(-[0-9A-Za-z.-]+)?$/.test(version);
+}

 function git(args: readonly string[]) {
 	return $`git -c core.fsmonitor=false -c core.untrackedCache=false -c fetch.pruneTags=false ${args}`;
@@ -189,6 +197,17 @@ function bumpVersion(current: string, bump: "major" | "minor" | "patch"): string

 async function cmdRelease(versionOrBump: string): Promise<void> {
 	console.log("\n=== Release Script ===\n");
+	// Validate explicit versions before any compare: the shared compareVersions
+	// never throws, so without this guard garbage like "999.bad" would be
+	// accepted and written into every package.json / Cargo.toml / tag.
+	if (versionOrBump !== "major" && versionOrBump !== "minor" && versionOrBump !== "patch") {
+		if (!isValidExplicitVersion(versionOrBump)) {
+			console.error(
+				`Error: Invalid version "${versionOrBump}". Expected a semver like 17.2.8 or v17.2.8-rc.1, or a bump keyword (major/minor/patch).`,
+			);
+			process.exit(1);
+		}
+	}

 	// 1. Pre-flight checks
 	console.log("Pre-flight checks...");
@@ -390,23 +409,25 @@ async function cmdRelease(versionOrBump: string): Promise<void> {
 // Main
 // =============================================================================

-const arg = process.argv[2];
+if (import.meta.main) {
+	const arg = process.argv[2];

-if (!arg) {
-	console.error("Usage:");
-	console.error("  bun scripts/release.ts <version|major|minor|patch>   Full release");
-	console.error("  bun scripts/release.ts watch                         Watch CI for current commit");
-	process.exit(1);
-}
+	if (!arg) {
+		console.error("Usage:");
+		console.error("  bun scripts/release.ts <version|major|minor|patch>   Full release");
+		console.error("  bun scripts/release.ts watch                         Watch CI for current commit");
+		process.exit(1);
+	}

-if (arg === "watch") {
-	await cmdWatch();
-} else if (arg === "major" || arg === "minor" || arg === "patch" || /^\d+\.\d+\.\d+$/.test(arg)) {
-	await cmdRelease(arg);
-} else {
-	console.error(`Unknown command or invalid version: ${arg}`);
-	console.error("Usage:");
-	console.error("  bun scripts/release.ts <version|major|minor|patch>   Full release");
-	console.error("  bun scripts/release.ts watch                         Watch CI for current commit");
-	process.exit(1);
+	if (arg === "watch") {
+		await cmdWatch();
+	} else if (arg === "major" || arg === "minor" || arg === "patch" || isValidExplicitVersion(arg)) {
+		await cmdRelease(arg);
+	} else {
+		console.error(`Unknown command or invalid version: ${arg}`);
+		console.error("Usage:");
+		console.error("  bun scripts/release.ts <version|major|minor|patch>   Full release");
+		console.error("  bun scripts/release.ts watch                         Watch CI for current commit");
+		process.exit(1);
+	}
 }