Skip to content

test(connectors): wait for the scrub write before reading state.json - #1215

Merged
Birfy merged 2 commits into
mainfrom
fix/1081-connectors-until-saved
Oct 26, 2026
Merged

Birfy merged 2 commits into
mainfrom
fix/1081-connectors-until-saved

Conversation

@Birfy

@Birfy Birfy commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #1081
Owner: Dev2
Reviewer: Birfy

The flake is a test-side race, measured on main 1e9f1844: no state.json write is dropped — the read lands 2–91 ms before the install-time scrub's write does. This change makes that ordering explicit and always exercised, and turns a missed deadline into a diagnosable failure.

What changed

core/test/connectors.test.js only (and its patches/ds-bot record):

  • Deterministic delay. Inside this one case, state.json's rename is held 400 ms (patched on node:fs/promises, released with syncBuiltinESMExports()) so the wait branch is walked on every run — polling alone passes whenever the write happens to land first. Scoped to this case, restored in finally, no product code touched.
  • Deadline throws with diagnostics. If the legacy address has not gone within 3 s, the case throws with what state.json still says plus the store's own text (not saving the Bot team: …, open in another dsh, has closed), instead of handing the stale text to the three assertions.
  • The three claims are unchanged: LEGACYCREDENTIAL absent, composio.test/tool_router absent, session equal to { id: 'trs_1', apps: ['gmail'] }.

The thinking on both points comes from #1216 (closed by its author, HugoXK).

Verification at the tip

Tip SHA: bd31c07
Ran:

cd core && node --import ./test/harness-resolve.mjs --test test/connectors.test.js
# pass 29 / fail 0

# the case itself, with the in-case 400 ms delay: pass, 782 ms (the wait is walked)
# knife A: wait call replaced by the old single read, delay kept -> 20/20 red at test/connectors.test.js:639
# knife B: delay removed (SLOW_WRITE_MS=0), wait kept      -> 5/5 green, and the wait read the file once (READS=1):
#          without the delay the write is already home, so the stale branch is never entered
# knife C: delay 4000 ms > the 3 s deadline                 -> fails with:
#          "until: the legacy session address to be dropped did not land within 3 s; state.json still says …;
#           the store said: nothing — it never refused the write"
node scripts/ds-bot-patches.mjs check
# ok 10 patches apply cleanly in order; ok core/ equals ds-bot/ + patches/ds-bot/
# (pin skipped out loud: this machine has no git)

The knife runs are this branch's own; no numbers are taken from #1216.

Not run (and why): full npm run verify here — npm ci cannot materialize esbuild (its postinstall spawn fails in this sandbox) and ~24 concurrent test files abort with uv_thread_create; CI is the first full-suite run at this tip.

Checkpoint

{
  "pr": 1215,
  "claim": "Dev2",
  "issue": 1081,
  "summary": "Makes the #1081 case prove the ordering instead of hoping for it: the case itself holds this home's state.json rename for 400 ms so the wait for the scrub's write is walked on every run, and a missed deadline now throws with the file's content and the store's read-only lines. The three assertions are unchanged and no product code is touched. The patch series records the test edit so core/ still equals ds-bot/ + patches/ds-bot/.",
  "files": ["core/test/connectors.test.js", "patches/ds-bot/0001-core-fixes.patch"],
  "checks": { "ran": "connectors.test.js 29/29; in-case 400 ms delay green; wait removed 20/20 red at :639; delay removed READS=1; ds-bot-patches check ok", "result": "green", "at": "bd31c07d91febf051239d84dac23846a83b8c931" },
  "next": "none — ready for review"
}

Docs-not-needed: test-only change; no subsystem behavior or architecture changes.

…1081)

Docs-not-needed: test-only change; no subsystem behavior or architecture changes.
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 26, 2026
@Birfy

Birfy commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

duplicate of #1216, branch kept

@Birfy Birfy closed this Oct 26, 2026
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 26, 2026
@Birfy Birfy reopened this Oct 26, 2026
@Birfy

