mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-01 15:32:11 +08:00
fix(upstream-merge): preserve ours' key order in package.json dep merge
Seeding the result with theirs' keys and appending ours-only keys at the end caused kilo-only deps (e.g. rotating-file-stream in packages/core) to relocate from the middle of the deps block to the end during the pre-merge transform. Git's textual 3-way merge then saw ours keeping the line in place and theirs adding the same key elsewhere, producing a duplicate JSON key in the merged file. Iterate ours first so kilo-only deps stay in their original position, then append any theirs-only keys at the end.
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import { expect, test } from "bun:test"
|
||||
import { fixCatalog, fixScripts } from "./transform-package-json"
|
||||
import { fixCatalog, fixScripts, mergeWithNewestVersions } from "./transform-package-json"
|
||||
|
||||
test("fixScripts preserves Kilo-only root scripts from base", () => {
|
||||
const ours = {
|
||||
@@ -84,3 +84,33 @@ test("fixCatalog is a no-op when catalog is absent", () => {
|
||||
fixCatalog(pkg, "package.json", changes)
|
||||
expect(changes.length).toBe(0)
|
||||
})
|
||||
|
||||
test("mergeWithNewestVersions preserves ours' key order so kilo-only deps don't relocate", () => {
|
||||
// Regression: when ours has a kilo-only dep in the middle (e.g. rotating-file-stream
|
||||
// alphabetically between npm-package-arg and semver) and theirs lacks it, the merge
|
||||
// result must keep that key in its original position. Previously this function
|
||||
// started from theirs' keys and appended ours-only keys at the end, causing git's
|
||||
// textual 3-way merge to produce a duplicate JSON key.
|
||||
const ours = {
|
||||
"npm-package-arg": "13.0.2",
|
||||
"rotating-file-stream": "3.2.9",
|
||||
semver: "^7.6.3",
|
||||
zod: "catalog:",
|
||||
}
|
||||
const theirs = {
|
||||
"npm-package-arg": "13.0.2",
|
||||
semver: "^7.6.3",
|
||||
zod: "catalog:",
|
||||
}
|
||||
const changes: string[] = []
|
||||
const result = mergeWithNewestVersions(ours, theirs, changes, "dependencies")
|
||||
expect(Object.keys(result)).toEqual(["npm-package-arg", "rotating-file-stream", "semver", "zod"])
|
||||
})
|
||||
|
||||
test("mergeWithNewestVersions appends theirs-only keys at the end", () => {
|
||||
const ours = { a: "1.0.0", b: "1.0.0" }
|
||||
const theirs = { a: "1.0.0", c: "1.0.0" }
|
||||
const changes: string[] = []
|
||||
const result = mergeWithNewestVersions(ours, theirs, changes, "dependencies")
|
||||
expect(Object.keys(result)).toEqual(["a", "b", "c"])
|
||||
})
|
||||
|
||||
@@ -108,8 +108,14 @@ function compareVersions(a: string, b: string): number | null {
|
||||
/**
|
||||
* Merge two dependency objects using "newest wins" strategy
|
||||
* For non-comparable versions (URLs, catalog:, workspace:*), upstream (theirs) wins
|
||||
*
|
||||
* Key order preserves ours' order first (so kilo-only deps stay in their
|
||||
* original position), then appends theirs-only keys at the end. This avoids
|
||||
* relocating existing keys, which would otherwise let git's textual merge
|
||||
* produce duplicate JSON keys (ours keeps the line in place, theirs appears
|
||||
* to "add" the same key elsewhere → both survive the merge).
|
||||
*/
|
||||
function mergeWithNewestVersions(
|
||||
export function mergeWithNewestVersions(
|
||||
ours: Record<string, string> | undefined,
|
||||
theirs: Record<string, string> | undefined,
|
||||
changes: string[],
|
||||
@@ -117,38 +123,38 @@ function mergeWithNewestVersions(
|
||||
): Record<string, string> {
|
||||
const result: Record<string, string> = {}
|
||||
|
||||
// Start with all of theirs
|
||||
if (theirs) {
|
||||
for (const [name, version] of Object.entries(theirs)) {
|
||||
result[name] = version
|
||||
// Seed with ours' keys in ours' order, applying newest-wins per key.
|
||||
if (ours) {
|
||||
for (const [name, ourVersion] of Object.entries(ours)) {
|
||||
const theirVersion = theirs?.[name]
|
||||
if (theirVersion === undefined) {
|
||||
result[name] = ourVersion
|
||||
changes.push(`${section}: preserved ${name}@${ourVersion} (kilo-only)`)
|
||||
continue
|
||||
}
|
||||
if (ourVersion === theirVersion) {
|
||||
result[name] = theirVersion
|
||||
continue
|
||||
}
|
||||
const cmp = compareVersions(ourVersion, theirVersion)
|
||||
if (cmp === null) {
|
||||
result[name] = theirVersion
|
||||
changes.push(`${section}: ${name} kept upstream ${theirVersion} (special format)`)
|
||||
} else if (cmp > 0) {
|
||||
result[name] = ourVersion
|
||||
changes.push(`${section}: ${name} ${theirVersion} -> ${ourVersion} (kilo newer)`)
|
||||
} else {
|
||||
result[name] = theirVersion
|
||||
if (cmp < 0) changes.push(`${section}: ${name} kept upstream ${theirVersion} (upstream newer)`)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Merge in ours, keeping newer versions
|
||||
if (ours) {
|
||||
for (const [name, ourVersion] of Object.entries(ours)) {
|
||||
const theirVersion = result[name]
|
||||
|
||||
if (!theirVersion) {
|
||||
// Dependency only exists in ours - keep it
|
||||
result[name] = ourVersion
|
||||
changes.push(`${section}: preserved ${name}@${ourVersion} (kilo-only)`)
|
||||
} else if (ourVersion !== theirVersion) {
|
||||
// Both have it with different versions - compare
|
||||
const comparison = compareVersions(ourVersion, theirVersion)
|
||||
|
||||
if (comparison === null) {
|
||||
// Can't compare (special format) - upstream wins per user preference
|
||||
changes.push(`${section}: ${name} kept upstream ${theirVersion} (special format)`)
|
||||
} else if (comparison > 0) {
|
||||
// Ours is newer
|
||||
result[name] = ourVersion
|
||||
changes.push(`${section}: ${name} ${theirVersion} -> ${ourVersion} (kilo newer)`)
|
||||
} else if (comparison < 0) {
|
||||
// Theirs is newer - already in result
|
||||
changes.push(`${section}: ${name} kept upstream ${theirVersion} (upstream newer)`)
|
||||
}
|
||||
// If equal, keep theirs (already in result)
|
||||
// Append any theirs-only keys at the end, preserving theirs' relative order.
|
||||
if (theirs) {
|
||||
for (const [name, version] of Object.entries(theirs)) {
|
||||
if (result[name] === undefined) {
|
||||
result[name] = version
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user