From 4ce3ff75b6dd458b9f38608423bba028cfc91921 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 15:46:16 -0700 Subject: [PATCH] fix(ce): the sync queue recorded "done" for a card that finished somewhere else (#2859) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A lane literal the census cannot see — it is a **call argument**, not a comparison — and the two sibling hooks disagree about how to record the same kind of fact: ```ts onTaskMoved: async (task, fromColumn, toColumn, ctx) => { await store.enqueueSyncAsync({ …, toColumn }); // the real column }, onTaskCompleted: async (task, ctx) => { await store.enqueueSyncAsync({ …, toColumn: "done" }); // ← a guess }, ``` On a board whose complete lane is named anything else, the sync-queue row names a column that board does not have. ## Why fix a field nothing reads `toColumn` is written to `ce_pipeline_sync_queue` and **never consumed by any logic** — I checked every reference; it appears only in the schema, the store's insert, and the row type. It is audit metadata. That is both why it went unnoticed and why it is worth one token: the single thing a wrong audit row costs you is the ability to reconstruct what happened after the fact. A queue that says a card went to `done` on a board with no `done` is worse than a queue with no column at all, because it reads as authoritative. ## Structural ratchet, and I would rather say so than imply otherwise `getCePipelineStore` requires a live PostgreSQL `AsyncDataLayer` and throws without one, so driving the hook means standing up PG to re-assert a one-token substitution — against the standing rule on slow tests. The repo already takes this trade in the same shape (`packages/core/src/__tests__/analytics-timing-roles-resolved.test.ts`, whose analytics aggregators have the identical problem). The test comment states plainly what it does and does not prove: it pins that the hook reads the card's own column and holds no completion literal; it does not exercise the write. Comments are stripped before the negative assertion — the test's own explanation names the old literal, and a ratchet that matches its own prose passes forever without checking anything. (That mistake is already in this program's history, which is why it is guarded here.) ## Revert proof (measured) Restore `toColumn: "done"` — **both** assertions fail (the positive one on the missing `task.column`, the negative one on the literal). ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion-plugin-examples/compound-engineering`) — clean - full plugin suite — 319 passed across 32 files 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **Bug Fixes** * Sync audit records now capture the task’s actual completion lane instead of assuming it is named “done.” * Improved audit accuracy for boards with custom completion lane names. * **Tests** * Added coverage to verify completion records use the task’s real destination lane. Co-authored-by: Claude Opus 5 (1M context) --- .changeset/ce-sync-audit-column.md | 7 +++ .../src/__tests__/sync-audit-column.test.ts | 44 +++++++++++++++++++ .../src/index.ts | 15 ++++++- 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 .changeset/ce-sync-audit-column.md create mode 100644 plugins/fusion-plugin-compound-engineering/src/__tests__/sync-audit-column.test.ts diff --git a/.changeset/ce-sync-audit-column.md b/.changeset/ce-sync-audit-column.md new file mode 100644 index 0000000000..5c4192dda8 --- /dev/null +++ b/.changeset/ce-sync-audit-column.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Compound Engineering sync records the lane a completed card actually reached. +category: fix +dev: `onTaskCompleted` enqueued `toColumn: "done"` while its sibling `onTaskMoved` records the real column, so on a renamed board the sync-queue audit row named a column the board does not have. Now records `task.column`. diff --git a/plugins/fusion-plugin-compound-engineering/src/__tests__/sync-audit-column.test.ts b/plugins/fusion-plugin-compound-engineering/src/__tests__/sync-audit-column.test.ts new file mode 100644 index 0000000000..e298712644 --- /dev/null +++ b/plugins/fusion-plugin-compound-engineering/src/__tests__/sync-audit-column.test.ts @@ -0,0 +1,44 @@ +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-16:05: + +THE INVARIANT: the CE sync queue records the lane a card actually reached. + +`onTaskMoved` records the real `toColumn` it is handed. `onTaskCompleted` wrote the literal `"done"`, +so on a board whose complete lane is named anything else the queue row named a column that board does +not have. Nothing READS `toColumn` — it is audit metadata, not a control signal — which is both why +it went unnoticed and why it matters: the single thing a wrong audit row costs you is the ability to +reconstruct what happened after the fact. + +WHY THIS IS A STRUCTURAL RATCHET AND NOT A BEHAVIOURAL TEST, stated rather than glossed. +`getCePipelineStore` requires a live PostgreSQL `AsyncDataLayer` and throws without one, so driving +the hook means standing up PG to re-assert a one-token substitution — against the standing rule not +to add slow tests. The repo already accepts this trade in the same shape (see +`packages/core/src/__tests__/analytics-timing-roles-resolved.test.ts`, whose analytics aggregators +have the same problem). + +What it therefore does and does not prove: it pins that the hook reads the card's own column and +holds no completion literal. It does not exercise the write. That is honest coverage of a metadata +field, not a substitute for one. + +REVERT PROOF, measured: restore `toColumn: "done"` and both assertions fail. +*/ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +describe("the CE sync queue records the real completion lane", () => { + const source = readFileSync(new URL("../index.ts", import.meta.url), "utf8"); + + it("onTaskCompleted enqueues the card's own column", () => { + expect(source).toContain("toColumn: task.column,"); + }); + + it("holds no hardcoded completion lane", () => { + // Comments are stripped first: this file's own explanation names the old literal, and a ratchet + // that matches its own prose passes forever without checking anything. + const code = source + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\/\/.*$/gm, ""); + + expect(code).not.toContain('toColumn: "done"'); + }); +}); diff --git a/plugins/fusion-plugin-compound-engineering/src/index.ts b/plugins/fusion-plugin-compound-engineering/src/index.ts index 729b5e04ce..4999b37c23 100644 --- a/plugins/fusion-plugin-compound-engineering/src/index.ts +++ b/plugins/fusion-plugin-compound-engineering/src/index.ts @@ -105,11 +105,24 @@ const plugin = definePlugin({ const store = getCePipelineStore(ctx); const link = await store.findByTaskIdAsync(task.id); if (!link) return; + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-16:05: + Record the lane the card ACTUALLY reached, not the built-in board's name for "finished". + + `onTaskMoved` two hooks up already records the real `toColumn` it is handed; this one wrote the + literal `"done"`, so on a board whose complete lane is named anything else the sync-queue row + claimed a column that board does not have. Nothing reads `toColumn` — it is audit metadata, not + a control signal, which is precisely why it went unnoticed and precisely why it matters: the + one thing a wrong audit row costs you is the ability to reconstruct what happened. + + `task.column` is the completed card's own lane, so the two hooks now record the same kind of + fact in the same way. + */ await store.enqueueSyncAsync({ cePipelineId: link.cePipelineId, taskId: task.id, reason: "task_completed", - toColumn: "done", + toColumn: task.column, }); if (!getReconcileOnHooks(ctx.settings)) return; void reconcileCePipelines(ctx)