Skip to content

Refactor creation of work trees #1753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed

Refactor creation of work trees #1753

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d0e47b1
Rmove Copilot CLI Agent manager and introduce ICopilotCLISession inte…
DonJayamanne Nov 2, 2025
eaf4145
Update src/extension/chatSessions/vscode-node/copilotCLIChatSessionsC…
DonJayamanne Nov 2, 2025
545219e
Updates
DonJayamanne Nov 2, 2025
54faba5
Updates
DonJayamanne Nov 2, 2025
a30af24
Refactor creation of work trees
DonJayamanne Nov 2, 2025
1a1c2b8
Merge branch 'main' into don/improved-workspacetree
DonJayamanne Nov 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 0 additions & 52 deletions src/extension/agents/copilotcli/node/copilotcliAgentManager.ts
View file
Open in desktop

This file was deleted.

44 changes: 33 additions & 11 deletions src/extension/agents/copilotcli/node/copilotcliSession.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { ILogService } from '../../../../platform/log/common/logService';
import { IWorkspaceService } from '../../../../platform/workspace/common/workspaceService';
import { CancellationToken } from '../../../../util/vs/base/common/cancellation';
import { DisposableStore } from '../../../../util/vs/base/common/lifecycle';
import { ChatRequestTurn2, ChatResponseThinkingProgressPart, ChatResponseTurn2, ChatSessionStatus, EventEmitter, LanguageModelTextPart } from '../../../../vscodeTypes';
import { ChatRequestTurn2, ChatResponseThinkingProgressPart, ChatResponseTurn2, ChatSessionStatus, EventEmitter, LanguageModelTextPart, Uri } from '../../../../vscodeTypes';
import { IToolsService } from '../../../tools/common/toolsService';
import { ExternalEditTracker } from '../../common/externalEditTracker';
import { getAffectedUrisForEditTool } from '../common/copilotcliTools';
Expand All @@ -19,7 +19,27 @@ import { buildChatHistoryFromEvents, processToolExecutionComplete, processToolEx
import { getCopilotLogger } from './logger';
import { getConfirmationToolParams, PermissionRequest } from './permissionHelpers';

export class CopilotCLISession extends DisposableStore {
export interface ICopilotCLISession {
readonly sessionId: string;
readonly status: vscode.ChatSessionStatus | undefined;
readonly onDidChangeStatus: vscode.Event<vscode.ChatSessionStatus | undefined>;

handleRequest(
prompt: string,
attachments: Attachment[],
toolInvocationToken: vscode.ChatParticipantToolToken,
stream: vscode.ChatResponseStream,
modelId: ModelProvider | undefined,
workingDirectory: string | undefined,
token: vscode.CancellationToken
): Promise<void>;

addUserMessage(content: string): void;
addUserAssistantMessage(content: string): void;
getSelectedModelId(): Promise<string | undefined>;
getChatHistory(): Promise<(ChatRequestTurn2 | ChatResponseTurn2)[]>;
}
export class CopilotCLISession extends DisposableStore implements ICopilotCLISession {
private _abortController = new AbortController();
private _pendingToolInvocations = new Map<string, vscode.ChatToolInvocationPart>();
private _editTracker = new ExternalEditTracker();
Expand All @@ -32,14 +52,6 @@ export class CopilotCLISession extends DisposableStore {

public readonly onDidChangeStatus = this._statusChange.event;

private _aborted?: boolean;
public get aborted(): boolean {
return this._aborted ?? false;
}
private readonly _onDidAbort = this.add(new EventEmitter<void>());

public readonly onDidAbort = this._onDidAbort.event;

constructor(
private readonly _sdkSession: Session,
@ILogService private readonly logService: ILogService,
Expand All @@ -64,7 +76,7 @@ export class CopilotCLISession extends DisposableStore {
yield* agent.query(prompt, attachments);
}

public async invoke(
public async handleRequest(
prompt: string,
attachments: Attachment[],
toolInvocationToken: vscode.ChatParticipantToolToken,
Expand Down Expand Up @@ -217,6 +229,16 @@ export class CopilotCLISession extends DisposableStore {
permissionRequest: PermissionRequest,
toolInvocationToken: vscode.ChatParticipantToolToken
): Promise<{ kind: 'approved' } | { kind: 'denied-interactively-by-user' }> {
if (permissionRequest.kind === 'read') {
// If user is reading a file in the workspace, auto-approve read requests.
// Outisde workspace reads (e.g., /etc/passwd) will still require approval.
const data = Uri.file(permissionRequest.path);
if (this.workspaceService.getWorkspaceFolder(data)) {
this.logService.trace(`[CopilotCLISession] Auto Approving request to read workspace file ${permissionRequest.path}`);
return { kind: 'approved' };
}
}

try {
const { tool, input } = getConfirmationToolParams(permissionRequest);
const result = await this.toolsService.invokeTool(tool,
Expand Down
12 changes: 3 additions & 9 deletions src/extension/agents/copilotcli/node/copilotcliSessionService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { Disposable, DisposableMap, DisposableStore, IDisposable, toDisposable }
import { IInstantiationService } from '../../../../util/vs/platform/instantiation/common/instantiation';
import { ChatSessionStatus } from '../../../../vscodeTypes';
import { ICopilotCLISDK } from './copilotCli';
import { CopilotCLISession } from './copilotcliSession';
import { CopilotCLISession, ICopilotCLISession } from './copilotcliSession';
import { stripReminders } from './copilotcliToolInvocationFormatter';
import { getCopilotLogger } from './logger';

Expand All @@ -41,8 +41,8 @@ export interface ICopilotCLISessionService {
deleteSession(sessionId: string): Promise<void>;

// Session wrapper tracking
getSession(sessionId: string, model: ModelProvider | undefined, readonly: boolean, token: CancellationToken): Promise<CopilotCLISession | undefined>;
createSession(prompt: string, model: ModelProvider | undefined, token: CancellationToken): Promise<CopilotCLISession>;
getSession(sessionId: string, model: ModelProvider | undefined, readonly: boolean, token: CancellationToken): Promise<ICopilotCLISession | undefined>;
createSession(prompt: string, model: ModelProvider | undefined, token: CancellationToken): Promise<ICopilotCLISession>;
}

export const ICopilotCLISessionService = createServiceIdentifier<ICopilotCLISessionService>('ICopilotCLISessionService');
Expand Down Expand Up @@ -240,12 +240,6 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS
session.add(sessionDisposables);
session.add(session.onDidChangeStatus(() => this._onDidChangeSessions.fire()));

sessionDisposables.add(session.onDidAbort(() => {
// We need to start with a new session.
// https://github.com/microsoft/vscode/issues/274169
session.dispose();
}));

this._sessionWrappers.set(sdkSession.sessionId, session);
return session;
} catch (error) {
Expand Down
4 changes: 1 addition & 3 deletions src/extension/chatSessions/vscode-node/chatSessions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { ClaudeAgentManager } from '../../agents/claude/node/claudeCodeAgent';
import { ClaudeCodeSdkService, IClaudeCodeSdkService } from '../../agents/claude/node/claudeCodeSdkService';
import { ClaudeCodeSessionService, IClaudeCodeSessionService } from '../../agents/claude/node/claudeCodeSessionService';
import { CopilotCLIModels, CopilotCLISDK, ICopilotCLIModels, ICopilotCLISDK } from '../../agents/copilotcli/node/copilotCli';
import { CopilotCLIAgentManager } from '../../agents/copilotcli/node/copilotcliAgentManager';
import { CopilotCLIPromptResolver } from '../../agents/copilotcli/node/copilotcliPromptResolver';
import { CopilotCLISessionService, ICopilotCLISessionService } from '../../agents/copilotcli/node/copilotcliSessionService';
import { ILanguageModelServer, LanguageModelServer } from '../../agents/node/langModelServer';
Expand Down Expand Up @@ -112,13 +111,12 @@ export class ChatSessionsContrib extends Disposable implements IExtensionContrib
const copilotcliSessionItemProvider = this._register(copilotcliAgentInstaService.createInstance(CopilotCLIChatSessionItemProvider, copilotCLIWorktreeManager));
this._register(vscode.chat.registerChatSessionItemProvider(this.copilotcliSessionType, copilotcliSessionItemProvider));
const promptResolver = copilotcliAgentInstaService.createInstance(CopilotCLIPromptResolver);
const copilotcliAgentManager = this._register(copilotcliAgentInstaService.createInstance(CopilotCLIAgentManager, promptResolver));
const copilotcliChatSessionContentProvider = copilotcliAgentInstaService.createInstance(CopilotCLIChatSessionContentProvider, copilotCLIWorktreeManager);
const summarizer = copilotcliAgentInstaService.createInstance(ChatSummarizerProvider);

const copilotcliChatSessionParticipant = copilotcliAgentInstaService.createInstance(
CopilotCLIChatSessionParticipant,
copilotcliAgentManager,
promptResolver,
copilotcliSessionItemProvider,
copilotSessionsProvider,
summarizer,
Expand Down
75 changes: 25 additions & 50 deletions src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import { Emitter, Event } from '../../../util/vs/base/common/event';
import { Disposable, DisposableStore, IDisposable } from '../../../util/vs/base/common/lifecycle';
import { localize } from '../../../util/vs/nls';
import { ICopilotCLIModels } from '../../agents/copilotcli/node/copilotCli';
import { CopilotCLIAgentManager } from '../../agents/copilotcli/node/copilotcliAgentManager';
import { CopilotCLISession } from '../../agents/copilotcli/node/copilotcliSession';
import { CopilotCLIPromptResolver } from '../../agents/copilotcli/node/copilotcliPromptResolver';
import { ICopilotCLISession } from '../../agents/copilotcli/node/copilotcliSession';
import { ICopilotCLISessionService } from '../../agents/copilotcli/node/copilotcliSessionService';
import { ChatSummarizerProvider } from '../../prompt/node/summarizer';
import { ICopilotCLITerminalIntegration } from './copilotCLITerminalIntegration';
Expand All @@ -35,12 +35,7 @@ export class CopilotCLIWorktreeManager {
constructor(
@IVSCodeExtensionContext private readonly extensionContext: IVSCodeExtensionContext) { }

async createWorktreeIfNeeded(sessionId: string, stream: vscode.ChatResponseStream): Promise<string | undefined> {
const isolationEnabled = this._sessionIsolation.get(sessionId) ?? false;
if (!isolationEnabled) {
return undefined;
}

async createWorktree(stream: vscode.ChatResponseStream): Promise<string | undefined> {
Copy link
Collaborator Author

@DonJayamanne DonJayamanne Nov 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@osortega
I think getting the caller to decide whether to call this method or not is an improvement, as opposed to giving more responsibility to this method.
Callers already have all of the information using the public methods in this class.

I believe that will also improve readability as sometimes we pass in untitled id and other cases we pass in the real session id (what confuses me is the fact that here we MUST pass in the old untitled id & not the real session id).

I'd like to remove that ambiguity from this class/code.

This comment was marked as resolved.

Sign in to view

try {
const worktreePath = await vscode.commands.executeCommand('git.createWorktreeWithDefaults') as string | undefined;
if (worktreePath) {
Expand Down Expand Up @@ -100,29 +95,6 @@ export class CopilotCLIWorktreeManager {
}
}

/**
* Convert a model ID to a ModelProvider object for the Copilot CLI SDK
*/
function getModelProvider(modelId: string | undefined): { type: 'anthropic' | 'openai'; model: string } | undefined {
if (!modelId) {
return undefined;
}

// Map model IDs to their provider and model name
if (modelId.startsWith('claude-')) {
return {
type: 'anthropic',
model: modelId
};
} else if (modelId.startsWith('gpt-')) {
return {
type: 'openai',
model: modelId
};
}

return undefined;
}

namespace SessionIdForCLI {
export function getResource(sessionId: string): vscode.Uri {
Expand Down Expand Up @@ -312,12 +284,13 @@ export class CopilotCLIChatSessionContentProvider implements vscode.ChatSessionC

export class CopilotCLIChatSessionParticipant {
constructor(
private readonly copilotcliAgentManager: CopilotCLIAgentManager,
private readonly promptResolver: CopilotCLIPromptResolver,
private readonly sessionItemProvider: CopilotCLIChatSessionItemProvider,
private readonly cloudSessionProvider: CopilotChatSessionsProvider | undefined,
private readonly summarizer: ChatSummarizerProvider,
private readonly worktreeManager: CopilotCLIWorktreeManager,
@IGitService private readonly gitService: IGitService,
@ICopilotCLIModels private readonly copilotCLIModels: ICopilotCLIModels,
@ICopilotCLISessionService private readonly sessionService: ICopilotCLISessionService,
) { }

Expand All @@ -338,26 +311,29 @@ export class CopilotCLIChatSessionParticipant {
return await this.handlePushConfirmationData(request, context, stream, token);
}

if (chatSessionContext.isUntitled) {
const untitledCopilotcliSessionId = SessionIdForCLI.parse(chatSessionContext.chatSessionItem.resource);
const workingDirectory = await this.worktreeManager.createWorktreeIfNeeded(untitledCopilotcliSessionId, stream);

const { copilotcliSessionId } = await this.copilotcliAgentManager.handleRequest(undefined, request, context, stream, undefined, workingDirectory, token);
if (!copilotcliSessionId) {
stream.warning(localize('copilotcli.failedToCreateSession', "Failed to create a new CopilotCLI session."));
return {};
}
this.sessionItemProvider.swap(chatSessionContext.chatSessionItem, { resource: SessionIdForCLI.getResource(copilotcliSessionId), label: request.prompt ?? 'CopilotCLI' });
const defaultModel = await this.copilotCLIModels.getDefaultModel();
const { resource } = chatSessionContext.chatSessionItem;
const id = SessionIdForCLI.parse(resource);
const preferredModel = _sessionModel.get(id);
// For existing sessions we cannot fall back, as the model info would be updated in _sessionModel
const modelId = this.copilotCLIModels.toModelProvider(preferredModel?.id || defaultModel.id);
const { prompt, attachments } = await this.promptResolver.resolvePrompt(request, token);
const isolationEnabled = this.worktreeManager.getIsolationPreference(id);

if (chatSessionContext.isUntitled) {
const workingDirectory = isolationEnabled ? await this.worktreeManager.createWorktree(stream) : undefined;
const session = await this.sessionService.createSession(prompt, modelId, token);
if (workingDirectory) {
await this.worktreeManager.storeWorktreePath(copilotcliSessionId, workingDirectory);
await this.worktreeManager.storeWorktreePath(session.sessionId, workingDirectory);
}

await session.handleRequest(prompt, attachments, request.toolInvocationToken, stream, modelId, workingDirectory, token);

this.sessionItemProvider.swap(chatSessionContext.chatSessionItem, { resource: SessionIdForCLI.getResource(session.sessionId), label: request.prompt ?? 'CopilotCLI' });

return {};
}

const { resource } = chatSessionContext.chatSessionItem;
const id = SessionIdForCLI.parse(resource);
const session = await this.sessionService.getSession(id, undefined, false, token);
if (!session) {
stream.warning(vscode.l10n.t('Chat session not found.'));
Expand All @@ -374,12 +350,11 @@ export class CopilotCLIChatSessionParticipant {
}

const workingDirectory = this.worktreeManager.getWorktreePath(id);

await this.copilotcliAgentManager.handleRequest(id, request, context, stream, getModelProvider(_sessionModel.get(id)?.id), workingDirectory, token);
await session.handleRequest(prompt, attachments, request.toolInvocationToken, stream, modelId, workingDirectory, token);
return {};
}

private async handleDelegateCommand(session: CopilotCLISession, request: vscode.ChatRequest, context: vscode.ChatContext, stream: vscode.ChatResponseStream, token: vscode.CancellationToken) {
private async handleDelegateCommand(session: ICopilotCLISession, request: vscode.ChatRequest, context: vscode.ChatContext, stream: vscode.ChatResponseStream, token: vscode.CancellationToken) {
if (!this.cloudSessionProvider) {
stream.warning(localize('copilotcli.missingCloudAgent', "No cloud agent available"));
return {};
Expand Down Expand Up @@ -411,7 +386,7 @@ export class CopilotCLIChatSessionParticipant {
}
}

private async handleConfirmationData(session: CopilotCLISession, request: vscode.ChatRequest, context: vscode.ChatContext, stream: vscode.ChatResponseStream, token: vscode.CancellationToken) {
private async handleConfirmationData(session: ICopilotCLISession, request: vscode.ChatRequest, context: vscode.ChatContext, stream: vscode.ChatResponseStream, token: vscode.CancellationToken) {
const results: ConfirmationResult[] = [];
results.push(...(request.acceptedConfirmationData?.map(data => ({ step: data.step, accepted: true, metadata: data?.metadata })) ?? []));
results.push(...((request.rejectedConfirmationData ?? []).filter(data => !results.some(r => r.step === data.step)).map(data => ({ step: data.step, accepted: false, metadata: data?.metadata }))));
Expand Down Expand Up @@ -460,7 +435,7 @@ export class CopilotCLIChatSessionParticipant {
}

private async recordPushToSession(
session: CopilotCLISession,
session: ICopilotCLISession,
userPrompt: string,
prInfo: { uri: string; title: string; description: string; author: string; linkTag: string },
token: vscode.CancellationToken
Expand Down