Fix: aborted steering prompt incorrectly terminates agent response#297020
Fix: aborted steering prompt incorrectly terminates agent response#297020connor4312 merged 3 commits intomainfrom
Conversation
…m queue Co-authored-by: connor4312 <2230985+connor4312@users.noreply.github.com>
|
@copilot |
…rompt is aborted Co-authored-by: connor4312 <2230985+connor4312@users.noreply.github.com>
Fixed in 41cae3f. Changed |
There was a problem hiding this comment.
Pull request overview
Fixes an issue in the chat agent “steering prompt” flow where cancelling a pending steering prompt could leave yieldRequested stuck as true, causing an in-flight agent response to terminate early even though no steering prompt would be delivered.
Changes:
- Add
resetYieldRequested()on the in-flightCancellableRequestand invoke it when the last pending steering request is removed. - Change the
setYieldRequestedplumbing end-to-end (workbench → main thread → ext host) to accept an explicitvalue: booleanand propagate bothtrueandfalse. - Update affected tests/mocks to match the new method signature.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/test/common/voiceChatService.test.ts | Updates test doubles to match the new setYieldRequested(requestId, value) signature. |
| src/vs/workbench/contrib/chat/test/common/chatService/chatService.test.ts | Updates the agent stub signature used by the steering/yield test. |
| src/vs/workbench/contrib/chat/common/participants/chatAgents.ts | Updates public/internal interfaces and forwarding (IChatAgentImplementation / IChatAgentService) to pass a boolean value. |
| src/vs/workbench/contrib/chat/common/chatService/chatServiceImpl.ts | Implements yield reset and ensures the autorun propagates yield state updates (true and false) to the agent service. |
| src/vs/workbench/api/common/extHostChatAgents2.ts | Updates $setYieldRequested to accept and apply the boolean value to the in-flight request. |
| src/vs/workbench/api/common/extHost.protocol.ts | Updates the ext host shape contract to $setYieldRequested(requestId, value). |
| src/vs/workbench/api/browser/mainThreadChatAgents2.ts | Forwards the new setYieldRequested(requestId, value) call across the main thread ↔ ext host boundary. |
| test('steering message queued triggers setYieldRequested', async () => { | ||
| const requestStarted = new DeferredPromise<void>(); | ||
| const completeRequest = new DeferredPromise<void>(); | ||
| let setYieldRequestedCalled = false; | ||
|
|
||
| const slowAgent: IChatAgentImplementation = { | ||
| async invoke(request, progress, history, token) { | ||
| requestStarted.complete(); | ||
| await completeRequest.p; | ||
| return {}; | ||
| }, | ||
| setYieldRequested(requestId: string) { | ||
| setYieldRequested(requestId: string, value: boolean) { | ||
| setYieldRequestedCalled = true; | ||
| }, | ||
| }; |
There was a problem hiding this comment.
This test now risks a false positive: ChatService propagates yieldRequested both when it becomes true and when it is false (including the initial false), so setYieldRequestedCalled may be set during request startup even if queueing a steering request never toggles it to true. Consider capturing the boolean value passed to setYieldRequested and asserting the transition to true happens only after the steering request is queued (and ideally add an assertion that it returns to false when the steering request is removed/cancelled).
Cancelling a pending steering prompt left
yieldRequestedpermanently set totrue, causing the in-flight agent response to terminate early even though no steering prompt would ever be delivered.The
autoruninchatServiceImpl.tsonly propagatedyieldRequested = trueto the extension host but never propagated the reset back tofalse. This meant that even whenyieldRequestedwas reset onCancellableRequest, the extension host's in-flight request still sawyieldRequested = trueand terminated the response.Changes
CancellableRequest: AddedresetYieldRequested()to set_yieldRequestedback tofalseChatService.removePendingRequest: After removing a request from the model, checks whether any steering requests remain in the queue; if none do, callsresetYieldRequested()on the active pending requestsetYieldRequestedchain: ChangedsetYieldRequestedthroughout the entire call chain (IChatAgentImplementation→IChatAgentService→mainThreadChatAgents2→extHost.protocol→extHostChatAgents2) to accept avalue: booleanparameterchatServiceImpl.tsautorun: Updated to always callsetYieldRequestedwith the current value (bothtrueandfalse), so when a steering prompt is cancelled andyieldRequestedresets tofalse, the reset is propagated all the way to the extension host's in-flight requestOriginal prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.