Skip to content

fix: enhance session resolution in sessionSupportsFork method#303624

Merged
TylerLeonhardt merged 1 commit intomainfrom
tyler/retail-camel
Mar 20, 2026
Merged

fix: enhance session resolution in sessionSupportsFork method#303624
TylerLeonhardt merged 1 commit intomainfrom
tyler/retail-camel

Conversation

@TylerLeonhardt
Copy link
Member

ref #300501

Copilot AI review requested due to automatic review settings March 20, 2026 21:47
@TylerLeonhardt TylerLeonhardt enabled auto-merge (squash) March 20, 2026 21:47
@TylerLeonhardt TylerLeonhardt self-assigned this Mar 20, 2026
@vs-code-engineering vs-code-engineering bot added this to the 1.113.0 milestone Mar 20, 2026
Copy link
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

This PR adjusts how contributed chat sessions are looked up when determining whether a session supports forking, addressing cases where the session may be stored under either the original resource or an alias.

Changes:

  • Update ChatSessionsService.sessionSupportsFork to check both the raw sessionResource key and the alias-resolved key when reading from _sessions.

Comment on lines +1216 to 1220
const session = this._sessions.get(sessionResource)
// Try to resolve in case an alias was used
?? this._sessions.get(this._resolveResource(sessionResource));
return !!session?.session.forkSession;
}
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

sessionSupportsFork now checks both the direct key and the alias-resolved key, but forkChatSession still only looks up _sessions.get(this._resolveResource(sessionResource)). In the alias scenario this can make the UI enable the Fork action while the actual fork call still throws "does not support forking" because it resolves to the untitled placeholder session. Consider centralizing the lookup (e.g., a private helper that tries direct then resolved) and using it in both sessionSupportsFork and forkChatSession so the capability check and execution path stay consistent.

See below for a potential fix:

	private getSessionForResource(sessionResource: URI): ContributedChatSessionData | undefined {
		// First try the direct key, then fall back to the alias-resolved key.
		return this._sessions.get(sessionResource)
			?? this._sessions.get(this._resolveResource(sessionResource));
	}

	public sessionSupportsFork(sessionResource: URI): boolean {
		const session = this.getSessionForResource(sessionResource);
		return !!session?.session.forkSession;
	}

	public async forkChatSession(sessionResource: URI, request: IChatSessionRequestHistoryItem | undefined, token: CancellationToken): Promise<IChatSessionItem> {
		const session = this.getSessionForResource(sessionResource);

Copilot uses AI. Check for mistakes.
@TylerLeonhardt TylerLeonhardt merged commit a3512f0 into main Mar 20, 2026
23 checks passed
@TylerLeonhardt TylerLeonhardt deleted the tyler/retail-camel branch March 20, 2026 22:18
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