Hide ChatSessionUri.parse

[mjbvz]

For #274403

[Copilot]

Pull Request Overview

This PR makes the LocalChatSessionUri.parse method private by removing its export, fulfilling issue #274403. The deprecated method is converted to an internal implementation detail while external callers are migrated to use parseLocalSessionId.

Key Changes

• The parse method in chatUri.ts is made private by removing the export keyword and its deprecation comment
• The chatViewPane.ts file is refactored to use parseLocalSessionId instead of the deprecated parse method
• The unused Schemas import is removed from chatViewPane.ts

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/vs/workbench/contrib/chat/common/chatUri.ts	Removed export and deprecation comment from parse method, making it internal
src/vs/workbench/contrib/chat/browser/chatViewPane.ts	Refactored session loading logic to use parseLocalSessionId and removed unused import

[Copilot]

[nitpick] The TODO comment asks "Is this logic still correct with sessions from different schemes?" This is a valid concern. The refactored code only handles sessions with the vscode-local-chat-session scheme (since parseLocalSessionId returns undefined for other schemes). If there are sessions with different schemes that need similar locking logic, they won't be handled anymore. The old code was also scheme-specific (checking for Schemas.vscodeLocalChatSession), so this aspect is unchanged, but it's worth verifying whether the refactor addresses the TODO's concern.

Suggested change

	// TODO: Is this logic still correct with sessions from different schemes?
	const local = LocalChatSessionUri.parseLocalSessionId(sessionId);
	if (local) {
	await this.chatSessionsService.canResolveChatSession(sessionId);
	const contributions = this.chatSessionsService.getAllChatSessionContributions();
	const contribution = contributions.find((c: IChatSessionsExtensionPoint) => c.type === localChatSessionType);
	if (contribution) {
	this.widget.lockToCodingAgent(contribution.name, contribution.displayName, contribution.type);
	}
	// Generalized to support sessions from different schemes
	const scheme = sessionId.scheme;
	switch (scheme) {
	case LocalChatSessionUri.scheme: {
	const local = LocalChatSessionUri.parseLocalSessionId(sessionId);
	if (local) {
	await this.chatSessionsService.canResolveChatSession(sessionId);
	const contributions = this.chatSessionsService.getAllChatSessionContributions();
	const contribution = contributions.find((c: IChatSessionsExtensionPoint) => c.type === localChatSessionType);
	if (contribution) {
	this.widget.lockToCodingAgent(contribution.name, contribution.displayName, contribution.type);
	}
	}
	break;
	}
	// Add additional cases here for other schemes as needed
	default:
	// No locking logic for unknown schemes (yet)
	break;

[Copilot]

The refactored code changes the behavior in a way that may be incorrect. The old code extracted the chatSessionType from the URI authority and used it to find the matching contribution. The new code always uses the constant localChatSessionType when searching for a contribution, which will only match contributions with type 'local'.

This means URIs like vscode-local-chat-session://custom-type/... would have worked in the old code (finding a contribution with type custom-type), but now will always search for type 'local' regardless of the URI's authority.

Consider preserving the original behavior by extracting the session type from the URI:

const parsed = parse(sessionId);
if (parsed) {
    await this.chatSessionsService.canResolveChatSession(sessionId);
    const contributions = this.chatSessionsService.getAllChatSessionContributions();
    const contribution = contributions.find((c: IChatSessionsExtensionPoint) => c.type === parsed.chatSessionType);
    if (contribution) {
        this.widget.lockToCodingAgent(contribution.name, contribution.displayName, contribution.type);
    }
}

Or, if the intention is to only support local sessions, this should be clarified.

Suggested change

	// TODO: Is this logic still correct with sessions from different schemes?
	const local = LocalChatSessionUri.parseLocalSessionId(sessionId);
	if (local) {
	await this.chatSessionsService.canResolveChatSession(sessionId);
	const contributions = this.chatSessionsService.getAllChatSessionContributions();
	const contribution = contributions.find((c: IChatSessionsExtensionPoint) => c.type === localChatSessionType);
	// Extract the session type from the URI authority and use it to find the matching contribution
	const parsed = LocalChatSessionUri.parse(sessionId);
	if (parsed) {
	await this.chatSessionsService.canResolveChatSession(sessionId);
	const contributions = this.chatSessionsService.getAllChatSessionContributions();
	const contribution = contributions.find((c: IChatSessionsExtensionPoint) => c.type === parsed.chatSessionType);