fix(triage): prevent orphaned deps when splitting tasks + detect worktree drift
Root cause: during a triage split the AI could set a child task's `dependencies` to the parent id. The parent is hard-deleted after the split, and the scheduler's dep check treats a missing id as unmet — permanently blocking the dependent. This stranded FN-2164 behind the deleted FN-2163. - core/store.deleteTask: refuse to delete when any live task still has the id in its `dependencies` array. Throws TaskHasDependentsError listing dependents so callers can rewrite or recover. Covers the triage-split path and any future caller. - engine/triage task_create: validate each proposed dependency before creating a child — reject the parent id, reject unknown task ids, allow siblings created earlier in the same split or pre-existing tasks. - engine/triage split cleanup: wrap the parent deleteTask in try/catch that keeps the parent alive (safer than stranding dependents) and logs the reason. - engine/triage prompts: both the mandatory-split and proactive-split prompts now explicitly state that subtask deps must never reference the parent. - dashboard/routes /subtasks/create-tasks: reject parent-id deps, drop unknown deps with an audit log entry, surface parentTaskCloseError + droppedDependencies in the response instead of silently swallowing them. - engine/executor: on execute entry, detect the drift state (in-progress task with no worktree) and emit a loud log + task log entry; the existing fresh-worktree path then recovers. Prevents silent "operating without a worktree" behavior that we saw on FN-2152. Tests: core: 2907/2907 pass (+5 new, incl. deleteTask guard regression) engine: 2554/2554 pass (+17 new, incl. task_create dep validation) dashboard: 9064/9064 pass (+2 new for /subtasks/create-tasks). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -23,7 +23,7 @@ const mockedExecSync = vi.mocked(execSync);
|
||||
import { runCommandAsync } from "./run-command.js";
|
||||
const mockedRunCommandAsync = vi.mocked(runCommandAsync);
|
||||
|
||||
import { TaskStore } from "./store.js";
|
||||
import { TaskStore, TaskHasDependentsError } from "./store.js";
|
||||
import { appendFile, readFile, writeFile, mkdir, rm, readdir, unlink } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { mkdtempSync, existsSync } from "node:fs";
|
||||
@@ -3888,6 +3888,50 @@ Task with acceptance criteria
|
||||
expect(logs).toEqual([]);
|
||||
});
|
||||
|
||||
it("deleteTask refuses when another live task depends on this id", async () => {
|
||||
// Regression for the triage-split bug: splitting a parent into children
|
||||
// used to hard-delete the parent even when a child carried the parent id
|
||||
// in its dependencies array, permanently blocking the child because the
|
||||
// scheduler treats missing-dep ids as unmet.
|
||||
const parent = await store.createTask({ description: "Parent to be split" });
|
||||
const child = await store.createTask({
|
||||
description: "Child that accidentally depends on parent",
|
||||
});
|
||||
await store.updateTask(child.id, { dependencies: [parent.id] });
|
||||
|
||||
await expect(store.deleteTask(parent.id)).rejects.toBeInstanceOf(TaskHasDependentsError);
|
||||
|
||||
// Parent must still exist so the dependent isn't stranded.
|
||||
const stillThere = await store.getTask(parent.id);
|
||||
expect(stillThere.id).toBe(parent.id);
|
||||
|
||||
// The error must name the dependent so callers/logs can triage it.
|
||||
try {
|
||||
await store.deleteTask(parent.id);
|
||||
} catch (err) {
|
||||
expect(err).toBeInstanceOf(TaskHasDependentsError);
|
||||
expect((err as TaskHasDependentsError).dependentIds).toContain(child.id);
|
||||
}
|
||||
|
||||
// After the dependent's reference is removed, delete succeeds.
|
||||
await store.updateTask(child.id, { dependencies: [] });
|
||||
await expect(store.deleteTask(parent.id)).resolves.toMatchObject({ id: parent.id });
|
||||
});
|
||||
|
||||
it("deleteTask allows deletion when a similarly-named id contains the target (substring false-positive guard)", async () => {
|
||||
// The LIKE probe uses '%id%'; ensure we don't misidentify e.g. FN-1 as
|
||||
// referencing FN-10 just because the id string appears inside a JSON
|
||||
// array containing "FN-10".
|
||||
const targetTask = await store.createTask({ description: "Target" }); // e.g. FN-001
|
||||
const similarId = `${targetTask.id}X`; // definitely not a real task id
|
||||
const other = await store.createTask({ description: "Other" });
|
||||
await store.updateTask(other.id, { dependencies: [similarId] });
|
||||
|
||||
// Should NOT throw — the LIKE probe's string match is disambiguated by
|
||||
// JSON.parse + array.includes.
|
||||
await expect(store.deleteTask(targetTask.id)).resolves.toMatchObject({ id: targetTask.id });
|
||||
});
|
||||
|
||||
it("deleting a task cascades agent log entry deletion", async () => {
|
||||
const task = await createTestTask();
|
||||
await store.appendAgentLog(task.id, "cascade me", "text");
|
||||
|
||||
Reference in New Issue
Block a user