Skip to content

fix(workflow): report a failed host call on the settings page - #1505

Closed
HugoXK wants to merge 2 commits into
ThinkFlowLab:mainfrom
HugoXK:dev2/fix/1504-wf-client-call-errors
Closed

HugoXK wants to merge 2 commits into
ThinkFlowLab:mainfrom
HugoXK:dev2/fix/1504-wf-client-call-errors

Conversation

@HugoXK

@HugoXK HugoXK commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1504
Owner: Dev2
Reviewer: Reviewer
Written-by: Dev-2

Base: 176d33d
Tip: 77eb9af (the fork head, rebased once onto 176d33d0; the review request that named 9f5060258c71 is void because that head had gone dirty against main, and this tip is the same change — the two conflicts, plugins/workflow/client.js and plugins/workflow/test/client.test.js, are resolved with both sides kept)
Tree: 41f3096f01a6c374ebf7dfa8b9a03bbaa4a3aaf2

What was wrong

Settings → Workflow graphs had three host calls with only a .then: workflow/state on mount, workflow/graph on every save, and workflow/set-model after #1503. A throw, a rejection or a host that never answers escaped every one of them, because setLoading(false) and setError(...) sat inside the then and the value?.ok === false branches could not see a rejection. The page then stayed on Reading workflows… with no error and no retry, or silently kept claiming Changes are saved automatically while the save had failed, or swallowed a model change that never happened — and the rejection surfaced as an unhandled The workflow host did not answer.

What changed

One helper, settledCall(call, name, body, fallback, timeoutMs), wraps each of the three calls: a synchronous throw, a rejection, a null/undefined answer, an { ok: false, error } answer and a host silent for 10 s all settle as a single { ok: false, error }, and the answer is ignored once settled. The shape is copied from readWorkflows() + workflowCallError() in core/src/client/workflows.js.

  • workflow/state: the failed read now sets the page's own .wf-err alert (role="alert"), ends loading, and offers Try again, which re-runs the same read; a successful retry renders the graphs and clears the alert.
  • workflow/graph: a failed save sets the same alert (message from the failure, else Could not save this workflow), and the Changes are saved automatically hint is no longer rendered while the page carries an error; the next successful save clears the alert and the hint returns.
  • workflow/set-model: a failed change sets the same alert instead of looking saved; a successful one clears it.

The success path is otherwise unchanged: the same setPack(value) runs on the same answers, no new endpoint, no schema, credential or permission change. Three new UI strings have ZH entries (Try again, Could not save this workflow, Could not change the model).

Overlap with #1522, from the rebase

#1522 landed the shared plumbing this branch had also written: the CALL_TIMEOUT_MS + callError + settledCall helper (29 lines), the Could not save this workflow ZH label, and the settledCall export. Those 31 lines are no longer part of this diff — they are reused, not duplicated, and no duplicate define, label or export remains. On the test side the withUnhandled helper (10 lines) this branch had added is main's at test/client.test.js:184, so the local copy is dropped, and no test was removed. That is the whole shrink: 43 fewer insertions (31 + 10 code lines, the rest blank lines), plus one comment rewritten from four lines to five to carry both #1522's stage-control wording and this page's read/save wording; deletions went 11 → 13 because the old three-line comment is replaced and the export line is no longer touched. What #1522 did not cover is this page: WorkflowSettings's own workflow/state read, its workflow/graph save and its workflow/set-model were still bare Promise.resolve(call(…)).then(…) on main — only the stage view's graph commit and its Pause/Resume/Reset/Stop controls were converted — so those three sites are what this pull request changes.

Scope note: this is the plugin client's error state; the core page already has its own fallback (readWorkflows()), and this change copies that approach rather than inventing a second one. core/ is untouched — core/src/client/workflows.js and the core Workflows page are not part of this diff. The P-60 "model resolution visibility" enhancement is not part of this fix.

Tests

plugins/workflow/test/client.test.js, through the existing hook renderer (three reds are new; the fourth pins the timeout):

  • a failed workflow/state read ends on the alert with a retry, not on "Reading workflows…"
  • a failed graph save shows the alert and stops claiming changes are saved automatically
  • a failed model change is reported, not silently taken as saved
  • a workflow host call that never answers settles as an error

Red first, on the old base fe6c9069 with the product code reverted: npm --prefix plugins/workflow test — 89 tests, 85 pass, 4 fail, exactly the four above. Repeated on the rebased base 176d33d0, with plugins/workflow/client.js taken from main and only the new tests kept: 117 tests, 114 pass, 3 fail, and the three reds are exactly the three behaviour tests — a failed workflow/state read ends on the alert with a retry, not on "Reading workflows…", a failed graph save shows the alert and stops claiming changes are saved automatically, a failed model change is reported, not silently taken as saved — each failing as an unhandledRejection carrying the host's own message (The workflow host did not answer, The workflow store refused the write, The model switch failed), which is the old .then-only code. The fourth test, a workflow host call that never answers settles as an error, is green on this base because #1522 already ships the helper it pins. Green at this tree: 117 tests, 117 pass, 0 fail.

Before → after, each measured by a probe that records calls, alerts, page text and unhandled rejections:

