Skip to content

Commit 62d0d43

Browse files
Copilotconnor4312
andauthored
Fix preLaunchTask running before concurrent session confirmation (microsoft#263294)
* Initial plan * Implement fix for preLaunchTask running before concurrent session confirmation Co-authored-by: connor4312 <2230985+connor4312@users.noreply.github.com> * Add test for concurrent session preLaunchTask fix Co-authored-by: connor4312 <2230985+connor4312@users.noreply.github.com> * Compare launch configurations instead of session labels for concurrent session detection Address feedback to use launch configuration comparison instead of session labels, which can be unpredictable as they may be changed by the debug server. The fix now compares configuration name, type, request, and workspace to reliably identify the same launch configuration. Co-authored-by: connor4312 <2230985+connor4312@users.noreply.github.com> * Extract launch configuration comparison logic into private method - Created `hasSameConfiguration()` private method to avoid code duplication - Updated `createSession()` to use the new method - Updated `doCreateSession()` to use configuration comparison instead of label comparison - Removed test file as requested Co-authored-by: connor4312 <2230985+connor4312@users.noreply.github.com> * Extract dialog confirmation logic into private method Co-authored-by: connor4312 <2230985+connor4312@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: connor4312 <2230985+connor4312@users.noreply.github.com>
1 parent d269d01 commit 62d0d43

1 file changed

Lines changed: 43 additions & 6 deletions

File tree

src/vs/workbench/contrib/debug/browser/debugService.ts

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import * as errors from '../../../../base/common/errors.js';
1313
import { Emitter, Event } from '../../../../base/common/event.js';
1414
import { DisposableStore, IDisposable } from '../../../../base/common/lifecycle.js';
1515
import { deepClone, equals } from '../../../../base/common/objects.js';
16+
1617
import severity from '../../../../base/common/severity.js';
1718
import { URI, URI as uri } from '../../../../base/common/uri.js';
1819
import { generateUuid } from '../../../../base/common/uuid.js';
@@ -518,6 +519,30 @@ export class DebugService implements IDebugService {
518519
return false;
519520
}
520521

522+
// Check for concurrent sessions before running preLaunchTask to avoid running the task if user cancels
523+
let userConfirmedConcurrentSession = false;
524+
if (options?.startedByUser && resolvedConfig && resolvedConfig.suppressMultipleSessionWarning !== true) {
525+
// Check if there's already a session with the same launch configuration
526+
const existingSessions = this.model.getSessions();
527+
const workspace = launch?.workspace;
528+
529+
const existingSession = existingSessions.find(s =>
530+
s.configuration.name === resolvedConfig!.name &&
531+
s.configuration.type === resolvedConfig!.type &&
532+
s.configuration.request === resolvedConfig!.request &&
533+
s.root === workspace
534+
);
535+
536+
if (existingSession) {
537+
// There is already a session with the same configuration, prompt user before running preLaunchTask
538+
const confirmed = await this.confirmConcurrentSession(existingSession.getLabel());
539+
if (!confirmed) {
540+
return false;
541+
}
542+
userConfirmedConcurrentSession = true;
543+
}
544+
}
545+
521546
const workspace = launch?.workspace || this.contextService.getWorkspace();
522547
const taskResult = await this.taskRunner.runTaskAndCheckErrors(workspace, resolvedConfig.preLaunchTask);
523548
if (taskResult === TaskRunResult.Failure) {
@@ -565,7 +590,7 @@ export class DebugService implements IDebugService {
565590
return false;
566591
}
567592

568-
const result = await this.doCreateSession(sessionId, launch?.workspace, { resolved: resolvedConfig, unresolved: unresolvedConfig }, options);
593+
const result = await this.doCreateSession(sessionId, launch?.workspace, { resolved: resolvedConfig, unresolved: unresolvedConfig }, options, userConfirmedConcurrentSession);
569594
if (result && guess && activeEditor && activeEditor.resource) {
570595
// Remeber user choice of environment per active editor to make starting debugging smoother #124770
571596
this.chosenEnvironments[activeEditor.resource.toString()] = { type: guess.debugger.type, dynamicLabel: guess.withConfig?.label };
@@ -596,13 +621,18 @@ export class DebugService implements IDebugService {
596621
/**
597622
* instantiates the new session, initializes the session, registers session listeners and reports telemetry
598623
*/
599-
private async doCreateSession(sessionId: string, root: IWorkspaceFolder | undefined, configuration: { resolved: IConfig; unresolved: IConfig | undefined }, options?: IDebugSessionOptions): Promise<boolean> {
624+
private async doCreateSession(sessionId: string, root: IWorkspaceFolder | undefined, configuration: { resolved: IConfig; unresolved: IConfig | undefined }, options?: IDebugSessionOptions, userConfirmedConcurrentSession = false): Promise<boolean> {
600625

601626
const session = this.instantiationService.createInstance(DebugSession, sessionId, configuration, root, this.model, options);
602-
if (options?.startedByUser && this.model.getSessions().some(s => s.getLabel() === session.getLabel()) && configuration.resolved.suppressMultipleSessionWarning !== true) {
603-
// There is already a session with the same name, prompt user #127721
604-
const result = await this.dialogService.confirm({ message: nls.localize('multipleSession', "'{0}' is already running. Do you want to start another instance?", session.getLabel()) });
605-
if (!result.confirmed) {
627+
if (!userConfirmedConcurrentSession && options?.startedByUser && this.model.getSessions().some(s =>
628+
s.configuration.name === configuration.resolved.name &&
629+
s.configuration.type === configuration.resolved.type &&
630+
s.configuration.request === configuration.resolved.request &&
631+
s.root === root
632+
) && configuration.resolved.suppressMultipleSessionWarning !== true) {
633+
// There is already a session with the same configuration, prompt user #127721
634+
const confirmed = await this.confirmConcurrentSession(session.getLabel());
635+
if (!confirmed) {
606636
return false;
607637
}
608638
}
@@ -665,6 +695,13 @@ export class DebugService implements IDebugService {
665695
}
666696
}
667697

698+
private async confirmConcurrentSession(sessionLabel: string): Promise<boolean> {
699+
const result = await this.dialogService.confirm({
700+
message: nls.localize('multipleSession', "'{0}' is already running. Do you want to start another instance?", sessionLabel)
701+
});
702+
return result.confirmed;
703+
}
704+
668705
private async launchOrAttachToSession(session: IDebugSession, forceFocus = false): Promise<void> {
669706
// register listeners as the very first thing!
670707
this.registerSessionListeners(session);

0 commit comments

Comments
 (0)