fix(workflow): Stop Settings Add link from submitting a pair the graph already has

changelog.d/2173-settings-link-duplicate.fixed.md

@@ -0,0 +1 @@
+**Add link** in Settings → Workflow graphs now reads the graph before it submits: a pair the graph already links is a no-op instead of a save the host refuses and discards whole, a pair given a different kind says which link is already there instead of the host's `listed twice`, and the **To** list marks the targets the chosen node already reaches ([#2173](https://github.com/ThinkFlowLab/FlyBot/issues/2173)).

docs/subsystems/workflow.md

@@ -14,6 +14,8 @@ Settings → Workflows (#1364) is this plugin's list. The page lives in the core
 
 A host call on that page that throws, rejects, or never answers ends on the page's own `.wf-err` alert instead of staying on `Reading workflows…` or looking saved: a failed read offers `Try again`, and a failed save or model change clears the `Changes are saved automatically` hint. A successful call clears the alert again.
 
+The **Add link** row applies the drawer's pair rule before it saves (#2173): a pair the graph already lists is a no-op, another kind over that same pair names the kind already there instead of submitting, and the **To** list marks the targets the chosen node already reaches. The host still refuses a duplicated pair — the form is what stops producing one.
+
 ## Owns
 
 Everything in `plugins/workflow/`: the graph rules, the run store, the matcher, the browser chat and the flowchart canvas.

plugins/workflow/client.js

@@ -91,6 +91,8 @@ window.__ModuleLoader__.load({
       Save: '保存',
       'Workflow graphs': 'Workflow 图',
       'Add link': '添加连线',
+      'This pair already has a {kind} link — replace or delete it first.': '这一对已有一条{kind}连线，请先替换或删除它。',
+      '(linked)': '（已连接）',
       'From node': '起点',
       'To node': '终点',
       'Pick a node': '选择一个节点',
@@ -658,6 +660,15 @@ window.__ModuleLoader__.load({
       return list.concat([{ from, to }])
     }
 
+    // The host keys a link by its pair alone, so a second edge over the same two Bots refuses the
+    // whole save whatever its kind (#2173). The Settings form has to read the kind that is there
+    // before it submits, and name it to the user.
+    const EDGE_KIND_LABEL = { forward: 'Link', condition: 'Condition', loop: 'Loop' }
+    const blankEdgeForm = () => ({ from: '', to: '', kind: 'forward', max: 5, until: { output: '', eq: '' } })
+    function edgeKindOf(edge) {
+      return edge.loop === true ? 'loop' : edge.when ? 'condition' : 'forward'
+    }
+
     // The Settings plane is zero-width, so world x sits on a ±WIRE_SPAN centre
     // line; the fitted drawer plane starts at 0, so it passes span 0 (#1513).
     function wirePath(from, to, span = WIRE_SPAN) {
@@ -1576,7 +1587,10 @@ window.__ModuleLoader__.load({
         })
       }
 
-      const [edgeForm, setEdgeForm] = useState({ from: '', to: '', kind: 'forward', max: 5, until: { output: '', eq: '' } })
+      const [edgeForm, setEdgeForm] = useState(blankEdgeForm)
+      // #2173: the To list says which targets the chosen From already reaches, so a repeat is
+      // visible before the form submits it.
+      const linkedTargets = new Set(edgeForm.from ? edges.filter(edge => edge.from === edgeForm.from).map(edge => edge.to) : [])
       const [toolsDraft, setToolsDraft] = useState(null)
       const toolsTimer = useRef(null)
       const toolsCommit = useRef(null)
@@ -1601,6 +1615,17 @@ window.__ModuleLoader__.load({
 
       const addEdge = () => {
         if (!edgeForm.from || !edgeForm.to) return
+        const existing = edges.find(edge => edge.from === edgeForm.from && edge.to === edgeForm.to)
+        // #2173: the drawer's withEdge rule, said out loud before the save. Repeating the pair is a
+        // no-op, because submitting it again would be the same graph the host refuses. Another
+        // kind over that pair is answered here, so the user reads that the pair is taken instead of
+        // the host's `Link A → B is listed twice` refusal, which saves nothing.
+        if (existing) {
+          const had = edgeKindOf(existing)
+          if (had === edgeForm.kind) { setEdgeForm(blankEdgeForm()); return }
+          setError(tr('This pair already has a {kind} link — replace or delete it first.', { kind: tr(EDGE_KIND_LABEL[had]) }))
+          return
+        }
         const edge = { from: edgeForm.from, to: edgeForm.to }
         if (edgeForm.kind === 'loop') {
           edge.loop = true
@@ -1610,7 +1635,7 @@ window.__ModuleLoader__.load({
           edge.when = edgeForm.until
         }
         save(nodes, edges.concat([edge]))
-        setEdgeForm({ from: '', to: '', kind: 'forward', max: 5, until: { output: '', eq: '' } })
+        setEdgeForm(blankEdgeForm())
       }
 
       return h('div', { className: 'wf-settings-canvas', 'data-flybot': 'workflow-settings' },
@@ -1743,7 +1768,8 @@ window.__ModuleLoader__.load({
                   'aria-label': tr('To node'),
                   value: edgeForm.to,
                   onChange: (event) => setEdgeForm({ ...edgeForm, to: event.target.value }),
-                }, h('option', { value: '' }, tr('Pick a node')), nodes.map(node => h('option', { key: node.id, value: node.id }, node.name)))),
+                }, h('option', { value: '' }, tr('Pick a node')), nodes.map(node => h('option', { key: node.id, value: node.id },
+                  linkedTargets.has(node.id) ? `${node.name} ${tr('(linked)')}` : node.name)))),
               edgeForm.kind === 'loop' ? h('label', { className: 'wf-field' },
                 h('span', null, tr('Max iterations')),
                 h('input', {

plugins/workflow/test/client.test.js

@@ -2750,7 +2750,7 @@
   // The hint's text is read when the drag is refused, so the host translation is put in place
   // after the drawer mounted: a stub standing in for the whole host at mount time is read by the
   // page's own reads (avatars, tool links) and hides the workflow list.
   globalThis.__flybot__ = { ...(previous ?? {}), t: (key) => (key === 'Tasks' ? '任务' : key) }
   try {
     nodeAt('r1').props.onPointerDown({ clientX: 40, clientY: 160, target: { dataset: {} } })
     canvas().props.onPointerMove({ clientX: 96, clientY: 208 })
@@ -3152,3 +3152,106 @@
   assert.ok(pairs(saved.body.edges).includes('writer>deployer'), 'the new pair is saved')
   assert.equal(alertsOf(view).length, 0, 'the save is not refused')
 })
+
+// #2173: Settings → Add link submits a pair the graph already has, and the host refuses the whole
+// save, so every other edit in that submission is lost and the page shows only "listed twice".
+// The drawer already avoids this (withEdge); the form has to decide before it saves, and the To
+// list has to say which targets are taken. The host's normalizeGraph keeps failing closed.
+function settingsLinksHost(graph, calls, { refuseDuplicates = true } = {}) {
+  return async (name, body) => {
+    calls.push({ name, body })
+    if (name === 'workflow/state') return { workflows: [graph] }
+    if (name === 'workflow/graph') {
+      if (refuseDuplicates) {
+        const seen = new Set()
+        for (const edge of body.edges) {
+          const key = `${edge.from}>${edge.to}`
+          if (seen.has(key)) return { ok: false, error: `Link ${edge.from} → ${edge.to} is listed twice` }
+          seen.add(key)
+        }
+      }
+      return { ok: true, workflows: [{ ...graph, nodes: body.nodes, edges: body.edges }] }
+    }
+    return { ok: true, workflows: [graph] }
+  }
+}
+
+async function openAddLinkForm(graph, calls) {
+  const settings = mount(client.WorkflowSettings, { call: settingsLinksHost(graph, calls), roster: { bots: [] } })
+  await settle()
+  button(settings.tree, 'Edit').props.onClick()
+  await settle()
+  const pick = (label) => findAll(settings.tree, node => node.props?.['aria-label'] === label)[0]
+  const choose = async (label, value) => {
+    pick(label).props.onChange({ target: { value } })
+    await settle()
+  }
+  return {
+    settings,
+    choose,
+    graphCalls: () => calls.filter(item => item.name === 'workflow/graph'),
+  }
+}
+
+test('#2173: adding a link the graph already has is a no-op, not a refused duplicate save', async () => {
+  await withUnhandled(async (unhandled) => {
+    const calls = []
+    const { settings, choose, graphCalls } = await openAddLinkForm(delivery(), calls)
+    await choose('From node', 'coder')
+    await choose('To node', 'reviewer')
+    button(settings.tree, 'Add link').props.onClick()
+    await settle()
+    assert.equal(graphCalls().length, 0, 'the pair the graph already has is not submitted again')
+    assert.deepEqual(alertsOf(settings), [], 'nothing is refused, because nothing was submitted')
+    const from = findAll(settings.tree, node => node.props?.['aria-label'] === 'From node')[0]
+    const to = findAll(settings.tree, node => node.props?.['aria-label'] === 'To node')[0]
+    assert.equal(from.props.value, '', 'the form clears as it does today')
+    assert.equal(to.props.value, '')
+    assert.deepEqual(unhandled, [])
+    settings.unmount()
+  })
+})
+
+test('#2173: a linked pair given another kind names the link already there instead of submitting', async () => {
+  const calls = []
+  const { settings, choose, graphCalls } = await openAddLinkForm(delivery(), calls)
+  await choose('Kind', 'condition')
+  await choose('From node', 'coder')
+  await choose('To node', 'reviewer')
+  await choose('when', 'verdict')
+  button(settings.tree, 'Add link').props.onClick()
+  await settle()
+  assert.equal(graphCalls().length, 0, 'the conflicting pair is not submitted blind')
+  const [alert] = alertsOf(settings)
+  assert.ok(alert, 'the user is told why nothing happened')
+  assert.match(alert, /Link/, 'the banner names the kind that is already there')
+  assert.match(alert, /replace|delete/i, 'and says what to do about it')
+  assert.equal(/listed twice/.test(alert), false, "the host's technical refusal is not what the user reads")
+  settings.unmount()
+})
+
+test('#2173: the To list marks a target the From node already reaches', async () => {
+  const calls = []
+  const { settings, choose } = await openAddLinkForm(delivery(), calls)
+  await choose('From node', 'coder')
+  const options = findAll(settings.tree, node => node.type === 'option').map(textOf)
+  assert.ok(options.includes('Reviewer (linked)'), 'a target coder already reaches is marked')
+  assert.ok(options.includes('Tester (linked)'), 'so is the other one')
+  assert.ok(options.includes('Deployer'), 'a target coder does not reach stays plain')
+  settings.unmount()
+})
+
+test('#2173: a pair the graph does not have still saves and shows', async () => {
+  const calls = []
+  const { settings, choose, graphCalls } = await openAddLinkForm(delivery(), calls)
+  await choose('From node', 'writer')
+  await choose('To node', 'deployer')
+  button(settings.tree, 'Add link').props.onClick()
+  await settle()
+  const saved = graphCalls().at(-1)
+  assert.ok(saved, 'the new pair is submitted')
+  const keys = saved.body.edges.map(edge => `${edge.from}>${edge.to}`)
+  assert.equal(keys.filter(key => key === 'writer>deployer').length, 1, 'exactly one edge')
+  assert.deepEqual(alertsOf(settings), [], 'and nothing is refused')
+  settings.unmount()
+})