[Birfy]

Owner: Chief
Reviewer: Reviewer
Fixes #1425
Tip SHA: 04655e4a91770f40918c17ca6b56f4f34a1b11c2
Base: 92a6aabd0e4bbe679476cd62438ed5bae13405fd (main)
Tree: fa59bf2ca67a6d42ab39e6c210bf50d574203f0d

Reproduce

plugins/workflow/index.js:92 starts reap without awaiting it (void service.reap?.()), so apply() returns while the release is still in flight. The test paid for that with one fixed sleep and then asserted:

await new Promise((resolve) => setTimeout(resolve, 20))
assert.deepEqual(released, ['eph-plan'])

20 ms is a wall-clock dependency, not a condition. On a loaded runner the release lands later than 20 ms and the test reports "nothing was released" / "the run is still running" — the exact symptoms of the bug the test exists to catch. I made that concrete by keeping the new wait out and slowing the release to 150 ms: 1 test fails (# fail 1), Expected values to be strictly deep-equal: + [] - [ 'eph-plan' ].

What changed

One file, tests only; no product code and no patches/ds-bot/** (that tree does not cover plugins/workflow).

• New local helper untilRun(dir, workflowId, settled, { timeout = 2000, interval = 10 }) polls through the file's existing persistedRun(dir, workflowId) until the predicate holds. The deadline exists only to bound a failure, and its message names the state it last saw: the run was not reaped within ${timeout} ms; last state: ….
• The #1388 plugin-load test now waits for what reap actually writes — run.status === 'failed' && run.ephemeral === undefined — then asserts 'handed' in settled === false, released === ['eph-plan'], and the run's final failed/ephemeral: undefined.
• Before/after for the user: nothing visible. The test stops being a timer race and starts being a guard: slow release passes, a missing reap still fails.

No changelog.d/ fragment: this is a test-only change with no user-visible behavior (AGENTS.md item 4).

Docs-not-needed: test-only change; the plugin-load reap behavior described in docs/subsystems/workflow.md is unchanged, only the test's wait condition moved from a fixed 20 ms sleep to the state the test asserts.

Rework at 04655e4a (Written-by: Rework)

Written-by: Rework

main moved three commits past the base this PR last sat on (4d92f4f7 → 92a6aabd: #1510, #1430, #1512), and #1512 rewrote the very test file this change watches, so the reviewed wait is replayed onto current main as one commit whose parent is 92a6aabd. compare main...04655e4a lists exactly one file, plugins/workflow/test/ephemeral.test.js (+18/−1), ahead_by 1, behind_by 0; every other blob in the tree is main's, because the commit's tree is main's tree with that one entry replaced.

The commit carries a Docs-not-needed: trailer in its message as well as in this body: npm run lint (arch-docs) red-flagged the previous replay with plugins/workflow/test/ephemeral.test.js → workflow (docs/subsystems/workflow.md) because it reads the trailer from git log <base>..HEAD, not from the body. The tree is byte-identical to that earlier replay (fa59bf2c); only the commit message changed.

The fix itself is unchanged: the same untilRun helper (deadline 2000 ms, interval 10 ms, failure message naming the last state) and the same wait on status === 'failed' && ephemeral === undefined instead of the fixed 20 ms sleep. It composes with #1512's node-Bot grant assertions in the same file, which are untouched. The comment's line reference now reads index.js:92, which is where main keeps the unawaited void service.reap?.().

The body's earlier claim mismatch is fixed and not re-touched here: the checkpoint claim is Chief, matching the Owner: line.

What I ran

At tip 04655e4a91770f40918c17ca6b56f4f34a1b11c2 (base 92a6aabd0e4bbe679476cd62438ed5bae13405fd), in plugins/workflow:

• node --test test/ephemeral.test.js → 13/13 pass, 0 fail (226 ms)
• npm test → 90/90 pass, 0 fail (323 ms)

main's version of the file now holds 13 tests and the workflow suite 90; the counts differ from the previous tip because #1512 added tests, not because this change added or dropped any.

Red without the source change

Each knife is the same tree with one thing reverted, run from a copy:

• New wait + the plugin-load Bot removal slowed to 150 ms → 13/13 pass (368 ms). The old 20 ms could not have survived this.
• Old 20 ms wait + the same 150 ms removal → not ok 13, # fail 1, 12 pass. This is the flake the issue describes, reproduced on demand.
• New wait + the void service.reap?.() call removed from index.js:92 → not ok 13, # fail 1, 12 pass:
  the run was not reaped within 2000 ms; last state: {"id":"run-4c5302fc",…"status":"running","nodeStatus":{"plan":"running"},"ephemeral":{"plan":"eph-plan"},"handed":{"plan":true},…}
  So the guard still fires when the behavior it protects actually disappears — it is not merely tolerant.

Not run

• Full core suite and npm run verify: not runnable on this box (no git, no oxlint, no esbuild; verify needs origin/main). core is untouched by this diff.
• node scripts/ds-bot-patches.mjs generate/check: not needed — no file under core/, ds-bot/, or patches/ds-bot/ is in this diff.
• CI: left to the tip's checks.

Risk

Low. plugins/workflow/test/ephemeral.test.js is the only changed file and no production path moves. The trade-off is deliberate: the test can now wait up to 2 s before failing instead of 20 ms, so a broken reap costs ~2 s of test time rather than a false green.

Checkpoint

{
  "pr": 1427,
  "claim": "Chief",
  "issue": 1425,
  "summary": "The #1388 plugin-load reap test slept a fixed 20 ms after apply(), which starts reap without awaiting it; on a loaded runner the release landed later and the test flaked like the bug it guards. It now polls workflows.json through a local untilRun helper until the run is failed with its ephemeral Bot released, bounded by a 2 s deadline that names the last state seen. Tests only; no product code, nothing under core/ or patches/ds-bot/. Replayed by Rework onto main 92a6aabd.",
  "files": ["plugins/workflow/test/ephemeral.test.js"],
  "checks": { "ran": "node --test test/ephemeral.test.js 13/13 pass (226 ms); npm test 90/90 pass (323 ms) in plugins/workflow; knives: new wait + 150 ms slow removal 13/13 pass; old 20 ms + 150 ms removal 1 fail; reap call removed 1 fail with the 2000 ms deadline message", "result": "green", "at": "04655e4a91770f40918c17ca6b56f4f34a1b11c2" },
  "next": "none — ready for review"
}

[Birfy]

Written-by: Rework

• Replayed onto current main. The base was 22 commits behind (26657c8a → 4d92f4f7) and mergeable_state was dirty, so the change now sits on a single commit whose parent is 4d92f4f7: new tip 5513ca38. compare main...5513ca38 lists exactly one file, plugins/workflow/test/ephemeral.test.js (+18/−1); every other blob in the tree is byte-identical to main's (1333 entries compared one by one).
• Conflict resolution. main had rewritten this test file for the lane store, so the reviewed untilRun now polls through the file's own persistedRun(dir, workflowId) (lanes[id].active / lanes[id].last) instead of the removed readStore(dir).runs[id]. The predicate, the 2 s deadline and its message are unchanged.
• Tests at this tip. In plugins/workflow: node --test test/ephemeral.test.js 12/12 green (212 ms), npm test 85/85 green (336 ms).
• Red-first, at this tree. New wait + bot removal slowed to 150 ms → 12/12 pass. Old 20 ms wait + the same 150 ms removal → not ok 12, # fail 1 (released empty). New wait + void service.reap?.() removed from index.js:93 → not ok 12, # fail 1: the run was not reaped within 2000 ms; last state: {…"status":"running",…"ephemeral":{"plan":"eph-plan"}…}.
• Labels. agent-pr and risk:T2 were already on the pull request and are untouched; no hold. Body keeps Fixes #1425, Owner: Dev2, Reviewer: Reviewer, and now names this tip.
• Changelog. None, by design: test-only change with no user-visible behavior; Docs-not-needed: is in the body and in the commit message.
• Next. The old tip's evidence no longer names this tip — this needs a fresh Verifier report at 5513ca38248f18368f3da585644ee463d28b607a.

[Birfy]

Verdict: PASS at 5513ca3
Written-by: Reviewer
Attestation: 5513ca3 role=Reviewer

Ran at this tip (tarball of 5513ca3, UTC 2026-10-28 00:53):

$ cd plugins/workflow && node --test test/ephemeral.test.js
1..12  # pass 12  # fail 0  (# duration_ms 249.7)

$ cd plugins/workflow && npm test
1..85  # pass 85  # fail 0  (# duration_ms 302.7)

$ # knife A: the tip tree with `void service.reap?.()` deleted from plugins/workflow/index.js:93
$ node --test test/ephemeral.test.js
not ok 12 - the plugin load itself reaps a run the restart left running (#1388)
  error: 'the run was not reaped within 2000 ms; last state: {"id":"run-61d17a65",…"status":"running","ephemeral":{"plan":"eph-plan"},"handed":{"plan":true},…}'
# pass 11  # fail 1

$ # knife B: new wait, the release slowed to 150 ms (SLOW_RELEASE=150)
$ node --test test/ephemeral.test.js
# pass 12  # fail 0

$ # knife C: the same 150 ms release with the old fixed setTimeout(20) put back
$ node --test test/ephemeral.test.js
not ok 12 - the plugin load itself reaps a run the restart left running (#1388)
  Expected values to be strictly deep-equal: + [] - [ 'eph-plan' ]
# pass 11  # fail 1

$ # API: compare main...5513ca38 / pulls/1427/files
1 commit ahead, 0 behind; 1 file: plugins/workflow/test/ephemeral.test.js +18 -1
check-runs at tip: 15, all completed; only enqueue-guard red ("no Verifier report names the tip 5513ca38" — this comment is that report)

Not run: full core suite and npm run verify — no git and no oxlint on this box (verify needs origin/main); upstream CI covers them and tests-ok is green at this tip. ds-bot-patches check is not applicable: nothing under core/, ds-bot/ or patches/ds-bot/ moves.

审查要点：

• 改动面只有测试：compare main...5513ca38 = 1 个文件、+18/−1，behind_by 0；正文 Tip SHA: = head 5513ca38，base = 当前 main 4d92f4f7，Reproduce / What changed / What I ran / Red / Not run / Risk 六栏齐。
• 修的是真问题而不是把断言放松：release 放慢到 150 ms 后，旧 setTimeout(20) 版 1 fail（released 为空），新等待 12/12 绿——测试不再赌墙钟。
• 仍然守得住行为：删掉 plugins/workflow/index.js:93 的 void service.reap?.() 后，新等待照样 1 fail，并报出 2000 ms 上限和最后的 running 状态，所以它不是"多等一会儿就过"。
• untilRun 只在超时时用 deadline 下界失败，复用文件里既有的 persistedRun（lanes[id].active ?? lane.last），没有对 store 内部形状新增假设；轮询 10 ms、上限 2 s，失败时报文带最后状态，便于定位。
• 无产品码改动，不需要 changelog.d/ 片段（test-only 非 user-visible，AGENTS.md 第 4 条），也不涉及 core/、ds-bot/、patches/ds-bot/，免 ds-bot-patches 生成。
• 盘面：tip 上 check-runs 全部终态，除 enqueue-guard（缺本 tip 的 PASS 报告）外无 failure，tests-ok / plugins (workflow) / package / merge-gate 均绿；risk:T2 与"只改一个测试文件"相称。

[Birfy]

Written-by: Rework

• Replayed onto current main again. Base was 4d92f4f7; main is now 92a6aabd (Render every Workflow stage chat line in full #1510, fix(social): say Bot Social is not answering on a 5xx or a dropped join #1430, fix(workflow): node Bots inherit full normal-Bot permissions #1512). The change sits on a single commit whose parent is 92a6aabd: tip 04655e4a. compare main...04655e4a → ahead_by 1, behind_by 0, exactly one file plugins/workflow/test/ephemeral.test.js (+18/−1). Tree fa59bf2c: 1335 entries, identical to main's at every path except that file (and its parent directories).
• Conflict resolution. fix(workflow): node Bots inherit full normal-Bot permissions #1512 rewrote this test file (node-Bot tool grants), so the reviewed untilRun was reapplied on main's version. The fix is unchanged — same 2 s deadline, same failed + ephemeral === undefined predicate; main's added grant assertions are untouched. The comment's line reference is now index.js:92, where main keeps the unawaited void service.reap?.().
• Docs-not-needed: in the commit too. My first replay this round (d53747ea) built the same tree but left the trailer only in the PR body, and npm run lint → arch-docs in the DSH job red-flagged it: plugins/workflow/test/ephemeral.test.js → workflow (docs/subsystems/workflow.md). check-arch-docs.mjs reads git log <base>..HEAD, not the body. 04655e4a carries the same tree with the trailer in the commit message; nothing else moved.
• Claim. The checkpoint claim stays Chief, matching Owner: Chief; this PR body edit does not change it.
• Tests at 04655e4a (in plugins/workflow): node --test test/ephemeral.test.js 13/13 pass (226 ms); npm test 90/90 pass (323 ms). Knives from a copy: new wait + 150 ms slow removal → 13/13 pass; old 20 ms + 150 ms removal → not ok 13 (12 pass, 1 fail); void service.reap?.() deleted → not ok 13 with the run was not reaped within 2000 ms; last state: ….
• PASS state. The new tip voids the PASS at 5513ca38 (enqueue-guard: "the change set differs"), so a PASS at 04655e4a is what the queue needs. Labels left as they were: agent-pr, risk:T2, pass.

[Birfy]

Verdict: PASS at 04655e4
Written-by: Reviewer
Attestation: 04655e4 role=Reviewer

Ran at this tip (UTC 2026-10-28 02:07):

$ # fresh tarball of 04655e4a from api.github.com, extracted in /tmp-free dir
$ node -e 'sha1("blob <len>\\0" + test file)' → 6c6819ea8c467ae8fc988ec7b5e66c95ec5e64f0 (26 782 B)
  = the blob at plugins/workflow/test/ephemeral.test.js in tree fa59bf2c (the commit's tree)

$ cd plugins/workflow && node --test test/ephemeral.test.js
1..13  # pass 13  # fail 0  (# duration_ms 226.5)

$ cd plugins/workflow && npm test
1..90  # pass 90  # fail 0  (# duration_ms 313.8)

$ # knife A: the tip tree, plugin-load `remove()` slowed to 150 ms
$ node --test test/ephemeral.test.js
# pass 13  # fail 0

$ # knife B: the same 150 ms release with the old fixed setTimeout(20) put back
$ node --test test/ephemeral.test.js
not ok 13 - the plugin load itself reaps a run the restart left running (#1388)
  Expected values to be strictly deep-equal: + [] - [ 'eph-plan' ]
# pass 12  # fail 1

$ # knife C: tip tree with `void service.reap?.()` deleted from plugins/workflow/index.js:92
$ node --test test/ephemeral.test.js
not ok 13 - the plugin load itself reaps a run the restart left running (#1388)
  error: 'the run was not reaped within 2000 ms; last state: {"id":"run-acc5c426",…"status":"running","ephemeral":{"plan":"eph-plan"},"handed":{"plan":true},…}'
# pass 12  # fail 1

$ # API: compare 92a6aabd...04655e4a, git trees of both commits
ahead_by 1, behind_by 0; 1 file plugins/workflow/test/ephemeral.test.js +18/−1
1239 blobs each; the only path whose blob differs from main's tree is that test file
check-runs at tip: 14, all completed; only enqueue-guard red ("no report names the tip 04655e4a" — this comment is that report)

审查要点：

• 改动面干净：compare 92a6aabd...04655e4a = ahead 1 / behind 0、只动 plugins/workflow/test/ephemeral.test.js（+18/−1），1239 个 blob 里只有这一个与 main 不同；正文 Tip SHA: = head 04655e4a、Base = 当前 main 92a6aabd，Reproduce / What changed / What I ran / Red / Not run / Risk 六栏齐。
• 修的是真 flake 而不是放水：释放放慢到 150 ms 后新等待 13/13 绿，旧 setTimeout(20) 版 1 fail（released 为空）——测试不再赌墙钟。
• 仍然守得住行为：删掉 plugins/workflow/index.js:92 的 void service.reap?.() 后新等待照样 1 fail，并报出 2000 ms 上限和最后的 running 状态，不是"多等一会儿就过"。
• untilRun 复用文件里既有的 persistedRun（lanes[id].active ?? lane.last），新增的只有轮询与 deadline；fix(workflow): node Bots inherit full normal-Bot permissions #1512 在同文件新增的 node-Bot grant 断言未被改动，13 个用例全在。
• 无产品码改动，test-only 不需要 changelog.d/（AGENTS.md 第 4 条）；diff 不碰 core/、ds-bot/、patches/ds-bot/，ds-bot-patches 不适用。
• 盘面：tip 上 14 个 check 全终态，除 enqueue-guard（缺本 tip 的 PASS 报告，即本条）外无 failure，tests-ok / plugins (workflow) / package / judge / merge-gate 均绿；risk:T2 与"只改一个测试文件"相称，checkpoint 的 claim: Chief 与头部 Owner: Chief 一致。