Skip to content

Re-enable Chat Sandbox home-read smoke test by re-warming chat after restart - #325594

Merged
Giuspepe merged 1 commit into
release/1.129from
gcianci/fix-flaky-sandbox-home-read-warmup
Jul 13, 2026
Merged

Re-enable Chat Sandbox home-read smoke test by re-warming chat after restart#325594
Giuspepe merged 1 commit into
release/1.129from
gcianci/fix-flaky-sandbox-home-read-warmup

Conversation

@Giuspepe

@Giuspepe Giuspepe commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Problem

The Chat Sandbox › allows reading a home directory file configured in allowRead smoke test is flaky. It failed 9/20 times in the flaky-test pipeline and was red on the release build, so #325575 skipped it on release/1.129 to unblock the build:

The observed error:

Error: Timed out waiting for response matching /HOME_READ_ALLOWED_EXIT_CODE=(\d+)/ in
'div[id="workbench.panel.chat"] .interactive-item-container.interactive-response .rendered-markdown'
  at Chat.pollForResponseText (test/automation/out/chat.js:185:15)

This PR re-enables the test with a real fix.

Root cause

The test (and the sibling allowNetwork test) call restartWithUpdatedSandboxSettings, which restarts the app so the chat.agent.sandbox settings reload deterministically (introduced in #325532). A full restart tears down the extension host along with the warmed-up chat participant and its mock LLM connection.

The helper only waited for the chat view DOM via waitForChatView() before sending the probe message. waitForChatView waits for the chat view to render, not for chat to be ready to answer. On loaded CI agents the first probe races an unready chat, the mock response never renders, and pollForResponseText times out after 120s.

#325532 fixed one race (settings not reloading → HOME_READ_ALLOWED_EXIT_CODE=1) but introduced this one (chat not re-warmed after restart → probe timeout).

Fix

  • Re-run the existing warmUpChat retry loop (already used on cold start, retries up to 180s) inside restartWithUpdatedSandboxSettings, right after waitForChatView(), so the probe is only sent once chat can reliably reach the mock LLM server.
  • Remove the it.skip added in test: skip flaky chat sandbox home-read smoke test #325575 to re-enable the test.

Verification (local A/B)

Ran the same loop locally against the installed Insiders build (--build/--stable-build pointed at Visual Studio Code - Insiders.app, 1.129.0-insider):

HOME_READ probe timeout (the bug) Passed Unrelated env failures*
Without fix (post-restart warm-up disabled) 3/8 3/8 2/8
With fix 0/13 12/13 1/13

Without the fix the exact CI error reproduces (Timed out waiting for response matching /HOME_READ_ALLOWED_EXIT_CODE=(\d+)/) at ~38%, close to the CI flaky rate of 9/20 (45%). With the fix it never occurs across 13 runs (8 on a main-based branch, 5 on this release/1.129-based branch).

* Unrelated failures are Timeout of 300000ms exceeded … main.js — the outer setup/before hook timing out from the loop launching the app repeatedly; not the chat race, and present in both columns.

Targeting

This PR targets release/1.129 (where the test is skipped) so the fix lands on the release branch and flows forward into main. Closes microsoft/vscode-engineering#3280.

Copilot AI review requested due to automatic review settings July 13, 2026 10:47
@Giuspepe
Giuspepe marked this pull request as draft July 13, 2026 10:52

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

Re-warms Chat Sandbox after application restarts to prevent probe requests racing chat initialization.

Changes:

  • Passes the logger into the restart helper.
  • Runs the existing chat warm-up loop after restart.
Show a summary per file
File Description
test/smoke/src/areas/chat/chatSandbox.test.ts Re-warms chat before sandbox probes.

Review details

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread test/smoke/src/areas/chat/chatSandbox.test.ts
…start

#325575 skipped the "allows reading a home directory file configured in
allowRead" smoke test because it went flaky after the app restart added
in #325532. This re-enables it with a proper fix.

restartWithUpdatedSandboxSettings restarts the app so the sandbox
settings reload deterministically. A full restart tears down the
extension host along with the warmed-up chat participant and its mock
LLM connection, but the helper only waited for the chat view DOM
(waitForChatView) before sending the probe. On loaded CI agents the
first probe races an unready chat and never receives a response:

  Timed out waiting for response matching /HOME_READ_ALLOWED_EXIT_CODE=(\d+)/

Re-run the existing warmUpChat retry loop after the restart so the probe
is only sent once chat can reliably reach the mock LLM server.

Verified locally against the installed Insiders build: with the
post-restart warm-up disabled the exact failure reproduces 3/8 runs;
with the fix it does not occur across 13 runs (8 on main, 5 here).

Fixes microsoft/vscode-engineering#3280

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Giuspepe
Giuspepe force-pushed the gcianci/fix-flaky-sandbox-home-read-warmup branch from d732a0b to 3c6f510 Compare July 13, 2026 10:59
@Giuspepe
Giuspepe changed the base branch from main to release/1.129 July 13, 2026 10:59
@Giuspepe Giuspepe changed the title Fix flaky Chat Sandbox smoke test by re-warming chat after restart Re-enable Chat Sandbox home-read smoke test by re-warming chat after restart Jul 13, 2026
@Giuspepe
Giuspepe marked this pull request as ready for review July 13, 2026 11:46
@Giuspepe
Giuspepe enabled auto-merge (squash) July 13, 2026 11:46
@Giuspepe
Giuspepe merged commit e4556ce into release/1.129 Jul 13, 2026
45 of 47 checks passed
@Giuspepe
Giuspepe deleted the gcianci/fix-flaky-sandbox-home-read-warmup branch July 13, 2026 14:23
@vs-code-engineering vs-code-engineering Bot added this to the 1.129.0 milestone Jul 13, 2026
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.

3 participants