From e965b2a836a725f77da8d5973c57bbd8fa068fd3 Mon Sep 17 00:00:00 2001 From: Quorafind Date: Wed, 7 Oct 2026 11:15:42 +0800 Subject: [PATCH 1/3] test(plugins/sandbox): Run the Windows host-launch test on Windows only The test hands wrap_host a Windows request, `C:\node\node.exe` among it, and checks that Node's folder is granted. Only a Windows Path splits that at its backslash: on macOS its parent is empty, so the v1.9.1 tag's test job failed on `--allow-read C:\node`. The product only takes this path on Windows, so the test now runs only there. --- crates/plugins/sandbox/src/container.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/plugins/sandbox/src/container.rs b/crates/plugins/sandbox/src/container.rs index 63a2c7b..f2354bf 100644 --- a/crates/plugins/sandbox/src/container.rs +++ b/crates/plugins/sandbox/src/container.rs @@ -454,6 +454,9 @@ mod tests { } } + // Only a Windows `Path` splits `C:\node\node.exe` at its backslash; on + // macOS Node's folder would read as empty, and only Windows runs this. + #[cfg(windows)] #[test] fn on_windows_the_host_runs_under_the_helper_with_a_pipe_its_reads_and_no_network() { let features = crate::runtime::windows::SandboxWinFeatures { From 41216223e567b743a96614a61ee2a6c1c1145007 Mon Sep 17 00:00:00 2001 From: Quorafind Date: Wed, 7 Oct 2026 11:15:43 +0800 Subject: [PATCH 2/3] ci(release): Test every crate even after one fails cargo test stops at the first failing test binary, so the v1.9.1 tag's failure in rebon-plugin-sandbox left every crate after it untested on macOS. With --no-fail-fast one run names every failure. --- .github/workflows/release.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f2a4f5e..2156502 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -246,8 +246,10 @@ jobs: # antivirus quarantines every freshly built test binary that drives a # JSON-RPC loop. This step is the only place they run, so it is the # gate. New feature plugins under crates/plugins/ go on this list. + # --no-fail-fast: a failure in one crate otherwise leaves every crate + # after it untested, and a release run should name all of them. run: >- - cargo test --locked + cargo test --locked --no-fail-fast -p rebon-core -p rebon-harness -p rebon-tool -p rebon-acp -p rebon-agent-core -p rebon-slash-commands -p rebon-acp-client -p rebon-config -p rebon-proto From fe39fd6d7039373ba45b0559714d9c0b53f6ab5d Mon Sep 17 00:00:00 2001 From: Quorafind Date: Wed, 7 Oct 2026 14:39:06 +0800 Subject: [PATCH 3/3] fix(tool): Stop waiting on a shell's pipes once its group is gone A background shell or monitor finished only when both output readers reached end of file. A process outside the shell's group that holds a pipe's write end keeps that from happening, and on macOS that can be any process another thread spawned while the pipe was made: without pipe2, std sets close-on-exec in a second step. The shell's group was already killed and reaped, yet its completion never came, which is how command_monitor_auto_stop failed under a loaded macOS runner. Once the child has exited and its group is reaped, the readers now share a two-second grace to drain what the pipes hold, then stop. Cutting a reader off is not an error, so a clean exit still reads as exited. --- crates/rebon-tool/src/shell_process.rs | 68 +++++++++++++++++++++++--- 1 file changed, 62 insertions(+), 6 deletions(-) diff --git a/crates/rebon-tool/src/shell_process.rs b/crates/rebon-tool/src/shell_process.rs index 6ef40ae..5f25286 100644 --- a/crates/rebon-tool/src/shell_process.rs +++ b/crates/rebon-tool/src/shell_process.rs @@ -31,6 +31,12 @@ const OUTPUT_RESPONSE_BYTES: usize = 64 * 1024; const MONITOR_LINE_BUFFER_BYTES: usize = 1024 * 1024; const MAX_RUNNING_SHELLS_PER_OWNER: usize = 16; const MAX_COMPLETED_SHELLS_PER_OWNER: usize = 64; +/// How long the output readers get to reach end of file once the process +/// group is gone. They normally reach it at once; they do not while a +/// process outside the group holds a pipe's write end. On macOS that can be +/// any process another thread spawned while the pipe was being made: without +/// `pipe2`, std marks a new pipe close-on-exec in a second step. +const READER_DRAIN_GRACE: Duration = Duration::from_secs(2); #[derive(Debug, Clone, Copy)] pub(crate) enum ShellOutputEncoding { @@ -1416,15 +1422,36 @@ async fn supervise_process( if let Err(err) = entry.process_tree.terminate() { entry.record_error(format!("failed to clean up process tree: {err}")); } - if let Err(err) = stdout_task.await { - entry.record_error(format!("background stdout reader failed: {err}")); - } - if let Err(err) = stderr_task.await { - entry.record_error(format!("background stderr reader failed: {err}")); - } + let drain_deadline = Instant::now() + READER_DRAIN_GRACE; + drain_reader(stdout_task, drain_deadline, &entry, "stdout").await; + drain_reader(stderr_task, drain_deadline, &entry, "stderr").await; entry.finish(wait_result); } +/// Let a reader finish what the pipe still holds, but no longer than +/// `deadline`: whatever keeps the pipe open past it is not the shell's. +/// Cutting it off is not an error, so a command that exited cleanly still +/// reads as exited. +async fn drain_reader( + mut reader: JoinHandle<()>, + deadline: Instant, + entry: &ShellEntry, + stream: &str, +) { + match tokio::time::timeout_at(deadline, &mut reader).await { + Ok(Ok(())) => {} + Ok(Err(err)) => entry.record_error(format!("background {stream} reader failed: {err}")), + Err(_) => { + tracing::debug!( + shell_id = %entry.shell_id, + stream, + "output cut off: pipe held open outside the process group" + ); + reader.abort(); + } + } +} + fn running_count(entries: &HashMap>, owner: &ShellOwner) -> usize { entries .values() @@ -2853,6 +2880,35 @@ mod tests { assert_eq!(lock(&controller.monitor_events).len(), 1); } + /// A process outside the group can hold stdout open after the group is + /// gone; on macOS any process spawned while the pipe was made can. Perl + /// stands in for one here: `setpgrp` leaves the group the stop reaps, + /// and it keeps the pipe for longer than the test waits. + #[cfg(unix)] + #[tokio::test] + async fn command_monitor_auto_stop_does_not_wait_for_a_pipe_held_outside_the_group() { + let _env = crate::test_env::hold_env(); + let script = "perl -e 'setpgrp(0, 0); sleep 10' & sleep 0.5; printf 'event\\n'; sleep 30"; + let registry = ShellProcessRegistry::new(); + let controller = Arc::new(RecordingTaskController::default()); + *lock(&controller.monitor_disposition) = Some(MonitorEventDisposition::AutoStop); + let context = context("session-a").with_task_runtime_controller(controller.clone()); + registry + .spawn_monitor( + &context, + configured_command(script), + script.into(), + "noisy events".into(), + "shell command (redacted)".into(), + None, + ) + .await + .unwrap(); + + let completion = wait_for_monitor_completion(&controller).await; + assert_eq!(completion.status, MonitorTaskCompletionStatus::AutoStopped); + } + #[test] fn output_ring_reports_stale_cursors() { let process_tree = Arc::new(ProcessTreeControl::finished_without_a_tree());