Make the bun"..." interpolator use the one npm-spec parser

millbun/integration/resources/invalid-bun-specifier/build.mill

@@ -0,0 +1,24 @@
+//| mill-version: 1.1.5
+//| mill-jvm-version: system
+//| mvnDeps:
+//| - com.tjclp::mill-bun_mill1:0.0.0-NIGHTLY
+
+package build
+
+import mill.*
+import mill.bun.bun
+import mill.scalajslib.*
+import mill.scalajslib.api.*
+import mill.scalajslib.bun.*
+
+object app extends BunScalaJSModule {
+  override def moduleDir = build.moduleDir
+  def scalaVersion = "3.8.2"
+  def scalaJSVersion = "1.22.0"
+
+  override def moduleKind = Task { ModuleKind.CommonJSModule }
+  override def sources = Task.Sources(moduleDir / "src")
+  // Empty *specifier*: the name half is valid, so the macro used to accept this and let
+  // parseDependency throw at task time instead.
+  override def bunDeps = Task { Seq(bun"react@") }
+}

millbun/integration/resources/invalid-bun-specifier/src/App.scala

@@ -0,0 +1 @@
+object App

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

@@ -27,6 +27,14 @@ object BunDependencyManifestIntegrationTests extends TestSuite {
       assert(!res.isSuccess)
     }
 
+    test("invalid bun specifier fails in build definitions") {
+      // The macro only checked the package-name half, so bun"react@" compiled cleanly and then
+      // threw from parseDependency during the install — defeating the interpolator's purpose.
+      val tester = this.tester("invalid-bun-specifier")
+      val res = tester.eval("app.bunDeps")
+      assert(!res.isSuccess)
+    }
+
     test("published dev-only modules do not emit runtime manifests") {
       val tester = this.tester("scalajs-dependency-manifests")
       val res = tester.eval("publishedDevOnlyLib.jar")

millbun/src/mill/bun/BunDep.scala

@@ -20,6 +20,18 @@ extension (sc: StringContext)
   inline def bun(inline args: Any*): String =
     ${ BunDepMacro.validateImpl('sc, 'args) }
 
+object BunDep:
+  /**
+   * Validate a dependency the macro could not check at compile time, returning it unchanged.
+   *
+   * Used for interpolated forms like `bun"react@$version"`, where the value only exists once the
+   * build evaluates. Failing here still beats failing inside `bun install`.
+   */
+  def validate(dep: String): String =
+    BunToolchainModule.parseDependency(dep) match
+      case Right(_)     => dep
+      case Left(message) => throw new IllegalArgumentException(s"Invalid bun dependency: $message")
+
 private object BunDepMacro:
   import scala.quoted.*
 
@@ -30,8 +42,9 @@ private object BunDepMacro:
         Expr(literal)
       case _ =>
         // Has interpolated parts, a non-literal StringContext, or runs inside another
-        // macro-generated context — build at runtime and skip compile-time validation.
-        '{ $sc.s($args*) }
+        // macro-generated context, so the value is not known at compile time. Validate when
+        // the build evaluates it instead, which still fails before any install runs.
+        '{ BunDep.validate($sc.s($args*)) }
 
   private def literalParts(sc: Expr[StringContext])(using Quotes): Option[Seq[String]] =
     import quotes.reflect.*
@@ -74,30 +87,15 @@ private object BunDepMacro:
 
     extractRepeatedArgs(args.asTerm.underlyingArgument).contains(Nil)
 
+  /**
+   * Validate against the same parser the build uses at task time.
+   *
+   * This deliberately delegates rather than re-deriving the rules: a second, weaker parser here
+   * meant `bun"react@"` compiled cleanly and then threw from `parseDependency` during the install,
+   * which is exactly what the interpolator exists to prevent.
+   */
   private def validateLiteral(dep: String)(using Quotes): Unit =
     import quotes.reflect.*
-    if dep.isEmpty then
-      report.errorAndAbort("bun dependency cannot be empty. Use bun\"package@version\" format.")
-    // Validate package name format
-    val name = if dep.startsWith("@") then
-      // Scoped: @scope/name or @scope/name@version
-      val afterScope = dep.drop(1)
-      if !afterScope.contains('/') then
-        report.errorAndAbort(
-          s"Invalid scoped package: '$dep'. Expected @scope/name or @scope/name@version"
-        )
-      val slashIdx = afterScope.indexOf('/')
-      val scopeName = afterScope.take(slashIdx)
-      if scopeName.isEmpty then
-        report.errorAndAbort(s"Invalid scoped package: '$dep'. Scope name is empty.")
-      val afterSlash = afterScope.drop(slashIdx + 1)
-      val nameOnly = if afterSlash.contains('@') then afterSlash.take(afterSlash.indexOf('@')) else afterSlash
-      if nameOnly.isEmpty then
-        report.errorAndAbort(s"Invalid scoped package: '$dep'. Package name is empty after scope.")
-      dep
-    else
-      // Unscoped: name or name@version
-      val nameOnly = if dep.contains('@') then dep.take(dep.indexOf('@')) else dep
-      if nameOnly.isEmpty then
-        report.errorAndAbort(s"Invalid package: '$dep'. Package name cannot be empty.")
-      dep
+    BunToolchainModule.parseDependency(dep).left.foreach { message =>
+      report.errorAndAbort(s"Invalid bun dependency: $message")
+    }

millbun/src/mill/bun/BunVendoredNodeModules.scala

@@ -44,9 +44,19 @@ object BunVendoredNodeModules:
           FileVisitResult.CONTINUE
     )
 
