From 942589a9c8d180ad20026eb4b9634999c124d844 Mon Sep 17 00:00:00 2001 From: Danyang Chen Date: Wed, 28 Oct 2026 01:26:38 +0800 Subject: [PATCH] fix(host): refuse the team tools to a deleted Bot's Sessions Replay of #1499 onto main cf72d485. Every changed file keeps the blob of the reviewed tip 37c5c645; only the parent moves to main. Written-by: Rework --- changelog.d/1453.fixed.md | 1 + core/src/host/roster.js | 12 ++ core/src/host/store.js | 16 ++- core/test/retired-sessions.test.js | 118 ++++++++++++++++++ docs/subsystems/core-host.md | 1 + patches/ds-bot/0002-flybot-api-host.patch | 39 +++++- .../files/test/retired-sessions.test.js | 118 ++++++++++++++++++ 7 files changed, 299 insertions(+), 6 deletions(-) create mode 100644 changelog.d/1453.fixed.md create mode 100644 core/test/retired-sessions.test.js create mode 100644 patches/ds-bot/files/test/retired-sessions.test.js diff --git a/changelog.d/1453.fixed.md b/changelog.d/1453.fixed.md new file mode 100644 index 000000000..25d4d95b0 --- /dev/null +++ b/changelog.d/1453.fixed.md @@ -0,0 +1 @@ +Deleting a Bot now takes its connector tools away from any Session that deletion emptied — its own chat, its earlier parts, its group Session and the parts of that Session — so a tool call already in flight is refused with `the Bot this Agent works for could not be determined` instead of running on the owner's connectors; the owner's own Sessions, an id no Bot ever had, and a live Bot or room that holds one of those ids are unchanged, and the record is in the team file so a reload still refuses (#1453). diff --git a/core/src/host/roster.js b/core/src/host/roster.js index 8da4441e6..be9022138 100644 --- a/core/src/host/roster.js +++ b/core/src/host/roster.js @@ -517,6 +517,18 @@ 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)]) + // #1453: keep who owned every Session this deletion empties — the own chat and its parts, the + // group Session and its parts — before the Bot and its seats leave the state, so an in-flight + // call fails closed instead of keeping the owner's connectors. The record only grows: nothing + // here prunes an older entry. + state.retiredSessions ??= {} + for (const part of parts) state.retiredSessions[part] = id + for (const room of Object.values(state.rooms)) { + if (!room.members.includes(id)) continue + const sessionId = room.sessions?.[id] + if (sessionId !== undefined) state.retiredSessions[sessionId] = id + for (const segment of room.segments?.[id] ?? []) state.retiredSessions[segment.sessionId] = id + } delete state.bots[id] delete state.remoteContacts?.[id] for (const part of parts) { diff --git a/core/src/host/store.js b/core/src/host/store.js index 759179f93..81b690a61 100644 --- a/core/src/host/store.js +++ b/core/src/host/store.js @@ -21,8 +21,8 @@ const isRecord = value => value !== null && typeof value === 'object' && !Array. export function install(rt) { const { ctx, home, statePath, config = {} } = rt - /** @type {{version:2, workspaceId?:string, mainBotIds:string[], members:Record, bots:Record, rooms:Record, agents:Record, capabilities:{items:Record}, exchanges:any[], exchangeTurns:Record, revision:number, invitations:Record, authSessions:Record, tasks:Record}} */ - const state = { version: STATE_VERSION, mainBotIds: [], members: {}, bots: {}, rooms: {}, agents: {}, capabilities: { items: {} }, exchanges: [], exchangeTurns: {}, questions: {}, prefs: {}, invitations: {}, authSessions: {}, tasks: {}, revision: 0 } + /** @type {{version:2, workspaceId?:string, mainBotIds:string[], members:Record, bots:Record, rooms:Record, agents:Record, capabilities:{items:Record}, exchanges:any[], exchangeTurns:Record, revision:number, invitations:Record, authSessions:Record, tasks:Record, retiredSessions:Record}} */ + const state = { version: STATE_VERSION, mainBotIds: [], members: {}, bots: {}, rooms: {}, agents: {}, capabilities: { items: {} }, exchanges: [], exchangeTurns: {}, questions: {}, prefs: {}, invitations: {}, authSessions: {}, tasks: {}, retiredSessions: {}, revision: 0 } let loaded = null let writing = Promise.resolve() // Every save this host has accepted since it started — a monotonic watermark, never decremented @@ -315,6 +315,11 @@ export function install(rt) { state.tasks = {} migrated = true } + // #1453: who owned the Sessions a deleted Bot left behind; a corrupt value resets. + if (!isRecord(state.retiredSessions)) { + state.retiredSessions = {} + migrated = true + } if (ensureOwner()) migrated = true if (legacy) { const ownerId = Object.keys(state.members)[0] @@ -569,6 +574,13 @@ export function install(rt) { } for (const [botId, sessionId] of Object.entries(room.sessions ?? {})) groupOwners.set(sessionId, { botId, roomId: room.id }) } + // #1453: a deleted Bot's Sessions keep the Bot that owned them, so a call still in flight is + // refused instead of being read as a plain Session. The record only fills what the live + // indexes do not claim: a live Bot or room holding the same id wins. + for (const [sessionId, botId] of Object.entries(state.retiredSessions ?? {})) { + if (typeof botId !== 'string' || chatOwners.has(sessionId) || groupOwners.has(sessionId) || pastOwners.has(sessionId)) continue + pastOwners.set(sessionId, { botId }) + } // The agents module owns the DSH Agent session index; it installs after this. rt.indexAgents?.() } diff --git a/core/test/retired-sessions.test.js b/core/test/retired-sessions.test.js new file mode 100644 index 000000000..308258240 --- /dev/null +++ b/core/test/retired-sessions.test.js @@ -0,0 +1,118 @@ +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' + +// #1453: deleting a Bot is how its owner takes the team tools away from it. The Sessions that +// the deletion empties — its own chat, its earlier parts, its group Session, and the parts of +// that group Session — must be refused every connector afterwards, also for a call already in +// flight; the refusal is decided by the team file, so a reload keeps it. A Session that never +// belonged to a Bot (the owner's own, an unknown id, no Agent at all) keeps the tools, and a +// live Bot or room holding one of those ids wins over the record of the deleted one. + +const COMPOSIO = ['mcp__composio__COMPOSIO_SEARCH_TOOLS', 'mcp__composio__COMPOSIO_MULTI_EXECUTE_TOOL'] +const GITHUB = ['mcp__github__create_issue', 'mcp__github__get_me'] +const ALL = [...COMPOSIO, ...GITHUB] +const REFUSED = /could not be determined/ +const RETIRED = ['writer', 'writer-2', 'room-writer', 'room-writer-old'] + +// chief (Main Bot); writer with the current own chat 'writer-2' and the earlier part 'writer'; +// room 'room' with the writer's group Session 'room-writer' and its earlier part +// 'room-writer-old'. +const team = () => ({ + version: 1, + mainBotIds: ['chief'], + bots: { + chief: { id: 'chief', sessionId: 'chief', name: 'Chief', role: 'Lead', instructions: '', createdAt: 1 }, + writer: { id: 'writer', sessionId: 'writer-2', name: 'Writer', role: 'Write', instructions: '', createdAt: 2, segments: [{ sessionId: 'writer' }] }, + }, + rooms: { + room: { id: 'room', name: 'Desk', members: ['chief', 'writer'], sessions: { writer: 'room-writer' }, segments: { writer: [{ sessionId: 'room-writer-old' }] }, 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() { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-retired-sessions-')) + writeFileSync(join(home, 'state.json'), JSON.stringify(team())) + const first = await open(home) + return { ...first, home } +} + +// What the installed connector guard answers: undefined = allowed, a sentence = refused. +const reason = (guards, name, id) => guards.map(check => check({ name, agent: { id, session: { id, header: {} } } })).find(answer => answer !== undefined) + +const refusal = (guards, id) => ALL.map(name => reason(guards, name, id)) + +test('a deleted Bot\'s own chat, its parts, its group Session and that Session\'s parts lose every connector, and the refusal survives a reload (#1453)', async (t) => { + const { ctx, rt, home } = await setup() + t.after(async () => { await ctx.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) }) + await rt.deleteBot('writer') + // Before the fix every one of these is undefined, so the guard reads the deleted Bot's + // Session as a plain one and the in-flight call keeps the owner's connectors. + for (const id of RETIRED) { + for (const answer of refusal(ctx.guards, id)) assert.match(answer, REFUSED, id) + } + // The team file keeps who owned them: one entry per Session the deletion emptied, for a reload to use. + assert.deepEqual(rt.state.retiredSessions, Object.fromEntries(RETIRED.map(id => [id, 'writer']))) + // The refusal is the team file's, not this process's: a host that reads the saved team again refuses too. + await ctx.dispose() + const second = await open(home) + t.after(async () => { await second.ctx.dispose() }) + for (const id of RETIRED) { + for (const answer of refusal(second.ctx.guards, id)) assert.match(answer, REFUSED, id) + } +}) + +test('the owner\'s own Session, an unknown id, no Agent, and a live Bot or room holding a retired id keep the connectors (#1453)', async (t) => { + const { ctx, rt, home } = await setup() + t.after(async () => { await ctx.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) }) + await rt.deleteBot('writer') + // This narrows the team; it must not close the owner out of their own Sessions. + for (const id of ['plain', 'never-a-bot']) { + for (const answer of refusal(ctx.guards, id)) assert.equal(answer, undefined, id) + } + for (const name of ALL) assert.equal(reason(ctx.guards, name, undefined), undefined, name) + // A live Bot or room that holds one of those ids wins over the record of the deleted Bot. + const ownerId = Object.keys(rt.state.members)[0] + rt.state.bots.reader = { id: 'reader', sessionId: 'writer-2', name: 'Reader', role: 'Read', instructions: '', createdAt: 3, ownerId } + rt.state.rooms.desk2 = { id: 'desk2', name: 'Desk 2', members: ['chief', 'reader'], sessions: { chief: 'room-writer' }, admin: 'chief', mode: 'everyone' } + await rt.save() + for (const answer of [...GITHUB].map(name => reason(ctx.guards, name, 'writer-2'))) assert.equal(answer, undefined, 'the live Bot\'s own chat') + for (const answer of GITHUB.map(name => reason(ctx.guards, name, 'room-writer'))) assert.equal(answer, undefined, 'the live room\'s Session') + // The room rule (#1237) still refuses composio there, and it is that sentence — not this fix's. + for (const answer of COMPOSIO.map(name => reason(ctx.guards, name, 'room-writer'))) assert.doesNotMatch(answer, REFUSED) + await ctx.dispose() + const second = await open(home) + t.after(async () => { await second.ctx.dispose() }) + assert.deepEqual(GITHUB.map(name => reason(second.ctx.guards, name, 'writer-2')).filter(answer => answer !== undefined), [], 'the live Bot still wins after a reload') + assert.deepEqual(GITHUB.map(name => reason(second.ctx.guards, name, 'room-writer')).filter(answer => answer !== undefined), [], 'the live room still wins after a reload') + for (const id of RETIRED.filter(id => id !== 'writer-2' && id !== 'room-writer')) { + for (const answer of refusal(second.ctx.guards, id)) assert.match(answer, REFUSED, id) + } +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 1e0c5f664..cf4fe2e6f 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -212,6 +212,7 @@ An admin's `@` used to be the room's control plane: every `@Dev #1234 rebase and - `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. +- 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. - 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`), `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 e6eebd61b..b8ac416ce 100644 --- a/patches/ds-bot/0002-flybot-api-host.patch +++ b/patches/ds-bot/0002-flybot-api-host.patch @@ -522,9 +522,21 @@ diff --git a/src/host/roster.js b/src/host/roster.js await rt.save() } -@@ -435,21 +435,29 @@ export function install(rt) { +@@ -435,21 +435,41 @@ export function install(rt) { // 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)]) ++ // #1453: keep who owned every Session this deletion empties — the own chat and its parts, the ++ // group Session and its parts — before the Bot and its seats leave the state, so an in-flight ++ // call fails closed instead of keeping the owner's connectors. The record only grows: nothing ++ // here prunes an older entry. ++ state.retiredSessions ??= {} ++ for (const part of parts) state.retiredSessions[part] = id ++ for (const room of Object.values(state.rooms)) { ++ if (!room.members.includes(id)) continue ++ const sessionId = room.sessions?.[id] ++ if (sessionId !== undefined) state.retiredSessions[sessionId] = id ++ for (const segment of room.segments?.[id] ?? []) state.retiredSessions[segment.sessionId] = id ++ } delete state.bots[id] + delete state.remoteContacts?.[id] for (const part of parts) { @@ -602,8 +614,8 @@ diff --git a/src/host/store.js b/src/host/store.js - /** @type {{version:1, workspaceId?:string, mainBotIds:string[], bots:Record, rooms:Record, agents:Record, capabilities:{items:Record}, exchanges:any[], exchangeTurns:Record, revision:number}} */ - const state = { version: 1, mainBotIds: [], bots: {}, rooms: {}, agents: {}, capabilities: { items: {} }, exchanges: [], exchangeTurns: {}, questions: {}, prefs: {}, revision: 0 } -+ /** @type {{version:2, workspaceId?:string, mainBotIds:string[], members:Record, bots:Record, rooms:Record, agents:Record, capabilities:{items:Record}, exchanges:any[], exchangeTurns:Record, revision:number, invitations:Record, authSessions:Record, tasks:Record}} */ -+ const state = { version: STATE_VERSION, mainBotIds: [], members: {}, bots: {}, rooms: {}, agents: {}, capabilities: { items: {} }, exchanges: [], exchangeTurns: {}, questions: {}, prefs: {}, invitations: {}, authSessions: {}, tasks: {}, revision: 0 } ++ /** @type {{version:2, workspaceId?:string, mainBotIds:string[], members:Record, bots:Record, rooms:Record, agents:Record, capabilities:{items:Record}, exchanges:any[], exchangeTurns:Record, revision:number, invitations:Record, authSessions:Record, tasks:Record, retiredSessions:Record}} */ ++ const state = { version: STATE_VERSION, mainBotIds: [], members: {}, bots: {}, rooms: {}, agents: {}, capabilities: { items: {} }, exchanges: [], exchangeTurns: {}, questions: {}, prefs: {}, invitations: {}, authSessions: {}, tasks: {}, retiredSessions: {}, revision: 0 } let loaded = null let writing = Promise.resolve() + // Every save this host has accepted since it started — a monotonic watermark, never decremented @@ -763,7 +775,7 @@ diff --git a/src/host/store.js b/src/host/store.js // LEGACY: Bots saved with a lab ran on that lab's default model; the model they // last ran on becomes their own, so dropping the lab changes nothing for them. if ('lab' in bot) { -@@ -161,12 +161,84 @@ export function install(rt) { +@@ -161,12 +161,89 @@ export function install(rt) { migrated = true } // Unnamed groups from before names followed their members may still list old names. @@ -799,6 +811,11 @@ diff --git a/src/host/store.js b/src/host/store.js + state.tasks = {} + migrated = true + } ++ // #1453: who owned the Sessions a deleted Bot left behind; a corrupt value resets. ++ if (!isRecord(state.retiredSessions)) { ++ state.retiredSessions = {} ++ migrated = true ++ } + if (ensureOwner()) migrated = true + if (legacy) { + const ownerId = Object.keys(state.members)[0] @@ -956,6 +973,20 @@ diff --git a/src/host/store.js b/src/host/store.js ctx.effect(() => () => { stopped = true return writing.finally(() => lock.release()) +@@ -313,6 +313,13 @@ export function install(rt) { + } + for (const [botId, sessionId] of Object.entries(room.sessions ?? {})) groupOwners.set(sessionId, { botId, roomId: room.id }) + } ++ // #1453: a deleted Bot's Sessions keep the Bot that owned them, so a call still in flight is ++ // refused instead of being read as a plain Session. The record only fills what the live ++ // indexes do not claim: a live Bot or room holding the same id wins. ++ for (const [sessionId, botId] of Object.entries(state.retiredSessions ?? {})) { ++ if (typeof botId !== 'string' || chatOwners.has(sessionId) || groupOwners.has(sessionId) || pastOwners.has(sessionId)) continue ++ pastOwners.set(sessionId, { botId }) ++ } + // The agents module owns the DSH Agent session index; it installs after this. + rt.indexAgents?.() + } @@ -328,6 +328,8 @@ export function install(rt) { delete room.sessions[botId] if (room.segments !== undefined) delete room.segments[botId] diff --git a/patches/ds-bot/files/test/retired-sessions.test.js b/patches/ds-bot/files/test/retired-sessions.test.js new file mode 100644 index 000000000..308258240 --- /dev/null +++ b/patches/ds-bot/files/test/retired-sessions.test.js @@ -0,0 +1,118 @@ +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' + +// #1453: deleting a Bot is how its owner takes the team tools away from it. The Sessions that +// the deletion empties — its own chat, its earlier parts, its group Session, and the parts of +// that group Session — must be refused every connector afterwards, also for a call already in +// flight; the refusal is decided by the team file, so a reload keeps it. A Session that never +// belonged to a Bot (the owner's own, an unknown id, no Agent at all) keeps the tools, and a +// live Bot or room holding one of those ids wins over the record of the deleted one. + +const COMPOSIO = ['mcp__composio__COMPOSIO_SEARCH_TOOLS', 'mcp__composio__COMPOSIO_MULTI_EXECUTE_TOOL'] +const GITHUB = ['mcp__github__create_issue', 'mcp__github__get_me'] +const ALL = [...COMPOSIO, ...GITHUB] +const REFUSED = /could not be determined/ +const RETIRED = ['writer', 'writer-2', 'room-writer', 'room-writer-old'] + +// chief (Main Bot); writer with the current own chat 'writer-2' and the earlier part 'writer'; +// room 'room' with the writer's group Session 'room-writer' and its earlier part +// 'room-writer-old'. +const team = () => ({ + version: 1, + mainBotIds: ['chief'], + bots: { + chief: { id: 'chief', sessionId: 'chief', name: 'Chief', role: 'Lead', instructions: '', createdAt: 1 }, + writer: { id: 'writer', sessionId: 'writer-2', name: 'Writer', role: 'Write', instructions: '', createdAt: 2, segments: [{ sessionId: 'writer' }] }, + }, + rooms: { + room: { id: 'room', name: 'Desk', members: ['chief', 'writer'], sessions: { writer: 'room-writer' }, segments: { writer: [{ sessionId: 'room-writer-old' }] }, 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() { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-retired-sessions-')) + writeFileSync(join(home, 'state.json'), JSON.stringify(team())) + const first = await open(home) + return { ...first, home } +} + +// What the installed connector guard answers: undefined = allowed, a sentence = refused. +const reason = (guards, name, id) => guards.map(check => check({ name, agent: { id, session: { id, header: {} } } })).find(answer => answer !== undefined) + +const refusal = (guards, id) => ALL.map(name => reason(guards, name, id)) + +test('a deleted Bot\'s own chat, its parts, its group Session and that Session\'s parts lose every connector, and the refusal survives a reload (#1453)', async (t) => { + const { ctx, rt, home } = await setup() + t.after(async () => { await ctx.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) }) + await rt.deleteBot('writer') + // Before the fix every one of these is undefined, so the guard reads the deleted Bot's + // Session as a plain one and the in-flight call keeps the owner's connectors. + for (const id of RETIRED) { + for (const answer of refusal(ctx.guards, id)) assert.match(answer, REFUSED, id) + } + // The team file keeps who owned them: one entry per Session the deletion emptied, for a reload to use. + assert.deepEqual(rt.state.retiredSessions, Object.fromEntries(RETIRED.map(id => [id, 'writer']))) + // The refusal is the team file's, not this process's: a host that reads the saved team again refuses too. + await ctx.dispose() + const second = await open(home) + t.after(async () => { await second.ctx.dispose() }) + for (const id of RETIRED) { + for (const answer of refusal(second.ctx.guards, id)) assert.match(answer, REFUSED, id) + } +}) + +test('the owner\'s own Session, an unknown id, no Agent, and a live Bot or room holding a retired id keep the connectors (#1453)', async (t) => { + const { ctx, rt, home } = await setup() + t.after(async () => { await ctx.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) }) + await rt.deleteBot('writer') + // This narrows the team; it must not close the owner out of their own Sessions. + for (const id of ['plain', 'never-a-bot']) { + for (const answer of refusal(ctx.guards, id)) assert.equal(answer, undefined, id) + } + for (const name of ALL) assert.equal(reason(ctx.guards, name, undefined), undefined, name) + // A live Bot or room that holds one of those ids wins over the record of the deleted Bot. + const ownerId = Object.keys(rt.state.members)[0] + rt.state.bots.reader = { id: 'reader', sessionId: 'writer-2', name: 'Reader', role: 'Read', instructions: '', createdAt: 3, ownerId } + rt.state.rooms.desk2 = { id: 'desk2', name: 'Desk 2', members: ['chief', 'reader'], sessions: { chief: 'room-writer' }, admin: 'chief', mode: 'everyone' } + await rt.save() + for (const answer of [...GITHUB].map(name => reason(ctx.guards, name, 'writer-2'))) assert.equal(answer, undefined, 'the live Bot\'s own chat') + for (const answer of GITHUB.map(name => reason(ctx.guards, name, 'room-writer'))) assert.equal(answer, undefined, 'the live room\'s Session') + // The room rule (#1237) still refuses composio there, and it is that sentence — not this fix's. + for (const answer of COMPOSIO.map(name => reason(ctx.guards, name, 'room-writer'))) assert.doesNotMatch(answer, REFUSED) + await ctx.dispose() + const second = await open(home) + t.after(async () => { await second.ctx.dispose() }) + assert.deepEqual(GITHUB.map(name => reason(second.ctx.guards, name, 'writer-2')).filter(answer => answer !== undefined), [], 'the live Bot still wins after a reload') + assert.deepEqual(GITHUB.map(name => reason(second.ctx.guards, name, 'room-writer')).filter(answer => answer !== undefined), [], 'the live room still wins after a reload') + for (const id of RETIRED.filter(id => id !== 'writer-2' && id !== 'room-writer')) { + for (const answer of refusal(second.ctx.guards, id)) assert.match(answer, REFUSED, id) + } +})