Skip to content

Commit ecf836d

Browse files
When delegating to Copilot CLI do not open chat editor (#2232)
* When delegating to Copilot CLI do not open chat editor * Update src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Address code review --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 2086d1a commit ecf836d

File tree

8 files changed

+96
-95
lines changed

8 files changed

+96
-95
lines changed

src/extension/agents/copilotcli/node/copilotcliPromptResolver.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,25 @@ export class CopilotCLIPromptResolver {
2626
@IIgnoreService private readonly ignoreService: IIgnoreService,
2727
) { }
2828

29-
public async resolvePrompt(request: vscode.ChatRequest, additionalReferences: vscode.ChatPromptReference[], token: vscode.CancellationToken): Promise<{ prompt: string; attachments: Attachment[] }> {
29+
/**
30+
* Generates the final prompt for the Copilot CLI agent, resolving variables and preparing attachments.
31+
* @param request
32+
* @param prompt Provide a prompt to override the request prompt
33+
* @param additionalReferences
34+
* @param token
35+
* @returns
36+
*/
37+
public async resolvePrompt(request: vscode.ChatRequest, prompt: string | undefined, additionalReferences: vscode.ChatPromptReference[], token: vscode.CancellationToken): Promise<{ prompt: string; attachments: Attachment[] }> {
3038
const references = request.references.concat(additionalReferences);
31-
if (request.prompt.startsWith('/')) {
32-
return { prompt: request.prompt, attachments: [] }; // likely a slash command, don't modify
39+
prompt = prompt ?? request.prompt;
40+
if (prompt.startsWith('/')) {
41+
return { prompt, attachments: [] }; // likely a slash command, don't modify
3342
}
3443
const [variables, attachments] = await this.constructChatVariablesAndAttachments(new ChatVariablesCollection(references), token);
3544
if (token.isCancellationRequested) {
36-
return { prompt: request.prompt, attachments: [] };
45+
return { prompt, attachments: [] };
3746
}
38-
const prompt = await raceCancellation(generateUserPrompt(request, variables, this.instantiationService), token);
47+
prompt = await raceCancellation(generateUserPrompt(request, prompt, variables, this.instantiationService), token);
3948
return { prompt: prompt ?? '', attachments };
4049
}
4150

src/extension/agents/copilotcli/node/copilotcliSessionService.ts

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS
5858

5959
private _sessionManager: Lazy<Promise<internal.LocalSessionManager>>;
6060
private _sessionWrappers = new DisposableMap<string, RefCountedSession>();
61-
private _newActiveSessions = new Map<string, ICopilotCLISessionItem>();
6261

6362

6463
private readonly _onDidChangeSessions = new Emitter<void>();
@@ -118,10 +117,6 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS
118117
// Convert SessionMetadata to ICopilotCLISession
119118
const diskSessions: ICopilotCLISessionItem[] = coalesce(await Promise.all(
120119
sessionMetadataList.map(async (metadata) => {
121-
if (this._newActiveSessions.has(metadata.sessionId)) {
122-
// This is a new session not yet persisted to disk by SDK
123-
return undefined;
124-
}
125120
const id = metadata.sessionId;
126121
const startTime = metadata.startTime.getTime();
127122
const endTime = metadata.modifiedTime.getTime();
@@ -170,7 +165,7 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS
170165
return allSessions;
171166
} catch (error) {
172167
this.logService.error(`Failed to get all sessions: ${error}`);
173-
return Array.from(this._newActiveSessions.values());
168+
return [];
174169
}
175170
}
176171

@@ -179,26 +174,10 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS
179174
const options = await this.createSessionsOptions({ model, workingDirectory, isolationEnabled, mcpServers, agent });
180175
const sessionManager = await raceCancellationError(this.getSessionManager(), token);
181176
const sdkSession = await sessionManager.createSession(options.toSessionOptions());
182-
const label = labelFromPrompt(prompt);
183-
const newSession: ICopilotCLISessionItem = {
184-
id: sdkSession.sessionId,
185-
label,
186-
timing: { startTime: sdkSession.startTime.getTime() }
187-
};
188-
this._newActiveSessions.set(sdkSession.sessionId, newSession);
189177
this.logService.trace(`[CopilotCLISession] Created new CopilotCLI session ${sdkSession.sessionId}.`);
190178

191179

192-
const session = this.createCopilotSession(sdkSession, options, sessionManager);
193-
194-
session.object.add(toDisposable(() => this._newActiveSessions.delete(sdkSession.sessionId)));
195-
session.object.add(session.object.onDidChangeStatus(() => {
196-
// This will get swapped out as soon as the session has completed.
197-
if (session.object.status === ChatSessionStatus.Completed || session.object.status === ChatSessionStatus.Failed) {
198-
this._newActiveSessions.delete(sdkSession.sessionId);
199-
}
200-
}));
201-
return session;
180+
return this.createCopilotSession(sdkSession, options, sessionManager);
202181
}
203182

204183
protected async createSessionsOptions(options: { model?: string; isolationEnabled?: boolean; workingDirectory?: Uri; mcpServers?: SessionOptions['mcpServers']; agent: SweCustomAgent | undefined }): Promise<CopilotCLISessionOptions> {
@@ -297,7 +276,6 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS
297276
} catch (error) {
298277
this.logService.error(`Failed to delete session ${sessionId}: ${error}`);
299278
} finally {
300-
this._newActiveSessions.delete(sessionId);
301279
this._sessionWrappers.deleteAndLeak(sessionId);
302280
// Possible the session was deleted in another vscode session or the like.
303281
this._onDidChangeSessions.fire();

src/extension/agents/copilotcli/node/test/copilotcliPromptResolver.spec.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,18 @@ describe('CopilotCLIPromptResolver', () => {
5656

5757
it('returns original prompt unchanged for slash command', async () => {
5858
const req = new TestChatRequest('/help something');
59-
const { prompt, attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, [], CancellationToken.None);
59+
const { prompt, attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, undefined, [], CancellationToken.None);
6060
expect(prompt).toBe('/help something');
6161
expect(attachments).toHaveLength(0);
6262
});
6363

64+
it('returns overridden prompt instead of using the request prompt', async () => {
65+
const req = new TestChatRequest('/help something');
66+
const { prompt, attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, 'What is 1+2', [], CancellationToken.None);
67+
expect(prompt).toBe('What is 1+2');
68+
expect(attachments).toHaveLength(0);
69+
});
70+
6471
it('collects file references and produces attachments plus reminder block', async () => {
6572
// Spy on stat to simulate file type
6673
const statSpy = vi.spyOn(fileSystemService, 'stat').mockResolvedValue({ type: FileType.File, size: 10 } as any);
@@ -73,7 +80,7 @@ describe('CopilotCLIPromptResolver', () => {
7380
{ id: 'file-b', value: fileB, name: 'b.ts', range: [14, 15] } // 'b'
7481
]);
7582

76-
const { prompt, attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, [], CancellationToken.None);
83+
const { prompt, attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, undefined, [], CancellationToken.None);
7784

7885
// Should have reminder block
7986
expect(prompt).toMatch(/<reminder>/);
@@ -103,7 +110,7 @@ describe('CopilotCLIPromptResolver', () => {
103110
{ id: 'diag-1', value: chatRefDiag }
104111
]);
105112

106-
const { prompt, attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, [], CancellationToken.None);
113+
const { prompt, attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, undefined, [], CancellationToken.None);
107114

108115
expect(prompt).toMatch(/Fix issues/);
109116
expect(prompt).toMatch(/IMPORTANT: this context may or may not be relevant to your tasks. You should not respond to this context unless it is highly relevant to your task./);
@@ -122,7 +129,7 @@ describe('CopilotCLIPromptResolver', () => {
122129
{ id: 'src-dir', value: dirUri, name: 'src', range: [5, 8] }
123130
]);
124131

125-
const { attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, [], CancellationToken.None);
132+
const { attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, undefined, [], CancellationToken.None);
126133
expect(attachments).toHaveLength(1);
127134
expect(attachments[0].type).toBe('directory');
128135
expect(attachments[0].displayName).toBe('src');
@@ -138,7 +145,7 @@ describe('CopilotCLIPromptResolver', () => {
138145
{ id: 'bad', value: badUri, name: 'unknown', range: [6, 13] }
139146
]);
140147

141-
const { attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, [], CancellationToken.None);
148+
const { attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, undefined, [], CancellationToken.None);
142149
expect(attachments).toHaveLength(0); // ignored
143150
expect(statSpy).toHaveBeenCalledTimes(1);
144151
expect(logSpy).toHaveBeenCalled();
@@ -153,15 +160,15 @@ describe('CopilotCLIPromptResolver', () => {
153160
{ id: 'file', value: fileUri, name: 'index.ts', range: [5, 10] }
154161
]);
155162

156-
const { attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, [], CancellationToken.None);
163+
const { attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, undefined, [], CancellationToken.None);
157164
expect(attachments).toHaveLength(0);
158165
expect(statSpy).toHaveBeenCalledTimes(1);
159166
expect(logSpy).toHaveBeenCalled();
160167
});
161168

162169
it('no reminder block when there are no references or diagnostics', async () => {
163170
const req = new TestChatRequest('Just a question');
164-
const { prompt } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, [], CancellationToken.None);
171+
const { prompt } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, undefined, [], CancellationToken.None);
165172
expect(prompt).toBe('Just a question');
166173
});
167174

@@ -191,7 +198,7 @@ describe('CopilotCLIPromptResolver', () => {
191198
regularFileRef
192199
]);
193200

194-
const { attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, [], CancellationToken.None);
201+
const { attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, undefined, [], CancellationToken.None);
195202

196203
// Prompt instructions should be filtered out, so only 2 attachments (prompt file + regular file)
197204
expect(attachments).toHaveLength(2);

src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts

Lines changed: 50 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { ConfigKey, IConfigurationService } from '../../../platform/configuratio
1111
import { IVSCodeExtensionContext } from '../../../platform/extContext/common/extensionContext';
1212
import { IGitService } from '../../../platform/git/common/gitService';
1313
import { toGitUri } from '../../../platform/git/common/utils';
14+
import { ILogService } from '../../../platform/log/common/logService';
1415
import { ITelemetryService } from '../../../platform/telemetry/common/telemetry';
1516
import { disposableTimeout } from '../../../util/vs/base/common/async';
1617
import { isCancellationError } from '../../../util/vs/base/common/errors';
@@ -406,10 +407,10 @@ export class CopilotCLIChatSessionParticipant extends Disposable {
406407
@ICopilotCLISessionService private readonly sessionService: ICopilotCLISessionService,
407408
@ITelemetryService private readonly telemetryService: ITelemetryService,
408409
@IToolsService private readonly toolsService: IToolsService,
409-
@IRunCommandExecutionService private readonly commandExecutionService: IRunCommandExecutionService,
410410
@IInstantiationService private readonly instantiationService: IInstantiationService,
411411
@IConfigurationService private readonly configurationService: IConfigurationService,
412412
@ICopilotCLISDK private readonly copilotCLISDK: ICopilotCLISDK,
413+
@ILogService private readonly logService: ILogService,
413414
) {
414415
super();
415416
}
@@ -441,7 +442,8 @@ export class CopilotCLIChatSessionParticipant extends Disposable {
441442

442443
const confirmationResults = this.getAcceptedRejectedConfirmationData(request);
443444
if (!chatSessionContext) {
444-
/* Invoked from a 'normal' chat or 'cloud button' without CLI session context */
445+
// Invoked from a 'normal' chat or 'cloud button' without CLI session context
446+
// Or cases such as delegating from Regular chat to CLI chat
445447
// Handle confirmation data
446448
return await this.handlePushConfirmationData(request, context, stream, token);
447449
}
@@ -452,7 +454,7 @@ export class CopilotCLIChatSessionParticipant extends Disposable {
452454
const additionalReferences = this.previousReferences.get(id) || [];
453455
this.previousReferences.delete(id);
454456
const [{ prompt, attachments }, modelId, sessionAgent, defaultAgent] = await Promise.all([
455-
this.promptResolver.resolvePrompt(request, additionalReferences, token),
457+
this.promptResolver.resolvePrompt(request, undefined, additionalReferences, token),
456458
this.getModelId(id),
457459
this.copilotCLIAgents.getSessionAgent(id),
458460
this.copilotCLIAgents.getDefaultAgent()
@@ -527,8 +529,8 @@ export class CopilotCLIChatSessionParticipant extends Disposable {
527529
return session;
528530
}
529531

530-
private async getModelId(sessionId: string): Promise<string | undefined> {
531-
const preferredModelId = _sessionModel.get(sessionId)?.id ?? (await this.copilotCLIModels.getDefaultModel())?.id;
532+
private async getModelId(sessionId: string | undefined): Promise<string | undefined> {
533+
const preferredModelId = (sessionId ? _sessionModel.get(sessionId)?.id : undefined) ?? (await this.copilotCLIModels.getDefaultModel())?.id;
532534
return preferredModelId ? this.copilotCLIModels.toModelProvider(preferredModelId) : undefined;
533535
}
534536

@@ -598,15 +600,15 @@ export class CopilotCLIChatSessionParticipant extends Disposable {
598600
const isolationEnabled = this.configurationService.getConfig(ConfigKey.Advanced.CLIIsolationEnabled);
599601
if (!isolationEnabled) {
600602
// No isolation, proceed without worktree
601-
return await this.createCLISessionAndOpen(request.prompt, request.references, context, undefined, false, stream, token);
603+
return await this.createCLISessionAndSubmitRequest(request, undefined, request.references, context, undefined, false, stream, token);
602604
}
603605

604606
// Check for uncommitted changes
605607
const currentRepository = this.gitService.activeRepository?.get();
606608
const hasUncommittedChanges = currentRepository?.changes && (currentRepository.changes.indexChanges.length > 0 || currentRepository.changes.workingTree.length > 0);
607609
if (!hasUncommittedChanges) {
608610
// No uncommitted changes, create worktree and proceed
609-
return await this.createCLISessionAndOpen(request.prompt, request.references, context, undefined, true, stream, token);
611+
return await this.createCLISessionAndSubmitRequest(request, undefined, request.references, context, undefined, true, stream, token);
610612
}
611613

612614
const message =
@@ -667,7 +669,7 @@ export class CopilotCLIChatSessionParticipant extends Disposable {
667669
const worktreePath = worktreePathValue ? URI.file(worktreePathValue) : undefined;
668670
if (!worktreePath) {
669671
stream.warning(vscode.l10n.t('Failed to create worktree. Proceeding without isolation.'));
670-
return await this.createCLISessionAndOpen(prompt, references, context, undefined, false, stream, token);
672+
return await this.createCLISessionAndSubmitRequest(request, prompt, references, context, undefined, false, stream, token);
671673
}
672674

673675
// Migrate changes from active repository to worktree
@@ -713,62 +715,74 @@ export class CopilotCLIChatSessionParticipant extends Disposable {
713715
}
714716
}
715717

716-
return await this.createCLISessionAndOpen(prompt, references, context, worktreePath, true, stream, token);
718+
return await this.createCLISessionAndSubmitRequest(request, prompt, references, context, worktreePath, true, stream, token);
717719
} else {
718720
// Skip changes, just create worktree without migration
719-
return await this.createCLISessionAndOpen(prompt, references, context, undefined, true, stream, token);
721+
return await this.createCLISessionAndSubmitRequest(request, prompt, references, context, undefined, true, stream, token);
720722
}
721723
}
722724

723-
private async createCLISessionAndOpen(
724-
prompt: string,
725+
private async createCLISessionAndSubmitRequest(
726+
request: vscode.ChatRequest,
727+
userPrompt: string | undefined,
725728
references: readonly vscode.ChatPromptReference[] | undefined,
726729
context: vscode.ChatContext,
727730
workingDirectory: Uri | undefined,
728731
isolationEnabled: boolean,
729732
stream: vscode.ChatResponseStream,
730733
token: vscode.CancellationToken
731-
): Promise<vscode.ChatResult | void> {
734+
): Promise<vscode.ChatResult> {
732735
let history: string | undefined;
733736

734737
if (this.hasHistoryToSummarize(context.history)) {
735738
stream.progress(vscode.l10n.t('Analyzing chat history'));
736739
history = await this.summarizer.provideChatSummary(context, token);
737740
}
738741

739-
const requestPrompt = history ? `${prompt}\n**Summary**\n${history}` : prompt;
742+
// Give priority to userPrompt if provided (e.g., from confirmation metadata)
743+
userPrompt = userPrompt || request.prompt;
744+
const requestPrompt = history ? `${userPrompt}\n**Summary**\n${history}` : userPrompt;
740745

741746
// Create worktree if isolation is enabled and we don't have one yet
742-
let finalWorkingDirectory = workingDirectory;
743-
if (isolationEnabled && !finalWorkingDirectory) {
747+
if (isolationEnabled && !workingDirectory) {
744748
const workTreePath = await this.worktreeManager.createWorktree(stream);
745-
finalWorkingDirectory = workTreePath ? URI.file(workTreePath) : undefined;
749+
workingDirectory = workTreePath ? URI.file(workTreePath) : undefined;
746750
}
747751

748752
// Fallback to default directory if worktree creation failed
749-
if (!finalWorkingDirectory && !isolationEnabled) {
750-
finalWorkingDirectory = await this.copilotCLISDK.getDefaultWorkingDirectory();
753+
if (!workingDirectory && !isolationEnabled) {
754+
workingDirectory = await this.copilotCLISDK.getDefaultWorkingDirectory();
751755
}
756+
const [{ prompt, attachments }, model, agent] = await Promise.all([
757+
this.promptResolver.resolvePrompt(request, requestPrompt, (references || []).concat([]), token),
758+
this.getModelId(undefined),
759+
this.copilotCLIAgents.getDefaultAgent().then(agent => this.copilotCLIAgents.resolveAgent(agent))
760+
]);
752761

753-
const session = await this.sessionService.createSession(requestPrompt, { workingDirectory: finalWorkingDirectory, isolationEnabled }, token);
762+
const session = await this.sessionService.createSession(requestPrompt, { workingDirectory, isolationEnabled, agent, model }, token);
763+
this.copilotCLIAgents.trackSessionAgent(session.object.sessionId, agent?.name);
764+
765+
// Do not await, we want this code path to be as fast as possible.
766+
if (isolationEnabled && workingDirectory) {
767+
void this.worktreeManager.storeWorktreePath(session.object.sessionId, workingDirectory.fsPath);
768+
}
769+
770+
// We don't want to block the caller anymore.
771+
// The caller is most likely a chat editor or the like.
772+
// Now that we've delegated it to a session, we can get out of here.
773+
// Else if the request takes say 10 minutes, the caller would be blocked for that long.
774+
session.object.handleRequest(request.id, prompt, attachments, model, token)
775+
.catch(error => {
776+
this.logService.error(`Failed to handle CLI session request: ${error}`);
777+
// Optionally: stream.error(error) to notify the user
778+
})
779+
.finally(() => {
780+
session.dispose();
781+
});
754782

755-
if (finalWorkingDirectory) {
756-
await this.worktreeManager.storeWorktreePath(session.object.sessionId, finalWorkingDirectory.fsPath);
757-
}
783+
stream.markdown(vscode.l10n.t('{0} agent has begun working on your request. Follow its progress in the Agents View.', 'Background Agent'));
758784

759-
try {
760-
// Since we're going to open a new session, store the references for later use.
761-
this.previousReferences.set(session.object.sessionId, (references || []).concat([]));
762-
await this.commandExecutionService.executeCommand('vscode.open', SessionIdForCLI.getResource(session.object.sessionId));
763-
await this.commandExecutionService.executeCommand('workbench.action.chat.submit', { inputValue: requestPrompt });
764-
return {};
765-
}
766-
finally {
767-
// The SDK doesn't save the session as no messages were added,
768-
// If we dispose this here, then we will not be able to find this session later.
769-
// So leave this session alive till it gets used using the `getSession` API later
770-
this._register(disposableTimeout(() => session.dispose(), WAIT_FOR_NEW_SESSION_TO_GET_USED));
771-
}
785+
return {};
772786
}
773787

774788
private hasHistoryToSummarize(history: readonly (vscode.ChatRequestTurn | vscode.ChatResponseTurn)[]): boolean {

0 commit comments

Comments
 (0)