Skip to content

sessions: split session model from view service#320139

Merged
sandy081 merged 3 commits into
mainfrom
sandy081/sessions-split-model-view
Jun 5, 2026
Merged

sessions: split session model from view service#320139
sandy081 merged 3 commits into
mainfrom
sandy081/sessions-split-model-view

Conversation

@sandy081
Copy link
Copy Markdown
Member

@sandy081 sandy081 commented Jun 5, 2026

What

Splits the Agents window session model from its view:

  • ISessionsManagementService (model, services/sessions) owns canonical session state — providers, active session, the in-progress new-session draft, CRUD, recency, and send paths.
  • ISessionsViewService (new core service, browser/sessionsViewService.ts) owns the visible-slot arrangement, focus, back/forward navigation, and per-session view persistence (restore).
  • The part (browser/parts/sessionsParts.ts + sessionsPartService.ts) becomes a passive renderer — it injects neither the model nor the view and only exposes updateVisibleSessions/focusSession/onDidFocusSession.

Why

The previous SessionsManagementService had grown to ~1.9k lines mixing model truth with grid/focus/restore concerns. Separating them gives a single source of truth for session state and a clear, reactive data-flow contract (see SESSIONS.md).

Notable changes

  • Open without focusing: per-activation preserveFocus intent is exposed as an IObservable published in the same transaction as activeSession, and consumed reactively in the view's reconcile autorun (no consume-once side-channel).
  • New-session draft encapsulation: replaced the leaked pendingNewSession getter + discardPendingNewSession() with a model-owned readonly newSession: IObservable<ISession | undefined> and a conditional discardNewSession(session?).
  • API merge: openNewSessionView merged into a single openNewSession(options?).
  • Bug fix (session type picker empty after reload): new-session creation now routes through ISessionsViewService.openNewSession so the draft is activated, not just created. The picker derives from the active session, so creating via the model alone left it blank.

Reviewer notes

  • Data-flow contract and the model-vs-view boundary table are documented in src/vs/sessions/SESSIONS.md.
  • SKILL.md Common Pitfalls updated with the patterns this refactor enforces (no side-channels, no internal "pending" surface, model-creates / view-activates).
  • Validated: compile-check-ts-native, valid-layers-check, eslint, hygiene precommit. Unit tests under services/sessions/test updated for the renamed/merged APIs.

Separate the session model (ISessionsManagementService) from the view
(ISessionsViewService) so the model owns canonical session state and the
view owns visible-slot arrangement, focus and per-session view
persistence. Introduces the new `browser/sessionsViewService.ts` core
service and `browser/parts/sessionsParts.ts`, makes the part a passive
renderer, and adds support for opening a session without focusing it
(reactive preserve-focus intent published atomically with the active
session).

Also encapsulates the in-progress new-session draft in the model as a
reactive `newSession` observable with a conditional `discardNewSession`,
merges `openNewSessionView` into `openNewSession`, and routes new-session
creation through the view so the draft is activated (fixing the session
type picker rendering empty after reload).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 5, 2026 16:45
@vs-code-engineering
Copy link
Copy Markdown
Contributor

vs-code-engineering Bot commented Jun 5, 2026

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@benibenj

Matched files:

  • src/vs/sessions/browser/parts/chatCompositeBar.ts
  • src/vs/sessions/browser/parts/sessionDropTarget.ts
  • src/vs/sessions/browser/parts/sessionsPartService.ts
  • src/vs/sessions/browser/parts/sessionsParts.ts
  • src/vs/sessions/browser/workbench.ts
  • src/vs/sessions/contrib/layout/browser/sessionLayoutController.ts
  • src/vs/sessions/contrib/layout/test/browser/sessionLayoutController.test.ts

@lszomoru

Matched files:

  • src/vs/sessions/services/sessions/browser/sessionNavigation.ts
  • src/vs/sessions/services/sessions/browser/sessionsManagementService.ts
  • src/vs/sessions/services/sessions/browser/visibleSessions.ts
  • src/vs/sessions/services/sessions/common/sessionsManagement.ts
  • src/vs/sessions/services/sessions/test/browser/sessionNavigation.test.ts
  • src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the Agents window sessions architecture by splitting the canonical session model (ISessionsManagementService) from a new view/controller service (ISessionsViewService) that owns visible-slot layout, focus, navigation, and restore/persistence, with the sessions part becoming a passive renderer.

Changes:

  • Added SessionsViewService (core) to own VisibleSessions, restore/persistence, focus, and Back/Forward navigation, and to mirror the visible active slot into the model.
  • Reduced SessionsManagementService (services) to model truth only (active session, new-session draft, CRUD/send paths), adding newSession, discardNewSession, createNewChatInSession, and onDidReplaceSession.
  • Updated actions/widgets/telemetry/layout/tests to route “open/show session” operations through the view service instead of the management service.
