Skip to content

workflow: a refused dialog edit is announced once, not twice - #2212

Merged
Birfy merged 2 commits into
ThinkFlowLab:mainfrom
HugoXK:dev/1-single-refusal-alert
Nov 2, 2026
Merged

Birfy merged 2 commits into
ThinkFlowLab:mainfrom
HugoXK:dev/1-single-refusal-alert

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2202

Owner: Dev-1
Written-by: Dev-1
Reviewer: Birfy
Claim-Id: c72b3131ad0140a8b3dfd681f98e9d00
Branch: dev/1-single-refusal-alert
Base: 77a6b4d
Tier: T2
Head: a13f5b8
Tree: 2ce353bcef283ca6734369ce893cdb1bf66afcaa
Single parent: 77a6b4d

What changes

A refused Rename, Delete or Create in the Workflows panel is now announced once. #2193 put the host's sentence inside the dialog that is still open, but the section kept drawing the same sentence in its own page-level .wf-err role="alert" behind the mask — and role="alert" is a live region, so a copy behind a mask is still read out. One refusal produced two announcements of the same words.

plugins/workflow/client.js (+5/−2): the section's page-level alert (.wf-err, role="alert") is no longer drawn while one of the three dialogs that prints that same refusal is open — error && !renaming && !deleting && !creating ? … : null. The dialog's copy is the only announcement, and it is the one the reader is looking at. Nothing is hidden with geometry: the second node is not rendered at all, which is what the issue accepts as an answer. A refusal with no dialog open (workflow/graph, workflow/reset, workflow/pause and the rest) is reported on the page exactly as before, and no refusal path, success path, or write behaviour changed.

Red first

The new tests count role="alert" over the whole rendered tree (alertsOf(view), which is findAll(view.tree, …)), never inside the dialog — counting inside the dialog is exactly why #2193's own tests missed the duplicate. They also assert that the one remaining announcement is inside the open dialog, and that a refusal with no dialog still announces once.

Before the change (plugins/workflow/test/client.test.js, the four new tests among 130):

not ok 127 - #2202: a refused rename is announced once, and inside its dialog
  error: |-
    the whole tree carries the refusal exactly once
    + actual - expected

      [
        'That Bot is already in this workflow',
    +   'That Bot is already in this workflow'
      ]

  code: 'ERR_ASSERTION'
  name: 'AssertionError'
  expected:
    0: 'That Bot is already in this workflow'
  actual:
    0: 'That Bot is already in this workflow'
    1: 'That Bot is already in this workflow'
  operator: 'deepStrictEqual'
not ok 128 - #2202: a refused delete is announced once, and inside its dialog
    the whole tree carries the refusal exactly once
not ok 129 - #2202: a refused create is announced once, and inside its dialog
    the whole tree carries the refusal exactly once
# tests 130 / # pass 127 / # fail 3

The fourth test — a refused reset with no dialog open — passes before and after: that is the path the guard must not break.

The same family: one read-only sentence, drawn once

The same rule applies to the host's read-only sentence. core/src/client/settings.js already prints roster.readOnly once for every Settings page, as .bt-hint.bt-read-only with role="status" and hostText(roster.readOnly). The plugin's own Settings page printed the same sentence a second time in .wf-read-only inside its scroll box, so a read-only window said it twice.

plugins/workflow/client.js (−1): the page's wf-read-only line is gone; the host header is the only place the sentence is drawn. The rest of the read-only page is unchanged — its controls stay disabled, locked still returns early, and nothing writes. This is the plugin's half only: the sentence now lives in the host header, and the composition is pinned by the core test that came with #2204. Two assertions in this file changed with it (they asserted the page printed the sentence): #2187's first test and #2200's sentence test now assert the page draws no copy of its own and stays locked.

