[cherry-pick] Re-enable Chat Sandbox home-read smoke test by re-warming chat after restart - #325624
Closed
vs-code-engineering[bot] wants to merge 1 commit into
Closed
[cherry-pick] Re-enable Chat Sandbox home-read smoke test by re-warming chat after restart#325624vs-code-engineering[bot] wants to merge 1 commit into
vs-code-engineering[bot] wants to merge 1 commit into
Conversation
…ng chat after restart
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.
Cherry-pick of #325594 from
release/1.129.Problem
The
Chat Sandbox › allows reading a home directory file configured in allowReadsmoke test is flaky. It failed 9/20 times in the flaky-test pipeline and was red on the release build, so #325575 skipped it onrelease/1.129to unblock the build:The observed error:
This PR re-enables the test with a real fix.
Root cause
The test (and the sibling
allowNetworktest) callrestartWithUpdatedSandboxSettings, which restarts the app so thechat.agent.sandboxsettings 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.waitForChatViewwaits 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, andpollForResponseTexttimes 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
warmUpChatretry loop (already used on cold start, retries up to 180s) insiderestartWithUpdatedSandboxSettings, right afterwaitForChatView(), so the probe is only sent once chat can reliably reach the mock LLM server.it.skipadded 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-buildpointed atVisual Studio Code - Insiders.app, 1.129.0-insider):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 amain-based branch, 5 on thisrelease/1.129-based branch).* Unrelated failures are
Timeout of 300000ms exceeded … main.js— the outer setup/beforehook 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 intomain. Closes microsoft/vscode-engineering#3280.