From a701b2e07bd78f970880448dea34a656ad9c073d Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Thu, 22 Oct 2026 16:29:38 +0800 Subject: [PATCH] Show a failed hold as Not sent, and make Approve all cover Bot Social A local hold's view dropped status and error, so a failed send was drawn as "Waiting for approval" with Approve/Reject that threw bot/stale and showed nothing. Keep status/error, draw failed as Not sent with the error and a Retry (the core decide retry), and surface a decision error. "Approve all" keyed on a missing status, but Bot Social holds carry status: 'pending', so it was absent for them (and skipped them when it did show); treat pending as waiting and leave failed to Retry. --- ...ocial-approval-failed-approve-all.fixed.md | 1 + docs/subsystems/social.md | 2 +- plugins/social/client.js | 27 ++++++++++++++-- plugins/social/test/social.test.js | 31 +++++++++++++++++++ 4 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 changelog.d/social-approval-failed-approve-all.fixed.md diff --git a/changelog.d/social-approval-failed-approve-all.fixed.md b/changelog.d/social-approval-failed-approve-all.fixed.md new file mode 100644 index 000000000..602672417 --- /dev/null +++ b/changelog.d/social-approval-failed-approve-all.fixed.md @@ -0,0 +1 @@ +Approving every waiting card now works for Bot Social holds too (the button was missing or skipped them), and a local hold whose send failed reads Not sent with its error and a Retry instead of offering Approve / Reject that silently do nothing. diff --git a/docs/subsystems/social.md b/docs/subsystems/social.md index 4e7b78d0c..2546a4af8 100644 --- a/docs/subsystems/social.md +++ b/docs/subsystems/social.md @@ -76,7 +76,7 @@ pins the list. The authority for the list is Bot Social's README and `docs/FACE_ ### Inline approvals and compact Friends' Bots (#114, #115) - A held send is never without a place (#371): the approval carries the Bot Social identity that wrote it (`approvalView` → `botId`; `pendingApprovals` maps it to the local Bot as `fromId`) and Bot Social's DLP categories (`holdReasons`: `commitment`, `pii.phone`, `pii.email`, `money`, `owner.sensitive_term`, `calendar.details`; the plain "all outbound needs approval" reason is not one). When the friend's chat is hidden, gone, home-level or not in the roster (`listedFriendChat`), the sender Bot's own chat draws the card; a listed friend chat alone shows it (#278). The card's second line reads `Held for your approval: contains commitment language` (host `t`, so zh in a zh UI) and the held text stays on the card, the bubble says only that it waits. The sender Bot's row also takes the wait dot (the `chatApprovals` provider answers for local Bots by `fromId`). No message is posted into a Bot's chat for this: it would reach the Bot's model. -- Held Bot Social drafts (`social/status`, `social/decide-approval`) and matching local `flybot/approvals` holds show as a compact in-chat card (`flybot.session.dock`, max-width ~460px, right-aligned like a bubble — not full chat width; a Bot's held reply to a friend has its card on that reply's bubble instead, invariant 8): **Waiting for approval**, one `Name → Name` line (real names, never raw `@handle` / `bot_01…`), a 3-line clamped preview with **Show more**, and **Approve** / **Reject** buttons plus a quiet **Always approve** link on one row for the owner (#260). **Always approve** is owner-only: it approves this card and turns on the existing Auto-send for that pair (per-friend `social/auto-accept`, or in a face room the writing Bot's own `social/face-auto` switch, #457) so later messages on that pair are not held; it does not decide a different sender/recipient hold and is off by default. A toast says **Auto-send on for X → Y. Change it anytime in settings.** **Approve all** sits above the cards when several are waiting; anyone else sees **Waiting for the owner's approval** with no buttons. The owner does not get a waiting bubble in their 1:1; composer holds are labelled **You**. New friends start with **Auto-send** off. The owner's click is consent (`confirm_code` in the same `decide_approval` / `set_auto_approve` call). After a decision the card reads Approved or Rejected. The social strip "N to approve" block is a one-line jump list (chat name + truncated text, no explainer, no wrapping one word per line). Auto-send of face-room posts is one compact checkbox per Bot of mine on the open room's overview (#457) — not on each pending row or room card, and not duplicated on a Face-to-face rooms header. The "Waiting for the owner's approval" ledger (`settings.approvalNotices`) keeps one entry per still-pending hold and drops a hold's entry as soon as it leaves the pending queue, so the persisted settings file does not grow with every hold the install ever had. +- Held Bot Social drafts (`social/status`, `social/decide-approval`) and matching local `flybot/approvals` holds show as a compact in-chat card (`flybot.session.dock`, max-width ~460px, right-aligned like a bubble — not full chat width; a Bot's held reply to a friend has its card on that reply's bubble instead, invariant 8): **Waiting for approval**, one `Name → Name` line (real names, never raw `@handle` / `bot_01…`), a 3-line clamped preview with **Show more**, and **Approve** / **Reject** buttons plus a quiet **Always approve** link on one row for the owner (#260). **Always approve** is owner-only: it approves this card and turns on the existing Auto-send for that pair (per-friend `social/auto-accept`, or in a face room the writing Bot's own `social/face-auto` switch, #457) so later messages on that pair are not held; it does not decide a different sender/recipient hold and is off by default. A toast says **Auto-send on for X → Y. Change it anytime in settings.** **Approve all** sits above the cards when several are waiting; anyone else sees **Waiting for the owner's approval** with no buttons. The owner does not get a waiting bubble in their 1:1; composer holds are labelled **You**. New friends start with **Auto-send** off. The owner's click is consent (`confirm_code` in the same `decide_approval` / `set_auto_approve` call). After a decision the card reads Approved or Rejected. The social strip "N to approve" block is a one-line jump list (chat name + truncated text, no explainer, no wrapping one word per line). Auto-send of face-room posts is one compact checkbox per Bot of mine on the open room's overview (#457) — not on each pending row or room card, and not duplicated on a Face-to-face rooms header. A local hold that failed (`status: 'failed'`) draws as **Not sent** with its error and a **Retry** (the core `decide` `retry`), never Approve / Reject; **Approve all** covers Bot Social holds (`status: 'pending'`) as well as local ones and leaves a failed hold to Retry, and a failed decision says why instead of throwing silently. The "Waiting for the owner's approval" ledger (`settings.approvalNotices`) keeps one entry per still-pending hold and drops a hold's entry as soon as it leaves the pending queue, so the persisted settings file does not grow with every hold the install ever had. - Live face-to-face rooms and friends' Bots sit in the core `TeamSidebar` list (globe / people mark), not a second **Friends' Bots** section. Closed, left, and ended matching rooms sit under **Past rooms**; the whole row opens the room (#304). The list does not say "The number is no longer valid." 1:1 friend chats get the same wait badge as rooms (`flybot.chatApprovals.setProvider` → `bot.pendingCount`). The social strip is **N to approve**, the + panels, and Past rooms — no **+ Add**, no per-friend groups, no Copy-handle row. **Add friend** and **Face-to-face room** are `flybot.new-chat.action` rows on the sidebar **+**. Add friend opens the join panel (handle, invite link, or code). Per-friend **Auto-send** and **Remove this Bot** live on that friend's Bot settings (`flybot.bot.details`). ### Face-to-face waiting grid, leave, and social + (#124, #125, #126) diff --git a/plugins/social/client.js b/plugins/social/client.js index 337515585..b2b297709 100644 --- a/plugins/social/client.js +++ b/plugins/social/client.js @@ -1059,6 +1059,17 @@ window.__ModuleLoader__.load({ return h('div', { className: 'fs-hold fs-chat-approval', 'data-flybot': 'chat-approval', 'data-role': 'other', role: 'status' }, h('div', { className: 'fs-name' }, WAITING_OTHERS)) } + if (item.status === 'failed') { + // A failed send cannot be approved or rejected: say it did not go and offer Retry, like + // the core's held-reply card. Approve/Reject on it would answer "already failed" silently. + const parties = holdParties(item) + return h('div', { className: 'fs-hold fs-chat-approval', 'data-flybot': 'chat-approval', 'data-state': 'failed', 'data-role': 'owner', role: 'status', title: item.error || undefined }, + h('div', { className: 'fs-label' }, 'Not sent'), + item.from || item.to || item.toName ? h('div', { className: 'fs-sub' }, `${parties.from} → ${parties.to}`) : null, + item.error ? h('div', { className: 'fs-sub', 'data-flybot': 'hold-error' }, item.error) : null, + h('div', { className: 'fs-actions fs-chat-actions' }, + 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 long = text.length > 160 || text.split(/\n/).length > 3 @@ -1152,6 +1163,9 @@ window.__ModuleLoader__.load({ return { id: approval.id, source: 'local', + // A failed local hold must read as failed and offer Retry, like the core overlay (#52). + status: approval.status, + ...(approval.error ? { error: String(approval.error) } : {}), from: composer ? 'You' : approval.senderName, // The local Bot that wrote it (not a line you typed): its own chat shows the card (#254). ...(!composer && typeof approval.request?.from === 'string' ? { fromId: approval.request.from } : {}), @@ -1279,6 +1293,7 @@ window.__ModuleLoader__.load({ const [items, setItems] = useState([]) const [local, setLocal] = useState([]) const [busy, setBusy] = useState('') + const [error, setError] = useState('') const load = () => { call('social/status', {}).then(value => { setItems(Array.isArray(value?.approvals) ? value.approvals : []) @@ -1317,6 +1332,7 @@ window.__ModuleLoader__.load({ if (!mine.length && !waitingOthers) return null const decide = async (item, decision) => { setBusy(item.id) + setError('') try { const always = decision === 'always' const mapped = always || decision === 'approve' || decision === 'approved' ? 'approve' : decision @@ -1342,20 +1358,27 @@ window.__ModuleLoader__.load({ setItems(drop) setLocal(drop) if (always && status === 'approved') actions?.notify?.(autoSendToast(autoSend)) + } catch (failure) { + // A stale or failed hold must say why, never swallow the click. + setError(String(failure?.message ?? failure).slice(0, 200)) } finally { setBusy('') } } + // A card is waiting when it is pending: a local hold (no status yet) or a Bot Social one + // (`status: 'pending'`). A failed hold is not approved; it is retried. + const waiting = entry => !entry.status || entry.status === 'pending' const approveAll = async () => { - for (const item of mine.filter(entry => !entry.status)) await decide(item, 'approve') + for (const item of mine.filter(waiting)) await decide(item, 'approve') } if (waitingOthers) { return h('div', { 'data-flybot': 'chat-approvals' }, h(ChatApproval, { item: { id: 'waiting' }, canDecide: false })) } return h('div', { 'data-flybot': 'chat-approvals' }, - mine.length > 1 && mine.some(entry => !entry.status) + mine.length > 1 && mine.some(waiting) ? h('div', { className: 'fs-chat-actions' }, h('button', { type: 'button', className: 'fs-btn fs-btn-sm', 'data-flybot': 'approve-all', disabled: !!busy, onClick: approveAll }, 'Approve all')) : null, + error ? h('div', { className: 'fs-chat-error', role: 'alert', 'data-flybot': 'chat-approval-error' }, error) : null, mine.map(item => h(ChatApproval, { key: item.id, item, canDecide: true, busy: busy === item.id, onDecide: decide, t, }))) diff --git a/plugins/social/test/social.test.js b/plugins/social/test/social.test.js index 6ea45d7e9..8a881bba1 100644 --- a/plugins/social/test/social.test.js +++ b/plugins/social/test/social.test.js @@ -3330,6 +3330,37 @@ test('#371: a local Bot\'s hold shows in its own chat when the friend\'s chat is assert.equal(fresh().holdReasonLine([], undefined), '') }) +test('#260: Approve all approves Bot Social holds too, not only local ones', async () => { + const calls = [] + const { walk, pluginOf } = loadSocialClient(async (endpoint, payload) => { calls.push([endpoint, payload]); return {} }) + const friend = { id: 'r1', name: 'Coder', remote: { handle: '@coder.bob.home' } } + const roster = { bots: [friend] } + const item = id => ({ id, kind: 'outbound_message', status: 'pending', from: 'Chief', to: '@coder.bob.home', toName: 'Bob', text: id }) + const tree = walk(pluginOf([[item('s1'), item('s2')], [], '']).ChatApprovalDock({ bot: friend, roster })) + const all = tree.find(node => node.props['data-flybot'] === 'approve-all') + assert.ok(all, 'Approve all is offered for Bot Social holds (they carry status: pending)') + await all.props.onClick() + const decided = calls.filter(([endpoint]) => endpoint === 'social/decide-approval').map(([, payload]) => payload.approvalId) + assert.deepEqual(decided.sort(), ['s1', 's2']) +}) + +test('#52: a failed local hold reads Not sent with its error and offers Retry', async () => { + const calls = [] + const { walk, pluginOf } = loadSocialClient(async (endpoint, payload) => { calls.push([endpoint, payload]); return {} }) + const friend = { id: 'r1', name: 'Coder', remote: { handle: '@coder.bob.home' } } + const roster = { bots: [friend] } + const hold = { id: 'f1', kind: 'outbound_message', source: 'local', status: 'failed', error: 'file changed after approval', from: 'Chief', to: '@coder.bob.home', toName: 'Bob', text: 'x' } + const tree = walk(pluginOf([[], [hold], '']).ChatApprovalDock({ bot: friend, roster })) + const card = tree.find(node => node.props['data-flybot'] === 'chat-approval') + assert.equal(card?.props['data-state'], 'failed') + assert.ok(tree.some(node => node.props['data-flybot'] === 'hold-error' && node.children.includes('file changed after approval'))) + assert.equal(tree.some(node => node.props.className === 'fs-btn fs-btn-sm fs-btn-primary' && node.children.includes('Approve')), false, 'no Approve on a failed hold') + const retry = tree.find(node => node.props['data-flybot'] === 'hold-retry') + assert.ok(retry, 'Retry is offered') + await retry.props.onClick() + assert.deepEqual(calls.filter(([endpoint]) => endpoint === 'decide').map(([, payload]) => [payload.id, payload.decision]), [['f1', 'retry']]) +}) + test('#126: a face-room hold shows in that chat with a readable name', () => { const { walk, pluginOf } = loadSocialClient(async () => ({})) const plugin = pluginOf([