Follow up script-install updates with pruning and a Windows daemon restart

README.md

@@ -2536,7 +2536,10 @@ kcap feedback --feedback                               # send feedback; prompts
 > Releases and checks its sha256 against the manifest, installing nothing on a
 > mismatch. Each release gets its own `versions/<version>` directory and the
 > `current` link is switched to it, so running processes keep the files they
-> started from; then the new binary runs `kcap refresh`. A daemon service
+> started from; then the new binary runs `kcap refresh`. The version it replaced
+> stays; older ones are deleted unless something still runs from them. A daemon
+> started from a script install restarts on the new version once it is idle,
+> Windows included. A daemon service
 > installed from a script install is moved onto the `current` path, so it
 > follows updates. Switching from npm, a daemon service installed under npm keeps
 > running the npm binary: run `kcap daemon service install` again once the

src/Capacitor.Cli.Core/Resources/help-update.txt

@@ -14,8 +14,11 @@ npm, downloads this platform's archive from GitHub Releases and checks its
 sha256 against the manifest; any mismatch installs nothing. The release
 goes into its own versions/<version> directory and the `current` link is
 switched to it, so running processes keep the files they started from.
-The new binary then runs `kcap refresh`. A daemon service installed from a
-script install is moved onto the `current` path, so it follows updates.
+The new binary then runs `kcap refresh`. The version it replaced stays;
+older ones are deleted unless something still runs from them. A daemon
+started from a script install restarts on the new version once it is idle,
+Windows included. A daemon service installed from a script install is moved
+onto the `current` path, so it follows updates.
 A daemon service installed under npm keeps running the npm binary after a
 switch to the script install: run `kcap daemon service install` again.
 

src/Capacitor.Cli.Daemon/Services/RestartCoordinator.cs

