fix(update): stop crashing on Windows after a successful update

src/Netclaw.Cli.Tests/Cli/UpdateCommandTests.cs

@@ -20,6 +20,15 @@ namespace Netclaw.Cli.Tests.Cli;
 [Collection("Update verification")]
 public sealed class UpdateCommandTests : IDisposable
 {
+    /// <summary>
+    /// xunit.v3 <c>SkipUnless</c> hook for tests that simulate Windows
+    /// file-lock failures by revoking directory write permission. The
+    /// simulation is POSIX-only (<c>File.SetUnixFileMode</c>) and is
+    /// ineffective for root, which bypasses directory permission bits.
+    /// </summary>
+    public static bool CanSimulateFileLock =>
+        !OperatingSystem.IsWindows() && Environment.UserName != "root";
+
     private readonly DisposableTempDir _dir = new();
     private readonly NetclawPaths _paths;
     private readonly Key _testSigningKey;
@@ -303,6 +312,150 @@ public async Task RunAsync_RejectsUnknownChannel()
         return doc.RootElement.GetProperty("Daemon").GetProperty("UpdateChannel").GetString();
     }
 
+    [Fact]
+    public void CleanupBackupFile_DoesNotDelete_RunningImageBackup_OnWindows()
+    {
+        var backupPath = Path.Combine(_dir.Path, "netclaw.exe.backup");
+        File.WriteAllText(backupPath, "old image");
+
+        // The running process's backup is the very image this process executes
+        // from; on Windows DeleteFile fails with UnauthorizedAccessException.
+        // NTFS path comparison is case-insensitive, so pin that here — a
+        // regression to Ordinal would leave the backup deleted.
+        var runningBackupPath = Path.Combine(_dir.Path, "NETCLAW.EXE.BACKUP");
+        UpdateCommand.CleanupBackupFile(backupPath, runningBackupPath, isWindows: true);
+
+        Assert.True(File.Exists(backupPath));
+    }
+
+    [Fact]
+    public void SwapBinaryIntoPlace_ReplacesTarget_AndBacksUpOldBinary()
+    {
+        var sourcePath = Path.Combine(_dir.Path, "new.exe");
+        var targetPath = Path.Combine(_dir.Path, "netclaw.exe");
+        var backupPath = targetPath + ".backup";
+        File.WriteAllText(sourcePath, "new image");
+        File.WriteAllText(targetPath, "old image");
+
+        UpdateCommand.SwapBinaryIntoPlace(sourcePath, targetPath, backupPath);
+
+        Assert.Equal("new image", File.ReadAllText(targetPath));
+        Assert.Equal("old image", File.ReadAllText(backupPath));
+    }
+
+    [Fact]
+    public void SwapBinaryIntoPlace_RestoresOldBinary_WhenNewBinaryMoveFails()
+    {
+        var sourcePath = Path.Combine(_dir.Path, "new.exe");
+        var targetPath = Path.Combine(_dir.Path, "netclaw.exe");
+        var backupPath = targetPath + ".backup";
+        File.WriteAllText(sourcePath, "new image");
+        File.WriteAllText(targetPath, "old image");
+
+        // Make the final move fail after the old binary was backed up: the
+        // install directory must never be left without an executable.
+        File.Delete(sourcePath);
+
+        Assert.ThrowsAny<Exception>(() => UpdateCommand.SwapBinaryIntoPlace(sourcePath, targetPath, backupPath));
+        // The old binary is rolled back into place; the backup is consumed by
+        // the restore, so the install directory is left with a working binary.
+        Assert.Equal("old image", File.ReadAllText(targetPath));
+        Assert.False(File.Exists(backupPath));
+    }
+
+    [Fact(SkipUnless = nameof(CanSimulateFileLock), Skip = "POSIX-only permission simulation (ineffective on Windows or as root)")]
+    [SlopwatchSuppress("SW001", "Simulates Windows file locks via POSIX directory permissions, which cannot run on Windows or as root.")]
+    public void SwapBinaryIntoPlace_LeavesTargetIntact_WhenStaleBackupDeleteFails()
+    {
+        if (OperatingSystem.IsWindows())
+            return; // SkipUnless gates the skip; this guard satisfies CA1416 for Unix-only APIs
+
+        var sourcePath = Path.Combine(_dir.Path, "new.exe");
+        var targetPath = Path.Combine(_dir.Path, "netclaw.exe");
+        var backupPath = targetPath + ".backup";
+        File.WriteAllText(sourcePath, "new image");
+        File.WriteAllText(targetPath, "old image");
+        File.WriteAllText(backupPath, "stale image");
+        var dir = Path.GetDirectoryName(targetPath)!;
+        var originalMode = File.GetUnixFileMode(dir);
+
+        try
+        {
+            // Remove write permission on the directory so the stale-backup
+            // delete fails with UnauthorizedAccessException — the same failure
+            // class as an AV-locked file on Windows. The target must be left
+            // untouched (no half-swap).
+            File.SetUnixFileMode(dir, UnixFileMode.UserRead | UnixFileMode.UserExecute
+                | UnixFileMode.GroupRead | UnixFileMode.GroupExecute
+                | UnixFileMode.OtherRead | UnixFileMode.OtherExecute);
+
+            Assert.ThrowsAny<Exception>(() => UpdateCommand.SwapBinaryIntoPlace(sourcePath, targetPath, backupPath));
+            Assert.Equal("old image", File.ReadAllText(targetPath));
+            Assert.Equal("stale image", File.ReadAllText(backupPath));
+        }
+        finally
+        {
+            File.SetUnixFileMode(dir, originalMode);
+        }
+    }
+
+    [Fact]
+    public void CleanupBackupFile_Deletes_OtherComponentBackup_OnWindows()
+    {
+        var backupPath = Path.Combine(_dir.Path, "netclawd.exe.backup");
+        File.WriteAllText(backupPath, "old image");
+        var runningBackupPath = Path.Combine(_dir.Path, "netclaw.exe") + ".backup";
+
+        UpdateCommand.CleanupBackupFile(backupPath, runningBackupPath, isWindows: true);
+
+        Assert.False(File.Exists(backupPath));
+    }
+
+    [Fact]
+    public void CleanupBackupFile_Deletes_Backup_OnNonWindows()
+    {
+        var backupPath = Path.Combine(_dir.Path, "netclaw.backup");
+        File.WriteAllText(backupPath, "old image");
+
+        // POSIX allows unlinking a running image, so even the running
+        // process's own backup is removed.
+        UpdateCommand.CleanupBackupFile(backupPath, runningBackupPath: backupPath, isWindows: false);
+
+        Assert.False(File.Exists(backupPath));
+    }
+
+    [Fact(SkipUnless = nameof(CanSimulateFileLock), Skip = "POSIX-only permission simulation (ineffective on Windows or as root)")]
+    [SlopwatchSuppress("SW001", "Simulates Windows file locks via POSIX directory permissions, which cannot run on Windows or as root.")]
+    public void CleanupBackupFile_DoesNotThrow_WhenDeleteFails()
+    {
+        if (OperatingSystem.IsWindows())
+            return; // SkipUnless gates the skip; this guard satisfies CA1416 for Unix-only APIs
+
+        var backupPath = Path.Combine(_dir.Path, "netclaw.backup");
+        File.WriteAllText(backupPath, "old image");
+        var dir = Path.GetDirectoryName(backupPath)!;
+        var originalMode = File.GetUnixFileMode(dir);
+
+        try
+        {
+            // Remove write permission on the directory so unlink fails with
+            // UnauthorizedAccessException — the same failure class as deleting
+            // a running image on Windows.
+            File.SetUnixFileMode(dir, UnixFileMode.UserRead | UnixFileMode.UserExecute
+                | UnixFileMode.GroupRead | UnixFileMode.GroupExecute
+                | UnixFileMode.OtherRead | UnixFileMode.OtherExecute);
+
+            UpdateCommand.CleanupBackupFile(backupPath, runningBackupPath: null, isWindows: false);
+
+            // Warned, not crashed; the leftover self-heals on the next update.
+            Assert.True(File.Exists(backupPath));
+        }
+        finally
+        {
+            File.SetUnixFileMode(dir, originalMode);
+        }
+    }
+
     [Theory]
     [MemberData(nameof(StartupUpdateSkippedCases))]
     public void ShouldRunStartupUpdateCheck_ReturnsFalse_ForInteractiveOrSelfUpdateFlows(string[] args)
