Skip to content

test(connectors): install the #1081 write-delay wrapper inside try, and assert it ran #1224

Closed

Description

@Birfy

来源:#1215 的 Reviewer 结论(评论 6407209855,Verdict: PASS at bd31c07d91febf051239d84dac23846a83b8c931,02:57:49Z)。结论里明确标注这两条非阻断,PASS 本身有效。#1215 已于 03:02:53Z 合入 a23995f328053a10967375483fbf26b76ea192f9(现在就是 main head),#1081 随之 closed/completed。这张单接的是「PASS 之后别烂在评论里」的部分。

#1215 现在让 core/test/connectors.test.js 里那个用例自己把本用例 home 的 state.json 的 rename 推迟 400 ms,从而每次都真的走一遍「等 install 期 scrub 写完」的分支。做法有效(Reviewer 两条 Node 版本上都量过 400 ms 的实际差值),但有两处会让它在未来静默退化。

1. 包装的安装点在 try 之外

fsp.rename = … 与 syncBuiltinESMExports() 写在 const second = setup({ home, warns }) 之前,try 从那之后才开始。setup() 若抛错,包装不会被还原,会留到本文件后续用例里。

实际影响小(被延时的目标只限「目标正好是本用例那个临时 home 的 state.json」,后续用例写别的 home),但正确写法是二选一:

  • 把安装挪进 try(还原仍在 finally);或
  • 加 t.after(() => { fsp.rename = realRename; syncBuiltinESMExports() })。

验收:让 setup() 抛一次(临时改一行即可)→ 包装必须被还原,后续用例不受影响;不改代码时同一个实验应能看到残留(先证明红,再改)。

2. 「确定性」隐式依赖 store 继续用具名 rename 导入

被包装的是 node:fs/promises 的具名导入(core/src/host/store.js:5)。若哪天 store 换成 fs.promises.rename 之类另一条路径,包装会静默不生效,用例退回到「大多数时候碰巧绿」——Reviewer 验过:延时置 0 时同样 29/29 绿。

要钉住它,最便宜的是加一个计数断言(例如 assert.equal(renameCalls >= 1, true, 'the wrapper never ran — store no longer uses the named rename import')),并在用例结束时断言计数。

注意这条断言钉的是什么:它钉的是「具名 rename 这条路径还在」,不是「延时真的生效」——SLOW_WRITE_MS=0 时包装仍会被调用,只是不延时。若要连「等待分支真的被走到」也钉住,需要一条不依赖墙钟的观测量(例如 scrub 那次 write 的完成顺序,或读计数 READS),这正是当初删掉 untilState 想避开的陷阱,别再造一个只看时间的断言。

验收:把 store.js 的 rename 换成 node:fs 的另一条等价路径 → 新断言必须红。

不做

  • 不改产品代码,core/src/host/store.js 的两句诊断文本仍按 #1215 的样子留着。
  • 若这张单要动 core/test/connectors.test.js,记得同步 patches/ds-bot/0001-core-fixes.patch 并跑 node scripts/ds-bot-patches.mjs check(有 git 时优先 generate;没有 git 才手改 patches 后 check)。
  • 标签留 Triage:从现有六个 area:* 里挑(不要再打已删除的 area:host)。

Activity

  1. added
    bugSomething isn't working
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    area:automationScheduled tasks and automation
    on Oct 26, 2026
  2. Birfy commented on Oct 26, 2026

    @Birfy
    CollaboratorAuthor

    Triage(2026-10-26):查重完成,结论 agent-ready(标签 bug + area:automation + agent-ready)。

    1. 查重

    • 不重复:来自 #1215 的评审尾项;#1215 已合入(merged_at 2026-10-26T03:02:53Z,tip bd31c07d),#1081 随之 closed/completed——所以这张是唯一接尾巴的单。#1216(同分支思路、作者自关)不再跟。
    • 别和 #1225、#1222 混:那两张是产品面,这张只动 core/test/connectors.test.js 与它的 patch 镜像。

    2. 为什么标 bug

    两条都是**「已经绿了,但将来会静默退化成碰巧绿」**:包装安装在 try 之外(setup() 抛错就漏还原)、以及成功依赖 store 继续用具名 rename 导入。退化后用例仍 29/29 绿,等于没有证据——这正是缺陷。

    3. 验收补充(正文两条之外的钉子)

    • 第 2 条那个「计数断言」只钉『具名 rename 路径还在』,不钉「延时真的生效」——正文自己写明了,照写,别再造成只看墙钟的断言。
    • 「先证明红再改」的顺序要留在 PR 正文里:setup() 抛错的实验、把 store.js 换成 fs.promises.rename 的实验,两条都要有改前的红。
    • 动 core/test/connectors.test.js 就要同步 patches/ds-bot/0001-core-fixes.patch 并跑 node scripts/ds-bot-patches.mjs check(有 git 优先 generate)。

    risk:T* 只由控制器写(不手写、不转录);认领评论里带分支名。

  3. Birfy commented on Oct 26, 2026

    @Birfy
    CollaboratorAuthor

    Claiming for Dev2 — branch fix/1224-connectors-wrapper-in-try, PR #1262.

    Plan, matching the two acceptance lines in the issue and the Triage note:

    1. Move the connectors.test.js:639 goes red under the full core suite: the legacy address is still in state.json when the test reads it #1081 wrapper install (fsp.rename = … + syncBuiltinESMExports()) inside the try, keeping the finally restore, so a throwing setup() cannot leak it. Red first: a probe where setup() throws plus a later test asserting fsp.rename was restored — it leaks on main.
    2. Add a renameCalls count asserted at the end of the case. It pins the named rename import path, not that the delay took effect. Red first: store.js writing through a node:fs rename — the case stays green on main, red with the count.

    Then hand-edit the test/connectors.test.js hunk in patches/ds-bot/0001-core-fixes.patch (no git on this host, so generate is unavailable) and prove it with node scripts/ds-bot-patches.mjs check.

  4. added
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    on Oct 26, 2026
  5. self-assigned this
    on Oct 26, 2026
  6. removed
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    on Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:automationScheduled tasks and automationbugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions