Harden the shared Bun download cache against eviction and torn publishes

.github/workflows/ci.yml

@@ -3,7 +3,6 @@ on:
   push:
     branches: [main]
   pull_request:
-    branches: [main]
 
 jobs:
   build:
@@ -29,7 +28,12 @@ jobs:
       - name: Unit Tests
         run: ./mill --no-server millbun.test
 
+      # The suite defaults to the managed toolchain; this job pins system Bun to the same
+      # version so `findOnPath` resolution and version verification keep CI coverage, while
+      # the managed-bun job covers the download/cache path.
       - name: Integration Tests
+        env:
+          MILL_BUN_USE_SYSTEM: 'true'
         run: ./mill --no-server millbun.integration
 
   managed-bun:

millbun/integration/src/mill/bun/BunManagedToolchainIntegrationTests.scala

@@ -20,3 +20,29 @@ object BunManagedToolchainIntegrationTests extends TestSuite:
       val executable = tester.out("app.bunExecutable").value[String]
       val version = os.proc(executable, "--version").call(stdout = os.Pipe).out.text().trim
       assert(version == "1.4.0")
+
+    test("an evicted download cache is repopulated, not trusted"):
+      val cacheDir = os.temp.dir()
+      val env = Map("MILL_BUN_CACHE_DIR" -> cacheDir.toString)
+      // Forked evals, not in-memory: `Task.env` reads the Mill process's own environment, so a
+      // per-eval env override only reaches a subprocess.
+      val tester = new IntegrationTester(
+        daemonMode = false,
+        workspaceSourcePath = resourceDir / "managed-bun",
+        millExecutable = millExe,
+        useInMemory = false
+      )
+      assert(tester.eval("app.bunExecutable", env = env).isSuccess)
+      // Proves MILL_BUN_CACHE_DIR reached the build before the eviction step relies on it.
+      assert(os.walk(cacheDir).exists(p => os.isFile(p)))
+
+      // Users legitimately evict the cache directory; the build must recover on its own
+      // rather than trust a stale task result pointing at a file that no longer exists.
+      os.remove.all(cacheDir)
+      os.makeDir.all(cacheDir)
+
+      assert(tester.eval("app.bunExecutable", env = env).isSuccess)
+      val executable = tester.out("app.bunExecutable").value[String]
+      assert(os.isFile(os.Path(executable)))
+      val version = os.proc(executable, "--version").call(stdout = os.Pipe).out.text().trim
+      assert(version == "1.4.0")

millbun/integration/src/mill/bun/BunTypeScriptIntegrationTests.scala

@@ -243,6 +243,10 @@ object BunTypeScriptIntegrationTests extends TestSuite {
       val tester = this.tester("typescript-tests")
       assert(tester.eval("app.test.npmInstall").isSuccess)
       assert(!os.exists(tester.workspacePath / "test" / "bun.lock"))
+      // And must actually reuse the outer install — not run a second one that merely succeeds
+      // because the lockfile requirement happens to be off in this suite.
+      val installPath = outputPath(tester, "app.test.npmInstall")
+      assert(installPath == tester.workspacePath / "out" / "app" / "npmInstall.dest")
     }
 
     test("bunEnv") {

millbun/src/mill/bun/BunToolchainModule.scala

@@ -206,13 +206,46 @@ object BunToolchainModule {
       )
       cached
     catch
-      case _: java.nio.file.FileAlreadyExistsException => cached
-      case _: java.nio.file.AtomicMoveNotSupportedException =>
-        // Cache on a different filesystem than the task dest: fall back to a plain copy.
-        os.copy.over(staged, cached, createFolders = true)
+      case _: java.nio.file.AtomicMoveNotSupportedException => publishViaCopy(staged, cached)
+      case scala.util.control.NonFatal(_) if os.exists(cached) =>
+        // Lost the publish race. The path is keyed by the verified checksum, so the winner's
+        // bytes are the right bytes. Windows reports this as a sharing violation rather than
+        // FileAlreadyExistsException, so match on the outcome, not the exception type.
         cached
   }
 