@@ -22,8 +22,8 @@ internal enum RestartRequestResult {
 /// Polls the running binary; a size/mtime change queues a restart-after-update that
 /// fires the chosen <see cref="IRestartStrategy"/> the moment the daemon is idle
 /// (no hosted agents, no in-flight eval). Also handles explicit requests from the
-/// control socket. Self-detection is macOS/Linux only (the poll is a no-op on Windows,
-/// where a running binary can't be replaced); the explicit request path works anywhere.
+/// control socket. On Windows self-detection covers script installs only; the explicit
+/// request path works anywhere.
 /// </summary>
 internal sealed partial class RestartCoordinator : BackgroundService {
     readonly TimeProvider _time;
@@ -78,18 +78,20 @@ internal void PrimeBaseline() {
         lock (_gate) _baseline = StatBinary();
     }
 
-    static BinaryStat? StatProcessBinary() {
-        // Windows can't replace a running binary in place, so self-detection is a no-op
-        // there: returning null means BinaryChanged() never sees a change. Explicit
-        // restart requests still work (they set _pending directly). Keeping the OS check
-        // here — rather than in Tick() — leaves the gate logic OS-agnostic and testable.
-        if (OperatingSystem.IsWindows()) return null;
+    static BinaryStat? StatProcessBinary() => StatBinaryAt(Environment.ProcessPath, OperatingSystem.IsWindows(), File.Exists);
+
+    /// <summary>
+    /// What the poll compares: the binary reached through a script install's <c>current</c>, which an
+    /// update switches rather than overwriting this file. Outside a script install Windows never replaces
+    /// a running binary in place, so there it returns null and BinaryChanged() never fires; explicit
+    /// restart requests still work.
+    /// </summary>
+    internal static BinaryStat? StatBinaryAt(string? processPath, bool isWindows, Func<string, bool> fileExists) {
+        if (processPath is null) return null;
+        if (isWindows && ScriptInstallLayout.FromBinary(processPath, fileExists) is null) return null;
 
         try {
-            // A script install flips `current` rather than replacing this file, so watch the path through it.
-            var p = Environment.ProcessPath is { } exe ? ScriptInstallLayout.Stabilize(exe) : null;
-            if (p is null) return null;
-            var fi = new FileInfo(p);
+            var fi = new FileInfo(ScriptInstallLayout.Stabilize(processPath, fileExists));
             return fi.Exists ? new BinaryStat(fi.Length, fi.LastWriteTimeUtc.Ticks) : null;
         } catch {
             return null; // transient (file being swapped mid-install)

src/Capacitor.Cli/Commands/ScriptUpdater.cs

@@ -13,7 +13,7 @@ namespace Capacitor.Cli.Commands;
 /// Installs one release into a script install, the way the install script does: download the RID's
 /// archive, verify it against the release manifest's sha256, extract it into its own
 /// <c>versions/&lt;v&gt;</c> and switch <c>current</c> to it. A running kcap keeps its own files, because
-/// no version directory that is in use is ever written.
+/// no version directory that is in use is ever written or pruned.
 ///
 /// <para>Verification fails closed: a manifest that cannot be read, has no valid checksum for this RID,
 /// or does not match the downloaded bytes installs nothing.</para>
@@ -23,6 +23,9 @@ public sealed partial class ScriptUpdater(KcapReleaseClient releases) {
     /// a test cannot make one off Windows.</summary>
     internal Action<ScriptInstallLayout, string> SwitchCurrent { get; init; } = DefaultSwitch;
 
+    /// <summary>Whether something runs from a version directory, so pruning must keep it.</summary>
+    internal Func<string, bool> InUse { get; init; } = AnyFileInUse;
+
     internal async Task<bool> InstallAsync(
             ScriptInstallLayout layout, string version, string channel, string rid, TextWriter stdout, TextWriter stderr,
             CancellationToken ct) {
@@ -53,8 +56,9 @@ internal async Task<bool> InstallAsync(
             return false;
         }
 
-        var dest = layout.VersionDir(version);
-        if (ActiveVersionDir(layout) is { } active && PathsEqual(active, dest)) {
+        var dest     = layout.VersionDir(version);
+        var previous = ActiveVersionDir(layout);
+        if (previous is not null && PathsEqual(previous, dest)) {
             await stdout.WriteLineAsync($"kcap {version} is already the active version.");
             TryWriteMarker(layout, channel, stderr);
             return true;
@@ -117,6 +121,9 @@ internal async Task<bool> InstallAsync(
             TryWriteMarker(layout, channel, stderr);
             await stdout.WriteLineAsync($"Installed kcap {version} to {layout.Root}");
 
+            var pruned = PruneVersions(layout, keep: [dest, previous]);
+            if (pruned > 0) await stdout.WriteLineAsync($"Removed {pruned} unused kcap version{(pruned == 1 ? "" : "s")}.");
+
             return true;
         } finally {
             try { Directory.Delete(work, recursive: true); } catch { /* best-effort */ }
@@ -155,6 +162,57 @@ internal static void TryWriteMarker(ScriptInstallLayout layout, string channel,
         }
     }
 
+    /// <summary>
+    /// Deletes the version directories and moved-aside copies an update leaves behind, keeping
+    /// <paramref name="keep"/>: the new version and the one it replaced, which a daemon or agent started
+    /// before the switch is likely still running. Anything else in use stays too. Returns how many went.
+    /// </summary>
+    internal int PruneVersions(ScriptInstallLayout layout, IEnumerable<string?> keep) {
+        var kept = keep.OfType<string>().ToArray();
+        var pruned = 0;
+
+        IEnumerable<DirectoryInfo> candidates;
+        try {
+            candidates = new DirectoryInfo(layout.Versions).EnumerateDirectories()
+                .Where(d => d.LinkTarget is null)
+                .Where(d => ReleaseManifest.IsValidVersion(d.Name) || d.Name.StartsWith(".replaced-", StringComparison.Ordinal))
+                .Where(d => !kept.Any(k => PathsEqual(k, d.FullName)))
+                .ToArray();
+        } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) {
+            return 0;
+        }
+
+        foreach (var dir in candidates) {
+            if (InUse(dir.FullName)) continue;
+
+            try {
+                dir.Delete(recursive: true);
+                pruned++;
+            } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) {
+                // Started after the probe, or not ours to delete: the next update tries again.
+            }
+        }
+
+        return pruned;
+    }
+
+    /// <summary>
+    /// True when a file in <paramref name="dir"/> cannot be opened exclusively for writing. A running
+    /// executable refuses that on Windows (sharing violation) and Linux (ETXTBSY); macOS does not, but
+    /// deleting a running binary there leaves the process its file.
+    /// </summary>
+    internal static bool AnyFileInUse(string dir) {
+        try {
+            foreach (var file in Directory.EnumerateFiles(dir, "*", SearchOption.AllDirectories)) {
+                using var _ = new FileStream(file, FileMode.Open, FileAccess.ReadWrite, FileShare.None);
+            }
+
+            return false;
+        } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) {
+            return true;
+        }
+    }
+
     /// <summary>The version directory <c>current</c> points at, or null when it is not a link.</summary>
     internal static string? ActiveVersionDir(ScriptInstallLayout layout) =>
         new DirectoryInfo(layout.Current).LinkTarget is { } target

test/Capacitor.Cli.Daemon.Tests.Unit/Services/RestartCoordinatorTests.cs

@@ -120,4 +120,31 @@ public async Task Request_restart_when_busy_queues_then_fires_when_idle() {
         c.Tick();
         await Assert.That(spy.Calls).IsEqualTo(1);
     }
+
+    /// <summary>On Windows only a script install is watched: an update switches its <c>current</c>, so the
+    /// binary reached through it changes while the file the daemon runs from does not.</summary>
+    [Test]
+    public async Task On_Windows_a_script_install_is_watched_through_current() {
+        using var tmp = new TempDir();
+        tmp.CreateFile("kcap/install.json", "{}");
+        var running    = tmp.CreateFile("kcap/versions/1.0.0/bin/kcap-daemon", "old");
+        var viaCurrent = tmp.CreateFile("kcap/current/bin/kcap-daemon", "old");
+
+        var before = RestartCoordinator.StatBinaryAt(running, isWindows: true, File.Exists);
+        File.WriteAllText(viaCurrent, "the new version");
+        var after = RestartCoordinator.StatBinaryAt(running, isWindows: true, File.Exists);
+
+        await Assert.That(before).IsNotNull();
+        await Assert.That(RestartDecision.BinaryChanged(before, after)).IsTrue();
+        await Assert.That(File.ReadAllText(running)).IsEqualTo("old");
+    }
+
+    [Test]
+    public async Task On_Windows_a_binary_outside_a_script_install_is_not_watched() {
+        using var tmp = new TempDir();
+        var exe = tmp.CreateFile("npm/bin/kcap-daemon", "x");
+
+        await Assert.That(RestartCoordinator.StatBinaryAt(exe, isWindows: true, File.Exists)).IsNull();
+        await Assert.That(RestartCoordinator.StatBinaryAt(exe, isWindows: false, File.Exists)).IsNotNull();
+    }
 }

test/Capacitor.Cli.Tests.Unit/Commands/ScriptUpdaterTests.cs

@@ -1,4 +1,5 @@
 using Capacitor.Cli.Commands;
+using Capacitor.Cli.Core.Install;
 
 namespace Capacitor.Cli.Tests.Unit.Commands;
 
@@ -11,9 +12,10 @@ public class ScriptUpdaterTests {
     static bool Unsupported => OperatingSystem.IsWindows();
 
     static async Task<(bool Ok, string Out, string Err)> Install(
-            ScriptReleaseFixture f, string version, string channel = "latest", TimeSpan? downloadTimeout = null) {
+            ScriptReleaseFixture f, string version, string channel = "latest", TimeSpan? downloadTimeout = null,
+            Func<string, bool>? inUse = null) {
         var (stdout, stderr) = (new StringWriter(), new StringWriter());
-        var ok = await new ScriptUpdater(f.Client(downloadTimeout))
+        var ok = await new ScriptUpdater(f.Client(downloadTimeout)) { InUse = inUse ?? ScriptUpdater.AnyFileInUse }
             .InstallAsync(f.Layout, version, channel, ScriptReleaseFixture.Rid, stdout, stderr, CancellationToken.None);
         return (ok, stdout.ToString(), stderr.ToString());
     }
@@ -144,13 +146,13 @@ public async Task The_active_version_is_left_in_place_and_nothing_is_downloaded(
     /// <summary>A process started from a version before <c>current</c> moved away may still run there, so
     /// installing that version again moves the old directory aside instead of deleting it.</summary>
     [Test]
-    public async Task A_leftover_version_directory_is_moved_aside_not_deleted() {
+    public async Task A_leftover_version_directory_in_use_is_moved_aside_not_deleted() {
         Skip.When(Unsupported, "the fixture's current is a symlink");
         using var f = new ScriptReleaseFixture();
         f.LeaveVersion("1.5.0", "leftover");
         f.Publish("1.5.0");
 
-        var (ok, _, _) = await Install(f, "1.5.0");
+        var (ok, _, _) = await Install(f, "1.5.0", inUse: dir => Path.GetFileName(dir).StartsWith(".replaced-", StringComparison.Ordinal));
 
         await Assert.That(ok).IsTrue();
         await Assert.That(File.ReadAllText(f.Layout.CurrentBin("kcap"))).IsEqualTo("kcap 1.5.0");
@@ -201,4 +203,50 @@ public async Task Switching_replaces_the_link_and_leaves_no_temporary_one() {
         await Assert.That(Directory.EnumerateFileSystemEntries(tmp.Path).Select(p => Path.GetFileName(p)!).ToArray())
             .IsEquivalentTo(["versions", "current"]);
     }
+
+    /// <summary>An update keeps the version it replaced, which a daemon started before the switch still runs,
+    /// and deletes the older ones.</summary>
+    [Test]
+    public async Task An_update_prunes_versions_older_than_the_one_it_replaced() {
+        Skip.When(Unsupported, "the fixture's current is a symlink");
+        using var f = new ScriptReleaseFixture();
+        f.LeaveVersion("0.9.0", "older");
+        f.Publish("1.5.0");
+
+        var (ok, output, _) = await Install(f, "1.5.0");
+
+        await Assert.That(ok).IsTrue();
+        await Assert.That(output).Contains("Removed 1 unused kcap version.");
+        await Assert.That(Directory.EnumerateDirectories(f.Layout.Versions).Select(p => Path.GetFileName(p)!).ToArray())
+            .IsEquivalentTo(["1.0.0", "1.5.0"]);
+    }
+
+    [Test]
+    public async Task Pruning_keeps_what_it_is_told_to_and_what_is_in_use() {
+        using var tmp = new TempDir();
+        var layout = new ScriptInstallLayout(tmp.Path);
+        foreach (var name in new[] { "1.0.0", "1.1.0", "1.2.0", "1.3.0-beta.1", ".replaced-1.2.0-abc", ".update-12-def", ".staging-34", "notes" })
+            tmp.CreateFile($"versions/{name}/bin/kcap");
+        var updater = new ScriptUpdater(null!) { InUse = dir => Path.GetFileName(dir) == "1.1.0" };
+
+        var pruned = updater.PruneVersions(layout, keep: [layout.VersionDir("1.3.0-beta.1"), layout.VersionDir("1.2.0"), null]);
+
+        await Assert.That(pruned).IsEqualTo(2);
+        await Assert.That(Directory.EnumerateDirectories(layout.Versions).Select(p => Path.GetFileName(p)!).ToArray())
+            .IsEquivalentTo(["1.1.0", "1.2.0", "1.3.0-beta.1", ".update-12-def", ".staging-34", "notes"]);
+    }
+
+    /// <summary>A file someone holds open marks its directory in use; a running binary refuses the same
+    /// exclusive write open on Windows and Linux.</summary>
+    [Test]
+    public async Task A_directory_with_a_file_held_open_is_in_use() {
+        using var tmp = new TempDir();
+        var dir = tmp.CreateDir("1.0.0");
+        var exe = tmp.CreateFile("1.0.0/bin/kcap");
+
+        await Assert.That(ScriptUpdater.AnyFileInUse(dir)).IsFalse();
+        using (new FileStream(exe, FileMode.Open, FileAccess.Read, FileShare.Read))
+            await Assert.That(ScriptUpdater.AnyFileInUse(dir)).IsTrue();
+        await Assert.That(ScriptUpdater.AnyFileInUse(dir)).IsFalse();
+    }
 }