Skip to content

fix(workflow): keep a read-only host out of reap and dispose - #2674

Merged
Birfy merged 1 commit into
mainfrom
fix/2673-readonly-reap
Nov 8, 2026
Merged

Birfy merged 1 commit into
mainfrom
fix/2673-readonly-reap

Conversation

@Birfy

@Birfy Birfy commented Nov 8, 2026 •

Copy link
Copy Markdown
Collaborator

Written-by: Dev

Fixes #2673

A read-only host is the second DSH window on the same DSH_HOME — the reader core/src/host/home-lock.js describes. It implemented readOnly() on eleven endpoints but not on the two lifecycle paths that write the whole store: loading the plugin called reap(), closing the window called dispose(), and both treated every run still marked running as one whose host had died — endRun(..., { code: 'host-ended' }), releaseAll(run.ephemeral), then a whole-store writeback from the reader's older copy.

reap() now returns immediately when readOnly() is true, next to the existing typeof release !== 'function' guard; dispose() clears its local timers and returns Promise.resolve() without ending runs, releasing step Bots or writing. The writer path (readOnly: () => false) is untouched: same endRun, same releaseAll, same write.

Owner: Dev
Reviewer: Reviewer

Red first

plugins/workflow/test/service.test.js (new; the issue's acceptance command), run against main 656fadab71170275eeeb924fdfddc33f8072ba81 with the implementation unchanged:

cd plugins/workflow && TZ=UTC DSH_HARNESS=<core>/dev/harness \
  node --import ./test/resolve.mjs --test test/service.test.js

not ok 1 - #2673: a read-only host does not reap the writer's live run
  error: the read-only host overwrote the writer's store
not ok 2 - #2673: a read-only host does not dispose the writer's live run
  error: the read-only host overwrote the writer's store
not ok 3 - #2673: a read-only host releases none of the writer's step Bots
  error: reap() released the writer's step Bots
  + [ 'eph-plan-1', 'eph-code-2' ]
ok 4 - #2673: a writable host still reaps and disposes the run it did not start
# tests 4  # pass 1  # fail 3

Both red lifecycles wrote the reader's stale copy: lane.active emptied, last.status = 'failed', failure.code = 'host-ended' — the writer's just-written plan: done / code: running overwritten. Test 4 is the writer-side guard and passed before and after.

Ran at this tip: 644cde0

  • cd plugins/workflow && TZ=UTC DSH_HARNESS=<core>/dev/harness node --import ./test/resolve.mjs --test test/service.test.js test/assign.test.js — 24/24 pass
  • cd plugins/workflow && TZ=UTC DSH_HARNESS=<core>/dev/harness node --import ./test/resolve.mjs --test "test/*.test.js" — 479 pass, 0 fail (the whole plugin suite: dispose, ephemeral, host, service)
  • node .github/scripts/changelog.mjs check — 23 fragments well formed

The four files were uploaded as blobs and the branch tree was compared against main 656fadab71170275eeeb924fdfddc33f8072ba81: status: ahead, 1 ahead, 0 behind, exactly these four paths — no file I did not touch.

Not run

  • npm run verify (whole repo): this box runs one full suite at a time; the only package touched is plugins/workflow and its suite ran in full above.
  • npm run lint / oxlint: no node_modules on this box, so oxlint is not installed.
  • arch-docs: it reads git log, and there is no git binary here. Its requirement is met the other way — the owned page docs/subsystems/workflow.md is in this commit.

Docs

docs/subsystems/workflow.md: the lifecycle paragraph now states the read-only boundary (a reader neither reaps nor disposes; the writer's own reap decides). No Docs-not-needed: trailer.

{
  "pr": 2674, "claim": "Dev", "issue": 2673,
  "summary": "A second, read-only DSH window on the same home no longer reaps or disposes the writer's live workflow. reap() returns at once when readOnly() is true, the way it already returns without a release hook, and dispose() clears only its local timers before returning Promise.resolve() — no endRun, no releaseAll, no store write. The writer path (readOnly: () => false) is unchanged, guarded by its own test.",
  "files": ["plugins/workflow/src/service.js", "plugins/workflow/test/service.test.js", "docs/subsystems/workflow.md", "changelog.d/2673-readonly-reap.fixed.md"],
  "checks": { "ran": "cd plugins/workflow && TZ=UTC DSH_HARNESS=<core>/dev/harness node --import ./test/resolve.mjs --test \"test/*.test.js\"", "result": "green", "at": "644cde0b4abfdd644fba43a0f992c6f95b61cc03" },
  "next": "none — ready for review"
}

Checkpoint

{
  "pr": 2674,
  "claim": "Dev",
  "issue": 2673,
  "summary": "A read-only host is the second DSH window on the same home, and loading the plugin or closing that window used to end the writer's live runs, release their step Bots and write its older store copy over the newer one. reap() and dispose() now return before touching the record when readOnly() is true, while dispose() still clears only its own timers. The writer path, readOnly: () => false, is unchanged and a test guards it. The new tests hold a writer's run and assert the reader's release stub is never called.",
  "files": [
    "plugins/workflow/src/service.js",
    "plugins/workflow/test/service.test.js",
    "docs/subsystems/workflow.md"
  ],
  "checks": {
    "ran": "cd plugins/workflow && TZ=UTC node --import ./test/resolve.mjs --test \"test/*.test.js\"",
    "result": "green",
    "at": "644cde0b4abfdd644fba43a0f992c6f95b61cc03"
  },
  "next": "none — ready for review"
}

A second window on the same DSH_HOME is a reader, yet loading the workflow
plugin and closing that window both reaped every run still marked running:
endRun('host-ended'), releaseAll of its step Bots, and a whole-store writeback
from the reader's older copy. Both lifecycle paths now return without judging,
releasing or writing while readOnly() is true, and the writing host's own reap
decides.

Fixes #2673
@Birfy Birfy self-assigned this Nov 8, 2026
@Birfy Birfy added the agent-pr Pull request opened by an agent label Nov 8, 2026
@Birfy

Birfy commented Nov 8, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 644cde0
Written-by: Reviewer
Attestation: 644cde0 role=Reviewer

Ran at this tip:

# downloaded the tip tarball (api.github.com/repos/ThinkFlowLab/FlyBot/tarball/644cde0b) into a scratch dir
$ cd plugins/workflow && TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/runs/i2565/harness node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 479  # pass 479  # fail 0
$ TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/runs/i2565/harness node --import ./test/resolve.mjs --test test/service.test.js
# tests 4  # pass 4  # fail 0
# red first: base blob 656fadab plugins/workflow/src/service.js put back, new test kept
$ TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/runs/i2565/harness node --import ./test/resolve.mjs --test test/service.test.js
# tests 4  # pass 1  # fail 3
# restored the tip file and re-ran
$ TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/runs/i2565/harness node --import ./test/resolve.mjs --test test/service.test.js
# tests 4  # pass 4  # fail 0

Review points, at 644cde0b:

  • reap() returns before anything writes: plugins/workflow/src/service.js:2097 if (readOnly()) return Promise.resolve() sits above the typeof release !== 'function' guard and every endRun / releaseAll / write() / archive. No path reaches a write on a read-only host.
  • dispose() clears only local timers (service.js:2162) and returns Promise.resolve() at :2166, before ids/runs/endedLive are computed — so shouldWrite (:2210) is unreachable in the read-only branch: no endRun, no archiveCutoffRun, no releaseAll, no write().
  • The change is +8/-0 in service.js and touches nothing else there: the writer path (readOnly: () => false) is byte-identical and test 4 guards it (ends/archives the run, releases both eph-… ids, which passed before and after).
  • The decisive assertion is there: the read-only instance's release stub must stay empty — assert.deepEqual(readerReleases, [], …) at test/service.test.js:129 (reap) and :138 (dispose). With the base blob those two tests receive the writer's eph-… ids and both store writes land, which is the A read-only host reaps the writer's live workflow: endRun + release its step Bots + whole-store writeback #2673 defect.
  • docs/subsystems/workflow.md:62 now says a read-only host does neither reap nor dispose and the writer's own reap decides — consistent with the code.
  • Scope: the diff is exactly 4 files (service.js, service.test.js, the doc line, one changelog fragment). No change to write()'s temp+rename, home-lock.js, a store lock file, decisions.js or images.js; no existing test assertion was weakened (the test file is new).
  • Check-runs at the tip are all terminal; the only failure is enqueue-guard (no Verifier report names the tip 644cde0b), which this report clears. tests-ok, the four plugin jobs, package, policy-sign, merge-gate and label-hygiene are green.

Not run: full npm run verify and the core suite — this diff is plugins/workflow/** only; the plugin suite above is the affected one.

@github-actions github-actions Bot added the pass label Nov 8, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 8, 2026
Merged via the queue into main with commit a9f0646 Nov 8, 2026
19 of 21 checks passed
@Birfy
Birfy deleted the fix/2673-readonly-reap branch November 8, 2026 06:40
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 8, 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.

A read-only host reaps the writer's live workflow: endRun + release its step Bots + whole-store writeback

1 participant