Show a failed hold as Not sent, and make Approve all cover Bot Social

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.

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)

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,
         })))

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([