diff --git a/.changeset/near-duplicate-clear-renamed-terminal.md b/.changeset/near-duplicate-clear-renamed-terminal.md new file mode 100644 index 0000000000..51a391ab56 --- /dev/null +++ b/.changeset/near-duplicate-clear-renamed-terminal.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Duplicate flags now clear when the canonical task finishes on a renamed board. +category: fix +dev: `clearNearDuplicateReferencesTo` resolves the canonical's own column flags before calling `isNearDuplicateCanonicalInactive`, which otherwise fell back to the legacy `done`/`archived` ids and read a completed canonical as still active, leaving `nearDuplicateOf` markers set forever. diff --git a/packages/core/src/__tests__/postgres/near-duplicate-clear-renamed-terminal.pg.test.ts b/packages/core/src/__tests__/postgres/near-duplicate-clear-renamed-terminal.pg.test.ts new file mode 100644 index 0000000000..d8fa5673aa --- /dev/null +++ b/packages/core/src/__tests__/postgres/near-duplicate-clear-renamed-terminal.pg.test.ts @@ -0,0 +1,258 @@ +/* +FNXC:WorkflowResolvedColumns 2026-07-30-03:20 (duplicate markers never cleared on a renamed board): + +`clearNearDuplicateReferencesTo` runs whenever a canonical task is completed, archived, or deleted, +and clears the `nearDuplicateOf` markers pointing at it. It first asks +`isNearDuplicateCanonicalInactive` whether the canonical really is finished — a safety check, so a +live canonical's markers are not cleared out from under an operator. + +That check was called WITHOUT the canonical's resolved column flags, so it fell back to the legacy +`done`/`archived` ids. On a board whose terminal lanes are named anything else, a canonical that had +just been completed read as still ACTIVE, the guard early-returned, and the markers were never +cleared. The flagged duplicates stayed parked behind a user decision that could never arrive. + +THE DIRECTION IS THE POINT, and I had it backwards in my first report of this seam. The failure is +NOT "markers cleared against a live canonical" — the legacy fallback errs toward "still active", so +the failure is markers that are never cleared at all. `isNearDuplicateCanonicalInactive`'s own note +says exactly this: it exists so a FINISHED canonical stops holding a flag open. + +Five of the predicate's six production call sites already resolved flags. This one did not, and it is +the one that runs on every archive/complete transition. + +The cases are DIFFERENTIAL: the same clear against two vocabularies whose roles are identical and +only the ids differ. `shipped` collides with no legacy literal, so a surviving `"done"` cannot pass +by luck. +*/ + +import { it, expect, beforeAll, beforeEach, afterEach, afterAll } from "vitest"; +import { + pgDescribe, + createSharedPgTaskStoreTestHarness, + type SharedPgTaskStoreHarness, +} from "../../__test-utils__/pg-test-harness.js"; +import { eq } from "drizzle-orm"; +import * as schema from "../../postgres/schema/index.js"; +import { BUILTIN_CODING_WORKFLOW_IR } from "../../index.js"; + +pgDescribe("near-duplicate markers clear when the canonical reaches a RENAMED terminal column", () => { + const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({ + prefix: "fusion_neardup_renamed", + }); + + beforeAll(h.beforeAll); + beforeEach(h.beforeEach); + afterEach(h.afterEach); + afterAll(h.afterAll); + + /** The builtin coding workflow with only its column ids renamed. */ + async function seedRenamedWorkflow(): Promise { + const RENAME: Record = { + todo: "drafting", + "in-progress": "building", + "in-review": "checking", + done: "shipped", + }; + const rename = (id: string | undefined) => (id && RENAME[id]) ?? id; + const ir = JSON.parse(JSON.stringify(BUILTIN_CODING_WORKFLOW_IR)) as { + id: string; + nodes?: { column?: string }[]; + columns?: { id: string }[]; + }; + ir.id = "custom:renamed-terminal-neardup"; + for (const node of ir.nodes ?? []) node.column = rename(node.column); + for (const column of ir.columns ?? []) column.id = rename(column.id) as string; + + /* Prove the rename landed, or a surviving "done" literal would pass by accident. */ + const ids = (ir.columns ?? []).map((column) => column.id); + expect(ids).toContain("shipped"); + expect(ids).not.toContain("done"); + + const created = await h.store().createWorkflowDefinition({ + name: "Renamed Terminal (near-duplicate)", + kind: "workflow", + ir, + } as never); + return (created as { id: string }).id; + } + + /** A duplicate task carrying a `nearDuplicateOf` marker pointing at `canonicalId`. */ + async function seedDuplicateOf(canonicalId: string, workflowId?: string): Promise { + const store = h.store(); + const duplicate = await store.createTask({ + title: "the duplicate", + description: "test", + column: "todo", + }); + if (workflowId) await store.writeTaskWorkflowSelection(duplicate.id, workflowId, []); + /* `updateTask` does not persist `sourceMetadata`, so the marker is seeded directly — the same + approach the sibling PG suites use for store-stamped fields. Seeding only; every assertion + below still reads back through the real `getTask` path. */ + await h + .adminDb() + .update(schema.project.tasks) + /* jsonb: pass the OBJECT. Stringifying stores a JSON scalar, and the clear query's + `source_metadata->>'nearDuplicateOf'` then matches nothing — the fixture reads back fine + through getTask (which parses either shape), so this would have looked seeded while the + production query could never find it. */ + .set({ sourceMetadata: { nearDuplicateOf: canonicalId, nearDuplicateScore: 0.9 } }) + .where(eq(schema.project.tasks.id, duplicate.id)); + store.taskCache.delete(duplicate.id); + + /* Prove the marker is actually present; "it was cleared" is vacuous otherwise. */ + const seeded = await store.getTask(duplicate.id); + expect((seeded.sourceMetadata as { nearDuplicateOf?: string })?.nearDuplicateOf).toBe(canonicalId); + return duplicate.id; + } + + async function markerStillSet(taskId: string): Promise { + const store = h.store(); + store.taskCache.delete(taskId); + const after = await store.getTask(taskId); + return (after.sourceMetadata as { nearDuplicateOf?: string })?.nearDuplicateOf !== undefined; + } + + /* Control: under the default vocabulary the marker clears. Passes before and after the fix, so a + generally broken clear path cannot hide behind the renamed case. */ + it("default vocabulary: completing the canonical clears the duplicate's marker", async () => { + const canonical = await h.store().createTask({ title: "canonical", description: "t", column: "todo" }); + const duplicate = await seedDuplicateOf(canonical.id); + + await h.store().clearNearDuplicateReferencesTo(canonical.id, { + column: "done" as never, + reason: "completed", + }); + + expect(await markerStillSet(duplicate)).toBe(false); + }); + + /* + The defect. Before the fix the guard read `shipped` as "still active" via the legacy ids, returned + early, and left the marker in place forever. + */ + it("renamed vocabulary: completing the canonical clears the duplicate's marker", async () => { + const wf = await seedRenamedWorkflow(); + const store = h.store(); + const canonical = await store.createTask({ title: "canonical", description: "t", column: "todo" }); + await store.writeTaskWorkflowSelection(canonical.id, wf, []); + const duplicate = await seedDuplicateOf(canonical.id, wf); + + /* + FNXC:WorkflowResolvedColumns 2026-07-31-04:25 (#2823 review — greptile, and it was right): + + DRIVEN THROUGH THE REAL COMPLETION MOVE, not by handing the consumer a column. + + Supplying `column: "shipped"` by hand proved the consumer resolves flags correctly — and hid the + fact that nothing in production ever gives it that value. `moveTaskInternal` gates on the RESOLVED + complete lane and then passed the literal `column: "done"`, so the consumer asked "is `done` + terminal on this board?", got no, and left every marker in place. The fix was inert through the + only path that reaches it. + + Moving the card is the whole point: it exercises the gate, the argument, and the consumer together, + which is the only arrangement that could have caught this. + */ + /* Adjacency is derived from the graph, so the card walks its board rather than jumping. */ + for (const lane of ["drafting", "building", "checking", "shipped"]) { + await store.moveTask(canonical.id, lane as never, { bypassGuards: true } as never); + } + + expect(await markerStillSet(duplicate)).toBe(false); + }); + + /* + FNXC:WorkflowResolvedColumns 2026-07-31-04:45 (#2823 review — what driving production actually showed): + + THE CASE THAT SEPARATES THE ARGUMENT FROM THE ACCIDENT. + + Driving the real completion move (the case above) does NOT distinguish the hardcoded `column: + "done"` from the resolved `toColumn`, and I checked by mutating both call sites: the suite stayed + green. The reason is that the consumer looks the passed column up in the canonical's IR, finds + nothing for `done` on a renamed board, and falls through to the LEGACY predicate — where `done` is + terminal. Right outcome, wrong reason. + + It stops being an accident on a board that DECLARES a `done` column without the complete trait. The + literal then resolves real flags, learns `done` is not terminal here, early-returns, and strands + every duplicate marker. `toColumn` names the lane the card actually reached and is unaffected. + + This is the only shape under which the argument is load-bearing, which is why it exists. + */ + it("clears the marker on a board that declares a NON-TERMINAL column named `done`", async () => { + const store = h.store(); + const definition = await store.createWorkflowDefinition({ + name: "done-is-not-complete", + ir: { + version: "v2", + name: "done-is-not-complete", + columns: [ + { id: "drafting", name: "Drafting", traits: [{ trait: "intake" }] }, + /* Declared, and deliberately NOT the complete lane. */ + { id: "done", name: "Done pile (not terminal)", traits: [] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], + nodes: [{ id: "start", kind: "start", column: "drafting" }, { id: "end", kind: "end", column: "shipped" }], + edges: [{ from: "start", to: "end" }], + }, + } as never); + const wf = (definition as unknown as { id: string }).id; + const canonical = await store.createTask({ title: "canonical", description: "t", column: "todo" }); + await store.writeTaskWorkflowSelection(canonical.id, wf, []); + const duplicate = await seedDuplicateOf(canonical.id, wf); + + for (const lane of ["drafting", "done", "shipped"]) { + await store.moveTask(canonical.id, lane as never, { bypassGuards: true } as never); + } + + expect(await markerStillSet(duplicate)).toBe(false); + }); + + /* + The paired negative, and the reason this is a supply fix rather than a deleted guard: a canonical + that is still LIVE must keep its duplicates' markers. Resolving the real flags must not degrade + into "every column is terminal", which would clear markers out from under an operator who has not + made the duplicate decision yet. + */ + it("renamed vocabulary: a canonical still in the WIP lane does NOT clear the marker", async () => { + const wf = await seedRenamedWorkflow(); + const store = h.store(); + const canonical = await store.createTask({ title: "canonical", description: "t", column: "todo" }); + await store.writeTaskWorkflowSelection(canonical.id, wf, []); + const duplicate = await seedDuplicateOf(canonical.id, wf); + + await store.clearNearDuplicateReferencesTo(canonical.id, { + column: "building" as never, + reason: "not-really-finished", + }); + + expect(await markerStillSet(duplicate)).toBe(true); + }); + + /* Same negative under the default vocabulary, so the retention is not an artifact of the rename. */ + it("default vocabulary: a canonical still in the WIP lane does NOT clear the marker", async () => { + const canonical = await h.store().createTask({ title: "canonical", description: "t", column: "todo" }); + const duplicate = await seedDuplicateOf(canonical.id); + + await h.store().clearNearDuplicateReferencesTo(canonical.id, { + column: "in-progress" as never, + reason: "not-really-finished", + }); + + expect(await markerStillSet(duplicate)).toBe(true); + }); + + /* A soft-deleted canonical is inactive regardless of column vocabulary — the deletedAt branch must + keep working, since it is the one path that never consults column flags at all. */ + it("a soft-deleted canonical clears the marker under a renamed board", async () => { + const wf = await seedRenamedWorkflow(); + const store = h.store(); + const canonical = await store.createTask({ title: "canonical", description: "t", column: "todo" }); + await store.writeTaskWorkflowSelection(canonical.id, wf, []); + const duplicate = await seedDuplicateOf(canonical.id, wf); + + await store.clearNearDuplicateReferencesTo(canonical.id, { + column: "building" as never, + deletedAt: new Date().toISOString(), + reason: "deleted", + }); + + expect(await markerStillSet(duplicate)).toBe(false); + }); +}); diff --git a/packages/core/src/task-store/branch-group-ops.ts b/packages/core/src/task-store/branch-group-ops.ts index da9b6a885d..dbe19c4810 100644 --- a/packages/core/src/task-store/branch-group-ops.ts +++ b/packages/core/src/task-store/branch-group-ops.ts @@ -15,6 +15,7 @@ import {runReconciliationAbort} from "../workflow-reconciliation.js"; import "../builtin-traits.js"; import {evaluateImplementationTaskBind} from "../agent-role-policy.js"; import {isNearDuplicateCanonicalInactive} from "../near-duplicate-canonical.js"; +import {resolveColumnFlags} from "../trait-registry.js"; import {__setTaskActivityLogLimitsForTesting} from "../task-store/comments.js"; import {listArtifacts as listArtifactsAsync} from "./async-comments-attachments.js"; import { and, eq, isNull, ne, sql } from "drizzle-orm"; @@ -41,7 +42,29 @@ export async function saveWorkflowRunBranchImpl(store: TaskStore, state: { taskI } export async function clearNearDuplicateReferencesToImpl(store: TaskStore, canonicalId: string, inactiveState: { column?: ColumnId | null; deletedAt?: string | null; reason: string },): Promise { - if (!isNearDuplicateCanonicalInactive(inactiveState)) { + /* + FNXC:WorkflowResolvedColumns 2026-07-30-03:10: + Resolve the CANONICAL's own column flags before asking whether it is inactive. Omitted, the + predicate falls back to the legacy `done`/`archived` ids, so on a renamed board a canonical that + has just been completed or archived (`shipped`, `filed`) reads as still ACTIVE — this guard + early-returns and the duplicate markers pointing at it are NEVER cleared. The flagged tasks stay + parked behind a user decision that can never arrive, which is the exact stranding the note on + `isNearDuplicateCanonicalInactive` says it was written to prevent. + + Five of this predicate's six production call sites already resolved flags; this one did not, and + it is the one that runs on every archive/complete transition. + + `undefined` on failure is deliberate and matches `moves.ts`: it degrades to the legacy id rather + than to absent traits that match nothing. + */ + const canonicalIr = await resolveWorkflowIrForTask(store, canonicalId).catch(() => undefined); + /* v1 IRs declare no columns, so there is nothing to resolve and the legacy fallback stands. + (`columnsOf` in workflow-lifecycle-traits.ts is module-private; this is the same narrowing, + inlined rather than widening that module's API for a single caller.) */ + const canonicalColumns = canonicalIr?.version === "v2" ? canonicalIr.columns : []; + const canonicalColumn = canonicalColumns.find((column) => column.id === inactiveState.column); + const canonicalFlags = canonicalColumn ? resolveColumnFlags(canonicalColumn) : undefined; + if (!isNearDuplicateCanonicalInactive(inactiveState, canonicalFlags)) { return []; } diff --git a/packages/core/src/task-store/moves.ts b/packages/core/src/task-store/moves.ts index 7cceb472e8..619d1603f0 100644 --- a/packages/core/src/task-store/moves.ts +++ b/packages/core/src/task-store/moves.ts @@ -532,8 +532,21 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum store.emit("task:updated", task); } if (toColumn === (moveLifecycle?.complete ?? "done")) { + /* + FNXC:WorkflowResolvedColumns 2026-07-31-04:20 (#2823 review — greptile): + PASS THE COLUMN THE CARD ACTUALLY REACHED, not the legacy name for it. + + The gate above already resolves the complete lane, so on a renamed board this fires correctly + — and then handed the consumer `column: "done"`, a column that board does not declare. The + consumer resolves the canonical's flags for the column it is GIVEN, so it asked "is `done` + terminal here?", got no, and left every duplicate marker in place. The conversion downstream + was inert through this path for exactly the boards it was written for. + + `toColumn` is the lane the card reached; the resolved-vs-literal question is already settled + by the gate one line up. + */ await store.clearNearDuplicateReferencesToFailSoft(id, { - column: "done", + column: toColumn, reason: "done", }); } @@ -1422,8 +1435,10 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum }); } if (toColumn === (moveLifecycle?.complete ?? "done")) { + /* FNXC:WorkflowResolvedColumns 2026-07-31-04:20 (#2823 review): the sibling of the call above — + same gate, same hardcoded argument, same inert result. Both move together. */ await store.clearNearDuplicateReferencesToFailSoft(id, { - column: "done", + column: toColumn, reason: "done", }); }