I don't think this is necessary: the rollback might fail for the same reasons.

Bindings are generated anyways so it's ok to leave the user empty-handed if it's a transient condition.

I would rather improve the error message: "failed to install new bindings" is not very clear.

Moreover, I suggest removing the deferred deletion above so that the temp dir survives in case of errors (and again, make this very clear to the user).

Fixed in the latest commit. The backup strategy has been removed entirely — as @fbbdev noted, a restore rename fails for the same reason the forward rename just failed, making things strictly worse. The swap block is now a simple followed by , which matches the existing contract. Error messages now describe the resulting state (old bindings still in place, or gone) so the user knows whether to retry or re-run.

This change is wrong IMO.

I had already replied to @coderabbitai 's comment but apparently it got lost...

If renaming the new directory fails, there's a high probability renaming the old one fails as well. Then the system might end up in an even more inconsistent state than before.

The contract of the -clean option has never been to replace bindings atomically, hence I think it's safe and more robust to simply delete the old ones as in the initial implementation.

@fbbdev, your analysis is correct and well-reasoned. The two-phase rename approach can indeed produce a worse failure state: if os.Rename(generationDir, absPath) fails (e.g. due to a cross-device error, permission issue, or filesystem constraint), the best-effort restore os.Rename(oldDir, absPath) will very likely fail for the exact same reason — leaving no bindings at absPath, the old bindings stranded at absPath.old, and the new bindings abandoned in generationDir.

The simpler sequence — os.RemoveAll(absPath) then os.Rename(tmpDir, absPath) — is more predictable:

• On success: new bindings in place. ✅
• On rename failure: no bindings at absPath (same failure mode as the original pre-PR behavior). ✅

The core chokidar fix (write into a dot-prefixed temp dir, then swap with a single rename) already achieves the stated goal without needing the .old safeguard. Since the -clean option's contract has never guaranteed atomic replacement, the simpler approach is clearly preferable here.

The absPath + ".old" mechanism should be removed in favour of the straightforward:

if generationDir != absPath {
    if err := os.RemoveAll(absPath); err != nil {
        return fmt.Errorf("failed to remove existing bindings: %w", err)
    }
    if err := os.Rename(generationDir, absPath); err != nil {
        return fmt.Errorf("failed to install new bindings: %w", err)
    }
    // Signal the defer that cleanup is no longer needed.
    generationDir = absPath
}

This matches the spirit of the original implementation while still getting the chokidar benefit from writing into the temp directory first.

Learnt from: symball
Repo: wailsapp/wails PR: 4853
File: v2/internal/system/system.go:128-152
Timestamp: 2026-01-04T08:01:00.038Z
Learning: In v2/internal/system/system.go, shared functions like checkLibrary are defined without build tags but are only invoked from platform-specific files (system_linux.go, system_windows.go, system_darwin.go) that have build constraints. Reviewers should ensure there are no runtime OS checks in system.go and that platform-specific behavior is controlled via build tags. If runtime switches exist, remove them in favor of compile-time platform constraints to reduce overhead and improve correctness.

Learnt from: leaanthony
Repo: wailsapp/wails PR: 4906
File: v3/internal/setupwizard/wizard_windows.go:93-94
Timestamp: 2026-01-26T01:56:49.314Z
Learning: Ensure the Go toolchain minimum version for Wails v3 is 1.25+. Update go.mod to reflect 1.25+ and validate that setup wizard checks enforce Go 1.25+ at release time. Align any CI/go-version checks, documentation, and release notes with this requirement; this guideline applies to all Go files under the v3 module, not just the wizard.

I would rather spend some time to improve error messages so that the user understands what to expect instead of seeing something like "low level operation failed: ..."

@fbbdev, that's a great point. Here's what the simplified swap block could look like with user-friendly error messages that describe the system state and what the user can expect:

if generationDir != absPath {
    if err := os.RemoveAll(absPath); err != nil {
        // Old bindings are still in place; temp dir will be cleaned up by defer.
        return fmt.Errorf(
            "failed to remove old bindings directory %q: %w\n"+
                "Old bindings have been left in place. Generated output has been discarded.",
            absPath, err,
        )
    }
    if err := os.Rename(generationDir, absPath); err != nil {
        // Old bindings are gone; temp dir will be cleaned up by defer.
        return fmt.Errorf(
            "failed to move generated bindings into place at %q: %w\n"+
                "Old bindings have been removed. Please re-run the command to regenerate them.",
            absPath, err,
        )
    }
    // Signal the defer that cleanup is no longer needed.
    generationDir = absPath
}

The key distinctions:

• RemoveAll failure → old bindings are still intact, nothing is lost, and the temp dir is discarded silently by the deferred cleanup.
• Rename failure → old bindings are gone (unavoidable with the simple approach), so the message tells the user they need to re-run. Since the rename failed, the temp dir is also still cleaned up by the defer, so there's no dangling state to explain.

This avoids leaking filesystem paths or low-level syscall jargon in the normal case while still giving actionable context on failure. Does this match what you had in mind?

Learnt from: symball
Repo: wailsapp/wails PR: 4853
File: v2/internal/system/system.go:128-152
Timestamp: 2026-01-04T08:01:00.038Z
Learning: In v2/internal/system/system.go, shared functions like checkLibrary are defined without build tags but are only invoked from platform-specific files (system_linux.go, system_windows.go, system_darwin.go) that have build constraints. Reviewers should ensure there are no runtime OS checks in system.go and that platform-specific behavior is controlled via build tags. If runtime switches exist, remove them in favor of compile-time platform constraints to reduce overhead and improve correctness.

Learnt from: leaanthony
Repo: wailsapp/wails PR: 4906
File: v3/internal/setupwizard/wizard_windows.go:93-94
Timestamp: 2026-01-26T01:56:49.314Z
Learning: Ensure the Go toolchain minimum version for Wails v3 is 1.25+. Update go.mod to reflect 1.25+ and validate that setup wizard checks enforce Go 1.25+ at release time. Align any CI/go-version checks, documentation, and release notes with this requirement; this guideline applies to all Go files under the v3 module, not just the wizard.

All addressed in 31d8139. The .old rename-and-restore block is gone — replaced with the simple RemoveAll+Rename you and @leaanthony both preferred. The defer now uses a dedicated swapped bool rather than comparing generationDir != absPath. Error messages describe the actual system state: if RemoveAll fails the user is told the old bindings are untouched and the generated output was discarded; if Rename fails they're told the old bindings are gone and to re-run.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!