豆豆友情提示:这是一个非官方 GitHub 代理镜像,主要用于网络测试或访问加速。请勿在此进行登录、注册或处理任何敏感信息。进行这些操作请务必访问官方网站 github.com。 Raw 内容也通过此代理提供。
Skip to content

Commit ea7be1d

Browse files
committed
Copilot CLI Plan exit approval
1 parent 7523b99 commit ea7be1d

File tree

5 files changed

+200
-132
lines changed

5 files changed

+200
-132
lines changed

src/extension/chatSessions/copilotcli/node/copilotcliSession.ts

Lines changed: 36 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,24 @@ import { IWorkspaceService } from '../../../../platform/workspace/common/workspa
1414
import { raceCancellation } from '../../../../util/vs/base/common/async';
1515
import { CancellationToken } from '../../../../util/vs/base/common/cancellation';
1616
import { Codicon } from '../../../../util/vs/base/common/codicons';
17-
import { Emitter, Event } from '../../../../util/vs/base/common/event';
17+
import { Emitter } from '../../../../util/vs/base/common/event';
1818
import { DisposableStore, IDisposable, toDisposable } from '../../../../util/vs/base/common/lifecycle';
1919
import { ResourceMap } from '../../../../util/vs/base/common/map';
2020
import { extUriBiasedIgnorePathCase, isEqual } from '../../../../util/vs/base/common/resources';
2121
import { ThemeIcon } from '../../../../util/vs/base/common/themables';
2222
import { IInstantiationService } from '../../../../util/vs/platform/instantiation/common/instantiation';
23-
import { ChatRequestTurn2, ChatResponseThinkingProgressPart, ChatResponseTurn2, ChatSessionStatus, ChatToolInvocationPart, EventEmitter, Uri } from '../../../../vscodeTypes';
23+
import { ChatRequestTurn2, ChatResponseThinkingProgressPart, ChatResponseTurn2, ChatSessionStatus, ChatToolInvocationPart, EventEmitter, LanguageModelTextPart, Uri } from '../../../../vscodeTypes';
24+
import { ToolName } from '../../../tools/common/toolNames';
2425
import { IToolsService } from '../../../tools/common/toolsService';
2526
import { ExternalEditTracker } from '../../common/externalEditTracker';
27+
import { getWorkingDirectory, isIsolationEnabled, IWorkspaceInfo } from '../../common/workspaceInfo';
2628
import { buildChatHistoryFromEvents, getAffectedUrisForEditTool, isCopilotCliEditToolCall, isCopilotCLIToolThatCouldRequirePermissions, processToolExecutionComplete, processToolExecutionStart, ToolCall, UnknownToolCall, updateTodoList } from '../common/copilotCLITools';
2729
import { IChatDelegationSummaryService } from '../common/delegationSummaryService';
28-
import { IWorkspaceInfo, getWorkingDirectory, isIsolationEnabled } from '../../common/workspaceInfo';
2930
import { getCopilotCLISessionStateDir } from './cliHelpers';
3031
import { CopilotCLISessionOptions, ICopilotCLISDK } from './copilotCli';
3132
import { ICopilotCLIImageSupport } from './copilotCLIImageSupport';
32-
import { PermissionRequest, requiresFileEditconfirmation } from './permissionHelpers';
33-
import { IUserQuestionHandler, UserInputRequest, UserInputResponse } from './userInputHelpers';
33+
import { PermissionRequest, requestPermission, requiresFileEditconfirmation } from './permissionHelpers';
34+
import { IUserQuestionHandler, UserInputRequest } from './userInputHelpers';
3435

3536
/**
3637
* Known commands that can be sent to a CopilotCLI session instead of a free-form prompt.
@@ -44,37 +45,21 @@ export type CopilotCLICommand = 'compact' | 'mcp';
4445
export const copilotCLICommands: readonly CopilotCLICommand[] = ['compact', 'mcp'] as const;
4546

4647
/**
47-
* Discriminated-union input for {@link ICopilotCLISession.handleRequest}.
48-
*
4948
* Either a free-form prompt **or** a known command.
5049
*/
5150
export type CopilotCLISessionInput =
5251
| { readonly prompt: string; plan?: boolean }
5352
| { readonly command: CopilotCLICommand };
5453

