client: a read-only room approval card draws no decision and says why

changelog.d/2381-room-approvals-readonly.fixed.md

@@ -0,0 +1 @@
+A window that holds the team read-only no longer draws **Approve** or **Reject** on a shared room's "Waiting for your approval" card in the details drawer: a decision is a write, so the host refuses it there and the two buttons could only produce a refusal after the click. The card keeps its headline, its count and each request's own text, and says why in the host's own sentence. A decision the host still refuses — in a writable window, or in a room with several members — now writes that reason on the card in one alert line instead of disappearing ([#2381](https://github.com/ThinkFlowLab/FlyBot/issues/2381)).

core/client.js

@@ -951,6 +951,7 @@ var ZH = {
   "{first}, {second} and {count} more are waiting for your approval": "{first}、{second} 等 {total} 个等你批准",
   "Waiting for your approval": "等你批准",
   "Waiting for their approval": "等对方批准",
+  "The decision was not sent.": "决定没有发出去。",
   "Your Bots post in rooms without asking each time. Use Auto-send in the room header to turn it off.": "你的 Bot 会在房间里直接发送，不再每次询问。要关掉，用房间标题里的自动发送。",
   "Turn on auto-send": "打开自动发送",
   "Approve and always auto-send in this room": "批准并在此房间始终自动发送",
@@ -19824,9 +19825,10 @@ function About({ text: text2, title = "About" }) {
     long ? (0, import_react44.createElement)("button", { type: "button", className: "bt-more", onClick: () => setOpen((value) => !value) }, open ? t("Show less") : t("Show more")) : null
   );
 }