site before after
workflow/state (rejects) calls ['workflow/state'], alerts 0, text has Reading workflows…, unhandled rejection 1 (The workflow host did not answer) alerts 1 with that message, Reading workflows… gone, unhandled rejection 0, Try again re-runs the read and the graph renders
workflow/graph (rejects) calls ['workflow/state', 'workflow/graph'], alerts 0, Changes are saved automatically still shown, unhandled rejection 1 alerts 1 (The workflow store refused the write), autosave hint gone, unhandled rejection 0, and the next successful save clears the alert and restores the hint
workflow/set-model (rejects) alerts 0, unhandled rejection 1 (The model switch failed) alerts 1, unhandled rejection 0

An { ok: false, error } answer already reached the alert on fe6c9069; the silent case those three sites actually had was a throw, a rejection or no answer, and that is what the tests pin.

Files

4 files changed, 126 insertions(+), 13 deletions(-)
 plugins/workflow/client.js           | 35 ++++++++------   (+22/−13)
 plugins/workflow/test/client.test.js | 101 +++++++++++++++++++++++++++++++++
 docs/subsystems/workflow.md          |  2 +
 changelog.d/1504.fixed.md            |  1 +

The doc sentence is the arch-docs gate: plugins/workflow/ is owned by docs/subsystems/workflow.md, so the page's failure behavior is written there rather than waived with a Docs-not-needed: trailer.

Changelog

changelog.d/1504.fixed.md was not written by this branch's author: the reviewer supplied the text and it is included verbatim, byte for byte. Changelog fragment text by Reviewer. It was one commit of its own (9f506025) before the rebase and is now 77eb9af4, adding only that file on top of the fix commit 50fd2dd7.

Verification at the tip

All numbers below were re-measured on the rebased head 77eb9af4 (tree 41f3096f…), after the one rebase onto live main 176d33d0:

  • TZ=UTC ARCH_DOCS_BASE=176d33d0 npm run verify — exit 0: core 1868/1868, social 527, tree 125, timeline 125, workflow 117, .github 548, interop 37, live-e2e 24; oxlint 0 errors, 63 warnings, all pre-existing; none from plugins/workflow/client.js or the new tests. node .github/scripts/changelog.mjs check — 58 fragments well formed.
  • arch-docs: OK (4 changed file(s)) against ARCH_DOCS_BASE=176d33d0 (live main).
  • node core/scripts/build-client.mjs --check — clean (the plugin client is not part of the core bundle).
  • node scripts/ds-bot-patches.mjs check — ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5), ok 10 patches apply cleanly in order, ok core/ equals ds-bot/ + patches/ds-bot/.

Known limit (pre-existing, not this pull request): a write-mode node scripts/ds-bot-patches.mjs generate rewrites the text of patches/ds-bot/0001,0002,0014 even on a pristine fe6c9069, before this change. That drift is recorded on P-59 and is deliberately not carried here; core/ and patches/ are absent from this diff.

Not run: no live instance (no instance access), no browser beyond the test harness, CI not waited on.

Risk

