test(coordination): cover writer fence read error path cleanup - #4266
Conversation
Signed-off-by: Lihua <1017343802@qq.com>
huangruiteng
left a comment
There was a problem hiding this comment.
动机
本 PR 的目标是为 loadLegacyCoordinationWriterFence 增加直接的回归保护。现有调用方 parity 测试能够观察归一化后的结果,但如果 reader 自身把带本地路径的文件系统错误泄露出去、丢失 reason_code,或者把目录读取误当成“文件缺失”,调用方层的归一化可能掩盖这些差异。这个问题低频,却会在协调状态异常时增加定位成本,并可能把本地路径带入诊断输出。作者没有改变 reader 的生产实现,而是从真实 reader 入口覆盖 path-bearing/path-free EACCES、真实 ENOENT 和真实目录 EISDIR,因此改动与问题边界匹配。
改动思路
测试直接导入现有 legacyCoordinationWriterFencePath 和 loadLegacyCoordinationWriterFence,把断言放在错误分类的权威边界,而不是重新实现一份 reader。第一组使用 Node fs.readFile mock 注入两种 EACCES 形态,确认错误仍是 failed、legacy_writer_fence_read_failed,诊断文本保留但不含 fence 路径,同时确认只按 [path, "utf8"] 读取一次,并在 finally 恢复 mock/ESM exports。第二组使用真实临时文件系统:不存在的路径必须是 missing,同一路径改成目录后必须是 failed/EISDIR,且目录不能被测试或 reader 删除。上层 retry/协调状态机仍由既有 callers 负责,本 PR 不新增第二个状态或副作用。
具体改动
tests/control_plane_ts/legacy_writer_fence_read_error.test.ts 新增 58 行:嵌套测试覆盖带路径和不带路径的 EACCES,真实临时目录覆盖 ENOENT/EISDIR,并用 t.after/finally 保证资源恢复。tsconfig.control-plane.json 增加一行,把该测试纳入控制面类型检查。生产文件 loopx/control_plane/coordination/legacy_writer_fence.ts 没有改动,现有 reader 仍是错误分类与路径清理的唯一 owner;rg 检查确认多个 coordination/runtime caller 继续消费同一返回 union。
关键代码讲解
loadLegacyCoordinationWriterFence(loopx/control_plane/coordination/legacy_writer_fence.ts:130)是被测入口:ENOENT 走missing,其它读取异常走failed,reason 中的路径被清理,reason code 保留。- “fence reader removes the filesystem error path”(
tests/control_plane_ts/legacy_writer_fence_read_error.test.ts:11)用同一 production reader 验证两种 EACCES 错误形态的结构化结果、读取参数和 ESM mock 恢复,避免只测一条字符串。 - “fence reader distinguishes a missing file from a real directory read failure”(同文件第 43 行)在真实文件系统上验证 ENOENT/EISDIR 的分界与目录保留,补足 mock 无法证明的真实边界。
对主干的风险
没有 blocking finding。改动只有测试和类型检查注册,未改变 runtime、schema、quota、prompt、权限或调度。已执行 focused Node 测试(4 passed)、与现有 caller parity 组合测试(26 passed)、npm run typecheck:control-plane -- --pretty false(exit 0)以及 git diff --check(通过);GitHub exact head 40377455c9da0f8e0fc0da1592be8f7d44df37f0 当前 25/25 checks 成功。负向路径覆盖了带路径 EACCES、无路径 EACCES、ENOENT、EISDIR 和资源恢复。剩余风险是不同操作系统未来可能提供新的错误文本/属性组合,现有测试不会穷举这些变体;若 reader 的错误契约扩展,应再补 characterization case。分支相对 main 显示 BEHIND,但 GitHub 报告仍为 mergeable,合并前请按 exact head 重新执行 merge-readiness。
我的整体评价
这是一个范围清晰、复用现有 coordination owner、没有生产副作用的测试补强。59 行变更全部服务于一个容易被 caller normalization 隐藏的语义边界,且 direct reader、真实目录和 parity 三层证据互补;没有新增 authority 或兼容迁移成本。基于完整 diff、调用方盘点、正负 walkthrough、26 个测试和 typecheck 结果,我对该 exact head APPROVE。无 blocking finding;未来只需在 reader 支持新平台错误形态时沿相同结构补测试。
English verdict: APPROVE for exact head 40377455c9da0f8e0fc0da1592be8f7d44df37f0. No blocking findings. The PR adds focused direct-reader coverage for path redaction and ENOENT/EISDIR classification, passes 4 focused tests, 26 parity tests, control-plane typecheck, and diff hygiene; the remote head is mergeable with 25/25 checks green.
Summary
Add direct regression coverage for
loadLegacyCoordinationWriterFenceremoving the filesystem error path while preserving the diagnostic. Existing caller parity normalization can hide this regression; these tests assert the production reader result directly.The deterministic cases inject a path-bearing
EACCESerror and an error withoutpath.ENOENTis covered separately because it returnsmissingbefore cleanup. A real temporary-filesystem case verifies missing-file versus directory-read failure and preservation of the directory. Register the new test in the control-plane typecheck.Closes #4191. This does not modify the parity-normalizer files in #4168 or any production code.
Validation
40377455c9da0f8e0fc0da1592be8f7d44df37f0npm run typecheck:control-plane, exact-path public-boundary scan, and diff checks.loopx canary premerge --from-git-diff: four selected checks plus three diff checks passed; zero failures, warnings, skips, or manual holds.Coverage: production reader semantics are tested directly with deterministic filesystem-error injection and a real filesystem case. The stub is restored together with builtin ESM exports, and the normal Node test runner isolates test files. No full provider conformance, PostgreSQL, live Goal, or Windows-local run is claimed; no provider/runtime behavior changes. Hosted CI remains separate validation.
Frontend / Visual Evidence
Scope and review
main.Boundary Checklist