@@ -471,5 +624,15 @@ public Task<SystemCommandResult> RunAsync(string command, string arguments)
                 : _results.Dequeue());
         }
     }
+}
+
+/// <summary>
+/// Supplies source-level Slopwatch suppressions without a runtime package dependency.
+/// </summary>
+[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
+internal sealed class SlopwatchSuppressAttribute(string ruleId, string reason) : Attribute
+{
+    public string RuleId { get; } = ruleId;
 
+    public string Reason { get; } = reason;
 }

src/Netclaw.Cli/Update/UpdateCommand.cs

@@ -267,15 +267,27 @@ private static async Task<int> PerformUpdateAsync(
                 var targetPath = Path.Combine(installDir, binaryName);
                 var backupPath = targetPath + ".backup";
 
-                // Backup existing binary
-                if (File.Exists(targetPath))
+                try
                 {
-                    if (File.Exists(backupPath))
-                        File.Delete(backupPath);
-                    File.Move(targetPath, backupPath);
+                    // Swap with automatic rollback: a failed swap restores the
+                    // previous binary so the install directory is never left
+                    // without an executable (which would brick the CLI).
+                    SwapBinaryIntoPlace(sourcePath, targetPath, backupPath);
+                }
+                catch (Exception ex)
+                {
+                    var targetRestored = File.Exists(targetPath);
+                    Console.WriteLine($"\n  Failed to replace {binaryName}: {ex.Message}");
+                    if (targetRestored)
+                    {
+                        Console.WriteLine("  The previous binary was restored. The daemon is stopped; start it with 'netclaw daemon start'.");
+                    }
+                    else
+                    {
+                        Console.WriteLine($"  The install directory is missing {binaryName}. Restore it from {binaryName}.backup, then start the daemon with 'netclaw daemon start'.");
+                    }
+                    return 1;
                 }
-
-                File.Move(sourcePath, targetPath);
 
                 // Set executable permission on Unix
                 if (!OperatingSystem.IsWindows())
@@ -299,15 +311,23 @@ private static async Task<int> PerformUpdateAsync(
                 Console.WriteLine(" done.");
             }
 
-            // Clean up backup files
+            // Clean up backup files. The backup of the currently running CLI
+            // binary is the image this process still executes from; on Windows
+            // DeleteFile on a running image fails with
+            // UnauthorizedAccessException. Leave it — the install step deletes
+            // stale backups before moving the new binary on the next update.
+            // A leftover backup must never turn a successful update into a
+            // fatal error, so any other delete failure only warns.
+            var runningBackupPath = Environment.ProcessPath is { } processPath
+                ? processPath + ".backup"
+                : null;
             foreach (var (component, _) in extractedPaths)
             {
                 var binaryName = OperatingSystem.IsWindows()
                     ? $"{component}.exe"
                     : component;
                 var backupPath = Path.Combine(installDir, binaryName + ".backup");
-                if (File.Exists(backupPath))
-                    File.Delete(backupPath);
+                CleanupBackupFile(backupPath, runningBackupPath, OperatingSystem.IsWindows());
             }
 
             Console.WriteLine($"\nUpdated to v{result.LatestVersion}.");
@@ -493,6 +513,85 @@ private static async Task ExtractTarGzAsync(string archivePath, string extractDi
         return null;
     }
 
+    /// <summary>
+    /// Replaces <paramref name="targetPath"/> with
+    /// <paramref name="sourcePath"/>, preserving the previous binary at
+    /// <paramref name="backupPath"/>. On failure the previous binary is
+    /// restored to <paramref name="targetPath"/> so a failed swap never
+    /// leaves the install directory without an executable.
+    /// </summary>
+    /// <param name="sourcePath">The new binary to install.</param>
+    /// <param name="targetPath">The installed binary to replace.</param>
+    /// <param name="backupPath">Where the previous binary is preserved.</param>
+    internal static void SwapBinaryIntoPlace(string sourcePath, string targetPath, string backupPath)
+    {
+        var movedOldToBackup = false;
+        try
+        {
+            if (File.Exists(targetPath))
+            {
+                if (File.Exists(backupPath))
+                    File.Delete(backupPath);
+                File.Move(targetPath, backupPath);
+                movedOldToBackup = true;
+            }
+
+            File.Move(sourcePath, targetPath);
+        }
+        catch
+        {
+            // Roll the previous binary back so a failed swap leaves a
+            // working binary in place instead of a missing executable.
+            if (movedOldToBackup && !File.Exists(targetPath) && File.Exists(backupPath))
+            {
+                try { File.Move(backupPath, targetPath); }
+                catch (Exception rollbackEx)
+                {
+                    // Best-effort rollback; the original swap failure is
+                    // rethrown below and reported to the user.
+                    Console.Error.WriteLine($"warn: failed to restore {targetPath} from {backupPath}: {rollbackEx.Message}");
+                }
+            }
+            throw;
+        }
+    }
+
+    /// <summary>
+    /// Deletes a leftover <c>.backup</c> file after a successful update.
+    /// On Windows the backup of the currently running image cannot be
+    /// deleted — the process still executes from that file, so DeleteFile
+    /// fails with <see cref="UnauthorizedAccessException"/>. Such backups are
+    /// removed by the install step on the next update, before it renames the
+    /// new binary. Any other delete failure only warns; a leftover backup must
+    /// never turn a successful update into a fatal error.
+    /// </summary>
+    /// <param name="backupPath">The backup file to delete.</param>
+    /// <param name="runningBackupPath">
+    /// The backup path of the currently running process
+    /// (<c>Environment.ProcessPath + ".backup"</c>), or <c>null</c> if unknown.
+    /// </param>
+    /// <param name="isWindows">True when running on Windows.</param>
+    internal static void CleanupBackupFile(string backupPath, string? runningBackupPath, bool isWindows)
+    {
+        if (isWindows
+            && runningBackupPath is not null
+            && string.Equals(backupPath, runningBackupPath, StringComparison.OrdinalIgnoreCase))
+        {
+            // Running image — cannot be deleted on Windows.
+            return;
+        }
+
+        try
+        {
+            if (File.Exists(backupPath))
+                File.Delete(backupPath);
+        }
+        catch (Exception ex)
+        {
+            Console.Error.WriteLine($"warn: could not remove backup {backupPath}: {ex.Message}");
+        }
+    }
+
     private static string GetInstallDirectory()
     {
         // Use the directory containing the current CLI binary