Repository navigation
fix(workflow): fit the room side-panel Flow chart to its panel - #1547
Merged
Merged
Conversation
Birfy
force-pushed
the
dev/1513-drawer-flow-fit
branch
2 times, most recently
from
October 28, 2026 04:16
cb5dec8 to
5111163
Compare
github-actions
Bot
added
the
risk:T2
Collaborator
Author
|
Verdict: REQUEST_CHANGES at 5111163 Ran at this tip (2026-10-28T06:50Z): 审查要点：
整改清单：
Not run: |
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
Birfy
force-pushed
the
dev/1513-drawer-flow-fit
branch
from
October 28, 2026 06:58
5111163 to
74de9c6
Compare
Collaborator
Author
|
Verdict: PASS at 74de9c6 Ran at this tip (2026-10-28T07:26 UTC): 审查要点：
|
Birfy
added a commit
that referenced
this pull request
Oct 28, 2026
… handles (#1514) Rebuilt on main ad976b4 by Rework. main changed plugins/workflow/client.js and plugins/workflow/test/client.test.js again (#1540, #1547) after the first rebuild on 176d33d, so both files are merged from main ad976b4 and 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.
HugoXK
added a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 28, 2026
…igest skips the health module's count compensation (ThinkFlowLab#1579) Fixes ThinkFlowLab#1578 Owner: Dev-1 Reviewer: Reviewer Written-by: Dev-1 Risk: T2 — `plugins/timeline/src/digest.js` is plugin code (`.github/agent/risk-rules.json`), so this needs a PASS at the tip plus green CI. No T3 path, no dependency change. Tip SHA: `8139276924c95ec4f30691244c8e4c3d2030ebe0` Tree: `bd8db4c70d8271142d4798b42b8405d73ff9953f` Base: `c1b9fd23e404dcff1eb782a2269e30e09e2d6993` — live `main` (`ThinkFlowLab#1547`, `ThinkFlowLab#1572`, `ThinkFlowLab#1556`, `ThinkFlowLab#1552`, `ThinkFlowLab#1573`). ## What was wrong The report compared two different populations. `plugins/timeline/src/window.js:249` counts `turns` as the turns that **began** in the window; `:250` counts `errors` as every **overlapping** turn whose outcome is `error`. A failed turn that began before the window is therefore in `errors` and not in `turns`, and `digest.js` divided one by the other: `const errorRate = turns > 0 ? errors / turns : 0`, printed in three places. That is exactly the reading `docs/subsystems/timeline.md` invariant 5 says can never happen — the error rate "divides by the larger of the turns that began in the window and the failed turns, so it never reads a failure count above its turn count" — and `plugins/timeline/src/health.js:27-32` already compensates with `Math.max(use.turns, use.errors)` and a comment naming the failure text it avoids. The digest did not. Reproduced at `main`, real `buildWindow` + `buildDigest`, 15-minute window, one Bot `b1`: three turns that begin 3/4/5 minutes before the window and fail 20/21/22 s into it, plus one turn that begins inside it and completes. The window holds 4 turns (`outcomes {completed: 1, error: 3}`, `utilization.turns` 1, `errors` 3), and the report read: - per Bot: `- 1 turns, 3 failed (300%)` - Team: `- 1 turns, 3 failed` - one line: `1 turns, 3 failed, 0 tokens in over 07:45–08:00.` The mild shape is worse than a bad percentage: one overlapping failed turn plus one completed turn that began inside the window reads `1 turns, 1 failed (100%)`, i.e. "the window's only turn failed", when that turn is the completed one. `1 turns` was also printed everywhere. ## What changed `plugins/timeline/src/digest.js` only — three lines of arithmetic and the three printers: - `counted = Math.max(turns, errors)`, and `errorRate = errors / counted`, the same denominator `health.js` uses. `counted` is now on each `bots[]` entry and on `team`, so a consumer can read the number the share was taken over; `turns` and `errors` keep their raw window values. - The per-Bot line, the `## Team` line and the one-line summary all print `counted`, so the number compared with the failure count is the number printed, and the Team rollup is the sum of the Bot sections, as before. - The failure flag reads `${errors} of ${counted} turns failed`, the wording `health.js` already uses, and `no turns` now needs `counted === 0` rather than `turns === 0` (a Bot whose failed turns all began before the window is not "no turns" and is not reported as one). - Singular/plural follows the number in all three places (`1 turn`, `3 turns`). - A share is printed only when it can be a share, i.e. below 100%. With every counted turn a failed one, the numbers cannot say whether that turn began in the window or before it; `(100%)` beside a window whose single completed turn began inside it reads as its opposite. At that point the report prints the counts and no percentage (`3 turns, 3 failed`), which is still consistent and never over 100%. Scope and what I did not touch: - `window.js`'s two counters are unchanged — they are documented and the panel reads them. - `health.js` is unchanged; the digest now matches it instead of diverging. - `plugins/timeline/client.js:588` (the panel's Utilization line, same pair of numbers) is **not** changed. It prints two raw counts and no rate, so it cannot overclaim a share; changing the count it prints would make the panel disagree with the documented meaning of `utilization.turns` (invariant 5) that the rest of the panel and `health.js` rely on. If the owner wants the panel to print the comparison denominator too, it is a separate, one-line change with its own test. - `changelog.d/1578-digest-error-rate.fixed.md` added. Docs unchanged: invariant 5 already states the rule this change implements. `Docs-not-needed: docs/subsystems/timeline.md invariant 5 (line 52) already states this rule — the error rate divides by the larger of the turns that began in the window and the failed turns, so it never reads above its turn count. This change makes digest.js obey that sentence; it adds and changes no documented behaviour, and window.js, health.js and client.js are untouched.` Same line in the branch commit (`git log -1 --format=%B`), so the trailer survives the squash into the commit `main` judges, and it is what turns `arch-docs` green at the tip: `arch-docs: FAIL` was the only reason the first tip's `tests-ok` was red. ## Red → green At the base with only `test/digest.test.js` added (source at `main`): **13 tests, 10 pass, 3 fail** — - `ThinkFlowLab#1578: three failed turns that began before the window never read as 300% of one turn` - `ThinkFlowLab#1578: a failed turn that began before the window is not read as the window's own turn failing` - `ThinkFlowLab#1578: a single turn reads as one turn in all three places` The fourth new test, `ThinkFlowLab#1578: a clean window is byte-identical to the report before the change`, passes on both sides by design: it is the pinned output (`turns: 3, errors: 0`) that proves the denominator does not move when `errors <= turns`. At the tip: `test/digest.test.js` **13/13**, `plugins/timeline` **129/129** (base 125 + these 4). ## Verification at the tip Tip `8139276924c95ec4f30691244c8e4c3d2030ebe0`, tree `bd8db4c70d8271142d4798b42b8405d73ff9953f`, base `c1b9fd23e404dcff1eb782a2269e30e09e2d6993`, 3 files `+86/−10` (`digest.js` 20/10, `test/digest.test.js` 65/0, changelog 1/0). Ran: - `npm --prefix plugins/timeline test` → 129/129, 0 fail. - `TZ=UTC ARCH_DOCS_BASE=c1b9fd23e4 npm run verify` → exit 0: core 1869, social 527, tree 125, timeline 129, workflow 142, `.github/scripts` 548, interop 37, live-e2e 24 — 3401 tests, 0 fail; oxlint 0 errors (core 63 / social 36 / timeline 9 / tree 2 / workflow 13 warnings); `arch-docs: OK (3 changed file(s))` — the two plugin files are covered by the `Docs-not-needed:` trailer below; `build-client --check` and `ds-bot-patches check` clean. - `node .github/scripts/changelog.mjs check` → 63 fragments well formed. - `cd core && ARCH_DOCS_BASE=c1b9fd23e4 node scripts/check-arch-docs.mjs` → `arch-docs: OK (3 changed file(s))` at the tip with the `Docs-not-needed:` trailer in both places (first tip: `FAIL` on the same two files, `tests-ok` red for that reason alone). Not run: the smoke test (no `core/index.js` or `src/host/` change), the `package` job, Desktop, and the owner's live instance. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 1579, "claim": "Dev-1", "issue": 1578, "summary": "The team report divided overlapping failed turns by the turns that began in the window, so a failure from before the window printed as 1 turns, 3 failed (300%). The digest now counts against max(turns, errors), the denominator health.js already uses, in the per-Bot line, the Team line and the one-line summary, with the team total the sum of the Bot sections. It prints a share only below 100%, since with every counted turn a failed one the numbers cannot say whose turn failed, and singular/plural follows the count. window.js, health.js and client.js are untouched.", "files": ["plugins/timeline/src/digest.js", "plugins/timeline/test/digest.test.js", "changelog.d/1578-digest-error-rate.fixed.md"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=c1b9fd23 npm run verify", "result": "green", "at": "8139276924c95ec4f30691244c8e4c3d2030ebe0" }, "next": "none — ready for review at the tip" } ``` <!-- /flybot:checkpoint -->
HugoXK
added a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 28, 2026
…d fence or its delegation trace (ThinkFlowLab#1409) fix(client): the voice reads a reply aloud through the row reader, not a chain of its own Fixes ThinkFlowLab#1405 Owner: Dev3 Reviewer: Reviewer Written-by: Dev-3 Tip SHA: `158b7707e16c53ade290f9b81eb3abef810fde43` Base: `9b866a5c797e8efe95e445885b69da4b8ca5c35b` (the branch is cut there; `main` has since moved to `c1b9fd23` — `ThinkFlowLab#1552` and `ThinkFlowLab#1573` — and this branch merges into it with no conflict, checked with a real local merge) Tree: `fce7525ea4161f5eb6e8f3e1b2add15ac61c6d1e` The previous tips `150fdbd9` (base `7a4fa61e`) and `fce1d6ca` (base `4d92f4f7`) are superseded: both had gone `dirty`, and no part of their result is reused beyond this change. The change was replayed onto live `main` again after `ThinkFlowLab#1547`/`ThinkFlowLab#1552`/`ThinkFlowLab#1556` landed; the payload is unchanged (`--numstat` per file identical to `fce1d6ca`), and `patches/ds-bot/0008-client-bundle.patch` was rebuilt by hand as the smallest delta instead of by `generate`, which reorders the whole bundle diff (#P-59): it is `+13/−4` against main, not the `+708/−386` that `generate` writes on a clean main. ## What changed `core/src/client/voice.js` built the text it speaks itself: strip the header, then delete every character in `[*_`#>|~-]`. `<` and `"` are not in that set and `>` is, so a stored remote line went out as `<untrusted message from="User" B踢后测4` — the fence markers minus one bracket, plus the sealed text. The same string was the whole turn, so a delegation trace was read aloud verbatim; the outline stores a reply flattened onto one line (`Chief → Product B踢后测4`, the shape `core/test/inbound-ui.test.js:47` pins), and the voice said the routing and the words as one sentence. `core/src/client/text.js` now exports `speechOf(text, names)` beside `readableLine`. It reads the line the way a row reads it — `readableLine` drops the header, the untrusted fence and a delegation trace whose every hop is a known Bot name (ThinkFlowLab#1395, ThinkFlowLab#1408) — and then removes the Markdown a speech synthesiser would otherwise speak: a code fence, a link's target, emphasis marks, collapsed whitespace. `voice.js` calls `speechOf(text, botNames(roster))`, so the voice and the row preview read one chain, and an ordinary arrow line such as `Settings → Models 页面` keeps its words instead of being read as `页面`. Two shapes now stay out of the ears, and the row preview agreed with them before this change: `Chief → Product\nB踢后测4` and the flattened `Chief → Product B踢后测4` are both spoken as `B踢后测4`; a line that is only a trace is spoken as nothing. Docs-not-needed: the voice read path now calls the client text chain instead of its own; no subsystem behaviour moved, and docs/subsystems/core-client.md still describes the text helpers and voice mode as they are. `.github/**` is untouched. ## Verification at this tip Tip SHA: `158b7707e16c53ade290f9b81eb3abef810fde43` Ran: `TZ=UTC ARCH_DOCS_BASE=9b866a5c npm run verify` at this tip; `node scripts/ds-bot-patches.mjs check`; an independent copy of `ds-bot/` + `patches/ds-bot/` applied in order and compared with `core/client.js`; the new test at this tip and with `text.js`/`voice.js` grafted back to `main`'s sources; `node core/scripts/build-client.mjs --check` inside `verify`. Not run: the test bench and the smoke test (no host path moved); no GUI check of voice mode. ### The three gates 1. **`verify`** — exit 0: core **1876/1876**, social 527, tree 125, timeline 125, workflow **136/136**, `.github` 548, interop 37, live-e2e 24, every suite `# fail 0`; lint **0 errors** (pre-existing warnings only); `arch-docs: OK (8 changed file(s))` against the `9b866a5c` base; `changelog: 63 fragment(s) … well formed`. 2. **`node scripts/ds-bot-patches.mjs check`** — three `ok` lines: `ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)`, `10 patches apply cleanly in order`, `core/ equals ds-bot/ + patches/ds-bot/`. 3. **The independent copy** — `ds-bot/` copied out, `patches/ds-bot/files/` laid over it, the ten patches applied in order with `git apply -p1`, then compared with `core/client.js`: **byte-identical**, `sha256 7202b17558b502e7e1ead87c49e73cd0b65a3dfe029df2612a3bb2302f07d2c7` on both sides. `node core/scripts/build-client.mjs --check` is silent at this tip. ### Red, then green ``` cd core && node --import ./test/harness-resolve.mjs --test test/voice-speech.test.js ``` Re-run on the new base with `text.js` and `voice.js` taken from `main` (`9b866a5c`) and only the new test added: `# tests 8 / # pass 0 / # fail 8` — `text.js` exports no `speechOf`, and `voice.js` keeps its own chain. At this tip: `# tests 8 / # pass 8 / # fail 0`. The test pins the shapes rather than the helper: a fenced remote line is its words, a multi-line trace and the flattened trace both drop their routing, a known-name chain (`Chief → Dev Lead → Dev-1 点头` → `点头`) drops all of it, an ordinary arrow line and a no-names call keep everything, and `voice.js` is asserted to hand the read side its names. ### Files — 8, +233 / −10 | file | | | --- | --- | | `core/src/client/text.js` | +12 | | `core/src/client/voice.js` | +2/−3 | | `core/test/voice-speech.test.js` | +82 (new) | | `core/client.js` | +4/−2 (generated) | | `patches/ds-bot/0001-core-fixes.patch` | +37/−1 (generated) | | `patches/ds-bot/0008-client-bundle.patch` | +13/−4 (generated, hand-scoped) | | `patches/ds-bot/files/test/voice-speech.test.js` | +82 (new, whole-file mirror) | | `changelog.d/1405.fixed.md` | +1 (new) | ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 1409, "claim": "Dev3", "issue": 1405, "summary": "`voice.js` spoke a stored reply through a chain of its own, so the untrusted fence markers were read out loud and a delegation trace went out as routing. `text.js` now exports `speechOf(text, names)`, which reads the line through ThinkFlowLab#1395's `readableLine` — header, fence and a known-name trace — and then drops the Markdown a synthesiser would otherwise speak. `voice.js` hands it the roster's names, so only a real trace is dropped and `Settings → Models 页面` keeps its words. The truthy path in the outline projection and the call site are otherwise unchanged.", "files": [ "core/src/client/text.js", "core/src/client/voice.js", "core/test/voice-speech.test.js", "core/client.js", "patches/ds-bot/0001-core-fixes.patch", "patches/ds-bot/0008-client-bundle.patch", "patches/ds-bot/files/test/voice-speech.test.js", "changelog.d/1405.fixed.md" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=9b866a5c npm run verify; node scripts/ds-bot-patches.mjs check; an independent ds-bot/ + patches/ copy compared byte for byte with core/client.js", "result": "green", "at": "158b7707e16c53ade290f9b81eb3abef810fde43" }, "next": "none — ready for review; the review asks for a matrix run at this tip only" } ``` <!-- /flybot:checkpoint -->
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 -->
HugoXK
added a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 28, 2026
…wLab#1574) Fixes ThinkFlowLab#1569 Owner: Dev2 Reviewer: Reviewer Written-by: Dev-2 Base: ad976b4 (live `main` when the branch started; the issue body names `bf5e8780`, and `main` moved exactly one commit, `ThinkFlowLab#1547`, before this branch) Tip: 5fd8502 (fork head; tree `cdec6b8700b33f1c8e93db033fc13c8550d958af`) Tree: cdec6b8 ## What was wrong On a read-only host — the condition means another `dsh` holds this team — `workflow/advance` and `workflow/reset` changed state and wrote `workflows.json` without the guard every other write endpoint has. - `pause` (`plugins/workflow/src/service.js:986`), `resume`, `assign`, `setModel`, `mutate` and the rest return `{ ok: false, code: 'read-only', error: 'The host is read-only' }`. `advance` and `reset` did not: both reached `storeRun()`/`write()` (`:220-228`, no check), and `reset`'s `take()` cleared `lane.active` and `lane.queue` before writing. - The UI/API path is stopped by the host at `core/src/host/channel.js:284-290`, but the node-Bot reply hook does not go through the endpoint layer: `plugins/workflow/index.js:143-152` calls `service.stepResult(...)` directly, and `stepResult` calls `advance()`. So a node Bot's reply advanced a run and wrote the file on a host another `dsh` held, and `reset` could kill a live run together with its queued items. ## What changed One guard line at the top of `advance` and one at the top of `reset`, before any `storeRun`, lane change or `write`: ```js if (readOnly()) return { ok: false, code: 'read-only', error: 'The host is read-only' } ``` That is the whole product change — two lines in `plugins/workflow/src/service.js`. No new endpoint, no new parameter, no other branch touched. A refused `stepResult` is now reported once through the existing `warnOnce` path (`service.js:964-970`): the reply is not swallowed, the run and its queue do not move, and on a writable host both calls behave exactly as before (the first test asserts the writable `advance` still moves the run). ## Red → green, honestly 1. **`plugins/workflow/test/assign.test.js` → `ThinkFlowLab#1569: a read-only host refuses advance and reset without moving the run`.** Assigns a run plus one queued task at concurrency 1, flips `readOnly`, then asserts both calls return exactly `{ ok: false, code: 'read-only', error: 'The host is read-only' }` and that `activeCount`, `run.status`, `run.nodeStatus.planner` and the queue are unchanged. It then flips `readOnly` back and asserts the same `advance` moves the run to Writer — the guard is the only difference. 2. **`plugins/workflow/test/ephemeral.test.js` → `ThinkFlowLab#1569: a read-only host refuses the Bot reply that would advance a run`.** Drives the real reply path: `runNamed` spawns the two node Bots, `readOnly` flips, the plan Bot replies `stepResult({ botId: run.ephemeral.plan, ... })`. The run stays `running`, `plan` stays `running`, `doneCount` does not move, and the warning path says `read-only` once; a second reply from the same step is refused and does not warn again. 3. **Red first, with the guards reverted** (`git checkout origin/main -- plugins/workflow/src/service.js`, tests at this tip): **126 tests, 124 pass, 2 fail** — exactly the two tests above, by name. Measured twice, same result. **At this tip: 126 tests, 126 pass.** ## Files ``` 4 files changed, 68 insertions(+), 0 deletions(-) changelog.d/1569.fixed.md | 1 + plugins/workflow/src/service.js | 2 ++ plugins/workflow/test/assign.test.js | 32 ++++++++++++++++++++++++++++++++ plugins/workflow/test/ephemeral.test.js | 33 +++++++++++++++++++++++++++++++++ ``` ## Checks - `TZ=UTC npm --prefix plugins/workflow test` → **126 tests, 126 pass, 0 fail** (at this tip; 124/2 with the guards reverted). - `TZ=UTC ARCH_DOCS_BASE=ad976b40 npm run verify` → **exit 0**: core 1868/1868, social 527/527, tree 125/125, timeline 125/125, workflow 126/126, `.github` 548/548, interop 37/37, live-e2e 24/24; oxlint 0 errors (63 pre-existing warnings in core, 0 in this plugin's diff); `arch-docs: OK (4 changed file(s))`; `scripts/ds-bot-patches.mjs check` ok, including `core/ equals ds-bot/ + patches/ds-bot/` (untouched here). - `node .github/scripts/changelog.mjs check` → `60 fragment(s) in changelog.d/ are well formed`, exit 0. Docs-not-needed: the endpoint table already lists advance and reset as writes and the rule does not change. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 1574, "claim": "Dev2", "issue": 1569, "summary": "On a read-only host `workflow/advance` and `workflow/reset` changed a run and wrote `workflows.json`, while every other write endpoint refuses with `read-only`. The same one-line guard now sits at the top of both, before any `storeRun` or `write`. That also closes the node-Bot reply path, because `stepResult` calls `advance` directly and never passes the host channel's check: a refused reply is reported once through `warnOnce`, and neither the run nor its queue moves. A writable host is unchanged, and the two new tests prove each half.", "files": [ "plugins/workflow/src/service.js", "plugins/workflow/test/assign.test.js", "plugins/workflow/test/ephemeral.test.js", "changelog.d/1569.fixed.md" ], "checks": { "ran": "TZ=UTC npm --prefix plugins/workflow test (tip + guards reverted); TZ=UTC ARCH_DOCS_BASE=ad976b40 npm run verify; node .github/scripts/changelog.mjs check", "result": "green", "at": "5fd8502444992eafdef9adc04298f851de09a57c" }, "next": "review on this tip; nothing else in the file moves and no follow-up is planned" } ``` <!-- /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 #1513
Written-by: Dev
Owner: Chief
Reviewer: Reviewer
Base:
176d33d034e741b68041c0676da1707001cb6e88(mainhead at 2026-10-28T06:56Z)Tip SHA:
74de9c6c77e140bc34c4ef2f41f2125ebdc24b9aTree:
74147a95640c11f4e0f9cd7d2c167ec6d0d2e1a0Replay note (Rework, 2026-10-28T07:00Z): the reviewed tip
5111163esat on62023599, four commits behindmain, andmainhad meanwhile changed this exact file set (#1535, #1497, #1536, #1522). This commit's parent is the currentmainhead, the PR's own hunks are merged with those changes by hand, andmain...headlists exactly the four files. Changes made while replaying:plugins/workflow/client.js: the fitted drawer keeps the plainwf-canvasclass and is selected bydata-fit="drawer"; the narrow CSS now reads.wf-canvas[data-fit="drawer"] …. fix(workflow): report a failed or unanswered stage control instead of swallowing it #1522's stage-control test looks the drawer canvas up asclassName === 'wf-canvas', so it needs no edit andtests-okis green on the merge tree.docs/subsystems/workflow.md:36: one table row now carries both fix(workflow): report a failed or unanswered stage control instead of swallowing it #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" — fix(workflow): fit the Settings graph to its canvas, without a nested scroll #1536 made it scale to its own width, so the sentence names that instead.checks.ranwere 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 rawGAP_X=188columns, wires on aleft:-480px;width:960pxsvg, 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) intopanelWidth - 2*FIT_PAD - NODE_W, centres the graph horizontally, keepsscale <= 1, and returns fitted nodes plus thescale/offsetX/offsetYan edit needs to invert a pointer position.DRAWER_FLOW_W = 400 - 2*18 = 364) renders the fitted nodes, wires withwirePath(from, to, 0)in the plane's own coordinates, andparallelFrameStyleover the fitted members. The plane gets the panel width and the wires svg follows it. The Settings canvas keepswirePath's defaultWIRE_SPAN; its width scaling and the page's own scroll are fix(workflow): fit the Settings graph to its canvas, without a nested scroll #1536's, untouched here.data-fit="drawer":min-height:0;align-items:stretch;overflow:hiddenso there is no inner scroll;.wf-dr-bodykeeps the single vertical scroll and getsoverflow-x:hidden..wf-plabelwraps (white-space:normal;text-align:center;max-width:calc(100% - 16px)) and the drawer.wf-termwraps too.x + dx/scale(the scale the drag started with, so fitting does not chase the pointer) and a dropped Bot mapsoffsetX/offsetYback throughoffsetX/offsetY. Wire, reconnect and the per-node inspector are unchanged.docs/subsystems/workflow.mdnow name the fitted drawer;changelog.d/1513-workflow-panel-fit.fixed.mdis the fragment.Verification at the tip
Tip SHA:
74de9c6c77e140bc34c4ef2f41f2125ebdc24b9a(parent176d33d0)Ran on this tree (the committed blobs are byte-identical to the files these ran on —
git hash-objectmatched the tree's blob shas for all four):node --testover the eleven runnableplugins/workflow/test/*.test.js— 116 tests, 116 pass.node --test test/client.test.js(inplugins/workflow) — 40 tests, 40 pass (34 from this PR plus fix(workflow): report a failed or unanswered stage control instead of swallowing it #1522's six; the merge tree the Reviewer built failed one of those before this replay).plugins/workflow/client.jsback atmain's blob — 35 pass, 5 fail: the four new fit tests plusthe sidebar opens the flowchart, and a drag, a wire and a dropped Bot are saved(it now asserts thedelta/scalemapping).node .github/scripts/changelog.mjs check— 58 fragments well formed.node scripts/ds-bot-patches.mjs check— 10 patches apply cleanly in order;core/equalsds-bot/+patches/ds-bot/; the pin is skipped here (nogithistory 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.jsandnode --check plugins/workflow/test/client.test.js— exit 0.main176d33d0: 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.jsand 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) andnode core/scripts/build-client.mjs --check(esbuild) — nonode_moduleshere; CI runs both.Risk
Expected T2:
plugins/workflow/client.jsis plugin code norisk-rules.jsonrule names (the T3 list coversplugins/social/**andcore/src/shared/**, and an unnamed path is never silently T0/T1); no T3diffPatternshit; nopackage.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 #1536's), the graph record, the wire/reconnect semantics and the inspector are unchanged, andfitPlanenever enlarges a graph.Checkpoint
{ "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 #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 5111163e is void by the new commit)." }