Skip to content

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

Closed
HugoXK wants to merge 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev2/fix/1048-mutate-writers
Closed

HugoXK wants to merge 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev2/fix/1048-mutate-writers

Conversation

@HugoXK

@HugoXK HugoXK commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

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

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

Fixes #1048

Base: c062fb0 (current main)
Tip SHA: 7855a81

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

Re-run at the tip on the clean tree of 7855a81d (tarball from my fork; node_modules symlinked to the shared checkout):

  • TZ=UTC node --test --test-concurrency=1 core/test/mutate-writers.test.js → # tests 2 / # pass 2 / # fail 0.
  • The same file with both sources reverted to c062fb02 and the tests kept → # tests 2 / # pass 0 / # fail 2; the delivery case fails with expected: 'light' (the other host's prefs.theme was overwritten), the agents case fails on the read-only refusal.
  • 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: 9 fragment(s) in changelog.d/ are well formed.

Earlier, on the full tree: TZ=UTC npm run verify → exit 0: core 1413/1413, social 453/453, tree 125/125, timeline 125/125, .github/scripts 417/417, interop 37/37, live-e2e 16/16. npm --prefix core run lint → 0 errors; arch-docs: OK.

Not run: the smoke test (dev/bench.sh smoke), which the guide asks for when src/host changes. It needs a bench with the DSH checkout and a free port; the two-host tests and the full suite cover this path instead.

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.

…b#1048)

`delivery.js` and `agents.js` still changed `state` and then called `save()`, so a
window that was read-only because the other dsh owned the team either dropped the
change (a friend's first line, a team exchange) or refused with the read-only error
(each DSH Agent write) once that host had closed. The change now goes through
`rt.mutate(fn)`: it takes the free lock, 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.

`beforeWrite()` refreshes before the Agent side effects (creating or archiving a
Session), so a window that is still read-only refuses without doing any.
`noteFriendLine` answers "first contact" as a promise now; its one caller already
awaited in the same function.

Tests: `core/test/mutate-writers.test.js`, one two-host case per file — red on the
old writers (the other host's `prefs.theme` lost, and the Agent write refused),
green after.

Docs: `docs/subsystems/core-host.md` records the remaining inline writers as
migrated instead of naming this issue as the migration.
@HugoXK

HugoXK commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Written-by: Dev-2

PASS request at 7855a81d9567bd86c175917c09ed06b429984d9a. The head is frozen: no push, rebase or amend from here; if a review asks for a change, it goes on this branch and the new tip comes back through the reviewer.

What to judge, in one pass:

  • delivery.js: noteFriendLine (a friend's first line, the unread mark) and the team-exchange record now apply inside rt.mutate(fn), with the Bot record re-resolved inside the callback; noteFriendLine answers first-contact as a promise and its one caller already awaits.
  • agents.js: add, adopt, archive and remove apply inside rt.mutate(fn) with the Agent re-resolved; beforeWrite() refreshes first so a released lock is taken over, and still refuses while another live dsh holds it — before any Session is created or archived.
  • Red then green: core/test/mutate-writers.test.js, one two-host case per file (home-lock.test.js:233 shape, Take the state lock on the next write, not on a poll (#1027) #1032). With the two sources reverted the tests are 2 fail (other host's prefs.theme lost; the Agent write refused with the read-only error); with the fix 2/2.
  • TZ=UTC npm run verify at 7855a81d9567bd86c175917c09ed06b429984d9a has not run in CI yet; locally it was exit 0 on this tree (core 1413/1413, social 453/453, tree 125/125, timeline 125/125, .github/scripts 417/417, interop 37/37, live-e2e 16/16), ds-bot-patches check ok, changelog.mjs check ok.
  • Not run locally: the smoke test; stated in the body.

The PR body carries the trade-offs and the two honest residuals (promise-returning noteFriendLine; a still-open other dsh still refuses the Agent writes).

@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 25, 2026
@Birfy

Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

Written-by: Dev-2

Body fixed per the routing rule: Owner: Dev2 / Reviewer: Reviewer are now in the first 20 lines, above Written-by:, with Fixes #1048 on its own line and Tip SHA: 7855a81d9567bd86c175917c09ed06b429984d9a. Written-by: is kept for provenance and is not standing in for the two routing lines.

Base re-read just now: c062fb02c3317990e77c1abb273e1350546136db, which is the current upstream main head — the branch is not behind, changed_files: 8, commits: 1.

node scripts/ds-bot-patches.mjs check at the tip, 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/

I also re-ran the evidence on a clean tree of 7855a81d rather than quoting the earlier numbers: the new file alone is # tests 2 / # pass 2 / # fail 0; with both sources reverted to c062fb02 and the tests kept it is # tests 2 / # pass 0 / # fail 2, the delivery case failing with expected: 'light' (the other host's prefs.theme overwritten) and the agents case on the read-only refusal.

@Reviewer please judge 7855a81d. The tip is frozen; I will not push another commit until there is a verdict on it.

@Birfy

Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

Written-by: Dev-2

This PR cannot be rebuilt, and that is not a slip of the rebase: its head is HugoXK/FlyBot, and this account has push: false there (/repos/HugoXK/FlyBot → {"push": false, "pull": true}; the Git Data API answers 404 on POST /git/blobs). Meanwhile main moved past it three times — store.js now carries #1041's announceTakeover, and patches/ds-bot/0001-core-fixes.patch, 0005-social-seams.patch and core-host.md have each been rewritten since c062fb02.

The same change, rebuilt on current main and re-run there, is #1092 — branch dev2/fix/1048-mutate-writers on ThinkFlowLab/FlyBot, tip 2a61d8e8 (parent 300cbbe8), 8 files +428/−66. Please close this one as superseded. If you would rather keep this branch, push 2a61d8e8 to it and close #1092 instead — I cannot write to the fork either way.

What moved between the bodies: the base, the tip, and the two patch rebuilds. The change itself, the two two-host cases and their red proof are the same, and the evidence section in #1092 is the one re-run on the current tree.

@Birfy

Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

Written-by: Dev-2

Closing as superseded: the same change landed as #1092 (merge 81ac2964), and this branch is on a fork this account cannot write to (HugoXK/FlyBot: push: false).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move the remaining inline state writers onto rt.mutate()

2 participants