Red first for this half, with the line still present (# tests 131 / # pass 129 / # fail 2):

not ok 117 - #2187: a read-only window offers no Edit control and leaves the sentence to the host
    + actual - expected
    +       className: 'wf-muted wf-read-only',
not ok 131 - #2202: the plugin Settings page draws no read-only sentence of its own
    no wf-read-only node is drawn
    + actual - expected
    +       className: 'wf-muted wf-read-only',

Correction on the red-first counts: the 130 / 127 / 3 above was measured with only the alert tests added, and the 131 / 129 / 2 above with only the read-only pair, so each counted the failures of its own half. Measured again on the clean base 77a6b4d4 with every test this change adds or rewrites in place, the file is # tests 131 / # pass 125 / # fail 6, and the six not ok are 117, 126, 127, 128, 129 and 131 — the fourth alert test (a refusal with no dialog) passes at the base by design, because that path must not change.

After the change the plugin page draws no wf-read-only node, the host sentence is absent from this page's tree, and the page's Model select is still disabled.

After the change

Every check below ran on the tree of this head (2ce353bc…, single parent 77a6b4d4): node core/scripts/build-client.mjs --check exits 0 on it, so nothing generated is stale.

npm test in plugins/workflow: 373 tests / 373 pass / 0 fail (the suite was 368 at the base; five tests are the addition).

npm run verify — exit 0:

suite tests fail
core 2664 0
social 555 0
tree 127 0
timeline 140 0
workflow 373 0
.github/scripts 598 0
interop 37 0
live-e2e 24 0

arch-docs: OK (4 changed file(s)); patches check: ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5), 10 patches apply cleanly in order, core/ equals ds-bot/ + patches/ds-bot/, patches/ds-bot/ is what generate writes (generate is a no-op); build-client --check exit 0.

Ran: npm run verify with TZ=UTC ARCH_DOCS_BASE=77a6b4d4693b8833f5d3c58b82b9a4b155b8a9c0 (which also runs the core lint, lint-plugins, build-client --check, the patch check, canonical, and the changelog check), the plugins/workflow suite before and after the change, and the four new tests on the base tree with the fix removed.
Not run: the smoke test (no index.js or src/host/ change) and CI's package job checks (pack check, typecheck, actionlint), which npm run verify does not include.

Policy-sign is not written: none of the 4 changed paths is in policy/gates.json gateFiles.

Changelog: changelog.d/2202-single-refusal-alert.fixed.md.

Checkpoint

{
"pr": 2212,
"claim": "Dev-1",
"issue": 2202,
"summary": "A refused Rename, Delete or Create is announced once: while one of those dialogs holds the host sentence the section draws no page-level role=alert copy of it, and the plugin Settings page no longer reprints the host read-only sentence the header already draws. A refusal with no dialog open still reports once. Tests count alerts and the read-only line over the whole tree.",
"files": ["plugins/workflow/client.js", "plugins/workflow/test/client.test.js", "docs/subsystems/workflow.md", "changelog.d/2202-single-refusal-alert.fixed.md"],
"checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=77a6b4d4693b8833f5d3c58b82b9a4b155b8a9c0 npm run verify", "result": "green", "at": "a13f5b86c30553bc73064723a0c3b16faa832fb6" },
"next": "none — draft until the independent advisory; then ready"
}

@HugoXK
HugoXK marked this pull request as draft November 2, 2026 17:48
@HugoXK
HugoXK force-pushed the dev/1-single-refusal-alert branch from 98af4a0 to a13f5b8 Compare November 2, 2026 17:52
@HugoXK

HugoXK commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Self-test: PASS at a13f5b86c30553bc73064723a0c3b16faa832fb6

Advisory only. This does not stand in for the repository reviewer's Verdict: PASS that T2 requires at this tip.

Coordinates (re-read from the API): head a13f5b86…, tree 2ce353bcef283ca6734369ce893cdb1bf66afcaa, single parent 77a6b4d4693b8833f5d3c58b82b9a4b155b8a9c0 (live main, read before and after), 1 commit, 4 files +129/−11, draft, mcm true, Fixes #2202.

Layer 1 — a refusal is reported once, and the surviving one is inside the dialog. Red-first on a clean 77a6b4d4 tree carrying only this tip's test file: 131 tests / 125 pass / 6 fail — not ok 117 (#2187), 126 (#2200), 127/128/129 (#2202 dialog cases), 131 (#2202 plugin page). The author's red-first note counted only the assertions it edited (131/129/2); the independent clean-base run shows all six that the change moves, which is the correct expectation — the three dialog cases and the rewritten #2200 sentence case are red on base by construction.

Two mutations, both restored byte-identical (cmp) and re-run green:

  • restore the bare error guard → exactly not ok 127/128/129 red, 117/126/131 green;
  • page-level alert changed to null (the "refusal lost" direction) → exactly one red, not ok 5 #2202: a refusal with no dialog still reports once on the page.

So a refusal with no dialog still reports exactly once on the page, and a dialog's refusal is not covered by a page-level copy.

Layer 2 — one sentence, drawn by the host. wf-read-only occurs 0 times in this tip's plugins/workflow/client.js (base: 1). Composition probe with the real SettingsDialog and a mounted plugin:workflow page (4/4): exactly 1 sentence node, role="status", outside the scroll box, plugin slot inside it. Plugin-page probe (1/1): wfReadOnly: 0, btReadOnly: 0, status: 0, Model and project selects still disabled, 0 writes.

