fix(bun): propagate non-zero exit from generate_bun_bundle on no-DB path (#9051)

backend/src/main.rs

@@ -305,8 +305,6 @@ async fn cache_hub_scripts(file_path: Option<String>) -> anyhow::Result<()> {
                 )
                 .await?;
 
-                let _ = windmill_common::worker::write_file(&job_dir, "main.js", &res.content)?;
-
                 if let Err(e) = windmill_worker::prebundle_bun_script(
                     &res.content,
                     &lock,

backend/tests/bun_jobs.rs

@@ -956,6 +956,221 @@ export function main() {
     Ok(())
 }
 
+// ============================================================================
+// Bundle Wrapper Safety Tests
+// ============================================================================
+
+/// Regression test for the "TS source ends up in the bun bundle cache" bug.
+///
+/// The wrapper-side hardening: `node_builder.ts` discarded `Bun.build`'s
+/// return value, so any silent-failure mode (`success: false` without
+/// throwing — `throw: false`, or a future Bun where defaults change) made
+/// the wrapper exit 0 even though no `main.js` was written. Pair that with
+/// a pre-existing `main.js` containing raw TypeScript and `save_cache`
+/// happily copied that TS into the bundle cache; the worker later choked
+/// on `type GpgKey = {`.
+///
+/// This test patches `node_builder.ts` to force the silent-failure shape
+/// and asserts that our wrapper now refuses to silently succeed — bun must
+/// exit non-zero so prebundling fails loudly instead of writing TypeScript
+/// into the bundle cache.
+#[test]
+fn test_bun_bundle_wrapper_catches_silent_failure() {
+    use std::process::Command;
+    use windmill_worker::{build_loader, LoaderMode, BUN_PATH};
+
+    let temp_dir = tempfile::tempdir().unwrap();
+    let dir = temp_dir.path();
+    let dir_str = dir.to_str().unwrap();
+
+    // Script imports a package that won't exist in node_modules.
+    std::fs::write(
+        dir.join("main.ts"),
+        r#"
+import x from "definitely-not-a-real-pkg-windmill-test";
+export function main() { return x; }
+"#,
+    )
+    .unwrap();
+
+    // Generate the real node_builder.ts via the production code path.
+    tokio::runtime::Runtime::new()
+        .unwrap()
+        .block_on(build_loader(
+            dir_str,
+            "http://localhost:8000",
+            "test_token",
+            "test-workspace",
+            "f/test/script",
+            LoaderMode::BunBundle,
+            &None,
+        ))
+        .expect("build_loader failed");
+
+    // Force the silent-failure shape by injecting `throw: false`. The
+    // wrapper's pre-fix `try/catch` would have swallowed this; the fixed
+    // wrapper inspects `result.success` and `result.outputs` and exits 1.
+    let path = dir.join("node_builder.ts");
+    let original = std::fs::read_to_string(&path).unwrap();
+    let patched = original.replace(
+        "external: [\"electron\"],",
+        "external: [\"electron\"], throw: false,",
+    );
+    assert_ne!(
+        original, patched,
+        "expected to find Bun.build options block to patch; node_builder.ts template changed?"
+    );
+    std::fs::write(&path, patched).unwrap();
+
+    // Pre-seed main.js with raw TypeScript (mimics the historical
+    // pre-write that originally seeded the bug).
+    std::fs::write(
+        dir.join("main.js"),
+        "type GpgKey = { email: string };\nexport const main = (): GpgKey => ({ email: \"\" });\n",
+    )
+    .unwrap();
+
+    let output = Command::new(BUN_PATH.as_str())
+        .args(["run", path.to_str().unwrap()])
+        .current_dir(dir)
+        .output()
+        .expect("Failed to run bun");
+
+    let stdout = String::from_utf8_lossy(&output.stdout);
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        !output.status.success(),
+        "node_builder.ts must exit non-zero when Bun.build silently fails to write a bundle.\nstdout:\n{stdout}\nstderr:\n{stderr}"
+    );
+    assert!(
+        stdout.contains("Failed to build node bundle"),
+        "expected diagnostic in stdout, got:\n{stdout}"
+    );
+}
+
+/// Regression test for the actual root cause of the "TS source in bundle
+/// cache" bug: `generate_bun_bundle` was awaiting `child_process.wait()`
+/// without checking the exit code on the no-DB path (used by Docker-build
+/// `windmill cache hubPaths.json`). bun would exit 1 after Bun.build threw,
+/// `wait().await?` propagated only IO errors, and `generate_bun_bundle`
+/// returned `Ok(())`. `save_cache` then copied a stale `main.js` (raw TS
+/// source) straight into the bundle cache.
+///
+/// This test runs `generate_bun_bundle` with `db: None` against a `node_builder.ts`
+/// that calls `process.exit(1)`, and asserts the function now returns an error.
+#[test]
+fn test_generate_bun_bundle_propagates_exit_status() {
+    use windmill_worker::{generate_bun_bundle, get_common_bun_proc_envs};
+
+    let temp_dir = tempfile::tempdir().unwrap();
+    let dir = temp_dir.path();
+    let dir_str = dir.to_str().unwrap();
+
+    // node_builder.ts that exits 1, mimicking what bun does when Bun.build throws.
+    std::fs::write(
+        dir.join("node_builder.ts"),
+        "console.log('simulated bun build failure');\nprocess.exit(1);\n",
+    )
+    .unwrap();
+
+    let runtime = tokio::runtime::Runtime::new().unwrap();
+    let envs = runtime.block_on(get_common_bun_proc_envs(None));
+
+    let result = runtime.block_on(generate_bun_bundle(
+        dir_str,
+        "test-workspace",
+        &uuid::Uuid::new_v4(),
+        "test-worker",
+        None, // db: None — this is the cache_hub_scripts path that had the bug
+        None,
+        &mut 0,
+        &mut None,
+        &envs,
+        &mut None,
+    ));
+
+    assert!(
+        result.is_err(),
+        "generate_bun_bundle must surface bun's non-zero exit on the no-DB path. \
+         If it returns Ok(()) when bun exited 1, save_cache will silently cache stale main.js content."
+    );
+    let err_msg = format!("{:?}", result.unwrap_err());
+    assert!(
+        err_msg.contains("non-zero status"),
+        "expected exit-status error, got: {err_msg}"
+    );
+}
+
+/// Regression test for the install_bun_lockfile no-DB path: same code shape as
+/// `generate_bun_bundle` (site 3 of the original bug) — `wait().await?` ignored
+/// non-zero bun exits. A `bun install` failure (e.g. malformed package.json)
+/// must now surface as an error so callers don't proceed with a half-installed
+/// node_modules.
+#[test]
+fn test_install_bun_lockfile_propagates_exit_status() {
+    use windmill_worker::{get_common_bun_proc_envs, install_bun_lockfile};
+    let temp_dir = tempfile::tempdir().unwrap();
+    let dir = temp_dir.path();
+    let dir_str = dir.to_str().unwrap();
+    // Malformed package.json -> bun install fails with exit 1.
+    std::fs::write(dir.join("package.json"), "this is not valid json").unwrap();
+    let runtime = tokio::runtime::Runtime::new().unwrap();
+    let envs = runtime.block_on(get_common_bun_proc_envs(None));
+    let result = runtime.block_on(install_bun_lockfile(
+        &mut 0,
+        &mut None,
+        &uuid::Uuid::new_v4(),
+        "test-workspace",
+        None, // db: None — no-DB path that had the bug
+        dir_str,
+        "test-worker",
+        envs,
+        false, // npm_mode
+        &mut None,
+        true, // quiet
+    ));
+    assert!(
+        result.is_err(),
+        "install_bun_lockfile must surface bun's non-zero exit on the no-DB path"
+    );
+    let err_msg = format!("{:?}", result.unwrap_err());
+    assert!(
+        err_msg.contains("non-zero status"),
+        "expected exit-status error, got: {err_msg}"
+    );
+}
+
+/// Regression test for the post-bundle existence check in `prebundle_bun_script`
+/// and `handle_bun_job`. Both call sites guard against the case where
+/// `generate_bun_bundle` returns `Ok(())` but `main.js` was never written —
+/// the upstream wait-status fix is the primary defense, this is the catch-all
+/// for any other silent-failure mode (Bun output-naming change, custom plugin
+/// swallowing the build, etc.). Without this check, `save_cache` would
+/// happily copy whatever's at the bundle path (often raw TypeScript that some
+/// other code path left there).
+#[test]
+fn test_ensure_bundle_output_exists_rejects_missing_file() {
+    use windmill_worker::ensure_bundle_output_exists;
+    let temp_dir = tempfile::tempdir().unwrap();
+    let dir = temp_dir.path();
+    let missing = dir.join("main.js").to_str().unwrap().to_string();
+
+    let result = ensure_bundle_output_exists(&missing);
+    assert!(
+        result.is_err(),
+        "ensure_bundle_output_exists must reject when the bundle file is missing"
+    );
+    let err_msg = format!("{:?}", result.unwrap_err());
+    assert!(
+        err_msg.contains("bun bundle output missing"),
+        "expected 'bun bundle output missing' in error, got: {err_msg}"
+    );
+
+    // Sanity: when the file does exist, it returns Ok.
+    std::fs::write(&missing, "// @bun\n").unwrap();
+    assert!(ensure_bundle_output_exists(&missing).is_ok());
+}
+
 // ============================================================================
 // Dedicated Worker Protocol Tests
 // ============================================================================

backend/windmill-worker/loader_builder.bun.js

@@ -1,5 +1,6 @@
+let buildResult;
 try {
-  await Bun.build({
+  buildResult = await Bun.build({
     entrypoints: ["./main.ts"],
     outdir: "./out",
     plugins: [p],
@@ -17,6 +18,11 @@ try {
   console.log(err);
   process.exit(1);
 }
+if (!buildResult?.success || !(buildResult.outputs?.length > 0)) {
+  for (const log of buildResult?.logs ?? []) console.log(log);
+  console.log("Failed to build bundle: success=" + buildResult?.success + ", outputs=" + (buildResult?.outputs?.length ?? 0));
+  process.exit(1);
+}
 
 const fs = require("fs/promises");
 

backend/windmill-worker/src/bun_executor.rs

@@ -471,7 +471,12 @@ pub async fn gen_bun_lockfile(
             }
             result?;
         } else {
-            Box::into_pin(child_process.wait()).await?;
+            let status = Box::into_pin(child_process.wait()).await?;
+            if !status.success() {
+                return Err(error::Error::ExecutionErr(format!(
+                    "bun build exited with non-zero status: {status:?}"
+                )));
+            }
         }
 
         let new_package_json = read_file_content(&format!("{job_dir}/package.json")).await?;
@@ -777,7 +782,12 @@ pub async fn install_bun_lockfile(
         }
         result?;
     } else {
-        Box::into_pin(child_process.wait()).await?;
+        let status = Box::into_pin(child_process.wait()).await?;
+        if !status.success() {
+            return Err(error::Error::ExecutionErr(format!(
+                "bun install exited with non-zero status: {status:?}"
+            )));
+        }
     }
 
     if has_file {
@@ -838,8 +848,9 @@ try {{
 }} catch (e) {{
 }}
 
+let result;
 try {{
-    await Bun.build({{
+    result = await Bun.build({{
         entrypoints: ["{job_dir_js}/wrapper.mjs"],
         outdir: "./",
         target: "node",
@@ -852,6 +863,11 @@ try {{
     console.log("Failed to build node bundle");
     process.exit(1);
 }}
+if (!result?.success || !(result.outputs?.length > 0)) {{
+    for (const log of result?.logs ?? []) console.log(log);
+    console.log("Failed to build node bundle: success=" + result?.success + ", outputs=" + (result?.outputs?.length ?? 0));
+    process.exit(1);
+}}
 "#
             ),
         )?;
@@ -880,8 +896,9 @@ plugin(p)
                 r#"
 {loader}
 
+let result;
 try {{
-    await Bun.build({{
+    result = await Bun.build({{
         entrypoints: ["{job_dir_js}/main.ts"],
         outdir: "./",
         target: "{}",
@@ -898,6 +915,11 @@ try {{
     console.log("Failed to build node bundle");
     process.exit(1);
 }}
+if (!result?.success || !(result.outputs?.length > 0)) {{
+    for (const log of result?.logs ?? []) console.log(log);
+    console.log("Failed to build node bundle: success=" + result?.success + ", outputs=" + (result?.outputs?.length ?? 0));
+    process.exit(1);
+}}
 "#,
                 if mode == LoaderMode::BunBundle {
                     "bun"
@@ -1008,7 +1030,12 @@ pub async fn generate_bun_bundle(
         )
         .await?;
     } else {
-        Box::into_pin(child_process.wait()).await?;
+        let status = Box::into_pin(child_process.wait()).await?;
+        if !status.success() {
+            return Err(error::Error::ExecutionErr(format!(
+                "bun build exited with non-zero status: {status:?}"
+            )));
+        }
     }
     Ok(())
 }
@@ -1137,6 +1164,9 @@ pub async fn prebundle_bun_script(
         content = format!("export {{ WorkflowCtx, StepSuspend, setWorkflowCtx }} from \"windmill-client\";\n{content}");
     }
     write_file(job_dir, "main.ts", &content)?;
+    // Remove any stale main.js so we never confuse a leftover (e.g. unbundled TS source
+    // a caller dropped at this path) with a fresh Bun bundle output.
+    let _ = std::fs::remove_file(&origin);
     build_loader(
         job_dir,
         base_internal_url,
@@ -1170,11 +1200,25 @@ pub async fn prebundle_bun_script(
     )
     .await?;
 
+    ensure_bundle_output_exists(&origin)?;
+
     save_cache(&local_path, &remote_path, &origin, false).await?;
 
     Ok(())
 }
 
+/// Refuse to cache a bundle if `Bun.build` finished without producing the
+/// expected output file. Belt-and-suspenders for any silent-failure mode the
+/// upstream wait-status / `result.success` checks don't already trip on.
+pub fn ensure_bundle_output_exists(bundle_path: &str) -> Result<()> {
+    if !std::path::Path::new(bundle_path).exists() {
+        return Err(error::Error::ExecutionErr(format!(
+            "bun bundle output missing at {bundle_path} after Bun.build — refusing to cache"
+        )));
+    }
+    Ok(())
+}
+
 pub const BUN_BUNDLE_OBJECT_STORE_PREFIX: &str = "bun_bundle/";
 
 async fn get_script_import_updated_at(db: &DB, w_id: &str, script_path: &str) -> Result<String> {
@@ -1902,15 +1946,10 @@ try {{
                 &mut Some(occupancy_metrics),
             )
             .await?;
+            let bundle_path = format!("{job_dir}/main.js");
+            ensure_bundle_output_exists(&bundle_path)?;
             if !local_path.is_empty() {
-                match save_cache(
-                    &local_path,
-                    &remote_path,
-                    &format!("{job_dir}/main.js"),
-                    false,
-                )
-                .await
-                {
+                match save_cache(&local_path, &remote_path, &bundle_path, false).await {
                     Err(e) => {
                         let em = format!("could not save {local_path} to bundle cache: {e:?}");
                         tracing::error!(em)

backend/windmill-worker/src/lib.rs

@@ -90,9 +90,10 @@ pub use otel_tracing_proxy_ee::{load_internal_otel_exporter, DENO_OTEL_INITIALIZ
 pub use worker::*;
 
 pub use bun_executor::{
-    build_loader, compute_bundle_local_and_remote_path, get_common_bun_proc_envs,
-    install_bun_lockfile, prebundle_bun_script, prepare_job_dir, LoaderMode,
-    BUN_DEDICATED_WORKER_ARGS, RELATIVE_BUN_BUILDER, RELATIVE_BUN_LOADER,
+    build_loader, compute_bundle_local_and_remote_path, ensure_bundle_output_exists,
+    generate_bun_bundle, get_common_bun_proc_envs, install_bun_lockfile, prebundle_bun_script,
+    prepare_job_dir, LoaderMode, BUN_DEDICATED_WORKER_ARGS, RELATIVE_BUN_BUILDER,
+    RELATIVE_BUN_LOADER,
 };
 #[cfg(any(feature = "private", test))]
 pub use bun_executor::{