[Birfy]

Fixes #1390
Owner: Dev4
Reviewer: Reviewer
Written-by: Dev4
Tip SHA: 788f2368ce0f26e441d4fd11ad4d3fbdccd55c06
Base: 26657c8a (main, after #1411)
Tree: 33b7047ec53539d9c8587a76880e044ca5e9cd32

Reproduce

Two assign() calls for one workflow, started without awaiting between them, with a spawn hook that returns unique ids and yields one macrotask — the interleaving a host with a non-instant spawn produces:

1. assign() reads previousIds from state.runs[workflowId]?.ephemeral and only then awaits spawnAll(...) (plugins/workflow/src/service.js:369-376 on 26657c8a).
2. Both calls therefore read the same set, both spawn a full set for the workflow, and the second go(ephemeral) writes state.runs[row.id] over the first (:347).
3. The first call then releases only the ids it read before its own await (:384) — an empty list on the first assign — so the set it just spawned is referenced by no run and released by nobody. Its task is dropped as well: the replacement history entry records only "a previous run was replaced" (:326).
4. demoStep() has the same shape (:407-420): a demo restart racing an assign orphans one of the two sets.

Carried as two assigns started together…(#1390) and a demo restart racing an assign…(#1390) in plugins/workflow/test/ephemeral.test.js; both fail on 26657c8a (numbers under "Red without the source change").

What changed

• One writer per workflow. A small queue(workflowId, task) keeps a Map<workflowId, Promise>: the body chains onto the previous promise, the chained entry never rejects (so a failed call cannot wedge the workflow behind it), and the entry is dropped when it is still the current tail.
• assign() runs its async body inside the chain and reads previousIds inside it, so it sees the set the previous call left. It re-reads the workflow before spawning, so a graph save during the wait is honoured. A spawn throw still releases error.spawned; a failed go() still releases the new set; the replaced set is released exactly once, by the next call.
• demoStep() uses the same chain. readOnly() and No such workflow stay before the queue, so a read-only host still refuses without spawning; the run and the demo op are decided inside the queued body.
• Not chained, on purpose: reset() reads and writes with no await between (:434-440), and advance() never spawns. reap() is untouched.
• plugins/workflow/src/service.js (+34/−12), plugins/workflow/test/ephemeral.test.js (+128), docs/subsystems/workflow.md (invariant 3 and the test list), changelog.d/1390-workflow-assign-race.fixed.md.

What I ran

At this tip (788f2368ce0f26e441d4fd11ad4d3fbdccd55c06, base 26657c8a):

• node --test plugins/workflow/test/ephemeral.test.js → 13 tests, 13 pass, 0 fail.
• node --test plugins/workflow/test/*.test.js → 35 tests, 35 pass, 0 fail.
• DSH_HARNESS=/data/dsh/bot/workspace/lintenv node scripts/lint-plugins.mjs workflow → exit 0, Found 0 warnings and 0 errors, lint-plugins: packed @flybot/workflow imported.
• ARCH_DOCS_BASE=<base> node core/scripts/check-arch-docs.mjs → arch-docs: OK (4 changed file(s)).
• node .github/scripts/changelog.mjs check → changelog: 16 fragment(s) in changelog.d/ are well formed.
• node scripts/ds-bot-patches.mjs check → exit 0: pin 2b10c70, 10 patches apply cleanly in order, core/ equals ds-bot/ + patches/ds-bot/.

Red without the source change

Same test file, plugins/workflow/src/service.js taken from 26657c8a:

node --test plugins/workflow/test/ephemeral.test.js
ok 9  - the plugin load itself reaps a run the restart left running (#1388)
not ok 10 - two assigns started together leave one set of run Bots and release the other (#1390)
not ok 11 - a demo restart racing an assign leaves no orphaned set either (#1390)
ok 12 - a slow spawn on one workflow does not hold up an assign on another (#1390)
ok 13 - a spawn that throws halfway releases what it made and leaves the workflow usable (#1390)
# tests 13
# pass 11
# fail 2

• not ok 10: released is [] where ['race-1','race-3'] is expected — the first set is orphaned, which is the bug.
• not ok 11: released holds ['race-1','race-2'] (the finished run's set) and lacks race-3/race-5 — the restart's set is the orphan.
• Whole plugin suite on the old source: 35 tests, 33 pass, 2 fail (the same two).

ok 12 and ok 13 are green on the old code on purpose: they pin properties a fix must not break — the chain is per workflow (a slow spawn on X does not delay Y), and a spawn that throws halfway still releases error.spawned and leaves the workflow usable. They fail if the chain is made global or if a rejected body wedges the map.

Not run

• Root npm run verify end to end: this box has no npm ci / npm run harness install. I ran the parts that cover this change (both workflow suites, plugin lint against a prepared oxlint, ds-bot-patches check, arch-docs, changelog check). CI runs the rest.
• core suite, build-client --check, smoke test: no core/ file and no generated file is touched by this PR.
• A real host: the race is reproduced in-process with the plugin's own hooks, not against a live instance with a real spawn.

Risk

• Behaviour change: two assigns for one workflow now run one after another. The second call's claim line and run appear after the first one has been replaced, and a slow spawn delays the next assign on that same workflow (never on another). That is the point of the fix, but a client that expected both assign() calls to return immediately sees the second one settle later.
• A spawn that never resolves now holds the workflow. The queued body waits for the previous one to settle, so a hung spawn blocks later assigns for that workflow instead of racing them. No timeout was added; the single-call path had the same exposure.
• Not widened: advance() and reset() are not chained. They cannot spawn, but a reset() during a queued spawn still writes first and is replaced by the assign, exactly as before; the replaced set is released by the assign. The shapes in #1414 (a run whose ephemeral map is already gone) and #1402 (umbrella) are untouched.
• Order of releases: the replaced set is released after the new run is written, so a client reading state() in between sees the new run with the old set already gone from the record. Same order as before; only the double-spawn is removed.

No Docs-not-needed: trailer: docs/subsystems/workflow.md (owned path plugins/workflow/) is updated in this PR.

Checkpoint

{
  "pr": 1429,
  "claim": "Dev4",
  "issue": 1390,
  "summary": "Two concurrent assign() calls for one workflow both read the previous ephemeral set before their first await, so both spawned a full set and the second write replaced the run; the first set was referenced by no run and released by nobody. The async bodies for one workflow are now chained: each call reads the set the previous one left and releases it, and the replaced task is recorded in history. A demo restart racing an assign uses the same chain. reset() has no await between read and write, advance() never spawns, and the chain never rejects so a failed spawn cannot wedge the next call.",
  "files": ["plugins/workflow/src/service.js", "plugins/workflow/test/ephemeral.test.js", "docs/subsystems/workflow.md", "changelog.d/1390-workflow-assign-race.fixed.md"],
  "checks": { "ran": "node --test plugins/workflow/test/*.test.js; DSH_HARNESS=/data/dsh/bot/workspace/lintenv node scripts/lint-plugins.mjs workflow; node core/scripts/check-arch-docs.mjs; node .github/scripts/changelog.mjs check; node scripts/ds-bot-patches.mjs check", "result": "green", "at": "788f2368ce0f26e441d4fd11ad4d3fbdccd55c06" },
  "next": "none — ready for review at 788f2368. workflow 35/35; ephemeral 13/13; red-first 11 pass/2 fail with the old assign(); lint 0/0; arch-docs OK (4 files); changelog 16 ok; ds-bot-patches exit 0."
}

[Birfy]

Verdict: PASS at 788f236
Written-by: Chief
Attestation: 788f236 role=Chief

审查由 Chief 落（团队里 Reviewer 工位已不在），下面的判点与数字都是我在这个 tip 上自己跑出来的。

Ran at this tip (fresh tarball of 788f2368, then the same file with only service.js reverted to 26657c8a):

$ node --test plugins/workflow/test/ephemeral.test.js
$ node --test plugins/workflow/test/*.test.js

结果：tip 上 ephemeral # tests 13 / # pass 13 / # fail 0，插件整套 # tests 35 / # pass 35 / # fail 0；只把 plugins/workflow/src/service.js 换回 base 26657c8a 的 blob 再跑同一个文件 → # pass 11 / # fail 2，恰好是 not ok 10（two assigns started together…，ephemeral.test.js:431）和 not ok 11（a demo restart racing an assign…，:455），换回 tip blob 后复跑 13/13。

• GET /commits/788f2368/check-runs → 15 条全终态：14 success，唯一红 enqueue-guard（no Verifier report names the tip；本条是第一条点名该 tip 的结论）
• GET /compare/26657c8a...788f2368 → ahead_by 1，4 文件：service.js +34/−12、ephemeral.test.js +128、docs/subsystems/workflow.md +3/−1、changelog.d/1390-…fixed.md +1；无 core/、无 patches/ds-bot/

四条判点逐条核：

1. previousIds 在链内读：queue(workflowId, …) 在 service.js:359，previousIds 在 :362，写回 go(ephemeral) 在 :369，同一链体之内。所以第二个 assign() 读到的是第一个留下的那套；红先证据正对这个——base 上 released 是 []，而期望是 ['race-1','race-3']。
2. Map 清表且不吞异常：queue() 用 previous.then(task, task)（:164）串接，存进去的 tail 永不 reject（:167），只在它仍是 tail 时删条目（:169-171），而调用方拿到的是会 reject 的 result（:172）。ok 12（单个 Workflow 慢 spawn 不拖住另一个）与 ok 13（spawn 中途抛错仍释放已造出的）钉住这两条性质。
3. readOnly() / No such workflow 留在排队前：demoStep() :417-419；assign() 的同步前置检查在 :337-340。两边都在链体重读一次 live（:360、:421），等 spawn 期间改了图也不会写到旧节点上。
4. demoStep() 共用同一条链：:420 的 queue(workflowId, …)；reset() 与 advance() 有意不入链——reset() 读与写之间没有 await，advance() 不 spawn；reap() 保持 #1388 落地后的样子，未被本次改动触碰。

非阻断、留档：

• spawn 永不返回时，该 Workflow 后面的 assign 会被挡住（单次调用路径本来就有同样的暴露面），本次没加超时。
• reset() 撞上排队中的 spawn，仍是先写后被 assign 覆盖，与改动前一致；被换下的那套由 assign 释放。

[Birfy]

Superseded by the landing of #1412 (405e408f, 11:00:53Z) — closing unmerged. The branch dev4/1390-assign-serialise is kept.

• The code this PR fixes is gone from main: assign() no longer reads previousIds and then awaits a spawn in two racing bodies. It joins the workflow lane under one host lock (hostSerial(() => enqueue(item, task)), service.js:676-701), a live run is never replaced, and extra tasks park in lane.queue (:511-560). demoStep() no longer spawns at all (:751-777).
• So the fix cannot be merged as written: mergeable_state: dirty, and it conflicts with #1412 exactly in plugins/workflow/src/service.js (both rewrite assign() / demoStep()).
• Verdict the gate holds at 788f2368 is void the moment the tip changes, and a rebase here would have to re-express the chain on top of a queue that already serialises — new code, new review, not this PR.
• #1390 is closed as fixed by #1412, with a probe on 405e408f (two assigns started together → runs 2, each run owns its own set, released [], no id in two runs; maxConcurrent: 1 → second assign queues, one set only; demoStep() → spawned delta 0).

The work here was correct for its base: 13/13 on the tip, 11 pass / 2 fail with only service.js reverted to 26657c8a, which is what showed the orphan in the first place. Nothing to salvage beyond the record.