Birfy commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Reopened — the earlier duplicate of #1216, branch kept is void. New call: this PR is the one that lands #1081 (in-repo branch, we can iterate on it); #1216 stays closed.

Two things adopted from #1216, both are where that version is stronger:

  1. Deterministic delay. Delay state.json's rename inside this case (as #1216 did: 400 ms plus syncBuiltinESMExports()), so the wait branch is actually exercised on every run. Polling alone passes whenever the write happens to land first, which means the race is never triggered.
  2. Timeout throws with diagnostics. On deadline, throw carrying what the file still says plus the store's own read-only lines (not saving the Bot team: …, open in another dsh, has closed) instead of returning the stale text and letting the three assertions read it.

This comment records that the deterministic-delay and diagnostics ideas come from #1216. Owner stays Dev2 alone; Fixes #1081 lives only on this PR. Branch kept, no force push.

…#1081)

The case now delays this home's state.json rename by 400 ms inside itself, so the wait
branch runs on every execution, and its deadline throws with the store's own read-only
lines instead of returning the stale text.

Docs-not-needed: test-only change; no subsystem behavior or architecture changes.
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 26, 2026
@Birfy

Birfy commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at bd31c07
Written-by: Reviewer
Attestation: bd31c07 role=Reviewer

测试侧的一单:只动 core/test/connectors.test.js 与它在 patches/ds-bot/0001-core-fixes.patch 里的记录,产品代码没碰(我核过文件清单和 diff)。你点的三处我都自己跑过,不引用正文的数字。

1. 延迟的作用域(读实现 + 量时间)

  • 补丁装在该用例体内:把 fsp.rename 换成「目标正好是本用例 home 的 state.json 时先等 400 ms」的包装,finally 里 fsp.rename = realRename 并再调一次 syncBuiltinESMExports() 还原。除这一处以外没有第二处全局改动。
  • 它确实生效,不是写上去好看:同一个用例 SLOW_WRITE_MS=400 时 546 ms、改成 0 时 160 ms(Node 22.23.3);Node 24.22.1 上是 784 ms 对 396 ms。差值就是那 400 ms,说明 store 走的 node:fs/promises 具名 rename(core/src/host/store.js:5)已被换掉——我在本机另写了两个模块的小实验,确认 syncBuiltinESMExports() 会把具名导入的绑定一起换掉。
  • 隔离性:包装只延时「目标正是本用例那个 state.json」的 rename,同文件其它用例用各自的 home,不受影响;整文件 29/29 绿。

2. 超时的诊断(刀 + 真实拒绝路径)

  • 刀 C 复现:SLOW_WRITE_MS=4000(> 3 s)→ 红,消息为 until: the legacy session address to be dropped did not land within 3 s; state.json still says {…}; the store said: nothing — it never refused the write。
  • 我把 store 真的拒绝写那条路也走了一遍(临时探针:另写一个 state.lock 指向另一个 live pid,跑完即删):消息里带上了 ds-bot: The Bot team in /tmp/… is open in another dsh (Web or Desktop, pid 1 on …) 与 ds-bot: not saving the Bot team: …。所以 warns 的接线是真的,不是只有 “nothing” 那一支。
  • 一处小瑕疵(不阻断):捕获写的是 String(line ?? format),所以 store.js:386 那句 the other dsh has closed 只会留下 where 参数、正则会匹配不到;真正会命中的是 not saving… / open in another dsh 两句,对这个用例够用。

3. 三条断言未变:LEGACYCREDENTIAL 不在、composio.test/tool_router 不在、session 恰为 { id: 'trs_1', apps: ['gmail'] };sent.posts.length === 1(会话复用)也还在。

其它

  • node scripts/ds-bot-patches.mjs check:ok 10 patches apply cleanly in order + ok core/ equals ds-bot/ + patches/ds-bot/,exit 0。这条 check 是活的:我中途在 core/test/ 留了一个多余文件,它立刻报 committed but not produced by the patches 并 exit 1。
  • 该用例连跑 5 次全绿;Node 22.23.3 与 Node 24.22.1 上整文件都是 29/29。
  • CI 我自己读了 head:14 条 check 全 success。enqueue-guard 的输出写着 no PASS needed: computed tier T1 —— 这张单按闸不要求 PASS,所以这条结论是审查记录、不是放行依据,入队仍归 Chief。

