Fix flaky Agent Host smoke startup - #328326
Closed
roblourens wants to merge 2 commits into
Closed
Conversation
Keep local Agent Host protocol subscriptions stable before lazy startup, make the Agents folder handoff wait for an advertised target, and skip workspace resolvers that cannot create sessions. Add regression coverage for both ordering failures. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Fixes flakiness in Agents Window smoke tests by ensuring session-type state is reliably observed during lazy Agent Host startup and by making the folder handoff/open-new-session flow resilient to late provider/session-type availability.
Changes:
- Eagerly constructs the Agent Host protocol client (while keeping transport/process startup lazy) to avoid “dummy event” subscriptions.
- Improves folder handoff to open a folder-scoped draft only once session targets are advertised, and persists recent workspace selection after success.
- Updates provider selection to skip workspace resolvers that advertise no session types; adds regression tests for both provider selection and root-state hydration timing.
Show a summary per file
| File | Description |
|---|---|
| src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts | Extends test helper to accept multiple providers; adds regression test for skipping resolvers with no session types. |
| src/vs/sessions/services/sessions/browser/sessionsManagementService.ts | Skips candidate providers that advertise zero session types when session type isn’t pinned. |
| src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts | Adds regression test for subscribe-then-snapshot hydration timing. |
| src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts | Subscribes to root state changes before reading initial snapshot to close a race. |
| src/vs/sessions/contrib/chat/electron-browser/chat.contribution.ts | Reworks Agents-window folder handoff to wait for advertised targets and open a folder-scoped draft via ISessionsService; updates recent workspaces. |
| src/vs/sessions/SESSIONS.md | Documents the revised folder handoff flow. |
| src/vs/platform/agentHost/electron-browser/localAgentHostService.ts | Creates protocol client eagerly over delayed transport; removes dummy subscriptions/events. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 4
- Review effort level: Lite
Keep the session-type listener alive until a folder draft is actually opened, coalesce changes that arrive during an in-flight attempt, and remove temporary path-bearing diagnostics. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 Agent Host and Codex new-session picker flakes reported in #328281.
The failing tests all stopped before sending a model request:
Agents Window (local AgentHost)and its sandbox variants failed 5 times across 3 of 20 iterations.Agents Window (Codex) > Test Codex sessionfailed 3 times across 3 of 20 iterations..sessions-chat-session-type-picker .action-label:not(.hidden).The exact task and process logs from Azure build 460202 showed that the Agent Host process and protocol connection eventually started successfully, but the Agents window never received enough session-type state to populate its new-session picker.
Root causes
1. Lazy Agent Host startup exposed unstable protocol subscriptions
#326768 made the local Agent Host process start lazily from observable enablement. As part of that change,
LocalAgentHostServiceClientdeferred constructingRemoteAgentHostProtocolClientuntilstartAgentHost().Before that client existed,
rootStatereturned a dummy subscription backed byEvent.None, and the notification/event getters returnedEvent.Noneas well. Consumers such asLocalAgentHostSessionsProvidercan be constructed before enablement settles. When they subscribed during that window, they subscribed permanently to the dummy event and never observed the real root-state snapshot. The host could later connect normally while the session-type picker stayed empty forever.This PR constructs the protocol client immediately over the existing delayed transport, while retaining lazy process/MessagePort startup. Consumers now always observe the same root-state subscription and event objects, regardless of when the process starts.
It also changes
LocalAgentHostSessionsProviderto subscribe before reading the current root state, closing the smaller read-then-subscribe gap where hydration could land between those operations.The draft fix in #328172 identified this issue, but its own Azure build 460205 still failed the SDK-sandbox test twice across 14 iterations because of the independent handoff race below.
2. The Agents-window folder handoff could silently lose its workspace
The initial folder handoff was added in #314602. It originally awaited opening the new-session view before selecting the folder.
#317309 moved the flow to the sessions grid and replaced that awaited view open with:
The optional lookup can legitimately return
undefinedwhile the new-session slot is still mounting. However, the handoff returned success as soon as any provider could resolve the folder, so the workspace selection was silently dropped and never retried. The picker then had no folder-scoped session types and remained hidden.This PR removes the mounted-view dependency. The handoff now:
ISessionsService;ISessionsManagementServiceto report an advertised session target for the folder;ISessionsService.openNewSession({ folderUri });The listener is tied to the contribution's lifetime rather than a timeout or lifecycle phase, because Agent Host session types can arrive arbitrarily late.
3. Folder creation could choose a provider that advertised no session types
The provider-selection loop introduced with the provider-spanning workspace picker in #317525 selected the first provider that could resolve a workspace. Without an explicitly pinned session type, it did not require that provider to advertise any usable session type.
After Agent Host migration defaults changed in #328127, multiple providers can resolve the same local folder while one of them is still hydrating or intentionally hidden. The handoff could therefore observe that some target was available, then
createNewSessionwould choose an earlier resolver with zero session types and fall back to an empty composer.The unpinned selection path now skips resolvers whose
getSessionTypes(folderUri)result is empty. Explicit provider/type selection keeps its existing validation and failure behavior.Why this is low risk
MessagePortClient. This does not start the Agent Host process early or bypass enablement/AI-disable gating.RemoteAgentHostProtocolClientsurfaces already used after startup; it removes temporary dummy objects rather than adding a second proxying layer.ISessionsService.openNewSessionpath as normal new-session creation, including the existing workspace-trust gate and provider routing.Validation
npm run typecheck-clientnpm run valid-layers-checknpm run precommitnpm run compile --prefix test/smoke./scripts/test.sh --run src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts./scripts/test.sh --run src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.tsnpm run smoketest-no-compile -- -g "Agents Window \\(local AgentHost" --tracingThe targeted smoke command reproduced the picker timeout locally before the complete fix. After fixing the advertised-target and provider-selection paths, all three Agent Host smoke tests passed without changing their timeout.
(Written by Copilot)