Skip to content

Commit 82bd5cb

Browse files
feat: track mapping of real copilot cli sessions to untitled sessions (#4696)
* feat: track mapping of real copilot cli sessions to untitled sessions * Update comments * 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> * wip * remove tests --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 09499c7 commit 82bd5cb

3 files changed

Lines changed: 75 additions & 7 deletions

File tree

.vscode/settings.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"files.trimTrailingWhitespace": true,
33
"[typescript]": {
44
"editor.insertSpaces": false,
5-
"editor.defaultFormatter": "vscode.typescript-language-features",
5+
"editor.defaultFormatter": "TypeScriptTeam.native-preview",
66
"editor.formatOnSave": true,
77
"editor.codeActionsOnSave": {
88
"source.organizeImports": "always"

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { DeferredPromise, disposableTimeout, IntervalTimer, SequencerByKey, Thro
2626
import { CancellationToken } from '../../../util/vs/base/common/cancellation';
2727
import { isCancellationError } from '../../../util/vs/base/common/errors';
2828
import { Emitter, Event } from '../../../util/vs/base/common/event';
29-
import { Disposable, DisposableStore, IDisposable, IReference, toDisposable } from '../../../util/vs/base/common/lifecycle';
29+
import { Disposable, DisposableStore, IDisposable, IReference } from '../../../util/vs/base/common/lifecycle';
3030
import { relative } from '../../../util/vs/base/common/path';
3131
import { basename, dirname, extUri, isEqual } from '../../../util/vs/base/common/resources';
3232
import { URI } from '../../../util/vs/base/common/uri';
@@ -156,6 +156,11 @@ export class CopilotCLIChatSessionItemProvider extends Disposable implements vsc
156156
// There's an issue in core (about holding onto ref of the Chat Model).
157157
// As a temporary solution, return the same untitled session id back to core until the session is completed.
158158
public readonly untitledSessionIdMapping = new Map<string, string>();
159+
/**
160+
* Until the untitled session is properly swappped with the new session, we should keep track of this mapping.
161+
* When VS Code asks for the session, always return the old untitled session Uri.
162+
*/
163+
public readonly sdkToUntitledUriMapping = new Map<string, Uri>();
159164
private readonly _onDidChangeChatSessionItems = this._register(new Emitter<void>());
160165
public readonly onDidChangeChatSessionItems: Event<void> = this._onDidChangeChatSessionItems.event;
161166

@@ -236,7 +241,7 @@ export class CopilotCLIChatSessionItemProvider extends Disposable implements vsc
236241
}
237242

238243
public async toChatSessionItem(session: ICopilotCLISessionItem): Promise<vscode.ChatSessionItem> {
239-
const resource = SessionIdForCLI.getResource(this.untitledSessionIdMapping.get(session.id) ?? session.id);
244+
const resource = this.sdkToUntitledUriMapping.get(session.id) ?? SessionIdForCLI.getResource(this.untitledSessionIdMapping.get(session.id) ?? session.id);
240245
const worktreeProperties = await this.worktreeManager.getWorktreeProperties(session.id);
241246
const workingDirectory = worktreeProperties?.worktreePath ? vscode.Uri.file(worktreeProperties.worktreePath)
242247
: session.workingDirectory;
@@ -1359,9 +1364,6 @@ export class CopilotCLIChatSessionParticipant extends Disposable {
13591364
sdkSessionId = session.object.sessionId;
13601365
const modeInstructions = this.createModeInstructions(request);
13611366
this.chatSessionMetadataStore.updateRequestDetails(sessionId, [{ vscodeRequestId: request.id, agentId: agent?.name ?? '', modeInstructions }]).catch(ex => this.logService.error(ex, 'Failed to update request details'));
1362-
if (isUntitled) {
1363-
disposables.add(toDisposable(() => this.sessionItemProvider.untitledSessionIdMapping.delete(session.object.sessionId)));
1364-
}
13651367

13661368
// Lock the repo option with more accurate information.
13671369
// Previously we just updated it with details of the folder.
@@ -1421,7 +1423,7 @@ export class CopilotCLIChatSessionParticipant extends Disposable {
14211423
_sessionBranch.delete(id);
14221424
_sessionIsolation.delete(id);
14231425
this.sessionItemProvider.untitledSessionIdMapping.delete(id);
1424-
this.sessionItemProvider.untitledSessionIdMapping.delete(session.object.sessionId);
1426+
this.sessionItemProvider.sdkToUntitledUriMapping.delete(session.object.sessionId);
14251427
this.folderRepositoryManager.deleteNewSessionFolder(id);
14261428
this.sessionItemProvider.swap(chatSessionContext.chatSessionItem, { resource: SessionIdForCLI.getResource(session.object.sessionId), label: request.prompt });
14271429
}
@@ -1721,6 +1723,7 @@ export class CopilotCLIChatSessionParticipant extends Disposable {
17211723
this.logService.info(`Using Copilot CLI session: ${session.object.sessionId} (isNewSession: ${isNewSession}, isolationEnabled: ${isIsolationEnabled(workspaceInfo)}, workingDirectory: ${workingDirectory}, worktreePath: ${worktreeProperties?.worktreePath})`);
17221724
if (isNewSession) {
17231725
this.sessionItemProvider.untitledSessionIdMapping.set(id, session.object.sessionId);
1726+
this.sessionItemProvider.sdkToUntitledUriMapping.set(session.object.sessionId, resource);
17241727
if (worktreeProperties) {
17251728
void this.copilotCLIWorktreeManagerService.setWorktreeProperties(session.object.sessionId, worktreeProperties);
17261729
}

src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ describe('CopilotCLIChatSessionParticipant.handleRequest', () => {
314314
override swap = vi.fn();
315315
override notifySessionsChange = vi.fn();
316316
override untitledSessionIdMapping = new Map<string, string>();
317+
override sdkToUntitledUriMapping = new Map<string, Uri>();
317318
override isNewSession = vi.fn((_session: string) => false);
318319
}();
319320
cloudProvider = new FakeCloudProvider();
@@ -2097,4 +2098,68 @@ describe('CopilotCLIChatSessionParticipant.handleRequest', () => {
20972098
);
20982099
});
20992100
});
2101+
2102+
describe('sdkToUntitledUriMapping lifecycle', () => {
2103+
it('populates sdkToUntitledUriMapping during request and cleans up after swap', async () => {
2104+
folderRepositoryManager.setNewSessionFolder('untitled:mapping-test', Uri.file(`${sep}workspace`));
2105+
2106+
let capturedSdkSessionId: string | undefined;
2107+
let mappingExistedDuringRequest = false;
2108+
TestCopilotCLISession.handleRequestHook = vi.fn(async () => {
2109+
const session = cliSessions[cliSessions.length - 1];
2110+
capturedSdkSessionId = session.sessionId;
2111+
mappingExistedDuringRequest = itemProvider.sdkToUntitledUriMapping.has(capturedSdkSessionId);
2112+
});
2113+
2114+
const request = new TestChatRequest('Hello');
2115+
const context = createChatContext('untitled:mapping-test', true);
2116+
const stream = new MockChatResponseStream();
2117+
const token = disposables.add(new CancellationTokenSource()).token;
2118+
2119+
await participant.createHandler()(request, context, stream, token);
2120+
2121+
// Mapping should have existed during the request
2122+
expect(mappingExistedDuringRequest).toBe(true);
2123+
// After the request completes and the session is swapped, the mapping should be cleaned up
2124+
expect(itemProvider.sdkToUntitledUriMapping.has(capturedSdkSessionId!)).toBe(false);
2125+
});
2126+
2127+
it('maps SDK session ID to the original untitled URI', async () => {
2128+
folderRepositoryManager.setNewSessionFolder('untitled:uri-check', Uri.file(`${sep}workspace`));
2129+
2130+
let capturedUri: Uri | undefined;
2131+
TestCopilotCLISession.handleRequestHook = vi.fn(async () => {
2132+
const session = cliSessions[cliSessions.length - 1];
2133+
capturedUri = itemProvider.sdkToUntitledUriMapping.get(session.sessionId);
2134+
});
2135+
2136+
const request = new TestChatRequest('Hello');
2137+
const context = createChatContext('untitled:uri-check', true);
2138+
const stream = new MockChatResponseStream();
2139+
const token = disposables.add(new CancellationTokenSource()).token;
2140+
2141+
await participant.createHandler()(request, context, stream, token);
2142+
2143+
expect(capturedUri).toBeDefined();
2144+
expect(capturedUri!.scheme).toBe('copilotcli');
2145+
expect(capturedUri!.path).toBe('/untitled:uri-check');
2146+
});
2147+
2148+
it('does not populate sdkToUntitledUriMapping for existing sessions', async () => {
2149+
const sessionId = 'existing-mapping-test';
2150+
const sdkSession = new MockCliSdkSession(sessionId, new Date());
2151+
manager.sessions.set(sessionId, sdkSession);
2152+
2153+
const request = new TestChatRequest('Continue');
2154+
const context = createChatContext(sessionId, false);
2155+
const stream = new MockChatResponseStream();
2156+
const token = disposables.add(new CancellationTokenSource()).token;
2157+
2158+
await participant.createHandler()(request, context, stream, token);
2159+
2160+
expect(cliSessions.length).toBe(1);
2161+
// Should NOT have set sdkToUntitledUriMapping for existing sessions
2162+
expect(itemProvider.sdkToUntitledUriMapping.size).toBe(0);
2163+
});
2164+
});
21002165
});

0 commit comments

Comments
 (0)