Skip to content

fix: Import Settings preview race that clobbers conflict markers - #14956

Merged
midleman merged 1 commit into
mainfrom
mi/import-settings-model-race
Jul 20, 2026
Merged

fix: Import Settings preview race that clobbers conflict markers#14956
midleman merged 1 commit into
mainfrom
mi/import-settings-model-race

Conversation

@midleman

@midleman midleman commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Fixes a race in the Import VS Code Settings command where the imported conflict-marker preview could be overwritten by the settings file model finishing its async on-disk resolve, leaving a marker-free file. This surfaced as a flaky e2e (the diff assertion saw <<<<<<< Existing appear, then vanish before it ran). Latent race in existing feature code; no behavior change.

Release Notes

New Features

  • N/A

Bug Fixes

  • Fixed an intermittent issue where the Import VS Code Settings preview could lose its conflict markers before they could be reviewed.

Validation Steps

@:vscode-settings @:web @:win

  1. Create a VS Code settings.json with a value that conflicts with your Positron settings.
  2. Run "Import VS Code Settings" and verify the diff preview reliably shows <<<<<<</>>>>>>> conflict markers.
  3. Accept the changes and confirm the imported settings are saved correctly.

E2E Triage Diagnosis

🟢 High confidence -- async race in the settings model likely caused the failure.
  • Spec: test/e2e/tests/import-vs-code-settings/import-vscode-settings.test.ts
    • Test: Verify diff displays and accepted settings are saved
  • Signal: trace shows the <<<<<<< conflict markers render immediately after import, then disappear before the assertion's toBeVisible() check runs -- appear-then-vanish, not a render failure
  • Frequency: 5/313 runs (1.6%), ubuntu/electron
  • Hypothesis: async on-disk settings-model resolve overwrote the freshly-written conflict-marker preview before the assertion ran

The Import VS Code Settings command opened the file-backed settings.json and
called setValue synchronously right after openEditor(). The file model resolves
its on-disk contents asynchronously, so a late resolve could overwrite the
freshly-written conflict-marker preview, leaving a marker-free file. This
manifested as a flaky e2e (the diff assertion saw the markers appear, then
vanish before it ran).

Resolve the model via ITextModelService.createModelReference first, so the
initial disk resolve is complete before setValue; setValue then dirties the
model, preventing a follow-up reload from clobbering the preview.
@github-actions

Copy link
Copy Markdown

E2E Tests 🚀
This PR will run tests tagged with: @:critical @:vscode-settings @:web @:win @:welcome

Why these tags?
Tag Source
@:critical Always runs (required)
@:vscode-settings PR description
@:web PR description
@:win PR description
@:welcome Changed files

More on automatic tags from changed files.

readme  valid tags

@midleman midleman changed the title Fix Import Settings preview race that clobbers conflict markers fix: Import Settings preview race that clobbers conflict markers Jul 17, 2026
@midleman
midleman requested a review from samclark2015 July 17, 2026 19:08
@midleman
midleman enabled auto-merge (squash) July 17, 2026 19:29

@samclark2015 samclark2015 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.

LGTM! Thanks for the catch!

@midleman
midleman merged commit dc5578a into main Jul 20, 2026
35 checks passed
@midleman
midleman deleted the mi/import-settings-model-race branch July 20, 2026 15:32
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants