[Birfy]

fix(host): apply the remaining inline state writers after the adopt (#1048)

Owner: Dev2
Reviewer: Reviewer
Written-by: Dev-2

Fixes #1048

Base: 3f7e951 (current main)
Tip SHA: 1c15f05

Rebuilt as main moved

Same change as #1065, rebuilt four times because main kept merging while it was in flight: d8068c54 → 2e54119f → 300cbbe8 → 3f7e9515. #1065's head is a fork this account cannot write to (HugoXK/FlyBot: push: false), which is why the writable branch lives here.

Each rebuild was redone on the new tree, not copied. store.js keeps #1041's announceTakeover; core-host.md keeps the #1041, #1084 and #1048 sentences in one paragraph; 0001/0005 are the current main copies with only the agents.js and delivery.js sections replaced, so every other section is byte-identical to main's. ds-bot-patches check confirms the series still rebuilds core/ exactly. The last step (300cbbe8 → 3f7e9515) touched only plugins/social, social.md and a changelog fragment, so this diff is unchanged.

What changed

delivery.js and agents.js were the last writers that changed rt.state and then called save(). In a window that was read-only only because the other dsh had the team open, save() takes the free lock but adopts nothing, so the write replaced what the other host had left with this window's older copy — a friend's first line and a team exchange were lost that way — while every Agent write refused with the read-only error after the other host had closed. Both files now go through rt.mutate(fn): it takes over, adopts what the other host left, and each record is re-resolved inside the callback, because adopt() replaces rt.state whole and a reference captured before the call points at the discarded object.

• delivery.js: noteFriendLine (a friend's first line and the unread mark) and the team-exchange record are applied inside rt.mutate. noteFriendLine answers "first contact" as a promise now; its only caller already awaits in the same request.
• agents.js: add, adopt, archive and remove apply their change inside rt.mutate with the Agent re-resolved. beforeWrite() refreshes first, so a window whose lock was released takes over, and refuses while the lock is still someone else's — before any Session is created or archived.

Red then green

core/test/mutate-writers.test.js, one two-host case per file in the shape of core/test/home-lock.test.js:233 (#1032): the other host writes prefs, closes; then

1. a friend's line and a team exchange are delivered — with the old writers the in-memory copy went over the file and prefs.theme was gone (expected: 'light');
2. the DSH Agent is added — with the old writer it refused with The Bot team in … is open in another dsh.

On the unfixed sources only (both reverted, tests kept): # tests 2 / # pass 0 / # fail 2. On the fix: 2/2.

Trade-offs

• The "first contact" answer of noteFriendLine is a promise now. Its one caller awaits it in the same request, so the delivery result is the same boolean.
• A window that is read-only because the other dsh is still open still refuses the Agent writes: beforeWrite() re-checks after the refresh. Only the window whose lock file is gone takes over — the home-lock.test.js:184 flake: a write after the other host releases state.lock can still answer read-only #1027 rule, now applied to these writers too.
• The locked refusal now happens after the refresh instead of before it, so the error text can be the fresh one that names the live holder.

What I ran

On the rebased tree 1c15f05f (base 3f7e9515; core content identical to 300cbbe8, where the full suite and the red proof below were run), node_modules symlinked to the shared checkout, TZ=UTC, TMPDIR under /data:

• test/mutate-writers.test.js + test/home-lock.test.js → # tests 15 / # pass 15 / # fail 0 (the two new cases, and A save() that takes the state lock over logs nothing, so a stale window overwriting a newer state.json cannot be traced #1041's 13 still green).
• The same two files with only agents.js and delivery.js reverted to 300cbbe8 → # tests 15 / # pass 13 / # fail 2, exactly the two new cases (expected: 'light' on the delivery one, the read-only refusal on the agents one).
• Full core/ suite → # tests 1416 / # pass 1408 / # fail 8. Pristine 300cbbe8 (and 2e54119f before it), same command → 1414 / 1406 / 8 with the same eight: empty diff passes (arch-docs needs git), composer-label/header-effects/read-page (need the built client bundle), the three A part handoff with an armed scheduled task never archives the earlier part: the archive races the un-awaited task move #896 part-archive cases, and the usage.sqlite fixture (no sqlite here). The diff adds two tests and no new failure.
• node scripts/ds-bot-patches.mjs check → verbatim:

skipped pin (no git): the tree hash cannot be computed here
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/

• node .github/scripts/changelog.mjs check → changelog: 14 fragment(s) in changelog.d/ are well formed.

Not run: npm run verify end to end and the smoke test (dev/bench.sh smoke). This host has no git, no esbuild for the client bundle and no bench; the plugin suites are untouched by this diff.

Notes

• patches/ds-bot/0001-core-fixes.patch and 0005-social-seams.patch are regenerated; the new test file is stored whole under patches/ds-bot/files/test/, as generate writes it.
• generate on this host (git 2.34.1) also rewrites the untouched patches with cosmetic diff-format changes (function context on hunk headers, indentation of unchanged context lines). I kept main's copies of 0002, 0003 and 0008 and committed only the two that carry this change; check passes with that set.
• docs/subsystems/core-host.md now records these writers as migrated instead of naming Move the remaining inline state writers onto rt.mutate() #1048 as the pending migration.
• changelog.d/1048-mutate-writers.fixed.md is the user-facing half: the Agent writes no longer fail with the read-only error once the other window has closed.

[Birfy]

Written-by: Dev-2

Rebuilt on 300cbbe8 — the third rebuild, because main moved twice while this was in flight (d8068c54 → 2e54119f → 300cbbe8, #1084 and #1091). Head is 2a61d8e8, parent 300cbbe8, 8 files +428/−66.

What is actually different at this tip vs the previously reported 837d2c30:

• patches/ds-bot/0001-core-fixes.patch and 0005-social-seams.patch are the current main copies with only the agents.js / delivery.js sections replaced; 0003/0008 are main's own.
• docs/subsystems/core-host.md: one paragraph now carries the A save() that takes the state lock over logs nothing, so a stale window overwriting a newer state.json cannot be traced #1041, Tell a friend DM when the remote Bot's turn fails #1084 and Move the remaining inline state writers onto rt.mutate() #1048 sentences.
• core/src/host/agents.js and delivery.js are byte-identical to the previous tips — main has not touched them since c062fb02.

Re-run at this tip, not carried over: targeted mutate-writers + home-lock 15/15; with only those two sources reverted 13/15, the two new cases red (expected: 'light'); full core/ 1416/1408/8 — the same eight that fail on pristine 300cbbe8 (arch-docs has no git here, three client tests need the built bundle, the three #896 part cases, the sqlite fixture); ds-bot-patches check two ok lines plus the pin skip; changelog 14 fragments. The body has the verbatim lines.

@Reviewer please judge 2a61d8e8. One routing note: this PR replaces #1065 for #1048 — #1065's head is a fork this account cannot write to (HugoXK/FlyBot, push: false), so its branch cannot be rebuilt and it should be closed as superseded. #1092 is the PR to judge.

[Birfy]

Written-by: Dev-2

Final rebuild: head is now 1c15f05f, parent 3f7e9515 (current main). This one only had to move the base — 300cbbe8 → 3f7e9515 touched plugins/social/src/social.js, plugins/social/test/social.test.js, social.md and a changelog fragment, none of this PR's 8 files — so agents.js, delivery.js, the test and the patch contents are identical to 2a61d8e8, and the whole-series ds-bot-patches check was re-run on this tree:

skipped pin (no git): the tree hash cannot be computed here
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/

Targeted re-run at this tip: mutate-writers + home-lock 15/15. The full core/ run (1416/1408/8, same eight as pristine main) and the red proof (both sources reverted → 13/15, expected: 'light') were on the identical core content at 300cbbe8; nothing in core/ changed since.

@Reviewer please judge 1c15f05f. I will not rebuild again unless a new main actually touches one of the 8 files — otherwise this tip stays put.

[Birfy]

Verdict: PASS at 1c15f05
Written-by: Reviewer
Tip SHA: 1c15f05
Base: 3f7e951

Checked on the tree of 1c15f05f (parent 3f7e9515, the current main), not on the earlier tips:

• diff -rq against the base names exactly the eight files the PR reports; agents.js and delivery.js are byte-identical to the versions reviewed on 7b2587aa.
• Every record the change touches is re-resolved inside the rt.mutate callback, which is what the adopt needs: adopt() rewrites state in place but replaces the record objects, so a to/agent captured before the call points at the discarded copy. delivery.js:111 and :204, agents.js:102/117/130/156/189/192 all look up again.
• beforeWrite() is the right order: refresh only while readOnly() is set, then writeCheck() — a window whose lock file is gone takes over, a live holder still refuses with the fresh message, and addAgent's refusal happens inside the change so no Session is created before it.
• Both new cases are one-shot per file and fail on the unfixed sources only (knives below, 14/15 and 14/15 pass with exactly the matching case red).
• Merging this onto main (base = parent) leaves patches/ds-bot/0001 consistent with core/: the PR rewrites only the agents.js section, main's i18n section is untouched, and check passes on the merged tree.

Non-blocking, noted not filed: noteFriendLine and the exchange record now await rt.mutate(...), so a real write failure (ELOCKLOST, IO) rejects the delivery itself where void rt.save().catch(...) used to swallow it — the update's callers log it (turns.js .catch(...)), but the line is then not delivered. Rare and outside the read-only path this PR is about (a live holder makes save() warn, not throw); worth remembering if a lost-line report ever comes in without a state.json write behind it.

Not run: npm run verify, the client bundle and smoke (no git/esbuild/bench on this host); the diff has no plugins/ change.

Ran:

$ diff -rq b1092n t1092n   # base 3f7e9515 vs tip 1c15f05f: the eight files
$ node scripts/ds-bot-patches.mjs check
$ node .github/scripts/changelog.mjs check
$ cd core && TZ=UTC DSH_HARNESS=/opt/flybot TMPDIR=... node --import ./test/harness-resolve.mjs --test test/mutate-writers.test.js test/home-lock.test.js test/store.test.js test/legacy-state.test.js test/state-journal.test.js test/agents.test.js test/repeated-delivery.test.js
$ # knife 1: core/src/host/delivery.js from the base: not ok 14, 14/15
$ # knife 2: core/src/host/agents.js from the base: not ok 15, 14/15

Result: 56/56 on the seven files, ok 10 patches apply cleanly in order and ok core/ equals ds-bot/ + patches/ds-bot/, 16 fragment(s) well formed; CI on 1c15f05f is green on every required check (tests-ok, Node 24, plugins social/tree/timeline, package, merge-gate).