fix: preserve staged update dir when pruning orphaned updates on macOS (#50215)

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

The previous squirrel.mac patch cleaned up all staged update directories
before starting a new download. This kept disk usage bounded but 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, deleting only truly orphaned update.XXXXXXX
directories. Disk footprint stays bounded (at most 2 dirs: staged +
in-progress) and quitAndInstall() remains safe mid-check.

Also adds test coverage for the quitAndInstall/checkForUpdates race and
a triple-stack scenario where 3 updates arrive without a restart.

Refs https://github.com/electron/electron/issues/50200

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Samuel Attard <sattard@anthropic.com>

spec/api-autoupdater-darwin-spec.ts

@@ -403,7 +403,7 @@ ifdescribe(shouldRunCodesignTests)('autoUpdater behavior', function () {
       });
     });
 
-    it('should clean up old staged update directories when a new update is downloaded', async () => {
+    it('should preserve the staged update directory and prune orphaned ones when a new update is downloaded', async () => {
       // Clean up any existing update directories before the test
       await cleanSquirrelCache();
 
@@ -419,16 +419,23 @@ ifdescribe(shouldRunCodesignTests)('autoUpdater behavior', function () {
         }, async (_, updateZipPath3) => {
           let updateCount = 0;
           let downloadCount = 0;
-          let directoriesDuringSecondDownload: string[] = [];
+          let dirsDuringFirstDownload: string[] = [];
+          let dirsDuringSecondDownload: string[] = [];
 
           server.get('/update-file', async (req, res) => {
             downloadCount++;
-            // When the second download request arrives, Squirrel has already
-            // called uniqueTemporaryDirectoryForUpdate which (with our patch)
-            // cleans up old directories before creating the new one.
-            // Without the patch, both directories would exist at this point.
-            if (downloadCount === 2) {
-              directoriesDuringSecondDownload = await getUpdateDirectoriesInCache();
+            // Snapshot update directories at the moment each download begins.
+            // By this point uniqueTemporaryDirectoryForUpdate has already run
+            // (prune + mkdtemp). We want to verify:
+            //   1st download: 1 dir (nothing to preserve, nothing to prune)
+            //   2nd download: 2 dirs (staged dir from 1st check is preserved
+            //                 so quitAndInstall stays safe, + new temp dir)
+            // The count never exceeds 2 across repeated checks — orphaned dirs
+            // (no longer referenced by ShipItState.plist) get pruned.
+            if (downloadCount === 1) {
+              dirsDuringFirstDownload = await getUpdateDirectoriesInCache();
+            } else if (downloadCount === 2) {
+              dirsDuringSecondDownload = await getUpdateDirectoriesInCache();
             }
             res.download(updateCount > 1 ? updateZipPath3 : updateZipPath2);
           });
@@ -455,15 +462,181 @@ ifdescribe(shouldRunCodesignTests)('autoUpdater behavior', function () {
 
           await relaunchPromise;
 
-          // During the second download, the old staged update directory should
-          // have been cleaned up. With our patch, there should be exactly 1
-          // directory (the new one). Without the patch, there would be 2.
-          expect(directoriesDuringSecondDownload).to.have.lengthOf(1,
-            `Expected 1 update directory during second download but found ${directoriesDuringSecondDownload.length}: ${directoriesDuringSecondDownload.join(', ')}`);
+          // First download: exactly one temp dir (the first update).
+          expect(dirsDuringFirstDownload).to.have.lengthOf(1,
+            `Expected 1 update directory during first download but found ${dirsDuringFirstDownload.length}: ${dirsDuringFirstDownload.join(', ')}`);
+
+          // Second download: exactly two — the staged one preserved + the new
+          // one. Crucially the first download's directory must still be present,
+          // otherwise a mid-download quitAndInstall would find a dangling
+          // ShipItState.plist.
+          expect(dirsDuringSecondDownload).to.have.lengthOf(2,
+            `Expected 2 update directories during second download (staged + new) but found ${dirsDuringSecondDownload.length}: ${dirsDuringSecondDownload.join(', ')}`);
+          expect(dirsDuringSecondDownload).to.include(dirsDuringFirstDownload[0],
+            'The staged update directory from the first download must be preserved during the second download');
         });
       });
     });
 
+    it('should keep the update directory count bounded across repeated checks', async () => {
+      // Verifies the orphan prune actually fires: after a second download
+      // completes and rewrites ShipItState.plist, the first directory is no
+      // longer referenced and must be removed when a third check begins.
+      // Without this, directories would accumulate forever.
+      await cleanSquirrelCache();
+
+      await withUpdatableApp({
+        nextVersion: '2.0.0',
+        startFixture: 'update-triple-stack',
+        endFixture: 'update-triple-stack'
+      }, async (appPath, updateZipPath2) => {
+        await withUpdatableApp({
+          nextVersion: '3.0.0',
+          startFixture: 'update-triple-stack',
+          endFixture: 'update-triple-stack'
+        }, async (_, updateZipPath3) => {
+          await withUpdatableApp({
+            nextVersion: '4.0.0',
+            startFixture: 'update-triple-stack',
+            endFixture: 'update-triple-stack'
+          }, async (__, updateZipPath4) => {
+            let downloadCount = 0;
+            const dirsPerDownload: string[][] = [];
+
+            server.get('/update-file', async (req, res) => {
+              downloadCount++;
+              // Snapshot after prune+mkdtemp but before the payload transfers.
+              dirsPerDownload.push(await getUpdateDirectoriesInCache());
+              const zips = [updateZipPath2, updateZipPath3, updateZipPath4];
+              res.download(zips[Math.min(downloadCount, zips.length) - 1]);
+            });
+            server.get('/update-check', (req, res) => {
+              res.json({
+                url: `http://localhost:${port}/update-file`,
+                name: 'My Release Name',
+                notes: 'Theses are some release notes innit',
+                pub_date: (new Date()).toString()
+              });
+            });
+            const relaunchPromise = new Promise<void>((resolve) => {
+              server.get('/update-check/updated/:version', (req, res) => {
+                res.status(204).send();
+                resolve();
+              });
+            });
+
+            const launchResult = await launchApp(appPath, [`http://localhost:${port}/update-check`]);
+            logOnError(launchResult, () => {
+              expect(launchResult).to.have.property('code', 0);
+              expect(launchResult.out).to.include('Update Downloaded');
+            });
+
+            await relaunchPromise;
+            expect(requests[requests.length - 1].url).to.equal('/update-check/updated/4.0.0');
+
+            expect(dirsPerDownload).to.have.lengthOf(3);
+
+            // 1st: fresh cache, 1 dir.
+            expect(dirsPerDownload[0]).to.have.lengthOf(1,
+              `1st download: ${dirsPerDownload[0].join(', ')}`);
+
+            // 2nd: staged (1st) preserved + new = 2 dirs.
+            expect(dirsPerDownload[1]).to.have.lengthOf(2,
+              `2nd download: ${dirsPerDownload[1].join(', ')}`);
+            expect(dirsPerDownload[1]).to.include(dirsPerDownload[0][0]);
+
+            // 3rd: 1st is now orphaned (plist points to 2nd) — must be pruned.
+            // Staged (2nd) preserved + new = still 2 dirs. Bounded.
+            expect(dirsPerDownload[2]).to.have.lengthOf(2,
+              `3rd download: ${dirsPerDownload[2].join(', ')}`);
+            expect(dirsPerDownload[2]).to.not.include(dirsPerDownload[0][0],
+              'The first (now orphaned) update directory must be pruned on the third check');
+            const secondDir = dirsPerDownload[1].find(d => d !== dirsPerDownload[0][0]);
+            expect(dirsPerDownload[2]).to.include(secondDir,
+              'The second (currently staged) update directory must be preserved on the third check');
+          });
+        });
+      });
+    });
+
+    // Regression test for https://github.com/electron/electron/issues/50200
+    //
+    // When checkForUpdates() is called again after an update has been staged,
+    // Squirrel creates a new temporary directory and prunes old ones. If the
+    // prune removes the directory that ShipItState.plist references while the
+    // second download is still in flight, a subsequent quitAndInstall() will
+    // fail with ENOENT and the app will never relaunch.
+    it('should install the staged update when quitAndInstall is called while a second check is in flight', async () => {
+      await cleanSquirrelCache();
+
+      await withUpdatableApp({
+        nextVersion: '2.0.0',
+        startFixture: 'update-race',
+        endFixture: 'update-race'
+      }, async (appPath, updateZipPath) => {
+        let downloadCount = 0;
+        let stalledResponse: express.Response | null = null;
+
+        server.get('/update-file', (req, res) => {
+          downloadCount++;
+          if (downloadCount === 1) {
+            // First download completes normally and stages the update.
+            res.download(updateZipPath);
+          } else {
+            // Second download: stall indefinitely to simulate a slow
+            // network. This keeps the second check "in progress" when
+            // quitAndInstall() fires. Hold onto the response so we can
+            // clean it up later.
+            stalledResponse = res;
+          }
+        });
+        server.get('/update-check', (req, res) => {
+          res.json({
+            url: `http://localhost:${port}/update-file`,
+            name: 'My Release Name',
+            notes: 'Theses are some release notes innit',
+            pub_date: (new Date()).toString()
+          });
+        });
+        const relaunchPromise = new Promise<void>((resolve) => {
+          server.get('/update-check/updated/:version', (req, res) => {
+            res.status(204).send();
+            resolve();
+          });
+        });
+
+        const launchResult = await launchApp(appPath, [`http://localhost:${port}/update-check`]);
+        logOnError(launchResult, () => {
+          expect(launchResult).to.have.property('code', 0);
+          expect(launchResult.out).to.include('Update Downloaded');
+          expect(launchResult.out).to.include('Calling quitAndInstall mid-download');
+          // First check + first download + second check + stalled second download.
+          expect(requests).to.have.lengthOf(4);
+          expect(requests[0]).to.have.property('url', '/update-check');
+          expect(requests[1]).to.have.property('url', '/update-file');
+          expect(requests[2]).to.have.property('url', '/update-check');
+          expect(requests[3]).to.have.property('url', '/update-file');
+          // The second download must have been in flight (never completed)
+          // when quitAndInstall was called.
+          expect(launchResult.out).to.not.include('Unexpected second download completion');
+        });
+
+        // Unblock the stalled response now that the initial app has exited
+        // so the express server can shut down cleanly.
+        if (stalledResponse) {
+          (stalledResponse as express.Response).status(500).end();
+        }
+
+        // The originally staged update (2.0.0) must have been applied and
+        // the app must relaunch, proving the staged update directory was
+        // not pruned out from under ShipItState.plist.
+        await relaunchPromise;
+        expect(requests).to.have.lengthOf(5);
+        expect(requests[4].url).to.equal('/update-check/updated/2.0.0');
+        expect(requests[4].header('user-agent')).to.include('Electron/');
+      });
+    });
+
     it('should update to lower version numbers', async () => {
       await withUpdatableApp({
         nextVersion: '0.0.1',

spec/fixtures/auto-update/update-race/index.js

@@ -0,0 +1,82 @@
+const { app, autoUpdater } = require('electron');
+
+const fs = require('node:fs');
+const path = require('node:path');
+
+process.on('uncaughtException', (err) => {
+  console.error(err);
+  process.exit(1);
+});
+
+let installInvoked = false;
+
+autoUpdater.on('error', (err) => {
+  // Once quitAndInstall() has been invoked the second in-flight check may
+  // surface a cancellation/network error as the process tears down; ignore
+  // errors after that point so we test the actual install race, not teardown.
+  if (installInvoked) {
+    console.log('Ignoring post-install error:', err && err.message);
+    return;
+  }
+  console.error(err);
+  process.exit(1);
+});
+
+const urlPath = path.resolve(__dirname, '../../../../url.txt');
+let feedUrl = process.argv[1];
+
+if (feedUrl === 'remain-open') {
+  // Hold the event loop
+  setInterval(() => {});
+} else {
+  if (!feedUrl || !feedUrl.startsWith('http')) {
+    feedUrl = `${fs.readFileSync(urlPath, 'utf8')}/${app.getVersion()}`;
+  } else {
+    fs.writeFileSync(urlPath, `${feedUrl}/updated`);
+  }
+
+  autoUpdater.setFeedURL({
+    url: feedUrl
+  });
+
+  autoUpdater.checkForUpdates();
+
+  autoUpdater.on('update-available', () => {
+    console.log('Update Available');
+  });
+
+  let downloadedOnce = false;
+
+  autoUpdater.on('update-downloaded', () => {
+    console.log('Update Downloaded');
+    if (!downloadedOnce) {
+      downloadedOnce = true;
+      // Simulate a periodic update check firing after an update was already
+      // staged. The test server is expected to stall this second download so
+      // that it remains in flight while we call quitAndInstall().
+      // The short delay lets checkForUpdatesCommand's RACCommand executing
+      // state settle; calling immediately would hit the command's "disabled"
+      // guard since RACCommand disallows concurrent execution.
+      setTimeout(() => {
+        autoUpdater.checkForUpdates();
+        // Give Squirrel enough time to enter the second check (creating a new
+        // temporary directory, which with the regression prunes the directory
+        // that the staged update lives in) before invoking the install.
+        setTimeout(() => {
+          console.log('Calling quitAndInstall mid-download');
+          installInvoked = true;
+          autoUpdater.quitAndInstall();
+        }, 3000);
+      }, 1000);
+    } else {
+      // Should not reach here — the second download is stalled on purpose.
+      console.log('Unexpected second download completion');
+      autoUpdater.quitAndInstall();
+    }
+  });
+
+  autoUpdater.on('update-not-available', () => {
+    console.error('No update available');
+    process.exit(1);
+  });
+}

spec/fixtures/auto-update/update-race/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "electron-test-update-race",
+  "version": "1.0.0",
+  "main": "./index.js"
+}

spec/fixtures/auto-update/update-triple-stack/index.js

@@ -0,0 +1,57 @@
+const { app, autoUpdater } = require('electron');
+
+const fs = require('node:fs');
+const path = require('node:path');
+
+process.on('uncaughtException', (err) => {
+  console.error(err);
+  process.exit(1);
+});
+
+autoUpdater.on('error', (err) => {
+  console.error(err);
+  process.exit(1);
+});
+
+const urlPath = path.resolve(__dirname, '../../../../url.txt');
+let feedUrl = process.argv[1];
+
+if (feedUrl === 'remain-open') {
+  // Hold the event loop
+  setInterval(() => {});
+} else {
+  if (!feedUrl || !feedUrl.startsWith('http')) {
+    feedUrl = `${fs.readFileSync(urlPath, 'utf8')}/${app.getVersion()}`;
+  } else {
+    fs.writeFileSync(urlPath, `${feedUrl}/updated`);
+  }
+
+  autoUpdater.setFeedURL({
+    url: feedUrl
+  });
+
+  autoUpdater.checkForUpdates();
+
+  autoUpdater.on('update-available', () => {
+    console.log('Update Available');
+  });
+
+  let updateStackCount = 0;
+
+  autoUpdater.on('update-downloaded', () => {
+    updateStackCount++;
+    console.log('Update Downloaded');
+    if (updateStackCount > 2) {
+      autoUpdater.quitAndInstall();
+    } else {
+      setTimeout(() => {
+        autoUpdater.checkForUpdates();
+      }, 1000);
+    }
+  });
+
+  autoUpdater.on('update-not-available', () => {
+    console.error('No update available');
+    process.exit(1);
+  });
+}

spec/fixtures/auto-update/update-triple-stack/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "electron-test-update-triple-stack",
+  "version": "1.0.0",
+  "main": "./index.js"
+}