feat(FN-5060): complete Step 3 — extract core duplicate guard helper
This commit is contained in:
gsxdsm
208
packages/core/src/__tests__/duplicate-guard.test.ts
Normal file
208
packages/core/src/__tests__/duplicate-guard.test.ts
Normal file
@@ -0,0 +1,208 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
import type { Column, Task, TaskStore } from "../types.js";
|
||||
import {
|
||||
__getDeterministicGuardMutexSize,
|
||||
reconcileDeterministicDuplicate,
|
||||
runDeterministicDuplicateGuard,
|
||||
} from "../duplicate-guard.js";
|
||||
|
||||
function mkTask(overrides: Partial<Task> & { id: string; description: string; column: Column }): Task {
|
||||
const now = new Date().toISOString();
|
||||
return {
|
||||
id: overrides.id,
|
||||
description: overrides.description,
|
||||
column: overrides.column,
|
||||
dependencies: [],
|
||||
createdAt: now,
|
||||
updatedAt: now,
|
||||
size: "M",
|
||||
subtasks: [],
|
||||
log: [],
|
||||
tags: [],
|
||||
blockedBy: [],
|
||||
source: { sourceType: "api" },
|
||||
...overrides,
|
||||
} as Task;
|
||||
}
|
||||
|
||||
function makeStore(seed: Task[] = []): { tasks: Task[]; store: TaskStore } {
|
||||
const tasks = [...seed];
|
||||
const store = {
|
||||
findRecentTasksByContentFingerprint: vi.fn().mockImplementation(async (fp: string, options?: { windowMs?: number; includeArchived?: boolean }) => {
|
||||
const windowMs = Math.max(1, Math.min(300_000, Math.trunc(options?.windowMs ?? 60_000)));
|
||||
const cutoff = Date.now() - windowMs;
|
||||
return tasks
|
||||
.filter((task) => task.source?.sourceMetadata?.contentFingerprint === fp)
|
||||
.filter((task) => (options?.includeArchived ?? false) || task.column !== "archived")
|
||||
.filter((task) => Date.parse(task.createdAt) >= cutoff)
|
||||
.sort((a, b) => Date.parse(a.createdAt) - Date.parse(b.createdAt));
|
||||
}),
|
||||
createTask: vi.fn().mockImplementation(async (input: { title?: string; description: string; source?: Task["source"] }) => {
|
||||
const now = new Date().toISOString();
|
||||
const created = mkTask({
|
||||
id: `FN-${tasks.length + 1}`,
|
||||
title: input.title,
|
||||
description: input.description,
|
||||
column: "todo",
|
||||
createdAt: now,
|
||||
updatedAt: now,
|
||||
source: input.source ?? { sourceType: "api" },
|
||||
});
|
||||
tasks.push(created);
|
||||
return created;
|
||||
}),
|
||||
updateTask: vi.fn().mockImplementation(async (id: string, updates: { sourceMetadataPatch?: Record<string, unknown> }) => {
|
||||
const task = tasks.find((item) => item.id === id);
|
||||
if (!task) return null;
|
||||
task.source = {
|
||||
...(task.source ?? { sourceType: "api" }),
|
||||
sourceMetadata: {
|
||||
...(task.source?.sourceMetadata ?? {}),
|
||||
...(updates.sourceMetadataPatch ?? {}),
|
||||
},
|
||||
};
|
||||
return task;
|
||||
}),
|
||||
moveTask: vi.fn().mockImplementation(async (id: string, column: Column) => {
|
||||
const task = tasks.find((item) => item.id === id);
|
||||
if (!task) return null;
|
||||
task.column = column;
|
||||
return task;
|
||||
}),
|
||||
recordActivity: vi.fn().mockResolvedValue(undefined),
|
||||
} as unknown as TaskStore;
|
||||
|
||||
return { tasks, store };
|
||||
}
|
||||
|
||||
const INPUT = {
|
||||
title: "Move retry counter badge next to GitHub tracking badge",
|
||||
description: "Move the retry counter badge to the left of the GitHub tracking badge",
|
||||
};
|
||||
|
||||
describe("runDeterministicDuplicateGuard", () => {
|
||||
it("returns duplicate when same fingerprint task already exists", async () => {
|
||||
const existing = mkTask({
|
||||
id: "FN-1",
|
||||
title: INPUT.title,
|
||||
description: INPUT.description,
|
||||
column: "todo",
|
||||
source: { sourceType: "api", sourceMetadata: { contentFingerprint: "fp" } },
|
||||
});
|
||||
const { store } = makeStore([existing]);
|
||||
|
||||
vi.spyOn(store, "findRecentTasksByContentFingerprint").mockResolvedValueOnce([existing]);
|
||||
const result = await runDeterministicDuplicateGuard(store, INPUT, { lockScope: "p-1" });
|
||||
expect(result.action).toBe("duplicate");
|
||||
expect(result.existing?.id).toBe("FN-1");
|
||||
result.releaseLock();
|
||||
});
|
||||
|
||||
it("serializes concurrent calls with same lock scope", async () => {
|
||||
const { store, tasks } = makeStore();
|
||||
const first = runDeterministicDuplicateGuard(store, INPUT, { lockScope: "p-1" });
|
||||
const secondPromise = runDeterministicDuplicateGuard(store, INPUT, { lockScope: "p-1" });
|
||||
|
||||
const firstResult = await first;
|
||||
expect(firstResult.action).toBe("proceed");
|
||||
|
||||
const created = await store.createTask({
|
||||
title: INPUT.title,
|
||||
description: INPUT.description,
|
||||
source: { sourceType: "api", sourceMetadata: { contentFingerprint: firstResult.fingerprint ?? undefined } },
|
||||
});
|
||||
expect(tasks).toHaveLength(1);
|
||||
firstResult.releaseLock();
|
||||
|
||||
const secondResult = await secondPromise;
|
||||
expect(secondResult.action).toBe("duplicate");
|
||||
expect(secondResult.existing?.id).toBe(created.id);
|
||||
secondResult.releaseLock();
|
||||
});
|
||||
|
||||
it("allows concurrent proceed without shared lock scope", async () => {
|
||||
const { store } = makeStore();
|
||||
const [a, b] = await Promise.all([
|
||||
runDeterministicDuplicateGuard(store, INPUT),
|
||||
runDeterministicDuplicateGuard(store, INPUT),
|
||||
]);
|
||||
expect(a.action).toBe("proceed");
|
||||
expect(b.action).toBe("proceed");
|
||||
});
|
||||
|
||||
it("allows proceed when duplicate is acknowledged", async () => {
|
||||
const existing = mkTask({ id: "FN-1", title: INPUT.title, description: INPUT.description, column: "todo", source: { sourceType: "api", sourceMetadata: { contentFingerprint: "fp" } } });
|
||||
const { store } = makeStore([existing]);
|
||||
vi.spyOn(store, "findRecentTasksByContentFingerprint").mockResolvedValueOnce([existing]);
|
||||
const result = await runDeterministicDuplicateGuard(store, INPUT, { lockScope: "p-1", acknowledgedDuplicates: ["FN-1"] });
|
||||
expect(result.action).toBe("proceed");
|
||||
result.releaseLock();
|
||||
});
|
||||
|
||||
it("bypass skips mutex allocation", async () => {
|
||||
const { store } = makeStore();
|
||||
const before = __getDeterministicGuardMutexSize();
|
||||
const result = await runDeterministicDuplicateGuard(store, INPUT, { lockScope: "p-1", bypass: true });
|
||||
const after = __getDeterministicGuardMutexSize();
|
||||
expect(result.action).toBe("proceed");
|
||||
expect(after).toBe(before);
|
||||
});
|
||||
|
||||
it("ignores rows older than window", async () => {
|
||||
const oldTs = new Date(Date.now() - 120_000).toISOString();
|
||||
const existing = mkTask({ id: "FN-1", title: INPUT.title, description: INPUT.description, column: "todo", createdAt: oldTs, updatedAt: oldTs, source: { sourceType: "api", sourceMetadata: { contentFingerprint: "fp" } } });
|
||||
const { store } = makeStore([existing]);
|
||||
vi.spyOn(store, "findRecentTasksByContentFingerprint").mockResolvedValueOnce([]);
|
||||
const result = await runDeterministicDuplicateGuard(store, INPUT, { lockScope: "p-1", windowMs: 60_000 });
|
||||
expect(result.action).toBe("proceed");
|
||||
result.releaseLock();
|
||||
});
|
||||
|
||||
it("returns null fingerprint for empty description", async () => {
|
||||
const { store } = makeStore();
|
||||
const result = await runDeterministicDuplicateGuard(store, { title: "x", description: "..." }, { lockScope: "p-1" });
|
||||
expect(result.action).toBe("proceed");
|
||||
expect(result.fingerprint).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("reconcileDeterministicDuplicate", () => {
|
||||
it("archives late-race loser and records activity metadata", async () => {
|
||||
const canonicalTs = new Date(Date.now() - 2_000).toISOString();
|
||||
const createdTs = new Date().toISOString();
|
||||
const canonical = mkTask({ id: "FN-1", title: INPUT.title, description: INPUT.description, column: "todo", createdAt: canonicalTs, updatedAt: canonicalTs, source: { sourceType: "api", sourceMetadata: { contentFingerprint: "fp" } } });
|
||||
const created = mkTask({ id: "FN-2", title: INPUT.title, description: INPUT.description, column: "todo", createdAt: createdTs, updatedAt: createdTs, source: { sourceType: "api", sourceMetadata: { contentFingerprint: "fp" } } });
|
||||
const { store } = makeStore([canonical, created]);
|
||||
vi.spyOn(store, "findRecentTasksByContentFingerprint").mockResolvedValueOnce([canonical, created]);
|
||||
|
||||
const result = await reconcileDeterministicDuplicate(store, { createdTask: created, fingerprint: "fp" });
|
||||
expect(result).toEqual({ outcome: "archived", canonical });
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-2", {
|
||||
sourceMetadataPatch: {
|
||||
contentFingerprint: "fp",
|
||||
deterministicDuplicateOf: "FN-1",
|
||||
},
|
||||
});
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-2", "archived");
|
||||
expect(store.recordActivity).toHaveBeenCalledWith(expect.objectContaining({
|
||||
type: "task:auto-archived-deterministic-duplicate",
|
||||
metadata: { canonicalTaskId: "FN-1", contentFingerprint: "fp" },
|
||||
}));
|
||||
});
|
||||
|
||||
it("fails open when archive move throws", async () => {
|
||||
const canonicalTs = new Date(Date.now() - 2_000).toISOString();
|
||||
const createdTs = new Date().toISOString();
|
||||
const canonical = mkTask({ id: "FN-1", title: INPUT.title, description: INPUT.description, column: "todo", createdAt: canonicalTs, updatedAt: canonicalTs, source: { sourceType: "api", sourceMetadata: { contentFingerprint: "fp" } } });
|
||||
const created = mkTask({ id: "FN-2", title: INPUT.title, description: INPUT.description, column: "todo", createdAt: createdTs, updatedAt: createdTs, source: { sourceType: "api", sourceMetadata: { contentFingerprint: "fp" } } });
|
||||
const { store } = makeStore([canonical, created]);
|
||||
vi.spyOn(store, "findRecentTasksByContentFingerprint").mockResolvedValueOnce([canonical, created]);
|
||||
vi.spyOn(store, "moveTask").mockRejectedValueOnce(new Error("archive failed"));
|
||||
|
||||
const warn = vi.fn();
|
||||
const result = await reconcileDeterministicDuplicate(store, { createdTask: created, fingerprint: "fp", logger: { warn } });
|
||||
expect(result).toEqual({ outcome: "kept", canonical: created });
|
||||
expect(warn).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
162
packages/core/src/duplicate-guard.ts
Normal file
162
packages/core/src/duplicate-guard.ts
Normal file
@@ -0,0 +1,162 @@
|
||||
import type { Task, TaskStore } from "./types.js";
|
||||
import { computeContentFingerprint } from "./duplicate-detection.js";
|
||||
|
||||
const DEFAULT_WINDOW_MS = 60_000;
|
||||
const MAX_WINDOW_MS = 300_000;
|
||||
const deterministicGuardLocks = new Map<string, Promise<void>>();
|
||||
|
||||
export interface DeterministicGuardOptions {
|
||||
windowMs?: number;
|
||||
lockScope?: string;
|
||||
acknowledgedDuplicates?: readonly string[];
|
||||
bypass?: boolean;
|
||||
logger?: { warn(msg: string, data?: Record<string, unknown>): void };
|
||||
}
|
||||
|
||||
export interface DeterministicGuardOutcome {
|
||||
action: "proceed" | "duplicate";
|
||||
fingerprint: string | null;
|
||||
existing?: Task;
|
||||
releaseLock: () => void;
|
||||
}
|
||||
|
||||
export function __getDeterministicGuardMutexSize(): number {
|
||||
return deterministicGuardLocks.size;
|
||||
}
|
||||
|
||||
function clampWindowMs(windowMs?: number): number {
|
||||
const requested = windowMs ?? DEFAULT_WINDOW_MS;
|
||||
return Math.max(1, Math.min(MAX_WINDOW_MS, Math.trunc(requested)));
|
||||
}
|
||||
|
||||
function noop(): void {}
|
||||
|
||||
export async function runDeterministicDuplicateGuard(
|
||||
store: TaskStore,
|
||||
input: { title?: string | null; description: string },
|
||||
opts?: DeterministicGuardOptions,
|
||||
): Promise<DeterministicGuardOutcome> {
|
||||
const fingerprint = computeContentFingerprint(input);
|
||||
if (opts?.bypass === true || !fingerprint) {
|
||||
return { action: "proceed", fingerprint, releaseLock: noop };
|
||||
}
|
||||
|
||||
const acknowledged = new Set(opts?.acknowledgedDuplicates ?? []);
|
||||
const windowMs = clampWindowMs(opts?.windowMs);
|
||||
|
||||
if (!opts?.lockScope) {
|
||||
try {
|
||||
const deterministicMatches = await store.findRecentTasksByContentFingerprint(fingerprint, {
|
||||
windowMs,
|
||||
includeArchived: false,
|
||||
});
|
||||
const deterministicConflict = deterministicMatches.find((match) => !acknowledged.has(match.id));
|
||||
if (deterministicConflict) {
|
||||
return { action: "duplicate", fingerprint, existing: deterministicConflict, releaseLock: noop };
|
||||
}
|
||||
} catch (error) {
|
||||
opts?.logger?.warn("Deterministic duplicate pre-check failed; proceeding", {
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
}
|
||||
return { action: "proceed", fingerprint, releaseLock: noop };
|
||||
}
|
||||
|
||||
const lockKey = `${opts.lockScope}:${fingerprint}`;
|
||||
const existingLock = deterministicGuardLocks.get(lockKey);
|
||||
if (existingLock) {
|
||||
await existingLock;
|
||||
}
|
||||
|
||||
let releaseCalled = false;
|
||||
let resolveGate: (() => void) | undefined;
|
||||
const gate = new Promise<void>((resolve) => {
|
||||
resolveGate = resolve;
|
||||
});
|
||||
deterministicGuardLocks.set(lockKey, gate);
|
||||
|
||||
const releaseLock = () => {
|
||||
if (releaseCalled) {
|
||||
return;
|
||||
}
|
||||
releaseCalled = true;
|
||||
resolveGate?.();
|
||||
deterministicGuardLocks.delete(lockKey);
|
||||
};
|
||||
|
||||
try {
|
||||
const deterministicMatches = await store.findRecentTasksByContentFingerprint(fingerprint, {
|
||||
windowMs,
|
||||
includeArchived: false,
|
||||
});
|
||||
const deterministicConflict = deterministicMatches.find((match) => !acknowledged.has(match.id));
|
||||
if (deterministicConflict) {
|
||||
return { action: "duplicate", fingerprint, existing: deterministicConflict, releaseLock };
|
||||
}
|
||||
} catch (error) {
|
||||
opts?.logger?.warn("Deterministic duplicate pre-check failed; proceeding", {
|
||||
lockKey,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
}
|
||||
|
||||
return { action: "proceed", fingerprint, releaseLock };
|
||||
}
|
||||
|
||||
export async function reconcileDeterministicDuplicate(
|
||||
store: TaskStore,
|
||||
args: {
|
||||
createdTask: Task;
|
||||
fingerprint: string | null;
|
||||
windowMs?: number;
|
||||
logger?: { warn(msg: string, data?: Record<string, unknown>): void };
|
||||
},
|
||||
): Promise<{ outcome: "kept" | "archived"; canonical: Task }> {
|
||||
if (!args.fingerprint) {
|
||||
return { outcome: "kept", canonical: args.createdTask };
|
||||
}
|
||||
|
||||
try {
|
||||
const siblings = await store.findRecentTasksByContentFingerprint(args.fingerprint, {
|
||||
windowMs: clampWindowMs(args.windowMs),
|
||||
includeArchived: false,
|
||||
});
|
||||
|
||||
const olderSibling = siblings.find((sibling) => sibling.id !== args.createdTask.id && sibling.createdAt < args.createdTask.createdAt);
|
||||
if (!olderSibling) {
|
||||
return { outcome: "kept", canonical: args.createdTask };
|
||||
}
|
||||
|
||||
await store.updateTask(args.createdTask.id, {
|
||||
sourceMetadataPatch: {
|
||||
contentFingerprint: args.fingerprint,
|
||||
deterministicDuplicateOf: olderSibling.id,
|
||||
},
|
||||
});
|
||||
await store.moveTask(args.createdTask.id, "archived");
|
||||
|
||||
try {
|
||||
await store.recordActivity({
|
||||
type: "task:auto-archived-deterministic-duplicate",
|
||||
taskId: args.createdTask.id,
|
||||
taskTitle: args.createdTask.title,
|
||||
details: `Auto-archived as deterministic duplicate of ${olderSibling.id}`,
|
||||
metadata: { canonicalTaskId: olderSibling.id, contentFingerprint: args.fingerprint },
|
||||
});
|
||||
} catch (error) {
|
||||
args.logger?.warn("Failed to record deterministic-duplicate activity", {
|
||||
taskId: args.createdTask.id,
|
||||
canonicalTaskId: olderSibling.id,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
}
|
||||
|
||||
return { outcome: "archived", canonical: olderSibling };
|
||||
} catch (error) {
|
||||
args.logger?.warn("Deterministic duplicate reconciliation failed; keeping created task", {
|
||||
taskId: args.createdTask.id,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
return { outcome: "kept", canonical: args.createdTask };
|
||||
}
|
||||
}
|
||||
@@ -139,6 +139,13 @@ export {
|
||||
type DuplicateMatchInput,
|
||||
} from "./duplicate-detection.js";
|
||||
export { getTaskDuplicateLineage } from "./duplicate-lineage.js";
|
||||
export {
|
||||
__getDeterministicGuardMutexSize,
|
||||
runDeterministicDuplicateGuard,
|
||||
reconcileDeterministicDuplicate,
|
||||
type DeterministicGuardOptions,
|
||||
type DeterministicGuardOutcome,
|
||||
} from "./duplicate-guard.js";
|
||||
export {
|
||||
findSameAgentDuplicates,
|
||||
archiveAsSameAgentDuplicate,
|
||||
|
||||
Reference in New Issue
Block a user