From 9ad099fad8df58d82c24c203aec7c40aa0e8afb5 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Sun, 20 Sep 2026 07:05:38 +0900 Subject: [PATCH 1/3] fix(task): apply command protection checks in queued-message drain --- src/core/task/Task.ts | 13 ++++-- .../ask-queued-message-drain.spec.ts | 44 +++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 55798437c3..e53bbf6c31 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1458,8 +1458,14 @@ export class Task extends EventEmitter implements TaskLike { // rendered, leaving them stuck on-screen). const provider = this.providerRef.deref() const state = provider ? await provider.getState() : undefined + // 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" ? undefined : this.messageQueueService.claimNextMessage() + partial === true || type === "command_output" || isProtected + ? undefined + : this.messageQueueService.claimNextMessage() const queuedAskResolution = queuedMessage ? queuedResponseForAsk(type, text) : undefined // `this.cwd`, not `provider.cwd`: // The path inside `text` was made relative to this task's workspace, @@ -1657,8 +1663,9 @@ export class Task extends EventEmitter implements TaskLike { // If a queued message arrives while we're blocked on an ask (e.g. a follow-up // suggestion click that was incorrectly queued due to UI state), consume it - // immediately so the task doesn't hang. - if (shouldDrainQueuedMessageForAsk && !this.messageQueueService.isEmpty()) { + // immediately so the task doesn't hang. Protected asks are exempt: like the + // pre-block drain above, they must wait for explicit user approval. + if (shouldDrainQueuedMessageForAsk && !isProtected && !this.messageQueueService.isEmpty()) { const message = this.messageQueueService.claimNextMessage() const resolution = message ? queuedResponseForAsk(type, text) : undefined if (message && resolution) { 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..4fbe998490 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,50 @@ 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 }) + expect(task.messageQueueService.messages[0]?.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 }) + expect(task.messageQueueService.messages[0]?.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) From 4198ee2628c3fafd99104ba5bba42f659afe2450 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Sun, 20 Sep 2026 08:46:25 +0900 Subject: [PATCH 2/3] test(task): assert queued message stays unclaimed via claimNextMessage --- src/core/task/__tests__/ask-queued-message-drain.spec.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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 4fbe998490..ef5ac08031 100644 --- a/src/core/task/__tests__/ask-queued-message-drain.spec.ts +++ b/src/core/task/__tests__/ask-queued-message-drain.spec.ts @@ -142,7 +142,9 @@ describe("Task.ask queued message drain", () => { task.approveAsk() const result = await ask expect(result).toMatchObject({ response: "yesButtonClicked", text: undefined }) - expect(task.messageQueueService.messages[0]?.text).toBe("run it anyway") + // 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 () => { @@ -163,7 +165,9 @@ describe("Task.ask queued message drain", () => { task.approveAsk() const result = await ask expect(result).toMatchObject({ response: "yesButtonClicked", text: undefined }) - expect(task.messageQueueService.messages[0]?.text).toBe("mid-wait message") + // 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 () => { From a3ea9df737ac67f6f950b4ffcc5c6a6ebc774c65 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Sun, 4 Oct 2026 06:44:44 +0900 Subject: [PATCH 3/3] test(task): pin protected write-tool asks against queued-message drain What: mirror the two protected-command drain tests for protected tool asks, using the write-tool path (alwaysAllowWrite on, alwaysAllowWriteProtected unset, isProtected set) that presentAssistantMessage uses for WriteToFileTool/EditTool/ ApplyDiff/ApplyPatch blocked writes. Why: the drain guards key on the isProtected flag, so protected write-tool asks are covered too, but only the command case was pinned (review round converged on this gap). Impact: red without the guards (4 protected tests fail), green with them (674 core/task tests pass); diff is tests only. --- .../ask-queued-message-drain.spec.ts | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) 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 ef5ac08031..bb74531d45 100644 --- a/src/core/task/__tests__/ask-queued-message-drain.spec.ts +++ b/src/core/task/__tests__/ask-queued-message-drain.spec.ts @@ -170,6 +170,72 @@ describe("Task.ask queued message drain", () => { 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)