Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/1453.fixed.md
Original file line number Diff line number Diff line change
@@ -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).
12 changes: 12 additions & 0 deletions core/src/host/roster.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
16 changes: 14 additions & 2 deletions core/src/host/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<string,any>, bots:Record<string,any>, rooms:Record<string,any>, agents:Record<string,any>, capabilities:{items:Record<string,any>}, exchanges:any[], exchangeTurns:Record<string,number[]>, revision:number, invitations:Record<string,any>, authSessions:Record<string,any>, tasks:Record<string,any>}} */
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<string,any>, bots:Record<string,any>, rooms:Record<string,any>, agents:Record<string,any>, capabilities:{items:Record<string,any>}, exchanges:any[], exchangeTurns:Record<string,number[]>, revision:number, invitations:Record<string,any>, authSessions:Record<string,any>, tasks:Record<string,any>, retiredSessions:Record<string,string>}} */
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
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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?.()
}
Expand Down
118 changes: 118 additions & 0 deletions core/test/retired-sessions.test.js
Original file line number Diff line number Diff line change
@@ -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)
}
})
1 change: 1 addition & 0 deletions docs/subsystems/core-host.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
39 changes: 35 additions & 4 deletions patches/ds-bot/0002-flybot-api-host.patch
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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<string,any>, rooms:Record<string,any>, agents:Record<string,any>, capabilities:{items:Record<string,any>}, exchanges:any[], exchangeTurns:Record<string,number[]>, 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<string,any>, bots:Record<string,any>, rooms:Record<string,any>, agents:Record<string,any>, capabilities:{items:Record<string,any>}, exchanges:any[], exchangeTurns:Record<string,number[]>, revision:number, invitations:Record<string,any>, authSessions:Record<string,any>, tasks:Record<string,any>}} */
+ 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<string,any>, bots:Record<string,any>, rooms:Record<string,any>, agents:Record<string,any>, capabilities:{items:Record<string,any>}, exchanges:any[], exchangeTurns:Record<string,number[]>, revision:number, invitations:Record<string,any>, authSessions:Record<string,any>, tasks:Record<string,any>, retiredSessions:Record<string,string>}} */
+ 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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down
Loading
Loading