sessions: model docked (Changes/Files) editors as DockedEditorInput (#325332)

* sessions: model docked (Changes/Files) editors as DockedEditorInput

Replace the ad-hoc reveal-suppression predicate (setEditorRevealOnOpenExclusion) with a proper DockedEditorInput base class that the Changes and Files editors extend. The single-pane workbench owns the policy: revealEditorOnOpen (renamed from _handleWillOpenEditor, now protected) is overridden so activating a docked editor does not reveal the editor area only while the detail panel is open and the editor area is closed. When the detail panel is closed, opening a docked editor reveals it as usual.

Also drive the editor-area collapse strategy off 'instanceof DockedEditorInput' instead of the coordinator's isManagedEditor heuristic, and close every non-docked editor when the editor area hides (restoring the ones that can be re-resolved from a resource; non-restorable ones such as an untitled Search editor are dropped since they can neither be serialized nor reopened after disposal).

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

* sessions: address CCR feedback on docked-editor docs/comments

Fix stale references left by the DockedEditorInput refactor:
- workbench.ts onWillOpenEditor comment now describes the base reveal and points to the SinglePaneWorkbench.revealEditorOnOpen override (the base no longer special-cases the Files tab).
- LAYOUT.md editor-area tab collapse now references SinglePaneEditorAreaCollapseStrategy._collapseNonManagedTabs / _restoreCollapsedTabs instead of the non-existent SinglePaneLayoutController._registerEditorAreaTabCollapse.
- SKILL.md 'Explicit managed-editor opens' entry now describes DockedEditorInput / the SinglePaneWorkbench override instead of the removed _handleWillOpenEditor exclusion.

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

---------

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

.github/skills/sessions/SKILL.md

@@ -87,7 +87,7 @@ Whenever the user flags a wrong pattern, rejects an approach, or gives design/ru
 
 - **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. An action-item label that needs to override that hover (either to suppress it for a non-interactive label, or to re-skin it) must use 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. (The single-pane diff-stats pill was once suppressed this way while static; it is now a clickable action that opens the multi-file diff, so it keeps the standard `--vscode-toolbar-hoverBackground` hover.)
 
-- **A managed tab must never reveal the docked editor content — but the core workbench must not hardcode which, and the excluded editor's deliberate open must reveal explicitly**: the empty Files placeholder (`EmptyFileEditorInput`) and the Changes multi-diff (`SessionChangesEditorInput`) surface their content in the detail panel, so activating either (e.g. the placeholder activating as a side effect of closing the Changes tab, or clicking the Changes tab) must not reveal the editor area. The core workbench `_handleWillOpenEditor` reveal handler skips revealing for editors matched by a **contrib-provided predicate** (`IAgentWorkbenchLayoutService.setEditorRevealOnOpenExclusion`), which the single-pane layout controller sets to its `_isManagedEditor` check — so the editor-type policy lives in contrib (which can `instanceof` the real inputs), not as hardcoded type-id literals in `src/vs/sessions/browser/*` (core). **Caveat:** because tab activation and opening a file diff both fire `onWillOpenEditor` for the *same* Changes editor (and the event carries no options to tell them apart), excluding it from the auto-reveal also blocks the deliberate file-open reveal — so the Changes view's `_openMultiFileDiffEditor` (clicking a file) must **explicitly** `setPartHidden(false, EDITOR_PART)` before opening (revealing before the open also avoids the multi-diff hanging while laid out in a hidden 0-size editor part). Do NOT use a broad "editor already in group -> skip" rule. Keep `_handleWillOpenEditor` a named method so it is unit-testable via `Reflect.get(Workbench.prototype, ...)`.
+- **A docked-detail editor must not reveal the editor area while the detail panel is already showing its content — model it as a base editor input the single-pane workbench recognises**: the empty Files placeholder (`EmptyFileEditorInput`) and the Changes multi-diff (`SessionChangesEditorInput`) surface their content in the docked detail panel (auxiliary bar). Activating one (closing a neighbouring tab so the workbench auto-opens the next editor via `editorGroupView.doCloseActiveEditor` → `doOpenEditor`, or clicking the tab) fires `onWillOpenEditor` *unsuppressed* and would otherwise reveal the hidden editor area. Both inputs extend the abstract base `DockedEditorInput` (`src/vs/sessions/common/dockedEditorInput.ts`, extends `EditorInput`). The base `Workbench.revealEditorOnOpen(e)` (the `onWillOpenEditor` handler — a **protected** method, renamed from `_handleWillOpenEditor`) does the generic reveal; `SinglePaneWorkbench` **overrides `revealEditorOnOpen`** and returns early (no reveal) when `e.editor instanceof DockedEditorInput && partVisibility.auxiliaryBar && !partVisibility.editor` — i.e. only when the detail panel is open and the editor area is closed — otherwise it calls `super.revealEditorOnOpen(e)`. So the docked-editor policy lives in `SinglePaneWorkbench` (the only workbench with a docked detail panel) via a proper type + the current part visibility, not a per-input marker, a contrib-registered predicate, or a remembered set. Note the condition means that when the detail panel is **closed** (whole side pane closed), opening a docked editor **does** reveal the editor area so its content is visible. **Caveat:** a deliberate open of the already-open Changes tab (session-header pill `ViewAllChangesAction`) or a file diff (`_openMultiFileDiffEditor`) while the detail panel is open is still suppressed by this rule, so those must **explicitly** reveal via `revealEditorPartExplicitly()` before opening (revealing before the open also avoids the multi-diff hanging while laid out in a hidden 0-size editor part). Keep `revealEditorOnOpen` a named protected method so it is unit-testable via `Reflect.get(...prototype, ...)`.
 
 - **Gate single-pane editor-title *layout/view* actions on `MainEditorAreaVisibleContext`; the Create Pull Request bar lives in the title bar, not the editor**: single-pane (`config.<DOCK_DETAIL_PANEL_SETTING>`-gated) editor-title *layout/view* items (Maximize/Restore, Toggle Details, Hide Editor, Open in Modal, the diff-view actions collapse/expand/toggle-inline/list-tree) must include `MainEditorAreaVisibleContext` so they disappear when the editor content is closed. The **Create Pull Request** anchor (`CHANGES_HEADER_ACTIONS_ID`) is *not* an editor-title action: it is contributed to `Menus.TitleBarSessionMenu` (the sessions title bar's session-actions area) by `ChangesHeaderActionsAction` in `changesViewActions.ts`, gated on `IsSessionsWindowContext` + `IsAuxiliaryWindowContext.toNegated()` + `config.<DOCK_DETAIL_PANEL_SETTING>` + `SessionHasChangesContext` (independent of editor-area visibility), and its `ChangesActionsBar` view item is registered for `(Menus.TitleBarSessionMenu, CHANGES_HEADER_ACTIONS_ID)` via `IActionViewItemService`. 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).
 
@@ -129,7 +129,7 @@ You **must** run these checks before declaring work complete:
 
 - **Single-pane detail (aux bar) ownership is split cleanly in two — visibility vs content — never three overlapping aux strategies**: in single-pane the auxiliary bar *is* the detail panel, so exactly two strategies touch it, with non-overlapping responsibilities. `SinglePaneDetailVisibilityStrategy` owns **only** per-session *shown/hidden* memory: it captures the user's choice ([D1]/[D2]), restores it on switch ([D3]) by revealing/hiding the aux **part** (`setPartHidden` / `hideAuxiliaryBarForRestore`), and handles the submit transition ([D4]). `SinglePaneDetailPanelStrategy` owns **everything about content**: which container (Changes/Files, mapped from the active editor), the transient browser-tab hide, editor-maximize → Changes, and the "nothing to show" hide (quick chat / no workspace / empty group → `Hidden`). Do NOT reintroduce a separate `EmptyAuxCleanup`/D10 strategy or desktop's saved-container machinery (`auxiliaryBarActiveViewContainerId` restore, `_openDefaultAuxiliaryBarContainer`, `_restoreSavedAuxiliaryBarContainerOnReveal`, pinned-container checks) into the visibility strategy — the container always follows the active editor, so a stored container preference is redundant and races the detail-panel mapping. Because the visibility strategy reveals the part and the detail-panel strategy fills it, the detail-panel strategy registers **immediately** in `_registerViewStateManagement` (not deferred to `Restored` like the managed tabs), so a reveal and its container open happen in the same turn.
 
-- **Single-pane is a *sibling* of the desktop controller and composes strategy objects — it does not extend `LayoutController`**: `SinglePaneLayoutController` (file `contrib/layout/browser/singlePaneLayoutController.ts`) extends `BaseLayoutController` directly, NOT the classic desktop `LayoutController`, so the desktop controller can be deprecated/deleted without touching single-pane. Its behaviour is composed from strategy objects under `contrib/layout/browser/singlePane/` (each a `Disposable`, created via `createInstance` with a leading `ISinglePaneLayoutContext` arg): `SinglePaneDetailVisibilityStrategy` (per-session detail shown/hidden: D1/D2/D3/D4) and `SinglePaneDetailPanelStrategy` (container + maximize + browser-hide + nothing-to-show hide) — the detail split above; `SinglePaneManagedTabsStrategy` + `SinglePaneEditorAreaCollapseStrategy` (share a `SinglePaneDockedTabsCoordinator` holding the tab `Sequencer`, `internallyClosingEditors`, `collapsedEditors`, and the `isManagedEditor`/`getChangesEditorResource` helpers); `SinglePaneQuickChatEditorHideStrategy`; `SinglePaneResponsiveSidebarStrategy` (owns the Toggle Details action + sidebar auto-hide); `SinglePaneNewSessionRulesStrategy` (R1). Shared controller state (`isRestoringSessionLayout`, `withSessionLayoutRestore`, `togglingSidePane`, the obs, `viewStateBySession`, `hidingAuxiliaryBarForRestore`/`hideAuxiliaryBarForRestore`) is exposed to strategies through `ISinglePaneLayoutContext` (built lazily in the controller because base's constructor calls the `_registerViewStateManagement`/`_registerAuxiliaryControllers` hooks *before* subclass field initializers run). The detail-visibility/detail-panel/responsive/R1 strategies register in `_registerViewStateManagement`; the managed-tab/collapse/quick-chat strategies register in `_registerAuxiliaryControllers` deferred to `LifecyclePhase.Restored`. **Fresh storage**: single-pane persists to `sessions.singlePane.layoutState` + `sessions.singlePane.newSessionViewState` (base `_layoutStateStorageKey`/`_legacyWorkingSetsStorageKey` are overridable; single-pane skips legacy migration), so it never shares state with the classic desktop controller — the test harness seeds both keys.
+- **Single-pane is a *sibling* of the desktop controller and composes strategy objects — it does not extend `LayoutController`**: `SinglePaneLayoutController` (file `contrib/layout/browser/singlePaneLayoutController.ts`) extends `BaseLayoutController` directly, NOT the classic desktop `LayoutController`, so the desktop controller can be deprecated/deleted without touching single-pane. Its behaviour is composed from strategy objects under `contrib/layout/browser/singlePane/` (each a `Disposable`, created via `createInstance` with a leading `ISinglePaneLayoutContext` arg): `SinglePaneDetailVisibilityStrategy` (per-session detail shown/hidden: D1/D2/D3/D4) and `SinglePaneDetailPanelStrategy` (container + maximize + browser-hide + nothing-to-show hide) — the detail split above; `SinglePaneManagedTabsStrategy` + `SinglePaneEditorAreaCollapseStrategy` (share a `SinglePaneDockedTabsCoordinator` holding the tab `Sequencer`, `internallyClosingEditors`, `collapsedEditors`, and the `getChangesEditorResource` helper; docked (managed) tabs are identified by `instanceof DockedEditorInput`); `SinglePaneQuickChatEditorHideStrategy`; `SinglePaneResponsiveSidebarStrategy` (owns the Toggle Details action + sidebar auto-hide); `SinglePaneNewSessionRulesStrategy` (R1). Shared controller state (`isRestoringSessionLayout`, `withSessionLayoutRestore`, `togglingSidePane`, the obs, `viewStateBySession`, `hidingAuxiliaryBarForRestore`/`hideAuxiliaryBarForRestore`) is exposed to strategies through `ISinglePaneLayoutContext` (built lazily in the controller because base's constructor calls the `_registerViewStateManagement`/`_registerAuxiliaryControllers` hooks *before* subclass field initializers run). The detail-visibility/detail-panel/responsive/R1 strategies register in `_registerViewStateManagement`; the managed-tab/collapse/quick-chat strategies register in `_registerAuxiliaryControllers` deferred to `LifecyclePhase.Restored`. **Fresh storage**: single-pane persists to `sessions.singlePane.layoutState` + `sessions.singlePane.newSessionViewState` (base `_layoutStateStorageKey`/`_legacyWorkingSetsStorageKey` are overridable; single-pane skips legacy migration), so it never shares state with the classic desktop controller — the test harness seeds both keys.
 
 - **Single-pane detail/tab behaviour lives ON the layout controller (or its strategies), not in separate contribution controllers or a shared service**: `SinglePaneLayoutController` 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: "is a session-switch restore in progress?" is just the base protected getter `this._isRestoringSessionLayout` (set by `_withSessionLayoutRestore`) — surfaced to the strategies via `ISinglePaneLayoutContext.isRestoringSessionLayout` — so a restore-driven editor change never force-reveals the detail or dismisses a managed tab. The base controller has `IChangesViewService` + `IContextKeyService` deps and a protected `_editorGroupsService` (a subclass can't add DI ctor params without redeclaring all base params, so 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`.
 
@@ -155,7 +155,7 @@ You **must** run these checks before declaring work complete:
 - **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.
-- **Explicit managed-editor opens must reveal outside the auto-reveal path — and mark the reveal *explicit***: managed Changes/Files tabs are excluded from `_handleWillOpenEditor` so tab activation and layout-driven restores do not reveal the docked editor. A deliberate user gesture that should show managed editor content (session-header Changes pill `ViewAllChangesAction`, opening a file diff in `_openMultiFileDiffEditor`) must reveal the editor part before opening the managed editor via `IAgentWorkbenchLayoutService.revealEditorPartExplicitly()` — **not** the generic `setPartHidden(false, EDITOR_PART)`. The generic call routes to `setEditorHidden(hidden, explicit=false)`, leaving `_editorRevealedExplicitly = false`, so R1 / the working-set apply (`_shouldHideEditorPartOnApply`) can re-hide it (especially across a session-switch race). `revealEditorPartExplicitly()` sets the explicit flag (and re-asserts it even when already visible, since `setEditorHidden` early-returns when the part is already visible). Do not weaken the managed-editor exclusion or add timing delays.
+- **Explicit managed-editor opens must reveal outside the auto-reveal path — and mark the reveal *explicit***: docked-detail Changes/Files editors (`DockedEditorInput`) are kept from revealing the docked editor by `SinglePaneWorkbench.revealEditorOnOpen` (see the entry above), so tab activation and layout-driven restores do not reveal it. A deliberate user gesture that should show managed editor content (session-header Changes pill `ViewAllChangesAction`, opening a file diff in `_openMultiFileDiffEditor`) must reveal the editor part before opening the managed editor via `IAgentWorkbenchLayoutService.revealEditorPartExplicitly()` — **not** the generic `setPartHidden(false, EDITOR_PART)`. The generic call routes to `setEditorHidden(hidden, explicit=false)`, leaving `_editorRevealedExplicitly = false`, so R1 / the working-set apply (`_shouldHideEditorPartOnApply`) can re-hide it (especially across a session-switch race). `revealEditorPartExplicitly()` sets the explicit flag (and re-asserts it even when already visible, since `setEditorHidden` early-returns when the part is already visible). Do not weaken the `DockedEditorInput` reveal suppression or add timing delays.
 - **A `MutableDisposable`-backed content slot must not `clearNode` its shared container on cleanup**: `EditorGroupView.setHeaderContent` appends a new content node into the shared `headerContainer`, then assigns the new store to `_headerContent` (a `MutableDisposable`) — which **synchronously disposes the previous store**. If that store's cleanup calls `clearNode(headerContainer)`, it wipes the freshly-appended new content (blank header, height stuck at 0, orphaned `ResizeObserver`). Fix: clear the previous content **before** appending the new one (`this._headerContent.clear()` at the top), and have each store's cleanup remove only **its own** node (`content.remove()`), never the shared container. This bug surfaces on consecutive header→header renders (e.g. `Changes(sessionA)` → `Changes(sessionB)`).
 
 - **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.