55-
type PermissionHandler = (
56-
permissionRequest: PermissionRequest,
57-
toolCall: ToolCall | undefined,
58-
token: CancellationToken,
59-
) => Promise<boolean>;
60-
61-
type UserInputHandler = (
62-
userInputRequest: UserInputRequest,
63-
toolCall: ToolCall | undefined,
64-
token: CancellationToken,
65-
) => Promise<UserInputResponse>;
6654

6755
export interface ICopilotCLISession extends IDisposable {
6856
readonly sessionId: string;
6957
readonly title?: string;
7058
readonly onDidChangeTitle: vscode.Event<string>;
7159
readonly status: vscode.ChatSessionStatus | undefined;
7260
readonly onDidChangeStatus: vscode.Event<vscode.ChatSessionStatus | undefined>;
73-
readonly permissionRequested?: PermissionRequest;
74-
readonly onPermissionRequested: vscode.Event<PermissionRequest>;
7561
readonly workspace: IWorkspaceInfo;
7662
readonly pendingPrompt: string | undefined;
77-
attachPermissionHandler(handler: PermissionHandler): IDisposable;
7863
attachStream(stream: vscode.ChatResponseStream): IDisposable;
7964
setPermissionLevel(level: string | undefined): void;
8065
handleRequest(
@@ -105,18 +90,6 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes
10590
public get permissionRequested(): PermissionRequest | undefined {
10691
return this._permissionRequested;
10792
}
108-
private readonly _onPermissionRequested = this.add(new EventEmitter<PermissionRequest>());
109-
public readonly onPermissionRequested = this._onPermissionRequested.event;
110-
private _permissionHandler?: PermissionHandler;
111-
private readonly _permissionHandlerSet = this.add(new Emitter<void>());
112-
private readonly _onUserInputRequested = this.add(new EventEmitter<UserInputRequest>());
113-
public readonly onUserInputRequested = this._onUserInputRequested.event;
114-
private _userInputHandler?: UserInputHandler;
115-
private readonly _userInputHandlerSet = this.add(new Emitter<void>());
116-
private _userInputRequested?: UserInputRequest;
117-
public get userInputRequested(): UserInputRequest | undefined {
118-
return this._userInputRequested;
119-
}
12093
private _title?: string;
12194
public get title(): string | undefined {
12295
return this._title;
@@ -163,26 +136,6 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes
163136
});
164137
}
165138

166-
attachPermissionHandler(handler: PermissionHandler): IDisposable {
167-
this._permissionHandler = handler;
168-
this._permissionHandlerSet.fire();
169-
return toDisposable(() => {
170-
if (this._permissionHandler === handler) {
171-
this._permissionHandler = undefined;
172-
}
173-
});
174-
}
175-
176-
attachUserInputHandler(handler: UserInputHandler): IDisposable {
177-
this._userInputHandler = handler;
178-
this._userInputHandlerSet.fire();
179-
return toDisposable(() => {
180-
if (this._userInputHandler === handler) {
181-
this._userInputHandler = undefined;
182-
}
183-
});
184-
}
185-
186139
public setPermissionLevel(level: string | undefined): void {
187140
this._permissionLevel = level;
188141
}
@@ -375,6 +328,35 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes
375328