-function SharedRoomApprovals({ room: room2 }) {
+function SharedRoomApprovals({ room: room2, roster }) {
   const [items, setItems] = (0, import_react44.useState)(room2.pendingApprovals ?? []);
   const [busy, setBusy] = (0, import_react44.useState)("");
+  const [error, setError] = (0, import_react44.useState)("");
   const face = room2.origin === "face-to-face";
   (0, import_react44.useEffect)(() => {
     const haveItems = Array.isArray(room2.pendingApprovals) && room2.pendingApprovals.length;
@@ -19845,11 +19847,15 @@ function SharedRoomApprovals({ room: room2 }) {
     };
   }, [room2.id, room2.viewerRole, room2.pendingCount, face]);
   if (room2.viewerRole !== "invited-owner" && !face || !items.length) return null;
+  const locked = lockedWindow(roster);
   const decide = async (item, decision) => {
     setBusy(item.id);
     try {
       await call("social/decide-approval", { approvalId: item.id, decision });
       setItems((list) => list.filter((entry) => entry.id !== item.id));
+      setError("");
+    } catch (failure) {
+      setError(failure?.message || t("The decision was not sent."));
     } finally {
       setBusy("");
     }
@@ -19862,9 +19868,11 @@ function SharedRoomApprovals({ room: room2 }) {
       "div",
       { key: item.id, className: "bt-set-row" },
       (0, import_react44.createElement)("span", { className: "bt-set-copy" }, item.text || item.summary || item.to || t("Outbound reply")),
-      (0, import_react44.createElement)("button", { type: "button", className: "bt-mini", disabled: busy === item.id, onClick: () => decide(item, "approve") }, t("Approve")),
-      (0, import_react44.createElement)("button", { type: "button", className: "bt-mini", disabled: busy === item.id, onClick: () => decide(item, "reject") }, t("Reject"))
-    ))
+      locked ? null : (0, import_react44.createElement)("button", { type: "button", className: "bt-mini", disabled: busy === item.id, onClick: () => decide(item, "approve") }, t("Approve")),
+      locked ? null : (0, import_react44.createElement)("button", { type: "button", className: "bt-mini", disabled: busy === item.id, onClick: () => decide(item, "reject") }, t("Reject"))
+    )),
+    locked ? (0, import_react44.createElement)("div", { className: "bt-hint bt-read-only", role: "status" }, hostText(roster.readOnly)) : null,
+    error ? (0, import_react44.createElement)("div", { className: "bt-error", role: "alert" }, error) : null
   );
 }
 function FaceMembers({ room: room2, roster, actions }) {
@@ -19908,7 +19916,7 @@ function GroupOverview({ room: room2, roster, actions, usage, useWork, openSetti
       room2.sharedFrom.invitedAt ? (0, import_react44.createElement)("div", { className: "bt-set-hint" }, t("Invited {when}", { when: new Date(room2.sharedFrom.invitedAt).toLocaleString() })) : null,
       (0, import_react44.createElement)("button", { type: "button", className: "bt-more", onClick: leaveMine }, t("Remove my Bot"))
     ) : face ? null : room2.notice ? (0, import_react44.createElement)(About, { key: "notice", title: "Notice", text: room2.notice }) : (0, import_react44.createElement)("button", { key: "notice", type: "button", className: "bt-more", onClick: openSettings }, t("Add a notice in group settings")),
-    face ? null : (0, import_react44.createElement)(SharedRoomApprovals, { key: "approvals", room: room2 }),
+    face ? null : (0, import_react44.createElement)(SharedRoomApprovals, { key: "approvals", room: room2, roster }),
     face && (room2.face?.status === "candidate" || room2.face?.status === "joining") ? null : face ? (0, import_react44.createElement)(FaceMembers, { key: "members", room: room2, roster, actions }) : (0, import_react44.createElement)(
       "div",
       { key: "members" },

core/src/client/details-drawer.js

@@ -104,9 +104,10 @@ function About({ text, title = 'About' }) {
 
 // A group's overview: what members read, who is in it, and who answers. Changes are
 // made in its settings.
-function SharedRoomApprovals({ room }) {
+function SharedRoomApprovals({ room, roster }) {
   const [items, setItems] = useState(room.pendingApprovals ?? [])
   const [busy, setBusy] = useState('')
+  const [error, setError] = useState('')
   const face = room.origin === 'face-to-face'
   useEffect(() => {
     const haveItems = Array.isArray(room.pendingApprovals) && room.pendingApprovals.length
@@ -122,19 +123,37 @@ function SharedRoomApprovals({ room }) {
     return () => { alive = false }
   }, [room.id, room.viewerRole, room.pendingCount, face])
   if ((room.viewerRole !== 'invited-owner' && !face) || !items.length) return null
+  // A decision is a write, so a read-only window cannot make one: the host would answer
+  // `bot/read-only` (or `bot/blocked-shared` in a room with several members) after the click.
+  // The card keeps its headline, count and rows; the two buttons give way to one line carrying the
+  // host's own sentence, the same shape the drawer's Settings view uses (#2381).
+  const locked = lockedWindow(roster)
   const decide = async (item, decision) => {
     setBusy(item.id)
     try {
       await call('social/decide-approval', { approvalId: item.id, decision })
       setItems(list => list.filter(entry => entry.id !== item.id))
+      setError('')
+    } catch (failure) {
+      // A refused decision used to vanish here: the card kept drawing the same two buttons and said
+      // nothing, so the click read as a no-op. The host's own words are the only reason to show.
+      setError(failure?.message || t('The decision was not sent.'))
     } finally { setBusy('') }
   }
   return h('div', { key: 'approvals', className: 'bt-shared-wait', 'data-flybot': 'room-pending-approvals' },
     h('div', { className: 'bt-section-title' }, t('Waiting for your approval'), h('span', { className: 'bt-chip' }, t('{count} to approve', { count: items.length }))),
     items.map(item => h('div', { key: item.id, className: 'bt-set-row' },
       h('span', { className: 'bt-set-copy' }, item.text || item.summary || item.to || t('Outbound reply')),
-      h('button', { type: 'button', className: 'bt-mini', disabled: busy === item.id, onClick: () => decide(item, 'approve') }, t('Approve')),
-      h('button', { type: 'button', className: 'bt-mini', disabled: busy === item.id, onClick: () => decide(item, 'reject') }, t('Reject')))))
+      locked
+        ? null
+        : h('button', { type: 'button', className: 'bt-mini', disabled: busy === item.id, onClick: () => decide(item, 'approve') }, t('Approve')),
+      locked
+        ? null
+        : h('button', { type: 'button', className: 'bt-mini', disabled: busy === item.id, onClick: () => decide(item, 'reject') }, t('Reject')))),
+    locked
+      ? h('div', { className: 'bt-hint bt-read-only', role: 'status' }, hostText(roster.readOnly))
+      : null,
+    error ? h('div', { className: 'bt-error', role: 'alert' }, error) : null)
 }
 
 // One row per person, each with the Bots they brought; my own Bot opens its chat (#229, #457).
@@ -178,7 +197,7 @@ function GroupOverview({ room, roster, actions, usage, useWork, openSettings })
         : room.notice
           ? h(About, { key: 'notice', title: 'Notice', text: room.notice })
           : h('button', { key: 'notice', type: 'button', className: 'bt-more', onClick: openSettings }, t('Add a notice in group settings')),
-    face ? null : h(SharedRoomApprovals, { key: 'approvals', room }),
+    face ? null : h(SharedRoomApprovals, { key: 'approvals', room, roster }),
     face && (room.face?.status === 'candidate' || room.face?.status === 'joining')
       ? null
       : face

core/src/client/i18n.js

@@ -898,6 +898,7 @@ const ZH = {
   '{first}, {second} and {count} more are waiting for your approval': '{first}、{second} 等 {total} 个等你批准',
   'Waiting for your approval': '等你批准',
   'Waiting for their approval': '等对方批准',
+  'The decision was not sent.': '决定没有发出去。',
   'Your Bots post in rooms without asking each time. Use Auto-send in the room header to turn it off.': '你的 Bot 会在房间里直接发送，不再每次询问。要关掉，用房间标题里的自动发送。',
   'Turn on auto-send': '打开自动发送',
   'Approve and always auto-send in this room': '批准并在此房间始终自动发送',

core/test/room-approvals-readonly.test.js

@@ -0,0 +1,192 @@
+// #2381: a read-only window still drew `Approve` / `Reject` on the shared room overview's
+// "Waiting for your approval" card (`details-drawer.js` `SharedRoomApprovals`). The card reaches
+// such a window on purpose — an approval is a read — but `decide` is not, so the two buttons could
+// only earn `bot/read-only` after the click, and the component swallowed that failure in a
+// `try/finally` with no `catch`, so the card said nothing at all.
+// The drawer's own Settings view already asks `lockedWindow(roster)` and prints the host's
+// sentence (`details-drawer.js:270`); this card never did. It now does, and a refused decision
+// writes one `role="alert"` line instead of vanishing.
+// React is replaced by the same tiny renderer `approvals-readonly.test.js` uses (no jsdom here);
+// the host call is replaced too, so "the click was refused and the card said why" is measurable.
+import assert from 'node:assert/strict'
+import { registerHooks } from 'node:module'
+import { test } from 'node:test'
+
+const live = { inst: null, idx: 0 }
+const reactStub = {
+  createElement: (type, props, ...children) => ({ type, props: { ...(props ?? {}), children: children.flat(Infinity) } }),
+  Fragment: 'F',
+  Component: class {},
+  useState(init) {
+    const { inst } = live
+    const i = live.idx++
+    if (!(i in inst.hooks)) inst.hooks[i] = { value: typeof init === 'function' ? init() : init }
+    const slot = inst.hooks[i]
+    return [slot.value, (next) => { slot.value = typeof next === 'function' ? next(slot.value) : next; inst.dirty = true }]
+  },
+  useEffect(fn, deps) {
+    const { inst } = live
+    const i = live.idx++
+    if (!(i in inst.hooks)) inst.hooks[i] = { deps: undefined }
+    inst.effects.push([i, fn, deps])
+  },
+}
+// The drawer keeps a `useRef` object and memoises values, so these three answer the way React does.
+reactStub.useRef = (value) => {
+  const { inst } = live
+  const i = live.idx++
+  if (!(i in inst.hooks)) inst.hooks[i] = { current: value }
+  return inst.hooks[i]
+}
+reactStub.useMemo = (fn) => {
+  const { inst } = live
+  const i = live.idx++
+  if (!(i in inst.hooks)) inst.hooks[i] = { value: fn() }
+  return inst.hooks[i].value
+}
+reactStub.useCallback = (fn) => {
+  const { inst } = live
+  const i = live.idx++
+  if (!(i in inst.hooks)) inst.hooks[i] = { value: fn }
+  return inst.hooks[i].value
+}
+for (const name of ['useLayoutEffect', 'useContext', 'useReducer', 'useId', 'useSyncExternalStore', 'useTransition', 'useDeferredValue', 'memo', 'forwardRef', 'createContext', 'createPortal', 'cloneElement', 'isValidElement', 'startTransition']) reactStub[name] = (value) => value
+globalThis.__reactStub = reactStub
+// Every host call the card makes, in order: `[endpoint, payload]`.
+const hostCalls = []
+let hostAnswer = () => Promise.resolve([])
+globalThis.__hostCall = (endpoint, payload) => { hostCalls.push([endpoint, payload]); return hostAnswer(endpoint, payload) }
+registerHooks({
+  resolve(specifier, context, nextResolve) {
+    if (specifier === 'react') return { url: 'data:text/javascript,const r=globalThis.__reactStub;export default r;export const {createElement,Fragment,Component,useState,useEffect,useRef,useMemo,useCallback,useLayoutEffect,useContext,useReducer,useId,useSyncExternalStore,useTransition,useDeferredValue,memo,forwardRef,createContext,createPortal,cloneElement,isValidElement,startTransition}=r', shortCircuit: true }
+    if (specifier === './api.js' && context.parentURL?.includes('/src/client/')) return { url: 'data:text/javascript,export const call = (endpoint, payload) => globalThis.__hostCall(endpoint, payload)', shortCircuit: true }
+    if (specifier === 'react-dom') return { url: 'data:text/javascript,export const createPortal = node => node', shortCircuit: true }
+    if (specifier === '@deepseek-ai/dsh-client-ui-primitives') return { url: 'data:text/javascript,export const MarkdownText = node => node;export const FishLogo = node => node', shortCircuit: true }
+    return nextResolve(specifier, context)
+  },
+})
+globalThis.window ??= globalThis
+globalThis.document ??= { createElement: () => ({ setAttribute() {} }), head: { appendChild() {} }, getElementById: () => null, addEventListener() {}, removeEventListener() {} }
+
+const LOCK = 'Another FlyBot window has this Bot team open, so this window is read-only.'
+const FAILURE = 'This Bot team is open in another window, so the decision was not sent.'
+const host = { id: 'b1', name: 'Helper', ownerId: 'm1', sessionId: 's1' }
+const writable = {
+  self: { memberId: 'm1' }, bots: [host], byId: { b1: host }, rooms: [], roomsById: {}, readOnly: '',
+  exchangeTurns: {}, prefs: {}, mainBotIds: [], connectors: [], memory: false, questions: {},
+}
+const locked = { ...writable, readOnly: LOCK }
+const room = {
+  id: 'r1', name: 'Team room', origin: 'group', viewerRole: 'invited-owner', pendingCount: 1,
+  pendingApprovals: [{ id: 'a1', text: 'Which file?' }], members: [], participants: [],
+}
+const withRoom = (roster) => ({ ...roster, roomsById: { r1: room }, rooms: [room] })
+
+const map = () => new Map()
+const fakeList = { byId: {}, projectionsBySession: {} }
+const actions = {
+  closeOverlay() {}, openSession() {}, openSettings() {}, openBrowser() {},
+  updateRoom: () => Promise.resolve(), notify() {}, reportError() {},
+}
+const drawer = (roster) => ({
+  hooks: {}, effects: [], rendered: false,
+  props: {
+    sessionId: 'r1', startTab: 'details', hasStrip: false, roster: withRoom(roster), actions,
+    useSessions: (pick) => pick(fakeList), useSessionStatus: (pick) => pick(map()),
+    useActivity: (pick) => pick(map()), useWork: (pick) => pick(map()),
+    useBrowserTabs: (pick) => pick(map()), useBrowserPages: (pick) => pick(map()),
+  },
+})
+// The drawer returns elements; React draws the function components inside them, so they are
+// expanded here the same way — every function component runs on the one hook runtime above.
+// The hook slots are positional, so the tree is expanded in one pass: the drawer runs first, then
+// each function component in tree order, the way the settings-drawer test draws it.
+const expand = (node) => {
+  if (Array.isArray(node)) return node.map(expand)
+  if (!node || typeof node !== 'object') return node
+  const props = node.props ?? {}
+  if (typeof node.type === 'function' && !node.type.prototype?.render) return expand(node.type(props))
+  return { ...node, props: { ...props, children: expand(props.children) } }
+}
+const render = (inst) => {
+  live.inst = inst
+  live.idx = 0
+  inst.effects = []
+  const tree = expand(DetailsDrawer(inst.props))
+  inst.rendered = true
+  return tree
+}
+const nodes = (node, out = []) => {
+  if (Array.isArray(node)) { for (const child of node) nodes(child, out); return out }
+  if (!node || typeof node !== 'object') return out
+  out.push(node)
+  nodes(node.props?.children, out)
+  return out
+}
+const text = (node) => {
+  if (node === null || node === undefined) return ''
+  if (Array.isArray(node)) return node.map(text).join('')
+  if (typeof node !== 'object') return String(node)
+  return text(node.props?.children)
+}
+const hasClass = (node, name) => typeof node.props?.className === 'string' && node.props.className.split(' ').includes(name)
+const DECISIONS = ['Approve', 'Reject']
+const decisionLabels = tree => nodes(tree)
+  .filter(node => node.type === 'button')
+  .map(node => text(node.props?.children))
+  .filter(label => DECISIONS.includes(label))
+  .sort()
+const hints = tree => nodes(tree).filter(node => hasClass(node, 'bt-read-only'))
+const alerts = tree => nodes(tree).filter(node => node.props?.role === 'alert')
+const button = (tree, label) => nodes(tree).find(node => node.type === 'button' && text(node.props?.children) === label)
+
+let DetailsDrawer
+test.before(async () => {
+  ;({ DetailsDrawer } = await import('../src/client/details-drawer.js'))
+})
+
+test('#2381: a read-only window draws neither Approve nor Reject on the room approval card', () => {
+  const drawn = render(drawer(locked))
+  assert.deepEqual(decisionLabels(drawn), [], `a read-only room card still offers ${JSON.stringify(decisionLabels(drawn))}`)
+  // What the card is for stays: the headline, the count and the row's own text.
+  const shown = text(drawn)
+  assert.match(shown, /Waiting for your approval/)
+  assert.match(shown, /Which file\?/)
+})
+
+test('#2381: the read-only room card carries the host sentence in one status line', () => {
+  const drawn = render(drawer(locked))
+  const lines = hints(drawn)
+  assert.equal(lines.length, 1, 'one read-only line on the card')
+  assert.equal(lines[0].props.role, 'status', 'the line is announced')
+  assert.equal(text(lines[0].props?.children), LOCK, "the line is the host's own sentence")
+  // A writable window draws no line and keeps both decisions: the zero above is the lock, not a
+  // card that never drew anything.
+  const open = render(drawer(writable))
+  assert.deepEqual(hints(open), [], 'a writable window draws no read-only line')
+  assert.deepEqual(decisionLabels(open), ['Approve', 'Reject'], 'a writable window keeps both decisions')
+})
+
+test('#2381: a refused decision says so in one alert line instead of vanishing', async () => {
+  hostCalls.length = 0
+  hostAnswer = () => Promise.reject(new Error(FAILURE))
+  const inst = drawer(writable)
+  try {
+    const approve = button(render(inst), 'Approve')
+    assert.ok(approve, 'the writable card draws Approve')
+    // The press is observed here: a rejection the component itself never catches escapes as an
+    // unhandled one, which is the shape this test exists to end.
+    const pressed = approve.props.onClick({ preventDefault() {}, stopPropagation() {} })
+    const escaped = await Promise.resolve(pressed).then(() => null, (error) => error)
+    await new Promise(resolve => setTimeout(resolve, 0))
+    const drawn = render(inst)
+    assert.deepEqual(hostCalls.map(([endpoint]) => endpoint), ['social/decide-approval'], 'the press reached the host')
+    const lines = alerts(drawn)
+    assert.equal(lines.length, 1, 'the refusal is written once')
+    assert.equal(text(lines[0].props?.children), FAILURE, 'the line is the real reason, not a swallowed failure')
+    assert.deepEqual(decisionLabels(drawn), ['Approve', 'Reject'], 'the card is still there after a refusal')
+    assert.equal(escaped, null, 'the refusal did not escape the component')
+  } finally {
+    hostAnswer = () => Promise.resolve([])
+  }
+})