fix(tooling): stop a .claude/worktrees git worktree from breaking pnpm lint and test:workflows (#439) - #442
Conversation
…al gates A git worktree under `.claude/worktrees/` — Claude Code's default location — is a full nested checkout, and it broke two gates independently. Since `pnpm lint` runs in both the pre-commit and pre-push hooks, a maintainer in that state could not commit or push without `--no-verify`. 1. `pnpm lint`: the worktree carries its own biome.json, so root `biome check .` aborted with "Found a nested root configuration". That is a configuration error, not a lint finding — the run stops before checking any file. 2. `pnpm test:workflows`: `git ls-files --others` reports an untracked directory as one trailing-slash entry rather than recursing, and listPublicClaudeAssets() hands those paths to readFileSync, so two tests died with EISDIR pointing at the helper rather than the directory. Both levers are needed. .gitignore alone does not fix Biome: biome.json sets no vcs.useIgnoreFile, so Biome never reads .gitignore, and nested-config discovery is independent of ignore rules. Verified by trying each fix alone. The asset helper also drops directory entries now, so any other stray directory surfaces as an assertion failure instead of crashing the suite. Verified with the worktree in place rather than moved aside: `biome check .` exits 0 over 417 files, and test:workflows is 39/39. The new test pins all three levers and was checked against copies with each one reverted. Closes #439
There was a problem hiding this comment.
代码评审报告: fix(tooling): stop a .claude/worktrees git worktree from breaking pnpm lint and test:workflows (#439)
风险等级: 低
处理建议: 评论
决策摘要: 可以合并——两条修复分别对应 issue #439 的两个独立根因,无应用代码、无外部调用方;但新增测试的第三条断言是重言式,并没有真正 pin 住 helper 这条"防御纵深"改动,建议合并前修正或降级其宣称。
级联分析
- 变更符号:
listPublicClaudeAssets()(scripts/tests/workflow-rules.test.mjs:19,文件内私有函数,未 export);配置项biome.jsonfiles.includes、.gitignore忽略规则。 - 受影响流程:
pnpm lint(biome check .) → pre-commit/pre-push 钩子quality:precommit;pnpm test:workflows;CI.github/workflows/ci.yml:28的pnpm quality:ci。另有scripts/workflows/contract-check.mjs:142,152同样消费git ls-files --others --exclude-standard,因.gitignore这一杠杆顺带受益(分类走classifyContractPaths,不做readFileSync,原本不会崩,只是噪声)。 - 变更集外调用方: 无。helper 的全部调用点均在同文件(
:510/:514/:527/:541,text search 确认);pnpm-workspace.yaml仅packages/*、apps/*,嵌套 checkout 不会被 pnpm/turbo 当作 workspace 包,test:workflows的 glob 限定在scripts/tests/。 - 置信度: medium —(text search + 文件读取,无代码图谱);本 checkout 无
node_modules,无法独立复跑biome check .验证!!通配语义,依赖作者报告的 417 files / exit 0(该证据本身有说服力:嵌套配置错误会整体中止,退出 0 即证明排除生效)。
问题发现
-
[中] 新增测试的第三条断言无法失败,没有 pin 住 helper 改动
- 证据:
scripts/tests/workflow-rules.test.mjs:510断言listPublicClaudeAssets().every((file) => !file.endsWith('/'))。该 helper 自身已在:29过滤掉尾斜杠条目,因此断言对当前实现恒真;而一旦单独回退:29的过滤,由于杠杆 1(.gitignore)已生效,git ls-files --others --exclude-standard .claude也不会再报出 worktree 目录条目,断言依然通过。PR 描述称"三个修复各自单独回退都会让对应断言失败",对这一条不成立(同时回退.gitignore时,:501会先抛出并中止本测试,看起来像失败,但失败的是第一条断言)。 - 受影响调用方/流程: 仅测试可信度。helper 的其他三个调用点(
:514/:527/:541)行为不变。 - 最小可行修复: 把过滤逻辑抽成纯函数(如
dropDirectoryEntries(entries))并对合成输入断言,例如assert.deepEqual(dropDirectoryEntries(['.claude/skills/a.md', '.claude/worktrees/x/']), ['.claude/skills/a.md']);或直接删掉这条断言,改为在 PR 说明中承认 helper 改动无回归测试覆盖。
- 证据:
-
[低] 注释与 CHANGELOG 把
git ls-files --others的行为说宽了- 证据:
scripts/tests/workflow-rules.test.mjs:25-28、:508-509与CHANGELOG.md:18均表述为"git ls-files --others把未跟踪目录报成一条尾斜杠条目"。默认情况下git ls-files --others会递归进普通未跟踪目录并逐个列出文件;折叠成单条目录条目发生在该目录是嵌套仓库(含.git文件/目录,如 linked worktree 或未初始化 submodule)时,或显式使用--directory时。 - 受影响调用方/流程: 无行为影响;但由此推出的"任何其他游离目录现在都会变成断言失败而不是崩溃"(PR 描述与 CHANGELOG)过度宣称——普通游离目录本来就不会产生目录条目。
- 最小可行修复: 注释与 CHANGELOG 中把"未跟踪目录"改为"未跟踪的嵌套仓库(worktree/submodule)"。
- 证据:
-
[低]
.gitignore规则是根锚定的,biome.json规则不是,两者覆盖范围不对齐- 证据:
.gitignore:55的.claude/worktrees/含中间斜杠,按 gitignore 语义锚定到仓库根;biome.json:62的!!**/.claude/worktrees匹配任意深度。若在子目录(如apps/desktop/)启动 Claude Code,worktree 落在apps/desktop/.claude/worktrees/,Biome 侧已覆盖,git 侧未覆盖。 - 受影响调用方/流程: 影响有限——
listPublicClaudeAssets()只列根.claude,不会 EISDIR;contract-check.mjs:142,152会把该目录条目计入 changed files,经classifyContractPaths归为非关键,属噪声而非失败。 - 最小可行修复: 若要与 Biome 对齐,
.gitignore改为**/.claude/worktrees/;:502的 check-ignore 断言在两种写法下都通过。
- 证据:
行级发现
- [scripts/tests/workflow-rules.test.mjs:510] 该断言对现实现恒真:helper 已在
:29过滤尾斜杠,且.gitignore杠杆生效后git ls-files --others --exclude-standard .claude也不会再产出目录条目,单独回退:29时它仍会通过。把过滤抽成纯函数并对合成输入断言,或删除这条断言。 - [scripts/tests/workflow-rules.test.mjs:25] 表述过宽:
git ls-files --others默认会递归普通未跟踪目录并逐文件列出;折叠为一条尾斜杠条目是"嵌套仓库"(worktree/submodule)或--directory的行为。改为"未跟踪的嵌套仓库",并同步收敛CHANGELOG.md:18的"任何其他游离目录"说法。 - [.gitignore:55] 该模式含中间斜杠,仅锚定仓库根,与
biome.json:62的**/.claude/worktrees覆盖范围不一致;若要覆盖从子目录启动 Claude Code 的情形,写成**/.claude/worktrees/。
Karpathy 评审
- 假设: 两条杠杆的必要性有明确论证且可证伪(Biome 未设
vcs.useIgnoreFile,不读.gitignore;嵌套 root config 发现独立于 ignore 规则),与 issue #439 的实验结论一致。未验证的隐含假设有两处:git check-ignore对"目录专用模式 + 路径不存在"的匹配行为(见"缺失覆盖"),以及pnpm contract:local的 GitNexus analyze 是否遵守.gitignore。 - 简洁性: 无猜测性抽象。三处改动都是最小杠杆,没有引入 flag、mode 或新层级;helper 只增加一次
filter,未破坏既有 dedupe/sort 语义。 - 结构质量: 无退化。配置项与既有
!!**/node_modules、!!**/dist同一约定同一位置;测试放在相邻的 ignore 类测试之后;.gitignore与 biome 注释互相指向,可追溯。无文件膨胀、无重复 helper、无逻辑错层。 - 变更范围: 与 issue #439 的三条 proposed fix 一一对应,4 文件 +34/−2,无无关重构或格式噪声,无应用代码。
- 验证: 验收标准(
biome check .与pnpm test:workflows在 worktree 在位时通过)有端到端证据——本分支的 commit/push 走完了真实钩子。弱点:新测试的第三条断言恒真(发现 1);对 Biome 的断言 pin 的是配置字面量'!!**/.claude/worktrees'而非行为,若日后 Biome 语法演进或配置规范化为单!,测试会在行为仍正确时红掉——在无法于测试内跑 biome 的前提下这是可接受折中,值得知晓。
缺失覆盖
- 在不存在
.claude/worktrees/的干净 clone 上确认:501-503通过:作者的验证环境中该目录实际存在,而 CI(.github/workflows/ci.yml:28→quality:ci→test:workflows)跑在全新 checkout 上。.gitignore的目录专用模式(尾斜杠)匹配需要解析 dtype,路径不存在时的行为与既有:485测试(.claude/*.local.md,非目录模式)不同类。这一项若失败会在 CI 直接可见,只需确认本 PR 的 CI 是绿的即可关闭。 - helper 过滤逻辑本身缺少能失败的回归测试(发现 1)。
- 未在 worktree 在位时跑过
pnpm contract:local(GitNexus analyze)。若 GitNexus 遵守.gitignore,杠杆 1 已覆盖;否则嵌套 checkout 仍会被索引,可能需要第三处排除。属后续观察项,不阻塞本 PR。
|
|
||
| // Callers readFileSync these entries, and `git ls-files --others` reports an | ||
| // untracked directory as one trailing-slash entry, which would throw EISDIR. | ||
| assert.ok(listPublicClaudeAssets().every((file) => !file.endsWith('/'))); |
There was a problem hiding this comment.
该断言对现实现恒真:helper 已在 :29 过滤尾斜杠,且 .gitignore 杠杆生效后 git ls-files --others --exclude-standard .claude 也不会再产出目录条目,单独回退 :29 时它仍会通过。把过滤抽成纯函数并对合成输入断言,或删除这条断言。
| }); | ||
| return [...new Set(`${tracked}\n${untracked}`.split('\n').filter(Boolean))].sort(); | ||
| const entries = [...new Set(`${tracked}\n${untracked}`.split('\n').filter(Boolean))]; | ||
| // `git ls-files --others` reports an untracked directory as a single entry |
There was a problem hiding this comment.
表述过宽:git ls-files --others 默认会递归普通未跟踪目录并逐文件列出;折叠为一条尾斜杠条目是"嵌套仓库"(worktree/submodule)或 --directory 的行为。改为"未跟踪的嵌套仓库",并同步收敛 CHANGELOG.md:18 的"任何其他游离目录"说法。
| .claude/**/settings.local.json | ||
| # 本地 git worktree(Claude Code 的默认位置)。跟踪它会让工具把一个完整 | ||
| # 的嵌套 checkout 当成仓库内容 —— 见 biome.json 的 includes 与 #439。 | ||
| .claude/worktrees/ |
There was a problem hiding this comment.
该模式含中间斜杠,仅锚定仓库根,与 biome.json:62 的 **/.claude/worktrees 覆盖范围不一致;若要覆盖从子目录启动 Claude Code 的情形,写成 **/.claude/worktrees/。
…claims Addresses repo-guard review on #442. Verdict was "可以合并"; these are the correctness items. The third assertion could not fail. It called listPublicClaudeAssets(), which applies the very filter under test, so it was tautological — and with the ignore rule in place git no longer emits a directory entry to catch either. The PR claimed each of the three fixes fails its assertion when reverted individually; that was untrue for this one. My earlier check tested the predicate against a synthetic list rather than the assertion as written, which is how it slipped through. The filter is now a named pure function asserted against synthetic input, so reverting it to identity fails the test. Verified both directions. Two claims were also wider than the behavior. `git ls-files --others` recurses into an ordinary untracked directory and lists its files; it collapses to a single trailing-slash entry only for a nested repository — a linked worktree or an uninitialised submodule — or under --directory. Confirmed in a scratch repo. So "any other stray directory would now fail as an assertion" was overstated: an ordinary directory never produced an entry to begin with. Comment and CHANGELOG now say nested repository. Finally, `.gitignore`'s `.claude/worktrees/` was root-anchored while Biome's `!!**/.claude/worktrees` matches any depth, so a worktree created from a subdirectory was covered by one lever and not the other. The ignore rule is now `**/.claude/worktrees/`; check-ignore verified at root, apps/desktop, and packages/core.
|
Addressed in 85a6ed0. All three findings were verified before acting; all three were correct. 发现 1 — 第三条断言是重言式 · 已修这条说到点子上了,而且它戳破的是我在 PR 描述里的一句不实陈述。 断言写的是 我原来的「三个修复各自回退都会失败」验证之所以没抓到,是因为我测的是谓词对合成列表,而不是断言本身。测了个和产品代码同构的东西,等于没测。 已按建议抽成具名纯函数 assert.deepEqual(
dropDirectoryEntries(['.claude/skills/a/SKILL.md', '.claude/worktrees/some-branch/']),
['.claude/skills/a/SKILL.md'],
);复验:当前实现通过,把函数回退成恒等函数则失败。这次是真的可证伪了。 发现 2 —
|
There was a problem hiding this comment.
代码评审报告: fix(tooling): stop a .claude/worktrees git worktree from breaking pnpm lint and test:workflows (#439)
风险等级: 中
处理建议: 请求修改
决策摘要: 两个 root cause 的修复本身正确、有测试锚定,可以合并;但 helper 的「defence in depth」实际做的是静默丢弃,与代码注释、PR 描述和 CHANGELOG 的「fails as an assertion」说法相反,并顺带削弱了两条既有 .claude 资产不变量——请修正实现或修正声明后再合。
级联分析
- 变更符号:
dropDirectoryEntries(新增)、listPublicClaudeAssets(scripts/tests/workflow-rules.test.mjs:24,28);非代码杠杆.gitignore:57、biome.json:62。 - 受影响流程:
pnpm lint(biome check .)、pnpm test:workflows(node --test scripts/tests/*.test.mjs),二者均在quality:precommit与 pre-commit/pre-push hook 路径上(package.json:28,35,40)。 - 变更集外调用方:
listPublicClaudeAssets与dropDirectoryEntries的调用方全部在同文件(:523,536,550),无外部消费者 (text search)。git ls-files --others --exclude-standard的另一处消费者scripts/workflows/contract-check.mjs:142,152只做路径分类,不readFileSync,且.claude/worktrees/**不命中criticalContractRules(contract-rules.mjs:66-78),因此 contract 门禁不受影响。pnpm-workspace.yaml仅匹配packages/*/apps/*,嵌套 checkout 不会被 turbo typecheck/test 拾取,与 issue「只有两个 gate 受影响」的判断一致。 - 置信度: medium — 无代码图谱证据,结论来自 diff + 只读文本检索;对这种 4 文件、无应用代码的 diff 覆盖已较完整,但
biome check .与 worktree 场景我无法在此环境复跑,依赖作者报告的 417 files / exit 0。
问题发现
-
[中] helper 把「崩溃」变成了「静默」,而不是声明中的「assertion failure」
- 证据:
dropDirectoryEntries在listPublicClaudeAssets()内部、返回前执行(scripts/tests/workflow-rules.test.mjs:33-34),因此过滤发生在所有消费者之前。对一个非 worktrees 路径下的嵌套仓库(例如未初始化的 submodule.claude/scratch/):- 改前:
:529-531的every(file => startsWith('.claude/workflows/') || startsWith('.claude/skills/'))会断言失败,:535的 portability 测试再以 EISDIR 崩溃。 - 改后:该条目在两处之前就被丢掉,两个测试都静默通过。
同理:555的assert.deepEqual(workflowAssets, ['.claude/workflows/oss-harness-engineering-workflow.js'])也不再能发现.claude/workflows/下的嵌套仓库。
- 改前:
- 受影响调用方/流程:
Claude reusable assets are public while local state stays private(:522)、public Harness workflow assets are portable(:535)、Claude Harness workflow has a single portable entrypoint(:549)。这是本 PR 引入的、针对.claude资产隐私/可移植性不变量的小幅覆盖回退。#439 的主线不受影响(worktrees 已被 ignore,根本不会出现该条目)。 - 最小可行修复: 把目录过滤移到真正
readFileSync的地方——listPublicClaudeAssets()原样返回全部条目,:540的读取循环前.filter(f => !f.endsWith('/'))。这样嵌套目录仍会触发:529的前缀断言(即声明中想要的「useful assertion failure」),portability 测试也不再 EISDIR。若保留现有位置,则需改注释与文案,明确说明这是「静默丢弃以避免崩溃」,而非「转为断言失败」。
- 证据:
-
[中] CHANGELOG 描述了一个代码没有实现的行为
- 证据:
CHANGELOG.md:18末句「The asset helper also drops directory entries, so an uninitialised submodule or another linked worktree fails as an assertion rather than crashing the suite.」按发现 1 的分析,改后既不崩溃也不断言失败,而是完全不可见。PR 描述中的「any other stray directory now surfaces as an assertion failure instead of crashing the suite」同样不成立。 - 受影响调用方/流程: CHANGELOG 是对外发布产物;
scripts/tests/workflow-rules.test.mjs:22-23的代码注释带有同一处错误陈述,会误导后续维护者认为该保护网存在。 - 最小可行修复: 采纳发现 1 的实现修复后 CHANGELOG 无需改;否则把该句改为「helper 丢弃目录条目,避免 EISDIR 直接中断整个 suite」,并同步修正
:22-23的注释。
- 证据:
行级发现
- [scripts/tests/workflow-rules.test.mjs:25] 过滤发生在 helper 返回前,因此
.claude/下任何嵌套仓库目录在:529的前缀断言和:555的deepEqual之前就被移除,结果是静默通过而非注释所称的 assertion failure;把过滤下移到:540的读取循环处即可同时避免 EISDIR 并保留这两条不变量。 - [CHANGELOG.md:18] 「fails as an assertion rather than crashing the suite」与实现不符(实际是静默丢弃);随实现修复保留,或改写为「避免 EISDIR 中断 suite」。
Karpathy 评审
- 假设: PR 明确并实验验证了「两个杠杆都必需」这一关键假设(Biome 未设
vcs.useIgnoreFile,不读.gitignore;nested-config 发现独立于 ignore 规则),并与 issue #439 的结论一致。!!前缀的语义未在 PR 中论证,但与biome.json:47-61既有 14 条同形式条目一致,且作者报告 worktree 在场时biome check .417 files/exit 0——若!!不生效,node_modules早已被扫入,因此该假设可接受(我无法在此环境复跑 biome)。 - 简洁性:
dropDirectoryEntries是单表达式函数,无过度抽象;.gitignore与biome.json各一行,无猜测性通用化。唯一的复杂度增量就是发现 1 中过滤位置的选择。 - 结构质量: 无文件膨胀、无共享路径特例、无薄 wrapper、无重复 canonical helper;三个杠杆各自落在拥有该概念的层(git ignore / biome config / test helper),层级归属正确。
- 变更范围: 4 文件严格对应 issue 的三条 proposed fix + CHANGELOG,无无关重构、无格式噪声,diff 每一行可追溯到既定目标。
- 验证: 三条杠杆都有可证伪断言(
:506-508ignore 规则、:511biome includes、:516-519合成输入下的 filter),作者声明逐条 revert 均能让对应断言失败。:513-515的注释正确解释了为何不在此调用listPublicClaudeAssets()(否则同义反复)。测试名「does not break the local gates」略强于实际断言(pin 的是三个前置条件而非端到端复现 worktree),但注释已说明,不作为阻塞项。
缺失覆盖
- 无。三条杠杆均已被断言锚定;发现 1 的修复若采纳,建议顺带补一条断言:构造含目录条目的合成列表,验证
.claude/下的非 workflows/skills 目录仍会触发:529的前缀断言,使「defence in depth」这一声明本身可证伪。
| // submodule. Callers readFileSync these entries, so such a path would abort | ||
| // the suite with EISDIR instead of a useful assertion failure (#439). | ||
| function dropDirectoryEntries(entries) { | ||
| return entries.filter((file) => !file.endsWith('/')); |
There was a problem hiding this comment.
过滤发生在 helper 返回前,因此 .claude/ 下任何嵌套仓库目录在 :529 的前缀断言和 :555 的 deepEqual 之前就被移除,结果是静默通过而非注释所称的 assertion failure;把过滤下移到 :540 的读取循环处即可同时避免 EISDIR 并保留这两条不变量。
|
|
||
| ### Fixed | ||
|
|
||
| - **tooling**: A git worktree under `.claude/worktrees/` — Claude Code's default location — no longer breaks the local quality gates, which also made the pre-commit and pre-push hooks unusable without `--no-verify`. The worktree is a full nested checkout, and it broke two gates independently: its own `biome.json` made root `biome check .` abort with a nested-root-configuration error before checking any file, and `git ls-files --others` reported it as a single trailing-slash entry — the way it reports an untracked *nested repository*, rather than recursing as it does for an ordinary directory — which `listPublicClaudeAssets()` then read with `readFileSync`, throwing `EISDIR`. Fixed on both levers, since Biome sets no `vcs.useIgnoreFile` and never consults `.gitignore`: a `**/.claude/worktrees/` ignore rule plus a matching `biome.json` `includes` exclusion. The asset helper also drops directory entries, so an uninitialised submodule or another linked worktree fails as an assertion rather than crashing the suite. (#439) |
There was a problem hiding this comment.
「fails as an assertion rather than crashing the suite」与实现不符(实际是静默丢弃);随实现修复保留,或改写为「避免 EISDIR 中断 suite」。
…listing Addresses the second repo-guard review on #442 (请求修改). The finding is correct and the regression was mine. Filtering inside listPublicClaudeAssets() ran before every consumer, so a stray nested repository under .claude/ was removed from the two tests whose job is to notice it. A `.claude/scratch/` submodule would have failed the public-prefix assertion; a nested repo under .claude/workflows/ would have failed the single-entrypoint deepEqual. After that change both passed silently. Only the portability test actually readFileSync's the entries, so only it needs the filter. Moved the filter to that one call site. Simulated both revisions against a synthetic stray entry: with the filter at the read site the prefix assertion and the entrypoint deepEqual both fail as intended and the read list has no directories; with the filter in the helper both silently passed. The claim in CHANGELOG and the code comment is now true rather than aspirational — "fails as an assertion rather than crashing the suite" was describing behavior the previous revision did not have. Reworded to say where the filter lives and what each consumer still catches.
|
Fixed in 55bf801. 这条是对的,而且退化是我引入的。 发现 1 · 已按建议的实现修复(不是改文案)
只有 净效果:EISDIR 依然被挡掉,而 发现 2 · 随实现修复自动成立,但措辞仍需调整采纳发现 1 的实现后,「fails as an assertion rather than crashing the suite」这句终于名副其实。不过原句写的是「The asset helper also drops directory entries」,而过滤已不在 helper 里,所以措辞仍然不准。CHANGELOG 与代码注释都已改写成说明过滤位置以及每个消费者各自还能抓到什么,PR 描述同步更新。 这轮的教训记一下:我上一版是把「避免崩溃」当成了目标,没意识到崩溃点和断言点是不同的消费者,于是用一个全局过滤同时抹掉了两者。评审把「静默丢弃 ≠ 断言失败」这个区分点出来,是这条修复真正的价值。 验证: |
There was a problem hiding this comment.
代码评审报告: fix(tooling): stop a .claude/worktrees git worktree from breaking pnpm lint and test:workflows (#439)
风险等级: 低
处理建议: 评论
决策摘要: 可以合并——两个 root cause 各自用了正确的杠杆,diff 范围最小,无应用代码影响;仅剩两处测试注释与实际实现不一致(其中一处是本次设计选择的论证依据),以及读取点过滤器本身无回归保护,均为非阻塞项。
级联分析
- 变更符号:
dropDirectoryEntries(新增,测试文件私有)、listPublicClaudeAssets(未改动,调用点行为在public Harness workflow assets are portable中被包裹) - 受影响流程:
pnpm lint(biome check .)、pnpm test:workflows(node --test scripts/tests/*.test.mjs)、pre-commit / pre-push 钩子中的quality:precommit;间接受益:contract:local的git ls-files --others(scripts/workflows/contract-check.mjs:142,152)此前会把.claude/worktrees/x/当作.claude/前缀的 critical repo-harness 路径,ignore 规则一并消除 (repo grep) - 变更集外调用方: 无。
dropDirectoryEntries只在本测试文件内使用 (text search);biome.json与.gitignore的消费方是本地工具链自身。已确认其它遍历型 gate 不会进入该目录:pnpm-workspace.yaml只匹配packages/*/apps/*,turbo test按 workspace 包运行,根tsconfig.json、根vitest.config.ts均不参与根级pnpm test/typecheck的树遍历 (repo files) - 置信度: medium — 无代码图谱证据,但受影响面全部是配置与单个测试文件,调用方可用 grep 穷举;Biome 2.4.16 的 scanner 是否因
files.includes排除而不再发现嵌套配置,我在此环境未实际执行,采信 PR 报告的biome check .→ 417 files / exit 0
问题发现
-
[低] 新测试内的注释与上一次提交后的实现相互矛盾
- 证据:
scripts/tests/workflow-rules.test.mjs:518写「the helper applies this filter itself」,但55bf801(test(tooling): filter directory entries at the read site, not in the listing)已经把过滤器移出listPublicClaudeAssets(),同文件:24-28明确论证「Filtering inside listPublicClaudeAssets() would hide such an entry」。同一文件两段注释给出相反的事实描述;PR 描述的 Impact Scope 一节「listPublicClaudeAssets()drops directory entries」也是同一处旧说法的残留。 - 受影响调用方/流程: 无运行时影响;影响的是后续维护者判断过滤器该放在哪一层——这正是本 PR 唯一有争议的设计决策。
- 最小可行修复: 把 518 行改成不含「helper 自己过滤」的表述,只保留仍然成立的理由(ignore 规则生效后 git 不再产出目录条目,因此走
listPublicClaudeAssets()无法构造被捕获的输入)。
- 证据:
-
[低] 「保持在 listing 之外」的论证对最可能的 stray 目录位置不成立
- 证据:
scripts/tests/workflow-rules.test.mjs:24-28声称目录条目会被 public-prefix 断言和 single-entrypoint deepEqual「informatively」捕获。但 public-prefix 断言(:533-536)只要求startsWith('.claude/workflows/')或.claude/skills/'——.claude/skills/some-skill/这样的嵌套仓库条目会直接通过;single-entrypoint deepEqual(:557-562)只覆盖.claude/workflows/前缀。也就是说.claude/skills/下的嵌套 checkout 无论过滤器放哪一层都会被静默略过。 - 受影响调用方/流程: 仅
test:workflows的诊断质量;不改变本 PR 修复的两个 gate。 - 最小可行修复: 把注释的适用范围收窄为「
.claude/下 workflows/skills 前缀之外的目录条目」,或让 public-prefix 断言额外拒绝以/结尾的条目,从而使论证与断言实际覆盖一致。
- 证据:
行级发现
- [scripts/tests/workflow-rules.test.mjs:518] 「the helper applies this filter itself」在
55bf801之后已不成立,且与本文件 24-28 行的设计论证直接冲突;删掉该分句,只保留「ignore 规则生效后 git 不再产出目录条目」这一仍然有效的理由。 - [scripts/tests/workflow-rules.test.mjs:26] public-prefix 断言只校验
.claude/workflows/或.claude/skills/前缀,.claude/skills/foo/形式的目录条目会通过它;把注释范围限定为这两个前缀之外的目录条目,或让该断言显式拒绝以/结尾的条目。
Karpathy 评审
- 假设: 关键假设「Biome 不读
.gitignore,因此必须双杠杆」在 issue #439 与 PR 中都做了隔离验证,且由:514-515的配置断言钉住,未留隐式依赖。剩余未言明的假设是 Biome scanner 在files.includes排除下不再发现嵌套配置——这一行为随 Biome 版本变化,好在@biomejs/biome已精确 pin 到2.4.16。 - 简洁性:
dropDirectoryEntries是三行纯函数,无多余抽象;.gitignore与biome.json各加一条,与既有!!**/node_modules等条目风格一致。没有发现可删除的复杂度。 - 结构质量: 无退化。过滤器放在读取点是有意识的权衡(保留 listing 的原始输出供前缀断言检查),不是随手放置;未新增 flag、模式分支或共享路径特例,文件规模无跨界。
- 变更范围: 4 个文件全部可追溯到 #439 的三条 proposed fix,无夹带重构或格式噪声。CHANGELOG 归入
[Unreleased] → Fixed,符合仓库既有写法。 - 验证:
.gitignore杠杆由git check-ignore(:510-512)钉住,Biome 杠杆由配置断言(:514-515)钉住,两者都可被 revert 证伪。第三条杠杆只钉住了dropDirectoryEntries的行为,没有钉住它在:543的应用;见下节。
缺失覆盖
:543的过滤器调用点无回归保护:删掉dropDirectoryEntries(...)包裹后,全部 39 个测试仍会通过(ignore 规则使 git 不再产出目录条目)。PR 已诚实指出用真实 helper 断言会变成 tautology,但结果是这条 defence-in-depth 只有函数本身被测、接线未被测。若要闭合,需要在临时目录里造一个嵌套仓库 fixture 并对该 fixture 调用 helper;成本是否值得由维护者判断,不构成合并阻塞。- 「Biome 在存在嵌套
biome.json时仍能完成检查」无自动化覆盖,仅靠:514-515的配置字符串断言 + PR 中的手工验证记录。这是本次修复中唯一依赖 PR 报告而非仓库内证据的环节(我在评审环境未执行biome check .);若未来升级 Biome,该杠杆的失效不会被 CI 捕获。
| assert.ok(biomeConfig.files.includes.includes('!!**/.claude/worktrees')); | ||
|
|
||
| // Assert the filter against synthetic input. Calling listPublicClaudeAssets() | ||
| // here would be tautological: the helper applies this filter itself, and with |
There was a problem hiding this comment.
「the helper applies this filter itself」在 55bf801 之后已不成立,且与本文件 24-28 行的设计论证直接冲突;删掉该分句,只保留「ignore 规则生效后 git 不再产出目录条目」这一仍然有效的理由。
| // | ||
| // Apply this only where entries are actually readFileSync'd. Filtering inside | ||
| // listPublicClaudeAssets() would hide such an entry from the callers that are | ||
| // supposed to catch it: the public-prefix assertion and the single-entrypoint |
There was a problem hiding this comment.
public-prefix 断言只校验 .claude/workflows/ 或 .claude/skills/ 前缀,.claude/skills/foo/ 形式的目录条目会通过它;把注释范围限定为这两个前缀之外的目录条目,或让该断言显式拒绝以 / 结尾的条目。
Third repo-guard review on #442. Both findings correct, both low. The justification for keeping the filter out of the listing helper was that the public-prefix assertion and the single-entrypoint deepEqual would catch a stray directory informatively. That held only outside the allowed prefixes: a nested repository at `.claude/skills/some-skill/` satisfies startsWith and passed, and the workflows deepEqual only covers `.claude/workflows/`. So the most likely stray location was silently skipped no matter where the filter sat. Rather than narrow the claim, the assertion now rejects trailing-slash entries outright, which makes it true for any directory entry under .claude/. Simulated: `.claude/skills/some-skill/` and `.claude/workflows/nested/` both passed before and both fail now; `.claude/scratch/` failed already; normal assets do not false-positive. Also drops a stale comment claiming the helper applies the filter itself — true before 55bf801 moved it to the read site, and contradicting the argument a few lines above it.
|
Fixed in f712f06. 两条都对。 发现 2 — 采用了「让论证成立」而非「收窄论证」这条很关键:我把过滤留在读取处的理由,是「前缀断言和单入口 deepEqual 会有用地捕获游离目录」。但这个论证只在允许前缀之外成立—— 评审给了两个选项,我选了后者(让断言覆盖与论证一致),因为前者等于承认这个设计决策的理由只对一部分情况有效。前缀断言现在显式拒绝尾斜杠条目:
现在注释里那句「the public-prefix assertion ... reports a stray nested repository anywhere under .claude/」是真的了。 发现 1 — 陈旧注释已删
验证: |
There was a problem hiding this comment.
代码评审报告: fix(tooling): stop a .claude/worktrees git worktree from breaking pnpm lint and test:workflows (#439)
风险等级: 低
处理建议: 评论
决策摘要: 可以合并——两个 root cause 各自对应正确的 lever,无应用代码影响,验证证据充分;剩下的只是测试层面的非阻塞问题(PR 的核心设计决定“过滤器放在读取点”本身没有被任何测试固定住)。
级联分析
- 变更符号:
dropDirectoryEntries(新增,文件内私有)、listPublicClaudeAssets(未改动,调用点行为改变)、biome.json的files.includes、根.gitignore。 - 受影响流程:
pnpm lint(biome check .)、pnpm test:workflows(node --test scripts/tests/*.test.mjs)、以及经由 pre-commitquality:precommit/ pre-pushquality:local的两个 hook(package.json:28-30,35,40)。 - 变更集外调用方:
listPublicClaudeAssets/dropDirectoryEntries的调用点全部在scripts/tests/workflow-rules.test.mjs(528、550、564),仓库内无其他引用 (text search)。git ls-files --others --exclude-standard的另一处消费者是scripts/workflows/contract-check.mjs:142,152;它只把结果当作 changed-file 列表分类,不做readFileSync,且因为带--exclude-standard,新的 ignore 规则同样把 worktree 从该列表里移除——顺带消除了 contract 门禁把嵌套 checkout 当成变更文件的可能 (read)。pnpm-workspace.yaml的packages/*、apps/*是有界 glob,嵌套 checkout 不会被 pnpm 识别成 workspace 包,所以不需要第三个 lever (read)。
- 置信度: medium(未使用代码图谱;结论来自 grep + 直接阅读被触碰配置与全部调用点,爆炸半径限于本地工具链,PR 另有实机门禁输出佐证)
问题发现
-
[中] PR 的核心设计决定(过滤器放在读取点而非 listing helper)没有被任何测试固定
- 证据: 新测试的第三条断言(
scripts/tests/workflow-rules.test.mjs:521-524)只验证dropDirectoryEntries这个纯函数会丢掉 trailing-slash 条目。如果后续有人把同一个过滤挪进listPublicClaudeAssets()(正是 issue #439 “Proposed fix” 第 3 条的字面写法,也是本 PR 自己在早期修订版里犯过、在55bf801才修掉的那次回归),这条断言依然通过,prefix 断言和 single-entrypointdeepEqual会重新变成静默通过。PR 说明里“placement 通过模拟.claude/scratch/验证”是一次手工验证,没有留下可执行的守卫。 - 受影响调用方/流程:
pnpm test:workflows对.claude/下杂散嵌套仓库的告警能力(528 的 prefix 断言、564 的 entrypointdeepEqual)。 - 最小可行修复: 复用本文件已有的注入模式(
getChangedFiles('local', { git: { lines } }),见 330-361),给listPublicClaudeAssets加一个可注入的 git runner,然后断言:喂入含.claude/skills/a/的 stub 输出时,helper 仍然返回该目录条目;同时把 prefix 谓词抽成命名纯函数并断言它对.claude/skills/a/返回 false。这样“不在 helper 里过滤”和“prefix 断言会拦下它”两个不变量都变成可失败的断言。
- 证据: 新测试的第三条断言(
-
[低] trailing-slash 判定在两处各写一遍,存在漂移风险
- 证据:
dropDirectoryEntries(30-32)用!file.endsWith('/'),528 的 prefix 断言又内联了同一个字面判断(540)。两处语义相同、意图不同(丢弃 vs 拒绝),但共享同一条“git 用尾斜杠表示嵌套仓库”的约定。 - 受影响调用方/流程: 仅本测试文件;若将来判定条件变化(例如同时处理
\或空条目),容易只改一处。 - 最小可行修复: 抽出
isDirectoryEntry(file),dropDirectoryEntries与 prefix 断言各自复用,约定只留一处。
- 证据:
-
[低] 新测试在一个名字下捆绑了三个互不相关的 lever,失败时归因成本偏高
- 证据:
510-525一个 test 内依次断言 gitignore 行为、biome.json配置内容、纯函数行为。测试名是 “does not break the local gates”,三条断言都没有 message。 - 受影响调用方/流程: 门禁失败时的可诊断性——这正是 issue #439 抱怨原始故障“错误信息指向错误位置”的同一类问题。
- 最小可行修复: 给三条断言各加一句 message(或拆成三个 test),指明是 ignore 规则、Biome 排除项还是目录过滤退化了。
- 证据:
行级发现
- [scripts/tests/workflow-rules.test.mjs:521] 这条断言只覆盖纯函数本身,无法阻止后续有人把过滤挪回
listPublicClaudeAssets()(本 PR 早期修订版的原回归);建议按上面的注入方式补一条“helper 必须保留目录条目”的断言。 - [scripts/tests/workflow-rules.test.mjs:30]
endsWith('/')与第 540 行的内联判断重复;抽成isDirectoryEntry并在两处复用,避免约定漂移。 - [scripts/tests/workflow-rules.test.mjs:510] 一个 test 同时断言 ignore 规则、Biome 配置和目录过滤且无 message;加上断言 message 或拆分,失败时才能直接看出是哪个 lever 退化。
Karpathy 评审
- 假设: 关键假设都已在 diff 内显式写明并可核对——Biome 不读
.gitignore(biome.json确无vcs.useIgnoreFile,已核对biome.json:43-63),git ls-files --others对嵌套仓库塌缩成单条尾斜杠条目。biome.json里!!前缀的语义未在仓库中任何地方记录,新行完全沿用既有!!**/node_modules等约定,且 PR 给出biome check .→ 417 files / exit 0 的实测,属于既有约定而非本 PR 引入的不确定性。 - 简洁性: 没有猜测性抽象。
dropDirectoryEntries是单行纯函数,唯一冗余是第 2 条发现里的重复字面判断。 - 结构质量: 无退化。测试文件从 ~671 增到 ~720 行,未跨 1000 行边界;没有新增分支散落到共享路径;两个配置改动都落在各自 canonical 位置(gitignore / biome includes),没有新增 wrapper 或层级。gitignore 从 issue 建议的
.claude/worktrees/放宽到**/.claude/worktrees/是对 Biome**语义的刻意对齐,注释里说明了理由,可接受。 - 变更范围: 与既定目标严格对应,4 个文件、无应用代码、无顺手重构或格式噪声。相对 issue #439 “Proposed fix” 第 3 条的唯一偏离(过滤放读取点而不是 helper 内)在 PR 描述和代码注释里都给了理由,且保留了更强的诊断能力,是对 issue 提案的改进而非缩水。
- 验证: 强度与风险匹配——
biome check .、turbo typecheck25/25、turbo test25/25、node --test39/39 都是在 worktree 在位的状态下跑的,并且 commit/push 走了真实 hook。biome.json那条断言是字符串级 pin,等价改写(如!!**/.claude/worktrees/**)会误报失败,但在不启动 Biome 进程的前提下这是合理取舍,且与本文件 pin CI yml / hook 内容的既有风格一致。
缺失覆盖
- 缺少固定“目录过滤不得回到
listPublicClaudeAssets()内部”的断言(发现 1);这是本 PR 已经真实发生过一次的回归,是合并后唯一值得补的测试。 .claude/下杂散嵌套仓库触发 prefix 断言 / entrypointdeepEqual失败的路径目前只有手工模拟证据,没有自动化用例;按发现 1 的注入方式可一并覆盖。- Biome 排除项的行为(而非配置字符串)无自动化覆盖:若 Biome 未来改变
files.includes对 nested root config 发现的作用,本测试仍会通过而pnpm lint会重新失败。属已知残留风险,不建议为此在测试里启动 Biome 进程。
| // Assert the filter against synthetic input: with the ignore rule in place | ||
| // git no longer emits a directory entry, so listPublicClaudeAssets() cannot | ||
| // produce one to catch here. | ||
| assert.deepEqual( |
There was a problem hiding this comment.
这条断言只覆盖纯函数本身,无法阻止后续有人把过滤挪回 listPublicClaudeAssets()(本 PR 早期修订版的原回归);建议按上面的注入方式补一条“helper 必须保留目录条目”的断言。
| // entry, so it reports a stray nested repository anywhere under .claude/ — | ||
| // including under an otherwise-allowed prefix — and dropping it first would | ||
| // turn that into a silent pass. | ||
| function dropDirectoryEntries(entries) { |
There was a problem hiding this comment.
endsWith('/') 与第 540 行的内联判断重复;抽成 isDirectoryEntry 并在两处复用,避免约定漂移。
| // it (Biome sets no vcs.useIgnoreFile, so .gitignore alone does not stop the | ||
| // nested-config error, which aborts the whole run before any file is checked). | ||
| // See #439. | ||
| test('a git worktree under .claude/worktrees does not break the local gates', () => { |
There was a problem hiding this comment.
一个 test 同时断言 ignore 规则、Biome 配置和目录过滤且无 message;加上断言 message 或拆分,失败时才能直接看出是哪个 lever 退化。
Linked Issue Or Context
Closes #439
Hit while working on #437 / #393 — every local gate run in that work needed the worktree temporarily moved aside.
Summary
A git worktree under
.claude/worktrees/(Claude Code's default location) is a full nested checkout, and it broke two local gates independently. Becausepnpm lintruns in both the pre-commit and pre-push hooks, a maintainer in that state could not commit or push without--no-verify.Two independent root causes
1.
pnpm lint— nested Biome config. The worktree carries its ownbiome.json, so rootbiome check .aborts:This is a configuration error, not a lint finding — the run stops before checking any file, so the gate is not merely noisy, it is inert.
2.
pnpm test:workflows— EISDIR.git ls-files --othersrecurses into an ordinary untracked directory and lists its files, but collapses to a single trailing-slash entry when the directory is a nested repository — which a linked worktree is:listPublicClaudeAssets()hands that toreadFileSync, and two tests die withEISDIRpointing at the helper rather than at the offending directory.Both levers are required
.gitignorealone does not fix Biome:biome.jsonsets novcs.useIgnoreFile, so Biome never consults.gitignore, and nested-config discovery is independent of ignore rules. I verified this by applying each fix in isolation —.gitignorealone leavesbiome check .failing identically.Conversely
.gitignoreis the right lever for cause 2, since--exclude-standardhonors it.Impact Scope
4 files, +34/−2. No application code.
.gitignore— ignore**/.claude/worktrees/(any depth, matching Biome's rule; a worktree created from a subdirectory lands under that subdirectory's.claude/)biome.json—"!!**/.claude/worktrees"infiles.includes, alongside the existing!!**/node_modules,!!**/distetc.scripts/tests/workflow-rules.test.mjs— directory entries dropped at the read site; the public-prefix assertion rejects them outright; new test pinning all three leversCHANGELOG.md—[Unreleased]→### FixedThe directory filter is applied at the point assets are read, not inside the listing helper. That distinction matters: only the portability test does
readFileSync, while the public-prefix assertion and the single-entrypointdeepEqualare supposed to notice a stray nested repository under.claude/. Filtering in the helper would have hidden it from both, turning two informative failures into silent passes — a regression this PR introduced in an earlier revision and fixed in55bf801.For that argument to hold everywhere, the public-prefix assertion now rejects trailing-slash entries outright — otherwise a nested repository under an allowed prefix (
.claude/skills/some-skill/) satisfiesstartsWithand slips past, which was the most likely stray location. Simulated: that case and.claude/workflows/nested/both passed before and both fail now;.claude/scratch/already failed; real assets do not false-positive.So: a stray nested repository anywhere under
.claude/fails informatively, and no longer aborts the suite withEISDIR. Ordinary stray directories are not covered and never were — git does not emit a directory entry for them.Verification
Run with the worktree in place, not moved aside — that is the point:
End-to-end proof: this branch's commit and push both ran the real
quality:precommit/ pre-push hooks to completion with the worktree present, without--no-verify. Before this change that was impossible.Full gate on
f712f06:biome check .→ 417 files, exit 0 (1 pre-existing fixablestyle/useTemplateinfo inpackages/hallucination-guard, untouched here)turbo typecheck→ 25/25 tasksturbo test→ 25/25 tasksnode --test scripts/tests/*.test.mjs→ 39/39The new test pins all three levers, and each is falsifiable: reverting the ignore rule, the Biome exclusion, or the directory filter each fails its own assertion. The filter is asserted as a named pure function against synthetic input — asserting via
listPublicClaudeAssets()was tautological, since the helper applied that filter itself and the ignore rule stops git emitting a directory entry to catch. That flaw was in the first revision and is fixed in85a6ed0.Filter placement was verified by simulating a stray
.claude/scratch/and a stray.claude/workflows/nested/against both revisions: at the read site the prefix assertion and entrypointdeepEqualboth fail as intended while the read list carries no directories; inside the helper both passed silently.GitNexus Impact Summary
classifyContractPathsreturns an emptycriticalset for all four files, so the GitNexus gate is advisory for this diff.detect_changesreports no changed symbols and no affected execution flows; the diff is ignore rules, tool configuration, a test helper, and a changelog line, none of which are indexed code symbols, so a per-symbolimpactrun has no target. The only consumer of the touched config is the local tooling itself.pnpm quality:precommitgreen onf712f06— see above.Checklist
pnpm quality:precommit, or explained why it could not run.pnpm quality:localfor critical skeleton changes, or explained why it could not run. (No critical skeleton change;quality:precommitran in full.)