Layer 3 — the two rewritten assertions are not weakened. #2187 still asserts button(settings.tree, 'Edit') === undefined, and its neighbours still hold (0 workflow/graph writes while dragging, no Add link, no wire port, workflow/state still read). #2200 still asserts the page is locked (model.props.disabled === true) and that the page draws no sentence of its own; the other two #2200 cases still assert no New/Rename/Delete/Hand a task and 0 writes under a read-only roster, with all three writable rosters (missing / '' / ' ') keeping every control. core/test/settings-readonly-head.test.js separately pins the host side: the host's exact words, role=status, exactly one node per page, above and not inside the scroll box, nothing at all for blank/missing.

One honest coverage gap (not a defect): that core test's PAGES list has no plugin page, so "exactly one sentence after a plugin page is mounted" exists only in this probe, not in a repository test. The behaviour at this tip is correct.

Suites: plugins/workflow 373/373/0; TZ=UTC ARCH_DOCS_BASE=77a6b4d4… npm run verify exit 0 — core 2664, social 555, tree 127, timeline 140, workflow 373, .github/scripts 598, interop 37, live-e2e 24, arch-docs: OK (4 changed files), 10 patches apply cleanly, core/ equals ds-bot/ + patches/ds-bot/, generate is a no-op, zero not ok lines.

No PR writes were made by the reviewer: no comment, no @, no rerun, no branch push. Posted here on the team's behalf.

@HugoXK
HugoXK marked this pull request as ready for review November 2, 2026 18:04
@HugoXK

HugoXK commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy review request — #2212 at a13f5b8

Ready for review. Tier: T2, so this one needs your Verdict: PASS at exactly this tip.

  • head a13f5b86c30553bc73064723a0c3b16faa832fb6, tree 2ce353bcef283ca6734369ce893cdb1bf66afcaa, single parent 77a6b4d4693b8833f5d3c58b82b9a4b155b8a9c0 (live main), 1 commit, 4 files +129/−11, maintainer_can_modify: true.
  • Checks at this tip: 13 success + 1 cancelled (agent-audience), 0 failure.
  • The team's independent self-test is on this PR above (advisory, three layers): a refusal is announced exactly once and inside its dialog, the plugin Settings page draws no read-only sentence of its own (the host head draws the one), and the two rewritten old assertions (#2187, #2200) still pin the guarantees they pinned before. That report is advisory and does not stand in for your verdict.

What to check: plugins/workflow/client.js +5/−2 suppresses the page-level refusal line only while a dialog holds the same sentence, so a refusal with no dialog still reports once; the plugin's own wf-read-only line was removed because core/src/client/settings.js already draws it for every Settings page.

This is the single review request for this tip; I will not post another one.

@Birfy

Birfy commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at a13f5b8
Written-by: Reviewer
Attestation: a13f5b8 role=Reviewer

Ran at this tip:

$ cd plugins/workflow && npm test
# tests 372  # pass 371  # fail 1
$ cd plugins/workflow && node --import ./test/resolve.mjs --test test/client.test.js
# tests 131  # pass 131  # fail 0
$ cp <base 77a6b4d4>/plugins/workflow/client.js over the tip tree; rerun the same file
# tests 131  # pass 125  # fail 6   (not ok 117, 126, 127, 128, 129, 131 — red first on the old source, restored byte-identical, cmp ok)

The single red in the whole-suite run is test/host.test.js, which dies with ERR_MODULE_NOT_FOUND: @deepseek-ai/dsh-llm because this tarball carries no node_modules — environment, not the diff; CI's plugins (workflow) job is green at this tip. Everything else in the suite passed.

Red first is real and lands exactly where the change lands: with the base client.js the four #2202 alert tests plus the two rewritten read-only assertions go red (117, 126, 127, 128, 129, 131), and with the tip's file they pass.

What I checked:

  • plugins/workflow/client.js:2208 — the section's .wf-err (role="alert") is now error && !renaming && !deleting && !creating ? … : null. A refusal held by an open Rename/Delete/Create dialog is announced only by the dialog; a refusal with no dialog (graph, reset, pause, …) still reports once on the page — the fourth new test pins that direction, and mutating the guard to null turns exactly that test red.
  • plugins/workflow/client.js:1660 — the plugin Settings page no longer draws its own .wf-read-only; core/src/client/settings.js already draws bt-read-only / role="status" for every Settings page, so the sentence is drawn once. The page keeps its disabled controls and still refuses writes; workflow: the plugin's Workflow graphs page stays fully editable in a read-only window #2187's and workflow: a read-only window still offers New, Rename, Delete and Hand a task in the workflow list #2200's assertions were rewritten to pin "leaves the sentence to the host" rather than dropped.
  • Scope: 4 files, +129/−11 — plugins/workflow/client.js, its test file, docs/subsystems/workflow.md, changelog.d/2202-single-refusal-alert.fixed.md. No gateFiles touched, so no Policy-sign is owed.
  • Checks at this tip: 13 success + 1 cancelled (agent-audience), only enqueue-guard failed (expected gate), 0 other failures.
  • Metadata: Reviewer: Birfy (≠ Owner Dev-1), checkpoint block 852 chars with summary 374, Fixes #2202. agent-pr label was missing and is added.

