feat(KB-142): add confirm gate and abort flow to task_add_dep tool

- Add confirm parameter to task_add_dep requiring explicit opt-in before destructive action
- On confirm, abort execution, discard worktree/branch, and move task to triage
- Add triage as valid transition from in-progress for dependency-triggered re-specification
- Auto-move todo tasks to triage when dependencies are added
- Update EXECUTOR_SYSTEM_PROMPT with task_add_dep confirm behavior documentation
This commit is contained in:
Dustin Byrne
2026-03-28 00:41:01 -04:00
parent a59fb5b857
commit 2bdd628d6d
5 changed files with 550 additions and 4 deletions

View File

@@ -2017,3 +2017,317 @@ 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 when confirm=true", async () => {
const { tools, store } = await captureAddDepTools();
const result = await tools.task_add_dep("call1", { task_id: "KB-OTHER", confirm: true });
expect(result.content[0].text).toContain("Added dependency");
expect(result.content[0].text).toContain("triage");
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 when confirm=true", async () => {
const { tools, store } = await captureAddDepTools();
await tools.task_add_dep("call1", { task_id: "KB-OTHER", confirm: true });
expect(store.logEntry).toHaveBeenCalledWith("KB-TEST", "Added dependency on KB-OTHER — stopping execution for re-specification");
});
it("appends to existing dependencies without overwriting when confirm=true", async () => {
const { tools, store } = await captureAddDepTools({ existingDeps: ["KB-001"] });
const result = await tools.task_add_dep("call1", { task_id: "KB-OTHER", confirm: true });
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");
});
it("returns warning without confirm=true and does NOT add dependency", async () => {
const { tools, store } = await captureAddDepTools();
const result = await tools.task_add_dep("call1", { task_id: "KB-OTHER" });
expect(result.content[0].text).toContain("stop execution and discard current work");
expect(result.content[0].text).toContain("confirm=true");
// Should NOT have updated dependencies
const depUpdateCalls = store.updateTask.mock.calls.filter(
(call: any[]) => call[1]?.dependencies !== undefined,
);
expect(depUpdateCalls).toHaveLength(0);
// Should NOT have logged any dep addition
const logCalls = store.logEntry.mock.calls.filter(
(call: any[]) => typeof call[1] === "string" && call[1].includes("Added dependency"),
);
expect(logCalls).toHaveLength(0);
});
it("validation errors (self-dep, not-found, dedup) return immediately without requiring confirm", async () => {
// Self-dep — no confirm needed
const { tools: tools1 } = await captureAddDepTools();
const selfResult = await tools1.task_add_dep("call1", { task_id: "KB-TEST" });
expect(selfResult.content[0].text).toContain("Cannot add self-dependency");
// Not found — no confirm needed
const { tools: tools2 } = await captureAddDepTools({ targetExists: false });
const notFoundResult = await tools2.task_add_dep("call1", { task_id: "KB-OTHER" });
expect(notFoundResult.content[0].text).toContain("not found");
// Dedup — no confirm needed
const { tools: tools3 } = await captureAddDepTools({ existingDeps: ["KB-OTHER"] });
const dedupResult = await tools3.task_add_dep("call1", { task_id: "KB-OTHER" });
expect(dedupResult.content[0].text).toContain("already a dependency");
});
it("with confirm=true triggers depAborted and disposes session", async () => {
const store = createMockStore();
store.getTask.mockImplementation(async (id: string) => {
if (id === "KB-DEP") {
return {
id: "KB-DEP",
title: "Test",
description: "Test task",
column: "in-progress",
dependencies: [],
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-TARGET") {
return {
id: "KB-TARGET",
title: "Target",
description: "Target task",
column: "todo",
dependencies: [],
steps: [],
currentStep: 0,
log: [],
prompt: "",
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
};
}
throw new Error(`Task ${id} not found`);
});
mockedExistsSync.mockReturnValue(true);
const disposeFn = vi.fn();
let capturedTools: any[] = [];
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
capturedTools = opts.customTools || [];
return {
session: {
prompt: vi.fn().mockImplementation(async () => {
// The agent calls task_add_dep with confirm=true during execution
const addDepTool = capturedTools.find((t: any) => t.name === "task_add_dep");
await addDepTool.execute("call1", { task_id: "KB-TARGET", confirm: true });
// After dispose is called, session.prompt throws
throw new Error("Session terminated");
}),
dispose: disposeFn,
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-DEP",
title: "Test",
description: "Test",
column: "in-progress",
dependencies: [],
steps: [],
currentStep: 0,
log: [],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
});
// Worktree removal should have been attempted
const worktreeRemoveCalls = mockedExecSync.mock.calls.filter(
(c) => typeof c[0] === "string" && (c[0] as string).includes("worktree remove"),
);
expect(worktreeRemoveCalls.length).toBeGreaterThan(0);
// Branch deletion should have been attempted
const branchDeleteCalls = mockedExecSync.mock.calls.filter(
(c) => typeof c[0] === "string" && (c[0] as string).includes("branch -D") && (c[0] as string).includes("kb/kb-dep"),
);
expect(branchDeleteCalls.length).toBeGreaterThan(0);
// Task should be moved to triage
expect(store.moveTask).toHaveBeenCalledWith("KB-DEP", "triage");
// Worktree and status should be cleared
expect(store.updateTask).toHaveBeenCalledWith("KB-DEP", { worktree: undefined, status: undefined });
// Task should NOT be marked as failed
expect(store.updateTask).not.toHaveBeenCalledWith("KB-DEP", { status: "failed" });
});
});

View File