Show a summary per file
File Description
src/vs/sessions/SESSIONS.md Updates the sessions architecture/spec documentation for the model vs view split.
src/vs/sessions/sessions.common.main.ts Switches eager entry-point imports to the new sessions part + view service.
src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts Updates tests to drive opening/restore through the new view service and stubs a passive part.
src/vs/sessions/services/sessions/test/browser/sessionNavigation.test.ts Adjusts navigation tests for the new opener abstraction and new management members.
src/vs/sessions/services/sessions/common/sessionsManagement.ts Reshapes the management interface to remove view concerns and add new-session + replace-session APIs.
src/vs/sessions/services/sessions/browser/visibleSessions.ts Adds atomic preserve-focus intent alongside active-session changes and DI decorators for services.
src/vs/sessions/services/sessions/browser/sessionsManagementService.ts Removes grid/navigation/restore logic from the model service; adds newSession tracking + replacement event.
src/vs/sessions/services/sessions/browser/sessionNavigation.ts Introduces ISessionOpener so navigation can be driven without depending on the core view service.
src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts Routes session-opening actions through ISessionsViewService.
src/vs/sessions/contrib/sessions/browser/views/sessionsView.ts Uses the view service for open/to-side behaviors and new-session actions.
src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts Consumes visibleSessions from the view service for rendering state.
src/vs/sessions/contrib/sessions/browser/sessionsTelemetry.contribution.ts Moves stickiness/visible-session telemetry to the view service.
src/vs/sessions/contrib/sessions/browser/sessionsActions.ts Rewires commands (picker, nav, pin/close/open) to the view service.
src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostActions.ts Updates provider actions that open the new-session view to use ISessionsViewService.
src/vs/sessions/contrib/providers/localChatSessions/browser/localChatSessions.contribution.ts Opens forked sessions through the view service.
src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostSkillButtons.test.ts Updates mocks/tests for renamed active-session setter.
src/vs/sessions/contrib/layout/test/browser/sessionLayoutController.test.ts Stubs ISessionsViewService.visibleSessions for layout controller tests.
src/vs/sessions/contrib/layout/browser/sessionLayoutController.ts Uses view-service visibility state for multi-session layout suppression and state save.
src/vs/sessions/contrib/codeReview/test/browser/codeReviewService.test.ts Updates active-session wiring in code-review tests.
src/vs/sessions/contrib/chat/electron-browser/chat.contribution.ts Routes session opening/new-session actions and new-session unsetting through the view service.
src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts Changes how the picker derives the “current” type from the active session.
src/vs/sessions/contrib/chat/browser/sessionsOpenerParticipant.ts Routes session open requests through the view service.
src/vs/sessions/contrib/chat/browser/newChatWidget.ts Creates/unsets drafts via the view service (so drafts are activated/visible).
src/vs/sessions/contrib/chat/browser/chat.contribution.ts Updates the “New Chat in Sessions Window” action to use the view service.
src/vs/sessions/browser/workbench.ts Uses the view service for restore and “new session” gestures.
src/vs/sessions/browser/sessionsViewService.ts New core service implementing visible-slot model, focus, restore, recency, and navigation.
src/vs/sessions/browser/parts/sessionsPartService.ts Turns the part service into a passive renderer contract (no longer owns lifecycle/autoruns).
src/vs/sessions/browser/parts/sessionsParts.ts New eager singleton that owns the sessions part lifecycle (mobile vs desktop), without observing model/view.
src/vs/sessions/browser/parts/sessionDropTarget.ts Routes drag/drop insertion through the view service.
src/vs/sessions/browser/parts/chatCompositeBar.ts Opens chats through the view service.
.github/skills/sessions/SKILL.md Updates sessions-layer “Common Pitfalls” guidance for the refactor.

Copilot's findings

  • Files reviewed: 31/31 changed files
  • Comments generated: 8

Comment thread src/vs/sessions/browser/sessionsViewService.ts Outdated
Comment thread src/vs/sessions/SESSIONS.md Outdated
Comment thread src/vs/sessions/SESSIONS.md Outdated
Comment thread src/vs/sessions/SESSIONS.md Outdated
Comment thread src/vs/sessions/SESSIONS.md Outdated
Comment thread src/vs/sessions/SESSIONS.md Outdated
Comment thread .github/skills/sessions/SKILL.md Outdated
Comment thread src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts Outdated
@sandy081 sandy081 enabled auto-merge (squash) June 5, 2026 16:56
- Restore the persist-on-refresh behavior in SessionTypePicker that the
  merge inadvertently dropped (so an auto-selected/defaulted type survives
  reload instead of leaving sessions.lastSelectedSessionType unset).
- Update SESSIONS.md, the openNewSession JSDoc, and code comments to the
  merged openNewSession / newSession / discardNewSession API and drop the
  reverted inherit-workspace-from-active-session documentation.
- Align the SKILL.md create-and-upgrade pitfall with SESSIONS.md (the
  upgrade listener lives for the consumer's lifetime; no timeout or
  LifecyclePhase give-up).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sandy081 sandy081 merged commit cbb9055 into main Jun 5, 2026
25 checks passed
@sandy081 sandy081 deleted the sandy081/sessions-split-model-view branch June 5, 2026 18:14
@vs-code-engineering vs-code-engineering Bot added this to the 1.124.0 milestone Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants