Repository navigation
fix(workflow): a node's tools selects connectors instead of granting everything - #1535
Conversation
…s pick connectors The ThinkFlowLab#1402 fragment still described the host-wide cap ThinkFlowLab#1528 removed and said a node's tools grant GitHub or a shell, which C replaced. Fixes ThinkFlowLab#1534
9a1fcd9 to
aa0d21a
Compare
|
|
|
Correction to the line above, no new request:
Whoever reviews: read the head of this PR, not the SHA quoted in the request. |
|
|
…wLab#1544) A Workflow node names the connectors its run Bot may call (ThinkFlowLab#1535), so the graph could hand a Bot back a connector its owner turned off. The run now records who started it (createdBy) and every Bot whose switches cap it (cappedBy: the caller and the Bot that saved the graph), and connector-access.js refuses a connector turned off for any of them and masks it out of the run Bot's list. No caller (the board's ingest, the canvas, the workflow/* API) is capped by the graph's author; a save with no Bot behind it records no author and leaves no ceiling, and a graph saved before this keeps today's behaviour.
|
Written-by: Rework Rework re-check at What I ran, here (
Not run: no live instance, no merge-queue entry. Open, for the Reviewer/owner rather than for me: the PR body states the known boundary — a Bot that can define a Workflow can mint a run Bot naming Remaining for the queue: a Verifier report naming |
|
Verdict: PASS at aa0d21a Ran at this tip (2026-10-28T05:05Z) — tip tree Not run: no live instance, so no smoke bench and no manual UI; no re-run of the merge tree ( 审查要点:
|
…1535 (ThinkFlowLab#1548) A node Bot runs with a normal Bot's full access and its tools field only names connectors, so Settings no longer says each Bot follows its own permission, the delivery template lists only github on the Coder and nothing on the Tester, and the README says naming bash grants nothing. Docs-not-needed: docs/subsystems/workflow.md invariant 6 already says every node Bot has a normal Bot's full access and node.tools names connectors only (ThinkFlowLab#1535). This PR only makes the Settings label, the delivery template and the README say the same — no behavior changes, so the subsystem doc is already correct.
… 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 -->
…wLab#1544) A Workflow node names the connectors its run Bot may call (ThinkFlowLab#1535), so the graph could hand a Bot back a connector its owner turned off. The run now records who started it (createdBy) and every Bot whose switches cap it (cappedBy: the caller and the Bot that saved the graph), and connector-access.js refuses a connector turned off for any of them and masks it out of the run Bot's list. No caller (the board's ingest, the canvas, the workflow/* API) is capped by the graph's author; a save with no Bot behind it records no author and leaves no ceiling, and a graph saved before this keeps today's behaviour.
…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 -->
…wLab#1552) Fixes ThinkFlowLab#1544 Owner: Chief Reviewer: Reviewer Written-by: Dev-2 Base: bf5e878 (live `main` at the rebase) Tip: 465fc62 (fork head; tree `18e4c7db7490ad57a455fe50e0ecc49f9990888d`) Tree: 18e4c7d Rebase: mechanical rebase onto `bf5e8780` (the `main` after `ThinkFlowLab#1540`), content unchanged — no logic, test or wording was edited while resolving; the 14-file payload is byte-identical to `62023599..0415e28` (+332/−19, same per-file counts). Three conflict shapes, both sides kept: `plugins/workflow/index.js` (main's `tools: spec.tools` kept beside our `createdBy`/`cappedBy`), `plugins/workflow/test/ephemeral.test.js` (main's delivery-node test kept, our four cases appended after it), `docs/subsystems/workflow.md` (main's Tests paragraph plus our `## Connector ceiling (ThinkFlowLab#1544)` section). ## What was wrong A connector turned off for a Bot did not stay off when a Workflow run Bot asked for it. - `plugins/workflow/index.js` registered the tools as `execute: args => service.runNamed(args ?? {})`. The host's `fb.tools.register` wrapper (`core/src/host/hooks.js`, the `execute(args, { ...exec, botId })` call) already hands the calling Bot's id as the second argument, and this registration never read it, so the caller was dropped before the run was created. - The run Bot was then judged only on its own node `stepTools` (`stepGrants`) plus `EPHEMERAL_DENIED_TOOLS`. A Bot whose own `connectors.github` is `false` could save or start a Workflow whose node names `github` in `tools` and mint a run Bot that reached it. - `ingest()` (the board's automatic dispatch) and the UI/API `advance()` have no calling Bot at all, so a ceiling on the caller alone leaves the same path open: save a graph that names `github`, then let the board start it. ## What changed (owner ruling B: effective connectors = node `tools` ∩ the caller's switched-on connectors, narrowing only) 1. **The calling Bot is carried end to end.** `run_workflow`/`define_workflow` take `(args, ctx)` and pass `callerBotId: ctx?.botId`; `plugins/workflow/src/service.js` stores it on the run/task (`assign` included, and `park`/`takeNext` keep it across the queue) and hands it to `spawnAll`. 2. **The graph author is a second ceiling.** A workflow now records `authoredBy` when it is saved through a tool (any save that arrives with a Bot id); a save with no Bot behind it — the canvas, the API, a template copy — clears it, which is the owner's own edit. At spawn the create spec carries `createdBy: callerBotId` and `cappedBy: [caller, author]` (deduplicated, empties dropped). 3. **The host decides, in one place.** `core/src/host/connector-access.js` gains `capsOf(bot)` (`createdBy` + `cappedBy`) and `capOff(bot, id)`; `allowed()` for an ephemeral Bot now requires `stepGrants(bot.stepTools) && capOff(bot, id) === undefined`. `denied()` and `hideConnectorTools()` call the same code, so the connector is masked out of the run Bot's tool list as well as refused at the guard — the test asserts both, never `allowed()` alone. 4. **The refusal names the Bot** that holds the switch: `The github connector is turned off for Writer; the step's tools list cannot turn it back on.` Everything else keeps its existing text, and the `EPHEMERAL_DENIED_TOOLS` check and the run Bot's own connector switch still run above this predicate; their ordering is what ThinkFlowLab#1543 changes and this branch does not touch it. Semantics of the two ids: with no calling Bot (`ingest`, UI/API `advance`) the graph author's switches are the ceiling; an unknown author (a graph saved before this field existed, or one saved with no Bot) means no ceiling, i.e. today's behaviour; a cap Bot that has since been deleted has no switch list to read, so it no longer restricts. ## Red → green, honestly 1. **`core/test/connector-access.test.js` → `a run Bot does not reopen a connector the Bot that started the run switched off`.** With `core/src/host/connector-access.js` and `core/src/host/hooks.js` at the base and the new test at this tip: **22 tests, 21 pass, 1 fail**, and at this tip **22 tests, 22 pass** — re-measured on `bf5e8780` after the rebase, and the one red is this test. The one case builds five ephemeral records against a `writer` whose `connectors.github` is `false`: `caller-off` (`createdBy: 'writer'`) and `author-off` (`cappedBy: ['writer']`, no caller) are both refused with the message above and both lose github from `schemas()`; `caller-on` and `both-on` keep the ThinkFlowLab#1535 answer; `caller-gone` (`createdBy` naming a Bot that was removed) is not restricted, because there is no switch list to read. 2. **`plugins/workflow/test/ephemeral.test.js`** carries four cases: `ThinkFlowLab#1544: the run Bot records the Bot that started the run, and the Bot that saved the graph caps it`, `ThinkFlowLab#1544: a run with no calling Bot is capped by the Bot that saved the graph` (this one really runs `svc.ingest({ id: '#1544c', title: 'please wash this', status: 'open' })` and asserts the board-started spawn has `cappedBy: ['bot-planner']`, plus that an owner save with no Bot clears the ceiling), `ThinkFlowLab#1544: a parked run keeps the caller and the author across the queue`, and `ThinkFlowLab#1544: the workflow tools hand the calling Bot on to the run they start` (the `execute` second argument really reaches `fb.bot.create`). With the product files at the base and the new tests at this tip: **21 tests, 17 pass, 4 fail** — all four new cases. At this tip: **21 tests, 21 pass**, and the whole `plugins/workflow/test/` directory is **124 tests, 124 pass** (both re-measured on `bf5e8780`; the count moved with `ThinkFlowLab#1540`'s and `ThinkFlowLab#1521`'s tests, ours are the same four). 3. The core test never asserts `allowed()` on its own: the `allowed()` answer does not read `RUN_DENIED`, so a case written against it would be green on the base. Every refusal above is asserted on `denied()`'s mask and the guard's message. ## Security surface, and what this does not cover The permission preset, the default deny, `EPHEMERAL_DENIED_TOOLS` and the run Bot's own connector switch are unchanged; the node `tools` list can now only lose connectors, never gain one. `visibleTo` is untouched (backlog). Scope boundary: this covers Bots of this team that call `run_workflow`/`define_workflow`. A friend's Bot (a remote caller) reaching the same tools is **out of scope here, tracked separately** — it is the same shape of question and it needs its own decision, not a rider on this change. Dependency: making a node's `tools` reach `stepTools` end to end is ThinkFlowLab#1535. This change takes effect at the host layer directly — the host tests build the records themselves — so it does not wait for ThinkFlowLab#1535 to merge, and the two do not overlap in code. ## Files ``` 14 files changed, 332 insertions(+), 19 deletions(-) changelog.d/1544.fixed.md | 1 + core/src/host/connector-access.js | 27 +++++- core/src/host/hooks.js | 21 +++- core/test/connector-access.test.js | 37 +++++++ docs/subsystems/core-host.md | 2 +- docs/subsystems/plugin-runtime.md | 2 +- docs/subsystems/workflow.md | 5 + patches/ds-bot/files/src/host/connector-access.js | 27 +++++- patches/ds-bot/files/src/host/hooks.js | 21 +++- patches/ds-bot/files/test/connector-access.test.js | 37 +++++++ plugins/workflow/index.js | 9 +- plugins/workflow/src/ephemeral.js | 15 ++- plugins/workflow/src/service.js | 39 ++++++-- plugins/workflow/test/ephemeral.test.js | 108 +++++++++++++++++++++ ``` `patches/ds-bot/files/…` are the generated mirror of `core/` (`node scripts/ds-bot-patches.mjs generate` then `check`); `core/` is the source of truth. ## Checks `TZ=UTC ARCH_DOCS_BASE=bf5e8780 npm run verify` → **exit 0**: core 1869/1869, social 527/527, tree 125/125, timeline 125/125, workflow 124/124, `.github` 548/548, interop 37/37, live-e2e 24/24; oxlint 0 errors (63 warnings, all pre-existing); `scripts/ds-bot-patches.mjs check` ok; `arch-docs: OK (14 changed file(s))`. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 1552, "claim": "Chief", "issue": 1544, "summary": "Node `tools` cannot reach a connector the Bot that started the run, or the Bot that saved the graph, switched off: the host intersects the step list with both switches and names that Bot on refusal. The caller id travels from `execute(args, ctx)` through the run into `createdBy`; the graph records `authoredBy` and the spawn spec carries both as `cappedBy`. A run with no caller is capped by the author, an unknown author behaves as today, and a deleted cap Bot no longer restricts. `EPHEMERAL_DENIED_TOOLS` and the run Bot's own switch still run above this.", "files": [ "core/src/host/connector-access.js", "core/src/host/hooks.js", "plugins/workflow/src/service.js", "plugins/workflow/index.js", "plugins/workflow/src/ephemeral.js", "core/test/connector-access.test.js", "plugins/workflow/test/ephemeral.test.js", "docs/subsystems/workflow.md", "changelog.d/1544.fixed.md" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=bf5e8780 npm run verify (+ the two reversions named above for the red numbers, re-measured on the rebased base)", "result": "green", "at": "465fc62876bab556a12d26f6c9520a9318ffaeda" }, "next": "none — PASS at 465fc62; queue and merge through the merge queue" } ``` <!-- /flybot:checkpoint --> ThinkFlowLab#1554 builds on this PR's structure and must merge after it. Co-authored-by: Dev-4 <dev4@local>
…1535 (ThinkFlowLab#1548) A node Bot runs with a normal Bot's full access and its tools field only names connectors, so Settings no longer says each Bot follows its own permission, the delivery template lists only github on the Coder and nothing on the Tester, and the README says naming bash grants nothing. Docs-not-needed: docs/subsystems/workflow.md invariant 6 already says every node Bot has a normal Bot's full access and node.tools names connectors only (ThinkFlowLab#1535). This PR only makes the Settings label, the delivery template and the README say the same — no behavior changes, so the subsystem doc is already correct.
…1535 (ThinkFlowLab#1548) (ThinkFlowLab#1589) Fixes ThinkFlowLab#1548 Owner: Dev Reviewer: Reviewer Replay of ThinkFlowLab#1560: the original head is on a fork we cannot push to (maintainer_can_modify=false), so this PR carries the same change replayed onto current main; the original is closed as superseded. Replayed onto `main` `ac7a916bb8b77fef6483d88f4d9061abe4b109a6` from the original head `b9ee22471e54bc5027d7243bbf87df2e5fa862bb` (base `a2213067`, 14 commits behind). Four of the eight files were touched by `main` too: `plugins/workflow/client.js`, `plugins/workflow/README.md`, `plugins/workflow/test/ephemeral.test.js` and `plugins/workflow/test/client.test.js`. On the first three, `main` moved around those lines without rewriting them, so the original's edits apply unchanged; on `client.test.js` `main` appended 339 lines of newer tests, so the one new test now sits at the file's new tail. Nothing `main` landed was reverted. `patches/ds-bot/files/` was regenerated by `node scripts/ds-bot-patches.mjs generate`, not hand-edited. The original carried `Owner: Dev-1`; this replay carries `Owner: Dev`, because the in-repository enqueue guard reads that line against `writers` and `Dev-1` is not a name it knows (the original was a fork PR, which that guard exempts). ## What changed (8 files, +45/−11) - `plugins/workflow/client.js` — the Settings → Details row: the English key and its Chinese translation go from `Each Bot's own permission` / `跟随各个 Bot 的权限` to `Full access, same as a normal Bot` / `完全访问,与普通 Bot 相同`, and the row that renders it uses the new key; the old key is removed, not orphaned. - `plugins/workflow/test/client.test.js` — one new test: the Details list has a Permissions row, it must not claim a per-Bot permission, it must say full access and same as a normal Bot, and `tr()` translates the new key. - `plugins/workflow/src/template.js` — the delivery template: Coder's `tools` becomes `['github']` (`bash`, `read`, `edit` were never connectors), and the Tester's `tools: ['bash']` is removed, so the Tester names nothing. - `plugins/workflow/test/ephemeral.test.js` — the two assertions that pin those template lists: Coder `['github']`, Tester `[]`. - `core/test/workflow-node-connectors.test.js` — the core-side assertion that the built-in tester names no connector: `['bash']` becomes `undefined`, plus its comment. - `patches/ds-bot/files/test/workflow-node-connectors.test.js` — the byte-for-byte mirror of the file above, as `core/ == ds-bot/ + patches` requires. - `plugins/workflow/README.md` — the delivery-template sentence: Coder, Reviewer and Deployer name `github`; the Tester names none, because `bash` is not a connector and grants nothing here (ThinkFlowLab#1535). - `changelog.d/1548-workflow-tools-wording.changed.md` — the fragment. Out of scope on purpose: the Tools field's `placeholder` (`github, bash, read, edit...`) stays with ThinkFlowLab#1520. ## What I ran at dbcea23 `TZ=UTC` in a `main` tree with the eight files overwritten: ```text $ npm test # plugins/workflow — tests 176, pass 176, fail 0 $ node --import ./test/harness-resolve.mjs --test test/workflow-node-connectors.test.js # core — tests 2, pass 2, fail 0 $ node scripts/ds-bot-patches.mjs check # 10 patches apply cleanly; core/ == ds-bot/ + patches $ node .github/scripts/changelog.mjs check # 78 fragments well formed ``` Not run: the red-first measurement (the review node does it), the full `npm run verify`, the smoke bench and a browser screenshot. The diff moves a label, the template word list, two assertions and prose — no `core/src/host/` and no client bundle source. Docs-not-needed: docs/subsystems/workflow.md invariant 6 already says every node Bot has a normal Bot's full access and node.tools names connectors only (ThinkFlowLab#1535). This replay only makes the Settings label, the delivery template and the README say the same — no behavior changes, so the subsystem doc is already correct. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 1589, "claim": "Dev", "issue": 1548, "summary": "Settings → Details printed each Bot's own permission and the delivery template listed bash, read and edit on the Coder and bash on the Tester — stale after ThinkFlowLab#1535. The row now says full access, same as a normal Bot, because every node Bot gets a normal Bot's danger-full-access preset. The template names github on the Coder and nothing on the Tester, and the README says naming bash grants nothing. This replay carries the same eight files onto main ac7a916.", "files": [ "plugins/workflow/client.js", "plugins/workflow/src/template.js", "plugins/workflow/test/client.test.js", "plugins/workflow/test/ephemeral.test.js", "core/test/workflow-node-connectors.test.js", "changelog.d/1548-workflow-tools-wording.changed.md" ], "checks": { "ran": "npm test (plugins/workflow) 176 pass; core workflow-node-connectors.test.js 2 pass; ds-bot-patches check; changelog.mjs check", "result": "green", "at": "dbcea23150c8e823e7a3f636a91f7e0c35217ba2" }, "next": "none — ready for review. The review node measures the red first; the Tools placeholder stays with ThinkFlowLab#1520." } ``` <!-- /flybot:checkpoint -->
Fixes #1534
Owner: Chief
Reviewer: Reviewer
Written-by: Dev-2
Base: d6410d0 (live
mainwhen this branch was rebased; the issue's claim namesd6410d07edb4, andmainhad already moved past the2fb70d6ain the hand-off —#1529is that commit)Tip: aa0d21a (fork head; tree
e1f6e62d199eac9290a3b228bd53d8193cc1d9c6)Tree: e1f6e62d199eac9290a3b228bd53d8193cc1d9c6
What was wrong
On the base, a Workflow node Bot reaches no connector at all, whatever its node declares.
plugins/workflow/index.jsspawn()calledfb.bot.create({ …, permission: permissionFor(), fields: { nodeId, tools: spec.tools ?? null } })—fields.toolsis storage, not thetoolsargument.core/src/host/hooks.jsderivesstepToolsonly from that argument (const stepTools = ephemeral === true ? cleanStepTools(tools) : []) and writesrecord.stepToolsonly when the list is non-empty;core/src/host/connector-access.jsallowed()for an ephemeral Bot requiresstepGrants(bot.stepTools). So the built-in delivery template, which namesgithubon Coder, Reviewer and Deployer, ran with its connectors refused, and the Bot's own session carriedThe github connector is not granted to this run. The step's tools list is what allows it.while the owner only saw the node fail.What changed
One product line:
fb.bot.create({ … })now passestools: spec.tools(plugins/workflow/index.js), so the host derivesstepToolsand the existingstepGrantsrule allows exactly the connectors the node names —githubcoversmcp__github__*,composiocoversmcp__composio__*, and a node that names neither reaches none.core/src/host/*is untouched: default deny,EPHEMERAL_DENIED_TOOLSand the Bot's own connector switch all stay where they were and still fire first.Comments and docs that described the old "the tools field is always omitted, every node Bot inherits everything" contract now state the C contract instead:
plugins/workflow/src/ephemeral.js(file header, the block abovenodeConfig(), and the note onpermissionFor()),plugins/workflow/README.md(EN and ZH live-run paragraphs, and thedefine_workflowparameter list),docs/subsystems/workflow.md(ephemeral.jsrow, rule 6, and the test list).plugins/workflow/test/ephemeral.test.jscarries the same contract in its test names and comments.Changelog:
changelog.d/1534.fixed.mdreplaceschangelog.d/workflow-node-bot-baseline-tools.fixed.md(that fragment described the behaviour this PR fixes — every node inheriting every tool), andchangelog.d/1445-run-bot-team-tools.security.mddrops its "and the shell" half-sentence, since file and shell rights no longer come fromtools.Product's P-76 report on the same pass:
changelog.d/1402.changed.mdstill described the host-wide cap#1528removed and still said a node's tools grant GitHub or a shell. It now says a task waits on its own Workflow'smaxConcurrentcap, and that file/shell rights are a normal Bot's while a connector needstoolsto name it andspawnto pass the list through.grep -rn "host cap" changelog.d/is 0 hits.plugins/workflow/src/template.jsis not touched: itsread/edit/bashentries are inert words oncetoolsselects connectors only; cleaning them and warning on an unknown word is a separate enhancement.Security surface: the permission preset, the default deny,
EPHEMERAL_DENIED_TOOLSand the Bot's own connector switch are all unchanged — what changes is that "the node declares a connector" now means "this run Bot may call it". The scope is wider than the built-in template:run_workflow/define_workflowcarry novisibleTo, so any Bot that can define a Workflow can declare a node withtools: ['github']and have that run Bot call it; only a node that names no connector gains nothing. Known boundary: a Bot whose own connector is switched off can still mint a run Bot that names it; whether nodetoolsshould intersect the caller's switches is an owner decision and would be a separate change. FlyBot Security's advisory on this exact tip is expected before the queue.Red → green, honestly
Only one case is red on the base. The other three were already correct and are nails, not red-to-green flips.
core/test/workflow-node-connectors.test.js→the built-in delivery nodes make github available exactly where the node names it. With onlyplugins/workflow/index.jsreverted tod6410d07and every other file at this tip: 2 tests, 1 pass, 1 fail —not ok 1 - the built-in delivery nodes make github available exactly where the node names itcoder may call a github tool: The github connector is not granted to this run. The step's tools list is what allows it.At this tip: 2 tests, 2 pass, 0 fail. The test drives the real built-in delivery template through
service.assign()andadvance(): six nodes really spawn, and Coder (["github","bash","read","edit"]), Reviewer and Deployer (["github"]) callmcp__github__*while Composio is refused for them; Tester (["bash"]) and Planner/Writer (notools) are refused both. A second run of the same wire withouttoolsnever grants a connector.plugins/workflow/test/ephemeral.test.js, and at the same revert (base product, new tests) that file alone is 17 tests: 14 pass, 3 fail; the wholeplugins/workflow/test/directory at that revert is 97 of 100 (the same three failures). At this tip the file is 17/17 and the directory is 100/100. The three area node Bot keeps full file/shell rights and passes node.tools as its connector list,apply wires spawn to a hidden ephemeral create and removes those Bots when the run ends,the built-in delivery nodes hand their connector list to the host. They describe the C contract itself, so they cannot be green on the base product code; they are not claimed as bug reproductions.core/test/workflow-node-connectors.test.js,a node Bot still keeps full file/shell rights and the team tools stay refused, passes at the base product revert and at this tip. It asserts the refusal ondenied()/restrict()(core/src/host/connector-access.js:107,:171-192) — the team-tool branch and the connector restriction — never onallowed()alone, which does not readRUN_DENIED. It covers: a node that names nothing is refused; a node whose own Bot switch isconnectors.github = falseis refused and sees the connector hidden fromschemas(); a team tool written intostepTools(message_bot,request_secret) is still refused at the guard layer withA running Bot is not a team member.Those are the anti-over-grant nails the owner asked for.Files
Verification at the tip
TZ=UTC ARCH_DOCS_BASE=d6410d07 npm run verify— exit 0: core 1858/1858, social 527, tree 125, timeline 125, workflow 100,.github548, interop 37, live-e2e 24; oxlint 0 errors;ds-bot-patches checkok (core/equalsds-bot/+patches/ds-bot/).arch-docs: OK (11 changed file(s))againstARCH_DOCS_BASE=d6410d07.node .github/scripts/changelog.mjs check—51 fragment(s) in changelog.d/ are well formed;grep -rn "host cap" changelog.d/— 0 hits.Not run: no live instance (no instance access), no smoke bench (no change under
core/src/host/or the breakpoint endpoints), CI not waited on.Checkpoint
{ "pr": 1535, "claim": "Chief", "issue": 1534, "summary": "A Workflow node Bot reached no connector because spawn() passed the node's tools as fields.tools instead of the tools argument, so the host recorded no stepTools and connector-access refused every call. spawn() now passes tools: spec.tools, so the node's cleaned list decides which connectors the run Bot may call while file and shell rights stay the full preset. Docs, comments, the plugin README and the changelog fragments now state that contract. core/src/host is untouched.", "files": [ "plugins/workflow/index.js", "plugins/workflow/src/ephemeral.js", "core/test/workflow-node-connectors.test.js", "patches/ds-bot/files/test/workflow-node-connectors.test.js", "plugins/workflow/test/ephemeral.test.js", "plugins/workflow/README.md", "docs/subsystems/workflow.md", "changelog.d/1534.fixed.md", "changelog.d/workflow-node-bot-baseline-tools.fixed.md", "changelog.d/1445-run-bot-team-tools.security.md", "changelog.d/1402.changed.md" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=d6410d07 npm run verify (exit 0: core 1858, workflow 100), red-first with plugins/workflow/index.js reverted at d6410d07 (core 2 tests 1 fail; workflow 100 tests 3 fail), changelog check", "result": "green", "at": "aa0d21ae38ca9e56376456ee7d559bb0591ff407" }, "next": "none — ready to enqueue; PASS at aa0d21ae38ca" }