+  /**
+   * Publish across filesystems by staging a copy next to the final path, so the last hop is a
+   * same-filesystem atomic rename. Copying straight to the published name would let a concurrent
+   * reader execute a partially written binary — and a build killed mid-copy would leave a
+   * truncated file that every later build trusts, because the path is the checksum.
+   */
+  private[bun] def publishViaCopy(staged: os.Path, cached: os.Path): os.Path = {
+    val tmp = cached / os.up /
+      s"${cached.last}.tmp-${ProcessHandle.current().pid()}-${System.nanoTime()}"
+    os.copy(staged, tmp, copyAttributes = true, createFolders = true)
+    try
+      java.nio.file.Files.move(
+        tmp.toNIO,
+        cached.toNIO,
+        java.nio.file.StandardCopyOption.ATOMIC_MOVE
+      )
+    catch
+      case scala.util.control.NonFatal(_) if os.exists(cached) => os.remove.all(tmp)
+    cached
+  }
+
+  /**
+   * A `PathRef` into the shared download cache.
+   *
+   * The file lives outside every `Task.dest`, so Mill must re-check it each evaluation: entries
+   * are immutable but evictable, and without revalidation an evicted executable is never
+   * re-downloaded — every bun invocation fails until the user guesses the right `clean`. Quick
+   * (mtime + size) signatures keep the per-evaluation cost to a stat.
+   */
+  private[bun] def cachedExecutableRef(cached: os.Path): PathRef =
+    PathRef(cached, quick = true).withRevalidate(PathRef.Revalidate.Always)
+
   /** Parse package.json-style `name@specifier` declarations without slicing scoped names incorrectly. */
   private[bun] def parseDependency(input: String): Either[String, NpmDependency] = {
     val trimmed = input.trim
@@ -323,7 +356,10 @@ object BunToolchainModule {
       if (os.isLink(path)) {
         os.makeDir.all(target / os.up)
         if (os.exists(target, followLinks = false)) os.remove(target)
-        os.symlink(target, os.readLink.absolute(path))
+        // Preserve the raw target: absolutizing a tree-internal relative link (bun's
+        // node_modules/.bin entries) would point the copy back into the source tree, so the
+        // copy stops being self-contained the moment the source is cleaned or relocated.
+        os.symlink(target, os.readLink(path))
       } else if (os.isDir(path, followLinks = false)) {
         os.makeDir.all(target)
       } else {
@@ -416,7 +452,8 @@ trait BunToolchainModule extends Module {
    * Lives outside the workspace, so Mill's filesystem checker does not restrict it.
    */
   def bunDownloadCacheDir: T[os.Path] = Task.Input {
-    Task.env.get("MILL_BUN_CACHE_DIR").filter(_.nonEmpty).map(os.Path(_))
+    Task.env.get("MILL_BUN_CACHE_DIR").filter(_.nonEmpty)
+      .map(os.Path(_, BuildCtx.workspaceRoot))
       .getOrElse(os.home / ".cache" / "mill-bun")
   }
 
@@ -560,7 +597,7 @@ trait BunToolchainModule extends Module {
 
     // Keyed by the verified checksum, so a cache hit is proof of the right bytes.
     val cached = bunDownloadCacheDir() / checksum / dist.executableName
-    if (os.exists(cached)) PathRef(cached)
+    if (os.exists(cached)) BunToolchainModule.cachedExecutableRef(cached)
     else {
       val archive = Task.dest / dist.assetName
       val staged = Task.dest / dist.executableName
@@ -570,7 +607,7 @@ trait BunToolchainModule extends Module {
         Task.fail(s"Bun archive checksum mismatch for $url: expected $checksum, received $actual")
       }
       BunToolchainModule.extractExecutable(archive, dist.executableName, staged)
-      PathRef(BunToolchainModule.publishToCache(staged, cached))
+      BunToolchainModule.cachedExecutableRef(BunToolchainModule.publishToCache(staged, cached))
     }
   }
 

millbun/src/mill/javascriptlib/bun/BunTypeScriptModule.scala

@@ -409,6 +409,18 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with
    */
   trait BunTypeScriptTests extends TypeScriptTests {
 
+    /**
+     * The outer module's Bun-specific TS toolchain, not upstream Mill's ts-node defaults.
+     *
+     * This trait extends upstream `TypeScriptTests`, so an unqualified `tsDeps()` resolves to
+     * the Node toolchain (`ts-node`, `tsconfig-paths`, `@types/node`) that the outer trait
+     * deliberately replaced. Those names always survived the outer-name filter in
+     * [[bunTestPackageJson]], so a bare test module's package.json never matched the outer's
+     * and the install-reuse path in [[npmInstall]] was unreachable — with `bunRequireLockfile`
+     * on, every bare test module demanded its own lockfile.
+     */
+    override def tsDeps: T[Seq[String]] = Task { outer.tsDeps() }
+
     /** Test timeout in milliseconds. 0 means no timeout. */
     def bunTestTimeout: T[Int] = Task { 0 }
 

millbun/src/mill/javascriptlib/bun/BunTypeScriptWebModule.scala

@@ -74,7 +74,11 @@ trait BunTypeScriptWebModule extends BunTypeScriptModule:
 
   /** Start Bun's HTML development server with source mirroring for native HMR. */
   def dev(): Command[Unit] = Task.Command {
-    val stage = webStage().path
+    // Serve from a private copy: the sync thread mirrors live edits (but never deletions) into
+    // the serving root, and `bundle` builds from the same cached stage — mutating it in place
+    // would let a file created and deleted during a dev session ship in the production bundle.
+    val stage = Task.dest / "stage"
+    mill.bun.BunToolchainModule.copyTree(webStage().path, stage)
     val entries = BunWebSupport.htmlEntries(webEntryPoints(), moduleDir, stage)
     val syncRoots = (sources() ++ generatedSources() ++ resources() ++ webEntryPoints() ++ webPublicSources())
       .filter(ref => os.exists(ref.path) && ref.path.startsWith(moduleDir))

millbun/test/src/mill/bun/BunToolchainTests.scala

@@ -126,6 +126,31 @@ object BunToolchainTests extends TestSuite:
       assert(BunToolchainModule.publishToCache(second, cached) == cached)
       assert(os.read(cached) == "bun-binary")
 
+    test("cross-filesystem publish preserves permissions and leaves no temp debris"):
+      val root = os.temp.dir()
+      val cached = root / "cache" / "def456" / "bun"
+      val staged = root / "staged" / "bun"
+      os.write(staged, "bun-binary", createFolders = true)
+      os.perms.set(staged, "rwxr-xr-x")
+
+      assert(BunToolchainModule.publishViaCopy(staged, cached) == cached)
+      assert(os.read(cached) == "bun-binary")
+      assert(os.perms(cached).toString == "rwxr-xr-x")
+      // The bytes must travel under a temp name and arrive by rename: a crash mid-publish can
+      // never leave a partial file at the published path, and success leaves nothing behind.
+      assert(os.list(cached / os.up) == Seq(cached))
+
+    test("cross-filesystem publish tolerates losing the race"):
+      val root = os.temp.dir()
+      val cached = root / "cache" / "0123" / "bun"
+      os.write(cached, "bun-binary", createFolders = true)
+      val staged = root / "staged" / "bun"
+      os.write(staged, "bun-binary", createFolders = true)
+
+      assert(BunToolchainModule.publishViaCopy(staged, cached) == cached)
+      assert(os.read(cached) == "bun-binary")
+      assert(os.list(cached / os.up) == Seq(cached))
+
     test("computes SHA-256"):
       val file = os.temp(contents = "hello")
       assert(

millbun/test/src/mill/bun/CopyTreeTests.scala

@@ -56,6 +56,21 @@ object CopyTreeTests extends TestSuite:
       assert(os.isLink(dest / "alias.js"))
       assert(os.read(dest / "alias.js") == "real")
 
+    test("relative symlink targets are preserved verbatim"):
+      // bun's node_modules/.bin entries are relative links; absolutizing them would point the
+      // copy back into the source tree, dangling as soon as the source task is cleaned.
+      val source = os.temp.dir()
+      os.write(source / "esbuild" / "bin" / "esbuild", "#!/usr/bin/env node", createFolders = true)
+      os.makeDir.all(source / ".bin")
+      os.symlink(source / ".bin" / "esbuild", os.RelPath("../esbuild/bin/esbuild"))
+
+      val dest = os.temp.dir()
+      BunToolchainModule.copyTree(source, dest)
+      assert(os.readLink(dest / ".bin" / "esbuild") == os.RelPath("../esbuild/bin/esbuild"))
+      // Self-contained: the copied link resolves inside the copy even after the source vanishes.
+      os.remove.all(source)
+      assert(os.read(dest / ".bin" / "esbuild") == "#!/usr/bin/env node")
+
     test("nested directories and empty directories are preserved"):
       val source = os.temp.dir()
       os.makeDir.all(source / "empty")