Fix extra prompting and manual debugger commands

This requires that we set the REPL to exit correctly. Note that we don't
have a notion of "disconnect" so we removed that logic. We are assuming
the debugger is "active" a little earlier now, that is, also when we've
received an LSP notification to start it (via a launch or attach
handler). Because we receive a notification on "disconnect" even for a
script just finishing, we need to know that the server is not active and
so not cancel the current prompt.

Author: andyleejordan

src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs

@@ -11,15 +11,16 @@
 using Microsoft.Extensions.Logging;
 using Microsoft.PowerShell.EditorServices.Logging;
 using Microsoft.PowerShell.EditorServices.Services;
-using OmniSharp.Extensions.DebugAdapter.Protocol.Events;
-using OmniSharp.Extensions.JsonRpc;
-using Microsoft.PowerShell.EditorServices.Services.TextDocument;
-using OmniSharp.Extensions.DebugAdapter.Protocol.Requests;
-using OmniSharp.Extensions.DebugAdapter.Protocol.Server;
 using Microsoft.PowerShell.EditorServices.Services.PowerShell;
 using Microsoft.PowerShell.EditorServices.Services.PowerShell.Context;
-using Microsoft.PowerShell.EditorServices.Services.PowerShell.Runspace;
 using Microsoft.PowerShell.EditorServices.Services.PowerShell.Execution;
