Skip to content

Commit 1d976f8

Browse files
pimfeltkampclaude
andauthored
Fix two version-comparison bugs in cryptohopper upgrade (#7)
Bug 1: alpha.10 < alpha.2 compareVersions used String#localeCompare on the full prerelease string, so "alpha.10" sorts before "alpha.2" alphabetically. SemVer 2.0.0 specifies dot-separated identifiers, with numeric ones compared numerically. Adds a comparePrerelease helper that splits on "." and applies the SemVer rules: - all-digit identifiers compared numerically - mixed/letter identifiers compared lexically - numeric identifiers always have lower precedence than non-numeric - longer-prefix-matching set wins Bug 2: fetchLatestRelease relied on GitHub's created_at order GitHub returns /releases sorted by created_at desc, which usually matches version order — but a hot-fix tag pushed after a newer release inverts that. Now sorts the filtered cli-v* list by version (highest first) using the now-correct compareVersions, so the latest is always the highest version regardless of publication order. Verified with a 9-case matrix covering: alpha.10 vs alpha.2 both directions, equality, release vs prerelease both directions, minor bumps, alpha vs beta vs rc, and longer-prefix prereleases. typecheck and build are clean. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent bc2bc9b commit 1d976f8

1 file changed

Lines changed: 44 additions & 2 deletions

File tree

src/upgrade/github-release.ts

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,19 @@ export async function fetchLatestRelease(repo: string = RELEASE_REPO): Promise<R
4242
const all = (await res.json()) as Release[];
4343
const cliReleases = all.filter((r) => !r.draft && r.tag_name.startsWith("cli-v"));
4444
if (cliReleases.length === 0) return null;
45+
// Sort by version (highest first). GitHub returns releases by created_at,
46+
// so a hot-fix on an older minor that's published *after* a newer release
47+
// would otherwise appear at index 0 and trick `upgrade` into doing nothing
48+
// (or downgrading).
49+
cliReleases.sort((a, b) => compareVersions(b.tag_name, a.tag_name));
4550
return cliReleases[0]!;
4651
}
4752

4853
export function versionFromTag(tag: string): string {
4954
return tag.replace(/^cli-v/, "");
5055
}
5156

52-
/** Returns negative if a is older than b. Small-but-deterministic semver. */
57+
/** Returns negative if a is older than b. SemVer 2.0.0 precedence rules. */
5358
export function compareVersions(a: string, b: string): number {
5459
const aRel = versionFromTag(a);
5560
const bRel = versionFromTag(b);
@@ -64,11 +69,48 @@ export function compareVersions(a: string, b: string): number {
6469
if (aPre && !bPre) return -1;
6570
if (!aPre && bPre) return 1;
6671
if (!aPre && !bPre) return 0;
67-
return aPre!.localeCompare(bPre!);
72+
return comparePrerelease(aPre!, bPre!);
6873
}
6974

7075
function splitVersion(v: string): [number[], string | null] {
7176
const [core, pre] = v.split("-", 2);
7277
const parts = (core ?? "0.0.0").split(".").map((n) => Number(n)).map((n) => (Number.isFinite(n) ? n : 0));
7378
return [parts, pre ?? null];
7479
}
80+
81+
/**
82+
* Compare two SemVer prerelease strings (the part after the `-`) per the
83+
* SemVer 2.0.0 precedence rules:
84+
* - Identifiers consisting of only digits are compared numerically.
85+
* - Identifiers with letters or hyphens are compared lexically in ASCII.
86+
* - Numeric identifiers always have lower precedence than non-numeric.
87+
* - A larger set of fields has higher precedence than a smaller one.
88+
*
89+
* Crucially, this means `alpha.10` > `alpha.2` (numeric compare on the
90+
* second identifier), which a naive `String#localeCompare` gets wrong.
91+
*/
92+
function comparePrerelease(a: string, b: string): number {
93+
const aIds = a.split(".");
94+
const bIds = b.split(".");
95+
const max = Math.max(aIds.length, bIds.length);
96+
for (let i = 0; i < max; i++) {
97+
const ai = aIds[i];
98+
const bi = bIds[i];
99+
if (ai === undefined) return -1;
100+
if (bi === undefined) return 1;
101+
const aNum = /^\d+$/.test(ai);
102+
const bNum = /^\d+$/.test(bi);
103+
if (aNum && bNum) {
104+
const d = Number(ai) - Number(bi);
105+
if (d !== 0) return d;
106+
} else if (aNum) {
107+
return -1;
108+
} else if (bNum) {
109+
return 1;
110+
} else {
111+
const d = ai.localeCompare(bi);
112+
if (d !== 0) return d;
113+
}
114+
}
115+
return 0;
116+
}

0 commit comments

Comments
 (0)