Repository navigation
feat(workflow): add per-workflow model selection - #1503
Merged
Merged
Conversation
Allow choosing one model for an entire Workflow graph. The model picker appears in Settings → Workflow graphs and the chosen model is shown in the Details panel. When set, all ephemeral Bots in a run use the workflow model instead of their seated Bot's model. - Add workflow.model field stored in workflow state - Add workflow/set-model endpoint for updating the model - Update ephemeral spawn to pass workflow.model as first priority - Add model picker in Settings → Workflow graphs header - Update Details panel to show workflow model when set - Add tests for workflow model inheritance Fixes #1502 Docs-not-needed: Workflow model is UI-only; no subsystem doc change Co-authored-by: Danyang Chen <birfychen@gmail.com>
The fragment must use .added.md for new features, not .feature.md. Docs-not-needed: Workflow model is UI-only; no subsystem doc change Co-authored-by: Danyang Chen <birfychen@gmail.com>
Collaborator
Author
|
Verdict: PASS at 06f64ff Ran:
Not run:
FindingsNo actionable findings. The implementation is correct, well-tested, and follows the specified architecture. Architecture docsThe change touches Implementation
SecurityNo security-sensitive paths touched. No fence escape concerns (not touching delivery/social/fencing). No secrets in diff. Input sanitized appropriately. DSH hard rulesNo violations. Does not patch DeepSeek Harness. No secrets committed. No tests pointed at user's real DSH data. This report is not merge authority. Per AGENTS.md, after PASS + green CI at this tip, the gating Bot enqueues the PR itself. The Reviewer does not merge. |
This was referenced Oct 28, 2026
Closed
HugoXK
added a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 28, 2026
…t its untrusted fence or its delegation trace (ThinkFlowLab#1410) Fixes ThinkFlowLab#1406 Owner: Dev4 Reviewer: Reviewer Written-by: Dev-4 Tip: `d0dbbc10004a350c740a6a4166ee95f5db50ccdd` Base: `fe6c90693f5ff32c1147d0b618041b1acaaa0d76` (`main`, `ThinkFlowLab#1503`) Tree: `e8874349f128cc7a5e0e1624571f1337e722aa7a` ## What changed The details drawer's copy export handed the clipboard the raw stored turn. `transcriptOf` (`core/src/client/details-drawer.js:21`) built its text from `stripHeader(entry.response).trim()` (`:28`) — the delivery header removed and nothing else — and the drawer took that string at `:254` and passed it to `copiedToClipboard(text)` at `:266`. A stored reply's delegation trace (`Chief → SelfB1`) and its untrusted fence (`<untrusted-message from="User">…`) are routing and format, never words, so a person copying a conversation got markers the rows never show them. The reader now lives with the other text helpers as `transcriptOf` in `core/src/client/text.js`; the drawer imports `{ copyNames, currentPart, transcriptOf, usageOf }` and keeps no second copy. `copyBodyOf(text, names)` reads a stored reply line by line: 1. drops the delivery header (`stripHeader`, until it stops moving); 2. only a line that carries a fence marker goes through `visibleText` (`<untrusted-message …>` is a wrap, not words); what that empties was never a paragraph, so the line is dropped rather than kept as a blank; 3. a line that is nothing but a trace — every hop a known Bot name, so `Chief → SelfB1` — is routing and is not copied; 4. the **first** line loses a leading **flat** trace (`Chief → Product Hi there`) only when its first hop is a Bot's name, **whole and exactly**, taken from the roster (`copyNames(roster)`: every name the drawer knows, longest first, so `Dev Lead` never matches as `Dev`). With no names — or an empty list — nothing is stripped at all; 5. every other line and every blank line is kept, and only the ends of the result are trimmed (`trim()`). A reply written in three paragraphs is copied as three paragraphs. Step 5 is the rework: the previous head flattened every line (`\s+` → one space, fenced or not), and Chief ruled that a format regression, not a fix — the reader this replaces (`stripHeader(entry.response).trim()`) kept the newlines, and D2's `exchange.js` keeps them too. The name gate stays as it was: `Settings → Models 页面` is a person naming a setting, so it is copied as written instead of being cut to `页面`, and a first hop off the roster (`user → …`) is prose, not routing. Read side only: no write path, no `previewOf` change, no `voice.js`, no `palette.js`, and `entry.prompt` keeps its `**You:**` label. `changelog.d/1406.fixed.md` is the user-visible note. ## The rebase onto `fe6c9069`, and why `0008` is hand-built The PR had been `mergeable: false` / `dirty` since `main` moved; it carried no Verdict and no review, so the rebase cost no reviewed state. The only conflicting file was `patches/ds-bot/0008-client-bundle.patch`; `core/client.js`, `core/src/client/text.js`, `core/src/client/details-drawer.js`, `docs/` and `0001`/`0012` merged cleanly. `ThinkFlowLab#1484`'s and `ThinkFlowLab#1490`'s lines are untouched: the only deletions in `details-drawer.js` and `client.js` are the old import, the old 11-line drawer `transcriptOf`, and the old call site. ### Method Main's `0008` is the base; exactly three edits were applied by hand, one per real `client.js` change: 1. `@@ -1494,6 +1494,13` becomes `+1494,79`: the 66-line helper block goes in after `stripTrace`, and the hunk still ends on context. 2. `@@ -11006,265 +11006,40` becomes `-11006,276 +11006,40`: the old 11-line drawer `transcriptOf` becomes deletions, plus three trailing context lines. **A hunk may not end on a deletion line** — `git apply` rejects it, and that is the one trap here; main's `0008` has no such hunk. 3. a new `@@ -11457,7 +11457,7` hunk at the call site: `… roster.exchangeTurns[part], copyNames(roster))`. `0002` and `0014` stay at main's bytes. `0001` (`+91 −1`) carries only the 90-line `text.js` helper; `0012` (`+27 −6`) only the drawer's new import and call site. ### Evidence - `node scripts/ds-bot-patches.mjs check` — three ok lines (quoted under *What I ran*). - Applying `0001`…`0008` onto a pristine `ds-bot/` checkout leaves `client.js` **byte-identical** to this branch's `core/client.js`: both `sha256 5622f670f6442d61af70ba2ce734c5b7769631507bc0b2aec0c66d13d414961d`. - `node core/scripts/build-client.mjs --check` — exit 0. - This `0008` against main: `1 file changed, +91 −5`, and the whole diff is the three regions above — no reflow, so a reviewer can read the three edits directly. ### Known limit: `generate` reflows `0008` `node scripts/ds-bot-patches.mjs generate` on this tree rewrites `0008` by **+1678 −1521** (3199 lines). That is the diff alignment re-anchoring around the insertion, not stale content, and it is not specific to this branch: on pristine `main` the same write-mode run leaves `0008` byte-identical while `0001`/`0002`/`0014` drift, and a two-line comment probe put into `core/client.js` alone makes `0008` churn 129 lines (67+/62−). Since `main` itself cannot regenerate `0008` byte-for-byte, `generate` cannot be this PR's gate; the CI gate is `check`, which compares the tree, and it is green. Same generator family as P-59; recorded in the backlog. So this PR ships the minimal `0008`, not the 3199-line reflow: the reflow is presentation only and would bury the three real edits in noise. ## Trade-offs, narrowed to what this actually fixes - **A copy keeps the reply's own lines.** Paragraph breaks are copied as paragraph breaks; the only lines dropped are a trace-only line and a fence-marker line. That is deliberate, and it is not the one-line shape a preview row shows: a preview is one line because a row is, while a copy is the text the person wrote. - **Only a trace whose first hop is a known Bot name is routing.** `Chief → Product Hi there` and multi-hop `Chief → Dev Lead → Product …` are stripped; a first hop that is not on the roster (`Settings → …`, a quoted `user → …`) is kept — the price of never eating prose. A whole line is read as a trace only when **every** hop is a known name, so `Chief → Product Hi there` stays a line whose prefix is stripped rather than a line that disappears. The fence-with-a-trace-inside shape is **not** handled here; it stays open on `ThinkFlowLab#1408` item ①. - **A reply whose own words spell a marker fragment is treated as a marker.** A body that literally reads `<untrusted-message …>` is read as a wrap. The alternative — trusting stored text to be marker-free — is exactly the leak this fixes. ## Red before, green after (same command) From `core/`: `node --import ./test/harness-resolve.mjs --test test/details-copy.test.js` At the old head `8bbc566c` (old reader, new tests) — 5 of 8 red; tests 1, 2 and 7 pass: ``` not ok 3 - the copy drops a flat trace whose first hop is a known Bot name + 'Chief → Product Hi there' - 'Hi there' not ok 4 - the copy drops a flat trace that sits behind a delivery header + 'Chief → Product Hi there' - 'Hi there' not ok 5 - the copy drops a flat trace with more than one hop + 'Chief → Dev Lead → Product 收到，开始' - '收到，开始' not ok 6 - the copy keeps a line whose first hop is not a Bot name, and keeps every line without names + '页面' - 'Settings → Models\n页面' not ok 8 - the copy drops a flat trace on the first line and keeps the paragraphs under it + 'second para' - 'Hi there\n\nsecond para' # tests 8 # pass 3 # fail 5 ``` `页面` and `second para` are the old reader eating the setting name and the first line; test 7 passes there only because that head flattened fenced lines, not unfenced paragraphs. At the rejected head `f7af3cc3` (same tree content: local `513319d`, `text.js` sha256 `24601d1dea966e65d62bdb6e5f93a5416c0639a4bc372f36e6351ffcc775fed4`, the one Chief and Test measured) — 4 of 8 red, the paragraph fold: ``` not ok 1 - the conversation copy shows the words of a stored line, never its trace or fence The input did not match the regular expression /\*\*Chief:\*\* plain words\nsecond line$/. Input: '**You:** typed by the owner\n\n**Chief:** B kick test 4 > hello\n\n**Chief:** another stored line\n\n**Chief:** plain words second line' not ok 6 - the copy keeps a line whose first hop is not a Bot name, and keeps every line without names + 'Settings → Models 页面' - 'Settings → Models\n页面' not ok 7 - the copy keeps the paragraph breaks a stored reply was written with + 'para one para two para three' - 'para one\n\npara two\n\npara three' not ok 8 - the copy drops a flat trace on the first line and keeps the paragraphs under it + 'Hi there second para' - 'Hi there\n\nsecond para' # tests 8 # pass 4 # fail 4 ``` At this tree: ``` ok 1 - the conversation copy shows the words of a stored line, never its trace or fence ok 2 - the details drawer copies through the shared transcript reader ok 3 - the copy drops a flat trace whose first hop is a known Bot name ok 4 - the copy drops a flat trace that sits behind a delivery header ok 5 - the copy drops a flat trace with more than one hop ok 6 - the copy keeps a line whose first hop is not a Bot name, and keeps every line without names ok 7 - the copy keeps the paragraph breaks a stored reply was written with ok 8 - the copy drops a flat trace on the first line and keeps the paragraphs under it # tests 8 # pass 8 # fail 0 ``` ## Knife With only the read-side loop reverted in place — `copyBodyOf` back to `body = COPY_FENCE.test(body) ? visibleText(body) : body.replace(/\s+/g, ' ').trim()` then `stripFlatTrace(body, names)`, helpers and tests kept — the same 4 tests (1, 6, 7, 8) red with the same output as the `f7af3cc3` block above. `core/src/client/text.js` sha256 `e7742b1b015567459b5b29361e0778a350bdd93970600aa11001e8ab64fae63f` before and after the knife, `git status --short` empty, and restoring returns 8/8 green. `core/client.js` is not read by this test — it reads the sources. ## What I ran `TZ=UTC npm run verify` at this tree — **exit 0**. (Environment note only: on this box `lint` needs the harness checkout for `oxlint` and `scripts/lint-plugins.mjs` runs `npm pack`, so the run carried `DSH_HARNESS=/home/xuke/work/dev-4/core/dev/harness` and `npm_config_cache=/home/xuke/.dsh/bot/workspace/.npm-cache`; nothing in the tree changes.) - `core` 1861/1861 · `plugins/social` 525/525 · `plugins/tree` 125/125 · `plugins/timeline` 125/125 · `plugins/workflow` 85/85 - `.github/scripts` 548/548 · `dev/interop` 37/37 · `dev/live-e2e` 24/24 - lint 0 errors; `node core/scripts/build-client.mjs --check` exit 0; `core/scripts/check-arch-docs.mjs` OK (10 changed files vs `fe6c9069`) - `node --test core/test/details-copy.test.js` 8/8 green at this tree - `node scripts/ds-bot-patches.mjs check`: ``` ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5) ok 10 patches apply cleanly in order ok core/ equals ds-bot/ + patches/ds-bot/ ``` Not run: a PASS request — that waits for the lead's word. No issue comment, no label change. ## The `0012` hunk, corrected Earlier I called the `patches/ds-bot/0012-room-membership.patch` change a pure hunk-header shift. That was wrong: `0012` also patches `core/src/client/details-drawer.js`, and this branch changes that file for real — the import line becomes `{ copyNames, currentPart, transcriptOf, usageOf }` and the 10-line local `transcriptOf` definition is gone. `0012` therefore carries a source change of its own (removed lines + the new import), plus the hunk-header move. Its room-membership logic is untouched, but the patch is a mirror of the source edit, not a context shift. ## Changed files (vs `fe6c9069`) | file | +/− | |---|---| | `changelog.d/1406.fixed.md` | +1 −0 | | `core/client.js` | +67 −12 | | `core/src/client/details-drawer.js` | +2 −14 | | `core/src/client/text.js` | +90 −0 | | `core/test/details-copy.test.js` | +102 −0 | | `docs/subsystems/core-client.md` | +1 −1 | | `patches/ds-bot/0001-core-fixes.patch` | +91 −1 | | `patches/ds-bot/0008-client-bundle.patch` | +91 −5 | | `patches/ds-bot/0012-room-membership.patch` | +27 −6 | | `patches/ds-bot/files/test/details-copy.test.js` | +102 −0 | 10 files, **+574 −39**. `0001`/`0012` are the rebase merge's generated form; `0008` is the hand-built minimal one described above. The gates that hold are `check` (tree comparison) and `build-client --check`, both green. ## Coordination with `ThinkFlowLab#1409` Same family as `ThinkFlowLab#1409` (the flat-line reader) and `ThinkFlowLab#1395` (which adds `readableLine(text, names)`). Once `ThinkFlowLab#1395` is on `main`, whichever of this PR and `ThinkFlowLab#1409` merges later rebases and switches its reader (`copyBodyOf` here, `speechOf` there) to that shared `readableLine(text, names)`; this PR keeps its own line reader and `stripFlatTrace` until then. ## Checkpoint <!-- flybot:checkpoint --> ```json {"pr":1410,"claim":"Dev4","issue":1406,"summary":"A stored reply was copied flattened: the reader sat in the drawer, dropped only the delivery header, and joined every line. It now lives in text.js and reads line by line: a trace-only line goes, the first line loses a leading trace only when its first hop is a roster Bot name, every other line and blank line is kept, and visibleText runs only on a fence-marker line. Eight tests: 5 red at 8bbc566, 4 red at f7af3cc, green here. Rebased onto fe6c906 with main's 0008 plus exactly the three client.js edits.","files":["changelog.d/1406.fixed.md","core/client.js","core/src/client/details-drawer.js","core/src/client/text.js","core/test/details-copy.test.js","docs/subsystems/core-client.md","patches/ds-bot/0001-core-fixes.patch","patches/ds-bot/0008-client-bundle.patch","patches/ds-bot/0012-room-membership.patch","patches/ds-bot/files/test/details-copy.test.js"],"checks":{"ran":"TZ=UTC npm run verify (exit 0: core 1861, social 525, tree 125, timeline 125, workflow 85, scripts 548, interop 37, live-e2e 24); build-client --check; ds-bot-patches check; arch-docs OK","result":"green","at":"d0dbbc10004a350c740a6a4166ee95f5db50ccdd"},"next":"review: the hand-built 0008's three edits; once ThinkFlowLab#1395 (readableLine) is on main, the later merge switches copyBodyOf to readableLine(text, names)"} ``` <!-- /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.
Owner: Dev
Reviewer: Reviewer
Allow choosing one model for an entire Workflow graph. The model picker appears in Settings → Workflow graphs and the chosen model is shown in the Details panel. When set, all ephemeral Bots in a run use the workflow model instead of their seated Bot's model.
Changes
workflow.modelfield stored in workflow stateworkflow/set-modelendpoint for updating the modelworkflow.modelas first priorityCheckpoint
{ "pr": 1503, "claim": "Dev", "issue": 1502, "summary": "Added per-workflow model selection. UI shows model picker in Settings → Workflow graphs and Details panel. Backend stores workflow.model and passes it to ephemeral spawn, which uses it before node.model or seated Bot model. Tests cover workflow model inheritance and default behavior when unset.", "files": [ "plugins/workflow/client.js", "plugins/workflow/src/service.js", "plugins/workflow/src/ephemeral.js", "plugins/workflow/index.js", "plugins/workflow/test/ephemeral.test.js", "changelog.d/1502-workflow-model.added.md" ], "checks": { "ran": "npm --prefix plugins/workflow test", "result": "green", "at": "06f64ff06979b816b10c8d5b92737a5a59cf1d77" }, "next": "none — ready for review" }Fixes #1502
Docs-not-needed: Workflow model is UI-only; no subsystem doc change