diff --git a/changelog.d/847-room-share-hold.fixed.md b/changelog.d/847-room-share-hold.fixed.md new file mode 100644 index 000000000..c6a44bf10 --- /dev/null +++ b/changelog.d/847-room-share-hold.fixed.md @@ -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)). diff --git a/docs/subsystems/social.md b/docs/subsystems/social.md index 95c6642e2..466808c3b 100644 --- a/docs/subsystems/social.md +++ b/docs/subsystems/social.md @@ -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 to the group ""`, `Group message to : `), 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) diff --git a/plugins/social/client.js b/plugins/social/client.js index 35f03bde2..424d12514 100644 --- a/plugins/social/client.js +++ b/plugins/social/client.js @@ -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) { @@ -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 }, diff --git a/plugins/social/src/social.js b/plugins/social/src/social.js index be02dc0f6..88698d6d7 100644 --- a/plugins/social/src/social.js +++ b/plugins/social/src/social.js @@ -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 } } @@ -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. diff --git a/plugins/social/test/social.test.js b/plugins/social/test/social.test.js index df169f3e5..896c8f140 100644 --- a/plugins/social/test/social.test.js +++ b/plugins/social/test/social.test.js @@ -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) @@ -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 () => ({}))