Agents window architecture — covers the agents-first app, layering, folder structure, chat widget, menus, contributions, entry points, and development guidelines. Use when implementing features or fixing issues in the Agents window.
Scanned 8/31/2026
Install to Claude Code
npx -y skills add chapmanjw/clawdius --skill sessions --agent claude-codeInstalls into .claude/skills of the current project.
Are you the author of Sessions?
Add the live security badge to your README — it updates automatically with every re-scan.
[](https://www.skillsdirectory.com/skills/chapmanjw-sessions)More formats (shields.io, HTML) on the badges page.
---
name: sessions
description: Agents window architecture — covers the agents-first app, layering, folder structure, chat widget, menus, contributions, entry points, and development guidelines. Use when implementing features or fixing issues in the Agents window.
---
## Before Making Any Changes
**MANDATORY:** Before writing or modifying any code in `src/vs/sessions/`, you **must** read these documents:
1. **`.github/instructions/coding-guidelines.instructions.md`** — Naming conventions, code style, string localization, disposable management, and DI patterns.
2. **`.github/instructions/source-code-organization.instructions.md`** — Layers, target environments, dependency injection, and folder structure conventions.
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.
## Specification Documents
| Document | Path | When to read |
|----------|------|-------------|
| 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. |
| AI Customizations | `src/vs/sessions/AI_CUSTOMIZATIONS.md` | Before working on the customization editor or tree view. Documents the management editor (in `vs/workbench`) and the tree view/overview (in `vs/sessions/contrib/aiCustomizationTreeView`). |
## Common Pitfalls
- **A sash element's `left`/`top` is the hit-area edge, not the split boundary**: `SplitView.getSashPosition` returns the exact boundary after the preceding view, then `Sash.layout` subtracts half the sash size so the draggable element is centered on that boundary. Align the Sessions/Editor and bottom-Panel grid sash hit areas to `agents.layout.floatingPanelGap`; do not apply that token to independent geometry such as the Auxiliary Bar's leading padding.
- **Wrong menu IDs**: Never use `MenuId.*` from `vs/platform/actions` for Agents window UI. Always use `Menus.*` from `browser/menus.ts`.
- **Durable chat source/origin references**: Store only `turnId` in durable fork/side-chat references. Active versus historical is mutable lifecycle state that consumers must resolve against the current `activeTurn` and retained `turns` when needed; do not encode lifecycle state in the reference type.
- **Selected side-chat text is an immutable snapshot, not a live range**: `SideChatSource.selection` / `ChatOrigin.selection` preserve the exact text captured at side-chat creation time. Never model it as offsets into the source transcript or try to recompute it from later DOM/protocol state.
- **`ChatSource` is fully discriminated**: Fork and side-chat sources both require explicit `kind` plus stable top-level `turnId`. Do not add no-kind compatibility helpers or route by structural property presence; switch directly on `source.kind`.
- **Sessions menu ids must live in the shared menu registry**: Do not declare sessions-owned `new MenuId(...)` constants ad hoc inside individual parts. Add them to `browser/menus.ts` under `Menus` with discoverable `SessionsEditor...` names so ownership and reuse stay obvious.
- **Events instead of observables**: Session state must flow through `IObservable`, not `Event`. Use `autorun`/`derived` for reactive UI, not `onDid*` event listeners.
- **Importing from providers**: Non-provider `contrib/*` code must never import from `contrib/providers/*`. Extract shared interfaces to `services/` or `common/`.
- **`IAgentSessionsService` in shared code**: `IAgentSessionsService` (`vs/workbench/contrib/chat/browser/agentSessions/agentSessionsService`) is a Copilot-provider internal and may be imported **only** by the Copilot chat sessions provider (`contrib/providers/copilotChatSessions/`). Shared sessions code (core/services/non-provider contribs, e.g. the sessions list or visible-sessions grid) must stay provider-agnostic and go through `ISession`/`ISessionsManagementService` — never reach into `model.observeSession(...)` etc. for lazy loading. This is enforced by an ESLint `no-restricted-imports` ban scoped to `src/vs/sessions/**` (Copilot provider exempted).
- **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.
- **Do not repeat subagent identity beside the open-chat pill**: The subagent pill's title is the complete inline affordance. Do not render the agent name or a generic "Subagent" phrase before it; that duplicates identity and adds visual noise.
- **Subagent model metadata is differential inline, but complete in the hover**: Show the subagent's model beside the pill by default and hide it only when it concretely matches the parent chat's selected model. Compare canonical ids, registered display names, and the parent input's selected-model metadata (active turns may not yet expose resolved response metadata). Keep the unfiltered subagent model in the hover/ARIA label regardless of inline visibility; if the parent model is unresolved, still show it inline because no match can be established.
- **Optional metadata owns its separator**: Keep separators such as `·` on the conditional metadata element and create that element hidden. An empty optional label must never leave punctuation behind while its reactive visibility is still resolving.
- **Live numeric labels must not jitter**: Apply `font-variant-numeric: tabular-nums` with `font-feature-settings: "tnum"` as a fallback to elapsed-time and other numeric labels that update in place.
- **Collapsed work summaries must quantify what they hide**: Prefer outcome-oriented copy such as "Completed 6 steps in 2m" over vague elapsed-only text such as "Worked for a few minutes." Count the visible items placed inside the disclosure so the summary matches what expanding it reveals.
- **Custom action proxies must propagate owner-observed state**: When an action view wraps a menu action in a proxy, state that controls surrounding UI (such as enabled/available) must also be written to the original menu action observed by the owner. Re-subscribe on `IActionViewItemService.onDidChange` for late factory registration, but do not assume the replacement proxy's state automatically reaches the menu action.
- **Editor feedback glyph placement**: Use Monaco's `lineNumberClassName` when the feedback affordance should replace the number only while its line is hovered; it eliminates a dedicated glyph lane while preserving the number at rest. Style the line-number pseudo-element as the full feedback control, including its themed hover background, so its visual and click target match.
- **Line-number decoration tooltips belong in Monaco decoration options**: A `lineNumberClassName` node is regenerated as the editor renders and scrolls, so DOM-managed hovers can silently attach to a stale or never-decorated element. Set the localized `lineNumberHoverMessage` with the same decoration instead; Monaco's glyph hover controller follows the rendered line-number lifecycle.
- **Compact multi-diff control alignment**: The file-header twistie, unchanged-region expand control, and fold control form one visual column in the Agents editor. Remove the header content's left padding and use the same small inset for both unchanged-region controls; do not let the shared multi-diff defaults leave each control at a separate horizontal offset.
- **Embedded multi-diff gutters need a shared minimum width**: Each embedded editor otherwise sizes line numbers from its own largest line number, causing the content and nearby feedback glyph to appear to drift between file entries. Set a common `lineNumbersMinChars` width for the compact multi-diff; it remains stable through three-digit line numbers and grows only when a file exceeds that reserved capacity.
- **Editor-content overlays must anchor to the inset pane, not the full editor group**: In single-pane mode the editor group spans both the editor and docked detail panel, while `EditorGroupView.editorPaneContainer` bounds only the editor content. Mount submit/navigation overlays to that pane container so their bottom-right position stays inside the diff when the detail panel is visible or resized.
- **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.
- **Sticky prompt navigation must match the rail/title reveal**: Previous/Next and the sticky title must all reveal the prompt (request) row aligned to the top via the shared `reveal(requestId)` — the same path the dock/ruler rail uses. Do not align the following response to keep the header pinned, and do not add a "navigation pin" that forces the header to stay visible after a jump: the header is a `top:0` overlay, so it would cover the freshly top-aligned prompt (the prompt shows only a sliver). Let the header follow scroll tracking (it hides once the prompt is at the top), consistent with the dock. Use the chat request-bubble hover background (`--vscode-chat-requestBubbleHoverBackground`, toolbar hover as fallback), composited over the opaque panel base, for the sticky title affordance rather than an underline.
- **Sticky prompt header transition is a label *roll*, not a moving band**: on prompt change the label text rolls (WAAPI slide+fade of absolutely-positioned line elements inside an `overflow:hidden` clip viewport), while the opaque band stays fixed. Do NOT translate the whole band to get an Explorer-style push-off: the band would move above the transcript top (its container `.interactive-session` is `overflow:visible`, so it'd overlap the session header) and clipping it would cut the band's soft drop-shadow. The roll gives the "header gives way to the next" feel with none of that risk. Gate the roll on the header already being visible (snap on first appearance/jumps) and honor `prefers-reduced-motion`.
- **Sticky prompt header must mirror the session header's box model, not the message column**: the band lives in a two-level structure — an outer full-width `.prompt-timeline-sticky` (positioning + reveal only), a `.prompt-timeline-sticky-content` centered host (`max-width: 950px; margin: 0 auto; padding: 0 10px`, matching `.session-view-centered-content` + `.chat-composite-bar.session-header-bar`'s 10px side padding), and an inner `.prompt-timeline-sticky-band` (`flex: 1`) that carries the background/border/shadow (mirroring `.chat-composite-bar-header`). This makes the band's background align exactly with the session header's bottom-border line above it. Do NOT paint the background on the full-width outer (bleeds past the header and over the scrollbar gutter — a ~1px right-edge "bump") or on the 950 message column (20px wider than the header, since the header is inset 10px). The band's `padding-left: 22px` (16px icon + 6px gap) reproduces the header's status-icon column so the prompt text lines up with the title and the 32px-inset prompts below.
- **Grid `onDidChange` is not a sash-drag signal**: the workbench `SerializableGrid`/`GridView` `onDidChange` fires for size changes and view add/remove, but not internal splitview sash drags. If logic must react to a part node being resized by a sash, route it through that part's `layout(width, ...)` callback, which receives the in-progress node width.
- **Docked detail collapse must use the raw sash width before clamping**: the docked auxiliary bar keeps a minimum visible width, so checking the clamped width can never detect a drag-to-zero collapse. Decide collapse from the raw requested sash width, then route the hide through `setPartHidden(AUXILIARYBAR_PART)` so context keys and per-session capture stay in sync.
- **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.
- **Provider-owned model/mode selection belongs in the loaded chat model, with draft persistence driven by debounce**: For AHP-backed chats, `setModel` / `setAgent` must push the selection into the loaded `IChatModel.inputModel` (like `_updateChatSessionState`) and let the draft-sync debounce emit `chat/draftChanged`. Do not immediately dispatch a model/agent-only draft from the provider, because it can overwrite unsaved typed text before the debounced full input-state draft is persisted.
- **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. Do **not** bound the upgrade with a timeout or even a lifecycle milestone like `LifecyclePhase.Eventually` — an agent host connects lazily and can surface its session types arbitrarily late, which would lock in the wrong fallback. Let the upgrade listener live for the consumer's lifetime instead.
- **Over-commenting**: Don't write long explanatory comments narrating what the code does or justifying ordinary patterns. **Hard rules**: JSDoc = 1–2 short sentences max (never enumerate every branch/feature, restate the signature, or list what the function does NOT do); inline method comments = 1 line max, only for a genuine workaround/non-obvious constraint, never to narrate the next statement. Default to no comment — if code needs a paragraph to explain, rename/extract instead. Before writing any comment longer than one line, delete it or shorten it to one line.
- **Inserting/removing DOM on demand for transient UI (e.g. inline rename inputs)**: Don't `insertBefore`/`appendChild`+`remove()` a widget on the *tab/row element itself* when an interaction starts/ends — that churns the parent's child list and depends on event ordering during teardown. Also don't eagerly build a heavy widget (e.g. an `InputBox`) per row "just in case", since most rows never use it. Instead, create a **stable, empty container** alongside the label once, toggle its visibility via a CSS class on the row (e.g. `.editing`), and create the widget **inside that container lazily** only while editing — disposing it and emptying the container (`reset(container)`) when done (`InputBox.dispose()` does not detach its own node). Prefer the shared themed widget (`InputBox` + `defaultInputBoxStyles`) over a hand-rolled `<input>`.
- **Collapsing distinct provider identities in pickers**: Do not collapse extension-backed chat session ids (e.g. `copilotcli`) and agent-host ids (e.g. `agent-host-copilotcli`) based only on friendly names or well-known provider enums. They can coexist in the Agents window and route to different infrastructure; keep the exact session type id through selection/delegation and hide ambiguous legacy targets when an agent-host target supersedes them.
- **Permission picker copy must stay provider-neutral and aligned**: Reuse the same labels and descriptions across Copilot Chat and Agent Host permission pickers. Avoid provider-specific phrasing such as "Copilot uses..." when the same choice appears in the Agents window.
- **Interactive tool denial must use the SDK's interactive-denial result, not a hard reject**: In the Agent Host Copilot provider, a user choosing **Skip** must resolve the permission request with `denied-interactively-by-user` so the SDK feeds the denial back to the model and continues the turn. Reserve `reject` for abort, disposal, or requests that cannot be presented to the user; returning it for a renderer decision terminates the session turn.
- **Internal Agents workspace in recent history**: Collapse every internal `User/agent-sessions.code-workspace` variant into one canonical **Agents Window** entry. Recognize the reserved path across profile and worktree user-data directories, and make the single picker entry point at the current environment's Agents workspace.
- **Resolving a session's provider via the create-only tracking map**: On the agent host, resolve the owning provider for any per-session operation (createChat, disposeChat, sendMessage, …) through `AgentService._findProviderForSession`, never the raw `_sessionToProvider` map. That map is populated only by `createSession`, so a **restored** session (alive in the state manager after a host restart but never created in this process) is absent from it — a direct lookup throws `no provider for session` and silently breaks the feature (e.g. Add Chat did nothing for restored sessions while messaging worked, because messaging already used the fallback). `_findProviderForSession` falls back to the session URI's scheme provider, which is what makes restored sessions work.
- **Dispatching per-chat side-channel actions (agent/model) to the session URI**: An agent-host session can own multiple peer chats, each with its own backend conversation (`CopilotAgent._chatSessions`). Conversation side-channel actions like `SessionAgentChanged`/`SessionModelChanged` must be dispatched to the per-chat **turn channel** (`_resolveTurnDispatchChannel`, which carries a `chatId` fragment for peer chats), not `session.toString()`. The session URI resolves to the session's *default* chat (`_sessions`), so dispatching there silently applies the change to the wrong conversation and an additional chat never sees the agent/model swap. The host must also forward the `chatChannel` through `agentSideEffects.handleAction` → `changeAgent`/`changeModel`, which apply it to `_chatSessions` when present. The protocol models `summary.agent`/`summary.model` at session level only, so equality guards comparing against session summary are valid for the default chat but must be skipped for peer chats.
- **Do not infer or fall back from a peer chat channel after progress was emitted**: Agent progress signals for chat-scoped actions, especially tool-call readiness and permission requests, must be emitted with the exact `ahp-chat://...` channel that owns the tool. Do not recover by scanning active turns, remapping `ChatToolCallConfirmed`, or using `parseDefaultChatUri(...) ?? sessionUri` in `AgentSideEffects`; malformed/misrouted chat channels should fail loudly so the producer or dispatch path is fixed. `handleToolCallConfirmed` and `_toolCallAgents` must use the chat channel URI containing the tool call; keying by the parent session URI makes confirmations miss the pending SDK request.
- **Do not synthesize default chat URIs in the workbench handler**: `AgentHostSessionHandler` must source the upstream default chat URI from hydrated `SessionState.defaultChat` / `SessionState.chats` and store that mapping in its chat-resource-to-upstream-URI map. Calling `buildDefaultChatUri(session)` in the handler assumes one server URI shape and hides protocol/provider bugs; dispatch turn lifecycle and pending/input actions through the mapped upstream chat URI instead.
- **Opening a subagent editor must carry the exact upstream chat channel**: A fragment-only editor resource forces `AgentHostSessionHandler` to rediscover the child in `SessionState.chats`, which races catalog hydration and renders "Cannot resolve chat". Encode the exact `ahp-chat://subagent/...` channel in an internal query parameter, validate its chat id and owning backend session, and subscribe directly.
- **Model subagents as chats, not sessions**: A subagent spawned from a tool call belongs to the parent session as an additional chat with `origin.kind === "tool"`, hidden from the chat tab strip. Do not call `restoreSession` for subagents; that creates `_sessionStates` without a matching `_chatStates` entry, so later chat actions hit "Action for unknown chat". Add a chat on the parent session and dispatch the subagent turn to that chat URI.
- **Keep case-sensitive ids out of URI authority**: URI authorities are case-insensitive, so do not place tool call ids in the `ahp-chat` authority. Subagent chat URIs use a stable `subagent` authority and put the encoded tool call id in the path; use `buildSubagentChatUri(...)` instead of `buildChatUri(..., \`subagent-${toolCallId}\`)`.
- **Selected custom agent must be in the SDK's `customAgents`, not just `pluginDirectories`**: The Copilot SDK validates the session-start `agent:` option (passed to `createSession`/`resumeSession`) against the `customAgents` list **by name only** — it does NOT consult `pluginDirectories`. `copilotSessionLauncher._buildSessionConfig` deliberately omits agents from file-dir plugins from `customAgents` (relying on the SDK's `pluginDirectories` discovery to avoid duplicates), so selecting a plugin/extension-contributed agent (e.g. "Inbox") otherwise fails with `Custom agent '<name>' not found`. The fix (`toSdkSessionCustomAgents`) force-adds the resolved selected agent into `customAgents` while every other file-dir agent still loads via `pluginDirectories`. Note the agent picker offers VS Code chat modes from `IChatModeService`, but only `plugin`/`extension` storage agents are synced to the host (`SYNCABLE_STORAGE_SOURCES`); `user`/`local` agents are never synced, so `_resolveAgentName` returns `undefined` for them and no `agent:` is sent.
- **Derive SDK custom-agent names exactly like `parseAgentFile`**: `_resolveAgentName` resolves the selected agent through the plugin parser, which trims the frontmatter `name` (`getStringValue('name')?.trim() || nameFromFile`). When building the SDK `customAgents` list (`toSdkCustomAgents`), derive the name the same way (`?.trim() || agent.name`); reading the raw frontmatter `name` without trimming yields a config name that won't match the trimmed `resolvedAgentName`, so the SDK still rejects the session with `Custom agent '<name>' not found`.
- **Peer chats have no server `summary`, so dedup side-channel dispatch against the last value sent for that chat**: equality guards before dispatching `SessionModelChanged`/`SessionAgentChanged` compare against `summary.model`/`summary.agent`, which only exist for the session's default chat. For peer chats, track the last-dispatched model/agent on the `AgentHostChatSession` instance (auto-cleaned on dispose) and diff against that — otherwise every peer-chat turn redundantly re-dispatches (and re-resolves the agent), and an intentional "clear selection" (`undefined`) can't be detected.
- **Scrollable transcript surfaces must use workbench scrollbars**: Don't make Agents/voice transcript regions scrollable with native `overflow-y: auto` on the content node. Wrap transcript content in `DomScrollableElement`/list widgets so scrollbars match VS Code theming and remain usable in narrow auxiliary-window layouts.
- **Background-sending a multi-chat composer must reset the composer *before* dispatching the send, not concurrently**: in `NewChatInSessionWidget._send`, creating the replacement untitled chat (`openNewChatInSession({ forceNew: true })` → `provider.createNewChat`) and the fire-and-forget background `sendRequest` both reach into shared chat-session state (`acquireOrLoadSession` / `getOrCreateChatSession`) for chats in the **same group**. Running them concurrently (send first, reset second) raced and left the sent chat stuck spinning with its message never dispatched, plus a second empty "New Chat" tab. Fully `await` the composer reset first, then fire the background send so it runs on its own.
- **Chat tab order is the provider's stable creation order; don't reorder in the renderer**: the agent host delivers `state.chats` in stable creation order (append on add, replace-in-place on update — see `agentHostStateManager`/the session reducer), and a genuinely new chat is appended last. The renderer's rebuild autorun (`chatCompositeBar.ts`) must render that order as-is. Do **not** partition/move in-composer `Untitled` chats to the end: a draft is already last, and reordering by status makes a tab jump when a draft commits out of creation order (e.g. sending the 3rd of three drafts first moved it to the front). A chat's `Untitled` presentation (via `AdditionalChat._isNew`, needed so `sessionView.ts` shows the composer) is independent of tab order and must not drive it. Also note `_restorePeerChats` (`agentService.ts`) must seed restored chats in `getChats()` order, not in `Promise.all` resolution order, or the catalog scrambles on reload.
- **A new chat must report `SessionStatus.Untitled` until its first request is sent, regardless of how the provider creates it**: `sessionView.ts` only shows the new-chat composer (which owns the Alt+Enter background-send handler) when `activeChat.status === Untitled`. The agent host commits a new peer chat *eagerly*, so its host status is `Completed` — surfacing the standard chat widget and breaking background send. Gate the chat's presented status on a provider-side `isNew` flag (`AdditionalChat.markNew`/`markSent`, set in `createNewChat` and cleared in `sendRequest`'s committed-chat branch), not on the host-reported status.
- **Service operations should return a result or throw, not `undefined` for unsupported cases**: capability-gated operations like `forkChatInSession` must throw when the provider/session cannot perform them. Keep fallback decisions in the caller before invoking the service instead of encoding fallback as an `undefined` service result.
- **A provisional session abandoned during commit detection must not be returned as successful**: its status can remain `InProgress` after its lifecycle owner is disposed, so consumers waiting for a terminal status never settle. Clean up the provisional session and reject the send when commit detection times out or the connection is lost.
- **Drop a fork when its turn point is unknown, don't forward it empty**: in `AgentService.createChat`/`createSession`, if the requested fork `turnId`/`turnIndex` resolves to no source turns, set `fork: undefined` and fall through to a fresh create. Forwarding the fork with an empty turn slice makes the Copilot provider call `sessions.fork` with no `toEventId`, inheriting the entire backend conversation while the new chat UI is seeded with zero turns — an inconsistent hidden-history chat.
- **Side-chat context belongs to the provider, not AgentService message mutation**: `AgentService` records and forwards the side-chat origin, but must not synthesize a first-turn `Chat` attachment or strip provider-added context on restore. Each supporting provider establishes hidden backend context and removes inherited/provider-added history from the turns it returns.
- **`MessageAttachmentKind.Chat` is generic and may reference unloaded chats**: resolve chat attachments through a generic async path, enforce same-session ownership before hydration, and restore the referenced session/chat when it is absent from the state manager. Do not name this logic after side chats or assume `/btw` is its only producer.
- **Agent capabilities are provider-specific**: do not implement side chats for an agent merely because the protocol supports them. Advertise `multipleChats.sideChat` and run shared side-chat tests only for providers with a complete provider-owned context/restore implementation.
- **User-created side chats use the standard peer-chat tab model; only tool-origin subagents stay hidden by default**: a `ChatOriginKind.SideChat` chat is a normal user-facing peer chat, so it must flow through `visibleChatTabs`, the Conversations menu, pickers, and close/reopen like any other peer chat. Do not create a separate editor/detail surface for it; reserve the hidden/read-only default only for tool-origin subagents.
- **`/btw` must bypass queue/steer, may anchor to `activeTurn`, and should activate the new peer chat through the normal sessions API**: mark the silent slash command `executeDuringRequest` so the chat widget invokes it independently, validate its anchor against completed turns or the current active turn, and after creating the side chat activate it via `ISessionsService.openChat(...)` before sending on that chat. Provider side-chat creation must lock on the new chat, not the source send key. Wrap the first provider prompt with a succinct instruction to prefer explanation over action and avoid work unless explicitly requested; include bounded user-visible active-turn markdown when native forks omit it, then strip the private wrapper from reconstructed visible history. Never inject reasoning or tool payloads.
- **A responsive-layout autorun must re-baseline (not react) to controller-driven restores, holding the flag across the async reveal**: the desktop [D7] responsive sidebar hides the sessions sidebar when small + editor + aux-bar are all open. Switching sessions restores layout via two async paths — the desktop aux-bar restore (`openView`/`openViewContainer`) **and** the base controller's editor working-set apply (`_applyWorkingSet`, which reveals the editor part *after* an `await` and runs on a `Sequencer` microtask). Both reveal parts in a *later* autorun run, so an inline "same-run session changed" check only absorbs the synchronous transition and the async reveal still auto-hid the sidebar on navigation. Fix: a shared base-controller `_withSessionLayoutRestore(work)` epoch wraps **both** restore paths (the working-set wrap is the critical one for non-modal editors); the D7 autorun re-baselines `_previousSpaceConstrained` while `_isRestoringSessionLayout` is true. Also gate the constrained derivation on `!multipleSessionsVisibleObs` so the feature is disabled with multiple sessions visible. Never use a `setTimeout` to bridge the async reveal — tie the flag to the actual promise.
- **A promise-tied "epoch" helper must decrement synchronously for void/sync work, only deferring for real Promises**: `_withSessionLayoutRestore` increments a depth counter, runs `work()`, and decrements when done. If it *always* schedules the decrement on a microtask (`Promise.resolve(result).finally(...)`) — even when `work()` returns `undefined` (the common no-op restore, e.g. a session with no workspace) — the depth stays elevated for the entire synchronous caller/test body, so `_isRestoringSessionLayout` reads `true` forever and the consumer (D7) silently stops acting. Only defer the decrement when `work()` returns a thenable; for void/sync (or throwing) work, decrement in the `finally`.
- **A quick-chat's workspace-less kind is seeded at adapter construction and only ever promoted — every path that can carry `_meta` must carry it**: `AgentHostSessionAdapter` seeds its session-kind (`QuickChatSessionKind` vs `WorkspaceSessionKind`) from `readSessionWorkspaceless(metadata._meta)` in the constructor, and `_promoteToQuickChatIfWorkspaceless` (from `update()`/`setMeta()`) later flips it to a quick chat the first time an authoritative `_meta` says workspace-less — never back, since an absent marker means "not included", not "cleared". So the `_meta.workspaceless` tag must ride on **every** metadata path: `_refreshSessions()`/`listSessions`, the live `_handleSessionAdded(summary)` notification, **and both ends of the AHP `root/listSessions` round-trip** (`protocolServerHandler.ts` and `remoteAgentHostProtocolClient.ts` build their wire items field-by-field, and `satisfies SessionSummary` does **not** catch a dropped *optional* field — this is exactly how the bug shipped twice). Dropping it makes a committed quick chat render under a section header labelled with its raw session UUID, leaking `<userHome>/.copilot/chats/<id>` as a `workspace` (breaking the archive-on-delete fallback, list badges, changes/files) until a later `_meta` heals it. `_persistCache` must overlay the adapter's **live** `isQuickChat` rather than the `_metaByRawId` snapshot, or the next `updateAdapter` silently strips the marker from the startup cache. On the host, `AgentService.listSessions()` overlays `_meta.workspaceless` onto the provider listing from the persisted `agentHost.workspaceless` session-database key (`AH_META_WORKSPACELESS_DB_KEY`) (the providers themselves, e.g. `CopilotAgent.listSessions()`, do not emit it) so restored sessions carry the tag once the state-manager live summary is gone. The host still keeps the tag on both the summary `_meta` and `SessionState._meta` (`createSessionState(summary)` copies it) so the channels stay consistent. **Note the provider tests cannot catch a wire-level drop** — `MockAgentHostService.listSessions` returns stored `IAgentSessionMetadata` verbatim, bypassing both mappers — so wire regressions need tests at the protocol layer.
- **Don't infer "quick chat" from `workspace === undefined`**: a session's workspace observable is `undefined` for genuine quick chats but also transiently/edge-case for workspace-bound sessions, so keying quick-chat UI (context keys, list grouping) on it is imprecise. Expose the intent explicitly via the optional `ISession.isQuickChat: IObservable<boolean>` (only quick-chat-capable providers set it; absent ⇒ `false`). The agent-host adapter exposes it as a monotonic observable seeded from `readSessionWorkspaceless(metadata._meta)` at construction and promoted by later authoritative `_meta`; non-quick-chat providers omit it. Consume it through `isQuickChatSession(session)` / `session.isQuickChat?.read(reader) ?? false`.
- **Workspace-less is inferred from absent `workingDirectory` — exclude forks from that inference**: in `CopilotAgent.createSession`, `isWorkspaceless` must be `!sessionConfig.fork && !sessionConfig.workingDirectory`. A fork that arrives without an explicit `workingDirectory` should inherit the source session's context, not be tagged `agentHost.workspaceless` and dropped into a scratch dir + quick-chat system prompt.
- **On a failed quick-chat create, don't activate an unrelated draft**: `SessionsService.openQuickChat` must, on `createQuickChat` throwing, log and return `undefined` without falling back to `_activate(newSession.get())` — that observable can hold a workspace-bound draft from a different call site, so activating it is surprising. Return the activated `IActiveSession` from `_activate`/`openQuickChat` (setActive is synchronous and yields the wrapper) and have the caller focus that exact value, rather than re-reading `activeSession.get()` afterwards.
- **Hide an aux-bar view CONTAINER via `hideIfEmpty: true` + a view `when`, not a container `when`**: `IViewContainerDescriptor` has **no `when`** property (only `IViewDescriptor` does). To conditionally hide a whole container (its tab/title) — e.g. the Agents-window Changes/Files containers for workspace-less sessions — put the context-key `when` on the inner view(s) and set `hideIfEmpty: true` on the container. Container visibility is `hideIfEmpty && activeViewDescriptors.length === 0` (`viewsService.updateViewContainerEnablementContextKey`), and `activeViewDescriptors` already respects each view's `when`, so the container hides reactively when all its views' `when` go false.
- **Don't gate "is this a quick chat?" routing on `isCreated && isQuickChat`**: a quick-chat **draft** is `Untitled`, so `isCreated` (`= status !== Untitled`, `visibleSessions.ts`) is `false` — yet it is still a quick chat. Gating quick-chat routing on `isCreated && isQuickChat` (e.g. the "New" action) makes a draft fall through to the workspace path (scratch-dir composer, no session-type picker, "No models available"). Route on **`isQuickChat` alone** so drafts and committed quick chats behave identically; use `isCreated` only when you genuinely need to distinguish a committed session from an in-composer draft.
- **Cmd+N in the Agents window is a new-**session** gesture only — don't fold quick-chat/peer-chat creation into it**: session creation (**Cmd+N**, `NewChatInSessionsWindowAction`/`workbench.action.sessions.newChat` → always `openNewSession`), quick-chat creation (Chats-section **"+"**, **Cmd+K Cmd+N**, `NewQuickChatAction` → `openQuickChat`), and peer-chat creation (chat **"+"**, **Cmd+T**, `AddChatToSessionAction`) are three **distinct keybindable actions**. Keep them separate — Cmd+N must not become context-aware/mirror-route to a quick chat based on the active session's kind.
- **The New action must never inherit a quick chat's folder into the workspace composer**: `openNewSessionFromActive` seeds the new-session composer with `activeSession.workspace.get()?.uri`. A quick chat is workspace-less by intent, but if its `workspace` observable is ever non-undefined (e.g. a stale-build/coupling leak where `_kind` resolved to `WorkspaceSessionKind` because `_meta.workspaceless` was dropped, exposing the host's scratch `~/.copilot/chats/<id>` cwd as a workspace), Cmd+N would carry that scratch dir into `createNewSession` → "New session in `<scratch>`", single/no session type, no picker, "No models available". Gate the folder inheritance on `activeSession.isQuickChat` so a quick chat **always** falls to the clean folder-picker composer regardless of any leaked workspace value.
- **A reused new-session composer must re-seed its workspace draft when it swaps out of quick-chat mode**: the session-type picker hides itself when it has no folder types (`sessionTypePicker` `_folderSessionTypes.length === 0`), which is the case whenever the composer has **no active session** (`refresh(undefined)` clears the types). A *freshly opened* new-session composer avoids this by seeding a workspace draft from the restored folder in its constructor — but the same `NewChatWidget` instance is **reused** across the quick-chat→new-session transition (`sessionView.ts` keeps `kind==='newSession'`), and Cmd+N's `openNewSession` discard branch only `_activate(undefined)`, leaving the reused composer session-less → picker hidden. Fix by re-running the constructor's seed (`_seedWorkspaceDraft()`) from an autorun when `_isQuickChatComposer` flips **true→false with no active session**, so the reused composer matches a fresh one (folder + visible picker). Don't assume the constructor-time restore covers a reused composer.
- **Every untitled-session-title fallback must be quick-chat aware**: an untitled session's title observable is `''`, so a hardcoded `localize(…, "New Session")` fallback shows "New Session" even for a quick chat (whose composer says "New Chat"). Route **all** such fallbacks through the shared `getUntitledSessionTitle(isQuickChat)` helper (`services/sessions/common/session.ts`, boolean param so each caller controls reader-tracked `.read(reader)` vs `.get()`). There are ≥5 sites — titlebar (`sessionsTitleBarWidget`), session header (×2: title + rename placeholder), list-row hover (`sessionHoverContent`), sessions picker (`sessionsActions`) — keep them on the helper; never hardcode "New Session". (The Cmd+N *action* title stays "New Session" — that action creates a session, unrelated to a session's own title.)
- **`NeedsInput` is still an active turn for live turn UI**: agent-host tool and input confirmations intentionally transition a running chat from `InProgress` to `NeedsInput` without ending `activeTurn`. Live status surfaces such as the chat input pills must use `isActiveSessionStatus` so they do not disappear until the next output returns the chat to `InProgress`.
- **Chat file pills must not hardcode an editor**: open their resource through the shared `chat.editorAssociations` resolution path so both completed-response pills and the session input toolbar respect the configured editor. Do not invoke `markdown.showPreview` directly.
- **Agent-host-only exclusions for built-in client tools belong in `ClientToolSetsContribution`, not the global tool registration**: `AgentHostActiveClientService.getClientTools` advertises enabled members of every non-deprecated tool set, including extension-contributed sets. Omit an unsupported built-in tool from the client tool sets so normal Copilot chat can continue using it; do not treat this contribution as the sole Agent Host allowlist.
- **Non-interactive MCP authentication probes must not create dynamic authentication providers**: Provider creation can prompt for manual client registration when dynamic registration is unsupported. With `allowInteraction: false`, only inspect existing providers and sessions; defer metadata discovery and provider creation until the user invokes the `mcpAuthenticationRequired` action.
- **Use structured maps for the state that is actually multi-keyed, not for an incidental cache**: If MCP tracking is addressed by session + server, model that source of truth directly with `NKeyMap`. Do not add a separate `NKeyMap` that merely caches serialized storage keys while leaving the real tracking state in nested or synchronized maps.
- **Subagent activity rows must preserve rich tool presentation, stable height, tool identity, and protocol intent**: Do not flatten markdown invocation messages into text, omit the shared tool icon, or show a raw terminal command when `ToolCallBase.intention` exists. Render invocation markdown with the shared chat/file-widget path and the registered/inferred compact tool icon, keep it constrained to one line within a static minimum-height slot so text, code, and file chips do not shift surrounding content, and use terminal intention before invocation-message fallback.
## 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 1–3 sentences: the anti-pattern, why it is wrong, and the preferred pattern.
- **Shared commands must delegate behavior to the layout service, not inspect a layout implementation**: `workbench.action.toggleAuxiliaryBar` must call the semantic `IWorkbenchLayoutService.toggleSecondarySideBar()` operation. Do not branch on optional layout properties or concrete workbench shape in the shared action; each workbench owns how its secondary-sidebar affordance maps to visible parts.
- **Definitive session deletion and temporary list eviction are different operations**: deletion clears durable provenance and pending state; filtering a still-existing session only removes its visible list entry. Keep the list-removal helper side-effect-free, and let each caller explicitly update its mutation generation instead of passing an "already incremented" boolean.
- **Durable user intent must never be discarded on `onDidChangeSessions.removed`**: pins, manual sort keys, and group membership (`SessionsListModelService`, `SessionGroupsService`) are cleared only on `ISessionsManagementService.onDidDeleteSession` (or archive), never on the provider's `removed` delta. `removed` is an *eviction*, not a deletion: `BaseAgentHostSessionsProvider._refreshSessions` reconciles against one listing that the host aggregates across all its agents, and an agent that cannot answer yet returns `[]` instead of failing (`CodexAgent.listSessions` returns `[]` for a missing `_githubToken`, a not-yet-downloaded SDK, or a failed `thread/list`; `ClaudeAgent.listSessions` does the same). Persisting the removal turned a ~300 ms startup race into permanent loss of the user's pins and groups. Runtime-only consumers of `removed` (terminals, grid slots, layout) are fine as-is — only *persisted* state needs the delete event.
- **`_refreshSessions` must not evict a cached session whose agent contributed no rows**: a listing with zero rows for an agent means "unknown", not "empty", so scope eviction to `listedAgentProviders` (the set of `AgentSession.provider(...)` schemes actually present in the response) and compare against `adapter.agentProvider`. Real deletions still arrive through `deleteSessions` and the `sessionRemoved` notification; the only cost is that an agent's *last* session, deleted elsewhere, lingers until it lists something again.
- **Keep session-list refresh filtering linear**: when retention pruning needs the complete backend key set, collect those keys while filtering entries in the original loop, then reconcile last-seen/pruning afterward. Do not introduce a candidate-map/filter/map pipeline when one loop plus one reconciliation call expresses the lifecycle more clearly.
- **Centralize session workspace filtering behind a semantic predicate**: refresh, add-notification, and summary-update paths should call one `_isSessionInWorkspace(entry)`-style helper. Keep key construction, working-directory parsing, pending-local lookup, and provenance checks out of each caller so the high-level list flow stays readable and all paths apply identical rules.
- **Feature-specific workspace storage must be gated at the storage service boundary**: Editor multi-root provenance is enabled only for a non-Sessions window with `WorkbenchState.WORKSPACE` and more than one open folder. Lazy-load it only after that gate passes; folder/empty/Agents windows must use ordinary path filtering and never read, write, refresh, or delete the persisted state.
- **Keep legacy single-folder filtering explicit when adding multi-root provenance**: zero-folder windows still include all sessions, and one-folder windows still use direct any-directory path containment. Call the provenance service only when the window has multiple folders; keep its internal scope gate as defense-in-depth.
- **Name semantic layout operations after the user-facing surface**: a shared operation must use the stable UI concept (`toggleSecondarySideBar()`), not the implementation term (`AuxiliaryBar`) that happens to back it in classic layouts. This keeps single-pane mappings clear and avoids leaking layout internals through the API.
- **Semantic layout commands need matching visibility and focus semantics**: when a shared command maps to a different surface, expose a semantic visibility query for its toggled state and transfer focus before hiding the currently focused mapped surface. Otherwise menu labels lie about the action and keyboard users retain focus in hidden content.
- **Expose a mapped visible surface through a named public operation**: when a shared layout operation maps to a different surface in the single-pane workbench, delegate through a public method named for that surface (for example, `toggleEditorPane()`), rather than embedding its implementation in the unrelated override. Use the surface's established user-facing terminology in accessibility announcements.
- **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. 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 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).
- **The managed Files placeholder tab is conditional, not always-on**: `SinglePaneManagedTabsStrategy` opens the empty Files placeholder (with the Changes tab) only when the editor group is empty on a view-open trigger (see the "add only when the group is empty" pitfall below), and removes it while a real (non-managed) file/diff is open in a visible editor area. It is **not** re-added when that file closes — the defaults return only when the group empties and the side pane is reopened. Drive the removal as a **one-shot reaction** on `onWillOpenEditor` (a genuinely new real-file open — skip re-activations where `group.contains(editor)` is already true, non-main-part groups, and restore-driven opens), *not* a standing `autorun`/editor-change invariant (that would re-remove a Files tab the user just added via `+`). 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 stay user-closable via "add only when the group is empty" — not a dismissal set**: `SinglePaneManagedTabsStrategy` owns the managed Changes/Files docked tabs. They re-ensure on many signals (session state, editor visibility, editor changes), so naively re-creating them makes a close feel un-closable (they are non-preview `pinEditor`, NOT sticky — they *do* have close buttons; the blocker is the re-ensure). The clean rule that needs **no** `_dismissedManagedTabs` bookkeeping: **open the default tabs only when the editor group is completely empty (`group.editors.length === 0`), and only on a "view opened" trigger** — a session switch (the add-allowed session-state autorun) or the layout service's `onDidRevealSidePane` event (fired by the workbench whenever the docked editor part and/or aux-bar detail transitions from *fully hidden* to visible). A plain editor-list / visibility change reconciles (e.g. removes the Files placeholder while a real file is open) but is **add-disallowed**. Why this is close-respecting for free: closing one managed tab while another (or a real file) remains leaves the group non-empty → not re-added; closing the last one closes the side pane → reopening it (empty group) restores the defaults. **Opening a file** fires `onDidRevealSidePane` too, but the sync is deferred on the docked-tab sequencer (which runs after `onWillOpenEditor` has added the editor), so the group is non-empty when it runs → defaults are not forced back → closing that file still closes the side pane. The add-disallowed editor-change trigger is essential: without it, closing the last tab (group empty) would immediately re-add the defaults and the pane could never close. **The layout-driven add is done on the *settled* restore, not during it**: the base controller fires `onDidEndSessionLayoutRestore` when the restore depth returns to 0 (after the — possibly async — working-set apply completes), exposed via `ISinglePaneLayoutContext`; the strategy reconciles off that ([Trigger D], `openDefaultsIfEmpty: true`). This is required for a **new session**, whose *empty* working set closes the previous session's docked tabs *after* the switch — reading the group *during* the async apply (an editor-change trigger) races the empty state and drops the Files tab; reconciling on the settled restore-end reads the reliably-empty group. Do **not** gate the add on `isRestoringSessionLayout` captured in the editor-change autorun — that fires mid-apply and is fragile. A *user* file-open/close is not a restore, so it stays add-disallowed and a close still sticks. **One exception — new-session submit**: when the active session transitions `isCreated` false → true (in place, or via a resource-replace commit), the new-session view already holds the Files placeholder, so the empty-group rule would skip opening Changes; the submit transition is treated as a one-shot "ensure the Changes tab (pinned first, **opened active**)" even when the group is non-empty — opening it *active* (not `inactive`) is what makes the detail panel map to the Changes container rather than the still-present Files placeholder; it is a genuine one-time transition, so it never fights a later user close. **Because submit fires two triggers** (the session-state autorun's `ensureChangesActive` **and**, via the submit restore, `onDidEndSessionLayoutRestore`'s Trigger D), a single shared generation counter would let the later trigger's reconcile supersede and drop the earlier's intent — so the triggers' intents are **accumulated** (`mergeTriggers`, OR-combined into a pending trigger consumed by the surviving reconcile, re-merged in `finally` if superseded mid-run) rather than replaced. **Scope the pending intents to the session they were queued for** (`IPendingReconcile.sessionKey` = the active session resource): a reconcile can be superseded mid-`await` (e.g. it stalls opening the Changes editor) by a **session switch**; if the superseded reconcile's `finally` merged its old trigger back **unconditionally**, an `ensureChangesActive`/`ensureAllInputs` intent for session A would leak onto session B and reopen a user-closed tab or activate Changes for the wrong session. Merge back (and accumulate on queue) **only when the successor targets the same `sessionKey`**; a session switch drops the previous session's stale intents. **Second exception — a details-only reveal**: when `onDidRevealSidePane` fires with the aux-bar detail panel visible but the editor area hidden (`isVisible(AUXILIARYBAR_PART) && !isVisible(EDITOR_PART)`), the docked details panel *shows* the managed docked inputs, so they are ensured (Changes if created + Files) **even when the group is non-empty** — restoring one the user had closed earlier. This is tied to the reveal gesture (a close *within* an open details view still sticks until the next reveal); an editor-included reveal keeps the strict empty-group rule. **Do NOT** re-introduce a `_dismissedManagedTabs` set, an `onDidCloseEditor` dismissal listener, infer the reopen from aux-bar visibility, or gate on a generic "side pane became visible (`editor || aux`)" check. The empty Files placeholder is tidied away when a real workspace file **opens** — a **one-shot** reaction on `onWillOpenEditor` (a real `file`/`vscode-remote` input, skipped during a restore), *not* a standing "no placeholder while a real file is open" invariant enforced every reconcile. The standing invariant broke `+` Files: adding the placeholder while a file was open re-triggered the reconcile which immediately removed it again. Because `+` Files opens an `EmptyFileEditorInput` (not a real file), the one-shot listener ignores it, so a user-added Files tab survives while a real file is open (a tidy `[Changes][file]` strip still results from a real-file open).
- **Editor-area collapse (closing non-docked tabs) fires only on a *detail-only* hide, never when the whole side pane closes**: `SinglePaneEditorAreaCollapseStrategy` reacts to the editor part hiding by closing every non-docked editor (capturing reopenable ones, dropping non-restorable ones). It must gate that on the **aux bar still being visible** (`isVisible(AUXILIARYBAR_PART)`): a *Detail-only* hide (Hide Editor keeps the detail) collapses the editors, but closing the **whole side pane** (both editor + aux hidden) must leave the editors intact so they return when the pane is reopened. The gate is reliable because the two hide paths order their `setPartHidden` calls consistently — `toggleSidePane` hides the **aux bar before the editor**, so when the editor-hidden event fires the aux is already hidden (⇒ skip collapse); `Hide Editor` sets the aux visible *before* hiding the editor (⇒ aux visible ⇒ collapse). Don't collapse purely off "editor part hidden" — that also dropped dirty/non-restorable editors when the user just closed the side pane.
- **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`).
- **Don't mirror an environment fact onto a shared widget API**: an Agents-window check (e.g. "is this the sessions window?") belongs on `IWorkbenchEnvironmentService.isSessionsWindow`, read directly by the consumer that needs it (e.g. `getChatAccessibilityHelpProvider` in `chatAccessibilityHelp.ts` injecting the service itself). Do not add an `isSessionsWindow`-style property to a shared interface like `IChatWidget` just to thread that fact through — it leaks a sessions-specific concept into shared workbench chat surfaces and every future consumer would need the same plumbing instead of injecting the service once.
- **A view-lifecycle `setChat`/`setModel`-style hook can be re-invoked for the *same* underlying resource**: `ChatView.setChat` fires again on unrelated status/interactivity observable changes, not only on a genuine view swap. A consumer like `ResponseSelectionSideChatController` that force-dismisses its own transient UI on every call discards an in-progress draft and, worse, clears a pending busy submission mid-flight, letting a duplicate submission race in. Compare the incoming resource against the previously tracked one and only treat a genuine change (or the first call) as dismiss-worthy; a same-resource re-invocation must preserve visible/busy state.
- **A pending async submission's completion/error handler must no-op after a genuine force-dismiss, not just after a same-resource re-invocation**: even with the same-resource guard above, `ResponseSelectionSideChatController._submit`'s `createAndSendSideChat().then()/.catch()` can still settle *after* the user has genuinely navigated away (a different-resource `setChat`, or any other force-dismiss) — reopening the overlay, restoring the typed query, refocusing the input, or showing a stale error notification for UI the user already dismissed. Capture a `_generation` counter bumped only on a genuine force-dismiss, snapshot it before the async call, and have the settle handlers bail when the counter no longer matches; don't rely solely on resource comparison, since the overlay's own dismissed state (not the chat identity) is what must gate the mutation.
## Validating Changes
You **must** run these checks before declaring work complete:
1. `npm run typecheck-client` — TypeScript compilation check. **Do not run `tsc` directly.**
2. `npm run valid-layers-check` — **MANDATORY.** Catches layering violations. If this fails, fix the imports before proceeding.
3. `scripts/test.sh --grep <pattern>` — unit tests for affected areas
## Progress dashboards / local HTML in the integrated browser
- The integrated browser blocks `file://` outside trusted workspace folders (403 "File does not reside within a trusted folder"). To preview session-folder HTML (e.g. a live progress dashboard), serve it over `http://127.0.0.1:<port>` with a background `python3 -m http.server` and open that URL instead.
- For a self-updating dashboard, embed `<meta http-equiv="refresh" content="5">` and keep the HTML task data in lockstep with the todo store — update both at every task transition so they never drift.
## Don't rely on the LLM to render links/actions from tool result text
- Tool result text is fed to the model, which may drop or reformat markdown links (e.g. render a session URI as an inline code span), so an explicit `[label](uri)` in a tool result is NOT a reliable way to give the user a clickable action.
- For a deterministic, client-rendered action (a "pill"/button) tied to a specific tool call, set `toolSpecificData` on the `ChatToolInvocation` in `stateToProgressAdapter.ts` (keyed on the tool name + parsed result) and add a custom subpart in `chatToolInvocationPart.ts` — the completed-state section already routes custom `toolSpecificData` kinds (see `resources`/`simpleToolInvocation`). Follow the `agentFeedbackReviewConfirmation` pattern.
- **Managed-tab reconciliation must run entirely under `suppressEditorPartAutoVisibility()`**: `SinglePaneManagedTabsStrategy._reconcileCore` closes stale/foreign managed tabs (e.g. a restored Changes tab whose session's workspace hasn't resolved yet on reload) via `_closeForeignChangesEditors` before ensuring the current ones. If any close 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. (Historical note: the managed Files placeholder was once removed/re-added reactively with a `_dismissedManagedTabs` set; that bookkeeping is gone — see the "add only when the group is empty" pitfall.)
- **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*. The workbench `handleDidCloseEditor` docked branch would otherwise see the empty group and close the whole side pane (reload/Cmd+N flicker: pane/Files-tab appears then vanishes). Fix: `_withSessionLayoutRestore` (base controller) holds `suppressEditorPartAutoVisibility()` for the whole (async) restore **only when `isSinglePaneLayoutEnabled`** (OFF layout unchanged), so layout-driven closes never reach `handleDidCloseEditor`. `suppressEditorPartAutoVisibility()` on `IAgentWorkbenchLayoutService` returns the `IDisposable` suppression window; the boolean it toggles is observed internally through the Workbench's protected `_isEditorPartAutoVisibilitySuppressed` getter. (Historical note: this fix also once protected a `_dismissedManagedTabs` set from being poisoned by external closes; that set no longer exists — managed tabs are now re-populated purely by "add only when the group is empty on a view-open trigger".)
- **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.
- **The width-based docked reveal-sync (`_syncEditorVisibility`) must bail while editor-part auto-visibility is suppressed**: `SinglePaneWorkbench._syncEditorVisibility` reveals/hides the docked editor purely from the node width (for user sash drags). A session-switch / reload layout restore holds `suppressEditorPartAutoVisibility` while it applies the working set, which can **widen the docked node** before the controller has set the target editor-part visibility. Because the width-sync ran regardless of suppression, restoring a **Detail-only** session (aux open, editor closed) flickered the editor open on switch (the working-set apply widened the node → reveal → the controller then re-hid it) and could persist it open on reload. Gate `_syncEditorVisibility` on `!this._isEditorPartAutoVisibilitySuppressed` (alongside the existing `_syncingEditorVisibility` reentrancy guard) so only a real user sash drag (unsuppressed) drives width-based visibility. Relatedly, `baseSessionLayoutController._applyWorkingSet`'s `isInitialRestore` branch must, for single-pane, apply `_shouldHideEditorPartOnApply(editorPartHidden)` after the working-set apply (a no-op for the classic layout) — otherwise a Detail-only session's persisted editor-hidden state is not re-applied on reload and the editor is left visible.
- **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), transient hiding for full-width editors, 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`, `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`.
- **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). `SinglePaneDetailPanelStrategy._syncForcedDetailTarget` reveals a hidden detail ONLY when it was temporarily hidden by a full-width editor (`_hiddenByFullWidthEditor`), 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. **Exception — opening the empty Files placeholder (`EmptyFileEditorInput`) reveals the Files detail** (its content, the Files tree, lives in the aux bar). This is a dedicated `onDidActiveEditorChange` listener in the strategy that reveals the aux bar when the placeholder *becomes the active editor* — NOT reactive logic inside the detail autorun (which re-reads `auxBarVisibleObs`, so it would re-reveal the instant the user hides the detail — bug: "can't hide the details view in the empty file editor at all"). Keying on **active editor** (not `onWillOpenEditor`) is deliberate: the managed auto-ensured Files tab is opened **inactive** as a background tab (`fileTabOptions`), so it never becomes active and never reveals — preserving the Editor-only default — while the `+` Files action and selecting the Files tab both make it active and reveal. Do NOT reveal from the `NewFileTabAction` instead: that misses tab-selection and other activation paths (tried and rejected — "does not work"). The listener is guarded by `isVisible(EDITOR_PART)` (don't reveal the detail alone while the whole side pane is closed, e.g. Scenario C reload) and `!ctx.isRestoringSessionLayout` (a restore-driven activation must not reveal). Because hiding the aux bar fires `onDidChangePartVisibility`, not `onDidActiveEditorChange`, the user's hide sticks while the placeholder stays active. Do NOT reintroduce a `DetailPanelTarget.FilesReveal` in the autorun or an `isEditorPartAutoVisibilitySuppressed()` layout-service API — the active-editor listener needs neither. The reopen default is layout-aware via the base `_defaultReopenSidePaneParts()` hook. When changing this, update the `[single-pane] reveals the Files detail when the empty Files placeholder becomes active` and `[Scenario C]` tests together.
- **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` + `HasDockedDetailsContext`).
- **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.
- **A draft replaced by a committed session must inherit the draft's side-pane layout before `_applyWorkingSet` runs**: some providers commit a new-session draft by firing `onDidReplaceSession` with a new session resource, not by flipping `isCreated` on the same resource. Without transferring the active draft's `_editorPartHiddenBySession` and aux-bar state, the committed resource has no saved layout, so the delayed B2 working-set apply treats it as a first-visit created session and reveals the editor (Editor-only default) even though the user submitted from the new-session Detail-only view. Handle the replacement event as D4 submit: copy the active draft's editor-hidden state to the committed resource, record Changes as the committed aux container, and open Changes only if the draft detail was visible; switching to an unrelated existing created session still uses the Editor-only default.
- **Single-pane D3c: a created session with NO saved detail state must be left in its current on-screen state — never force-hidden**: the detail (aux-bar) restore for a created single-pane session (`SinglePaneDetailVisibilityStrategy._syncDetailVisibility` D3c) must **only** act when `viewStateBySession` has a saved entry — hide when it says hidden, reveal when it says visible. When there is **no** saved state (`savedState === undefined`), return without touching the aux bar. Force-hiding on the no-state path re-closes the detail the user had open in the new-session view on submit: the committed session's resource can change again *after* the initial draft→committed transition, so a later restore run lands in D3c with no saved state and `previousIsCreated` already `true` (the intrinsic `!previousIsCreated && isCreated` submit detection ([D4]) no longer matches), and would re-hide. Leaving the current state also covers a first-time-seen created session gracefully; the detail-panel strategy keeps the container in sync, and the visibility is captured on the next switch-away or user toggle. (The intrinsic [D4] submit routing to `_onNewSessionSubmitted` is still kept for the *clean* first transition — it records the state and opens Changes — but D3c-leave-current is the backstop for every follow-up run.)
- **A replace-based submit must be detected *intrinsically* in the aux/detail restore autorun (`!previousIsCreated && isCreated`), not via `_onSessionReplaced`**: the same ordering trap as the editor reveal, but for the detail (aux-bar) visibility. `sessionsService` listens to `onDidReplaceSession` *first* (it's a core service) and its handler calls `updateSession` → sets `activeSession` in a transaction → the single-pane `SinglePaneDetailVisibilityStrategy` D3 restore autorun fires **synchronously inside that handler**. The layout controller's `_onSessionReplaced` (registered later, at `BlockRestore`) runs *after* — so any aux-state transfer it does is too late: the autorun has already run D3c. The classic same-resource `isSubmit` guard (`!isSessionSwitch && !previousIsCreated && isCreated`) misses this because the agent-host/Copilot provider commits by *replacing* the draft with a **new resource** (`isSessionSwitch` is true). Fix: relax `isSubmit` to `previousSessionResource && !previousIsCreated && isCreated && !viewStateBySession.has(activeSessionResource)` — detect the submit purely from the transition, independent of `_onSessionReplaced` ordering. The `!has(state)` guard keeps a genuine navigation from a draft to an *existing* created session on the normal D3 restore path. `_onSessionReplaced` then only needs to cover the **background** submit (a session committed while a *different* session is active, so the autorun never fires for it). Because the committed resource can still change again after the first transition, this intrinsic detection alone isn't enough — pair it with the D3c-leave-current rule above. General rule: for any "on submit, preserve/transfer layout" logic, detect the submit from the reactive transition the consumer already observes — never from a flag/transfer set by a separately-registered `onDidReplaceSession` listener.
- **`onDidReplaceSession` always means submit — never re-check `from.status === Untitled`, and never try to preserve visibility via a flag consumed by `runOnChange`**: two related traps when suppressing the docked-editor reveal on new-session submit. (1) By the time `onDidReplaceSession` fires, the draft has *already* transitioned `Untitled→Completed`, so a `_isNewSessionReplacement(from,to)` guard checking `from.status === SessionStatus.Untitled` is **always false** and silently skips the whole editor-hidden/aux transfer. The event is documented to fire only when an untitled draft is atomically replaced by its committed session, so treat *every* `onDidReplaceSession` as a submit — no status guard. (2) The B2 working-set `runOnChange` (on the workspace-gated `activeSessionForWorkingSet` derive) fires **before** the synchronous `onDidReplaceSession` handler, so a boolean flag set in `_onSessionReplaced` and read synchronously in `runOnChange` is captured stale (`false`) and cannot suppress the reveal. The correct, ordering-robust mechanism is to have `_onSessionReplaced` write the draft's live editor-part visibility into `_editorPartHiddenBySession[to]` **synchronously**; because `_applyWorkingSet` reads that map inside its `Sequencer.queue` async microtask body (which runs *after* the sync replace handler), the reveal decision (`_shouldRevealEditorPartOnApply`/`_shouldRevealEditorPartForEmptyWorkingSet`) sees `editorPartHidden=true` and skips. Do NOT add a `preserveEditorPartVisibility` apply option keyed off event ordering — it's impossible to set in time.
- **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, `SinglePaneLayoutController._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.
- **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.
- **Single-pane detail sync must re-read aux-bar visibility when queued work runs**: the detail-panel autorun queues container opens through a sequencer, so a task can be computed while the previous session's detail is visible and run after D3 has restored the incoming session's detail to hidden. Do not trust an `auxBarVisible` value captured before the queue boundary; read `isVisible(AUXILIARYBAR_PART)` inside the queued sync, otherwise `openViewContainer` can re-reveal the detail and overwrite the incoming session's saved hidden state.
- **The managed Changes tab must open non-stealing so the working-set-restored active editor is preserved**: on a single-pane session switch, `baseSessionLayoutController._applyWorkingSet` restores the session's editor working set including *which* editor was active (e.g. `package.json`). `SinglePaneManagedTabsStrategy` then idempotently re-ensures the pinned Changes tab — but if `changesEditorOptions` opens it as active (no `inactive` / `activation`), it steals active state from the just-restored editor, so the wrong tab is active after the switch. Give `changesEditorOptions` `inactive: true` + `activation: EditorActivation.PRESERVE` (matching `fileTabOptions`); the workbench still makes it active when the group is empty (`active: this.count === 0 || !options?.inactive`, `editorGroupView.doOpenEditor`), so the first-visit created-session default (Changes active) is preserved while a restored session keeps its own active editor.
- **Save the outgoing session's working set eagerly on the *raw* active session change, not on the workspace-gated `activeSessionForWorkingSet` derive**: the derive (`baseSessionLayoutController`) holds back while the incoming session's workspace folders resolve, and other autoruns driven by the *raw* `ISessionsService.activeSession` (e.g. the single-pane managed-tabs sync) **async-close the outgoing session's docked editors** (`_closeInactiveChangesEditors`) during that lag. If the working-set save is on the lagged derive it runs *after* those closes, so the outgoing session's Changes tab (or whichever editor was active) is already gone and its active state is lost — on return the working set restores the wrong active editor (symptom: switching back to a session whose Changes tab was active shows a different tab active). Fix: a dedicated `[B2] runOnChange(activeSession, …)` saves the previous session's working set **synchronously** (guarded by resource-inequality, `!Untitled`, `!_isRestoringSessionLayout`) — this runs before the managed-tab sequencer microtask closes anything — and the save is removed from the gated apply `runOnChange` (which now only applies). Save doesn't need workspace readiness; only apply does. Pairs with making the managed Changes tab open non-stealing (`inactive: true` + `EditorActivation.PRESERVE`) so the restored active editor is preserved. The unit harness can't reproduce the derive-lag directly, so assert the decoupling: switching to a session whose workspace isn't in the folders (gated apply holds back) still records a `saveWorkingSet` for the outgoing session.
- **Docked side-pane width persistence must be symmetric about the detail (aux) width**: in single-pane the docked detail (auxiliary bar) lives *inside* the editor grid node, so the workbench persists the pure editor-content width (`_persistedEditorWidth` = node − detail) and the grid descriptor reconstructs node = editor-content + detail. These must use the **same condition** for including the detail: only when the detail is visible (`partVisibility.auxiliaryBar`). Subtracting the detail width *unconditionally* at save while adding it back only when the detail is visible shrank an **Editor-only** session's side pane by the detail width on *every* reload, compounding toward zero ("side pane always tiny on reload"). Fix `_persistedEditorWidth` to subtract only when the detail is visible.
- **Side-pane (editor grid node) size is workbench-level, not per session**: the editor grid node width is owned by the workbench grid and persisted globally (`workbench.sessions.partSizes` via `_savePartSizes`/`createDesktopGridDescriptor`), so switching sessions keeps the same width and reload restores it in one paint. Do **not** add a per-session width map in the layout controller that re-applies a width on session switch/reveal — it makes switching sessions jump the side pane around, and a post-paint restore on reload flickers. The 60% first-open default (`SIDE_PANE_WIDTH_RATIO` in `parts/editorPartSizing.ts`, applied by the single-pane `_applyEditorSplitSize` override on the first reveal that has no size to restore) is the only width the layout intentionally sets; everything else is the user's persisted grid size.
- **Per-session menu visibility must flow through scoped context keys, not widget DOM hiding or global active-session keys**: toolbars evaluate `when` against their scoped `IContextKeyService`, while their pickers act on scoped `ISessionContext`. Expose provider-specific availability as an observable on `ISession` and publish it through `setSessionContextKeys`, so every visible session surface evaluates the same scoped state declaratively.
- **Explicit resource scopes must carry their owner and announce eligibility changes**: never bind a plan/file to the globally active session because multiple sessions can render concurrently. Pass the owning session resource into registration and fire the scope-change event when the binding is added or removed so already-open editors refresh.
- **Per-editor overlay actions must carry editor command context**: an overlay can be invoked from an inactive split group, so resolving commands from the globally active editor can mutate the wrong resource. Set the toolbar context from its owning group and resolve it through `resolveCommandsContext`.
- **Keep validation proportional to the change**: once focused tests cover a small correction, do not chain repeated reviewers or broad suites unless a concrete failure warrants escalation.
No comments yet. Be the first to comment!