diff --git a/core/src/host/store.js b/core/src/host/store.js index 69b468a9..266844dd 100644 --- a/core/src/host/store.js +++ b/core/src/host/store.js @@ -377,14 +377,24 @@ export function install(rt) { for (const room of stale) void rt.retitleRoom(room) })() + // The one takeover line, printed from either entry point: refresh() when a reading host + // follows the file, save() when a stale read-only window writes (#1041). It names the host + // that let go, so "this window wrote over someone else's state.json" can be traced from the + // log afterwards. + const announceTakeover = (previous) => { + const where = previous.pid ? `pid ${previous.pid}${previous.host ? ` on ${previous.host}` : ''}` : 'unknown process' + warn('ds-bot: the other dsh has closed; this one now saves the Bot team (was %s)', where) + } + // A reading host follows the file and takes over once the other host has gone. const refresh = async () => { await load() const hadHolder = holder !== undefined if (hadHolder) { + const previous = holder claimLock() if (holder === undefined) { - warn('ds-bot: the other dsh has closed; this one now saves the Bot team') + announceTakeover(previous) warnedReadOnly = false adopt(await readFileState()) return @@ -432,8 +442,12 @@ export function install(rt) { // (#1027). Taking over adopts nothing by itself: whoever called save() may already have // changed `state`, and adopt() replaces it whole (#1032). const takeover = () => { + const previous = holder claimLock() if (holder !== undefined) return false + // The same line refresh() prints (#1041). A stale window that writes over the file the + // other host left has no other trace, so the takeover is announced here too. + if (previous !== undefined) announceTakeover(previous) warnedReadOnly = false return true } diff --git a/core/test/home-lock.test.js b/core/test/home-lock.test.js index ed950da6..413bb0a8 100644 --- a/core/test/home-lock.test.js +++ b/core/test/home-lock.test.js @@ -249,6 +249,33 @@ test('#1032: mutate() adopts what the other host left before the change, so both assert.equal((await call('state', {})).value.readOnly, null) }) +test('#1041: a save() that takes over the lock says so once, naming the host it took it from', async (t) => { + const home = tempHome(t) + writeFileSync(join(home, 'state.json'), JSON.stringify(TEAM)) + writeFileSync(join(home, 'state.lock'), JSON.stringify({ pid: process.ppid, host: hostname(), token: 'other' })) + const call = start(t, home) + // rt.warn reaches ctx.logger.warn at call time (src/host/diagnostics.js), so replacing it + // after start() still catches every line the store prints. + const said = [] + call.ctx.logger.warn = (format, line) => { said.push(format === '%s' ? line : `${format} ${line}`) } + const takeovers = () => said.filter(line => /the other dsh has closed/.test(line)) + + assert.match((await call('state', {})).value.readOnly, /open in another dsh/) + assert.equal(takeovers().length, 0) + + // The other host closes and this window has not read since: a write no request drives + // takes the free lock, and says so once with the host that let go. + rmSync(join(home, 'state.lock')) + await call.rt.save() + assert.equal(takeovers().length, 1) + assert.match(takeovers()[0], new RegExp(`was pid ${process.ppid} on ${hostname()}`)) + assert.equal(JSON.parse(readFileSync(join(home, 'state.json'), 'utf8')).revision, 8) + + // A window that already holds the lock writes without announcing a takeover. + await call.rt.save() + assert.equal(takeovers().length, 1) +}) + test('read-only host (#14): FlyBot queries answer, plugin reads answer, writes and polling stop', async (t) => { const home = tempHome(t) writeFileSync(join(home, 'state.json'), JSON.stringify(TEAM)) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 5e269c92..b6f88d5b 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -113,7 +113,7 @@ A member's own line calls the members it names in two shapes where the call used - The saved team (`state.json`) is versioned. Version 2 (#47) carries the identity schema — `members` records, a per-Bot `ownerId`, the owner's `designatedChiefId`. A legacy version-1 file is migrated on load with a one-time `state.json.v1` backup; an existing backup counts only when it is a readable file holding a parseable version-1 state, and anything else (or any malformed ownership) fails closed (read-only). An older binary refuses a newer version read-only. This is schema foundation only — team mode stays disabled, see [team](team.md). On top of it, member auth (#48) adds host module `src/host/member-auth.js`, installed after `store` in `MODULES` (`core/index.js`): `invitations` and `authSessions` maps live on state (no `STATE_VERSION` bump), the channel resolves the actor from the `x-flybot-member-session` header, and endpoints are `auth-invite` / `auth-redeem` / `auth-revoke` / `auth-whoami` / `auth-remove-member` / `auth-revoke-invite` with `body.actorId` ignored; `auth-invite` alone also opens for an anonymous caller on a host with no sessions yet (bootstrap first invite), `auth-revoke-invite` kills an unredeemed invitation (redeeming it afterwards fails with `revoked`), `removeMember` refuses a member that still owns a Bot (`has_bots`), and the auth mutations are serialized behind an in-process lock so concurrent `removeMember` calls cannot both pass the last-owner check. Sign-in surface only — still not team mode; details in [team](team.md). On top of it, owner-scoped access (#49) adds host module `src/host/authz.js`, installed after member-auth: when any `authSessions` exist, the channel `authorize()` default-denies cross-owner Bot/room/session access, blocks shared non-isolatable resources (secrets, connectors, folder picks, browser bridge, update-install, diagnostics, set-prefs, `add-agent` — the DSH Agent is one per deployment and carries no owner) with `bot/blocked-shared` when more than one member exists, stamps `create-bot` with `actor.memberId`, and filters `view()`/`live()` to the actor; with no sessions the single-human behavior is unchanged. Session targets resolve through the store's owner maps — a Bot id via `botOf`, current chats and group Sessions and earlier parts via `selfOf`/`groupOwners`/`pastOwners` (a shared room Session also resolves for its current participants, #50) — and unresolvable ids deny. `approval-answer` gates on the approval's own Bot or room (resolved by approval id), never on caller-supplied session fields. `flybot/*` plugin endpoints (src/host/hooks.js) reach any signed-in actor for reads; plugin writes follow the shared rule. `view()`/`live()` narrowing (`rt.ownedView`/`rt.ownedLive` in authz.js) keeps per-session entries (`questions`, `exchangeTurns`) the actor owns, and deployment-wide surfaces (agents, secrets, connectors, memory reviews) only while a single member exists — the same rule as the shared writes; rooms the actor belongs to and the Bots in them (#50) stay visible. `create-room` refuses a request with neither a member Bot nor any participant. On top of it, room membership (#50) adds host module `src/host/room-membership.js`, installed after groups: a room may carry typed `participants` (`human` / local `bot` / granted remote `bot` with `handle` + owner fields, roles `admin` / `member`, `joinedSeq` / `leftSeq`); log lines carry `authorId` + `authorKind`; history is the membership window (current members and the owner of an invited Bot see `seq >=` their interval; leave withdraws access; rejoin starts a new interval); local cross-owner rooms accept each side's *granted* Bots (#90: default deny, Chief included); a friend's Bot from `social/remote-bots` (`cleanRemote`) may be added and ungranted Bots stay private, and a room with a friend's Bot holds only granted local Bots (a Bot that joined by room code, `viaCode`, only for that room); `view()` labels remotes with an owner badge and shows an invited owner **Shared room from \**. Face-to-face rooms (#83) reuse the same `participants` / membership-window / owner-badge presentation with `origin: 'face-to-face'`: room-only remotes may sit in the room without a person-grant or a friendship, `presentRoom` treats the viewer as a member (not Shared room from), leave skips the two-Bot floor, and `fanoutRoomLine` goes to `rt.faceRoomFanout` instead of friend `flybotSend`. Still not team mode — see [team](team.md). Per-Bot grants (#90) add host module `src/host/reach.js`, installed after room-membership: `state.grants[botId]` is the list of independent grants (one Bot → one grantee, optionally room-scoped); default deny, Chief included; a Bot whose owner is not a member has none. Grants are set with `rt.setGrants`, exposed as `BotView.granted`, `flybot.bot.hasGrant` / `hasAnyGrant`, `flybot.grants` and the `bot.grants` event. `reach()` is the owner policy: a friend's Bot may reach only local Bots granted to that sender, only a granted Bot (or one code-joined in that room) may message a friend's Bot, another owner's Bot is reached only if both sides have matching grants or both are members of the room the message belongs to. It is not a middleware: hooks.js runs it before the `bot.send` chain, again on approval release, and on every deliver with a sender or remote origin (resumed, retried, deferred, re-sent), so no verdict or `bypass` can skip it and a queued message is judged by what is true when it is delivered. Revoking a grant, or replacing its Chief, removes it from rooms that have friends' Bots or several owners at once (`rt.evictUnshared`). Approval binding (#52) completes the held-send half of this: `state.flybot.approvals` records carry `requestId`, a sha256 `version` over the request content, server-derived `ownerId` and `origin` (`via`/`hop`/audience) — claims in the message (`meta`, text) never confer authority. `flybot/decide` is allowed through authz for any signed-in actor and the per-record check happens in hooks (`decide` requires the sender Bot's owner's session, `bot/forbidden` otherwise, `bot/unauthorized` without a session); `release()` revalidates before dispatch and marks the record `stale` when the request changed, the sender or target Bot is gone (also invalidated eagerly on `deleteBot`), or the Bot changed owners; a demoted Chief's sender-side holds are invalidated eagerly on `setMain` (not inside `staleReason`), and `release()` re-checks immediately before dispatch so an in-flight approve cannot go out after a concurrent invalidation. Unknown `invalidateFor` field values throw. `flybot/approvals` lists only the actor's own holds while enforcement is on; decided stale records keep their attribution in state but appear in no member's list. Which statuses still wait for that decision is one rule in `src/shared/approval-waiting.js` (#754): `decide()`, `heldOf()` in the chat view, the in-flight filter that invalidates holds, `PENDING_MAX` and the `APPROVALS_KEPT` cleanup all ask `isApprovalWaiting()`, and `channel.js` counts a row's waiting replies with it, so a `pending_approval` hold (Bot Social's own spelling) is decidable, drawn as waiting and never dropped mid-wait. -- One writer per home (`src/host/home-lock.js`): the first dsh to load `~/.dsh` takes `state.lock` (pid, host, a random token in the file, its mtime bumped on a timer) and a second window on the same data only reads, answering `bot/read-only` and naming the other host. Takeover is **on the next write**, not on a poll (#1027): a window that was read-only claims the free lock inside `save()` too, and a change made outside a request (a plugin, a scheduled task, a delivery) belongs in `rt.mutate(fn)`, which takes over and adopts what the other host left *before* the change is made and written. The writers that still change `rt.state` inline and then call `save()` are safe on the next write — `save()` claims the free lock — but they do not adopt first, so a write from a window that was read-only can miss what the other host left; moving them over is #1048. `save()` itself never adopts: `adopt()` replaces `rt.state` whole, so adopting there would discard a change the caller has already made (`#1032`). A lock file still naming a live holder on this host (its pid answers) keeps the window read-only, which is the #14 rule. +- One writer per home (`src/host/home-lock.js`): the first dsh to load `~/.dsh` takes `state.lock` (pid, host, a random token in the file, its mtime bumped on a timer) and a second window on the same data only reads, answering `bot/read-only` and naming the other host. Takeover is **on the next write**, not on a poll (#1027): a window that was read-only claims the free lock inside `save()` too, and a change made outside a request (a plugin, a scheduled task, a delivery) belongs in `rt.mutate(fn)`, which takes over and adopts what the other host left *before* the change is made and written. The writers that still change `rt.state` inline and then call `save()` are safe on the next write — `save()` claims the free lock — but they do not adopt first, so a write from a window that was read-only can miss what the other host left; moving them over is #1048. `save()` itself never adopts: `adopt()` replaces `rt.state` whole, so adopting there would discard a change the caller has already made (`#1032`). Either takeover prints one line — `the other dsh has closed; this one now saves the Bot team (was pid on )` — from `refresh()` and from `save()` alike, so a window that writes over the file another host left leaves a trace (#1041). A lock file still naming a live holder on this host (its pid answers) keeps the window read-only, which is the #14 rule. - `list_bots` (`src/host/tools.js`) tags a Bot with `N to approve` while one of its outbound sends is held for the owner's decision (#587): a count of that Bot's pending held sends (`state.flybot.approvals`), never the held message's text, so a coordinator reading the roster sees who is blocked and the tag clears once the owner decides. Covered by the roster case in `core/test/flybot.test.js`. - The Main Bot list is deployment-wide (#613): `set-main` requires the actor to own `body.id` and, when `body.replace` names the Main Bot whose place the target takes, that Bot too, so a member cannot swap another owner's Main Bot for their own or demote it (both are `bot/forbidden`); `roster.setMain` keeps the same invariant for every caller, refusing a `replace` across owners and leaving `state.mainBotIds` unchanged. A swap inside one owner's team, the plain promotion of a Bot the caller owns, and the deployment-wide list itself are unchanged. diff --git a/patches/ds-bot/0001-core-fixes.patch b/patches/ds-bot/0001-core-fixes.patch index 8602bd4b..7326f482 100644 --- a/patches/ds-bot/0001-core-fixes.patch +++ b/patches/ds-bot/0001-core-fixes.patch @@ -6730,7 +6730,7 @@ diff --git a/test/home-lock.test.js b/test/home-lock.test.js return call } -@@ -185,3 +185,83 @@ test('a reading host follows the other host\'s keys, never writes them, and keep +@@ -185,3 +185,110 @@ test('a reading host follows the other host\'s keys, never writes them, and keep const saved = JSON.parse(readFileSync(secretsPath, 'utf8')).secrets.map(secret => secret.name).sort() assert.deepEqual(saved, ['FIRST_KEY', 'MINE', 'SECOND_KEY', 'THIRD_KEY']) }) @@ -6794,6 +6794,33 @@ diff --git a/test/home-lock.test.js b/test/home-lock.test.js + assert.equal((await call('state', {})).value.readOnly, null) +}) + ++test('#1041: a save() that takes over the lock says so once, naming the host it took it from', async (t) => { ++ const home = tempHome(t) ++ writeFileSync(join(home, 'state.json'), JSON.stringify(TEAM)) ++ writeFileSync(join(home, 'state.lock'), JSON.stringify({ pid: process.ppid, host: hostname(), token: 'other' })) ++ const call = start(t, home) ++ // rt.warn reaches ctx.logger.warn at call time (src/host/diagnostics.js), so replacing it ++ // after start() still catches every line the store prints. ++ const said = [] ++ call.ctx.logger.warn = (format, line) => { said.push(format === '%s' ? line : `${format} ${line}`) } ++ const takeovers = () => said.filter(line => /the other dsh has closed/.test(line)) ++ ++ assert.match((await call('state', {})).value.readOnly, /open in another dsh/) ++ assert.equal(takeovers().length, 0) ++ ++ // The other host closes and this window has not read since: a write no request drives ++ // takes the free lock, and says so once with the host that let go. ++ rmSync(join(home, 'state.lock')) ++ await call.rt.save() ++ assert.equal(takeovers().length, 1) ++ assert.match(takeovers()[0], new RegExp(`was pid ${process.ppid} on ${hostname()}`)) ++ assert.equal(JSON.parse(readFileSync(join(home, 'state.json'), 'utf8')).revision, 8) ++ ++ // A window that already holds the lock writes without announcing a takeover. ++ await call.rt.save() ++ assert.equal(takeovers().length, 1) ++}) ++ +test('read-only host (#14): FlyBot queries answer, plugin reads answer, writes and polling stop', async (t) => { + const home = tempHome(t) + writeFileSync(join(home, 'state.json'), JSON.stringify(TEAM)) diff --git a/patches/ds-bot/0002-flybot-api-host.patch b/patches/ds-bot/0002-flybot-api-host.patch index 7f41243c..82762b0f 100644 --- a/patches/ds-bot/0002-flybot-api-host.patch +++ b/patches/ds-bot/0002-flybot-api-host.patch @@ -669,10 +669,9 @@ diff --git a/src/host/store.js b/src/host/store.js + warn('ds-bot: unreadable state %s: %s', statePath, backupProblem) + return undefined + } - stuck = undefined - return parsed - } -- stuck = 'saved by a newer DS Bot; update DS Bot' ++ stuck = undefined ++ return parsed ++ } + if (parsed?.version === STATE_VERSION) { + const problem = ownershipProblem(parsed) + if (problem !== undefined) { @@ -680,9 +679,10 @@ diff --git a/src/host/store.js b/src/host/store.js + warn('ds-bot: state %s was refused: %s', statePath, problem) + return undefined + } -+ stuck = undefined -+ return parsed -+ } + stuck = undefined + return parsed + } +- stuck = 'saved by a newer DS Bot; update DS Bot' + stuck = 'saved by a newer FlyBot; update FlyBot' return undefined } @@ -797,7 +797,33 @@ diff --git a/src/host/store.js b/src/host/store.js const load = () => loaded ??= (async () => { claimLock() if (holder !== undefined) warn('ds-bot: %s', readOnly()) -@@ -235,10 +235,31 @@ +@@ -185,14 +185,24 @@ + for (const room of stale) void rt.retitleRoom(room) + })() + ++ // The one takeover line, printed from either entry point: refresh() when a reading host ++ // follows the file, save() when a stale read-only window writes (#1041). It names the host ++ // that let go, so "this window wrote over someone else's state.json" can be traced from the ++ // log afterwards. ++ const announceTakeover = (previous) => { ++ const where = previous.pid ? `pid ${previous.pid}${previous.host ? ` on ${previous.host}` : ''}` : 'unknown process' ++ warn('ds-bot: the other dsh has closed; this one now saves the Bot team (was %s)', where) ++ } ++ + // A reading host follows the file and takes over once the other host has gone. + const refresh = async () => { + await load() + const hadHolder = holder !== undefined + if (hadHolder) { ++ const previous = holder + claimLock() + if (holder === undefined) { +- warn('ds-bot: the other dsh has closed; this one now saves the Bot team') ++ announceTakeover(previous) + warnedReadOnly = false + adopt(await readFileState()) + return +@@ -235,10 +235,35 @@ return warnReadOnly() } @@ -806,8 +832,12 @@ diff --git a/src/host/store.js b/src/host/store.js + // (#1027). Taking over adopts nothing by itself: whoever called save() may already have + // changed `state`, and adopt() replaces it whole (#1032). + const takeover = () => { ++ const previous = holder + claimLock() + if (holder !== undefined) return false ++ // The same line refresh() prints (#1041). A stale window that writes over the file the ++ // other host left has no other trace, so the takeover is announced here too. ++ if (previous !== undefined) announceTakeover(previous) + warnedReadOnly = false + return true + }