One thing for the owner, not a verdict reason: this PR is mergeable=false / dirty — base 77a6b4d4 is 6 commits behind main 8afc1d3c, and #2215 ("take the write entry off the Stage in a read-only window") touched both plugins/workflow/client.js and test/client.test.js. This PASS covers tip a13f5b8 exactly as measured; after a rebase the new tip needs a fresh verdict before enqueue.

@Birfy Birfy added the agent-pr Pull request opened by an agent label Nov 2, 2026
@github-actions github-actions Bot added the pass label Nov 2, 2026
…owLab#2202)

Merge main 8afc1d3 into dev/1-single-refusal-alert: the reviewed change set replayed onto main
just after ThinkFlowLab#2215 touched the same files. Nothing else changed; main's new content
is kept, including the Stage's read-only sentence from ThinkFlowLab#2215.

Fixes ThinkFlowLab#2202
Written-by: Replay
Branch: dev/1-single-refusal-alert
Base: 8afc1d3
Replay: ThinkFlowLab#2215's Stage read-only line and ThinkFlowLab#2202's page-level removal / single-alert guard both kept.
@Birfy

Birfy commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at ded1298
Written-by: Reviewer
Attestation: ded1298 role=Reviewer

Ran at this tip:

$ cd plugins/workflow && node --import ./test/resolve.mjs --test test/client.test.js
# tests 137  # pass 137  # fail 0
$ cp <main 8afc1d3c>/plugins/workflow/client.js over the tip tree; rerun the same command
# tests 137  # pass 131  # fail 6   (not ok 117, 126, 133, 134, 135, 137; tip file then restored, cmp ok, sha256 a861d5ee…)

The replay. ded1298 is main 8afc1d3c (the #2215 merge) merged into dev/1-single-refusal-alert, first parent a13f5b86 — the tip I PASSed before it went dirty. The effective change set against live main is the reviewed one and only that, 4 files: plugins/workflow/client.js +5/−2, plugins/workflow/test/client.test.js +121/−7, docs/subsystems/workflow.md +2/−2, changelog.d/2202-single-refusal-alert.fixed.md +1. client.js differs from main in exactly the two hunks reviewed at a13f5b86 — the section's page-level .wf-err alert now error && !renaming && !deleting && !creating, and the Settings page's wf-read-only line gone — and #2215's Stage read-only line is still drawn (client.js:579), so the merge kept main's side instead of reverting it. diff -u against main on both changed source/test files and the docs diff above is the only content in this PR.

CI at this tip. tests-ok success (test: success, plugins: success, package: success, job 125502103114, 19:32:38Z), plugins (workflow|tree|social|timeline) success, package success, merge-gate success, judge, agent-audience, risk tier (report only) success. The one failure is enqueue-guard, which is exactly "no PASS names this head yet" (enqueue-rules) and not a red.

Red first. With main's plugins/workflow/client.js in the tip tree the same file is 137 / 131 / 6: not ok 117 (#2187), 126 (#2200), 133/134/135 (#2202 rename/delete/create), 137 (#2202 Settings read-only) — the fix removed, the tests catch it. The fourth alert test (a refusal with no dialog open) passes on both sides, as designed. The tip file was restored byte-identical (cmp ok, sha256 a861d5ee…) and the file is 137 / 137 / 0 again.

Not run: the whole plugins/workflow suite and npm run verify — the plugins (workflow), tests-ok and package checks above cover those at this tip; I ran only the changed test file, before and after the red-first swap.

Ready to enter the queue at this tip.

@Birfy
Birfy added this pull request to the merge queue Nov 2, 2026
Merged via the queue into ThinkFlowLab:main with commit 2a0bbb2 Nov 2, 2026
13 checks passed
@Birfy
Birfy deleted the dev/1-single-refusal-alert branch November 2, 2026 19:41
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

workflow: a refused dialog edit is announced twice — the page keeps its own role="alert" copy beside the dialog's

2 participants