Conversation
check-pi-dependencies and check-pi-patches decided whether a Pi package was
installed as pnpm's patched instance by looking for a `patch_hash=` segment
in the resolved symlink. pnpm shortens the `.pnpm` entry directory when a
path gets too long, which drops that segment, so a correctly patched
install was reported as unpatched on Windows.
Read the hashes from `node_modules/.pnpm/lock.yaml` instead. pnpm keys every
patched snapshot as `name@version(patch_hash=<hex>)`, so the lockfile keeps
the full hash regardless of how the store directory is named. Both checks
now also fail with a distinct message when nothing patched is recorded at
all, rather than only when the hash is missing from pnpm-lock.yaml.
The fork test in native-pi-session.test.ts derived the publication filename
with `foreignPath.split("/")`, which cannot split a Windows path, so it
compared a whole `D:\...` path against a bare readdir entry. `path.basename`
gives the same answer on POSIX and fixes the comparison on Windows.
Regression coverage builds a workspace whose `.pnpm` entry names are
shortened the way a long Windows path forces, and asserts both scripts
accept it and still reject an unpatched install or a hash pnpm-lock.yaml
does not record.
fixes vastsa#1361
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(#1361):让 Pi patch 检查与路径平台无关,并修正 fork 测试里的路径断言
感谢 @cnjackli 的定位。两处问题都确认属实,我按建议的思路实现了修复,并补上了回归测试。
1.
check:pi-dependencies/check:pi-patches误报 patched 实例两个脚本原先用「
realpathSync(...)的结果里是否含字面patch_hash=」来判断装的是不是 pnpm 的 patched 实例。pnpm 在路径过长时会缩短.pnpm条目目录名(Windows 长路径下必然触发),缩短后路径里就没有patch_hash=这一段了,于是正确打过补丁的安装被判成未打补丁。改为读
node_modules/.pnpm/lock.yaml。pnpm 给每个 patched 快照的键就是'name@version(patch_hash=<64hex>)(...)',无论条目目录名怎么缩短,这个键都保留完整 hash —— 也就是 issue 里建议的虚拟店 lockfile 证据。顺带把两类失败区分开了:
is not installed as a patched instancepnpm-lock.yaml里 →absent from pnpm-lock.yaml: <hash>Linux 上的判定结果不变,只是证据来源从「路径字符串」换成了 lockfile 键。
2.
native-pi-session.test.ts的 Windows 路径断言L784 原来用
foreignPath.split("/").at(-1)!取文件名。Windows 的分隔符是\,切不开,.at(-1)返回整条绝对路径,再去和readdirSync返回的裸文件名比较,必然不等。改成path.basename(foreignPath):POSIX 上结果完全相同(顺带去掉了!断言),Windows 上正确。回归测试
新增
scripts/pi-patch-hash.test.mjs,7 个 case:.pnpm条目名被缩短(不含patch_hash=)的 workspace,断言两个脚本都通过 —— 这正是 Windows 上的形态pnpm-lock.yaml里的情况两个 check 脚本加了
--root <dir>,让测试能指向 fixture(沿用check-pr-base-main.mjs的--cwd写法)。测试已挂到ci.yml的jsjob。按 AGENTS.md §12「先写会在旧代码上失败的测试」的要求,我实测过:把
git show HEAD:的修复前脚本放进同一个 fixture 目录跑,两个都失败,报错正是 issue 里描述的那两条 ——does not resolve to pnpm's patched package instance和installed patch hash is absent from pnpm-lock.yaml;换成修复后的脚本则通过。实际跑过的验证
node --test scripts/pi-patch-hash.test.mjs→ 7 passednode --check三个脚本语法通过node scripts/check-pr-base-main.mjs→ passed(head 含最新 origin/main)pnpm-lock.yaml,以及本机 pnpm 12 实打实装出来的一个 patched 依赖核对过,两种键格式都正确CI
本机磁盘只剩 1.6G,装不下
pnpm install,所以 lint / build / typecheck / vitest 都交给 CI。四个 job 全绿:Head contains latest baseDocs checksJS build / typecheck / lint / architecture / testUnit-test the Pi patch checks)Rust host-core format / lint / test其中 Typecheck、Lint、Build、architecture budgets、新的回归测试、以及包含被改的
native-pi-session.test.ts的 vitest 全套,都在上面那个 JS job 里跑过并通过了。我也没有 Windows 机器:
basename那处改动在 POSIX 上可证等价,脚本那处是靠 fixture 覆盖 shortened 目录名的形态来验证的,不是真机 Windows 验证。