fix(launcher): address review feedback on setup and runtime cleanup

scripts/package-bun-source.mjs

@@ -207,7 +207,7 @@ async function run(command, args, options = {}) {
   });
 }
 
-async function downloadArchive(source, cacheDir, fetchImpl) {
+export async function downloadArchive(source, cacheDir, fetchImpl) {
   const archivePath = path.join(cacheDir, `${source.name}-${source.revision}.tar.gz`);
   try {
     if ((await sha256File(archivePath)) === source.sha256) return archivePath;
@@ -218,22 +218,26 @@ async function downloadArchive(source, cacheDir, fetchImpl) {
 
   const url = `https://codeload.github.com/${source.repository}/tar.gz/${source.revision}`;
   const temporaryPath = `${archivePath}.download-${randomUUID()}`;
-  const response = await fetchImpl(url);
-  if (!response.ok || !response.body)
-    throw new Error(`Unable to download ${url}: HTTP ${response.status}`);
-  await pipeline(
-    Readable.fromWeb(response.body),
-    createWriteStream(temporaryPath, { flags: 'wx' }),
-  );
-  const actualSha256 = await sha256File(temporaryPath);
-  if (actualSha256 !== source.sha256) {
-    await fs.rm(temporaryPath, { force: true });
-    throw new Error(
-      `Source checksum mismatch for ${source.name}: expected ${source.sha256}, received ${actualSha256}.`,
+  try {
+    const response = await fetchImpl(url);
+    if (!response.ok || !response.body)
+      throw new Error(`Unable to download ${url}: HTTP ${response.status}`);
+    await pipeline(
+      Readable.fromWeb(response.body),
+      createWriteStream(temporaryPath, { flags: 'wx' }),
     );
+    const actualSha256 = await sha256File(temporaryPath);
+    if (actualSha256 !== source.sha256) {
+      throw new Error(
+        `Source checksum mismatch for ${source.name}: expected ${source.sha256}, received ${actualSha256}.`,
+      );
+    }
+    await fs.rename(temporaryPath, archivePath);
+    return archivePath;
+  } finally {
+    // Cleanup must not replace the original download, validation, or rename error.
+    await fs.rm(temporaryPath, { force: true }).catch(() => {});
   }
-  await fs.rename(temporaryPath, archivePath);
-  return archivePath;
 }
 
 async function materializeArchive(source, root, cacheDir, fetchImpl) {

scripts/package-bun-source.test.ts

@@ -1,7 +1,10 @@
 import { describe, expect, test } from 'bun:test';
+import { createHash } from 'node:crypto';
 import fs from 'node:fs/promises';
+import os from 'node:os';
 import path from 'node:path';
 import {
+  downloadArchive,
   parseRegisteredRepositories,
   parseSourceManifest,
   validateBunPins,
@@ -11,6 +14,31 @@ import {
 const projectRoot = path.resolve(import.meta.dir, '..');
 
 describe('Bun corresponding-source manifest', () => {
+  test('removes partial downloads when placing a valid archive fails', async () => {
+    const cacheDir = await fs.mkdtemp(path.join(os.tmpdir(), 'subminer-bun-source-test-'));
+    try {
+      const payload = new Uint8Array([1, 2, 3]);
+      const source = {
+        name: 'fixture',
+        repository: 'example/fixture',
+        revision: 'a'.repeat(40),
+        sha256: createHash('sha256').update(payload).digest('hex'),
+      };
+      const archivePath = path.join(cacheDir, `${source.name}-${source.revision}.tar.gz`);
+      await expect(
+        downloadArchive(source, cacheDir, async () => {
+          await fs.mkdir(archivePath);
+          return { ok: true, status: 200, body: new Response(payload).body };
+        }),
+      ).rejects.toMatchObject({ syscall: 'rename' });
+
+      const entries = await fs.readdir(cacheDir);
+      expect(entries.filter((entry) => entry.includes('.download-'))).toEqual([]);
+    } finally {
+      await fs.rm(cacheDir, { recursive: true, force: true });
+    }
+  });
+
   test('pins the source to the revision reported by the distributed binary', async () => {
     const manifest = parseSourceManifest(
       JSON.parse(

src/main.ts

@@ -1523,10 +1523,7 @@ const firstRunSetupService = createFirstRunSetupService({
   },
   installCommandLineLauncher: async () => {
     const snapshot = await installCommandLineLauncher(createCommandLineLauncherRuntimeOptions());
-    const ok =
-      snapshot.status === 'ready' ||
-      snapshot.status === 'installed_bun_missing' ||
-      snapshot.status === 'not_on_path';
+    const ok = snapshot.status === 'ready' || snapshot.status === 'not_on_path';
     return {
       ok,
       installPath: snapshot.installPath,

src/main/runtime/managed-launcher.test.ts

@@ -305,3 +305,33 @@ test('Windows stages a new runtime version while the prior Bun executable is run
   assert.equal(fs.existsSync(path.dirname(first.bunPath)), false);
   assert.ok(fs.existsSync(expectedSecond.bunPath));
 });
+
+test('Windows cleanup removes an obsolete runtime with no Bun executable', (t) => {
+  if (process.platform !== 'win32') return;
+  const root = workspace(t);
+  const current = windowsManagedRuntimePaths({
+    platform: 'win32',
+    localAppData: root,
+    appVersion: '2.0.0',
+  });
+  const obsolete = windowsManagedRuntimePaths({
+    platform: 'win32',
+    localAppData: root,
+    appVersion: '1.0.0',
+  });
+  fs.mkdirSync(path.join(path.dirname(obsolete.bunPath), 'licenses'), { recursive: true });
+  fs.writeFileSync(
+    path.join(path.dirname(obsolete.bunPath), 'licenses', 'Bun-LICENSE.md'),
+    'license',
+  );
+  fs.mkdirSync(path.dirname(current.bunPath), { recursive: true });
+
+  cleanupOldWindowsManagedRuntimes({
+    platform: 'win32',
+    localAppData: root,
+    appVersion: '2.0.0',
+  });
+
+  assert.equal(fs.existsSync(path.dirname(obsolete.bunPath)), false);
+  assert.ok(fs.existsSync(path.dirname(current.bunPath)));
+});

src/main/runtime/managed-launcher.ts

@@ -101,7 +101,7 @@ export function cleanupOldWindowsManagedRuntimes(
       if (!entry.isDirectory() || entry.name === path.win32.basename(paths.directory)) continue;
       const oldDirectory = path.win32.join(paths.rootDirectory, entry.name);
       try {
-        fs.rmSync(path.win32.join(oldDirectory, 'bun.exe'));
+        fs.rmSync(path.win32.join(oldDirectory, 'bun.exe'), { force: true });
       } catch {
         continue;
       }

src/main/runtime/update/launcher-updater.test.ts

@@ -160,7 +160,7 @@ test('app-managed wrappers are never overwritten by the standalone release scrip
 });
 
 test('GUI updates defer recognized standalone launcher migration to app startup', async () => {
-  let accessed = false;
+  const accessed: string[] = [];
   let downloaded = false;
   const result = await updateLauncherAtPath({
     launcherPath: '/home/tester/.local/bin/subminer',
@@ -174,8 +174,8 @@ test('GUI updates defer recognized standalone launcher migration to app startup'
     fs: {
       stat: async () => ({ isFile: () => true }),
       readFile: async () => Buffer.from('#!/bin/sh\n# SubMiner launcher\n'),
-      access: async () => {
-        accessed = true;
+      access: async (targetPath) => {
+        accessed.push(targetPath);
       },
       writeFile: async () => {},
       chmod: async () => {},
@@ -189,7 +189,7 @@ test('GUI updates defer recognized standalone launcher migration to app startup'
     path: '/home/tester/.local/bin/subminer',
     message: 'Launcher migration is deferred until the updated SubMiner app starts.',
   });
-  assert.equal(accessed, false);
+  assert.deepEqual(accessed, ['/home/tester/.local/bin/subminer', '/home/tester/.local/bin']);
   assert.equal(downloaded, false);
 });
 
@@ -213,9 +213,7 @@ test('release launcher updater propagates GUI migration deferral', async () => {
     fs: {
       stat: async () => ({ isFile: () => true }),
       readFile: async () => Buffer.from('#!/bin/sh\n# SubMiner launcher\n'),
-      access: async () => {
-        throw new Error('must not check writability before app startup');
-      },
+      access: async () => {},
       writeFile: async () => {},
       chmod: async () => {},
       rename: async () => {},
@@ -227,3 +225,36 @@ test('release launcher updater propagates GUI migration deferral', async () => {
   assert.match(result.message ?? '', /deferred until the updated SubMiner app starts/);
   assert.equal(downloaded, false);
 });
+
+test('GUI migration reports a protected launcher when its file or parent is not writable', async () => {
+  const launcherPath = '/usr/local/bin/subminer';
+  for (const protectedPath of [launcherPath, '/usr/local/bin']) {
+    const result = await updateLauncherAtPath({
+      launcherPath,
+      assetUrl: 'https://example.test/subminer',
+      expectedSha256: launcherHash,
+      deferRecognizedLauncherUpdate: true,
+      download: async () => {
+        throw new Error('Protected launchers must not download a replacement.');
+      },
+      fs: {
+        stat: async () => ({ isFile: () => true }),
+        readFile: async () => Buffer.from('#!/usr/bin/env bun\n// SubMiner launcher\n'),
+        access: async (targetPath) => {
+          if (targetPath === protectedPath) throw new Error('EACCES');
+        },
+        writeFile: async () => {
+          throw new Error('Protected launchers must not be written.');
+        },
+        chmod: async () => {},
+        rename: async () => {},
+        unlink: async () => {},
+      },
+    });
+    assert.equal(result.status, 'protected', protectedPath);
+    assert.equal(
+      result.command,
+      buildProtectedLauncherUpdateCommand('https://example.test/subminer', launcherPath),
+    );
+  }
+});

src/main/runtime/update/launcher-updater.ts

@@ -112,16 +112,9 @@ export async function updateLauncherAtPath(options: {
       message: 'Existing executable does not look like a SubMiner launcher.',
     };
   }
-  if (options.deferRecognizedLauncherUpdate) {
-    return {
-      status: 'skipped',
-      path: options.launcherPath,
-      message: 'Launcher migration is deferred until the updated SubMiner app starts.',
-    };
-  }
-
   try {
     await fsDeps.access(options.launcherPath);
+    await fsDeps.access(path.dirname(options.launcherPath));
   } catch {
     return {
       status: 'protected',
@@ -130,6 +123,14 @@ export async function updateLauncherAtPath(options: {
     };
   }
 
+  if (options.deferRecognizedLauncherUpdate) {
+    return {
+      status: 'skipped',
+      path: options.launcherPath,
+      message: 'Launcher migration is deferred until the updated SubMiner app starts.',
+    };
+  }
+
   const data = await options.download();
   const actualSha256 = sha256(data);
   if (actualSha256 !== options.expectedSha256.toLowerCase()) {