Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions apps/vscode-e2e/src/fixtures/subtasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,19 @@ const SUBTASK_FAST_CHILD_MARKER = "SUBTASK_CHILD_IMMEDIATE_COMPLETION"
const SUBTASK_XPROFILE_PARENT_MARKER = "SUBTASK_PARENT_CROSS_PROFILE"
const SUBTASK_XPROFILE_SAME_CHILD_MARKER = "SUBTASK_CHILD_SAME_PROFILE"
const SUBTASK_XPROFILE_DIFFERENT_CHILD_MARKER = "SUBTASK_CHILD_DIFFERENT_PROFILE"
const SUBTASK_FANOUT_PARENT_MARKER = "SUBTASK_PARENT_FANOUT_CONCURRENT"
const SUBTASK_FANOUT_CHILD_MARKER = "SUBTASK_CHILD_FANOUT_CONCURRENT"

const SUBTASK_CHILD_PROMPT = `${SUBTASK_CHILD_MARKER}: Ask the user exactly this follow-up question: What is the square root of 81? After the user answers, complete with only the answer.`
export const SUBTASK_PARENT_PROMPT = `${SUBTASK_PARENT_MARKER}: Use the new_task tool exactly once. Create an ask-mode subtask with this exact message: "${SUBTASK_CHILD_PROMPT}" Do not answer directly.`
export const SUBTASK_CHILD_FOLLOWUP_ANSWER = "9"
export const SUBTASK_FAST_CHILD_RESULT = "Fast child completed"
const SUBTASK_FAST_CHILD_PROMPT = `${SUBTASK_FAST_CHILD_MARKER}: Complete immediately with the exact result "${SUBTASK_FAST_CHILD_RESULT}".`
export const SUBTASK_FAST_PARENT_PROMPT = `${SUBTASK_FAST_PARENT_MARKER}: Use the new_task tool exactly once. Create an ask-mode subtask with this exact message: "${SUBTASK_FAST_CHILD_PROMPT}" Do not answer directly.`
export const SUBTASK_FANOUT_PARENT_FOLLOWUP = "Parent fan-out is still active?"
export const SUBTASK_FANOUT_CHILD_RESULT = "Fan-out child completed"
const SUBTASK_FANOUT_CHILD_PROMPT = `${SUBTASK_FANOUT_CHILD_MARKER}: Complete with the exact result "${SUBTASK_FANOUT_CHILD_RESULT}".`
export const SUBTASK_FANOUT_PARENT_PROMPT = `${SUBTASK_FANOUT_PARENT_MARKER}: Use the new_task tool exactly once. Create an ask-mode subtask with this exact message: "${SUBTASK_FANOUT_CHILD_PROMPT}" After delegation, ask the user exactly this follow-up question: ${SUBTASK_FANOUT_PARENT_FOLLOWUP}`

const SUBTASK_INTERRUPT_CHILD_PROMPT = `${SUBTASK_INTERRUPT_CHILD_MARKER}: Ask the user exactly this follow-up question: What is the square root of 81? After the user answers, complete with only the answer.`
export const SUBTASK_INTERRUPT_PARENT_PROMPT = `${SUBTASK_INTERRUPT_PARENT_MARKER}: Use the new_task tool exactly once. Create an ask-mode subtask with this exact message: "${SUBTASK_INTERRUPT_CHILD_PROMPT}" Do not answer directly. When the subtask returns, complete with the exact result "Interrupted parent resumed".`
Expand Down Expand Up @@ -135,6 +141,63 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
},
})

mock.addFixture({
match: {
userMessage: new RegExp(SUBTASK_FANOUT_PARENT_MARKER),
sequenceIndex: 0,
},
response: {
toolCalls: [
{
name: "new_task",
arguments: JSON.stringify({
mode: "ask",
message: SUBTASK_FANOUT_CHILD_PROMPT,
}),
id: "call_subtasks_fanout_parent_new_task_001",
},
],
},
})

