diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 7f92f3647f..dc07aadf25 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1791,9 +1791,14 @@ export class Task extends EventEmitter implements TaskLike { // has to see and stalling a hands-free session. Leaving the message // unclaimed lets the policy decision stand and keeps the message queued. const queueMayAnswerThisAsk = !(blanketDenyEngaged && type === "command") + // A queued message must not short-circuit protected asks (e.g. + // DCG-blocked commands): claiming one here skips checkAutoApproval, and + // the drain below would auto-approve what protection intentionally + // leaves pending for explicit user approval. const queuedMessage = partial === true || type === "command_output" || + isProtected || !queueMayAnswerThisAsk || !this.mayDrainQueuedMessageForAsk() ? undefined @@ -2186,10 +2191,13 @@ export class Task extends EventEmitter implements TaskLike { // suggestion click that was incorrectly queued due to UI state), consume it // immediately so the task doesn't hang. Command asks under blanket deny are // excluded (`queueMayAnswerThisAsk`): a queued message must never stand in - // for the explicit approval the policy withheld. + // for the explicit approval the policy withheld. Protected asks are exempt + // too: like the pre-block drain above, they must wait for explicit user + // approval. if ( queueMayAnswerThisAsk && shouldDrainQueuedMessageForAsk && + !isProtected && !queuedCommandPolicyCheck && this.mayDrainQueuedMessageForAsk() ) { diff --git a/src/core/task/__tests__/ask-queued-message-drain.spec.ts b/src/core/task/__tests__/ask-queued-message-drain.spec.ts index b137130174..bb74531d45 100644 --- a/src/core/task/__tests__/ask-queued-message-drain.spec.ts +++ b/src/core/task/__tests__/ask-queued-message-drain.spec.ts @@ -122,6 +122,120 @@ describe("Task.ask queued message drain", () => { expect(task.messageQueueService.isEmpty()).toBe(true) }) + it("does not auto-approve a protected command ask from a queued message", async () => { + const task = await createTask({ + getState: async () => ({ autoApprovalEnabled: true, alwaysAllowExecute: true }), + }) + task.messageQueueService.addMessage("run it anyway") + + const ask = task.ask("command", "rm -rf build", false, undefined, true) + + // The drain must not resolve the protected ask; only an explicit user + // approval may. + const premature = await Promise.race([ + ask, + new Promise<"pending">((resolve) => setTimeout(() => resolve("pending"), 250)), + ]) + expect(premature).toBe("pending") + expect(task.messageQueueService.isEmpty()).toBe(false) + + task.approveAsk() + const result = await ask + expect(result).toMatchObject({ response: "yesButtonClicked", text: undefined }) + // claimNextMessage only returns unclaimed messages: the queued message must + // still be present and unclaimed after the protected ask resolved. + expect(task.messageQueueService.claimNextMessage()?.text).toBe("run it anyway") + }) + + it("does not auto-approve a protected command ask when a message is queued mid-wait", async () => { + const task = await createTask({ + getState: async () => ({ autoApprovalEnabled: true, alwaysAllowExecute: true }), + }) + + const ask = task.ask("command", "rm -rf build", false, undefined, true) + task.messageQueueService.addMessage("mid-wait message") + + const premature = await Promise.race([ + ask, + new Promise<"pending">((resolve) => setTimeout(() => resolve("pending"), 250)), + ]) + expect(premature).toBe("pending") + expect(task.messageQueueService.isEmpty()).toBe(false) + + task.approveAsk() + const result = await ask + expect(result).toMatchObject({ response: "yesButtonClicked", text: undefined }) + // claimNextMessage only returns unclaimed messages: the queued message must + // still be present and unclaimed after the protected ask resolved. + expect(task.messageQueueService.claimNextMessage()?.text).toBe("mid-wait message") + }) + + // The drain guards key on the isProtected flag, so they also cover protected + // write-tool asks (WriteToFileTool/EditTool/ApplyDiff/ApplyPatch mark blocked + // writes protected via presentAssistantMessage). These mirror the command pair + // above: alwaysAllowWrite covers the write, but without alwaysAllowWriteProtected + // the protection keeps the ask pending, and a queued message must not stand in + // for the explicit approval. + it("does not auto-approve a protected write-tool ask from a queued message", async () => { + const task = await createTask({ + getState: async () => ({ autoApprovalEnabled: true, alwaysAllowWrite: true }), + }) + task.messageQueueService.addMessage("write it anyway") + + const ask = task.ask( + "tool", + JSON.stringify({ tool: "newFileCreated", path: "AGENTS.md" }), + false, + undefined, + true, + ) + + // The drain must not resolve the protected ask; only an explicit user + // approval may. + const premature = await Promise.race([ + ask, + new Promise<"pending">((resolve) => setTimeout(() => resolve("pending"), 250)), + ]) + expect(premature).toBe("pending") + expect(task.messageQueueService.isEmpty()).toBe(false) + + task.approveAsk() + const result = await ask + expect(result).toMatchObject({ response: "yesButtonClicked", text: undefined }) + // claimNextMessage only returns unclaimed messages: the queued message must + // still be present and unclaimed after the protected ask resolved. + expect(task.messageQueueService.claimNextMessage()?.text).toBe("write it anyway") + }) + + it("does not auto-approve a protected write-tool ask when a message is queued mid-wait", async () => { + const task = await createTask({ + getState: async () => ({ autoApprovalEnabled: true, alwaysAllowWrite: true }), + }) + + const ask = task.ask( + "tool", + JSON.stringify({ tool: "newFileCreated", path: "AGENTS.md" }), + false, + undefined, + true, + ) + task.messageQueueService.addMessage("mid-wait message") + + const premature = await Promise.race([ + ask, + new Promise<"pending">((resolve) => setTimeout(() => resolve("pending"), 250)), + ]) + expect(premature).toBe("pending") + expect(task.messageQueueService.isEmpty()).toBe(false) + + task.approveAsk() + const result = await ask + expect(result).toMatchObject({ response: "yesButtonClicked", text: undefined }) + // claimNextMessage only returns unclaimed messages: the queued message must + // still be present and unclaimed after the protected ask resolved. + expect(task.messageQueueService.claimNextMessage()?.text).toBe("mid-wait message") + }) + it("claims lifecycle feedback that arrives while an ask is waiting", async () => { const task = await createTask() const ask = task.ask("tool", JSON.stringify({ tool: "finishTask" }), false)