fix: keep the //nodejs interop rewrite to CommonJS packages only

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014bq66fSmaPvLtjnDPfMYcj

backend/tests/bun_jobs.rs

@@ -1284,7 +1284,8 @@ fn test_generate_bun_bundle_propagates_exit_status() {
 /// see the named exports of a CommonJS package that builds `module.exports`
 /// dynamically (lodash & co.), so a named import fails to instantiate and a
 /// namespace import yields nothing but `default`. The bundle must import such a
-/// package as a namespace and read the names off `default` instead.
+/// package as a namespace and read the names off `default` instead — while
+/// leaving an ESM package's named imports as the live bindings node gives it.
 #[test]
 fn test_node_loader_cjs_named_export_interop() {
     use std::process::Command;
@@ -1313,12 +1314,27 @@ module.exports = api;
     )
     .unwrap();
 
+    // An ESM package whose exported binding changes after evaluation.
+    let esm_dir = dir.join("node_modules").join("live-esm-pkg");
+    std::fs::create_dir_all(&esm_dir).unwrap();
+    std::fs::write(
+        esm_dir.join("package.json"),
+        r#"{ "name": "live-esm-pkg", "version": "1.0.0", "type": "module", "main": "index.js" }"#,
+    )
+    .unwrap();
+    std::fs::write(
+        esm_dir.join("index.js"),
+        "export let count = 0;\nexport function bump() { count++; }\n",
+    )
+    .unwrap();
+
     std::fs::write(
         dir.join("main.ts"),
         r#"
 import { greet } from "dyn-cjs-pkg";
 import * as pkg from "dyn-cjs-pkg";
-export function main() { return [greet("a"), pkg.greet("b")]; }
+import { count, bump } from "live-esm-pkg";
+export function main() { bump(); return [greet("a"), pkg.greet("b"), count]; }
 "#,
     )
     .unwrap();
@@ -1370,7 +1386,7 @@ console.log(JSON.stringify(Main.main()));
         "node rejected the bundle:\nstdout:\n{stdout}\nstderr:\n{}",
         String::from_utf8_lossy(&run.stderr)
     );
-    assert_eq!(stdout.trim(), r#"["greet a","greet b"]"#);
+    assert_eq!(stdout.trim(), r#"["greet a","greet b",1]"#);
 }
 
 /// Regression test for the install_bun_lockfile no-DB path: same code shape as

backend/windmill-worker/node_cjs_interop.js

@@ -6,8 +6,12 @@
 // verbatim and node hits the limitation; rewrite them to a namespace import plus
 // a lookup that falls back to `default` (i.e. `module.exports`).
 //
-// Named bindings become snapshots instead of live bindings, which only differs
-// for an ESM package that mutates an exported binding after evaluation.
+// Only packages node loads as CommonJS are rewritten: the rewrite turns named
+// imports into snapshots, which would freeze an ESM export its package mutates
+// after evaluation.
+
+import { readFileSync } from "node:fs";
+import { dirname, join } from "node:path";
 
 const WM_IDENT = "[A-Za-z_$][A-Za-z0-9_$]*";
 const WM_NS_CLAUSE = `\\*\\s*as\\s+${WM_IDENT}`;
@@ -15,12 +19,21 @@ const WM_NAMED_CLAUSE = "\\{[^{}]*\\}";
 const WM_CLAUSE = `(?:${WM_NS_CLAUSE}|${WM_NAMED_CLAUSE}|${WM_IDENT}(?:\\s*,\\s*(?:${WM_NS_CLAUSE}|${WM_NAMED_CLAUSE}))?)`;
 const WM_IMPORT = `(^|[;}\\n])import\\s*(?:(${WM_CLAUSE})\\s*from\\s*)?(?:"([^"\\n]*)"|'([^'\\n]*)')`;
 
-function wmRewriteExternalImports(code, externals) {
+function wmRewriteExternalImports(code, externals, jobDir) {
   if (!externals || externals.length === 0) {
     return code;
   }
-  const isExternal = (spec) =>
-    externals.some((name) => spec === name || spec.startsWith(name + "/"));
+  const eligible = new Map();
+  const needsInterop = (spec) => {
+    let ok = eligible.get(spec);
+    if (ok === undefined) {
+      ok =
+        externals.some((name) => spec === name || spec.startsWith(name + "/")) &&
+        wmIsCommonJs(spec, jobDir);
+      eligible.set(spec, ok);
+    }
+    return ok;
+  };
 
   // Import statements are found on a copy whose literals are blanked out, so
   // that generated code holding an import statement in a string is not touched.
@@ -34,7 +47,7 @@ function wmRewriteExternalImports(code, externals) {
       continue;
     }
     const spec = m[3] !== undefined ? m[3] : m[4];
-    if (isExternal(spec)) {
+    if (needsInterop(spec)) {
       found.push({ at: hit.index, len: hit[0].length, lead: m[1], clause: m[2], spec });
     }
   }
@@ -106,6 +119,38 @@ function wmRewriteExternalImports(code, externals) {
   );
 }
 
+// Node's own rule for the format of the file a specifier resolves to: the
+// extension decides, and `.js` follows the `type` of the closest package.json.
+function wmIsCommonJs(spec, jobDir) {
+  let file;
+  try {
+    file = Bun.resolveSync(spec, jobDir);
+  } catch (err) {
+    return true;
+  }
+  if (file.endsWith(".mjs")) {
+    return false;
+  }
+  if (!file.endsWith(".js")) {
+    return true;
+  }
+  let dir = dirname(file);
+  for (;;) {
+    let pkg = null;
+    try {
+      pkg = JSON.parse(readFileSync(join(dir, "package.json"), "utf8"));
+    } catch (err) {}
+    if (pkg !== null) {
+      return pkg.type !== "module";
+    }
+    const parent = dirname(dir);
+    if (parent === dir) {
+      return true;
+    }
+    dir = parent;
+  }
+}
+
 function wmParseImportClause(clause) {
   let def = null;
   let ns = null;

backend/windmill-worker/src/bun_executor.rs

@@ -912,7 +912,7 @@ if (!result?.success || !(result.outputs?.length > 0)) {{
 try {{
     const bundlePath = "{job_dir_js}/wrapper.js";
     const bundle = await Bun.file(bundlePath).text();
-    const interoped = wmRewriteExternalImports(bundle, fileNames);
+    const interoped = wmRewriteExternalImports(bundle, fileNames, "{job_dir_js}");
     if (interoped !== bundle) {{
         await Bun.write(bundlePath, interoped);
     }}