@@ -40,6 +40,12 @@ 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\")" }),
confirm: Type.Optional(Type.Boolean({ description: "Set to true to confirm adding the dependency. Required because adding a dep to an in-progress task will stop execution and discard current work." })),
});
const reviewStepParams = Type.Object({
step: Type.Number({ description: "Step number to review" }),
type: Type.Union(
@@ -83,6 +89,8 @@ When creating multiple related tasks, declare dependencies between them:
\`task_create(description="load door sounds", dependencies=[])\` → returns KB-050
\`task_create(description="play sound on door open/close", dependencies=["KB-050"])\`
**Discovered a dependency:** \`task_add_dep(task_id="KB-XXX")\` — use when you discover mid-execution that another task must be completed first. This will return a warning first — you must call again with \`confirm=true\` to proceed. Adding a dependency stops execution, discards current work, and moves the task to triage for re-specification.
## Cross-model review via review_step tool
You have a \`review_step\` tool. It spawns a SEPARATE reviewer agent (different
@@ -150,6 +158,8 @@ export class TaskExecutor {
private activeSessions = new Map<string, { dispose: () => void }>();
/** Tasks that were paused mid-execution (to avoid marking them as "failed"). */
private pausedAborted = new Set<string>();
/** Tasks that had a dependency added mid-execution (abort + discard worktree). */
private depAborted = new Set<string>();
constructor(
private store: TaskStore,
@@ -255,6 +265,9 @@ export class TaskExecutor {
executorLog.log(`Starting ${task.id}: ${task.title || task.description.slice(0, 60)}`);
// Hoist worktreePath so it's accessible in the catch block for dep-abort cleanup
let worktreePath = task.worktree || join(this.rootDir, ".worktrees", generateWorktreeName(this.rootDir));
try {
// Check dependencies
const allTasks = await this.store.listTasks();
@@ -272,7 +285,6 @@ export class TaskExecutor {
const branchName = `kb/${task.id.toLowerCase()}`;
// Use generateWorktreeName for human-friendly directory names (adjective-noun pattern)
// instead of task.id, so worktrees are named like ".worktrees/swift-falcon"
let worktreePath = task.worktree || join(this.rootDir, ".worktrees", generateWorktreeName(this.rootDir));
let isResume = existsSync(worktreePath);
let acquiredFromPool = false;
const settings = await this.store.getSettings();
@@ -355,6 +367,7 @@ 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),
];
@@ -392,6 +405,13 @@ export class TaskExecutor {
const agentPrompt = buildExecutionPrompt(detail, this.rootDir, settings);
await session.prompt(agentPrompt);
// If dependency was added during execution, discard worktree and move to triage
if (this.depAborted.has(task.id)) {
this.depAborted.delete(task.id);
await this.handleDepAbortCleanup(task.id, worktreePath);
return;
}
// If paused during execution, don't move to in-review
if (this.pausedAborted.has(task.id)) {
this.pausedAborted.delete(task.id);
@@ -421,7 +441,11 @@ export class TaskExecutor {
await agentWork();
}
} catch (err: any) {
if (this.pausedAborted.has(task.id)) {
if (this.depAborted.has(task.id)) {
// Dependency added mid-execution — discard worktree and move to triage
this.depAborted.delete(task.id);
await this.handleDepAbortCleanup(task.id, worktreePath);
} else if (this.pausedAborted.has(task.id)) {
// Task was paused mid-execution — move to todo, don't mark as failed
executorLog.log(`${task.id} paused — moving to todo`);
this.pausedAborted.delete(task.id);
@@ -543,6 +567,91 @@ 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. " +
"Adding a dependency to an in-progress task will stop execution " +
"and discard current work, so confirm=true is required. " +
"Without confirm=true, a warning is returned first.",
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: {},
};
}
// Confirmation gate — destructive action for in-progress tasks
if (!params.confirm) {
return {
content: [{
type: "text" as const,
text: `Warning: adding a dependency to an in-progress task will stop execution and discard current work. Call with confirm=true to proceed.`,
}],
details: {},
};
}
// Add the dependency
await store.updateTask(taskId, { dependencies: [...existing, targetId] });
await store.logEntry(taskId, `Added dependency on ${targetId} — stopping execution for re-specification`);
// Trigger abort flow (same pattern as pausedAborted)
this.depAborted.add(taskId);
const session = this.activeSessions.get(taskId);
session?.dispose();
return {
content: [{
type: "text" as const,
text: `Added dependency on ${targetId}. Stopping execution — task will move to triage for re-specification.`,
}],
details: {},
};
},
};
}
private createTaskDoneTool(taskId: string, onDone: () => void): ToolDefinition {
const store = this.store;
return {
@@ -722,6 +831,37 @@ export class TaskExecutor {
};
}
/**
* Clean up after a dep-abort: remove worktree, delete branch, move task to triage.
* Shared between the try-block (graceful return) and catch-block (error) paths.
*/
private async handleDepAbortCleanup(taskId: string, worktreePath: string): Promise<void> {
executorLog.log(`${taskId} dependency added — work discarded, moved to triage for re-specification`);
// Remove worktree
try {
execSync(`git worktree remove "${worktreePath}" --force`, { cwd: this.rootDir, stdio: "pipe" });
} catch {
// Worktree may already be gone
}
// Delete the branch
const branch = `kb/${taskId.toLowerCase()}`;
try {
execSync(`git branch -D "${branch}"`, { cwd: this.rootDir, stdio: "pipe" });
} catch {
// Branch may not exist
}
// Clear worktree tracking
this.activeWorktrees.delete(taskId);
// Update task: clear worktree and status, move to triage
await this.store.updateTask(taskId, { worktree: undefined, status: undefined });
await this.store.moveTask(taskId, "triage");
await this.store.logEntry(taskId, "Execution stopped — work discarded, moved to triage for re-specification");
}
// ── Worktree management ────────────────────────────────────────────
/**