Skip to content

sessions: defer managed Changes tab reconciliation until working-set restore settles - #332123

Open
Alexandru Dima (alexdima) wants to merge 2 commits into
mainfrom
agents/log-analysis-error-fix-prioritization-dc8a5390
Open

sessions: defer managed Changes tab reconciliation until working-set restore settles#332123
Alexandru Dima (alexdima) wants to merge 2 commits into
mainfrom
agents/log-analysis-error-fix-prioritization-dc8a5390

Conversation

@alexdima

@alexdima Alexandru Dima (alexdima) commented Aug 22, 2026

Copy link
Copy Markdown
Member

I generated this by scanning my logs, pls take if it's good/useful


🤖

Summary

Defer the single-pane managed Changes/Files tab reconciliation while a session working-set restore is in progress, then let the existing restore-end reconciliation apply the latest accumulated intent against the settled editor group.

This prevents the managed-tab coordinator from replacing a stale Changes tab concurrently with IEditorGroupsService.applyWorkingSet, which can dispose and recreate the editor group underneath that replacement.

User-visible symptom and log evidence

While switching rapidly among recent Agent Host sessions in the Agents Window, the renderer repeatedly logged a canceled editor open immediately followed by a disposed scoped instantiation service:

2026-08-21 18:34:57.934 [error] Canceled: Canceled
    at ...setInput
    at ...doOpenEditor
    at ...replaceEditors
    at ..._reconcileForeignChangesEditors
    at ..._reconcileCore
    at ..._reconcile
2026-08-21 18:34:57.934 [error] InstantiationService has been disposed: Error: InstantiationService has been disposed
    at ..._throwIfDisposed
    at ...createInstance
    at ...doInstantiateEditorPane
    at ...doCreateEditorPane
    at ...doShowError
    at ...replaceEditors
    at ..._reconcileForeignChangesEditors

I found 10 occurrences of InstantiationService has been disposed across recent Insiders sessions on August 19-21, all with the same managed-tab reconciliation path.

The second error is a consequence of the first: once the editor open is canceled, the editor pane attempts to show an error placeholder, but the old editor group's scoped instantiation service has already been disposed by the concurrent working-set replacement.

Root cause

A session switch currently drives two related asynchronous paths:

  1. BaseLayoutController applies the incoming session's editor working set under _withSessionLayoutRestore(...).
  2. SinglePaneDockedTabsCoordinator observes the active-session change and immediately queues an ambient managed-tab reconciliation.

The coordinator's _reconcileForeignChangesEditors can therefore call group.replaceEditors(...) for the incoming session's Changes tab while applyWorkingSet(...) is still replacing the outgoing editor group. The coordinator holds the group captured before the asynchronous operation settles, so its replacement can continue against a group whose scoped services have since been disposed.

The controller already exposes the intended synchronization boundary:

  • ctx.isRestoringSessionLayout is true for the full asynchronous restore.
  • ctx.onDidEndSessionLayoutRestore queues a reconciliation after the restore depth returns to zero.

The ambient session-change path was not respecting that boundary.

Fix

queueReconcile still records the latest target, merges triggers for the same session, and advances the generation while restoration is active, but it does not start work against the transient editor group.

_reconcile also checks isRestoringSessionLayout before consuming _pending. This covers work that was queued immediately before a restore began but had not started yet.

When restoration settles, the existing onDidEndSessionLayoutRestore listener queues the final reconciliation. Because _pending was retained rather than discarded, trigger intent is preserved and merged with the restore-end trigger, while the newest generation continues to supersede stale queued work.

This is intentionally not an exception filter and does not suppress Canceled or disposed-service errors. It removes the ordering race that causes them.

Regression coverage

The tests cover both sides of the scheduling boundary:

  1. A restore is already active when a session switch requests reconciliation. The test verifies that queueReconcile retains the intent without scheduling replacement, then applies it after restore end.
  2. A stale Changes replacement is queued first, and a gated restore starts before the sequencer callback runs. The test verifies that the _reconcile guard leaves the old tab untouched during restore and that the restore-end reconciliation performs exactly one replacement for the active session.

Together they verify that:

  • no replaceEditors call occurs against a transient editor group;
  • the pending target is retained across restoration;
  • exactly one replacement occurs after restoration settles;
  • the resulting Changes tab belongs to the active session.

Validation

  • npm run transpile-client
  • focused managed-tab session-switch tests: 8 passing
  • ./scripts/test.sh --run src/vs/sessions/contrib/layout/test/browser/desktopSessionLayoutController.test.ts
    • 141 passing
  • pre-commit hygiene checks on all changed files
  • git diff --check

Reviewer context

Sandeep Somavarapu (@sandy081), this touches the generation/pending-intent logic introduced as part of the recent single-pane lifecycle stabilization work. The key behavior to review is that pending intents remain accumulated during restore and are consumed only by the already-existing restore-end reconciliation, rather than being dropped or run against the outgoing group.

Avoid replacing the active Changes tab while an asynchronous session working-set restore is still replacing the editor group. Keep the latest reconcile intent pending so the existing restore-end trigger applies it against the settled group.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@vs-code-engineering

Copy link
Copy Markdown
Contributor

📬 CODENOTIFY

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

Benjamin Christopher Simmonds (@benibenj)

Matched files:

  • src/vs/sessions/contrib/layout/browser/singlePane/singlePaneDockedTabsCoordinator.ts
  • src/vs/sessions/contrib/layout/test/browser/desktopSessionLayoutController.test.ts

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

Defers managed-tab reconciliation until session working-set restoration settles, preventing editor replacement races.

Changes:

  • Preserve pending reconciliation intent during restores.
  • Reconcile after restore completion.
  • Add regression coverage for session switching.
Show a summary per file
File Description
singlePaneDockedTabsCoordinator.ts Defers reconciliation during layout restoration.
desktopSessionLayoutController.test.ts Tests deferred Changes-tab replacement.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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

Exercise the restore guard inside the sequencer callback by queueing a stale Changes replacement before a gated restore begins. Verify that replacement remains pending until the restore-end reconciliation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@wenma531

This comment was marked as spam.

@wenma531

This comment was marked as spam.

@wenma531

This comment was marked as spam.

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