-  /** Merge vendored node_modules from a classpath entry into a destination. */
+  /** Merge vendored node_modules from a classpath entry into a destination.
+    *
+    * Refuses a symlinked destination: in workspace mode `node_modules` is a link into the shared
+    * workspace install's `Task.dest`, and merging through it would silently mutate another task's
+    * output — shared by every other member of the workspace.
+    */
   def mergeFromClasspathEntry(entry: os.Path, destNodeModules: os.Path): Boolean =
-    if os.isDir(entry) then mergeFromDir(entry, destNodeModules, entry.toString)
+    if os.isLink(destNodeModules) then
+      throw new RuntimeException(
+        s"Refusing to merge vendored Bun runtime into $destNodeModules: it is a symlink to " +
+          s"${os.readLink.absolute(destNodeModules)}, which belongs to another task."
+      )
+    else if os.isDir(entry) then mergeFromDir(entry, destNodeModules, entry.toString)
     else if os.exists(entry) && entry.ext == "jar" then mergeFromJar(entry, destNodeModules)
     else false
 

millbun/src/mill/scalajslib/bun/BunScalaJSModule.scala

@@ -254,8 +254,21 @@ trait BunScalaJSModule extends ScalaJSModule with BunToolchainModule with BunPac
         transitiveUnmanagedDeps().nonEmpty ||
         bunPackageJsonExtras().value.nonEmpty
 
+    val ownResourceRoots = resources().map(_.path).toSet
+    val vendoredEntries = runClasspath().map(_.path).filterNot(ownResourceRoots.contains)
+
     bunWorkspaceInstall() match
       case Some(workspaceInstall) =>
+        // Vendored trees must not be merged here: node_modules is a link into the workspace
+        // install's dest, so merging would mutate a directory shared by every workspace member.
+        val vendored = vendoredEntries.filter(BunVendoredNodeModules.hasVendoredNodeModules)
+        if vendored.nonEmpty then
+          Task.fail(
+            s"Bun workspace members cannot consume vendored runtime dependencies: " +
+              s"${vendored.map(_.last).mkString(", ")}. Depend on the manifest-only artifact, or " +
+              "install this module outside the workspace."
+          )
+
         val installed = workspaceInstall.path
         if os.exists(installed / "node_modules") then
           os.symlink(dest / "node_modules", installed / "node_modules")
@@ -281,9 +294,7 @@ trait BunScalaJSModule extends ScalaJSModule with BunToolchainModule with BunPac
             env = bunEnv()
           )
 
-    val ownResourceRoots = resources().map(_.path).toSet
-    val vendoredEntries = runClasspath().map(_.path).filterNot(ownResourceRoots.contains)
-    mergeVendoredNodeModules(vendoredEntries, dest / "node_modules")
+        mergeVendoredNodeModules(vendoredEntries, dest / "node_modules")
 
     PathRef(dest)
   }

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

@@ -50,7 +50,32 @@ object BunDepTests extends TestSuite {
       assert(deps.head.startsWith("@anthropic-ai"))
     }
 
-    // Invalid literal coverage lives in integration tests so the interpolator
+    test("the interpolator and the task-time parser accept the same inputs") {
+      // The macro used to run its own weaker parser, so bun"react@" compiled and then threw
+      // during the install. Both must now agree, in both directions.
+      Seq("react@^19.0.0", "@types/node", "zod", "lodash@~4.17.0", "@scope/pkg@1.0.0").foreach {
+        valid =>
+          assert(BunDep.validate(valid) == valid)
+          assert(BunToolchainModule.parseDependency(valid).isRight)
+      }
+
+      Seq("", "react@", "@types", "@types/bun@", "@/pkg").foreach { invalid =>
+        val error = intercept[IllegalArgumentException](BunDep.validate(invalid))
+        assert(error.getMessage.contains("Invalid bun dependency"))
+        assert(BunToolchainModule.parseDependency(invalid).isLeft)
+      }
+    }
+
+    test("interpolated forms are validated when the build evaluates them") {
+      // Not knowable at compile time, so this is the runtime backstop.
+      val version = "^19.0.0"
+      assert(bun"react@$version" == "react@^19.0.0")
+
+      val empty = ""
+      intercept[IllegalArgumentException](bun"react@$empty")
+    }
+
+    // Invalid *literal* coverage lives in integration tests so the interpolator
     // is compiled in a normal build.mill context rather than inside another macro.
   }
 }

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

@@ -100,6 +100,25 @@ object BunVendoredNodeModulesTests extends TestSuite {
 
       assert(err.getMessage.contains("Vendored Bun bundle conflict"))
     }
+
+    test("merging through a symlinked destination is refused") {
+      // In workspace mode node_modules links into the shared install's Task.dest. Merging
+      // through it would silently mutate a directory every workspace member depends on.
+      val source = os.temp.dir()
+      writeVendoredPackage(source, "react", "19.1.1")
+
+      val shared = os.temp.dir() / "shared-node-modules"
+      os.makeDir.all(shared)
+      val member = os.temp.dir()
+      os.symlink(member / "node_modules", shared)
+
+      val err = intercept[RuntimeException] {
+        BunVendoredNodeModules.mergeFromClasspathEntry(source, member / "node_modules")
+      }
+      assert(err.getMessage.contains("belongs to another task"))
+      // The critical assertion: nothing leaked into the shared directory.
+      assert(os.list(shared).isEmpty)
+    }
   }
 
   private def writeVendoredPackage(root: os.Path, name: String, version: String): Unit = {