Skip to content

Commit 494e8eb

Browse files
allquixoticclaude
andcommitted
fix(queue): never destroy user messages sent while no ask is pending
The primary 'my message went nowhere' sink: ask() clears askResponse/ askResponseText at the start of every complete ask, so a messageResponse arriving while no ask() was blocked (webview raced a phase transition — e.g. during the API-request phase — or submitUserMessage was invoked mid-stream) was silently destroyed. - Task: track pendingAskCount around ask()'s wait region; orphaned text-bearing messageResponses are rerouted into the task's message queue, where the queue/steer handoff delivers them at the next safe boundary (followup/idle ask auto-dispatch, resume, or run end). - ChatView: when a state push built during a transient no-current-task window (cancel/rehydrate, delegation swap) blips currentTaskId to undefined, queue with the last known task id for the visible conversation instead of dropping the message on the floor. - Tests: pin the orphaned-response invariants (queued not destroyed, direct delivery while an ask waits, auto-delivery at the next followup ask, button responses unchanged) and the blip fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 1b10cb3 commit 494e8eb

4 files changed

Lines changed: 319 additions & 92 deletions

File tree

src/core/task/Task.ts

Lines changed: 118 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,11 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
314314
private askResponseImages?: string[]
315315
public lastMessageTs?: number
316316
private autoApprovalTimeoutRef?: NodeJS.Timeout
317+
// Number of ask() calls currently blocked waiting for a response. Used by
318+
// handleWebviewAskResponse to detect "orphaned" messageResponses: ask() clears
319+
// askResponse/askResponseText at the start of every new complete ask, so a
320+
// response written while nothing is waiting would be silently destroyed.
321+
private pendingAskCount = 0
317322

318323
// Tool Use
319324
consecutiveMistakeCount: number = 0
@@ -1383,97 +1388,108 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
13831388

13841389
const timeouts: NodeJS.Timeout[] = []
13851390