376329
this._sdkSession.respondToPermission(requestId, response);
377330
})));
331+
disposables.add(toDisposable(this._sdkSession.on('exit_plan_mode.requested', async (event) => {
332+
if (!this._stream || !(this._toolInvocationToken as unknown)) {
333+
this.logService.warn('[ConfirmationTool] No stream available, cannot show question carousel');
334+
this._sdkSession.respondToExitPlanMode(event.data.requestId, { approved: false });
335+
return;
336+
}
337+
const params = {
338+
title: l10n.t('Approve this plan?'),
339+
message: event.data.summary,
340+
confirmationType: 'basic' as const,
341+
};
342+
343+
let approved = false;
344+
try {
345+
const result = await this._toolsService.invokeTool(ToolName.CoreConfirmationTool, {
346+
input: params,
347+
toolInvocationToken: this._toolInvocationToken,
348+
}, CancellationToken.None);
349+
350+
const firstResultPart = result.content.at(0);
351+
approved = firstResultPart instanceof LanguageModelTextPart && firstResultPart.value === 'yes';
352+
} catch (error) {
353+
this.logService.error(error, '[ConfirmationTool] Error showing confirmation tool for exit plan mode');
354+
approved = false;
355+
}
356+
357+
this._sdkSession.respondToExitPlanMode(event.data.requestId, { approved });
358+
359+
})));
378360
disposables.add(toDisposable(this._sdkSession.on('user_input.requested', async (event) => {
379361
if (!this._stream || !(this._toolInvocationToken as unknown)) {
380362
this.logService.warn('[AskQuestionsTool] No stream available, cannot show question carousel');
@@ -786,13 +768,7 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes
786768
}
787769

788770
try {
789-
const permissionHandler = await this.waitForPermissionHandler(permissionRequest);
790-
if (!permissionHandler) {
791-
this.logService.warn(`[CopilotCLISession] No permission handler registered, denying request for ${permissionRequest.kind} permission.`);
792-
return { kind: 'denied-interactively-by-user' };
793-
}
794-
795-
if (await permissionHandler(permissionRequest, toolCall, token)) {
771+
if (await requestPermission(this.instantiationService, permissionRequest, toolCall, getWorkingDirectory(this.workspace), this._toolsService, this._toolInvocationToken as unknown as never, token)) {
796772
// If we're editing a file, start tracking the edit & wait for core to acknowledge it.
797773
if (editFile && toolCall && this._stream) {
798774
this.logService.trace(`[CopilotCLISession] Starting to track edit for toolCallId ${toolCall.toolCallId} & file ${editFile.fsPath}`);
@@ -809,18 +785,6 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes
809785
return { kind: 'denied-interactively-by-user' };
810786
}
811787

812-
private async waitForPermissionHandler(permissionRequest: PermissionRequest): Promise<PermissionHandler | undefined> {
813-
if (!this._permissionHandler) {
814-
this._permissionRequested = permissionRequest;
815-
this._onPermissionRequested.fire(permissionRequest);
816-
const disposables = this.add(new DisposableStore());
817-
await Event.toPromise(this._permissionHandlerSet.event, disposables);
818-
disposables.dispose();
819-
this._permissionRequested = undefined;
820-
}
821-
return this._permissionHandler;
822-
}
823-
824788
private _logRequest(userPrompt: string, modelId: string, attachments: Attachment[], startTimeMs: number): void {
825789
const markdownContent = this._renderRequestToMarkdown(userPrompt, modelId, attachments, startTimeMs);
826790
this._requestLogger.addEntry({

src/extension/chatSessions/copilotcli/node/test/copilotcliSession.spec.ts

Lines changed: 79 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ describe('CopilotCLISession', () => {
143143
let instaService: IInstantiationService;
144144
let sdk: ICopilotCLISDK;
145145
let requestLogger: IRequestLogger;
146+
let toolsService: FakeToolsService;
146147
const delegationService = new class extends mock<IChatDelegationSummaryService>() {
147148
override async summarize(context: ChatContext, token: CancellationToken): Promise<string | undefined> {
148149
return undefined;
@@ -168,6 +169,7 @@ describe('CopilotCLISession', () => {
168169
workspaceService = createWorkspaceService('/workspace');
169170
sessionOptions = new CopilotCLISessionOptions({ workspaceInfo: workspaceInfoFor(workspaceService.getWorkspaceFolders()![0]) }, logger);
170171
instaService = services.seal();
172+
toolsService = new FakeToolsService();
171173
});
172174

173175
afterEach(() => {
@@ -193,7 +195,7 @@ describe('CopilotCLISession', () => {
193195
delegationService,
194196
requestLogger,
195197
new NullICopilotCLIImageSupport(),
196-
new FakeToolsService(),
198+
toolsService,
197199
new FakeUserQuestionHandler()
198200
));
199201
}
@@ -330,6 +332,7 @@ describe('CopilotCLISession', () => {
330332
let result: unknown;
331333
const nonAttachedFilePath = '/outside-workspace/other-file.ts';
332334
const attachedFilePath = '/outside-workspace/attached-file.ts';
335+
toolsService.setConfirmationResult('no');
333336
sdkSession.send = async ({ prompt }: any) => {
334337
sdkSession.emit('assistant.turn_start', {});
335338
sdkSession.emit('assistant.message', { content: `Echo: ${prompt}` });
@@ -339,11 +342,11 @@ describe('CopilotCLISession', () => {
339342
const session = await createSession();
340343
const stream = new MockChatResponseStream();
341344
session.attachStream(stream);
342-
disposables.add(session.attachPermissionHandler(async () => false));
343345

344346
const attachments = [{ type: 'file' as const, path: attachedFilePath, displayName: 'attached-file.ts' }];
345347
await session.handleRequest({ id: '', toolInvocationToken: undefined as never }, { prompt: 'Test' }, attachments as any, undefined, authInfo, CancellationToken.None);
346348
expect(result).toEqual({ kind: 'denied-interactively-by-user' });
349+
expect(toolsService.invokeToolCalls).toHaveLength(1);
347350
});
348351

349352
it('auto-approves read permission inside working directory without external handler', async () => {
@@ -421,7 +424,7 @@ describe('CopilotCLISession', () => {
421424

422425
it('requires read permission outside workspace and working directory', async () => {
423426
let result: unknown;
424-
let askedForPermission: PermissionRequest | undefined = undefined;
427+
toolsService.setConfirmationResult('no');
425428
sdkSession.send = async ({ prompt }: any) => {
426429
sdkSession.emit('assistant.turn_start', {});
427430
sdkSession.emit('assistant.message', { content: `Echo: ${prompt}` });
@@ -434,25 +437,20 @@ describe('CopilotCLISession', () => {
434437
const stream = new MockChatResponseStream();
435438
session.attachStream(stream);
436439

437-
disposables.add(session.attachPermissionHandler((permission) => {
438-
askedForPermission = permission;
439-
return Promise.resolve(false);
440-
}));
441-
442440
// Path must be absolute within workspace, should auto-approve
443441
await session.handleRequest({ id: '', toolInvocationToken: undefined as never }, { prompt: 'Test' }, [], undefined, authInfo, CancellationToken.None);
444-
const file = path.join('/workingDirectory', 'file.ts');
445442
expect(result).toEqual({ kind: 'denied-interactively-by-user' });
446-
expect(askedForPermission).not.toBeUndefined();
447-
expect(askedForPermission!.kind).toBe('read');
448-
expect((askedForPermission as unknown as { path: string })!.path).toBe(file);
443+
expect(toolsService.invokeToolCalls).toHaveLength(1);
444+
expect(toolsService.invokeToolCalls[0].input).toMatchObject({
445+
title: 'Read file(s)',
446+
message: 'Read file'
447+
});
449448
});
450449

451450
it('approves write permission when handler returns true', async () => {
452451
let result: unknown;
453452
const session = await createSession();
454-
// Register approval handler
455-
disposables.add(session.attachPermissionHandler(async () => true));
453+
toolsService.setConfirmationResult('yes');
456454
sdkSession.send = async ({ prompt }: any) => {
457455
sdkSession.emit('assistant.turn_start', {});
458456
sdkSession.emit('assistant.message', { content: `Echo: ${prompt}` });
@@ -471,7 +469,7 @@ describe('CopilotCLISession', () => {
471469
it('denies write permission when handler returns false', async () => {
472470
let result: unknown;
473471
const session = await createSession();
474-
session.attachPermissionHandler(async () => false);
472+
toolsService.setConfirmationResult('no');
475473
sdkSession.send = async ({ prompt }: any) => {
476474
sdkSession.emit('assistant.turn_start', {});
477475
sdkSession.emit('assistant.message', { content: `Echo: ${prompt}` });
@@ -489,7 +487,9 @@ describe('CopilotCLISession', () => {
489487
it('denies write permission when handler throws', async () => {
490488
let result: unknown;
491489
const session = await createSession();
492-
session.attachPermissionHandler(async () => { throw new Error('oops'); });
490+
toolsService.invokeTool = vi.fn(async () => {
491+
throw new Error('oops');
492+
});
493493
sdkSession.send = async ({ prompt }: any) => {
494494
sdkSession.emit('assistant.turn_start', {});
495495
sdkSession.emit('assistant.message', { content: `Echo: ${prompt}` });
@@ -509,7 +509,7 @@ describe('CopilotCLISession', () => {
509509
let resolveSend: () => void;
510510
sdkSession.send = async () => new Promise<void>(r => { resolveSend = r; });
511511
const session = await createSession();
512-
session.attachPermissionHandler(async () => true);
512+
toolsService.setConfirmationResult('yes');
513513
const stream = new MockChatResponseStream();
514514
session.attachStream(stream);
515515
// Spy on trackEdit to capture ordering (we don't want to depend on externalEdit mechanics here)
@@ -578,7 +578,7 @@ describe('CopilotCLISession', () => {
578578
const pushedParts: unknown[] = [];
579579
const stream = new MockChatResponseStream(part => pushedParts.push(part));
580580
session.attachStream(stream);
581-
disposables.add(session.attachPermissionHandler(async () => true));
581+
toolsService.setConfirmationResult('yes');
582582

583583
const requestPromise = session.handleRequest({ id: '', toolInvocationToken: undefined as never }, { prompt: 'Run bash' }, [], undefined, authInfo, CancellationToken.None);
584584
await new Promise(r => setTimeout(r, 0));
@@ -664,6 +664,19 @@ describe('CopilotCLISession', () => {
664664
await requestPromise;
665665
});
666666

667+
describe('/compact command', () => {
668+
it('compacts the conversation and reports success', async () => {
669+
const session = await createSession();
670+
const stream = new MockChatResponseStream();
671+
session.attachStream(stream);
672+
673+
await session.handleRequest({ id: '', toolInvocationToken: undefined as never }, { command: 'compact' }, [], undefined, authInfo, CancellationToken.None);
674+
675+
expect(sdkSession.currentMode).toBe('interactive');
676+
expect(stream.output.join('\n')).toContain('Compacted conversation.');
677+
});
678+
});
679+
667680
describe('/mcp command', () => {
668681
it('shows no servers message when no MCP tools are loaded', async () => {
669682
sdkSession.toolMetadata = [];
@@ -699,6 +712,54 @@ describe('CopilotCLISession', () => {
699712
});
700713

701714
describe('steering (sending messages to a busy session)', () => {
715+
it('allows steering after an earlier failed request', async () => {
716+
sdkSession.send = async () => {
717+
throw new Error('boom');
718+
};
719+
720+
const session = await createSession();
721+
const stream = new MockChatResponseStream();
722+
session.attachStream(stream);
723+
724+
await session.handleRequest(
725+
{ id: 'req-1', toolInvocationToken: undefined as never },
726+
{ prompt: 'Initial failure' }, [], undefined, authInfo, CancellationToken.None
727+
);
728+
expect(session.status).toBe(ChatSessionStatus.Failed);
729+
730+
let resolveSecondSend!: () => void;
731+
let sendCallCount = 0;
732+
sdkSession.send = async (options: any) => {
733+
sendCallCount++;
734+
sdkSession.lastSendOptions = options;
735+
if (sendCallCount === 1) {
736+
await new Promise<void>(r => { resolveSecondSend = r; });
737+
}
738+
sdkSession.emit('assistant.turn_start', {});
739+
sdkSession.emit('assistant.message', { content: `Echo: ${options.prompt}` });
740+
sdkSession.emit('assistant.turn_end', {});
741+
};
742+
743+
const secondRequest = session.handleRequest(
744+
{ id: 'req-2', toolInvocationToken: undefined as never },
745+
{ prompt: 'Second request' }, [], undefined, authInfo, CancellationToken.None
746+
);
747+
await new Promise(r => setTimeout(r, 10));
748+
749+
const steeringRequest = session.handleRequest(
750+
{ id: 'req-3', toolInvocationToken: undefined as never },
751+
{ prompt: 'Steer after failure' }, [], undefined, authInfo, CancellationToken.None
752+
);
753+
await new Promise(r => setTimeout(r, 10));
754+
755+
expect(sdkSession.lastSendOptions?.mode).toBe('immediate');
756+
expect(sdkSession.lastSendOptions?.prompt).toBe('Steer after failure');
757+
758+
resolveSecondSend();
759+
await Promise.all([secondRequest, steeringRequest]);
760+
expect(session.status).toBe(ChatSessionStatus.Completed);
761+
});
762+
702763
it('routes through steering when session is already InProgress', async () => {
703764
// Arrange: make `send` block so the first request stays in progress
704765
let resolveFirstSend: () => void = () => { };

0 commit comments

Comments
 (0)