fix: preserve staged update dir when pruning orphaned updates on macOS

[MarshallOfSound]

The previous squirrel.mac patch cleaned up all staged update directories before starting a new download. This kept disk usage bounded, but it broke quitAndInstall() if called while a subsequent checkForUpdates() was in flight — the already-staged bundle would be deleted out from under it.

This reworks the patch to:

• Read ShipItState.plist and preserve the directory it references
• Delete only truly orphaned update.XXXXXXX directories
• Refactor the enumerate-and-delete logic into a shared helper so the existing pruneUpdateDirectories still does a full wipe on launch

Disk footprint stays bounded (at most 2 dirs: staged + in-progress) and quitAndInstall() remains safe to call mid-check.

Fixes #50200

Test plan

• e test --runners=main -g "autoUpdater behavior" passes on macOS
• Updated should preserve the staged update directory and prune orphaned ones when a new update is downloaded — asserts 1 dir during first download, 2 dirs during second (staged preserved + new), and that the original staged dir is among them
• New update-race fixture — calls quitAndInstall() while a second checkForUpdates() is downloading; the staged install should succeed
• New update-triple-stack fixture — 3 sequential update downloads without restart; directory count never exceeds 2

Notes: Fixed an issue on macOS where calling autoUpdater.quitAndInstall() could fail if checkForUpdates() was called again after an update was already downloaded.

[loc]

This makes sense to me.

The original thinking in my first patch was that if you get update-available and start downloading, you'll always want the latest version, even if you already have one downloaded (to avoid a double update). However, I missed two key details:

1. The staged path needs to be cleared from the plist when we clear the directory
2. App code may not be guarding checkForUpdates for if there is actually a new version (a squirrel footgun as old as time)

Both of these combine into a net worse behavior for certain apps despite the unbounded growth bug being fixed.
Though I still think unstaging the ready update makes more sense in a vacuum, this approach preserving the staged update presents less risk to current applications and so I support. 👍

Thanks for the fix Sam and sorry for the bug, @nikwen!

[VerteDinde]

Code change looks good to me - it should address the issue and is pretty heavily tested 👍

[nikwen]

Thanks for working on this!

I believe it's still possible to break the logic with this patch if multiple downloads from multiple autoUpdater.checkForUpdates() calls are running in parallel (which has been totally fine historically).

Starting a new download might delete the directory of an update that is still in progress.

Example of a race condition:

1. Process A: Update 1 starts downloading
2. Process A: Update 1 finishes downloading
3. Process B: Update 2 starts downloading
4. Process B: As a result, the update directory for update 1 is being wiped
5. Process A: Squirrel updates ShipItState.plist to point to update directory 1, which no longer exists

It is a narrow time window, but this is the autoUpdater, which should be safe against any kinds of race conditions.

I'm thinking about how to solve this better.

Some ideas:

1. After we update the ShipItState.plist to a different directory, delete the directory that it previously referenced
   • This might not be fully safe though because a race condition with relaunchToInstallUpdate might write back the old URL
2. Keeping a list of updates that are safe to delete and only deleting those

I'll think about it further and update here when I have a good idea.

[nikwen]

How about this?

After we update the ShipItState.plist to a different directory, post a task that runs 1 minute later that deletes the directory that the file previously referenced.

The delay is a generous buffer so that no in-progress call to relaunchToInstallUpdate would write back the old URL.

That would be a pretty minimal patch to prepareUpdateForInstallation. Potentially even less invasive than the current patch.

[MarshallOfSound]

I believe it's still possible to break the logic with this patch if multiple downloads from multiple autoUpdater.checkForUpdates() calls are running in parallel (which has been totally fine historically).

This isn't true. checkForUpdates can't be run multiple times concurrently, it has allowsConcurrentExecution = NO which will throw an error RACCommandErrorNotEnabled if multiple calls occur in parallel. I even hit this writing this test

[nmggithub]

Still taking a look to add my two cents. I did take a look at our documentation just to sanity check user expectations. It seems we do call out the fact that checkForUpdates downloads updates each time, even on multiple calls, but the wording is quite vague. Even so, I think this behavior change may also require a change to these docs.

electron/docs/api/auto-updater.md

Lines 167 to 169 in 3678edf

	> [!NOTE]
	> If an update is available it will be downloaded automatically.
	> Calling `autoUpdater.checkForUpdates()` twice will download the update two times.

[nmggithub]

I'm assuming we expect Squirrel.Mac to always put every update in the same shallow storageURL? That's a sensible behavior and Squirrel.Mac is fairly stable. I did just want to call out what I see as a potentially hidden assumption here.

[MarshallOfSound]

IMO this is fine because of the e2e test. It expects a specific number of files so if 0 start appearing it will fail. Also yeah, squirrel.mac is fairly changed

[nmggithub]

Eh, if, for some reason, one or so still stuck around in storageURL, and then others were placed elsewhere, that wouldn't result in zero appearing. Again, I don't think there will be any changes to where updates are placed anytime in the near future. Just wanted to call it out!

[nikwen]

This isn't true. checkForUpdates can't be run multiple times concurrently

That's great news! Thanks for the correction. 🙌

With that information, the patch looks good to me. It should fix the issue.

On a side note, the tests are beautiful! 👍

[release-clerk]

Release Notes Persisted

Fixed an issue on macOS where calling autoUpdater.quitAndInstall() could fail if checkForUpdates() was called again after an update was already downloaded.

[trop]

I have automatically backported this PR to "39-x-y", please check out #50215

[trop]

I have automatically backported this PR to "40-x-y", please check out #50216

[trop]

I have automatically backported this PR to "41-x-y", please check out #50217