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/550-group-questions.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Group chats: when a Bot works on something handed out in a group and needs to ask you a question, the question card now shows in that group instead of in the Bot's private chat. Answer it there and the Bot carries on with the work, reporting back in the group; the Bot's private chat stays untouched ([#550](https://github.com/ThinkFlowLab/FlyBot/issues/550)).
10 changes: 9 additions & 1 deletion core/src/host/authz.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ export function install(rt) {
return false
}

// A card a Bot asks in a group about work handed over from there (#550) is its owner's to see
// and answer, like that Bot's approvals; the room's owner may too. Other members get no card.
const questionFor = (actor, sessionId, question) => question?.botId === undefined
|| botOwnedBy(actor, question.botId) || roomOwnedBy(actor, sessionId)

rt.botOwnedBy = botOwnedBy
rt.roomOwnedBy = roomOwnedBy
rt.roomMemberOf = roomMemberOf
Expand Down Expand Up @@ -162,7 +167,7 @@ export function install(rt) {
bots: full.bots.filter(bot => bot.ownerId === actor.memberId || sharedBots.has(bot.id)),
rooms,
self: { memberId: actor.memberId, displayName: actor.displayName },
questions: bySession(full.questions),
questions: Object.fromEntries(Object.entries(bySession(full.questions)).filter(([id, question]) => questionFor(actor, id, question))),
exchangeTurns: bySession(full.exchangeTurns),
remoteTypedTurns: bySession(full.remoteTypedTurns),
remoteAwaiting: bySession(full.remoteAwaiting),
Expand Down Expand Up @@ -347,6 +352,9 @@ export function install(rt) {
if (String(body?.choice ?? '').startsWith('always')) return botOwnedBy(actor, approval.botId) ? ALLOW : deny('bot/forbidden', 'Only the Bot\'s owner can always allow')
return botOwnedBy(actor, approval.botId) || roomOwnedBy(actor, approval.roomId) ? ALLOW : deny('bot/forbidden', 'You do not own that approval')
}
if ((ep === 'answer-question' || ep === 'dismiss-question') && !questionFor(actor, String(body?.sessionId ?? ''), state.questions?.[String(body?.sessionId ?? '')])) {
return deny('bot/forbidden', 'This question is not yours to answer')
}
return requireSession(body?.sessionId)
}

Expand Down
7 changes: 7 additions & 0 deletions core/src/host/channel.js
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,13 @@ export function install(rt) {
const sessionId = rt.botOf(body.sessionId) ? rt.chatOf(body.sessionId) : past && past.roomId === undefined ? rt.chatOf(past.botId) : body.sessionId
const question = state.questions[sessionId]
if (question === undefined || question.id !== body.questionId) return fail('This question is no longer open', 'bot/stale')
// A card in a group about work handed over from there (#550) answers the Bot that asked.
if (question.botId !== undefined && rt.roomOf(sessionId) !== undefined) {
const leaked = unshareableReference(actor, String(body.answer?.custom ?? ''))
if (leaked !== undefined) return fail('You do not own that chat', 'bot/forbidden')
const answered = await rt.answerGroupQuestion(sessionId, question, body.answer, actor)
return answered.ok ? ok(true) : fail(answered.error, answered.code)
}
const reply = questionReply(question, body.answer)
if (reply === undefined) return fail('Nothing to send', 'bot/invalid')
const leaked = unshareableReference(actor, reply.text)
Expand Down
10 changes: 6 additions & 4 deletions core/src/host/delivery.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ export function install(rt) {
return resolved.agent
}

const headerFor = (role, fromName, roomName, audience, note = 'new message', deferred = false, { resumed = false, retried = false } = {}) => {
const headerFor = (role, fromName, roomName, audience, note = 'new message', deferred = false, { resumed = false, retried = false, answeredIn } = {}) => {
if (answeredIn !== undefined) return `[The user answered your question card in the group "${answeredIn}" · your reply goes back to ${fromName} there]`
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]'
Expand Down Expand Up @@ -126,8 +127,9 @@ export function install(rt) {
// text (resume.js, turns.js), and `resent` is a message a shutdown cancelled; their
// exchange is recorded already too. `askedBy` is the member whose @mention called a
// group turn (groups.js). `fromRoomId`: the group whose Session the sender wrote
// from; the reply goes back to that Session (turns.js).
async function deliver({ fromId, toId, sessionId, text, role, hop = 0, replyTo, roomId, audience, note, exchangeId = randomUUID(), steer = false, deferred = false, resumed = false, retried = false, resent = false, askedBy, fromRoomId, fromName: givenName, untrusted = false, remote, attachments, origin: givenOrigin }) {
// from; the reply goes back to that Session (turns.js). `answeredIn`: the group where the user
// answered the Bot's question card about work handed over from there (tools.js, #550).
async function deliver({ fromId, toId, sessionId, text, role, hop = 0, replyTo, roomId, audience, note, exchangeId = randomUUID(), steer = false, deferred = false, resumed = false, retried = false, resent = false, askedBy, fromRoomId, answeredIn, fromName: givenName, untrusted = false, remote, attachments, origin: givenOrigin }) {
const from = rt.botOf(fromId)
const to = rt.botOf(toId)
if (to === undefined) throw new Error(`Unknown Bot ${toId}`)
Expand Down Expand Up @@ -174,7 +176,7 @@ export function install(rt) {
// queues for the next turn (see turns.js for how each is routed).
const body = fromId ? stripSessionChips(text) : text
const message = createUserMessage({
content: [{ type: 'text', text: `${headerFor(role, fromName, room?.name, audience, note, deferred, { resumed, retried })}\n${untrusted ? fenceUntrusted(fromName, body) : body}` }],
content: [{ type: 'text', text: `${headerFor(role, fromName, room?.name, audience, note, deferred, { resumed, retried, answeredIn })}\n${untrusted ? fenceUntrusted(fromName, body) : body}` }],
source,
})
if (steer) agent.steer(message)
Expand Down
5 changes: 5 additions & 0 deletions core/src/host/roster.js
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,7 @@ export function install(rt) {
if (room === undefined) throw new Error('Unknown group chat')
for (const botId of Object.keys(room.sessions ?? {})) rt.dropGroupSession(room, botId)
delete state.rooms[id]
delete state.questions[id]
rt.roomInbox.delete(id)
const failed = await saveForDelete()
// Armed scheduled tasks refuse a plain archive and would keep delivering into
Expand Down Expand Up @@ -532,6 +533,10 @@ export function install(rt) {
delete state.exchangeTurns[part]
delete state.remoteTypedTurns?.[part]
}
// A card it asked in a group (#550) has nobody left to answer.
for (const [key, question] of Object.entries(state.questions)) {
if (question?.botId === id) delete state.questions[key]
}
const touched = []
for (const room of Object.values(state.rooms)) {
if (!room.members.includes(id)) continue
Expand Down
88 changes: 88 additions & 0 deletions core/src/host/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import { COLORS, EXTRA_COLORS, MAX_HOPS, MAX_UNREAD_REQUESTS, ROOM_BOT_ROUNDS, R
import { unquote } from './text.js'
import { isListedRemoteChat } from '../shared/remote-label.js'
import { currentParticipants } from './room-membership.js'
import { questionReply, replyText } from './parts.js'
import { OTHER, settled } from './turn-origin.js'
import { cleanName } from './delivery.js'

export const output = { schema: { type: 'string' }, render: (_args, value) => [{ type: 'text', text: String(value) }] }
export const GROUP_REF = { type: 'string', required: true, description: 'Exact group chat name from list_bots' }
Expand Down Expand Up @@ -524,6 +527,8 @@ export function install(rt) {
.filter(option => option.label !== '')
.slice(0, 6)
if (options.length === 0) return 'Give at least one option.'
const handed = handedFromGroup(id)
if (handed !== undefined) return askInGroup(handed, args, options, exec)
// A hidden Bot-to-Bot turn shows none of its calls, so its card goes to the
// composer dock, which takes questions without a call.
const turn = rt.live.get(id)?.turn
Expand All @@ -544,6 +549,89 @@ export function install(rt) {
},
}))

// Work a group member hands a Bot with message_bot runs in a hidden turn of the Bot's own chat,
// and its reply goes back to that member's Session in the group (turns.js). The user handed the
// work out there, so a question about it is asked there too, never in the Bot's own chat (#550).
const handedFromGroup = (sessionId) => {
if (!rt.chatOwners.has(sessionId)) return undefined
const info = rt.live.get(sessionId)?.info
if (info?.role !== 'request' || info.audience === 'user' || info.remote !== undefined) return undefined
const room = rt.roomOf(info.senderRoomId)
if (room?.sessions?.[info.senderSessionId] === undefined) return undefined
return { room, info }
}

// The card sits in the group's composer dock, named for the Bot that asks. One card per group:
// while another Bot's waits there, this one asks in its reply, which goes back to the group.
async function askInGroup({ room, info }, args, options, exec) {
const id = exec.agent?.id
const bot = rt.botOf(rt.selfOf(id))
const open = state.questions[room.id]
if (open !== undefined && open.botId !== bot?.id) {
return `Another question is waiting in the group "${room.name}". Ask in your reply instead; it goes back to the group.`
}
state.questions[room.id] = {
id: randomUUID(),
header: bot?.name ?? 'Bot',
question: String(args.question ?? '').trim(),
...(args.detail ? { detail: String(args.detail).trim() } : {}),
options,
multiSelect: args.multiSelect === true,
// The group's card always takes an answer in the user's own words: a line typed in the
// group goes to the group's round, not to this Bot.
allowCustom: true,
askedAt: Date.now(),
botId: bot?.id,
replyTo: { botId: info.senderSessionId, hop: info.hop ?? 0 },
// The answer carries on this work with the authority it had (turn-origin.js).
origin: settled(rt, id) ?? OTHER,
}
await rt.save()
exec.concludeTurn?.()
return `The card is on screen in the group "${room.name}", where this work came from; the user's answer arrives as your next message.`
}

// The answer to such a card reaches the Bot the way the work did: a hidden request from the
// member that handed it over, so the Bot's reply goes back to that member's Session in the group
// and nothing new shows in the Bot's own chat. The group sees the answer as a quiet notice.
async function answerGroupQuestion(roomId, question, answer, actor) {
const reply = questionReply(question, answer)
if (reply === undefined) return { error: 'Nothing to send', code: 'bot/invalid' }
const room = rt.roomOf(roomId)
const bot = rt.botOf(question.botId)
delete state.questions[roomId]
if (room === undefined || bot === undefined) {
await rt.save()
return { error: 'This question is no longer open', code: 'bot/stale' }
}
// Someone other than the Bot's owner answering (a room owner) does not lend the owner's authority.
const byOwner = actor === undefined || actor === null || !rt.authzEnforced?.() || actor.memberId === bot.ownerId
const text = replyText(reply.text)
try {
await rt.deliver({
fromId: question.replyTo?.botId,
toId: bot.id,
role: 'request',
hop: question.replyTo?.hop ?? 0,
fromRoomId: room.id,
answeredIn: room.name,
origin: byOwner ? question.origin : OTHER,
text,
})
} catch (error) {
state.questions[roomId] ??= question
throw error
}
await rt.save()
const who = cleanName(actor?.displayName, 'You')
const given = JSON.parse(reply.text).answers[0]
const values = [...given.selected, ...(given.custom ? [given.custom] : [])].join(', ').replace(/\s+/g, ' ').slice(0, 300)
void rt.postToRoom?.(room, { id: 'face-notice', name: 'Room' }, `${who} answered ${bot.name}: ${question.question} → ${values}`, { quiet: true, notice: true })
.catch(error => rt.warn('ds-bot: the answer notice in %s was not shown: %s', room.name, error))
return { ok: true }
}
rt.answerGroupQuestion = answerGroupQuestion

// dsh's own question tool holds the turn open until the user answers, and every
// message from another Bot or a group round queues behind it. Bot Sessions ask
// through ask_user instead, so the tool is taken out of their tool list.
Expand Down
5 changes: 3 additions & 2 deletions core/src/host/turns.js
Original file line number Diff line number Diff line change
Expand Up @@ -461,8 +461,9 @@ export function install(rt) {
}
const own = rt.chatOwners.get(id)
if (own !== undefined) rt.dropKickoff(own)
// Any user message answers or supersedes the open question card.
if (state.questions[id] !== undefined) {
// Any user message answers or supersedes the open question card. A group's card waits for
// its own answer: a line typed in the group goes to its round, not to the Bot that asked (#550).
if (state.questions[id] !== undefined && !rt.roomOf(id)) {
delete state.questions[id]
void rt.save().catch(() => {})
}
Expand Down
27 changes: 27 additions & 0 deletions core/test/authz.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -382,3 +382,30 @@ test('views keep the sole owner whole and narrow per owner with two members', as
assert.deepEqual(live2.connectors, [])
assert.equal(live2.memoryReviews, 0)
})

test('a group\'s question card is its Bot owner\'s to see and answer, not every member\'s (#550)', async (t) => {
const { rt } = runtime(t, TEAM)
await rt.load()
const sessionA = await redeemOwner(rt)
const sessionB = await redeemOwner(rt)
rt.state.bots.a1 = { id: 'a1', sessionId: 'a1', name: 'A', instructions: '', createdAt: 2, ownerId: 'owner-1' }
rt.state.bots.b1 = { id: 'b1', sessionId: 'b1', name: 'B', instructions: '', createdAt: 3, ownerId: 'owner-2' }
rt.state.rooms.rx = {
id: 'rx', name: 'Shared', members: ['a1', 'b1'], admin: 'a1', sessions: { a1: 'gs-a1', b1: 'gs-b1' },
participants: [
{ kind: 'human', id: 'owner-1', role: 'admin', joinedSeq: 0 },
{ kind: 'human', id: 'owner-2', role: 'member', joinedSeq: 0 },
],
}
rt.state.questions.rx = { id: 'q1', question: 'Which branch?', options: [{ label: 'dev' }], botId: 'a1' }
await rt.save()
const actorA = await rt.authenticate(sessionA.sessionToken)
const actorB = await rt.authenticate(sessionB.sessionToken)
assert.equal(rt.authorize(actorB, 'send', { sessionId: 'rx' }).ok, true)
assert.equal(rt.authorize(actorA, 'answer-question', { sessionId: 'rx' }).ok, true)
assert.equal(rt.authorize(actorB, 'answer-question', { sessionId: 'rx' }).ok, false)
assert.equal(rt.authorize(actorB, 'dismiss-question', { sessionId: 'rx' }).ok, false)
const full = { bots: Object.values(rt.state.bots), rooms: [], questions: { rx: rt.state.questions.rx } }
assert.deepEqual(Object.keys(rt.ownedView(full, actorA).questions), ['rx'])
assert.deepEqual(Object.keys(rt.ownedView(full, actorB).questions), [])
})
14 changes: 11 additions & 3 deletions core/test/diagnostics.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,17 @@ test('warnings are kept, cleaned, written to the log, and read back after a rest
assert.doesNotMatch(report.markdown, /sk-abc|\/home\//)
assert.equal(report.entries[0].code, 'RATE_LIMIT', 'newest first')

// Written as JSON lines; a new host reads them back.
await new Promise(resolve => setTimeout(resolve, 50))
assert.equal(readFileSync(join(home, 'logs', 'diagnostics.jsonl'), 'utf8').trim().split('\n').length, 2)
// Written as JSON lines in the background; a new host reads them back. Wait for the
// lines rather than a fixed pause, which a busy CI disk can outlast (#551 queue run).
const logLines = () => {
try {
return readFileSync(join(home, 'logs', 'diagnostics.jsonl'), 'utf8').trim().split('\n').length
} catch {
return 0 // not written yet
}
}
for (let waited = 0; logLines() < 2 && waited < 5000; waited += 20) await new Promise(resolve => setTimeout(resolve, 20))
assert.equal(logLines(), 2)
const again = host(home)
assert.deepEqual((await again.diagnostics.entries()).map(entry => entry.code), ['EACCES', 'RATE_LIMIT'])
assert.match(await again.diagnostics.logText(), /\n {4}at TestContext/)
Expand Down
Loading
Loading