Repository navigation
fix(workflow): a switched-off connector stays off for a run - #1552
Merged
Merged
Conversation
HugoXK
force-pushed
the
dev/2-connector-launder
branch
from
October 28, 2026 04:14
86fb9e7 to
0415e28
Compare
Contributor
Author
github-actions
Bot
added
the
risk:T3
HugoXK
force-pushed
the
dev/2-connector-launder
branch
from
October 28, 2026 07:13
0415e28 to
465fc62
Compare
Contributor
Author
|
@Birfy review request — PR #1552 at Self-proof, checked twice (mine and Dev-2's): Counts at this tip: |
…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.
Collaborator
|
Verdict: PASS at 465fc62 Ran at this tip (2026-10-28T08:32Z): 审查要点：
|
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 -->
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 #1544
Owner: Chief
Reviewer: Reviewer
Written-by: Dev-2
Base: bf5e878 (live
mainat the rebase)Tip: 465fc62 (fork head; tree
18e4c7db7490ad57a455fe50e0ecc49f9990888d)Tree: 18e4c7db7490ad57a455fe50e0ecc49f9990888d
Rebase: mechanical rebase onto
bf5e8780(themainafter#1540), content unchanged — no logic, test or wording was edited while resolving; the 14-file payload is byte-identical to62023599..0415e28c(+332/−19, same per-file counts). Three conflict shapes, both sides kept:plugins/workflow/index.js(main'stools: spec.toolskept beside ourcreatedBy/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 (#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.jsregistered the tools asexecute: args => service.runNamed(args ?? {}). The host'sfb.tools.registerwrapper (core/src/host/hooks.js, theexecute(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.stepTools(stepGrants) plusEPHEMERAL_DENIED_TOOLS. A Bot whose ownconnectors.githubisfalsecould save or start a Workflow whose node namesgithubintoolsand mint a run Bot that reached it.ingest()(the board's automatic dispatch) and the UI/APIadvance()have no calling Bot at all, so a ceiling on the caller alone leaves the same path open: save a graph that namesgithub, then let the board start it.What changed (owner ruling B: effective connectors = node
tools∩ the caller's switched-on connectors, narrowing only)run_workflow/define_workflowtake(args, ctx)and passcallerBotId: ctx?.botId;plugins/workflow/src/service.jsstores it on the run/task (assignincluded, andpark/takeNextkeep it across the queue) and hands it tospawnAll.authoredBywhen 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 carriescreatedBy: callerBotIdandcappedBy: [caller, author](deduplicated, empties dropped).core/src/host/connector-access.jsgainscapsOf(bot)(createdBy+cappedBy) andcapOff(bot, id);allowed()for an ephemeral Bot now requiresstepGrants(bot.stepTools) && capOff(bot, id) === undefined.denied()andhideConnectorTools()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, neverallowed()alone.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 theEPHEMERAL_DENIED_TOOLScheck and the run Bot's own connector switch still run above this predicate; their ordering is what fix(core): a run Bot's switched-off connector is reported as "not granted to this run" #1543 changes and this branch does not touch it.Semantics of the two ids: with no calling Bot (
ingest, UI/APIadvance) 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
core/test/connector-access.test.js→a run Bot does not reopen a connector the Bot that started the run switched off. Withcore/src/host/connector-access.jsandcore/src/host/hooks.jsat 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 onbf5e8780after the rebase, and the one red is this test. The one case builds five ephemeral records against awriterwhoseconnectors.githubisfalse:caller-off(createdBy: 'writer') andauthor-off(cappedBy: ['writer'], no caller) are both refused with the message above and both lose github fromschemas();caller-onandboth-onkeep the fix(workflow): a node's tools selects connectors instead of granting everything #1535 answer;caller-gone(createdBynaming a Bot that was removed) is not restricted, because there is no switch list to read.plugins/workflow/test/ephemeral.test.jscarries four cases:#1544: the run Bot records the Bot that started the run, and the Bot that saved the graph caps it,#1544: a run with no calling Bot is capped by the Bot that saved the graph(this one really runssvc.ingest({ id: '#1544c', title: 'please wash this', status: 'open' })and asserts the board-started spawn hascappedBy: ['bot-planner'], plus that an owner save with no Bot clears the ceiling),#1544: a parked run keeps the caller and the author across the queue, and#1544: the workflow tools hand the calling Bot on to the run they start(theexecutesecond argument really reachesfb.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 wholeplugins/workflow/test/directory is 124 tests, 124 pass (both re-measured onbf5e8780; the count moved with#1540's and#1521's tests, ours are the same four).allowed()on its own: theallowed()answer does not readRUN_DENIED, so a case written against it would be green on the base. Every refusal above is asserted ondenied()'s mask and the guard's message.Security surface, and what this does not cover
The permission preset, the default deny,
EPHEMERAL_DENIED_TOOLSand the run Bot's own connector switch are unchanged; the nodetoolslist can now only lose connectors, never gain one.visibleTois 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
toolsreachstepToolsend to end is #1535. This change takes effect at the host layer directly — the host tests build the records themselves — so it does not wait for #1535 to merge, and the two do not overlap in code.Files
patches/ds-bot/files/…are the generated mirror ofcore/(node scripts/ds-bot-patches.mjs generatethencheck);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,.github548/548, interop 37/37, live-e2e 24/24; oxlint 0 errors (63 warnings, all pre-existing);scripts/ds-bot-patches.mjs checkok;arch-docs: OK (14 changed file(s)).Checkpoint
{ "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 465fc62876bab556a12d26f6c9520a9318ffaeda; queue and merge through the merge queue" }#1554 builds on this PR's structure and must merge after it.