client: a read-only window's Settings → Workflows graph page draws the host sentence twice

changelog.d/2524-workflows-duplicate-sentence.fixed.md

@@ -0,0 +1 @@
+In a read-only window, **Settings → Workflows** no longer prints the host's read-only sentence twice: with a workflow graph open, the graph's edit bar drew a second copy of the sentence the Settings header already carries, under its own `role="status"`, so the same words sat on one screen twice and a screen reader announced them twice. The edit bar now shows **Back** only, the header's **Reload** stays the way out, and a writable window keeps **Edit** exactly as it was ([#2524](https://github.com/ThinkFlowLab/FlyBot/issues/2524)).

core/client.js

@@ -17295,7 +17295,7 @@ function WorkflowsView({ phase = "loading", pack = null, error = "", opened = nu
         { className: "bt-wf-actions" },
         (0, import_react37.createElement)("button", { type: "button", onClick: () => onBack?.() }, t("Back")),
         showUse ? (0, import_react37.createElement)("button", { type: "button", onClick: () => onUse?.(opened.source === "factory" ? opened.id : opened.fromFactory) }, t("Use this")) : null,
-        locked ? (0, import_react37.createElement)("span", { className: "bt-wf-status", role: "status" }, hostText(readOnly)) : opened?.source === "own" ? (0, import_react37.createElement)("button", { type: "button", onClick: () => onEditing?.(!editing) }, editing ? t("Finish editing") : t("Edit")) : null
+        locked ? null : opened?.source === "own" ? (0, import_react37.createElement)("button", { type: "button", onClick: () => onEditing?.(!editing) }, editing ? t("Finish editing") : t("Edit")) : null
       ),
       (0, import_react37.createElement)("h3", { className: "bt-wf-title" }, graph.name),
       graph.description ? (0, import_react37.createElement)("p", { className: "bt-wf-hint" }, graph.description) : null,

core/src/client/workflows.js

@@ -3,7 +3,7 @@
 // sidebar run uses. A read always settles: a host that never answers becomes
 // an error, an empty store says so, and the file path is not the page.
 import { createElement as h, useEffect, useRef, useState } from 'react'
-import { hostText, t } from './i18n.js'
+import { t } from './i18n.js'
 
 export const READ_MS = 8000
 
@@ -562,11 +562,12 @@ export function WorkflowsView({ phase = 'loading', pack = null, error = '', open
   const templates = pack?.templates ?? []
   const templateIds = new Set(templates.map((item) => item.id))
   const yours = (pack?.workflows ?? []).filter((item) => !templateIds.has(item.id))
-  // #2176: a window that holds the team read-only carries the host's own sentence. The page
-  // shows that sentence where Edit was and never opens the editor, so `workflow/graph` is not
-  // offered in the first place instead of being refused after the layout was built. `Use this`
-  // writes `workflow/create`, which the channel's READS list does not hold either, so a
-  // read-only window does not offer that one either.
+  // #2176: a window that holds the team read-only never opens the editor, so `workflow/graph`
+  // is not offered in the first place instead of being refused after the layout was built.
+  // `Use this` writes `workflow/create`, which the channel's READS list does not hold either,
+  // so a read-only window does not offer that one either. The host's standing sentence stays
+  // in the Settings header, once per page: this edit bar drew a second copy of it here, and a
+  // second live region is read out twice.
   const readOnly = typeof roster?.readOnly === 'string' ? roster.readOnly.trim() : ''
   const locked = readOnly !== ''
   const showUse = !locked && (opened?.source === 'factory' || opened?.fromFactory !== undefined)
@@ -600,7 +601,7 @@ export function WorkflowsView({ phase = 'loading', pack = null, error = '', open
         h('button', { type: 'button', onClick: () => onBack?.() }, t('Back')),
         showUse ? h('button', { type: 'button', onClick: () => onUse?.(opened.source === 'factory' ? opened.id : opened.fromFactory) }, t('Use this')) : null,
         locked
-          ? h('span', { className: 'bt-wf-status', role: 'status' }, hostText(readOnly))
+          ? null
           : opened?.source === 'own' ? h('button', { type: 'button', onClick: () => onEditing?.(!editing) }, editing ? t('Finish editing') : t('Edit')) : null),
       h('h3', { className: 'bt-wf-title' }, graph.name),
       graph.description ? h('p', { className: 'bt-wf-hint' }, graph.description) : null,

core/test/workflows-page.test.js

@@ -1389,13 +1389,15 @@ test('a forward edge into the next row turns above the tallest CJK named card (#
 // the layout — and then the state is discarded with one technical line.
 const READ_ONLY = 'another dsh holds the team, so this window is read-only'
 
-test('#2176: a read-only window renders the host sentence instead of Edit and offers no rename or delete', () => {
+// The host's standing sentence belongs to the Settings header, once per page: this edit bar
+// used to draw a second copy of it where Edit would be, and a second live region is read out
+// twice. The header keeps the sentence and its one Reload; the graph keeps Back.
+test('#2176: a read-only graph draws no editing control and leaves the host sentence to the header', () => {
   const view = renderView({ phase: 'ready', pack, opened: { source: 'own', id: 'wf-mine' }, error: '', roster: { bots: [], readOnly: READ_ONLY } })
   const editingControl = all(view).filter((entry) => entry.type === 'button' && (text(entry) === 'Edit' || text(entry) === 'Finish editing'))
   assert.deepEqual(editingControl.map(text), [], 'a read-only window offers no editing control')
-  const sentence = all(view).find((entry) => entry.props?.className === 'bt-wf-status' && text(entry) === READ_ONLY)
-  assert.ok(sentence, 'the host sentence is rendered on the page')
-  assert.equal(text(sentence), READ_ONLY, 'the page repeats the host wording, it does not invent one')
+  assert.equal(all(view).some((entry) => text(entry) === READ_ONLY), false, 'the edit bar adds no second copy of the host sentence')
+  assert.equal(all(view).some((entry) => entry.props?.role === 'status' && text(entry) === READ_ONLY), false, 'and no second live region carrying it')
   assert.equal(all(view).some((entry) => entry.props?.['data-port']), false, 'no port handles to drag')
   // The read side is untouched: the graph, its cards and the run chips still render.
   assert.ok(all(view).some((entry) => entry.props?.['data-node'] === 'coder'), 'the opened graph still renders')

core/test/workflows-readonly-once.test.js

@@ -0,0 +1,159 @@
+// The standing read-only sentence belongs to the dialog header, once per page
+// (settings-readonly-pages.test.js). That page list never opened a workflow, so it could not
+// see the second copy: in a window that holds the team read-only, Settings → Workflows drew
+// the host's sentence in the header and again in the graph's edit bar, a second live region
+// read out twice. The edit bar now draws Back only — the header's Reload is the whole
+// screen's way out — and a writable window keeps Edit where it was.
+import { test } from 'node:test'
+import assert from 'node:assert/strict'
+import { fileURLToPath } from 'node:url'
+import { runInNewContext } from 'node:vm'
+import { build } from 'esbuild'
+
+const { outputFiles } = await build({
+  stdin: {
+    contents: [
+      "export { SettingsDialog } from './src/client/settings.js'",
+      "export { WorkflowsView } from './src/client/workflows.js'",
+    ].join('\n'),
+    resolveDir: fileURLToPath(new URL('..', import.meta.url)),
+  },
+  bundle: true, format: 'cjs', platform: 'browser', write: false, logLevel: 'silent',
+  external: ['react', 'react-dom', '@deepseek-ai/dsh-client-ui-primitives'],
+})
+
+const h = (type, props, ...children) => ({ type, props: { ...props, children: children.flat(Infinity) } })
+
+// One render pass: the dialog and the graph canvas both call hooks, and nothing here
+// re-renders, so each component is drawn with a fresh set of slots.
+let slots = []
+let index = 0
+const hook = () => { const at = index++; return at }
+const react = {
+  createElement: h, Component: class {}, Fragment: Symbol('F'),
+  useMemo: (fn) => { const at = hook(); if (!(at in slots)) slots[at] = fn(); return slots[at] },
+  useState: (initial) => {
+    const at = hook()
+    if (!(at in slots)) slots[at] = typeof initial === 'function' ? initial() : initial
+    return [slots[at], (value) => { slots[at] = typeof value === 'function' ? value(slots[at]) : value }]
+  },
+  useCallback: (fn) => { const at = hook(); if (!(at in slots)) slots[at] = fn; return slots[at] },
+  useRef: (value) => { const at = hook(); if (!(at in slots)) slots[at] = { current: value }; return slots[at] },
+  useEffect: (fn) => { const at = hook(); if (!(at in slots)) slots[at] = fn },
+  useLayoutEffect: () => { hook() },
+  useId: () => { hook(); return 'id' },
+  useSyncExternalStore: (_subscribe, get) => { hook(); return get() },
+}
+const win = { location: { hash: '' }, addEventListener() {}, removeEventListener() {}, matchMedia: () => ({ matches: false, addEventListener() {}, removeEventListener() {} }) }
+const module = { exports: {} }
+runInNewContext(outputFiles[0].text, {
+  module, exports: module.exports, console, setTimeout, clearTimeout,
+  requestAnimationFrame: () => 0, cancelAnimationFrame() {},
+  document: { hidden: false, addEventListener() {}, removeEventListener() {}, createElement: () => ({ getContext: () => ({}) }) },
+  navigator: { language: 'en' },
+  localStorage: { getItem: () => null, setItem() {} },
+  window: win,
+  require: (name) => {
+    if (name === 'react') return react
+    if (name === 'react-dom') return { createPortal: node => node }
+    if (name === '@deepseek-ai/dsh-client-ui-primitives') return {}
+    throw new Error(`Unexpected platform module ${name}`)
+  },
+})
+const { SettingsDialog, WorkflowsView } = module.exports
+
+const SENTENCE = 'The Bot team in /home/xuke/.flybot is open in another dsh (Web or Desktop, pid 41 on host). Close the other one and reload.'
+
+const chief = { id: 'chief', name: 'Chief', role: 'Researcher', model: 'mock/m1', workDir: '/work/chief', instructions: 'Run the team.', pinned: true }
+const baseRoster = {
+  bots: [chief], byId: { chief }, rooms: [], roomsById: {}, mainBotIds: ['chief'], mainBotId: 'chief',
+  prefs: { theme: 'deepseek', motion: 'quiet', notifications: true },
+  models: [], connectors: [], capabilities: [], secrets: [], humans: [], remoteBots: [], memory: true,
+  questions: {}, exchangeTurns: {},
+}
+const lockedRoster = { ...baseRoster, readOnly: SENTENCE }
+
+const lane = {
+  id: 'w1',
+  name: 'Hotfix lane',
+  description: 'A short lane.',
+  chain: 'Planner → Coder',
+  nodes: [
+    { id: 'planner', name: 'Planner', role: 'Planner', x: 0, y: 0 },
+    { id: 'coder', name: 'Coder', role: 'Coder', x: 0, y: 108 },
+  ],
+  edges: [{ from: 'planner', to: 'coder' }],
+  layers: [['planner'], ['coder']],
+}
+const pack = { ok: true, templates: [], workflows: [lane] }
+
+// Every action a locked page could reach; a press that lands in one is a write.
+const actions = () => new Proxy({ openSettings() {}, closeSettings() {}, openSession() {}, notify() {}, showDetails() {} }, {
+  get: (target, name) => (name in target ? target[name] : () => {}),
+})
+
+const text = (node) => {
+  if (Array.isArray(node)) return node.map(text).join('')
+  if (typeof node === 'string') return node
+  if (node && typeof node === 'object') return text(node.props?.children)
+  return ''
+}
+
+// Draw the tree the way the screen would: a function component is called, a host element is
+// kept. One pass is enough — the test asks what the first screen carries.
+const draw = (node, out = [], depth = 0) => {
+  if (Array.isArray(node)) { for (const item of node) draw(item, out, depth); return out }
+  if (!node || typeof node !== 'object') return out
+  if (typeof node.type === 'function' && depth < 10) {
+    const saved = slots
+    const savedIndex = index
+    slots = []
+    index = 0
+    let rendered
+    try { rendered = node.type({ ...node.props }) } catch { out.push(node); return out } finally { slots = saved; index = savedIndex }
+    return draw(rendered, out, depth + 1)
+  }
+  out.push(node)
+  draw(node.props?.children, out, depth)
+  return out
+}
+
+// The sentence as a reader meets it: one text block per drawn copy, whatever element and
+// class carry it. A wrapper and the node inside it are one copy, not two.
+const sentences = (tree) => draw(tree).filter(node => text(node) === SENTENCE
+  && !draw(node.props?.children).some(child => text(child) === SENTENCE))
+
+const view = (over = {}) => h(WorkflowsView, { phase: 'ready', pack, opened: { source: 'own', id: 'w1' }, error: '', roster: lockedRoster, ...over })
+const dialog = (roster) => SettingsDialog({ page: 'workflows', id: null, roster, actions: actions(), useUpdate: () => ({}) })
+const buttons = (tree) => draw(tree).filter(node => node.type === 'button')
+
+test('a read-only window draws the host sentence once, not twice, with a workflow graph open', () => {
+  const header = sentences(dialog(lockedRoster))
+  assert.equal(header.length, 1, 'the dialog header keeps the standing sentence')
+  assert.equal(header[0].props.role, 'status', 'the header sentence stays the live region')
+  assert.equal(text(header[0]), SENTENCE, "the header keeps the host's own wording")
+
+  const graph = sentences(view())
+  assert.equal(graph.length, 0, 'the open graph does not repeat the sentence the header already carries')
+  assert.equal(header.length + graph.length, 1, 'the screen carries the host sentence once')
+  assert.equal(draw(view()).some(node => node.props?.role === 'status' && text(node) === SENTENCE), false,
+    'the edit bar adds no second live region')
+})
+
+test('the read-only graph keeps Back and adds no second Reload, Edit or Use this', () => {
+  const labels = buttons(view()).map(text)
+  assert.deepEqual(labels, ['Back'], 'Back is the only control the edit bar draws')
+  assert.equal(labels.includes('Reload'), false, "the header's Reload is the screen's one Reload")
+  assert.equal(labels.includes('Edit') || labels.includes('Finish editing'), false, 'the graph offers no editing control')
+  assert.equal(labels.includes('Use this'), false, 'the graph offers no copy of a preset')
+})
+
+test('a writable window keeps the graph editor and draws no read-only sentence', () => {
+  assert.equal(sentences(dialog(baseRoster)).length, 0, 'a writable header draws no sentence')
+  const tree = view({ roster: baseRoster })
+  assert.equal(sentences(tree).length, 0, 'a writable graph draws no sentence')
+  const labels = buttons(tree).map(text)
+  assert.ok(labels.includes('Edit'), 'Edit stays in a writable window')
+  assert.equal(labels.includes('Reload'), false, 'a writable window draws no Reload')
+  assert.ok(draw(tree).some(node => node.props?.['data-node'] === 'coder'), 'the opened graph still renders')
+})