Skip to content

Commit cf6e54a

Browse files
DonJayamanneCopilot
DonJayamanne
and
Copilot
authored
refactor: remove debug logger from CopilotCLISession and update related tests (#4744)
* refactor: remove debug logger from CopilotCLISession and update related tests * Fixes * Update src/extension/chatSessions/copilotcli/node/test/copilotCliSessionService.spec.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/extension/chatSessions/copilotcli/node/test/copilotCliSessionService.spec.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent a80eee6 commit cf6e54a

5 files changed

Lines changed: 18 additions & 20 deletions

File tree

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import type { Attachment, SendOptions, Session, SessionOptions } from '@github/c
77
import * as l10n from '@vscode/l10n';
88
import type * as vscode from 'vscode';
99
import type { ChatParticipantToolToken } from 'vscode';
10-
import { IChatDebugFileLoggerService } from '../../../../platform/chat/common/chatDebugFileLoggerService';
1110
import { ConfigKey, IConfigurationService } from '../../../../platform/configuration/common/configurationService';
1211
import { ILogService } from '../../../../platform/log/common/logService';
1312
import { CopilotChatAttr, GenAiAttr, GenAiOperationName, IOTelService, ISpanHandle, SpanKind, SpanStatusCode, truncateForOTel } from '../../../../platform/otel/common/index';
@@ -159,19 +158,10 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes
159158
@IUserQuestionHandler private readonly _userQuestionHandler: IUserQuestionHandler,
160159
@IConfigurationService private readonly configurationService: IConfigurationService,
161160
@IOTelService private readonly _otelService: IOTelService,
162-
@IChatDebugFileLoggerService private readonly _debugFileLogger: IChatDebugFileLoggerService,
163161
@IChatPromptFileService private readonly _chatPromptFileService: IChatPromptFileService,
164162
) {
165163
super();
166164
this.sessionId = _sdkSession.sessionId;
167-
this._debugFileLogger.startSession(this.sessionId).catch(err => {
168-
this.logService.error('[CopilotCLISession] Failed to start debug log session', err);
169-
});
170-
this.add(toDisposable(() => {
171-
this._debugFileLogger.endSession(this.sessionId).catch(err => {
172-
this.logService.error('[CopilotCLISession] Failed to end debug log session', err);
173-
});
174-
}));
175165
}
176166

