Log a re-matched card on the source workflow

changelog.d/1668-reroute-source-log.fixed.md

@@ -0,0 +1 @@
+A workflow card re-matched into another workflow now leaves one `Task re-matched · <task> → <destination>` line on the source graph's own log, so the graph that lost the card says where it went instead of the card silently vanishing ([#1668](https://github.com/ThinkFlowLab/FlyBot/issues/1668)).

docs/subsystems/workflow.md

@@ -62,7 +62,7 @@ Everything in `plugins/workflow/`: the graph rules, the run store, the matcher,
 5. A node declares up to eight inputs and eight outputs. A required input is an output of the same name on an ancestor, or `task`, `title`, or `brief` on a root. A start with no provider is refused, and Bots spawned for that attempt are released. On a Bot-backed run a successor stays queued until its required inputs are present. If every forward predecessor is done, failed or skipped and an input is still missing, that node fails and the run stops. The dispatch text carries `Input <name>: <value>`. The Bot reports `OUTPUT <name>: <value>`; a single declared output may be the reply body. A declared output a gate reads is required (§3): when a `when`/`else` or `until` edge reads it and the step's reply does not produce it, the step fails with `failure.text` `Missing OUTPUT <names>` instead of settling, so the gate never reads silence as "not this branch" and the run cannot close `done` on it; a delivered value that does not equal `eq` still completes the step and skips the untaken branch (#1773). Nodes start in edge order.
 6. Each node is a real ephemeral Bot for that run, then it is removed. A permanent sidebar Bot (`botId`) is optional and is not required for the step to run. **File and shell rights are the same as a normal sidebar Bot's:** every node Bot gets the `danger-full-access` preset, and `EPHEMERAL_DENIED_TOOLS` still refuses the team-admin tools at the guard and approval layers (#1466, 29 names). **`node.tools` selects connectors only:** `spawn` passes the node's cleaned list to `fb.bot.create`, the host records it as `stepTools`, and `connector-access.js` allows `mcp__github__*` for `github` and `mcp__composio__*` for `composio` — a connector the node did not name is refused, and a node that names none reaches none. The list is also stored in `fields.tools` for the UI. A hidden run Bot cannot show an approval card, so the approval seam answers the same way the guard does. The host stores this on the run Bot as `stepTools` and `permission`.
 7. A loop edge (`loop: true`, `max` from 1 to 8, default 3, optional `until: { output, eq }`) is the only cycle for success paths. The body runs again until `until` matches or `max` runs have finished, and the forward edges wait until then. Restarting the body drops the branch choice made inside it and queues a sibling that the previous pass skipped, so the next pass can take the other side. A `when: { output, eq }` edge is taken only when that output equals `eq`. `when: { else: true }` is taken only when no other `when` matched. A `when` or `until` that is present but cannot be honoured — a blank output name, or one outside `/^[a-z][a-z0-9-]{0,40}$/` such as `result_ok` — refuses the save with `A condition link needs an output name like "ready"` (or the loop's own line) instead of being dropped: before #2155 the refused `when` was indistinguishable from an absent one and the edge fired unconditionally, which is the opposite of what the author asked. An absent `when`/`until` is still a plain link, no early stop. Settings → Workflow graphs keeps **Add link** disabled for a Condition with a blank output name, and the refusal shows in the page's `.wf-err` alert. A node whose forward edges are all closed is `skipped` and does not block a join. A run cannot spin: every loop has a max. An `onFail` edge (`onFail: true`, `max` recoveries from 1 to 8, default 3) provides failure recovery: when a node fails, if an `onFail` edge points from that node back to an upstream target and recoveries remain (tracked in `failAttempts` per edge), the workflow resets every node from the recovery target through the failing node (the target to `running`, the rest to `queued`) and re-runs them in order. After `max` recoveries are used, the node fails permanently and the run stops. An `onFail` edge does not interfere with `failOnce` retry (which restarts the same node once) or success-path `loop` edges. Settings → Workflow graphs can add a link, a condition, or a loop; a failure recovery edge is added through `define_workflow` (`onFail: true`) and has no Settings control yet.
-8. Auto-assign considers an `open` card only. Every saved board write reaches the hook, including a card a Bot records for itself through `track_tasks`, so a `workflow:<id>` on the card a Bot wrote starts that run (#1462). A saved Bot claim beats a workflow trigger. Otherwise first hit wins: a `workflow:` token, then `workflowId` / `workflow`, then an assignee that names a workflow, then a `bot:` token, then an assignee that names a local Bot, then the longest trigger in the title, note, criteria or tags — an ASCII trigger must match a whole word (`notice` does not match `noticeable`, `pr` does not match `Improve`), while a non-ASCII trigger matches inside a longer text because CJK has no word boundary. A miss returns no route. A second ingest of the same task and target is a no-op, and a live run for that task id is not restarted. A stored claim yields only to the card's own named target — a `workflow:`/`bot:` token, `workflowId`/`workflow`, or an assignee naming a workflow or local Bot — and only when that target is live and different: the claim and its route go, the card is matched again, and exactly one of each remains. A trigger, or a card that names nothing, never invalidates a claim; a claim naming a workflow or Bot that no longer exists keeps today's behaviour, because this invalidates on named intent and does not guess. `heldTasks`/`heldTaskCount` on `workflow/state` are that claim map read back per workflow, and `rematch: true` is the person's explicit drop for one task (#1647).
+8. Auto-assign considers an `open` card only. Every saved board write reaches the hook, including a card a Bot records for itself through `track_tasks`, so a `workflow:<id>` on the card a Bot wrote starts that run (#1462). A saved Bot claim beats a workflow trigger. Otherwise first hit wins: a `workflow:` token, then `workflowId` / `workflow`, then an assignee that names a workflow, then a `bot:` token, then an assignee that names a local Bot, then the longest trigger in the title, note, criteria or tags — an ASCII trigger must match a whole word (`notice` does not match `noticeable`, `pr` does not match `Improve`), while a non-ASCII trigger matches inside a longer text because CJK has no word boundary. A miss returns no route. A second ingest of the same task and target is a no-op, and a live run for that task id is not restarted. A stored claim yields only to the card's own named target — a `workflow:`/`bot:` token, `workflowId`/`workflow`, or an assignee naming a workflow or local Bot — and only when that target is live and different: the claim and its route go, the card is matched again, and exactly one of each remains. A trigger, or a card that names nothing, never invalidates a claim; a claim naming a workflow or Bot that no longer exists keeps today's behaviour, because this invalidates on named intent and does not guess. `heldTasks`/`heldTaskCount` on `workflow/state` are that claim map read back per workflow, and `rematch: true` is the person's explicit drop for one task (#1647). A claim that yields leaves one line on the source workflow's log — `Task re-matched · <task> → <destination>` — in the same store write that attaches the new target, so the graph that lost the card says where it went (#1668).
 9. The browser draws text only. Names and messages are React text children. The unit renderer asserts a hostile name is not markup.
 10. `enabled: false` registers no endpoint, tool or listener and writes nothing.
 11. A sidebar click opens the stage or a dialog and leaves the Workflows section in place. The stage and the dialogs portal onto `document.body`. When that container is missing they stay in the section tree; the click does not throw.

plugins/workflow/src/service.js

@@ -246,13 +246,13 @@
   }
 
   function clearRunTimers(runId) {
     for (const [key, entry] of [...timers]) {
       if (entry.runId === runId) clearTimerAt(key)
     }
     for (const key of [...warned]) {
       if (key.endsWith(`:${runId}`)) warned.delete(key)
     }
     for (const key of [...handOffTries.keys()]) {
       if (key.startsWith(`${runId}:`)) handOffTries.delete(key)
     }
   }
@@ -289,7 +289,7 @@
     // keep its deadline: without it the run holds its lane slot with nothing left
     // to free it until the owner presses Stop again (P-560).
     const stopRetry = run.status === 'running' && run.paused === true && run.releasePending === true
     for (const [key, entry] of [...timers]) {
       if (entry.runId !== run.id) continue
       if (run.status !== 'running' || (run.paused === true && !stopRetry) || !running.has(entry.nodeId)) clearTimerAt(key)
     }
@@ -562,7 +562,7 @@
       const missing = unmetRequired(item, id, run.produced, run.task)
       if (missing.length === 0) { started.push(id); continue }
       const preds = item.edges.filter(edge => edge.to === id && edge.loop !== true).map(edge => edge.from)
       const stuck = preds.length === 0 || preds.every(pred => status[pred] === 'done' || status[pred] === 'failed' || status[pred] === 'skipped')
       if (stuck) {
         status[id] = 'failed'
         blocked.push({ id, missing })
@@ -1812,6 +1812,10 @@
           return { ok: true, matched: false }
         }
       }
+      // #1668: the side that lost the card keeps one line saying where it went.
+      // The push below rides the write that attaches the new target, so the
+      // target's own lines stay exactly as they are and nothing is written twice.
+      const from = held?.kind === 'workflow' && held.id !== decision.id ? workflow(held.id) : undefined
       const key = `${decision.kind}:${decision.id}`
       const prev = state.routes[taskId]
       if (prev?.key === key) return { ok: true, matched: true, already: true, decision }
@@ -1830,6 +1834,17 @@
       const assigned = this.assign({ workflowId: decision.id, taskId, title, ...(typeof task.brief === 'string' && task.brief !== '' ? { brief: task.brief } : {}) })
       const finish = (value) => {
         if (value?.ok === false) return value
+        if (from) {
+          const label = summarizeTask(task)
+          const to = workflow(decision.id)
+          const name = to?.name ?? decision.id
+          push(from.id, [{
+            kind: 'system',
+            key: 'Task re-matched · {task} → {to}',
+            params: { task: label, to: name },
+            text: `任务已改投 · ${label} → ${name}`,
+          }], now())
+        }
         rememberClaim(taskId, decision)
         write()
         return { ...present(), matched: true, already: false, decision }

plugins/workflow/test/claim-reroute-log.test.js

@@ -0,0 +1,208 @@
+// #1668: when a card is re-matched into another workflow (#1647), the target
+// workflow got its "Task assigned" / "Started" lines and the source workflow got
+// nothing — the held card vanished from a graph with no explanation.
+//
+// The re-route branch now pushes one system line on the source side, in the same
+// store write that attaches the new target, so the graph that lost the card says
+// where it went. The target's own lines are untouched, and an ingest that changes
+// nothing still writes nothing.
+import assert from 'node:assert/strict'
+import { mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { test } from 'node:test'
+import { createWorkflowService } from '../src/service.js'
+
+const CLOCK = 1_700_000_000_000
+const roster = [
+  { id: 'main', name: 'Chief', role: 'lead', isMain: true, remote: false },
+]
+
+function readStore(dir) {
+  return JSON.parse(readFileSync(join(dir, 'workflows.json'), 'utf8'))
+}
+
+function serviceAt(dir) {
+  let seq = 0
+  return createWorkflowService({
+    dir,
+    persist: true,
+    now: () => CLOCK,
+    bots: () => roster,
+    spawn: async () => `eph-${++seq}`,
+    release: async () => {},
+    dispatch: async () => ({ ok: true }),
+    // Step deadlines are timers; they are not part of what these cases assert.
+    setTimer: () => 0,
+    clearTimer: () => {},
+  })
+}
+
+function named(svc, name, extra = {}) {
+  const created = svc.create({
+    name,
+    nodes: [{ id: 'only', name: 'Only', prompt: 'Do it.' }],
+    edges: [],
+    ...extra,
+  })
+  assert.equal(created.ok, true, created.error)
+  return created.workflows.find(item => item.name === name)
+}
+
+// `state()` hands back the live `state.logs[...]` array, so every read is a copy:
+// a later push must not change a snapshot this file is comparing against.
+const messagesOf = (svc, name) => svc.state().workflows.find(item => item.name === name).messages.slice()
+
+// A reroute: Alpha holds the card, then the card names Beta.
+async function rerouted(dir) {
+  const svc = serviceAt(dir)
+  const alpha = named(svc, 'Alpha')
+  const beta = named(svc, 'Beta')
+  const first = await svc.ingest({ id: 't-1', status: 'open', title: 'Ship it', workflowId: alpha.id })
+  assert.deepEqual(first.decision, { kind: 'workflow', id: alpha.id })
+  assert.deepEqual(first.already, false)
+  return { svc, alpha, beta }
+}
+
+// The read-only shape of a log entry: what the stage draws, minus the store's
+// ids and the names two sibling workflows spell differently.
+function shape(entries, task, workflow) {
+  return JSON.parse(JSON.stringify(entries).replaceAll(task, '<task>').replaceAll(workflow, '<workflow>'))
+    .map(entry => {
+      const copy = { ...entry }
+      delete copy.id
+      return copy
+    })
+}
+
+test('#1668: a reroute leaves exactly one line on the source workflow, naming task and destination', async () => {
+  const dir = mkdtempSync(join(tmpdir(), 'wf-reroute-log-'))
+  try {
+    const { svc, alpha, beta } = await rerouted(dir)
+    const sourceBefore = messagesOf(svc, 'Alpha')
+
+    const moved = await svc.ingest({ id: 't-1', status: 'open', title: 'Ship it', workflowId: beta.id })
+    assert.deepEqual({ matched: moved.matched, already: moved.already }, { matched: true, already: false })
+
+    const sourceAfter = messagesOf(svc, 'Alpha')
+    assert.equal(sourceAfter.length, sourceBefore.length + 1, 'the source log gained exactly one entry')
+    assert.deepEqual(sourceAfter.slice(0, sourceBefore.length), sourceBefore, 'the earlier source entries are untouched')
+    const line = sourceAfter.at(-1)
+    assert.equal(line.kind, 'system')
+    assert.equal(line.key, 'Task re-matched · {task} → {to}')
+    assert.equal(line.params.task, 't-1 · Ship it', 'the line names the task')
+    assert.equal(line.params.to, 'Beta', 'the line names the destination workflow')
+    assert.equal(line.text, '任务已改投 · t-1 · Ship it → Beta')
+    assert.equal(line.at, CLOCK)
+
+    // The line is stored, not just painted from memory: a reload sees it too.
+    const reloaded = serviceAt(dir)
+    const onDisk = messagesOf(reloaded, 'Alpha')
+    assert.equal(onDisk.length, sourceBefore.length + 1)
+    assert.equal(onDisk.at(-1).key, 'Task re-matched · {task} → {to}')
+    assert.equal(readStore(dir).logs[alpha.id].at(-1).params.to, 'Beta')
+    assert.equal(messagesOf(reloaded, 'Beta').length, 4, 'the target got its own assignment lines, nothing more')
+  } finally {
+    rmSync(dir, { recursive: true, force: true })
+  }
+})
+
+test('#1668: the target side keeps the entries it wrote before, entry for entry', async () => {
+  const dir = mkdtempSync(join(tmpdir(), 'wf-reroute-target-'))
+  try {
+    const { svc, beta } = await rerouted(dir)
+    const before = messagesOf(svc, 'Beta')
+    await svc.ingest({ id: 't-1', status: 'open', title: 'Ship it', workflowId: beta.id })
+
+    // A first-time assignment into a third lane is the control: main wrote the
+    // same three entries there for any fresh card.
+    const gamma = named(svc, 'Gamma')
+    await svc.ingest({ id: 't-2', status: 'open', title: 'Ship elsewhere', workflowId: gamma.id })
+
+    const moved = messagesOf(svc, 'Beta').slice(before.length)
+    const control = messagesOf(svc, 'Gamma').slice(before.length)
+    assert.deepEqual(
+      shape(moved, 't-1 · Ship it', 'Beta'),
+      shape(control, 't-2 · Ship elsewhere', 'Gamma'),
+      'the reroute writes the same entries a fresh assignment writes, and nothing else',
+    )
+    assert.equal(moved.some(entry => String(entry.key).includes('re-matched')), false, 'the source line never leaks onto the target')
+    assert.deepEqual(messagesOf(svc, 'Beta').slice(0, before.length), before, 'the target log before the reroute is unchanged')
+  } finally {
+    rmSync(dir, { recursive: true, force: true })
+  }
+})
+
+test('#1668: a claim that simply matches appends nothing and writes no bytes', async () => {
+  const dir = mkdtempSync(join(tmpdir(), 'wf-reroute-noop-'))
+  try {
+    const { svc, alpha } = await rerouted(dir)
+    const sourceBefore = messagesOf(svc, 'Alpha')
+    const targetBefore = messagesOf(svc, 'Beta')
+    const bytes = readFileSync(join(dir, 'workflows.json'))
+    const mtimeMs = statSync(join(dir, 'workflows.json')).mtimeMs
+
+    const again = await svc.ingest({ id: 't-1', status: 'open', title: 'Ship it', workflowId: alpha.id })
+    assert.equal(again.already, true)
+
+    assert.deepEqual(messagesOf(svc, 'Alpha'), sourceBefore, 'the source log did not move')
+    assert.deepEqual(messagesOf(svc, 'Beta'), targetBefore, 'and neither did the target')
+    assert.deepEqual(readFileSync(join(dir, 'workflows.json')), bytes, 'the store is byte-identical')
+    assert.equal(statSync(join(dir, 'workflows.json')).mtimeMs, mtimeMs, 'the store was not rewritten')
+  } finally {
+    rmSync(dir, { recursive: true, force: true })
+  }
+})
+
+test('#1668: already:true appends 0 entries, even on a reroute whose target already runs the card', async () => {
+  const dir = mkdtempSync(join(tmpdir(), 'wf-reroute-live-'))
+  try {
+    const svc = serviceAt(dir)
+    const alpha = named(svc, 'Alpha')
+    const beta = named(svc, 'Beta')
+
+    // Beta already runs t-1 with no claim; a first ingest hands the claim to
+    // Alpha; naming Beta again is a reroute, but there is no run to start.
+    await svc.assign({ workflowId: beta.id, taskId: 't-1', title: 'Ship it' })
+    await svc.ingest({ id: 't-1', status: 'open', title: 'Ship it', workflowId: alpha.id })
+
+    const sourceBefore = messagesOf(svc, 'Alpha')
+    const targetBefore = messagesOf(svc, 'Beta')
+    const already = await svc.ingest({ id: 't-1', status: 'open', title: 'Ship it', workflowId: beta.id })
+    assert.equal(already.already, true, 'the live run is not restarted')
+    assert.deepEqual(already.decision, { kind: 'workflow', id: beta.id })
+
+    assert.deepEqual(messagesOf(svc, 'Alpha'), sourceBefore, 'an already:true ingest writes no source line')
+    assert.deepEqual(messagesOf(svc, 'Beta'), targetBefore, 'and no target line')
+    assert.deepEqual(readStore(dir).claims['t-1'], { kind: 'workflow', id: beta.id }, 'the claim still moved')
+  } finally {
+    rmSync(dir, { recursive: true, force: true })
+  }
+})
+
+test('#1668: MAX_MESSAGES keeps the source log at 200 and keeps the newest line', async () => {
+  const dir = mkdtempSync(join(tmpdir(), 'wf-reroute-cap-'))
+  try {
+    const svc = serviceAt(dir)
+    const alpha = named(svc, 'Alpha')
+    const beta = named(svc, 'Beta')
+
+    // A store that already holds more than the cap: 220 seeded lines.
+    const store = readStore(dir)
+    store.logs[alpha.id] = Array.from({ length: 220 }, (_, i) => ({ id: `seed-${i}`, kind: 'system', text: `seed ${i}` }))
+    writeFileSync(join(dir, 'workflows.json'), JSON.stringify(store))
+    const reloaded = serviceAt(dir)
+
+    await reloaded.ingest({ id: 't-1', status: 'open', title: 'Ship it', workflowId: alpha.id })
+    await reloaded.ingest({ id: 't-1', status: 'open', title: 'Ship it', workflowId: beta.id })
+
+    const source = messagesOf(reloaded, 'Alpha')
+    assert.equal(source.length, 200, 'the source log is capped, never 201')
+    assert.equal(source.at(-1).key, 'Task re-matched · {task} → {to}', 'the newest line is the one kept')
+    assert.equal(source[0].id, 'seed-24', 'the oldest entries were dropped as before')
+    // The cap is one slice on the same array; the target still fits under it.
+    assert.equal(messagesOf(reloaded, 'Beta').length, 4)
+  } finally {
+    rmSync(dir, { recursive: true, force: true })
+  }
+})