Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. Streaming targets
(`stream = true`, or `aster <target> --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
Expand Down
3 changes: 3 additions & 0 deletions src/cli/skills.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <target> --stream` inherit the
caller's terminal.

## Common end-to-end flows

Expand Down
61 changes: 55 additions & 6 deletions src/executor/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand All @@ -73,7 +77,7 @@ impl<'a> Executor<'a> {
output_mode: OutputMode::Normal,
full_logs: false,
use_cache: true,
null_stdin: false,
null_stdin: true,
}
}

Expand All @@ -84,7 +88,7 @@ impl<'a> Executor<'a> {
output_mode,
full_logs: false,
use_cache: true,
null_stdin: false,
null_stdin: true,
}
}

Expand All @@ -99,7 +103,7 @@ impl<'a> Executor<'a> {
output_mode,
full_logs,
use_cache: true,
null_stdin: false,
null_stdin: true,
}
}

Expand All @@ -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
Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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
);
}
}
69 changes: 69 additions & 0 deletions tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading