fix(node): enforce Frappe-compatible Node version before asset build

bench_cli/managers/python_env_manager.py

@@ -18,6 +18,26 @@
 
 _BUNDLE_RE = re.compile(r"^(.+)\.bundle\.[A-Z0-9]{8}\.(js|css)$")
 
+# Frappe's asset build refuses to run on an unsupported Node: v15 needs >= 18,
+# v16/develop need >= 24. We target the highest so one bench can build either.
+# Keep in sync with the nodesource channel in `_install_node_linux`.
+REQUIRED_NODE_MAJOR = 24
+
+
+def _installed_node_major() -> int | None:
+    """Major version of the ``node`` on PATH, or None if absent/unparseable."""
+    import subprocess
+
+    node = which("node")
+    if not node:
+        return None
+    try:
+        result = subprocess.run([node, "--version"], capture_output=True, text=True)
+    except OSError:
+        return None
+    match = re.match(r"v?(\d+)\.", result.stdout.strip())
+    return int(match.group(1)) if match else None
+
 
 class PythonEnvManager:
     def __init__(self, bench: "Bench") -> None:
@@ -113,17 +133,49 @@ def uninstall_app(self, app_name: str) -> None:
         run_command([uv, "pip", "uninstall", "--python", python, app_name], stream_output=True)
 
     def install_node(self) -> None:
-        if not which("node"):
-            if is_macos():
-                run_command(["brew", "install", "node"])
-            elif is_alpine():
-                # nodesource ships no musl builds; use Alpine's own packages.
-                get_package_manager().install("nodejs", "npm")
-            else:
-                self._install_node_linux()
+        """Ensure a Frappe-compatible Node.js (and yarn) are available.
+
+        A stale Node already on PATH is the common failure mode: the build then
+        dies deep inside yarn with an opaque "engine is incompatible" error. So
+        we (re)install when Node is missing *or* older than
+        ``REQUIRED_NODE_MAJOR``, and fail fast with an actionable message if a
+        supported Node still isn't reachable afterwards. Cheap to call
+        repeatedly — a single ``node --version`` check when already satisfied.
+        """
+        if (_installed_node_major() or 0) < REQUIRED_NODE_MAJOR:
+            self._install_node()
+            major = _installed_node_major()
+            if major is None:
+                raise BenchError(
+                    f"Node.js >= {REQUIRED_NODE_MAJOR} is required to build assets, but Node "
+                    "could not be installed automatically. Install it and re-run."
+                )
+            if major < REQUIRED_NODE_MAJOR:
+                raise BenchError(
+                    f"Node.js {major} is on PATH but Frappe needs >= {REQUIRED_NODE_MAJOR} to "
+                    "build assets. Upgrade Node (e.g. `brew upgrade node`, or via nvm/fnm) so a "
+                    "newer `node` comes first on PATH, then re-run."
+                )
         if not which("yarn"):
             self._install_yarn()
 
+    def _install_node(self) -> None:
+        if is_macos():
+            # `brew install` is a no-op when an older keg is already linked, so
+            # follow up with an upgrade; ignore failure when Node isn't managed
+            # by Homebrew (the version check then surfaces a clear error).
+            run_command(["brew", "install", "node"])
+            if (_installed_node_major() or 0) < REQUIRED_NODE_MAJOR:
+                try:
+                    run_command(["brew", "upgrade", "node"])
+                except BenchError:
+                    pass
+        elif is_alpine():
+            # nodesource ships no musl builds; use Alpine's own packages.
+            get_package_manager().install("nodejs", "npm")
+        else:
+            self._install_node_linux()
+
     def _install_yarn(self) -> None:
         if is_macos():
             run_command(["npm", "install", "-g", "yarn"])
@@ -142,6 +194,7 @@ def install_node_dependencies(self) -> None:
                 )
 
     def build_assets(self) -> None:
+        self.install_node()
         run_command(
             [*self.bench.frappe_call, "frappe", "build", "--force"],
             cwd=self.bench.sites_path,
@@ -160,6 +213,9 @@ def build_assets_for_app(self, app: "App") -> None:
             self._setup_prebuilt_assets(app.config.name, app_public_dir, dist_dir)
             return
 
+        # Built from source past this point — ensure a supported Node toolchain.
+        self.install_node()
+
         if (app.path / "package.json").exists():
             print(f"  Installing JS dependencies for {app.config.name}...")
             sys.stdout.flush()

tests/test_commands.py

@@ -827,3 +827,64 @@ def test_orchestrator_rollback_stops_mariadb_and_sets_maintenance() -> None:
     mariadb.start.assert_called_once()
     volume.rollback_snapshot.assert_called_once_with("bench-pool/shop", "tag1")
     assert bench.set_maintenance_mode.call_args_list == [call(True), call(False)]
+
+
+# ── Node.js version enforcement ───────────────────────────────────────────────
+
+
+def test_install_node_skips_when_version_satisfies(tmp_path: Path) -> None:
+    """A new-enough Node already on PATH must not trigger a reinstall."""
+    from bench_cli.managers.python_env_manager import PythonEnvManager, REQUIRED_NODE_MAJOR
+
+    mgr = PythonEnvManager(make_bench(tmp_path))
+    with patch("bench_cli.managers.python_env_manager._installed_node_major", return_value=REQUIRED_NODE_MAJOR), \
+         patch("bench_cli.managers.python_env_manager.which", return_value="/usr/bin/yarn"), \
+         patch.object(PythonEnvManager, "_install_node") as mock_install:
+        mgr.install_node()
+        mock_install.assert_not_called()
+
+
+def test_install_node_reinstalls_when_too_old(tmp_path: Path) -> None:
+    """A stale Node (e.g. 14) must be upgraded before the build runs."""
+    from bench_cli.managers.python_env_manager import PythonEnvManager, REQUIRED_NODE_MAJOR
+
+    mgr = PythonEnvManager(make_bench(tmp_path))
+    with patch("bench_cli.managers.python_env_manager._installed_node_major",
+               side_effect=[14, REQUIRED_NODE_MAJOR]), \
+         patch("bench_cli.managers.python_env_manager.which", return_value="/usr/bin/yarn"), \
+         patch.object(PythonEnvManager, "_install_node") as mock_install:
+        mgr.install_node()
+        mock_install.assert_called_once()
+
+
+def test_install_node_raises_when_still_too_old(tmp_path: Path) -> None:
+    """If a supported Node still isn't reachable after install, fail with a
+    clear, actionable error instead of a cryptic yarn engine failure."""
+    from bench_cli.managers.python_env_manager import PythonEnvManager, REQUIRED_NODE_MAJOR
+
+    mgr = PythonEnvManager(make_bench(tmp_path))
+    with patch("bench_cli.managers.python_env_manager._installed_node_major", side_effect=[14, 14]), \
+         patch.object(PythonEnvManager, "_install_node"):
+        with pytest.raises(BenchError, match=rf">= {REQUIRED_NODE_MAJOR}"):
+            mgr.install_node()
+
+
+def test_install_node_raises_when_missing_after_install(tmp_path: Path) -> None:
+    from bench_cli.managers.python_env_manager import PythonEnvManager
+
+    mgr = PythonEnvManager(make_bench(tmp_path))
+    with patch("bench_cli.managers.python_env_manager._installed_node_major", side_effect=[None, None]), \
+         patch.object(PythonEnvManager, "_install_node"):
+        with pytest.raises(BenchError, match="could not be installed"):
+            mgr.install_node()
+
+
+def test_node_requirement_matches_nodesource_channel() -> None:
+    """The Linux installer's nodesource channel must track REQUIRED_NODE_MAJOR
+    so missing-Node installs land on a supported version."""
+    import inspect
+
+    from bench_cli.managers.python_env_manager import PythonEnvManager, REQUIRED_NODE_MAJOR
+
+    source = inspect.getsource(PythonEnvManager._install_node_linux)
+    assert f"setup_{REQUIRED_NODE_MAJOR}.x" in source