Skip to content

fix(#1361): 让 Pi patch 检查与路径平台无关,并修正 fork 测试里的路径断言 - #1363

Open
Yi-111-a wants to merge 1 commit into
vastsa:mainfrom
Yi-111-a:fix/pi-patch-hash-read-virtual-store
Open

Yi-111-a wants to merge 1 commit into
vastsa:mainfrom
Yi-111-a:fix/pi-patch-hash-read-virtual-store

Conversation

@Yi-111-a

@Yi-111-a Yi-111-a commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

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 证据。

顺带把两类失败区分开了:

  • 虚拟店 lockfile 里根本没有该包的 patched 快照 → is not installed as a patched instance
  • 有快照但 hash 不在 pnpm-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:

  • helper 读 scoped / 非 scoped 快照键、只取该版本的 hash、版本号按字面匹配
  • 构造一个 .pnpm 条目名被缩短(不含 patch_hash=)的 workspace,断言两个脚本都通过 —— 这正是 Windows 上的形态
  • 断言两个脚本仍然拒绝:没有 patched 快照的安装、以及 hash 不在 pnpm-lock.yaml 里的情况

两个 check 脚本加了 --root <dir>,让测试能指向 fixture(沿用 check-pr-base-main.mjs 的 --cwd 写法)。测试已挂到 ci.yml 的 js job。

按 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 passed
  • 修复前脚本在同 fixture 上失败(见上)
  • node --check 三个脚本语法通过
  • node scripts/check-pr-base-main.mjs → passed(head 含最新 origin/main)
  • helper 的正则对着仓库真实的 pnpm-lock.yaml,以及本机 pnpm 12 实打实装出来的一个 patched 依赖核对过,两种键格式都正确

CI

本机磁盘只剩 1.6G,装不下 pnpm install,所以 lint / build / typecheck / vitest 都交给 CI。四个 job 全绿:

Check 结果
Head contains latest base pass
Docs checks pass
JS build / typecheck / lint / architecture / test pass(含新增的 Unit-test the Pi patch checks)
Rust host-core format / lint / test pass

其中 Typecheck、Lint、Build、architecture budgets、新的回归测试、以及包含被改的 native-pi-session.test.ts 的 vitest 全套,都在上面那个 JS job 里跑过并通过了。

我也没有 Windows 机器:basename 那处改动在 POSIX 上可证等价,脚本那处是靠 fixture 覆盖 shortened 目录名的形态来验证的,不是真机 Windows 验证。

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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant