fix active session when changing from untitled to committed session#297969
fix active session when changing from untitled to committed session#297969
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes an issue where the active session tracking breaks when an untitled session is committed and replaced with a new committed resource. The fix adds an isUntitled boolean field to the IActiveSessionItem interface and implements logic to detect and handle the commit transition by looking up the new committed session via the chat widget.
Changes:
- Added
isUntitledfield toIActiveSessionIteminterface to track whether a session is untitled - Implemented special handling in
refreshActiveSessionFromModel()to detect when an untitled session has been committed and find the new committed session - Updated all locations where
IActiveSessionItemobjects are created to include theisUntitledfield
Comments suppressed due to low confidence (1)
src/vs/sessions/contrib/sessions/browser/sessionsManagementService.ts:167
- When an untitled session is committed but the committed session cannot be found (either due to no widgets, or widget has no viewModel/sessionResource, or session not in model), the function returns without updating the active session. This leaves the active session pointing to a resource (the untitled one) that no longer exists in the agentSessionsService.model. Consider falling back to showNextSession() or openNewSessionView() in this case to ensure the UI is in a consistent state.
if (currentActive.isUntitled) {
// The untitled session was committed by the extension via
// onDidCommitChatSessionItem, which replaces the untitled
// resource with a new committed resource. The commit handler
// already swapped the ChatViewPane widget to the new resource,
// so find it by checking the widget's current session resource.
const chatViewWidgets = this.chatWidgetService.getWidgetsByLocations(ChatAgentLocation.Chat);
const committedResource = chatViewWidgets[0]?.viewModel?.sessionResource;
const committedSession = committedResource ? this.agentSessionsService.model.getSession(committedResource) : undefined;
if (committedSession) {
this.setActiveSession(committedSession);
}
} else {
| const committedResource = chatViewWidgets[0]?.viewModel?.sessionResource; | ||
| const committedSession = committedResource ? this.agentSessionsService.model.getSession(committedResource) : undefined; | ||
| if (committedSession) { | ||
| this.setActiveSession(committedSession); |
There was a problem hiding this comment.
The approach of accessing chatViewWidgets[0] assumes there's always at least one chat widget when an untitled session is committed. If no widgets exist or if the first widget doesn't have a valid viewModel/sessionResource, the committedResource will be undefined. Consider adding explicit handling for these cases, such as logging a warning or falling back to showNextSession, to avoid leaving the active session in an inconsistent state when the commit happens but the widget lookup fails.
This issue also appears on line 155 of the same file.
| const committedResource = chatViewWidgets[0]?.viewModel?.sessionResource; | |
| const committedSession = committedResource ? this.agentSessionsService.model.getSession(committedResource) : undefined; | |
| if (committedSession) { | |
| this.setActiveSession(committedSession); | |
| if (!chatViewWidgets.length) { | |
| this.logService.warn('[SessionsManagementService] Unable to resolve committed session for untitled session: no chat widgets found.'); | |
| this.showNextSession(); | |
| return; | |
| } | |
| const committedResource = chatViewWidgets[0].viewModel?.sessionResource; | |
| if (!committedResource) { | |
| this.logService.warn('[SessionsManagementService] Unable to resolve committed session for untitled session: chat widget has no session resource.'); | |
| this.showNextSession(); | |
| return; | |
| } | |
| const committedSession = this.agentSessionsService.model.getSession(committedResource); | |
| if (committedSession) { | |
| this.setActiveSession(committedSession); | |
| } else { | |
| this.logService.warn('[SessionsManagementService] Unable to resolve committed session for untitled session: no session found for committed resource.'); | |
| this.showNextSession(); |
| this.lastSelectedSession = session.resource; | ||
| const [repository, worktree] = this.getRepositoryFromMetadata(session.metadata); | ||
| activeSessionItem = { | ||
| isUntitled: this.chatService.getSession(session.resource)?.contributedChatSession?.isUntitled ?? true, |
There was a problem hiding this comment.
The default value of 'true' for isUntitled may be incorrect for local agent sessions. The contributedChatSession field is only set for remote (non-local) sessions in the chat service. For local sessions, this will always be undefined, causing isUntitled to always default to 'true' even for committed local sessions. Consider checking if the session is a local session using LocalChatSessionUri.isLocalSession() and defaulting to 'false' for local sessions, or finding another way to determine if an agent session is untitled.
No description provided.