1386-
// Auto-approval (state/approval) was resolved above before the message was added.
1387-
if (approval.decision === "approve") {
1388-
this.approveAsk()
1389-
} else if (approval.decision === "deny") {
1390-
this.denyAsk()
1391-
} else if (approval.decision === "timeout") {
1392-
// Store the auto-approval timeout so it can be cancelled if user interacts
1393-
this.autoApprovalTimeoutRef = setTimeout(() => {
1394-
const { askResponse, text, images } = approval.fn()
1395-
this.handleWebviewAskResponse(askResponse, text, images)
1396-
this.autoApprovalTimeoutRef = undefined
1397-
}, approval.timeout)
1398-
timeouts.push(this.autoApprovalTimeoutRef)
1399-
}
1400-
1401-
// The state is mutable if the message is complete and the task will
1402-
// block (via the `pWaitFor`).
1403-
const isBlocking = !(this.askResponse !== undefined || this.lastMessageTs !== askTs)
1404-
const hasDeferredMessage = !this.messageQueueService.isEmpty()
1405-
const shouldPauseQueuedDrainForAsk = this.deferQueuedMessageDrainUntilResume && isResumableAsk(type)
1406-
const shouldAutoDispatchDeferredMessageForAsk =
1407-
this.canAutoDispatchDeferredMessageForAsk(type) && !shouldPauseQueuedDrainForAsk
1408-
const hasAutoDispatchCandidate = hasDeferredMessage && shouldAutoDispatchDeferredMessageForAsk
1409-
const isStatusMutable = !partial && isBlocking && !hasAutoDispatchCandidate && approval.decision === "ask"
1410-
1411-
if (isStatusMutable) {
1412-
const statusMutationTimeout = 2_000
1413-
1414-
if (isInteractiveAsk(type)) {
1415-
timeouts.push(
1416-
setTimeout(() => {
1417-
const message = this.findMessageByTimestamp(askTs)
1418-
1419-
if (message) {
1420-
this.interactiveAsk = message
1421-
this.emit(RooCodeEventName.TaskInteractive, this.taskId)
1422-
/* v8 ignore next 3 -- fires inside 2s timer after ask() resolves; not reachable in unit tests */
1423-
void this.postTaskMessageToWebview({ type: "interactionRequired" }).catch((error) => {
1424-
console.error("[Task#ask] postTaskMessageToWebview interactionRequired failed:", error)
1425-
})
1426-
}
1427-
}, statusMutationTimeout),
1428-
)
1429-
} else if (isResumableAsk(type)) {
1430-
timeouts.push(
1431-
setTimeout(() => {
1432-
const message = this.findMessageByTimestamp(askTs)
1433-
1434-
if (message) {
1435-
this.resumableAsk = message
1436-
this.emit(RooCodeEventName.TaskResumable, this.taskId)
1437-
}
1438-
}, statusMutationTimeout),
1439-
)
1440-
} else if (isIdleAsk(type)) {
1441-
timeouts.push(
1442-
setTimeout(() => {
1443-
const message = this.findMessageByTimestamp(askTs)
1444-
1445-
if (message) {
1446-
this.idleAsk = message
1447-
this.emit(RooCodeEventName.TaskIdle, this.taskId)
1448-
}
1449-
}, statusMutationTimeout),
1450-
)
1391+
// From here until the pWaitFor below resolves, a response can legitimately arrive
1392+
// (auto-approval, button click, auto-dispatched queue message). Mark the ask as
1393+
// pending so handleWebviewAskResponse delivers directly instead of queueing.
1394+
this.pendingAskCount++
1395+
try {
1396+
// Auto-approval (state/approval) was resolved above before the message was added.
1397+
if (approval.decision === "approve") {
1398+
this.approveAsk()
1399+
} else if (approval.decision === "deny") {
1400+
this.denyAsk()
1401+
} else if (approval.decision === "timeout") {
1402+
// Store the auto-approval timeout so it can be cancelled if user interacts
1403+
this.autoApprovalTimeoutRef = setTimeout(() => {
1404+
const { askResponse, text, images } = approval.fn()
1405+
this.handleWebviewAskResponse(askResponse, text, images)
1406+
this.autoApprovalTimeoutRef = undefined
1407+
}, approval.timeout)
1408+
timeouts.push(this.autoApprovalTimeoutRef)
14511409
}
1452-
} else if (hasAutoDispatchCandidate) {
1453-
this.consumeDeferredMessageForAsk(type)
1454-
}
14551410

1456-
// Wait for askResponse to be set
1457-
await pWaitFor(
1458-
() => {
1459-
if (this.abort) {
1460-
return true
1461-
}
1462-
if (this.askResponse !== undefined || this.lastMessageTs !== askTs) {
1463-
return true
1411+
// The state is mutable if the message is complete and the task will
1412+
// block (via the `pWaitFor`).
1413+
const isBlocking = !(this.askResponse !== undefined || this.lastMessageTs !== askTs)
1414+
const hasDeferredMessage = !this.messageQueueService.isEmpty()
1415+
const shouldPauseQueuedDrainForAsk = this.deferQueuedMessageDrainUntilResume && isResumableAsk(type)
1416+
const shouldAutoDispatchDeferredMessageForAsk =
1417+
this.canAutoDispatchDeferredMessageForAsk(type) && !shouldPauseQueuedDrainForAsk
1418+
const hasAutoDispatchCandidate = hasDeferredMessage && shouldAutoDispatchDeferredMessageForAsk
1419+
const isStatusMutable = !partial && isBlocking && !hasAutoDispatchCandidate && approval.decision === "ask"
1420+
1421+
if (isStatusMutable) {
1422+
const statusMutationTimeout = 2_000
1423+
1424+
if (isInteractiveAsk(type)) {
1425+
timeouts.push(
1426+
setTimeout(() => {
1427+
const message = this.findMessageByTimestamp(askTs)
1428+
1429+
if (message) {
1430+
this.interactiveAsk = message
1431+
this.emit(RooCodeEventName.TaskInteractive, this.taskId)
1432+
/* v8 ignore next 3 -- fires inside 2s timer after ask() resolves; not reachable in unit tests */
1433+
void this.postTaskMessageToWebview({ type: "interactionRequired" }).catch((error) => {
1434+
console.error(
1435+
"[Task#ask] postTaskMessageToWebview interactionRequired failed:",
1436+
error,
1437+
)
1438+
})
1439+
}
1440+
}, statusMutationTimeout),
1441+
)
1442+
} else if (isResumableAsk(type)) {
1443+
timeouts.push(
1444+
setTimeout(() => {
1445+
const message = this.findMessageByTimestamp(askTs)
1446+
1447+
if (message) {
1448+
this.resumableAsk = message
1449+
this.emit(RooCodeEventName.TaskResumable, this.taskId)
1450+
}
1451+
}, statusMutationTimeout),
1452+
)
1453+
} else if (isIdleAsk(type)) {
1454+
timeouts.push(
1455+
setTimeout(() => {
1456+
const message = this.findMessageByTimestamp(askTs)
1457+
1458+
if (message) {
1459+
this.idleAsk = message
1460+
this.emit(RooCodeEventName.TaskIdle, this.taskId)
1461+
}
1462+
}, statusMutationTimeout),
1463+
)
14641464
}
1465+
} else if (hasAutoDispatchCandidate) {
1466+
this.consumeDeferredMessageForAsk(type)
1467+
}
14651468

1466-
// If a deferred message arrives while we're blocked on a handoff ask (for example
1467-
// a follow-up suggestion click that was queued while the agent still owned the turn),
1468-
// consume it immediately so the task doesn't hang.
1469-
if (shouldAutoDispatchDeferredMessageForAsk && !this.messageQueueService.isEmpty()) {
1470-
this.consumeDeferredMessageForAsk(type)
1471-
}
1469+
// Wait for askResponse to be set
1470+
await pWaitFor(
1471+
() => {
1472+
if (this.abort) {
1473+
return true
1474+
}
1475+
if (this.askResponse !== undefined || this.lastMessageTs !== askTs) {
1476+
return true
1477+
}
14721478

1473-
return false
1474-
},
1475-
{ interval: 100 },
1476-
)
1479+
// If a deferred message arrives while we're blocked on a handoff ask (for example
1480+
// a follow-up suggestion click that was queued while the agent still owned the turn),
1481+
// consume it immediately so the task doesn't hang.
1482+
if (shouldAutoDispatchDeferredMessageForAsk && !this.messageQueueService.isEmpty()) {
1483+
this.consumeDeferredMessageForAsk(type)
1484+
}
1485+
1486+
return false
1487+
},
1488+
{ interval: 100 },
1489+
)
1490+
} finally {
1491+
this.pendingAskCount--
1492+
}
14771493

14781494
if (this.abort) {
14791495
throw new Error(`[RooCode#ask] task ${this.taskId}.${this.instanceId} aborted`)
@@ -1507,6 +1523,22 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
15071523
}
15081524

15091525
handleWebviewAskResponse(askResponse: ClineAskResponse, text?: string, images?: string[]) {
1526+
// Orphaned message guard: if a text-bearing response arrives while NO ask() is
1527+
// waiting (e.g. the webview raced a phase transition and sent askResponse instead
1528+
// of queueMessage, or submitUserMessage was invoked mid-stream via the API),
1529+
// writing it to askResponse* would silently destroy it — ask() clears those
1530+
// fields at the start of every new ask. Route it into the message queue instead,
1531+
// where the normal queue/steer handoff will deliver it at the next safe boundary.
1532+
if (
1533+
askResponse === "messageResponse" &&
1534+
this.pendingAskCount === 0 &&
1535+
!this.abort &&
1536+
(text?.trim() || images?.length)
1537+
) {
1538+
this.messageQueueService.addMessage(text ?? "", images)
1539+
return
1540+
}
1541+
15101542
// Clear any pending auto-approval timeout when user responds
15111543
this.cancelAutoApprovalTimeout()
15121544

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

Lines changed: 127 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1556,8 +1556,17 @@ describe("Cline", () => {
15561556

15571557
// Verify handleWebviewAskResponse was called directly (not webview)
15581558
expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "test message", ["image1.png"])
1559-
// Should NOT route through webview anymore
1560-
expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
1559+
// With no ask pending, the message must be preserved in the queue rather
1560+
// than written into the askResponse fields (which the next ask() clears).
1561+
expect(task.messageQueueService.getMessagesByMode("queue")).toEqual([
1562+
expect.objectContaining({ text: "test message", images: ["image1.png"] }),
1563+
])
1564+
// Should NOT route through the webview as user input (only queue-state
1565+
// broadcasts are allowed).
1566+
const inputRoutingCalls = vi
1567+
.mocked(mockProvider.postMessageToWebview)
1568+
.mock.calls.filter(([message]: any[]) => message?.type === "invoke")
1569+
expect(inputRoutingCalls).toHaveLength(0)
15611570
})
15621571

15631572
it("should handle empty messages gracefully", async () => {
@@ -2949,6 +2958,122 @@ describe("Deferred message dispatch boundaries", () => {
29492958
})
29502959
})
29512960

2961+
describe("Orphaned askResponse routing", () => {
2962+
// ask() clears askResponse/askResponseText at the start of every new complete ask,
2963+
// so a messageResponse that arrives while NO ask() is blocked would be silently
2964+
// destroyed (the reported "my message went nowhere" bug: e.g. the webview raced a
2965+
// phase transition and sent askResponse instead of queueMessage during the API
2966+
// request phase). These specs pin the invariant: such responses are rerouted into
2967+
// the task's message queue and delivered at the next safe boundary instead.
2968+
2969+
function createProvider(): any {
2970+
const storageUri = { fsPath: path.join(os.tmpdir(), "test-storage") }
2971+
const ctx = {
2972+
globalState: {
2973+
get: vi.fn().mockImplementation((_key: keyof GlobalState) => undefined),
2974+
update: vi.fn().mockResolvedValue(undefined),
2975+
keys: vi.fn().mockReturnValue([]),
2976+
},
2977+
globalStorageUri: storageUri,
2978+
workspaceState: {
2979+
get: vi.fn().mockImplementation((_key) => undefined),
2980+
update: vi.fn().mockResolvedValue(undefined),
2981+
keys: vi.fn().mockReturnValue([]),
2982+
},
2983+
secrets: {
2984+
get: vi.fn().mockResolvedValue(undefined),
2985+
store: vi.fn().mockResolvedValue(undefined),
2986+
delete: vi.fn().mockResolvedValue(undefined),
2987+
},
2988+
extensionUri: { fsPath: "/mock/extension/path" },
2989+
extension: { packageJSON: { version: "1.0.0" } },
2990+
} as unknown as vscode.ExtensionContext
2991+
2992+
const output = {
2993+
appendLine: vi.fn(),
2994+
append: vi.fn(),
2995+
clear: vi.fn(),
2996+
show: vi.fn(),
2997+
hide: vi.fn(),
2998+
dispose: vi.fn(),
2999+
}
3000+
3001+
const provider = new ClineProvider(ctx, output as any, "sidebar", new ContextProxy(ctx)) as any
3002+
provider.postMessageToWebview = vi.fn().mockResolvedValue(undefined)
3003+
provider.postStateToWebview = vi.fn().mockResolvedValue(undefined)
3004+
provider.postStateToWebviewWithoutTaskHistory = vi.fn().mockResolvedValue(undefined)
3005+
provider.getState = vi.fn().mockResolvedValue({})
3006+
return provider
3007+
}
3008+
3009+
const apiConfig: ProviderSettings = {
3010+
apiProvider: "anthropic",
3011+
apiModelId: "claude-3-5-sonnet-20241022",
3012+
apiKey: "test-api-key",
3013+
} as any
3014+
3015+
function createTask(): Task {
3016+
return new Task({
3017+
provider: createProvider(),
3018+
apiConfiguration: apiConfig,
3019+
task: "initial task",
3020+
startTask: false,
3021+
})
3022+
}
3023+
3024+
it("queues a messageResponse that arrives while no ask is pending", () => {
3025+
const task = createTask()
3026+
3027+
task.handleWebviewAskResponse("messageResponse", "orphaned message", ["img.png"])
3028+
3029+
// The message must land in the queue instead of the askResponse fields
3030+
// (where the next ask() would wipe it).
3031+
expect(task.messageQueueService.getMessagesByMode("queue")).toEqual([
3032+
expect.objectContaining({ text: "orphaned message", images: ["img.png"], deliveryMode: "queue" }),
3033+
])
3034+
expect((task as any).askResponse).toBeUndefined()
3035+
expect((task as any).askResponseText).toBeUndefined()
3036+
})
3037+
3038+
it("still records button responses (no text) while no ask is pending", () => {
3039+
const task = createTask()
3040+
3041+
task.handleWebviewAskResponse("yesButtonClicked")
3042+
3043+
expect(task.messageQueueService.isEmpty()).toBe(true)
3044+
expect((task as any).askResponse).toBe("yesButtonClicked")
3045+
})
3046+
3047+
it("delivers a messageResponse directly while an ask is blocked waiting", () => {
3048+
const task = createTask()
3049+
3050+
// Simulate an ask() blocked in its wait region (p-wait-for is module-mocked in
3051+
// this spec, so drive the pending counter directly).
3052+
;(task as any).pendingAskCount = 1
3053+
task.handleWebviewAskResponse("messageResponse", "direct answer")
3054+
3055+
expect((task as any).askResponse).toBe("messageResponse")
3056+
expect((task as any).askResponseText).toBe("direct answer")
3057+
// Nothing should have leaked into the queue.
3058+
expect(task.messageQueueService.isEmpty()).toBe(true)
3059+
})
3060+
3061+
it("auto-delivers a previously orphaned message at the next followup ask", async () => {
3062+
const task = createTask()
3063+
vi.spyOn(task as any, "saveClineMessages").mockResolvedValue(undefined)
3064+
3065+
// Orphaned: no ask pending yet.
3066+
task.handleWebviewAskResponse("messageResponse", "queued while streaming")
3067+
expect(task.messageQueueService.isEmpty()).toBe(false)
3068+
3069+
// The next auto-dispatchable ask must consume it instead of hanging.
3070+
const result = await task.ask("followup", "next question")
3071+
expect(result.response).toBe("messageResponse")
3072+
expect(result.text).toBe("queued while streaming")
3073+
expect(task.messageQueueService.isEmpty()).toBe(true)
3074+
})
3075+
})
3076+
29523077
describe("pushToolResultToUserContent", () => {
29533078
let mockProvider: any
29543079
let mockApiConfig: ProviderSettings

0 commit comments

Comments
 (0)