feat(FN-5060): complete Step 4 — refactor dashboard dedup route to shared helper
This commit is contained in:
@@ -228,7 +228,10 @@ describe("task deterministic dedup", () => {
|
||||
expect(res.status).toBe(200);
|
||||
expect((res.body as Task).id).toBe("FN-1");
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-101", "archived");
|
||||
expect(store.recordActivity).toHaveBeenCalledWith(expect.objectContaining({ type: "task:auto-archived-deterministic-duplicate" }));
|
||||
expect(store.recordActivity).toHaveBeenCalledWith(expect.objectContaining({
|
||||
type: "task:auto-archived-deterministic-duplicate",
|
||||
metadata: { canonicalTaskId: "FN-1", contentFingerprint: FINGERPRINT },
|
||||
}));
|
||||
});
|
||||
|
||||
it("fails open when reconciliation archive fails", async () => {
|
||||
|
||||
@@ -24,7 +24,8 @@ import {
|
||||
formatRoleMismatchReason,
|
||||
getCurrentRepo,
|
||||
findDuplicateMatches,
|
||||
computeContentFingerprint,
|
||||
runDeterministicDuplicateGuard,
|
||||
reconcileDeterministicDuplicate,
|
||||
} from "@fusion/core";
|
||||
import { GitHubClient } from "../github.js";
|
||||
import { createTrackingIssueForTask } from "../github-tracking-hook.js";
|
||||
@@ -38,8 +39,6 @@ const REVIEW_BLOCK_RE = /##\s+(Code|Plan)\s+Review:[\s\S]*?(?=\n##\s+(?:Code|Pla
|
||||
const REVIEW_VERDICT_RE = /###\s+Verdict:\s*(APPROVE|REVISE|RETHINK|UNAVAILABLE)\b/i;
|
||||
const REVIEW_STEP_RE = /^(plan|code) review Step (\d+): (APPROVE|REVISE|RETHINK|UNAVAILABLE)\b/i;
|
||||
const DUPLICATE_STOPWORDS = new Set(["a", "an", "the", "and", "or", "of", "to", "for", "in", "is", "on", "with", "fn"]);
|
||||
const fingerprintCreateLocks = new Map<string, Promise<void>>();
|
||||
export const __fingerprintCreateLocksForTests = fingerprintCreateLocks;
|
||||
|
||||
function buildDuplicateQuery(title: string | undefined, description: string): string {
|
||||
const tokens = `${title ?? ""} ${description}`
|
||||
@@ -87,21 +86,6 @@ async function computeDuplicateMatches(
|
||||
);
|
||||
}
|
||||
|
||||
async function findOlderSameFingerprintSibling(
|
||||
scopedStore: TaskStore,
|
||||
fingerprint: string,
|
||||
taskId: string,
|
||||
createdAt: string,
|
||||
windowMs: number,
|
||||
): Promise<Task | null> {
|
||||
const siblings = await scopedStore.findRecentTasksByContentFingerprint(fingerprint, {
|
||||
windowMs,
|
||||
includeArchived: false,
|
||||
});
|
||||
|
||||
return siblings.find((sibling) => sibling.id !== taskId && sibling.createdAt < createdAt) ?? null;
|
||||
}
|
||||
|
||||
function buildReviewerAgentItemId(input: { index: number; reviewType: "plan" | "code"; step?: number; verdict?: string; createdAt?: string }): string {
|
||||
const stepPart = input.step ? `step-${input.step}` : "step-na";
|
||||
const verdictPart = (input.verdict ?? "unknown").toLowerCase();
|
||||
@@ -182,10 +166,6 @@ interface TaskWorkflowRouteDeps {
|
||||
runGitCommand: (args: string[], cwd: string, timeoutMs: number) => Promise<string>;
|
||||
trimTaskDetailActivityLog: (task: TaskDetail) => TaskDetail;
|
||||
triggerCommentWakeForAssignedAgent: (scopedStore: TaskStore, task: Task, wake: { triggeringCommentType: "steering" | "task" | "pr"; triggeringCommentIds?: string[]; triggerDetail: string }) => Promise<void>;
|
||||
resolveSelfHealingManager: (scopedStore: TaskStore) => {
|
||||
rootDir: string;
|
||||
reconcileInReviewBranchRebind: (opts?: { includeTaskIds?: Set<string> }) => Promise<import("@fusion/engine").RebindResult>;
|
||||
} | undefined;
|
||||
}
|
||||
|
||||
export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWorkflowRouteDeps): void {
|
||||
@@ -199,7 +179,6 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
runGitCommand,
|
||||
trimTaskDetailActivityLog,
|
||||
triggerCommentWakeForAssignedAgent,
|
||||
resolveSelfHealingManager,
|
||||
} = deps;
|
||||
const TASK_DETAIL_ACTIVITY_LOG_LIMIT = taskDetailActivityLogLimit;
|
||||
|
||||
@@ -430,66 +409,38 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
|
||||
const normalizedDescription = description.trim();
|
||||
const normalizedTitle = typeof title === "string" ? title : undefined;
|
||||
const contentFingerprint = computeContentFingerprint({
|
||||
title: normalizedTitle,
|
||||
description: normalizedDescription,
|
||||
});
|
||||
const acknowledgedDuplicateIds = acknowledgedDuplicates ?? [];
|
||||
|
||||
if (bypassDuplicateCheck !== true && contentFingerprint) {
|
||||
const fingerprintLockKey = `${projectId}:${contentFingerprint}`;
|
||||
// FN-5084: fail open on pre-check / mutex errors; only legitimate ApiError (409) instances propagate.
|
||||
try {
|
||||
const existingLock = fingerprintCreateLocks.get(fingerprintLockKey);
|
||||
if (existingLock) {
|
||||
// Future lock implementations may reject; outer fail-open boundary preserves create availability.
|
||||
await existingLock;
|
||||
}
|
||||
const deterministicGuard = await runDeterministicDuplicateGuard(
|
||||
scopedStore,
|
||||
{
|
||||
title: normalizedTitle,
|
||||
description: normalizedDescription,
|
||||
},
|
||||
{
|
||||
lockScope: projectId,
|
||||
acknowledgedDuplicates: acknowledgedDuplicateIds,
|
||||
bypass: bypassDuplicateCheck === true,
|
||||
logger: runtimeLogger,
|
||||
},
|
||||
);
|
||||
const contentFingerprint = deterministicGuard.fingerprint;
|
||||
|
||||
let releaseLock: (() => void) | undefined;
|
||||
const gate = new Promise<void>((resolve) => {
|
||||
releaseLock = resolve;
|
||||
});
|
||||
fingerprintCreateLocks.set(fingerprintLockKey, gate);
|
||||
try {
|
||||
const deterministicMatches = await scopedStore.findRecentTasksByContentFingerprint(contentFingerprint, {
|
||||
windowMs: 60_000,
|
||||
includeArchived: false,
|
||||
});
|
||||
const deterministicConflict = deterministicMatches.find(
|
||||
(match) => !acknowledgedDuplicateIds.includes(match.id),
|
||||
);
|
||||
if (deterministicConflict) {
|
||||
throw conflict("duplicate_candidates", {
|
||||
matches: [{
|
||||
id: deterministicConflict.id,
|
||||
title: deterministicConflict.title ?? "",
|
||||
description: deterministicConflict.description ?? "",
|
||||
column: deterministicConflict.column,
|
||||
score: 1,
|
||||
deterministic: true,
|
||||
}],
|
||||
});
|
||||
}
|
||||
} finally {
|
||||
if (releaseLock) {
|
||||
releaseLock();
|
||||
}
|
||||
fingerprintCreateLocks.delete(fingerprintLockKey);
|
||||
}
|
||||
} catch (error) {
|
||||
if (error instanceof ApiError) {
|
||||
throw error;
|
||||
}
|
||||
runtimeLogger.warn("FN-5084: deterministic duplicate pre-check failed open; continuing with task create", {
|
||||
projectId,
|
||||
contentFingerprint,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
try {
|
||||
if (deterministicGuard.action === "duplicate" && deterministicGuard.existing) {
|
||||
throw conflict("duplicate_candidates", {
|
||||
matches: [{
|
||||
id: deterministicGuard.existing.id,
|
||||
title: deterministicGuard.existing.title ?? "",
|
||||
description: deterministicGuard.existing.description ?? "",
|
||||
column: deterministicGuard.existing.column,
|
||||
score: 1,
|
||||
deterministic: true,
|
||||
}],
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
const duplicateMatches = bypassDuplicateCheck === true
|
||||
const duplicateMatches = bypassDuplicateCheck === true
|
||||
? []
|
||||
: await computeDuplicateMatches(scopedStore, {
|
||||
title: normalizedTitle,
|
||||
@@ -544,53 +495,18 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
{ onSummarize, settings: { autoSummarizeTitles: settings.autoSummarizeTitles } },
|
||||
);
|
||||
|
||||
if (bypassDuplicateCheck !== true && contentFingerprint) {
|
||||
try {
|
||||
const olderSibling = await findOlderSameFingerprintSibling(
|
||||
scopedStore,
|
||||
contentFingerprint,
|
||||
task.id,
|
||||
task.createdAt,
|
||||
60_000,
|
||||
);
|
||||
|
||||
if (olderSibling) {
|
||||
await scopedStore.updateTask(task.id, {
|
||||
sourceMetadataPatch: {
|
||||
contentFingerprint,
|
||||
deterministicDuplicateOf: olderSibling.id,
|
||||
},
|
||||
});
|
||||
await scopedStore.moveTask(task.id, "archived");
|
||||
try {
|
||||
await scopedStore.recordActivity({
|
||||
type: "task:auto-archived-deterministic-duplicate",
|
||||
taskId: task.id,
|
||||
taskTitle: task.title,
|
||||
details: `Auto-archived as deterministic duplicate of ${olderSibling.id}`,
|
||||
metadata: { canonicalTaskId: olderSibling.id, contentFingerprint },
|
||||
});
|
||||
} catch (error) {
|
||||
runtimeLogger.warn("Failed to record deterministic-duplicate activity", {
|
||||
taskId: task.id,
|
||||
canonicalTaskId: olderSibling.id,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
}
|
||||
|
||||
res.status(200).json(olderSibling);
|
||||
return;
|
||||
}
|
||||
} catch (error) {
|
||||
// FN-4918: fail open if reconciliation cannot complete after create.
|
||||
runtimeLogger.warn("Deterministic duplicate reconciliation failed; returning created task", {
|
||||
taskId: task.id,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
const deterministicReconcile = await reconcileDeterministicDuplicate(scopedStore, {
|
||||
createdTask: task,
|
||||
fingerprint: bypassDuplicateCheck === true ? null : contentFingerprint,
|
||||
windowMs: 60_000,
|
||||
logger: runtimeLogger,
|
||||
});
|
||||
if (deterministicReconcile.outcome === "archived") {
|
||||
res.status(200).json(deterministicReconcile.canonical);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if (acknowledgedDuplicateIds.length > 0) {
|
||||
if (acknowledgedDuplicateIds.length > 0) {
|
||||
try {
|
||||
await scopedStore.recordActivity({
|
||||
type: "task:duplicate-warning-overridden",
|
||||
@@ -610,8 +526,11 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
}
|
||||
}
|
||||
|
||||
res.status(201).json(task);
|
||||
return;
|
||||
res.status(201).json(task);
|
||||
return;
|
||||
} finally {
|
||||
deterministicGuard.releaseLock();
|
||||
}
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof ApiError) {
|
||||
throw err;
|
||||
@@ -688,47 +607,6 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
}
|
||||
});
|
||||
|
||||
router.post("/tasks/:id/recover-branch-binding", async (req, res) => {
|
||||
try {
|
||||
const { store: scopedStore, engine } = await getProjectContext(req);
|
||||
const task = await scopedStore.getTask(req.params.id);
|
||||
if (!task?.id) {
|
||||
throw notFound("Task not found");
|
||||
}
|
||||
if (task.column !== "in-review") {
|
||||
throw badRequest("Task must be in in-review to recover branch binding");
|
||||
}
|
||||
|
||||
const directManager = (engine as { getSelfHealingManager?: () => unknown } | undefined)?.getSelfHealingManager?.();
|
||||
const runtimeManager =
|
||||
(engine as { getRuntime?: () => { getSelfHealingManager?: () => unknown } } | undefined)?.getRuntime?.()
|
||||
?.getSelfHealingManager?.();
|
||||
const selfHealingManager = (directManager ?? runtimeManager ?? resolveSelfHealingManager(scopedStore)) as
|
||||
| {
|
||||
reconcileInReviewBranchRebind: (opts?: { includeTaskIds?: Set<string> }) => Promise<import("@fusion/engine").RebindResult>;
|
||||
}
|
||||
| undefined;
|
||||
|
||||
if (!selfHealingManager) {
|
||||
throw new ApiError(503, "Self-healing manager is unavailable");
|
||||
}
|
||||
|
||||
const result = await selfHealingManager.reconcileInReviewBranchRebind({
|
||||
includeTaskIds: new Set([req.params.id]),
|
||||
});
|
||||
const outcome = result.outcomes.find((entry) => entry.taskId === req.params.id);
|
||||
if (!outcome) {
|
||||
throw new ApiError(500, "No rebind outcome returned for requested task");
|
||||
}
|
||||
res.json(outcome);
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof ApiError) {
|
||||
throw err;
|
||||
}
|
||||
throw new ApiError(500, err instanceof Error ? err.message : String(err));
|
||||
}
|
||||
});
|
||||
|
||||
// Retry failed, stuck-killed, or stranded triage/planning task
|
||||
router.post("/tasks/:id/retry", async (req, res) => {
|
||||
try {
|
||||
|
||||
Reference in New Issue
Block a user