mock.addFixture({
match: {
predicate: (req: ChatCompletionRequest) =>
lastUserMessageContains(req, SUBTASK_FANOUT_CHILD_MARKER) &&
!requestContains(req, [SUBTASK_FANOUT_PARENT_MARKER]),
},
latency: 15_000,
response: {
toolCalls: [
{
name: "attempt_completion",
arguments: JSON.stringify({ result: SUBTASK_FANOUT_CHILD_RESULT }),
id: "call_subtasks_fanout_child_completion_002",
},
],
},
})

mock.addFixture({
match: {
predicate: (req: ChatCompletionRequest) =>
requestContains(req, [SUBTASK_FANOUT_PARENT_MARKER, "Delegated to child task"]) &&
!requestContains(req, [SUBTASK_RESULT_INJECTION]),
},
response: {
toolCalls: [
{
name: "ask_followup_question",
arguments: JSON.stringify({
question: SUBTASK_FANOUT_PARENT_FOLLOWUP,
follow_up: [{ text: "continue" }],
}),
id: "call_subtasks_fanout_parent_followup_003",
},
],
},
})

// The parent prompt embeds SUBTASK_FAST_CHILD_MARKER verbatim, so parent-resume turns
// can also match a bare substring check (same collision class as #561). Exclude the
// parent marker so those turns fall through to the parent-resume fixture below.
Expand Down
60 changes: 60 additions & 0 deletions apps/vscode-e2e/src/suite/subtasks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
SUBTASK_API_HANG_RESUME_MESSAGE,
SUBTASK_CHILD_FOLLOWUP_ANSWER,
SUBTASK_FAST_CHILD_RESULT,
SUBTASK_FANOUT_PARENT_FOLLOWUP,
SUBTASK_FANOUT_PARENT_PROMPT,
SUBTASK_FAST_PARENT_PROMPT,
SUBTASK_INTERRUPT_CHILD_FOLLOWUP_ANSWER,
SUBTASK_INTERRUPT_PARENT_PROMPT,
Expand Down Expand Up @@ -129,6 +131,64 @@ suite("Roo Code Subtasks", function () {
}
})

test("fan-out keeps parent executing while child request is in flight", async () => {
const api = globalThis.api
const asks: Record<string, ClineMessage[]> = {}

const messageHandler = ({ taskId, message }: { taskId: string; message: ClineMessage }) => {
if (message.type === "ask") {
asks[taskId] = asks[taskId] || []
asks[taskId].push(message)
}
}

api.on(RooCodeEventName.Message, messageHandler)

try {
api.setTaskSchedulerMaxConcurrency(2)

const parentTaskId = await api.startNewTask({
configuration: {
mode: "ask",
alwaysAllowModeSwitch: true,
alwaysAllowSubtasks: true,
autoApprovalEnabled: true,
enableCheckpoints: false,
},
text: SUBTASK_FANOUT_PARENT_PROMPT,
})

let childTaskId: string | undefined
await waitFor(() => {
const stack = api.getCurrentTaskStack()
const current = stack.at(-1)
if (current && current !== parentTaskId) {
childTaskId = current
return stack.includes(parentTaskId)
}
return false
})

await waitFor(() =>
(asks[parentTaskId] ?? []).some(
({ ask, text }) => ask === "followup" && text?.includes(SUBTASK_FANOUT_PARENT_FOLLOWUP),
),
)

const stack = api.getCurrentTaskStack()
assert.ok(stack.includes(parentTaskId), "Fan-out parent should remain in the live task stack")
assert.ok(stack.includes(childTaskId!), "Fan-out child should remain in the live task stack")
assert.strictEqual(stack.at(-1), childTaskId, "Child should remain the focused task while parent runs")
} finally {
api.off(RooCodeEventName.Message, messageHandler)
while (api.getCurrentTaskStack().length > 0) {
await api.clearCurrentTask()
}
api.setTaskSchedulerMaxConcurrency(1)
await waitFor(() => api.getCurrentTaskStack().length === 0).catch(() => {})
}
})

