From 2d8197118846f5ea463c1f121235e229c98d190f Mon Sep 17 00:00:00 2001 From: Francesco Renzi Date: Tue, 5 May 2026 08:16:18 -0700 Subject: [PATCH 1/4] Move dap setup to setup job step --- src/Runner.Worker/Dap/DapDebugger.cs | 18 ++++ src/Runner.Worker/JobExtension.cs | 74 +++++++++++++++++ src/Runner.Worker/JobRunner.cs | 63 -------------- src/Test/L0/Worker/JobExtensionL0.cs | 120 +++++++++++++++++++++++++++ 4 files changed, 212 insertions(+), 63 deletions(-) diff --git a/src/Runner.Worker/Dap/DapDebugger.cs b/src/Runner.Worker/Dap/DapDebugger.cs index 89879e2ff2c..ec3adaf3ede 100644 --- a/src/Runner.Worker/Dap/DapDebugger.cs +++ b/src/Runner.Worker/Dap/DapDebugger.cs @@ -243,6 +243,24 @@ public async Task OnJobCompletedAsync() { if (_state != DapSessionState.NotStarted) { + // Pause so the user can inspect final job state before we tear down. + if (IsActive && _pauseOnNextStep) + { + try + { + var cancellationToken = _jobContext?.CancellationToken ?? CancellationToken.None; + + Trace.Info("Job completed — pausing for inspection"); + SendStoppedEvent("completed", "Job completed — inspect variables before the session ends."); + + await WaitForCommandAsync(cancellationToken); + } + catch (Exception ex) + { + Trace.Warning($"DAP job-completed pause error: {ex.Message}"); + } + } + try { OnJobCompleted(); diff --git a/src/Runner.Worker/JobExtension.cs b/src/Runner.Worker/JobExtension.cs index efcba0f531c..991e4b37b60 100644 --- a/src/Runner.Worker/JobExtension.cs +++ b/src/Runner.Worker/JobExtension.cs @@ -16,6 +16,7 @@ using GitHub.Runner.Common; using GitHub.Runner.Common.Util; using GitHub.Runner.Sdk; +using GitHub.Runner.Worker.Dap; using GitHub.Services.Common; using Newtonsoft.Json; using Pipelines = GitHub.DistributedTask.Pipelines; @@ -50,6 +51,7 @@ public sealed class JobExtension : RunnerService, IJobExtension private Task _diskSpaceCheckTask = null; private CancellationTokenSource _serviceConnectivityCheckToken = new(); private Task _serviceConnectivityCheckTask = null; + private IDapDebugger _dapDebugger; // Download all required actions. // Make sure all condition inputs are valid. @@ -481,6 +483,53 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel Trace.Info($"Start checking service connectivity in background."); _serviceConnectivityCheckTask = CheckServiceConnectivityAsync(context, _serviceConnectivityCheckToken.Token); + // Start the DAP debugger and wait for a client connection inside + // "Set up job" so the step stays in-progress while we wait. + if (jobContext.Global.Debugger?.Enabled == true) + { + Trace.Info("Debugger enabled — starting inside Set up job"); + context.Output("Starting debugger…"); + + try + { + _dapDebugger = HostContext.GetService(); + await _dapDebugger.StartAsync(jobContext); + } + catch (Exception ex) + { + Trace.Error($"Failed to start DAP debugger: {ex.Message}"); + AddDebuggerConnectionTelemetry(jobContext, $"Failed: {ex.Message}"); + context.Error("Failed to start debugger."); + context.Result = TaskResult.Failed; + throw; + } + + context.Output("Waiting for debugger client to connect…"); + + try + { + await _dapDebugger.WaitUntilReadyAsync(); + context.Output("Debugger connected."); + AddDebuggerConnectionTelemetry(jobContext, "Connected"); + } + catch (OperationCanceledException) when (jobContext.CancellationToken.IsCancellationRequested) + { + Trace.Info("Job was cancelled before debugger client connected."); + AddDebuggerConnectionTelemetry(jobContext, "Canceled"); + context.Error("Job was cancelled before debugger client connected."); + context.Result = TaskResult.Canceled; + throw; + } + catch (Exception ex) + { + Trace.Error($"DAP debugger failed to become ready: {ex.Message}"); + AddDebuggerConnectionTelemetry(jobContext, $"Failed: {ex.Message}"); + context.Error("The debugger failed to start or no debugger client connected in time."); + context.Result = TaskResult.Failed; + throw; + } + } + return steps; } catch (OperationCanceledException ex) when (jobContext.CancellationToken.IsCancellationRequested) @@ -507,6 +556,15 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel } } + private static void AddDebuggerConnectionTelemetry(IExecutionContext jobContext, string result) + { + jobContext.Global.JobTelemetry.Add(new JobTelemetry + { + Type = JobTelemetryType.General, + Message = $"DebuggerConnectionResult: {result}" + }); + } + private string GetWorkflowReference(IDictionary variables) { var reference = ""; @@ -782,6 +840,22 @@ public async Task FinalizeJob(IExecutionContext jobContext, Pipelines.AgentJobRe } finally { + // Pause for debugger inspection, then tear down the DAP session. + // OnJobCompletedAsync pauses first, then sends terminated/exited + // events and stops the transport. + if (_dapDebugger != null) + { + context.Output("Job completed — pausing for debugger inspection. Press continue to finish."); + try + { + await _dapDebugger.OnJobCompletedAsync(); + } + catch (Exception ex) + { + Trace.Warning($"DAP debugger cleanup error: {ex.Message}"); + } + } + context.Debug("Finishing: Complete job"); context.Complete(); } diff --git a/src/Runner.Worker/JobRunner.cs b/src/Runner.Worker/JobRunner.cs index 2ccad0c0ce2..8308b434220 100644 --- a/src/Runner.Worker/JobRunner.cs +++ b/src/Runner.Worker/JobRunner.cs @@ -13,7 +13,6 @@ using GitHub.Runner.Common; using GitHub.Runner.Common.Util; using GitHub.Runner.Sdk; -using GitHub.Runner.Worker.Dap; using GitHub.Services.Common; using GitHub.Services.WebApi; using Sdk.RSWebApi.Contracts; @@ -29,7 +28,6 @@ public interface IJobRunner : IRunnerService public sealed class JobRunner : RunnerService, IJobRunner { - private const string DebuggerConnectionTelemetryPrefix = "DebuggerConnectionResult"; private IJobServerQueue _jobServerQueue; private RunnerSettings _runnerSettings; private ITempDirectoryManager _tempDirectoryManager; @@ -114,7 +112,6 @@ public async Task RunAsync(AgentJobRequestMessage message, Cancellat IExecutionContext jobContext = null; CancellationTokenRegistration? runnerShutdownRegistration = null; - IDapDebugger dapDebugger = null; try { // Create the job execution context. @@ -181,25 +178,6 @@ public async Task RunAsync(AgentJobRequestMessage message, Cancellat _tempDirectoryManager = HostContext.GetService(); _tempDirectoryManager.InitializeTempDirectory(jobContext); - // Setup the debugger - if (jobContext.Global.Debugger?.Enabled == true) - { - Trace.Info("Debugger enabled for this job run"); - - try - { - dapDebugger = HostContext.GetService(); - await dapDebugger.StartAsync(jobContext); - } - catch (Exception ex) - { - Trace.Error($"Failed to start DAP debugger: {ex.Message}"); - AddDebuggerConnectionTelemetry(jobContext, $"Failed: {ex.Message}"); - jobContext.Error("Failed to start debugger."); - return await CompleteJobAsync(server, jobContext, message, TaskResult.Failed); - } - } - // Get the job extension. Trace.Info("Getting job extension."); @@ -242,33 +220,6 @@ public async Task RunAsync(AgentJobRequestMessage message, Cancellat await Task.WhenAny(_jobServerQueue.JobRecordUpdated.Task, Task.Delay(1000)); } - // Wait for DAP debugger client connection and handshake after "Set up job" - // so the job page shows the setup step before we block on the debugger - if (dapDebugger != null) - { - try - { - await dapDebugger.WaitUntilReadyAsync(); - AddDebuggerConnectionTelemetry(jobContext, "Connected"); - } - catch (OperationCanceledException) when (jobRequestCancellationToken.IsCancellationRequested) - { - Trace.Info("Job was cancelled before debugger client connected."); - AddDebuggerConnectionTelemetry(jobContext, "Canceled"); - jobContext.Error("Job was cancelled before debugger client connected."); - return await CompleteJobAsync(server, jobContext, message, TaskResult.Canceled); - } - catch (Exception ex) - { - Trace.Error($"DAP debugger failed to become ready: {ex.Message}"); - AddDebuggerConnectionTelemetry(jobContext, $"Failed: {ex.Message}"); - - // If debugging was requested but the debugger is not available, fail the job - jobContext.Error("The debugger failed to start or no debugger client connected in time."); - return await CompleteJobAsync(server, jobContext, message, TaskResult.Failed); - } - } - // Run all job steps Trace.Info("Run all job steps."); var stepsRunner = HostContext.GetService(); @@ -309,11 +260,6 @@ public async Task RunAsync(AgentJobRequestMessage message, Cancellat runnerShutdownRegistration = null; } - if (dapDebugger != null) - { - await dapDebugger.OnJobCompletedAsync(); - } - await ShutdownQueue(throwOnFailure: false); } } @@ -495,15 +441,6 @@ private async Task CompleteJobAsync(IJobServer jobServer, IExecution throw new AggregateException(exceptions); } - private static void AddDebuggerConnectionTelemetry(IExecutionContext jobContext, string result) - { - jobContext.Global.JobTelemetry.Add(new JobTelemetry - { - Type = JobTelemetryType.General, - Message = $"{DebuggerConnectionTelemetryPrefix}: {result}" - }); - } - private void MaskTelemetrySecrets(List jobTelemetry) { foreach (var telemetryItem in jobTelemetry) diff --git a/src/Test/L0/Worker/JobExtensionL0.cs b/src/Test/L0/Worker/JobExtensionL0.cs index 40c495a8183..b9cfc7e26a0 100644 --- a/src/Test/L0/Worker/JobExtensionL0.cs +++ b/src/Test/L0/Worker/JobExtensionL0.cs @@ -760,5 +760,125 @@ public async Task SnapshotPreflightChecks_BothChecks_Enabled_AllConditionsMet_Su Environment.SetEnvironmentVariable("RUNNER_ENVIRONMENT", null); Environment.SetEnvironmentVariable("GITHUB_ACTIONS_IMAGE_GEN_ENABLED", null); } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async Task DebuggerStartedInSetupJobWhenEnabled() + { + using (TestHostContext hc = CreateTestContext()) + { + var jobExtension = new JobExtension(); + jobExtension.Initialize(hc); + + // Enable debugger on the message + _message.EnableDebugger = true; + _message.DebuggerTunnel = new Pipelines.DebuggerTunnelInfo + { + TunnelId = "test-tunnel", + ClusterId = "test-cluster", + HostToken = "test-token", + Port = 9229 + }; + + // Re-initialize the execution context so it picks up debugger config + _jobEc = new Runner.Worker.ExecutionContext(); + _jobEc.Initialize(hc); + _jobEc.InitializeJob(_message, _tokenSource.Token); + + // Set up mock debugger + var mockDebugger = new Mock(); + mockDebugger.Setup(x => x.StartAsync(It.IsAny())).Returns(Task.CompletedTask); + mockDebugger.Setup(x => x.WaitUntilReadyAsync()).Returns(Task.CompletedTask); + hc.SetSingleton(mockDebugger.Object); + + _actionManager.Setup(x => x.PrepareActionsAsync(It.IsAny(), It.IsAny>(), It.IsAny())) + .Returns(Task.FromResult(new PrepareResult(new List(), new Dictionary()))); + + List result = await jobExtension.InitializeJob(_jobEc, _message); + + // Verify DAP debugger was started and waited on + mockDebugger.Verify(x => x.StartAsync(It.IsAny()), Times.Once); + mockDebugger.Verify(x => x.WaitUntilReadyAsync(), Times.Once); + + // Verify steps are still returned correctly + Assert.Equal(5, result.Count); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async Task DebuggerNotStartedInSetupJobWhenDisabled() + { + using (TestHostContext hc = CreateTestContext()) + { + var jobExtension = new JobExtension(); + jobExtension.Initialize(hc); + + // Debugger NOT enabled on the message — should not be started + + // Set up mock debugger (should NOT be called) + var mockDebugger = new Mock(); + hc.SetSingleton(mockDebugger.Object); + + _actionManager.Setup(x => x.PrepareActionsAsync(It.IsAny(), It.IsAny>(), It.IsAny())) + .Returns(Task.FromResult(new PrepareResult(new List(), new Dictionary()))); + + List result = await jobExtension.InitializeJob(_jobEc, _message); + + // Verify DAP debugger was NOT started during setup job + mockDebugger.Verify(x => x.StartAsync(It.IsAny()), Times.Never); + mockDebugger.Verify(x => x.WaitUntilReadyAsync(), Times.Never); + + Assert.Equal(5, result.Count); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async Task DebuggerCleanedUpInFinalizeJob() + { + using (TestHostContext hc = CreateTestContext()) + { + var jobExtension = new JobExtension(); + jobExtension.Initialize(hc); + + // Enable debugger on the message + _message.EnableDebugger = true; + _message.DebuggerTunnel = new Pipelines.DebuggerTunnelInfo + { + TunnelId = "test-tunnel", + ClusterId = "test-cluster", + HostToken = "test-token", + Port = 9229 + }; + + // Re-initialize the execution context so it picks up debugger config + _jobEc = new Runner.Worker.ExecutionContext(); + _jobEc.Initialize(hc); + _jobEc.InitializeJob(_message, _tokenSource.Token); + + // Set up mock debugger + var mockDebugger = new Mock(); + mockDebugger.Setup(x => x.StartAsync(It.IsAny())).Returns(Task.CompletedTask); + mockDebugger.Setup(x => x.WaitUntilReadyAsync()).Returns(Task.CompletedTask); + mockDebugger.Setup(x => x.OnJobCompletedAsync()).Returns(Task.CompletedTask); + hc.SetSingleton(mockDebugger.Object); + + _actionManager.Setup(x => x.PrepareActionsAsync(It.IsAny(), It.IsAny>(), It.IsAny())) + .Returns(Task.FromResult(new PrepareResult(new List(), new Dictionary()))); + + // Run InitializeJob to start the debugger + await jobExtension.InitializeJob(_jobEc, _message); + + // Run FinalizeJob — should pause (inside OnJobCompletedAsync) then clean up + await jobExtension.FinalizeJob(_jobEc, _message, DateTime.UtcNow); + + // Verify OnJobCompletedAsync was called (it handles pause + cleanup) + mockDebugger.Verify(x => x.OnJobCompletedAsync(), Times.Once); + } + } } } From c4c1b25b56c2d25871a9218fb9c0311faac8c838 Mon Sep 17 00:00:00 2001 From: Francesco Renzi Date: Tue, 5 May 2026 09:48:49 -0700 Subject: [PATCH 2/4] review --- src/Runner.Worker/Dap/DapDebugger.cs | 7 +++++++ src/Runner.Worker/JobExtension.cs | 15 ++++++++++++++ src/Test/L0/Worker/DapDebuggerL0.cs | 30 ++++++++++++++++++++++------ 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/Runner.Worker/Dap/DapDebugger.cs b/src/Runner.Worker/Dap/DapDebugger.cs index ec3adaf3ede..5f5214dacc8 100644 --- a/src/Runner.Worker/Dap/DapDebugger.cs +++ b/src/Runner.Worker/Dap/DapDebugger.cs @@ -1320,6 +1320,13 @@ private async Task WaitForCommandAsync(CancellationToken cancellationToken) _commandTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); } + // If cancellation already fired before we created the new TCS, + // the registration callback targeted the old one. Unblock now. + if (cancellationToken.IsCancellationRequested) + { + _commandTcs.TrySetResult(DapCommand.Disconnect); + } + Trace.Info("Waiting for debugger command..."); var command = await _commandTcs.Task; diff --git a/src/Runner.Worker/JobExtension.cs b/src/Runner.Worker/JobExtension.cs index 991e4b37b60..bf451ce9994 100644 --- a/src/Runner.Worker/JobExtension.cs +++ b/src/Runner.Worker/JobExtension.cs @@ -550,6 +550,21 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel } finally { + // If InitializeJob failed after the debugger was started, + // clean it up here since FinalizeJob won't run. + if (context.Result != null && _dapDebugger != null) + { + try + { + await _dapDebugger.OnJobCompletedAsync(); + } + catch (Exception ex) + { + Trace.Warning($"DAP debugger cleanup during failed init: {ex.Message}"); + } + _dapDebugger = null; + } + context.Debug("Finishing: Set up job"); context.Complete(); } diff --git a/src/Test/L0/Worker/DapDebuggerL0.cs b/src/Test/L0/Worker/DapDebuggerL0.cs index 9454b1a3a67..2d1ac596bab 100644 --- a/src/Test/L0/Worker/DapDebuggerL0.cs +++ b/src/Test/L0/Worker/DapDebuggerL0.cs @@ -744,14 +744,32 @@ public async Task OnJobCompletedSendsTerminatedAndExitedEvents() await ReadDapMessageAsync(stream, TimeSpan.FromSeconds(5)); await waitTask; - // Complete the job — events are sent via OnJobCompletedAsync - await _debugger.OnJobCompletedAsync(); + // Complete the job — OnJobCompletedAsync pauses when stepping, + // so run it in the background and send continue to unblock. + var completedTask = _debugger.OnJobCompletedAsync(); + + // Read the stopped event from the pause + var stoppedMsg = await ReadDapMessageAsync(stream, TimeSpan.FromSeconds(5)); + Assert.Contains("\"event\":\"stopped\"", stoppedMsg); + + // Send continue to unblock the pause + await SendRequestAsync(stream, new Request + { + Seq = 2, + Type = "request", + Command = "continue" + }); + + await completedTask; - var msg1 = await ReadDapMessageAsync(stream, TimeSpan.FromSeconds(5)); - var msg2 = await ReadDapMessageAsync(stream, TimeSpan.FromSeconds(5)); + // Read remaining messages — continue response + continued event + terminated + exited + var allMessages = new System.Text.StringBuilder(); + for (int i = 0; i < 4; i++) + { + allMessages.Append(await ReadDapMessageAsync(stream, TimeSpan.FromSeconds(5))); + } - // Both events should arrive (order may vary) - var combined = msg1 + msg2; + var combined = allMessages.ToString(); Assert.Contains("\"event\":\"terminated\"", combined); Assert.Contains("\"event\":\"exited\"", combined); } From da5bc7d8597adf6c171c674b5b6d6b3c284a394f Mon Sep 17 00:00:00 2001 From: Francesco Renzi Date: Wed, 6 May 2026 04:06:38 -0700 Subject: [PATCH 3/4] feedback --- src/Runner.Worker/Dap/DapDebugger.cs | 14 +++++---- src/Runner.Worker/JobExtension.cs | 5 ++-- src/Test/L0/Worker/DapDebuggerL0.cs | 40 +++++++++++++++++++++++++ src/Test/L0/Worker/JobExtensionL0.cs | 44 ++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 7 deletions(-) diff --git a/src/Runner.Worker/Dap/DapDebugger.cs b/src/Runner.Worker/Dap/DapDebugger.cs index 5f5214dacc8..122c4c017a4 100644 --- a/src/Runner.Worker/Dap/DapDebugger.cs +++ b/src/Runner.Worker/Dap/DapDebugger.cs @@ -243,17 +243,21 @@ public async Task OnJobCompletedAsync() { if (_state != DapSessionState.NotStarted) { - // Pause so the user can inspect final job state before we tear down. + // Pause so the user can inspect final job state before we tear down, + // but only if the user was stepping through (not if they hit continue). if (IsActive && _pauseOnNextStep) { try { - var cancellationToken = _jobContext?.CancellationToken ?? CancellationToken.None; + if (_jobContext != null) + { + var cancellationToken = _jobContext.CancellationToken; - Trace.Info("Job completed — pausing for inspection"); - SendStoppedEvent("completed", "Job completed — inspect variables before the session ends."); + Trace.Info("Job completed — pausing for inspection"); + SendStoppedEvent("completed", "Job completed — inspect variables before the session ends."); - await WaitForCommandAsync(cancellationToken); + await WaitForCommandAsync(cancellationToken); + } } catch (Exception ex) { diff --git a/src/Runner.Worker/JobExtension.cs b/src/Runner.Worker/JobExtension.cs index bf451ce9994..059ef54fc2f 100644 --- a/src/Runner.Worker/JobExtension.cs +++ b/src/Runner.Worker/JobExtension.cs @@ -498,7 +498,7 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel catch (Exception ex) { Trace.Error($"Failed to start DAP debugger: {ex.Message}"); - AddDebuggerConnectionTelemetry(jobContext, $"Failed: {ex.Message}"); + AddDebuggerConnectionTelemetry(jobContext, $"Failed: {ex.GetType().Name}"); context.Error("Failed to start debugger."); context.Result = TaskResult.Failed; throw; @@ -523,7 +523,7 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel catch (Exception ex) { Trace.Error($"DAP debugger failed to become ready: {ex.Message}"); - AddDebuggerConnectionTelemetry(jobContext, $"Failed: {ex.Message}"); + AddDebuggerConnectionTelemetry(jobContext, $"Failed: {ex.GetType().Name}"); context.Error("The debugger failed to start or no debugger client connected in time."); context.Result = TaskResult.Failed; throw; @@ -869,6 +869,7 @@ public async Task FinalizeJob(IExecutionContext jobContext, Pipelines.AgentJobRe { Trace.Warning($"DAP debugger cleanup error: {ex.Message}"); } + _dapDebugger = null; } context.Debug("Finishing: Complete job"); diff --git a/src/Test/L0/Worker/DapDebuggerL0.cs b/src/Test/L0/Worker/DapDebuggerL0.cs index 2d1ac596bab..f1a29306ff1 100644 --- a/src/Test/L0/Worker/DapDebuggerL0.cs +++ b/src/Test/L0/Worker/DapDebuggerL0.cs @@ -827,5 +827,45 @@ public void ResolveTunnelConnectTimeoutIgnoresZeroValue() }); } } + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async Task WaitForCommandAsyncUnblocksOnCancellationDuringWait() + { + using (CreateTestContext()) + { + var port = GetFreePort(); + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10)); + var jobContext = CreateJobContextWithTunnel(cts.Token, port); + await _debugger.StartAsync(jobContext.Object); + + var waitTask = _debugger.WaitUntilReadyAsync(); + using var client = await ConnectClientAsync(port); + var stream = client.GetStream(); + await SendRequestAsync(stream, new Request + { + Seq = 1, + Type = "request", + Command = "configurationDone" + }); + + await ReadDapMessageAsync(stream, TimeSpan.FromSeconds(5)); + await waitTask; + + // Start OnJobCompletedAsync — it will pause because _pauseOnNextStep is true + var completedTask = _debugger.OnJobCompletedAsync(); + + // Read the stopped event + var stoppedMsg = await ReadDapMessageAsync(stream, TimeSpan.FromSeconds(5)); + Assert.Contains("\"event\":\"stopped\"", stoppedMsg); + + // Cancel the job while waiting — should unblock the pause + cts.Cancel(); + + // OnJobCompletedAsync should complete without hanging + var finished = await Task.WhenAny(completedTask, Task.Delay(TimeSpan.FromSeconds(5))); + Assert.Equal(completedTask, finished); + } + } } } diff --git a/src/Test/L0/Worker/JobExtensionL0.cs b/src/Test/L0/Worker/JobExtensionL0.cs index b9cfc7e26a0..d769d8b5545 100644 --- a/src/Test/L0/Worker/JobExtensionL0.cs +++ b/src/Test/L0/Worker/JobExtensionL0.cs @@ -880,5 +880,49 @@ public async Task DebuggerCleanedUpInFinalizeJob() mockDebugger.Verify(x => x.OnJobCompletedAsync(), Times.Once); } } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async Task FinalizeJobHandlesDebuggerCleanupException() + { + using (TestHostContext hc = CreateTestContext()) + { + var jobExtension = new JobExtension(); + jobExtension.Initialize(hc); + + // Enable debugger on the message + _message.EnableDebugger = true; + _message.DebuggerTunnel = new Pipelines.DebuggerTunnelInfo + { + TunnelId = "test-tunnel", + ClusterId = "test-cluster", + HostToken = "test-token", + Port = 9229 + }; + + // Re-initialize the execution context so it picks up debugger config + _jobEc = new Runner.Worker.ExecutionContext(); + _jobEc.Initialize(hc); + _jobEc.InitializeJob(_message, _tokenSource.Token); + + // Set up mock debugger — OnJobCompletedAsync throws + var mockDebugger = new Mock(); + mockDebugger.Setup(x => x.StartAsync(It.IsAny())).Returns(Task.CompletedTask); + mockDebugger.Setup(x => x.WaitUntilReadyAsync()).Returns(Task.CompletedTask); + mockDebugger.Setup(x => x.OnJobCompletedAsync()).ThrowsAsync(new InvalidOperationException("tunnel disposed")); + hc.SetSingleton(mockDebugger.Object); + + _actionManager.Setup(x => x.PrepareActionsAsync(It.IsAny(), It.IsAny>(), It.IsAny())) + .Returns(Task.FromResult(new PrepareResult(new List(), new Dictionary()))); + + await jobExtension.InitializeJob(_jobEc, _message); + + // FinalizeJob should not throw even when OnJobCompletedAsync throws + await jobExtension.FinalizeJob(_jobEc, _message, DateTime.UtcNow); + + mockDebugger.Verify(x => x.OnJobCompletedAsync(), Times.Once); + } + } } } From 89b6b32e3e0cefb241222a377586cdb9f4449ee2 Mon Sep 17 00:00:00 2001 From: Francesco Renzi Date: Wed, 6 May 2026 07:48:47 -0700 Subject: [PATCH 4/4] feedback --- src/Runner.Worker/Dap/DapDebugger.cs | 6 +---- src/Runner.Worker/Dap/IDapDebugger.cs | 1 + src/Runner.Worker/JobExtension.cs | 38 +++++++++++++-------------- src/Test/L0/Worker/JobExtensionL0.cs | 2 ++ 4 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/Runner.Worker/Dap/DapDebugger.cs b/src/Runner.Worker/Dap/DapDebugger.cs index 122c4c017a4..3b3ec7cbfaf 100644 --- a/src/Runner.Worker/Dap/DapDebugger.cs +++ b/src/Runner.Worker/Dap/DapDebugger.cs @@ -251,12 +251,10 @@ public async Task OnJobCompletedAsync() { if (_jobContext != null) { - var cancellationToken = _jobContext.CancellationToken; - Trace.Info("Job completed — pausing for inspection"); SendStoppedEvent("completed", "Job completed — inspect variables before the session ends."); - await WaitForCommandAsync(cancellationToken); + await WaitForCommandAsync(_jobContext.CancellationToken); } } catch (Exception ex) @@ -274,8 +272,6 @@ public async Task OnJobCompletedAsync() Trace.Warning($"DAP OnJobCompleted error: {ex.Message}"); } } - - await StopAsync(); } public async Task StopAsync() diff --git a/src/Runner.Worker/Dap/IDapDebugger.cs b/src/Runner.Worker/Dap/IDapDebugger.cs index 533626a4287..07ebcab6926 100644 --- a/src/Runner.Worker/Dap/IDapDebugger.cs +++ b/src/Runner.Worker/Dap/IDapDebugger.cs @@ -22,5 +22,6 @@ public interface IDapDebugger : IRunnerService Task OnStepStartingAsync(IStep step); void OnStepCompleted(IStep step); Task OnJobCompletedAsync(); + Task StopAsync(); } } diff --git a/src/Runner.Worker/JobExtension.cs b/src/Runner.Worker/JobExtension.cs index 059ef54fc2f..838009fc9c2 100644 --- a/src/Runner.Worker/JobExtension.cs +++ b/src/Runner.Worker/JobExtension.cs @@ -69,6 +69,7 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel List preJobSteps = new(); List jobSteps = new(); + var initSucceeded = false; using (var register = jobContext.CancellationToken.Register(() => { context.CancelToken(); })) { try @@ -494,20 +495,9 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel { _dapDebugger = HostContext.GetService(); await _dapDebugger.StartAsync(jobContext); - } - catch (Exception ex) - { - Trace.Error($"Failed to start DAP debugger: {ex.Message}"); - AddDebuggerConnectionTelemetry(jobContext, $"Failed: {ex.GetType().Name}"); - context.Error("Failed to start debugger."); - context.Result = TaskResult.Failed; - throw; - } - context.Output("Waiting for debugger client to connect…"); + context.Output("Waiting for debugger client to connect…"); - try - { await _dapDebugger.WaitUntilReadyAsync(); context.Output("Debugger connected."); AddDebuggerConnectionTelemetry(jobContext, "Connected"); @@ -517,19 +507,18 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel Trace.Info("Job was cancelled before debugger client connected."); AddDebuggerConnectionTelemetry(jobContext, "Canceled"); context.Error("Job was cancelled before debugger client connected."); - context.Result = TaskResult.Canceled; throw; } catch (Exception ex) { - Trace.Error($"DAP debugger failed to become ready: {ex.Message}"); + Trace.Error($"DAP debugger failed: {ex.Message}"); AddDebuggerConnectionTelemetry(jobContext, $"Failed: {ex.GetType().Name}"); context.Error("The debugger failed to start or no debugger client connected in time."); - context.Result = TaskResult.Failed; throw; } } + initSucceeded = true; return steps; } catch (OperationCanceledException ex) when (jobContext.CancellationToken.IsCancellationRequested) @@ -551,12 +540,12 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel finally { // If InitializeJob failed after the debugger was started, - // clean it up here since FinalizeJob won't run. - if (context.Result != null && _dapDebugger != null) + // tear down the transport here since FinalizeJob won't run. + if (!initSucceeded && _dapDebugger != null) { try { - await _dapDebugger.OnJobCompletedAsync(); + await _dapDebugger.StopAsync(); } catch (Exception ex) { @@ -867,7 +856,18 @@ public async Task FinalizeJob(IExecutionContext jobContext, Pipelines.AgentJobRe } catch (Exception ex) { - Trace.Warning($"DAP debugger cleanup error: {ex.Message}"); + Trace.Warning($"DAP debugger completion error: {ex.Message}"); + } + finally + { + try + { + await _dapDebugger.StopAsync(); + } + catch (Exception ex) + { + Trace.Warning($"DAP debugger stop error: {ex.Message}"); + } } _dapDebugger = null; } diff --git a/src/Test/L0/Worker/JobExtensionL0.cs b/src/Test/L0/Worker/JobExtensionL0.cs index d769d8b5545..a286e62db6a 100644 --- a/src/Test/L0/Worker/JobExtensionL0.cs +++ b/src/Test/L0/Worker/JobExtensionL0.cs @@ -911,6 +911,7 @@ public async Task FinalizeJobHandlesDebuggerCleanupException() mockDebugger.Setup(x => x.StartAsync(It.IsAny())).Returns(Task.CompletedTask); mockDebugger.Setup(x => x.WaitUntilReadyAsync()).Returns(Task.CompletedTask); mockDebugger.Setup(x => x.OnJobCompletedAsync()).ThrowsAsync(new InvalidOperationException("tunnel disposed")); + mockDebugger.Setup(x => x.StopAsync()).Returns(Task.CompletedTask); hc.SetSingleton(mockDebugger.Object); _actionManager.Setup(x => x.PrepareActionsAsync(It.IsAny(), It.IsAny>(), It.IsAny())) @@ -922,6 +923,7 @@ public async Task FinalizeJobHandlesDebuggerCleanupException() await jobExtension.FinalizeJob(_jobEc, _message, DateTime.UtcNow); mockDebugger.Verify(x => x.OnJobCompletedAsync(), Times.Once); + mockDebugger.Verify(x => x.StopAsync(), Times.Once); } } }