From c950821befbc6d410b2306fc413cd05f4442e981 Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Thu, 6 Aug 2026 20:29:07 +0300 Subject: [PATCH 1/3] Bump Devolutions.Now.Policy packages to 2026.8.5 and add remote broker cancellation - Update Devolutions.Now.Policy.Api/.Client from 2026.7.28 to 2026.8.5. - Replace ExecuteAndWait with explicit Execute + status polling so the operation id is owned locally and cancellation is deterministic. - On user cancellation, request broker-side cancel and wait (bounded) for the terminal status; honor Completed/Failed if the remote process wins the race, otherwise return the Canceled veredict. - Handle the new Canceling/Canceled operation statuses. - Remove StatusResponse.Stdout handling (removed upstream); captured output will be restored via the per-operation event channel in a follow-up change. CaptureOutput stays enabled in the request. - Add scriptable broker transport test seam and cancellation tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../UniGetUI.PackageEngine.AgentBroker.csproj | 4 +- .../PackageOperations.cs | 216 +++++++++++---- .../PackageOperationsTests.cs | 246 ++++++++++++++++++ 3 files changed, 410 insertions(+), 56 deletions(-) diff --git a/src/UniGetUI.PackageEngine.AgentBroker/UniGetUI.PackageEngine.AgentBroker.csproj b/src/UniGetUI.PackageEngine.AgentBroker/UniGetUI.PackageEngine.AgentBroker.csproj index 949b8e3918..21adcaea67 100644 --- a/src/UniGetUI.PackageEngine.AgentBroker/UniGetUI.PackageEngine.AgentBroker.csproj +++ b/src/UniGetUI.PackageEngine.AgentBroker/UniGetUI.PackageEngine.AgentBroker.csproj @@ -6,8 +6,8 @@ - - + + diff --git a/src/UniGetUI.PackageEngine.Operations/PackageOperations.cs b/src/UniGetUI.PackageEngine.Operations/PackageOperations.cs index e8cf5e9f47..ecb5659cfa 100644 --- a/src/UniGetUI.PackageEngine.Operations/PackageOperations.cs +++ b/src/UniGetUI.PackageEngine.Operations/PackageOperations.cs @@ -1,4 +1,3 @@ -using System.Text; using UniGetUI.Core.Classes; using UniGetUI.Core.Data; using UniGetUI.Core.Logging; @@ -17,8 +16,12 @@ using BrokerClientErrorKind = Devolutions.Now.Policy.Client.BrokerClientErrorKind; using BrokerClientException = Devolutions.Now.Policy.Client.BrokerClientException; using BrokerClientOptions = Devolutions.Now.Policy.Client.BrokerClientOptions; +using BrokerDecision = Devolutions.Now.Policy.Api.Decision; using BrokerElevation = Devolutions.Now.Policy.Api.Elevation; using BrokerOperationStatus = Devolutions.Now.Policy.Api.OperationStatus; +using BrokerStatusResponse = Devolutions.Now.Policy.Api.StatusResponse; +using OperationCancelQuery = Devolutions.Now.Policy.Client.OperationCancelQuery; +using OperationStatusQuery = Devolutions.Now.Policy.Client.OperationStatusQuery; #if WINDOWS using UniGetUI.PackageEngine.Managers.WingetManager; #endif @@ -40,6 +43,21 @@ public abstract class PackageOperation : AbstractProcessOperation /// internal static Func? BrokerTransportFactory; + /// + /// Interval between broker operation status polls. Internal so tests can shorten it. + /// + internal static int BrokerStatusPollIntervalMs = 500; + + /// + /// Maximum time to wait for the broker to accept a cancel request. + /// + internal static TimeSpan BrokerCancelRequestTimeout = TimeSpan.FromSeconds(5); + + /// + /// Maximum time to wait for a canceled broker operation to reach a terminal status. + /// + internal static TimeSpan BrokerCancelConfirmTimeout = TimeSpan.FromSeconds(30); + protected List DesktopShortcutsBeforeStart = []; public readonly IPackage Package; @@ -240,38 +258,45 @@ private async Task PerformBrokerOperation() try { - // Send to broker and poll until completion, honoring operation cancellation. - var status = await client.ExecuteAndWait(request, CancellationToken); + // Submit the operation explicitly (instead of ExecuteAndWait) so the + // operation id is available for broker-side cancellation. + var execution = await client.Execute(request, CancellationToken); - // Log status details. - Line($"Broker status: {status.Status}, exitCode={status.ExitCode}", LineType.Information); - if (!string.IsNullOrWhiteSpace(status.Message)) + if (execution.Decision.Decision != BrokerDecision.Allow) { - Line($" Message: {status.Message}", LineType.Information); + string denialReason = execution.Decision.Reason ?? CoreTools.Translate("No reason provided"); + Line($"Operation denied by policy: {denialReason}", LineType.Error); + Metadata.FailureTitle = CoreTools.Translate("Operation denied by policy"); + Metadata.FailureMessage = denialReason; + return OperationVeredict.Failure; } - var output = DisplayBrokerOutput(status.Stdout); - if (status.Status == BrokerOperationStatus.Completed) + if (execution.Operation is null) { - var veredict = await GetProcessVeredict(status.ExitCode ?? -1, output); - if (veredict is OperationVeredict.Success) - { - Line("Operation completed successfully via agent broker.", LineType.Information); - } - else if (!string.IsNullOrWhiteSpace(status.Message)) - { - Metadata.FailureMessage = status.Message; - } + Line("Broker allowed the operation but did not return an operation submission.", LineType.Error); + Metadata.FailureTitle = CoreTools.Translate("Operation failed via broker"); + Metadata.FailureMessage = CoreTools.Translate( + "The broker accepted the request but did not report an operation to track."); + return OperationVeredict.Failure; + } + + string operationId = execution.Operation.OperationId; + Line($"Broker accepted operation: {operationId}", LineType.VerboseDetails); + + // NOTE: execution.Operation.EventChannel (live output streaming) is intentionally + // not consumed yet; brokered operations show no captured output until then. - return veredict; + BrokerStatusResponse status; + try + { + status = await WaitForBrokerTerminalStatus(client, operationId, CancellationToken); + } + catch (OperationCanceledException) when (CancellationToken.IsCancellationRequested) + { + return await CancelBrokerOperation(client, operationId); } - // Operation failed — surface a user-visible error. - string reason = status.Message ?? $"Exit code: {status.ExitCode}"; - Line($"Operation failed via broker: {reason}", LineType.Error); - Metadata.FailureTitle = CoreTools.Translate("Operation denied or failed via broker"); - Metadata.FailureMessage = reason; - return OperationVeredict.Failure; + return await InterpretBrokerTerminalStatus(status); } catch (OperationCanceledException) { @@ -296,55 +321,138 @@ private async Task PerformBrokerOperation() } /// - /// Fails the operation because the agent broker is unreachable: brokered operations - /// must not fall back to local execution, since policy evaluation and kill/pre/post - /// actions are owned by the broker. Sets the failure metadata and raises - /// so the UI can notify the user. + /// Polls the broker until the operation reaches a terminal status + /// (Completed, Failed or Canceled). /// - private OperationVeredict HandleBrokerUnavailable() + private static async Task WaitForBrokerTerminalStatus( + BrokerClient client, + string operationId, + CancellationToken cancellationToken) { - Line("Agent broker is not available. The operation cannot continue.", LineType.Error); - Logger.Error("[AgentBroker] Broker not available, aborting operation"); - string message = CoreTools.Translate( - "The Devolutions Agent broker is not available. The operation cannot be performed. Please ensure the Devolutions Agent is installed and running."); - Metadata.FailureTitle = CoreTools.Translate("Agent broker unavailable"); - Metadata.FailureMessage = message; - BrokerUnavailable?.Invoke(this, message); - return OperationVeredict.Failure; + while (true) + { + await Task.Delay(BrokerStatusPollIntervalMs, cancellationToken); + + var status = await client.QueryStatus( + new OperationStatusQuery { OperationId = operationId }, + cancellationToken); + + if (status.Status is BrokerOperationStatus.Completed + or BrokerOperationStatus.Failed + or BrokerOperationStatus.Canceled) + { + return status; + } + } } - private List DisplayBrokerOutput(string? encodedStdout) + /// + /// Requests broker-side cancellation of a running operation, then waits (bounded) + /// for the operation to reach a terminal status. The remote process may win the + /// race and complete or fail before the cancel takes effect; in that case the + /// terminal status is honored instead of reporting a cancellation. + /// + private async Task CancelBrokerOperation(BrokerClient client, string operationId) { - List output = []; - if (string.IsNullOrWhiteSpace(encodedStdout)) + Line("Cancellation requested; asking broker to cancel the remote operation...", LineType.Information); + + try + { + using var cancelTimeout = new CancellationTokenSource(BrokerCancelRequestTimeout); + var cancelResponse = await client.Cancel( + new OperationCancelQuery { OperationId = operationId }, + cancelTimeout.Token); + Line($"Broker acknowledged cancel request: {cancelResponse.Status}", LineType.VerboseDetails); + } + catch (Exception ex) { - return output; + // Best-effort: the cancel request is idempotent, and the operation may already + // have reached a terminal state. Still wait below for the terminal status. + Logger.Warn($"[AgentBroker] Cancel request for operation {operationId} failed: {ex}"); + Line("Broker cancel request failed; checking final operation status...", LineType.Information); } - string decoded; try { - decoded = Encoding.UTF8.GetString(Convert.FromBase64String(encodedStdout)); + using var confirmTimeout = new CancellationTokenSource(BrokerCancelConfirmTimeout); + var status = await WaitForBrokerTerminalStatus(client, operationId, confirmTimeout.Token); + + if (status.Status is not BrokerOperationStatus.Canceled) + { + // The remote process finished before the cancel took effect. + Line($"Broker operation finished before cancellation took effect: {status.Status}", LineType.Information); + return await InterpretBrokerTerminalStatus(status); + } } - catch (FormatException ex) + catch (Exception ex) { - Logger.Error($"[AgentBroker] Broker returned invalid base64 stdout: {ex}"); - Line("Broker returned captured output in an invalid format.", LineType.Error); - return output; + // The user asked for cancellation; do not surface polling failures as errors. + Logger.Warn($"[AgentBroker] Could not confirm terminal status of canceled operation {operationId}: {ex}"); + } + + Line("Broker operation was canceled.", LineType.Error); + return OperationVeredict.Canceled; + } + + /// + /// Maps a terminal broker status response to an operation veredict, setting + /// failure metadata where appropriate. + /// + private async Task InterpretBrokerTerminalStatus(BrokerStatusResponse status) + { + Line($"Broker status: {status.Status}, exitCode={status.ExitCode}", LineType.Information); + if (!string.IsNullOrWhiteSpace(status.Message)) + { + Line($" Message: {status.Message}", LineType.Information); + } + + if (status.Status is BrokerOperationStatus.Canceled) + { + Line("Broker operation was canceled.", LineType.Error); + return OperationVeredict.Canceled; } - foreach (var line in decoded.Replace("\r\n", "\n").Replace('\r', '\n').Split('\n')) + if (status.Status is BrokerOperationStatus.Completed) { - if (line.Length == 0) + // Captured output is not available anymore over the status endpoint; live + // output will be restored through the per-operation event channel. + var veredict = await GetProcessVeredict(status.ExitCode ?? -1, []); + if (veredict is OperationVeredict.Success) { - continue; + Line("Operation completed successfully via agent broker.", LineType.Information); + } + else if (!string.IsNullOrWhiteSpace(status.Message)) + { + Metadata.FailureMessage = status.Message; } - output.Add(line); - Line(line, LineType.Information); + return veredict; } - return output; + // Operation failed — surface a user-visible error. + string reason = status.Message ?? $"Exit code: {status.ExitCode}"; + Line($"Operation failed via broker: {reason}", LineType.Error); + Metadata.FailureTitle = CoreTools.Translate("Operation denied or failed via broker"); + Metadata.FailureMessage = reason; + return OperationVeredict.Failure; + } + + /// + /// Fails the operation because the agent broker is unreachable: brokered operations + /// must not fall back to local execution, since policy evaluation and kill/pre/post + /// actions are owned by the broker. Sets the failure metadata and raises + /// so the UI can notify the user. + /// + private OperationVeredict HandleBrokerUnavailable() + { + Line("Agent broker is not available. The operation cannot continue.", LineType.Error); + Logger.Error("[AgentBroker] Broker not available, aborting operation"); + string message = CoreTools.Translate( + "The Devolutions Agent broker is not available. The operation cannot be performed. Please ensure the Devolutions Agent is installed and running."); + Metadata.FailureTitle = CoreTools.Translate("Agent broker unavailable"); + Metadata.FailureMessage = message; + BrokerUnavailable?.Invoke(this, message); + return OperationVeredict.Failure; } private static BrokerClient CreateBrokerClient(bool requestedElevation) => diff --git a/src/UniGetUI.PackageEngine.Tests/PackageOperationsTests.cs b/src/UniGetUI.PackageEngine.Tests/PackageOperationsTests.cs index 01f9de8af0..95381eeab8 100644 --- a/src/UniGetUI.PackageEngine.Tests/PackageOperationsTests.cs +++ b/src/UniGetUI.PackageEngine.Tests/PackageOperationsTests.cs @@ -14,8 +14,21 @@ using UniGetUI.PackageEngine.Tests.Infrastructure.Builders; using UniGetUI.PackageEngine.Tests.Infrastructure.Fakes; using UniGetUI.PackageOperations; +using BrokerApiConstants = Devolutions.Now.Policy.Api.BrokerApi; using BrokerClientErrorKind = Devolutions.Now.Policy.Client.BrokerClientErrorKind; using BrokerClientException = Devolutions.Now.Policy.Client.BrokerClientException; +using BrokerJson = Devolutions.Now.Policy.Api.BrokerJson; +using BrokerApiCancelResponse = Devolutions.Now.Policy.Api.CancelResponse; +using BrokerApiCapabilitiesResponse = Devolutions.Now.Policy.Api.CapabilitiesResponse; +using BrokerApiDecision = Devolutions.Now.Policy.Api.Decision; +using BrokerApiDecisionInfo = Devolutions.Now.Policy.Api.DecisionInfo; +using BrokerApiExecutionResponse = Devolutions.Now.Policy.Api.ExecutionResponse; +using BrokerApiManagerCapability = Devolutions.Now.Policy.Api.ManagerCapability; +using BrokerApiManagerName = Devolutions.Now.Policy.Api.ManagerName; +using BrokerApiOperation = Devolutions.Now.Policy.Api.Operation; +using BrokerApiOperationStatus = Devolutions.Now.Policy.Api.OperationStatus; +using BrokerApiOperationSubmission = Devolutions.Now.Policy.Api.OperationSubmission; +using BrokerApiStatusResponse = Devolutions.Now.Policy.Api.StatusResponse; using BrokerTransportKind = Devolutions.Now.Policy.Api.Transport; using BrokerTransportRequest = Devolutions.Now.Policy.Client.BrokerTransportRequest; using BrokerTransportResponse = Devolutions.Now.Policy.Client.BrokerTransportResponse; @@ -595,6 +608,117 @@ private static async Task AssertBrokerUnavailableFailure(FakeBrokerTransport tra } } + /// + /// Runs an install operation against a scripted broker transport with the UseAgentBroker + /// setting enabled, fast status polling, and short cancel timeouts. The caller scripts the + /// transport behavior and can trigger operation cancellation from transport callbacks. + /// + private static async Task RunBrokeredOperation( + ScriptedBrokerTransport transport, + Action? configureCancellation = null) + { + bool originalSetting = Settings.Get(Settings.K.UseAgentBroker); + int originalPollInterval = PackageOperation.BrokerStatusPollIntervalMs; + TimeSpan originalCancelRequestTimeout = PackageOperation.BrokerCancelRequestTimeout; + TimeSpan originalCancelConfirmTimeout = PackageOperation.BrokerCancelConfirmTimeout; + var manager = new PackageManagerBuilder() + .WithName("Chocolatey") + .ConfigureManager(m => + { + m.ExecutablePath = "C:\\test-tools\\choco.exe"; + m.ExecutableArguments = "--test"; + }) + .Build(); + var package = new PackageBuilder().WithManager(manager).Build(); + PackageOperation.BrokerTransportFactory = () => transport; + PackageOperation.BrokerStatusPollIntervalMs = 5; + PackageOperation.BrokerCancelRequestTimeout = TimeSpan.FromSeconds(2); + PackageOperation.BrokerCancelConfirmTimeout = TimeSpan.FromSeconds(2); + Settings.Set(Settings.K.UseAgentBroker, true); + try + { + using var operation = new BrokerProbingInstallPackageOperation(package, new InstallOptions()); + if (configureCancellation is not null) + { + // Attach a cancellation source the same way MainThread() would, so the + // operation's CancellationToken plumbing is exercised end-to-end. + var cancellationSource = new CancellationTokenSource(); + typeof(AbstractOperation) + .GetField("RunCancellationSource", BindingFlags.Instance | BindingFlags.NonPublic)! + .SetValue(operation, cancellationSource); + configureCancellation(cancellationSource); + } + + return await operation.InvokePerformOperationForTests().WaitAsync(TimeSpan.FromSeconds(10)); + } + finally + { + Settings.Set(Settings.K.UseAgentBroker, originalSetting); + PackageOperation.BrokerTransportFactory = null; + PackageOperation.BrokerStatusPollIntervalMs = originalPollInterval; + PackageOperation.BrokerCancelRequestTimeout = originalCancelRequestTimeout; + PackageOperation.BrokerCancelConfirmTimeout = originalCancelConfirmTimeout; + } + } + + [Fact] + public async Task CancelingBrokeredOperationRequestsRemoteCancelAndYieldsCanceledVeredict() + { + var transport = new ScriptedBrokerTransport(); + transport.StatusScript.Enqueue(BrokerApiOperationStatus.Running); + transport.StatusScript.Enqueue(BrokerApiOperationStatus.Running); + transport.StatusAfterCancel = BrokerApiOperationStatus.Canceled; + + var veredict = await RunBrokeredOperation( + transport, + cancellation => transport.OnStatusQueried = () => + { + if (transport.StatusQueryCount >= 2) + cancellation.Cancel(); + }); + + Assert.Equal(OperationVeredict.Canceled, veredict); + Assert.Equal(1, transport.CancelRequestCount); + Assert.Contains("/v1/package-operations/cancel", transport.RequestedPaths); + } + + [Fact] + public async Task CanceledBrokeredOperationHonorsCompletedTerminalStatusWhenProcessWinsTheRace() + { + var transport = new ScriptedBrokerTransport(); + transport.StatusScript.Enqueue(BrokerApiOperationStatus.Running); + // The remote process finishes before the broker-side cancel takes effect. + transport.StatusAfterCancel = BrokerApiOperationStatus.Completed; + transport.CompletedExitCode = 0; + + var veredict = await RunBrokeredOperation( + transport, + cancellation => transport.OnStatusQueried = () => cancellation.Cancel()); + + Assert.Equal(OperationVeredict.Success, veredict); + Assert.Equal(1, transport.CancelRequestCount); + } + + [Fact] + public async Task FailedBrokerCancelRequestStillYieldsCanceledVeredict() + { + var transport = new ScriptedBrokerTransport + { + FailCancelRequests = true, + }; + transport.StatusScript.Enqueue(BrokerApiOperationStatus.Running); + // The broker keeps reporting a non-terminal status, so the bounded + // confirmation wait times out and the cancellation is honored anyway. + transport.StatusAfterCancel = BrokerApiOperationStatus.Canceling; + + var veredict = await RunBrokeredOperation( + transport, + cancellation => transport.OnStatusQueried = () => cancellation.Cancel()); + + Assert.Equal(OperationVeredict.Canceled, veredict); + Assert.Equal(1, transport.CancelRequestCount); + } + private static IReadOnlyList GetInnerOperations( AbstractOperation operation, string fieldName @@ -685,6 +809,128 @@ public Task Send( public void Dispose() { } } + /// + /// A scriptable broker transport implementing the full happy-path endpoint surface + /// (health, capabilities, execute, get-status, cancel) with responses built from the + /// real Api types via . Status responses are drained from + /// ; once a cancel request has been received (or the script + /// is empty), is reported instead. + /// + private sealed class ScriptedBrokerTransport : IBrokerTransport + { + private const string OperationId = "test-operation-1"; + + public List RequestedPaths { get; } = []; + public Queue StatusScript { get; } = new(); + public BrokerApiOperationStatus StatusAfterCancel { get; set; } = BrokerApiOperationStatus.Canceled; + public int CompletedExitCode { get; set; } + public bool FailCancelRequests { get; set; } + public int StatusQueryCount { get; private set; } + public int CancelRequestCount { get; private set; } + public Action? OnStatusQueried { get; set; } + + private bool cancelReceived; + + public BrokerTransportKind Kind => BrokerTransportKind.HttpNamedPipe; + + public Task Send( + BrokerTransportRequest request, + CancellationToken cancellationToken = default + ) + { + RequestedPaths.Add(request.Path); + return request.Path switch + { + "/v1/health" => Json("{}"), + "/v1/capabilities" => Json(BrokerJson.Serialize(BuildCapabilities())), + "/v1/package-operations/execute" => Json(BrokerJson.Serialize(BuildExecutionResponse())), + "/v1/package-operations/get-status" => HandleStatusQuery(), + "/v1/package-operations/cancel" => HandleCancelRequest(), + _ => throw new BrokerClientException( + BrokerClientErrorKind.InvalidRequest, + $"Unexpected request path: {request.Path}", + request.Path), + }; + } + + public void Dispose() { } + + private static Task Json(string body) => + Task.FromResult(new BrokerTransportResponse { StatusCode = 200, Body = body }); + + private Task HandleStatusQuery() + { + StatusQueryCount++; + OnStatusQueried?.Invoke(); + BrokerApiOperationStatus status = + cancelReceived || StatusScript.Count == 0 + ? StatusAfterCancel + : StatusScript.Dequeue(); + + return Json(BrokerJson.Serialize(new BrokerApiStatusResponse + { + ResponseKind = BrokerApiConstants.StatusResponseKind, + ResponseVersion = BrokerApiConstants.Version, + OperationId = OperationId, + Status = status, + ExitCode = status is BrokerApiOperationStatus.Completed ? CompletedExitCode : null, + })); + } + + private Task HandleCancelRequest() + { + CancelRequestCount++; + if (FailCancelRequests) + { + throw new BrokerClientException( + BrokerClientErrorKind.BrokerError, + "Simulated cancel failure", + "/v1/package-operations/cancel"); + } + + cancelReceived = true; + return Json(BrokerJson.Serialize(new BrokerApiCancelResponse + { + ResponseKind = BrokerApiConstants.CancelResponseKind, + ResponseVersion = BrokerApiConstants.Version, + OperationId = OperationId, + Status = BrokerApiOperationStatus.Canceling, + })); + } + + private static BrokerApiCapabilitiesResponse BuildCapabilities() => new() + { + ResponseKind = BrokerApiConstants.CapabilitiesResponseKind, + ResponseVersion = BrokerApiConstants.Version, + MaxRequestBodyBytes = 1_000_000, + Transports = [BrokerTransportKind.HttpNamedPipe], + Managers = + [ + new BrokerApiManagerCapability + { + Manager = BrokerApiManagerName.Chocolatey, + Operations = [BrokerApiOperation.Install, BrokerApiOperation.Update, BrokerApiOperation.Uninstall], + SupportsCustomParameters = true, + SupportsCustomInstallLocation = true, + SupportsCaptureOutput = true, + }, + ], + }; + + private static BrokerApiExecutionResponse BuildExecutionResponse() => new() + { + ResponseKind = BrokerApiConstants.ExecutionResponseKind, + ResponseVersion = BrokerApiConstants.Version, + Decision = new BrokerApiDecisionInfo { Decision = BrokerApiDecision.Allow }, + Operation = new BrokerApiOperationSubmission + { + OperationId = OperationId, + Status = BrokerApiOperationStatus.Starting, + SubmittedAt = DateTimeOffset.UtcNow, + }, + }; + } + private class InspectableInstallPackageOperation : InstallPackageOperation { public InspectableInstallPackageOperation( From f227124fbd2f682d14c88fbf7f0b83d8ad6193bb Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Thu, 6 Aug 2026 22:10:37 +0300 Subject: [PATCH 2/3] fix(broker): address review feedback on remote cancellation - Add an overall BrokerOperationTimeout (default 1h) so a broker that never reports a terminal status fails the operation with a clear message - Log canceled-operation lines as Information instead of Error - Replace reflection-based cancellation injection in tests with an internal SetRunCancellationSourceForTests hook on AbstractOperation - Serve a real HealthResponse from the scripted test transport - Add a test covering the new operation-timeout failure path Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../AbstractOperation.cs | 10 +++++ .../PackageOperations.cs | 28 ++++++++++-- .../PackageOperationsTests.cs | 45 ++++++++++++++++--- 3 files changed, 74 insertions(+), 9 deletions(-) diff --git a/src/UniGetUI.PackageEngine.Operations/AbstractOperation.cs b/src/UniGetUI.PackageEngine.Operations/AbstractOperation.cs index 233d17a43f..18d06ba5d1 100644 --- a/src/UniGetUI.PackageEngine.Operations/AbstractOperation.cs +++ b/src/UniGetUI.PackageEngine.Operations/AbstractOperation.cs @@ -125,6 +125,16 @@ protected CancellationToken CancellationToken } } + /// + /// Test hook: installs the cancellation source that MainThread() would normally create, + /// so tests invoking PerformOperation() directly can exercise Cancel(). + /// + internal void SetRunCancellationSourceForTests(CancellationTokenSource source) + { + lock (CancellationLock) + RunCancellationSource = source; + } + private bool TrySetActiveInnerOperation(AbstractOperation operation) { bool cancellationRequested; diff --git a/src/UniGetUI.PackageEngine.Operations/PackageOperations.cs b/src/UniGetUI.PackageEngine.Operations/PackageOperations.cs index ecb5659cfa..ef1e20429d 100644 --- a/src/UniGetUI.PackageEngine.Operations/PackageOperations.cs +++ b/src/UniGetUI.PackageEngine.Operations/PackageOperations.cs @@ -58,6 +58,13 @@ public abstract class PackageOperation : AbstractProcessOperation /// internal static TimeSpan BrokerCancelConfirmTimeout = TimeSpan.FromSeconds(30); + /// + /// Upper bound for a brokered operation to reach a terminal status before the + /// operation is reported as failed. Protects against a broker that keeps + /// reporting a non-terminal status indefinitely. + /// + internal static TimeSpan BrokerOperationTimeout = TimeSpan.FromHours(1); + protected List DesktopShortcutsBeforeStart = []; public readonly IPackage Package; @@ -287,20 +294,33 @@ private async Task PerformBrokerOperation() // not consumed yet; brokered operations show no captured output until then. BrokerStatusResponse status; + using var operationTimeout = new CancellationTokenSource(BrokerOperationTimeout); + using var polling = CancellationTokenSource.CreateLinkedTokenSource( + CancellationToken, operationTimeout.Token); try { - status = await WaitForBrokerTerminalStatus(client, operationId, CancellationToken); + status = await WaitForBrokerTerminalStatus(client, operationId, polling.Token); } catch (OperationCanceledException) when (CancellationToken.IsCancellationRequested) { return await CancelBrokerOperation(client, operationId); } + catch (OperationCanceledException) when (operationTimeout.IsCancellationRequested) + { + string timeoutMessage = CoreTools.Translate( + "The operation did not finish within the allotted time. It may still be running on the agent."); + Line($"Broker operation timed out after {BrokerOperationTimeout}.", LineType.Error); + Logger.Error($"[AgentBroker] Operation {operationId} did not reach a terminal status within {BrokerOperationTimeout}"); + Metadata.FailureTitle = CoreTools.Translate("Operation failed via broker"); + Metadata.FailureMessage = timeoutMessage; + return OperationVeredict.Failure; + } return await InterpretBrokerTerminalStatus(status); } catch (OperationCanceledException) { - Line("Broker operation was canceled.", LineType.Error); + Line("Broker operation was canceled.", LineType.Information); return OperationVeredict.Canceled; } catch (BrokerClientException ex) when (ex.Kind is BrokerClientErrorKind.BrokerUnavailable) @@ -390,7 +410,7 @@ private async Task CancelBrokerOperation(BrokerClient client, Logger.Warn($"[AgentBroker] Could not confirm terminal status of canceled operation {operationId}: {ex}"); } - Line("Broker operation was canceled.", LineType.Error); + Line("Broker operation was canceled.", LineType.Information); return OperationVeredict.Canceled; } @@ -408,7 +428,7 @@ private async Task InterpretBrokerTerminalStatus(BrokerStatus if (status.Status is BrokerOperationStatus.Canceled) { - Line("Broker operation was canceled.", LineType.Error); + Line("Broker operation was canceled.", LineType.Information); return OperationVeredict.Canceled; } diff --git a/src/UniGetUI.PackageEngine.Tests/PackageOperationsTests.cs b/src/UniGetUI.PackageEngine.Tests/PackageOperationsTests.cs index 95381eeab8..64d5966fb5 100644 --- a/src/UniGetUI.PackageEngine.Tests/PackageOperationsTests.cs +++ b/src/UniGetUI.PackageEngine.Tests/PackageOperationsTests.cs @@ -23,11 +23,14 @@ using BrokerApiDecision = Devolutions.Now.Policy.Api.Decision; using BrokerApiDecisionInfo = Devolutions.Now.Policy.Api.DecisionInfo; using BrokerApiExecutionResponse = Devolutions.Now.Policy.Api.ExecutionResponse; +using BrokerApiHealthResponse = Devolutions.Now.Policy.Api.HealthResponse; +using BrokerApiHealthStatus = Devolutions.Now.Policy.Api.HealthStatus; using BrokerApiManagerCapability = Devolutions.Now.Policy.Api.ManagerCapability; using BrokerApiManagerName = Devolutions.Now.Policy.Api.ManagerName; using BrokerApiOperation = Devolutions.Now.Policy.Api.Operation; using BrokerApiOperationStatus = Devolutions.Now.Policy.Api.OperationStatus; using BrokerApiOperationSubmission = Devolutions.Now.Policy.Api.OperationSubmission; +using BrokerApiServerContext = Devolutions.Now.Policy.Api.ServerContext; using BrokerApiStatusResponse = Devolutions.Now.Policy.Api.StatusResponse; using BrokerTransportKind = Devolutions.Now.Policy.Api.Transport; using BrokerTransportRequest = Devolutions.Now.Policy.Client.BrokerTransportRequest; @@ -615,12 +618,14 @@ private static async Task AssertBrokerUnavailableFailure(FakeBrokerTransport tra /// private static async Task RunBrokeredOperation( ScriptedBrokerTransport transport, - Action? configureCancellation = null) + Action? configureCancellation = null, + TimeSpan? operationTimeout = null) { bool originalSetting = Settings.Get(Settings.K.UseAgentBroker); int originalPollInterval = PackageOperation.BrokerStatusPollIntervalMs; TimeSpan originalCancelRequestTimeout = PackageOperation.BrokerCancelRequestTimeout; TimeSpan originalCancelConfirmTimeout = PackageOperation.BrokerCancelConfirmTimeout; + TimeSpan originalOperationTimeout = PackageOperation.BrokerOperationTimeout; var manager = new PackageManagerBuilder() .WithName("Chocolatey") .ConfigureManager(m => @@ -634,6 +639,8 @@ private static async Task RunBrokeredOperation( PackageOperation.BrokerStatusPollIntervalMs = 5; PackageOperation.BrokerCancelRequestTimeout = TimeSpan.FromSeconds(2); PackageOperation.BrokerCancelConfirmTimeout = TimeSpan.FromSeconds(2); + if (operationTimeout is not null) + PackageOperation.BrokerOperationTimeout = operationTimeout.Value; Settings.Set(Settings.K.UseAgentBroker, true); try { @@ -643,9 +650,7 @@ private static async Task RunBrokeredOperation( // Attach a cancellation source the same way MainThread() would, so the // operation's CancellationToken plumbing is exercised end-to-end. var cancellationSource = new CancellationTokenSource(); - typeof(AbstractOperation) - .GetField("RunCancellationSource", BindingFlags.Instance | BindingFlags.NonPublic)! - .SetValue(operation, cancellationSource); + operation.SetRunCancellationSourceForTests(cancellationSource); configureCancellation(cancellationSource); } @@ -658,6 +663,7 @@ private static async Task RunBrokeredOperation( PackageOperation.BrokerStatusPollIntervalMs = originalPollInterval; PackageOperation.BrokerCancelRequestTimeout = originalCancelRequestTimeout; PackageOperation.BrokerCancelConfirmTimeout = originalCancelConfirmTimeout; + PackageOperation.BrokerOperationTimeout = originalOperationTimeout; } } @@ -719,6 +725,23 @@ public async Task FailedBrokerCancelRequestStillYieldsCanceledVeredict() Assert.Equal(1, transport.CancelRequestCount); } + [Fact] + public async Task BrokeredOperationThatNeverReachesTerminalStatusFailsAfterTimeout() + { + var transport = new ScriptedBrokerTransport + { + // The broker keeps reporting Running forever. + StatusAfterCancel = BrokerApiOperationStatus.Running, + }; + + var veredict = await RunBrokeredOperation( + transport, + operationTimeout: TimeSpan.FromMilliseconds(200)); + + Assert.Equal(OperationVeredict.Failure, veredict); + Assert.Equal(0, transport.CancelRequestCount); + } + private static IReadOnlyList GetInnerOperations( AbstractOperation operation, string fieldName @@ -841,7 +864,7 @@ public Task Send( RequestedPaths.Add(request.Path); return request.Path switch { - "/v1/health" => Json("{}"), + "/v1/health" => Json(BrokerJson.Serialize(BuildHealthResponse())), "/v1/capabilities" => Json(BrokerJson.Serialize(BuildCapabilities())), "/v1/package-operations/execute" => Json(BrokerJson.Serialize(BuildExecutionResponse())), "/v1/package-operations/get-status" => HandleStatusQuery(), @@ -898,6 +921,18 @@ private Task HandleCancelRequest() })); } + private static BrokerApiHealthResponse BuildHealthResponse() => new() + { + ResponseKind = BrokerApiConstants.HealthResponseKind, + ResponseVersion = BrokerApiConstants.Version, + Server = new BrokerApiServerContext + { + ServerVersion = "0.0.0-tests", + Transport = BrokerTransportKind.HttpNamedPipe, + }, + Status = BrokerApiHealthStatus.Ready, + }; + private static BrokerApiCapabilitiesResponse BuildCapabilities() => new() { ResponseKind = BrokerApiConstants.CapabilitiesResponseKind, From a4fdd9c13f8221e6dbda38500a12bf672827134e Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Fri, 7 Aug 2026 12:45:48 +0300 Subject: [PATCH 3/3] style: fix using-directive ordering in PackageOperationsTests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../PackageOperationsTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/UniGetUI.PackageEngine.Tests/PackageOperationsTests.cs b/src/UniGetUI.PackageEngine.Tests/PackageOperationsTests.cs index 64d5966fb5..2029198600 100644 --- a/src/UniGetUI.PackageEngine.Tests/PackageOperationsTests.cs +++ b/src/UniGetUI.PackageEngine.Tests/PackageOperationsTests.cs @@ -14,12 +14,9 @@ using UniGetUI.PackageEngine.Tests.Infrastructure.Builders; using UniGetUI.PackageEngine.Tests.Infrastructure.Fakes; using UniGetUI.PackageOperations; -using BrokerApiConstants = Devolutions.Now.Policy.Api.BrokerApi; -using BrokerClientErrorKind = Devolutions.Now.Policy.Client.BrokerClientErrorKind; -using BrokerClientException = Devolutions.Now.Policy.Client.BrokerClientException; -using BrokerJson = Devolutions.Now.Policy.Api.BrokerJson; using BrokerApiCancelResponse = Devolutions.Now.Policy.Api.CancelResponse; using BrokerApiCapabilitiesResponse = Devolutions.Now.Policy.Api.CapabilitiesResponse; +using BrokerApiConstants = Devolutions.Now.Policy.Api.BrokerApi; using BrokerApiDecision = Devolutions.Now.Policy.Api.Decision; using BrokerApiDecisionInfo = Devolutions.Now.Policy.Api.DecisionInfo; using BrokerApiExecutionResponse = Devolutions.Now.Policy.Api.ExecutionResponse; @@ -32,6 +29,9 @@ using BrokerApiOperationSubmission = Devolutions.Now.Policy.Api.OperationSubmission; using BrokerApiServerContext = Devolutions.Now.Policy.Api.ServerContext; using BrokerApiStatusResponse = Devolutions.Now.Policy.Api.StatusResponse; +using BrokerClientErrorKind = Devolutions.Now.Policy.Client.BrokerClientErrorKind; +using BrokerClientException = Devolutions.Now.Policy.Client.BrokerClientException; +using BrokerJson = Devolutions.Now.Policy.Api.BrokerJson; using BrokerTransportKind = Devolutions.Now.Policy.Api.Transport; using BrokerTransportRequest = Devolutions.Now.Policy.Client.BrokerTransportRequest; using BrokerTransportResponse = Devolutions.Now.Policy.Client.BrokerTransportResponse;