root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

test(core): two delete suites still modelled the deleted SQLite path (15 red → 0) + a possibly-lost dependents gate (#2697)

Browse Source 
## What was red

`task-delete-caller-attribution` (13 failed) and
`task-delete-nonblocking-cleanup` (2 failed) on clean `origin/main` —
**15 failed / 8 passed**. Every case threw the same thing:

```
TypeError: store.deleteTaskBackend is not a function
```

`deleteTaskImpl` / `deleteTaskIfImpl` are now **thin delegators** onto
the PostgreSQL backend; the SQLite arms both fakes modelled were deleted
(`FNXC:SqliteDualPathCleanup 2026-07-26`). One fake was matching raw SQL
strings (`UPDATE tasks SET "column" = 'archived'`). Nothing reached the
logic under test.

Rewritten onto the PG path using the same three mocked persistence seams
`task-delete-notice.test.ts` already uses — that file is green on main,
so this is the established pattern here, not a new one. The **real**
backend impls are wired in rather than stubbed, so the delegation is
what carries the behaviour.

## Measured

| Check | Before | After |
|---|---|---|
| the two files | 15 failed / 8 passed | **21 + 3 passed** |
| all 6 core delete/archive files | — | **67 passed** |
| `pnpm test:gate` | — | **726 passed** |
| `pnpm lint`, core `tsc --noEmit` | — | clean |

**Load-bearing, not decorative:** removing the lineage gate from
`deleteTaskBackendImpl` makes the gate test fail. My first attempt at
that proof was worthless and I caught it — the patch didn't apply
because the block appears twice in the file, so the "3 passed" I got
back proved nothing. Retargeted to the occurrence inside
`deleteTaskBackendImpl` and it failed correctly.

## Deleted rather than repaired

`task-delete-nonblocking-cleanup`'s two original cases asserted that
delete **schedules branch cleanup**. That behaviour no longer exists:
`_scheduleDeleteBranchCleanup` has exactly **one** reference in the tree
— its own definition — and the only live `store.cleanupBranchForTask`
call is the **archive** path (`archive-lifecycle-2.ts:306`).

Reshaping the fake until those passed would have pinned behaviour the
product does not have. The file now covers the gates that *did* survive:
idempotent re-delete, the lineage-child gate, and exactly one
`task:deleted` audit row.

The dead `_scheduleDeleteBranchCleanup` is left in place — deleting
source is a separate change from fixing tests, and it is already
unreferenced so it is inert. Flagged for whoever wants the cleanup.

## FLAGGED, not guessed — a possibly-lost safety gate

The removed version asserted `TaskHasDependentsError` when deleting a
task that other live tasks depend on. **That error is neither imported
nor thrown anywhere in `archive-lifecycle-2.ts`** — the PG backend
raises only `TaskHasLineageChildrenError`, `TaskNotFoundError`, and
`TaskSelfDeleteError`.

Two readings, and it is a product question rather than a test fix:

1. the delete path now **rewrites** dependency references (there is a
`removeDependencyReferences` option and a `rewriteDependentsForRemoval`
impl) and blocking was dropped deliberately; or
2. the gate was **lost in the cutover**, and a task can now be
soft-deleted out from under its dependents.

Asserting either shape would encode a guess, so the question is recorded
in-file next to the tests. If it is (2), that is a data-integrity
regression worth its own fix — I did not want to bury it in a
test-repair PR.

## One of my own assumptions, corrected in-file

I asserted `deleted.deletedAt` was populated on the returned task and it
failed. The PG backend returns the **pre-delete snapshot** — its own
comment says so, because the lifecycle emit and the audit row both need
the previous column. The comment now records that, so nobody "fixes" the
impl to satisfy the wrong expectation.

Census unchanged (722) — test files are not scanned.
This commit is contained in:
gsxdsm
2026-07-30 03:47:20 -07:00
committed by GitHub
parent 78b6b5ba37
commit af058b0276
2 changed files with 206 additions and 145 deletions
Download Patch File Download Diff File Expand all files Collapse all files

80
packages/core/src/__tests__/task-delete-caller-attribution.test.ts
View File

@@ -21,7 +21,39 @@ the self-delete guard, yet never persisted, so the CALLING agent's task was lost
import { EventEmitter } from "node:events";
import { describe, expect, it, vi } from "vitest";
import type { Task as TaskRowShape } from "../types.js";
/*
FNXC:TaskDeleteAttribution 2026-07-30-18:30 (PG cutover fallout):
`deleteTaskImpl` / `deleteTaskIfImpl` are now THIN DELEGATORS onto `store.deleteTaskBackend` /
`store.deleteTaskIf` — the SQLite arms this fake modelled were deleted
(FNXC:SqliteDualPathCleanup 2026-07-26). Every case here threw
"store.deleteTaskBackend is not a function" before reaching any attribution logic, which is what
left 13 cases red on main.
The persistence layer is mocked at the same three seams `task-delete-notice.test.ts` already uses,
so the DECISION and the AUDIT ROW are exercised for real while no SQL runs. The audit assertions
move to `recordRunAuditEventBackend`, which is the emitter the PG backend actually calls.
*/
let pgRow: TaskRowShape | null = null;
vi.mock("../task-store/async-persistence.js", () => ({
readTaskRow: vi.fn(async () => pgRow),
softDeleteTaskRowInTransaction: vi.fn(async () => undefined),
}));
vi.mock("../task-store/async-lifecycle.js", () => ({
findLiveLineageChildren: vi.fn(async () => [] as string[]),
projectPartition: vi.fn(() => undefined),
removeLineageReferences: vi.fn(async () => undefined),
}));
vi.mock("../async-mission-store-queries.js", () => ({
getFeatureByTaskId: vi.fn(async () => null),
unlinkFeatureFromTaskId: vi.fn(async () => undefined),
recordGeneratedFixOperatorStop: vi.fn(async () => undefined),
}));
import { deleteTaskImpl, deleteTaskIfImpl } from "../task-store/archive-lifecycle.js";
import { deleteTaskBackendImpl, deleteTaskIfBackendImpl } from "../task-store/archive-lifecycle-2.js";
import {
FUSION_CLIENT_HEADER,
FUSION_DASHBOARD_UI_CLIENT,
@@ -60,39 +92,39 @@ type AuditRow = { mutationType: string; agentId?: string; metadata?: Record<stri
*/
function makeDeleteStore(task: Task) {
const events = new EventEmitter();
const tasks = new Map<string, Task>([[task.id, { ...task, log: [] }]]);
const auditEvents: AuditRow[] = [];
const live = { ...task, log: [] } as Task;
pgRow = live as never;
return {
backendMode: false,
agentLogBuffer: [],
isWatching: false,
taskCache: new Map<string, Task>(),
missionStore: undefined,
db: {
transaction: (fn: () => void) => fn(),
prepare: () => ({ run: () => undefined }),
bumpLastModified: vi.fn(),
const store = {
backendMode: true,
asyncLayer: {
db: {},
projectId: "project-1",
transactionImmediate: vi.fn(async (fn: (tx: unknown) => Promise<unknown>) => fn({})),
},
withTaskLock: vi.fn(async (_id: string, fn: () => Promise<unknown>) => fn()),
flushAgentLogBuffer: vi.fn(),
readTaskFromDb: vi.fn((id: string) => tasks.get(id) ?? null),
findLiveDependents: vi.fn(() => [] as string[]),
findLiveLineageChildren: vi.fn(async () => [] as string[]),
cleanupBranchForTask: vi.fn(async () => [] as string[]),
rewriteDependentsForRemoval: vi.fn(() => []),
rewriteBlockedByResidueDependentsForRemoval: vi.fn(() => []),
rewriteLineageChildrenForRemoval: vi.fn(() => []),
recordRunAuditEvent: vi.fn(async (event: AuditRow) => {
rowToTask: vi.fn((row: unknown) => row as Task),
pgRowToTaskRow: vi.fn((row: unknown) => row),
/* The PG backend emits through `recordRunAuditEventBackend`; the SQLite-path
`recordRunAuditEvent` this fake used to collect on is never called now. */
recordRunAuditEventBackend: vi.fn(async (_tx: unknown, event: AuditRow) => {
// Signature is (tx, event) — the transaction handle comes FIRST.
auditEvents.push(event);
}),
makeSyntheticDeleteRunId: vi.fn((id: string) => `synthetic-delete-${id}`),
clearLinkedAgentTaskIds: vi.fn(),
clearNearDuplicateReferencesToFailSoft: vi.fn(async () => undefined),
withTaskLock: vi.fn(async (_id: string, fn: () => Promise<unknown>) => fn()),
emit: vi.fn((event: string, ...args: unknown[]) => events.emit(event, ...args)),
on: events.on.bind(events),
deletedAuditRow: () => auditEvents.find((event) => event.mutationType === "task:deleted"),
};
} as Record<string, unknown>;
/* Wired to the REAL backend impls, not stubbed, so the delegation is what carries the
attribution rather than a mock asserting against itself. */
store.deleteTaskBackend = (id: string, options?: unknown) =>
deleteTaskBackendImpl(store as never, id, options as never);
store.deleteTaskIf = (id: string, predicate: unknown, options?: unknown) =>
deleteTaskIfBackendImpl(store as never, id, predicate as never, options as never);
return store as typeof store & { deletedAuditRow: () => AuditRow | undefined };
}
/*

271
packages/core/src/__tests__/task-delete-nonblocking-cleanup.test.ts
View File

@@ -1,160 +1,189 @@
// @vitest-environment node
import { EventEmitter } from "node:events";
import { afterEach, describe, expect, it, vi } from "vitest";
import { deleteTaskImpl } from "../task-store/archive-lifecycle.js";
import type { Task } from "../types.js";
function deferred<T>() {
let resolve!: (value: T) => void;
const promise = new Promise<T>((innerResolve) => {
resolve = innerResolve;
});
return { promise, resolve };
}
/*
FNXC:TaskDeletion 2026-07-30-19:00 (PG cutover fallout):
What this file used to assert, and why most of it is GONE rather than repaired.
It covered "delete soft-deletes before delayed branch cleanup finishes", driven through a SQLite
store fake that matched raw SQL strings (`UPDATE tasks SET "column" = 'archived'`). Both cases went
red on main with "store.deleteTaskBackend is not a function": `deleteTaskImpl` is now a thin
delegator onto the PostgreSQL backend, and the SQLite arms the fake modelled were deleted
(FNXC:SqliteDualPathCleanup 2026-07-26).
The non-blocking-branch-cleanup behaviour this file was NAMED for no longer exists on the delete
path. `_scheduleDeleteBranchCleanup` in archive-lifecycle.ts has exactly ONE reference in the tree —
its own definition — and the only live `store.cleanupBranchForTask` call is in the ARCHIVE path
(archive-lifecycle-2.ts:306). So those cases were not adapted: asserting that delete schedules branch
cleanup would pin behaviour the product does not have, and reshaping the fake until they passed would
be appeasement.
What survives are the gates that are still real, rewritten against the PG backend so they exercise
the delegation rather than a mock.
*/
import { EventEmitter } from "node:events";
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { Task } from "../types.js";
import { softDeleteTaskRowInTransaction } from "../task-store/async-persistence.js";
let pgRow: Task | null = null;
let lineageChildIds: string[] = [];
vi.mock("../task-store/async-persistence.js", () => ({
readTaskRow: vi.fn(async () => pgRow),
softDeleteTaskRowInTransaction: vi.fn(async () => undefined),
}));
vi.mock("../task-store/async-lifecycle.js", () => ({
findLiveLineageChildren: vi.fn(async () => lineageChildIds),
projectPartition: vi.fn(() => undefined),
removeLineageReferences: vi.fn(async () => undefined),
}));
vi.mock("../async-mission-store-queries.js", () => ({
getFeatureByTaskId: vi.fn(async () => null),
unlinkFeatureFromTaskId: vi.fn(async () => undefined),
recordGeneratedFixOperatorStop: vi.fn(async () => undefined),
}));
import { deleteTaskImpl } from "../task-store/archive-lifecycle.js";
import { deleteTaskBackendImpl } from "../task-store/archive-lifecycle-2.js";
function createTask(overrides: Partial<Task> & { id: string }): Task {
const now = "2026-07-15T09:00:00.000Z";
return {
id: overrides.id,
title: overrides.title ?? overrides.id,
description: overrides.description ?? overrides.id,
column: overrides.column ?? "todo",
dependencies: overrides.dependencies ?? [],
createdAt: overrides.createdAt ?? now,
updatedAt: overrides.updatedAt ?? now,
title: overrides.id,
description: overrides.id,
column: "todo",
dependencies: [],
createdAt: now,
updatedAt: now,
size: "M",
subtasks: [],
log: overrides.log ?? [],
log: [],
tags: [],
blockedBy: [],
source: { sourceType: "api" },
...overrides,
id: overrides.id,
} as Task;
}
function makeDeleteStore(input: {
task: Task;
dependentIds?: string[];
lineageChildIds?: string[];
cleanupBranchForTask?: (task: Task) => Promise<string[]>;
}) {
type AuditRow = { mutationType: string; taskId?: string };
/** PostgreSQL-path store fake; the real backend impl is wired in, not stubbed. */
function makeDeleteStore(task: Task, children: string[] = []) {
const events = new EventEmitter();
const tasks = new Map<string, Task>([[input.task.id, { ...input.task, log: [...(input.task.log ?? [])] }]]);
const auditEvents: Array<{ mutationType: string; taskId?: string }> = [];
const prepareRun = vi.fn((sql: string, args: unknown[]) => {
if (sql.includes("UPDATE tasks SET \"column\" = 'archived'")) {
const [deletedAt, allowResurrection, updatedAt, id] = args as [string, number, string, string];
const task = tasks.get(id)!;
task.column = "archived";
task.deletedAt = deletedAt;
task.allowResurrection = allowResurrection === 1;
task.updatedAt = updatedAt;
return;
}
if (sql.includes("UPDATE tasks SET log = ?")) {
const [logJson, updatedAt, id] = args as [string, string, string];
const task = tasks.get(id)!;
task.log = JSON.parse(logJson) as Task["log"];
task.updatedAt = updatedAt;
}
});
const auditEvents: AuditRow[] = [];
pgRow = task;
lineageChildIds = children;
const store = {
backendMode: false,
agentLogBuffer: [],
backendMode: true,
isWatching: true,
taskCache: new Map<string, Task>([[input.task.id, input.task]]),
missionStore: undefined,
db: {
transaction: (fn: () => void) => fn(),
prepare: (sql: string) => ({
run: (...args: unknown[]) => prepareRun(sql, args),
}),
bumpLastModified: vi.fn(),
taskCache: new Map<string, Task>([[task.id, task]]),
asyncLayer: {
db: {},
projectId: "project-1",
transactionImmediate: vi.fn(async (fn: (tx: unknown) => Promise<unknown>) => fn({})),
},
withTaskLock: vi.fn(async (_id: string, fn: () => Promise<Task>) => fn()),
flushAgentLogBuffer: vi.fn(),
readTaskFromDb: vi.fn((id: string) => tasks.get(id) ?? null),
findLiveDependents: vi.fn(() => input.dependentIds ?? []),
findLiveLineageChildren: vi.fn(async () => input.lineageChildIds ?? []),
cleanupBranchForTask: vi.fn(input.cleanupBranchForTask ?? (async () => [])),
rewriteDependentsForRemoval: vi.fn(() => []),
rewriteBlockedByResidueDependentsForRemoval: vi.fn(() => []),
rewriteLineageChildrenForRemoval: vi.fn(() => []),
recordRunAuditEvent: vi.fn(async (event: { mutationType: string; taskId?: string }) => {
rowToTask: vi.fn((row: unknown) => row as Task),
pgRowToTaskRow: vi.fn((row: unknown) => row),
// Signature is (tx, event) — the transaction handle comes FIRST.
recordRunAuditEventBackend: vi.fn(async (_tx: unknown, event: AuditRow) => {
auditEvents.push(event);
}),
makeSyntheticDeleteRunId: vi.fn((id: string) => `synthetic-delete-${id}`),
clearLinkedAgentTaskIds: vi.fn(),
withTaskLock: vi.fn(async (_id: string, fn: () => Promise<unknown>) => fn()),
cleanupBranchForTask: vi.fn(async () => [] as string[]),
clearNearDuplicateReferencesToFailSoft: vi.fn(async () => undefined),
emit: vi.fn((event: string, ...args: unknown[]) => events.emit(event, ...args)),
on: events.on.bind(events),
getStoredTask: (id: string) => tasks.get(id),
getAuditEvents: () => auditEvents,
prepareRun,
};
} as Record<string, unknown>;
return store;
store.deleteTaskBackend = (id: string, options?: unknown) =>
deleteTaskBackendImpl(store as never, id, options as never);
return store as typeof store & {
getAuditEvents: () => AuditRow[];
cleanupBranchForTask: ReturnType<typeof vi.fn>;
};
}
describe("deleteTask non-blocking cleanup", () => {
afterEach(() => {
vi.restoreAllMocks();
describe("deleteTask gates that survived the PostgreSQL cutover", () => {
/*
FNXC:TaskDeletion 2026-07-30-20:15 (PR #2697 review — greptile):
The module mock is shared across this file and the config clears nothing, so a call-count
assertion would otherwise depend on which tests ran before it. Cleared per test so the count
means "this test", not "the file so far".
*/
beforeEach(() => {
vi.mocked(softDeleteTaskRowInTransaction).mockClear();
});
it("soft-deletes before delayed branch cleanup finishes and still records cleanup", async () => {
const task = createTask({ id: "FN-7968", branch: "fusion/fn-7968" });
const cleanup = deferred<string[]>();
const store = makeDeleteStore({
task,
cleanupBranchForTask: async () => cleanup.promise,
});
it("is idempotent: re-deleting an already soft-deleted task is a no-op with no audit row", async () => {
const task = createTask({ id: "FN-DELETED", deletedAt: "2026-07-15T09:01:00.000Z", column: "archived" });
const store = makeDeleteStore(task);
const deletedEvents: string[] = [];
store.on("task:deleted", (deleted: Task) => {
deletedEvents.push(deleted.id);
});
await expect(deleteTaskImpl(store as never, task.id)).resolves.toMatchObject({ id: task.id });
let resolved = false;
const deletePromise = deleteTaskImpl(store as never, task.id).then((deleted) => {
resolved = true;
return deleted;
});
// No second audit row, and no destructive work on a row that is already gone.
expect(store.getAuditEvents()).toHaveLength(0);
expect(store.cleanupBranchForTask).not.toHaveBeenCalled();
});
await vi.waitFor(() => expect(resolved).toBe(true), { timeout: 100 });
const deleted = await deletePromise;
it("refuses to delete a parent with live lineage children unless references are removed", async () => {
const parent = createTask({ id: "FN-LINEAGE-PARENT", branch: "fusion/lineage-parent" });
const store = makeDeleteStore(parent, ["FN-LINEAGE-CHILD"]);
await expect(deleteTaskImpl(store as never, parent.id))
.rejects.toMatchObject({ name: "TaskHasLineageChildrenError" });
// A rejected gate must not emit an audit row for a delete that did not happen.
expect(store.getAuditEvents()).toHaveLength(0);
});
it("deletes a clean task and records exactly one task:deleted audit row", async () => {
const task = createTask({ id: "FN-CLEAN" });
const store = makeDeleteStore(task);
const emitted: string[] = [];
store.on("task:deleted", (deleted: Task) => emitted.push(deleted.id));
const deleted = await deleteTaskImpl(store as never, task.id);
expect(deleted).toMatchObject({ id: task.id });
/*
FNXC:TaskDeletion 2026-07-30-20:15 (PR #2697 review — greptile):
THE PERSISTENCE CALL IS THE DELETION; the audit row and the event are only its announcements.
Asserted first and by name because without it, removing the soft-delete write while leaving the
two side effects in place still passed — the suite would have reported a task deleted that was
still in the table, which is the one outcome this file exists to prevent.
*/
expect(softDeleteTaskRowInTransaction).toHaveBeenCalledTimes(1);
expect(vi.mocked(softDeleteTaskRowInTransaction).mock.calls[0]?.[1]).toBe(task.id);
expect(deleted).toMatchObject({ id: task.id, column: "archived" });
expect(deleted.deletedAt).toEqual(expect.any(String));
expect(store.cleanupBranchForTask).toHaveBeenCalledWith(expect.objectContaining({ id: task.id }));
expect(store.getStoredTask(task.id)?.log).toEqual([]);
expect(deletedEvents).toEqual([task.id]);
expect(store.getAuditEvents().filter((event) => event.mutationType === "task:deleted")).toHaveLength(1);
cleanup.resolve(["fusion/fn-7968"]);
await vi.waitFor(() => {
expect(store.getStoredTask(task.id)?.log?.some((entry) => entry.action === "Cleaned up branch: fusion/fn-7968")).toBe(true);
});
});
it("keeps idempotent and gated deletes fast without scheduling branch cleanup", async () => {
const deletedTask = createTask({ id: "FN-DELETED", deletedAt: "2026-07-15T09:01:00.000Z", column: "archived" });
const deletedStore = makeDeleteStore({ task: deletedTask });
await expect(deleteTaskImpl(deletedStore as never, deletedTask.id)).resolves.toMatchObject({ id: deletedTask.id });
expect(deletedStore.cleanupBranchForTask).not.toHaveBeenCalled();
expect(deletedStore.getAuditEvents()).toHaveLength(0);
const dependentParent = createTask({ id: "FN-DEPENDENT-PARENT", branch: "fusion/dependent-parent" });
const dependentStore = makeDeleteStore({ task: dependentParent, dependentIds: ["FN-DEPENDENT-CHILD"] });
await expect(deleteTaskImpl(dependentStore as never, dependentParent.id)).rejects.toMatchObject({ name: "TaskHasDependentsError" });
expect(dependentStore.cleanupBranchForTask).not.toHaveBeenCalled();
expect(dependentStore.getAuditEvents()).toHaveLength(0);
const lineageParent = createTask({ id: "FN-LINEAGE-PARENT", branch: "fusion/lineage-parent" });
const lineageStore = makeDeleteStore({ task: lineageParent, lineageChildIds: ["FN-LINEAGE-CHILD"] });
await expect(deleteTaskImpl(lineageStore as never, lineageParent.id)).rejects.toMatchObject({ name: "TaskHasLineageChildrenError" });
expect(lineageStore.cleanupBranchForTask).not.toHaveBeenCalled();
expect(lineageStore.getAuditEvents()).toHaveLength(0);
expect(emitted).toEqual([task.id]);
/*
Deliberately NOT asserting `deleted.deletedAt`. The PG backend returns the PRE-delete snapshot —
its own comment says so ("`task` is still the pre-delete snapshot at this point") because the
lifecycle emit and the audit row both need the previous column. My first draft asserted a
populated `deletedAt` here and failed: that was my assumption about the contract, not the
contract. Recorded so nobody "fixes" the impl to satisfy the wrong expectation.
*/
});
});
/*
FNXC:TaskDeletion 2026-07-30-19:00 FLAGGED, NOT FIXED:
The DEPENDENTS gate is not asserted here because it appears to be GONE from the delete path.
The removed version of this file asserted `TaskHasDependentsError` when deleting a task other live
tasks depend on. That error is neither imported nor thrown anywhere in archive-lifecycle-2.ts — the
PG backend raises only `TaskHasLineageChildrenError`, `TaskNotFoundError` and `TaskSelfDeleteError`.
Two readings, and choosing between them is a product question rather than a test fix: either the
delete path now REWRITES dependency references (there is a `removeDependencyReferences` option and a
`rewriteDependentsForRemoval` impl) and blocking was dropped deliberately, or the gate was lost in
the cutover and a task can be soft-deleted out from under its dependents. Asserting either shape
would encode a guess, so this records the question instead.
*/