From a13f5b86c30553bc73064723a0c3b16faa832fb6 Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Tue, 3 Nov 2026 01:52:16 +0800 Subject: [PATCH] workflow: announce a refused dialog edit once, not twice (#2202) --- .../2202-single-refusal-alert.fixed.md | 1 + docs/subsystems/workflow.md | 4 +- plugins/workflow/client.js | 7 +- plugins/workflow/test/client.test.js | 128 +++++++++++++++++- 4 files changed, 129 insertions(+), 11 deletions(-) create mode 100644 changelog.d/2202-single-refusal-alert.fixed.md diff --git a/changelog.d/2202-single-refusal-alert.fixed.md b/changelog.d/2202-single-refusal-alert.fixed.md new file mode 100644 index 000000000..4f78652bb --- /dev/null +++ b/changelog.d/2202-single-refusal-alert.fixed.md @@ -0,0 +1 @@ +A refused Rename, Delete or Create in the Workflows panel is announced once now: the dialog that holds the host's sentence is the only `role="alert"` for that refusal, and the section no longer draws a second copy behind the mask that a screen reader read out as well. A refusal with no dialog open is still reported once on the page. In the same family, a read-only window no longer prints the host's read-only sentence twice on Settings → Workflow graphs: the Settings header draws it once for every page, so the workflow page drops its own copy (#2202). diff --git a/docs/subsystems/workflow.md b/docs/subsystems/workflow.md index 9dbd43c8d..9c6a97e6b 100644 --- a/docs/subsystems/workflow.md +++ b/docs/subsystems/workflow.md @@ -14,7 +14,7 @@ Settings → Workflows (#1364) is this plugin's list. The page lives in the core A host call on that page that throws, rejects, or never answers ends on the page's own `.wf-err` alert instead of staying on `Reading workflows…` or looking saved: a failed read offers `Try again`, and a failed save or model change clears the `Changes are saved automatically` hint. A successful call clears the alert again. -A confirmation dialog the host refuses — Rename, Delete, or Create — keeps the dialog open and prints the host's own sentence inside the dialog as a `role="alert"` line, instead of leaving it on the section's `.wf-err` alert behind the mask; opening any of the three clears the previous refusal, and the success path is unchanged. +A confirmation dialog the host refuses — Rename, Delete, or Create — keeps the dialog open and prints the host's own sentence inside the dialog as the **only** `role="alert"` for that refusal: while one of those three dialogs holds the sentence the section draws no `.wf-err` copy of it, because a live region behind the mask is still read out and one refusal would be announced twice (#2193, #2202). Opening any of the three clears the previous refusal, a refusal with no dialog open still reports once on the page, and the success path is unchanged. The **Add link** row applies the drawer's pair rule before it saves (#2173): a pair the graph already lists is a no-op, another kind over that same pair names the kind already there instead of submitting, and the **To** list marks the targets the chosen node already reaches. The host still refuses a duplicated pair — the form is what stops producing one. @@ -54,7 +54,7 @@ Everything in `plugins/workflow/`: the graph rules, the run store, the matcher, 1. Friends' Bots are not nodes. A `remote` Bot is refused by create, define and the matcher. 2. The canvas and the run share one graph: nodes, edges and positions. A drag commits `workflow/graph`, but only while **Edit** is on — a factory preset is copied first (#1378). A wire or a reconnect that would list a pair the graph already carries commits nothing new, because `normalizeGraph` refuses a graph that lists one pair twice and then saves none of it: unguarded, the plane would show no edge, no change and only a save error (#2163). Status highlighting reads that same graph. Replacing edges does not recompute who is already running. The Settings canvas paints that graph on the same 960px design plane, scaled to the width the settings page gives it, so the canvas is not a second scroll container inside the page's own scroll; its pointer deltas are divided by the scale, and its wires keep a stronger stroke than a plain divider so an edge can be followed at that size (#1483). - While another dsh holds the Bot team the host is read-only (`roster.readOnly`): the page still reads through `workflow/state`, prints the host's own sentence under `role="status"`, and offers no **Edit**, **Add link** or wire port, so it cannot take a change that core would refuse with `bot/read-only` and then drop (#2187). The same window draws the sidebar's workflow list without **New**, the row's **Rename**/**Delete** or **Hand a task to a Workflow**: the list is read-only too, and an `roster.readOnly` that is not a string, or holds only whitespace, counts as writable — trimmed the way `core/src/client/workflows.js:570` trims it (#2200). + While another dsh holds the Bot team the host is read-only (`roster.readOnly`): the page still reads through `workflow/state`, keeps its controls disabled, and offers no **Edit**, **Add link** or wire port, so it cannot take a change that core would refuse with `bot/read-only` and then drop (#2187). The same window draws the sidebar's workflow list without **New**, the row's **Rename**/**Delete** or **Hand a task to a Workflow**: the list is read-only too, and an `roster.readOnly` that is not a string, or holds only whitespace, counts as writable — trimmed the way `core/src/client/workflows.js:570` trims it (#2200). The page draws no read-only sentence of its own: the host's Settings header already prints one for every page it shows (`core/src/client/settings.js` `bt-read-only`, `role="status"`, `hostText(roster.readOnly)`), and a second copy inside the page said the same thing twice (#2202). 3. **A live run advances only when its ephemeral Bot replies, following the graph's edges in order.** That reply advances the run and may appear on the workflow stage log; it is not delivered into a persistent sidebar Bot conversation (for example Chief). The `workflow-step` send middleware denies those replies after `stepResult` runs. Nodes start only when their forward predecessors finish and edges reach them. **Early termination (#1537, #1538, #1546, #1570, #1571):** When a node completes **or fails**, the run ends immediately with status `done`, sets `run.earlyEnd` to the condition name, and does not advance to successors, take a success-path `loop`, or burn `onFail` recovery attempts. The two markers that report the task itself end it only on a structured signal (#1546): `needs-human` and `already-claimed` must be a declared OUTPUT whose value **equals** the marker, or a whole reply line that is exactly `EARLY-END ` in any case. A step that quotes an issue title, negates the label or reports it absent keeps running. `tip-unchanged` (#1570) and `open-pr` (#1571) accept that same structured signal — in a declared OUTPUT the value counts as a whole word, so a brief that names `data-open-project` is not `open-pr` (#1866) — and also honor their prose fallbacks (`tip unchanged` / `unchanged tip`, `open PR already` / `already has an open PR` / `has an open PR` / `found an open PR`), so a Bot that forgets the structured marker cannot soft-lock Claim↔Verify on the same tip. See `checkEarlyEnd` in Key files. Factory prompt guidance lives in `EARLY_END_GUIDANCE` (`plugins/workflow/src/template.js`) for Settings graphs that wire Triage / Claim / Verify: Verify must escalate once on an unchanged tip, not re-run the full suite. There is no scripted or seeded run path: `workflow/state` exposes only `live === true` runs — the running ones, plus the last run when it is still `live` and ended `failed` — a failed sibling stays on the stage beside whatever is running now, with `run.failure` (`{ code, at, nodeName?, text? }`) naming what ended it, so the room does not have to open Runs to learn that a run failed (#1493, #1494). The failed sibling is left out when a running run carries the same task id: that is a retry, and the live run keeps the row (#1507). History entries carry the run `id` and the same `failure`, so a Runs row shows when the run ended and why, and a row for a run already on the stage is not listed twice. A run does not create a schedule. `scheduleLabel` is display text. When the host can create Bots, starting a live run (`assign`, `run_workflow`, `ingest`) spawns one ephemeral Bot per node from that node's prompt, role, model and tools, sends it the step from the Main Bot, and removes every instance when the run finishes, fails for good, or is reset. A `failOnce` retry keeps them. They are hidden and left out of the sidebar. Loading the plugin does not spawn Bots for the default workflows. Plugin unload (`dispose`) and the load-time `reap` both end a live run still marked `running`: in-flight nodes are failed, the run is archived as `failed`, and any ephemeral Bot ids are released. A run already written as `running` with no ephemeral map (an older unload) is healed the same way on the next load (#1414). Loading is also the one moment a run the host left behind is noticed when its ephemeral map is still present: `reap` releases those instances and records the run as `failed`. Nothing resumes it. A run that already ended keeps its own status (#1388). `workflow/reset` stopping one live run ends it the same way before its Bots are released: nodes still `running` become `failed`, so the strip cannot keep painting a run that is over (#1424). A graph edit that drops a node that is still running ends that run the way Stop does: the node is failed, the run is archived `failed`, its node Bots are released, and the queue drains (#1549); an id whose release failed stays named on that archived run, and the next load's `reap` retries it, instead of the edit forgetting it (the #1616 contract). Every wait is bounded (#1530). A dispatch that does not land is not marked handed: the chat names the code and the attempt, and it is retried after a short backoff at most twice, never twice inside one hand-off, so a 429 is not hammered. When it still does not land, the step and the run fail with the reason written into the chat. Each running live step also holds a deadline — its workflow's `stepTimeoutMinutes`, `DEFAULT_STEP_TIMEOUT_MINUTES` (45 minutes) unless the record sets one, an integer from 1 to 120 given at create, through `define_workflow`, or through `workflow/step-timeout` (armed even when the host cannot spawn ephemeral Bots; #1530, #1580); when it expires the node and the run fail as `failed`, the run's ephemeral Bots are released, the chat and `run.failure` say the step's deadline was reached (`step deadline reached (45 min)`, the number its lane set), and `warn` names it once; the line never reads as a reply that did not arrive, because a step still working is killed by the same timer (#1868). The deadline is cleared as soon as the step moves, and by Pause, Stop, Reset, delete, `reap` and `dispose`; a paused run holds none, and Resume arms it again from that moment. A timer never outlives the run that started it. A reply that matches no running step — its run ended, or the node already moved — is reported through `warn` once per Bot and run instead of being dropped silently. A reply that says `could not answer your message` or `model call failed` fails that node and does not start its successor. A 429 or rate-limit mention in any other reply does not. Stop and Reset (`workflow/reset`) follow the same order as `dispose`: they release the run's Bots before the record forgets their ids, and an id whose release failed stays named on the still-live run, so the next Stop, Reset, dispose or load-time `reap` retries it (#1476). A run left that way is marked `paused` and `releasePending` and shows as Paused: it hands off no further node, `resume` is refused with an error, and Stop again is the retry. A whole-workflow Reset that could not release every Bot freezes every run it did not reset, because each of them has lost the ids whose release did go. The markers go away when the run ends: Stop again, `dispose`, `reap`, or its own completion. 4. Each workflow keeps a FIFO queue and up to `maxConcurrent` live runs (default 2, an integer from 1 to 8 on the record, via `workflow/concurrency`, or in Settings → Workflow graphs). A task that does not fit is enqueued. When a run finishes, every workflow with a free slot is drained, oldest enqueue first. A new assign on a workflow that already has a queue waits behind it. A task does not replace a running run or release that run's Bots. Migrating an old store keeps only runs that were still `running` with ephemeral Bot ids; other legacy scripted rows are dropped from active state (finished tasks may remain in lane history). History records a finished or failed live run. Lowering the cap does not stop a run that has already started. Assigning `{ botId }` claims the card for that local Bot and does not enqueue. A remote Bot is refused. `activeCount` is `runningOf(lane)` — live runs with `live === true` and `status === 'running'`. The stage aggregate chip and the Runs tab (运行记录) must agree: every such run appears as its own running row and progress card with its own `status` (never a hardcoded Running), the chip count does not exceed that list, and a failed sibling appears with its time and reason. On the Runs list, the clock stays in a non-shrinking column with a gap before the status chip, and a Failed row paints the Failed chip once — a generic Failed with no host detail does not also draw `wf-hist-reason` (#1618). 5. A node declares up to eight inputs and eight outputs. A required input is an output of the same name on an ancestor, or `task`, `title`, or `brief` on a root. A start with no provider is refused, and Bots spawned for that attempt are released. On a Bot-backed run a successor stays queued until its required inputs are present. If every forward predecessor is done, failed or skipped and an input is still missing, that node fails and the run stops. The dispatch text carries `Input : `. The Bot reports `OUTPUT : `; a single declared output may be the reply body. A declared output a gate reads is required (§3): when a `when`/`else` or `until` edge reads it and the step's reply does not produce it, the step fails with `failure.text` `Missing OUTPUT ` instead of settling, so the gate never reads silence as "not this branch" and the run cannot close `done` on it; a delivered value that does not equal `eq` still completes the step and skips the untaken branch (#1773). Nodes start in edge order. diff --git a/plugins/workflow/client.js b/plugins/workflow/client.js index 3a4eb97f6..331043e4f 100644 --- a/plugins/workflow/client.js +++ b/plugins/workflow/client.js @@ -1657,7 +1657,6 @@ window.__ModuleLoader__.load({ return h('div', { className: 'wf-settings-canvas', 'data-flybot': 'workflow-settings' }, error ? h('div', { className: 'wf-err', role: 'alert' }, error) : null, - locked ? h('div', { className: 'wf-muted wf-read-only', role: 'status' }, readOnlyReason) : null, error && !pack && !loading ? h('button', { type: 'button', className: 'wf-btn', onClick: () => setAttempt(value => value + 1) }, tr('Try again')) : null, @@ -2206,7 +2205,11 @@ window.__ModuleLoader__.load({ locked ? null : h('button', { type: 'button', className: 'wf-row wf-assign-row', onClick: () => { setAssignOpen(true); void loadTasks() } }, h('span', { className: 'wf-ico' }, glyph()), h('span', { className: 'wf-txt' }, h('span', { className: 'wf-t1' }, tr('Hand a task to a Workflow')))), - error ? h('div', { className: 'wf-err', role: 'alert' }, error) : null, + // #2202: `role="alert"` is a live region, so a copy behind the mask is still read out. + // While Rename, Delete or Create holds this same refusal in its own dialog, that dialog + // carries the only announcement; the section draws no second copy of the same sentence. + // A refusal with no dialog open is reported here as before. + error && !renaming && !deleting && !creating ? h('div', { className: 'wf-err', role: 'alert' }, error) : null, readError ? h('div', { className: 'wf-err', role: 'alert' }, readError) : null, open ? portal(h(Stage, { workflow: open, roster, stageLeft, drawer, tab, focusedRunId, diff --git a/plugins/workflow/test/client.test.js b/plugins/workflow/test/client.test.js index a69b39dd1..a0cc351d2 100644 --- a/plugins/workflow/test/client.test.js +++ b/plugins/workflow/test/client.test.js @@ -3366,13 +3366,13 @@ async function mountReadOnly(graph, calls) { return settings } -test('#2187: a read-only window offers no Edit control and shows the host reason', async () => { +test('#2187: a read-only window offers no Edit control and leaves the sentence to the host', async () => { const calls = [] const settings = await mountReadOnly(delivery(), calls) assert.equal(button(settings.tree, 'Edit'), undefined, 'a read-only window offers no Edit control') - const statuses = findAll(settings.tree, node => node.props?.role === 'status') - assert.ok(statuses.length > 0, 'the page says why the graph cannot change') - assert.ok(statuses.some(node => textOf(node).includes(READ_ONLY_REASON)), 'in the host\'s own words') + // #2202: the host's Settings header already draws `roster.readOnly` once for every page it + // shows (`core/src/client/settings.js`, `.bt-hint.bt-read-only`), so this page draws no copy. + assert.deepEqual(findAll(settings.tree, node => String(node.props?.className ?? '').includes('wf-read-only')), [], 'the page draws no second copy of the host sentence') settings.unmount() }) @@ -3551,10 +3551,124 @@ test('#2200: a writable window keeps New, Rename, Delete and Hand a task', async } }) -test('#2200: the host sentence #2187 put on the page is still there', async () => { +test('#2200: the read-only page stays locked and leaves the sentence to the host header', async () => { const view = mount(client.WorkflowSettings, { call: sidebarHost(LIST_GRAPH(), []), roster: { bots: [], readOnly: READ_ONLY_REASON } }) await settle() - const statuses = findAll(view.tree, node => node.props?.role === 'status') - assert.ok(statuses.some(node => textOf(node).includes(READ_ONLY_REASON)), 'the page still says why in the host\'s own words') + // #2202: `roster.readOnly` is drawn once by the host's Settings header for every page it shows + // (`core/src/client/settings.js` `bt-read-only`), so this page must not print it again. + assert.equal(textOf(view.tree).includes(READ_ONLY_REASON), false, 'the host sentence is left to the header, not printed here') + const model = findAll(view.tree, node => node.type === 'select' && node.props?.['aria-label'] === 'Model')[0] + assert.equal(model.props.disabled, true, 'and the page is still locked for writing') view.unmount() }) + +// #2202: one refusal, one announcement. `role="alert"` is a live region, so a copy left behind +// the mask is still read out beside the dialog's own copy. These assertions count over the whole +// tree (`alertsOf`), never inside the dialog — counting inside the dialog is exactly what let the +// duplicate through in #2193's tests. +test('#2202: a refused rename is announced once, and inside its dialog', async () => { + const { call } = refusedRename() + const view = mount(client.Section, { roster: { bots: [] }, call, actions: {} }) + await settle() + const row = findAll(view.tree, node => node.type === 'button' && textOf(node).includes('Lane A'))[0] + row.props.onContextMenu({ preventDefault() {}, stopPropagation() {}, clientX: 10, clientY: 10 }) + await settle() + findAll(view.tree, node => node.type === 'button' && textOf(node) === 'Rename')[0].props.onClick() + await settle() + let dialog = findAll(view.tree, node => node.props?.role === 'dialog')[0] + assert.ok(dialog, 'the rename dialog is open') + findAll(dialog, node => node.type === 'button' && node.props?.className === 'wf-btn wf-dark')[0].props.onClick() + await settle() + assert.deepEqual(alertsOf(view), [refusal], 'the whole tree carries the refusal exactly once') + dialog = findAll(view.tree, node => node.props?.role === 'dialog')[0] + assert.deepEqual(findAll(dialog, node => node.props?.role === 'alert').map(textOf), [refusal], 'and that one announcement is inside the open dialog') + view.unmount() +}) + +test('#2202: a refused delete is announced once, and inside its dialog', async () => { + const graph = { ...delivery(), id: 'lane-a', name: 'Lane A' } + const call = async (name) => { + if (name === 'workflow/state') return { workflows: [graph], templates: [] } + if (name === 'workflow/delete') return { ok: false, error: refusal } + return { ok: true, workflows: [graph] } + } + const view = mount(client.Section, { roster: { bots: [] }, call, actions: {} }) + await settle() + const row = findAll(view.tree, node => node.type === 'button' && textOf(node).includes('Lane A'))[0] + row.props.onContextMenu({ preventDefault() {}, stopPropagation() {}, clientX: 10, clientY: 10 }) + await settle() + findAll(view.tree, node => node.type === 'button' && textOf(node) === 'Delete')[0].props.onClick() + await settle() + let dialog = findAll(view.tree, node => node.props?.role === 'dialog')[0] + assert.ok(dialog, 'the delete dialog is open') + findAll(dialog, node => node.type === 'button' && node.props?.className === 'wf-btn wf-danger')[0].props.onClick() + await settle() + assert.deepEqual(alertsOf(view), [refusal], 'the whole tree carries the refusal exactly once') + dialog = findAll(view.tree, node => node.props?.role === 'dialog')[0] + assert.deepEqual(findAll(dialog, node => node.props?.role === 'alert').map(textOf), [refusal], 'and that one announcement is inside the open dialog') + view.unmount() +}) + +test('#2202: a refused create is announced once, and inside its dialog', async () => { + const call = async (name) => { + if (name === 'workflow/state') return { workflows: [], templates: [] } + if (name === 'workflow/create') return { ok: false, error: refusal } + return { ok: true, workflows: [] } + } + const view = mount(client.Section, { roster: { bots: [{ id: 'b1', name: 'Bot One' }] }, call, actions: {} }) + await settle() + findAll(view.tree, node => node.type === 'button' && textOf(node) === 'New')[0].props.onClick() + await settle() + let dialog = findAll(view.tree, node => node.props?.role === 'dialog')[0] + findAll(dialog, node => node.type === 'input' && node.props?.value === '')[0]?.props.onChange({ target: { value: 'Lane B' } }) + await settle() + dialog = findAll(view.tree, node => node.props?.role === 'dialog')[0] + findAll(dialog, node => node.type === 'button' && textOf(node) === 'Add a Bot')[0].props.onClick() + await settle() + dialog = findAll(view.tree, node => node.props?.role === 'dialog')[0] + findAll(dialog, node => node.type === 'button' && node.props?.className === 'wf-btn wf-dark')[0].props.onClick() + await settle() + assert.deepEqual(alertsOf(view), [refusal], 'the whole tree carries the refusal exactly once') + dialog = findAll(view.tree, node => node.props?.role === 'dialog')[0] + assert.ok(dialog, 'the create dialog is still open') + assert.deepEqual(findAll(dialog, node => node.props?.role === 'alert').map(textOf), [refusal], 'and that one announcement is inside the open dialog') + view.unmount() +}) + +test('#2202: a refusal with no dialog still reports once on the page', async (t) => { + await withUnhandled(async (unhandled) => { + const graph = delivery() + const call = async (name) => { + if (name === 'workflow/state') return { workflows: [graph], templates: [] } + if (name === 'workflow/reset') throw new Error('The reset was refused') + return { ok: true, workflows: [graph] } + } + const view = mount(client.Section, { roster: { bots: [] }, call, actions: {} }) + t.after(() => view.unmount()) + await settle() + findAll(view.tree, node => node.type === 'button' && textOf(node).includes('发布交付流水线'))[0].props.onClick() + await settle() + findAll(view.tree, node => node.type === 'button' && textOf(node) === 'Flow')[0].props.onClick() + button(view.tree, '↺ Reset').props.onClick() + await settle() + assert.deepEqual(alertsOf(view), ['The reset was refused'], 'no dialog holds this sentence, so the page reports it once') + assert.deepEqual(findAll(view.tree, node => node.props?.role === 'dialog'), [], 'and no dialog was opened') + assert.deepEqual(unhandled, []) + }) +}) + +// #2202: one read-only window, one sentence. The host's Settings header draws `roster.readOnly` +// once for every page it shows (`core/src/client/settings.js` `bt-read-only`); this page used to +// print the same sentence again inside its own scroll box. The composition itself is pinned by +// the core test that came with #2204 — here the plugin only proves it draws no copy of its own. +test('#2202: the plugin Settings page draws no read-only sentence of its own', async () => { + const calls = [] + const settings = await mountReadOnly(delivery(), calls) + assert.deepEqual(findAll(settings.tree, node => String(node.props?.className ?? '').includes('wf-read-only')), [], 'no wf-read-only node is drawn') + assert.equal(textOf(settings.tree).includes(READ_ONLY_REASON), false, 'and the host sentence is not printed here a second time') + // The rest of the read-only page is unchanged: its controls stay disabled and nothing writes. + const project = findAll(settings.tree, node => node.type === 'select' && node.props?.['aria-label'] === 'Project')[0] + assert.equal(project.props.disabled, true, 'a read-only page still disables its controls') + assert.deepEqual(calls.filter(item => item.name !== 'workflow/state'), [], 'and still writes nothing') + settings.unmount() +})