Revert "feat(KB-140): add task_add_dep tool to executor"
This reverts commit a9dae834a59b60701874ee09e0b1f86d0d98b3e2.
This commit is contained in:
@@ -2017,184 +2017,3 @@ describe("Plan RETHINK verdict handling", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ── task_add_dep tool tests ──────────────────────────────────────────
|
||||
|
||||
describe("task_add_dep tool", () => {
|
||||
/**
|
||||
* Helper: run executor with a customized mock store and capture custom tools.
|
||||
* The mock store's getTask is configured to:
|
||||
* - Return the executing task (KB-TEST) with configurable dependencies
|
||||
* - Return a target task (KB-OTHER) when requested
|
||||
* - Throw for unknown task IDs
|
||||
*/
|
||||
async function captureAddDepTools(opts?: { existingDeps?: string[]; targetExists?: boolean }) {
|
||||
const existingDeps = opts?.existingDeps ?? [];
|
||||
const targetExists = opts?.targetExists ?? true;
|
||||
|
||||
const store = createMockStore();
|
||||
store.getTask.mockImplementation(async (id: string) => {
|
||||
if (id === "KB-TEST") {
|
||||
return {
|
||||
id: "KB-TEST",
|
||||
title: "Test",
|
||||
description: "Test task",
|
||||
column: "in-progress",
|
||||
dependencies: existingDeps,
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
prompt: "# test\n## Steps\n### Step 0: Preflight\n- [ ] check",
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
};
|
||||
}
|
||||
if (id === "KB-OTHER" && targetExists) {
|
||||
return {
|
||||
id: "KB-OTHER",
|
||||
title: "Other task",
|
||||
description: "Another task",
|
||||
column: "todo",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
prompt: "",
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
};
|
||||
}
|
||||
throw new Error(`Task ${id} not found`);
|
||||
});
|
||||
|
||||
store.updateStep.mockResolvedValue({
|
||||
steps: [
|
||||
{ name: "Preflight", status: "done" },
|
||||
{ name: "Implement", status: "in-progress" },
|
||||
],
|
||||
});
|
||||
|
||||
mockedExistsSync.mockReturnValue(true);
|
||||
|
||||
let capturedTools: any[] = [];
|
||||
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
|
||||
capturedTools = opts.customTools || [];
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
sessionManager: {
|
||||
getLeafId: vi.fn().mockReturnValue("leaf-id"),
|
||||
branchWithSummary: vi.fn(),
|
||||
},
|
||||
navigateTree: vi.fn().mockResolvedValue({ cancelled: false }),
|
||||
},
|
||||
} as any;
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute({
|
||||
id: "KB-TEST",
|
||||
title: "Test",
|
||||
description: "Test",
|
||||
column: "in-progress",
|
||||
dependencies: existingDeps,
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
const tools: Record<string, any> = {};
|
||||
for (const t of capturedTools) {
|
||||
tools[t.name] = t.execute;
|
||||
}
|
||||
return { tools, store };
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockedExistsSync.mockReturnValue(true);
|
||||
});
|
||||
|
||||
it("adds a valid dependency via store.updateTask", async () => {
|
||||
const { tools, store } = await captureAddDepTools();
|
||||
|
||||
const result = await tools.task_add_dep("call1", { task_id: "KB-OTHER" });
|
||||
|
||||
expect(result.content[0].text).toContain("Added dependency");
|
||||
expect(result.content[0].text).toContain("KB-TEST now depends on KB-OTHER");
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-TEST", {
|
||||
dependencies: ["KB-OTHER"],
|
||||
});
|
||||
});
|
||||
|
||||
it("returns error for self-dependency", async () => {
|
||||
const { tools, store } = await captureAddDepTools();
|
||||
|
||||
const result = await tools.task_add_dep("call1", { task_id: "KB-TEST" });
|
||||
|
||||
expect(result.content[0].text).toContain("Cannot add self-dependency");
|
||||
expect(result.content[0].text).toContain("KB-TEST cannot depend on itself");
|
||||
// store.updateTask should NOT have been called for dependency update
|
||||
// (it may be called for worktree path updates, so we check specifically for dependencies)
|
||||
const depUpdateCalls = store.updateTask.mock.calls.filter(
|
||||
(call: any[]) => call[1]?.dependencies !== undefined,
|
||||
);
|
||||
expect(depUpdateCalls).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("returns error for non-existent target task", async () => {
|
||||
const { tools, store } = await captureAddDepTools({ targetExists: false });
|
||||
|
||||
const result = await tools.task_add_dep("call1", { task_id: "KB-OTHER" });
|
||||
|
||||
expect(result.content[0].text).toContain("KB-OTHER not found");
|
||||
expect(result.content[0].text).toContain("Cannot add dependency on a non-existent task");
|
||||
const depUpdateCalls = store.updateTask.mock.calls.filter(
|
||||
(call: any[]) => call[1]?.dependencies !== undefined,
|
||||
);
|
||||
expect(depUpdateCalls).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("returns informational message for duplicate dependency without duplicating", async () => {
|
||||
const { tools, store } = await captureAddDepTools({ existingDeps: ["KB-OTHER"] });
|
||||
|
||||
const result = await tools.task_add_dep("call1", { task_id: "KB-OTHER" });
|
||||
|
||||
expect(result.content[0].text).toContain("already a dependency");
|
||||
expect(result.content[0].text).toContain("No changes made");
|
||||
const depUpdateCalls = store.updateTask.mock.calls.filter(
|
||||
(call: any[]) => call[1]?.dependencies !== undefined,
|
||||
);
|
||||
expect(depUpdateCalls).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("logs the dependency addition via store.logEntry", async () => {
|
||||
const { tools, store } = await captureAddDepTools();
|
||||
|
||||
await tools.task_add_dep("call1", { task_id: "KB-OTHER" });
|
||||
|
||||
expect(store.logEntry).toHaveBeenCalledWith("KB-TEST", "Added dependency on KB-OTHER");
|
||||
});
|
||||
|
||||
it("appends to existing dependencies without overwriting", async () => {
|
||||
const { tools, store } = await captureAddDepTools({ existingDeps: ["KB-001"] });
|
||||
|
||||
// Need to make KB-OTHER exist for this test
|
||||
const result = await tools.task_add_dep("call1", { task_id: "KB-OTHER" });
|
||||
|
||||
expect(result.content[0].text).toContain("Added dependency");
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-TEST", {
|
||||
dependencies: ["KB-001", "KB-OTHER"],
|
||||
});
|
||||
});
|
||||
|
||||
it("is registered in customTools array", async () => {
|
||||
const { tools } = await captureAddDepTools();
|
||||
|
||||
expect(tools.task_add_dep).toBeDefined();
|
||||
expect(typeof tools.task_add_dep).toBe("function");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -40,10 +40,6 @@ const taskCreateParams = Type.Object({
|
||||
),
|
||||
});
|
||||
|
||||
const taskAddDepParams = Type.Object({
|
||||
task_id: Type.String({ description: "The ID of the task to depend on (e.g. \"KB-001\")" }),
|
||||
});
|
||||
|
||||
const reviewStepParams = Type.Object({
|
||||
step: Type.Number({ description: "Step number to review" }),
|
||||
type: Type.Union(
|
||||
@@ -84,8 +80,6 @@ You have tools to report progress. The board updates in real-time.
|
||||
|
||||
**Out-of-scope work found during execution:** \`task_create(description="what needs doing")\`
|
||||
|
||||
**Discovered a dependency:** \`task_add_dep(task_id="KB-XXX")\` — use when you discover mid-execution that another task must be completed first
|
||||
|
||||
## Cross-model review via review_step tool
|
||||
|
||||
You have a \`review_step\` tool. It spawns a SEPARATE reviewer agent (different
|
||||
@@ -358,7 +352,6 @@ export class TaskExecutor {
|
||||
this.createTaskUpdateTool(task.id, codeReviewVerdicts, sessionRef, stepCheckpoints),
|
||||
this.createTaskLogTool(task.id),
|
||||
this.createTaskCreateTool(),
|
||||
this.createTaskAddDepTool(task.id),
|
||||
this.createTaskDoneTool(task.id, () => { taskDone = true; }),
|
||||
this.createReviewStepTool(task.id, worktreePath, detail.prompt, codeReviewVerdicts, sessionRef, stepCheckpoints),
|
||||
];
|
||||
@@ -547,73 +540,6 @@ export class TaskExecutor {
|
||||
};
|
||||
}
|
||||
|
||||
private createTaskAddDepTool(taskId: string): ToolDefinition {
|
||||
const store = this.store;
|
||||
return {
|
||||
name: "task_add_dep",
|
||||
label: "Add Dependency",
|
||||
description:
|
||||
"Declare a dependency on an existing task. Use when you discover " +
|
||||
"mid-execution that another task must be completed first. " +
|
||||
"The dependency is appended to this task's dependencies array.",
|
||||
parameters: taskAddDepParams,
|
||||
execute: async (_id: string, params: Static<typeof taskAddDepParams>) => {
|
||||
const targetId = params.task_id;
|
||||
|
||||
// Prevent self-dependency
|
||||
if (targetId === taskId) {
|
||||
return {
|
||||
content: [{
|
||||
type: "text" as const,
|
||||
text: `Cannot add self-dependency: ${taskId} cannot depend on itself.`,
|
||||
}],
|
||||
details: {},
|
||||
};
|
||||
}
|
||||
|
||||
// Validate target task exists
|
||||
try {
|
||||
await store.getTask(targetId);
|
||||
} catch {
|
||||
return {
|
||||
content: [{
|
||||
type: "text" as const,
|
||||
text: `Task ${targetId} not found. Cannot add dependency on a non-existent task.`,
|
||||
}],
|
||||
details: {},
|
||||
};
|
||||
}
|
||||
|
||||
// Read current task to get existing dependencies
|
||||
const currentTask = await store.getTask(taskId);
|
||||
const existing = currentTask.dependencies;
|
||||
|
||||
// Dedup check
|
||||
if (existing.includes(targetId)) {
|
||||
return {
|
||||
content: [{
|
||||
type: "text" as const,
|
||||
text: `${targetId} is already a dependency of ${taskId}. No changes made.`,
|
||||
}],
|
||||
details: {},
|
||||
};
|
||||
}
|
||||
|
||||
// Add the dependency
|
||||
await store.updateTask(taskId, { dependencies: [...existing, targetId] });
|
||||
await store.logEntry(taskId, `Added dependency on ${targetId}`);
|
||||
|
||||
return {
|
||||
content: [{
|
||||
type: "text" as const,
|
||||
text: `Added dependency: ${taskId} now depends on ${targetId}.`,
|
||||
}],
|
||||
details: {},
|
||||
};
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
private createTaskDoneTool(taskId: string, onDone: () => void): ToolDefinition {
|
||||
const store = this.store;
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user