Skip to content

Fix flaky Agent Host smoke startup - #328326

Closed
roblourens wants to merge 2 commits into
mainfrom
roblou/agents/flaky-agent-host-smoke-tests-fix
Closed

Fix flaky Agent Host smoke startup#328326
roblourens wants to merge 2 commits into
mainfrom
roblou/agents/flaky-agent-host-smoke-tests-fix

Conversation

@roblourens

Copy link
Copy Markdown
Member

Summary

Fixes the Agent Host and Codex new-session picker flakes reported in #328281.

The failing tests all stopped before sending a model request:

  • Linux: Agents Window (local AgentHost) and its sandbox variants failed 5 times across 3 of 20 iterations.
  • Windows: Agents Window (Codex) > Test Codex session failed 3 times across 3 of 20 iterations.
  • Every failure timed out waiting for .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, LocalAgentHostServiceClient deferred constructing RemoteAgentHostProtocolClient until startAgentHost().

Before that client existed, rootState returned a dummy subscription backed by Event.None, and the notification/event getters returned Event.None as well. Consumers such as LocalAgentHostSessionsProvider can 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 LocalAgentHostSessionsProvider to 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:

sessionsPartService.getSessionView(...)? .selectWorkspace(...)

The optional lookup can legitimately return undefined while 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:

  1. opens the new-session state through ISessionsService;
  2. waits for ISessionsManagementService to report an advertised session target for the folder;
  3. creates and activates the draft directly through ISessionsService.openNewSession({ folderUri });
  4. persists the selected workspace only after creation succeeds.

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 createNewSession would 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

  • The protocol client is created eagerly, but its transport still waits on the existing delayed MessagePortClient. This does not start the Agent Host process early or bypass enablement/AI-disable gating.
  • The root-state fix exposes the same RemoteAgentHostProtocolClient surfaces already used after startup; it removes temporary dummy objects rather than adding a second proxying layer.
  • The provider subscribes before its initial read, a standard subscribe-then-snapshot pattern covered by a regression test that hydrates root state during listener installation.
  • Folder handoff now uses the same ISessionsService.openNewSession path as normal new-session creation, including the existing workspace-trust gate and provider routing.
  • Provider selection only changes the unpinned fallback case where the old candidate could not create a session anyway. Explicit provider and explicit session-type paths are unchanged.
  • No timeouts were increased and no failures are hidden. The waits are driven by actual session-type state changes.

Validation

  • npm run typecheck-client
  • npm run valid-layers-check
  • npm run precommit
  • npm run compile --prefix test/smoke
  • ./scripts/test.sh --run src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts
    • 162 passing, 11 pending
  • ./scripts/test.sh --run src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts
    • 74 passing, 2 pending
  • npm run smoketest-no-compile -- -g "Agents Window \\(local AgentHost" --tracing
    • 3 passing:
      • Agent Host
      • Agent Host sandbox
      • Agent Host SDK sandbox

The 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)

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>
Copilot AI review requested due to automatic review settings July 30, 2026 23:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/vs/sessions/contrib/chat/electron-browser/chat.contribution.ts
Comment thread src/vs/sessions/contrib/chat/electron-browser/chat.contribution.ts Outdated
Comment thread src/vs/sessions/contrib/chat/electron-browser/chat.contribution.ts Outdated
Comment thread src/vs/sessions/contrib/chat/electron-browser/chat.contribution.ts Outdated
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants