sessions: decouple BranchPicker from active session via IBranchPickerModel#325712
Draft
benvillalobos wants to merge 2 commits into
Draft
sessions: decouple BranchPicker from active session via IBranchPickerModel#325712benvillalobos wants to merge 2 commits into
benvillalobos wants to merge 2 commits into
Conversation
…Model The BranchPicker was tightly coupled to ICopilotChatSession, requiring an active session to function. This prevented reuse in contexts without a session (e.g. the automations dialog), forcing PR #325514 to duplicate all branch-loading and picker UI logic. Introduce IBranchPickerModel — a minimal interface (branches, branch, loading, disabled, setBranch) that captures what the picker actually needs. The picker becomes a controlled view over this model with no knowledge of sessions, providers, or isolation modes. The session-to-model wiring moves to the instantiation site in copilotChatSessionsActions.ts as an inline derived() that maps the ICopilotChatSession fields and computes disabled from isolation mode. This keeps provider-specific logic in provider code. Key design decisions: - disabled (not isolationMode) because each consumer computes disabled differently — the picker shouldn't know about isolation - IObservable<IBranchPickerModel | undefined> (not concrete class) so consumers can provide any implementation - No new service or registry — the model is instance-scoped state - BranchPicker drops its ISessionsProvidersService dependency entirely Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d30007f-3037-4015-a850-60e8d3852d66
Contributor
There was a problem hiding this comment.
Pull request overview
Decouples the branch picker UI from active sessions through an observable model.
Changes:
- Adds
IBranchPickerModel. - Adapts Copilot sessions to the new model.
- Updates picker rendering and selection logic.
Show a summary per file
| File | Description |
|---|---|
copilotChatSessionsActions.ts |
Creates the model adapter for Copilot sessions. |
branchPicker.ts |
Refactors the picker to consume the model contract. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Medium
Comment on lines
+20
to
+26
| /** Minimal contract the BranchPicker needs to render and interact. */ | ||
| export interface IBranchPickerModel { | ||
| readonly branches: IObservable<readonly string[]>; | ||
| readonly branch: IObservable<string | undefined>; | ||
| readonly loading: IObservable<boolean>; | ||
| readonly disabled: IObservable<boolean>; | ||
| setBranch(name: string): void; |
Comment on lines
+148
to
+150
| const model = this._model.get(); | ||
| const isLoading = model?.loading.get() ?? false; | ||
| const isDisabled = model?.disabled.get() ?? false; |
benvillalobos
added a commit
that referenced
this pull request
Jul 13, 2026
Adds a branch picker to the automations dialog when worktree isolation is selected. Uses the shared BranchPicker widget via IBranchPickerModel, eliminating duplicated branch-loading and picker UI code. Moves BranchPicker from contrib/providers/copilotChatSessions to contrib/chat/browser so it can be imported by non-provider contribs without layering violations. Extracts IsolationGroupModel as a pure state model for testability. Based on work from bv/automations-isolation-checkbox, rebased on top of the IBranchPickerModel decoupling in #325712. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d30007f-3037-4015-a850-60e8d3852d66
benvillalobos
added a commit
that referenced
this pull request
Jul 13, 2026
Adds a branch picker to the automations dialog when worktree isolation is selected. Uses the shared BranchPicker widget via IBranchPickerModel, eliminating duplicated branch-loading and picker UI code. Moves BranchPicker from contrib/providers/copilotChatSessions to contrib/chat/browser so it can be imported by non-provider contribs without layering violations. Extracts IsolationGroupModel as a pure state model for testability. Based on work from bv/automations-isolation-checkbox, rebased on top of the IBranchPickerModel decoupling in #325712. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d30007f-3037-4015-a850-60e8d3852d66
BranchPicker needs to be importable by non-provider contribs (e.g. automations). Moving it from contrib/providers/copilotChatSessions to contrib/chat/browser satisfies the sessions layer rules. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d30007f-3037-4015-a850-60e8d3852d66
benvillalobos
force-pushed
the
bv/branchpicker-no-session
branch
from
1984d01 to
027002c
Compare
benvillalobos
added a commit
that referenced
this pull request
Jul 14, 2026
Adds a branch picker to the automations dialog when worktree isolation is selected. Uses the shared BranchPicker widget via IBranchPickerModel, eliminating duplicated branch-loading and picker UI code. Moves BranchPicker from contrib/providers/copilotChatSessions to contrib/chat/browser so it can be imported by non-provider contribs without layering violations. Extracts IsolationGroupModel as a pure state model for testability. Based on work from bv/automations-isolation-checkbox, rebased on top of the IBranchPickerModel decoupling in #325712. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d30007f-3037-4015-a850-60e8d3852d66
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduces IBranchPickerModel so the BranchPicker can be reused without a session (e.g. automations dialog).