microsoft / microsoft/vscode

Editor group restore scrambles MRU order and preview editor when an editor fails to deserialize

Open
#332,620 0 comments 0 reactions 1 assignee Claimed by @benibenj View on GitHub
workbench-tabs
Dominant language
TypeScript
Stars
193k
Forks
42.4k
PR merge metrics
PR metrics pending

Description

### Bug description

`EditorGroupModel.deserialize` (`src/vs/workbench/common/editor/editorGroupModel.ts`) coalesces the editors array when an editor serializer returns nothing (a dead editor, e.g. an uninstalled extension's editor), but keeps using the raw serialized indices for `mru` and `preview`. Only the sticky index is adjusted. After compaction every later index is off by the number of dead editors before it.

For a group with editors `[A(dead), B, C]`, serialized mru `[0, 1, 2]` and preview `2`:

- `this.mru = coalesce(data.mru.map(i => this.editors[i]))` maps to `[undefined->dropped, B, C]` losing A's slot semantics and mis-ordering recency: B ends up most recent.
- `this.preview = this.editors[data.preview]` points at the wrong editor or past the end.
- The restored active editor can be wrong; activating such a mismatched entry later hits `doSetSelection`'s `mru.splice(indexOf(-1), 1)`, which deletes an unrelated innocent entry from the MRU list.

This has been broken for any restore containing a dead editor since the coalescing was introduced (#96820 adjusted only sticky).

### Steps to reproduce

1. Open a group where one editor belongs to an extension that is then uninstalled, close VS Code.
2. Reload: the group restores without the dead editor but MRU order and preview point at shifted entries.
3. Activate tabs until a wrong MRU entry disappears.

### Expected behavior

After coalescing, old-to-new index remapping is applied to `data.mru` and `data.preview` so recency order, selection and preview survive a restore with dead editors exactly as they were serialized.

### Version tested

Commit `38ec3d57f91b` on `main`; reproduced with round-trip unit tests using a serializer that returns undefined per id. A fix is ready that remaps both fields through an old-to-new index map.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.