fix: run packaged shell scripts and capture their output

crates/tower-package/src/native.rs

@@ -144,19 +144,7 @@ impl Package {
             });
         }
 
-        // Shell scripts (.sh) are executed relative to package_path, while Python
-        // scripts are executed relative to the app working directory. Check the
-        // path that matches the runtime's actual execution behavior.
-        let resolved_path = if manifest.invoke.ends_with(".sh") {
-            package_path.join(&manifest.invoke)
-        } else {
-            let working_dir = if manifest.version == Some(1) {
-                package_path.to_path_buf()
-            } else {
-                package_path.join(&manifest.app_dir_name)
-            };
-            working_dir.join(&manifest.invoke)
-        };
+        let resolved_path = Self::resolve_invoke_path(manifest, package_path);
 
         if !resolved_path.exists() {
             return Err(Error::MissingScript {
@@ -177,6 +165,30 @@ impl Package {
         Ok(())
     }
 
+    /// The directory an app runs in: the package root for v1 packages, the app directory
+    /// otherwise.
+    pub fn working_dir(manifest: &Manifest, package_path: &Path) -> PathBuf {
+        if manifest.version == Some(1) {
+            package_path.to_path_buf()
+        } else {
+            package_path.join(&manifest.app_dir_name)
+        }
+    }
+
+    /// Where the manifest's invoke script lives under `package_path`. Scripts resolve from the
+    /// working directory, where the packager puts them. Shell scripts fall back to the package
+    /// root, where the runtime used to look for them.
+    pub fn resolve_invoke_path(manifest: &Manifest, package_path: &Path) -> PathBuf {
+        let in_working_dir = Self::working_dir(manifest, package_path).join(&manifest.invoke);
+        if manifest.invoke.ends_with(".sh") && !in_working_dir.exists() {
+            let in_root = package_path.join(&manifest.invoke);
+            if in_root.exists() {
+                return in_root;
+            }
+        }
+        in_working_dir
+    }
+
     // build creates a new package from a PackageSpec. PackageSpec is typically composed of fields
     // copied from the Towerfile. The most important thing to know is that the collection of file
     // globs to include in the package.
@@ -653,26 +665,40 @@ mod test {
     }
 
     #[test]
-    fn test_validate_invoke_script_shell_resolves_from_package_root() {
+    fn test_validate_invoke_script_shell_resolves_from_app_dir() {
         let dir = TempDir::new().unwrap();
         let app_dir = dir.path().join("app");
         std::fs::create_dir_all(&app_dir).unwrap();
-        // Shell script placed at package root level (not inside app/)
-        std::fs::write(dir.path().join("run.sh"), "#!/bin/bash").unwrap();
+        // The packager puts every app file, shell scripts included, under app/.
+        std::fs::write(app_dir.join("run.sh"), "#!/bin/bash").unwrap();
         let manifest = make_manifest("./run.sh", None);
-        // Should resolve from package_path for .sh files
         assert!(Package::validate_invoke_script(&manifest, dir.path()).is_ok());
+        assert_eq!(
+            Package::resolve_invoke_path(&manifest, dir.path()),
+            app_dir.join("./run.sh")
+        );
     }
 
     #[test]
-    fn test_validate_invoke_script_shell_not_in_app_dir() {
+    fn test_validate_invoke_script_shell_falls_back_to_package_root() {
         let dir = TempDir::new().unwrap();
         let app_dir = dir.path().join("app");
         std::fs::create_dir_all(&app_dir).unwrap();
-        // Shell script placed inside app/ but runtime looks at package root
-        std::fs::write(app_dir.join("run.sh"), "#!/bin/bash").unwrap();
+        std::fs::write(dir.path().join("run.sh"), "#!/bin/bash").unwrap();
         let manifest = make_manifest("./run.sh", None);
-        // Should fail because runtime looks at package_path.join(invoke), not working_dir
+        assert!(Package::validate_invoke_script(&manifest, dir.path()).is_ok());
+        assert_eq!(
+            Package::resolve_invoke_path(&manifest, dir.path()),
+            dir.path().join("./run.sh")
+        );
+    }
+
+    #[test]
+    fn test_validate_invoke_script_python_does_not_fall_back_to_package_root() {
+        let dir = TempDir::new().unwrap();
+        std::fs::create_dir_all(dir.path().join("app")).unwrap();
+        std::fs::write(dir.path().join("main.py"), "print('hi')").unwrap();
+        let manifest = make_manifest("./main.py", None);
         let err = Package::validate_invoke_script(&manifest, dir.path()).unwrap_err();
         assert!(matches!(err, Error::MissingScript { .. }));
     }

crates/tower-runtime/src/local.rs

@@ -29,7 +29,7 @@ use nix::{
 
 use tokio_util::sync::CancellationToken;
 
-use tower_package::{Manifest, Package};
+use tower_package::Package;
 use tower_telemetry::debug;
 use tower_uv::Uv;
 
@@ -149,11 +149,7 @@ async fn inner_execute_local_app(
     let package_path = package.unpacked_path.clone().unwrap().to_path_buf();
 
     // set for later on.
-    let working_dir = if package.manifest.version == Some(1) {
-        package_path.to_path_buf()
-    } else {
-        package_path.join(&package.manifest.app_dir_name)
-    };
+    let working_dir = Package::working_dir(&package.manifest, &package_path);
 
     debug!(ctx: &ctx, " - working directory: {:?}", &working_dir);
 
@@ -182,19 +178,26 @@ async fn inner_execute_local_app(
         return Err(Error::Cancelled);
     }
 
+    // Without a cache, an app runs in protected mode: its process gets a cleared environment
+    // plus only what we pass in.
+    let protected_mode = opts.cache_dir.is_none();
+
     if is_bash_package(&package) {
-        let child = execute_bash_program(
+        let script = Package::resolve_invoke_path(manifest, &package_path);
+        let mut child = execute_bash_program(
             &ctx,
             working_dir,
-            package_path,
-            &manifest,
+            script,
+            protected_mode,
             secrets,
             params,
             other_env_vars,
             &import_paths,
         )
         .await?;
 
+        forward_output(&mut child, Channel::Program, &opts.output_sender);
+
         let code = wait_for_process(ctx.clone(), &cancel_token, child).await;
         // `wait_for_process` returns -1 on both cancellation and IO error;
         // disambiguate so a cancellation surfaces as `Status::Cancelled`
@@ -204,9 +207,6 @@ async fn inner_execute_local_app(
         }
         Ok(code)
     } else {
-        // we put Uv in to protected mode when there's no caching configured/enabled.
-        let protected_mode = opts.cache_dir.is_none();
-
         let uv = Uv::new(opts.cache_dir, protected_mode).await?;
         let env_vars = make_env_vars(
             &ctx,
@@ -227,23 +227,7 @@ async fn inner_execute_local_app(
         }
 
         let mut child = uv.venv(&working_dir, &env_vars).await?;
-
-        // Drain the logs to the output channel.
-        let stdout = child.stdout.take().expect("no stdout");
-        tokio::spawn(drain_output(
-            FD::Stdout,
-            Channel::Setup,
-            opts.output_sender.clone(),
-            BufReader::new(stdout),
-        ));
-
-        let stderr = child.stderr.take().expect("no stderr");
-        tokio::spawn(drain_output(
-            FD::Stderr,
-            Channel::Setup,
-            opts.output_sender.clone(),
-            BufReader::new(stderr),
-        ));
+        forward_output(&mut child, Channel::Setup, &opts.output_sender);
 
         // Wait for venv to finish up.
         let res = wait_for_process(ctx.clone(), &cancel_token, child).await;
@@ -333,23 +317,7 @@ async fn inner_execute_local_app(
         }
 
         let mut child = uv.run(&working_dir, &program_path, &env_vars).await?;
-
-        // Drain the logs to the output channel.
-        let stdout = child.stdout.take().expect("no stdout");
-        tokio::spawn(drain_output(
-            FD::Stdout,
-            Channel::Program,
-            opts.output_sender.clone(),
-            BufReader::new(stdout),
-        ));
-
-        let stderr = child.stderr.take().expect("no stderr");
-        tokio::spawn(drain_output(
-            FD::Stderr,
-            Channel::Program,
-            opts.output_sender.clone(),
-            BufReader::new(stderr),
-        ));
+        forward_output(&mut child, Channel::Program, &opts.output_sender);
 
         let code = wait_for_process(ctx.clone(), &cancel_token, child).await;
         if cancel_token.is_cancelled() {
@@ -473,8 +441,8 @@ impl App for LocalApp {
 async fn execute_bash_program(
     ctx: &tower_telemetry::Context,
     cwd: PathBuf,
-    package_path: PathBuf,
-    manifest: &Manifest,
+    script: PathBuf,
+    protected_mode: bool,
     secrets: HashMap<String, String>,
     params: HashMap<String, String>,
     other_env_vars: HashMap<String, String>,
@@ -483,26 +451,39 @@ async fn execute_bash_program(
     let bash_path = find_bash().await?;
     debug!(ctx: &ctx, "using bash at {:?}", bash_path);
 
-    debug!(ctx: &ctx, " - bash script {}", manifest.invoke);
+    debug!(ctx: &ctx, " - bash script {:?}", script);
 
-    let child = Command::new(bash_path)
-        .current_dir(&cwd)
-        .arg(package_path.join(manifest.invoke.clone()))
+    let env_vars = tower_uv::normalize_env_vars(&make_env_vars(
+        &ctx,
+        &cwd,
+        &secrets,
+        &params,
+        &other_env_vars,
+        import_paths,
+    ));
+
+    let mut cmd = Command::new(bash_path);
+    cmd.current_dir(&cwd)
+        .arg(script)
         .stdin(Stdio::null())
         .stdout(Stdio::piped())
         .stderr(Stdio::piped())
-        .envs(make_env_vars(
-            &ctx,
-            &cwd,
-            &secrets,
-            &params,
-            &other_env_vars,
-            import_paths,
-        ))
-        .kill_on_drop(true)
-        .spawn()?;
+        .kill_on_drop(true);
 
-    Ok(child)
+    // Its own process group lets cancellation (`killpg`) reach everything the script starts.
+    #[cfg(unix)]
+    {
+        cmd.process_group(0);
+    }
+
+    if protected_mode {
+        cmd.env_clear();
+    }
+
+    // After env_clear, so protected mode keeps these.
+    cmd.envs(env_vars);
+
+    Ok(cmd.spawn()?)
 }
 
 fn make_env_var_key(src: &str) -> String {
@@ -608,23 +589,27 @@ async fn run_setup_child(
     output_sender: &OutputSender,
     mut child: Child,
 ) -> i32 {
+    forward_output(&mut child, Channel::Setup, output_sender);
+    wait_for_process(ctx.clone(), cancel_token, child).await
+}
+
+/// Sends the child's stdout and stderr lines to `output` on `channel` until the pipes close.
+fn forward_output(child: &mut Child, channel: Channel, output: &OutputSender) {
     let stdout = child.stdout.take().expect("no stdout");
     tokio::spawn(drain_output(
         FD::Stdout,
-        Channel::Setup,
-        output_sender.clone(),
+        channel,
+        output.clone(),
         BufReader::new(stdout),
     ));
 
     let stderr = child.stderr.take().expect("no stderr");
     tokio::spawn(drain_output(
         FD::Stderr,
-        Channel::Setup,
-        output_sender.clone(),
+        channel,
+        output.clone(),
         BufReader::new(stderr),
     ));
-
-    wait_for_process(ctx.clone(), cancel_token, child).await
 }
 
 async fn wait_for_process(

crates/tower-runtime/tests/example-apps/07-bash-script/Towerfile

@@ -0,0 +1,3 @@
+[app]
+name = "07-bash-script"
+script = "./run.sh"

crates/tower-runtime/tests/example-apps/07-bash-script/run.sh

@@ -0,0 +1,19 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+if [ "${MODE:-}" = "sleep" ]; then
+  # Start a child and wait on it, so tests can check that cancelling stops it too.
+  sleep 300 &
+  echo "child pid: $!"
+  wait
+  exit 0
+fi
+
+echo "The greeting is: ${GREETING:-unset}"
+echo "The parent variable is: ${BASH_TEST_PARENT_VARIABLE:-unset}"
+
+# More than a pipe buffer (64 KiB), so the script blocks unless its output is read.
+pad=$(printf '%0.sx' $(seq 1 90))
+for i in $(seq 1 2000); do
+  echo "filler $i $pad"
+done