From fbd5f63e19eff24efe3501a4b4d0500bf067badc Mon Sep 17 00:00:00 2001 From: HugoXK Date: Thu, 5 Nov 2026 02:44:02 +0000 Subject: [PATCH] workflow: let the card decide its brief when a re-match hands it on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The re-match path omitted `brief` when the card held none, and `assign` reads a missing `brief` as "keep the lane's cached facts" — so a card the author cleared came back with the old text (#2486). Pass the card's own state instead: its brief when it holds one, an explicit `''` when it does not, which is the clear `#2475` defined. Docs-not-needed: docs/subsystems/workflow.md already states this contract — "A call that passes `brief` is the latest word — `brief: ''` clears the card's facts". This change makes the ingest path follow it and adds no new contract. --- .../2486-ingest-brief-authority.fixed.md | 1 + plugins/workflow/src/service.js | 6 +- .../test/ingest-brief-authority.test.js | 97 +++++++++++++++++++ 3 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 changelog.d/2486-ingest-brief-authority.fixed.md create mode 100644 plugins/workflow/test/ingest-brief-authority.test.js diff --git a/changelog.d/2486-ingest-brief-authority.fixed.md b/changelog.d/2486-ingest-brief-authority.fixed.md new file mode 100644 index 000000000..a256c0665 --- /dev/null +++ b/changelog.d/2486-ingest-brief-authority.fixed.md @@ -0,0 +1 @@ +A workflow card whose facts were cleared no longer brings them back when it is re-matched into another lane: the re-match path now passes the card's current brief — its text when it holds facts, an explicit empty one when it does not — so the lane cache can no longer resurrect instructions the author removed ([#2486](https://github.com/ThinkFlowLab/FlyBot/issues/2486)). diff --git a/plugins/workflow/src/service.js b/plugins/workflow/src/service.js index e6915a69c..6ff241c75 100644 --- a/plugins/workflow/src/service.js +++ b/plugins/workflow/src/service.js @@ -1887,7 +1887,11 @@ export function createWorkflowService({ } const title = cleanLine(task.title, 300) if (!title) return { ok: false, error: 'A task needs a title' } - const assigned = this.assign({ workflowId: decision.id, taskId, title, ...(typeof task.brief === 'string' && task.brief !== '' ? { brief: task.brief } : {}) }) + // #2486: the card is the authority for its own facts. Omitting `brief` asks + // `assign` to keep whatever the lane cached, so a card the author cleared + // would bring the old text back; an empty string is the explicit clear + // `#2475` defined, and a card that still holds facts passes them verbatim. + const assigned = this.assign({ workflowId: decision.id, taskId, title, brief: typeof task.brief === 'string' ? task.brief : '' }) const finish = (value) => { if (value?.ok === false) return value logSource() diff --git a/plugins/workflow/test/ingest-brief-authority.test.js b/plugins/workflow/test/ingest-brief-authority.test.js new file mode 100644 index 000000000..491b67e64 --- /dev/null +++ b/plugins/workflow/test/ingest-brief-authority.test.js @@ -0,0 +1,97 @@ +// #2486: `#2477` made an explicit `brief` the authority on the assign side — it +// replaces the cached facts, and `brief: ''` clears them. The re-match path still +// omitted the key when the card held no brief, so `assign` read "keep what the +// lane already knows" and the facts the author cleared came back on the next hop. +// +// The card is the authority once it reaches `ingest`: what it holds travels, and +// what it no longer holds is cleared explicitly rather than left to the cache. +import assert from 'node:assert/strict' +import { mkdtempSync, readFileSync, rmSync } 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 FACTS = 'OLD facts.\nSecond line.' + +function serviceAt(dir) { + return createWorkflowService({ dir, persist: true, now: () => CLOCK, bots: () => [] }) +} + +// The lane cache lives on the store (`state.briefs`), which `state()` does not +// expose — it is read back from the file the service itself wrote. +function store(dir) { + return JSON.parse(readFileSync(join(dir, 'workflows.json'), 'utf8')) +} + +function named(svc, name) { + const created = svc.create({ + name, + nodes: [{ id: 'only', name: 'Only', prompt: 'Do it.' }], + edges: [], + }) + assert.equal(created.ok, true, created.error) + return created.workflows.find(item => item.name === name) +} + +function runOf(svc, workflowId, taskId) { + const item = svc.state().workflows.find(workflow => workflow.id === workflowId) + return item.runs.find(run => run?.task?.id === taskId) ?? item.queue.find(run => run.id === taskId) +} + +test('#2486: a card whose facts were cleared re-matches with no brief, and the old text does not come back', () => { + const dir = mkdtempSync(join(tmpdir(), 'wf2486-')) + try { + const svc = serviceAt(dir) + const alpha = named(svc, 'Alpha') + const beta = named(svc, 'Beta') + const gamma = named(svc, 'Gamma') + svc.assign({ workflowId: alpha.id, taskId: 't-a1', title: 'Ship it', brief: FACTS }) + + // The author clears the card (`brief: ''` on the card), so the board hands + // `ingest` a task that carries no brief at all. + svc.ingest({ id: 't-a1', status: 'open', title: 'Ship it', workflowId: beta.id }) + + const moved = runOf(svc, beta.id, 't-a1') + assert.equal( + moved?.task?.brief, + undefined, + "the card no longer holds facts, so the re-matched run must start without them", + ) + assert.equal( + store(dir).briefs['t-a1']?.brief, + undefined, + "the lane cache must not keep the facts the card no longer holds", + ) + + // A later assignment that omits `brief` still means "the card's facts", and + // the card is empty — so the old text must not return through the cache. + svc.assign({ workflowId: gamma.id, taskId: 't-a1', title: 'Ship it' }) + assert.equal( + runOf(svc, gamma.id, 't-a1')?.task?.brief, + undefined, + 'a call that passes no brief must not resurrect the cleared facts', + ) + } finally { + rmSync(dir, { recursive: true, force: true }) + } +}) + +test('#2486: a card that still holds facts re-matches with them, and the cache keeps them', () => { + const dir = mkdtempSync(join(tmpdir(), 'wf2486-')) + try { + const svc = serviceAt(dir) + const alpha = named(svc, 'Alpha') + const beta = named(svc, 'Beta') + svc.assign({ workflowId: alpha.id, taskId: 't-b', title: 'Ship it', brief: FACTS }) + + svc.ingest({ id: 't-b', status: 'open', title: 'Ship it', brief: FACTS, workflowId: beta.id }) + + const moved = runOf(svc, beta.id, 't-b') + assert.equal(moved?.task?.brief, FACTS, 'the card holds facts, so the re-matched run receives them verbatim') + assert.equal(store(dir).briefs['t-b']?.brief, FACTS, 'the lane cache carries the same facts') + } finally { + rmSync(dir, { recursive: true, force: true }) + } +})