diff --git a/changelog.d/1520.fixed.md b/changelog.d/1520.fixed.md new file mode 100644 index 000000000..46d05c737 --- /dev/null +++ b/changelog.d/1520.fixed.md @@ -0,0 +1 @@ +The node inspector's **Tools** field now says what it does: it names the connectors the run Bot may use (`github` → `mcp__github__*`, `composio` → the Composio connector), file and shell access is always a normal Bot's full access, and a node that names no connector gets none, in English and in Chinese. Its placeholder names `github, composio` instead of advertising `bash`, `read` and `edit` as if they granted something. Typing a second connector one character at a time now keeps it: the field holds a local draft and saves on blur or after a short pause, where before every keystroke saved a list the host re-normalized, so `github, composio` typed by hand became the single useless token `githubcomposio` and the connector was silently refused. ([#1520](https://github.com/ThinkFlowLab/FlyBot/issues/1520)) diff --git a/plugins/workflow/client.js b/plugins/workflow/client.js index db49166c1..1d3fa976d 100644 --- a/plugins/workflow/client.js +++ b/plugins/workflow/client.js @@ -182,6 +182,11 @@ window.__ModuleLoader__.load({ // plane; only the drawer fits every node and edge to the panel width (#1513). const DRAWER_FLOW_W = 364 const FIT_PAD = 12 + // The Tools field saves a list, and the host normalizes that list on every save + // (cleanTools drops a trailing empty token). Committing per keystroke therefore + // ate the separator the next name needed, so the field keeps its text while the + // user types and commits it on blur, or after this pause (#1520). + const TOOLS_COMMIT_MS = 600 function workflowNodeIds(workflow) { return new Set((workflow?.nodes ?? []).map(node => node.id)) @@ -935,6 +940,7 @@ window.__ModuleLoader__.load({ const nodes = current?.nodes ?? [] const edges = current?.edges ?? [] const selected = nodes.find(node => node.id === selectedNode) + const toolsKey = current && selected ? `${current.id}:${selected.id}` : '' const save = (nodes, edges) => { if (!current) return @@ -949,6 +955,27 @@ window.__ModuleLoader__.load({ } const [edgeForm, setEdgeForm] = useState({ from: '', to: '', kind: 'forward', max: 5, until: { output: '', eq: '' } }) + const [toolsDraft, setToolsDraft] = useState(null) + const toolsTimer = useRef(null) + const toolsCommit = useRef(null) + + useEffect(() => () => { + if (toolsTimer.current) clearTimeout(toolsTimer.current) + }, []) + + const commitTools = (nodeId, text) => { + const next = text.split(',').map(item => item.trim()).filter(Boolean) + const saved = nodes.find(node => node.id === nodeId)?.tools ?? [] + if (next.length === saved.length && next.every((item, index) => item === saved[index])) return + updateNode(nodeId, { tools: next }) + } + + const flushTools = () => { + if (toolsTimer.current) { clearTimeout(toolsTimer.current); toolsTimer.current = null } + const pending = toolsCommit.current + toolsCommit.current = null + if (pending) pending() + } const addEdge = () => { if (!edgeForm.from || !edgeForm.to) return @@ -1036,13 +1063,29 @@ window.__ModuleLoader__.load({ value: selected.role || '', onChange: (event) => updateNode(selected.id, { role: event.target.value }), })), - h('label', { className: 'wf-field' }, + h('label', { className: 'wf-field', 'data-flybot': 'node-tools' }, h('span', null, tr('Tools')), h('input', { - value: (selected.tools ?? []).join(', '), - placeholder: 'github, bash, read, edit...', - onChange: (event) => updateNode(selected.id, { tools: event.target.value.split(',').map(s => s.trim()).filter(Boolean) }), - })))) : null, + 'aria-label': tr('Tools'), + value: toolsDraft && toolsDraft.key === toolsKey ? toolsDraft.text : (selected.tools ?? []).join(', '), + placeholder: 'github, composio', + onChange: (event) => { + const text = event.target.value + setToolsDraft({ key: toolsKey, text }) + toolsCommit.current = () => commitTools(selected.id, text) + if (toolsTimer.current) clearTimeout(toolsTimer.current) + toolsTimer.current = setTimeout(() => { + toolsTimer.current = null + const pending = toolsCommit.current + toolsCommit.current = null + pending?.() + }, TOOLS_COMMIT_MS) + }, + onBlur: () => { flushTools(); setToolsDraft(draft => (draft && draft.key === toolsKey ? null : draft)) }, + }), + h('div', { className: 'wf-tools-help', 'data-flybot': 'node-tools-note' }, + h('span', null, '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.'), + h('span', { lang: 'zh-CN' }, '这里填这个节点 Bot 能用的连接器:github 对应 mcp__github__*,composio 对应 Composio 连接器。文件与 shell 权限始终与普通 Bot 相同(完全放开);不填连接器时这个节点一个都拿不到。'))))) : null, editing && nodes.length >= 2 ? h('div', { className: 'wf-edge-form' }, h('h3', null, tr('Add link')), h('div', { className: 'wf-edge-fields' }, @@ -1629,6 +1672,7 @@ window.__ModuleLoader__.load({ .wf-modal-body{flex:1;overflow-y:auto;padding:12px 20px;min-height:0} .wf-modal-footer{padding:12px 20px;border-top:1px solid var(--bt-line,rgba(20,20,20,.08));display:flex;justify-content:flex-end;gap:8px;flex:none;background:var(--bt-main,#fcfcfc)} .wf-hint{font-size:12px;color:var(--bt-ink-2,rgba(20,20,20,.6));margin:0} +.wf-tools-help{display:flex;flex-direction:column;gap:2px;font-size:11px;line-height:1.45;color:var(--bt-ink-3,#9a9a9a)} .wf-step-h{font-size:12px;color:var(--bt-ink-2,rgba(20,20,20,.6));margin:14px 0 8px;display:flex;gap:6px;align-items:center} .wf-step-h b{width:18px;height:18px;border-radius:50%;background:var(--bt-user,#070707);color:var(--bt-user-ink,#fcfcfc);display:grid;place-items:center;font-size:11px} .wf-cards{display:grid;grid-template-columns:repeat(3,1fr);gap:8px} diff --git a/plugins/workflow/test/client.test.js b/plugins/workflow/test/client.test.js index 2281f335f..d5449a941 100644 --- a/plugins/workflow/test/client.test.js +++ b/plugins/workflow/test/client.test.js @@ -3,6 +3,7 @@ import assert from 'node:assert/strict' import { readFileSync } from 'node:fs' import { test } from 'node:test' +import { cleanTools } from '../src/graph.js' // `rectWidth` is what a host element reports from getBoundingClientRect, so a // component that measures its own container can be driven from a plain object. @@ -1147,6 +1148,116 @@ test('a failed resume from the run strip is reported', async (t) => { }) }) +// #1520: the Tools field selects connectors, and nothing on screen said so. The +// note is read in one render in both languages, so the explanation never depends +// on the host's locale detection. +const toolsField = view => findAll(view.tree, node => node.type === 'label' && textOf(node.children?.[0] ?? '') === 'Tools')[0] +const toolsInput = view => toolsField(view)?.children?.find(child => child?.type === 'input') + +test('the Tools field says it names connectors and that file and shell access is always full', async () => { + const graph = delivery() + const call = async (name) => { + if (name === 'workflow/state') return { workflows: [graph] } + return { ok: true, workflows: [graph] } + } + const settings = mount(client.WorkflowSettings, { call, roster: { bots: [] } }) + await settle() + button(settings.tree, 'Edit').props.onClick() + await settle() + findAll(settings.tree, node => node.props?.['data-node'] === 'coder')[0].props.onClick() + await settle() + + const field = toolsField(settings) + assert.ok(field, 'the inspector renders a Tools field') + const note = field.children.find(child => child?.props?.['data-flybot'] === 'node-tools-note') + assert.ok(note, 'the Tools field explains what it names') + const noteIndex = field.children.indexOf(note) + const inputIndex = field.children.findIndex(child => child?.type === 'input') + assert.ok(noteIndex > inputIndex, 'the note sits under the input, not above it') + const text = textOf(note) + assert.match(text, /Names the connectors this run Bot may use/) + assert.match(text, /github grants mcp__github__\*/) + assert.match(text, /composio grants the Composio connector/) + assert.match(text, /File and shell access is always full, like a normal Bot/) + assert.match(text, /a node that names no connector gets none/) + assert.match(text, /连接器/, 'the note reads in Chinese as well as English') + assert.match(text, /普通 Bot/) + + const input = field.children[inputIndex] + assert.equal(input.props.placeholder, 'github, composio', 'the placeholder names two connectors and nothing else') + settings.unmount() +}) + +// #1520: the value was rebuilt from the saved list on every keystroke, and the host +// drops a trailing empty token, so a separator typed between two names did not +// survive the round trip: "github, composio" typed one character at a time saved as +// ["githubcomposio"] and the connector was silently refused. The field keeps a local +// draft and commits it on blur, or after a short pause. +async function toolsInspector() { + const graph = delivery() + graph.nodes = graph.nodes.map(node => node.id === 'coder' ? { ...node, tools: [] } : node) + const calls = [] + const call = async (name, body) => { + calls.push({ name, body }) + if (name === 'workflow/state') return { workflows: [graph] } + if (name === 'workflow/graph') { + // The host normalizes the list on every save, exactly as src/graph.js does. + graph.nodes = body.nodes.map(node => ({ ...node, tools: cleanTools(node.tools) })) + return { ok: true, workflows: [graph] } + } + return { ok: true, workflows: [graph] } + } + const view = mount(client.WorkflowSettings, { call, roster: { bots: [] } }) + await settle() + button(view.tree, 'Edit').props.onClick() + await settle() + findAll(view.tree, node => node.props?.['data-node'] === 'coder')[0].props.onClick() + await settle() + const saved = () => calls.filter(item => item.name === 'workflow/graph').at(-1) + ?.body.nodes.find(node => node.id === 'coder')?.tools + return { view, calls, field: () => toolsInput(view), saved } +} + +test('the Tools field takes a second connector typed one character at a time', async () => { + const typedView = await toolsInspector() + assert.ok(typedView.field(), 'the inspector renders the Tools input') + let text = '' + for (const char of 'github, composio') { + text += char + typedView.field().props.onChange({ target: { value: text } }) + await settle() + assert.equal(typedView.field().props.value, text, 'the draft keeps every character just typed') + } + assert.equal(typedView.calls.some(item => item.name === 'workflow/graph'), false, 'no save round trip while the user types') + typedView.field().props.onBlur() + await settle() + assert.deepEqual(typedView.saved(), ['github', 'composio'], 'typing one character at a time keeps both connectors') + assert.equal(typedView.field().props.value, 'github, composio', 'the field shows the saved list after the commit') + typedView.view.unmount() + + const pastedView = await toolsInspector() + pastedView.field().props.onChange({ target: { value: 'github, composio' } }) + await settle() + pastedView.field().props.onBlur() + await settle() + assert.deepEqual(pastedView.saved(), ['github', 'composio'], 'pasting the whole string saves the same list') + pastedView.view.unmount() + + const pausedView = await toolsInspector() + pausedView.field().props.onChange({ target: { value: 'github, ' } }) + await settle() + assert.equal(pausedView.saved(), undefined, 'the field has not saved yet') + await new Promise(resolve => setTimeout(resolve, 800)) + await settle() + assert.deepEqual(pausedView.saved(), ['github'], 'a short pause commits the list without a blur') + pausedView.field().props.onChange({ target: { value: 'github, composio' } }) + await settle() + pausedView.field().props.onBlur() + await settle() + assert.deepEqual(pausedView.saved(), ['github', 'composio'], 'a pause in the middle does not eat the separator') + pausedView.view.unmount() +}) + test('a stage control that never answers settles as an error', async () => { const value = await client.settledCall(async () => new Promise(() => {}), 'workflow/pause', { workflowId: 'delivery' }, 'The run did not answer', 5) assert.deepEqual(value, { ok: false, error: 'The run did not answer' })