From 954894f1d49b25604250b5a1a5384536739d5c95 Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Tue, 4 Aug 2026 12:14:36 +0900 Subject: [PATCH 1/5] refactor: centralize version comparison in pi-utils --- packages/coding-agent/src/cli/update-cli.ts | 20 +--- packages/coding-agent/src/utils/changelog.ts | 9 +- .../coding-agent/src/web/scrapers/hackage.ts | 13 +-- packages/utils/src/index.ts | 1 + packages/utils/src/version.ts | 99 +++++++++++++++++++ packages/utils/test/version.test.ts | 72 ++++++++++++++ scripts/ci-release-notes.ts | 18 +--- scripts/release.ts | 9 +- 8 files changed, 182 insertions(+), 59 deletions(-) create mode 100644 packages/utils/src/version.ts create mode 100644 packages/utils/test/version.test.ts diff --git a/packages/coding-agent/src/cli/update-cli.ts b/packages/coding-agent/src/cli/update-cli.ts index f106e72d1..ac2e2cd60 100644 --- a/packages/coding-agent/src/cli/update-cli.ts +++ b/packages/coding-agent/src/cli/update-cli.ts @@ -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 { }; } -/** - * 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; diff --git a/packages/coding-agent/src/utils/changelog.ts b/packages/coding-agent/src/utils/changelog.ts index 456fc23f2..b64431dae 100644 --- a/packages/coding-agent/src/utils/changelog.ts +++ b/packages/coding-agent/src/utils/changelog.ts @@ -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; diff --git a/packages/coding-agent/src/web/scrapers/hackage.ts b/packages/coding-agent/src/web/scrapers/hackage.ts index 997e90a69..339c2178a 100644 --- a/packages/coding-agent/src/web/scrapers/hackage.ts +++ b/packages/coding-agent/src/web/scrapers/hackage.ts @@ -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); diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 8a2ba3b8c..7d1363b7d 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -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 { diff --git a/packages/utils/src/version.ts b/packages/utils/src/version.ts new file mode 100644 index 000000000..15088117e --- /dev/null +++ b/packages/utils/src/version.ts @@ -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; +} diff --git a/packages/utils/test/version.test.ts b/packages/utils/test/version.test.ts new file mode 100644 index 000000000..bb6d485d3 --- /dev/null +++ b/packages/utils/test/version.test.ts @@ -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); + }); +}); diff --git a/scripts/ci-release-notes.ts b/scripts/ci-release-notes.ts index 1228d39dc..51bc8c9bb 100755 --- a/scripts/ci-release-notes.ts +++ b/scripts/ci-release-notes.ts @@ -1,4 +1,5 @@ #!/usr/bin/env bun + /** * Generate aggregated release notes from per-package CHANGELOG.md files. * @@ -28,6 +29,7 @@ * underneath; this only adds curated context. */ +import { compareVersions } from "@oh-my-pi/pi-utils"; import { $, Glob } from "bun"; const changelogGlob = new Glob("packages/*/CHANGELOG.md"); @@ -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. */ diff --git a/scripts/release.ts b/scripts/release.ts index 0399fce85..790c1f01f 100755 --- a/scripts/release.ts +++ b/scripts/release.ts @@ -1,4 +1,5 @@ #!/usr/bin/env bun +import { compareVersions } from "@oh-my-pi/pi-utils"; /** * Release script for pi-mono * @@ -186,14 +187,6 @@ 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 { console.log("\n=== Release Script ===\n"); From 726698e2b0d4ffad4401ad135998233f123746db Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Wed, 5 Aug 2026 01:09:31 +0900 Subject: [PATCH 2/5] 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 --- packages/coding-agent/CHANGELOG.md | 7 +++ packages/coding-agent/src/cli/update-cli.ts | 38 +------------- packages/utils/CHANGELOG.md | 4 ++ scripts/ci-release-notes.ts | 2 +- scripts/release.test.ts | 36 ++++++++++++++ scripts/release.ts | 55 ++++++++++++++------- 6 files changed, 87 insertions(+), 55 deletions(-) create mode 100644 scripts/release.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 22a72debb..94b193953 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -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 diff --git a/packages/coding-agent/src/cli/update-cli.ts b/packages/coding-agent/src/cli/update-cli.ts index ac2e2cd60..9cfc95413 100644 --- a/packages/coding-agent/src/cli/update-cli.ts +++ b/packages/coding-agent/src/cli/update-cli.ts @@ -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 { 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) { diff --git a/packages/utils/CHANGELOG.md b/packages/utils/CHANGELOG.md index c195b1d2c..8761240b5 100644 --- a/packages/utils/CHANGELOG.md +++ b/packages/utils/CHANGELOG.md @@ -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 diff --git a/scripts/ci-release-notes.ts b/scripts/ci-release-notes.ts index 51bc8c9bb..0f2b7d1fa 100755 --- a/scripts/ci-release-notes.ts +++ b/scripts/ci-release-notes.ts @@ -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"; diff --git a/scripts/release.test.ts b/scripts/release.test.ts new file mode 100644 index 000000000..cc820cb76 --- /dev/null +++ b/scripts/release.test.ts @@ -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); + }); +}); diff --git a/scripts/release.ts b/scripts/release.ts index 790c1f01f..4e79926b8 100755 --- a/scripts/release.ts +++ b/scripts/release.ts @@ -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 { 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 { // 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 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 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 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 Full release"); + console.error(" bun scripts/release.ts watch Watch CI for current commit"); + process.exit(1); + } } From df3fa86f7d6396ac4b5353b07078162539c1def2 Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Wed, 5 Aug 2026 01:59:08 +0900 Subject: [PATCH 3/5] fix(release): reject prereleases and normalize the version prefix Wave-2 review findings on #7586: - the guard accepted `17.2.8-rc.1`, and the publish step passes no --tag, so a prerelease would have become the npm `latest` every unqualified install and `omp update` resolve - `v17.2.8` passed the guard and reached Cargo.toml verbatim, which cargo rejects only after every manifest was rewritten; validation now returns the normalized version and callers write that --- scripts/release.test.ts | 52 +++++++++++++++++++++-------------------- scripts/release.ts | 32 ++++++++++++++++++------- 2 files changed, 50 insertions(+), 34 deletions(-) diff --git a/scripts/release.test.ts b/scripts/release.test.ts index cc820cb76..c7f640715 100644 --- a/scripts/release.test.ts +++ b/scripts/release.test.ts @@ -1,36 +1,38 @@ import { describe, expect, test } from "bun:test"; -import { isValidExplicitVersion } from "./release"; +import { validateExplicitVersion } from "./release"; -describe("isValidExplicitVersion", () => { +describe("validateExplicitVersion", () => { 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); + 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("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("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 leading v prefix", () => { - expect(isValidExplicitVersion("v17.2.8")).toBe(true); - expect(isValidExplicitVersion("V17.2.8")).toBe(false); + 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 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); + 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); }); }); diff --git a/scripts/release.ts b/scripts/release.ts index 4e79926b8..832c3ab5c 100755 --- a/scripts/release.ts +++ b/scripts/release.ts @@ -16,12 +16,22 @@ 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. + * 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 isValidExplicitVersion(version: string): boolean { - return /^v?\d+\.\d+\.\d+(-[0-9A-Za-z.-]+)?$/.test(version); +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[]) { @@ -199,14 +209,18 @@ async function cmdRelease(versionOrBump: string): Promise { 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. + // 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") { - if (!isValidExplicitVersion(versionOrBump)) { + const normalized = validateExplicitVersion(versionOrBump); + if (normalized === null) { 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).`, + `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 @@ -421,7 +435,7 @@ if (import.meta.main) { if (arg === "watch") { await cmdWatch(); - } else if (arg === "major" || arg === "minor" || arg === "patch" || isValidExplicitVersion(arg)) { + } else if (arg === "major" || arg === "minor" || arg === "patch" || validateExplicitVersion(arg) !== null) { await cmdRelease(arg); } else { console.error(`Unknown command or invalid version: ${arg}`); From ee492dbe7518c72ca904b66144e7404a8cb6341a Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Wed, 5 Aug 2026 02:45:42 +0900 Subject: [PATCH 4/5] fix(ci): run the release version guard in CI Wave-3 review finding on #7586: - scripts/release.test.ts ran in neither CI nor the documented root command, so the release-version regression assertions guarded nothing - add it to test:scripts and invoke that file from the workspace job - the job deliberately does not run all of test:scripts: musl-release fails on main, so the whole group would red this job on an unrelated break --- .github/workflows/ci.yml | 7 ++++++- package.json | 2 +- packages/coding-agent/CHANGELOG.md | 11 ++++++----- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 82e5e6a24..a82dbc0fe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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) diff --git a/package.json b/package.json index e763563f7..c3f0dc7a2 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 94b193953..0b32d4c82 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,11 +2,6 @@ ## [Unreleased] -## [17.2.8] - 2026-08-04 - -### 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. @@ -15,6 +10,12 @@ - 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 + +### 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. + ## [17.2.7] - 2026-08-03 ### Changed From 3a9b823630ab230897f10447364309dbb1528135 Mon Sep 17 00:00:00 2001 From: metaphorics <152830360+metaphorics@users.noreply.github.com> Date: Wed, 5 Aug 2026 04:50:17 +0900 Subject: [PATCH 5/5] fix(release): import the comparator without loading native bindings Wave-4 review finding on #7586: - release.ts took compareVersions from the pi-utils barrel, which pulls file-lock and procmgr into the graph; those import pi-natives, whose entrypoint calls loadNative() and throws in a checkout without the built addon - that also meant scripts/release.test.ts never executed under test:scripts, so the guard wired into CI last round proved nothing - take the dependency-free version subpath instead --- scripts/release.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/release.ts b/scripts/release.ts index 832c3ab5c..156ac11b7 100755 --- a/scripts/release.ts +++ b/scripts/release.ts @@ -1,5 +1,5 @@ #!/usr/bin/env bun -import { compareVersions } from "@oh-my-pi/pi-utils"; +import { compareVersions } from "@oh-my-pi/pi-utils/version"; /** * Release script for pi-mono *