From 26e39c7905dccff12f3584567285ed4100b1cb52 Mon Sep 17 00:00:00 2001 From: 0xMink Date: Thu, 12 Feb 2026 02:53:54 -0500 Subject: [PATCH] fix(orchestrator): enforce todo-gated completion for orchestrator mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Orchestrator mode can prematurely complete tasks when context condensation causes the model to lose track of outstanding work. Force preventCompletionWithOpenTodos for orchestrator mode regardless of the user setting. Also require a todo list to exist before orchestrator completion is allowed — completion without a plan is structurally premature. --- src/core/tools/AttemptCompletionTool.ts | 22 ++- .../__tests__/attemptCompletionTool.spec.ts | 166 ++++++++++++++++++ 2 files changed, 185 insertions(+), 3 deletions(-) diff --git a/src/core/tools/AttemptCompletionTool.ts b/src/core/tools/AttemptCompletionTool.ts index a406a15c8b4..3fe60244229 100644 --- a/src/core/tools/AttemptCompletionTool.ts +++ b/src/core/tools/AttemptCompletionTool.ts @@ -49,9 +49,25 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { return } - const preventCompletionWithOpenTodos = vscode.workspace - .getConfiguration(Package.name) - .get("preventCompletionWithOpenTodos", false) + const isOrchestratorMode = task.taskMode === "orchestrator" + + const preventCompletionWithOpenTodos = + isOrchestratorMode || + vscode.workspace.getConfiguration(Package.name).get("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") diff --git a/src/core/tools/__tests__/attemptCompletionTool.spec.ts b/src/core/tools/__tests__/attemptCompletionTool.spec.ts index 9aac6296c6c..6f72318b653 100644 --- a/src/core/tools/__tests__/attemptCompletionTool.spec.ts +++ b/src/core/tools/__tests__/attemptCompletionTool.spec.ts @@ -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 = {