Then read the relevant spec for the area you are changing (see table below). If you modify the implementation, you must update the corresponding spec to keep it in sync.
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 1–3 sentences: the anti-pattern, why it is wrong, and the preferred pattern.
-
Default/main chat title persistence differs per provider — don't unify them: For the agent host, the default (main) chat title is independent of the session title (AgentHostSessionAdapter._defaultChatTitleOverride, persisted on the host as customChatTitle:<defaultChatUri>); it must be seeded back on restore via restoreSession/_ensureDefaultChat (mirroring _restorePeerChats), or it reverts to the session title after a process restart / idle eviction. For the local chat sessions provider (localChatSessionsProvider), the primary chat and the session intentionally share one _title observable, so renaming the first/main chat updates the session title live — this is by design; do not "fix" it to be independent. Only additional (non-primary) local chats have their own title.
-
ISession.capabilities must be observable, not a live plain getter: capabilities can hydrate/change after a session first surfaces (e.g. an agent host whose root state arrives after the session's first SessionState). A plain getter cannot be tracked by the context-key autorun (setActiveSessionContextKeys reads it inside an autorun), so supportsMultipleChats/sessionSupportsFork would stay stale, and a multi-chat catalog processed while supportsMultipleChats was still false would stay collapsed to [defaultChat]. Expose capabilities as IObservable<ISessionCapabilities> (agent host derives it from connection.rootState via observableFromEvent + derivedOpts with structuralEquals; static providers use constObservable), have consumers read .read(reader)/.get(), and re-apply the chat catalog from the last SessionState in an autorun on capability change. Do not fix this by firing _onDidChangeSessions — the active-session context autorun tracks the session's own observables, not the provider's session-list event.
-
A managed/default editor tab must be re-ensured every sync, not opened once: the per-session editor working-set restore (baseSessionLayoutController [B2] _applyWorkingSet) runs on session activation and is not docked-gated, so it reinstates the session's saved editor set and will close any tab not in it (e.g. a set persisted before a new tab existed). A controller that opens a default tab once (guarded by a Set of initialized sessions) therefore loses it permanently after the restore. Ensure managed tabs (the pinned Changes tab, the default File tab) idempotently on every sync (group.editors.some(e => e instanceof X) → open if absent), exactly like the Changes tab — never with an open-once Set. Also: IEditorService.openEditor(input, group) on a typed EditorInput binds group to the options param (overload is (editor, options?, group?)); pass openEditor(input, undefined, group).
-
A "keep the editor closed" rule must not react to the editor's visibility transition: the single-pane new-session rule (R1, _registerSinglePaneNewSessionRules) must drive its hide off the session + active editor, never off onDidChangePartVisibility/an editorVisibleObs. onWillOpenEditor reveals the editor before the opened file becomes the active editor, and toggling the detail panel off reveals the empty editor via setAuxiliaryBarHidden — both fire the visibility event synchronously with the active editor still stale (managed empty tab). A rule that re-runs on that transition re-hides the editor the user just asked to show (file never appears; the whole side pane vanishes on detail-toggle). Hide only when the active editor is non-real content, read the current visibility untracked when deciding to hide, and block spurious width-based reveals at the source with setSuppressDockedEditorRevealSync(true) rather than a visibility backstop. Refinement: dropping the visibility trigger entirely loses the backstop for automatic post-activation reveals (working-set restore, an editor left visible across a session switch), so the editor can appear in a fresh new-session view. Keep the visibility trigger but gate the hide on an explicit-reveal flag: the workbench records _editorRevealedExplicitly (set true only on onWillOpenEditor and the detail-toggle reveal, cleared on any hide and on the suppress false->true transition so a stale cross-session flag can't leak) and exposes isEditorRevealedExplicitly(); R1 re-hides only when the editor is visible and the reveal was not explicit.
-
When the docked editor is hidden while the detail stays visible, clear the sidebar-grow snapshots: hiding the editor resizes the docked node down to the detail width (_dockedAuxiliaryBarWidth). Any _editorSizeGrownForSidebarHide/_detailWidthGrownForSidebarHide snapshot captured earlier (while the editor was visible and the sessions list was hidden) is now stale — restoring it when the sessions list is later shown re-inflates the node so the detail fills the whole side pane instead of the chat reclaiming the freed editor width. setEditorHidden(true) must drop both snapshots in the detail-still-visible branch.
-
Overriding a workbench toolbar hover needs matching specificity: the core rule .monaco-workbench .monaco-action-bar:not(.vertical) .action-label:not(.disabled):hover (and the :hover outline rule) sets the toolbar hover background/outline at ~6-7 class specificity. A static (non-interactive) action-item label (e.g. the single-pane diff-stats label) that should show no hover affordance must override with an equal-or-higher-specificity selector (prefix .monaco-workbench ... .monaco-action-bar:not(.vertical) .action-item.<class> .action-label:hover), not a short .<class> .action-label:hover that loses the cascade.
-
A managed placeholder editor must never reveal the docked editor content: the empty Files placeholder tab (EmptyFileEditorInput) activates as a side effect of closing another tab (e.g. the Changes tab), firing onWillOpenEditor before it is in the group model. The workbench onWillOpenEditor reveal handler must skip revealing for that placeholder (check e.editor.typeId === 'workbench.editors.agentSessions.emptyFile' — a literal, since src/vs/sessions/browser/* is core and must not import the contrib EmptyFileEditorInput). Do NOT use a broad "editor already in group -> skip" rule: that also stops a hidden editor from revealing when the user re-opens an already-open real file. Extract the handler into a named method (_handleWillOpenEditor) so it is unit-testable via Reflect.get(Workbench.prototype, ...).
-
Gate single-pane editor-title actions on MainEditorAreaVisibleContext: every single-pane (config.<DOCK_DETAIL_PANEL_SETTING>-gated) editor-title menu item (Maximize/Restore, Toggle Details, Hide Editor, Open in Modal) must include MainEditorAreaVisibleContext in its when so the whole action set disappears when the editor content is closed — the docked tab bar stays but its right-hand actions do not. The docked reveal-sync (_syncDockedEditorVisibility) must be symmetric: it reveals when the node widens past the detail width and hides (sets partVisibility.editor=false, flips MainEditorAreaVisibleContext) when a sash drag squeezes the editor content back down to the detail width — same guards (_syncingDockedEditorVisibility, _suppressDockedEditorRevealSync, _dockDetailPanel, and only while the detail is visible).
-
The managed Files placeholder tab is conditional, not always-on: ChangesTabController shows the empty Files tab only when the editor area is closed OR no real (non-managed) editor is open; once a real file/diff is open in a visible editor area it removes the placeholder (and re-adds it when the area closes). Drive this off an autorun that also reads the editor-area visibility (observableFromEvent(onDidChangePartVisibility, () => isVisible(EDITOR_PART, mainWindow))) and an editor-change signal (observableSignalFromEvent(this, Event.any(onDidActiveEditorChange, onDidEditorsChange))), and keep the ensure/remove idempotent so it settles instead of looping. Close/open the placeholder under suppressEditorPartAutoVisibility().
-
A per-session aux-bar (detail) capture must be skipped during a session-switch restore: the D2 onDidChangePartVisibility listener that records a created session's detail visibility must bail while _isRestoringSessionLayout is true. During restore an external component (e.g. DetailPanelController) can transiently reveal the aux bar for the incoming active editor; capturing that overwrites the session's saved detail-hidden state, so switching back shows the detail even though the user had closed it. Keep the aux-bar sync work synchronous (fire-and-forget the view-open calls) so the restore epoch ends promptly and legitimate post-restore user toggles are still captured.
-
The detail-panel forced reveal must be gated on the editor content being visible: DetailPanelController._syncForcedTarget reveals the docked detail (aux bar) when a Changes/File editor becomes active while the detail is hidden. That reveal must additionally require isVisible(EDITOR_PART, mainWindow) — otherwise, on a window reload where the user had closed the whole side pane (editor + detail both hidden, persisted), the restored managed tab becoming active re-reveals the detail and the closed state is lost. Reveal the detail only to accompany a visible editor; when the whole pane is intentionally closed, leave it closed.
-
Auto-managed tabs must still be user-closable — remember user dismissals instead of blindly re-ensuring: ChangesTabController re-ensures the managed Changes/Files tabs on many signals (session state, editor visibility, editor changes). Without care this re-creates a tab the instant the user closes it, so the tab feels un-closable (the managed tabs are non-preview pinEditor, NOT sticky — they do have close buttons; the blocker is the re-ensure, not a missing button). Track user-initiated closes in a _dismissedManagedTabs set (via onDidCloseEditor, ignoring the controller's own closes tracked in an _internallyClosingEditors set) and skip ensuring a dismissed kind. Clear the set only on a session change or when the side pane is reopened from fully closed (edge-detected via a stored _sidePaneWasVisible), so closes stick within the session while reopening/switching re-populates (Scenario B).
-
D10 (empty aux-bar cleanup) must gate on quick-chat, not the racy container-active check, or it flickers the side pane closed on reload: the Agents-window Changes/Files aux-bar views gate on SessionHasWorkspaceContext + WorkspaceFolderCountContext, which are set asynchronously (via the setActiveSessionContextKeys autorun reading the session's async workspace) after a session activates/reloads. So right after D3b/DetailPanelController/a manual toggle reveals the aux bar, isViewContainerActive(Files/Changes) is transiently false (context keys not settled) even for a real workspace session. The D10 reconcile (_syncAuxiliaryBarPartVisibility, which runs synchronously on the onDidChangePartVisibility(visible) signal and only ever hides) then closes the just-opened side pane, and since it never re-reveals, it stays closed — the reload "side pane opens then closes" flicker, "Files not shown when opening the side pane", and "new-session side-pane state not remembered". Fix: D10 hides only when the aux is genuinely empty for the active session's lifetime — no active session, or a workspace-less quick chat (activeSession.isQuickChat?.get() === true, its Changes+Files permanently gated off) — never for a workspace-backed session whose gating context keys are merely still settling. Do NOT use the transient _hasActiveAuxViewContainers() result to hide a workspace session's aux.
-
DetailPanelController must not hide the detail on an empty editor group in the new-session view: _computeTarget returns Hidden when the main editor part is empty (a created session's all-tabs-closed → whole side pane closed). But the new-session (uncreated) view's editor group is transiently empty while its Files tab is (re)ensured, and its Files detail is open by default and owned by the layout controller's D3b. Gating the empty-group Hidden on activeSession.isCreated avoids a transient hide that the D2 visibility listener would otherwise capture as the new-session preference — making every subsequent cmd+n open with the side pane hidden. Combined with the editor-visibility reveal gate, the new-session default stays open while a user's explicit hide is still remembered (D3b _newSessionViewState).
-
Managed-tab reconciliation must run entirely under suppressEditorPartAutoVisibility(): ChangesTabController._syncChangesEditor closes stale managed tabs (e.g. a restored Changes tab whose session's workspace hasn't resolved yet on reload) before ensuring the current ones. If any close (esp. _closeInactiveChangesEditors) runs unsuppressed and empties the group, the workbench handleDidCloseEditor docked branch treats it as "user closed all tabs" and closes the whole side pane — the reload flicker where the side pane appears then vanishes. Wrap the full reconciliation body in one suppression window so transient empty states are never mistaken for a user action; don't rely on per-open suppressions alone. Relatedly, the managed Files placeholder tab must NOT be auto-removed based on editor-area visibility (old Scenario 9 auto-remove) — that removal also emptied the group on reload; only remove it on an explicit user close (tracked via _dismissedManagedTabs).
-
Layout-driven editor closes (working-set apply) must not be mistaken for user closes: On any single-pane session switch (incl. Cmd+N to a new untitled session and reload), the base controller applies the target session's editor working set — an empty working set closes the managed Changes/Files tabs externally. Two bugs resulted: (1) the workbench handleDidCloseEditor docked branch saw an empty group and closed the whole side pane (reload/Cmd+N flicker: pane/Files-tab appears then vanishes); (2) ChangesTabController._handleEditorClosed recorded the external close as a user dismissal, poisoning _dismissedManagedTabs so the Files tab toggled on/off on alternate Cmd+N presses. Fix: _withSessionLayoutRestore (base controller) holds suppressEditorPartAutoVisibility() for the whole (async) restore only when isSinglePaneLayoutEnabled (OFF layout unchanged), so layout-driven closes never reach handleDidCloseEditor; and _handleEditorClosed ignores closes while layoutService.isEditorPartAutoVisibilitySuppressed() is true, so only a genuine user close dismisses a managed tab. Added isEditorPartAutoVisibilitySuppressed() to IAgentWorkbenchLayoutService as the shared "this close is layout-driven, not a user action" signal.
-
DetailPanelController must not force-reveal the detail during a layout-driven restore: _syncForcedTarget reveals the aux-bar detail to accompany the active Changes/Files editor. On a session switch the target session's editor working set is restored, making its Changes/Files editor active — an editor change that is NOT a user open. If the user had hidden the detail for that session, that restore-driven editor change would force-reveal it, losing the per-session detail-hidden state. Gate the reveal (setPartHidden(false, AUXILIARYBAR)) on !isEditorPartAutoVisibilitySuppressed() (in addition to the existing editor-visible guard): during _withSessionLayoutRestore the base controller holds suppressEditorPartAutoVisibility() (single-pane), so a restore-driven forced target skips the reveal while a genuine user editor open (unsuppressed) still reveals. Note the existing [D3c/single-pane] test passed because it simulated the reveal synchronously during apply (so D3 re-hid last); the real bug is the async DetailPanelController reveal firing on the editor-change after D3 already hid.
-
Single-pane detail/tab behaviour lives ON the layout controller, not in separate controllers or a shared service: SinglePaneDesktopSessionLayoutController owns both the managed docked tabs (pinned Changes multi-diff + empty Files placeholder) and the detail-panel mapping (active editor → Changes/Files container, aux-bar reveal/hide). They were previously ChangesTabController/DetailPanelController (registered by a SinglePaneModeController contribution) coordinating via global IAgentWorkbenchLayoutService flags, then briefly via an ISessionLayoutCoordinatorService. Both were removed: because everything is now one class, "is a session-switch restore in progress?" is just the base protected getter this._isRestoringSessionLayout (set by _withSessionLayoutRestore), so a restore-driven editor change never force-reveals the detail or dismisses a managed tab. The two sub-behaviours register in the _registerAuxiliaryControllers() base hook, deferred to LifecyclePhase.Restored so managed tabs reconcile on top of the restored editor group. The base controller gained IChangesViewService + IContextKeyService deps and a protected _editorGroupsService to support this (a subclass can't add DI ctor params without redeclaring all base params, so the shared services live on the base). Tests: the layout harness got activeGroupEditors/closeSuppressionFlags, a real mainPart.activeGroup, an activateAux opt-in that resolves the lifecycle, and a TestSinglePaneController.runWithRestore(...) seam to hold _isRestoringSessionLayout across an async editor change; changesTabController.test.ts was deleted and its scenarios moved into desktopSessionLayoutController.test.ts.
-
Single-pane created-session default is Editor-only (Changes editor, detail closed) — the detail is not force-opened by editor activation: a Changes/file editor becoming active must NOT auto-reveal the docked detail (aux bar). SinglePaneDesktopSessionLayoutController._syncForcedDetailTarget reveals a hidden detail ONLY when it was transiently hidden by a browser tab (_hiddenByBrowser), never when it is hidden by the per-session default or an explicit user hide; when the detail is visible it still switches the container (Changes/Files) to match the active editor. The reopen default is layout-aware via the base _defaultReopenSidePaneParts() hook (base returns both parts; single-pane returns {editor:true, auxiliaryBar:false} for a created session and {editor:false, auxiliaryBar:true} for a new-session view). The detail is opened only via Toggle Details or restored per-session state. When changing this, update the [single-pane] ... file tab activation / [per-session detail] / reopen-default tests together — they encode the reveal semantics.
-
Editor-title actions that only make sense with a restorable editor must also gate on EditorMaximizedContext.negate(): the single-pane "Hide Editor" action is meaningless while the editor area is maximized, so its when includes EditorMaximizedContext.negate() (in addition to MainEditorAreaVisibleContext + SinglePaneDetailChangesOrFilesActiveContext).
-
R1 (new-session editor hide) must be transition-triggered, not level-triggered on the active editor: hiding the editor in the new-session view must fire only when the editor just became visible (visibility false→true) or when the view was just entered with the editor already visible (inherited-visible editor) — never merely because the active editor changed to a managed placeholder while the editor is already visible. A level-triggered rule ("hide whenever active editor is non-real content and editor visible") wrongly hides the editor when the user switches to the Files tab with a file already open (the reveal-sync suppression re-arm clears isEditorRevealedExplicitly, so the level rule then hides). Track previousEditorVisible + previousInNewSessionView in the autorun and hide only on (editorJustRevealed || justEnteredNewSessionView) && !isEditorRevealedExplicitly(). The two workbench methods setSuppressDockedEditorRevealSync (blocks width-based reveals at the source, avoiding flicker) and isEditorRevealedExplicitly (distinguishes an explicit toggle-details-off/file-open reveal that must stick) are still required by R1 — they are independent of the ChangesTab/DetailPanel controller merge.
-
Single-pane created sessions need the docked editor part revealed on switch — the isModal gate in _applyWorkingSet skips it: baseSessionLayoutController._applyWorkingSet only reveals the editor part when !isModal (i.e. workbench.editor.useModal !== 'all'), because in the classic layout editors open in a modal part. But in single-pane the docked editor lives in the grid even when useModal is 'all' (the default), so that gate wrongly skips the reveal and a created session's side pane looks fully closed (worse once the Changes editor no longer force-reveals the detail). Fix: compute revealEditorPart = !editorPartHidden && !isInitialRestore && (isSinglePaneLayoutEnabled ? isCreatedSession : !isModal) and also reveal for the 'empty' working-set case in single-pane (a first-visit created session has no saved editors but still shows its managed Changes editor). This restores the Editor-only default while respecting the per-session editorPartHidden (Detail-only / side-pane-closed) state and excluding new-session views (R1 keeps their editor closed). Note the layout test harness leaves isSinglePaneLayoutEnabled falsy by default, so base single-pane branches are inert in tests unless a test opts in via the singlePaneLayoutEnabled create option.
-
R1 can drop setSuppressDockedEditorRevealSync — always hide on new-session-view entry instead: the width-based reveal-sync suppression (setSuppressDockedEditorRevealSync/_suppressDockedEditorRevealSync) was removed. It did two jobs: (1) block a momentary width-reveal of the editor in the new-session view, and (2) clear _editorRevealedExplicitly on entering the view so R1 re-hides an inherited-explicit editor across a session switch (the working-set apply runs under suppressEditorPartAutoVisibility, so handleDidCloseEditor doesn't clear the flag naturally). Job (1) is now handled by R1 re-hiding any non-explicit reveal (a sash-drag reveal flickers then re-hides — acceptable). Job (2) is handled by making R1's hide condition justEnteredNewSessionView || (editorJustRevealed && !isEditorRevealedExplicitly()) — i.e. entering the new-session view always resets to editor-closed (a stale cross-session explicit flag can't keep the editor open), while the explicit flag is only honored for in-session reveals (toggle-details-off revealing the empty editor). isEditorRevealedExplicitly is still needed for that in-session case.
-
Quick chats have no side pane — don't auto-reveal the editor part, and hide it when switching in from a session that had it open: in single-pane, SinglePaneDesktopSessionLayoutController._shouldRevealEditorPartOnApply must exclude quick chats (!editorPartHidden && isCreatedSession && !isQuickChat); a created quick chat would otherwise reveal the docked editor part on switch (bug: "side pane opened automatically for quick chat"). Excluding the reveal is not enough — switching in from a workspace session leaves the editor part visible (the working-set apply is suppressed and never hides it), so a dedicated _registerQuickChatEditorHide() autorun hides the editor part while a quick chat's editor group is empty (gated on _isMainPartEmpty() so a real editor, e.g. the integrated browser, opened in a quick chat is never hidden). The aux bar is already handled by D10 + the detail-panel Hidden target.
-
An auto-collapsed sessions list must be restored once the side pane is fully hidden: the single-pane responsive rule auto-collapses the sessions list to free width for a visible side pane (Toggle Details, opening a file). It must also restore an auto-hidden list when the side pane becomes fully hidden (both editor and aux bar closed) — e.g. switching to a quick chat (no side pane) — otherwise the list is left collapsed with nothing to make room for (bug: "sessions list closed even though the side pane is hidden"). Implement as an autorun in _registerResponsiveSidebar on an observableFromEvent(onDidChangePartVisibility, () => editorVisible || auxVisible) (the value-dedup is essential: hiding the sidebar itself doesn't change the computed side-pane visibility, so the pre-reveal auto-hide from opening an editor is never undone). Restore only when _sidebarAutoHidden is true, so a list the user closed manually stays closed.
-
Single-pane per-session editor-part visibility must be restored both ways — _applyWorkingSet only ever revealed it: baseSessionLayoutController._applyWorkingSet revealed the editor part when a session wanted it visible but never hid it, so returning to a session whose docked editor was closed (Detail-only or whole side pane closed) left the editor visible/inherited from the previously-active session (bug: "side pane opened when returning to a session where it was closed"). The per-session _editorPartHiddenBySession state was only consumed to suppress the reveal (!editorPartHidden), never to actively hide. Fix: add a symmetric Template-Method hook _shouldHideEditorPartOnApply(editorPartHidden) (base returns false — classic layout doesn't treat editor-part visibility as per-session; single-pane returns editorPartHidden && isCreated && !isQuickChat) and, in both the empty and non-empty _applyWorkingSet branches, hide the editor part (mutually exclusive with revealing, skipped on isInitialRestore which preserves the workbench-restored visibility). The hide runs inside _withSessionLayoutRestore's suppressEditorPartAutoVisibility window so it is never mistaken for a user close. Note the aux bar was already restored both ways by the inherited D3 _syncAuxiliaryBarVisibility; only the editor part lacked the hide.
-
Per-session editor-part (side-pane) hidden state must be captured eagerly on the visibility change, not lazily re-read at switch-away: baseSessionLayoutController._saveWorkingSet used to record _editorPartHiddenBySession[prev] = !isVisible(EDITOR_PART) at the moment it saved the outgoing session. That races: the working-set derive (activeSessionForWorkingSet) lags the raw activeSession (it gates on workspace-folder readiness), so other autoruns driven by the raw active session (managed-tab open, D3 aux sync) have already revealed the editor for the incoming session by the time _saveWorkingSet(prev) runs — so the previous session gets recorded as editorPartHidden=false and its closed side pane reopens on return (symptom: only the editor content re-appears, details stay closed, and no setEditorHidden fires on the switch because nothing on the switch path toggles it). Fix: capture it in a [B2] onDidChangePartVisibility(EDITOR_PART) listener (mirroring the existing [B1] panel-visibility capture) guarded by !multipleSessionsVisibleObs && !_isRestoringSessionLayout, so the value is written the instant the user closes/opens the side pane and layout-driven restore changes are ignored. Remove the lazy read from _saveWorkingSet entirely (keeping it would let the racy switch-time value overwrite the good eager one). The unit harness can't reproduce the derive-lag, so add a focused test that fires the EDITOR_PART event to assert eager capture, plus one that fires a reveal inside _withSessionLayoutRestore to assert the captured closed state is preserved.