fix(bar): stage downloads and swap so reinstall never strands the user

The reinstall guard deleted the existing bundle before download, so a
transient download or extraction failure left no app on disk. The
archive now extracts into a hidden staging directory inside the
Applications folder; the old bundle is removed only after the new one
is verified in staging, then renamed into place. Every failure before
the swap leaves the previous install untouched, and staging is cleaned
up on all paths.

src/commands/bar/install-subcommand.ts

@@ -513,10 +513,58 @@ export async function handleBarInstall(
   const { downloadUrl } = releaseInfo;
   console.log(`[i] Installing CCS Bar from ${BAR_RELEASE_TAG}...`);
 
-  // 1b. Stale reinstall guard: remove the existing bundle BEFORE extraction so a
+  // 1b. Stage-then-swap: extract into a hidden staging dir on the same filesystem
-  //     malformed archive that lacks the expected bundle cannot silently leave the
+  //     so the rename to appPath is atomic-ish. The old bundle is only removed
-  //     old version in place.  The removal window is intentionally small — release
+  //     AFTER the new one is verified in staging — a transient download/extraction
-  //     info is already resolved so we are committed to downloading.
+  //     failure during reinstall therefore leaves the previous working install intact.
+  fs.mkdirSync(appsDir, { recursive: true });
+  let stagingDir: string;
+  try {
+    stagingDir = fs.mkdtempSync(path.join(appsDir, '.ccs-bar-staging-'));
+  } catch (err) {
+    const msg = err instanceof Error ? err.message : String(err);
+    console.error(`[X] Could not create staging directory in ${appsDir}: ${msg}`);
+    return;
+  }
+
+  // Helper: remove staging dir, ignoring errors (best-effort cleanup).
+  function cleanupStaging(): void {
+    try {
+      fs.rmSync(stagingDir, { recursive: true, force: true });
+    } catch {
+      /* ignore */
+    }
+  }
+
+  // 2. Download and extract into staging, NOT appsDir.
+  try {
+    await downloadAndExtract(downloadUrl, stagingDir);
+  } catch (err) {
+    const msg = err instanceof Error ? err.message : String(err);
+    console.error(`[X] Download or extraction failed: ${msg}`);
+    cleanupStaging();
+    return;
+  }
+
+  // 3. Finding #12: assert that the expected .app bundle was actually extracted
+  //    into the staging dir before touching the old install.
+  const stagedApp = path.join(stagingDir, BAR_APP_NAME);
+  if (!fs.existsSync(stagedApp)) {
+    // Report what was actually extracted to help diagnose archive issues.
+    let extracted: string[] = [];
+    try {
+      extracted = fs.readdirSync(stagingDir);
+    } catch {
+      /* ignore */
+    }
+    const found = extracted.length > 0 ? extracted.join(', ') : '(none)';
+    console.error(`[X] Extraction succeeded but "${BAR_APP_NAME}" was not found in staging.`);
+    console.error(`[i] Files found in staging: ${found}`);
+    cleanupStaging();
+    return;
+  }
+
+  // 3b. New bundle verified in staging — now safe to remove the old install.
   if (fs.existsSync(appPath)) {
     try {
       removeExistingApp(appPath);
@@ -524,34 +572,23 @@ export async function handleBarInstall(
       const msg = err instanceof Error ? err.message : String(err);
       console.error(`[X] Could not remove the existing CCS Bar.app: ${msg}`);
       console.error('[i] Close any running instance and try again.');
+      cleanupStaging();
       return;
     }
   }
 
-  // 2. Download and extract into ~/Applications.
+  // 3c. Atomic-ish rename: move staged bundle into the final location.
   try {
-    await downloadAndExtract(downloadUrl, appsDir);
+    fs.renameSync(stagedApp, appPath);
   } catch (err) {
     const msg = err instanceof Error ? err.message : String(err);
-    console.error(`[X] Download or extraction failed: ${msg}`);
+    console.error(`[X] Could not move staged app into place: ${msg}`);
+    cleanupStaging();
     return;
   }
 
-  // 3. Finding #12: assert that the expected .app bundle was actually extracted
+  // Staging dir is now empty (bundle was moved out); remove it.
-  //    before reporting success.
+  cleanupStaging();
-  if (!fs.existsSync(appPath)) {
-    // Report what was actually extracted to help diagnose archive issues.
-    let extracted: string[] = [];
-    try {
-      extracted = fs.readdirSync(appsDir);
-    } catch {
-      /* ignore */
-    }
-    const found = extracted.length > 0 ? extracted.join(', ') : '(none)';
-    console.error(`[X] Extraction succeeded but "${BAR_APP_NAME}" was not found in ${appsDir}.`);
-    console.error(`[i] Files found in ${appsDir}: ${found}`);
-    return;
-  }
 
   // 4. Read the real version from the extracted bundle's Info.plist.
   const installedVersion = readAppBundleVersion(appPath);

tests/unit/commands/bar-command.test.ts

@@ -2767,10 +2767,10 @@ describe('bar install: whitespace-only CFBundleShortVersionString yields null (F
 });
 
 // ---------------------------------------------------------------------------
-// Review finding — stale reinstall: old bundle removed before extraction
+// Data Loss finding — stage-then-swap: old bundle preserved on failure
 // ---------------------------------------------------------------------------
 
-describe('bar install: stale reinstall guard — removeExistingApp (review finding)', () => {
+describe('bar install: stage-then-swap safety (Data Loss finding)', () => {
   const FAKE_DOWNLOAD_URL =
     'https://github.com/kaitranntt/ccs/releases/download/ccs-bar-latest/CCS-Bar.app.zip';
 
@@ -2792,36 +2792,134 @@ describe('bar install: stale reinstall guard — removeExistingApp (review findi
     };
   }
 
-  it('removes old bundle before calling downloadAndExtract on reinstall', async () => {
+  it('downloadAndExtract receives a staging dir inside appsDir, NOT appsDir itself', async () => {
-    // Pre-seed a fake CCS Bar.app so the already-installed path is exercised.
+    // The new contract: downloadAndExtract dest is a hidden staging dir under appsDir,
+    // not appsDir directly. This keeps the old install untouched during download.
     const appsDir = path.join(tempHome, 'Applications');
-    const appPath = path.join(appsDir, 'CCS Bar.app');
+    const destDirs: string[] = [];
-    fs.mkdirSync(appPath, { recursive: true });
-
-    let appExistedAtDownload = true; // pessimistic default
     const { handleBarInstall } = await loadInstallSubcommand();
 
     await handleBarInstall([], {
       ...baseDeps(appsDir),
-      // Default removeExistingApp (fs.rmSync) is used — it will remove the dir.
       downloadAndExtract: async (_url: string, dest: string) => {
-        // Assert: old bundle must be gone when extraction is invoked.
+        destDirs.push(dest);
-        appExistedAtDownload = fs.existsSync(appPath);
+        // Create the expected bundle inside staging so the verify step passes.
-        // Place a fresh bundle so the post-extract assertion passes.
         fs.mkdirSync(path.join(dest, 'CCS Bar.app'), { recursive: true });
       },
     });
 
-    expect(appExistedAtDownload).toBe(false);
+    expect(destDirs).toHaveLength(1);
+    const dest = destDirs[0];
+    // Must NOT equal appsDir
+    expect(dest).not.toBe(appsDir);
+    // Must be a child of appsDir (same filesystem for atomic rename)
+    expect(dest.startsWith(appsDir + path.sep)).toBe(true);
+    // Dotted prefix to minimise Finder noise
+    expect(path.basename(dest)).toMatch(/^\..+/);
   });
 
-  it('aborts install and does NOT call downloadAndExtract when removeExistingApp throws', async () => {
+  it('downloadAndExtract throws during reinstall: old bundle still present, removeExistingApp NOT called', async () => {
-    // Pre-seed the old bundle so the removal path is triggered.
+    // Core data-loss guard: if download/extraction fails, the old working install
+    // must remain intact — removeExistingApp must never have been called.
+    const appsDir = path.join(tempHome, 'Applications');
+    const appPath = path.join(appsDir, 'CCS Bar.app');
+    // Pre-seed the existing install.
+    fs.mkdirSync(appPath, { recursive: true });
+    fs.writeFileSync(path.join(appPath, 'sentinel'), 'old-install');
+
+    let removeCalled = false;
+    const { handleBarInstall } = await loadInstallSubcommand();
+
+    await handleBarInstall([], {
+      ...baseDeps(appsDir),
+      removeExistingApp: (_path: unknown) => {
+        removeCalled = true;
+        fs.rmSync(String(_path), { recursive: true, force: true });
+      },
+      downloadAndExtract: async (_url: string, _dest: string) => {
+        throw new Error('network timeout');
+      },
+    });
+
+    // removeExistingApp must NOT have been called — download failed before the swap.
+    expect(removeCalled).toBe(false);
+    // Old bundle must still be present.
+    expect(fs.existsSync(appPath)).toBe(true);
+    expect(fs.existsSync(path.join(appPath, 'sentinel'))).toBe(true);
+
+    const allOutput = consoleOutput.join('\n');
+    expect(allOutput).toMatch(/\[X\].*Download or extraction failed/i);
+  });
+
+  it('extracted archive missing the bundle: old bundle still present, install aborts with diagnostics', async () => {
+    // If the archive extracts successfully but does not contain CCS Bar.app,
+    // the old install must be left untouched and [X] with file listing printed.
+    const appsDir = path.join(tempHome, 'Applications');
+    const appPath = path.join(appsDir, 'CCS Bar.app');
+    fs.mkdirSync(appPath, { recursive: true });
+    fs.writeFileSync(path.join(appPath, 'sentinel'), 'old-install');
+
+    let removeCalled = false;
+    const { handleBarInstall } = await loadInstallSubcommand();
+
+    await handleBarInstall([], {
+      ...baseDeps(appsDir),
+      removeExistingApp: (_path: unknown) => {
+        removeCalled = true;
+        fs.rmSync(String(_path), { recursive: true, force: true });
+      },
+      downloadAndExtract: async (_url: string, dest: string) => {
+        // Extraction produces a wrong-named artifact — CCS Bar.app absent.
+        fs.mkdirSync(dest, { recursive: true });
+        fs.writeFileSync(path.join(dest, 'WrongName.app'), 'dummy');
+      },
+    });
+
+    // removeExistingApp must NOT have been called.
+    expect(removeCalled).toBe(false);
+    // Old bundle still present.
+    expect(fs.existsSync(appPath)).toBe(true);
+
+    const allOutput = consoleOutput.join('\n');
+    expect(allOutput).toMatch(/\[X\]/);
+    expect(allOutput).toMatch(/CCS Bar\.app/);
+    // Diagnostics: should list staging contents
+    expect(allOutput).toMatch(/WrongName\.app/);
+  });
+
+  it('success path: staging dir removed, app present at appPath', async () => {
+    const appsDir = path.join(tempHome, 'Applications');
+    const appPath = path.join(appsDir, 'CCS Bar.app');
+    let stagingDirUsed = '';
+    const { handleBarInstall } = await loadInstallSubcommand();
+
+    await handleBarInstall([], {
+      ...baseDeps(appsDir),
+      downloadAndExtract: async (_url: string, dest: string) => {
+        stagingDirUsed = dest;
+        fs.mkdirSync(path.join(dest, 'CCS Bar.app'), { recursive: true });
+      },
+    });
+
+    // App at the final location.
+    expect(fs.existsSync(appPath)).toBe(true);
+    // Staging dir cleaned up.
+    if (stagingDirUsed) {
+      expect(fs.existsSync(stagingDirUsed)).toBe(false);
+    }
+
+    const allOutput = consoleOutput.join('\n');
+    expect(allOutput).toMatch(/\[OK\].*CCS Bar/);
+    expect(allOutput).not.toMatch(/\[X\]/);
+  });
+
+  it('aborts install and prints [X] when removeExistingApp throws (old app present = no swap)', async () => {
+    // removeExistingApp now runs AFTER staging verification.
+    // If it throws, the new bundle is still in staging (not yet moved); old app untouched.
     const appsDir = path.join(tempHome, 'Applications');
     const appPath = path.join(appsDir, 'CCS Bar.app');
     fs.mkdirSync(appPath, { recursive: true });
 
-    let downloadCalled = false;
     const { handleBarInstall } = await loadInstallSubcommand();
 
     await handleBarInstall([], {
@@ -2829,13 +2927,11 @@ describe('bar install: stale reinstall guard — removeExistingApp (review findi
       removeExistingApp: (_path: unknown) => {
         throw new Error('Permission denied');
       },
-      downloadAndExtract: async (_url: string, _dest: string) => {
+      downloadAndExtract: async (_url: string, dest: string) => {
-        downloadCalled = true;
+        fs.mkdirSync(path.join(dest, 'CCS Bar.app'), { recursive: true });
       },
     });
 
-    expect(downloadCalled).toBe(false);
-
     const allOutput = consoleOutput.join('\n');
     expect(allOutput).toMatch(/\[X\].*Could not remove.*CCS Bar\.app/);
   });