From 8dbfc9dee7b9c33e9d4f5247b951e2f228c9891e Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Tue, 3 Nov 2026 19:16:23 +0800 Subject: [PATCH] client: a read-only Inbox keeps only Open and says why In a read-only window every Inbox action was a write the host refuses, and the window said nothing in place about it. The rows and their Open stay; the four writes (answer a question, the task input, Accept, Send back) are not drawn and one sentence at the top of the dialog carries the host's own read-only words. An approval keeps Reject and Allow: approval-answer is one of the reads a read-only channel allows. The gate reuses lockedWindow from schedules.js; a writable window renders the same tree, pinned by a sha256 of the fixture. Fixes #2284 --- .../2284-inbox-readonly-actions.fixed.md | 1 + core/client.js | 6 +- core/src/client/inbox.js | 12 +- core/test/inbox-readonly.test.js | 157 ++++++++++++++++++ docs/subsystems/core-client.md | 2 +- patches/ds-bot/0008-client-bundle.patch | 8 +- patches/ds-bot/files/src/client/inbox.js | 12 +- .../ds-bot/files/test/inbox-readonly.test.js | 157 ++++++++++++++++++ 8 files changed, 345 insertions(+), 10 deletions(-) create mode 100644 changelog.d/2284-inbox-readonly-actions.fixed.md create mode 100644 core/test/inbox-readonly.test.js create mode 100644 patches/ds-bot/files/test/inbox-readonly.test.js diff --git a/changelog.d/2284-inbox-readonly-actions.fixed.md b/changelog.d/2284-inbox-readonly-actions.fixed.md new file mode 100644 index 000000000..1a5b116a1 --- /dev/null +++ b/changelog.d/2284-inbox-readonly-actions.fixed.md @@ -0,0 +1 @@ +A read-only window's Inbox now keeps every row and its **Open** action and draws no write the host would refuse: no answer field or **Submit** on a question or a task input, and no **Accept** or **Send back** on a result. One sentence at the top of the dialog carries the host's own read-only words, the same hint the other read-only surfaces draw. An approval keeps **Reject** and **Allow**, because `approval-answer` is one of the reads a read-only channel allows, and a writable window draws exactly the tree it drew before. ([#2284](https://github.com/ThinkFlowLab/FlyBot/issues/2284)) diff --git a/core/client.js b/core/client.js index f079fe294..5e7669861 100644 --- a/core/client.js +++ b/core/client.js @@ -9091,6 +9091,7 @@ function InboxCard({ card, roster, now, busy, error, onAnswer, onOpen }) { const answerable = card.kind === "approval" && card.approval?.answerable === true; const evidence = card.kind === "task" && card.status === "result" ? card.task?.result?.links ?? [] : []; const sentBack = card.kind === "task" && card.status === "task" ? card.task?.followUp?.note : void 0; + const locked = lockedWindow(roster); const target = card.kind === "task" ? card.task?.target?.href : void 0; const writeBack = card.kind === "task" && card.task?.writeBack?.state === "failed" ? card.task.writeBack : void 0; const unconfirmed = card.kind === "task" && card.task?.writeBack?.state === "unconfirmed" ? card.task.writeBack : void 0; @@ -9130,8 +9131,7 @@ function InboxCard({ card, roster, now, busy, error, onAnswer, onOpen }) { }, t("Comment goes to {target}", { target: target.replace(/^https:\/\/github\.com\//, "") }))) : null, writeBack ? (0, import_react14.createElement)("span", { className: "bt-inbox-writeback", role: "alert", title: writeBack.error }, t("Could not comment on {target}: {error}", { target: writeBack.target, error: writeBack.error })) : null, unconfirmed ? (0, import_react14.createElement)("span", { className: "bt-inbox-writeback", title: unconfirmed.target }, t("Could not confirm the comment on {target} went out", { target: unconfirmed.target })) : null, - (0, import_react14.createElement)(AnswerFields, { card, busy, onAnswer }), - (0, import_react14.createElement)(ResultActions, { card, busy, onAnswer }), + ...locked ? [] : [(0, import_react14.createElement)(AnswerFields, { card, busy, onAnswer }), (0, import_react14.createElement)(ResultActions, { card, busy, onAnswer })], error ? (0, import_react14.createElement)("span", { className: "bt-inbox-error", role: "alert" }, error) : null ), (0, import_react14.createElement)( @@ -9178,6 +9178,7 @@ function InboxDialog({ roster, useWork, useSessions, actions, leaving = false }) const timer = setTimeout(() => setFeedback(""), FEEDBACK_MS); return () => clearTimeout(timer); }, [feedback]); + const locked = lockedWindow(roster); const model = inboxModel({ roster, work, tasks, sessions, workflows, now }); const groups = visibleNeeds(model, answered); const answer = (card, choice) => { @@ -9213,6 +9214,7 @@ function InboxDialog({ roster, useWork, useSessions, actions, leaving = false }) (0, import_react14.createElement)( "div", { className: "bt-mem-body bt-inbox-body" }, + ...locked ? [(0, import_react14.createElement)("p", { key: "readonly", className: "bt-hint bt-read-only", role: "status" }, hostText(roster.readOnly))] : [], (0, import_react14.createElement)("div", { className: "bt-inbox-feedback", role: "status", "aria-live": "polite" }, feedback), GROUPS.map((group) => { const settled = group.id === "done" ? model.doneAll ?? groups.done : null; diff --git a/core/src/client/inbox.js b/core/src/client/inbox.js index 72a9d7a19..8a81f0a52 100644 --- a/core/src/client/inbox.js +++ b/core/src/client/inbox.js @@ -18,6 +18,7 @@ import { botNames, currentPart, previewOf, relativeShort } from './text.js' import { hostText, t } from './i18n.js' import { failureSentence } from '../shared/failure-copy.js' import { columnOf } from '../shared/task-board.js' +import { lockedWindow } from './schedules.js' // Done (and a failed task) ages out after 7 days. export const KEEP_MS = 7 * 86_400_000 @@ -549,6 +550,10 @@ export function InboxCard({ card, roster, now, busy, error, onAnswer, onOpen }) // Where a decision is written back (#1154): the issue or PR the task came from is on the card, and // a comment that could not be posted says so once, on the card, instead of staying in a log. A // comment nobody posted within a day is `unconfirmed` (#1172): its own quiet line, not an error. + // #2284: a read-only window keeps every row and its Open action and draws no write the host + // would refuse (answer a question, the task input, Accept, Send back). An approval keeps its + // Reject and Allow: `approval-answer` is one of the reads a read-only channel allows. + const locked = lockedWindow(roster) const target = card.kind === 'task' ? card.task?.target?.href : undefined const writeBack = card.kind === 'task' && card.task?.writeBack?.state === 'failed' ? card.task.writeBack : undefined const unconfirmed = card.kind === 'task' && card.task?.writeBack?.state === 'unconfirmed' ? card.task.writeBack : undefined @@ -572,8 +577,7 @@ export function InboxCard({ card, roster, now, busy, error, onAnswer, onOpen }) }, t('Comment goes to {target}', { target: target.replace(/^https:\/\/github\.com\//, '') }))) : null, writeBack ? h('span', { className: 'bt-inbox-writeback', role: 'alert', title: writeBack.error }, t('Could not comment on {target}: {error}', { target: writeBack.target, error: writeBack.error })) : null, unconfirmed ? h('span', { className: 'bt-inbox-writeback', title: unconfirmed.target }, t('Could not confirm the comment on {target} went out', { target: unconfirmed.target })) : null, - h(AnswerFields, { card, busy, onAnswer }), - h(ResultActions, { card, busy, onAnswer }), + ...(locked ? [] : [h(AnswerFields, { card, busy, onAnswer }), h(ResultActions, { card, busy, onAnswer })]), error ? h('span', { className: 'bt-inbox-error', role: 'alert' }, error) : null), h('span', { className: 'bt-inbox-actions' }, answerable ? h('button', { type: 'button', className: 'bt-soft bt-soft-sm', disabled: busy, onClick: () => onAnswer(card, 'reject') }, t('Reject')) : null, @@ -622,6 +626,7 @@ export function InboxDialog({ roster, useWork, useSessions, actions, leaving = f const timer = setTimeout(() => setFeedback(''), FEEDBACK_MS) return () => clearTimeout(timer) }, [feedback]) + const locked = lockedWindow(roster) const model = inboxModel({ roster, work, tasks, sessions, workflows, now }) const groups = visibleNeeds(model, answered) const answer = (card, choice) => { @@ -654,6 +659,9 @@ export function InboxDialog({ roster, useWork, useSessions, actions, leaving = f h('span', { className: 'bt-mem-head-gap' }), h('button', { type: 'button', className: 'bt-icon-btn', 'aria-label': t('Close inbox'), autoFocus: true, onClick: close }, h(CloseIcon))), h('div', { className: 'bt-mem-body bt-inbox-body' }, + // #2284: one sentence at the top of the dialog, in the host's own words, so a window that + // draws no write says why instead of leaving the reads silent about it. + ...(locked ? [h('p', { key: 'readonly', className: 'bt-hint bt-read-only', role: 'status' }, hostText(roster.readOnly))] : []), h('div', { className: 'bt-inbox-feedback', role: 'status', 'aria-live': 'polite' }, feedback), GROUPS.map(group => { // #2050: the model keeps every finished row, so the group can draw the page's remainder diff --git a/core/test/inbox-readonly.test.js b/core/test/inbox-readonly.test.js new file mode 100644 index 000000000..da42e9cb9 --- /dev/null +++ b/core/test/inbox-readonly.test.js @@ -0,0 +1,157 @@ +import assert from 'node:assert/strict' +import { createHash } from 'node:crypto' +import { test } from 'node:test' +import { fileURLToPath } from 'node:url' +import { runInNewContext } from 'node:vm' +import { build } from 'esbuild' + +// #2284: in a read-only window every Inbox action was a write the host refuses, and the window +// said nothing in place about why. The rows and their Open action stay; the four writes +// (answer a question, the task input, Accept, Send back) are not drawn, one sentence at the +// top of the dialog carries the host's own words, and an approval keeps Reject and Allow — +// `approval-answer` is one of the reads a read-only channel allows. A writable window draws +// exactly the tree it drew before this change: the hash below is taken from the unmodified +// tree, so the same fixture must keep its bytes. +const bundle = await build({ + entryPoints: [fileURLToPath(new URL('../src/client/inbox.js', import.meta.url))], + bundle: true, format: 'cjs', platform: 'node', write: false, + external: ['react', 'react-dom', '@deepseek-ai/dsh-client-ui-primitives'], +}) +const module = { exports: {} } +let hookState = [] +let hookCursor = 0 +const effects = [] +const NOW = Date.UTC(2026, 10, 3, 9, 0, 0) +// The card's meta line is a relative time read from the wall clock; freeze it so the recorded +// tree below is the same picture on every run. +class FrozenDate extends Date { static now() { return NOW } } +runInNewContext(bundle.outputFiles[0].text, { + module, exports: module.exports, setTimeout: () => 0, clearTimeout: () => {}, setInterval: () => 0, clearInterval: () => {}, Date: FrozenDate, Promise, + require: (name) => { + const platform = { + react: { + createElement: (type, props, ...children) => ({ type, props: props ?? {}, children: children.flat(Infinity) }), + useState: initial => { + const index = hookCursor++ + if (!(index in hookState)) hookState[index] = typeof initial === 'function' ? initial() : initial + return [hookState[index], value => { hookState[index] = typeof value === 'function' ? value(hookState[index]) : value }] + }, + useEffect: fn => { effects.push(fn) }, useLayoutEffect: () => {}, useRef: current => ({ current }), useCallback: fn => fn, useMemo: fn => fn(), Fragment: 'fragment', + }, + 'react-dom': {}, + '@deepseek-ai/dsh-client-ui-primitives': {}, + } + if (!(name in platform)) throw new Error(`no platform module ${name}`) + return platform[name] + }, +}) +const { InboxCard, InboxDialog } = module.exports + +const OWN = new Set(['InboxCard', 'Status', 'AnswerFields', 'ResultActions']) +const expand = (node) => { + if (Array.isArray(node)) return node.flatMap(expand) + if (node === null || typeof node !== 'object') return node + if (OWN.has(node.type?.name)) return expand(node.type({ ...node.props, children: node.children })) + return { ...node, children: node.children.map(expand) } +} +const all = (node, out = []) => { + if (Array.isArray(node)) { for (const item of node) all(item, out); return out } + if (node && typeof node === 'object') { out.push(node); all(node.children, out) } + return out +} +const textOf = node => (typeof node === 'string' ? node : node && typeof node === 'object' ? node.children.map(textOf).join('') : '') +const byClass = (tree, name) => all(tree).filter(node => typeof node.props?.className === 'string' && node.props.className.split(' ').includes(name)) +const buttons = tree => all(tree).filter(node => node.type === 'button') +const labels = tree => buttons(tree).map(textOf) +const fields = tree => all(tree).filter(node => node.type === 'input') + +const LOCK = 'The Bot team is open in another dsh window.' +const bot = { id: 'b1', name: 'Anvil', ownerId: 'm1', sessionId: 's1' } +const writable = { self: { memberId: 'm1' }, bots: [bot], rooms: [], byId: { b1: bot }, roomsById: {}, readOnly: '' } +const locked = { ...writable, readOnly: LOCK } +const blank = { ...writable, readOnly: ' ' } +const missing = { ...writable } +delete missing.readOnly + +const card = (extra) => ({ bot, at: NOW - 60_000, title: 'A card', chat: 'b1', ...extra }) +const resultCard = card({ id: 'task:t4', kind: 'task', status: 'result', title: 'Draft is ready', task: { id: 't4', result: { summary: 'Draft is ready', links: [] } } }) +const questionCard = card({ id: 'question:q1', kind: 'question', status: 'input', title: 'Which branch?', question: { id: 'q1', question: 'Which branch?', options: [{ label: 'A' }, { label: 'B' }], allowCustom: true } }) +const inputCard = card({ id: 'task:t5', kind: 'task', status: 'input', title: 'Task five', task: { id: 't5', inputRequest: { id: 'i1', question: 'Which branch?' } } }) +const approvalCard = card({ id: 'approval:a1', kind: 'approval', status: 'approval', title: 'pwsh', chat: 'r1', approval: { id: 'a1', tool: 'pwsh', answerable: true, roomId: 'r1' } }) + +const draw = (roster, drawn, extra = {}) => { + hookState = [] + hookCursor = 0 + return expand(InboxCard({ card: drawn, roster, now: NOW, busy: false, error: '', onAnswer: () => {}, onOpen: () => {}, ...extra })) +} +const dialog = (roster) => { + hookState = [] + hookCursor = 0 + return expand(InboxDialog({ roster, useWork: pick => pick({}), useSessions: pick => pick({ byId: {} }), actions: { tasks: () => Promise.resolve([]) } })) +} +// A normalised tree: element name, the props that decide what is drawn, and the text. Functions +// are not part of it, so the hash answers "same picture", not "same closures". +const shape = node => (typeof node === 'string' ? node + : Array.isArray(node) ? node.map(shape) + : node && typeof node === 'object' + ? [node.type?.name ?? String(node.type), node.props?.className ?? '', node.props?.role ?? '', node.props?.title ?? '', node.props?.['aria-label'] ?? '', + ...(node.type === 'input' ? [node.props?.type ?? '', node.props?.disabled === true ? 'off' : 'on'] : []), + ...node.children.map(shape)] + : node) +const treeHash = (tree) => { + const bytes = Buffer.from(JSON.stringify(shape(tree)), 'utf8') + return { sha256: createHash('sha256').update(bytes).digest('hex'), bytes: bytes.length } +} + +// The writable picture, recorded on the unmodified tree. Both a blank and a missing `readOnly` +// stay on the writable path and draw exactly this. +const WRITABLE_SHA256 = 'b19823e6ca04389a3325e312aaf4807b84b55b882a255c3546d15c9c8ec23d41' +const WRITABLE_BYTES = 3088 + +test('a read-only result card keeps Open and draws no Accept and no Send back', () => { + assert.deepEqual(labels(draw(locked, resultCard)), ['Open']) +}) + +test('a read-only question card draws no options, no answer field and no Submit, only Open', () => { + const tree = draw(locked, questionCard) + assert.deepEqual(labels(tree), ['Open']) + assert.equal(fields(tree).length, 0) +}) + +test('a read-only task input draws no field and no Submit, only Open', () => { + const tree = draw(locked, inputCard) + assert.deepEqual(labels(tree), ['Open']) + assert.equal(fields(tree).length, 0) +}) + +test('the read-only dialog carries the host sentence once, at the top, as a status', () => { + const tree = dialog(locked) + const hints = byClass(tree, 'bt-read-only') + assert.equal(hints.length, 1) + assert.equal(hints[0].props.role, 'status') + assert.equal(textOf(hints[0]), LOCK) + const order = all(tree) + const hintAt = order.indexOf(hints[0]) + const firstGroup = order.findIndex(node => node.props?.className === 'bt-inbox-group') + assert.ok(hintAt >= 0 && hintAt < firstGroup, `hint at ${hintAt}, first group at ${firstGroup}`) + // The writable dialog says nothing it did not say before. + assert.equal(byClass(dialog(writable), 'bt-read-only').length, 0) +}) + +test('an approval in a read-only window keeps Reject, Allow and its always-choices (#2284)', () => { + assert.deepEqual(labels(draw(locked, approvalCard)), ['Reject', 'Allow once', 'Always allow for this Bot', 'Always allow for this group', 'Open']) +}) + +test('a writable window still draws the writes and keeps the recorded tree byte for byte', () => { + assert.deepEqual(labels(draw(writable, resultCard)), ['Send back', 'Accept', 'Open']) + assert.deepEqual(labels(draw(writable, questionCard)), ['A', 'B', 'Submit', 'Open']) + assert.deepEqual(labels(draw(writable, inputCard)), ['Submit', 'Open']) + // A blank or missing `readOnly` is the writable path, not the locked one. + for (const roster of [writable, blank, missing]) { + assert.deepEqual(labels(draw(roster, resultCard)), ['Send back', 'Accept', 'Open']) + assert.equal(byClass(dialog(roster), 'bt-read-only').length, 0) + } + const hash = treeHash([resultCard, questionCard, inputCard, approvalCard].map(item => draw(writable, item))) + assert.equal(hash.sha256, WRITABLE_SHA256) + assert.equal(hash.bytes, WRITABLE_BYTES) +}) diff --git a/docs/subsystems/core-client.md b/docs/subsystems/core-client.md index 935e74ad7..c7c4ffd5c 100644 --- a/docs/subsystems/core-client.md +++ b/docs/subsystems/core-client.md @@ -46,7 +46,7 @@ The browser side of the patched `ds-bot` core (`core/`): sources in `core/src/cl | `core/src/shared/approval-waiting.js` | The one rule for "this hold still waits for the owner's decision" (#734, #754): `isApprovalWaiting(status)` is true for `pending` and `pending_approval` and false for every settled spelling (`approved`, `auto`, `failed`, `rejected`, `stale`, missing). The host writes `pending`; Bot Social's own outbound hold arrives as `pending_approval`. Host and client both import this one file — the held-reply card, the approvals overlay and the chat bubble on this side, `hooks.js` (`decide()`, `heldOf()`, the cap, the in-flight filter, the cleanup) and `channel.js` (a row's wait badge) on the other — and the overlay asks the host for `[...WAITING_STATUSES, 'failed']`, so the two spellings cannot drift apart again on one side only. Tests: `core/test/approval-waiting.test.js`, `core/test/host-approval-status.test.js`. | | `core/src/shared/held-retry.js` | The one rule for "this failure can still be retried" (#968): `canRetry(record)` is false for `retry: false`, for `noRetry: true`, and for a record whose stored `error`/`code` names a terminal reason (`no such remote bot` / `is not shared with your Bots` / `is no longer shared with you` / `has not granted chat.public` / `bot_not_shared` / `bot_not_found`; a bare `capability_not_granted` is not one, #988), true otherwise. The host states the same answer in two spellings (`noRetry` on an approval, `retry: false` on a held reply), so both cards and Bot Social's own dock ask this one function instead of keeping a copy each; a hold written before the flag existed is read from its stored sentence at render time, with no state migration. Tests: `core/test/held-retry.test.js`. | | `core/src/client/schedules.js` | Settings → Scheduled tasks, a Bot's card and the details line. Each row has **Edit** (pencil; the row turns into the same form that creates a task, filled in, with **Save changes**; a new task opens with no target chosen — **Send to** reads "Pick a Bot or group chat" and **Create task** stays disabled until one is picked, so one click cannot create the task for the first Bot (#1334); an edited task keeps its time zone, a cron rule it cannot show is kept), **Copy** (`CopyRow`, #1287: the row turns into the create form's target picker, minus the chat the task already runs in, and **Copy** posts `schedule-create` with `copyFrom: { sessionId, id }` — the host writes the row, so the copy keeps the source's next run) and **Pause** / **Resume**; a paused row reads **Paused** (#354). A row with a recorded run adds one line under it: "Running since …", "Last run …", "Last run … · Stopped" or, in red, "Last run … · Failed: "; a failed run of a task that is not paused gets **Retry**, which runs the task once now (`actions.scheduleRetry`, #473). With Automation tasks off, the hint reads "Switch on Automation tasks on the Plugins page." and the button **Open Plugins** closes the Settings dialog and opens DSH's Plugins panel (`actions.selectPanel('plugins')`); the Harness settings dialog has no such switch (#378). The page opens with the team's **Quiet hours** card (#1166): **Off** with a **Turn on** button, or **On, 22:00–07:00** with the window's two time fields and **Save window** (the settings write refuses an invalid window, and the host's named refusal appears on the form's error line, as a `role="alert"` so it is announced and not only printed (#2232)). While quiet hours are on and an `every`/`cron` task exists, the view's `quietUnchecked` flag draws one line saying such a task can still run inside the window: that flag is the only place the rule is computed — the page does not scan the task list itself (#1199) — and it asks for the view again right after the settings write, so the line does not wait for the 30-second poll. Never a silent claim of silence, and no suppressed count. A repeat whose interval is not a whole number of minutes is stated as seconds (`Every 90 seconds`, #1651): the edit form opens holding that exact value in the **seconds** unit, and saving a task whose repeat was not touched sends the stored interval back unchanged instead of a rounded number of minutes. The details panel's line (`ScheduleSummary`) lists every task of that Bot, paused and finished ones included, because a Bot whose tasks are all paused has tasks while a Bot with none is the only empty state; each row answers when — `Next: