diff --git a/changelog.d/1518.fixed.md b/changelog.d/1518.fixed.md new file mode 100644 index 000000000..6367b4e1c --- /dev/null +++ b/changelog.d/1518.fixed.md @@ -0,0 +1 @@ +The Workflow stage chat scrolls itself only while it is already at the bottom. A workflow node's own reply is filtered out of the stage, so it no longer drags the chat down, and a reader who scrolled up keeps their place. ([#1518](https://github.com/ThinkFlowLab/FlyBot/issues/1518)) diff --git a/docs/subsystems/workflow.md b/docs/subsystems/workflow.md index 442ed88a0..2cae95be8 100644 --- a/docs/subsystems/workflow.md +++ b/docs/subsystems/workflow.md @@ -56,12 +56,12 @@ Everything in `plugins/workflow/`: the graph rules, the run store, the matcher, 10. `enabled: false` registers no endpoint, tool or listener and writes nothing. 11. A sidebar click opens the stage or a dialog and leaves the Workflows section in place. The stage and the dialogs portal onto `document.body`. When that container is missing they stay in the section tree; the click does not throw. 12. A node's prompt and the graph are one record. Settings → Workflow graphs (canvas-first with per-node inspector), the flowchart drawer (collapsed by default, inspector on select), and `define_workflow` all write `workflow/graph`. The drawer paints that record fitted to its fixed 400px panel: `fitPlane` scales node centres (never the node boxes) into the panel width, wires and parallel frames use the fitted positions, and a pointer delta is divided by the fit scale before it is stored, so an edit still commits the same world coordinates. The Settings canvas paints the same record scaled to its own width (invariant 2), not as a second scroll container. A run reads that record; it does not define it. After a live run finishes, the stage shows the last finished live run (`lane.last` with `live === true`) until a new live run starts. +13. The stage chat follows a new line only while the reader is at the bottom. A line the stage filters out (a workflow node's own reply) changes no pixel, so it moves the chat nowhere; a reader who scrolled up keeps their place until they return to the bottom. ## Tests -`plugins/workflow/test/`: graph advance and join, graph edit, assign and persistence, the FIFO queue and concurrency cap, Bot-reply advance, a dispatch that fails without being marked handed, a bounded hand-off retry and a step deadline that fails the run and clears its timer (#1530), a per-workflow `stepTimeoutMinutes` that replaces the ten-minute constant and is refused outside 1..120 (#1580), disposing releases the Bots before the record drops their ids and a released runner name is reusable again, assign-to-bot versus assign-to-workflow, inputs and outputs along edges, a loop that breaks and an if/else branch, failure recovery (onFail edges with max attempts, exhaustion behavior, and compatibility with failOnce and loop edges), templates, `define` / `runNamed`, the matcher, ephemeral spawn and cleanup, a stopped run with its settled node map (#1424), the reply middleware, the client canvas, the settings page (including a condition and a loop), rename and delete (including factory-id rows such as `delivery`), and a sidebar click that leaves the Workflows section in place. Run `npm test` in `plugins/workflow`. The host seam that applies `ownerBotId` is `core/test/task-route.test.js`, which also pins that a card written through `track_tasks` reaches the listener (#1462). The host omission of run Bots, and the grant of `stepTools` plus the hidden-approval answer, is `core/test/flybot.test.js`. Which connectors a run Bot may call is `core/test/connector-access.test.js`, and the chain from a node's `tools` through `spawn` to that guard — the built-in delivery nodes included — is `core/test/workflow-node-connectors.test.js`. +`plugins/workflow/test/`: graph advance and join, graph edit, assign and persistence, the FIFO queue and concurrency cap, Bot-reply advance, a dispatch that fails without being marked handed, a bounded hand-off retry and a step deadline that fails the run and clears its timer (#1530), a per-workflow `stepTimeoutMinutes` that replaces the ten-minute constant and is refused outside 1..120 (#1580), disposing releases the Bots before the record drops their ids and a released runner name is reusable again, assign-to-bot versus assign-to-workflow, inputs and outputs along edges, a loop that breaks and an if/else branch, failure recovery (onFail edges with max attempts, exhaustion behavior, and compatibility with failOnce and loop edges), templates, `define` / `runNamed`, the matcher, ephemeral spawn and cleanup, a stopped run with its settled node map (#1424), the reply middleware, the client canvas, the settings page (including a condition and a loop), rename and delete (including factory-id rows such as `delivery`), the stage chat's scroll rule (a new visible line follows, a filtered node reply moves nothing), and a sidebar click that leaves the Workflows section in place. Run `npm test` in `plugins/workflow`. The host seam that applies `ownerBotId` is `core/test/task-route.test.js`, which also pins that a card written through `track_tasks` reaches the listener (#1462). The host omission of run Bots, and the grant of `stepTools` plus the hidden-approval answer, is `core/test/flybot.test.js`. Which connectors a run Bot may call is `core/test/connector-access.test.js`, and the chain from a node's `tools` through `spawn` to that guard — the built-in delivery nodes included — is `core/test/workflow-node-connectors.test.js`. ## Connector ceiling (#1544) A connector a Bot switched off is not reopened by a Workflow, whichever surface started the run. A run records the Bots whose switches cap it: the Bot whose `run_workflow` call started it (`createdBy`), and the Bot that saved the graph (`authoredBy` on the workflow record, set by `define_workflow` and by any save that names a Bot). Every node's run Bot carries `cappedBy` — the caller, else the graph's author — and `core/src/host/connector-access.js` intersects the node's `tools` with those switches, so a node that names a connector its caller or author turned off is refused with `turned off for ; the step's tools list cannot turn it back on`, and the tool is masked out of the run Bot's list too. A run nobody's tool call started (the board's `ingest`, the canvas, the `workflow/*` API) has no caller and is capped by the graph's author alone; a save with no Bot behind it records no author, which is the owner's own edit and leaves no ceiling; a graph saved before this has none either. `visibleTo` on `run_workflow`/`define_workflow` is unchanged: the ceiling is the fix, not limiting who may define a Workflow. - diff --git a/plugins/workflow/client.js b/plugins/workflow/client.js index 316defddc..ea16f5d49 100644 --- a/plugins/workflow/client.js +++ b/plugins/workflow/client.js @@ -521,14 +521,22 @@ window.__ModuleLoader__.load({ // Stage: view/control only — progress and flowchart; no message composer (#1459). function Stage({ workflow, roster, stageLeft, drawer, tab, focusedRunId, onFocusRun, onCloseDrawer, onToggleDrawer, onOpenDrawer, onTab, onAssign, onTip, onReset, editing, onToggleEdit, onCommit, onPause, onResume, onStop }) { const chatRef = useRef(null) + const chatFoot = useRef({ height: 0, shown: 0 }) const liveRuns = liveStageRuns(workflow) const viewRun = resolveFocusedRun(workflow, focusedRunId) const showSummary = (workflow.queueDepth ?? 0) > 0 || liveRuns.length > 1 + const visible = stageMessages(workflow, workflow.messages, roster) useEffect(() => { const el = chatRef.current - if (el) el.scrollTop = el.scrollHeight + if (!el) { chatFoot.current = { height: 0, shown: 0 }; return } + const prev = chatFoot.current + chatFoot.current = { height: el.scrollHeight, shown: visible.length } + // A stage line that is filtered out changes no pixel, so it must not move the chat. + if (visible.length <= prev.shown) return + // Follow a new line only from the bottom; a reader who scrolled up keeps their place. + if (prev.height > 0 && el.scrollTop + (el.clientHeight ?? 0) < prev.height - 24) return + el.scrollTop = el.scrollHeight }, [workflow.messages.length]) - const visible = stageMessages(workflow, workflow.messages, roster) return h('div', { className: 'wf-stage', style: { left: stageLeft } }, h('div', { className: 'wf-main' }, h('div', { className: 'wf-top' }, diff --git a/plugins/workflow/test/client.test.js b/plugins/workflow/test/client.test.js index e3079c201..ad3e25085 100644 --- a/plugins/workflow/test/client.test.js +++ b/plugins/workflow/test/client.test.js @@ -39,7 +39,29 @@ const react = { } function mount(Component, props = {}) { - const root = { instances: new Map(), pending: [], rendering: false, dirty: false, tree: undefined } + const root = { instances: new Map(), elements: new Map(), pending: [], rendering: false, dirty: false, tree: undefined } + // The renderer has no DOM: a host element's ref gets a stand-in that answers + // getBoundingClientRect from `live.rectWidth` and keeps a scroll box, so a + // component that measures or scrolls its own container is observable — the + // stage chat's scroll effect is a no-op otherwise, because useRef keeps + // chatRef.current null. + const elementAt = (path) => { + let el = root.elements.get(path) + if (!el) { + el = { + scrollTop: 0, scrollHeight: 0, clientHeight: 0, focused: false, focus() { this.focused = true }, + getBoundingClientRect: () => ({ width: live.rectWidth, height: 0 }), dataset: {}, + } + root.elements.set(path, el) + } + return el + } + const attachRef = (ref, path) => { + if (!ref) return + const el = elementAt(path) + if (typeof ref === 'function') ref(el) + else if (typeof ref === 'object') ref.current = el + } const expand = (node, path) => { if (node === null || node === undefined || typeof node === 'boolean') return undefined if (typeof node !== 'object') return String(node) @@ -52,14 +74,7 @@ function mount(Component, props = {}) { live.idx = 0 return expand(node.type({ ...node.props, children: node.children }), `${path}/c`) } - // The renderer has no DOM: a host element's ref gets a stand-in that answers - // getBoundingClientRect from `live.rectWidth`. - const ref = node.props?.ref - if (ref) { - const el = { getBoundingClientRect: () => ({ width: live.rectWidth, height: 0 }), dataset: {} } - if (typeof ref === 'function') ref(el) - else if (typeof ref === 'object') ref.current = el - } + attachRef(node.props?.ref, path) return { type: node.type, props: node.props, children: node.children.map((child, i) => expand(child, `${path}.${i}`)).flat(Infinity).filter(child => child !== undefined) } } const render = () => { @@ -81,6 +96,7 @@ function mount(Component, props = {}) { render() return { get tree() { return root.tree }, + rerender(nextProps = {}) { Object.assign(props, nextProps); root.schedule() }, unmount() { for (const inst of root.instances.values()) for (const slot of inst.hooks) slot?.cleanup?.(); root.instances.clear() }, } } @@ -729,6 +745,57 @@ test('stage chat hides workflow node Bot lines but keeps owner Bots and system l assert.deepEqual(visible.map(message => message.id), ['1', '3', '4']) }) +// The stage chat follows a new line only while the reader is at the bottom: the +// scroll effect used the raw message count, so a filtered node reply dragged it +// down without one visible line changing. +async function openStageChat(messages) { + const graph = delivery() + graph.messages = messages + const call = async (name) => { + if (name === 'workflow/state') return { workflows: [graph], templates: [{ id: 'delivery', name: '发布交付流水线' }] } + return { ok: true, workflows: [graph] } + } + const view = mount(client.Section, { roster: { bots: [] }, call, actions: {} }) + await settle() + findAll(view.tree, node => node.type === 'button' && textOf(node).includes('发布交付流水线'))[0].props.onClick() + await settle() + const chat = findAll(view.tree, node => node.props?.className === 'wf-chat')[0] + assert.ok(chat, 'the stage chat did not mount, so its scroll effect could not run') + const chatEl = chat.props.ref?.current + assert.ok(chatEl, 'mount() never attached the stage chat ref, so the scroll effect is a no-op') + return { view, graph, chatEl } +} + +test('the stage chat follows a new visible line to the bottom', async () => { + const { view, graph, chatEl } = await openStageChat([{ id: 'm1', kind: 'user', text: '一条可见的说明' }]) + chatEl.clientHeight = 100 + chatEl.scrollHeight = 800 + chatEl.scrollTop = 800 + graph.messages = [...graph.messages, { id: 'm2', kind: 'system', text: '任务已进入 Workflow' }] + chatEl.scrollHeight = 900 + view.rerender() + assert.equal(chatEl.scrollTop, 900, 'a new visible stage line did not scroll the chat to the bottom') + view.unmount() +}) + +test('the stage chat keeps its place when the new lines are filtered node replies', async () => { + const { view, graph, chatEl } = await openStageChat([ + { id: 'm1', kind: 'user', text: '一条可见的说明' }, + { id: 'm2', kind: 'system', text: '任务已进入 Workflow' }, + ]) + chatEl.clientHeight = 100 + chatEl.scrollHeight = 800 + chatEl.scrollTop = 0 // the reader scrolled up + graph.messages = [...graph.messages, + { id: 'm3', kind: 'bot', botId: 'coder', text: 'node step output' }, + { id: 'm4', kind: 'bot', botId: 'writer', text: 'another node step output' }, + ] + assert.equal(client.stageMessages(graph, graph.messages, { bots: [] }).length, 2, 'the fixture must filter both new node replies out of the stage chat') + view.rerender() + assert.equal(chatEl.scrollTop, 0, 'a filtered node reply still dragged the stage chat to the bottom') + view.unmount() +}) + async function stageChatText(messages, roster) { const graph = delivery() graph.messages = messages