[Birfy]

Written-by: Dev

Fixes #2704

Owner: Dev
Reviewer: Reviewer

What changed

A window that starts read-only behind another writer now treats taking state.lock as a state change. The first call that finds the host writable re-reads workflows.json once, then runs the load-time reaper once on that adopted store. That ends the dead window's running run as host-ended and releases its step Bots; lane.queue is never touched, so a card that was only queued keeps its place and starts when the lane drains. A writer that later loses the lock is still refused, and write() refuses before its first byte if the host became read-only or this window never took the store over.

Red first

test/takeover.test.js (real child processes, real state.lock, real kill -9) with plugins/workflow/src/service.js replaced by the base blob of 71855d9d:

not ok 1 - F1 a window that takes over re-reads the store before its first write (#2704)
  actual: [ "before" ]   expected: [ "after", "before" ]
not ok 2 - F2 a takeover reaps the run the dead window left running, and leaves queued cards (#2704)
  the takeover did not release the dead run's step Bots once
# pass 0  # fail 2

Ran at this tip:

$ cd plugins/workflow && TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness node --import ./test/resolve.mjs --test test/takeover.test.js
# tests 2  pass 2  fail 0
$ TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 491  pass 491  fail 0
$ cd ../.. && DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness node scripts/lint-plugins.mjs
# Found 16 warnings and 0 errors.
$ node .github/scripts/changelog.mjs check
# changelog: 1 fragment(s) in changelog.d/ are well formed

Not run: whole-repo npm run verify (the brief limits this to the workflow suite; no core/ file changed, so scripts/ds-bot-patches.mjs and build-client.mjs --check do not apply). core/scripts/check-arch-docs.mjs needs a git history this box does not have; the owned page docs/subsystems/workflow.md is updated in the diff.

Checkpoint

{
"pr": 2707,
"claim": "Dev",
"issue": 2704,
"summary": "A read-only window that takes the workflow store over re-reads workflows.json before its first change and runs the load-time reaper once. It no longer writes the copy it loaded behind the dead window, and no longer leaves that window's running run holding the lane. Queued cards are untouched and start when the lane drains.",
"files": [
"plugins/workflow/src/service.js",
"plugins/workflow/test/takeover.test.js",
"docs/subsystems/workflow.md",
"changelog.d/2704-workflow-takeover.fixed.md"
],
"checks": {
"ran": "workflow suite + takeover test; scripts/lint-plugins.mjs; .github/scripts/changelog.mjs check",
"result": "green",
"at": "04ec1403b8024cb689f272f98ba9122887c69f80"
},
"next": "none — ready for review"
}

[Birfy]

Verdict: PASS at 04ec140
Written-by: Reviewer
Attestation: 04ec140 role=Reviewer

Ran at this tip:

$ cd plugins/workflow && node --import ./test/resolve.mjs --test test/takeover.test.js
# tests 2  # pass 2  # fail 0
$ node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 491  # pass 491  # fail 0
$ cp /data/dsh/bot/workspace/p2707/base/plugins/workflow/src/service.js src/service.js && node --import ./test/resolve.mjs --test test/takeover.test.js
# tests 2  # pass 0  # fail 2  (F1 and F2 red without the change)
$ diff -u /data/dsh/bot/workspace/p2707/base/plugins/workflow/src/service.js src/service.js
# only the takeover/reap hunks above; no maxConcurrent, assign, setConcurrency, stop or reset semantics moved

Review points, against the tip tree extracted from the tarball (harness packages linked from a local 0.2.1-rc.2 install; a first run without them failed only on ERR_MODULE_NOT_FOUND @deepseek-ai/dsh-llm, not on code):

• F1 (takeover = state change). writer = !readOnly() at construction; writable() sends the first call that finds the host writable through takeover() → adoptStore() (re-read + migrate + purge + seq) → re-check readOnly() → only then writer = true. Read order is lock → re-read → write, one time only; a writer that later loses the lock is refused exactly as before (reapRuns and write() both re-check). write() refuses before its first byte when !writer, so an endpoint that skipped the guard cannot land a stale copy.
• F2 (reap once). The load-time body is extracted verbatim into reapRuns(); takeover() starts it once and reap()/dispose() hand that same promise back instead of releasing twice. lane.queue is untouched: F2 asserts the queued card starts, the dead run is archived failed/host-ended, its Bots are released exactly once, and activeCount/queueDepth return to 1/1. A second call is a no-op (run no longer running, no ephemeral map), and a failed release keeps its id for the next retry.
• Rest of the +171/−86 is the extraction, the writable() guard on each write endpoint, and the state()/dispose() refresh — no change to maxConcurrent, assign/queue rules, setConcurrency, stop or reset return values or codes.
• Red first confirmed: base blob of service.js (71855d9d) with the new test file makes both F1 and F2 fail; the tip passes.
• Tests are real: takeover.test.js spawns real child processes, uses the real core/src/host/home-lock.js and a real kill -9; neither the lock nor the store is mocked.
• Scope: 4 files (service.js, takeover.test.js, docs/subsystems/workflow.md, changelog.d/2704-workflow-takeover.fixed.md); no core/, no inflight.json is truncated before it is written, so a crash drops every turn it was meant to resume #2705/A timeline trace that started behind another writer never retries the lock, and flush() reports success while dropping spans #2706 files.
• Checks at tip: tests-ok, plugins (workflow|social|tree|timeline), package, merge-gate, reviewed tip and risk tier green; only enqueue-guard red (no PASS yet) plus a cancelled judge run from a duplicate workflow trigger.