From 9ae09b3e07346a8c48fb6187d8d837c38c03f513 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=99=B4=E7=8C=AB?= Date: Thu, 12 Nov 2026 05:37:18 +0900 Subject: [PATCH] fix(dsh-ledger): ignore the pnpm peer suffix when comparing lock versions and keep it when rewriting pnpm appends a peer-resolution suffix to an importer's resolved version when the package or one of its dependencies has a satisfied peer: `version: 0.1.213(@types/node@16.9.1)`. The ledger code took the whole scalar for the version. A healthy profile was reported as copy-vs-lock, and convergence (`bili doctor --fix`, the pass after an in-place update) rewrote the line to the bare version: a write where nothing had drifted, leaving the importer without the suffix its snapshots key carries. - splitPeerSuffix(): split the scalar at the first "(". - readPnpmLockResolution returns the version without the suffix, so the state and the findings carry a plain version. - patchNestedEntries compares the version part only; when the version really moves it swaps that part and keeps the suffix and the quoting the line already had. - tests/issue3496-dsh-ledger-convergence.test.ts: v9 fixture with a peer suffix; healthy triple gives no finding and zero writes; real drift changes only the two importer lines and keeps the suffix; multi-peer, quoted and v6-v8 nested shapes. packages:/snapshots: stay untouched as before (#3509, #3570). --- src/dsh-ledger.ts | 31 +++- .../issue3496-dsh-ledger-convergence.test.ts | 133 ++++++++++++++++++ 2 files changed, 160 insertions(+), 4 deletions(-) diff --git a/src/dsh-ledger.ts b/src/dsh-ledger.ts index 084b0c343..d3e95a342 100644 --- a/src/dsh-ledger.ts +++ b/src/dsh-ledger.ts @@ -82,6 +82,18 @@ function unquote(v: string): string { return v.replace(/^["']|["']$/g, ""); } +/** Split a pnpm lockfile `version:` value into the resolved version and the + * peer-resolution suffix pnpm appends when the package (or something it + * depends on) has peers — `1.2.3(@types/node@16.9.1)` → ["1.2.3", + * "(@types/node@16.9.1)"]. The suffix names which peer set this install was + * resolved against; it is NOT part of the version, so every comparison with + * the on-disk copy must ignore it, and a rewrite must carry it over (the + * `snapshots:` key it points at carries the same suffix). */ +function splitPeerSuffix(v: string): [version: string, peerSuffix: string] { + const at = v.indexOf("("); + return at > 0 ? [v.slice(0, at), v.slice(at)] : [v, ""]; +} + /** Plain-YAML-safe scalar: quote anything that could be misread (leading * indicators like `>` or the alias indicator `*`, whitespace, …). Versions * and ^/~ specs stay bare; a leading `*` is quoted because YAML would parse @@ -125,6 +137,8 @@ interface LockResolution { * - flat (lockfile ≤5.4): same-line values — under the top-level * `specifiers:` section the value is the declared spec, under the flat * `dependencies:` map it is the RESOLVED version. + * The resolved version is returned WITHOUT pnpm's peer suffix + * (`1.2.3(@types/node@16.9.1)` reads as `1.2.3`) — see splitPeerSuffix. * Returns undefined when no lockfile exists or records no entry. */ function readPnpmLockResolution(profileDir: string): LockResolution | undefined { let text: string; @@ -158,7 +172,7 @@ function readPnpmLockResolution(profileDir: string): LockResolution | undefined const spec = inner.match(/^\s*specifier:\s*(\S+)/); if (spec && out.specifier === undefined) out.specifier = unquote(spec[1]); const ver = inner.match(/^\s*version:\s*(\S+)/); - if (ver && out.version === undefined) out.version = unquote(ver[1]); + if (ver && out.version === undefined) out.version = splitPeerSuffix(unquote(ver[1]))[0]; } } return out.specifier !== undefined || out.version !== undefined ? out : undefined; @@ -295,9 +309,18 @@ function patchNestedEntries(lines: string[], from: number, to: number, spec: str const vm = inner.match(/^([ \t]*)version:[ \t]*(\S.*)?$/); if (vm !== null) { sawVersion = true; - if (unquote(vm[2] ?? "") !== version) { - lines[j] = `${vm[1]}version: ${yamlScalar(version)}`; - details.push({ file: "pnpm-lock.yaml", field: "version", before: unquote(vm[2] ?? ""), after: version }); + // Compare and swap the version part only: a peer suffix + // (`1.2.3(@types/node@16.9.1)`) is kept verbatim, in the + // quoting style the line already has (pnpm writes it bare; + // yamlScalar would quote it and churn the line). + const raw = (vm[2] ?? "").trimEnd(); + const cur = unquote(raw); + const [curVersion, peerSuffix] = splitPeerSuffix(cur); + if (curVersion !== version) { + const quote = /^["']/.test(raw) ? raw[0] : ""; + const next = peerSuffix === "" ? yamlScalar(version) : `${quote}${version}${peerSuffix}${quote}`; + lines[j] = `${vm[1]}version: ${next}`; + details.push({ file: "pnpm-lock.yaml", field: "version", before: cur, after: `${version}${peerSuffix}` }); } } } diff --git a/tests/issue3496-dsh-ledger-convergence.test.ts b/tests/issue3496-dsh-ledger-convergence.test.ts index 64866227a..dd7de54a3 100644 --- a/tests/issue3496-dsh-ledger-convergence.test.ts +++ b/tests/issue3496-dsh-ledger-convergence.test.ts @@ -92,6 +92,51 @@ function pnpmLockV9(spec: string, version: string): string { ].join("\n"); } +/** pnpm v9 lockfile where the importer's resolution carries a PEER SUFFIX — + * what pnpm writes when billion-context (or a dependency of it) has a peer + * that the profile satisfies: `version: 0.1.197(@types/node@16.9.1)`, with + * the suffixed key under `snapshots:` and the bare key under `packages:`. */ +function pnpmLockV9Peer(spec: string, version: string, peerSuffix = "(@types/node@16.9.1)"): string { + return [ + "lockfileVersion: '9.0'", + "", + "settings:", + " autoInstallPeers: false", + " excludeLinksFromLockfile: false", + "", + "importers:", + "", + " .:", + " dependencies:", + " '@types/node':", + " specifier: 16.9.1", + " version: 16.9.1", + " billion-context:", + ` specifier: ${spec}`, + ` version: ${version}${peerSuffix}`, + " other-dep:", + " specifier: ^1.2.3", + " version: 1.2.3(@types/node@16.9.1)", + "", + "packages:", + "", + " '@types/node@16.9.1':", + " resolution: {integrity: sha512-fake==}", + "", + ` billion-context@${version}:`, + " resolution: {integrity: sha512-fake==}", + "", + "snapshots:", + "", + " '@types/node@16.9.1': {}", + "", + ` billion-context@${version}${peerSuffix}:`, + " dependencies:", + " '@types/node': 16.9.1", + "", + ].join("\n"); +} + /** Flat pnpm ≤5.4 lockfile (same-line values). */ function pnpmLockFlat(spec: string, version: string): string { return [ @@ -278,6 +323,94 @@ test("#3496: healthy triple — ZERO writes, byte-identical files", () => { } }); +test("#3496: healthy triple with a pnpm peer suffix on the lock version — no finding, ZERO writes", () => { + const root = mkdtempSync(path.join(tmpdir(), "bc-i3496-peer-a-")); + try { + // `version: 0.1.197(@types/node@16.9.1)` IS v0.1.197 — the suffix names + // the peer set, not a different version. Reading it as one made every + // such profile a permanent copy-vs-lock finding, and made convergence + // strip the suffix (pointing the importer at a snapshots key that does + // not exist). + const dir = makeProfile(root, "peer", { spec: "^0.1.197", copyVersion: "0.1.197", pnpmLock: pnpmLockV9Peer("^0.1.197", "0.1.197") }); + assert.equal(readDshLedgerState(dir).lockVersion, "0.1.197", "the peer suffix is not part of the resolved version"); + assert.deepEqual(dshProfileLedgerFindings(dir), []); + const before = profileFiles(dir); + const beforeMt = profileMtimes(dir); + const res = convergeDshLedger(dir, "0.1.197"); + assert.equal(res.error, undefined); + assert.deepEqual(res.changed, []); + assert.deepEqual(res.details, []); + assertZeroWrites(dir, before, beforeMt); + } finally { + rmrf(root); + rmrf(__cacheRoot); + } +}); + +test("#3496: peer-suffixed lock version — real drift swaps the version part only, the suffix survives", () => { + const root = mkdtempSync(path.join(tmpdir(), "bc-i3496-peer-b-")); + try { + const dir = makeProfile(root, "peerdrift", { spec: "^0.1.197", copyVersion: "0.1.197", pnpmLock: pnpmLockV9Peer("^0.1.196", "0.1.196") }); + const findings = dshProfileLedgerFindings(dir); + assert.deepEqual(findings.map((f) => f.kind), ["copy-vs-lock", "manifest-vs-lock"]); + assert.equal(findings[0]!.lockVersion, "0.1.196", "findings name the version, not the peer set"); + const before = readFileSync(path.join(dir, "pnpm-lock.yaml"), "utf8"); + const res = convergeDshLedger(dir, "0.1.197"); + assert.equal(res.error, undefined); + assert.deepEqual(res.changed, ["pnpm-lock.yaml"]); + assert.deepEqual(res.details.map((d) => [d.field, d.before, d.after]), [ + ["specifier", "^0.1.196", "^0.1.197"], + ["version", "0.1.196(@types/node@16.9.1)", "0.1.197(@types/node@16.9.1)"], + ]); + const after = readFileSync(path.join(dir, "pnpm-lock.yaml"), "utf8"); + // Exactly the two billion-context importer lines move; the version line + // stays bare (pnpm's own style — no quoting churn) and keeps its suffix. + const expected = before + .replace(" specifier: ^0.1.196", " specifier: ^0.1.197") + .replace(" version: 0.1.196(@types/node@16.9.1)", " version: 0.1.197(@types/node@16.9.1)"); + assert.equal(after, expected); + assert.ok(after.includes(" version: 1.2.3(@types/node@16.9.1)"), "another dependency's suffixed version is not ours to touch"); + assert.ok(after.includes(" billion-context@0.1.196(@types/node@16.9.1):"), "store bookkeeping sections stay untouched (#3570)"); + assert.deepEqual(dshProfileLedgerFindings(dir), [], "re-audit after convergence must be clean"); + // Second pass is a no-op: the suffixed record is already in step. + const mt = profileMtimes(dir); + const again = convergeDshLedger(dir, "0.1.197"); + assert.deepEqual(again.changed, []); + assertZeroWrites(dir, profileFiles(dir), mt); + } finally { + rmrf(root); + rmrf(__cacheRoot); + } +}); + +test("#3496: patchPnpmLock peer suffix — multi-peer and quoted values keep their shape; v6–v8 nested entries too", () => { + // several peers chain their parentheses; the whole tail is the suffix + const multi = pnpmLockV9Peer("^0.1.196", "0.1.196", "(@types/node@16.9.1)(zod@4.1.8)"); + const inStep = patchPnpmLock(multi, "^0.1.196", "0.1.196"); + assert.equal(inStep.kind, "ok"); + assert.deepEqual(inStep.kind === "ok" ? inStep.details : undefined, [], "same version + suffix is in step — no details, so no write"); + const moved = patchPnpmLock(multi, "^0.1.196", "0.1.197"); + assert.equal(moved.kind, "ok"); + if (moved.kind === "ok") { + assert.ok(moved.next.includes(" version: 0.1.197(@types/node@16.9.1)(zod@4.1.8)\n")); + assert.deepEqual(moved.details.map((d) => [d.field, d.before, d.after]), [ + ["version", "0.1.196(@types/node@16.9.1)(zod@4.1.8)", "0.1.197(@types/node@16.9.1)(zod@4.1.8)"], + ]); + } + // a quoted value keeps its quotes + const quoted = pnpmLockV9Peer("^0.1.196", "0.1.196").replace("version: 0.1.196(@types/node@16.9.1)", "version: '0.1.196(@types/node@16.9.1)'"); + const q = patchPnpmLock(quoted, "^0.1.196", "0.1.197"); + assert.equal(q.kind, "ok"); + if (q.kind === "ok") assert.ok(q.next.includes(" version: '0.1.197(@types/node@16.9.1)'\n")); + // v6–v8: nested entries under the top-level dependencies map carry the same suffix form + const v6 = pnpmLockV6("^0.1.196", "0.1.196").replace(" version: 0.1.196", " version: 0.1.196(@types/node@16.9.1)"); + const v6Same = patchPnpmLock(v6, "^0.1.196", "0.1.196"); + assert.deepEqual(v6Same.kind === "ok" ? v6Same.details : undefined, []); + const v6Moved = patchPnpmLock(v6, "^0.1.196", "0.1.197"); + assert.equal(v6Moved.kind, "ok"); + if (v6Moved.kind === "ok") assert.ok(v6Moved.next.includes(" version: 0.1.197(@types/node@16.9.1)\n")); +}); + test("#3496: flat pnpm ≤5.4 lockfile converges both same-line sections", () => { const root = mkdtempSync(path.join(tmpdir(), "bc-i3496-e-")); try {