fix: correct queue advancement off-by-one and child mode tracking in advanceSubtaskQueue

- Fix off-by-one: dispatch subtaskQueue[currentIndex] instead of
  subtaskQueue[nextIndex], preventing the first queued item from being
  skipped
- Fix completedMode: fetch child history to get the child actual mode
  instead of incorrectly using the parent historyItem.mode
- Update tests to reflect corrected queue semantics (subtaskQueueIndex
  represents the next item to dispatch, not the currently running item)
This commit is contained in:
Roo Code 2026-05-12 03:45:30 +00:00
parent 4b9c7ac155
commit dcb182db93
2 changed files with 32 additions and 24 deletions

View file

@ -64,7 +64,7 @@ describe("advanceSubtaskQueue", () => {
getCurrentTask: vi.fn().mockReturnValue({ taskId: "child-1" }),
removeClineFromStack: vi.fn().mockResolvedValue(undefined),
getTaskWithId: vi.fn().mockResolvedValue({
historyItem: makeHistoryItem({ id: "child-1", status: "active" }),
historyItem: makeHistoryItem({ id: "child-1", mode: "code", status: "active" }),
}),
updateTaskHistory: vi.fn().mockResolvedValue(undefined),
handleModeSwitch: vi.fn().mockResolvedValue(undefined),
@ -73,6 +73,8 @@ describe("advanceSubtaskQueue", () => {
log: vi.fn(),
}
// Queue items represent ADDITIONAL subtasks after the initial child.
// subtaskQueueIndex=0 means queue[0] is the next to dispatch.
const subtaskQueue: SubtaskQueueItem[] = [
{ mode: "code", message: "Step 1" },
{ mode: "debug", message: "Step 2" },
@ -88,7 +90,7 @@ describe("advanceSubtaskQueue", () => {
const result = await (ClineProvider.prototype as any).advanceSubtaskQueue.call(provider, {
parentTaskId: "parent-1",
childTaskId: "child-1",
completionResultSummary: "Step 1 done",
completionResultSummary: "Initial task done",
historyItem,
})
@ -102,11 +104,11 @@ describe("advanceSubtaskQueue", () => {
expect.objectContaining({ id: "child-1", status: "completed" }),
)
// Should have switched mode to next subtask's mode
expect(provider.handleModeSwitch).toHaveBeenCalledWith("debug")
// Should have switched mode to queue[0]'s mode (the next item to dispatch)
expect(provider.handleModeSwitch).toHaveBeenCalledWith("code")
// Should have created the next child with the queued message
expect(provider.createTask).toHaveBeenCalledWith("Step 2", undefined, undefined, {
// Should have created the next child with queue[0]'s message
expect(provider.createTask).toHaveBeenCalledWith("Step 1", undefined, undefined, {
initialTodos: [],
initialStatus: "active",
startTask: false,
@ -115,12 +117,13 @@ describe("advanceSubtaskQueue", () => {
// Should have started the next child
expect(mockChild.start).toHaveBeenCalled()
// Should have updated parent with advanced queue index
// Should have updated parent with advanced queue index (0 -> 1)
// completedMode comes from child's history (mode: "code")
expect(provider.updateTaskHistory).toHaveBeenCalledWith(
expect.objectContaining({
id: "parent-1",
subtaskQueueIndex: 1,
subtaskResults: [{ taskId: "child-1", mode: "unknown", summary: "Step 1 done" }],
subtaskResults: [{ taskId: "child-1", mode: "code", summary: "Initial task done" }],
awaitingChildId: "child-2",
delegatedToId: "child-2",
}),
@ -131,7 +134,7 @@ describe("advanceSubtaskQueue", () => {
RooCodeEventName.TaskDelegationCompleted,
"parent-1",
"child-1",
"Step 1 done",
"Initial task done",
)
expect(emitSpy).toHaveBeenCalledWith(RooCodeEventName.TaskDelegated, "parent-1", "child-2")
})
@ -141,7 +144,7 @@ describe("advanceSubtaskQueue", () => {
getCurrentTask: vi.fn().mockReturnValue({ taskId: "child-2" }),
removeClineFromStack: vi.fn().mockResolvedValue(undefined),
getTaskWithId: vi.fn().mockResolvedValue({
historyItem: makeHistoryItem({ id: "child-2", status: "active" }),
historyItem: makeHistoryItem({ id: "child-2", mode: "code", status: "active" }),
}),
updateTaskHistory: vi.fn().mockResolvedValue(undefined),
handleModeSwitch: vi.fn(),
@ -151,11 +154,13 @@ describe("advanceSubtaskQueue", () => {
formatAggregatedQueueResults: (ClineProvider.prototype as any).formatAggregatedQueueResults,
}
// Queue has 1 item, subtaskQueueIndex=1 means queue[0] was already dispatched.
// Now that child completes and the queue is exhausted.
const subtaskQueue: SubtaskQueueItem[] = [{ mode: "code", message: "Step 1" }]
const historyItem = makeHistoryItem({
subtaskQueue,
subtaskQueueIndex: 0,
subtaskQueueIndex: 1,
subtaskResults: [{ taskId: "child-1", mode: "code", summary: "Step 1 done" }],
childIds: ["child-1", "child-2"],
})

View file

@ -3397,15 +3397,10 @@ export class ClineProvider
return { handled: false, aggregatedSummary: completionResultSummary }
}
// currentIndex is the next queue item to dispatch (0-based).
// When the initial child (from mode/message params) completes, currentIndex is 0,
// meaning queue[0] should be dispatched first.
const currentIndex = subtaskQueueIndex ?? 0
const nextIndex = currentIndex + 1
// Record this child's result
const completedMode = historyItem.mode ?? "unknown"
const updatedResults = [
...(subtaskResults ?? []),
{ taskId: childTaskId, mode: completedMode, summary: completionResultSummary },
]
// Close current child if still open
const current = this.getCurrentTask()
@ -3413,9 +3408,11 @@ export class ClineProvider
await this.removeClineFromStack()
}
// Mark child as completed
// Fetch child history to get the child's actual mode and mark it completed
let completedMode = "unknown"
try {
const { historyItem: childHistory } = await this.getTaskWithId(childTaskId)
completedMode = childHistory.mode ?? "unknown"
await this.updateTaskHistory({ ...childHistory, status: "completed" })
} catch (err) {
this.log(
@ -3425,6 +3422,12 @@ export class ClineProvider
)
}
// Record this child's result using the child's actual mode
const updatedResults = [
...(subtaskResults ?? []),
{ taskId: childTaskId, mode: completedMode, summary: completionResultSummary },
]
// Emit completion event for the finished child
try {
this.emit(RooCodeEventName.TaskDelegationCompleted, parentTaskId, childTaskId, completionResultSummary)
@ -3432,11 +3435,11 @@ export class ClineProvider
// non-fatal
}
if (nextIndex <= subtaskQueue.length - 1) {
if (currentIndex < subtaskQueue.length) {
// More subtasks in queue — start the next one
const nextSubtask = subtaskQueue[nextIndex]
const nextSubtask = subtaskQueue[currentIndex]
this.log(
`[advanceSubtaskQueue] Auto-advancing queue: subtask ${nextIndex + 1}/${subtaskQueue.length} (mode: ${nextSubtask.mode})`,
`[advanceSubtaskQueue] Auto-advancing queue: subtask ${currentIndex + 1}/${subtaskQueue.length} (mode: ${nextSubtask.mode})`,
)
// Switch mode
@ -3466,7 +3469,7 @@ export class ClineProvider
awaitingChildId: nextChild.taskId,
childIds,
subtaskQueue,
subtaskQueueIndex: nextIndex,
subtaskQueueIndex: currentIndex + 1,
subtaskResults: updatedResults,
})