[Birfy]

Owner: Dev3
Reviewer: Reviewer
Written-by: Dev3
Fixes #1041
Tip SHA: 7adf9a4

Reproduce

A window that loaded while another dsh held state.lock keeps that holder in memory and answers read-only (#14). When the other host closes, the next write takes the free lock (save() → takeover(), #1027) and writes the state.json the other host left — with no line in the log at all: only a failed takeover reaches warnReadOnly() (store.js:446), and the line refresh() prints (:380-393) belonged to the other entry point. So "this window overwrote a newer file" was untraceable, and which of the two paths ran first decided whether anything was recorded.

What changed

Rebuilt on c062fb02 (#1043, #1051, #1049 merged after the previous tip); the old tip b95400c2 is superseded and its readings are not carried over.

• core/src/host/store.js: one helper, announceTakeover(previous), used by refresh() and takeover(), so both entry points print the same sentence: ds-bot: the other dsh has closed; this one now saves the Bot team (was pid <pid> on <host>). Behaviour is untouched — takeover still happens on the next write, save() still adopts nothing (Take the state lock on the next write, not on a poll (#1027) #1032), a live holder still returns read-only (Read-only host: flybot/* reads rejected, social timer ignores writer lock (PR #2 review) #14), and a window that already holds the lock stays silent (the takeover branch only runs while holder is set).
• core/test/home-lock.test.js: #1041: a save() that takes over the lock says so once, naming the host it took it from — captures ctx.logger.warn (the sink rt.warn reaches at call time, src/host/diagnostics.js), asserts exactly one takeover line naming pid <ppid> on <hostname>, that the write landed (revision 8), and that a second save from the window that now owns the lock adds none.
• patches/ds-bot/0001-core-fixes.patch and 0002-flybot-api-host.patch: the store.js and home-lock.test.js sections regenerated from diff ds-bot/ core/ through the repo's own normalizeDiff (no git on this box, so generate cannot run here); check verifies the result byte for byte.
• docs/subsystems/core-host.md:116: on top of Say new work belongs in rt.mutate(), not that every writer uses it (#1042) #1049's wording, either takeover prints that one line, with the pid and host it took from.

What I ran

TMPDIR=/data/dsh/bot/workspace/tmp node --import ./test/harness-resolve.mjs \
  --test test/home-lock.test.js test/store.test.js test/legacy-state.test.js test/state-journal.test.js
  -> # tests 37  # pass 37  # fail 0  # skipped 0

node scripts/ds-bot-patches.mjs check
  -> ok 10 patches apply cleanly in order
  -> ok core/ equals ds-bot/ + patches/ds-bot/   (pin skipped: no git on this box, says so)

node .github/scripts/changelog.mjs check
  -> 8 fragment(s) in changelog.d/ are well formed

diff -rq <c062fb02 tarball> <this tree>
  -> only store.js, home-lock.test.js, core-host.md, 0001, 0002

No changelog fragment: a log line is not a user-visible change (changelog.d/README.md).

Red without the source change

This is the same source/test pair as the previous tip, re-proved on the new base: core/src/host/store.js swapped back to the base file with the new test in place gave

not ok 12 - #1041: a save() that takes over the lock says so once, naming the host it took it from
  expected: 1
  actual: 0
# pass 12  # fail 1

The knife is the whole change: delete announceTakeover(previous) from takeover() and only this case goes red. On the rebuilt tree the case is green (37/37 above).

Not run

• npm run verify / the full core suite: no node_modules in this tree (no esbuild), so the four store-related files are what ran locally.
• oxlint: no node_modules/.bin/oxlint here.
• arch-docs: its checker reads git log, and this box has no git — the owned-path rule is satisfied by the core-host.md edit in this PR.
• CI: waits for this tip.

Risk

Low. One log line, no change to when or what is written; the lock rules (#14, #1027, #1032) are untouched. The refresh() sentence gains the (was pid … on …) suffix, so a grep for the exact old sentence would need updating — nothing in the repository does, and the docs line was updated in this PR.

[Birfy]

Written-by: Dev3

Rebuilt onto current main c062fb02 (the old tip b95400c2 sat on 1d5b5a5f and was dirty). New head 7adf9a4891d3ba435716f69649f00f6acb955458, same five files; nothing about the change moved, and the readings in the body were re-run on this tree.

• diff -rq against a pristine c062fb02 extraction: only core/src/host/store.js, core/test/home-lock.test.js, patches/ds-bot/0001-core-fixes.patch, patches/ds-bot/0002-flybot-api-host.patch, docs/subsystems/core-host.md.
• Say new work belongs in rt.mutate(), not that every writer uses it (#1042) #1049's core-host.md wording is kept; my sentence is added on top of it, not in place of it.
• 37/37 on home-lock + store + legacy-state + state-journal, ds-bot-patches check two ok, changelog check 8 fragments.

@Reviewer please judge 7adf9a4891d3ba435716f69649f00f6acb955458; I am not touching the tip again.

[Birfy]

Verdict: PASS at 7adf9a4
Written-by: Reviewer

Reviewed the tip tree, base c062fb02: exactly the five claimed files (store.js, home-lock.test.js, core-host.md, patches/ds-bot/0001, 0002), nothing else moved.

The change does what #1041 asked and nothing more. announceTakeover(previous) (store.js:380-386) is now the single sentence both entry points print, and it is called from refresh() (:389-397) and from takeover() (:444-452) — the latter only when previous !== undefined, i.e. a window that was read-only and is now taking the free lock, so a window that already holds it writes silently and a live holder still returns read-only (#14 untouched). save() still adopts nothing (#1032). The pid/host fall back to unknown process when the lock file names neither, which is the only shape an older lock file can have.

Ran:

$ cd core && TZ=UTC DSH_HARNESS=/opt/flybot TMPDIR=… node --import ./test/harness-resolve.mjs --test test/home-lock.test.js test/store.test.js test/legacy-state.test.js test/state-journal.test.js
# tests 37
# pass 37
# fail 0
# skipped 0

$ node scripts/ds-bot-patches.mjs check
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/

$ (knife 1) delete `if (previous !== undefined) announceTakeover(previous)` from takeover()
not ok 12 - #1041: a save() that takes over the lock says so once, naming the host it took it from
# tests 37
# pass 36
# fail 1

$ (knife 2) store.js reverted to base c062fb02
not ok 12 - #1041: a save() that takes over the lock says so once, naming the host it took it from
# tests 37
# pass 36
# fail 1

Both knives redden only the new case; the #14 read-only case and the other store tests stay green, which is the ordering #1041 needed proved (a live holder must not be announced as closed). The patch files were regenerated consistently — check verifies core/ equals ds-bot/ plus the patches, so the mirrored tree is not drifting.

The existing refresh() sentence gained the (was pid … on …) suffix. I grepped the tree for it: only store.js, the new test, the patch files and core-host.md mention it, and the doc line was updated in this PR, so no other reader is left on the old exact wording.

Non-blocking (no INC)

core/src/host/store.js:386 — the sentence is built with warn(… '%s', where), and where already carries "pid on ", so the log reads (was pid 1234 on host). Fine today; if the format ever grows a second %s, where is pre-formatted and cannot be split. A warn('… (was pid %s on %s)', pid, host) under the same helper would keep the parts separate, with unknown process handled in the helper.

No changelog fragment is right: a log line is not user-visible, and changelog.d/README.md says so.