sessions: fix chat history picker not opening selected session#312158
sessions: fix chat history picker not opening selected session#312158roblourens merged 2 commits intomainfrom
Conversation
The chat history picker (workbench.action.chat.history) showed sessions but clicking one didn't open it in the Agents window. The default session opener calls IChatWidgetService.openSession with ChatViewPaneTarget, which opens the ChatViewId view -- but in the Agents window that view is gated by a when clause on IsNewChatSessionContext / IsNewChatInSessionContext, so it stayed hidden. Register a SessionsOpenerParticipant that routes opens through ISessionsManagementService.openSession, which updates the active session/chat context keys so the right view becomes visible and loads the session. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The chat history picker now hides sessions in the Completed state so the list focuses on sessions that are still active or need attention. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes session selection from workbench.action.chat.history in the Agents window by ensuring session opens flow through the sessions app’s management layer so the correct context keys are updated and the chat view becomes visible.
Changes:
- Add a
ISessionOpenerParticipantimplementation invs/sessionsto reroute session opens viaISessionsManagementService.openSession. - Register the participant as a sessions workbench contribution at startup.
Show a summary per file
| File | Description |
|---|---|
| src/vs/sessions/contrib/chat/browser/sessionsOpenerParticipant.ts | Adds a session opener participant that routes opens through ISessionsManagementService to update sessions-window state/context keys. |
| src/vs/sessions/contrib/chat/browser/chat.contribution.ts | Registers the new participant contribution during startup. |
Copilot's findings
Comments suppressed due to low confidence (1)
src/vs/sessions/contrib/chat/browser/sessionsOpenerParticipant.ts:27
- If
sessionsManagementService.getSession(session.resource)returnsundefined, this returnsfalseand falls back to the default opener. In the Agents window the default opener is the path that can fail to show anything due to theChatViewIdwhenclause, so this leaves the original issue unaddressed for any sessions not represented inISessionsManagementService(e.g., provider-filtered session types). Consider handling the “not found” case by still updating the sessions window context keys / routing, or by broadeningISessionsManagementService.openSessionto support opening by resource even when not in the sessions list.
const target = sessionsManagementService.getSession(session.resource);
if (!target) {
return false;
}
- Files reviewed: 3/3 changed files
- Comments generated: 3
| const target = sessionsManagementService.getSession(session.resource); | ||
| if (!target) { |
There was a problem hiding this comment.
target is assigned but never used. With noUnusedLocals enabled in this repo, this will fail TypeScript compilation. Either remove the variable and inline the check, or use the returned session object for something meaningful.
| const target = sessionsManagementService.getSession(session.resource); | |
| if (!target) { | |
| if (!sessionsManagementService.getSession(session.resource)) { |
| if (!target) { | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
When this participant handles the open, it bypasses openSessionDefault, which marks the IAgentSession as read (session.setRead(true)). Consider preserving that behavior here as well so sessions opened from the history picker don't remain unread.
This issue also appears on line 24 of the same file.
| target.setRead(true); |
| class SessionsOpenerParticipant implements ISessionOpenerParticipant { | ||
|
|
||
| async handleOpenSession(accessor: ServicesAccessor, session: IAgentSession, openOptions?: ISessionOpenOptions): Promise<boolean> { | ||
| const sessionsManagementService = accessor.get(ISessionsManagementService); | ||
| const target = sessionsManagementService.getSession(session.resource); | ||
| if (!target) { | ||
| return false; | ||
| } | ||
|
|
||
| await sessionsManagementService.openSession(session.resource, { preserveFocus: openOptions?.editorOptions?.preserveFocus }); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This change introduces new session-opening behavior specific to the Agents window, but there’s no unit test coverage. There are existing tests for other ISessionOpenerParticipant implementations (e.g. GrowthSessionOpenerParticipant), and src/vs/sessions/contrib/chat/test/browser already hosts lightweight tests. Consider adding a test that asserts this participant calls ISessionsManagementService.openSession(...) and returns true (and that it returns false when it should fall back).
In the Agents window,
workbench.action.chat.historyshowed the session picker but selecting a session did nothing.Cause
The default session opener (
openSessionDefaultinagentSessionsOpener.ts) callsIChatWidgetService.openSession(..., ChatViewPaneTarget), which opensChatViewId. In the Agents window that view is registered with awhenclause gated onIsNewChatSessionContext.negate() && IsNewChatInSessionContext.negate(), so when the user is on the new-chat view it stays hidden and nothing happens.Fix
Register a
SessionsOpenerParticipantinvs/sessionsthat intercepts session opens and routes them throughISessionsManagementService.openSession. That service updates the active session / context keys properly, so the chat view becomes visible and loads the picked session.(Written by Copilot)