Repository navigation
[PR] fix(dsh-ledger): ignore pnpm's peer suffix when comparing lockfile versions, preserve it when rewriting | fix(dsh-ledger): 比较 lockfile 版本时忽略 pnpm 的 peer 后缀，改写时保留它 - #6314
Conversation
…ions 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 (ranxianglei#3509, ranxianglei#3570).
ranxianglei
changed the title
There was a problem hiding this comment.
Note
Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.
🟢 Approval recommended
The change is narrowly scoped to handling pnpm peer-suffix metadata in importer version parsing/patching and is accompanied by regression tests for both healthy and drifted cases, with no outstanding review issues indicated.
No open issues 🎉
What changed in this PR
Fixes false-positive DSH ledger drift and unintended pnpm-lock.yaml rewrites when pnpm encodes peer-resolution metadata as a suffix in importer version: scalars (e.g. 0.1.213(@types/node@16.9.1)), by comparing only the resolved version while preserving any suffix during updates.
Changes:
- Add
splitPeerSuffix()and use it to ignore pnpm peer suffixes when reading nested importerversion:values. - Update pnpm lockfile patching to compare/swap only the version portion while preserving any existing peer suffix and line formatting.
- Add regression tests covering healthy and drifted profiles with peer-suffixed versions, including quoted and multi-peer suffix variants.
| File | Description |
|---|---|
| tests/issue3496-dsh-ledger-convergence.test.ts | Adds pnpm v9 peer-suffix fixtures and convergence test cases to ensure healthy profiles produce no findings/writes and drifted profiles preserve peer suffixes during fixes. |
| src/dsh-ledger.ts | Implements peer-suffix-aware parsing and patching for pnpm nested importer version: fields so comparisons ignore the suffix while rewrites retain it. |
Review effort: Balanced
BBleae
changed the title
ranxianglei
changed the title
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) Progress report — review of this PR is in flight (the previous run was force-restarted after a stalled tool call; nothing was lost, all findings so far are below). Dup check: not a duplicate — no other open issue/PR touches the pnpm peer-suffix handling in Verified so far:
In progress: running the test suite locally — the 27-case file at the PR head, plus a revert-to-master check that the 3 new cases fail without the change. Then CI status via the mirror cache and the final 8-question verdict. One reply with the full review coming shortly. |
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) Review of PR #6314 — verdict: mergeable (可合). Dup check: not a duplicate (no other open issue/PR touches the pnpm peer-suffix handling in the #3496 ledger). Labels applied: What I verified independently (not taken from the PR description):
One disclosed behavior change (intended, this is the fix): healthy suffixed profiles now produce zero writes where master wrote a corrupted bare version; 结构化八问
收工判定:可合——根因修复、双向测试钉死、CI 全绿、diff 干净,无决策依赖项,等 owner 合并即可。 |
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) Status confirmation (no new work): the full review verdict is already posted above (the "verdict: mergeable (可合)" comment) — bug verified in code, 27/27 tests at the PR head with exactly the 3 new cases failing on master's code, local gates clean, CI 30/30 green on sha |
Fixes #6320
修的是什么
包本身（或它依赖的某个包）有 peer、而项目又满足了这个 peer 时，pnpm 会在 importer 条目里解析出的版本后面写一段 peer 解析后缀：
src/dsh-ledger.ts原来把这一整串都当成版本：readPnpmLockResolution把0.1.213(@types/node@16.9.1)作为lockVersion返回，dshProfileLedgerFindings拿它和副本的0.1.213按字符串比较。健康的 profile 因此出一条copy-vs-lockfinding，bili doctor --fix（fixDshLedgers，master上这项审计的调用方）再把它当成漂移去修。patchNestedEntries也做同样的比较，所以convergeDshLedger（bili doctor --fix，以及原地更新之后的收敛）会把这一行改写成不带后缀的version: 0.1.213。这是在什么都没漂移的 profile 上写了一次文件，而且写完 importer 指向billion-context@0.1.213，snapshots:下的键却还是billion-context@0.1.213(@types/node@16.9.1)。这个后缀在真实的 DSH profile 里就有（pnpm v9 lockfile，
(@types/node@…)），不是少见的形状。改动
splitPeerSuffix()，在第一个(处把这串切开。后缀说的是这次安装按哪组 peer 解析，不属于版本。readPnpmLockResolution返回去掉后缀的版本，state 和 finding 里的lockVersion都是纯版本号。patchNestedEntries只比较版本部分。真要挪版本时，只换版本那一段，后缀和这一行原来的引号风格都保留（pnpm 写的是不带引号的值，交给yamlScalar会被加上引号）。扁平格式（lockfile <= 5.4）和 npm lockfile 的路径没动。没有改动
kernel/**。测试
在
tests/issue3496-dsh-ledger-convergence.test.ts里加了三个用例，用一个 v9 fixture：importer 的版本带 peer 后缀，packages:下是不带后缀的键，snapshots:下是带后缀的键。lockVersion读成纯版本，没有 finding，零写入（字节和 mtime 都不变）；billion-context的那两行变了，后缀保留且不加引号，另一个依赖带后缀的版本没被碰，再审计是干净的，第二遍什么都不写；patchPnpmLock：多个串起来的 peer、带引号的值、以及 v6-v8 的嵌套条目。三个用例在
master上都失败，加上改动后都通过。这个文件加上改动是 27/27（不加是 24/27）。npm run typecheck、check:unused-exports、check:test-import-graph和git diff --check都干净。另外在本地 Windows（Node 22，离线）上把整套tests/*.test.ts跑了一遍做粗查：这个模块里和它附近的测试没有失败；那次跑出来的几处失败是环境造成的（Windows、离线），都在无关的测试里。整套的结论以 CI 为准。这个 PR 不做的
packages:/snapshots:仍然不动，这是 #3509 定下的，跟踪在 #3570（已关闭）。带 peer 后缀时，真挪了版本以后的情况和那边一样：importer 写的是billion-context@<new>(<peers>)，snapshots:下没有这个键（原来丢掉后缀的做法也避不开：不带后缀的键同样不在snapshots:下）。我在本地 fixture 里看了 pnpm 10.20.0 对这种 lockfile 的处理：普通的pnpm install先打印Broken lockfile: no entry for 'billion-context@0.1.213(@types/node@16.9.1)'，再打印The lockfile is broken! Resolution step will be performed to fix it.并重新解析，和 #3570 记录的 pnpm 9 / 11 的表现一致；pnpm install --frozen-lockfile停在ERR_PNPM_LOCKFILE_MISSING_DEPENDENCY。（fixture 是离线的，所以我只看到重新解析开始，没看到它跑完。）为了让这个 PR 保持小，只做了比较和后缀这两处。如果还是想要 #3570 里三段一起同步的做法，说一声，我可以另开一个 PR 跟进。