diff --git a/changelog.d/2253-readonly-context-menu-writes.fixed.md b/changelog.d/2253-readonly-context-menu-writes.fixed.md new file mode 100644 index 00000000..2470583b --- /dev/null +++ b/changelog.d/2253-readonly-context-menu-writes.fixed.md @@ -0,0 +1 @@ +A read-only FlyBot window no longer draws the Bot and group right-click menu's writes — Pin and Unpin, Duplicate, Make Main Bot, Remove Main Bot role, Hide from sidebar, Rename group, Delete and Delete group — because the host refuses every one of them; Edit Bot, Group settings, Mark as Unread and Copy conversation ID stay, and a writable window is unchanged ([#2253](https://github.com/ThinkFlowLab/FlyBot/issues/2253)). diff --git a/core/client.js b/core/client.js index 5e766986..852a1d35 100644 --- a/core/client.js +++ b/core/client.js @@ -19450,6 +19450,7 @@ function ContextMenu({ menu, roster, actions, leaving = false }) { const isMainBot = isMainOf(roster, menu.id); const isTile = roster.mainBotId === menu.id; const mainCount = (roster.mainBotIds ?? []).length; + const locked = typeof roster?.readOnly === "string" && roster.readOnly.trim() !== ""; const groups = bot?.remote ? [ [ [t("Mark as Unread"), UnreadIcon, () => actions.markUnread(menu.id)] @@ -19461,26 +19462,26 @@ function ContextMenu({ menu, roster, actions, leaving = false }) { [[t("Copy conversation ID"), CopyIcon, () => navigator.clipboard?.writeText(currentPart(bot))]] ] : [ [ - isTile ? null : [entry.pinned ? t("Unpin") : t("Pin"), PinIcon, () => actions.setFlags(menu.id, { pinned: !entry.pinned })], + isTile || locked ? null : [entry.pinned ? t("Unpin") : t("Pin"), PinIcon, () => actions.setFlags(menu.id, { pinned: !entry.pinned })], [t("Mark as Unread"), UnreadIcon, () => actions.markUnread(menu.id)] ], [ // One way to edit a Bot (#385): its name is edited in Edit Bot too. bot ? [t("Edit Bot"), GearIcon, () => actions.openSettings("bots", bot.id)] : null, room2 ? [t("Group settings"), GearIcon, () => actions.openSettings("groups", room2.id)] : null, - room2 ? [t("Rename group"), PencilIcon, () => actions.renameBot(room2.id)] : null, - bot ? [t("Duplicate"), DuplicateIcon, async () => { + room2 && !locked ? [t("Rename group"), PencilIcon, () => actions.renameBot(room2.id)] : null, + bot && !locked ? [t("Duplicate"), DuplicateIcon, async () => { const copy2 = await actions.duplicateBot(bot.id); actions.openSession(copy2.id); }] : null, - bot && !isMainBot ? [t("Make Main Bot"), StarLineIcon, () => actions.setMain(bot.id, true)] : null, - bot && isMainBot && mainCount > 1 ? [t("Remove Main Bot role"), StarLineIcon, () => actions.setMain(bot.id, false)] : null + bot && !isMainBot && !locked ? [t("Make Main Bot"), StarLineIcon, () => actions.setMain(bot.id, true)] : null, + bot && isMainBot && mainCount > 1 && !locked ? [t("Remove Main Bot role"), StarLineIcon, () => actions.setMain(bot.id, false)] : null ], bot ? pluginItems(structure, bot, actions, roster).map(([label, run]) => [label, LinkIcon, run]) : [], [[t("Copy conversation ID"), CopyIcon, () => navigator.clipboard?.writeText(bot ? currentPart(bot) : menu.id)]], [ - isMainBot ? null : [t("Hide from sidebar"), HideIcon, () => actions.setFlags(menu.id, { hidden: true })], - isMainBot ? null : [room2 ? t("Delete group") : t("Delete"), TrashIcon, () => bot ? actions.deleteBot(bot.id) : actions.deleteRoom(menu.id), true] + isMainBot || locked ? null : [t("Hide from sidebar"), HideIcon, () => actions.setFlags(menu.id, { hidden: true })], + isMainBot || locked ? null : [room2 ? t("Delete group") : t("Delete"), TrashIcon, () => bot ? actions.deleteBot(bot.id) : actions.deleteRoom(menu.id), true] ] ]; const shown = groups.map((group) => group.filter(Boolean)).filter((group) => group.length > 0); diff --git a/core/src/client/context-menu.js b/core/src/client/context-menu.js index 1200de95..9f1c8438 100644 --- a/core/src/client/context-menu.js +++ b/core/src/client/context-menu.js @@ -31,6 +31,9 @@ export function ContextMenu({ menu, roster, actions, leaving = false }) { const isMainBot = isMainOf(roster, menu.id) const isTile = roster.mainBotId === menu.id const mainCount = (roster.mainBotIds ?? []).length + // A window that holds the team read-only draws none of the writes the host refuses + // (#2232, #2238 read the sentence the same way): a string that trims to nothing is writable. + const locked = typeof roster?.readOnly === 'string' && roster.readOnly.trim() !== '' const groups = bot?.remote ? [ [ @@ -44,23 +47,23 @@ export function ContextMenu({ menu, roster, actions, leaving = false }) { ] : [ [ - isTile ? null : [entry.pinned ? t('Unpin') : t('Pin'), PinIcon, () => actions.setFlags(menu.id, { pinned: !entry.pinned })], + isTile || locked ? null : [entry.pinned ? t('Unpin') : t('Pin'), PinIcon, () => actions.setFlags(menu.id, { pinned: !entry.pinned })], [t('Mark as Unread'), UnreadIcon, () => actions.markUnread(menu.id)], ], [ // One way to edit a Bot (#385): its name is edited in Edit Bot too. bot ? [t('Edit Bot'), GearIcon, () => actions.openSettings('bots', bot.id)] : null, room ? [t('Group settings'), GearIcon, () => actions.openSettings('groups', room.id)] : null, - room ? [t('Rename group'), PencilIcon, () => actions.renameBot(room.id)] : null, - bot ? [t('Duplicate'), DuplicateIcon, async () => { const copy = await actions.duplicateBot(bot.id); actions.openSession(copy.id) }] : null, - bot && !isMainBot ? [t('Make Main Bot'), StarLineIcon, () => actions.setMain(bot.id, true)] : null, - bot && isMainBot && mainCount > 1 ? [t('Remove Main Bot role'), StarLineIcon, () => actions.setMain(bot.id, false)] : null, + room && !locked ? [t('Rename group'), PencilIcon, () => actions.renameBot(room.id)] : null, + bot && !locked ? [t('Duplicate'), DuplicateIcon, async () => { const copy = await actions.duplicateBot(bot.id); actions.openSession(copy.id) }] : null, + bot && !isMainBot && !locked ? [t('Make Main Bot'), StarLineIcon, () => actions.setMain(bot.id, true)] : null, + bot && isMainBot && mainCount > 1 && !locked ? [t('Remove Main Bot role'), StarLineIcon, () => actions.setMain(bot.id, false)] : null, ], bot ? pluginItems(structure, bot, actions, roster).map(([label, run]) => [label, LinkIcon, run]) : [], [[t('Copy conversation ID'), CopyIcon, () => navigator.clipboard?.writeText(bot ? currentPart(bot) : menu.id)]], [ - isMainBot ? null : [t('Hide from sidebar'), HideIcon, () => actions.setFlags(menu.id, { hidden: true })], - isMainBot ? null : [room ? t('Delete group') : t('Delete'), TrashIcon, () => (bot ? actions.deleteBot(bot.id) : actions.deleteRoom(menu.id)), true], + isMainBot || locked ? null : [t('Hide from sidebar'), HideIcon, () => actions.setFlags(menu.id, { hidden: true })], + isMainBot || locked ? null : [room ? t('Delete group') : t('Delete'), TrashIcon, () => (bot ? actions.deleteBot(bot.id) : actions.deleteRoom(menu.id)), true], ], ] const shown = groups.map(group => group.filter(Boolean)).filter(group => group.length > 0) diff --git a/core/test/context-menu-readonly.test.js b/core/test/context-menu-readonly.test.js new file mode 100644 index 00000000..e4f6b433 --- /dev/null +++ b/core/test/context-menu-readonly.test.js @@ -0,0 +1,143 @@ +// A window that holds the team read-only draws none of the core right-click menu's writes. +// The host refuses each of them with `bot/read-only` (`core/src/host/channel.js`: none of +// set-flags, duplicate-bot, set-main, delete-bot, delete-room is in READS), so an offered +// control can only produce a refusal toast. The menu reads `roster.readOnly` the way the +// sidebar header and the scheduled-tasks page read it: a string that trims to nothing is a +// writable window (#2176, #2199, #2232, #2238). +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { fileURLToPath } from 'node:url' +import { runInNewContext } from 'node:vm' +import { build } from 'esbuild' +import { indexRoster } from '../src/client/sources.js' + +const STUB = ` +export const call = async () => ({}) +` +const stubApi = { + name: 'stub-api', + setup(context) { + context.onResolve({ filter: /(^|\/)api\.js$/ }, () => ({ path: 'flybot-api-stub', namespace: 'stub' })) + context.onLoad({ filter: /.*/, namespace: 'stub' }, () => ({ contents: STUB, loader: 'js' })) + }, +} + +const bundle = await build({ + entryPoints: [fileURLToPath(new URL('../src/client/context-menu.js', import.meta.url))], + bundle: true, format: 'cjs', platform: 'node', write: false, + external: ['react', 'react-dom'], + plugins: [stubApi], +}) + +// A render pass over the hook shim: one pass is enough here, because the menu reads its inputs +// and draws; hooks only have to answer what the component reads during that one call. +let pass = null +const sandbox = { + module: { exports: {} }, exports: null, Date, Promise, console, + window: { addEventListener: () => {}, removeEventListener: () => {}, innerWidth: 1400, innerHeight: 900 }, + document: { body: {} }, + require: (name) => { + const platform = { + react: { + createElement: (type, props, ...children) => ({ type, props: props ?? {}, children: children.flat(Infinity) }), + useState: (initial) => { + const index = pass === null ? 0 : pass.index++ + if (pass === null) return [typeof initial === 'function' ? initial() : initial, () => {}] + if (!(index in pass.values)) pass.values[index] = typeof initial === 'function' ? initial() : initial + return [pass.values[index], (next) => { pass.values[index] = typeof next === 'function' ? next(pass.values[index]) : next }] + }, + useEffect: () => {}, useLayoutEffect: () => {}, + useRef: current => ({ current }), useCallback: fn => fn, useMemo: fn => fn(), Fragment: 'fragment', + Component: class { constructor(props) { this.props = props } setState() {} render() { return null } }, + }, + 'react-dom': { createPortal: element => element }, + } + if (!(name in platform)) throw new Error(`no platform module ${name}`) + return platform[name] + }, +} +sandbox.exports = sandbox.module.exports +runInNewContext(bundle.outputFiles[0].text, sandbox) +const { ContextMenu } = sandbox.module.exports + +const renderNode = (node) => { + if (Array.isArray(node)) return node.map(renderNode) + if (node === null || typeof node !== 'object') return node + if (typeof node.type === 'function') return renderNode(node.type({ ...node.props, children: node.children })) + return { ...node, children: (node.children ?? []).map(renderNode) } +} +const draw = (element) => { + pass = { index: 0, values: [] } + const tree = renderNode(element()) + pass = null + return tree +} + +const textOf = node => (node?.children ?? []).flat(Infinity).filter(child => typeof child === 'string').join('') +// Every row the menu draws, by its visible label. +const menuItems = (node, found = []) => { + if (node === null || typeof node !== 'object') return found + if (node.props?.role === 'menuitem') found.push(textOf(node)) + for (const child of node.children ?? []) menuItems(child, found) + return found +} + +const bots = [ + { id: 'chief', name: 'Chief', sessionId: 'chief', parts: ['chief'] }, + { id: 'chief2', name: 'Chief Two', sessionId: 'chief2', parts: ['chief2'], pinned: true }, + { id: 'writer', name: 'Writer', sessionId: 'writer', parts: ['writer'] }, +] +const rooms = [{ id: 'room', name: 'Launch', members: ['chief', 'writer'] }] +const rosterOf = (extra = {}) => indexRoster({ bots, rooms, mainBotId: 'chief', mainBotIds: ['chief', 'chief2'], ...extra }) + +// Every label whose click writes through the host, wherever the menu puts it. +const WRITES = ['Pin', 'Unpin', 'Duplicate', 'Make Main Bot', 'Remove Main Bot role', 'Hide from sidebar', 'Delete', 'Rename group', 'Delete group'] +const KEEPS = ['Mark as Unread', 'Copy conversation ID'] + +const actions = () => ({ + closeMenu: () => {}, markUnread: () => {}, openSettings: () => {}, setFlags: () => {}, renameBot: () => {}, + duplicateBot: async () => ({ id: 'copy' }), openSession: () => {}, setMain: () => {}, deleteBot: () => {}, + deleteRoom: () => {}, reportError: () => {}, +}) +const drawMenu = (roster, menu) => draw(() => ContextMenu({ menu, roster, actions: actions(), leaving: false })) +const menusOf = roster => ({ + bot: drawMenu(roster, { kind: 'bot', id: 'writer', x: 10, y: 10 }), + main: drawMenu(roster, { kind: 'bot', id: 'chief2', x: 10, y: 10 }), + room: drawMenu(roster, { kind: 'room', id: 'room', x: 10, y: 10 }), +}) +const labelsOf = menus => Object.fromEntries(Object.entries(menus).map(([name, tree]) => [name, menuItems(tree)])) + +test('a read-only window offers no write in any core menu', () => { + const labels = labelsOf(menusOf(rosterOf({ readOnly: 'This window holds the team read-only.' }))) + for (const [menu, items] of Object.entries(labels)) { + assert.deepEqual(items.filter(label => WRITES.includes(label)), [], `the ${menu} menu still offers a write`) + } +}) + +test('a writable window keeps every write exactly as it is', () => { + const labels = labelsOf(menusOf(rosterOf())) + assert.deepEqual(labels.bot, ['Pin', 'Mark as Unread', 'Edit Bot', 'Duplicate', 'Make Main Bot', 'Copy conversation ID', 'Hide from sidebar', 'Delete']) + assert.deepEqual(labels.main, ['Unpin', 'Mark as Unread', 'Edit Bot', 'Duplicate', 'Remove Main Bot role', 'Copy conversation ID']) + assert.deepEqual(labels.room, ['Pin', 'Mark as Unread', 'Group settings', 'Rename group', 'Copy conversation ID', 'Hide from sidebar', 'Delete group']) +}) + +test('the rows that do not write stay in both windows', () => { + const readOnly = labelsOf(menusOf(rosterOf({ readOnly: 'read only' }))) + const writable = labelsOf(menusOf(rosterOf())) + for (const labels of [readOnly, writable]) { + assert.deepEqual([...labels.bot, ...labels.room].filter(label => KEEPS.includes(label)).sort(), ['Copy conversation ID', 'Copy conversation ID', 'Mark as Unread', 'Mark as Unread']) + assert.ok(labels.bot.includes('Edit Bot')) + assert.ok(labels.room.includes('Group settings')) + } + assert.deepEqual(readOnly.bot, ['Mark as Unread', 'Edit Bot', 'Copy conversation ID']) + assert.deepEqual(readOnly.main, ['Mark as Unread', 'Edit Bot', 'Copy conversation ID']) + assert.deepEqual(readOnly.room, ['Mark as Unread', 'Group settings', 'Copy conversation ID']) +}) + +test('a host sentence that trims to nothing is a writable window', () => { + for (const readOnly of [undefined, '', ' ']) { + const labels = labelsOf(menusOf(rosterOf({ readOnly }))) + assert.ok(labels.bot.includes('Delete'), `readOnly ${JSON.stringify(readOnly)} hid the writes`) + assert.ok(labels.room.includes('Delete group'), `readOnly ${JSON.stringify(readOnly)} hid the room writes`) + } +}) diff --git a/docs/subsystems/core-client.md b/docs/subsystems/core-client.md index c7c4ffd5..6783f459 100644 --- a/docs/subsystems/core-client.md +++ b/docs/subsystems/core-client.md @@ -224,7 +224,7 @@ The member's own chat keeps its `busyAcks[sessionId]` bubble and its visibility - Slot `flybot.settings.page` (`0003-flybot-api-client`, types in `flybot.d.ts`, #337) adds a page to the Settings list right after Connectors: `{ id, order, label, icon?, component }`. `SettingsDialog` (`settings.js`) lists `useSettingsPages()` entries under the key `plugin:` and draws the component through `SettingsPageSlot` (a failing component draws nothing, it does not close the dialog); a page whose plugin is gone opens General. `label` is an English key for the host `t()`; the component gets `{ roster, actions, call, run, t, Section, Row }` (the host translator, the Settings section card and row, so the page matches the others and reads zh from `i18n.js`). Social registers **Bot Social** (the server, see [social](social.md)). Test: `core/test/settings-page-slot.test.js`. - Slot `flybot.account.header` (`0003-flybot-api-client`) is a compact block at the top of the account menu (`FlyBotAccountHeader`); social uses it for the owner's own handle (#181). - Slot `flybot.new-chat.action` (`0003-flybot-api-client`, types in `flybot.d.ts`) is a row on the sidebar **+** new-chat menu, after New group chat. Plugins register `{ id, order, label, hint?, visible?, run }`; the picker looks `label` / `hint` up with `t()` like the neighbouring rows. A string returned from `run` is shown as a toast (`actions.notify`). Social registers Face-to-face room and Add friend; see [plugin-runtime](plugin-runtime.md) and [social](social.md). -- Sidebar **+** (`mode === 'new'`) opens a short action menu (`data-menu`, no To:/composer) until **New chat** or **New group chat**. That menu does not list individual Bots (Chief and the rest stay behind **New chat**). The Agent row in that FlyBot menu is **Add coding agent**; every row has its own mark (`ChatIcon`, `BotIcon`, `PeopleIcon`, a plugin row's `icon`, drawn through `SlotIcon` behind its own error boundary with the people mark as fallback; a function or memo/forwardRef component, `TerminalIcon`), one verb style (New … / Add …) and one short hint with no closing period (#385: *Message one of your Bots*, *Set up a Bot with its own role*, *Chat with several of your Bots*, *Meet people nearby with a 6-digit number*, *Paste an invite link or code*, *A coding session without a Bot*). A Bot's right-click menu has **Edit Bot** and no separate **Rename Bot** (the name is edited there); a `flybot.sidebar.structure` entry's `menuItems` rows follow in their own group, and the menu hands each one the window's read-only flag (`readOnly: true` only when `roster.readOnly` trims to a non-empty string), so an entry hides a write the host would refuse instead of painting its failure (#2238); **Add DSH Agent** stays the DSH product string on Agent rows elsewhere (#126, #181). +- Sidebar **+** (`mode === 'new'`) opens a short action menu (`data-menu`, no To:/composer) until **New chat** or **New group chat**. That menu does not list individual Bots (Chief and the rest stay behind **New chat**). The Agent row in that FlyBot menu is **Add coding agent**; every row has its own mark (`ChatIcon`, `BotIcon`, `PeopleIcon`, a plugin row's `icon`, drawn through `SlotIcon` behind its own error boundary with the people mark as fallback; a function or memo/forwardRef component, `TerminalIcon`), one verb style (New … / Add …) and one short hint with no closing period (#385: *Message one of your Bots*, *Set up a Bot with its own role*, *Chat with several of your Bots*, *Meet people nearby with a 6-digit number*, *Paste an invite link or code*, *A coding session without a Bot*). A Bot's right-click menu has **Edit Bot** and no separate **Rename Bot** (the name is edited there); a `flybot.sidebar.structure` entry's `menuItems` rows follow in their own group, and the menu hands each one the window's read-only flag (`readOnly: true` only when `roster.readOnly` trims to a non-empty string), so an entry hides a write the host would refuse instead of painting its failure (#2238); a read-only window (`roster.readOnly` a non-empty string, read the same way) also draws none of the menu's own core writes — no Pin/Unpin, Duplicate, Make Main Bot, Remove Main Bot role, Hide from sidebar, Rename group, Delete or Delete group, because the host refuses every one of them with `bot/read-only` — while Edit Bot, Group settings, Mark as Unread and Copy conversation ID stay; the Agent row's menu is not a Bot menu and keeps its own rows. Tests: `core/test/context-menu-readonly.test.js`; **Add DSH Agent** stays the DSH product string on Agent rows elsewhere (#126, #181). - Group bubbles, notices, and failed-reply lines never show a raw `` wrap or a developer error (`llm-deepseek: no API key`). `remoteMessageText` unwraps stored fences; `failureLabel` maps missing-key / unknown model failures to a short friendly line. Details stay in Settings → Feedback. - A closed face room's status line already says who left (#1103). `GroupReplies` (`leaveNoticeCoveredByClose` in `cells.js`) does not also draw a short leave notice for that person (`SelfB1025 left the room`, or `Coder (SelfB1025) left`). A leave while the room is still open, a leave the status line does not name, and every other notice stay. The social plugin uses the same rule when it decides whether to post the line. Test: `core/test/tool-cells.test.js`. - `GroupReplies` draws a `user` reply with `other: true` (another member's face-room line, #180) as `PersonMessage` (`cells.js`): the person's name, the person mark (`PersonMark` in `mark.js`) and a received `--bt-bubble` bubble, not a centred `bt-event` notice and not the sent `--bt-user` style; own lines keep the right-hand bubble (#229). `other` comes only from the host's person-line post (`hooks.js` `postHuman`), never from text. Another home member's line in a shared group (`UserMessageCell`, author stamped by the host) gets the same person mark and the received surface (`.bt-umsg-other .bt-ubody`). `PersonMark({ name })` draws the person's initial on a color picked from the name (`personColor`, the core default-color hash of `person:`), so one person looks the same on every home and two people are told apart; without a name it is the plain silhouette (#182). diff --git a/patches/ds-bot/0003-flybot-api-client.patch b/patches/ds-bot/0003-flybot-api-client.patch index 57f0b9ac..d0beffcb 100644 --- a/patches/ds-bot/0003-flybot-api-client.patch +++ b/patches/ds-bot/0003-flybot-api-client.patch @@ -1244,7 +1244,7 @@ diff --git a/src/client/context-menu.js b/src/client/context-menu.js const ref = useRef(null) const [place, setPlace] = useState({ left: menu.x, top: menu.y }) useLayoutEffect(() => { -@@ -27,28 +27,41 @@ export function ContextMenu({ menu, roster, actions, leaving = false }) { +@@ -27,28 +27,44 @@ export function ContextMenu({ menu, roster, actions, leaving = false }) { const isMainBot = isMainOf(roster, menu.id) const isTile = roster.mainBotId === menu.id const mainCount = (roster.mainBotIds ?? []).length @@ -1268,6 +1268,9 @@ diff --git a/src/client/context-menu.js b/src/client/context-menu.js - isMainBot ? null : [room ? t('Delete group') : t('Delete'), TrashIcon, () => (bot ? actions.deleteBot(bot.id) : actions.deleteRoom(menu.id)), true], - ], - ].map(group => group.filter(Boolean)).filter(group => group.length > 0) ++ // A window that holds the team read-only draws none of the writes the host refuses ++ // (#2232, #2238 read the sentence the same way): a string that trims to nothing is writable. ++ const locked = typeof roster?.readOnly === 'string' && roster.readOnly.trim() !== '' + const groups = bot?.remote + ? [ + [ @@ -1281,23 +1284,23 @@ diff --git a/src/client/context-menu.js b/src/client/context-menu.js + ] + : [ + [ -+ isTile ? null : [entry.pinned ? t('Unpin') : t('Pin'), PinIcon, () => actions.setFlags(menu.id, { pinned: !entry.pinned })], ++ isTile || locked ? null : [entry.pinned ? t('Unpin') : t('Pin'), PinIcon, () => actions.setFlags(menu.id, { pinned: !entry.pinned })], + [t('Mark as Unread'), UnreadIcon, () => actions.markUnread(menu.id)], + ], + [ + // One way to edit a Bot (#385): its name is edited in Edit Bot too. + bot ? [t('Edit Bot'), GearIcon, () => actions.openSettings('bots', bot.id)] : null, + room ? [t('Group settings'), GearIcon, () => actions.openSettings('groups', room.id)] : null, -+ room ? [t('Rename group'), PencilIcon, () => actions.renameBot(room.id)] : null, -+ bot ? [t('Duplicate'), DuplicateIcon, async () => { const copy = await actions.duplicateBot(bot.id); actions.openSession(copy.id) }] : null, -+ bot && !isMainBot ? [t('Make Main Bot'), StarLineIcon, () => actions.setMain(bot.id, true)] : null, -+ bot && isMainBot && mainCount > 1 ? [t('Remove Main Bot role'), StarLineIcon, () => actions.setMain(bot.id, false)] : null, ++ room && !locked ? [t('Rename group'), PencilIcon, () => actions.renameBot(room.id)] : null, ++ bot && !locked ? [t('Duplicate'), DuplicateIcon, async () => { const copy = await actions.duplicateBot(bot.id); actions.openSession(copy.id) }] : null, ++ bot && !isMainBot && !locked ? [t('Make Main Bot'), StarLineIcon, () => actions.setMain(bot.id, true)] : null, ++ bot && isMainBot && mainCount > 1 && !locked ? [t('Remove Main Bot role'), StarLineIcon, () => actions.setMain(bot.id, false)] : null, + ], + bot ? pluginItems(structure, bot, actions, roster).map(([label, run]) => [label, LinkIcon, run]) : [], + [[t('Copy conversation ID'), CopyIcon, () => navigator.clipboard?.writeText(bot ? currentPart(bot) : menu.id)]], + [ -+ isMainBot ? null : [t('Hide from sidebar'), HideIcon, () => actions.setFlags(menu.id, { hidden: true })], -+ isMainBot ? null : [room ? t('Delete group') : t('Delete'), TrashIcon, () => (bot ? actions.deleteBot(bot.id) : actions.deleteRoom(menu.id)), true], ++ isMainBot || locked ? null : [t('Hide from sidebar'), HideIcon, () => actions.setFlags(menu.id, { hidden: true })], ++ isMainBot || locked ? null : [room ? t('Delete group') : t('Delete'), TrashIcon, () => (bot ? actions.deleteBot(bot.id) : actions.deleteRoom(menu.id)), true], + ], + ] + const shown = groups.map(group => group.filter(Boolean)).filter(group => group.length > 0) diff --git a/patches/ds-bot/0008-client-bundle.patch b/patches/ds-bot/0008-client-bundle.patch index 5c6b3d81..7b9eb9b6 100644 --- a/patches/ds-bot/0008-client-bundle.patch +++ b/patches/ds-bot/0008-client-bundle.patch @@ -24118,11 +24118,12 @@ diff --git a/client.js b/client.js const bot = roster.byId[menu.id]; const room2 = roster.roomsById[menu.id]; const entry = bot ?? room2; -@@ -10884,13 +10884,22 @@ function ContextMenu({ menu, roster, actions, leaving = false }) { +@@ -10884,37 +10884,49 @@ function ContextMenu({ menu, roster, actions, leaving = false }) { const isMainBot = isMainOf(roster, menu.id); const isTile = roster.mainBotId === menu.id; const mainCount = (roster.mainBotIds ?? []).length; - const groups = [ ++ const locked = typeof roster?.readOnly === "string" && roster.readOnly.trim() !== ""; + const groups = bot?.remote ? [ + [ + [t("Mark as Unread"), UnreadIcon, () => actions.markUnread(menu.id)] @@ -24134,7 +24135,8 @@ diff --git a/client.js b/client.js + [[t("Copy conversation ID"), CopyIcon, () => navigator.clipboard?.writeText(currentPart(bot))]] + ] : [ [ - isTile ? null : [entry.pinned ? t("Unpin") : t("Pin"), PinIcon, () => actions.setFlags(menu.id, { pinned: !entry.pinned })], +- isTile ? null : [entry.pinned ? t("Unpin") : t("Pin"), PinIcon, () => actions.setFlags(menu.id, { pinned: !entry.pinned })], ++ isTile || locked ? null : [entry.pinned ? t("Unpin") : t("Pin"), PinIcon, () => actions.setFlags(menu.id, { pinned: !entry.pinned })], [t("Mark as Unread"), UnreadIcon, () => actions.markUnread(menu.id)] ], [ @@ -24142,16 +24144,25 @@ diff --git a/client.js b/client.js + // One way to edit a Bot (#385): its name is edited in Edit Bot too. bot ? [t("Edit Bot"), GearIcon, () => actions.openSettings("bots", bot.id)] : null, room2 ? [t("Group settings"), GearIcon, () => actions.openSettings("groups", room2.id)] : null, - room2 ? [t("Rename group"), PencilIcon, () => actions.renameBot(room2.id)] : null, -@@ -10901,20 +10901,22 @@ function ContextMenu({ menu, roster, actions, leaving = false }) { - bot && !isMainBot ? [t("Make Main Bot"), StarLineIcon, () => actions.setMain(bot.id, true)] : null, - bot && isMainBot && mainCount > 1 ? [t("Remove Main Bot role"), StarLineIcon, () => actions.setMain(bot.id, false)] : null +- room2 ? [t("Rename group"), PencilIcon, () => actions.renameBot(room2.id)] : null, +- bot ? [t("Duplicate"), DuplicateIcon, async () => { ++ room2 && !locked ? [t("Rename group"), PencilIcon, () => actions.renameBot(room2.id)] : null, ++ bot && !locked ? [t("Duplicate"), DuplicateIcon, async () => { + const copy2 = await actions.duplicateBot(bot.id); + actions.openSession(copy2.id); + }] : null, +- bot && !isMainBot ? [t("Make Main Bot"), StarLineIcon, () => actions.setMain(bot.id, true)] : null, +- bot && isMainBot && mainCount > 1 ? [t("Remove Main Bot role"), StarLineIcon, () => actions.setMain(bot.id, false)] : null ++ bot && !isMainBot && !locked ? [t("Make Main Bot"), StarLineIcon, () => actions.setMain(bot.id, true)] : null, ++ bot && isMainBot && mainCount > 1 && !locked ? [t("Remove Main Bot role"), StarLineIcon, () => actions.setMain(bot.id, false)] : null ], + bot ? pluginItems(structure, bot, actions, roster).map(([label, run]) => [label, LinkIcon, run]) : [], [[t("Copy conversation ID"), CopyIcon, () => navigator.clipboard?.writeText(bot ? currentPart(bot) : menu.id)]], [ - isMainBot ? null : [t("Hide from sidebar"), HideIcon, () => actions.setFlags(menu.id, { hidden: true })], - isMainBot ? null : [room2 ? t("Delete group") : t("Delete"), TrashIcon, () => bot ? actions.deleteBot(bot.id) : actions.deleteRoom(menu.id), true] +- isMainBot ? null : [t("Hide from sidebar"), HideIcon, () => actions.setFlags(menu.id, { hidden: true })], +- isMainBot ? null : [room2 ? t("Delete group") : t("Delete"), TrashIcon, () => bot ? actions.deleteBot(bot.id) : actions.deleteRoom(menu.id), true] ++ isMainBot || locked ? null : [t("Hide from sidebar"), HideIcon, () => actions.setFlags(menu.id, { hidden: true })], ++ isMainBot || locked ? null : [room2 ? t("Delete group") : t("Delete"), TrashIcon, () => bot ? actions.deleteBot(bot.id) : actions.deleteRoom(menu.id), true] ] - ].map((group) => group.filter(Boolean)).filter((group) => group.length > 0); - return (0, import_react29.createElement)( diff --git a/patches/ds-bot/files/test/context-menu-readonly.test.js b/patches/ds-bot/files/test/context-menu-readonly.test.js new file mode 100644 index 00000000..e4f6b433 --- /dev/null +++ b/patches/ds-bot/files/test/context-menu-readonly.test.js @@ -0,0 +1,143 @@ +// A window that holds the team read-only draws none of the core right-click menu's writes. +// The host refuses each of them with `bot/read-only` (`core/src/host/channel.js`: none of +// set-flags, duplicate-bot, set-main, delete-bot, delete-room is in READS), so an offered +// control can only produce a refusal toast. The menu reads `roster.readOnly` the way the +// sidebar header and the scheduled-tasks page read it: a string that trims to nothing is a +// writable window (#2176, #2199, #2232, #2238). +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { fileURLToPath } from 'node:url' +import { runInNewContext } from 'node:vm' +import { build } from 'esbuild' +import { indexRoster } from '../src/client/sources.js' + +const STUB = ` +export const call = async () => ({}) +` +const stubApi = { + name: 'stub-api', + setup(context) { + context.onResolve({ filter: /(^|\/)api\.js$/ }, () => ({ path: 'flybot-api-stub', namespace: 'stub' })) + context.onLoad({ filter: /.*/, namespace: 'stub' }, () => ({ contents: STUB, loader: 'js' })) + }, +} + +const bundle = await build({ + entryPoints: [fileURLToPath(new URL('../src/client/context-menu.js', import.meta.url))], + bundle: true, format: 'cjs', platform: 'node', write: false, + external: ['react', 'react-dom'], + plugins: [stubApi], +}) + +// A render pass over the hook shim: one pass is enough here, because the menu reads its inputs +// and draws; hooks only have to answer what the component reads during that one call. +let pass = null +const sandbox = { + module: { exports: {} }, exports: null, Date, Promise, console, + window: { addEventListener: () => {}, removeEventListener: () => {}, innerWidth: 1400, innerHeight: 900 }, + document: { body: {} }, + require: (name) => { + const platform = { + react: { + createElement: (type, props, ...children) => ({ type, props: props ?? {}, children: children.flat(Infinity) }), + useState: (initial) => { + const index = pass === null ? 0 : pass.index++ + if (pass === null) return [typeof initial === 'function' ? initial() : initial, () => {}] + if (!(index in pass.values)) pass.values[index] = typeof initial === 'function' ? initial() : initial + return [pass.values[index], (next) => { pass.values[index] = typeof next === 'function' ? next(pass.values[index]) : next }] + }, + useEffect: () => {}, useLayoutEffect: () => {}, + useRef: current => ({ current }), useCallback: fn => fn, useMemo: fn => fn(), Fragment: 'fragment', + Component: class { constructor(props) { this.props = props } setState() {} render() { return null } }, + }, + 'react-dom': { createPortal: element => element }, + } + if (!(name in platform)) throw new Error(`no platform module ${name}`) + return platform[name] + }, +} +sandbox.exports = sandbox.module.exports +runInNewContext(bundle.outputFiles[0].text, sandbox) +const { ContextMenu } = sandbox.module.exports + +const renderNode = (node) => { + if (Array.isArray(node)) return node.map(renderNode) + if (node === null || typeof node !== 'object') return node + if (typeof node.type === 'function') return renderNode(node.type({ ...node.props, children: node.children })) + return { ...node, children: (node.children ?? []).map(renderNode) } +} +const draw = (element) => { + pass = { index: 0, values: [] } + const tree = renderNode(element()) + pass = null + return tree +} + +const textOf = node => (node?.children ?? []).flat(Infinity).filter(child => typeof child === 'string').join('') +// Every row the menu draws, by its visible label. +const menuItems = (node, found = []) => { + if (node === null || typeof node !== 'object') return found + if (node.props?.role === 'menuitem') found.push(textOf(node)) + for (const child of node.children ?? []) menuItems(child, found) + return found +} + +const bots = [ + { id: 'chief', name: 'Chief', sessionId: 'chief', parts: ['chief'] }, + { id: 'chief2', name: 'Chief Two', sessionId: 'chief2', parts: ['chief2'], pinned: true }, + { id: 'writer', name: 'Writer', sessionId: 'writer', parts: ['writer'] }, +] +const rooms = [{ id: 'room', name: 'Launch', members: ['chief', 'writer'] }] +const rosterOf = (extra = {}) => indexRoster({ bots, rooms, mainBotId: 'chief', mainBotIds: ['chief', 'chief2'], ...extra }) + +// Every label whose click writes through the host, wherever the menu puts it. +const WRITES = ['Pin', 'Unpin', 'Duplicate', 'Make Main Bot', 'Remove Main Bot role', 'Hide from sidebar', 'Delete', 'Rename group', 'Delete group'] +const KEEPS = ['Mark as Unread', 'Copy conversation ID'] + +const actions = () => ({ + closeMenu: () => {}, markUnread: () => {}, openSettings: () => {}, setFlags: () => {}, renameBot: () => {}, + duplicateBot: async () => ({ id: 'copy' }), openSession: () => {}, setMain: () => {}, deleteBot: () => {}, + deleteRoom: () => {}, reportError: () => {}, +}) +const drawMenu = (roster, menu) => draw(() => ContextMenu({ menu, roster, actions: actions(), leaving: false })) +const menusOf = roster => ({ + bot: drawMenu(roster, { kind: 'bot', id: 'writer', x: 10, y: 10 }), + main: drawMenu(roster, { kind: 'bot', id: 'chief2', x: 10, y: 10 }), + room: drawMenu(roster, { kind: 'room', id: 'room', x: 10, y: 10 }), +}) +const labelsOf = menus => Object.fromEntries(Object.entries(menus).map(([name, tree]) => [name, menuItems(tree)])) + +test('a read-only window offers no write in any core menu', () => { + const labels = labelsOf(menusOf(rosterOf({ readOnly: 'This window holds the team read-only.' }))) + for (const [menu, items] of Object.entries(labels)) { + assert.deepEqual(items.filter(label => WRITES.includes(label)), [], `the ${menu} menu still offers a write`) + } +}) + +test('a writable window keeps every write exactly as it is', () => { + const labels = labelsOf(menusOf(rosterOf())) + assert.deepEqual(labels.bot, ['Pin', 'Mark as Unread', 'Edit Bot', 'Duplicate', 'Make Main Bot', 'Copy conversation ID', 'Hide from sidebar', 'Delete']) + assert.deepEqual(labels.main, ['Unpin', 'Mark as Unread', 'Edit Bot', 'Duplicate', 'Remove Main Bot role', 'Copy conversation ID']) + assert.deepEqual(labels.room, ['Pin', 'Mark as Unread', 'Group settings', 'Rename group', 'Copy conversation ID', 'Hide from sidebar', 'Delete group']) +}) + +test('the rows that do not write stay in both windows', () => { + const readOnly = labelsOf(menusOf(rosterOf({ readOnly: 'read only' }))) + const writable = labelsOf(menusOf(rosterOf())) + for (const labels of [readOnly, writable]) { + assert.deepEqual([...labels.bot, ...labels.room].filter(label => KEEPS.includes(label)).sort(), ['Copy conversation ID', 'Copy conversation ID', 'Mark as Unread', 'Mark as Unread']) + assert.ok(labels.bot.includes('Edit Bot')) + assert.ok(labels.room.includes('Group settings')) + } + assert.deepEqual(readOnly.bot, ['Mark as Unread', 'Edit Bot', 'Copy conversation ID']) + assert.deepEqual(readOnly.main, ['Mark as Unread', 'Edit Bot', 'Copy conversation ID']) + assert.deepEqual(readOnly.room, ['Mark as Unread', 'Group settings', 'Copy conversation ID']) +}) + +test('a host sentence that trims to nothing is a writable window', () => { + for (const readOnly of [undefined, '', ' ']) { + const labels = labelsOf(menusOf(rosterOf({ readOnly }))) + assert.ok(labels.bot.includes('Delete'), `readOnly ${JSON.stringify(readOnly)} hid the writes`) + assert.ok(labels.room.includes('Delete group'), `readOnly ${JSON.stringify(readOnly)} hid the room writes`) + } +})