Merge PR #7586: refactor: centralize version comparison in pi-utils (@metaphorics)

This commit is contained in:
can1357
2026-08-05 01:11:56 +02:00
13 changed files with 291 additions and 115 deletions
+6 -1
View File
@@ -307,7 +307,12 @@ jobs:
- name: Test workspace packages and repo scripts (TS)
env:
OMP_TEST_CONCURRENCY: "4"
run: bun run ci:test:ts:workspace
run: |
bun run ci:test:ts:workspace
# Not `test:scripts`: scripts/musl-release.test.ts fails on main
# (its install.sh smoke-check executes a fake binary), so running
# the whole group here would red this job on an unrelated break.
bun test scripts/release.test.ts
test_coding_agent_singleton:
name: Test coding-agent singleton/global-state (TS)
+1 -1
View File
@@ -117,7 +117,7 @@
"build:native": "bun --cwd=packages/natives run build",
"test": "bun scripts/ci-test-ts.ts local",
"test:ts": "bun scripts/ci-test-ts.ts local-ts",
"test:scripts": "bun test scripts/ci-release-build-binaries.test.ts scripts/musl-release.test.ts scripts/ci-release-publish.test.ts",
"test:scripts": "bun test scripts/ci-release-build-binaries.test.ts scripts/musl-release.test.ts scripts/ci-release-publish.test.ts scripts/release.test.ts",
"test:rs": "bun scripts/run-rs-task.ts test:rs",
"check": "bun run --parallel check:ts check:rs",
"check:ts": "bun run check:tools && bun run --workspaces --if-present check",
+7
View File
@@ -5,6 +5,13 @@
### Changed
- Restored the legacy project-scoped session directory naming scheme and removed its automatic migration ([#7646](https://github.com/can1357/oh-my-pi/issues/7646)).
### 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.8] - 2026-08-04
+2 -56
View File
@@ -10,7 +10,7 @@ import * as os from "node:os";
import * as path from "node:path";
import { Transform } from "node:stream";
import { pipeline } from "node:stream/promises";
import { $env, $which, APP_NAME, isEnoent, VERSION } from "@oh-my-pi/pi-utils";
import { $env, $which, APP_NAME, compareVersions, isEnoent, VERSION } from "@oh-my-pi/pi-utils";
import { $ } from "bun";
import chalk from "chalk";
import { theme } from "../modes/theme/theme";
@@ -498,24 +498,6 @@ async function getLatestRelease(): Promise<ReleaseInfo> {
};
}
/**
* Compare semver versions. Returns:
* - negative if a < b
* - 0 if a == b
* - positive if a > b
*/
function compareVersions(a: string, b: string): number {
const pa = a.split(".").map(Number);
const pb = b.split(".").map(Number);
for (let i = 0; i < Math.max(pa.length, pb.length); i++) {
const na = pa[i] || 0;
const nb = pb[i] || 0;
if (na !== nb) return na - nb;
}
return 0;
}
interface BunInstallCachePruneResult {
scannedPackages: number;
removedEntries: number;
@@ -532,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 });
@@ -689,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) {
+5 -4
View File
@@ -198,9 +198,10 @@ function parseChangelogContent(content: string): ChangelogEntry[] {
}
/**
* Compare versions. Returns: -1 if v1 < v2, 0 if v1 === v2, 1 if v1 > v2
* Compare changelog entries by their parsed version parts.
* Returns: -1 if v1 < v2, 0 if v1 === v2, 1 if v1 > v2
*/
export function compareVersions(v1: ChangelogEntry, v2: ChangelogEntry): number {
export function compareChangelogEntries(v1: ChangelogEntry, v2: ChangelogEntry): number {
if (v1.major !== v2.major) return v1.major - v2.major;
if (v1.minor !== v2.minor) return v1.minor - v2.minor;
return v1.patch - v2.patch;
@@ -232,7 +233,7 @@ export function getNewEntries(entries: ChangelogEntry[], lastVersion: string): C
return [];
}
return entries.filter(entry => compareVersions(entry, parsedLastVersion) > 0);
return entries.filter(entry => compareChangelogEntries(entry, parsedLastVersion) > 0);
}
/**
@@ -328,7 +329,7 @@ export async function resolveStartupChangelogForDisplay(options: {
}
if (options.mode === "hidden") {
const currentVersion = parseChangelogVersion(options.currentVersion);
if (currentVersion && compareVersions(currentVersion, parsedLastVersion) > 0) {
if (currentVersion && compareChangelogEntries(currentVersion, parsedLastVersion) > 0) {
await writeLastChangelogVersion(options.currentVersion, options.agentDir);
}
return undefined;
@@ -1,4 +1,4 @@
import { tryParseJson } from "@oh-my-pi/pi-utils";
import { compareVersions, tryParseJson } from "@oh-my-pi/pi-utils";
import type { RenderResult, SpecialHandler } from "./types";
import { buildResult, loadPage } from "./types";
@@ -20,17 +20,6 @@ interface ParsedCabal {
stability?: string;
}
function compareVersions(a: string, b: string): number {
const aParts = a.split(".").map(part => Number.parseInt(part, 10) || 0);
const bParts = b.split(".").map(part => Number.parseInt(part, 10) || 0);
const max = Math.max(aParts.length, bParts.length);
for (let i = 0; i < max; i++) {
const delta = (aParts[i] || 0) - (bParts[i] || 0);
if (delta !== 0) return delta;
}
return 0;
}
function extractCabalField(content: string, fieldName: string): string | undefined {
const pattern = new RegExp(`^${fieldName}:\\s*(.*)$`, "im");
const match = content.match(pattern);
+4
View File
@@ -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
+1
View File
@@ -34,6 +34,7 @@ export * from "./tab-spacing";
export * from "./temp";
export * from "./tls-fetch";
export * from "./type-guards";
export * from "./version";
export * from "./which";
function isPlainObject(val: object): val is Record<string, unknown> {
+99
View File
@@ -0,0 +1,99 @@
const DIGITS = /^\d+$/;
/**
* Compare two version strings.
*
* Canonical comparator that supersedes the historical in-repo copies
* (update-cli, hackage scraper, release scripts):
* - inputs are trimmed and at most one leading `v`/`V` is stripped
* - dot-separated segments are compared numerically, missing trailing
* segments count as 0, so `1.2` === `1.2.0` and any segment count works
* - a SemVer-2.0 prerelease suffix sorts before the plain release
* (`1.0.0-beta` < `1.0.0`); prerelease identifiers follow SemVer order
* (numeric < alphanumeric, numeric compared by value, alphanumeric
* compared lexically, longer sets of equal fields win)
* - SemVer build metadata begins at the first `+` and does not participate
* in precedence; it is stripped before core/prerelease parsing
* - malformed numeric segments compare as 0 (`1.2.x` === `1.2.0`)
* - never throws; returns only -1 | 0 | 1
*/
export function compareVersions(a: string, b: string): number {
const pa = parseVersion(a);
const pb = parseVersion(b);
const core = compareNumericParts(pa.core, pb.core);
if (core !== 0) return core;
return comparePrerelease(pa.prerelease, pb.prerelease);
}
interface ParsedVersion {
core: string[];
prerelease: string[] | null;
}
function parseVersion(version: string): ParsedVersion {
const trimmed = version.trim();
const stripped = trimmed.startsWith("v") || trimmed.startsWith("V") ? trimmed.slice(1) : trimmed;
const plusIndex = stripped.indexOf("+");
const withoutBuild = plusIndex === -1 ? stripped : stripped.slice(0, plusIndex);
const dashIndex = withoutBuild.indexOf("-");
if (dashIndex === -1) {
return { core: withoutBuild.split("."), prerelease: null };
}
return {
core: withoutBuild.slice(0, dashIndex).split("."),
prerelease: withoutBuild.slice(dashIndex + 1).split("."),
};
}
/** Compare dot-separated numeric segments; missing/malformed segments count as 0. */
function compareNumericParts(a: string[], b: string[]): number {
const length = Math.max(a.length, b.length);
for (let i = 0; i < length; i++) {
// Missing or malformed segments compare as 0.
const sa = a[i];
const sb = b[i];
const result = compareDigits(
sa !== undefined && DIGITS.test(sa) ? sa : "0",
sb !== undefined && DIGITS.test(sb) ? sb : "0",
);
if (result !== 0) return result;
}
return 0;
}
/** Exact integer comparison of digit strings, avoiding float overflow. */
function compareDigits(a: string, b: string): number {
const na = a.replace(/^0+/, "") || "0";
const nb = b.replace(/^0+/, "") || "0";
if (na.length !== nb.length) return na.length < nb.length ? -1 : 1;
if (na < nb) return -1;
if (na > nb) return 1;
return 0;
}
/** SemVer-2.0 prerelease ordering; null means a plain release, which wins. */
function comparePrerelease(a: string[] | null, b: string[] | null): number {
if (a === null || b === null) {
return a === b ? 0 : a === null ? 1 : -1;
}
const length = Math.max(a.length, b.length);
for (let i = 0; i < length; i++) {
const ia = a[i];
const ib = b[i];
if (ia === undefined) return -1;
if (ib === undefined) return 1;
const aNumeric = DIGITS.test(ia);
const bNumeric = DIGITS.test(ib);
if (aNumeric && bNumeric) {
const result = compareDigits(ia, ib);
if (result !== 0) return result;
} else if (aNumeric !== bNumeric) {
return aNumeric ? -1 : 1;
} else if (ia !== ib) {
return ia < ib ? -1 : 1;
}
}
return 0;
}
+72
View File
@@ -0,0 +1,72 @@
import { describe, expect, it } from "bun:test";
import { compareVersions } from "../src/version";
describe("compareVersions", () => {
it("trims whitespace and strips one leading v/V", () => {
expect(compareVersions(" 1.2.3 ", "1.2.3")).toBe(0);
expect(compareVersions("v1.2.3", "1.2.3")).toBe(0);
expect(compareVersions("V1.2.3", "v1.2.3")).toBe(0);
expect(compareVersions(" v1.2.3 ", "1.2.3")).toBe(0);
});
it("zero-pads missing trailing segments", () => {
expect(compareVersions("1.2", "1.2.0")).toBe(0);
expect(compareVersions("1", "1.0.0.0")).toBe(0);
expect(compareVersions("1.2", "1.2.1")).toBe(-1);
expect(compareVersions("1.2.3", "1.2")).toBe(1);
});
it("supports arbitrary segment counts", () => {
expect(compareVersions("1.2.3.4.5", "1.2.3.4.5")).toBe(0);
expect(compareVersions("1.2.3.4", "1.2.3.5")).toBe(-1);
expect(compareVersions("1.0.0.1", "1.0.0")).toBe(1);
});
it("orders SemVer prereleases before the plain release", () => {
expect(compareVersions("1.0.0-beta", "1.0.0")).toBe(-1);
expect(compareVersions("1.0.0", "1.0.0-rc.1")).toBe(1);
expect(compareVersions("v1.0.0-beta", "1.0.0")).toBe(-1);
expect(compareVersions("1.1.0-alpha", "1.0.0-beta")).toBe(1);
});
it("compares prerelease identifiers per SemVer 2.0", () => {
expect(compareVersions("1.0.0-alpha", "1.0.0-beta")).toBe(-1);
expect(compareVersions("1.0.0-rc.1", "1.0.0-rc.2")).toBe(-1);
// numeric, not lexical: rc.10 > rc.9
expect(compareVersions("1.0.0-rc.9", "1.0.0-rc.10")).toBe(-1);
// a larger set of equal fields has higher precedence
expect(compareVersions("1.0.0-alpha", "1.0.0-alpha.1")).toBe(-1);
// numeric identifiers sort before alphanumeric ones
expect(compareVersions("1.0.0-1", "1.0.0-alpha")).toBe(-1);
expect(compareVersions("1.0.0-beta.2", "1.0.0-beta.1")).toBe(1);
});
it("strips SemVer build metadata before comparing", () => {
// build metadata does not affect precedence
expect(compareVersions("1.0.1+linux", "1.0.0")).toBe(1);
expect(compareVersions("1.0.0-rc.1+abc", "1.0.0-rc.1+xyz")).toBe(0);
expect(compareVersions("1.0.0+build1", "1.0.0+build2")).toBe(0);
expect(compareVersions("1.0.0+linux", "1.0.0+mac")).toBe(0);
expect(compareVersions("1.0.0+linux", "1.0.1+linux")).toBe(-1);
expect(compareVersions("v1.2.3+meta", "1.2.3")).toBe(0);
expect(compareVersions(" 1.0.0+meta ", "1.0.0")).toBe(0);
});
it("compares malformed numeric segments as 0", () => {
expect(compareVersions("1.2.x", "1.2.0")).toBe(0);
expect(compareVersions("1.x", "1.0")).toBe(0);
expect(compareVersions("1.2.x", "1.2.1")).toBe(-1);
});
it("never throws and always returns -1, 0, or 1", () => {
expect(compareVersions("not_a_version", "0")).toBe(0);
expect(compareVersions("", "")).toBe(0);
expect(compareVersions("v", "")).toBe(0);
// hyphenated garbage parses as a prerelease suffix and loses to the release
expect(compareVersions("not-a-version", "1.0.0")).toBe(-1);
expect(compareVersions("3.0.0", "1.0.0")).toBe(1);
expect(compareVersions("1.0.0", "3.0.0")).toBe(-1);
// exact numeric comparison beyond float precision
expect(compareVersions("1.2.99999999999999999999", "1.2.100000000000000000000")).toBe(-1);
});
});
+2 -16
View File
@@ -1,4 +1,5 @@
#!/usr/bin/env bun
/**
* Generate aggregated release notes from per-package CHANGELOG.md files.
*
@@ -29,6 +30,7 @@
*/
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";
@@ -37,22 +39,6 @@ const REPO = process.env.OMP_REPO ?? process.env.GITHUB_REPOSITORY ?? "can1357/o
// alphabetically after these.
const CATEGORY_ORDER = ["Breaking Changes", "Added", "Changed", "Fixed", "Removed"] as const;
/** Compare two `X.Y.Z` (or `vX.Y.Z`) version strings; non-semver returns 0. */
export function compareVersions(a: string, b: string): number {
const am = a
.replace(/^v/, "")
.trim()
.match(/^(\d+)\.(\d+)\.(\d+)$/);
const bm = b
.replace(/^v/, "")
.trim()
.match(/^(\d+)\.(\d+)\.(\d+)$/);
if (!am || !bm) return 0;
if (am[1] !== bm[1]) return Number(am[1]) - Number(bm[1]);
if (am[2] !== bm[2]) return Number(am[2]) - Number(bm[2]);
return Number(am[3]) - Number(bm[3]);
}
export interface ChangelogVersionSpan {
version: string;
/** 0-indexed line of the `## [X.Y.Z]` heading. */
+38
View File
@@ -0,0 +1,38 @@
import { describe, expect, test } from "bun:test";
import { validateExplicitVersion } from "./release";
describe("validateExplicitVersion", () => {
test("rejects malformed versions", () => {
expect(validateExplicitVersion("999.bad")).toBe(null);
expect(validateExplicitVersion("17")).toBe(null);
expect(validateExplicitVersion("17.2")).toBe(null);
expect(validateExplicitVersion("17.2.8.9")).toBe(null);
expect(validateExplicitVersion("v17.2.8.9")).toBe(null);
expect(validateExplicitVersion("abc")).toBe(null);
expect(validateExplicitVersion("")).toBe(null);
expect(validateExplicitVersion("v")).toBe(null);
expect(validateExplicitVersion("17.2.8-")).toBe(null);
});
test("rejects prerelease suffixes (not supported by this release path)", () => {
// Prereleases would be published as npm `latest` because the downstream
// publish runs `npm publish` with no `--tag`.
expect(validateExplicitVersion("17.2.8-rc.1")).toBe(null);
expect(validateExplicitVersion("v17.2.8-beta")).toBe(null);
expect(validateExplicitVersion("1.0.0-alpha")).toBe(null);
expect(validateExplicitVersion("1.0.0-alpha.1.2")).toBe(null);
expect(validateExplicitVersion("1.0.0-0.3.7")).toBe(null);
expect(validateExplicitVersion("1.0.0-x.7.z.92")).toBe(null);
});
test("accepts bare three-segment numeric versions and returns them unchanged", () => {
expect(validateExplicitVersion("17.2.8")).toBe("17.2.8");
expect(validateExplicitVersion("0.0.0")).toBe("0.0.0");
expect(validateExplicitVersion("1.0.0")).toBe("1.0.0");
});
test("accepts leading v prefix and normalizes to the bare version", () => {
expect(validateExplicitVersion("v17.2.8")).toBe("17.2.8");
expect(validateExplicitVersion("V17.2.8")).toBe(null);
});
});
+53 -25
View File
@@ -1,4 +1,5 @@
#!/usr/bin/env bun
import { compareVersions } from "@oh-my-pi/pi-utils/version";
/**
* Release script for pi-mono
*
@@ -14,6 +15,24 @@ 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 with an optional
* leading `v` and NO prerelease suffix. Prereleases are rejected because the
* downstream publish (`scripts/ci-release-publish.ts`) runs `npm publish` with
* no `--tag`, which would promote a prerelease to the npm `latest` dist-tag —
* hitting every unqualified install and the `/latest` endpoint `omp update`
* reads. Bump keywords (major/minor/patch) are handled separately and must not
* be routed through this check.
*
* Returns the normalized bare version (leading `v` stripped) when accepted, or
* `null` when rejected. Callers must use the returned value for all writes so
* no downstream manifest (package.json, Cargo.toml, tag) ever sees a `v`
* prefix — Cargo rejects `version = "v17.2.8"`.
*/
export function validateExplicitVersion(version: string): string | null {
const match = /^v?(\d+\.\d+\.\d+)$/.exec(version);
return match ? match[1] : null;
}
function git(args: readonly string[]) {
return $`git -c core.fsmonitor=false -c core.untrackedCache=false -c fetch.pruneTags=false ${args}`;
@@ -186,16 +205,23 @@ function bumpVersion(current: string, bump: "major" | "minor" | "patch"): string
}
}
function compareVersions(a: string, b: string): number {
const [aMajor, aMinor, aPatch] = parseVersion(a);
const [bMajor, bMinor, bPatch] = parseVersion(b);
if (aMajor !== bMajor) return aMajor - bMajor;
if (aMinor !== bMinor) return aMinor - bMinor;
return aPatch - bPatch;
}
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. The
// validator also normalizes a leading `v` to the bare version so every
// downstream write (manifests, Cargo.toml, tag) uses `17.2.8`, not `v17.2.8`.
if (versionOrBump !== "major" && versionOrBump !== "minor" && versionOrBump !== "patch") {
const normalized = validateExplicitVersion(versionOrBump);
if (normalized === null) {
console.error(
`Error: Invalid version "${versionOrBump}". Expected a semver like 17.2.8 or v17.2.8 (prereleases such as 17.2.8-rc.1 are not supported by this release path), or a bump keyword (major/minor/patch).`,
);
process.exit(1);
}
versionOrBump = normalized;
}
// 1. Pre-flight checks
console.log("Pre-flight checks...");
@@ -397,23 +423,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" || validateExplicitVersion(arg) !== null) {
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);
}
}