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/847-room-share-hold.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A friend's Bot added to a local group chat now reaches that friend's FlyBot with no approval card: the host clears Bot Social's blanket hold on the room's own `FLYBOT-ROOM-SHARE` envelope (the same allow-list a face-room line uses), so the invited home sees the shared room as soon as the Bot is added and each later group line goes out with it. A hold with a finding of its own keeps its card, and a card that does appear reads one plain sentence (the invite, or the group message) instead of the raw envelope and its local ids ([#847](https://github.com/ThinkFlowLab/FlyBot/issues/847)).
2 changes: 1 addition & 1 deletion docs/subsystems/social.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ pins the list. The authority for the list is Bot Social's README and `docs/FACE_

### Inbound shared groups

- Bot Social has no FlyBot-group record (`list_conversations` is 1:1; inbox is `message.received` after a send). The plugin therefore carries a `FLYBOT-ROOM-SHARE` envelope on `send_as_member` text. `pollOnce` calls `flybot.rooms.applyInbound` before 1:1 chat delivery, so the Bot's owner sees the group as soon as they are added. `via: 'room'` / `via: 'room-share'` skip the social outbound hold (the add-to-group click is consent); the invited owner's Bot replies still go through `pending_approval`. A roster fetch that no longer lists a remote, or `social/revoke-grant`, leaves those rooms / hides inbound projections.
- Bot Social has no FlyBot-group record (`list_conversations` is 1:1; inbox is `message.received` after a send). The plugin therefore carries a `FLYBOT-ROOM-SHARE` envelope on `send_as_member` text. `pollOnce` calls `flybot.rooms.applyInbound` before 1:1 chat delivery, so the Bot's owner sees the group as soon as they are added. `via: 'room'` / `via: 'room-share'` skip the social outbound hold (the add-to-group click is consent), and a send the host sees come back `pending_approval` clears Bot Social's own blanket hold by itself (`autoSendRoomEnvelope`, the `holdIsBlanket()` allow-list of #737): the invite and the room's lines leave with no card, while a hold with a finding of its own keeps its card. That card reads one sentence about the envelope (`Invite <Bot> to the group "<name>"`, `Group message to <Bot>: <line>`), never the JSON or a local `session-…` id (#847). The invited owner's Bot replies still go through `pending_approval`. A roster fetch that no longer lists a remote, or `social/revoke-grant`, leaves those rooms / hides inbound projections.

### Discarded promise `.catch` (#213)

Expand Down
56 changes: 38 additions & 18 deletions plugins/social/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -1048,29 +1048,49 @@ window.__ModuleLoader__.load({
h('div', { className: 'fs-wait-code-digits', 'data-flybot': 'face-wait-code-digits' }, digits),
h(FaceQr, { code: digits, at: qrAt, offsetMs: qrOffsetMs }))
}
function shownHoldText(value) {
const raw = String(value ?? '')
const mark = 'FLYBOT-FACE-HUMAN'
const at = raw.indexOf(mark)
if (at < 0) return stripHoldPrefix(raw)
// The JSON tail of a fenced envelope, by brace matching so a `}` inside a string does not end it
// early. An absent or unparsable tail answers undefined and each caller keeps its own fallback.
function envelopeTail(raw, at) {
const start = raw.indexOf('{', at)
if (start < 0) return raw.slice(0, at).trim()
if (start < 0) return undefined
let depth = 0
let end = -1
for (let i = start; i < raw.length; i += 1) {
const ch = raw[i]
if (ch === '{') depth += 1
else if (ch === '}') {
if (raw[i] === '{') depth += 1
else if (raw[i] === '}') {
depth -= 1
if (depth === 0) { end = i; break }
if (depth > 0) continue
try { return JSON.parse(raw.slice(start, i + 1)) } catch { return undefined }
}
}
if (end < 0) return stripHoldPrefix(raw.slice(0, at).trim())
try {
const parsed = JSON.parse(raw.slice(start, end + 1))
const text = typeof parsed?.text === 'string' ? parsed.text.trim() : ''
if (text) return stripHoldPrefix(text)
} catch { /* fall through */ }
return undefined
}
// A room-share envelope is the group record a friend's FlyBot reads, never a line for a person
// (#847): the card says what it does in one sentence, and never prints the JSON or a local id.
// `to` is the name the card's own header already shows, so both name that Bot the same way.
function shownRoomShareText(raw, at, to) {
const who = cleanRoomName(to, '') || 'the invited Bot'
const parsed = envelopeTail(raw, at)
const name = typeof parsed?.name === 'string' ? cleanRoomName(parsed.name, '') : ''
const group = name ? `the group "${name}"` : 'a group chat'
const op = typeof parsed?.op === 'string' ? parsed.op : ''
const line = typeof parsed?.text === 'string' ? parsed.text.trim() : ''
if (op === 'join') return `Invite ${who} to ${group}`
if (op === 'leave') return `Remove ${who} from ${group}`
if (op === 'line') return line ? `Group message to ${who}: ${line}` : `Group message to ${who}`
return `Group update for ${who} in ${group}`
}
function shownHoldText(value, to) {
const raw = String(value ?? '')
// Two envelopes can carry the other's marker in their own text, so the earlier one wins.
const roomAt = raw.indexOf('FLYBOT-ROOM-SHARE')
const faceAt = raw.indexOf('FLYBOT-FACE-HUMAN')
if (roomAt >= 0 && (faceAt < 0 || roomAt < faceAt)) return shownRoomShareText(raw, roomAt, to)
const at = faceAt
if (at < 0) return stripHoldPrefix(raw)
const parsed = envelopeTail(raw, at)
if (parsed === undefined) return raw.slice(0, at).trim()
const text = typeof parsed?.text === 'string' ? parsed.text.trim() : ''
if (text) return stripHoldPrefix(text)
return stripHoldPrefix(raw.slice(0, at).trim())
}
function stripHoldPrefix(text) {
Expand Down Expand Up @@ -1718,7 +1738,7 @@ window.__ModuleLoader__.load({
h('button', { type: 'button', className: 'fs-btn fs-btn-sm fs-btn-primary', 'data-flybot': 'hold-retry', disabled: !!busy, onClick: () => onDecide?.(item, 'retry') }, 'Retry')))
}
const { from, to } = holdParties(item)
const text = shownHoldText(item.text || item.summary || '')
const text = shownHoldText(item.text || item.summary || '', to)
const long = text.length > 160 || text.split(/\n/).length > 3
const reasonLine = holdReasonLine(item.reasons, t)
return h('div', { className: 'fs-hold fs-chat-approval', 'data-flybot': 'chat-approval', 'data-role': 'owner', role: 'region', 'aria-label': WAITING_OWNER },
Expand Down
37 changes: 34 additions & 3 deletions plugins/social/src/social.js
Original file line number Diff line number Diff line change
Expand Up @@ -1474,10 +1474,14 @@ export function createSocial(service, config = {}, { client: given, api: givenAp
if (msg.approved === true && msg.via !== 'room' && msg.via !== 'room-share') await approveTypedLine(sent)
// Auto-send (#417) let this line out without a FlyBot card, so Bot Social's own hold would
// park it with nobody left to decide it: the owner's switch already answered the same
// question (#737). Room lines stay with the friend's owner, and a hold with a finding of
// its own keeps its card. Only Auto-send on this pair reaches here unapproved: the chain
// holds every other send before the transport runs.
// question (#737). A hold with a finding of its own keeps its card. Only Auto-send on this
// pair reaches here unapproved: the chain holds every other send before the transport runs.
else if (msg.via !== 'room' && msg.via !== 'room-share' && typeof msg.from === 'string' && approvalFor(msg.from, bot.home) === 'auto') await autoSendRemoteLine(sent)
// A room's own envelope is the click that added the Bot, not a line whose text the friend's
// owner reads first (#847): Bot Social's blanket hold on it asks the same question again, in
// a chat nobody opens, and the group never reaches the invited home. Only a hold with no
// finding of its own is cleared; anything else keeps its card.
else if ((msg.via === 'room' || msg.via === 'room-share') && !msg.attachments?.length) await autoSendRoomEnvelope(sent)
}
return { exchangeId: sent?.message?.message_id ?? msg.exchangeId }
}
Expand Down Expand Up @@ -1520,6 +1524,33 @@ export function createSocial(service, config = {}, { client: given, api: givenAp
await confirmedCall('decide_approval', { approval_id: approvalId, decision: 'approved' })
.catch(error => warn('direct-chat owner line not approved yet; the next poll retries', error))
}

// The room's own envelope clears Bot Social's blanket hold by itself (#847): the click on "Add
// member" is the owner's consent for the room, and `docs/subsystems/social.md` already promises the
// invited home sees the group as soon as the Bot is added. The allow-list is the face-room one
// (#737, #766): a reason this build does not know, another DLP category or a risk flag is a
// finding, so that envelope keeps its card and its owner still decides it.
async function autoSendRoomEnvelope(sent) {
const approvalId = sent?.message?.approval_id ?? sent?.approval_id
if (!approvalId) return false
try {
// Read straight from the server, like `autoSendRemoteLine`: the poll path would post a waiting
// notice for a hold this is about to clear.
const result = await client.callTool('list_approvals', { status: 'pending' })
const raw = result?.approvals ?? (Array.isArray(result) ? result : [])
const held = raw.find(item => String(item?.approval_id ?? item?.id ?? '') === String(approvalId))
if (!held) {
warn(`room envelope: Bot Social has not listed the hold ${approvalId} yet; it waits on its card`)
return false
}
if (!holdIsBlanket(held)) return false
await confirmedCall('decide_approval', { approval_id: approvalId, decision: 'approved' })
return true
} catch (error) {
warn('room envelope hold not cleared; it waits on its card', error)
return false
}
}
// Approves the Bot Social hold on a line Auto-send (#417) let out with no FlyBot card (#737), only
// when Bot Social held it for no finding of its own (no DLP category on the hold). Anything else
// stays on its card, where the owner decides it, exactly as with Auto-send off.
Expand Down
66 changes: 66 additions & 0 deletions plugins/social/test/social.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1137,6 +1137,46 @@ test('#227: an owner line whose approve call failed is finished by the poll, nev
await waitUntil(() => server.queue.length === 0)
})

test('#847: a room envelope clears Bot Social\'s blanket hold: the add click is the consent', async (t) => {
const { ctx, call, stop } = await team()
t.after(stop)
const server = holdingServer()
const social = createSocial(ctx.flybot, { autoAccept: false, inboxPollMs: 0 }, { client: createMcpClient({ server: 'https://s', token: 'owner-token', fetch: server.fetch }), home: { id: 'h1', handle: '@danyang.home' } })
t.after(() => social.dispose())
await grantTeam(ctx.flybot)
await social.registerAll()
const envelope = (op, extra = {}) => `FLYBOT-ROOM-SHARE\n${JSON.stringify({ v: 1, op, roomId: 'session-e2439f82', name: 'You, Chief, Chief', invitedHandle: '@coder.bob.home', seq: 0, ...extra })}`
assert.equal((await ctx.flybot.bot.send({ from: 'a', to: '@coder.bob.home', text: envelope('join'), via: 'room-share' })).action, 'allow')
await waitUntil(() => server.queue.length === 0)
const first = server.calls.filter(c => c.name === 'decide_approval').at(-1)
assert.equal(first?.args.decision, 'approved', 'the join is approved by the host, not by the owner')
assert.match(String(first?.args.approval_id), /^apr-1-/)
assert.deepEqual((await call('flybot/social/status', {})).value.approvals, [], 'no card for the join')
// A line in the same room carries the same consent: one approval, not one per line.
assert.equal((await ctx.flybot.bot.send({ from: 'a', to: '@coder.bob.home', text: envelope('line', { text: 'SelfTest A2: hello', seq: 1 }), via: 'room' })).action, 'allow')
await waitUntil(() => server.queue.length === 0)
assert.equal(server.calls.filter(c => c.name === 'decide_approval').length >= 2, true, 'the room line is cleared too')
})

test('#847: a room envelope held with a finding of its own keeps its card', async (t) => {
const { ctx, call, stop } = await team()
t.after(stop)
const server = holdingServer({ reasons: ['mvp.all_outbound_requires_approval', 'dlp.commitment'] })
const social = createSocial(ctx.flybot, { autoAccept: false, inboxPollMs: 0 }, { client: createMcpClient({ server: 'https://s', token: 'owner-token', fetch: server.fetch }), home: { id: 'h1', handle: '@danyang.home' } })
t.after(() => social.dispose())
await grantTeam(ctx.flybot)
await social.registerAll()
const join = `FLYBOT-ROOM-SHARE\n${JSON.stringify({ v: 1, op: 'join', roomId: 'session-e2439f82', name: 'You, Chief, Chief', invitedHandle: '@coder.bob.home', seq: 0 })}`
assert.equal((await ctx.flybot.bot.send({ from: 'a', to: '@coder.bob.home', text: join, via: 'room-share' })).action, 'allow')
for (let i = 0; i < 50 && !server.calls.some(c => c.name === 'list_approvals'); i += 1) await new Promise(resolve => setTimeout(resolve, 10))
assert.equal(server.calls.some(c => c.name === 'list_approvals'), true, 'the hold was looked at')
assert.equal(server.calls.some(c => c.name === 'decide_approval'), false, 'a hold with a finding waits for its owner')
assert.equal(server.queue.length, 1)
const pending = (await call('flybot/social/status', {})).value.approvals
assert.equal(pending.length, 1)
assert.equal(pending[0].text.startsWith('FLYBOT-ROOM-SHARE'), true, 'the card still carries the envelope for the host to read')
})

test('#258: a Bot\'s reply the owner approved on its card leaves at once: no second hold on Bot Social', async (t) => {
const { ctx, call, stop } = await team()
t.after(stop)
Expand Down Expand Up @@ -3544,6 +3584,32 @@ test('#188: holds of a left room or with no known room stay out of the list and
assert.deepEqual(ids, ['apr-live'])
})

test('#847: a room-share hold card says what the envelope does, never the envelope', () => {
const { walk, pluginOf } = loadSocialClient(async () => ({}))
const plugin = pluginOf([[], [], ''])
const card = (text) => walk(plugin.ChatApproval({
item: { id: 'apr-room', kind: 'outbound_message', from: 'Chief', toName: 'Chief (SelfTest B2)', text, summary: text },
canDecide: true,
}))
const draft = (tree) => tree.find(n => n.props.className === 'fs-draft')?.children.join('')
const join = `FLYBOT-ROOM-SHARE\n${JSON.stringify({ v: 1, op: 'join', roomId: 'session-e2439f82-0c89-4bba-a7d7-7d60572c19fd', name: 'You, Chief, Chief', inviterName: 'a friend', invitedHandle: '@chief.selftestb2.home', seq: 0 })}`
const joined = card(join)
assert.equal(draft(joined), 'Invite Chief (SelfTest B2) to the group "You, Chief, Chief"')
assert.equal(JSON.stringify(joined).includes('FLYBOT-ROOM-SHARE'), false, 'the wire marker never reaches a person')
assert.equal(JSON.stringify(joined).includes('session-e2439f82'), false, 'a local session id never reaches a person')

const line = `FLYBOT-ROOM-SHARE\n${JSON.stringify({ v: 1, op: 'line', roomId: 'session-e2439f82', name: 'You, Chief, Chief', text: 'SelfTest A2: hello', seq: 3 })}`
assert.equal(draft(card(line)), 'Group message to Chief (SelfTest B2): SelfTest A2: hello')

// A broken or unknown shape is still a sentence, never the JSON tail.
assert.equal(draft(card('FLYBOT-ROOM-SHARE\n{not json')), 'Group update for Chief (SelfTest B2) in a group chat')
assert.equal(draft(card(`FLYBOT-ROOM-SHARE\n${JSON.stringify({ v: 1, op: 'join', roomId: 'session-1' })}`)), 'Invite Chief (SelfTest B2) to a group chat')

// The face-room envelope keeps its own answer, even when its text mentions the other marker.
const packed = encodeFaceHuman({ who: 'BoxA', text: 'the group says FLYBOT-ROOM-SHARE is a marker' })
assert.equal(draft(card(packed)), 'the group says FLYBOT-ROOM-SHARE is a marker')
})

test('#159: a face-room hold never shows the FLYBOT-FACE-HUMAN envelope', () => {
const packed = encodeFaceHuman({ who: 'BoxA', text: 'hello from the owner' })
const { walk, pluginOf } = loadSocialClient(async () => ({}))
Expand Down
Loading