177167
attachStream(stream: vscode.ChatResponseStream): IDisposable {

‎src/extension/chatSessions/copilotcli/node/copilotcliSessionService.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import { CopilotCliBridgeSpanProcessor } from './copilotCliBridgeSpanProcessor';
4747
import { CopilotCLISession, ICopilotCLISession } from './copilotcliSession';
4848
import { ICopilotCLISkills } from './copilotCLISkills';
4949
import { ICopilotCLIMCPHandler } from './mcpHandler';
50+
import { IChatDebugFileLoggerService } from '../../../../platform/chat/common/chatDebugFileLoggerService';
5051

5152
const COPILOT_CLI_WORKSPACE_JSON_FILE_KEY = 'github.copilot.cli.workspaceSessionFile';
5253

@@ -145,6 +146,7 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS
145146
@IChatSessionWorkspaceFolderService private readonly workspaceFolderService: IChatSessionWorkspaceFolderService,
146147
@IChatSessionWorktreeService private readonly worktreeManager: IChatSessionWorktreeService,
147148
@IOTelService private readonly _otelService: IOTelService,
149+
@IChatDebugFileLoggerService private readonly _debugFileLogger: IChatDebugFileLoggerService,
148150
) {
149151
super();
150152
this.monitorSessionFiles();
@@ -844,6 +846,14 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS
844846

845847
private createCopilotSession(sdkSession: Session, options: CopilotCLISessionOptions, sessionManager: internal.LocalSessionManager, readonly = false, nowait = false): RefCountedSession {
846848
const session = this.instantiationService.createInstance(CopilotCLISession, options, sdkSession);
849+
this._debugFileLogger.startSession(session.sessionId).catch(err => {
850+
this.logService.error('[CopilotCLISession] Failed to start debug log session', err);
851+
});
852+
session.add(toDisposable(() => {
853+
this._debugFileLogger.endSession(session.sessionId).catch(err => {
854+
this.logService.error('[CopilotCLISession] Failed to end debug log session', err);
855+
});
856+
}));
847857
// Wire the bridge processor so the session can register traceId → sessionId mappings
848858
session.setBridgeProcessor(this._bridgeProcessor);
849859
// Wire SDK trace context updater so the session can propagate traceparent to SDK spans

‎src/extension/chatSessions/copilotcli/node/test/copilotCliSessionService.spec.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,13 +149,13 @@ describe('CopilotCLISessionService', () => {
149149
}
150150
}();
151151
}
152-
return disposables.add(new CopilotCLISession(options, sdkSession, logService, workspaceService, sdk, new MockChatSessionMetadataStore(), instantiationService, delegationService, new NullRequestLogger(), new NullICopilotCLIImageSupport(), new FakeToolsService(), new FakeUserQuestionHandler(), accessor.get(IConfigurationService), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' })), new NullChatDebugFileLoggerService(), new class extends mock<IChatPromptFileService>() { override get customAgentPromptFiles() { return []; } }));
152+
return disposables.add(new CopilotCLISession(options, sdkSession, logService, workspaceService, sdk, new MockChatSessionMetadataStore(), instantiationService, delegationService, new NullRequestLogger(), new NullICopilotCLIImageSupport(), new FakeToolsService(), new FakeUserQuestionHandler(), accessor.get(IConfigurationService), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' })), new class extends mock<IChatPromptFileService>() { override get customAgentPromptFiles() { return []; } }));
153153
}
154154
} as unknown as IInstantiationService;
155155
const configurationService = accessor.get(IConfigurationService);
156156
const nullMcpServer = disposables.add(new NullMcpService());
157157
const titleService = new NullCustomSessionTitleService();
158-
service = disposables.add(new CopilotCLISessionService(logService, sdk, instantiationService, new NullNativeEnvService(), new MockFileSystemService(), new CopilotCLIMCPHandler(logService, authService, configurationService, nullMcpServer), cliAgents, workspaceService, titleService, configurationService, new MockSkillLocations(), delegationService, new MockChatSessionMetadataStore(), new NullAgentSessionsWorkspace(), new NullChatSessionWorkspaceFolderService(), new NullChatSessionWorktreeService(), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' }))));
158+
service = disposables.add(new CopilotCLISessionService(logService, sdk, instantiationService, new NullNativeEnvService(), new MockFileSystemService(), new CopilotCLIMCPHandler(logService, authService, configurationService, nullMcpServer), cliAgents, workspaceService, titleService, configurationService, new MockSkillLocations(), delegationService, new MockChatSessionMetadataStore(), new NullAgentSessionsWorkspace(), new NullChatSessionWorkspaceFolderService(), new NullChatSessionWorktreeService(), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' })), new NullChatDebugFileLoggerService()));
159159
manager = await service.getSessionManager() as unknown as MockCliSdkSessionManager;
160160
});
161161

@@ -314,7 +314,7 @@ describe('CopilotCLISessionService', () => {
314314
return undefined;
315315
}
316316
}();
317-
const partialService = disposables.add(new CopilotCLISessionService(logService, sdk, instantiationService, new NullNativeEnvService(), fileSystem, new CopilotCLIMCPHandler(logService, authService, configurationService, nullMcpServer), new NullCopilotCLIAgents(), new NullWorkspaceService(), titleService, configurationService, new MockSkillLocations(), delegationService, new MockChatSessionMetadataStore(), new NullAgentSessionsWorkspace(), new NullChatSessionWorkspaceFolderService(), new NullChatSessionWorktreeService(), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' }))));
317+
const partialService = disposables.add(new CopilotCLISessionService(logService, sdk, instantiationService, new NullNativeEnvService(), fileSystem, new CopilotCLIMCPHandler(logService, authService, configurationService, nullMcpServer), new NullCopilotCLIAgents(), new NullWorkspaceService(), titleService, configurationService, new MockSkillLocations(), delegationService, new MockChatSessionMetadataStore(), new NullAgentSessionsWorkspace(), new NullChatSessionWorkspaceFolderService(), new NullChatSessionWorktreeService(), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' })), new NullChatDebugFileLoggerService()));
318318

319319
await mkdir(sessionDir.fsPath, { recursive: true });
320320
await writeNodeFile(join(sessionDir.fsPath, 'events.jsonl'), [
@@ -349,7 +349,7 @@ describe('CopilotCLISessionService', () => {
349349
const delegationService = new class extends mock<IChatDelegationSummaryService>() {
350350
override extractPrompt(): { prompt: string; reference: never } | undefined { return undefined; }
351351
}();
352-
const partialService = disposables.add(new CopilotCLISessionService(logService, sdk, instantiationService, new NullNativeEnvService(), fileSystem, new CopilotCLIMCPHandler(logService, authService, configurationService, nullMcpServer), new NullCopilotCLIAgents(), new NullWorkspaceService(), titleService, configurationService, new MockSkillLocations(), delegationService, new MockChatSessionMetadataStore(), new NullAgentSessionsWorkspace(), new NullChatSessionWorkspaceFolderService(), new NullChatSessionWorktreeService(), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' }))));
352+
const partialService = disposables.add(new CopilotCLISessionService(logService, sdk, instantiationService, new NullNativeEnvService(), fileSystem, new CopilotCLIMCPHandler(logService, authService, configurationService, nullMcpServer), new NullCopilotCLIAgents(), new NullWorkspaceService(), titleService, configurationService, new MockSkillLocations(), delegationService, new MockChatSessionMetadataStore(), new NullAgentSessionsWorkspace(), new NullChatSessionWorkspaceFolderService(), new NullChatSessionWorktreeService(), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' })), new NullChatDebugFileLoggerService()));
353353

354354
await mkdir(sessionDir.fsPath, { recursive: true });
355355
const eventsFilePath = join(sessionDir.fsPath, 'events.jsonl');
@@ -418,7 +418,7 @@ describe('CopilotCLISessionService', () => {
418418
return undefined;
419419
}
420420
}();
421-
const partialService = disposables.add(new CopilotCLISessionService(logService, sdk, instantiationService, new NullNativeEnvService(), fileSystem, new CopilotCLIMCPHandler(logService, authService, configurationService, nullMcpServer), new NullCopilotCLIAgents(), new NullWorkspaceService(), titleService, configurationService, new MockSkillLocations(), delegationService, new MockChatSessionMetadataStore(), new NullAgentSessionsWorkspace(), new NullChatSessionWorkspaceFolderService(), new NullChatSessionWorktreeService(), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' }))));
421+
const partialService = disposables.add(new CopilotCLISessionService(logService, sdk, instantiationService, new NullNativeEnvService(), fileSystem, new CopilotCLIMCPHandler(logService, authService, configurationService, nullMcpServer), new NullCopilotCLIAgents(), new NullWorkspaceService(), titleService, configurationService, new MockSkillLocations(), delegationService, new MockChatSessionMetadataStore(), new NullAgentSessionsWorkspace(), new NullChatSessionWorkspaceFolderService(), new NullChatSessionWorktreeService(), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' })), new NullChatDebugFileLoggerService()));
422422
const partialManager = await partialService.getSessionManager() as unknown as MockCliSdkSessionManager;
423423

424424
const session = new MockCliSdkSession(sessionId, new Date('2024-01-01T00:00:00.000Z'));
@@ -460,7 +460,7 @@ describe('CopilotCLISessionService', () => {
460460
const delegationService = new class extends mock<IChatDelegationSummaryService>() {
461461
override extractPrompt(): { prompt: string; reference: never } | undefined { return undefined; }
462462
}();
463-
const partialService = disposables.add(new CopilotCLISessionService(logService, sdk, instantiationService, new NullNativeEnvService(), fileSystem, new CopilotCLIMCPHandler(logService, authService, configurationService, nullMcpServer), new NullCopilotCLIAgents(), new NullWorkspaceService(), titleService, configurationService, new MockSkillLocations(), delegationService, new MockChatSessionMetadataStore(), new NullAgentSessionsWorkspace(), new NullChatSessionWorkspaceFolderService(), new NullChatSessionWorktreeService(), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' }))));
463+
const partialService = disposables.add(new CopilotCLISessionService(logService, sdk, instantiationService, new NullNativeEnvService(), fileSystem, new CopilotCLIMCPHandler(logService, authService, configurationService, nullMcpServer), new NullCopilotCLIAgents(), new NullWorkspaceService(), titleService, configurationService, new MockSkillLocations(), delegationService, new MockChatSessionMetadataStore(), new NullAgentSessionsWorkspace(), new NullChatSessionWorkspaceFolderService(), new NullChatSessionWorktreeService(), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' })), new NullChatDebugFileLoggerService()));
464464
const partialManager = await partialService.getSessionManager() as unknown as MockCliSdkSessionManager;
465465

466466
// Session has a summary with '<' (which forces the session-load fallback path)

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import type { Session, SessionOptions } from '@github/copilot/sdk';
77
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
88
import type { ChatContext, ChatParticipantToolToken } from 'vscode';
9-
import { NullChatDebugFileLoggerService } from '../../../../../platform/chat/common/chatDebugFileLoggerService';
109
import { ConfigKey, IConfigurationService } from '../../../../../platform/configuration/common/configurationService';
1110
import { ILogService } from '../../../../../platform/log/common/logService';
1211
import { NoopOTelService, resolveOTelConfig } from '../../../../../platform/otel/common/index';
@@ -236,7 +235,6 @@ describe('CopilotCLISession', () => {
236235
new FakeUserQuestionHandler(),
237236
configurationService,
238237
new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' })),
239-
new NullChatDebugFileLoggerService(),
240238
new class extends mock<IChatPromptFileService>() { override get customAgentPromptFiles() { return []; } }(),
241239
));
242240
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -367,13 +367,13 @@ describe('CopilotCLIChatSessionParticipant.handleRequest', () => {
367367
}
368368
}();
369369
}
370-
const session = new TestCopilotCLISession(options, sdkSession, logService, workspaceService, sdk, new MockChatSessionMetadataStore(), instantiationService, delegationService, new NullRequestLogger(), new NullICopilotCLIImageSupport(), new FakeToolsService(), new FakeUserQuestionHandler(), accessor.get(IConfigurationService), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' })), new NullChatDebugFileLoggerService(), disposables.add(new MockChatPromptFileService()));
370+
const session = new TestCopilotCLISession(options, sdkSession, logService, workspaceService, sdk, new MockChatSessionMetadataStore(), instantiationService, delegationService, new NullRequestLogger(), new NullICopilotCLIImageSupport(), new FakeToolsService(), new FakeUserQuestionHandler(), accessor.get(IConfigurationService), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' })), disposables.add(new MockChatPromptFileService()));
371371
cliSessions.push(session);
372372
return disposables.add(session);
373373
}
374374
} as unknown as IInstantiationService;
375375
customSessionTitleService = new CustomSessionTitleService(new MockExtensionContext() as unknown as IVSCodeExtensionContext, accessor.get(IInstantiationService), logService, new MockChatSessionMetadataStore());
376-
sessionService = disposables.add(new CopilotCLISessionService(logService, sdk, instantiationService, new NullNativeEnvService(), fileSystem, mcpHandler, new NullCopilotCLIAgents(), workspaceService, customSessionTitleService, accessor.get(IConfigurationService), new MockSkillLocations(), delegationService, new MockChatSessionMetadataStore(), { _serviceBrand: undefined, isAgentSessionsWorkspace: false } as IAgentSessionsWorkspace, workspaceFolderService, worktree, new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' }))));
376+
sessionService = disposables.add(new CopilotCLISessionService(logService, sdk, instantiationService, new NullNativeEnvService(), fileSystem, mcpHandler, new NullCopilotCLIAgents(), workspaceService, customSessionTitleService, accessor.get(IConfigurationService), new MockSkillLocations(), delegationService, new MockChatSessionMetadataStore(), { _serviceBrand: undefined, isAgentSessionsWorkspace: false } as IAgentSessionsWorkspace, workspaceFolderService, worktree, new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' })), new NullChatDebugFileLoggerService()));
377377

378378
manager = await sessionService.getSessionManager() as unknown as MockCliSdkSessionManager;
379379
contentProvider = new class extends mock<CopilotCLIChatSessionContentProvider>() {

0 commit comments

Comments
 (0)