refactor: move guest lifecycle polling into a GuestLifecycleService (#157) (#228)

WaitForStatusTransition and InvokeGuestTask lived on PveCmdletBase with
hard-coded qemu/lxc paths, a Thread.Sleep(2000) loop and their own
PveHttpClient, so ADR 0015's lock-clear wait and ADR 0020's flock retry
could only be exercised through the 45-minute integration lane.

Moves both into a new GuestLifecycleService on PveServiceBase, following
TaskService's shape: a parameterless ctor, an IPveHttpClient-injecting
ctor, and an internal ctor with a Func<TimeSpan, Task> pollDelay seam so
a test can drive the poll loop without sleeping. PveCmdletBase keeps the
same protected method signatures as thin forwarders wiring WriteVerbose
into a single Action<string>? onProgress callback, so none of the 14
cmdlet call sites change.

ParseLinks moves to a pure CorosyncLinks.Parse in Core (dictionary plus
the malformed entries), with PveCmdletBase.ParseLinks kept as a forwarder
that emits the WriteWarning — the same forwarder shape as the lifecycle
methods, so the warning stays in one place instead of being copied into
the three cluster cmdlets that call it.

Behaviour is unchanged: same status/current polling per guest type, same
GuestStatusSnapshot.Evaluate lock semantics, same filtered PveApiException
catch, same GuestLockRetry wrapping, same PveTaskTimeoutException.

Co-authored-by: goodolclint-claude[bot] <323206664+goodolclint-claude[bot]@users.noreply.github.com>

src/PSProxmoxVE.Core/Services/GuestLifecycleService.cs

@@ -0,0 +1,180 @@
+using System;
+using System.Net;
+using System.Threading;
+using System.Threading.Tasks;
+using PSProxmoxVE.Core.Authentication;
+using PSProxmoxVE.Core.Client;
+using PSProxmoxVE.Core.Exceptions;
+using PSProxmoxVE.Core.Models.Vms;
+using PSProxmoxVE.Core.Utilities;
+
+namespace PSProxmoxVE.Core.Services
+{
+    /// <summary>
+    /// Issues guest (VM/container) lifecycle operations and waits for them to actually take
+    /// effect: past the task PVE returns, and past the guest config lock the task's completion
+    /// does not account for. See docs/decisions/ ADR 0015 (the config-lock wait) and ADR 0020
+    /// (the flock retry <see cref="GuestLockRetry"/> implements).
+    /// </summary>
+    public class GuestLifecycleService : PveServiceBase
+    {
+        private static readonly TimeSpan PollInterval = TimeSpan.FromSeconds(2);
+
+        private readonly Func<TimeSpan, Task> _pollDelay;
+
+        /// <summary>Initializes a new instance that creates its own HTTP clients.</summary>
+        public GuestLifecycleService() : this(Sleep) { }
+
+        /// <summary>Initializes a new instance that uses the supplied HTTP client for all requests.</summary>
+        /// <param name="client">The HTTP client to use. The caller owns its lifetime.</param>
+        public GuestLifecycleService(IPveHttpClient client) : this(client, Sleep) { }
+
+        /// <summary>
+        /// Test seam: same as <see cref="GuestLifecycleService()"/> but with the wait between
+        /// status polls replaceable, so a test can drive the loop without sleeping for it.
+        /// </summary>
+        /// <param name="pollDelay">Invoked with each poll interval instead of sleeping.</param>
+        internal GuestLifecycleService(Func<TimeSpan, Task> pollDelay)
+        {
+            _pollDelay = pollDelay ?? throw new ArgumentNullException(nameof(pollDelay));
+        }
+
+        /// <summary>
+        /// Test seam: same as <see cref="GuestLifecycleService(IPveHttpClient)"/> but with the
+        /// wait between status polls replaceable.
+        /// </summary>
+        /// <param name="client">The HTTP client to use. The caller owns its lifetime.</param>
+        /// <param name="pollDelay">Invoked with each poll interval instead of sleeping.</param>
+        internal GuestLifecycleService(IPveHttpClient client, Func<TimeSpan, Task> pollDelay) : base(client)
+        {
+            _pollDelay = pollDelay ?? throw new ArgumentNullException(nameof(pollDelay));
+        }
+
+        private static Task Sleep(TimeSpan duration)
+        {
+            Thread.Sleep(duration);
+            return Task.CompletedTask;
+        }
+
+        /// <summary>
+        /// Issues a guest operation and waits for the task it returns, reissuing the pair while
+        /// PVE rejects it for the guest's config flock.
+        ///
+        /// PVE takes that flock inside the worker for most guest operations, so the failure
+        /// surfaces as a failed task rather than a failed request and cannot be retried at the
+        /// HTTP layer. <c>lock_config</c> raises it before doing any work, so a reissue repeats
+        /// nothing.
+        /// </summary>
+        /// <param name="session">The authenticated PVE session.</param>
+        /// <param name="node">The node the task runs on.</param>
+        /// <param name="issueOperation">Issues the API call; invoked again on each retry.</param>
+        /// <param name="onProgress">
+        ///   Invoked with a progress message before each reissue. A caller with somewhere to
+        ///   report progress should pass one — a wait this long is otherwise indistinguishable
+        ///   from a hang.
+        /// </param>
+        /// <returns>The completed task, or the issued task when the call returned no UPID.</returns>
+        public PveTask InvokeGuestTask(
+            PveSession session,
+            string node,
+            Func<PveTask> issueOperation,
+            Action<string>? onProgress = null)
+        {
+            if (issueOperation == null) throw new ArgumentNullException(nameof(issueOperation));
+
+            return Invoke(session, client =>
+            {
+                var taskService = new TaskService(client);
+                return GuestLockRetry.Execute(
+                    () =>
+                    {
+                        var task = issueOperation();
+                        return string.IsNullOrEmpty(task.Upid)
+                            ? task
+                            : taskService.WaitForTask(session, node, task.Upid);
+                    },
+                    onRetry: ex => onProgress?.Invoke($"Guest is locked, retrying: {ex.Message}"));
+            });
+        }
+
+        /// <summary>
+        /// Waits for a PVE task to complete, then polls guest status until it matches
+        /// <paramref name="expectedStatus"/> and its config lock has cleared. Used by lifecycle
+        /// cmdlets (Start, Stop, Suspend, Resume, etc.) when -Wait is specified.
+        /// </summary>
+        /// <param name="session">The authenticated PVE session.</param>
+        /// <param name="node">The cluster node name.</param>
+        /// <param name="issueOperation">
+        ///   Issues the lifecycle API call. Invoked again on each retry, so it must be safe to
+        ///   repeat — see <see cref="InvokeGuestTask"/>.
+        /// </param>
+        /// <param name="vmid">The VM or container ID to poll.</param>
+        /// <param name="expectedStatus">The expected status string (e.g. "running", "stopped", "paused").</param>
+        /// <param name="timeoutSeconds">Maximum seconds to wait for the status transition. Default 60.</param>
+        /// <param name="isContainer">True to poll container status instead of VM status.</param>
+        /// <param name="onProgress">
+        ///   Invoked with a progress message when a status poll fails and is retried, and
+        ///   forwarded to <see cref="InvokeGuestTask"/> for its own retry reporting.
+        /// </param>
+        /// <returns>The completed task.</returns>
+        public PveTask WaitForStatusTransition(
+            PveSession session,
+            string node,
+            Func<PveTask> issueOperation,
+            int vmid,
+            string expectedStatus,
+            int timeoutSeconds = 60,
+            bool isContainer = false,
+            Action<string>? onProgress = null)
+        {
+            var task = InvokeGuestTask(session, node, issueOperation, onProgress);
+
+            // We query the status/current endpoint directly instead of the list endpoint
+            // because it returns qmpstatus (needed for paused state detection — PVE reports
+            // status=running but qmpstatus=paused for suspended VMs).
+            var statusResource = isContainer
+                ? $"nodes/{Uri.EscapeDataString(node)}/lxc/{vmid}/status/current"
+                : $"nodes/{Uri.EscapeDataString(node)}/qemu/{vmid}/status/current";
+
+            var deadline = DateTime.UtcNow.AddSeconds(timeoutSeconds);
+            var lastMatched = false;
+
+            return Invoke(session, client =>
+            {
+                while (DateTime.UtcNow < deadline)
+                {
+                    try
+                    {
+                        var json = client.GetAsync(statusResource).GetAwaiter().GetResult();
+                        var snapshot = GuestStatusSnapshot.Evaluate(json, expectedStatus);
+                        lastMatched = snapshot.StatusMatched;
+
+                        // snapshot.Locked is the config `lock:` property (backup, clone, migrate,
+                        // snapshot) — not the /var/lock/qemu-server flock, which PVE does not
+                        // expose. The flock race is handled by retrying, not by waiting.
+                        if (snapshot.StatusMatched && !snapshot.Locked)
+                            return task;
+                    }
+                    catch (PveApiException ex) when (
+                        ex.StatusCode != HttpStatusCode.Unauthorized
+                        && ex.StatusCode != HttpStatusCode.Forbidden
+                        && ex.StatusCode != HttpStatusCode.NotFound)
+                    {
+                        onProgress?.Invoke($"Status poll failed, retrying: {ex.Message}");
+                    }
+
+                    _pollDelay(PollInterval).GetAwaiter().GetResult();
+                }
+
+                // The guest still reports the expected status on the final poll and only the
+                // lock outlasted the deadline.
+                if (lastMatched)
+                    return task;
+
+                throw new PveTaskTimeoutException(
+                    task.Upid ?? "unknown",
+                    TimeSpan.FromSeconds(timeoutSeconds));
+            });
+        }
+    }
+}

src/PSProxmoxVE.Core/Utilities/CorosyncLinks.cs

@@ -0,0 +1,40 @@
+using System;
+using System.Collections.Generic;
+
+namespace PSProxmoxVE.Core.Utilities
+{
+    /// <summary>
+    /// Parses Corosync link strings (e.g. "link0=10.0.0.1") as sent to the cluster
+    /// create/join/add-node endpoints.
+    /// </summary>
+    public static class CorosyncLinks
+    {
+        /// <summary>
+        /// Parses an array of Corosync link strings into a dictionary.
+        /// </summary>
+        /// <param name="links">Array of link strings in "linkN=address" format.</param>
+        /// <returns>
+        /// The parsed dictionary (null if <paramref name="links"/> is null, or if every entry
+        /// was malformed), and the malformed entries verbatim — the caller decides how to
+        /// report them.
+        /// </returns>
+        public static (Dictionary<string, string>? Links, IReadOnlyList<string> Malformed) Parse(string[]? links)
+        {
+            if (links == null) return (null, Array.Empty<string>());
+
+            var result = new Dictionary<string, string>();
+            var malformed = new List<string>();
+            foreach (var link in links)
+            {
+                var parts = link?.Split(new[] { '=' }, 2) ?? Array.Empty<string>();
+                if (parts.Length != 2 || string.IsNullOrWhiteSpace(parts[0]) || string.IsNullOrWhiteSpace(parts[1]))
+                {
+                    malformed.Add(link ?? string.Empty);
+                    continue;
+                }
+                result[parts[0].Trim()] = parts[1].Trim();
+            }
+            return (result.Count > 0 ? result : null, malformed);
+        }
+    }
+}

src/PSProxmoxVE/Cmdlets/PveCmdletBase.cs

@@ -2,7 +2,6 @@ using System;
 using System.Collections.Generic;
 using System.Management.Automation;
 using PSProxmoxVE.Core.Authentication;
-using PSProxmoxVE.Core.Client;
 using PSProxmoxVE.Core.Errors;
 using PSProxmoxVE.Core.Exceptions;
 using PSProxmoxVE.Core.Models.Vms;
@@ -204,52 +203,9 @@ namespace PSProxmoxVE.Cmdlets
             int timeoutSeconds = 60,
             bool isContainer = false)
         {
-            var task = InvokeGuestTask(session, node, issueOperation);
-
-            // Then poll status/current until VM/container reaches the expected status.
-            // We query the status/current endpoint directly instead of the list endpoint
-            // because it returns qmpstatus (needed for paused state detection — PVE reports
-            // status=running but qmpstatus=paused for suspended VMs).
-            var statusResource = isContainer
-                ? $"nodes/{Uri.EscapeDataString(node)}/lxc/{vmid}/status/current"
-                : $"nodes/{Uri.EscapeDataString(node)}/qemu/{vmid}/status/current";
-
-            var deadline = DateTime.UtcNow.AddSeconds(timeoutSeconds);
-            var lastMatched = false;
-            using var pollClient = new PveHttpClient(session);
-            while (DateTime.UtcNow < deadline)
-            {
-                try
-                {
-                    var json = pollClient.GetAsync(statusResource).GetAwaiter().GetResult();
-                    var snapshot = GuestStatusSnapshot.Evaluate(json, expectedStatus);
-                    lastMatched = snapshot.StatusMatched;
-
-                    // snapshot.Locked is the config `lock:` property (backup, clone, migrate,
-                    // snapshot) — not the /var/lock/qemu-server flock, which PVE does not
-                    // expose. The flock race is handled by retrying, not by waiting.
-                    if (snapshot.StatusMatched && !snapshot.Locked)
-                        return task;
-                }
-                catch (PSProxmoxVE.Core.Exceptions.PveApiException ex) when (
-                    ex.StatusCode != System.Net.HttpStatusCode.Unauthorized
-                    && ex.StatusCode != System.Net.HttpStatusCode.Forbidden
-                    && ex.StatusCode != System.Net.HttpStatusCode.NotFound)
-                {
-                    WriteVerbose($"Status poll failed, retrying: {ex.Message}");
-                }
-
-                System.Threading.Thread.Sleep(2000);
-            }
-
-            // The guest still reports the expected status on the final poll and only the
-            // lock outlasted the deadline.
-            if (lastMatched)
-                return task;
-
-            throw new PveTaskTimeoutException(
-                task.Upid ?? "unknown",
-                TimeSpan.FromSeconds(timeoutSeconds));
+            return new GuestLifecycleService().WaitForStatusTransition(
+                session, node, issueOperation, vmid, expectedStatus, timeoutSeconds, isContainer,
+                onProgress: WriteVerbose);
         }
 
         /// <summary>
