feat(FN-1017): enable proactive subtask splitting during triage and review
- Update triage agent to proactively suggest splitting large tasks into subtasks during specification - Tighten reviewer guidance to flag undersplit tasks that should be broken down further - Add comprehensive tests for proactive subtask creation in triage (192 lines) - Add reviewer tests for undersplit task detection (12 lines) - Update README with documentation on proactive subtask splitting behavior
This commit is contained in:
gsxdsm
@@ -361,6 +361,18 @@ describe("REVIEWER_SYSTEM_PROMPT", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("instructs planner to use task_create for undersplit tasks", () => {
|
||||
// The reviewer's REVISE feedback must explicitly direct the planner to
|
||||
// create child tasks via task_create rather than just flagging the issue.
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("task_create");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain(
|
||||
"create 2–5 child tasks",
|
||||
);
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain(
|
||||
"Do NOT write a parent PROMPT.md",
|
||||
);
|
||||
});
|
||||
|
||||
it("includes user comment coverage criterion in spec review format", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("User comment coverage");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("missing coverage is a blocking REVISE");
|
||||
|
||||
@@ -128,9 +128,16 @@ When reviewing specs, actively assess whether the task should have been broken i
|
||||
- A task affects 3+ different packages but wasn't split
|
||||
- A task has multiple clearly independent deliverables combined into one
|
||||
|
||||
**How to flag:**
|
||||
**How to flag an undersplit task:**
|
||||
Say explicitly: "This task should be broken into subtasks because [specific reason]."
|
||||
Recommend the number of child tasks (2-5) and what each should cover.
|
||||
**Critically**, instruct the planner to take these actions in your REVISE feedback:
|
||||
1. Use the \`task_create\` tool to create 2–5 child tasks from the oversized spec
|
||||
2. Do NOT write a parent PROMPT.md — the parent will be closed automatically after children are created
|
||||
3. Each child task should cover one coherent deliverable with clear scope boundaries
|
||||
|
||||
Example REVISE feedback for an undersplit task:
|
||||
"This task should be broken into 3 subtasks because it spans the engine, dashboard, and CLI packages with independent deliverables. Use task_create to create: (1) engine logic, (2) dashboard UI, (3) CLI integration. Do not write a parent PROMPT."
|
||||
|
||||
**Do NOT flag if:**
|
||||
- Steps are sequential and tightly coupled (e.g., a pipeline where each step depends on the previous)
|
||||
|
||||
@@ -904,6 +904,198 @@ describe("taskCreate tool model inheritance", () => {
|
||||
}));
|
||||
});
|
||||
|
||||
describe("proactive subtask creation (task_create always available)", () => {
|
||||
it("task_create tool is included in triage tools regardless of breakIntoSubtasks", () => {
|
||||
const store = createMockStore();
|
||||
const processor = new TriageProcessor(store, "/test/root");
|
||||
const createdSubtasksRef = { current: [] };
|
||||
|
||||
const tools = (processor as any).createTriageTools({
|
||||
parentTaskId: "FN-400",
|
||||
allowTaskCreate: true,
|
||||
createdSubtasksRef,
|
||||
});
|
||||
|
||||
const toolNames = tools.map((t: any) => t.name);
|
||||
expect(toolNames).toContain("task_create");
|
||||
expect(toolNames).toContain("task_list");
|
||||
expect(toolNames).toContain("task_get");
|
||||
expect(tools).toHaveLength(3);
|
||||
});
|
||||
|
||||
it("task_create tool succeeds and tracks created subtask", async () => {
|
||||
const parentTask: Task = {
|
||||
id: "FN-400",
|
||||
description: "Large task without breakIntoSubtasks",
|
||||
column: "triage",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
};
|
||||
|
||||
const createdSubtask: Task = {
|
||||
id: "FN-401",
|
||||
description: "Child task",
|
||||
column: "triage",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
};
|
||||
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue(parentTask),
|
||||
createTask: vi.fn().mockResolvedValue(createdSubtask),
|
||||
});
|
||||
|
||||
const processor = new TriageProcessor(store, "/test/root");
|
||||
const createdSubtasksRef = { current: [] };
|
||||
|
||||
const tools = (processor as any).createTriageTools({
|
||||
parentTaskId: "FN-400",
|
||||
allowTaskCreate: true,
|
||||
createdSubtasksRef,
|
||||
});
|
||||
|
||||
const taskCreateTool = tools.find((t: any) => t.name === "task_create");
|
||||
const result = await taskCreateTool.execute("call-1", {
|
||||
description: "Child task description",
|
||||
title: "Child Task",
|
||||
dependencies: [],
|
||||
});
|
||||
|
||||
// Should NOT return an error about task creation being disabled
|
||||
const text = result.content[0].text;
|
||||
expect(text).not.toContain("ERROR");
|
||||
expect(text).not.toContain("not enabled");
|
||||
expect(text).toContain("Created child task FN-401");
|
||||
|
||||
// Subtask should be tracked in the ref
|
||||
expect(createdSubtasksRef.current).toContain("FN-401");
|
||||
|
||||
// Should inherit parent model settings
|
||||
expect(store.createTask).toHaveBeenCalledWith(expect.objectContaining({
|
||||
title: "Child Task",
|
||||
description: "Child task description",
|
||||
}));
|
||||
});
|
||||
|
||||
it("closes parent after proactive split even when breakIntoSubtasks is undefined", async () => {
|
||||
// Test that the post-session closure path doesn't gate on breakIntoSubtasks.
|
||||
// Strategy: capture the customTools from createKbAgent, then have
|
||||
// promptWithFallback invoke the task_create tool to simulate the agent
|
||||
// proactively splitting an oversized task.
|
||||
const task: Task = {
|
||||
id: "FN-500",
|
||||
description: "Oversized task without breakIntoSubtasks flag",
|
||||
column: "triage",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
};
|
||||
|
||||
const childTask1: Task = {
|
||||
id: "FN-501",
|
||||
description: "Child part 1",
|
||||
column: "triage",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
};
|
||||
const childTask2: Task = {
|
||||
id: "FN-502",
|
||||
description: "Child part 2",
|
||||
column: "triage",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
};
|
||||
|
||||
const taskDetail: TaskDetail = {
|
||||
...task,
|
||||
prompt: "",
|
||||
attachments: [],
|
||||
// breakIntoSubtasks is explicitly undefined
|
||||
};
|
||||
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue(taskDetail),
|
||||
createTask: vi.fn()
|
||||
.mockResolvedValueOnce(childTask1)
|
||||
.mockResolvedValueOnce(childTask2),
|
||||
});
|
||||
|
||||
// Capture customTools from createKbAgent call
|
||||
let capturedCustomTools: any[] = [];
|
||||
const mockDispose = vi.fn();
|
||||
mockCreateKbAgent.mockImplementation(async (opts: any) => {
|
||||
capturedCustomTools = opts.customTools || [];
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: mockDispose,
|
||||
subscribe: vi.fn(),
|
||||
sessionManager: {
|
||||
getLeafId: vi.fn().mockReturnValue(null),
|
||||
navigateTree: vi.fn(),
|
||||
},
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
// Make promptWithFallback invoke the task_create tool twice to simulate
|
||||
// the agent proactively splitting the oversized task
|
||||
const { promptWithFallback } = await import("./pi.js");
|
||||
(promptWithFallback as ReturnType<typeof vi.fn>).mockImplementationOnce(
|
||||
async () => {
|
||||
const taskCreateTool = capturedCustomTools.find(
|
||||
(t: any) => t.name === "task_create",
|
||||
);
|
||||
expect(taskCreateTool).toBeDefined();
|
||||
// Simulate agent creating two child tasks
|
||||
await taskCreateTool.execute("call-1", {
|
||||
description: "Child part 1",
|
||||
title: "Part 1",
|
||||
dependencies: [],
|
||||
});
|
||||
await taskCreateTool.execute("call-2", {
|
||||
description: "Child part 2",
|
||||
title: "Part 2",
|
||||
dependencies: [],
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
const processor = new TriageProcessor(store, "/test/root", {
|
||||
pollIntervalMs: 100_000,
|
||||
});
|
||||
|
||||
await processor.specifyTask(task);
|
||||
|
||||
// The parent task should be deleted because subtasks were created,
|
||||
// even though breakIntoSubtasks was NOT set
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-500",
|
||||
expect.stringContaining("Converted into subtasks: FN-501, FN-502"),
|
||||
);
|
||||
expect(store.deleteTask).toHaveBeenCalledWith("FN-500");
|
||||
});
|
||||
});
|
||||
|
||||
describe("bounded recovery retries for triage", () => {
|
||||
it("sets recoveryRetryCount and nextRecoveryAt on first transient error via specifyTask", async () => {
|
||||
const task = {
|
||||
|
||||
@@ -477,7 +477,7 @@ export class TriageProcessor {
|
||||
const customTools = [
|
||||
...this.createTriageTools({
|
||||
parentTaskId: task.id,
|
||||
allowTaskCreate: detail.breakIntoSubtasks === true,
|
||||
allowTaskCreate: true,
|
||||
createdSubtasksRef,
|
||||
}),
|
||||
this.createReviewSpecTool(
|
||||
@@ -579,7 +579,7 @@ export class TriageProcessor {
|
||||
// Re-raise errors that pi-coding-agent swallowed after exhausting retries.
|
||||
checkSessionError(session);
|
||||
|
||||
if (detail.breakIntoSubtasks && createdSubtasksRef.current.length > 0) {
|
||||
if (createdSubtasksRef.current.length > 0) {
|
||||
const childTaskIds = createdSubtasksRef.current.join(", ");
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
@@ -882,25 +882,17 @@ export class TriageProcessor {
|
||||
label: "Create Child Task",
|
||||
description:
|
||||
"Create a child task (subtask) while breaking a larger task into smaller pieces. " +
|
||||
"Use this when breakIntoSubtasks is enabled and the work can be split into 2-5 independently executable tasks. " +
|
||||
"Use this when the work can be split into 2-5 independently executable tasks, " +
|
||||
"either because the user requested subtask breakdown or because the task is " +
|
||||
"oversized (8+ steps, 3+ packages, multiple independent deliverables). " +
|
||||
"The created task will be a child of the current task being triaged.",
|
||||
parameters: taskCreateParams,
|
||||
execute: async (
|
||||
_callId: string,
|
||||
params: Static<typeof taskCreateParams>,
|
||||
) => {
|
||||
if (!options.allowTaskCreate) {
|
||||
return {
|
||||
content: [
|
||||
{
|
||||
type: "text" as const,
|
||||
text: "ERROR: Task creation is not enabled for this task. The user did not request subtask breakdown.",
|
||||
},
|
||||
],
|
||||
details: {},
|
||||
};
|
||||
}
|
||||
|
||||
// task_create is always available during triage to support both
|
||||
// explicit breakIntoSubtasks and proactive splitting of oversized tasks.
|
||||
try {
|
||||
// Fetch parent task to inherit model settings
|
||||
let parentTask: Awaited<ReturnType<typeof store.getTask>> | undefined;
|
||||
@@ -949,11 +941,7 @@ export class TriageProcessor {
|
||||
},
|
||||
};
|
||||
|
||||
const tools: ToolDefinition[] = [taskList, taskGet];
|
||||
if (options.allowTaskCreate) {
|
||||
tools.push(taskCreate);
|
||||
}
|
||||
return tools;
|
||||
return [taskList, taskGet, taskCreate];
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user