Store upgrade keeps pg-runtime-prev in place instead of deleting and recreating it (prep for #4052)

Darling/Darling.Tests/DarlingStoreUpgradeRevertTests.cs

@@ -687,7 +687,13 @@ public void RevertRuntime_Reverts_RecordsTheBlock_AndSaysSo()
             Assert.Equal(OldRuntime, host.RuntimeInPlace());
             Assert.Equal(PackageHash, File.ReadAllText(host.BlockedMarker));
             Assert.False(File.Exists(host.StampFile));
-            Assert.False(Directory.Exists(DarlingStoreUpgrade.PreviousRuntimeRootFor(host.RuntimeRoot)));
+
+            /* #4052: the cleanup now EMPTIES pg-runtime-prev rather than deleting and recreating it, so the
+               folder itself survives (it is not the service's to remove under the narrowed install-root
+               grant), but nothing is left inside it. */
+            var previousRoot = DarlingStoreUpgrade.PreviousRuntimeRootFor(host.RuntimeRoot);
+            Assert.True(Directory.Exists(previousRoot));
+            Assert.Empty(Directory.EnumerateFileSystemEntries(previousRoot));
         }
         finally
         {

Darling/Darling.Tests/DarlingStoreUpgradeTests.cs

@@ -904,7 +904,42 @@ public async Task RuntimeAdvance_APreviousRuntimeItCanClear_IsReplacedAndTheSwap
     }
 
     /// <summary>
-    /// #3908's rollback guard, on the swap path. After the swap, this release's own stamp names the package it
+    /// #4052: the narrowed install-root grant leaves the service Modify on <c>pg-runtime-prev</c> itself but
+    /// only Read &amp; Execute on the root above it, so a delete-then-recreate of the folder can delete and then
+    /// fail to recreate. The rescue must EMPTY the folder in place rather than delete and recreate it — this
+    /// proves the folder surviving the rescue (same folder, not a new one) by comparing its creation time
+    /// before and after.
+    /// </summary>
+    [Fact]
+    public async Task RuntimeAdvance_LeavesPgRuntimePrevInPlace_RatherThanDeletingAndRecreatingIt()
+    {
+        var root = Directory.CreateTempSubdirectory("darling-prev-inplace-");
+        try
+        {
+            var host = PlantHostAwaitingARuntimeSwap(root.FullName);
+
+            var previousRoot = DarlingStoreUpgrade.PreviousRuntimeRootFor(host.RuntimeRoot);
+            var staleFile = Path.Combine(previousRoot, "pgsql", "bin", "postgres.exe");
+            Directory.CreateDirectory(Path.GetDirectoryName(staleFile)!);
+            File.WriteAllText(staleFile, "previous runtime");
+            var creationTimeBefore = Directory.GetCreationTimeUtc(previousRoot);
+
+            var advance = await new DarlingStoreUpgrade(new CapturingLogger()).TryAdvanceRuntimeAsync(
+                host.RuntimeRoot, host.Package, host.DataDirectory,
+                (_, _) => Task.FromResult(false),
+                TestContext.Current.CancellationToken);
+
+            Assert.True(advance.Swapped);
+            Assert.True(Directory.Exists(previousRoot));
+            Assert.Equal(creationTimeBefore, Directory.GetCreationTimeUtc(previousRoot));
+        }
+        finally
+        {
+            TryDeleteTree(root.FullName);
+        }
+    }
+
+    /// <summary>#3908's rollback guard, on the swap path. After the swap, this release's own stamp names the package it
     /// installed, and the legacy file names the package every 3.3 to 3.8 release shipped. Each of those releases
     /// returns early when that file equals its own package, so a rollback to one keeps this runtime instead of
     /// swapping in one that cannot load the store. The legacy value is never this release's own hash.

Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs

@@ -997,12 +997,7 @@ just as surely as an unstamped one did on DARLING01. */
 
         try
         {
-            if (Directory.Exists(previousRoot))
-            {
-                Directory.Delete(previousRoot, recursive: true);
-            }
-
-            Directory.CreateDirectory(previousRoot);
+            EmptyDirectory(previousRoot);
         }
         catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
         {
@@ -1033,7 +1028,7 @@ must never spend that. */
             _logger.LogWarning(
                 "Could not rescue the current runtime to {Previous} ({Message}) — something is holding it open. The store starts on its existing runtime and the update is retried on the next start.",
                 previousPgsql, ex.Message);
-            TryDeleteDirectory(previousRoot);
+            TryEmptyDirectory(previousRoot);
             return new RuntimeAdvance(false, null, zipHash);
         }
 
@@ -1064,7 +1059,7 @@ would leave an unbootable runtime behind. */
 
             Directory.Move(previousPgsql, pgsqlDirectory);
             TryDeleteDirectory(failedExtract);
-            TryDeleteDirectory(previousRoot);
+            TryEmptyDirectory(previousRoot);
             throw;
         }
 
@@ -1188,7 +1183,7 @@ either works completely or fails without touching anything. */
         }
 
         TryDeleteDirectory(failedRuntime);
-        TryDeleteDirectory(previousRoot);
+        TryEmptyDirectory(previousRoot);
 
         /* Record the failing package so the next start does not run the same doomed upgrade again, and
            drop the stamp so the runtime on disk is not claimed to be the shipped one. #3927: this is
@@ -2284,6 +2279,65 @@ internal static void TryDeleteDirectory(string path)
         }
     }
 
+    /// <summary>
+    /// Empties <paramref name="path"/> — deletes every child file and subfolder, recursively — without
+    /// deleting the folder itself, and creates it only if it is missing. #4052: the install root is narrowed
+    /// to Read &amp; Execute for the service account, which keeps Modify on <c>pg-runtime\</c> and
+    /// <c>pg-runtime-prev\</c> themselves but not on the root above them — so a delete-then-recreate of
+    /// <c>pg-runtime-prev</c> can delete (Modify on the folder covers that) and then fail to recreate (no
+    /// rights on the root to add a new child there). Emptying in place needs no right on the root at all.
+    /// </summary>
+    /// <exception cref="UnauthorizedAccessException">
+    /// The folder does not exist and cannot be created — the installer or upgrade script must be re-run to
+    /// pre-create it under the now-narrowed root.
+    /// </exception>
+    internal static void EmptyDirectory(string path)
+    {
+        if (Directory.Exists(path))
+        {
+            foreach (var entry in Directory.EnumerateFileSystemEntries(path))
+            {
+                if (Directory.Exists(entry))
+                {
+                    Directory.Delete(entry, recursive: true);
+                }
+                else
+                {
+                    File.Delete(entry);
+                }
+            }
+
+            return;
+        }
+
+        try
+        {
+            Directory.CreateDirectory(path);
+        }
+        catch (UnauthorizedAccessException ex)
+        {
+            throw new UnauthorizedAccessException(
+                $"{path} does not exist and this service account cannot create it under the narrowed install root. Re-run the installer or upgrade script to pre-create it.",
+                ex);
+        }
+    }
+
+    /// <summary>
+    /// Best-effort <see cref="EmptyDirectory"/> — never throws, matching <see cref="TryDeleteDirectory"/>'s
+    /// contract for the cleanup call sites that only log a warning on failure.
+    /// </summary>
+    internal static void TryEmptyDirectory(string path)
+    {
+        try
+        {
+            EmptyDirectory(path);
+        }
+        catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
+        {
+            /* Best effort — the caller logs the consequence that matters. */
+        }
+    }
+
     /* ============================ retained rollback copies ============================ */
 
     internal static string RetainedDataDirectoryFor(string dataDirectory, int oldMajor)