From 2e4b59399a5f0babb2a21cde5237f1708a620060 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Catriel=20M=C3=BCller?= Date: Thu, 11 Jun 2026 11:31:04 -0300 Subject: [PATCH] fix: address Bun merge validation review --- script/upstream/merge.ts | 11 +++++++++-- .../transforms/transform-package-json.test.ts | 14 +++++++++++--- .../upstream/transforms/transform-package-json.ts | 5 ++--- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/script/upstream/merge.ts b/script/upstream/merge.ts index ca30c69d87..d6eae52ee2 100644 --- a/script/upstream/merge.ts +++ b/script/upstream/merge.ts @@ -213,10 +213,17 @@ function manager(content: string): string | undefined { return typeof pkg.packageManager === "string" ? pkg.packageManager : undefined } +async function managerAt(ref: string): Promise { + const result = await $`git show ${ref}:package.json`.quiet().nothrow() + if (result.exitCode === 0) return manager(result.stdout.toString()) + logger.warn(`Could not read package.json at ${ref}; excluding it from Bun packageManager validation`) + return undefined +} + async function validateBun(base: string, upstream: string): Promise { const current = manager(await Bun.file("package.json").text()) - const ours = manager(await $`git show ${base}:package.json`.text()) - const theirs = manager(await $`git show ${upstream}:package.json`.text()) + const ours = await managerAt(base) + const theirs = await managerAt(upstream) assertBunPackageManager(current, ours, theirs) logger.success(`Validated Bun packageManager: ${current ?? "missing"}`) } diff --git a/script/upstream/transforms/transform-package-json.test.ts b/script/upstream/transforms/transform-package-json.test.ts index 59020c4366..0efa2feaeb 100644 --- a/script/upstream/transforms/transform-package-json.test.ts +++ b/script/upstream/transforms/transform-package-json.test.ts @@ -134,10 +134,10 @@ test("mergeWithNewestVersions appends theirs-only keys at the end", () => { expect(Object.keys(result)).toEqual(["a", "b", "c"]) }) -test("selectBunPackageManager keeps the newer Bun version", () => { +test("selectBunPackageManager keeps the newer Bun version and prefers Kilo on ties", () => { expect(selectBunPackageManager("bun@1.3.14", "bun@1.3.13")).toBe("bun@1.3.14") expect(selectBunPackageManager("bun@1.3.14", "bun@1.3.15")).toBe("bun@1.3.15") - expect(selectBunPackageManager("bun@1.3.14", "bun@1.3.14")).toBe("bun@1.3.14") + expect(selectBunPackageManager("bun@1.3.14+kilo", "bun@1.3.14+upstream")).toBe("bun@1.3.14+kilo") }) test("selectBunPackageManager preserves valid versions over malformed values", () => { @@ -152,7 +152,15 @@ test("fixPackageManager prevents root Bun downgrades", () => { const changes: string[] = [] fixPackageManager(pkg, "package.json", ours, changes) expect(pkg.packageManager).toBe("bun@1.3.14") - expect(changes).toEqual(["packageManager: bun@1.3.13 -> bun@1.3.14 (Kilo newer)"]) + expect(changes).toEqual(["packageManager: bun@1.3.13 -> bun@1.3.14 (preserved Kilo pin)"]) +}) + +test("fixPackageManager restores a valid Kilo pin over malformed upstream", () => { + const pkg: Record = { packageManager: "bun@latest" } + const changes: string[] = [] + fixPackageManager(pkg, "package.json", { packageManager: "bun@1.3.14" }, changes) + expect(pkg.packageManager).toBe("bun@1.3.14") + expect(changes).toEqual(["packageManager: bun@latest -> bun@1.3.14 (preserved Kilo pin)"]) }) test("fixPackageManager accepts upstream Bun upgrades", () => { diff --git a/script/upstream/transforms/transform-package-json.ts b/script/upstream/transforms/transform-package-json.ts index 7986a847db..8061f16e41 100644 --- a/script/upstream/transforms/transform-package-json.ts +++ b/script/upstream/transforms/transform-package-json.ts @@ -109,14 +109,13 @@ function bun(value: unknown): { value: string; version: string } | null { if (typeof value !== "string") return null const match = value.match(/^bun@(\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?)$/) if (!match) return null - if (compareVersions(match[1], match[1]) === null) return null return { value, version: match[1] } } export function selectBunPackageManager(ours: unknown, theirs: unknown): string | undefined { const left = bun(ours) const right = bun(theirs) - if (left && right) return compareVersions(left.version, right.version)! > 0 ? left.value : right.value + if (left && right) return compareVersions(left.version, right.version)! >= 0 ? left.value : right.value if (left) return left.value if (right) return right.value return undefined @@ -132,7 +131,7 @@ export function fixPackageManager( const next = selectBunPackageManager(ours?.packageManager, pkg.packageManager) if (!next || pkg.packageManager === next) return const prior = typeof pkg.packageManager === "string" ? pkg.packageManager : "missing or invalid" - changes.push(`packageManager: ${prior} -> ${next} (Kilo newer)`) + changes.push(`packageManager: ${prior} -> ${next} (preserved Kilo pin)`) pkg.packageManager = next }