Skip to content

Avoid stale simple dialog folder updates#321357

Merged
alexr00 merged 3 commits into
microsoft:mainfrom
pony-maggie:fix-simple-dialog-cancel-stale-update
Jul 14, 2026
Merged

Avoid stale simple dialog folder updates#321357
alexr00 merged 3 commits into
microsoft:mainfrom
pony-maggie:fix-simple-dialog-cancel-stale-update

Conversation

@pony-maggie

Copy link
Copy Markdown
Contributor

Fixes #152089

This fixes a race in the simple file dialog when a slower folder update is canceled by a newer one. Previously, the canceled update could still resume after fileService.resolve and write currentFolder / userEnteredPathSegment before checking the cancellation token. That stale state could then affect subsequent input handling on slow file system providers.

The update now computes the next folder state locally and only commits it after item creation finishes and the token is still active. createBackItem also uses the folder passed into createItems instead of reading this.currentFolder, so the current folder does not need to be mutated before the async item work completes.

Verification:

  • PATH="/opt/homebrew/opt/node@24/bin:$PATH" npm run compile-client
  • red/green regression: the new browser test failed before the fix with file:///slow/ === file:///fast/, then passed after the fix
  • PATH="/opt/homebrew/opt/node@24/bin:$PATH" npm run test-browser-no-install -- --run src/vs/workbench/services/dialogs/test/browser/simpleFileDialog.test.ts --browser chromium-chrome
  • git diff --check

Copilot AI review requested due to automatic review settings June 15, 2026 02:23

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 was unable to run its full agentic suite in this review.

This PR prevents a slow/canceled updateItems call in SimpleFileDialog from overwriting the state produced by a newer folder update, and adds a regression test to lock in the behavior.

Changes:

  • Delay committing currentFolder / userEnteredPathSegment until after cancellation is checked.
  • Use a stable folder reference (currFolder) when computing parent/root behavior.
  • Add a browser test to ensure canceled slow updates don’t override newer updates.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/vs/workbench/services/dialogs/test/browser/simpleFileDialog.test.ts Adds a regression test covering the “slow canceled update must not overwrite newer folder” scenario
src/vs/workbench/services/dialogs/browser/simpleFileDialog.ts Defers state assignment until after cancellation and uses a stable folder reference to avoid racey overwrites

Comment thread src/vs/workbench/services/dialogs/test/browser/simpleFileDialog.test.ts Outdated
@vs-code-engineering

Copy link
Copy Markdown
Contributor

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@alexr00

Matched files:

  • src/vs/workbench/services/dialogs/browser/simpleFileDialog.ts
  • src/vs/workbench/services/dialogs/test/browser/simpleFileDialog.test.ts

@pony-maggie

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@alexr00 alexr00 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for the PR!

@alexr00
alexr00 enabled auto-merge (squash) July 14, 2026 10:14
@alexr00
alexr00 merged commit b67e357 into microsoft:main Jul 14, 2026
29 checks passed
@vs-code-engineering vs-code-engineering Bot added this to the 1.130.0 milestone Jul 14, 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.

Using folder picker on vscode.dev overwrites input

4 participants