diff --git a/changelog.d/2607.fixed.md b/changelog.d/2607.fixed.md new file mode 100644 index 000000000..b0a9eaf90 --- /dev/null +++ b/changelog.d/2607.fixed.md @@ -0,0 +1 @@ +Deleting a Bot now ends its seat in every group chat in the same save: the group's member list and a shared room's guest roster stop naming it, and the next membership change can no longer write the deleted id back into the room, also after a reload ([#2607](https://github.com/ThinkFlowLab/FlyBot/issues/2607)). diff --git a/core/src/host/room-membership.js b/core/src/host/room-membership.js index e37c04f58..17d614ab1 100644 --- a/core/src/host/room-membership.js +++ b/core/src/host/room-membership.js @@ -66,6 +66,16 @@ export function currentParticipants(room) { return (room?.participants ?? []).filter(p => p.leftSeq === undefined) } +// The one derivation of `room.members` from the seats. `deleteBot` (roster.js) shares it so a +// deletion closes the seat and derives `members` in the same step (#2607): a separate filter +// leaves a current participant behind, and the next membership write rebuilds the deleted id. +export function syncMembers(room) { + room.members = currentParticipants(room).filter(p => p.kind === 'bot' && !p.remote).map(p => p.id) + const botAdmin = currentParticipants(room).find(p => p.kind === 'bot' && !p.remote && p.role === 'admin') + if (botAdmin) room.admin = botAdmin.id + else if (room.admin && !room.members.includes(room.admin)) room.admin = room.members[0] ?? null +} + export function isRemoteParticipant(p) { return p?.kind === 'bot' && p.remote === true } @@ -407,13 +417,6 @@ export function install(rt) { return stored } - const syncMembers = (room) => { - room.members = currentParticipants(room).filter(p => p.kind === 'bot' && !p.remote).map(p => p.id) - const botAdmin = currentParticipants(room).find(p => p.kind === 'bot' && !p.remote && p.role === 'admin') - if (botAdmin) room.admin = botAdmin.id - else if (room.admin && !room.members.includes(room.admin)) room.admin = room.members[0] ?? null - } - // #1987: a group without a chosen name is named after its seats, so a seat that joins or leaves moves // the name with it — and the group's own Session, the title the user reads, moves with the name. `room` // is passed for both the comparison and the write: `autoRoomName` reads the people and the remote guests diff --git a/core/src/host/roster.js b/core/src/host/roster.js index 0c8c98499..cbcc8d086 100644 --- a/core/src/host/roster.js +++ b/core/src/host/roster.js @@ -6,7 +6,7 @@ import { MAX_AVATAR_IMAGE, ROOM_MAX_MEMBERS, ROOM_MODES, ROOM_NAME_MAX, ROOM_NOT import { currentColor, cut, isColor, unquote } from './text.js' import { nameProblem } from '../shared/mentions.js' import { cleanExternal } from './room-participant-external.js' -import { implicitRoomOwner } from './room-membership.js' +import { currentParticipants, implicitRoomOwner, syncMembers } from './room-membership.js' import { ownEntry } from './store.js' export function install(rt) { @@ -649,9 +649,21 @@ export function install(rt) { } const touched = [] for (const room of Object.values(state.rooms)) { - if (!room.members.includes(id)) continue - room.members = room.members.filter(member => member !== id) - if (room.admin === id) room.admin = null + const seat = currentParticipants(room).find(p => p.kind === 'bot' && !p.remote && p.id === id) + if (!room.members.includes(id) && seat === undefined) continue + // #2607: end the seat the way leaving does, then derive `members` from the seats. A separate + // filter leaves the participant current, and the next membership write writes the id back. + if (Array.isArray(room.participants)) { + if (seat !== undefined) { + seat.leftSeq = room.seq ?? 0 + seat.leftAt = Date.now() + } + if (room.admin === id) room.admin = null + syncMembers(room) + } else { + room.members = room.members.filter(member => member !== id) + if (room.admin === id) room.admin = null + } if (room.taskLead === id) delete room.taskLead rt.dropGroupSession(room, id) touched.push(room) diff --git a/core/test/delete-bot-seat.test.js b/core/test/delete-bot-seat.test.js new file mode 100644 index 000000000..0e05b0a9f --- /dev/null +++ b/core/test/delete-bot-seat.test.js @@ -0,0 +1,139 @@ +import assert from 'node:assert/strict' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { 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' + +// #2607: deleting a Bot ends its seats in every room, the way leaving does. Before this the deletion +// only filtered the legacy `room.members` array, the room's `participants` entry stayed current, and +// the next membership write rebuilt `room.members` from that entry — the deleted id came back and the +// following save wrote it to disk. Two rooms keep the older shape without `participants`: that path +// must still drop the id from `members` and stay without a participant list. + +const live = room => (room.participants ?? []).filter(p => p.leftSeq === undefined).map(p => `${p.kind}:${p.id}`) + +const team = () => ({ + version: 2, + revision: 1, + mainBotIds: ['chief'], + members: { owner: { id: 'owner', createdAt: 1, designatedChiefId: 'chief' } }, + bots: { + chief: { id: 'chief', sessionId: 'chief', name: 'Chief', role: 'Lead', instructions: '', createdAt: 1, ownerId: 'owner' }, + writer: { id: 'writer', sessionId: 'writer', name: 'Writer', role: 'Write', instructions: '', createdAt: 2, ownerId: 'owner' }, + coder: { id: 'coder', sessionId: 'coder', name: 'Coder', role: 'Code', instructions: '', createdAt: 3, ownerId: 'owner' }, + }, + rooms: { + // The Bot being deleted is a plain member here. + desk: { + id: 'desk', + name: 'Desk', + members: ['chief', 'writer'], + admin: 'chief', + named: true, + log: [], + seq: 0, + seen: {}, + participants: [ + { kind: 'human', id: 'owner', role: 'admin', displayName: 'Owner', joinedSeq: 0, joinedAt: 1 }, + { kind: 'bot', id: 'chief', role: 'admin', displayName: 'Chief', joinedSeq: 0, joinedAt: 1 }, + { kind: 'bot', id: 'writer', role: 'member', displayName: 'Writer', joinedSeq: 0, joinedAt: 1 }, + ], + }, + // The deleted Bot is the room's only admin and its only Bot — `members` is empty afterwards. + solo: { + id: 'solo', + name: 'Solo', + members: ['writer'], + admin: 'writer', + named: true, + log: [], + seq: 0, + seen: {}, + participants: [ + { kind: 'human', id: 'owner', role: 'admin', displayName: 'Owner', joinedSeq: 0, joinedAt: 1 }, + { kind: 'bot', id: 'writer', role: 'admin', displayName: 'Writer', joinedSeq: 0, joinedAt: 1 }, + ], + }, + // No `participants` array: the shipped single-owner shape. + legacy: { id: 'legacy', name: 'Legacy', members: ['chief', 'writer'], admin: 'chief', mode: 'everyone' }, + }, + exchanges: [], + exchangeTurns: {}, + questions: {}, + revision: 1, +}) + +function context() { + const ctx = fakeContext() + Object.assign(ctx, { + sessions: { get: () => undefined, flush: async () => {} }, + workspaceRegistry: { get: () => undefined, resolveByPath: async () => undefined, create: async path => ({ id: 'w1', path }), archiveSession: async () => {} }, + sessionController: { create: async () => ({ sessionId: 'new1' }), resolveAgent: async id => ({ agent: { id, session: { id, header: {} } } }), selectModel: async () => {} }, + sessionTitle: { rename: async () => {} }, + llm: { listProviders: () => [] }, + }) + return ctx +} + +async function open(home) { + const ctx = context() + const rt = createRuntime(ctx, { home, github: false, composio: false, diagnosticsFile: false }) + await rt.load() + return { ctx, rt } +} + +async function setup(t) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-delete-seat-')) + writeFileSync(join(home, 'state.json'), JSON.stringify(team())) + const first = await open(home) + t.after(async () => { + await first.ctx.dispose() + rmSync(home, { recursive: true, force: true, maxRetries: 5 }) + }) + return { home, rt: first.rt, ctx: first.ctx } +} + +test('deleting a Bot closes its participant seat and the next membership write does not write it back (#2607)', async (t) => { + const { home, rt, ctx } = await setup(t) + await rt.deleteBot('writer') + assert.equal(rt.botOf('writer'), undefined, 'the Bot record is gone') + const desk = rt.state.rooms.desk + const seat = desk.participants.find(p => p.kind === 'bot' && p.id === 'writer') + assert.ok(seat.leftSeq !== undefined, 'the deleted Bot no longer holds a current seat') + assert.equal(typeof seat.leftAt, 'number', 'the seat records when it ended') + assert.deepEqual(live(desk), ['human:owner', 'bot:chief'], 'the participant list no longer answers the deleted Bot') + assert.deepEqual(rt.participantsOf(desk).map(p => `${p.kind}:${p.id}`), ['human:owner', 'bot:chief'], 'the drawer reads the same list') + assert.deepEqual(desk.members, ['chief'], 'members is derived from the seats, not filtered beside them') + + // One ordinary membership write is what used to resurrect the id through `syncMembers`. + await rt.addRoomParticipant('desk', { kind: 'bot', id: 'coder' }) + assert.deepEqual(desk.members, ['chief', 'coder'], 'the deleted id does not come back') + assert.deepEqual(live(desk), ['human:owner', 'bot:chief', 'bot:coder']) + + // The deletion and the write were saved, so a host that reads the same state.json must agree. + await ctx.dispose() + const second = await open(home) + t.after(async () => { await second.ctx.dispose() }) + const reread = second.rt.state.rooms.desk + assert.equal(second.rt.botOf('writer'), undefined) + assert.equal(live(reread).includes('bot:writer'), false, 'the saved seat stays closed') + assert.deepEqual(reread.members, ['chief', 'coder'], 'the saved members list does not carry the deleted id') +}) + +test('a deleted Bot that was the only admin, and a room left with no Bots, take the next membership write (#2607)', async (t) => { + const { rt } = await setup(t) + await rt.deleteBot('writer') + const solo = rt.state.rooms.solo + assert.deepEqual(solo.members, [], 'the room is left with no Bots') + assert.equal(solo.admin, null, 'the deleted Bot is not still the admin') + assert.deepEqual(live(solo), ['human:owner']) + await rt.addRoomParticipant('solo', { kind: 'bot', id: 'coder' }) + assert.deepEqual(solo.members, ['coder']) + assert.deepEqual(live(solo), ['human:owner', 'bot:coder']) + // The room without a participants array keeps the shipped shape. + const legacy = rt.state.rooms.legacy + assert.equal(legacy.participants, undefined, 'no participant list is invented for a legacy room') + assert.deepEqual(legacy.members, ['chief']) +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 1835675fb..5f07b6645 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -238,6 +238,7 @@ An admin's `@` used to be the room's control plane: every `@Dev #1234 rebase and - 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. - **A Main Bot retires a Bot; it never deletes one (#638).** `archive_bot` (`tools.js`, Main Bot only) calls `roster.archiveBot`: it writes `fields['flybot-tree'].retired = true` through `rt.setBotField` — the same path and `change` event a plugin's `fields.set` uses, so the tree and the sidebar refresh — and sets `bot.hidden = true`, the flag the tree's own archive uses to keep a Bot out of the default list while Search still finds it (#452). The record, its chat, parts and memory stay where they are, so its history stays readable; retirement has no un-retire path, and the restore entries in the sidebar and the palette only clear `hidden` (`core/src/client/new-chat.js:104`, `core/src/client/palette.js:60`), which opens the chat again but leaves the record retired. The tree's `forestOf` skips a retired record, so its slot under `maxChildren` is free again and adoption leaves it alone. A Main Bot, the caller itself and an unknown name are refused, nothing reparents, and `list_bots` leaves a retired Bot out (the record is still found by name). Retirement is terminal (#1700): the tools that resolve a Bot through `findTeamBot` — `message_bot`, `stop_bot`, `update_bot`, `create_group`, `update_group` (`add_members`, `admin`, `task_lead`) and `archive_bot` — refuse a retired record exactly as they refuse an unknown name, so `message_bot` sends nothing and starts no turn, and `archive_bot` aimed at an already-retired name answers the same "No Bot named …" line. Since [#1709](https://github.com/ThinkFlowLab/FlyBot/issues/1709) the same `retiredBot` predicate (`store.js`, one copy, imported by every caller) is the one check the host paths outside `findTeamBot` apply, and each of them is listed here: `track_tasks`' assignee and a group `@Name` resolve through it, so no card is written and no turn starts (no delivery, no Session); `attach_capability`/`detach_capability` and a `remember`/`recall` `scope:` treat a retired record exactly as an unknown name; `schedule-create`/`schedule-update`/`schedule-copy` refuse it with the same `No such Bot or group chat` answer, and `schedule-resume` refuses a paused row of a retired Bot the same way; `task-create` (the endpoint the client calls), a plugin's `routeOpenTask` and `task-send-back` (which opens a rework card on the same owner) refuse it with `No such Bot`; a send held for approval whose target has been retired meanwhile is stale (`the target Bot was retired`) instead of delivered; and a restart neither picks up a retired Bot's interrupted turn nor sends again what was queued for it. The task drive never wakes a retired owner: `drivingTasks` returns nothing for it, so a card still on the board is not delivered, and a card that arrives through none of these doors is left exactly where it is. Archiving also disarms the target's own chat and the group Sessions it speaks through, every part of each: `rt.pauseBotSchedules` (installed by `schedules.js`) takes the Bot's current part plus each earlier part in `bot.segments`, and each `room.sessions[botId]` plus each earlier part in `room.segments[botId]`, writes each active row — with the part it came from — into `state.pausedSchedules` before DSH drops it, the way pausing one task does, so a task left on an old Session by a part hand-off is not still armed after retirement (#1799), and neither is one armed where it speaks in a group; a room's own Session is left alone, since its rows belong to the whole group, and `archive_bot`'s answer says `Paused N task(s)`—or how many are still armed—instead of staying silent. Retiring means to the board what deleting means: `archiveBot` calls `rt.taskBotDeleted` with `the responsible Bot was retired`, so the cards a retired Bot still holds in `open`, `claimed`, `working` or `blocked` fail instead of waiting for a Bot that will not run again, while a card in `result-ready` stays for its requester to accept or cancel. Still not covered: an un-retire path (there is none), the memory and chat the record keeps, and the connectors a retired Bot's own Session no longer starts. Deleting remains the UI's `delete-bot` call (`roster.deleteBot`), which archives the Bot's memory first. Since [#1801](https://github.com/ThinkFlowLab/FlyBot/issues/1801) retirement also ends the reach a share gave: `archiveBot` deletes `state.grants[id]` and calls `rt.reachChanged(id)`, the way deletion clears sharing, so a friend's Bot granted the record no longer resolves it from outside and nothing is delivered to a Bot that will not run. Covered by `core/test/archive-bot.test.js` and `core/test/retired-bot-paths.test.js`. - A deleted Bot's Sessions keep their owner (#1453): `state.retiredSessions` maps a Session id to the Bot id that owned it, written by `roster.deleteBot` for the own chat and its parts and for each group Session and its parts before the Bot leaves the state, so `indexSessions` can put them in `pastOwners` again on the next rebuild. The record only fills what the live indexes do not claim — a live Bot or room holding the same id wins — and it only grows, one entry per Session of each Bot ever deleted, with no pruning in this change. `selfOf` then names a Bot that no longer exists, so `connector-access.js` `whoseAgent` answers `{ unknown: true }` and every connector is refused with `the Bot this Agent works for could not be determined`, in the process that deleted the Bot and after a reload, because the team file carries the record. The owner's own Session, an id no Bot ever had and a call with no Agent keep the connectors. The schedule tools are not covered by this change: they carry no connector id and are refused or allowed before that branch, and widening them is the #1237 decision. +- Deleting a Bot ends its room seats (#2607): `roster.deleteBot` closes the Bot's `participants` entry with `leftSeq`/`leftAt` and derives `room.members` through `syncMembers` in `room-membership.js`, the same rule a leave uses, so `participantsOf()`, a shared room's roster and `room.members` stop naming a Bot `state.bots` no longer holds and the next membership write cannot rebuild the id from a current seat; the save that follows keeps the closed seat. A room without a `participants` array keeps the shipped `members` filter. A deletion is not `removeRoomParticipant`: it never refuses on the two-Bot floor. Covered by `core/test/delete-bot-seat.test.js`. - Context sharing (#53) is specified in [team](team.md), **Context sharing foundation**: the own-chat brief is share-gated (`share-brief` / `briefShared`), a memory review reads only its owner as the user (`isUserLine`) and labels another human as `peer` (not `bot`), and keeps cross-owner entries out of the team scope (`teamAllowedFrom`), a retired Bot is never reviewed — neither a lagging group member is brought along nor a review queued before the archive runs, so its archived `MEMORY.md` is not rewritten and no model call is billed for it (#1786), `list_bots` lists only the caller's owner's Bots (a granted child: that child's `createdBy` subtree), Bots granted to the same owner, and Bots in the current room — another owner's folder and instructions stay private — and a `dsh-session:` chip in actor-authored `send` / `room-post` / `answer-question` / `schedule-create` text must resolve for the author (`sessionOwnedBy`; `schedule-create` authz keys off `body.target`). Bot-authored group posts and Bot-to-Bot / own-chat followups drop chips so they cannot tunnel a private chat into another Session. Host modules: `src/host/groups.js`, `src/host/memory.js`, `src/host/tools.js`, `src/host/channel.js`, `src/host/room-membership.js`, `src/host/authz.js`. Team mode stays disabled. diff --git a/patches/ds-bot/0002-flybot-api-host.patch b/patches/ds-bot/0002-flybot-api-host.patch index 3e6d18651..83a61cc4b 100644 --- a/patches/ds-bot/0002-flybot-api-host.patch +++ b/patches/ds-bot/0002-flybot-api-host.patch @@ -168,7 +168,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js +import { currentColor, cut, isColor, unquote } from './text.js' import { nameProblem } from '../shared/mentions.js' +import { cleanExternal } from './room-participant-external.js' -+import { implicitRoomOwner } from './room-membership.js' ++import { currentParticipants, implicitRoomOwner, syncMembers } from './room-membership.js' +import { ownEntry } from './store.js' export function install(rt) { @@ -688,7 +688,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js next = others.with(at, id) } else if (primary) { next = [id, ...others] -@@ -417,12 +417,43 @@ export function install(rt) { +@@ -417,14 +417,45 @@ export function install(rt) { } // The sidebar shows every Main Bot, so a hidden Bot would vanish again once demoted. if (main) delete bot.hidden @@ -698,9 +698,9 @@ diff --git a/src/host/roster.js b/src/host/roster.js - } state.mainBotIds = next + rt.syncDesignatedChief() -+ await rt.save() -+ } -+ + await rt.save() + } + + // #638: retire a Bot instead of deleting it. The tree already reads fields['flybot-tree'].retired: + // adoption skips the record and it no longer counts against maxChildren, while its chat, its parts + // and its memory stay where they are. `hidden` keeps it out of the default Bot list the way the @@ -731,12 +731,14 @@ diff --git a/src/host/roster.js b/src/host/roster.js + delete state.grants[id] + await rt.reachChanged?.(id) + } - await rt.save() ++ await rt.save() + return { bot, freed } - } - ++ } ++ async function deleteBot(id) { -@@ -434,22 +434,42 @@ export function install(rt) { + const bot = rt.botOf(id) + if (bot === undefined) throw new Error('Unknown Bot') +@@ -434,22 +434,54 @@ export function install(rt) { const current = rt.chatOf(id) // Questions and exchange turns are kept per Session, so every part of the chat is cleared. const parts = new Set([id, current, ...(bot.segments ?? []).map(segment => segment.sessionId)]) @@ -765,9 +767,24 @@ diff --git a/src/host/roster.js b/src/host/roster.js } const touched = [] for (const room of Object.values(state.rooms)) { - if (!room.members.includes(id)) continue - room.members = room.members.filter(member => member !== id) - if (room.admin === id) room.admin = null +- if (!room.members.includes(id)) continue +- room.members = room.members.filter(member => member !== id) +- if (room.admin === id) room.admin = null ++ const seat = currentParticipants(room).find(p => p.kind === 'bot' && !p.remote && p.id === id) ++ if (!room.members.includes(id) && seat === undefined) continue ++ // #2607: end the seat the way leaving does, then derive `members` from the seats. A separate ++ // filter leaves the participant current, and the next membership write writes the id back. ++ if (Array.isArray(room.participants)) { ++ if (seat !== undefined) { ++ seat.leftSeq = room.seq ?? 0 ++ seat.leftAt = Date.now() ++ } ++ if (room.admin === id) room.admin = null ++ syncMembers(room) ++ } else { ++ room.members = room.members.filter(member => member !== id) ++ if (room.admin === id) room.admin = null ++ } + if (room.taskLead === id) delete room.taskLead rt.dropGroupSession(room, id) touched.push(room) diff --git a/patches/ds-bot/files/src/host/room-membership.js b/patches/ds-bot/files/src/host/room-membership.js index e37c04f58..17d614ab1 100644 --- a/patches/ds-bot/files/src/host/room-membership.js +++ b/patches/ds-bot/files/src/host/room-membership.js @@ -66,6 +66,16 @@ export function currentParticipants(room) { return (room?.participants ?? []).filter(p => p.leftSeq === undefined) } +// The one derivation of `room.members` from the seats. `deleteBot` (roster.js) shares it so a +// deletion closes the seat and derives `members` in the same step (#2607): a separate filter +// leaves a current participant behind, and the next membership write rebuilds the deleted id. +export function syncMembers(room) { + room.members = currentParticipants(room).filter(p => p.kind === 'bot' && !p.remote).map(p => p.id) + const botAdmin = currentParticipants(room).find(p => p.kind === 'bot' && !p.remote && p.role === 'admin') + if (botAdmin) room.admin = botAdmin.id + else if (room.admin && !room.members.includes(room.admin)) room.admin = room.members[0] ?? null +} + export function isRemoteParticipant(p) { return p?.kind === 'bot' && p.remote === true } @@ -407,13 +417,6 @@ export function install(rt) { return stored } - const syncMembers = (room) => { - room.members = currentParticipants(room).filter(p => p.kind === 'bot' && !p.remote).map(p => p.id) - const botAdmin = currentParticipants(room).find(p => p.kind === 'bot' && !p.remote && p.role === 'admin') - if (botAdmin) room.admin = botAdmin.id - else if (room.admin && !room.members.includes(room.admin)) room.admin = room.members[0] ?? null - } - // #1987: a group without a chosen name is named after its seats, so a seat that joins or leaves moves // the name with it — and the group's own Session, the title the user reads, moves with the name. `room` // is passed for both the comparison and the write: `autoRoomName` reads the people and the remote guests diff --git a/patches/ds-bot/files/test/delete-bot-seat.test.js b/patches/ds-bot/files/test/delete-bot-seat.test.js new file mode 100644 index 000000000..0e05b0a9f --- /dev/null +++ b/patches/ds-bot/files/test/delete-bot-seat.test.js @@ -0,0 +1,139 @@ +import assert from 'node:assert/strict' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { 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' + +// #2607: deleting a Bot ends its seats in every room, the way leaving does. Before this the deletion +// only filtered the legacy `room.members` array, the room's `participants` entry stayed current, and +// the next membership write rebuilt `room.members` from that entry — the deleted id came back and the +// following save wrote it to disk. Two rooms keep the older shape without `participants`: that path +// must still drop the id from `members` and stay without a participant list. + +const live = room => (room.participants ?? []).filter(p => p.leftSeq === undefined).map(p => `${p.kind}:${p.id}`) + +const team = () => ({ + version: 2, + revision: 1, + mainBotIds: ['chief'], + members: { owner: { id: 'owner', createdAt: 1, designatedChiefId: 'chief' } }, + bots: { + chief: { id: 'chief', sessionId: 'chief', name: 'Chief', role: 'Lead', instructions: '', createdAt: 1, ownerId: 'owner' }, + writer: { id: 'writer', sessionId: 'writer', name: 'Writer', role: 'Write', instructions: '', createdAt: 2, ownerId: 'owner' }, + coder: { id: 'coder', sessionId: 'coder', name: 'Coder', role: 'Code', instructions: '', createdAt: 3, ownerId: 'owner' }, + }, + rooms: { + // The Bot being deleted is a plain member here. + desk: { + id: 'desk', + name: 'Desk', + members: ['chief', 'writer'], + admin: 'chief', + named: true, + log: [], + seq: 0, + seen: {}, + participants: [ + { kind: 'human', id: 'owner', role: 'admin', displayName: 'Owner', joinedSeq: 0, joinedAt: 1 }, + { kind: 'bot', id: 'chief', role: 'admin', displayName: 'Chief', joinedSeq: 0, joinedAt: 1 }, + { kind: 'bot', id: 'writer', role: 'member', displayName: 'Writer', joinedSeq: 0, joinedAt: 1 }, + ], + }, + // The deleted Bot is the room's only admin and its only Bot — `members` is empty afterwards. + solo: { + id: 'solo', + name: 'Solo', + members: ['writer'], + admin: 'writer', + named: true, + log: [], + seq: 0, + seen: {}, + participants: [ + { kind: 'human', id: 'owner', role: 'admin', displayName: 'Owner', joinedSeq: 0, joinedAt: 1 }, + { kind: 'bot', id: 'writer', role: 'admin', displayName: 'Writer', joinedSeq: 0, joinedAt: 1 }, + ], + }, + // No `participants` array: the shipped single-owner shape. + legacy: { id: 'legacy', name: 'Legacy', members: ['chief', 'writer'], admin: 'chief', mode: 'everyone' }, + }, + exchanges: [], + exchangeTurns: {}, + questions: {}, + revision: 1, +}) + +function context() { + const ctx = fakeContext() + Object.assign(ctx, { + sessions: { get: () => undefined, flush: async () => {} }, + workspaceRegistry: { get: () => undefined, resolveByPath: async () => undefined, create: async path => ({ id: 'w1', path }), archiveSession: async () => {} }, + sessionController: { create: async () => ({ sessionId: 'new1' }), resolveAgent: async id => ({ agent: { id, session: { id, header: {} } } }), selectModel: async () => {} }, + sessionTitle: { rename: async () => {} }, + llm: { listProviders: () => [] }, + }) + return ctx +} + +async function open(home) { + const ctx = context() + const rt = createRuntime(ctx, { home, github: false, composio: false, diagnosticsFile: false }) + await rt.load() + return { ctx, rt } +} + +async function setup(t) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-delete-seat-')) + writeFileSync(join(home, 'state.json'), JSON.stringify(team())) + const first = await open(home) + t.after(async () => { + await first.ctx.dispose() + rmSync(home, { recursive: true, force: true, maxRetries: 5 }) + }) + return { home, rt: first.rt, ctx: first.ctx } +} + +test('deleting a Bot closes its participant seat and the next membership write does not write it back (#2607)', async (t) => { + const { home, rt, ctx } = await setup(t) + await rt.deleteBot('writer') + assert.equal(rt.botOf('writer'), undefined, 'the Bot record is gone') + const desk = rt.state.rooms.desk + const seat = desk.participants.find(p => p.kind === 'bot' && p.id === 'writer') + assert.ok(seat.leftSeq !== undefined, 'the deleted Bot no longer holds a current seat') + assert.equal(typeof seat.leftAt, 'number', 'the seat records when it ended') + assert.deepEqual(live(desk), ['human:owner', 'bot:chief'], 'the participant list no longer answers the deleted Bot') + assert.deepEqual(rt.participantsOf(desk).map(p => `${p.kind}:${p.id}`), ['human:owner', 'bot:chief'], 'the drawer reads the same list') + assert.deepEqual(desk.members, ['chief'], 'members is derived from the seats, not filtered beside them') + + // One ordinary membership write is what used to resurrect the id through `syncMembers`. + await rt.addRoomParticipant('desk', { kind: 'bot', id: 'coder' }) + assert.deepEqual(desk.members, ['chief', 'coder'], 'the deleted id does not come back') + assert.deepEqual(live(desk), ['human:owner', 'bot:chief', 'bot:coder']) + + // The deletion and the write were saved, so a host that reads the same state.json must agree. + await ctx.dispose() + const second = await open(home) + t.after(async () => { await second.ctx.dispose() }) + const reread = second.rt.state.rooms.desk + assert.equal(second.rt.botOf('writer'), undefined) + assert.equal(live(reread).includes('bot:writer'), false, 'the saved seat stays closed') + assert.deepEqual(reread.members, ['chief', 'coder'], 'the saved members list does not carry the deleted id') +}) + +test('a deleted Bot that was the only admin, and a room left with no Bots, take the next membership write (#2607)', async (t) => { + const { rt } = await setup(t) + await rt.deleteBot('writer') + const solo = rt.state.rooms.solo + assert.deepEqual(solo.members, [], 'the room is left with no Bots') + assert.equal(solo.admin, null, 'the deleted Bot is not still the admin') + assert.deepEqual(live(solo), ['human:owner']) + await rt.addRoomParticipant('solo', { kind: 'bot', id: 'coder' }) + assert.deepEqual(solo.members, ['coder']) + assert.deepEqual(live(solo), ['human:owner', 'bot:coder']) + // The room without a participants array keeps the shipped shape. + const legacy = rt.state.rooms.legacy + assert.equal(legacy.participants, undefined, 'no participant list is invented for a legacy room') + assert.deepEqual(legacy.members, ['chief']) +})