From 3f8e951312ecf1bb9a3c28b30a3358d6df14861d Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:05:01 +0800 Subject: [PATCH] fix(workflow): keep a refused dialog open with the host's sentence (#2193) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #2193 Written-by: Dev-4 Owner: Dev-4 Reviewer: Birfy Claim-Id: f3aedce4e41744a1b6e7e336cee2add2 Branch: dev/4-workflow-dialog-error Base: 9ede9258ba24bab59725d8b55f05e524065df24b Tier: T2 ## What this fixes `confirmRename`, `confirmDelete` and `create` each paint the host's refusal into the page-level `error` and return, but the `setRenaming(null)` / `setDeleting(null)` / `setCreating(false)` that would close the dialog sits after the `return`. The dialog stays on screen with no text of its own, so the host's sentence is written to the sidebar behind the mask and reads as nothing having happened. ## The change `plugins/workflow/client.js` (+14/-11): - `RenameModal`, `DeleteModal` and `CreateModal` take an `error` prop and render it inside their own body as `h('div', { className: 'wf-err', role: 'alert' }, error)`. - The three `portal()` call sites pass the page's `error` through. - Opening any of the three dialogs clears the previous refusal (`setError('')` on New, on the context Rename and on the context Delete), so a sentence from an earlier dialog cannot appear in a later one. - The refusal paths and the success paths are unchanged: the dialog still stays open on refusal, and a successful call still clears the error and reloads. `plugins/workflow/src/**` is untouched. ## Red first Clean `git archive 9ede9258` tree with only `plugins/workflow/test/client.test.js` from this branch in place (the tree's `plugins/workflow/client.js` is the base blob, sha256 `63f745fda4624a34f64981ebb64e4a566c8247b09316109cc527002646aad0fa`): ``` not ok 121 - #2193: a refused rename keeps its dialog open and shows the host sentence inside it not ok 122 - #2193: a refused delete keeps its dialog open and shows the host sentence inside it not ok 123 - #2193: a refused create keeps its dialog open and shows the host sentence inside it # tests 123 / # pass 120 / # fail 3 ``` Each failure is the same shape: `actual []`, `expected ['That Bot is already in this workflow']` — the dialog's subtree carries no `role="alert"` at all. The tests drive the page through a `call` that answers a refusal, `{ ok: false, error }`, which is the shape the host answers with; the sentence is the one the service itself emits for a repeated Bot (`plugins/workflow/src/service.js`, `seenBots`). Each test also pins that the dialog is still rendered after the refusal. At this tip: `# tests 123 / # pass 123 / # fail 0`. --- docs/subsystems/workflow.md | 2 + plugins/workflow/client.js | 25 +++++---- plugins/workflow/test/client.test.js | 84 ++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 11 deletions(-) diff --git a/docs/subsystems/workflow.md b/docs/subsystems/workflow.md index bf08bd98b..2617d7404 100644 --- a/docs/subsystems/workflow.md +++ b/docs/subsystems/workflow.md @@ -14,6 +14,8 @@ 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. + 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. A Condition (`when`) or Loop (`until`) link takes its output name from the **From** node's declared `outputs` (#2178): the field is a select over exactly those names, **Add link** waits for one, and a node that declares none keeps the text box for an older graph. The host still refuses a name it cannot honour — `cleanWhen` and `PORT_RE` are unchanged. diff --git a/plugins/workflow/client.js b/plugins/workflow/client.js index f58903676..cbf5872b2 100644 --- a/plugins/workflow/client.js +++ b/plugins/workflow/client.js @@ -1381,7 +1381,7 @@ window.__ModuleLoader__.load({ })) } - function RenameModal({ workflow, onClose, onConfirm }) { + function RenameModal({ workflow, onClose, onConfirm, error }) { const [name, setName] = useState(workflow?.name ?? '') return h('div', { className: 'wf-mask', onClick: (event) => { if (event.target === event.currentTarget) onClose() } }, h('div', { className: 'wf-modal', role: 'dialog', 'aria-modal': true, 'aria-label': tr('Rename workflow') }, @@ -1390,20 +1390,22 @@ window.__ModuleLoader__.load({ h('button', { type: 'button', className: 'wf-icon', 'aria-label': tr('Close'), onClick: onClose }, '×'))), h('div', { className: 'wf-modal-body' }, h('label', { className: 'wf-field' }, tr('Name'), - h('input', { value: name, onChange: (event) => setName(event.target.value) }))), + h('input', { value: name, onChange: (event) => setName(event.target.value) })), + error ? h('div', { className: 'wf-err', role: 'alert' }, error) : null), h('div', { className: 'wf-modal-footer' }, h('button', { type: 'button', className: 'wf-btn', onClick: onClose }, tr('Cancel')), h('button', { type: 'button', className: 'wf-btn wf-dark', onClick: () => onConfirm?.(name) }, tr('Rename'))))) } - function DeleteModal({ workflow, onClose, onConfirm }) { + function DeleteModal({ workflow, onClose, onConfirm, error }) { return h('div', { className: 'wf-mask', onClick: (event) => { if (event.target === event.currentTarget) onClose() } }, h('div', { className: 'wf-modal', role: 'dialog', 'aria-modal': true, 'aria-label': tr('Delete workflow') }, h('div', { className: 'wf-modal-header' }, h('h3', null, tr('Delete workflow'), h('button', { type: 'button', className: 'wf-icon', 'aria-label': tr('Close'), onClick: onClose }, '×'))), h('div', { className: 'wf-modal-body' }, - h('p', { className: 'wf-hint' }, tr('Are you sure you want to delete "{name}"? This cannot be undone.', { name: workflow?.name ?? '' }))), + h('p', { className: 'wf-hint' }, tr('Are you sure you want to delete "{name}"? This cannot be undone.', { name: workflow?.name ?? '' })), + error ? h('div', { className: 'wf-err', role: 'alert' }, error) : null), h('div', { className: 'wf-modal-footer' }, h('button', { type: 'button', className: 'wf-btn', onClick: onClose }, tr('Cancel')), h('button', { type: 'button', className: 'wf-btn wf-danger', onClick: onConfirm }, tr('Delete'))))) @@ -1472,7 +1474,7 @@ window.__ModuleLoader__.load({ h('button', { type: 'button', className: 'wf-btn wf-dark', disabled: !canAssign, onClick: () => { if (canAssign) onConfirm({ taskId, target }) } }, tr('Assign'))))) } - function CreateModal({ roster, templates, onClose, onCreate, onTemplate }) { + function CreateModal({ roster, templates, onClose, onCreate, onTemplate, error }) { const locals = (roster?.bots ?? []).filter(bot => bot && bot.remote !== true) const [name, setName] = useState('') const [steps, setSteps] = useState([]) @@ -1511,6 +1513,7 @@ window.__ModuleLoader__.load({ h('input', { type: 'checkbox', checked: parallel, onChange: (event) => setParallel(event.target.checked) }), tr('Parallel')), h('button', { type: 'button', className: 'wf-btn', onClick: add }, tr('Add a Bot')))), + error ? h('div', { className: 'wf-err', role: 'alert' }, error) : null, h('ol', { className: 'wf-plan' }, steps.map((layer, index) => h('li', { key: index }, layer.map(node => node.name).join(' ‖ '))))), h('div', { className: 'wf-modal-footer' }, h('button', { type: 'button', className: 'wf-btn', onClick: onClose }, tr('Cancel')), @@ -2166,7 +2169,7 @@ window.__ModuleLoader__.load({ return h('div', { 'data-flybot': 'workflow' }, h('div', { className: 'wf-sec' }, h('span', null, tr('Workflows')), - h('button', { type: 'button', className: 'wf-new', onClick: () => setCreating(true) }, tr('New'))), + h('button', { type: 'button', className: 'wf-new', onClick: () => { setError(''); setCreating(true) } }, tr('New'))), workflows.length > 0 ? h('label', { className: 'wf-filter' }, h('span', null, tr('Filter by Project')), h('select', { @@ -2254,7 +2257,7 @@ window.__ModuleLoader__.load({ onClose: () => setAssignOpen(false), onConfirm: confirmAssign, onRetry: loadTasks, })) : null, creating ? portal(h(CreateModal, { - roster, templates: pack?.templates, onClose: () => setCreating(false), onCreate: create, + roster, templates: pack?.templates, error, onClose: () => setCreating(false), onCreate: create, onTemplate: (template) => { void call('workflow/create', { template }).then(value => { if (value?.ok === false) { setError(value.error ?? ''); return } @@ -2276,15 +2279,15 @@ window.__ModuleLoader__.load({ onPointerDown: (event) => event.stopPropagation(), onClick: (event) => event.stopPropagation(), }, - h('button', { type: 'button', onClick: () => { setRenaming(rowMenu.workflow); setRowMenu(null) } }, tr('Rename')), - h('button', { type: 'button', className: 'wf-danger-text', onClick: () => { setDeleting(rowMenu.workflow); setRowMenu(null) } }, tr('Delete')))) : null, + h('button', { type: 'button', onClick: () => { setError(''); setRenaming(rowMenu.workflow); setRowMenu(null) } }, tr('Rename')), + h('button', { type: 'button', className: 'wf-danger-text', onClick: () => { setError(''); setDeleting(rowMenu.workflow); setRowMenu(null) } }, tr('Delete')))) : null, renaming ? portal(h(RenameModal, { - workflow: renaming, + workflow: renaming, error, onClose: () => setRenaming(null), onConfirm: (name) => { void confirmRename(renaming.id, name) }, })) : null, deleting ? portal(h(DeleteModal, { - workflow: deleting, + workflow: deleting, error, onClose: () => setDeleting(null), onConfirm: () => { void confirmDelete(deleting.id) }, })) : null) diff --git a/plugins/workflow/test/client.test.js b/plugins/workflow/test/client.test.js index 5464e89f4..b0c02a704 100644 --- a/plugins/workflow/test/client.test.js +++ b/plugins/workflow/test/client.test.js @@ -3412,3 +3412,87 @@ test('#2187: a read-only window still reads the graph through workflow/state', a assert.deepEqual(alertsOf(settings), [], 'no refusal banner for a page that wrote nothing') settings.unmount() }) + +// #2193: a confirmation dialog that the host refuses must keep the host's own +// sentence inside the dialog. It used to land in the sidebar behind the mask, so +// the dialog stayed on screen with nothing in it saying why. +const refusal = 'That Bot is already in this workflow' + +function refusedRename() { + const graph = { ...delivery(), id: 'lane-a', name: 'Lane A' } + const call = async (name) => { + if (name === 'workflow/state') return { workflows: [graph], templates: [] } + if (name === 'workflow/rename') return { ok: false, error: refusal } + return { ok: true, workflows: [graph] } + } + return { graph, call } +} + +test('#2193: a refused rename keeps its dialog open and shows the host sentence inside it', 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() + dialog = findAll(view.tree, node => node.props?.role === 'dialog')[0] + assert.ok(dialog, 'a refused rename leaves the dialog open') + assert.deepEqual(findAll(dialog, node => node.props?.role === 'alert').map(textOf), [refusal]) + view.unmount() +}) + +test('#2193: a refused delete keeps its dialog open and shows the host sentence inside it', 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() + dialog = findAll(view.tree, node => node.props?.role === 'dialog')[0] + assert.ok(dialog, 'a refused delete leaves the dialog open') + assert.deepEqual(findAll(dialog, node => node.props?.role === 'alert').map(textOf), [refusal]) + view.unmount() +}) + +test('#2193: a refused create keeps its dialog open and shows the host sentence inside it', 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] + assert.ok(dialog, 'the create dialog is open') + 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() + dialog = findAll(view.tree, node => node.props?.role === 'dialog')[0] + assert.ok(dialog, 'a refused create leaves the dialog open') + assert.deepEqual(findAll(dialog, node => node.props?.role === 'alert').map(textOf), [refusal]) + view.unmount() +})