Repository navigation
fix(workflow): fit the Settings graph to its canvas, without a nested scroll - #1536
Merged
Birfy merged 1 commit intoOct 28, 2026
Conversation
… scroll Settings → Workflow graphs drew the same 960px design plane as the room drawer, inside its own overflow:auto box nested in the settings page's scroll, so the canvas carried a second vertical and horizontal scrollbar and its edges — which paint outside the zero-width plane — ran off and were cut at the border. GraphCanvas now measures its canvas, scales the design plane to that width through one .wf-canvas-fit layer and reports the scaled height, so the settings page keeps the only scroll axis and every node and edge stays inside the canvas. Pointer deltas are divided by the scale, so a drag still follows the pointer, and the canvas draws its wires with a stronger stroke than a plain divider so an edge can be followed at that size. The canvas stays the editing surface: same graph, same drag and wire handlers.
HugoXK
force-pushed
the
dev/1483-fix-workflow-settings-graph-canvas-nests
branch
from
October 28, 2026 05:15
cfe60a5 to
2927239
Compare
Collaborator
|
Verdict: PASS at 2927239 Ran at this tip (2026-10-28T05:36Z): 审查要点:
|
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 -->
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 #1483
Owner: Chief
Reviewer: Reviewer
Written-by: Dev-3
Base:
620235991e46c24836d03b16b76d642c9f7a4aa1(main, #1539)Tip:
2927239487a27e2edeb6be752768d815c36e6efcTree:
28b7ca6670381e79415add9fee1d6e095e095775What was wrong
Two scroll containers, one inside the other. The settings dialog already scrolls its page (
.bt-settings-scroll{flex:1;overflow-y:auto}), and inside it the Settings canvas was its own.wf-canvas-view{overflow:auto}box around a 960px-wide graph — so the canvas carried a second vertical and a horizontal scrollbar in that page, and the wheel scrolled the inner one first.The edges looked faint and stopped at the border for the same reason: the wires are drawn by a layer that paints outside the zero-width
.wf-plane(.wf-wires{left:-480px;width:960px;overflow:visible}) and inheritedstroke: var(--bt-line, …), which is the ~8% divider ink. With the canvas clipping at ~700px, every edge past the border was cut and left a stub running into a grey gutter instead of reaching its node.#1490merged with this issue in itsFixeslist, and this surface still reproduced after it (comment 6457940325): that change did not move the canvas container or the plane's width, so this pull request changes those instead.The fix
The Settings canvas fits the graph to its own width — one scroll axis, the page's — rather than keeping the 960px plane and asking for a horizontal scrollbar. The graph is authored on a fixed 960px design space shared with the run, so scaling that space is the only way to keep every node's stored
x/yand every edge endpoint unchanged while the dialog gets narrower.GraphCanvasmeasures its own canvas (aResizeObserver, read once on mount) and draws the plane through one.wf-canvas-fitlayer withtransform: scale(min(1, width/960)), reporting the scaled height to the plane. The canvas keeps its box but no longer scrolls, so the settings page keeps the only scroll axis. Its wires get a stronger stroke for this canvas only (--bt-ink-2, 2.5px), so an edge can still be followed at 0.67×.This is deliberately the same idea as the drawer fix in #1533, reached differently, because the two containers are not the same problem: the drawer has a fixed 400px width and can take it as a prop (
FIT_WIDTH), while the Settings canvas is as wide as the dialog is — so it measures itself. The two pull requests share no line ofclient.jsand no CSS rule; whichever lands second needs no rebase.The canvas stays the editing surface: same graph, same drag and wire handlers, same inspector. Only the coordinate arithmetic moves with the scale — a pointer delta in screen pixels is divided by it, so a node dragged under a fitted plane follows the pointer instead of running away from it.
What changed
Four files, +101/−11.
plugins/workflow/client.js—GraphCanvasgets astageRefand a measuredpanelWidth, computesscaleanddrawnHeight, wraps the wires and nodes in.wf-canvas-fit, and divides themovegesture by the scale. CSS:.wf-canvas-viewstops scrolling (overflow:visible),.wf-canvas-fitadds the scaling layer, and.wf-canvas-view .wf-wires pathraises the edge stroke for this canvas only. Scope:GraphCanvasand those three rules —Flow(the drawer, fix(workflow): room side-panel Flow tab overflows its panel; orphan/clipped edges and nested scroll #1513), the Settings header, inspector and edge form, and every other block are untouched;core/andsrc/host/are not touched at all.plugins/workflow/test/client.test.js— the hook renderer now gives a host element'srefa stand-in whosegetBoundingClientRect()answers from a test-set width, because there is no DOM layout to measure. One new test pins the canvas width, the fit transform, the scaled height, that every node and wire endpoint is inside the canvas, and that a drag is divided by the scale. The existing CSS test now asserts the new contract.docs/subsystems/workflow.md— invariant 2 says the Settings canvas paints the same plane scaled to its width, is not a second scroll container, and follows that scale when dragged.changelog.d/1483.fixed.md— the existing fragment rewritten to describe this change.Red, then green
Rebased onto
620235991e46c24836d03b16b76d642c9f7a4aa1(main, #1539); the numbers below were re-measured there.plugins/workflow,node --test "test/*.test.js":620235991e46c24836d03b16b76d642c9f7a4aa1: 105 tests / 105 pass / 0 fail.620235991e46c24836d03b16b76d642c9f7a4aa1plus the new test file only: 106 / 104 / 2. The two red arethe settings canvas fits the graph to its own width, and a drag follows that scale— its first assertion isthe canvas draws the graph at the width it has, not on a 960px plane(NaN !== 640: the plane reported no width at all) — and the rewrittenthe room flowchart canvas scrolls in its panel, and the settings canvas leaves one scroll to the page(the.wf-canvas-view{overflow:visible}rule did not match).2927239487: 106 / 106 / 0 (the client file alone 31 / 31 / 0).The rebase conflicted only in
docs/subsystems/workflow.md:mainhad meanwhile rewritten invariant 3 (#1537/#1538 early termination) while this branch had extended invariant 2. Both sides are kept —main's invariant 3 verbatim, this branch's #1483 sentence on invariant 2.client.jsand its test auto-merged; no other file was touched by both sides.The full
plugins/workflowtotal at the tip is 106; unchanged suites: core 1866, social 527, tree 125, timeline 125,.github548, interop 37, live-e2e 24 — all 0 fail.What I ran
TZ=UTC ARCH_DOCS_BASE=62023599 npm run verifyat the tip → exit 0: 3358 tests, 0 fail; lint 0 errors (the 7 workflow warnings are pre-existingunicornspread/length hints insrc/, present on main);arch-docs: OK (4 changed file(s));node .github/scripts/changelog.mjs check→ 56 fragments well formed; theenqueue-rulescheckpoint and guard checks return[].Not run: no browser screenshot and no smoke run — the unit renderer has no layout, so the test pins the measured width, the scale, the scaled height and the pointer arithmetic, not pixels;
index.jsandsrc/host/are untouched, so the smoke bench is not called for.plugins (workflow)and Node 24 run on CI.Checkpoint
{ "pr": 1536, "claim": "Chief", "issue": 1483, "summary": "Settings → Workflow graphs drew a 960px canvas in an overflow:auto box inside the page's own scroll, so it grew a second vertical and horizontal scrollbar and edges painted outside the zero-width plane were cut at the border. GraphCanvas now measures its canvas, scales the design plane to that width through one .wf-canvas-fit layer and reports the scaled height, leaving the page the only scroll axis with every node and edge inside. A drag divides its delta by that scale and the wires get a stronger stroke; the canvas stays the editing surface.", "files": [ "plugins/workflow/client.js", "plugins/workflow/test/client.test.js" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=62023599 npm run verify; the plugins/workflow suite at 620235991e and at 620235991e plus the new tests only", "result": "green", "at": "2927239487a27e2edeb6be752768d815c36e6efc" }, "next": "none — ready for review. The drawer's own fit is #1513, a separate pull request that touches only Flow and .wf-fit." }