+using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host;
+using Microsoft.PowerShell.EditorServices.Services.PowerShell.Runspace;
+using Microsoft.PowerShell.EditorServices.Services.TextDocument;
+using OmniSharp.Extensions.DebugAdapter.Protocol.Events;
+using OmniSharp.Extensions.DebugAdapter.Protocol.Requests;
+using OmniSharp.Extensions.DebugAdapter.Protocol.Server;
+using OmniSharp.Extensions.JsonRpc;
 
 namespace Microsoft.PowerShell.EditorServices.Handlers
 {
@@ -122,6 +123,9 @@ public LaunchAndAttachHandler(
 
         public async Task<LaunchResponse> Handle(PsesLaunchRequestArguments request, CancellationToken cancellationToken)
         {
+            // The debugger has officially started. We use this to later check if we should stop it.
+            ((PsesInternalHost)_executionService).DebugContext.IsActive = true;
+
             _debugEventHandlerService.RegisterEventHandlers();
 
             // Determine whether or not the working directory should be set in the PowerShellContext.
@@ -213,6 +217,9 @@ public async Task<LaunchResponse> Handle(PsesLaunchRequestArguments request, Can
 
         public async Task<AttachResponse> Handle(PsesAttachRequestArguments request, CancellationToken cancellationToken)
         {
+            // The debugger has officially started. We use this to later check if we should stop it.
+            ((PsesInternalHost)_executionService).DebugContext.IsActive = true;
+
             _debugStateService.IsAttachSession = true;
 
             _debugEventHandlerService.RegisterEventHandlers();

src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs

@@ -94,7 +94,7 @@ public void EnableDebugMode()
             _psesHost.Runspace.Debugger.SetDebugMode(DebugModes.LocalScript | DebugModes.RemoteScript);
         }
 
-        public void Abort() => SetDebugResuming(DebuggerResumeAction.Stop, isDisconnect: true);
+        public void Abort() => SetDebugResuming(DebuggerResumeAction.Stop);
 
         public void BreakExecution() => _psesHost.Runspace.Debugger.SetDebuggerStepMode(enabled: true);
 
@@ -106,10 +106,12 @@ public void EnableDebugMode()
 
         public void StepOver() => SetDebugResuming(DebuggerResumeAction.StepOver);
 
-        public void SetDebugResuming(DebuggerResumeAction debuggerResumeAction, bool isDisconnect = false)
+        public void SetDebugResuming(DebuggerResumeAction debuggerResumeAction)
         {
-            // NOTE: We exit because the paused/stopped debugger is currently in a prompt REPL, and
-            // to resume the debugger we must exit that REPL.
+            // We exit because the paused/stopped debugger is currently in a prompt REPL, and to
+            // resume the debugger we must exit that REPL. If we're continued from 'c' or 's', this
+            // is already set and so is a no-op; but if the user clicks the continue or step button,
+            // then this came over LSP and we need to set it.
             _psesHost.SetExit();
 
             if (LastStopEventArgs is not null)
@@ -127,23 +129,31 @@ public void SetDebugResuming(DebuggerResumeAction debuggerResumeAction, bool isD
                 return;
             }
 
-            if (debuggerResumeAction is DebuggerResumeAction.Stop)
+            // If we're stopping (or disconnecting, which is the same thing in LSP-land), then we
+            // want to cancel any debug prompts, remote prompts, debugged scripts, etc. However, if
+            // the debugged script has exited normally (or was quit with 'q'), we still get an LSP
+            // notification that eventually lands here with a stop event. In this case, the debug
+            // context is NOT active and we do not want to cancel the regular REPL.
+            if (!_psesHost.DebugContext.IsActive)
             {
-                // If we're disconnecting we want to unwind all the way back to the default, local
-                // state. So we use UnwindCallStack here to ensure every context frame is cancelled.
-                if (isDisconnect)
-                {
-                    _psesHost.UnwindCallStack();
-                    return;
-                }
+                return;
+            }
 
-                _psesHost.CancelIdleParentTask();
+            // If the debugger is active and we're stopping, we need to unwind everything.
+            if (debuggerResumeAction is DebuggerResumeAction.Stop)
+            {
+                // TODO: We need to assign cancellation tokens to each frame, because the current
+                // logic results in a deadlock here when we try to cancel the scopes...which
+                // includes ourself (hence running it in a separate thread).
+                Task.Run(() => _psesHost.UnwindCallStack());
                 return;
             }
 
+            // Otherwise we're continuing or stepping (i.e. resuming) so we need to cancel the
+            // debugger REPL.
             if (_psesHost.CurrentFrame.IsRepl)
             {
-                _psesHost.CancelCurrentTask();
+                _psesHost.CancelIdleParentTask();
             }
         }
 
@@ -166,15 +176,14 @@ public void ProcessDebuggerResult(DebuggerCommandResults debuggerResult)
         {
             if (debuggerResult?.ResumeAction is not null)
             {
-                SetDebugResuming(debuggerResult.ResumeAction.Value);
-
-                // If a debugging command like `c` is specified in a nested remote
-                // debugging prompt we need to unwind the nested execution loop.
-                if (_psesHost.CurrentFrame.IsRemote)
+                // Since we're processing a command like 'c' or 's' remotely, we need to tell the
+                // host to stop the remote REPL loop.
+                if (debuggerResult.ResumeAction is not DebuggerResumeAction.Stop || _psesHost.CurrentFrame.IsRemote)
                 {
                     _psesHost.ForceSetExit();
                 }
 
+                SetDebugResuming(debuggerResult.ResumeAction.Value);
                 RaiseDebuggerResumingEvent(new DebuggerResumingEventArgs(debuggerResult.ResumeAction.Value));
 
                 // The Terminate exception is used by the engine for flow control

src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs

@@ -724,6 +724,19 @@ private void DoOneRepl(CancellationToken cancellationToken)
                 return;
             }
 
+            // TODO: We must remove this awful logic, it causes so much pain. The StopDebugContext()
+            // requires that we're not in a prompt that we're skipping, otherwise the debugger is
+            // "active" but we haven't yet hit a breakpoint.
+            //
+            // When a task must run in the foreground, we cancel out of the idle loop and return to
+            // the top level. At that point, we would normally run a REPL, but we need to
+            // immediately execute the task. So we set _skipNextPrompt to do that.
+            if (_skipNextPrompt)
+            {
+                _skipNextPrompt = false;
+                return;
+            }
+
             // We use the REPL as a poll to check if the debug context is active but PowerShell
             // indicates we're no longer debugging. This happens when PowerShell was used to start
             // the debugger (instead of using a Code launch configuration) via Wait-Debugger or
@@ -736,15 +749,6 @@ private void DoOneRepl(CancellationToken cancellationToken)
                 StopDebugContext();
             }
 
-            // When a task must run in the foreground, we cancel out of the idle loop and return to the top level.
-            // At that point, we would normally run a REPL, but we need to immediately execute the task.
-            // So we set _skipNextPrompt to do that.
-            if (_skipNextPrompt)
-            {
-                _skipNextPrompt = false;
-                return;
-            }
-
             try
             {
                 string prompt = GetPrompt(cancellationToken);