Repository navigation
chore(workflow): remove the dead workflow/say surface (#1475) - #1497
Conversation
|
|
553aebd to
510ccfb
Compare
|
Written-by: Rework — Rework check at |
|
Verdict: PASS at 510ccfb Ran at this tip (2026-10-28T05:35Z) — tree unpacked from the fork head, 1261 blobs byte-identical to Not run: no smoke bench, no live DSH instance and no browser (this change touches neither host nor client); no local 审查要点:
|
The stage composer that called workflow/say went away in ThinkFlowLab#1461 (owner decision in ThinkFlowLab#1459), so nothing in the product can send one. What was left still lied when called: say() always recorded the text and, while a node ran, invented a "收到。我继续处理「…」" receipt no Bot ever said. Delete the registration (index.js), the say() method and that fabricated line (service.js), the endpoint token in docs/subsystems/workflow.md and the bare `say` in the plugin README. The read-only assign test keeps its half; the say half and its /Planner|收到/ assertion go. A new host test boots the real ctx.flybot and pins the acceptance shape: the plugin is loaded (workflow/state answers), and flybot/workflow/say is unknown to the host — `Unknown FlyBot endpoint workflow/say` with code bot/not-found, not { ok: true } and not an empty object. Fixes ThinkFlowLab#1475
… 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.
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
… 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 -->
…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 -->
chore(workflow): remove the dead workflow/say surface (#1475)
Fixes #1475
Owner: Chief
Reviewer: Reviewer
Written-by: Dev-2
Base: 6202359
Tip: 510ccfb (the fork head; its tree is
fd6f410b6f96f94691abaddb18b398aec1afb2e7, the tree every check below ran on)Tree: fd6f410b6f96f94691abaddb18b398aec1afb2e7
Base note: the reviewed tree was cut on
1b6f588c; livemainhas since moved 20 commits to620235991e46c24836d03b16b76d642c9f7a4aa1, and this branch was rebased onto it once (no further rebases). The only conflict wasdocs/subsystems/workflow.md—mainhad rewritten the same endpoint sentence (themaxConcurrentcontrol,workflow/rename,workflow/delete); it is kept in full withworkflow/saydropped from the list.plugins/workflow/README.md,index.jsandsrc/service.jsauto-merged: the diff against livemainis still exactly the four deletions plus the three test-harness files (9 files, +154/−30).What changed
workflow/saywas a host command with a handler and no caller, and its only behavior was to write a fabricated receipt into the workflow chat. The surface is gone, in four places:plugins/workflow/index.js— theworkflow/sayhandler registration.plugins/workflow/src/service.js—say()(15 lines), including the fabricated{ kind: 'bot', … 收到。我继续处理「…」 }message.docs/subsystems/workflow.md— theworkflow/saytoken; the other nine write commands are unchanged.plugins/workflow/README.md— the baresaytoken (the issue author added this line to chore(workflow): remove the dead workflow/say surface and its fake receipt #1475's In scope at 2026-10-27T15:54:59Z).Nothing else moves:
stageMessages()and its filtering,plugins/workflow/client.js, the stored workflows and the remaining endpoints stay as they were; there is no migration and historical records are untouched.Three changes outside the four deletions, needed so the acceptance assertion runs against a real host instead of a unit mock:
plugins/workflow/package.json: thetestscript gains--import ./test/resolve.mjs.plugins/workflow/test/resolve.mjs(new): module resolution copied fromplugins/tree/test/resolve.mjs— three lines adapted (the@flybot/treecomment and the error string).ds-botresolves to the repo-rootcore/(orFLYBOT_CORE), and@deepseek-ai/*from inside it to a DSH checkout (DSH_HARNESSor<core>/dev/harness).plugins/workflow/test/fake-cordis.js(new): a minimal Cordis-like context, copied verbatim fromplugins/tree/test/fake-cordis.jsexcept its first comment line.The package's
fileslist is unchanged, sotest/is not shipped;.github/scripts/check-pack.mjsandplugin-imports.test.mjspack and import exactly that list. CI already setsDSH_HARNESS(${{ github.workspace }}/core/dev/harnessin.github/actions/flybot-tests/action.yml) and still runsnpm testinplugins/workflow— the flag only changes what that script loads. Run without a core or harness locally andresolve.mjsthrows@flybot/workflow tests: no FlyBot core at … (set FLYBOT_CORE): a prerequisite of the test, not a regression of anything shipped.The acceptance shape
Deleting
say()is dead-code removal; the acceptance point is the endpoint's shape, not the method, and there is no test for the method itself.plugins/workflow/test/host.test.jsboots the real host (ds-bot's public entry in the fake Cordis context,ctx.plugin(workflow, { persist: false })) and asserts both halves:flybot/workflow/stateanswers{ ok: true, … }— the plugin is loaded, so the next answer cannot be "no plugin registered";flybot/workflow/sayanswers{ ok: false, error: { code: 'bot/not-found', message: 'Unknown FlyBot endpoint workflow/say', details: {} } }— the route stripsflybot/(core/src/host/channel.js) and the dispatcher throws that message and code (core/src/host/hooks.js). Not{ ok: true }, not{}, notundefined.workflow/sayand收到。我继续处理have no hits left inplugins/product code,core/,docs/or the client bundle. The only remaining occurrences are inside the two tests that assert the name is gone (assign.test.jsregistration list,host.test.jshost answer): a test that pins an absence has to name what is absent, so "zero hits" cannot hold for the test files themselves, and this pull request does not claim it does.No changelog fragment: unreachable surface removal.
Verification at the tip
Tip SHA: 510ccfb
All numbers were measured on this pull request's tree
fd6f410b6f96f94691abaddb18b398aec1afb2e7:TZ=UTC ARCH_DOCS_BASE=620235991e46c24836d03b16b76d642c9f7a4aa1 npm run verify— exit 0: core 1866, social 527, tree 125, timeline 125, workflow 106,.github548, interop 37, live-e2e 24; oxlint 0 errors (only the repository's existing warnings: 63/36/9/2/7/0);build-client --checkclean;arch-docs: OK (9 changed file(s));ds-bot-patches check:ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)andok 10 patches apply cleanly in order;node .github/scripts/changelog.mjs check:56 fragment(s) … are well formed.npm --prefix plugins/workflow teston this tree — 106 tests, 106 pass, 0 fail.62023599, unchanged — 105 tests, 105 pass, 0 fail.package.jsonon live base62023599, product code untouched — 106 tests, 104 pass, 2 fail:assign.test.jsapply registers the workflow endpoints and stays quiet when disabled(the registration list still carriesworkflow/say) andhost.test.jsworkflow plugin on the real host: the removed say endpoint is unknown, not an empty answer(the real host answers{ ok: true, … }).19aa2e88…against1b6f588c; the tree moved, so every number above was measured again rather than carried over.Not run: no live instance (no instance access), no browser beyond the test harness; CI was not waited on.
Checkpoint
{ "pr": 1497, "claim": "Chief", "issue": 1475, "summary": "The `workflow/say` host command is gone: its handler registration, the `say()` method that fabricated a 收到。我继续处理 receipt in the workflow chat, and its two documentation tokens. `stageMessages()`, the client, the stored workflows and the other endpoints are untouched, and no migration runs. A real-host test pins the outcome: `flybot/workflow/state` still answers `{ok:true}`, while `flybot/workflow/say` now answers `{ok:false,error:{code:'bot/not-found'}}`. A resolve hook, a fake Cordis context and an `--import` let that assertion run; the package ships no tests.", "files": ["plugins/workflow/src/service.js", "plugins/workflow/index.js", "plugins/workflow/test/host.test.js", "plugins/workflow/test/assign.test.js", "plugins/workflow/test/resolve.mjs", "plugins/workflow/test/fake-cordis.js", "plugins/workflow/package.json", "docs/subsystems/workflow.md", "plugins/workflow/README.md"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=620235991e46 npm run verify, exit 0 (core 1866, social 527, tree 125, timeline 125, workflow 106, .github 548, interop 37, live-e2e 24, oxlint 0, arch-docs OK 9, changelog ok) on tree fd6f410b; workflow suite at base 62023599 105/105; graft 104/106, 2 red", "result": "green", "at": "510ccfb23c16091be8fdb9652e801799814c7cbf" }, "next": "none — ready for review" }