Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
22 changes: 19 additions & 3 deletions src/core/tools/AttemptCompletionTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,25 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> {
return
}

const preventCompletionWithOpenTodos = vscode.workspace
.getConfiguration(Package.name)
.get<boolean>("preventCompletionWithOpenTodos", false)
const isOrchestratorMode = task.taskMode === "orchestrator"

const preventCompletionWithOpenTodos =
isOrchestratorMode ||
vscode.workspace.getConfiguration(Package.name).get<boolean>("preventCompletionWithOpenTodos", false)

// Orchestrator must have a todo list to complete — completion without a plan is premature.
if (isOrchestratorMode && (!task.todoList || task.todoList.length === 0)) {
task.consecutiveMistakeCount++
task.recordToolError("attempt_completion")

pushToolResult(
formatResponse.toolError(
"Orchestrator tasks require a todo list before completion. Use update_todo_list to create a plan and track progress before attempting completion.",
),
)

return
}

const hasIncompleteTodos = task.todoList && task.todoList.some((todo) => todo.status !== "completed")

Expand Down
166 changes: 166 additions & 0 deletions src/core/tools/__tests__/attemptCompletionTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,172 @@ describe("attemptCompletionTool", () => {
)
})

describe("orchestrator completion guard", () => {
it("should reject completion in orchestrator mode when no todo list exists", async () => {
const block: AttemptCompletionToolUse = {
type: "tool_use",
name: "attempt_completion",
params: { result: "Task completed successfully" },
nativeArgs: { result: "Task completed successfully" },
partial: false,
}

mockTask.taskMode = "orchestrator" as any
mockTask.todoList = undefined

const callbacks: AttemptCompletionCallbacks = {
askApproval: mockAskApproval,
handleError: mockHandleError,
pushToolResult: mockPushToolResult,
askFinishSubTaskApproval: mockAskFinishSubTaskApproval,
toolDescription: mockToolDescription,
}
await attemptCompletionTool.handle(mockTask as Task, block, callbacks)

expect(mockTask.consecutiveMistakeCount).toBe(1)
expect(mockTask.recordToolError).toHaveBeenCalledWith("attempt_completion")
expect(mockPushToolResult).toHaveBeenCalledWith(
expect.stringContaining("Orchestrator tasks require a todo list"),
)
expect(mockTask.ask).not.toHaveBeenCalled()
})

it("should reject completion in orchestrator mode when todo list is empty", async () => {
const block: AttemptCompletionToolUse = {
type: "tool_use",
name: "attempt_completion",
params: { result: "Task completed successfully" },
nativeArgs: { result: "Task completed successfully" },
partial: false,
}

mockTask.taskMode = "orchestrator" as any
mockTask.todoList = []

const callbacks: AttemptCompletionCallbacks = {
askApproval: mockAskApproval,
handleError: mockHandleError,
pushToolResult: mockPushToolResult,
askFinishSubTaskApproval: mockAskFinishSubTaskApproval,
toolDescription: mockToolDescription,
}
await attemptCompletionTool.handle(mockTask as Task, block, callbacks)

expect(mockTask.consecutiveMistakeCount).toBe(1)
expect(mockTask.recordToolError).toHaveBeenCalledWith("attempt_completion")
expect(mockPushToolResult).toHaveBeenCalledWith(
expect.stringContaining("Orchestrator tasks require a todo list"),
)
expect(mockTask.ask).not.toHaveBeenCalled()
})

it("should reject completion in orchestrator mode with incomplete todos regardless of setting", async () => {
const block: AttemptCompletionToolUse = {
type: "tool_use",
name: "attempt_completion",
params: { result: "Task completed successfully" },
nativeArgs: { result: "Task completed successfully" },
partial: false,
}

mockTask.taskMode = "orchestrator" as any
mockTask.todoList = [
{ id: "1", content: "First task", status: "completed" },
{ id: "2", content: "Second task", status: "pending" },
]

// Setting is disabled, but orchestrator should still enforce
mockGetConfiguration.mockReturnValue({
get: vi.fn((key: string, defaultValue: any) => {
if (key === "preventCompletionWithOpenTodos") {
return false
}
return defaultValue
}),
})

const callbacks: AttemptCompletionCallbacks = {
askApproval: mockAskApproval,
handleError: mockHandleError,
pushToolResult: mockPushToolResult,
askFinishSubTaskApproval: mockAskFinishSubTaskApproval,
toolDescription: mockToolDescription,
}
await attemptCompletionTool.handle(mockTask as Task, block, callbacks)

expect(mockTask.consecutiveMistakeCount).toBe(1)
expect(mockTask.recordToolError).toHaveBeenCalledWith("attempt_completion")
expect(mockPushToolResult).toHaveBeenCalledWith(
expect.stringContaining("Cannot complete task while there are incomplete todos"),
)
expect(mockTask.ask).not.toHaveBeenCalled()
})

it("should allow completion in orchestrator mode when all todos are completed", async () => {
const block: AttemptCompletionToolUse = {
type: "tool_use",
name: "attempt_completion",
params: { result: "Task completed successfully" },
nativeArgs: { result: "Task completed successfully" },
partial: false,
}

mockTask.taskMode = "orchestrator" as any
mockTask.todoList = [
{ id: "1", content: "First task", status: "completed" },
{ id: "2", content: "Second task", status: "completed" },
]

const callbacks: AttemptCompletionCallbacks = {
askApproval: mockAskApproval,
handleError: mockHandleError,
pushToolResult: mockPushToolResult,
askFinishSubTaskApproval: mockAskFinishSubTaskApproval,
toolDescription: mockToolDescription,
}
await attemptCompletionTool.handle(mockTask as Task, block, callbacks)

expect(mockTask.consecutiveMistakeCount).toBe(0)
expect(mockTask.recordToolError).not.toHaveBeenCalled()
})

it("should not apply orchestrator guard to non-orchestrator modes", async () => {
const block: AttemptCompletionToolUse = {
type: "tool_use",
name: "attempt_completion",
params: { result: "Task completed successfully" },
nativeArgs: { result: "Task completed successfully" },
partial: false,
}

mockTask.taskMode = "code" as any
mockTask.todoList = undefined

// Setting is disabled
mockGetConfiguration.mockReturnValue({
get: vi.fn((key: string, defaultValue: any) => {
if (key === "preventCompletionWithOpenTodos") {
return false
}
return defaultValue
}),
})

const callbacks: AttemptCompletionCallbacks = {
askApproval: mockAskApproval,
handleError: mockHandleError,
pushToolResult: mockPushToolResult,
askFinishSubTaskApproval: mockAskFinishSubTaskApproval,
toolDescription: mockToolDescription,
}
await attemptCompletionTool.handle(mockTask as Task, block, callbacks)

// Non-orchestrator with no todos and setting disabled should succeed
expect(mockTask.consecutiveMistakeCount).toBe(0)
expect(mockTask.recordToolError).not.toHaveBeenCalled()
})
})

describe("tool failure guardrail", () => {
it("should prevent completion when a previous tool failed in the current turn", async () => {
const block: AttemptCompletionToolUse = {
Expand Down
Loading