@@ -267,18 +223,7 @@ namespace PSProxmoxVE.Cmdlets
         /// <returns>The completed task, or the issued task when the call returned no UPID.</returns>
         protected PveTask InvokeGuestTask(PveSession session, string node, Func<PveTask> issueOperation)
         {
-            if (issueOperation == null) throw new ArgumentNullException(nameof(issueOperation));
-
-            var taskService = new TaskService();
-            return GuestLockRetry.Execute(
-                () =>
-                {
-                    var task = issueOperation();
-                    return string.IsNullOrEmpty(task.Upid)
-                        ? task
-                        : taskService.WaitForTask(session, node, task.Upid);
-                },
-                onRetry: ex => WriteVerbose($"Guest is locked, retrying: {ex.Message}"));
+            return new GuestLifecycleService().InvokeGuestTask(session, node, issueOperation, onProgress: WriteVerbose);
         }
 
         /// <summary>
@@ -304,20 +249,10 @@ namespace PSProxmoxVE.Cmdlets
         /// <returns>Dictionary of parsed link entries, or null if input is null.</returns>
         protected Dictionary<string, string>? ParseLinks(string[]? links)
         {
-            if (links == null) return null;
-
-            var result = new Dictionary<string, string>();
-            foreach (var link in links)
-            {
-                var parts = link.Split(new[] { '=' }, 2);
-                if (parts.Length != 2 || string.IsNullOrWhiteSpace(parts[0]) || string.IsNullOrWhiteSpace(parts[1]))
-                {
-                    WriteWarning($"Ignoring malformed link entry '{link}'. Expected format: 'link0=10.0.0.1'");
-                    continue;
-                }
-                result[parts[0].Trim()] = parts[1].Trim();
-            }
-            return result.Count > 0 ? result : null;
+            var (result, malformed) = CorosyncLinks.Parse(links);
+            foreach (var link in malformed)
+                WriteWarning($"Ignoring malformed link entry '{link}'. Expected format: 'link0=10.0.0.1'");
+            return result;
         }
     }
 }