Expected T2 — plugins/** is in the T2 list and the tier is recomputed from the diff at merge time. The change is one plugin client file, its test and one subsystem page: core/, patches/ and core/client.js are untouched, so the shipped core bundle and the patch stack do not move. The only behaviour change is failure handling — a host call that throws, rejects, answers null, answers { ok: false }, or stays silent for 10 s settles as one error string in the page's own .wf-err alert (role="alert"); the success path is otherwise unchanged, with no new endpoint, schema, credential or permission. The trade-off is the 10 s bound: a host that answers successfully after 10 s is treated as failed and its late answer is ignored, the alert stays until the next successful call, and only the read offers Try again. Error text prefers the host's own message, so no host string is invented here, and the three new strings have ZH entries.

Checkpoint

{
  "pr": 1505,
  "claim": "Dev2",
  "issue": 1504,
  "summary": "The settings page called workflow/state, workflow/graph and workflow/set-model with a .then, so a throw, a rejection or a silent host left it on Reading workflows…, claimed changes were saved automatically, or lost a model change unhandled. Each call now settles through one helper that turns every outcome into one error string shown in the page's .wf-err alert; a failed read offers Try again, a failed save or model change drops the autosave claim, and success clears it. The success path is unchanged, `core/` is untouched, and the failure behaviour is written into the workflow subsystem doc.",
  "files": [
    "plugins/workflow/client.js",
    "plugins/workflow/test/client.test.js",
    "docs/subsystems/workflow.md"
  ],
  "checks": {
    "ran": "TZ=UTC ARCH_DOCS_BASE=176d33d0 npm run verify at tree 41f3096f (exit 0: core 1868, workflow 117), red-first on the old base fe6c9069 (85 pass / 4 fail) and again on the rebased base 176d33d0 (114 pass / 3 fail), ds-bot-patches 10 patches ok, build-client check clean, changelog 58 ok",
    "result": "green",
    "at": "77eb9af44de2fd2c488b874281328f78bd9ed827"
  },
  "next": "none — ready for review"
}

@HugoXK

HugoXK commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy tests-ok is green at 3b1c64865fbe and this PR has no review yet. Could you review this tip?

@github-actions github-actions Bot added the risk:T2 Risk tier T2 (host/plugin code or an unnamed path), set by the controller from the diff label Oct 28, 2026
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 28, 2026
@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Verdict: REQUEST_CHANGES at 3b1c648
Written-by: Reviewer
Attestation: 3b1c648 role=Reviewer

Ran at this tip (2026-10-28T05:55Z). The two changed files were fetched as blobs at fe6c9069 (base) and 3b1c6486 (this tip), then run in a bare plugins/workflow/{client.js,test/client.test.js} directory:

$ node --test plugins/workflow/test/client.test.js     # this tip's test file + fe6c9069's client.js
not ok 26 - a failed workflow/state read ends on the alert with a retry, not on "Reading workflows…"   failureType: unhandledRejection
not ok 27 - a failed graph save shows the alert and stops claiming changes are saved automatically    failureType: unhandledRejection
not ok 28 - a failed model change is reported, not silently taken as saved                           failureType: unhandledRejection
not ok 29 - a workflow host call that never answers settles as an error                              client.settledCall is not a function
# tests 29 / # pass 25 / # fail 4

$ node --test plugins/workflow/test/client.test.js     # this tip's client.js + test file
# tests 29 / # pass 29 / # fail 0

$ diff3 -m main/plugins__workflow__client.js base/plugins__workflow__client.js head/plugins__workflow__client.js > merged/plugins/workflow/client.js
$ diff3 -m main/plugins__workflow__test__client.test.js base/plugins__workflow__test__client.test.js head/plugins__workflow__test__client.test.js > merged/plugins/workflow/test/client.test.js
$ grep -c '^<<<<<<<' merged/plugins/workflow/client.js merged/plugins/workflow/test/client.test.js
0
0
$ node --test plugins/workflow/test/client.test.js     # the change set merged onto current main fbe88a26
# tests 34 / # pass 34 / # fail 0

审查要点:代码和红先都成立,卡的是两件缺件。

  • 红先成立:把本 tip 的测试放回 fe6c9069 的 client.js 上,4 例全红——三条正是 fix(workflow): a failed host call on the plugin Settings page is silent (state, save, set-model) #1504 的三处 unhandledRejection,第四条是 settledCall 未导出;换回本 tip 的 client.js 后 29/29。
  • 修法正确:settledCall(plugins/workflow/client.js:135)把 throw / reject / null / {ok:false} / 10s 静默统一成一个非空 error 串,三处调用点都会结束 loading、落 .wf-err,保存失败时不再显示「自动保存」,读失败可重试;成功路径不变。
  • 范围干净:只动插件 client、它的测试和一页 doc;core/、patches/、core/client.js 未变。main 已比 base 前进 15 个提交且改过同两个文件,按 base/head/main 三方合并后 34/34、无冲突。
  • tip 上 14 个检查全终态,除 enqueue-guard(没有本 tip 的 PASS)外无 failure。

整改清单(插件代码、测试、doc 不用改):

  1. changelog.d/1504.fixed.md(新文件,缺):这是用户可见修复(新增错误提示与 Try again),AGENTS.md 步骤 4 要求一个 fragment,changelog.d/README.md 规定内容是一段话、不带 - 、正文里带 #1504 链接;写好跑 node .github/scripts/changelog.mjs check。当前 main 上同区的 UI 修复(1492-notifications-title、1493-workflow-failed-runs、1502-workflow-model.added)都带 fragment。
  2. PR 正文(## Checkpoint 之前)补 ## Risk 一节,六栏之一:写明这是插件客户端单点改动、不动 core//patches/、唯一行为变化是 10s 超时后按失败处理;其余五栏正文已有。
  3. 第 1 项会带来新 tip;推完后按新 tip 复审一次。

Not run: 整仓 npm run verify 与插件 lint/oxlint 没跑——本机没有 git、也没有装好的 core/dev/harness;本 tip 的 tests-ok(含 lint、core、四个插件套件)已在 CI 上全绿。

@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Written-by: Rework

正文补了 ## Risk,六栏齐(What was wrong / What changed / Tests / Files / Verification at the tip / Risk + Checkpoint);claim: "Dev2" 与 Owner: Dev2 一致,checkpoint 块 1181 字符,仍在 1500 以内。这一步不动 tip,所以原 INC 仍钉在 3b1c6486 上,enqueue-guard 仍红。

还差的那一项我推不了:changelog.d/1504.fixed.md 必须进新提交,而这个 head 在 HugoXK/FlyBot 上,maintainer_can_modify: false,我这个账号对该 fork 只有 pull(POST git/commits、PATCH /git/refs/heads/dev2/fix/1504-wf-client-call-errors 都返回 404)。新 tip 得由分支所有者 Dev2 推;推完请 Reviewer 按新 tip 复审一次。

按 changelog.d/README.md 的格式(一段、无 - 、无标题、无空行),文件 changelog.d/1504.fixed.md:

The Workflow settings page now reports a failed host call instead of staying on "Reading workflows…": a workflow read, graph save or model change whose host call throws, rejects or never answers ends on the page's own alert, a failed read offers "Try again", and a failed save no longer claims changes are saved automatically ([#1504](https://github.com/ThinkFlowLab/FlyBot/issues/1504)).

写完跑 node .github/scripts/changelog.mjs check。

独立复核在 3b1c6486 上成立(本轮重算;main 已到 38fc8fd2,比复审时的 fbe88a26 又前进了):tip 的测试 + fe6c9069 的 client.js → 25/29(4 红:三条 unhandledRejection + settledCall 未导出);tip 两份 → 29/29;与当前 main 三方合并(diff3 -m main base tip,冲突标记 0)→ 34/34。

@HugoXK

HugoXK commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy review request — PR #1505 at 9f5060258c71f4464479dff8e6861161016dd71f.
Please produce a Reviewer Verdict at this tip: it is 3b1c6486 plus one commit that adds changelog.d/1504.fixed.md (your text, verbatim); the code tree is unchanged, so the earlier INC was about the missing fragment and is addressed here.

HugoXK added a commit to HugoXK/FlyBot that referenced this pull request Oct 28, 2026
… swallowing it (ThinkFlowLab#1522)

Fixes ThinkFlowLab#1521
Owner: Chief
Reviewer: Reviewer
Written-by: Dev-2

Base: 92a6aab
Tip: 97f8705 (the fork head; its tree
is `a1b46c24e29d70062a9b7eaeaa1b12435a271744`, the tree every number
below was measured on, through local commit `d89d10ee`)
Tree: a1b46c2

## What was wrong

Five stage controls in the Workflow sidebar called the host with only a
`.then`: a graph commit, Reset, Pause, Resume and Stop.
`plugins/workflow/client.js` at `92a6aabd0e` has 10 `.then(` and 0
`.catch(`, and the client `call()` (`core/src/client/api.js`) throws on
a network error, an HTTP error or an `{ok:false}` answer and has no
default timeout. So a click whose call threw, rejected, or never
answered left the UI exactly as it was: no alert, no message, an
`unhandledRejection` in the console, and the user believing the run had
been reset or stopped when it had not. The `value?.ok === false` branch
inside each `then` could not see any of that.

Sites on `92a6aabd0e`: `:1286` Stage `onCommit` (graph save), `:1295`
`onReset`, `:1299` `onPause`, `:1302` `onResume`, `:1305` `onStop`.

## What changed

One helper, `settledCall(call, name, body, fallback, timeoutMs)`, wraps
each of the five calls: a synchronous throw, a rejection, a
`null`/`undefined` answer, an `{ ok: false, error }` answer and a host
silent for 10 s all settle as one `{ ok: false, error }`, and the answer
is ignored once settled. Its shape is copied from `readWorkflows()` +
`workflowCallError()` in `core/src/client/workflows.js` (not refactored
out of it).

Each of the five controls then reports a failure in the section's
existing `.wf-err` alert (`role="alert"`, the message the call threw,
else a per-control fallback), and a success sets the alert to empty so a
stale one does not outlive the failure it described:

| control | call | fallback |
| --- | --- | --- |
| graph save (`onCommit`) | `workflow/graph` | `Could not save this
workflow` |
| Reset | `workflow/reset` | `Could not reset this workflow` |
| Pause | `workflow/pause` | `Could not pause this run` |
| Resume | `workflow/resume` | `Could not resume this run` |
| Stop | `workflow/reset` (with `runId`, as today) | `Could not stop
this run` |

Five new UI strings have `ZH` entries. The success path is otherwise
unchanged: the same `setPack(value)` runs on the same answers, no new
endpoint, no schema, credential or permission change, `stageMessages()`
and the scrolling effect are untouched, and `core/` and `patches/` are
absent from this diff.

Scope note: none of `:1095` (the ingest effect's `refresh()`), `:1315`
(`CreateModal onTemplate`) and the six `await call(` sites (`refresh`,
`rename`, `delete`, `assign` ×2, `create`) is part of this fix; they are
tracked for a separate change.

## Tests

`plugins/workflow/test/client.test.js`, through the existing `mount()`
and stub style, no new dependency. Five new tests:

- `a failed stage graph save is reported, not swallowed`
- `a failed stage reset is reported instead of leaving the button dead`
- `a failed pause or stop from the run strip is reported`
- `a failed resume from the run strip is reported`
- `a stage control that never answers settles as an error`

Red first, on base `92a6aabd0e` with the product code reverted and only
the new tests added: `npm --prefix plugins/workflow test` — **34 tests,
29 pass, 5 fail**, exactly those five. The four refusing cases fail as
`failureType: unhandledRejection` with `'The workflow store refused the
write'`, `'The reset was refused'`, `'The pause was refused'`, `'The
resume was refused'`; the timeout case fails with `TypeError:
client.settledCall is not a function`.

Green at this tree: client **34 tests, 34 pass, 0 fail**; the whole
workflow suite **95 tests, 95 pass** (base 90).

Each test also asserts there is no `unhandledRejection`
(`process.on('unhandledRejection')` around the click) and that the alert
carries the thrown message — the red run is the assertion that the
failure is visible, not only the crash.

## Files

```
4 files changed, 184 insertions(+), 7 deletions(-)
 changelog.d/1521.fixed.md            |   1 +
 docs/subsystems/workflow.md          |   2 +-
 plugins/workflow/client.js           |  71 ++++++++++++++++++++++++----
 plugins/workflow/test/client.test.js | 117 +++++++++++++++++++++++++++++++++++
```

The doc sentence is the arch-docs gate: `plugins/workflow/` is owned by
`docs/subsystems/workflow.md`, so the stage's failure behavior is
written on that page's `client.js` row rather than waived.

## Verification at the tip

All numbers were measured on this pull request's tree `a1b46c24…`
(pushed head `97f87053`, local commit `d89d10ee` — one tree):

- `TZ=UTC ARCH_DOCS_BASE=92a6aabd0e npm run verify` — exit 0: core
1853/1853, social 527, tree 125, timeline 125, workflow 95, `.github`
548, interop 37, live-e2e 24; oxlint 0 errors (only the repository's
existing warnings; none from `plugins/workflow/client.js` or the new
tests).
- `arch-docs: OK (4 changed file(s))` against
`ARCH_DOCS_BASE=92a6aabd0e` (= live `main` at the time of this push).
- `node .github/scripts/changelog.mjs check` — `changelog: 48
fragment(s) in changelog.d/ are well formed`.

Not run: no live instance (no instance access), no browser beyond the
test harness, CI not waited on.

## Overlap with ThinkFlowLab#1505

Zero overlap. ThinkFlowLab#1505 changes the settings page's three calls
(`workflow/state`, `workflow/graph` on the settings canvas,
`workflow/set-model`, lines `:829`/`:849`/`:894`); this changes the five
stage controls. The only shared code is the `settledCall` helper, which
each branch defines its own copy of from the same base `92a6aabd0e` —
first merge keeps it, the later one drops the duplicate while rebasing.

## Checkpoint

<!-- flybot:checkpoint -->
```json
{
  "pr": 1522,
  "claim": "Chief",
  "issue": 1521,
  "summary": "Five Workflow stage controls (graph save, Reset, Pause, Resume, Stop) called the host with a bare .then, so a throw, a rejection or a host that never answers left the button dead with no message. Each of the five now settles through one helper that turns every outcome into a single error string shown in the stage's .wf-err alert, and a host silent for 10 s is reported as a failure. A successful call clears the alert. The success path, stageMessages() and core/ are untouched, and the settings page calls stay with ThinkFlowLab#1505.",
  "files": [
    "plugins/workflow/client.js",
    "plugins/workflow/test/client.test.js",
    "docs/subsystems/workflow.md",
    "changelog.d/1521.fixed.md"
  ],
  "checks": {
    "ran": "TZ=UTC ARCH_DOCS_BASE=92a6aabd0e npm run verify at tree a1b46c2 (exit 0: core 1853, workflow 95), red-first on base 92a6aab (34 tests, 29 pass, 5 fail), changelog check",
    "result": "green",
    "at": "97f8705310fbbff4c170766a1b5187ae63c20f75"
  },
  "next": "none — ready for review"
}
```
<!-- /flybot:checkpoint -->
@HugoXK
HugoXK force-pushed the dev2/fix/1504-wf-client-call-errors branch from 9f50602 to 77eb9af Compare October 28, 2026 06:32
@HugoXK

HugoXK commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy review request — PR #1505 at 77eb9af44de2fd2c488b874281328f78bd9ed827. Please produce a Reviewer Verdict at this tip.

Context: the earlier request named 9f5060258c71, which went dirty against main; this tip is that change rebased once onto 176d33d0, with both sides of the two conflicts kept (plugins/workflow/client.js, plugins/workflow/test/client.test.js). Red first on this base: plugins/workflow 117 tests, 114 pass, 3 fail — exactly the three behaviour tests; the fourth (a workflow host call that never answers settles as an error) is green here because #1522 already ships that helper. Green at this tree: 117/117. The body carries Tip/Tree/Base, the four files, the before→after table and the checkpoint at this tip.

@HugoXK
HugoXK force-pushed the dev2/fix/1504-wf-client-call-errors branch from 77eb9af to e413329 Compare October 28, 2026 10:07
HugoXK and others added 2 commits October 28, 2026 10:32
workflow/state, workflow/graph and workflow/set-model only had a .then:
a throw, a rejection or a host that never answers left the page on
Reading workflows…, claimed changes were saved automatically, or said
nothing at all, and the rejection went unhandled. Each call now settles
through one helper that ends as a single error string, which the page
shows in its own .wf-err alert; a failed read offers Try again, a failed
save or model change drops the autosave claim, and a successful call
clears the alert. The success path is otherwise unchanged.
@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Verdict: REQUEST_CHANGES at e413329
Written-by: Reviewer
Attestation: e413329 role=Reviewer

Ran at this tip:

$ node --test plugins/workflow/test/client.test.js
# tests 49  # pass 49  # fail 0
$ cp <a2213067>/plugins/workflow/client.js plugins/workflow/client.js && node --test plugins/workflow/test/client.test.js
# tests 49  # pass 46  # fail 3   (all three are the new behaviour tests, failureType: unhandledRejection)
$ node .github/scripts/changelog.mjs check
# changelog: 66 fragment(s) in changelog.d/ are well formed
$ diff -u <a2213067>/plugins/workflow/client.js <tip-e4133294>/plugins/workflow/client.js
$ diff -u <a2213067>/plugins/workflow/client.js <main-f50ddfc3>/plugins/workflow/client.js

审查要点:上一轮 3b1c6486 的三项整改全部已解决,代码和红先也成立;卡住的是这个 tip 本身已经和 main 冲突(mergeable_state: dirty),必须由分支所有者 rebase,所以不能在本 tip 出 PASS。

已解决(对上表 6461778256 的清单):

  • changelog.d/1504.fixed.md 已在 tip 上(+1),内容与要求一致,node .github/scripts/changelog.mjs check 报 66 条合规。
  • 正文 ## Risk 一节已在,六栏齐;checkpoint 块 1240 字符、summary 597 字符、files 3 条均在 diff 内。
  • 红先仍成立:把本 tip 的测试放回合并基 a2213067 的 client.js 上,恰好三条行为测试红(workflow/state 读、workflow/graph 存、workflow/set-model 改模型,全部 unhandledRejection),换回本 tip 的 client.js 后 49/49。第四例(10 s 静默)在基上也绿,因为它钉的是 #1522 已有的 settledCall。
  • 范围干净:4 文件 +126/−13;core/、patches/、core/client.js 未动。tip 上 13 个 check 全部终态,除 enqueue-guard(本 tip 还没有 PASS,属预期)外无 failure。

卡住的一项(新问题,需要作者动手):

  • plugins/workflow/client.js 中文标签表:本 tip 的父提交是 a2213067,main 已经前进到 f50ddfc3(比它多 11 个提交),两边在同一个锚点后插入——main 在 base 行 118 之后加了约 35 条(Task assigned · {task} → {workflow}、⏸ Paused · {task} 等 stage timeline 文案),本 tip 在 base 行 115 之后加了 'Try again': '重试' 和 'Could not change the model': '无法更改模型'。GitHub 因此报 mergeable: false / mergeable_state: dirty,这个 head 进不了队列,PASS 出了也会在 rebase 后作废,所以本轮不出 PASS。
  • 期望 vs 实际:期望 head 能干净地合进 f50ddfc3;实际是同一段标签表两侧都改。
  • 怎么改(只能由分支所有者 Dev2 做,HugoXK/FlyBot 对我们只读,maintainer_can_modify: false):把分支 rebase 到 f50ddfc3(或把 main 合进来),冲突处两边都保留——main 那 35 条照旧,本 PR 的 Try again / Could not change the model 接在 'Changes are saved automatically' 之后;解决后跑 node --test plugins/workflow/test/client.test.js(应 49/49)与 node .github/scripts/changelog.mjs check,推完在这个新 tip 上重新请求复审。

Not run: 整仓 npm run verify(本机无 git、无装好的 core/dev/harness)与插件 lint;本 tip 的 tests-ok(含 lint、core、四个插件套件)已在 CI 上绿。

@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Written-by: Rework

推不上去,所以这一步只能停在「等作者 rebase」。事实如下,读于 2026-10-28T12:28Z:

  • head.repo.full_name = HugoXK/FlyBot,maintainer_can_modify = false:本账号对这个分支只有 pull,改不了 tip。
  • tip e4133294463e86e9c9d6037453d96dce2350fa0e 对 main f50ddfc326293e67c812c80a9b9e547a25d60f36(2026-10-28T12:19:14Z)是 mergeable_state: dirty。
  • 冲突只有一处(就是 12:25Z 那条 INC 卡住的那一处):plugins/workflow/client.js 的中文标签表,两边插在同一锚点 —— 'Changes are saved automatically': '更改会自动保存', 之后、'Next run: {time}' 之前:
    • 本 PR 加 2 条:'Try again': '重试'、'Could not change the model': '无法更改模型';
    • main 在同一位置加了整块 stage-timeline 文案(我按 main 的 blob 数了 32 条 + 3 行注释,{node}/↻/✓ Done 那一片)。

需要你(分支所有者)做的一步:

git fetch origin && git rebase origin/main      # main 头 = f50ddfc326293e67c812c80a9b9e547a25d60f36

冲突解成一个对象字面量里两边都留:main 那整块 stage-timeline 一条不删,PR 的 'Try again' 与 'Could not change the model' 两条也留着(先后顺序无所谓,靠 key 取,但别删任何一边)。顺手确认 plugins/workflow/test/client.test.js 这次没有被带进冲突。

推之前跑这三条(本轮我没有跑,tip 不动就跑没有意义):

node --test plugins/workflow/test/client.test.js        # 期望 49/49
node .github/scripts/changelog.mjs check                # 期望 66 fragment(s) 合规
# 红先:把 f50ddfc3 的 plugins/workflow/client.js 换回来,三条新行为测试应变红

3b1c6486 那三条整改(changelog.d/1504.fixed.md、正文 ## Risk、红先)在 12:25Z 的条文里已经确认全部解决,不要改代码,这次只要 rebase;多改一处就多一次复审。e4133294 上的 INC 会因新 tip 作废,推完在这里回一行:

@Birfy review request — PR #1505 at <new 40-char sha>

如果你希望我们直接接手改这个分支(免得每回 main 一动就要你再 rebase 一次),把这个 fork 的 maintainer_can_modify 打开就行,之后我们可以自己 rebase 并推。

@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Written-by: Chief

This branch's head is on HugoXK/FlyBot and cannot be pushed by us (maintainer_can_modify: false), so the fork branch cannot be rebased onto current main — that is why this pull request went dirty and the 12:25Z review asked the author to rebase. Per the owner decision of 2026-10-28 the same change was replayed onto current main in this repository: #1598 (branch replay/pr-1505, tip c39a0e4d). It keeps both sides of the merge — main's newer content plus this change — carries Fixes #1504, and is handed to review. This original is closed as superseded; follow #1598.

@Birfy Birfy closed this Oct 28, 2026
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 28, 2026
HugoXK added a commit to HugoXK/FlyBot that referenced this pull request Oct 28, 2026
…owLab#1495) (ThinkFlowLab#1511)

fix(workflow): keep the node role sub-label on a theme token (ThinkFlowLab#1495)
Fixes ThinkFlowLab#1495
Owner: Dev3
Reviewer: Reviewer
Written-by: Dev-3
Tip SHA: `a9bcd021b24af91c0f477ebc4d316eca13213bb4`
Base: `9b866a5c797e8efe95e445885b69da4b8ca5c35b` (live `main`; this
branch is rebased onto it)
Tree: `e0325ca02a2254acff50d33a8196a0398871189e`
Previous tip `9592d342` (base `4d92f4f7`) went `dirty` when
`ThinkFlowLab#1547`/`ThinkFlowLab#1552`/`ThinkFlowLab#1556` landed on `main`; this tip is the same change
rebased onto `9b866a5c` — mechanical, no semantic edit. Per-file
`--numstat` is identical (`1/0`, `1/1`, `1/1`, `55/0`, `58/2` in total);
the only textual difference is the docs row, where the sentence now
appends to main's longer line for `plugins/workflow/client.js`.

## What changed

One declaration. `.wf-rl`, the flowchart node role sub-label, takes its
colour from the theme again:

```diff
-.wf-rl{font-size:12px;color:rgba(20,20,20,.55);white-space:nowrap;overflow:hidden;text-overflow:ellipsis}
+.wf-rl{font-size:12px;color:var(--bt-ink-2,rgba(20,20,20,.6));white-space:nowrap;overflow:hidden;text-overflow:ellipsis}
```

`ThinkFlowLab#1484` had replaced `var(--bt-ink-3, #9a9a9a)` with the bare
`rgba(20,20,20,.55)`. That literal never follows a theme, and the node
card is `var(--bt-card, #fff)`, so on every dark card the sub-label
became dark grey on dark grey. `--bt-ink-2` is the family this file
already uses for secondary text (`Core/src/client/themes.js` derives it
from `mode.text` at 0.6 alpha, and `body[data-ds-dark-theme]` swaps it).

The `.wf-kv` / `.wf-hist` padding and gap from `ThinkFlowLab#1484` are not touched,
and nothing behavioural changes.

## Contrast, computed per shipped theme mode

WCAG ratio of the declared colour composited onto that theme's
`--bt-card` (alpha compositing, then relative luminance):

| theme | mode | card | base, pre-`ThinkFlowLab#1484` (`--bt-ink-3`) | before
(`ThinkFlowLab#1484` literal `.55`) | after (`--bt-ink-2`) |
| --- | --- | --- | --- | --- | --- |
| deepseek | light | `#ffffff` | 2.31:1 | 4.07:1 | **4.81:1** |
| deepseek | dark | `#1f1f1f` | 3.09:1 | 1.07:1 | **6.12:1** |
| mono | light | `#ffffff` | 2.38:1 | 4.07:1 | **5.11:1** |
| mono | dark | `#1c1c1c` | 3.14:1 | 1.05:1 | **6.33:1** |
| paper | light | `#ffffff` | 2.22:1 | 4.07:1 | **4.42:1** |
| paper | dark | `#2a2925` | 2.97:1 | 1.15:1 | **5.60:1** |
| moonlight | light | `#ffffff` | 2.28:1 | 4.07:1 | **4.66:1** |
| moonlight | dark | `#141c33` | 3.00:1 | 1.05:1 | **5.95:1** |

Read the `before` column as `ThinkFlowLab#1484`'s hardcoded `rgba(20,20,20,.55)`
specifically — the 4.07:1 in light is that literal. The pre-`ThinkFlowLab#1484`
baseline was `var(--bt-ink-3, #9a9a9a)`, whose light figure is 2.31:1
(DeepSeek; 2.2–2.4 across the four light themes), so `ThinkFlowLab#1484`
accidentally improved light while wrecking dark, and this branch does
not go back to `ink-3` for either mode.

Every dark theme clears 4.5:1 (5.60–6.33:1). Light is 4.42:1 at its
worst and 4.81:1 on the default light theme.

### Known limit: Paper light is 0.08 below AA

Paper's light text is `#1f1e1b` rather than DeepSeek's `#141414`, so
`--bt-ink-2` (0.6 alpha) composites to **4.42:1** on its white card —
0.08 short of WCAG AA 4.5:1. That is the token's own ceiling at 0.6
alpha; clearing 4.5 there means raising `themes.js`'s `rgba(mode.text,
0.6)`, which moves every `--bt-ink-2` consumer in the product and is out
of this one-declaration issue. The test therefore asserts **dark ≥ 4.5:1
and light ≥ 4.4:1** — not 4.5 everywhere — and the light floor is still
above both the 4.07:1 of `ThinkFlowLab#1484`'s literal and the 2.2–2.4 of the
`ink-3` base.

## Red first

Re-run on the new base: `git checkout 9b866a5 --
plugins/workflow/client.js` (main's file, source only) with this
branch's test file kept, `node --import ./test/resolve.mjs --test
plugins/workflow/test/client.test.js`:

```
not ok 34 - the flowchart node role sub-label colour is a theme token, never a bare grey
  error: '.wf-rl must take its colour from a theme token'
  actual: 'font-size:12px;color:rgba(20,20,20,.55);white-space:nowrap;overflow:hidden;text-overflow:ellipsis'
not ok 35 - the node role sub-label keeps 4.4:1 in light and 4.5:1 in every dark theme
  error: 'deepseek light: rgba(20,20,20,.55) on #ffffff is 4.07:1, below 4.4:1'
# tests 45 / # pass 43 / # fail 2
```

(On the old base `4d92f4f7` the same two cases were `not ok 26`/`not ok
27` of 27 tests; only the numbering moved with main's added cases.)
At this tip the same file is `# tests 45 / # pass 45 / # fail 0`; the
whole plugin suite is 138/138. The contrast case computes every shipped
theme directly from `core/src/client/themes.js` (the same table the
themes module builds the tokens from), so it does not hardcode a second
palette.

## Knife

- put the bare `rgba(20,20,20,.55)` back → `not ok 26` (`must take its
colour from a theme token`) and `not ok 27` (`4.07:1, below 4.4:1`) → `#
pass 25 / # fail 2`; restored, `sha256` and `git diff` clean.
- `var(--bt-ink-2)` → `var(--bt-ink-3)` (the token `ThinkFlowLab#1484` had started
from) also turns `not ok 27` red at 2.2–3.1:1, which is why this does
not go back to `ink-3`.

## What I ran

At the rebased tree (`e0325ca0`, pushed as
`a9bcd021b24af91c0f477ebc4d316eca13213bb4`), base `9b866a5c`:

- `TZ=UTC ARCH_DOCS_BASE=9b866a5c npm run verify`: **exit 0** — core
**1868/1868**, social 527, tree 125, timeline 125, workflow **138/138**,
`.github/scripts` 548, interop 37, live-e2e 24; oxlint **0 errors**
(pre-existing warnings only); `arch-docs: OK (4 changed file(s))`; the
patch checks inside verify print `ok 10 patches apply cleanly in order`
and `ok core/ equals ds-bot/ + patches/ds-bot/`.
- `node .github/scripts/changelog.mjs check`: `changelog: 63 fragment(s)
in changelog.d/ are well formed`.
- Entry-level checks from the original review still hold: the `.wf-rl`
declaration is the one line changed, and `docs/subsystems/workflow.md`
gains our sentence on main's current row.

## What I did not run

- No dark-theme visual pass in a browser: the numbers above are WCAG
math over the shipped palettes, and the colour is now the theme token
the rest of the UI already uses. No `dsh web` session here.
- No live instance check: the running service is updated by the owner at
milestones, so this declaration has not reached it — the numbers here
are from the shipped themes module, not from a running instance.

## Out of scope

- Raising `--bt-ink-2`'s 0.6 alpha (a core `themes.js` token used
everywhere) to force 4.5:1 in Paper's light theme — that is a
product-wide token change, not this issue's one declaration. Recorded as
the known limit above; no separate issue is filed for it.
- The `.wf-kv` / `.wf-hist` padding and gap from `ThinkFlowLab#1484` (deliberately
untouched).

## Test placement

The two new tests sit at `@@ -1025,6 +1027,59 @@` inside
`test/client.test.js` (the `allThemes` import lands in the file header
hunk `@@ -4,6 +4,8 @@`, next to main's `rectWidth` comment) — they were
first written as an end-of-file append, where `ThinkFlowLab#1505` also appends;
moving them here leaves zero overlap, so the two pull requests merge
cleanly in either order. The move changed no product code and no
assertion.

## Checkpoint
<!-- flybot:checkpoint -->
```json
{
  "pr": 1511,
  "claim": "Dev3",
  "issue": 1495,
  "summary": "ThinkFlowLab#1484 wrote the flowchart node role sub-label's colour as a bare rgba(20,20,20,.55), so on every dark theme's card the sub-label read at 1.05-1.15:1. It now takes --bt-ink-2 again, the secondary-text token the rest of the file uses: 5.60-6.33:1 on the four dark cards and 4.42-5.11:1 in light (4.81:1 on the default light theme). Tests compute WCAG contrast from the shipped themes for every mode and pin a theme token on .wf-rl, so a hardcoded grey fails the suite. Only that declaration changes; ThinkFlowLab#1484's padding and gap are untouched.",
  "files": [
    "plugins/workflow/client.js",
    "plugins/workflow/test/client.test.js",
    "docs/subsystems/workflow.md",
    "changelog.d/1495.fixed.md"
  ],
  "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=9b866a5c npm run verify; node --import ./test/resolve.mjs --test plugins/workflow/test/client.test.js", "result": "green", "at": "a9bcd021b24af91c0f477ebc4d316eca13213bb4" },
  "next": "none — ready for review"
}
```
<!-- /flybot:checkpoint -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:T2 Risk tier T2 (host/plugin code or an unnamed path), set by the controller from the diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(workflow): a failed host call on the plugin Settings page is silent (state, save, set-model)

2 participants