feat(FN-5084): merge fusion/fn-5084
This commit is contained in:
5
.changeset/FN-5084-deterministic-precheck-fail-open.md
Normal file
5
.changeset/FN-5084-deterministic-precheck-fail-open.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Make the FN-4918 deterministic duplicate pre-check fail open: transient store query errors, mutex bookkeeping failures, and leader-lock rejections no longer 500 the `POST /tasks` endpoint. Legitimate 409 `duplicate_candidates` responses are unchanged.
|
||||
@@ -40,6 +40,12 @@ Dashboard `POST /tasks` now performs a pre-create duplicate gate using token-ove
|
||||
|
||||
This deterministic layer complements (does not replace) the FN-4829 similarity warning gate. `bypassDuplicateCheck: true` on `POST /tasks` disables both gates. FN-4892 remains a separate engine-side same-agent intake heuristic at triage finalize.
|
||||
|
||||
##### Fail-open boundary (FN-5084)
|
||||
|
||||
The deterministic **pre-check** now fails open: transient store query failures, in-process mutex bookkeeping failures, and leader-lock promise rejections do not block task creation. The route logs one `runtimeLogger.warn` line tagged `FN-5084` (including `projectId` and `contentFingerprint`) and continues through the FN-4829 similarity gate to normal create handling.
|
||||
|
||||
Only legitimate deterministic duplicate detections continue to propagate as `409` via `ApiError` from `conflict("duplicate_candidates", ...)`. The post-create FN-4918 reconciliation pass remains the second line of defense.
|
||||
|
||||
### Intake auto-archive (ghost-bug preflight + same-agent duplicate)
|
||||
|
||||
Fusion applies two conservative intake heuristics that may auto-archive newly filed tasks before execution starts:
|
||||
|
||||
@@ -2,7 +2,10 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import express from "express";
|
||||
import { computeContentFingerprint, type Column, type Task, type TaskStore } from "@fusion/core";
|
||||
import { registerTaskWorkflowRoutes } from "../routes/register-task-workflow-routes.js";
|
||||
import {
|
||||
__fingerprintCreateLocksForTests,
|
||||
registerTaskWorkflowRoutes,
|
||||
} from "../routes/register-task-workflow-routes.js";
|
||||
import { request as performRequest } from "../test-request.js";
|
||||
import { ApiError, sendErrorResponse } from "../api-error.js";
|
||||
|
||||
@@ -132,6 +135,14 @@ function buildApp(seed: Task[] = []) {
|
||||
}
|
||||
|
||||
describe("task deterministic dedup", () => {
|
||||
beforeEach(() => {
|
||||
__fingerprintCreateLocksForTests.clear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
__fingerprintCreateLocksForTests.clear();
|
||||
});
|
||||
|
||||
it("blocks sequential duplicate create with deterministic 409", async () => {
|
||||
const { app } = buildApp([
|
||||
mkTask({ id: "FN-1", title: TITLE, description: DESCRIPTION, column: "todo", source: { sourceType: "api", sourceMetadata: { contentFingerprint: FINGERPRINT } } }),
|
||||
@@ -231,4 +242,84 @@ describe("task deterministic dedup", () => {
|
||||
expect(res.status).toBe(201);
|
||||
expect(runtimeLogger.warn).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe("fail-open boundary (FN-5084)", () => {
|
||||
it("continues create when deterministic store query throws", async () => {
|
||||
const { app, store, runtimeLogger } = buildApp();
|
||||
const queryMock = store.findRecentTasksByContentFingerprint as ReturnType<typeof vi.fn>;
|
||||
queryMock.mockRejectedValueOnce(new Error("transient sqlite error"));
|
||||
|
||||
const res = await performRequest(app, "POST", "/api/tasks", JSON.stringify({ title: TITLE, description: DESCRIPTION }), { "content-type": "application/json" });
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
expect(store.createTask).toHaveBeenCalledTimes(1);
|
||||
expect(runtimeLogger.warn).toHaveBeenCalledTimes(1);
|
||||
expect(runtimeLogger.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining("FN-5084"),
|
||||
expect.objectContaining({
|
||||
contentFingerprint: FINGERPRINT,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps deterministic conflict as 409 and does not log fail-open warning", async () => {
|
||||
const { app, runtimeLogger } = buildApp([
|
||||
mkTask({ id: "FN-1", title: TITLE, description: DESCRIPTION, column: "todo", source: { sourceType: "api", sourceMetadata: { contentFingerprint: FINGERPRINT } } }),
|
||||
]);
|
||||
|
||||
const res = await performRequest(app, "POST", "/api/tasks", JSON.stringify({ title: TITLE, description: DESCRIPTION }), { "content-type": "application/json" });
|
||||
expect(res.status).toBe(409);
|
||||
expect(runtimeLogger.warn).not.toHaveBeenCalledWith(
|
||||
expect.stringContaining("FN-5084"),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it("continues create when leader lock promise rejects", async () => {
|
||||
const { app, store, runtimeLogger } = buildApp();
|
||||
const lockKey = `p-1:${FINGERPRINT}`;
|
||||
const rejectedLeaderLock = Promise.reject(new Error("leader lock failed"));
|
||||
rejectedLeaderLock.catch(() => {});
|
||||
__fingerprintCreateLocksForTests.set(lockKey, rejectedLeaderLock);
|
||||
|
||||
const res = await performRequest(app, "POST", "/api/tasks", JSON.stringify({ title: TITLE, description: DESCRIPTION }), { "content-type": "application/json" });
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
expect(store.createTask).toHaveBeenCalledTimes(1);
|
||||
expect(runtimeLogger.warn).toHaveBeenCalledTimes(1);
|
||||
expect(runtimeLogger.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining("FN-5084"),
|
||||
expect.objectContaining({ contentFingerprint: FINGERPRINT }),
|
||||
);
|
||||
});
|
||||
|
||||
it("releases lock on fail-open path so follow-up request resolves", async () => {
|
||||
const { app, store } = buildApp();
|
||||
const queryMock = store.findRecentTasksByContentFingerprint as ReturnType<typeof vi.fn>;
|
||||
queryMock.mockRejectedValueOnce(new Error("transient sqlite error"));
|
||||
|
||||
const first = await performRequest(app, "POST", "/api/tasks", JSON.stringify({ title: TITLE, description: DESCRIPTION }), { "content-type": "application/json" });
|
||||
const second = await performRequest(app, "POST", "/api/tasks", JSON.stringify({ title: TITLE, description: DESCRIPTION }), { "content-type": "application/json" });
|
||||
|
||||
expect(first.status).toBe(201);
|
||||
expect([201, 200, 409]).toContain(second.status);
|
||||
});
|
||||
|
||||
it("skips deterministic lookup when bypassDuplicateCheck is true", async () => {
|
||||
const { app, store } = buildApp();
|
||||
const queryMock = store.findRecentTasksByContentFingerprint as ReturnType<typeof vi.fn>;
|
||||
queryMock.mockRejectedValue(new Error("transient sqlite error"));
|
||||
|
||||
const res = await performRequest(
|
||||
app,
|
||||
"POST",
|
||||
"/api/tasks",
|
||||
JSON.stringify({ title: TITLE, description: DESCRIPTION, bypassDuplicateCheck: true }),
|
||||
{ "content-type": "application/json" },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
expect(queryMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -39,6 +39,7 @@ const REVIEW_VERDICT_RE = /###\s+Verdict:\s*(APPROVE|REVISE|RETHINK|UNAVAILABLE)
|
||||
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}`
|
||||
@@ -432,41 +433,54 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
|
||||
if (bypassDuplicateCheck !== true && contentFingerprint) {
|
||||
const fingerprintLockKey = `${projectId}:${contentFingerprint}`;
|
||||
const existingLock = fingerprintCreateLocks.get(fingerprintLockKey);
|
||||
if (existingLock) {
|
||||
await existingLock;
|
||||
}
|
||||
|
||||
let releaseLock: (() => void) | undefined;
|
||||
const gate = new Promise<void>((resolve) => {
|
||||
releaseLock = resolve;
|
||||
});
|
||||
fingerprintCreateLocks.set(fingerprintLockKey, gate);
|
||||
// FN-5084: fail open on pre-check / mutex errors; only legitimate ApiError (409) instances propagate.
|
||||
try {
|
||||
const deterministicMatches = await scopedStore.findRecentTasksByContentFingerprint(contentFingerprint, {
|
||||
windowMs: 60_000,
|
||||
includeArchived: false,
|
||||
const existingLock = fingerprintCreateLocks.get(fingerprintLockKey);
|
||||
if (existingLock) {
|
||||
// Future lock implementations may reject; outer fail-open boundary preserves create availability.
|
||||
await existingLock;
|
||||
}
|
||||
|
||||
let releaseLock: (() => void) | undefined;
|
||||
const gate = new Promise<void>((resolve) => {
|
||||
releaseLock = resolve;
|
||||
});
|
||||
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,
|
||||
}],
|
||||
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);
|
||||
}
|
||||
} finally {
|
||||
if (releaseLock) {
|
||||
releaseLock();
|
||||
} catch (error) {
|
||||
if (error instanceof ApiError) {
|
||||
throw error;
|
||||
}
|
||||
fingerprintCreateLocks.delete(fingerprintLockKey);
|
||||
runtimeLogger.warn("FN-5084: deterministic duplicate pre-check failed open; continuing with task create", {
|
||||
projectId,
|
||||
contentFingerprint,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -22,7 +22,7 @@ const qualityAppTests = [
|
||||
const qualityApiTests = [
|
||||
// Critical HTTP/server behavior: auth, task/project/settings mutation,
|
||||
// git/GitHub, agents, nodes, chat/files, realtime, and isolation guards.
|
||||
"src/__tests__/{api-error,auth-middleware,auth-middleware-integration,chat-attachment-routes,chat-routes,file-service,github,github-webhooks,initialize,planning-flow-diagnostics-guardrail,pr-routes-auto-merge,pr-routes.contract,project-routes,project-store-resolver,remote-access-routes,remote-auth,routes-agent-budget,routes-agent-keys,routes-agent-permissions,routes-agent-ratings,routes-agent-runs,routes-agent-soul-memory,routes-agents,routes-automation,routes-git,routes-github,routes-nodes,routes-nodes-sync-contract,routes-secrets-sync,routes-settings,routes-task-commit-associations,routes-tasks,routes-tasks-duplicate-check,server,server-static-assets,server-webhook,server.events,setup-routes,sse,sse-buffer,test-isolation-guard,update-check-route,websocket}.test.ts",
|
||||
"src/__tests__/{api-error,auth-middleware,auth-middleware-integration,chat-attachment-routes,chat-routes,file-service,github,github-webhooks,initialize,planning-flow-diagnostics-guardrail,pr-routes-auto-merge,pr-routes.contract,project-routes,project-store-resolver,remote-access-routes,remote-auth,routes-agent-budget,routes-agent-keys,routes-agent-permissions,routes-agent-ratings,routes-agent-runs,routes-agent-soul-memory,routes-agents,routes-automation,routes-git,routes-github,routes-nodes,routes-nodes-sync-contract,routes-secrets-sync,routes-settings,routes-task-commit-associations,routes-tasks,routes-tasks-deterministic-dedup,routes-tasks-duplicate-check,server,server-static-assets,server-webhook,server.events,setup-routes,sse,sse-buffer,test-isolation-guard,update-check-route,websocket}.test.ts",
|
||||
"src/routes/__tests__/{custom-provider-routes,custom-providers,register-docker-node-routes,stash-recovery-routes}.test.ts",
|
||||
];
|
||||
|
||||
|
||||
Reference in New Issue
Block a user