tests/PSProxmoxVE.Core.Tests/Services/GuestLifecycleServiceTests.cs

@@ -0,0 +1,292 @@
+using System;
+using System.Collections.Generic;
+using System.Net;
+using System.Threading.Tasks;
+using Moq;
+using Xunit;
+using PSProxmoxVE.Core.Authentication;
+using PSProxmoxVE.Core.Client;
+using PSProxmoxVE.Core.Exceptions;
+using PSProxmoxVE.Core.Models.Vms;
+using PSProxmoxVE.Core.Services;
+
+namespace PSProxmoxVE.Core.Tests.Services
+{
+    public class GuestLifecycleServiceTests
+    {
+        private const string TestNode = "pve1";
+        private const string TestUpid = "UPID:pve1:000ABC:00000001:5F1234AB:qmstart:100:root@pam:";
+        private const int VmId = 100;
+
+        private static PveSession CreateSession() =>
+            new PveSession("pve1.example.com", 8006, true, "PVE:root@pam:TEST_TOKEN");
+
+        private static PveTask IssuedTask() => new PveTask { Upid = TestUpid, Status = "running", Node = TestNode };
+
+        private const string TasksStatusFragment = "/tasks/";
+        private const string StoppedTaskJson = @"{ ""data"": { ""upid"": """ + TestUpid + @""", ""status"": ""stopped"", ""exitstatus"": ""OK"", ""user"": ""root@pam"" } }";
+
+        /// <summary>Stubs the task-status poll (issued by <see cref="TaskService.WaitForTask"/>) to resolve immediately.</summary>
+        private static void StubTaskCompletesImmediately(Mock<IPveHttpClient> mockClient)
+        {
+            mockClient.Setup(c => c.GetAsync(It.Is<string>(s => s.Contains(TasksStatusFragment))))
+                .ReturnsAsync(StoppedTaskJson);
+        }
+
+        private static string GuestStatusJson(string status, bool locked = false)
+        {
+            var lockField = locked ? @",""lock"":""backup""" : "";
+            return $@"{{ ""data"": {{ ""status"":""{status}""{lockField} }} }}";
+        }
+
+        private static (GuestLifecycleService service, List<TimeSpan> delays) ServiceWithRecordedDelays(Mock<IPveHttpClient> mockClient)
+        {
+            var delays = new List<TimeSpan>();
+            var service = new GuestLifecycleService(mockClient.Object, d => { delays.Add(d); return Task.CompletedTask; });
+            return (service, delays);
+        }
+
+        [Fact]
+        public void WaitForStatusTransition_ReachesExpectedStatusOnSecondPoll_ReturnsTaskAndPollsQemuPath()
+        {
+            var mockClient = new Mock<IPveHttpClient>();
+            StubTaskCompletesImmediately(mockClient);
+            mockClient.SetupSequence(c => c.GetAsync(It.Is<string>(s => s.Contains("status/current"))))
+                .ReturnsAsync(GuestStatusJson("stopped"))
+                .ReturnsAsync(GuestStatusJson("running"));
+
+            var (service, delays) = ServiceWithRecordedDelays(mockClient);
+
+            var task = service.WaitForStatusTransition(
+                CreateSession(), TestNode, IssuedTask, VmId, "running", timeoutSeconds: 60);
+
+            Assert.Equal(TestUpid, task.Upid);
+            var expectedPath = $"nodes/{TestNode}/qemu/{VmId}/status/current";
+            mockClient.Verify(c => c.GetAsync(expectedPath), Times.Exactly(2));
+            Assert.Single(delays);
+        }
+
+        [Fact]
+        public void WaitForStatusTransition_ContainerGuest_PollsLxcPath()
+        {
+            var mockClient = new Mock<IPveHttpClient>();
+            StubTaskCompletesImmediately(mockClient);
+            mockClient.Setup(c => c.GetAsync(It.Is<string>(s => s.Contains("status/current"))))
+                .ReturnsAsync(GuestStatusJson("running"));
+
+            var (service, _) = ServiceWithRecordedDelays(mockClient);
+
+            var task = service.WaitForStatusTransition(
+                CreateSession(), TestNode, IssuedTask, VmId, "running", timeoutSeconds: 60, isContainer: true);
+
+            Assert.Equal(TestUpid, task.Upid);
+            var expectedPath = $"nodes/{TestNode}/lxc/{VmId}/status/current";
+            mockClient.Verify(c => c.GetAsync(expectedPath), Times.Once);
+        }
+
+        [Fact]
+        public void WaitForStatusTransition_StatusMatchedButLocked_KeepsPollingUntilLockClears()
+        {
+            var mockClient = new Mock<IPveHttpClient>();
+            StubTaskCompletesImmediately(mockClient);
+            mockClient.SetupSequence(c => c.GetAsync(It.Is<string>(s => s.Contains("status/current"))))
+                .ReturnsAsync(GuestStatusJson("running", locked: true))
+                .ReturnsAsync(GuestStatusJson("running", locked: true))
+                .ReturnsAsync(GuestStatusJson("running", locked: false));
+
+            var (service, delays) = ServiceWithRecordedDelays(mockClient);
+
+            var task = service.WaitForStatusTransition(
+                CreateSession(), TestNode, IssuedTask, VmId, "running", timeoutSeconds: 60);
+
+            Assert.Equal(TestUpid, task.Upid);
+            var expectedPath = $"nodes/{TestNode}/qemu/{VmId}/status/current";
+            mockClient.Verify(c => c.GetAsync(expectedPath), Times.Exactly(3));
+            Assert.Equal(2, delays.Count);
+        }
+
+        [Fact]
+        public void WaitForStatusTransition_MatchedOnFinalPollButStillLocked_ReturnsTaskInsteadOfThrowing()
+        {
+            // ADR 0015: the guest reports the expected status right up to the deadline; only
+            // the lock outlasted the wait, and that must not fail the call.
+            var mockClient = new Mock<IPveHttpClient>();
+            StubTaskCompletesImmediately(mockClient);
+            mockClient.Setup(c => c.GetAsync(It.Is<string>(s => s.Contains("status/current"))))
+                .ReturnsAsync(GuestStatusJson("running", locked: true));
+
+            var (service, _) = ServiceWithRecordedDelays(mockClient);
+
+            var task = service.WaitForStatusTransition(
+                CreateSession(), TestNode, IssuedTask, VmId, "running", timeoutSeconds: 1);
+
+            Assert.Equal(TestUpid, task.Upid);
+        }
+
+        [Fact]
+        public void WaitForStatusTransition_NeverMatches_ThrowsPveTaskTimeoutExceptionWithTheUpid()
+        {
+            var mockClient = new Mock<IPveHttpClient>();
+            StubTaskCompletesImmediately(mockClient);
+            mockClient.Setup(c => c.GetAsync(It.Is<string>(s => s.Contains("status/current"))))
+                .ReturnsAsync(GuestStatusJson("stopped"));
+
+            var (service, _) = ServiceWithRecordedDelays(mockClient);
+
+            var ex = Assert.Throws<PveTaskTimeoutException>(() =>
+                service.WaitForStatusTransition(
+                    CreateSession(), TestNode, IssuedTask, VmId, "running", timeoutSeconds: 1));
+
+            Assert.Equal(TestUpid, ex.Upid);
+        }
+
+        [Fact]
+        public void WaitForStatusTransition_MatchedEarlierThenDriftedAway_StillTimesOut()
+        {
+            // ADR 0015: the fallback tests the most recent observation, not "matched at some
+            // point during the wait" — a guest that reached the expected status (here, while
+            // still locked, so the wait keeps going) and then drifted away has not satisfied
+            // the wait, even though it matched earlier.
+            var mockClient = new Mock<IPveHttpClient>();
+            StubTaskCompletesImmediately(mockClient);
+            var statusPath = $"nodes/{TestNode}/qemu/{VmId}/status/current";
+            var pollCount = 0;
+            mockClient.Setup(c => c.GetAsync(statusPath))
+                .ReturnsAsync(() =>
+                {
+                    pollCount++;
+                    return pollCount <= 2
+                        ? GuestStatusJson("running", locked: true)
+                        : GuestStatusJson("stopped");
+                });
+
+            var (service, _) = ServiceWithRecordedDelays(mockClient);
+
+            var ex = Assert.Throws<PveTaskTimeoutException>(() =>
+                service.WaitForStatusTransition(
+                    CreateSession(), TestNode, IssuedTask, VmId, "running", timeoutSeconds: 1));
+
+            Assert.Equal(TestUpid, ex.Upid);
+            Assert.True(pollCount > 2, $"expected more than two polls before the deadline, got {pollCount}");
+        }
+
+        [Fact]
+        public void WaitForStatusTransition_A500FromThePoll_IsRetriedAndReportedThroughOnProgress()
+        {
+            var mockClient = new Mock<IPveHttpClient>();
+            StubTaskCompletesImmediately(mockClient);
+            var statusPath = $"nodes/{TestNode}/qemu/{VmId}/status/current";
+            mockClient.SetupSequence(c => c.GetAsync(statusPath))
+                .ThrowsAsync(new PveApiException(HttpStatusCode.InternalServerError, "temporary failure", statusPath, "GET"))
+                .ReturnsAsync(GuestStatusJson("running"));
+
+            var (service, _) = ServiceWithRecordedDelays(mockClient);
+            var progressMessages = new List<string>();
+
+            var task = service.WaitForStatusTransition(
+                CreateSession(), TestNode, IssuedTask, VmId, "running", timeoutSeconds: 60,
+                onProgress: progressMessages.Add);
+
+            Assert.Equal(TestUpid, task.Upid);
+            Assert.Single(progressMessages);
+            Assert.Contains("temporary failure", progressMessages[0]);
+        }
+
+        [Theory]
+        [InlineData(HttpStatusCode.Unauthorized)]
+        [InlineData(HttpStatusCode.Forbidden)]
+        [InlineData(HttpStatusCode.NotFound)]
+        public void WaitForStatusTransition_AnAuthOrNotFoundFailure_PropagatesInsteadOfRetrying(HttpStatusCode statusCode)
+        {
+            var mockClient = new Mock<IPveHttpClient>();
+            StubTaskCompletesImmediately(mockClient);
+            var statusPath = $"nodes/{TestNode}/qemu/{VmId}/status/current";
+            mockClient.Setup(c => c.GetAsync(statusPath))
+                .ThrowsAsync(new PveApiException(statusCode, "denied", statusPath, "GET"));
+
+            var (service, _) = ServiceWithRecordedDelays(mockClient);
+
+            var ex = Assert.Throws<PveApiException>(() =>
+                service.WaitForStatusTransition(
+                    CreateSession(), TestNode, IssuedTask, VmId, "running", timeoutSeconds: 60));
+
+            Assert.Equal(statusCode, ex.StatusCode);
+            mockClient.Verify(c => c.GetAsync(statusPath), Times.Once);
+        }
+
+        [Fact]
+        public void InvokeGuestTask_ReissuesOnGuestLockFailure()
+        {
+            var mockClient = new Mock<IPveHttpClient>();
+            StubTaskCompletesImmediately(mockClient);
+
+            var invocationCount = 0;
+            PveTask Issue()
+            {
+                invocationCount++;
+                if (invocationCount == 1)
+                    throw new PveApiException(
+                        HttpStatusCode.InternalServerError,
+                        "can't lock file '/var/lock/qemu-server/lock-100.conf' - got timeout",
+                        $"nodes/{TestNode}/qemu/{VmId}/config",
+                        "PUT");
+                return IssuedTask();
+            }
+
+            var service = new GuestLifecycleService(mockClient.Object);
+            var progressMessages = new List<string>();
+
+            var task = service.InvokeGuestTask(CreateSession(), TestNode, Issue, onProgress: progressMessages.Add);
+
+            Assert.Equal(TestUpid, task.Upid);
+            Assert.Equal(2, invocationCount);
+            Assert.Single(progressMessages);
+        }
+
+        [Fact]
+        public void InvokeGuestTask_TaskFailsOnGuestLock_ReissuesTheWholeOperation()
+        {
+            // ADR 0020: PVE takes the flock inside the worker for most guest operations, so
+            // the failure surfaces as a failed task rather than a failed request and the whole
+            // issue-and-wait pair must be reissued, not just the request.
+            var mockClient = new Mock<IPveHttpClient>();
+            const string lockFailedJson = @"{ ""data"": { ""upid"": """ + TestUpid + @""", ""status"": ""stopped"", "
+                + @"""exitstatus"": ""can't lock file '/var/lock/qemu-server/lock-100.conf' - got timeout"", "
+                + @"""user"": ""root@pam"" } }";
+            mockClient.SetupSequence(c => c.GetAsync(It.Is<string>(s => s.Contains(TasksStatusFragment))))
+                .ReturnsAsync(lockFailedJson)
+                .ReturnsAsync(StoppedTaskJson);
+
+            var invocationCount = 0;
+            PveTask Issue()
+            {
+                invocationCount++;
+                return IssuedTask();
+            }
+
+            var service = new GuestLifecycleService(mockClient.Object);
+            var progressMessages = new List<string>();
+
+            var task = service.InvokeGuestTask(CreateSession(), TestNode, Issue, onProgress: progressMessages.Add);
+
+            Assert.Equal(TestUpid, task.Upid);
+            Assert.Equal(2, invocationCount);
+            Assert.Single(progressMessages);
+        }
+
+        [Fact]
+        public void InvokeGuestTask_EmptyUpid_ReturnsTheIssuedTaskWithoutWaiting()
+        {
+            var mockClient = new Mock<IPveHttpClient>();
+            var issued = new PveTask { Upid = "", Status = "running", Node = TestNode };
+            PveTask Issue() => issued;
+
+            var service = new GuestLifecycleService(mockClient.Object);
+
+            var task = service.InvokeGuestTask(CreateSession(), TestNode, Issue);
+
+            Assert.Same(issued, task);
+            mockClient.Verify(c => c.GetAsync(It.IsAny<string>()), Times.Never);
+        }
+    }
+}

tests/PSProxmoxVE.Core.Tests/Utilities/CorosyncLinksTests.cs

@@ -0,0 +1,70 @@
+using PSProxmoxVE.Core.Utilities;
+using Xunit;
+
+namespace PSProxmoxVE.Core.Tests.Utilities
+{
+    public class CorosyncLinksTests
+    {
+        [Fact]
+        public void Parse_NullInput_ReturnsNullDictionaryAndNoMalformedEntries()
+        {
+            var (links, malformed) = CorosyncLinks.Parse(null);
+
+            Assert.Null(links);
+            Assert.Empty(malformed);
+        }
+
+        [Fact]
+        public void Parse_WellFormedEntries_ReturnsTrimmedKeysAndValues()
+        {
+            var (links, malformed) = CorosyncLinks.Parse(new[] { "link0= 10.0.0.1 ", "link1=10.0.0.2" });
+
+            Assert.NotNull(links);
+            Assert.Equal("10.0.0.1", links!["link0"]);
+            Assert.Equal("10.0.0.2", links["link1"]);
+            Assert.Empty(malformed);
+        }
+
+        [Theory]
+        [InlineData("link0")]
+        [InlineData("link0=")]
+        [InlineData("=10.0.0.1")]
+        [InlineData("")]
+        public void Parse_MalformedEntry_IsReportedAndOmitted(string entry)
+        {
+            var (links, malformed) = CorosyncLinks.Parse(new[] { entry });
+
+            Assert.Null(links);
+            Assert.Equal(new[] { entry }, malformed);
+        }
+
+        [Fact]
+        public void Parse_NullEntry_IsReportedAsMalformedRatherThanThrowing()
+        {
+            var (links, malformed) = CorosyncLinks.Parse(new[] { "link0=10.0.0.1", null! });
+
+            Assert.NotNull(links);
+            Assert.Single(links!);
+            Assert.Equal(new[] { "" }, malformed);
+        }
+
+        [Fact]
+        public void Parse_ValueContainingEquals_SplitsOnlyOnTheFirstOne()
+        {
+            var (links, malformed) = CorosyncLinks.Parse(new[] { "link0=10.0.0.1=extra" });
+
+            Assert.NotNull(links);
+            Assert.Equal("10.0.0.1=extra", links!["link0"]);
+            Assert.Empty(malformed);
+        }
+
+        [Fact]
+        public void Parse_AllEntriesMalformed_ReturnsNullDictionaryWithEveryEntryReported()
+        {
+            var (links, malformed) = CorosyncLinks.Parse(new[] { "garbage", "link0=" });
+
+            Assert.Null(links);
+            Assert.Equal(new[] { "garbage", "link0=" }, malformed);
+        }
+    }
+}