Repository navigation
sessions: keep the draft's editors when submit replaces it with a new session - #343333
Merged
Sandeep Somavarapu (sandy081) merged 1 commit intoOct 21, 2026
Merged
Sandeep Somavarapu (sandy081) merged 1 commit into
Sandeep Somavarapu (sandy081) merged 1 commit into
Conversation
… session A provider can commit a draft by replacing it with a session under a different resource. The visible-sessions model switches to the committed session before the layout controller handles the replacement, so the owner-change apply found no saved working set (drafts never save one) and applied an empty one, closing the draft's Changes and Files tabs. The committed session now adopts the editors on screen instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: Benjamin Christopher Simmonds (@benibenj)Matched files:
|
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
2 open issues
What changed in this PR
Fixes a layout regression where submitting a draft that is replaced by a new session under a different resource would close the draft’s side-pane tabs/editors due to an 'empty' working-set apply happening before _onSessionReplaced could carry state over.
Changes:
- Add a one-shot marker in
BaseLayoutControllerto skip applying'empty'working sets for a committed session that replaces the active draft under a different resource. - Clear the marker when the active session moves away.
- Add a new managed-tabs submit test covering both event orderings and editor visibility variants.
| File | Description |
|---|---|
| src/vs/sessions/contrib/layout/test/browser/desktopLayoutController.test.ts | Adds regression coverage ensuring resource-replace submit keeps the draft’s tabs and does not apply 'empty'. |
| src/vs/sessions/contrib/layout/browser/baseSessionLayoutController.ts | Introduces a marker to prevent an 'empty' working-set apply from closing editors during draft→committed resource replacement. |
Review effort: Lite
Comment on lines
1250
to
+1256
| if (isStaleChatOwner() || !this._editorWorkingSetService.beginApply(owner)) { | ||
| return; | ||
| } | ||
| if (workingSet === 'empty' && sessionResource && isEqual(sessionResource, this._workingSetAdoptingReplacement)) { | ||
| this._workingSetAdoptingReplacement = undefined; | ||
| return; | ||
| } |
Comment on lines
+2157
to
+2158
| }); | ||
| harness.visibleSessionsObs.set([committed], undefined); |
Benjamin Christopher Simmonds (benibenj)
approved these changes
Oct 21, 2026
Sandeep Somavarapu (sandy081)
deleted the
sandy081/agents/empty-editor-sidepane-issue
branch
October 21, 2026 20:57
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.
When a provider commits a draft by replacing it with a session under a different resource, the draft's side-pane tabs (Changes, Files and any opened files) were closed on submit.
Cause
SessionsServicehandlesonDidReplaceSessionfirst and switches the active session synchronously. The layout controller then sees an owner change from the draft key to the committed key and queues a working-set apply. Drafts never save a working set, so the apply resolves to'empty'and closes the draft's editors._onSessionReplacedruns only afterwards, too late to carry anything over.Fix
BaseLayoutController._onSessionReplacedmarks the committed session when it replaced the active draft under a different resource and has no saved working set. The queued apply, which runs after both listeners, skips the'empty'apply for that session, so it keeps the editors on screen and saves them normally when the user leaves. The mark is cleared if the active session moves elsewhere, so another session's tabs can't carry over.Unchanged: switching from a draft to another existing session still applies that session's working set (possibly empty).
Notes for reviewers
'empty'" apply goes back to Agents - add support for session working sets #310225 / sessions: persist layout state per session and across reloads #315696. Existing submit tests didn't configure workspace folders, so no working-set apply ran in them.Testing
[managed tabs / submit] a resource-replace submit keeps the draft's tabs, 4 variants (Editor visible or Details only; replace event before or after the session switch). All 4 fail without the fix and pass with it:./scripts/test.sh --run src/vs/sessions/contrib/layout/test/browser/desktopLayoutController.test.ts --grep "resource-replace submit keeps"./scripts/test.sh --runGlob "vs/sessions/contrib/layout/test/**/*.test.js": 291 passingnpm run typecheck-client: passednpm run eslintand hygiene on the changed files: passed