diff --git a/Tools/windows/PACKAGING.md b/Tools/windows/PACKAGING.md index f29a85b1..2058207e 100644 --- a/Tools/windows/PACKAGING.md +++ b/Tools/windows/PACKAGING.md @@ -195,6 +195,32 @@ native Windows file-sharing lock. It isolates daemon and shortcut operations; the real-product packaging gate separately exercises scheduled-daemon install/upgrade/rollback/uninstall. +## Successful update backup cleanup + +After an upgrade commits, setup creates a sibling +`.GraphCode-rollback-.cleanup-ready` sidecar before attempting to delete the +previous installation. The marker is created only for a committed transaction; +the unmarked `.GraphCode-rollback-` directories described above remain +recovery-owned and must not be removed automatically. + +An in-app update can leave the committed backup locked because the old +`graphcode-windows.exe` is still running from it. Normal installed-shell startup +scans only exact cleanup-ready sidecars beside `current`, takes an exclusive +lease on each marker, and confirms every remaining backup file can be opened +exclusively before deleting the directory. Multiple shell processes therefore +cannot own the same cleanup attempt. A lock, access error, race, or partial +deletion leaves the marker in place for a bounded background retry and a later +launch; cleanup never blocks startup or touches the active `current` +installation. + +If setup cannot create the success marker, the upgrade remains successful and +warns. A backup that also cannot be deleted is deliberately left unmarked: this +may retain expendable files, but it cannot authorize deletion of failed-recovery +state. `Packaging.Rollback.Tests.ps1` covers the commit-only marker and its +secondary-failure behavior. `WindowsRollbackCleanup.zig` tests marked cleanup, +unmarked recovery preservation, locked-file retry, malformed markers, orphan +markers, and concurrent marker leases. + ## Uninstall with GraphCode still running Uninstall removes the installation completely or changes nothing. Before diff --git a/Tools/windows/PackageRuntime.ps1 b/Tools/windows/PackageRuntime.ps1 index 7763988a..9ee412ba 100644 --- a/Tools/windows/PackageRuntime.ps1 +++ b/Tools/windows/PackageRuntime.ps1 @@ -361,6 +361,22 @@ function Move-InstallDirectory([string] $source, [string] $destination) { $destination = $ExecutionContext.SessionState.Path.GetUnresolvedProviderPathFromPSPath($destination) [IO.Directory]::Move($source, $destination) } +function Get-RollbackCleanupMarker([string] $backup) { + $backup = $ExecutionContext.SessionState.Path.GetUnresolvedProviderPathFromPSPath($backup) + return "$backup.cleanup-ready" +} +function New-RollbackCleanupMarker([string] $backup) { + $marker = Get-RollbackCleanupMarker $backup + $stream = [IO.File]::Open($marker, [IO.FileMode]::CreateNew, [IO.FileAccess]::Write, [IO.FileShare]::None) + try { + $bytes = [Text.Encoding]::UTF8.GetBytes("committed`n") + $stream.Write($bytes, 0, $bytes.Length) + $stream.Flush($true) + } finally { + $stream.Dispose() + } + return $marker +} function Install-Package([bool] $upgrade) { try { $root = Open-Package ($(if ($Package) { $Package } else { Fail "-Package is required" })) @@ -455,7 +471,6 @@ function Install-Package([bool] $upgrade) { throw $failure } finally { $cleanup = @($stage) - if ($committed) { $cleanup += $backup } if (-not $keepShortcutBackup) { $cleanup += $shortcutBackup } foreach ($path in $cleanup) { if (Test-Path -LiteralPath $path) { @@ -464,6 +479,24 @@ function Install-Package([bool] $upgrade) { } } } + if ($committed -and $previousInstallMoved -and (Test-Path -LiteralPath $backup)) { + $cleanupMarker = Get-RollbackCleanupMarker $backup + try { + New-RollbackCleanupMarker $backup | Out-Null + } catch { + Write-Warning "GraphCode packaging: could not authorize later cleanup of committed backup '$backup': $($_.Exception.Message)" + } + try { + Remove-Item -LiteralPath $backup -Recurse -Force + } catch { + Write-Warning "GraphCode packaging: could not clean transaction directory '$backup': $($_.Exception.Message)" + } + if (-not (Test-Path -LiteralPath $backup) -and (Test-Path -LiteralPath $cleanupMarker)) { + try { Remove-Item -LiteralPath $cleanupMarker -Force } catch { + Write-Warning "GraphCode packaging: removed committed backup but could not clean transaction marker '$cleanupMarker': $($_.Exception.Message)" + } + } + } } Write-Output "Installed GraphCode $($metadata.version) at $InstallRoot" } finally { diff --git a/Tools/windows/Tests/Packaging.Rollback.Tests.ps1 b/Tools/windows/Tests/Packaging.Rollback.Tests.ps1 index 3f04c424..d40925bb 100644 --- a/Tools/windows/Tests/Packaging.Rollback.Tests.ps1 +++ b/Tools/windows/Tests/Packaging.Rollback.Tests.ps1 @@ -9,7 +9,8 @@ $errors = $null $ast = [Management.Automation.Language.Parser]::ParseFile( (Join-Path $repoRoot "Tools\windows\PackageRuntime.ps1"), [ref]$tokens, [ref]$errors) if ($errors.Count) { throw "Packaging script has parse errors: $errors" } -foreach ($name in @("Fail", "Require", "Copy-Tree", "Move-InstallDirectory", "Install-Package", "Open-Package", "Close-Package")) { +foreach ($name in @("Fail", "Require", "Copy-Tree", "Move-InstallDirectory", "Get-RollbackCleanupMarker", + "New-RollbackCleanupMarker", "Install-Package", "Open-Package", "Close-Package")) { $definition = $ast.Find({ param($node) $node -is [Management.Automation.Language.FunctionDefinitionAst] -and $node.Name -eq $name @@ -21,6 +22,7 @@ $copyTree = (Get-Command Copy-Tree).ScriptBlock $moveDirectory = (Get-Command Move-InstallDirectory).ScriptBlock $openPackage = (Get-Command Open-Package).ScriptBlock $closePackage = (Get-Command Close-Package).ScriptBlock +$newRollbackCleanupMarker = (Get-Command New-RollbackCleanupMarker).ScriptBlock function Assert([bool] $condition, [string] $message) { if (-not $condition) { throw "RED: $message" } @@ -86,6 +88,12 @@ function Move-InstallDirectory([string] $source, [string] $destination) { } & $moveDirectory $source $destination } +function New-RollbackCleanupMarker([string] $backup) { + if ($case -eq "committed-cleanup-secondary") { + throw "injected cleanup marker failure" + } + & $newRollbackCleanupMarker $backup +} function Remove-Item { [CmdletBinding()] param( @@ -98,17 +106,24 @@ function Remove-Item { if ($case -eq "remove-new" -and $targets -contains $InstallRoot) { throw "injected promoted install removal failure" } + if ($case -in @("committed-cleanup-blocked", "committed-cleanup-secondary") -and + @($targets | Where-Object { (Split-Path $_ -Leaf) -like ".GraphCode-rollback-*" }).Count) { + throw "injected committed cleanup lock" + } Microsoft.PowerShell.Management\Remove-Item @PSBoundParameters } $versionWasProvided = $false $NoScheduledTask = $false $failures = [Collections.Generic.List[string]]::new() +$executed = 0 try { foreach ($case in @("stop", "backup-move", "restore-move", "remove-new", "stop-new", "copy", "staged-verification", "snapshot", "remove-task", "promote", "path", "shortcut", "start", "restart", "restore-shortcut", "success", "fresh-success", "fresh-failure", - "portable-success", "locked-install", "relative-success")) { + "portable-success", "locked-install", "relative-success", "committed-cleanup-blocked", + "committed-cleanup-secondary")) { + $executed++ $lock = $null $pushed = $false try { @@ -125,8 +140,10 @@ try { } $state = @{ stops = 0; starts = 0; closed = $false; shortcut = "old" } $NoScheduledTask = $case -in @("portable-success", "locked-install") - $succeeds = $case -in @("success", "fresh-success", "portable-success", "relative-success") + $succeeds = $case -in @("success", "fresh-success", "portable-success", "relative-success", + "committed-cleanup-blocked", "committed-cleanup-secondary") $errorMessage = $null + $installOutput = @() $label = $case if ($case -eq "fresh-failure") { $case = "start" } if ($case -eq "locked-install") { @@ -138,16 +155,31 @@ try { $pushed = $true $InstallRoot = ".\current" } - try { Install-Package (-not $fresh) | Out-Null } catch { $errorMessage = $_.Exception.Message } + try { $installOutput = @(Install-Package (-not $fresh) 3>&1) } catch { $errorMessage = $_.Exception.Message } Assert $state.closed "$label did not close the package" Assert ($succeeds -eq (-not $errorMessage)) "$label returned an unexpected result: $errorMessage" $backups = @(Get-ChildItem -LiteralPath $caseRoot -Directory -Filter ".GraphCode-rollback-*") + $cleanupMarkers = @(Get-ChildItem -LiteralPath $caseRoot -File -Filter ".GraphCode-rollback-*.cleanup-ready") $shortcuts = @(Get-ChildItem -LiteralPath $caseRoot -Directory -Filter ".GraphCode-shortcut-*") $stages = @(Get-ChildItem -LiteralPath $caseRoot -Directory -Filter ".GraphCode-install-*") Assert ($stages.Count -eq 0) "$label leaked its unpromoted stage" if ($succeeds) { Assert ((Get-Content (Join-Path $InstallRoot "bin\graphcoded.exe")) -eq "new") "$label did not install the new payload" - Assert ($backups.Count -eq 0 -and $shortcuts.Count -eq 0) "$label left transaction debris" + if ($label -eq "committed-cleanup-blocked") { + Assert ($backups.Count -eq 1) "$label did not retain the cleanup-blocked committed backup" + Assert ($cleanupMarkers.Count -eq 1) "$label did not mark the committed backup as cleanup-ready" + Assert ($cleanupMarkers[0].BaseName -eq $backups[0].Name) "$label marker did not identify its rollback backup" + Assert (($installOutput | Out-String) -match "could not clean transaction directory") "$label did not warn about blocked cleanup" + } elseif ($label -eq "committed-cleanup-secondary") { + Assert ($backups.Count -eq 1) "$label did not retain the cleanup-blocked committed backup" + Assert ($cleanupMarkers.Count -eq 0) "$label authorized cleanup after marker creation failed" + $warningText = $installOutput | Out-String + Assert ($warningText -match "could not authorize later cleanup" -and + $warningText -match "could not clean transaction directory") "$label did not report both cleanup failures" + } else { + Assert ($backups.Count -eq 0 -and $cleanupMarkers.Count -eq 0) "$label left transaction debris" + } + Assert ($shortcuts.Count -eq 0) "$label left shortcut transaction debris" Assert ($state.shortcut -eq "new") "$label did not update the shortcut" Assert ($NoScheduledTask -eq ($state.starts -eq 0 -and $state.stops -eq 0)) "$label ignored scheduled-task policy" continue @@ -161,13 +193,16 @@ try { Assert ($backups.Count -eq 1) "$label deleted the only recoverable installation" Assert ((Get-Content (Join-Path $backups[0].FullName "sentinel.txt")) -eq "previous installation") "$label corrupted the recovery payload" Assert ($errorMessage.Contains($backups[0].FullName)) "$label did not report the retained backup path: $errorMessage" + Assert ($cleanupMarkers.Count -eq 0) "$label marked a required recovery backup as cleanup-ready" Assert ($state.starts -eq $(if ($case -eq "stop-new") { 1 } else { 0 })) "$label started a daemon before the old payload was restored" } elseif ($fresh) { Assert (-not (Test-Path -LiteralPath $InstallRoot)) "$label left the failed fresh installation" + Assert ($cleanupMarkers.Count -eq 0) "$label marked a failed fresh install as cleanup-ready" } else { Assert (Test-Path -LiteralPath (Join-Path $InstallRoot "sentinel.txt")) "$label deleted the previous installation" Assert ((Get-Content (Join-Path $InstallRoot "bin\graphcoded.exe")) -eq "old") "$label did not restore the old payload" Assert ($backups.Count -eq 0) "$label left an unnecessary payload backup" + Assert ($cleanupMarkers.Count -eq 0) "$label marked a restored backup as cleanup-ready" } if ($case -in @("restart", "restore-shortcut", "restore-move", "remove-new", "stop-new")) { $secondary = switch ($case) { @@ -221,7 +256,8 @@ try { } } if ($failures.Count) { throw ($failures -join "`n") } - Write-Output "Package transaction ownership and recoverable rollback contracts: PASS" + if ($executed -ne 23) { throw "Package rollback contract executed $executed of 23 cases" } + Write-Output "Package transaction ownership and recoverable rollback contracts (23 cases): PASS" } finally { if (Test-Path -LiteralPath $fixture) { Microsoft.PowerShell.Management\Remove-Item -LiteralPath $fixture -Recurse -Force diff --git a/Tools/windows/Tests/WindowsShell.Tests.ps1 b/Tools/windows/Tests/WindowsShell.Tests.ps1 index 88369e3a..d264164d 100644 --- a/Tools/windows/Tests/WindowsShell.Tests.ps1 +++ b/Tools/windows/Tests/WindowsShell.Tests.ps1 @@ -544,6 +544,7 @@ foreach ($path in @( "src\UpdateOfferPresentation.zig", "src\WindowsOnboarding.zig", "src\WindowsProductSettings.zig", + "src\WindowsRollbackCleanup.zig", "src\Accessibility.zig", "src\DesignTokens.zig", "src\Wire.zig", @@ -880,6 +881,20 @@ Invoke-Native "Windows update install executable tests" { & $zig test src\WindowsUpdateInstall.zig -target x86_64-windows-msvc -lc -lwinhttp "-I$include" } finally { Pop-Location } } +Invoke-Native "Committed rollback cleanup executable tests" { + $depotRoot = Split-Path (Split-Path $repoRoot -Parent) -Parent + $winghosttyRoot = if ($env:GRAPHCODE_WINGHOSTTY_ROOT) { + $env:GRAPHCODE_WINGHOSTTY_ROOT + } else { + Join-Path $depotRoot "Winghostty-worktrees\host-integration" + } + $include = Join-Path $winghosttyRoot "include" + Push-Location $shellRoot + try { + & $zig test src\WindowsRollbackCleanup.zig -target x86_64-windows-msvc ` + -lc -lkernel32 "-I$include" + } finally { Pop-Location } +} Invoke-Native "Frame buffer executable tests" { Push-Location $shellRoot try { & $zig test src\FrameBuffer.zig } finally { Pop-Location } diff --git a/Tools/windows/validation-matrix.md b/Tools/windows/validation-matrix.md index 1768b3f1..09bfb4fb 100644 --- a/Tools/windows/validation-matrix.md +++ b/Tools/windows/validation-matrix.md @@ -19,7 +19,7 @@ The Windows port must have runnable commands before implementation fleets begin. | Signed catalog integrity and publisher policy | `pwsh Tools\windows\Tests\Packaging.Signing.Tests.ps1` | | Native SignTool PS1 signature and tamper detection (SDK required) | `pwsh Tools\windows\Tests\Packaging.ScriptSigning.Tests.ps1` | | Native missing/idle task stop and deletion | `pwsh Tools\windows\Tests\Packaging.Scheduler.Tests.ps1` | -| Failed-upgrade preservation and recoverable rollback | `pwsh Tools\windows\Tests\Packaging.Rollback.Tests.ps1` | +| Failed-upgrade preservation, committed-backup marking, and recoverable rollback | `pwsh Tools\windows\Tests\Packaging.Rollback.Tests.ps1` | | Uninstall refusal with live sessions/locked files and all-or-nothing removal | `pwsh Tools\windows\Tests\Packaging.Uninstall.Tests.ps1` | | Standalone setup under PowerShell 5.1 and 7 | `pwsh Tools\windows\Tests\Packaging.Standalone.Tests.ps1` | | Release publishing: signing gates, asset labeling, workflow contract | `pwsh Tools\windows\Tests\Release.Tests.ps1` | @@ -27,6 +27,7 @@ The Windows port must have runnable commands before implementation fleets begin. | Native accessibility provider (MSVC and Windows SDK) | `pwsh -NoProfile -File Tools\windows\Tests\AccessibilityProvider.Native.Tests.ps1` | | Native accessibility harness failure attribution (mock tools, no native fixture) | `pwsh -NoProfile -File Tools\windows\Tests\AccessibilityProvider.Native.Tests.Harness.Tests.ps1` | | Native release feed, URL handoff, and allocation safety | `pwsh Tools\windows\Tests\WindowsShell.Tests.ps1 -ZigExecutable $env:GRAPHCODE_ZIG0152` (includes updater tests) | +| Successful-update rollback cleanup, locked retry, and failed-recovery preservation | `pwsh Tools\windows\Tests\WindowsShell.Tests.ps1 -ZigExecutable $env:GRAPHCODE_ZIG0152` (includes `WindowsRollbackCleanup.zig`) | | Real-product packaging and source/standalone install/upgrade/rollback/uninstall | `pwsh Tools\windows\validate.ps1 -Task packaging` | | Shared Swift package | `swift test --package-path ` once extracted | | macOS app/daemon/CLI | `make test` | diff --git a/graphcode-windows/src/WindowsRollbackCleanup.zig b/graphcode-windows/src/WindowsRollbackCleanup.zig new file mode 100644 index 00000000..34b0d412 --- /dev/null +++ b/graphcode-windows/src/WindowsRollbackCleanup.zig @@ -0,0 +1,380 @@ +const std = @import("std"); +const c = @import("Win32.zig").c; + +const rollback_prefix = ".GraphCode-rollback-"; +const marker_suffix = ".cleanup-ready"; +const retry_count = 4; +const retry_delay_ns = 500 * std.time.ns_per_ms; + +pub const Result = struct { + deleted: usize = 0, + locked: usize = 0, + failed: usize = 0, + orphan_markers: usize = 0, + + fn pending(self: Result) bool { + return self.locked != 0 or self.failed != 0; + } +}; + +const Outcome = enum { + deleted, + locked, + failed, + orphan_marker, + skipped, +}; + +const DeleteApi = struct { + context: ?*anyopaque = null, + deleteTree: *const fn (context: ?*anyopaque, path: []const u8) anyerror!void = deleteTree, +}; + +fn deleteTree(_: ?*anyopaque, path: []const u8) !void { + try std.fs.deleteTreeAbsolute(path); +} + +pub fn start(allocator: std.mem.Allocator) void { + const thread = std.Thread.spawn(.{}, cleanupWorker, .{allocator}) catch |err| { + std.log.warn("Unable to start committed rollback cleanup: {s}", .{@errorName(err)}); + return; + }; + thread.detach(); +} + +fn cleanupWorker(allocator: std.mem.Allocator) void { + var attempt: usize = 0; + while (attempt < retry_count) : (attempt += 1) { + const result = cleanupInstalled(allocator) catch |err| { + std.log.warn("Committed rollback cleanup failed: {s}", .{@errorName(err)}); + return; + }; + if (!result.pending()) return; + if (attempt + 1 < retry_count) std.Thread.sleep(retry_delay_ns); + } +} + +pub fn cleanupInstalled(allocator: std.mem.Allocator) !Result { + const parent = try installedParent(allocator) orelse return .{}; + defer allocator.free(parent); + return cleanupParent(allocator, parent); +} + +fn installedParent(allocator: std.mem.Allocator) !?[]u8 { + const executable = try std.fs.selfExePathAlloc(allocator); + defer allocator.free(executable); + const bin = std.fs.path.dirnameWindows(executable) orelse return null; + if (!std.ascii.eqlIgnoreCase(std.fs.path.basenameWindows(bin), "bin")) return null; + const install_root = std.fs.path.dirnameWindows(bin) orelse return null; + if (!std.ascii.eqlIgnoreCase(std.fs.path.basenameWindows(install_root), "current")) return null; + const setup = try std.fs.path.join(allocator, &.{ install_root, "GraphCode-Setup.ps1" }); + defer allocator.free(setup); + std.fs.accessAbsolute(setup, .{}) catch return null; + const parent = std.fs.path.dirnameWindows(install_root) orelse return null; + return try allocator.dupe(u8, parent); +} + +pub fn cleanupParent(allocator: std.mem.Allocator, parent: []const u8) !Result { + var result = Result{}; + var directory = try std.fs.openDirAbsolute(parent, .{ .iterate = true, .no_follow = true }); + defer directory.close(); + var iterator = directory.iterate(); + while (try iterator.next()) |entry| { + if (entry.kind != .file or !validMarkerName(entry.name)) continue; + switch (try cleanupMarker(allocator, parent, entry.name)) { + .deleted => result.deleted += 1, + .locked => result.locked += 1, + .failed => result.failed += 1, + .orphan_marker => result.orphan_markers += 1, + .skipped => {}, + } + } + return result; +} + +fn validMarkerName(name: []const u8) bool { + if (!std.mem.startsWith(u8, name, rollback_prefix) or !std.mem.endsWith(u8, name, marker_suffix)) return false; + const rollback_name = name[0 .. name.len - marker_suffix.len]; + if (rollback_name.len != rollback_prefix.len + 36) return false; + const guid = rollback_name[rollback_prefix.len..]; + for (guid, 0..) |character, index| { + const hyphen = index == 8 or index == 13 or index == 18 or index == 23; + if (hyphen) { + if (character != '-') return false; + } else if (!std.ascii.isHex(character)) { + return false; + } + } + return true; +} + +fn cleanupMarker( + allocator: std.mem.Allocator, + parent: []const u8, + marker_name: []const u8, +) !Outcome { + return cleanupMarkerWith(allocator, parent, marker_name, .{}); +} + +fn cleanupMarkerWith( + allocator: std.mem.Allocator, + parent: []const u8, + marker_name: []const u8, + delete_api: DeleteApi, +) !Outcome { + const marker_path = try std.fs.path.join(allocator, &.{ parent, marker_name }); + defer allocator.free(marker_path); + const lease = openExclusive(allocator, marker_path, false) catch |err| return switch (err) { + error.FileNotFound => .skipped, + error.SharingViolation, error.LockViolation => .locked, + else => .failed, + }; + var lease_open = true; + defer { + if (lease_open) _ = c.CloseHandle(lease); + } + + const rollback_name = marker_name[0 .. marker_name.len - marker_suffix.len]; + const rollback_path = try std.fs.path.join(allocator, &.{ parent, rollback_name }); + defer allocator.free(rollback_path); + { + var rollback = std.fs.openDirAbsolute(rollback_path, .{ .iterate = true, .no_follow = true }) catch |err| switch (err) { + error.FileNotFound => { + _ = c.CloseHandle(lease); + lease_open = false; + return removeMarker(marker_path, .orphan_marker); + }, + else => return .failed, + }; + defer rollback.close(); + + var walker = rollback.walk(allocator) catch return .failed; + defer walker.deinit(); + while (walker.next() catch return .failed) |entry| { + switch (entry.kind) { + .file => { + const file_path = try std.fs.path.join(allocator, &.{ rollback_path, entry.path }); + defer allocator.free(file_path); + const handle = openExclusive(allocator, file_path, true) catch |err| return switch (err) { + error.SharingViolation, error.LockViolation => .locked, + else => .failed, + }; + _ = c.CloseHandle(handle); + }, + .directory => {}, + else => return .failed, + } + } + } + delete_api.deleteTree(delete_api.context, rollback_path) catch |err| switch (err) { + error.FileNotFound => {}, + error.AccessDenied, error.FileBusy => return .locked, + else => return .failed, + }; + _ = c.CloseHandle(lease); + lease_open = false; + return removeMarker(marker_path, .deleted); +} + +fn removeMarker(marker_path: []const u8, outcome: Outcome) Outcome { + std.fs.deleteFileAbsolute(marker_path) catch |err| switch (err) { + error.FileNotFound => return outcome, + else => return .failed, + }; + return outcome; +} + +fn openExclusive(allocator: std.mem.Allocator, path: []const u8, writable: bool) !c.HANDLE { + const wide = try std.unicode.utf8ToUtf16LeAllocZ(allocator, path); + defer allocator.free(wide); + const attributes = c.GetFileAttributesW(wide.ptr); + if (attributes == c.INVALID_FILE_ATTRIBUTES) return lastFileError(); + const access: c.DWORD = if (writable and attributes & c.FILE_ATTRIBUTE_READONLY == 0) + c.GENERIC_READ | c.GENERIC_WRITE + else + c.GENERIC_READ; + const handle = c.CreateFileW( + wide.ptr, + access, + 0, + null, + c.OPEN_EXISTING, + c.FILE_ATTRIBUTE_NORMAL, + null, + ); + if (handle == c.INVALID_HANDLE_VALUE) return lastFileError(); + return handle; +} + +fn lastFileError() anyerror { + return switch (c.GetLastError()) { + c.ERROR_FILE_NOT_FOUND, c.ERROR_PATH_NOT_FOUND => error.FileNotFound, + c.ERROR_SHARING_VIOLATION => error.SharingViolation, + c.ERROR_LOCK_VIOLATION => error.LockViolation, + c.ERROR_ACCESS_DENIED => error.AccessDenied, + else => error.UnexpectedFileError, + }; +} + +fn writeFixture(dir: std.fs.Dir, path: []const u8, data: []const u8) !void { + if (std.fs.path.dirname(path)) |parent| try dir.makePath(parent); + try dir.writeFile(.{ .sub_path = path, .data = data }); +} + +test "successful update rollback cleanup deletes only committed backups" { + if (@import("builtin").os.tag != .windows) return error.SkipZigTest; + const allocator = std.testing.allocator; + var temporary = std.testing.tmpDir(.{}); + defer temporary.cleanup(); + const parent = try temporary.dir.realpathAlloc(allocator, "."); + defer allocator.free(parent); + const committed = rollback_prefix ++ "11111111-1111-1111-1111-111111111111"; + const recovery = rollback_prefix ++ "22222222-2222-2222-2222-222222222222"; + try writeFixture(temporary.dir, committed ++ "\\bin\\graphcode-windows.exe", "old committed shell"); + try writeFixture(temporary.dir, committed ++ marker_suffix, "committed\n"); + try writeFixture(temporary.dir, recovery ++ "\\sentinel.txt", "required recovery"); + + const result = try cleanupParent(allocator, parent); + try std.testing.expectEqual(@as(usize, 1), result.deleted); + try std.testing.expectError(error.FileNotFound, temporary.dir.access(committed, .{})); + try std.testing.expectError(error.FileNotFound, temporary.dir.access(committed ++ marker_suffix, .{})); + try temporary.dir.access(recovery, .{}); + const sentinel = try temporary.dir.readFileAlloc(allocator, recovery ++ "\\sentinel.txt", 64); + defer allocator.free(sentinel); + try std.testing.expectEqualStrings("required recovery", sentinel); +} + +test "successful update rollback cleanup retains locked backups for retry" { + if (@import("builtin").os.tag != .windows) return error.SkipZigTest; + const allocator = std.testing.allocator; + var temporary = std.testing.tmpDir(.{}); + defer temporary.cleanup(); + const parent = try temporary.dir.realpathAlloc(allocator, "."); + defer allocator.free(parent); + const committed = rollback_prefix ++ "33333333-3333-3333-3333-333333333333"; + const relative_executable = committed ++ "\\bin\\graphcode-windows.exe"; + try writeFixture(temporary.dir, relative_executable, "locked committed shell"); + try writeFixture(temporary.dir, committed ++ marker_suffix, "committed\n"); + const executable = try temporary.dir.realpathAlloc(allocator, relative_executable); + defer allocator.free(executable); + const wide = try std.unicode.utf8ToUtf16LeAllocZ(allocator, executable); + defer allocator.free(wide); + const lock = c.CreateFileW( + wide.ptr, + c.GENERIC_READ, + c.FILE_SHARE_READ, + null, + c.OPEN_EXISTING, + c.FILE_ATTRIBUTE_NORMAL, + null, + ); + try std.testing.expect(lock != c.INVALID_HANDLE_VALUE); + + const blocked = try cleanupParent(allocator, parent); + try std.testing.expectEqual(@as(usize, 1), blocked.locked); + try temporary.dir.access(committed, .{}); + try temporary.dir.access(committed ++ marker_suffix, .{}); + + _ = c.CloseHandle(lock); + const retried = try cleanupParent(allocator, parent); + try std.testing.expectEqual(@as(usize, 1), retried.deleted); + try std.testing.expectError(error.FileNotFound, temporary.dir.access(committed, .{})); + try std.testing.expectError(error.FileNotFound, temporary.dir.access(committed ++ marker_suffix, .{})); +} + +test "successful update rollback cleanup leases markers and rejects malformed names" { + if (@import("builtin").os.tag != .windows) return error.SkipZigTest; + const allocator = std.testing.allocator; + var temporary = std.testing.tmpDir(.{}); + defer temporary.cleanup(); + const parent = try temporary.dir.realpathAlloc(allocator, "."); + defer allocator.free(parent); + const committed = rollback_prefix ++ "44444444-4444-4444-4444-444444444444"; + try writeFixture(temporary.dir, committed ++ "\\payload.txt", "stale"); + try writeFixture(temporary.dir, committed ++ marker_suffix, "committed\n"); + try writeFixture(temporary.dir, rollback_prefix ++ "not-a-guid" ++ marker_suffix, "ignore\n"); + try writeFixture( + temporary.dir, + rollback_prefix ++ "55555555-5555-5555-5555-555555555555" ++ marker_suffix, + "orphan\n", + ); + const marker = try temporary.dir.realpathAlloc(allocator, committed ++ marker_suffix); + defer allocator.free(marker); + const lease = try openExclusive(allocator, marker, false); + + const blocked = try cleanupParent(allocator, parent); + try std.testing.expectEqual(@as(usize, 1), blocked.locked); + try std.testing.expectEqual(@as(usize, 1), blocked.orphan_markers); + try temporary.dir.access(committed, .{}); + try temporary.dir.access(rollback_prefix ++ "not-a-guid" ++ marker_suffix, .{}); + + _ = c.CloseHandle(lease); + const retried = try cleanupParent(allocator, parent); + try std.testing.expectEqual(@as(usize, 1), retried.deleted); +} + +test "successful update rollback cleanup holds its marker lease through deletion" { + if (@import("builtin").os.tag != .windows) return error.SkipZigTest; + const allocator = std.testing.allocator; + var temporary = std.testing.tmpDir(.{}); + defer temporary.cleanup(); + const parent = try temporary.dir.realpathAlloc(allocator, "."); + defer allocator.free(parent); + const committed = rollback_prefix ++ "66666666-6666-6666-6666-666666666666"; + const marker_name = committed ++ marker_suffix; + try writeFixture(temporary.dir, committed ++ "\\payload.txt", "stale"); + try writeFixture(temporary.dir, marker_name, "committed\n"); + const entered = c.CreateEventW(null, 1, 0, null); + try std.testing.expect(entered != null); + defer _ = c.CloseHandle(entered); + const release = c.CreateEventW(null, 1, 0, null); + try std.testing.expect(release != null); + defer _ = c.CloseHandle(release); + + const DeleteContext = struct { + entered: c.HANDLE, + release: c.HANDLE, + + fn run(raw: ?*anyopaque, path: []const u8) !void { + const self: *@This() = @ptrCast(@alignCast(raw.?)); + if (c.SetEvent(self.entered) == 0) return error.TestEventFailed; + if (c.WaitForSingleObject(self.release, 5_000) != c.WAIT_OBJECT_0) return error.TestEventFailed; + try std.fs.deleteTreeAbsolute(path); + } + }; + const CleanupContext = struct { + parent: []const u8, + outcome: Outcome = .failed, + delete_context: *DeleteContext, + + fn run(self: *@This()) void { + self.outcome = cleanupMarkerWith( + std.heap.c_allocator, + self.parent, + marker_name, + .{ .context = self.delete_context, .deleteTree = DeleteContext.run }, + ) catch .failed; + } + }; + var delete_context = DeleteContext{ .entered = entered, .release = release }; + var cleanup_context = CleanupContext{ .parent = parent, .delete_context = &delete_context }; + const thread = try std.Thread.spawn(.{}, CleanupContext.run, .{&cleanup_context}); + var joined = false; + errdefer { + if (!joined) { + _ = c.SetEvent(release); + thread.join(); + } + } + try std.testing.expectEqual(c.WAIT_OBJECT_0, c.WaitForSingleObject(entered, 5_000)); + + const competing = try cleanupParent(allocator, parent); + try std.testing.expectEqual(@as(usize, 1), competing.locked); + try temporary.dir.access(committed, .{}); + if (c.SetEvent(release) == 0) return error.TestEventFailed; + thread.join(); + joined = true; + try std.testing.expectEqual(Outcome.deleted, cleanup_context.outcome); + try std.testing.expectError(error.FileNotFound, temporary.dir.access(committed, .{})); + try std.testing.expectError(error.FileNotFound, temporary.dir.access(marker_name, .{})); +} diff --git a/graphcode-windows/src/main.zig b/graphcode-windows/src/main.zig index 6685d04b..f58fe0c9 100644 --- a/graphcode-windows/src/main.zig +++ b/graphcode-windows/src/main.zig @@ -1,6 +1,7 @@ const std = @import("std"); const App = @import("App.zig").App; const Diagnostics = @import("Diagnostics.zig"); +const WindowsRollbackCleanup = @import("WindowsRollbackCleanup.zig"); const Win32 = @import("Win32.zig"); const c = Win32.c; const build_options = @import("build_options"); @@ -22,6 +23,7 @@ pub fn main() !void { return; } } + WindowsRollbackCleanup.start(allocator); var app = App.init(allocator) catch |err| { if (err == error.InstanceAlreadyRunning) { @import("App.zig").restoreCurrentWorkspace(allocator) catch |restore_error| {