{{ message }}
Repository navigation
fix(agentHost): ignore the failed execution's idle in the next Copilot turn - #340421
Merged
Merged
Conversation
…t turn A root session.error ends the protocol turn immediately, but the SDK emits that failed execution's terminal session.idle afterwards. When the client starts the next turn first, the stale idle completed the new pending turn empty and its send was dropped during preparation. Track the outstanding idle and ignore it until the next turn's provider boundary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Base:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A subagent turn-start can bypass the new guard, allowing the stale idle to complete and drop a retry.
1 open finding
What changed in this PR
Prevents a failed Copilot execution’s delayed idle event from prematurely completing a retry in Agent Host.
Changes:
- Tracks the outstanding idle and clears it at root turn boundaries.
- Adds regression tests for retry timing and missing terminal idle events.
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Ignore the preceding failed execution's idle independently of child turn-start state. Cover child events during preparation and after admission, zero-output recovery and pre-consumption errors without weakening lifecycle assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserve both the root failed-idle boundary reset and managed-plugin failure handling. Revalidated 1579 Copilot unit tests and 30 strict error/recovery and resume executions on the merged revision. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
roblourens
marked this pull request as ready for review
October 9, 2026 01:30
roblourens
approved these changes
Oct 9, 2026
Paul (pwang347)
approved these changes
Oct 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the flaky Agent Host E2E test
Copilot › provider errors: bad request remains classified and allows a subsequent turn('' !== 'RECOVERED'), seen on #340401, #340407 and #339896.session.errornow records that the failed execution's terminalsession.idleis still outstanding.user.messageecho,assistant.turn_start, turn-starting notification), or is consumed by the failed execution's root idle.Root cause
wrapper.onSessionErrorclears the active turn as soon as a root error arrives, so the client seeschat/errorand can send the next turn right away.session.idleafterwards.wrapper.onIdlecompletes even apendingturn (defensive handling for degenerate no-op sends), so a late idle completed the new turn with no response.send()then loggedTurn changed during preparation; dropping sendand the prompt never reached the model.Session error … 400at 44.377 →chat/error44.381 →after-errorturn started 44.388 →sendMessage called44.421 →Session idle44.422 →after-errorturnComplete44.426 →Turn changed during preparation; dropping send44.483.Session Context
providerTurnStarted, is authoritative: a background child's turn-start also changes the latter flag, without establishing a new root boundary.a phase chat reopened while its close still drains an edit stays openis unchanged.Changes
src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts_failedExecutionIdlePending. Set it on rootsession.error; consume it on the next root idle; clear it only on new root execution boundaries. Protect the replacement independently of a child changing the root turn's started flag.src/vs/platform/agentHost/test/node/copilotAgentSession.test.tssrc/vs/platform/agentHost/test/node/e2e/README.mdVerification
7e88f0be3155, the added child-start cases reproduce the review finding: 7 focused tests passed, 2 failed. Withfcf1bd35b53c, all 9 focused ownership/compatibility tests passed.env -u COPILOT_HOME VSCODE_SKIP_PRELAUNCH=1 ./scripts/test.sh --run src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts --run src/vs/platform/agentHost/test/node/copilotAgent.test.ts --reporter dot— final merged revision7b9aadf2922: 1579 passed, 1 existing pending. The ambient developerCOPILOT_HOMEmust be unset so unit tests use their injected homes. Earlier takeover revision had 1490 passes; an unrelated startup-migration notification test failed once under combined load, passed on isolated unchanged-baseline comparison, and subsequent full runs passed. No migration code/test was edited.env -u COPILOT_HOME VSCODE_SKIP_PRELAUNCH=1 ./scripts/test-integration.sh --run src/vs/platform/agentHost/test/node/e2e/providers/copilotAgentHostE2E.integrationTest.ts --grep 'provider errors:|resumes a failed turn in place|resumes the same turn after repeated failures' --reporter dot— five final-revision executions, all 6 tests passed each time: 30 strict-replay passes.npm run eslint -- src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts— passed.node build/lib/formatter.ts --verify src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts src/vs/platform/agentHost/test/node/copilotAgentSession.test.tsandgit diff --check— passed.npm run typecheck-client— passed on the final merged revision. The older PR base had one unrelated Auto-tier test incompatibility with the shared installed SDK; incorporating current main resolved that baseline mismatch without changing shared dependencies or adding an unrelated fix.7b9aadf2922, including Compile & Hygiene and all three Electron integration jobs. A Copilot extension test setup dependency download timed out (ETIMEDOUT), then passed on its isolated retry without code changes.272K, detached DOM) also occurred identically on unchanged main 482526. Its isolated retry passed without code changes. No UI or smoke code changes and no required-check bypass.Starts and reopens a Dev Container session over WSLinitially timed out waiting forDEV_CONTAINER_WSL_RESPONSE. The same test passed in 482533 at the identical head. Failure artifacts are preserved; the mock model request arrived after the response assertion had already timed out. The isolated Windows retry (forceRetryAllJobs=false) explicitly executed and passed this test in 57.970 seconds, with 92 smoke tests passing and no code changes. Both the build and all reported PR checks are now green. This is not the emptyRECOVEREDsignature; the original smoke timeout's root cause remains unclassified despite the passing retry.(Written by Copilot)