// Smoke: child completing normally must resume the parent task.
test("child task returns to parent after normal completion", async () => {
const api = globalThis.api
Expand Down
5 changes: 5 additions & 0 deletions packages/types/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ export interface RooCodeAPI extends EventEmitter<RooCodeAPIEvents> {
* @returns An array of task IDs.
*/
getCurrentTaskStack(): string[]
/**
* Sets the TaskScheduler concurrency for extension-host tests.
* Intended for test/integration harnesses that need to exercise fan-out.
*/
setTaskSchedulerMaxConcurrency(maxConcurrency: number): void
/**
* Clears the current task.
*/
Expand Down
8 changes: 7 additions & 1 deletion src/__tests__/provider-delegation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,9 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
isViewLaunched: false,
recentTasksCache: undefined,
taskHistoryStore,
// Rollback looks up the just-created child by id to decide whether it
// still needs evicting, independent of current focus.
taskRegistry: { getById: vi.fn((id: string) => (id === "child-1" ? child : undefined)) },
} as unknown as ClineProvider

await expect(
Expand All @@ -381,8 +384,11 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
).rejects.toThrow(persistError)

expect(childRun).not.toHaveBeenCalled()
// 1st call (step 3): closes the parent to enforce the single-open invariant.
expect(removeClineFromStack).toHaveBeenNthCalledWith(1)
expect(removeClineFromStack).toHaveBeenNthCalledWith(2)
// 2nd call (rollback): evicts the just-created child by id, regardless of
// current focus — see Story 3.2b fan-out rollback fix.
expect(removeClineFromStack).toHaveBeenNthCalledWith(2, "child-1")
expect(deleteTaskWithId).toHaveBeenCalledWith("child-1", false)
expect(createTaskWithHistoryItem).toHaveBeenCalledWith(parentHistoryItem)
})
Expand Down
36 changes: 35 additions & 1 deletion src/core/task/TaskScheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,37 @@ import { type Task } from "./Task"
*/
export class TaskScheduler {
private readonly sem: TaskSemaphore
readonly maxConcurrency: number

constructor(maxConcurrency = 1) {
this.maxConcurrency = maxConcurrency
this.sem = new TaskSemaphore(maxConcurrency)
}

get waiting(): number {
return this.sem.waiting
}

/** Number of permits not currently held by a running task. */
get available(): number {
return this.sem.available
}

/**
* Reserve a permit only if one is immediately free, without queueing.
* Returns a release function on success, or `undefined` if none was free.
*
* Use this (not `available > 0` followed later by `schedule()`) when a
* caller needs to make an irreversible decision — e.g. keeping a parent
* task alive for fan-out — based on whether a child can actually run
* concurrently. Checking `available` and then `await`-ing other work
* before calling `schedule()` leaves a window where another caller can
* consume the last permit; reserving it immediately closes that window.
*/
async tryReserve(): Promise<(() => void) | undefined> {
return this.sem.tryAcquire()
}

/**
* Acquire a permit for `task`, call `run()`, and release on completion.
*
Expand All @@ -30,7 +52,19 @@ export class TaskScheduler {
* without calling `run()`.
*/
async schedule(task: Task, run: () => Promise<void>): Promise<void> {
const release = await this.sem.acquire()
return this.runWithRelease(await this.sem.acquire(), task, run)
}

/**
* Run `task` using a permit already obtained via `tryReserve()`, instead of
* acquiring a new one. Same abort/abandon and release-on-completion
* semantics as `schedule()`.
*/
async runWithReservation(release: () => void, task: Task, run: () => Promise<void>): Promise<void> {
return this.runWithRelease(release, task, run)
}

private async runWithRelease(release: () => void, task: Task, run: () => Promise<void>): Promise<void> {
if (task.abort || task.abandoned) {
release()
return
Expand Down
Loading
Loading