Repository navigation
fix(workflow): report a failed or unanswered stage control instead of swallowing it - #1522
Merged
Merged
Conversation
Contributor
Author
|
|
Collaborator
|
Verdict: PASS at 97f8705 Ran at this tip (2026-10-28T06:04Z): 审查要点:
|
Birfy
added a commit
that referenced
this pull request
Oct 28, 2026
… handles (#1514) Rebuilt on main 176d33d by Rework. main had since changed plugins/workflow/client.js and plugins/workflow/test/client.test.js (#1535, #1497, #1536, #1522), so this commit carries main's changes merged with the branch's three files; the changelog fragment is unchanged from 768a08c. Written-by: Rework Docs-not-needed: UI-only form styling and handle placement; no workflow architecture or API change.
Birfy
added a commit
that referenced
this pull request
Oct 28, 2026
The room's right-hand Flow tab painted the Settings world canvas in a 400px panel, so nodes past the first column, their wires and the parallel-branch label were cut at the panel edge behind an inner horizontal scrollbar. fitPlane() scales node centres (never the node boxes) into the drawer's 364px content box; the drawer paints the fitted nodes, wires and frames, and a drag or drop divides its pointer delta by the fit scale so the stored graph keeps world coordinates. Labels wrap, the inner horizontal scroll is gone, and the Settings canvas keeps its own behaviour (#1536). Replayed onto main 176d33d (#1535, #1497, #1536, #1522 landed after the original base). The fitted drawer keeps the plain wf-canvas class and is selected by data-fit="drawer", so the stage-control tests and main's canvas lookup both hold; the docs row keeps both sentences. Fixes #1513 Written-by: Rework
HugoXK
added a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 28, 2026
… retry (ThinkFlowLab#1540) Fixes ThinkFlowLab#1530 Owner: Chief Reviewer: Reviewer Written-by: Dev-1 Base: `176d33d034e741b68041c0676da1707001cb6e88` Tip: `fda04ef4807c33603a0b431dca7306abe1f7d36a` Tree: `5bff772de34533ba689ca013ae9c95b391048db9` ## What changed **Before:** a live step was bounded by nothing. When `dispatch` returned `deny`/`hold`/`no-sender`, `handOff` wrote one chat line and moved on; when the Bot never replied, nothing noticed. The run stayed `running` with its node in flight, holding a lane slot, forever. Both paths look the same from outside and have the same cause — no wait had a limit — so one change covers them. **After:** - A hand-off that does not land is retried twice with a short backoff (`HANDOFF_RETRY_DELAYS_MS = [1000, 2000]`, never twice inside one hand-off, so a 429 is not hammered). The chat names the code and the attempt. When it still does not land, the step and the run fail with the reason written into the chat, and the Bots are released. - Every running step holds a deadline (`STEP_DEADLINE_MS`, 10 minutes, one constant). At the deadline the node and the run fail as `failed`, the chat says which step timed out, `warn` names it once, the ephemeral Bots are released, and the queue drains. - Timers are keyed by run and node: cleared when the step moves, on Pause, and by Stop, Reset, delete, `reap` and `dispose`. A paused run holds none; Resume arms it again from that moment. - `stepResult` no longer drops a reply that matches no running step (its run ended, or the node already moved): one `warn` per Bot and run, and one when the advance itself refuses. `core/` is untouched; this is `plugins/workflow` only. Rebased onto live main `176d33d0` (`ThinkFlowLab#1522`) after `ThinkFlowLab#1535`, `ThinkFlowLab#1497`, `ThinkFlowLab#1536` and `ThinkFlowLab#1522` landed. Conflicts were only in `docs/subsystems/workflow.md` — the `service.js` / `ephemeral.js` table rows, invariants 2–3 and the Tests list, which those PRs also rewrote — resolved by keeping main's newer text and putting this PR's bounded-wait text back into it. `plugins/workflow/src/service.js` and `plugins/workflow/README.md` auto-merged. The rebase was mechanical (Dev-3): no logic, message or test changed, and the diff against `176d33d0` adds exactly the lines the old diff against `62023599` added, line for line (5 files, +435/−14). ## Notes for review - **The old hand-off contract is unchanged, so `test/queue.test.js` is not touched.** `dispatch no-sender does not mark the node handed and the chat names it` still holds: a first failed dispatch still writes the chat line naming the code (`… 没有接到这一步(no-sender),1 秒后重试(1/2)。`), still leaves `handed` unset, and the run is still `running` at that moment — the retry is a timer, so the test's synchronous assertions see one dispatch. Only the give-up path is new (a second line, then `failed`). No contract change to note. - **The new test does not use `t.mock.timers`.** `plugins/workflow/src/service.js` already takes injected `setTimer`/`clearTimer`, so the test injects its own scheduler (`test/deadline.test.js:20-50`). That scheduler re-scans its queue after each fired timer and awaits it, so one `tick(RETRY_SPAN + 1)` runs a retry that arms the next one; the assertions cover it (`dispatched.length === 3`, `timers.pending() === 0`). This side-steps the nested-`tick` pitfall entirely instead of depending on it. - **The real deadline handle is `unref()`ed** (`setTimerAt`, `service.js:164-175`), so suites that use real timers (`queue.test.js`, `ephemeral.test.js`, whose live runs never reply and are never disposed) are not held open for ten minutes: the whole `plugins/workflow` suite runs in 320 ms. ## Verification at the tip Tip SHA: `fda04ef4807c33603a0b431dca7306abe1f7d36a` (tree `5bff772de34533ba689ca013ae9c95b391048db9`, base `176d33d034e741b68041c0676da1707001cb6e88`) The earlier `Verdict: PASS` was recorded on the old tip `21e9621db9`; the base moved and the PR went `dirty`, so that PASS does not name this tip. This tip is that same content rebased: the added lines are identical to the old diff, line for line (5 files, +435/−14), and no behaviour changed. Ran a red-first test, `plugins/workflow/test/deadline.test.js` (7 cases, injected scheduler so a timer count is assertable), all on the new base `176d33d0`: - base alone: `plugins/workflow` 113/113 pass. With this test file added it does not even load (`does not provide an export named 'HANDOFF_RETRY_DELAYS_MS'`). - base with only the two constants added (no behaviour): 0 pass / 7 fail — the run stays `running` on every path. - at the tip: 7/7 pass, and the `plugins/workflow` suite is 120/120 pass — main's 113 plus this PR's 7. - `TZ=UTC ARCH_DOCS_BASE=176d33d0 npm run verify` → exit 0: core 1868, social 527, tree 125, timeline 125, workflow 120, `.github/scripts` 548, interop 37, live-e2e 24 (3374 tests, 0 fail), plus oxlint, arch-docs (5 changed files), plugin lint, `build-client --check` and `ds-bot-patches check`. Not run: the smoke test (no `core/index.js` or `src/host/` change), Desktop, and against the owner's live instance (out of scope: the store lives outside this workspace). ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 1540, "claim": "Chief", "issue": 1530, "summary": "A live Workflow step had no bound on its wait, so a hand-off that never landed or a Bot that never replied left the run running forever. A dispatch that does not land is now retried twice with a short backoff, then fails the step with the reason in the chat; each running step holds a ten-minute deadline that fails the run, names the step, warns once, releases its Bots and drains the queue. Timers are keyed by run and node and cleared when the step moves, on Pause, and by Stop, Reset, delete, reap and dispose. A reply matching no running step is warned about instead of being dropped.", "files": ["plugins/workflow/src/service.js", "plugins/workflow/test/deadline.test.js", "docs/subsystems/workflow.md", "plugins/workflow/README.md", "changelog.d/1530-workflow-step-deadline.fixed.md"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=176d33d0 npm run verify", "result": "green", "at": "fda04ef4807c33603a0b431dca7306abe1f7d36a" }, "next": "none — ready for review" } ``` <!-- /flybot:checkpoint -->
HugoXK
pushed a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 28, 2026
…FlowLab#1547) Fixes ThinkFlowLab#1513 Written-by: Dev Owner: Chief Reviewer: Reviewer Base: `176d33d034e741b68041c0676da1707001cb6e88` (`main` head at 2026-10-28T06:56Z) Tip SHA: `74de9c6c77e140bc34c4ef2f41f2125ebdc24b9a` Tree: `74147a95640c11f4e0f9cd7d2c167ec6d0d2e1a0` Replay note (Rework, 2026-10-28T07:00Z): the reviewed tip `5111163e` sat on `62023599`, four commits behind `main`, and `main` had meanwhile changed this exact file set (ThinkFlowLab#1535, ThinkFlowLab#1497, ThinkFlowLab#1536, ThinkFlowLab#1522). This commit's parent is the current `main` head, the PR's own hunks are merged with those changes by hand, and `main...head` lists exactly the four files. Changes made while replaying: - `plugins/workflow/client.js`: the fitted drawer keeps the plain `wf-canvas` class and is selected by `data-fit="drawer"`; the narrow CSS now reads `.wf-canvas[data-fit="drawer"] …`. ThinkFlowLab#1522's stage-control test looks the drawer canvas up as `className === 'wf-canvas'`, so it needs no edit and `tests-ok` is green on the merge tree. - `docs/subsystems/workflow.md:36`: one table row now carries both ThinkFlowLab#1522's stage-control sentence and this PR's fitted-drawer sentence. Invariant 12 no longer claims the Settings canvas "keeps its own scrolling world plane" — ThinkFlowLab#1536 made it scale to its own width, so the sentence names that instead. - The old body's Settings sentence and the checkpoint's `checks.ran` were stale; both are replaced below with what this tip actually ran. ## What changed The drawer Flow tab mounted the same absolute-coordinate canvas as Settings → Workflow graphs: `.wf-plane{width:0}`, node centres at raw `GAP_X=188` columns, wires on a `left:-480px;width:960px` svg, all inside a `.wf-canvas{overflow:auto}` in a 400px panel whose content box is 364px. Nodes past the first column, their wires, the parallel-branch label and the run strip were cut at the panel edge, inside an inner horizontal scrollbar next to the drawer's own vertical one. - `fitPlane(nodes, panelWidth)` is the new pure helper (exported): it scales node **centres** (never the node boxes, so text stays readable) into `panelWidth - 2*FIT_PAD - NODE_W`, centres the graph horizontally, keeps `scale <= 1`, and returns fitted nodes plus the `scale` / `offsetX` / `offsetY` an edit needs to invert a pointer position. - The drawer Flow (`DRAWER_FLOW_W = 400 - 2*18 = 364`) renders the fitted nodes, wires with `wirePath(from, to, 0)` in the plane's own coordinates, and `parallelFrameStyle` over the fitted members. The plane gets the panel width and the wires svg follows it. The Settings canvas keeps `wirePath`'s default `WIRE_SPAN`; its width scaling and the page's own scroll are ThinkFlowLab#1536's, untouched here. - The drawer canvas carries `data-fit="drawer"`: `min-height:0;align-items:stretch;overflow:hidden` so there is no inner scroll; `.wf-dr-body` keeps the single vertical scroll and gets `overflow-x:hidden`. `.wf-plabel` wraps (`white-space:normal;text-align:center;max-width:calc(100% - 16px)`) and the drawer `.wf-term` wraps too. - Editing still commits world coordinates: a drag stores `x + dx/scale` (the scale the drag started with, so fitting does not chase the pointer) and a dropped Bot maps `offsetX/offsetY` back through `offsetX`/`offsetY`. Wire, reconnect and the per-node inspector are unchanged. - Docs: the client.js row and invariant 12 in `docs/subsystems/workflow.md` now name the fitted drawer; `changelog.d/1513-workflow-panel-fit.fixed.md` is the fragment. ## Verification at the tip Tip SHA: `74de9c6c77e140bc34c4ef2f41f2125ebdc24b9a` (parent `176d33d0`) Ran on this tree (the committed blobs are byte-identical to the files these ran on — `git hash-object` matched the tree's blob shas for all four): - `node --test` over the eleven runnable `plugins/workflow/test/*.test.js` — 116 tests, 116 pass. - `node --test test/client.test.js` (in `plugins/workflow`) — 40 tests, 40 pass (34 from this PR plus ThinkFlowLab#1522's six; the merge tree the Reviewer built failed one of those before this replay). - Red first: the same test file with `plugins/workflow/client.js` back at `main`'s blob — 35 pass, 5 fail: the four new fit tests plus `the sidebar opens the flowchart, and a drag, a wire and a dropped Bot are saved` (it now asserts the `delta/scale` mapping). - `node .github/scripts/changelog.mjs check` — 58 fragments well formed. - `node scripts/ds-bot-patches.mjs check` — 10 patches apply cleanly in order; `core/` equals `ds-bot/` + `patches/ds-bot/`; the pin is skipped here (no `git` history in the extracted tree) and this diff touches neither. - `node .github/scripts/check-mojibake.mjs --check` — 0 occurrences of U+FFFD in 1243 text files. - `node --check plugins/workflow/client.js` and `node --check plugins/workflow/test/client.test.js` — exit 0. - Diff from `main` `176d33d0`: 4 files, +192/−55 (client.js +98/−45, tests +91/−8, docs +2/−2, fragment +1) — inside the T2 limits (250 lines / 12 files). Not run (and why): - `test/host.test.js` and the core suite — they load the DSH harness (`npm run harness`), which is not installed on this box; CI runs them on Node 24. - `node scripts/lint-plugins.mjs` (oxlint) and `node core/scripts/build-client.mjs --check` (esbuild) — no `node_modules` here; CI runs both. - A live 400px browser recheck: this box has no browser. The acceptance is covered by the pure fit test, the wire-anchor/plane test, the CSS assertions and the drawer-vs-Settings mount test. ## Risk Expected **T2**: `plugins/workflow/client.js` is plugin code no `risk-rules.json` rule names (the T3 list covers `plugins/social/**` and `core/src/shared/**`, and an unnamed path is never silently T0/T1); no T3 `diffPatterns` hit; no `package.json` / lockfile. T0/T1 would need the change to be pure copy, which a layout mapping is not. Scope is the drawer Flow only: the Settings canvas scaling (that is ThinkFlowLab#1536's), the graph record, the wire/reconnect semantics and the inspector are unchanged, and `fitPlane` never enlarges a graph. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 1547, "claim": "Chief", "issue": 1513, "summary": "The 400px room side-panel Flow tab now paints the graph fitted to the panel instead of the Settings world canvas. fitPlane scales node centres (never the node boxes) into the panel width, wires and parallel frames draw in that fitted space, and drag/drop pointer deltas are divided by the fit scale so the stored graph keeps world coordinates. Labels wrap instead of clipping mid-word and the inner horizontal scrollbar is gone; the Settings canvas keeps ThinkFlowLab#1536's own scaling untouched.", "files": [ "plugins/workflow/client.js", "plugins/workflow/test/client.test.js", "docs/subsystems/workflow.md", "changelog.d/1513-workflow-panel-fit.fixed.md" ], "checks": { "ran": "plugins/workflow node --test (11 files) 116/116; client.test.js 40/40; red first with main's client.js blob 35/40 (5 fail: 4 new + drag/drop); changelog check 58 fragments; ds-bot-patches check 10 patches; mojibake ok; node --check exit 0", "result": "green", "at": "74de9c6c77e140bc34c4ef2f41f2125ebdc24b9a" }, "next": "none — ready for review; T2 needs the Reviewer's PASS at this tip (the REQUEST_CHANGES at 5111163 is void by the new commit)." } ``` <!-- /flybot:checkpoint -->
HugoXK
pushed a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 28, 2026
Two effects tested the same "a run is live" condition and both armed a 2 s interval, so every tick read the whole workflow/state twice ([0, 2001, 2001, 4003] on the probe). refresh() also awaited the host without settling a refusal, so an unreachable store became an unhandled rejection and the stage painted its empty state. Fold the two effects into one keyed on the boolean, route the read through settledCall (ThinkFlowLab#1522), and put the refusal on the section's alert instead of "No workflows yet."; the poll keeps retrying and a successful read clears it. Fixes ThinkFlowLab#1564 Docs-not-needed: plugins/workflow/client.js changes how the stage polls and reports a refused read; docs/subsystems/workflow.md already says the stage "refreshes state while runs are active" and that a failing stage control ends on the .wf-err alert, and no contract or data shape changes. Rebased onto main a221306 by Dev-4, content unchanged: only plugins/workflow/test/client.test.js conflicted (live main and this commit both append tests at end of file); main's block stays first and this commit's two tests follow it, with no line dropped or rewritten on either side.
HugoXK
added a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 28, 2026
ThinkFlowLab#1563) Fixes ThinkFlowLab#1559 Owner: Dev3 Reviewer: Reviewer Written-by: Dev-3 Base: `176d33d034e741b68041c0676da1707001cb6e88` (`main`, ThinkFlowLab#1522) Tip: `3f0f3566978b1a02244ce55438f51dd93d274f6b` Tree: `8f4a9a7bc63b13099cc2ca44f767c6ec3845d546` ## What changed **Before:** every trigger was matched with `lower.includes(word)`, so a short ASCII trigger fired on any longer word that contains it. `notice` — shipped by the Delivery template (`template.js:61`, `triggers: ['notice', '交付']`) — routed `noticeable bug in logging`, and `pr` routed `Improve`, `Prepare` and `Sprint` (`service.js:734` accepts a two-character trigger). A wrong hit does not heal: `ingest` starts the run and `rememberClaim` pins the card to that workflow, so a later ingest returns the same claim. **After:** `plugins/workflow/src/match.js` asks one new helper, `containsTrigger(lower, word)`: - a trigger made only of ASCII characters is matched as a whole word — `(?<![a-z0-9_])…(?![a-z0-9_])` over the lowercased haystack — so `notice` does not match `noticeable`, `notices` or `noticed`, and `pr` does not match `Improve`, `Prepare` or `Sprint`, while `notice`, `PR: bump deps` and `open a PR to fix it` still match; - a trigger carrying any non-ASCII character keeps `lower.includes(word)`, because CJK has no word boundary: `周报` still matches `写周报`, and the real card `缩短交付组群 notice` still routes to Delivery (`notice` is the longer trigger); - the `< 2` character guard and the longest-trigger-wins tie-break are untouched, so `pr`, `UI` and `CI` stay legal triggers. Docs-only in the same PR: `plugins/workflow/src/match.js:9`, `docs/subsystems/workflow.md:54` and `plugins/workflow/README.md:18` no longer say "contained in"; they say what the code does, including that `tags` is part of the haystack. `match.js` already folds `tags` in (`haystack()`); the runtime is unchanged, only the statements were wrong. Not touched: the queue, the claim ledger, the persisted state format, `service.js`, `template.js`, `core/` or `patches/`. ## Verification at the tip Tip SHA: `3f0f3566978b1a02244ce55438f51dd93d274f6b` (tree `8f4a9a7bc63b13099cc2ca44f767c6ec3845d546`, base `176d33d034e741b68041c0676da1707001cb6e88`) Red first, at base `176d33d0`, with only the new test added to `plugins/workflow/test/assign.test.js`: - base alone: `plugins/workflow` 113/113 pass. - base + the new test: `assign.test.js` 14 tests, **13 pass / 1 fail**. The only failure is `an ASCII trigger matches a whole word, and a non-ASCII trigger still matches inside one`; the first failed assertion is `noticeable bug in logging` expected `null`, actual `{ kind: 'workflow', id: 'delivery' }`. - at the tip: that test passes, `assign.test.js` 14/14, and the whole `plugins/workflow` suite is **114/114**. The test covers both counterexamples (`notice` vs `noticeable bug in logging`, `pr` vs `Improve onboarding docs`) plus `Prepare release notes` and `Sprint planning`, pins the positives that must not regress (`notice`, `notice: deploy paused`, `缩短交付组群 notice`, `写周报`, `周五周报`, bare `pr`, `open a PR to fix it`, `PR: bump deps`), and asserts the `< 2` guard still refuses a one-character trigger. `node .github/scripts/changelog.mjs check` → `changelog: 58 fragment(s) in changelog.d/ are well formed`. `TZ=UTC ARCH_DOCS_BASE=176d33d0 npm run verify` → exit 0: core 1868, social 527, tree 125, timeline 125, workflow 114, `.github/scripts` 548, interop 37, live-e2e 24 (3374 tests, 0 fail), plus oxlint (0 errors; core 63, social 36, timeline 9, tree 2 and workflow 8 existing warnings), `arch-docs: OK (5 changed file(s))`, plugin lint, `build-client --check` and `ds-bot-patches check`. Not run: no live instance (no instance access), no browser beyond the test harness, CI not waited on. ## Files ``` 5 files changed, 53 insertions(+), 5 deletions(-) changelog.d/1559-workflow-trigger-word-boundary.fixed.md | 1 + docs/subsystems/workflow.md | 2 +- plugins/workflow/README.md | 2 +- plugins/workflow/src/match.js | 19 ++++++++++++++++-- plugins/workflow/test/assign.test.js | 34 ++++++++++++++++++++++++++++++ ``` ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 1563, "claim": "Dev3", "issue": 1559, "summary": "Auto-assign matched every trigger as a plain substring, so `notice` routed `noticeable bug in logging` and `pr` routed `Improve`/`Prepare`/`Sprint`, and a wrong hit stuck because the claim is remembered. An ASCII trigger now matches only as a whole word, while a non-ASCII trigger keeps substring matching (`周报` matches `写周报`, `缩短交付组群 notice` still routes to Delivery). The `< 2` guard and longest-trigger-wins tie-break are unchanged. The three \"contained in\" statements now describe the code, including `tags` in the haystack; the queue, claim ledger and state are untouched.", "files": [ "plugins/workflow/src/match.js", "plugins/workflow/test/assign.test.js", "docs/subsystems/workflow.md", "plugins/workflow/README.md", "changelog.d/1559-workflow-trigger-word-boundary.fixed.md" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=176d33d0 npm run verify at tree 8f4a9a7; red-first on base 176d33d (assign 14 tests, 13 pass, 1 fail); changelog check", "result": "green", "at": "3f0f3566978b1a02244ce55438f51dd93d274f6b" }, "next": "none — ready for review" } ``` <!-- /flybot:checkpoint -->
HugoXK
added a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 28, 2026
…hinkFlowLab#1565) Fixes ThinkFlowLab#1564 Owner: Dev-4 Reviewer: Reviewer Written-by: Dev-4 Base: `a2213067fb3cc2f746e5f9256b09a21adec15738` (`main`, ThinkFlowLab#1567) Tip: `eba939f9892cb471f017ecb28b1bcea7aa53d972` Tree: `fcf2f9885a5a906bfe17aa87765f8d00a6e4405c` ## What changed `plugins/workflow/client.js`, the sidebar section that also owns the stage. Two defects in the same place: 1. **Two polls for one condition.** Two `useEffect`s tested "some workflow has an active run" — one on `[pack]`, one on `pack?.workflows?.some(...)` — and each armed its own 2 s `setInterval`, so every tick read the whole `workflow/state` twice (the issue's probe: `[0, 2001, 2001, 4003]`). They are now one effect keyed on the boolean, so the timer is armed once while a run is live and no longer re-armed on every poll response. 2. **A refused read was painted as an empty stage.** `refresh()` awaited the host call directly and neither caller handled a rejection, so an unreachable store became an unhandled rejection and `pack` stayed `null`, which rendered `No workflows yet.`. The read now goes through `settledCall` (the `ThinkFlowLab#1522` helper the stage controls already use — no second helper shape), a refusal lands on the section's `.wf-err` alert (`role="alert"`), the empty state is suppressed while a read has failed, and the same single poll keeps retrying every 2 s until a read succeeds and clears the alert. No server change, no new endpoint. `WorkflowSettings` (`ThinkFlowLab#1504`), the Runs row metadata (P-97) and `core/` are untouched. ## Rebase This tip replays that same change onto live main `a2213067`; the earlier Verdict PASS was written at `88b4b689` (base `176d33d0`) and does not carry to this tip. Only `plugins/workflow/test/client.test.js` conflicted, because live main and this commit both append tests at the end of the file. Main's block stays first, this commit's two tests follow it, and the merge restores the blank line that separates them, so the appended lines are byte-identical to the original 67. `plugins/workflow/client.js` (13+/19−) and `changelog.d/1564-stage-poll.fixed.md` applied with no hand edit. Self-proof, all machine comparisons: - `git diff --numstat 176d33d 88b4b68` = `1/0`, `13/19`, `67/0`; `git diff --numstat a221306 eba939f` = the same three rows, 3 files `+81 −19`. - The `+` payload and the `−` payload of every file are byte-identical between the two diffs (`grep '^+'`/`'^-'` then `cmp`): client.js 13 added and 19 removed, test file 67 added and 0 removed, changelog 1 added. - The new `client.test.js` is byte-for-byte live main's file plus the same 2816 appended bytes; the changelog fragment is byte-identical to the one at `88b4b689`. ## Tests Red first in `plugins/workflow/test/client.test.js`, measured on this tree with `client.js` reverted to live main `a2213067` and the tests kept: | case | base `a2213067` (tests kept, source reverted) | this tip | | --- | --- | --- | | one live run arms one poll and one tick is one read | fail — `exactly one poll is armed for one live run, saw 2` (`2 !== 1`), and a tick read the store twice | pass | | a failed read is reported and retried, not shown as no workflows | fail — `failureType: 'unhandledRejection'`, `The workflow store is unreachable`, stack `refresh` ← `render`; no alert, `No workflows yet.` on screen | pass | The timing case stubs `setInterval`/`clearInterval` and counts real `workflow/state` calls, so it does not sleep through a tick. The failure case drives one refused read and one successful retry through the same stub and asserts the alert clears and the workflows appear. - `plugins/workflow` at this tip: **144/144**; `test/client.test.js` **47/47**. - Red check on the same tree: `client.js` back at `a2213067`, tests kept → **142 pass / 2 fail** of 144, exactly the two new cases. - `node .github/scripts/changelog.mjs check`: 66 fragments well formed. - `TZ=UTC ARCH_DOCS_BASE=a2213067fb3cc2f746e5f9256b09a21adec15738 npm run verify` → exit 0: core 1870, social 527, tree 125, timeline 125, workflow 144, `.github/scripts` 548, interop 37, live-e2e 24, all 0 fail; lint 0 errors (warnings 63/36/9/2/15/1/0, none in `client.js`); `arch-docs: OK (3 changed file(s))`; `build-client --check` silent; `ds-bot-patches check` = 10 patches apply cleanly in order + `core/` equals `ds-bot/` + patches. - Not run: the CI `package` job (pack, typecheck, actionlint) and the smoke test — neither is part of `verify`, and this PR changes neither `core/index.js` nor `core/src/host/`. T2 (`plugins/**`), so this needs a Verdict naming this tip before it is enqueued. Docs-not-needed: `plugins/workflow/client.js` changes how the stage polls and how it reports a refused read; `docs/subsystems/workflow.md` already says the stage "refreshes state while runs are active" and that a failing stage control ends on the section's `.wf-err` alert, and no contract, endpoint or data shape changes. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 1565, "claim": "Dev-4", "issue": 1564, "summary": "Two effects armed a 2 s poll for the same 'a run is live' condition, so every tick read workflow/state twice; they are now one effect keyed on that boolean. A refused read went through a bare await, escaped as an unhandled rejection and painted the empty stage; it now settles through settledCall, shows on the .wf-err alert, suppresses 'No workflows yet.', and the same poll retries until a read succeeds. Rebased onto main a221306: only the client.test.js tail merged, both sides kept.", "files": [ "plugins/workflow/client.js", "plugins/workflow/test/client.test.js", "changelog.d/1564-stage-poll.fixed.md" ], "checks": { "ran": "plugins/workflow 144/144 and client.test.js 47/47 at this tip; red first with client.js at a221306: 142 pass/2 fail; verify exit 0 (core 1870, social 527, tree 125, timeline 125, scripts 548, interop 37, live-e2e 24, all 0 fail); changelog 66; lint 0 errors", "result": "green", "at": "eba939f9892cb471f017ecb28b1bcea7aa53d972" }, "next": "review: a Verdict at eba939f (T2; the PASS at 88b4b68 does not carry), then enqueue" } ``` <!-- /flybot:checkpoint --> Co-authored-by: Dev-4 <dev4@local>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #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 commitd89d10ee)Tree: a1b46c24e29d70062a9b7eaeaa1b12435a271744
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.jsat92a6aabd0ehas 10.then(and 0.catch(, and the clientcall()(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, anunhandledRejectionin the console, and the user believing the run had been reset or stopped when it had not. Thevalue?.ok === falsebranch inside eachthencould not see any of that.Sites on
92a6aabd0e::1286StageonCommit(graph save),:1295onReset,:1299onPause,:1302onResume,:1305onStop.What changed
One helper,
settledCall(call, name, body, fallback, timeoutMs), wraps each of the five calls: a synchronous throw, a rejection, anull/undefinedanswer, 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 fromreadWorkflows()+workflowCallError()incore/src/client/workflows.js(not refactored out of it).Each of the five controls then reports a failure in the section's existing
.wf-erralert (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:onCommit)workflow/graphCould not save this workflowworkflow/resetCould not reset this workflowworkflow/pauseCould not pause this runworkflow/resumeCould not resume this runworkflow/reset(withrunId, as today)Could not stop this runFive new UI strings have
ZHentries. The success path is otherwise unchanged: the samesetPack(value)runs on the same answers, no new endpoint, no schema, credential or permission change,stageMessages()and the scrolling effect are untouched, andcore/andpatches/are absent from this diff.Scope note: none of
:1095(the ingest effect'srefresh()),:1315(CreateModal onTemplate) and the sixawait 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 existingmount()and stub style, no new dependency. Five new tests:a failed stage graph save is reported, not swalloweda failed stage reset is reported instead of leaving the button deada failed pause or stop from the run strip is reporteda failed resume from the run strip is reporteda stage control that never answers settles as an errorRed first, on base
92a6aabd0ewith 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 asfailureType: unhandledRejectionwith'The workflow store refused the write','The reset was refused','The pause was refused','The resume was refused'; the timeout case fails withTypeError: 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
The doc sentence is the arch-docs gate:
plugins/workflow/is owned bydocs/subsystems/workflow.md, so the stage's failure behavior is written on that page'sclient.jsrow rather than waived.Verification at the tip
All numbers were measured on this pull request's tree
a1b46c24…(pushed head97f87053, local commitd89d10ee— one tree):TZ=UTC ARCH_DOCS_BASE=92a6aabd0e npm run verify— exit 0: core 1853/1853, social 527, tree 125, timeline 125, workflow 95,.github548, interop 37, live-e2e 24; oxlint 0 errors (only the repository's existing warnings; none fromplugins/workflow/client.jsor the new tests).arch-docs: OK (4 changed file(s))againstARCH_DOCS_BASE=92a6aabd0e(= livemainat 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 #1505
Zero overlap. #1505 changes the settings page's three calls (
workflow/state,workflow/graphon the settings canvas,workflow/set-model, lines:829/:849/:894); this changes the five stage controls. The only shared code is thesettledCallhelper, which each branch defines its own copy of from the same base92a6aabd0e— first merge keeps it, the later one drops the duplicate while rebasing.Checkpoint
{ "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 #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 a1b46c24 (exit 0: core 1853, workflow 95), red-first on base 92a6aabd0e (34 tests, 29 pass, 5 fail), changelog check", "result": "green", "at": "97f8705310fbbff4c170766a1b5187ae63c20f75" }, "next": "none — ready for review" }