From b04758245f641f0f2d6c56317d463251d5b2f424 Mon Sep 17 00:00:00 2001 From: Arne Roomann-Kurrik Date: Fri, 28 Aug 2026 13:27:50 -0700 Subject: [PATCH 1/3] fix: close stdin for captured child processes Captured targets inherited the caller's TTY as stdin while stdout/stderr were piped into the progress UI, so tools that prompt (e.g. pnpm's modules-purge confirm) hung with no visible prompt. Close stdin by default for captured execution; streaming targets still inherit the terminal. Fixes #72 --- README.md | 4 +++ src/cli/skills.md | 3 ++ src/executor/runner.rs | 61 +++++++++++++++++++++++++++++++++---- tests/integration.rs | 69 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 131 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 8287c90..faa2234 100644 --- a/README.md +++ b/README.md @@ -236,6 +236,10 @@ command = "sh -c 'generator | formatter > src/generated.rs'" cache = { enabled = false } ``` +Captured targets receive a closed stdin, so tools that prompt on a TTY fail +instead of hanging behind Aster's progress UI. Targets with `stream = true` or +`aster run --stream` inherit the caller's terminal. + Only the `files_list` capability is supported. When present, `{files}` is required to be a standalone command argument and is safely expanded into individual path arguments. It cannot be embedded in a quoted shell script or diff --git a/src/cli/skills.md b/src/cli/skills.md index edcad69..9aa1325 100644 --- a/src/cli/skills.md +++ b/src/cli/skills.md @@ -329,6 +329,9 @@ Target commands use shell-style quoting but execute the parsed program directly. Pipes, redirects, substitutions, and `&&` require an explicit shell command such as `sh -c 'generator | formatter > output'`. `{files}` is expanded safely only for targets declaring `files_list` and must occupy a standalone argument. +Captured targets receive a closed stdin, so tools that prompt on a TTY fail +instead of hanging; `stream = true` and `aster run --stream` inherit the +caller's terminal. ## Common end-to-end flows diff --git a/src/executor/runner.rs b/src/executor/runner.rs index 0b79b37..2c6cb66 100644 --- a/src/executor/runner.rs +++ b/src/executor/runner.rs @@ -62,6 +62,10 @@ pub struct Executor<'a> { /// Whether to use caching use_cache: bool, /// Whether child targets should receive a closed stdin. + /// + /// Captured execution always uses a closed stdin so tools that prompt on a + /// TTY fail instead of hanging behind Aster's progress UI. Streaming + /// targets inherit the caller's terminal separately. null_stdin: bool, } @@ -73,7 +77,7 @@ impl<'a> Executor<'a> { output_mode: OutputMode::Normal, full_logs: false, use_cache: true, - null_stdin: false, + null_stdin: true, } } @@ -84,7 +88,7 @@ impl<'a> Executor<'a> { output_mode, full_logs: false, use_cache: true, - null_stdin: false, + null_stdin: true, } } @@ -99,7 +103,7 @@ impl<'a> Executor<'a> { output_mode, full_logs, use_cache: true, - null_stdin: false, + null_stdin: true, } } @@ -115,14 +119,15 @@ impl<'a> Executor<'a> { output_mode, full_logs, use_cache, - null_stdin: false, + null_stdin: true, } } /// Prevent targets from reading the caller's terminal. /// - /// This is used for dev-service prerequisites, which execute in a worker - /// while the dashboard remains the foreground terminal process. + /// Captured execution already closes stdin. This is kept so call sites such + /// as dev-service prerequisites can document that the child must not share + /// the dashboard's terminal. pub fn with_null_stdin(mut self) -> Self { self.null_stdin = true; self @@ -1144,6 +1149,9 @@ fn run_command( .stdout(Stdio::piped()) .stderr(Stdio::piped()); if null_stdin { + // Closed stdin makes isatty(0) false even after setsid(): the child + // still inherits a TTY *fd* unless we replace it. Tools that prompt + // when stdin is a TTY then hang behind the progress UI. cmd.stdin(Stdio::null()); } @@ -2017,4 +2025,45 @@ mod tests { assert_eq!(result.output, "test output"); assert_eq!(result.duration_ms, 100); } + + #[test] + fn captured_execution_closes_stdin_by_default() { + let tmp = tempfile::tempdir().unwrap(); + assert!(Executor::new(tmp.path()).null_stdin); + assert!( + Executor::with_output_mode(tmp.path(), crate::cli::output::OutputMode::Json).null_stdin + ); + assert!( + Executor::with_options(tmp.path(), crate::cli::output::OutputMode::Json, false) + .null_stdin + ); + assert!( + Executor::with_all_options( + tmp.path(), + crate::cli::output::OutputMode::Json, + false, + false + ) + .null_stdin + ); + assert!(Executor::new(tmp.path()).with_null_stdin().null_stdin); + } + + #[cfg(unix)] + #[test] + fn captured_command_stdin_is_not_a_tty() { + let tmp = tempfile::tempdir().unwrap(); + let result = run_command( + "//a:prompt", + "sh -c 'if [ -t 0 ]; then echo tty; else echo not-a-tty; fi'", + tmp.path(), + true, + ); + assert!(result.success, "command failed: {}", result.output); + assert!( + result.output.contains("not-a-tty"), + "expected closed stdin, got: {}", + result.output + ); + } } diff --git a/tests/integration.rs b/tests/integration.rs index 112e342..e167a74 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -128,6 +128,75 @@ cache = { enabled = false } assert!(!child_exists, "target process survived Aster termination"); } +#[cfg(unix)] +#[test] +fn captured_targets_do_not_hang_on_stdin_prompts() { + use std::io::Read; + use std::process::Stdio; + + let tmp = TempDir::new().unwrap(); + setup_workspace(&tmp); + write_package_json(&tmp, "app/package.json", r#"{"name":"app"}"#); + write_aster_toml( + &tmp, + "app/aster.toml", + r#" +[targets.prompt] +command = "sh -c 'read -r _ignored; printf ready > marker'" +cache = { enabled = false } +"#, + ); + + let mut aster = Command::new(env!("CARGO_BIN_EXE_aster")) + .current_dir(tmp.path()) + .args(["run", "//app:prompt", "--no-cache"]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + + // Keep Aster's stdin write-end open. If the target inherited it, `read` + // would block until this process closed the pipe — the #72 hang. + let _stdin = aster.stdin.take(); + let mut stdout = aster.stdout.take().unwrap(); + let mut stderr = aster.stderr.take().unwrap(); + let stdout_handle = std::thread::spawn(move || { + let mut buf = Vec::new(); + stdout.read_to_end(&mut buf).ok(); + buf + }); + let stderr_handle = std::thread::spawn(move || { + let mut buf = Vec::new(); + stderr.read_to_end(&mut buf).ok(); + buf + }); + + let deadline = Instant::now() + Duration::from_secs(5); + let status = loop { + match aster.try_wait().unwrap() { + Some(status) => break status, + None if Instant::now() < deadline => std::thread::sleep(Duration::from_millis(20)), + None => { + let _ = aster.kill(); + let _ = aster.wait(); + panic!("aster hung: captured target inherited stdin and blocked on read"); + } + } + }; + + let stdout = String::from_utf8_lossy(&stdout_handle.join().unwrap()).into_owned(); + let stderr = String::from_utf8_lossy(&stderr_handle.join().unwrap()).into_owned(); + assert!( + status.success(), + "aster failed: status={status:?} stdout={stdout} stderr={stderr}" + ); + assert_eq!( + fs::read_to_string(tmp.path().join("app/marker")).unwrap(), + "ready" + ); +} + #[test] fn heterogeneous_run_preserves_quoted_arguments() { let tmp = TempDir::new().unwrap(); From f3cb5016e5944a14a3d8938f2f5f8fee85b65cc3 Mon Sep 17 00:00:00 2001 From: Arne Roomann-Kurrik Date: Fri, 28 Aug 2026 13:40:39 -0700 Subject: [PATCH 2/3] polish: address round 1 review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Docs named `aster run --stream`, which is not a flag on `aster run` — corrected to `aster --stream`, the invocation that actually inherits the terminal. - Vestigial `null_stdin` bool / `with_null_stdin()` no-op — declined: keeping the documented call site is intentional; collapsing the flag is scope expansion. Constructor tests guard the default. Reviewers: grok-native, claude-cli, review-principles --- README.md | 4 ++-- src/cli/skills.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index faa2234..c4090e3 100644 --- a/README.md +++ b/README.md @@ -237,8 +237,8 @@ cache = { enabled = false } ``` Captured targets receive a closed stdin, so tools that prompt on a TTY fail -instead of hanging behind Aster's progress UI. Targets with `stream = true` or -`aster run --stream` inherit the caller's terminal. +instead of hanging behind Aster's progress UI. Streaming targets +(`stream = true`, or `aster --stream`) inherit the caller's terminal. Only the `files_list` capability is supported. When present, `{files}` is required to be a standalone command argument and is safely expanded into diff --git a/src/cli/skills.md b/src/cli/skills.md index 9aa1325..a1c3d78 100644 --- a/src/cli/skills.md +++ b/src/cli/skills.md @@ -330,7 +330,7 @@ Pipes, redirects, substitutions, and `&&` require an explicit shell command such as `sh -c 'generator | formatter > output'`. `{files}` is expanded safely only for targets declaring `files_list` and must occupy a standalone argument. Captured targets receive a closed stdin, so tools that prompt on a TTY fail -instead of hanging; `stream = true` and `aster run --stream` inherit the +instead of hanging; `stream = true` and `aster --stream` inherit the caller's terminal. ## Common end-to-end flows From 2176ae75cb1b663ddb41445985021c2bc7da0f9f Mon Sep 17 00:00:00 2001 From: Arne Roomann-Kurrik Date: Fri, 28 Aug 2026 14:40:28 -0700 Subject: [PATCH 3/3] ci: serialize dev_services tests on macOS The macOS suite failed three supervisor tests on port-bind timeouts while Ubuntu and the stdin hang tests passed. Those tests already contended for ephemeral ports when run beside other process tests; run the crate alone and single-threaded on macOS. --- .github/workflows/ci.yml | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b0af22e..73cf04b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,15 +25,18 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: dtolnay/rust-toolchain@4cda84d5c5c54efe2404f9d843567869ab1699d4 - # This multi-supervisor test intentionally releases candidate ports before - # launch. Isolate it on macOS, where concurrent process tests can claim or - # starve those ports long enough to exceed its startup assertion. + # Supervisor tests bind real ports. Isolate that crate on macOS: concurrent + # process tests can claim or starve those ports long enough to exceed + # startup assertions. - if: runner.os == 'macOS' run: | - cargo test --locked --all-targets --all-features -- \ - --skip dynamic_port_bundles_are_distinct_propagated_and_released - cargo test --locked --test dev_services --all-features \ - dynamic_port_bundles_are_distinct_propagated_and_released + cargo test --locked --lib --bins --all-features + cargo test --locked --all-features \ + --test config_bug_bash \ + --test dev_tls \ + --test integration \ + --test watch_tests + cargo test --locked --test dev_services --all-features -- --test-threads=1 - if: runner.os != 'macOS' run: cargo test --locked --all-targets --all-features - run: scripts/test-dynamic-service-ports