fix(mcp): order prerelease identifiers by semver precedence#3049
Conversation
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-04 10:16:01 UTC
⏸️ Suggested Action - Manual Review Review summary Nits — 4 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3049 +/- ##
=======================================
Coverage 96.10% 96.10%
=======================================
Files 261 261
Lines 28872 28889 +17
Branches 10510 10517 +7
=======================================
+ Hits 27746 27763 +17
Misses 492 492
Partials 634 634
🚀 New features to boost your workflow:
|
Summary
compareMcpSemverinsrc/services/mcp-compatibility.tscompared two prerelease tags by running a singlenumeric-collation
localeCompareover the whole prerelease string. Semver (§11.4) compares prereleasetags identifier by identifier (dot-separated), which this got wrong in two ways:
than an alphanumeric one. Whole-string numeric collation instead compares leading digit runs
numerically, so it mis-ranks pairs where the alphanumeric identifier starts with a smaller digit:
Number()-based numeric compare loses precision pastNumber.MAX_SAFE_INTEGER, so distinct large identifiers collapse to equal:Fix: a per-identifier
comparePrereleasethat applies semver precedence — numeric identifiers rankedbelow alphanumeric; two numeric identifiers compared as decimal strings (with no leading zeros, a
longer digit string is the larger number and equal-length strings compare lexicographically), avoiding any
Number()precision loss; alphanumeric identifiers compared lexically; and a shorter identifier listranked lower when all preceding fields are equal. The pre-existing case-insensitive behavior
(
RC.1==rc.1) is preserved intentionally.No linked issue: small, self-evident correctness fix to one deterministic semver helper, with no public
API, auth, schema, or deploy surface change — fits the repo's
preferred(not required) linked-issuepolicy.
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
git diff --check(clean),npm run typecheck(clean), and a focusedtest:coverageover
test/unit/mcp-compatibility.test.ts— 8 tests pass and the changed lines are at 100% branchcoverage (the numeric decimal-string, numeric-vs-alpha both directions, alpha, and field-length
branches are all exercised; the only uncovered line in the file, 38, is a pre-existing snapshot builder
outside this diff), so
codecov/patchis 100% on this diff.src/-onlychange to one deterministic helper touching no UI/API/OpenAPI/DB/workflow surface, so those jobs are
no-ops for this diff; they run on CI (Linux).
Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.Notes
auth/UI/OpenAPI boxes are N/A.
test/unit/mcp-compatibility.test.ts:compareMcpSemver("0.3.0-2","0.3.0-1a")is now
-1, and two large numeric identifiers pastMAX_SAFE_INTEGERorder as-1/1instead ofcollapsing to
0. All 16 existing prerelease-precedence assertions still hold under the per-identifiercomparator.