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/188-hold-room-scope.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A held Bot line now shows only in the face-to-face room it was held in; a stale card and waiting dot from an old room no longer appear in a new one. ([#188](https://github.com/ThinkFlowLab/FlyBot/issues/188))
1 change: 1 addition & 0 deletions docs/subsystems/social.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Everything in `plugins/social/` (entry `plugins/social/index.js` and any helpers
### Face-room human turns and UI polish (#166, #167)

- A plain human line in an open face-to-face room is logged and fanned out; it does not start a Chief/Bot model turn unless that Bot is @mentioned or @all (core `openingSpeakers`; `plugins/social/test` and `core/test/rounds.test.js`).
- A held room line belongs to one room (#188): the send path records `settings.holdRooms[approvalId] = faceRoomId` when Bot Social's card does not name the room, and `pendingApprovals` drops holds with no known room (legacy) or whose room is left / closed / past here, so no other room shows the card and no waiting dot stays on an old row. The room scope in the client, the `roomApprovals` provider and `hidden-remotes` match the room id strictly.
- Face-room holds (`face_room_message` / `roomId`) attach `pendingCount` to that room only. They do not badge an unrelated friend row via `from`/`to` handle. The social strip no longer draws an **N to approve** tag under the last sidebar row; the gold wait dot on the owning RoomRow / 1:1 remains.
- Approval preview strips `To face room:` / `To the room:` and wraps on word boundaries (`overflow-wrap:break-word; word-break:normal`), with **Show more** when long.
- Join notices are remembered by handle and by exact text so a `member.joined` event plus the members loop does not post two identical joined lines.
Expand Down
7 changes: 3 additions & 4 deletions plugins/social/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -844,14 +844,13 @@
const list = Array.isArray(items) ? items : []
const handle = bot?.remote?.handle
const faceId = room?.face?.roomId
const face = room?.origin === 'face-to-face' || Boolean(faceId)
const handles = new Set((room?.participants ?? []).map(p => p.handle).filter(Boolean))
if (handle) handles.add(handle)
return list.filter(item => {
if (item.kind === 'face_room_message') {
if (faceId && (!item.roomId || item.roomId === faceId)) return true
if (room?.id && item.roomId === room.id) return true
return Boolean(face && !item.roomId)
// A room line belongs to the one room it was held in (#188); no room means no card.
if (faceId && item.roomId === faceId) return true
return Boolean(room?.id && item.roomId === room.id)
}
if (faceId && item.roomId === faceId) return true
if (room?.id && item.roomId === room.id) return true
Expand Down Expand Up @@ -1304,7 +1303,7 @@
statusTask ??= call('social/status', {}).then(value => {
const changed = JSON.stringify(value) !== JSON.stringify(status.value)
status = { at: Date.now(), value }
if (changed) for (const fn of [...statusListeners]) { try { fn() } catch (error) { console.error('flybot-social: status listener failed', error) } }

Check warning on line 1306 in plugins/social/client.js

View workflow job for this annotation

GitHub Actions / plugins (social)

unicorn(no-useless-spread)

Using a spread operator here creates a new array unnecessarily.
return value
}).finally(() => { statusTask = undefined })
return statusTask
Expand Down
2 changes: 1 addition & 1 deletion plugins/social/src/hidden-remotes.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export function approvalsForSession(items, session = {}) {
if (handle) handles.add(handle)
return list.filter(item => {
if (item.kind === 'face_room_message') {
if (faceId && (!item.roomId || item.roomId === faceId)) return true
if (faceId && item.roomId === faceId) return true
if (roomId && item.roomId === roomId) return true
return false
}
Expand Down
23 changes: 22 additions & 1 deletion plugins/social/src/social.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@
const TEXTUAL = new Set(['text/plain', 'text/markdown', 'text/csv', 'text/tab-separated-values', 'application/json'])
const sizeLabel = n => (n < 1024 ? `${n} B` : n < 1048576 ? `${(n / 1024).toFixed(1)} KB` : `${(n / 1048576).toFixed(1)} MB`)
// Remote strings that reach model-visible text: one line, no brackets or markup, capped.
export const cleanText = (value, fallback = '') => String(value ?? '').replace(/[\u0000-\u001f\u007f-\u009f\u2028\u2029]/g, ' ').replace(/[[\]"<>`]/g, '').replace(/\s+/g, ' ').trim().slice(0, 60) || fallback

Check warning on line 84 in plugins/social/src/social.js

View workflow job for this annotation

GitHub Actions / plugins (social)

eslint(no-control-regex)

Unexpected control characters
// Owner-only draft body: keep punctuation the friend will see; strip controls and cap.
export const draftText = (value) => String(value ?? '').replace(/[\u0000-\u001f\u007f-\u009f\u2028\u2029]/g, ' ').replace(/\s+/g, ' ').trim().slice(0, 800)

Check warning on line 86 in plugins/social/src/social.js

View workflow job for this annotation

GitHub Actions / plugins (social)

eslint(no-control-regex)

Unexpected control characters
// Owner cards never show the FLYBOT-FACE-HUMAN wire envelope (#159).
export function approvalDraftText(value) {
const raw = draftText(value)
Expand Down Expand Up @@ -136,7 +136,7 @@
}
const SEEN_KEPT = 1000
const CREDENTIAL_NAME = /(^id_(rsa|dsa|ecdsa|ed25519)|\.(pem|key|p12|pfx|kdbx|keychain)$|credential|secret|token|password|passwd|^flybot-social.*\.json$|^\.?env$|^\.netrc$|^known_hosts$|^authorized_keys$)/i
const safeName = name => String(name ?? 'file').replace(/[\u0000-\u001f\u007f/\\:]/g, '_').replace(/^\.+/, '_').slice(0, 120) || 'file'

Check warning on line 139 in plugins/social/src/social.js

View workflow job for this annotation

GitHub Actions / plugins (social)

eslint(no-control-regex)

Unexpected control characters
// The text Copy puts on the clipboard for a per-Bot invite: what to do with the link, for a
// person in FlyBot or for their Grok bot.
export function inviteText({ handle, name, home, url, expiresAt }) {
Expand Down Expand Up @@ -575,7 +575,7 @@
if (result?.unavailable) return
} catch (error) {
if (/invalid_avatar|invalid_color|invalid_request/i.test(`${error?.code ?? ''} ${error?.message ?? ''}`)) {
settings.lookSynced = { ...(settings.lookSynced ?? {}), [bot.id]: fingerprint }

Check warning on line 578 in plugins/social/src/social.js

View workflow job for this annotation

GitHub Actions / plugins (social)

unicorn(no-useless-fallback-in-spread)

Empty fallbacks in spreads are unnecessary
saveSettings()
return
}
Expand Down Expand Up @@ -1722,6 +1722,22 @@
if (keys.length > 200) for (const key of keys.sort((a, b) => map[a] - map[b]).slice(0, keys.length - 200)) delete map[key]
saveSettings()
}
// The room a held room line belongs to (#188). Bot Social's approval card may not name it, so
// the send path records it; a hold with no known room is legacy and stays out of every room.
const holdRooms = () => (settings.holdRooms ??= {})
function rememberHoldRoom(id, faceRoomId) {
if (!id || !faceRoomId) return
const map = holdRooms()
map[id] = { roomId: faceRoomId, at: now() }
const keys = Object.keys(map)
if (keys.length > 200) for (const key of keys.sort((a, b) => map[a].at - map[b].at).slice(0, keys.length - 200)) delete map[key]
saveSettings()
}
const faceHoldLive = (roomId) => {
const hosted = roomId ? flybot.rooms.ofFace?.(roomId) : undefined
if (hosted?.face?.status === 'left' || hosted?.face?.status === 'closed') return false
return !(settings.pastFaceRooms ?? []).some(item => item.faceRoomId === roomId)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Undo the past-room marker after a failed leave

When social/face-leave encounters a transient error, leaveFaceRoom has already added the room to pastFaceRooms at lines 2231-2232, but the non-alreadyLeftFace branch rethrows without removing that provisional entry. This new predicate then filters every hold from the still-hosted open room on the next status poll, and a later settings write can persist the bad marker; delay or roll back the past-room update when the leave request fails.

Useful? React with 👍 / 👎.

}
const ownLineApproving = new Set()
async function approveOwnLine(item) {
if (!item?.id || ownLineApproving.has(item.id)) return
Expand All @@ -1743,6 +1759,10 @@
const items = raw
.map(item => approvalView(item, { localName: localNameOf }))
.filter(item => item && (item.kind === 'outbound_message' || item.kind === 'face_room_message') && (item.status === 'pending' || item.status === 'pending_approval'))
// A held room line belongs to one live room only (#188): no known room (legacy) or a room
// that is left / closed here means no card and no waiting dot anywhere.
.map(item => (item.kind === 'face_room_message' && !item.roomId && holdRooms()[item.id]?.roomId ? { ...item, roomId: holdRooms()[item.id].roomId } : item))
.filter(item => item.kind !== 'face_room_message' || (ownLineIds()[item.id] || (item.roomId && faceHoldLive(item.roomId))))
const own = items.filter(item => ownLineIds()[item.id])
const held = items.filter(item => !ownLineIds()[item.id])
pendingCache = { at: now(), items: held }
Expand Down Expand Up @@ -1839,7 +1859,7 @@
const faceId = room?.face?.roomId
return pendingCache.items.filter((item) => {
if (item.kind === 'face_room_message') {
return Boolean(faceId) && (!item.roomId || item.roomId === faceId || item.roomId === room.id)
return Boolean(item.roomId) && (item.roomId === faceId || item.roomId === room.id)
}
if (item.roomId) return item.roomId === room.id || (faceId && item.roomId === faceId)
return handles.has(norm(item.from)) || handles.has(norm(item.to))
Expand Down Expand Up @@ -2263,6 +2283,7 @@
try {
const sent = await faceApi.send(serverId, { text: outbound.text, botId: outbound.botId })
if (sent?.status === 'pending_approval') {
rememberHoldRoom(sent.approvalId, serverId)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Record rooms for the direct face-send path

When a caller uses the still-registered social/face-send endpoint, its pending branch at lines 2351-2356 only invalidates pendingCache and never executes this new mapping even though it knows the face-room ID. On the Bot Social responses this patch is meant to support, where list_approvals omits room_id, the new filter consequently treats the just-created hold as legacy and removes it, so the returned approval cannot be approved from the room UI; record the approval-to-room mapping in that send path too.

Useful? React with 👍 / 👎.

// A person's own line is theirs: typing it is the approval, so it goes out at once
// with no card (#179). Only Bot-written room lines wait for Approve / Reject.
if (outbound.human && sent.approvalId) {
Expand Down
38 changes: 37 additions & 1 deletion plugins/social/test/social.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2398,7 +2398,7 @@ test('Add friend mints and copies an invite link in one click', async () => {
test('#126: a face-room hold shows in that chat with a readable name', () => {
const { walk, pluginOf } = loadSocialClient(async () => ({}))
const plugin = pluginOf([
[{ id: 'apr-face', kind: 'face_room_message', from: 'Chief', toName: 'the room', text: 'hello there' }],
[{ id: 'apr-face', kind: 'face_room_message', roomId: 'frm_1', from: 'Chief', toName: 'the room', text: 'hello there' }],
[],
'',
])
Expand All @@ -2410,6 +2410,42 @@ test('#126: a face-room hold shows in that chat with a readable name', () => {
assert.ok(tree.some(n => n.props.className === 'fs-draft' && n.children.includes('hello there')))
})

test('#188: a face-room hold shows only in its own room, never in a newer one', () => {
const holds = [
{ id: 'apr-old', kind: 'face_room_message', roomId: 'frm_old', from: 'Chief', toName: 'the room', text: 'old line' },
{ id: 'apr-legacy', kind: 'face_room_message', from: 'Chief', toName: 'the room', text: 'no room known' },
]
const { walk, pluginOf } = loadSocialClient(async () => ({}))
const plugin = pluginOf([holds, [], ''])
const tree = walk(plugin.ChatApprovalDock({
room: { origin: 'face-to-face', id: 'g2', face: { roomId: 'frm_new' } },
}))
assert.equal(tree.filter(n => n.props['data-flybot'] === 'chat-approval').length, 0)
const own = walk(pluginOf([holds, [], '']).ChatApprovalDock({
room: { origin: 'face-to-face', id: 'g1', face: { roomId: 'frm_old' } },
}))
assert.equal(own.filter(n => n.props['data-flybot'] === 'chat-approval').length, 1)
})

test('#188: holds of a left room or with no known room stay out of the list and the waiting dots', async (t) => {
const { ctx, stop } = await team()
t.after(stop)
const server = withFaceRooms(mockServer())
const hold = (id, extra = {}) => ({ approval_id: id, kind: 'face_room_message', status: 'pending', bot_id: 'm1', message: { payload: { text: id } }, summary: `To the room: ${id}`, ...extra })
server.tools.list_approvals = () => ({ approvals: [hold('apr-live', { room_id: 'frm_live' }), hold('apr-left', { room_id: 'frm_left' }), hold('apr-legacy')] })
await ctx.flybot.rooms.upsertFace({ roomId: 'frm_live', name: 'Live', status: 'open', members: [] })
await ctx.flybot.rooms.upsertFace({ roomId: 'frm_left', name: 'Old', status: 'left', members: [] })
const social = createSocial(ctx.flybot, { server: 'https://s', token: 'owner-token', fetch: server.fetch, inboxPollMs: 0, autoAccept: false }, {
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 ids = (await social.pendingApprovals({ fresh: true })).map(item => item.id)
assert.deepEqual(ids, ['apr-live'])
})

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