Repository navigation
fix(workflow): explain the node inspector's Tools field and keep a typed connector list - #1573
Conversation
1728d91 to
a053280
Compare
#1520) The node inspector's Tools input rebuilt its value from the saved list on every keystroke, and the host drops a trailing empty token when it normalizes that list, so a separator typed between two connector names did not survive one round trip: "github, composio" typed one character at a time saved as the single token "githubcomposio", which stepGrants does not know, so the node got no connector at all. The field now holds a local draft and commits it on blur or after a short pause; the host normalization and the spawn path are not touched. The field also gains the explanation it never had, in English and Chinese: it names the connectors the run Bot may use (github grants mcp__github__*, composio grants the Composio connector), file and shell access is always a normal Bot's full access, and a node that names no connector gets none. The placeholder names github, composio instead of advertising bash, read and edit as if they granted anything. Fixes #1520 Docs-not-needed: plugins/workflow/client.js copy plus an input-local draft; the workflow subsystem doc already states that node.tools names connectors only and every node Bot gets a normal Bot's file and shell access.
a053280 to
fc6cddf
Compare
|
Verdict: PASS at fc6cddf Ran at this tip (2026-10-28T08:58Z). The tree I ran is the real tip: I rebuilt it from base $ cd plugins/workflow && node --import ./test/resolve.mjs --test "test/*.test.js" tests 138, pass 138, fail 0$ node --import ./test/resolve.mjs --test test/client.test.js tests 45, pass 45, fail 0Red first — the same suite with tests 138, pass 136, fail 2not ok 68 — the Tools field says it names connectors and that file and shell access is always full ( $ node scripts/lint-plugins.mjs → 0 errors; packed @flybot/workflow imported Scope: CI at this tip: 14 check-runs, 13 success; the only failure is Not blocking, noted: the unmount cleanup clears the 600 ms timer without flushing, so a value typed and abandoned inside that window is dropped (disclosed in the body's Risk), and the new note is hardcoded bilingual rather than a |
|
Verdict: PASS at fc6cddf Replaces: the PASS at fc6cddf merge-gate rejected for having no Ran: line. The conclusion is unchanged; this body adds the commands it ran. Ran at this tip (2026-10-28T08:58Z–09:05Z), on a tree rebuilt from base The two red tests are the two this change adds:
Scope: CI at this tip: 14 check-runs, 13 success; the only failure is Not blocking, noted: the unmount cleanup clears the 600 ms timer without flushing, so a value typed and abandoned inside that window is dropped (disclosed in the body's Risk), and the new note is hardcoded bilingual rather than a |
…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 -->
…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 -->
Fixes #1520
Owner: Chief
Reviewer: Reviewer
Tip SHA:
fc6cddff4eed35dd1b43828965fbcef8f351248bBase:
9b866a5c797e8efe95e445885b69da4b8ca5c35b(main; replayed onto it after #1514 landed on the same two files)Docs-not-needed:
plugins/workflow/client.jscopy plus an input-local draft state; the workflow subsystem doc already states thatnode.toolsnames connectors only and every node Bot gets a normal Bot's file and shell access.What changed
The field explains itself, in English and Chinese. Under the Tools input the inspector now says: Names the connectors this run Bot may use: github grants mcp__github__*, composio grants the Composio connector. File and shell access is always full, like a normal Bot, and a node that names no connector gets none. — followed by the same sentence in Chinese. The placeholder changes from
github, bash, read, edit...togithub, composio, becausebash,readandeditare not connectors and grant nothing here.Typing a second connector works. The value used to be rebuilt from the saved list on every keystroke (
(selected.tools ?? []).join(', ')) whileonChangesaved immediately; the host'scleanToolsdrops a trailing empty token, so the re-render ate the separator andgithub, composiotyped one character at a time saved as the single tokengithubcomposio.stepGrantsknows onlygithub,composioandmcp__<id>__*, andrestrict()had already taken the connector tools away, so the node ran with no connector and nothing said so. The input now keeps a local draft (toolsDraft) and commits it on blur or after a 600 ms pause; typing and pasting end in the same saved list. The host normalization and the spawn path are untouched.Verification at the tip
Tip SHA:
fc6cddff4eed35dd1b43828965fbcef8f351248bRan:
npm --prefix plugins/workflow testat the tip → 138 / 138 / 0.client.jsblob: 43 / 45 / 2 —the Tools field says it names connectors and that file and shell access is always fullfailed onthe Tools field explains what it names;the Tools field takes a second connector typed one character at a timefailed with actual'github'against expected'github,'after the ninth character.npm --prefix plugins/social test527 / 527 / 0,plugins/tree125 / 125 / 0,plugins/timeline125 / 125 / 0.node core/scripts/build-client.mjs --checkclean,node scripts/ds-bot-patches.mjs checkclean,node scripts/lint-plugins.mjs0 errors,node .github/scripts/changelog.mjs check60 fragments well formed.core/is byte-identical to main in this branch and both failures are this sandbox —empty diff passesneedsgit(absent here),fixture usage.sqlite has one real calls row with all five fieldsreads a fixture this extraction does not carry.Not run (and why):
npm run verifyend to end: this sandbox has nogitbinary, sonpm --prefix core run lintstops atarch-docs: FAIL — cannot diff against <base>(the oxlint half ran: 0 errors, 63 pre-existing warnings). TheDocs-not-needed:trailer is in the branch commit and in this body for the CI half that can diff.core/src/host/, the plugin's host seams andsrc/ephemeral.jsare untouched.Risk
T2: the diff is under
plugins/**, so it needs a PASS at this tip plus greentests-okbefore the queue. The behavior change is confined to the inspector input: a typed list now commits on blur or after 600 ms instead of on every keystroke, so a save can land up to 600 ms later than before but never later than leaving the field, and the unmount cleanup drops a pending timer. The saved shape, the host normalization (cleanTools),stepGrantsand the spawn path are unchanged, so no run sees a differentnode.tools.mainmoved twice while this was open (#1572, then #1514 on these same two files); the branch was replayed each time and the diff is still only these three paths.Checkpoint
{ "pr": 1573, "claim": "Chief", "issue": 1520, "summary": "The inspector's Tools field now says it names the connectors the run Bot may use (github → mcp__github__*, composio → Composio), that file and shell access is always a normal Bot's, and that naming none reaches none, in English and Chinese. Its placeholder is github, composio, not bash, read and edit. The field keeps a local draft and commits on blur or a pause, so a second connector typed by hand no longer loses its separator: before, the host's trailing-token normalization saved githubcomposio and silently refused the connector. Only client.js, its test and one changelog fragment change.", "files": [ "plugins/workflow/client.js", "plugins/workflow/test/client.test.js", "changelog.d/1520.fixed.md" ], "checks": { "ran": "npm --prefix plugins/workflow test (138/138); npm --prefix plugins/social|tree|timeline test; node core/scripts/build-client.mjs --check; node scripts/ds-bot-patches.mjs check; node scripts/lint-plugins.mjs", "result": "green", "at": "fc6cddff4eed35dd1b43828965fbcef8f351248b" }, "next": "none — ready for review; T2 needs a PASS at this tip and green tests-ok before the queue." }