Skip to content

Commit ce71bef

Browse files
daniel-lxsdaniel-lxs
committed
fix: defer new_task tool_result until subtask completes for native protocol
For native tools, the API requires strict message ordering (User > Assistant > Tool Call > Tool Result). The new_task tool was causing 400 errors because: 1. NewTaskTool pushed a tool_result immediately 2. Parent task paused for subtask 3. completeSubtask added a separate user message 4. This resulted in consecutive user messages which broke the ordering This fix defers the tool_result until the subtask completes: - NewTaskTool stores pendingNewTaskToolCallId but doesn't push tool_result (native only) - Task loop stays alive via isPaused condition in stack push - completeSubtask pushes the actual tool_result with subtask's real result - After waitForSubtask, content is copied to currentUserContent Benefits: - No more 400 errors with native tools - Parent task now sees what the subtask actually accomplished - XML protocol behavior is unchanged
1 parent 3cd3357 commit ce71bef

File tree

5 files changed

+201
-9
lines changed

5 files changed

+201
-9
lines changed

‎src/core/assistant-message/presentAssistantMessage.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,7 @@ export async function presentAssistantMessage(cline: Task) {
938938
pushToolResult,
939939
removeClosingTag,
940940
toolProtocol,
941+
toolCallId: block.id,
941942
})
942943
break
943944
case "attempt_completion": {

‎src/core/task/Task.ts‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
157157
readonly rootTaskId?: string
158158
readonly parentTaskId?: string
159159
childTaskId?: string
160+
pendingNewTaskToolCallId?: string
160161

161162
readonly instanceId: string
162163
readonly metadata: TaskMetadata
@@ -1967,10 +1968,29 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
19671968
try {
19681969
await this.say("subtask_result", lastMessage)
19691970

1970-
await this.addToApiConversationHistory({
1971-
role: "user",
1972-
content: [{ type: "text", text: `[new_task completed] Result: ${lastMessage}` }],
1973-
})
1971+
// Check if using native protocol to determine how to add the subtask result
1972+
const modelInfo = this.api.getModel().info
1973+
const toolProtocol = resolveToolProtocol(this.apiConfiguration, modelInfo)
1974+
1975+
if (toolProtocol === "native" && this.pendingNewTaskToolCallId) {
1976+
// For native protocol, push the actual tool_result with the subtask's real result.
1977+
// NewTaskTool deferred pushing the tool_result until now so that the parent task
1978+
// gets useful information about what the subtask actually accomplished.
1979+
this.userMessageContent.push({
1980+
type: "tool_result",
1981+
tool_use_id: this.pendingNewTaskToolCallId,
1982+
content: `[new_task completed] Result: ${lastMessage}`,
1983+
} as Anthropic.ToolResultBlockParam)
1984+
1985+
// Clear the pending tool call ID
1986+
this.pendingNewTaskToolCallId = undefined
1987+
} else {
1988+
// For XML protocol (or if no pending tool call ID), add as a separate user message
1989+
await this.addToApiConversationHistory({
1990+
role: "user",
1991+
content: [{ type: "text", text: `[new_task completed] Result: ${lastMessage}` }],
1992+
})
1993+
}
19741994
} catch (error) {
19751995
this.providerRef
19761996
.deref()
@@ -2073,6 +2093,14 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
20732093
provider.log(`[subtasks] paused ${this.taskId}.${this.instanceId}`)
20742094
await this.waitForSubtask()
20752095
provider.log(`[subtasks] resumed ${this.taskId}.${this.instanceId}`)
2096+
2097+
// After subtask completes, completeSubtask has pushed content to userMessageContent.
2098+
// Copy it to currentUserContent so it gets sent to the API in this iteration.
2099+
if (this.userMessageContent.length > 0) {
2100+
currentUserContent.push(...this.userMessageContent)
2101+
this.userMessageContent = []
2102+
}
2103+
20762104
const currentMode = (await provider.getState())?.mode ?? defaultModeSlug
20772105

20782106
if (currentMode !== this.pausedModeSlug) {
@@ -2992,7 +3020,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
29923020
this.consecutiveMistakeCount++
29933021
}
29943022

2995-
if (this.userMessageContent.length > 0) {
3023+
// Push to stack if there's content OR if we're paused waiting for a subtask.
3024+
// When paused, we push an empty item so the loop continues to the pause check.
3025+
if (this.userMessageContent.length > 0 || this.isPaused) {
29963026
stack.push({
29973027
userContent: [...this.userMessageContent], // Create a copy to avoid mutation issues
29983028
includeFileDetails: false, // Subsequent iterations don't need file details

‎src/core/task/__tests__/Task.spec.ts‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1966,4 +1966,153 @@ describe("Queued message processing after condense", () => {
19661966
expect(spyB).toHaveBeenCalledWith("B message", undefined)
19671967
expect(taskB.messageQueueService.isEmpty()).toBe(true)
19681968
})
1969+
1970+
describe("completeSubtask native protocol handling", () => {
1971+
let mockProvider: any
1972+
let mockApiConfig: any
1973+
1974+
beforeEach(() => {
1975+
vi.clearAllMocks()
1976+
1977+
if (!TelemetryService.hasInstance()) {
1978+
TelemetryService.createInstance([])
1979+
}
1980+
1981+
mockApiConfig = {
1982+
apiProvider: "anthropic",
1983+
apiKey: "test-key",
1984+
}
1985+
1986+
mockProvider = {
1987+
context: {
1988+
globalStorageUri: { fsPath: "/test/storage" },
1989+
},
1990+
getState: vi.fn().mockResolvedValue({
1991+
apiConfiguration: mockApiConfig,
1992+
}),
1993+
say: vi.fn(),
1994+
postStateToWebview: vi.fn().mockResolvedValue(undefined),
1995+
postMessageToWebview: vi.fn().mockResolvedValue(undefined),
1996+
updateTaskHistory: vi.fn().mockResolvedValue(undefined),
1997+
log: vi.fn(),
1998+
}
1999+
})
2000+
2001+
it("should push tool_result to userMessageContent for native protocol with pending tool call ID", async () => {
2002+
// Create a task with a model that supports native tools
2003+
const task = new Task({
2004+
provider: mockProvider,
2005+
apiConfiguration: {
2006+
...mockApiConfig,
2007+
apiProvider: "anthropic",
2008+
toolProtocol: "native", // Explicitly set native protocol
2009+
},
2010+
task: "parent task",
2011+
startTask: false,
2012+
})
2013+
2014+
// Mock the API to return a native protocol model
2015+
vi.spyOn(task.api, "getModel").mockReturnValue({
2016+
id: "claude-3-5-sonnet-20241022",
2017+
info: {
2018+
contextWindow: 200000,
2019+
maxTokens: 8192,
2020+
supportsPromptCache: true,
2021+
supportsNativeTools: true,
2022+
defaultToolProtocol: "native",
2023+
} as ModelInfo,
2024+
})
2025+
2026+
// For native protocol, NewTaskTool does NOT push tool_result immediately.
2027+
// It only sets the pending tool call ID. The actual tool_result is pushed by completeSubtask.
2028+
task.pendingNewTaskToolCallId = "test-tool-call-id"
2029+
2030+
// Call completeSubtask
2031+
await task.completeSubtask("Subtask completed successfully")
2032+
2033+
// For native protocol, should push the actual tool_result with the subtask's result
2034+
expect(task.userMessageContent).toHaveLength(1)
2035+
expect(task.userMessageContent[0]).toEqual({
2036+
type: "tool_result",
2037+
tool_use_id: "test-tool-call-id",
2038+
content: "[new_task completed] Result: Subtask completed successfully",
2039+
})
2040+
2041+
// Should NOT have added a user message to apiConversationHistory
2042+
expect(task.apiConversationHistory).toHaveLength(0)
2043+
2044+
// pending tool call ID should be cleared
2045+
expect(task.pendingNewTaskToolCallId).toBeUndefined()
2046+
})
2047+
2048+
it("should add user message to apiConversationHistory for XML protocol", async () => {
2049+
// Create a task with a model that doesn't support native tools
2050+
const task = new Task({
2051+
provider: mockProvider,
2052+
apiConfiguration: {
2053+
...mockApiConfig,
2054+
apiProvider: "anthropic",
2055+
},
2056+
task: "parent task",
2057+
startTask: false,
2058+
})
2059+
2060+
// Mock the API to return an XML protocol model (no native tool support)
2061+
vi.spyOn(task.api, "getModel").mockReturnValue({
2062+
id: "claude-2",
2063+
info: {
2064+
contextWindow: 100000,
2065+
maxTokens: 4096,
2066+
supportsPromptCache: false,
2067+
supportsNativeTools: false,
2068+
} as ModelInfo,
2069+
})
2070+
2071+
// Call completeSubtask
2072+
await task.completeSubtask("Subtask completed successfully")
2073+
2074+
// For XML protocol, should add to apiConversationHistory
2075+
expect(task.apiConversationHistory).toHaveLength(1)
2076+
expect(task.apiConversationHistory[0]).toEqual(
2077+
expect.objectContaining({
2078+
role: "user",
2079+
content: [{ type: "text", text: "[new_task completed] Result: Subtask completed successfully" }],
2080+
}),
2081+
)
2082+
2083+
// Should NOT have added to userMessageContent
2084+
expect(task.userMessageContent).toHaveLength(0)
2085+
})
2086+
2087+
it("should set isPaused to false after completeSubtask", async () => {
2088+
const task = new Task({
2089+
provider: mockProvider,
2090+
apiConfiguration: mockApiConfig,
2091+
task: "parent task",
2092+
startTask: false,
2093+
})
2094+
2095+
// Mock the API to return an XML protocol model
2096+
vi.spyOn(task.api, "getModel").mockReturnValue({
2097+
id: "claude-2",
2098+
info: {
2099+
contextWindow: 100000,
2100+
maxTokens: 4096,
2101+
supportsPromptCache: false,
2102+
supportsNativeTools: false,
2103+
} as ModelInfo,
2104+
})
2105+
2106+
// Set isPaused to true (simulating waiting for subtask)
2107+
task.isPaused = true
2108+
task.childTaskId = "child-task-id"
2109+
2110+
// Call completeSubtask
2111+
await task.completeSubtask("Subtask completed")
2112+
2113+
// Should reset paused state
2114+
expect(task.isPaused).toBe(false)
2115+
expect(task.childTaskId).toBeUndefined()
2116+
})
2117+
})
19692118
})

‎src/core/tools/BaseTool.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ export interface ToolCallbacks {
1818
pushToolResult: PushToolResult
1919
removeClosingTag: RemoveClosingTag
2020
toolProtocol: ToolProtocol
21+
toolCallId?: string
2122
}
2223

2324
/**

‎src/core/tools/NewTaskTool.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export class NewTaskTool extends BaseTool<"new_task"> {
3030

3131
async execute(params: NewTaskParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
3232
const { mode, message, todos } = params
33-
const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks
33+
const { askApproval, handleError, pushToolResult, toolProtocol, toolCallId } = callbacks
3434

3535
try {
3636
// Validate required parameters.
@@ -133,9 +133,20 @@ export class NewTaskTool extends BaseTool<"new_task"> {
133133
return
134134
}
135135

136-
pushToolResult(
137-
`Successfully created new task in ${targetMode.name} mode with message: ${unescapedMessage} and ${todoItems.length} todo items`,
138-
)
136+
// For native protocol, defer the tool_result until the subtask completes.
137+
// The actual result (including what the subtask accomplished) will be pushed
138+
// by completeSubtask. This gives the parent task useful information about
139+
// what the subtask actually did.
140+
if (toolProtocol === "native" && toolCallId) {
141+
task.pendingNewTaskToolCallId = toolCallId
142+
// Don't push tool_result here - it will come from completeSubtask with the actual result.
143+
// The task loop will stay alive because isPaused is true (see Task.ts stack push condition).
144+
} else {
145+
// For XML protocol, push the result immediately (existing behavior)
146+
pushToolResult(
147+
`Successfully created new task in ${targetMode.name} mode with message: ${unescapedMessage} and ${todoItems.length} todo items`,
148+
)
149+
}
139150

140151
return
141152
} catch (error) {

0 commit comments

Comments
 (0)