sessions: respect hidden secondary side bar on session switch (#320075)

Restore the invariant that revealing the editor part also reveals the
auxiliary bar (e.g. opening a file from chat shows the secondary side
bar), but suppress that enforcement while restoring a session's editor
working set on session switch. The working-set reveal is programmatic,
so the session's saved auxiliary bar visibility now wins and a side bar
the user hid for a session stays hidden when returning to it.

Adds the LAYOUT_CONTROLLER.md spec documenting per-session layout state
and links it from LAYOUT.md, README.md, and the sessions skill.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Sandeep Somavarapu
2026-06-05 11:30:42 +00:00
committed by GitHub
co-authored by Copilot
parent d97df0aaf5
commit 231a1f025b
6 changed files with 277 additions and 7 deletions
+8
View File
@@ -18,6 +18,7 @@ Then read the relevant spec for the area you are changing (see table below). If
|----------|------|-------------|
| Layer rules | `src/vs/sessions/LAYERS.md` | Before adding any cross-module imports. Defines the internal layer hierarchy (`core``services``contrib``providers`) with ESLint-enforced import restrictions. Key rule: `contrib/*` must NOT import from `contrib/providers/*`. |
| Layout spec | `src/vs/sessions/LAYOUT.md` | Before changing any part, grid structure, titlebar, or CSS. Documents the fixed grid layout (Sidebar \| ChatBar \| AuxiliaryBar), part positions, the modal editor system, per-session layout state persistence, and the titlebar's three-section design. |
| Layout controller spec | `src/vs/sessions/LAYOUT_CONTROLLER.md` | Before changing `LayoutController` or per-session layout state. Details how the auxiliary bar, panel, and editor working sets are captured/restored when switching sessions, multi-session suppression, the auto-reveal-on-changes flow, workspace-folder ordering, and storage/migration. |
| Sessions spec | `src/vs/sessions/SESSIONS.md` | Before changing session/provider interfaces or data flow. Covers the pluggable provider model (`ISessionsProvider``ISessionsProvidersService``ISessionsManagementService`), `ISession`/`IChat` interfaces, observable state propagation, workspace/folder model, and session type system. |
| Sessions list spec | `src/vs/sessions/SESSIONS_LIST.md` | Before changing the sessions sidebar list. Covers the tree widget (`WorkbenchObjectTree`), renderers, grouping (workspace/date), filtering (type/status/archived/read), pinning, read/unread state, workspace capping, mobile adaptations, storage keys, and registered actions. |
| Mobile spec | `src/vs/sessions/MOBILE.md` | Before adding any phone-specific UI. Covers the mobile part subclass architecture, viewport classification (phone < 640px), `MobileTitlebarPart`, drawer-based sidebar, `MobilePickerSheet`, view/action gating with `IsPhoneLayoutContext`, and the desktop → mobile component mapping. |
@@ -30,6 +31,13 @@ Then read the relevant spec for the area you are changing (see table below). If
- **Importing from providers**: Non-provider `contrib/*` code must never import from `contrib/providers/*`. Extract shared interfaces to `services/` or `common/`.
- **Missing entry point import**: New contribution files must be imported in the appropriate `sessions.*.main.ts` entry point to be loaded (for example `sessions.common.main.ts`, `sessions.desktop.main.ts`, `sessions.web.main.ts`, or `sessions.web.main.internal.ts`).
- **Modifying workbench code**: Prefer extending/wrapping workbench classes in the sessions layer over modifying shared workbench components.
- **Timeouts as fixes**: Never use `setTimeout`/`disposableTimeout`/arbitrary delays to fix bugs or implement behaviour. They are race-prone guesses that mask the real ordering/state problem. Drive logic off deterministic signals instead — observables (`autorun`/`derived`), explicit events (`onDidChange*`), lifecycle phases, or awaiting the actual async operation.
- **Stashed state read back later (side-channels)**: Never stash a value on a service during one method call and read it back from a separate query later, assuming it is still valid (e.g. a `Set`/flag set in `openSession` and consumed by a `shouldX()` pull-API). This is fragile temporal coupling. Instead, make it reactive state that is set **atomically together with its source of truth** and consumed reactively. Example: per-activation intent like "open in background / preserve focus" is exposed as an `IObservable` set in the **same transaction** as `activeSession` (via a single internal setter so it can never go stale), and read with `.read(reader)` in the consumer's `autorun` — never via a consume-once getter.
- **Blocking on a "pending/waiting" state instead of creating + upgrading**: When an entity (e.g. a draft session) depends on something that registers asynchronously, don't withhold creation behind a pending/waiting state. Prefer creating immediately with the best available data, then **replace/upgrade** it once the awaited dependency arrives (driven by an `onDidChange*`/observable signal), cancelling the upgrade if the user changes the inputs meanwhile and giving up at a deterministic lifecycle milestone (e.g. `LifecyclePhase.Eventually`) rather than a timeout. This keeps the UI populated and avoids fragile "is everything ready yet?" gating.
## Capturing Feedback (meta-rule)
Whenever the user flags a wrong pattern, rejects an approach, or gives design/rules feedback, **automatically add it** as a concise pitfall/learning to this `Common Pitfalls` section (or the most relevant spec doc) in the same change — without being asked again. Keep each entry 13 sentences: the anti-pattern, why it is wrong, and the preferred pattern.
## Validating Changes
+2 -2
View File
@@ -153,7 +153,7 @@ When the editor part is shown in the grid (not as a modal), its title toolbar (`
When the auxiliary bar is hidden the editor becomes the rightmost card and expands into the freed space; the workbench's 10px right gutter still applies, and a `.noauxiliarybar` rule in `browser/media/style.css` restores the editor's right border and right corner radii so it keeps its card appearance.
The auxiliary-bar invariant (§10) is only enforced when the editor part *becomes* visible, so this toggle can collapse the side part while the editor stays open.
The auxiliary-bar invariant (§10) is enforced when the editor part *becomes* visible — for example, opening a file from chat reveals the editor and also reveals the secondary side bar. The chevron toggle can still collapse the side part while the editor stays open. The one exception is **restoring a session's editor working set on session switch**: that reveal is programmatic and honors the session's saved auxiliary bar visibility, so a side bar the user hid for a session stays hidden when returning to it.
The main editor part can be explicitly revealed for workflows that target it directly.
@@ -212,7 +212,7 @@ All session-window contributions use `WindowVisibility.Sessions` to only appear
## 10. Per-Session Layout State
`LayoutController` (`contrib/layout/browser/sessionLayoutController.ts`) manages layout state as the user switches between sessions. All state is persisted to workspace storage so it survives restarts.
`LayoutController` (`contrib/layout/browser/sessionLayoutController.ts`) manages layout state as the user switches between sessions. All state is persisted to workspace storage so it survives restarts. This section is a summary — see **[LAYOUT_CONTROLLER.md](LAYOUT_CONTROLLER.md)** for the full specification (switch trigger, multi-session handling, auto-reveal, persistence, and invariants).
### Auxiliary Bar
+175
View File
@@ -0,0 +1,175 @@
# Layout Controller — Per-Session Layout State
This document specifies the behaviour of `LayoutController`
([contrib/layout/browser/sessionLayoutController.ts](contrib/layout/browser/sessionLayoutController.ts)),
the contribution that manages workbench layout as the user switches between sessions.
It is the detailed companion to [LAYOUT.md §10 Per-Session Layout State](LAYOUT.md#10-per-session-layout-state).
---
## 1. Overview
The Agents window keeps a single **active session** but lets the user move between many.
Each session "owns" a small amount of layout state — which side parts are visible and which
editors are open — so that returning to a session restores the working context the user left it in.
`LayoutController` owns three independent pieces of per-session state, all keyed by session
resource (`URI`) and persisted to workspace storage:
| State | Storage map | Scope |
|-------|-------------|-------|
| Auxiliary bar (secondary side bar) | `_viewStateBySession` | visibility + active view container |
| Panel (terminal / debug output) | `_panelVisibilityBySession` | visibility only |
| Editor working set | `_workingSets` | open editors in the grid editor part |
All state flows from the `activeSession` **observable** (never events). The controller derives
`activeSessionResourceObs`, `activeSessionHasChangesObs`, `activeSessionIsUntitledObs`,
`activeSessionHasWorkspaceObs`, and `multipleSessionsVisibleObs`, then reacts with `autorun`.
---
## 2. The Switch Trigger
Each sync is an `autorun` that reads `activeSessionResourceObs`. The controller keeps a local
`previousSessionResource` so it can detect a **real switch** (`previous !== active`) versus an
initial load or an unrelated re-evaluation.
### Multiple visible sessions
When more than one session is visible at once (the Sessions Part grid shows several session views),
**all per-session sync is suppressed**:
- The aux-bar / panel sync autoruns bail out early (`multipleSessionsVisibleObs`).
- A dedicated autorun **clears** `_viewStateBySession`, `_panelVisibilityBySession`, and
`_pendingTurnStateByResource` for every visible session.
This guarantees that after collapsing back to a single session the **default visibility logic**
(§3.2) runs again instead of restoring stale single-session state. Editor working sets are *not*
cleared — they survive multi-session mode.
---
## 3. Auxiliary Bar
Skipped entirely on mobile web (`isWeb && isMobile`) to avoid disruptive auto-expand on narrow viewports.
### 3.1 Switching away — capture
`_captureViewState(previousSession)` records, for the **outgoing** session:
- `auxiliaryBarVisible` — whether the aux bar is currently visible.
- `auxiliaryBarActiveViewContainerId` — the active aux-bar view container (Files vs Changes).
### 3.2 Switching to — restore
`_syncAuxiliaryBarVisibility(resource, hasWorkspace, isUntitled, hasChanges)` applies state in
strict priority order:
1. **No resource / no workspace** → do nothing.
2. **Untitled session** → open the Files container (`SESSIONS_FILES_CONTAINER_ID`), leave visibility as is.
3. **Saved state exists**:
- was **hidden** → hide the aux bar and stop.
- was visible with an active container → reopen that container and stop.
4. **No saved state (first visit) — defaults**:
- session **has changes** → open the Changes view (`CHANGES_VIEW_ID`).
- otherwise → open the Files container.
### 3.3 Auto-reveal on new changes
A separate autorun watches for turn completion (also skipped on mobile web). When a chat request
is submitted (`onDidSubmitRequest`), the controller records `IPendingTurnState`
(`hadChangesBeforeSend`, `submittedAt`) for the session. When that session's `lastTurnEnd`
advances past `submittedAt`:
- if there were **no** changes before the turn but there **are** changes now, the aux bar is
revealed (`setPartHidden(false, AUXILIARYBAR_PART)`) and the session's saved view state is
cleared so it stays visible on the next switch.
This only applies to the single-visible-session case; the pending state is dropped when multiple
sessions are visible.
### 3.4 Editor / aux-bar invariant
The editor part must not be left visible without the auxiliary bar
(`_enforceAuxiliaryBarWhenEditorVisible`): when the editor part *becomes* visible the aux bar is
revealed. So opening a file from chat reveals the editor **and** the secondary side bar.
The one exception is **working-set restoration on session switch** (§5): that editor reveal is
programmatic, so the invariant is suppressed (`_suppressAuxiliaryBarEnforcement`) and the
session's saved aux-bar visibility wins. A side bar the user hid for a session therefore stays
hidden when they return to it. The suppression is a synchronous re-entrancy guard around the
`setPartHidden(false, EDITOR_PART)` call — the part-visibility event fires synchronously, so the
guard reliably covers exactly that reveal.
---
## 4. Panel
`_syncPanelVisibility(resource)`:
- No active session → hide the panel.
- Otherwise restore `_panelVisibilityBySession.get(resource)`, defaulting to **hidden** when there
is no record.
The per-session record is updated whenever the user toggles the panel: an
`onDidChangePartVisibility` listener for `PANEL_PART` writes the new visibility for the active
session (suppressed while multiple sessions are visible).
---
## 5. Editor Working Sets
Active only when `workbench.editor.useModal` is **not** `'all'` (editors live in the grid editor
part rather than as modal overlays). Driven by `_useModalConfigObs`.
### 5.1 Workspace-folder ordering
The `activeSession` observable updates **before** the workbench's workspace folders update. To
avoid restoring editors into the wrong workspace, `activeSessionForWorkingSet`
(`derivedObservableWithCache`) holds back the new session until the workspace folders reflect its
working directory.
### 5.2 Save / apply on switch
Using `runOnChange(activeSessionForWorkingSet, ...)`:
- **Outgoing session** (skip untitled): `_saveWorkingSet` snapshots the currently open editors as a
named working set (`session-working-set:<resource>`); sessions with no visible editors store nothing.
- **Incoming session**: `_applyWorkingSet` restores its saved working set (or `'empty'`). All
applies are serialized through a `Sequencer`. When not in modal mode and the working set is
non-empty, the editor part is revealed before/after applying via `_revealEditorPartForWorkingSet`,
which suppresses the editor→aux-bar invariant (§3.4) so the session's saved aux-bar visibility is
honored.
On initial load (no previous session) the controller only applies a working set if one is already
saved for the incoming session — it never applies `'empty'`, to avoid closing editors being restored.
### 5.3 Cleanup
`onDidChangeSessions` removes working sets for **archived** or **deleted** sessions
(`_deleteWorkingSet`, which also drops the corresponding view state).
---
## 6. Persistence
- All state serializes to the workspace-scoped key `sessions.layoutState` on
`IStorageService.onWillSaveState` (`_saveState`), with a `StorageTarget.MACHINE` target.
- `_saveState` captures the active session's current view state and working set (skipping untitled /
multi-session cases) and writes one `ISessionLayoutEntry` per known session resource.
- `_loadState` reads `sessions.layoutState`; if absent it performs a one-time migration from the
legacy `sessions.workingSets` key and then removes it. Corrupted data is dropped defensively.
---
## 7. Key Invariants
- **Observables, not events**, drive all session-switch logic.
- **Multiple visible sessions** disable per-session view/panel sync and clear that state (working
sets preserved).
- **Default visibility** (§3.2 step 4) only applies when a session has no saved aux-bar state.
- The **editor part implies the auxiliary bar** when it *becomes* visible (e.g. opening a file from
chat), **except** during working-set restoration on session switch, where the session's saved
aux-bar visibility wins (so a hidden side bar is respected).
- Working-set save/apply waits for **workspace folders** to catch up with the active session.
+1
View File
@@ -20,6 +20,7 @@ The Agents Window (`Workbench`) provides a simplified, fixed-layout workbench ta
| Document | Description |
|----------|-------------|
| [LAYOUT.md](LAYOUT.md) | Workbench layout specification — grid structure, parts, titlebar, per-session layout state |
| [LAYOUT_CONTROLLER.md](LAYOUT_CONTROLLER.md) | Per-session layout state — how the auxiliary bar, panel, and editor working sets are captured/restored on session switch |
| [LAYERS.md](LAYERS.md) | Import layering rules — what each layer can and cannot import, ESLint enforcement |
| [SESSIONS.md](SESSIONS.md) | Sessions architecture — layers, provider model, core interfaces, data flow, metadata contract |
| [MOBILE.md](MOBILE.md) | Mobile layout specification |
@@ -65,6 +65,14 @@ export class LayoutController extends Disposable {
private readonly _workingSetSequencer = new Sequencer();
private readonly _useModalConfigObs;
/**
* Set while a working set is being restored on session switch. The editor
* part is revealed programmatically in this case, so the "editor implies
* auxiliary bar" invariant is suppressed to honor the session's saved
* auxiliary bar visibility (e.g. the user hid it for this session).
*/
private _suppressAuxiliaryBarEnforcement = false;
constructor(
@IWorkbenchLayoutService private readonly _layoutService: IWorkbenchLayoutService,
@ISessionsManagementService private readonly _sessionManagementService: ISessionsManagementService,
@@ -306,6 +314,9 @@ export class LayoutController extends Disposable {
// --- Auxiliary bar ---
private _enforceAuxiliaryBarWhenEditorVisible(): void {
if (this._suppressAuxiliaryBarEnforcement) {
return;
}
if (
this._layoutService.isVisible(Parts.EDITOR_PART, mainWindow) &&
!this._layoutService.isVisible(Parts.AUXILIARYBAR_PART)
@@ -314,6 +325,20 @@ export class LayoutController extends Disposable {
}
}
/**
* Reveals the editor part without triggering the "editor implies auxiliary
* bar" invariant. Used when restoring a session's working set so the
* session's saved auxiliary bar visibility is respected.
*/
private _revealEditorPartForWorkingSet(): void {
this._suppressAuxiliaryBarEnforcement = true;
try {
this._layoutService.setPartHidden(false, Parts.EDITOR_PART);
} finally {
this._suppressAuxiliaryBarEnforcement = false;
}
}
private _captureViewState(sessionResource: URI): void {
const auxiliaryBarVisible = this._layoutService.isVisible(Parts.AUXILIARYBAR_PART);
const activeViewContainerId = this._paneCompositePartService.getActivePaneComposite(ViewContainerLocation.AuxiliaryBar)?.getId();
@@ -468,12 +493,12 @@ export class LayoutController extends Disposable {
}
if (!isModal && !this._layoutService.isVisible(Parts.EDITOR_PART, mainWindow)) {
this._layoutService.setPartHidden(false, Parts.EDITOR_PART);
this._revealEditorPartForWorkingSet();
}
const result = await this._editorGroupsService.applyWorkingSet(workingSet, { preserveFocus });
if (!isModal && result && !this._layoutService.isVisible(Parts.EDITOR_PART, mainWindow)) {
this._layoutService.setPartHidden(false, Parts.EDITOR_PART);
this._revealEditorPartForWorkingSet();
}
});
}
@@ -4,6 +4,7 @@
*--------------------------------------------------------------------------------------------*/
import assert from 'assert';
import { timeout } from '../../../../../base/common/async.js';
import { Emitter, Event } from '../../../../../base/common/event.js';
import { DisposableStore } from '../../../../../base/common/lifecycle.js';
import { constObservable, ISettableObservable, observableValue } from '../../../../../base/common/observable.js';
@@ -112,14 +113,23 @@ suite('LayoutController', () => {
let setPartHiddenCalls: { hidden: boolean; part: Parts }[];
let activePaneCompositeId: string | undefined;
function createLayoutController(): LayoutController {
interface ICreateOptions {
readonly useModal?: 'off' | 'some' | 'all';
readonly workspaceFolders?: readonly { readonly uri: URI }[];
readonly layoutState?: readonly object[];
}
function createLayoutController(options: ICreateOptions = {}): LayoutController {
const instaService = store.add(new TestInstantiationService());
storageService = store.add(new TestStorageService());
if (options.layoutState) {
storageService.store('sessions.layoutState', JSON.stringify(options.layoutState), StorageScope.WORKSPACE, 0);
}
instaService.stub(IStorageService, storageService);
const configService = new TestConfigurationService();
configService.setUserConfiguration('workbench.editor.useModal', 'all');
configService.setUserConfiguration('workbench.editor.useModal', options.useModal ?? 'all');
instaService.stub(IConfigurationService, configService);
activeSessionObs = observableValue<IActiveSession | undefined>('activeSession', undefined);
@@ -151,7 +161,12 @@ suite('LayoutController', () => {
}
override setPartHidden(hidden: boolean, part: Parts): void {
setPartHiddenCalls.push({ hidden, part });
const wasVisible = partVisibility.get(part) ?? true;
partVisibility.set(part, !hidden);
// Mirror production: fire the visibility change synchronously when it actually changes
if (wasVisible === hidden) {
onDidChangePartVisibility.fire({ partId: part, visible: !hidden });
}
}
override hasFocus(_part: Parts): boolean { return false; }
override readonly onDidChangePartVisibility = onDidChangePartVisibility.event;
@@ -193,7 +208,7 @@ suite('LayoutController', () => {
instaService.stub(IWorkspaceContextService, new class extends mock<IWorkspaceContextService>() {
override readonly onDidChangeWorkspaceFolders = Event.None;
override getWorkspace(): IWorkspace { return { id: 'test', folders: [] }; }
override getWorkspace(): IWorkspace { return { id: 'test', folders: (options.workspaceFolders ?? []) as IWorkspace['folders'] }; }
});
return store.add(instaService.createInstance(LayoutController));
@@ -279,6 +294,52 @@ suite('LayoutController', () => {
);
});
// --- Editor / auxiliary bar invariant ---
test('reveals auxiliary bar when the editor part becomes visible', () => {
createLayoutController();
partVisibility.set(Parts.EDITOR_PART, true);
partVisibility.set(Parts.AUXILIARYBAR_PART, false);
setPartHiddenCalls = [];
// Simulate the editor part becoming visible (e.g. opening a file from chat)
onDidChangePartVisibility.fire({ partId: Parts.EDITOR_PART, visible: true });
assert.ok(
setPartHiddenCalls.some(c => c.part === Parts.AUXILIARYBAR_PART && c.hidden === false),
'auxiliary bar should be revealed when the editor becomes visible'
);
});
test('does not force auxiliary bar visible when restoring editor working set on session switch', async () => {
const session = makeSession(URI.parse('session:1'));
createLayoutController({
useModal: 'some',
workspaceFolders: [{ uri: URI.file('/repo') }],
layoutState: [{
sessionResource: 'session:1',
editorWorkingSet: { id: 'ws-1', name: 'ws-1' },
viewState: { auxiliaryBarVisible: false, auxiliaryBarActiveViewContainerId: undefined },
}],
});
partVisibility.set(Parts.EDITOR_PART, false);
partVisibility.set(Parts.AUXILIARYBAR_PART, false);
setPartHiddenCalls = [];
activeSessionObs.set(session, undefined);
// Flush the working-set sequencer (queued microtasks)
await timeout(0);
assert.ok(
setPartHiddenCalls.some(c => c.part === Parts.EDITOR_PART && c.hidden === false),
'editor part should be revealed by the working set restore'
);
assert.ok(
!setPartHiddenCalls.some(c => c.part === Parts.AUXILIARYBAR_PART && c.hidden === false),
'auxiliary bar must not be forced visible during working set restore'
);
});
// --- Panel visibility ---
test('hides panel by default when no record exists', () => {