Skip to content

fix: run packaged shell scripts and capture their output - #379

Open
migueltheduck wants to merge 1 commit into
developfrom
fix/bash-runtime
Open

migueltheduck wants to merge 1 commit into
developfrom
fix/bash-runtime

Conversation

@migueltheduck

@migueltheduck migueltheduck commented Nov 10, 2026 •

Copy link
Copy Markdown

Why

Since bundle v2 the packager puts every app file under app/, but validate_invoke_script and the runtime still resolved a .sh entry point from the package root. Every CLI-built shell app therefore fails with MissingScript, locally and on Tower.

Once found, the bash path had three more problems the uv path doesn't:

  • It never read the script's stdout or stderr. Output was lost, and a script that wrote more than the pipe buffer blocked forever.
  • It didn't start a process group, so cancelling killed bash but left its children running.
  • In protected mode it inherited the host environment instead of starting from a cleared one.

What

  • Package::resolve_invoke_path resolves the script from the app directory. Only for .sh does it fall back to the package root, so hand-built bundles with invoke = "app/run.sh" keep working. Packaging-time validation and the runtime both use it.
  • The bash branch forwards stdout and stderr to the program channel. A forward_output helper replaces the copies of the drain code in the uv steps.
  • execute_bash_program runs in its own process group, normalizes env vars as uv run does (tower_uv::normalize_env_vars is now pub), and clears the environment in protected mode.
  • New example app 07-bash-script with two tests: output and env in protected mode (about 200 KB of output), and cancel stopping a backgrounded sleep. Both fail on develop.

Compatibility

Bundles that already point invoke at the right path keep working: v1 bundles resolve as before, and a hand-built invoke = "app/run.sh" is still found through the root fallback. In protected mode a shell script now gets the same environment as a Python app (Tower's variables plus the host PATH) instead of the whole host environment.

Since bundle v2 the packager puts every app file under app/, but the
runtime still resolved .sh entry points from the package root, so every
packaged shell app failed to load with MissingScript. Shell scripts now
resolve from the app directory, falling back to the package root.

The bash path also never read the script's stdout or stderr, losing its
output and blocking any script that filled the pipe buffer. It now
forwards both to the program channel, runs in its own process group so
cancellation stops the script's children, and clears the environment in
protected mode, as uv run does.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@coderabbitai

coderabbitai Bot commented Nov 10, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@github-actions

Copy link
Copy Markdown
Contributor


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant