From 1c15f05f5a898cbbe93fac73f1203dcec9fcca30 Mon Sep 17 00:00:00 2001 From: Danyang Chen Date: Sun, 25 Oct 2026 10:16:41 +0800 Subject: [PATCH] fix(host): apply the remaining inline state writers after the adopt (#1048) Rebuilt on 3f7e9515: `store.js` keeps #1041's `announceTakeover`, `core-host.md` carries the #1041, #1084 and #1048 lines, and the patch series is regenerated against the current `store.js`/`turns.js`/`hooks.js`. --- changelog.d/1048-mutate-writers.fixed.md | 1 + core/src/host/agents.js | 76 +++++----- core/src/host/delivery.js | 32 ++-- core/test/mutate-writers.test.js | 104 +++++++++++++ docs/subsystems/core-host.md | 2 +- patches/ds-bot/0001-core-fixes.patch | 139 +++++++++++++++++- patches/ds-bot/0005-social-seams.patch | 36 +++-- .../ds-bot/files/test/mutate-writers.test.js | 104 +++++++++++++ 8 files changed, 428 insertions(+), 66 deletions(-) create mode 100644 changelog.d/1048-mutate-writers.fixed.md create mode 100644 core/test/mutate-writers.test.js create mode 100644 patches/ds-bot/files/test/mutate-writers.test.js diff --git a/changelog.d/1048-mutate-writers.fixed.md b/changelog.d/1048-mutate-writers.fixed.md new file mode 100644 index 000000000..9017f6ebc --- /dev/null +++ b/changelog.d/1048-mutate-writers.fixed.md @@ -0,0 +1 @@ +A window that was read-only only because the other dsh had the team open now takes the free lock for the writes that were still inline, instead of dropping them or refusing with the read-only error: a friend's first line and the unread mark, a team exchange, and every DSH Agent change (add, adopt, archive, remove). The change the other host left survives, because the write adopts and re-resolves its records before it applies ([#1048](https://github.com/ThinkFlowLab/FlyBot/issues/1048)). diff --git a/core/src/host/agents.js b/core/src/host/agents.js index cd6b1fe72..45d629801 100644 --- a/core/src/host/agents.js +++ b/core/src/host/agents.js @@ -77,6 +77,13 @@ export function install(rt) { const readOnly = rt.readOnly() if (readOnly !== undefined) throw new Error(readOnly) } + // A change made outside a request may start in a window that was read-only only because + // the other dsh had the team open: refresh first — it adopts what that host left and + // takes the free lock (#1027, #1032) — then refuse if the lock is still someone else's. + const beforeWrite = async () => { + if (rt.readOnly() !== undefined) await rt.refresh() + writeCheck() + } const record = (agentId) => { const agent = state.agents[agentId] if (agent === undefined) throw new Error('Unknown DSH Agent') @@ -84,16 +91,15 @@ export function install(rt) { } async function createAgentSession(agentId, workspaceId) { - writeCheck() - const agent = record(agentId) + record(agentId) + await beforeWrite() // A known workspace id wins (the Agent's latest Session may live outside the // team workspace after a workspace switch); anything else gets the team's own. const target = typeof workspaceId === 'string' && ctx.workspaceRegistry.get(workspaceId) ? workspaceId : await rt.ensureWorkspace() const { sessionId } = await ctx.sessionController.create({ workspaceId: target }) - agent.sessions.push(sessionId) - await rt.save() + await rt.mutate(() => { record(agentId).sessions.push(sessionId) }) // agent/created fired before the session joined the index, so restrict again now. await restrictTeamTools(await rt.resolveAgent(sessionId)) return sessionId @@ -103,48 +109,54 @@ export function install(rt) { // picker or a fork) is adopted into the same Agent: it keeps dsh's surface and // the team tools stay hidden. async function adoptAgentSession(agentId, sessionId) { - writeCheck() - const agent = record(agentId) - if (agent.sessions.includes(sessionId)) return sessionId + record(agentId) + await beforeWrite() + if (record(agentId).sessions.includes(sessionId)) return sessionId await rt.resolveAgent(sessionId) - const owned = rt.selfOf(sessionId) !== undefined || rt.botOf(sessionId) !== undefined - || rt.roomOf(sessionId) !== undefined || agentOf(sessionId) !== undefined - if (owned) throw new Error('Session already belongs to the team') - agent.sessions.push(sessionId) - await rt.save() + // The ownership check reads the records, so it runs inside the change, after the adopt. + await rt.mutate(() => { + const owned = rt.selfOf(sessionId) !== undefined || rt.botOf(sessionId) !== undefined + || rt.roomOf(sessionId) !== undefined || agentOf(sessionId) !== undefined + if (owned) throw new Error('Session already belongs to the team') + const agent = record(agentId) + if (!agent.sessions.includes(sessionId)) agent.sessions.push(sessionId) + }) await restrictTeamTools(await rt.resolveAgent(sessionId)) return sessionId } async function addAgent() { - writeCheck() - if (Object.keys(state.agents).length > 0) throw new Error('The team already has a DSH Agent') - const agent = { id: randomUUID(), name: AGENT_NAME, sessions: [], createdAt: Date.now() } - state.agents[agent.id] = agent + const id = randomUUID() + await rt.mutate(() => { + writeCheck() + if (Object.keys(state.agents).length > 0) throw new Error('The team already has a DSH Agent') + state.agents[id] = { id, name: AGENT_NAME, sessions: [], createdAt: Date.now() } + }) try { - await createAgentSession(agent.id) + await createAgentSession(id) } catch (error) { // The first Session may already be created and saved: archive it and // persist the rollback, or the Agent returns on restart with its team // tools never denied. - const createdSessions = [...agent.sessions] - delete state.agents[agent.id] + const createdSessions = [...record(id).sessions] + await rt.mutate(() => { delete state.agents[id] }).catch(() => { /* rollback is best effort; the original error is what the caller sees */ }) for (const sessionId of createdSessions) await ctx.workspaceRegistry.archiveSession(sessionId).catch(() => { /* rollback is best effort; the original error is what the caller sees */ }) - await rt.save().catch(() => { /* rollback is best effort; the original error is what the caller sees */ }) throw error } - return agent + return record(id) } async function archiveAgentSession(agentId, sessionId) { - writeCheck() - const agent = record(agentId) - if (!agent.sessions.includes(sessionId)) throw new Error('Session does not belong to the DSH Agent') + record(agentId) + await beforeWrite() + if (!record(agentId).sessions.includes(sessionId)) throw new Error('Session does not belong to the DSH Agent') // Archive first: the Session leaves the Agent only once it is actually gone, // so a failed archive stays retryable and keeps its tool restriction. await ctx.workspaceRegistry.archiveSession(sessionId) - agent.sessions = agent.sessions.filter(id => id !== sessionId) - await rt.save() + await rt.mutate(() => { + const agent = record(agentId) + agent.sessions = agent.sessions.filter(id => id !== sessionId) + }) } async function renameAgentSession(agentId, sessionId, title) { @@ -158,9 +170,9 @@ export function install(rt) { } async function removeAgent(agentId) { - writeCheck() - const agent = record(agentId) - const sessions = [...agent.sessions] + record(agentId) + await beforeWrite() + const sessions = [...record(agentId).sessions] const kept = [] let failure for (const sessionId of sessions) { @@ -174,12 +186,10 @@ export function install(rt) { // What could not be archived stays with the Agent, so it keeps its tool // restriction and the removal can be retried from the same place. if (kept.length > 0) { - agent.sessions = kept - await rt.save() + await rt.mutate(() => { record(agentId).sessions = kept }) throw failure } - delete state.agents[agentId] - await rt.save() + await rt.mutate(() => { delete state.agents[agentId] }) } Object.assign(rt, { agentOf, addAgent, createAgentSession, adoptAgentSession, archiveAgentSession, renameAgentSession, removeAgent }) diff --git a/core/src/host/delivery.js b/core/src/host/delivery.js index 95a0aeedb..e5418a716 100644 --- a/core/src/host/delivery.js +++ b/core/src/host/delivery.js @@ -101,19 +101,25 @@ export function install(rt) { // line it has not shown yet arrived (its row turns unread), and the senders seen so far, kept apart // from the roster, tell the first line from a sender. A group line is the group's, not the Bot's. const CONTACTS_KEPT = 200 - const noteFriendLine = (to, sessionId, roomId, handle) => { + const noteFriendLine = async (to, sessionId, roomId, handle) => { if (to === undefined || roomId || (sessionId !== undefined && sessionId !== rt.chatOf(to.id))) return false const key = String(handle) - const contacts = (state.remoteContacts ??= {})[to.id] ??= {} - const first = contacts[key] === undefined const now = Date.now() - if (first) { - contacts[key] = now - const keys = Object.keys(contacts) - if (keys.length > CONTACTS_KEPT) for (const old of keys.slice(0, keys.length - CONTACTS_KEPT)) delete contacts[old] - } - to.friendLineAt = now - void rt.save().catch(() => {}) + let first = false + // A change made outside a request: `mutate` adopts what the other host left before the + // change, so the Bot record is re-resolved inside the callback, never used from before it. + await rt.mutate(() => { + const bot = state.bots[to.id] + if (bot === undefined) return + const contacts = (state.remoteContacts ??= {})[bot.id] ??= {} + first = contacts[key] === undefined + if (first) { + contacts[key] = now + const keys = Object.keys(contacts) + if (keys.length > CONTACTS_KEPT) for (const old of keys.slice(0, keys.length - CONTACTS_KEPT)) delete contacts[old] + } + bot.friendLineAt = now + }) return first } @@ -166,7 +172,7 @@ export function install(rt) { return exchangeId } const origin = originOf({ from, to, fromId, remote, given: givenOrigin }) - const firstContact = remote?.handle && !deferred && !resumed && !retried && !resent ? noteFriendLine(to, sessionId, roomId, remote.handle) : false + const firstContact = remote?.handle && !deferred && !resumed && !retried && !resent ? await noteFriendLine(to, sessionId, roomId, remote.handle) : false // Session events reject `undefined` members, so optional fields are omitted instead. const source = { kind: SOURCE_KIND, @@ -194,8 +200,8 @@ export function install(rt) { ...(remote?.handle && Array.isArray(attachments) && attachments.length ? { attachments: attachments.map(file => ({ id: String(file.id ?? ''), name: String(file.name), size: Number(file.size), mime: String(file.mime ?? '') })) } : {}), } if (EXCHANGE_ROLES.has(role) && !deferred && !resumed && !retried && !resent) { - state.exchanges.push({ id: exchangeId, from: fromId, to: toId, text, role, time: Date.now(), ...(replyTo ? { replyTo } : {}), ...(audience === 'user' ? { audience } : {}) }) - void rt.save().catch(() => {}) + const exchange = { id: exchangeId, from: fromId, to: toId, text, role, time: Date.now(), ...(replyTo ? { replyTo } : {}), ...(audience === 'user' ? { audience } : {}) } + await rt.mutate(() => { state.exchanges.push(exchange) }) } await rt.ensureCapabilities(targetSessionId) const agent = await resolveAgent(targetSessionId) diff --git a/core/test/mutate-writers.test.js b/core/test/mutate-writers.test.js new file mode 100644 index 000000000..0f19b1647 --- /dev/null +++ b/core/test/mutate-writers.test.js @@ -0,0 +1,104 @@ +import assert from 'node:assert/strict' +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { hostname, tmpdir } from 'node:os' +import { join } from 'node:path' +import { test } from 'node:test' +import { createRuntime } from '../index.js' +import { fakeContext } from './fake-ctx.js' + +// #1048: the writers that still change `state` inline and then `void rt.save()` must go +// through `rt.mutate(fn)` instead. A window that was read-only only because the other dsh +// had the team open takes the free lock on its next write (#1027), and that write must not +// replace what the other host left with this window's older copy (#1032): the change goes +// *after* the adopt, with every record re-resolved inside the callback. + +const TEAM = { + version: 1, + mainBotIds: ['chief'], + bots: { chief: { id: 'chief', sessionId: 'chief', name: 'Chief', role: 'Lead', instructions: '', createdAt: 1 } }, + rooms: {}, + agents: {}, + exchanges: [], + exchangeTurns: {}, + questions: {}, + revision: 7, +} + +const tempHome = (t) => { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-mutate-writers-')) + t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 })) + return home +} + +// A runtime playing the reading host: another live dsh holds the team, so this window +// loads the team and cannot change it. `takeOver` is what the other host's exit looks +// like — it saves once more, then closes, without this window having read since. +async function readingWindow(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 ctx = fakeContext() + const agents = new Map() + let sessions = 0 + const agentFor = async (id) => { + if (agents.has(id)) return agents.get(id) + const agent = { + id, + session: { id }, + ctx: { tools: { restrict: () => {} }, effect: execute => execute() }, + followup: () => {}, + } + agents.set(id, agent) + return agent + } + Object.assign(ctx, { + workspaceRegistry: { get: () => undefined, resolveByPath: async () => undefined, create: async path => ({ id: 'w1', path }), archiveSession: async () => {} }, + sessionController: { + create: async () => ({ sessionId: `new${++sessions}` }), + resolveAgent: async id => ({ agent: await agentFor(id) }), + }, + sessionTitle: { rename: async () => {} }, + sessions: { flush: async () => {} }, + }) + const rt = createRuntime(ctx, { home, github: false, diagnosticsFile: false }) + t.after(() => ctx.dispose()) + await rt.load() + assert.match(rt.readOnly(), /open in another dsh/, 'the rig starts on the reading side') + + // The other host writes a change this window has not seen, then closes. + writeFileSync(join(home, 'state.json'), JSON.stringify({ ...TEAM, revision: 8, prefs: { theme: 'light' } })) + rmSync(join(home, 'state.lock')) + const saved = () => JSON.parse(readFileSync(join(home, 'state.json'), 'utf8')) + return { ctx, rt, home, saved, sessionsMade: () => sessions } +} + +test('#1048 delivery: a friend line written from a window that was read-only keeps what the other host left', async (t) => { + const { rt, saved } = await readingWindow(t) + + // A friend's first line in the Bot's own chat: noteFriendLine records the contact and + // marks the row unread, then saves — an inline writer today. + // `flybotChecked`: the approval that let the line in; this test is about the write below it. + await rt.deliver({ toId: 'chief', role: 'request', text: 'hi', remote: { handle: '@friend.home', name: 'Friend' }, flybotChecked: true }) + // The exchange record is the second inline writer in the same request. + await rt.deliver({ fromId: 'chief', toId: 'chief', role: 'reply', text: 'hello back' }) + // The writers save without being awaited, so settle the write chain before reading. + await rt.save() + + const file = saved() + assert.equal(file.prefs.theme, 'light', 'the change the other host left survives') + assert.equal(typeof file.remoteContacts.chief['@friend.home'], 'number', 'the first contact is written') + assert.equal(typeof file.bots.chief.friendLineAt, 'number', 'the unread mark is written') + assert.ok(file.exchanges.some(exchange => exchange.role === 'reply'), 'the exchange is written') +}) + +test('#1048 agents: adding the DSH Agent from a window that was read-only keeps what the other host left', async (t) => { + const { rt, saved, sessionsMade } = await readingWindow(t) + + const agent = await rt.addAgent() + + const file = saved() + assert.equal(file.prefs.theme, 'light', 'the change the other host left survives') + assert.equal(sessionsMade(), 1, 'one Session is created for the new Agent') + assert.deepEqual(file.agents[agent.id].sessions, ['new1'], 'the Agent and its Session are written') + assert.equal(rt.readOnly(), undefined, 'the free lock is taken over') +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 1a5b9c5aa..1a63aaa64 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`). 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. +- 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 changed `rt.state` inline and then called `save()` are gone (#1048): `delivery.js` (a friend's first line and its unread mark, a team exchange) and `agents.js` (add, adopt, archive and remove of the DSH Agent) take the free lock and adopt first, re-resolving their records inside the change, so a write from a window that was read-only keeps what the other host left instead of replacing it with its older copy. `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 88be1d87d..cc36b426f 100644 --- a/patches/ds-bot/0001-core-fixes.patch +++ b/patches/ds-bot/0001-core-fixes.patch @@ -2016,17 +2016,146 @@ diff --git a/src/client/voice.js b/src/client/voice.js diff --git a/src/host/agents.js b/src/host/agents.js --- a/src/host/agents.js +++ b/src/host/agents.js -@@ -129,8 +129,8 @@ export function install(rt) { +@@ -77,6 +77,13 @@ export function install(rt) { + const readOnly = rt.readOnly() + if (readOnly !== undefined) throw new Error(readOnly) + } ++ // A change made outside a request may start in a window that was read-only only because ++ // the other dsh had the team open: refresh first — it adopts what that host left and ++ // takes the free lock (#1027, #1032) — then refuse if the lock is still someone else's. ++ const beforeWrite = async () => { ++ if (rt.readOnly() !== undefined) await rt.refresh() ++ writeCheck() ++ } + const record = (agentId) => { + const agent = state.agents[agentId] + if (agent === undefined) throw new Error('Unknown DSH Agent') +@@ -84,16 +84,15 @@ export function install(rt) { + } + + async function createAgentSession(agentId, workspaceId) { +- writeCheck() +- const agent = record(agentId) ++ record(agentId) ++ await beforeWrite() + // A known workspace id wins (the Agent's latest Session may live outside the + // team workspace after a workspace switch); anything else gets the team's own. + const target = typeof workspaceId === 'string' && ctx.workspaceRegistry.get(workspaceId) + ? workspaceId + : await rt.ensureWorkspace() + const { sessionId } = await ctx.sessionController.create({ workspaceId: target }) +- agent.sessions.push(sessionId) +- await rt.save() ++ await rt.mutate(() => { record(agentId).sessions.push(sessionId) }) + // agent/created fired before the session joined the index, so restrict again now. + await restrictTeamTools(await rt.resolveAgent(sessionId)) + return sessionId +@@ -103,48 +103,54 @@ export function install(rt) { + // picker or a fork) is adopted into the same Agent: it keeps dsh's surface and + // the team tools stay hidden. + async function adoptAgentSession(agentId, sessionId) { +- writeCheck() +- const agent = record(agentId) +- if (agent.sessions.includes(sessionId)) return sessionId ++ record(agentId) ++ await beforeWrite() ++ if (record(agentId).sessions.includes(sessionId)) return sessionId + await rt.resolveAgent(sessionId) +- const owned = rt.selfOf(sessionId) !== undefined || rt.botOf(sessionId) !== undefined +- || rt.roomOf(sessionId) !== undefined || agentOf(sessionId) !== undefined +- if (owned) throw new Error('Session already belongs to the team') +- agent.sessions.push(sessionId) +- await rt.save() ++ // The ownership check reads the records, so it runs inside the change, after the adopt. ++ await rt.mutate(() => { ++ const owned = rt.selfOf(sessionId) !== undefined || rt.botOf(sessionId) !== undefined ++ || rt.roomOf(sessionId) !== undefined || agentOf(sessionId) !== undefined ++ if (owned) throw new Error('Session already belongs to the team') ++ const agent = record(agentId) ++ if (!agent.sessions.includes(sessionId)) agent.sessions.push(sessionId) ++ }) + await restrictTeamTools(await rt.resolveAgent(sessionId)) + return sessionId + } + + async function addAgent() { +- writeCheck() +- if (Object.keys(state.agents).length > 0) throw new Error('The team already has a DSH Agent') +- const agent = { id: randomUUID(), name: AGENT_NAME, sessions: [], createdAt: Date.now() } +- state.agents[agent.id] = agent ++ const id = randomUUID() ++ await rt.mutate(() => { ++ writeCheck() ++ if (Object.keys(state.agents).length > 0) throw new Error('The team already has a DSH Agent') ++ state.agents[id] = { id, name: AGENT_NAME, sessions: [], createdAt: Date.now() } ++ }) + try { +- await createAgentSession(agent.id) ++ await createAgentSession(id) + } catch (error) { + // The first Session may already be created and saved: archive it and + // persist the rollback, or the Agent returns on restart with its team // tools never denied. - const createdSessions = [...agent.sessions] - delete state.agents[agent.id] +- const createdSessions = [...agent.sessions] +- delete state.agents[agent.id] - for (const sessionId of createdSessions) await ctx.workspaceRegistry.archiveSession(sessionId).catch(() => {}) - await rt.save().catch(() => {}) ++ const createdSessions = [...record(id).sessions] ++ await rt.mutate(() => { delete state.agents[id] }).catch(() => { /* rollback is best effort; the original error is what the caller sees */ }) + for (const sessionId of createdSessions) await ctx.workspaceRegistry.archiveSession(sessionId).catch(() => { /* rollback is best effort; the original error is what the caller sees */ }) -+ await rt.save().catch(() => { /* rollback is best effort; the original error is what the caller sees */ }) throw error } - return agent +- return agent ++ return record(id) + } + + async function archiveAgentSession(agentId, sessionId) { +- writeCheck() +- const agent = record(agentId) +- if (!agent.sessions.includes(sessionId)) throw new Error('Session does not belong to the DSH Agent') ++ record(agentId) ++ await beforeWrite() ++ if (!record(agentId).sessions.includes(sessionId)) throw new Error('Session does not belong to the DSH Agent') + // Archive first: the Session leaves the Agent only once it is actually gone, + // so a failed archive stays retryable and keeps its tool restriction. + await ctx.workspaceRegistry.archiveSession(sessionId) +- agent.sessions = agent.sessions.filter(id => id !== sessionId) +- await rt.save() ++ await rt.mutate(() => { ++ const agent = record(agentId) ++ agent.sessions = agent.sessions.filter(id => id !== sessionId) ++ }) + } + + async function renameAgentSession(agentId, sessionId, title) { +@@ -158,9 +158,9 @@ export function install(rt) { + } + + async function removeAgent(agentId) { +- writeCheck() +- const agent = record(agentId) +- const sessions = [...agent.sessions] ++ record(agentId) ++ await beforeWrite() ++ const sessions = [...record(agentId).sessions] + const kept = [] + let failure + for (const sessionId of sessions) { +@@ -174,12 +174,10 @@ export function install(rt) { + // What could not be archived stays with the Agent, so it keeps its tool + // restriction and the removal can be retried from the same place. + if (kept.length > 0) { +- agent.sessions = kept +- await rt.save() ++ await rt.mutate(() => { record(agentId).sessions = kept }) + throw failure + } +- delete state.agents[agentId] +- await rt.save() ++ await rt.mutate(() => { delete state.agents[agentId] }) + } + + Object.assign(rt, { agentOf, addAgent, createAgentSession, adoptAgentSession, archiveAgentSession, renameAgentSession, removeAgent }) diff --git a/src/host/approvals.js b/src/host/approvals.js --- a/src/host/approvals.js +++ b/src/host/approvals.js diff --git a/patches/ds-bot/0005-social-seams.patch b/patches/ds-bot/0005-social-seams.patch index f26d5aa65..dd5311374 100644 --- a/patches/ds-bot/0005-social-seams.patch +++ b/patches/ds-bot/0005-social-seams.patch @@ -968,7 +968,7 @@ diff --git a/src/host/delivery.js b/src/host/delivery.js if (deferred) return `[Message from ${fromName} · you read it during your last task; answer it now]` if (retried && role !== 'room') return '[Your last reply wrote a tool call as text, so nothing ran]' if (resumed && role === 'resume') return '[DeepSeek Harness restarted while you were working]' -@@ -22,15 +22,67 @@ export function install(rt) { +@@ -22,15 +22,73 @@ export function install(rt) { if (role === 'stopped') return `[Stopped by ${fromName}]` if (role === 'room') return `[Group chat "${roomName}" · ${note}]` if (role === 'report') return `[Group chat "${roomName}" · replies to your post]` @@ -994,19 +994,25 @@ diff --git a/src/host/delivery.js b/src/host/delivery.js + // line it has not shown yet arrived (its row turns unread), and the senders seen so far, kept apart + // from the roster, tell the first line from a sender. A group line is the group's, not the Bot's. + const CONTACTS_KEPT = 200 -+ const noteFriendLine = (to, sessionId, roomId, handle) => { ++ const noteFriendLine = async (to, sessionId, roomId, handle) => { + if (to === undefined || roomId || (sessionId !== undefined && sessionId !== rt.chatOf(to.id))) return false + const key = String(handle) -+ const contacts = (state.remoteContacts ??= {})[to.id] ??= {} -+ const first = contacts[key] === undefined + const now = Date.now() -+ if (first) { -+ contacts[key] = now -+ const keys = Object.keys(contacts) -+ if (keys.length > CONTACTS_KEPT) for (const old of keys.slice(0, keys.length - CONTACTS_KEPT)) delete contacts[old] -+ } -+ to.friendLineAt = now -+ void rt.save().catch(() => {}) ++ let first = false ++ // A change made outside a request: `mutate` adopts what the other host left before the ++ // change, so the Bot record is re-resolved inside the callback, never used from before it. ++ await rt.mutate(() => { ++ const bot = state.bots[to.id] ++ if (bot === undefined) return ++ const contacts = (state.remoteContacts ??= {})[bot.id] ??= {} ++ first = contacts[key] === undefined ++ if (first) { ++ contacts[key] = now ++ const keys = Object.keys(contacts) ++ if (keys.length > CONTACTS_KEPT) for (const old of keys.slice(0, keys.length - CONTACTS_KEPT)) delete contacts[old] ++ } ++ bot.friendLineAt = now ++ }) + return first + } + @@ -1062,7 +1068,7 @@ diff --git a/src/host/delivery.js b/src/host/delivery.js + return exchangeId + } + const origin = originOf({ from, to, fromId, remote, given: givenOrigin }) -+ const firstContact = remote?.handle && !deferred && !resumed && !retried && !resent ? noteFriendLine(to, sessionId, roomId, remote.handle) : false ++ const firstContact = remote?.handle && !deferred && !resumed && !retried && !resent ? await noteFriendLine(to, sessionId, roomId, remote.handle) : false // Session events reject `undefined` members, so optional fields are omitted instead. const source = { kind: SOURCE_KIND, @@ -1087,8 +1093,10 @@ diff --git a/src/host/delivery.js b/src/host/delivery.js + ...(remote?.handle && Array.isArray(attachments) && attachments.length ? { attachments: attachments.map(file => ({ id: String(file.id ?? ''), name: String(file.name), size: Number(file.size), mime: String(file.mime ?? '') })) } : {}), } if (EXCHANGE_ROLES.has(role) && !deferred && !resumed && !retried && !resent) { - state.exchanges.push({ id: exchangeId, from: fromId, to: toId, text, role, time: Date.now(), ...(replyTo ? { replyTo } : {}), ...(audience === 'user' ? { audience } : {}) }) - void rt.save().catch(() => {}) +- state.exchanges.push({ id: exchangeId, from: fromId, to: toId, text, role, time: Date.now(), ...(replyTo ? { replyTo } : {}), ...(audience === 'user' ? { audience } : {}) }) +- void rt.save().catch(() => {}) ++ const exchange = { id: exchangeId, from: fromId, to: toId, text, role, time: Date.now(), ...(replyTo ? { replyTo } : {}), ...(audience === 'user' ? { audience } : {}) } ++ await rt.mutate(() => { state.exchanges.push(exchange) }) } - const targetSessionId = sessionId ?? rt.chatOf(toId) await rt.ensureCapabilities(targetSessionId) diff --git a/patches/ds-bot/files/test/mutate-writers.test.js b/patches/ds-bot/files/test/mutate-writers.test.js new file mode 100644 index 000000000..0f19b1647 --- /dev/null +++ b/patches/ds-bot/files/test/mutate-writers.test.js @@ -0,0 +1,104 @@ +import assert from 'node:assert/strict' +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { hostname, tmpdir } from 'node:os' +import { join } from 'node:path' +import { test } from 'node:test' +import { createRuntime } from '../index.js' +import { fakeContext } from './fake-ctx.js' + +// #1048: the writers that still change `state` inline and then `void rt.save()` must go +// through `rt.mutate(fn)` instead. A window that was read-only only because the other dsh +// had the team open takes the free lock on its next write (#1027), and that write must not +// replace what the other host left with this window's older copy (#1032): the change goes +// *after* the adopt, with every record re-resolved inside the callback. + +const TEAM = { + version: 1, + mainBotIds: ['chief'], + bots: { chief: { id: 'chief', sessionId: 'chief', name: 'Chief', role: 'Lead', instructions: '', createdAt: 1 } }, + rooms: {}, + agents: {}, + exchanges: [], + exchangeTurns: {}, + questions: {}, + revision: 7, +} + +const tempHome = (t) => { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-mutate-writers-')) + t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 })) + return home +} + +// A runtime playing the reading host: another live dsh holds the team, so this window +// loads the team and cannot change it. `takeOver` is what the other host's exit looks +// like — it saves once more, then closes, without this window having read since. +async function readingWindow(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 ctx = fakeContext() + const agents = new Map() + let sessions = 0 + const agentFor = async (id) => { + if (agents.has(id)) return agents.get(id) + const agent = { + id, + session: { id }, + ctx: { tools: { restrict: () => {} }, effect: execute => execute() }, + followup: () => {}, + } + agents.set(id, agent) + return agent + } + Object.assign(ctx, { + workspaceRegistry: { get: () => undefined, resolveByPath: async () => undefined, create: async path => ({ id: 'w1', path }), archiveSession: async () => {} }, + sessionController: { + create: async () => ({ sessionId: `new${++sessions}` }), + resolveAgent: async id => ({ agent: await agentFor(id) }), + }, + sessionTitle: { rename: async () => {} }, + sessions: { flush: async () => {} }, + }) + const rt = createRuntime(ctx, { home, github: false, diagnosticsFile: false }) + t.after(() => ctx.dispose()) + await rt.load() + assert.match(rt.readOnly(), /open in another dsh/, 'the rig starts on the reading side') + + // The other host writes a change this window has not seen, then closes. + writeFileSync(join(home, 'state.json'), JSON.stringify({ ...TEAM, revision: 8, prefs: { theme: 'light' } })) + rmSync(join(home, 'state.lock')) + const saved = () => JSON.parse(readFileSync(join(home, 'state.json'), 'utf8')) + return { ctx, rt, home, saved, sessionsMade: () => sessions } +} + +test('#1048 delivery: a friend line written from a window that was read-only keeps what the other host left', async (t) => { + const { rt, saved } = await readingWindow(t) + + // A friend's first line in the Bot's own chat: noteFriendLine records the contact and + // marks the row unread, then saves — an inline writer today. + // `flybotChecked`: the approval that let the line in; this test is about the write below it. + await rt.deliver({ toId: 'chief', role: 'request', text: 'hi', remote: { handle: '@friend.home', name: 'Friend' }, flybotChecked: true }) + // The exchange record is the second inline writer in the same request. + await rt.deliver({ fromId: 'chief', toId: 'chief', role: 'reply', text: 'hello back' }) + // The writers save without being awaited, so settle the write chain before reading. + await rt.save() + + const file = saved() + assert.equal(file.prefs.theme, 'light', 'the change the other host left survives') + assert.equal(typeof file.remoteContacts.chief['@friend.home'], 'number', 'the first contact is written') + assert.equal(typeof file.bots.chief.friendLineAt, 'number', 'the unread mark is written') + assert.ok(file.exchanges.some(exchange => exchange.role === 'reply'), 'the exchange is written') +}) + +test('#1048 agents: adding the DSH Agent from a window that was read-only keeps what the other host left', async (t) => { + const { rt, saved, sessionsMade } = await readingWindow(t) + + const agent = await rt.addAgent() + + const file = saved() + assert.equal(file.prefs.theme, 'light', 'the change the other host left survives') + assert.equal(sessionsMade(), 1, 'one Session is created for the new Agent') + assert.deepEqual(file.agents[agent.id].sessions, ['new1'], 'the Agent and its Session are written') + assert.equal(rt.readOnly(), undefined, 'the free lock is taken over') +})