fix(core): duplicate markers were never cleared on a board with renamed terminal columns (#2823)
Third unowned finding picked up after batch-core (#2783) merged without addressing its reports. Same class as #2819, opposite failure direction. ## The defect `clearNearDuplicateReferencesTo` runs on every complete/archive/delete of a canonical task 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 call omitted the canonical's resolved column flags, so it fell back to the legacy `done`/`archived` ids. On a renamed board a just-completed canonical (`shipped`, `filed`) 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 exact stranding the predicate's own FNXC note says it was written to prevent. ## I had the direction backwards, and it matters My first report of this seam described it as *markers cleared against a live canonical*. That is wrong. The legacy fallback errs toward "still active", so the failure is the opposite: markers that never clear at all. Same seam, same missing argument, entirely different symptom to look for — which is why the direction is worth pinning in a test rather than reasoning about. ## Measured With the fix reverted, exactly one case flips: ``` ✓ default vocabulary: completing the canonical clears the duplicate's marker × renamed vocabulary: completing the canonical clears the duplicate's marker ✓ renamed vocabulary: a canonical still in the WIP lane does NOT clear the marker ✓ default vocabulary: a canonical still in the WIP lane does NOT clear the marker ✓ a soft-deleted canonical clears the marker under a renamed board Tests 1 failed | 4 passed (5) ``` With it: `Tests 5 passed (5)`. ## Both negatives included A canonical still in the WIP lane must **not** clear its duplicates' markers, under each vocabulary. 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. The soft-deleted path is covered too, since that branch never consults column flags at all. ## A fixture trap worth keeping `sourceMetadata` is `jsonb` and must be seeded as an **object**. Seeding a stringified value reads back fine through `getTask` — it parses either shape — while the production query's `source_metadata->>'nearDuplicateOf'` matches nothing. The fixture looks correctly seeded and the code under test can never find the row. My first version had this, and the self-check in the seed helper is what caught it. ## Scope Five of this predicate's six production call sites already resolved flags. This was the sixth, and the one that runs on every archive/complete transition. Not touched here: the same function's SQL predicate excludes duplicates by literal `ne(column, "archived")` / `ne(column, "done")`. That is a per-row question across many rows in one statement, not a one-line supply, so it is flagged rather than guessed at. The five **engine** call sites of this predicate are separately reported on #2785 and surfaced only via #2822. ## Verification `pnpm test:gate` green, new suite 5/5, `tsc -p packages/core` 0, lint 0, changeset included. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
7
.changeset/near-duplicate-clear-renamed-terminal.md
Normal file
7
.changeset/near-duplicate-clear-renamed-terminal.md
Normal file
@@ -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.
|
||||
@@ -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<string> {
|
||||
const RENAME: Record<string, string> = {
|
||||
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<string> {
|
||||
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<boolean> {
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -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<Task[]> {
|
||||
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 [];
|
||||
}
|
||||
|
||||
|
||||
@@ -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",
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user