两条非阻断,记在这里:

  • 包装的安装点在 try 之外。 fsp.rename = … 与 syncBuiltinESMExports() 写在 const second = setup({ home, warns }) 之前,try 从那之后才开始;setup() 若抛,包装会留到本文件后续用例。实际影响很小(被延时的目标只是本用例那个临时 home 的 state.json,后续用例写别的 home),但挪进 try 或加一个 t.after 还原会更稳。
  • 「确定性」依赖 store 继续用具名 rename 导入。 若哪天换成 fs.promises.rename 之类的另一条路径,包装会静默不生效,用例退回「大多数时候碰巧绿」——我验过:延时置 0 时它同样 29/29 绿。要把它钉住,最便宜的是加一个计数断言(包装至少被调用一次)。

Ran:

$ cd <head tree> && node scripts/ds-bot-patches.mjs check
# ok 10 patches apply cleanly in order; ok core/ equals ds-bot/ + patches/ds-bot/; exit 0  (pin skipped: no git here)
$ TZ=UTC node --test core/test/connectors.test.js            # Node 22.23.3
# tests 29  # pass 29  # fail 0
$ TZ=UTC node --test --test-name-pattern="an address saved by an older version" core/test/connectors.test.js   # x5
# 5/5 runs: pass 1, fail 0; duration ~546 ms each
$ TZ=UTC /path/node-24.22.1 --test core/test/connectors.test.js                                            # Node 24.22.1
# tests 29  # pass 29  # fail 0
$ perl -pi -e 's/^const SLOW_WRITE_MS = 400$/const SLOW_WRITE_MS = 0/' core/test/connectors.test.js && TZ=UTC node --test --test-name-pattern=… core/test/connectors.test.js
# pass 1; duration 160 ms  (Node 22) / 396 ms (Node 24)  -- the 400 ms is what the wrapper adds
$ 同一用例把 untilFile 调用换回单次 readFileSync(延时保留)&& node --test --test-name-pattern=…
# not ok 1 - an address saved by an older version is dropped  (pass 0 / fail 1)
$ SLOW_WRITE_MS=4000 && node --test --test-name-pattern=…
# not ok 1 … error: 'until: … did not land within 3 s; state.json still says {…}; the store said: nothing — it never refused the write'
$ 探针:写入指向另一个 live pid 的 state.lock 后重跑同一用例
# not ok 1 … error: '… the store said: ds-bot: The Bot team in /tmp/dsb-connectors-… is open in another dsh (Web or Desktop, pid 1 on …) | ds-bot: not saving the Bot team: …'
$ LC_ALL=C grep -c $'\xef\xbf\xbd' core/test/connectors.test.js patches/ds-bot/0001-core-fixes.patch
# 0 / 0

Not run:

  • 全量 npm run verify:本机没有这个 checkout 的完整安装(npm ci 的 esbuild 装不上),我只借用了一份已装好的 @deepseek-ai harness(0.2.0-rc.2)跑单文件,并单独装了 Node 24 的二进制。整棵树的 lint / 并发跑全部测试文件以 CI 为准,CI 在 head 上 tests-ok、package、plugins (…)、Node 24 全 success。
  • 没有真起第二个 dsh 进程来触发 read-only:那条是用 state.lock 探针模拟的(跑完删除),只验证诊断文本,不验证 dsh 的真实行为。

@github-actions github-actions Bot added the pass label Oct 26, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 26, 2026
Merged via the queue into main with commit a23995f Oct 26, 2026
13 of 14 checks passed
@Birfy
Birfy deleted the fix/1081-connectors-until-saved branch October 26, 2026 03:02
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

connectors.test.js:639 goes red under the full core suite: the legacy address is still in state.json when the test reads it

1 participant