From 0be3dfa2d89ce5bfff0f86452e48e7962fcfcdb6 Mon Sep 17 00:00:00 2001 From: Lloyd Pick Date: Fri, 21 Aug 2026 16:10:20 -0400 Subject: [PATCH] fix(store): resolve partial semver versions like "1.20" in findBestVersion listVersions() filtered stored version strings with strict semver.valid(), which requires a full X.Y.Z string. A version stored as "1.20" (major.minor, e.g. Cilium's own docs versioning scheme) failed that check and was silently dropped, making it permanently invisible to findBestVersion() even when explicitly requested. Separately, semver.maxSatisfying() also requires every candidate to itself be a fully valid semver string, so even after accepting "1.20" into the version list it still wouldn't match against a requested range. Fix both: filter with semver.coerce() so coercible partial versions are kept (non-versions like "stable"/"latest" correctly still fail coercion and stay excluded), and match on coerced candidates in findBestVersion(), mapping the winner back to its original stored string so downstream lookups use what's actually in the store. Related to but broader than #475, which reports non-semver labels like "latest" specifically; this also covers legitimate partial version formats that #475 doesn't. Fixes #480 --- src/store/DocumentManagementService.test.ts | 39 +++++++++++++++++++++ src/store/DocumentManagementService.ts | 25 +++++++++++-- 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/store/DocumentManagementService.test.ts b/src/store/DocumentManagementService.test.ts index 752004b38..943bcd7ce 100644 --- a/src/store/DocumentManagementService.test.ts +++ b/src/store/DocumentManagementService.test.ts @@ -393,6 +393,17 @@ describe("DocumentManagementService", () => { expect(versions).toEqual(["2.0.0", "2.0.0-beta", "1.0.0"]); expect(mockStore.queryUniqueVersions).toHaveBeenCalledWith(library); // Fix: Use mockStoreInstance }); + + it("should include partial versions coercible to semver (e.g. '1.20', '5'), sorted descending", async () => { + const library = "test-lib"; + mockStore.queryUniqueVersions.mockResolvedValue(["1.20", "5", "stable", "2.0.0"]); + + const versions = await docService.listVersions(library); + // "stable" is not coercible to semver and stays excluded; "1.20" and + // "5" are real partial versions (major.minor / major-only) and must + // not be silently dropped just because they aren't full X.Y.Z. + expect(versions).toEqual(["5", "2.0.0", "1.20"]); + }); }); describe("findBestVersion", () => { @@ -509,6 +520,34 @@ describe("DocumentManagementService", () => { docService.findBestVersion(library, "invalid-format"), ).rejects.toThrow(VersionNotFoundInStoreError); }); + + it("should resolve a partial version like '1.20' stored exactly as-is", async () => { + mockStore.queryUniqueVersions.mockResolvedValue(["1.20", "2.0.0"]); + mockStore.checkDocumentExists.mockResolvedValue(false); + + const result = await docService.findBestVersion(library, "1.20"); + // Must return the original stored string ("1.20"), not a coerced + // "1.20.0" — downstream lookups key off what's actually in the store. + expect(result).toEqual({ bestMatch: "1.20", hasUnversioned: false }); + }); + + it("should pick the highest partial version when no target is given", async () => { + mockStore.queryUniqueVersions.mockResolvedValue(["1.20", "5", "2.0.0"]); + mockStore.checkDocumentExists.mockResolvedValue(false); + + const result = await docService.findBestVersion(library); + expect(result).toEqual({ bestMatch: "5", hasUnversioned: false }); + }); + + it("should still fall through to unversioned for a non-semver label like 'stable'", async () => { + // "stable" is not a version at all — it should never resolve as a + // bestMatch, with or without the partial-version fix. + mockStore.queryUniqueVersions.mockResolvedValue(["1.20", "stable"]); + mockStore.checkDocumentExists.mockResolvedValue(true); + + const result = await docService.findBestVersion(library, "stable"); + expect(result).toEqual({ bestMatch: null, hasUnversioned: true }); + }); }); describe("listLibraries", () => { diff --git a/src/store/DocumentManagementService.ts b/src/store/DocumentManagementService.ts index 683cc5c21..5f5871fad 100644 --- a/src/store/DocumentManagementService.ts +++ b/src/store/DocumentManagementService.ts @@ -268,7 +268,11 @@ export class DocumentManagementService { */ async listVersions(library: string): Promise { const versions = await this.store.queryUniqueVersions(library); - const validVersions = versions.filter((v) => semver.valid(v)); + // Accept anything coercible to semver (e.g. "1.20", "5"), not just strict + // X.Y.Z strings — otherwise real-world partial versions are silently + // dropped and become invisible to findBestVersion(). Non-version labels + // like "stable"/"latest" correctly still fail coercion and stay excluded. + const validVersions = versions.filter((v) => semver.coerce(v) !== null); return sortVersionsDescending(validVersions); } @@ -320,8 +324,22 @@ export class DocumentManagementService { let bestMatch: string | null = null; + // semver.maxSatisfying() requires every candidate to itself be a fully + // valid X.Y.Z semver string — a stored version like "1.20" fails that + // even though listVersions() now accepts it. Match on coerced versions, + // then map the winner back to its original stored string so downstream + // lookups (e.g. checkDocumentExists) use the string that's actually in + // the store. + const coercedToOriginal = new Map(); + for (const v of versionStrings) { + const coerced = semver.coerce(v); + if (coerced) coercedToOriginal.set(coerced.version, v); + } + const coercedCandidates = Array.from(coercedToOriginal.keys()); + if (!targetVersion || targetVersion === "latest") { - bestMatch = semver.maxSatisfying(versionStrings, "*"); + const coercedBest = semver.maxSatisfying(coercedCandidates, "*"); + bestMatch = coercedBest ? (coercedToOriginal.get(coercedBest) ?? null) : null; } else { const versionRegex = /^(\d+)(?:\.(?:x(?:\.x)?|\d+(?:\.(?:x|\d+))?))?$|^$/; if (!semver.valid(targetVersion) && !versionRegex.test(targetVersion)) { @@ -338,7 +356,8 @@ export class DocumentManagementService { range = `${range} || <=${targetVersion}`; } // If it was already a valid range (like '1.x'), use it directly - bestMatch = semver.maxSatisfying(versionStrings, range); + const coercedBest = semver.maxSatisfying(coercedCandidates, range); + bestMatch = coercedBest ? (coercedToOriginal.get(coercedBest) ?? null) : null; } }