Skip to content

Commit 9c715fe

Browse files
rubberduck203claude
andcommitted
fix: keep scope-intercept alive on SIGINT/SIGTERM/SIGHUP so child can clean up
When scope-intercept wraps a long-running process (e.g. as the shebang on a server start script), Ctrl+C in the terminal delivers SIGINT to every member of the foreground process group — both scope-intercept and the wrapped command. Without a custom signal handler, scope-intercept follows the default and aborts immediately, closing the captured stdout/stderr pipes. The child then dies with SIGPIPE before its own SIGINT trap can finish, leaving services like overmind/nginx orphaned. Install Tokio Unix signal listeners for SIGINT, SIGTERM, and SIGHUP at the start of run_command. Their only job is to consume the default- terminate behavior so scope-intercept stays alive while the child runs its trap. The OS already delivered the signal to the child via the shared process group, so no manual forwarding is needed. When an interrupt has fired and the child exits non-zero, skip the known-error analysis, retry, and bug-report flows — the user asked to quit, not to be quizzed. OutputCapture and the scope doctor path are intentionally untouched; this is scoped to the intercept binary, which is the one wrapping interactive long-lived workloads. Includes a new regression test that spawns scope-intercept in its own process group, sends SIGINT via killpg (mimicking a terminal), and asserts the child's bash trap ran and the exit code is 130. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent a7ebe62 commit 9c715fe

4 files changed

Lines changed: 139 additions & 0 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ which = { version = "8.0", features = ["regex"] }
8080
assert_cmd = "2.2.0"
8181
assert_fs = "1.1.3"
8282
escargot = "0.5.15"
83+
libc = "0.2"
8384
predicates = "3.1.4"
8485
tempfile = "3.27"
8586

src/bin/scope-intercept.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ use human_panic::setup_panic;
66
use std::env;
77
use std::io::Cursor;
88
use std::sync::Arc;
9+
use std::sync::atomic::{AtomicBool, Ordering};
910
use tokio::io::BufReader;
11+
use tokio::signal::unix::{SignalKind, signal};
1012
use tracing::{Level, enabled, error, info, warn};
1113

1214
/// A wrapper CLI that can be used to capture output from a program, check if there are known errors
@@ -76,6 +78,26 @@ async fn run_command(opts: Cli) -> anyhow::Result<i32> {
7678
let current_dir = std::env::current_dir()?;
7779
let path = env::var("PATH").unwrap_or_default();
7880

81+
// SIGINT/SIGTERM/SIGHUP are delivered to every member of our foreground
82+
// process group, including the child, so its own cleanup traps run. We
83+
// just need to keep ourselves alive long enough for the child to finish
84+
// shutting down — otherwise our captured stdout/stderr pipes close and
85+
// the child dies with SIGPIPE before its trap can complete.
86+
let interrupted = Arc::new(AtomicBool::new(false));
87+
for kind in [
88+
SignalKind::interrupt(),
89+
SignalKind::terminate(),
90+
SignalKind::hangup(),
91+
] {
92+
let mut stream = signal(kind)?;
93+
let interrupted = interrupted.clone();
94+
tokio::spawn(async move {
95+
while stream.recv().await.is_some() {
96+
interrupted.store(true, Ordering::SeqCst);
97+
}
98+
});
99+
}
100+
79101
let capture = OutputCapture::capture_output(CaptureOpts {
80102
working_dir: &current_dir,
81103
args: &command,
@@ -93,6 +115,13 @@ async fn run_command(opts: Cli) -> anyhow::Result<i32> {
93115
return Ok(exit_code);
94116
}
95117

118+
// The user explicitly asked to quit — don't surprise them with
119+
// known-error prompts, retry, or bug-report offers. Just propagate the
120+
// child's exit code (typically 130 for SIGINT, 143 for SIGTERM).
121+
if interrupted.load(Ordering::SeqCst) {
122+
return Ok(exit_code);
123+
}
124+
96125
error!(target: "user", "Command failed, checking for a known error");
97126
let found_config = opts.config_options.load_config().await.unwrap_or_else(|e| {
98127
error!(target: "user", "Unable to load configs from disk: {:?}", e);

tests/scope_intercept.rs

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
use assert_fs::fixture::{FileWriteStr, PathChild};
22
use predicates::boolean::PredicateBooleanExt;
33
use predicates::prelude::predicate;
4+
use std::os::unix::process::CommandExt;
5+
use std::process::Command as StdCommand;
6+
use std::time::{Duration, Instant};
47

58
#[allow(dead_code)]
69
mod common;
@@ -158,3 +161,108 @@ fn test_intercept_no_known_errors_match() {
158161

159162
helper.clean_work_dir();
160163
}
164+
165+
// When scope-intercept wraps a long-running interactive process (e.g.
166+
// used as a shebang on a server start script), Ctrl+C must let the
167+
// child run its own SIGINT trap and exit cleanly. Before this fix,
168+
// scope-intercept terminated immediately on SIGINT, closing the
169+
// captured stdout/stderr pipes and killing the child with SIGPIPE
170+
// before its trap could complete.
171+
#[test]
172+
fn test_intercept_forwards_sigint_and_waits_for_child_cleanup() {
173+
let helper = ScopeTestHelper::new(
174+
"test_intercept_forwards_sigint_and_waits_for_child_cleanup",
175+
"empty",
176+
);
177+
178+
let work_dir = helper.work_dir.path().to_path_buf();
179+
let cleanup_marker = work_dir.join("cleanup.done");
180+
let ready_marker = work_dir.join("ready");
181+
182+
helper
183+
.work_dir
184+
.child("trap.sh")
185+
.write_str(
186+
"#!/bin/bash\n\
187+
trap 'echo trapped > cleanup.done; exit 130' INT\n\
188+
touch ready\n\
189+
# sleep in short increments so the trap fires promptly\n\
190+
for _ in $(seq 1 30); do sleep 1; done\n",
191+
)
192+
.unwrap();
193+
194+
let intercept_bin = assert_cmd::cargo::cargo_bin("scope-intercept");
195+
let mut command = StdCommand::new(intercept_bin);
196+
command
197+
.current_dir(&work_dir)
198+
.env("NO_COLOR", "1")
199+
.args(["--", "bash", "trap.sh"]);
200+
// Put scope-intercept in its own process group so killpg(pid, SIGINT)
201+
// hits both it and the wrapped bash, mirroring how a terminal delivers
202+
// Ctrl+C to the foreground process group.
203+
unsafe {
204+
command.pre_exec(|| {
205+
if libc::setsid() == -1 {
206+
return Err(std::io::Error::last_os_error());
207+
}
208+
Ok(())
209+
});
210+
}
211+
let mut child = command.spawn().expect("failed to spawn scope-intercept");
212+
213+
// Wait for the bash trap to be installed.
214+
let deadline = Instant::now() + Duration::from_secs(5);
215+
while !ready_marker.exists() {
216+
if Instant::now() > deadline {
217+
child.kill().ok();
218+
panic!(
219+
"trap.sh never wrote ready marker; scope-intercept may not be running the script"
220+
);
221+
}
222+
std::thread::sleep(Duration::from_millis(50));
223+
}
224+
225+
let pid = child.id() as i32;
226+
// SAFETY: killpg(2) with a valid pgid and SIGINT delivers to every
227+
// member of the group. scope-intercept is the group leader (setsid
228+
// above), so this hits both it and the wrapped bash.
229+
let rc = unsafe { libc::killpg(pid, libc::SIGINT) };
230+
assert_eq!(
231+
rc,
232+
0,
233+
"killpg({pid}, SIGINT) failed: {}",
234+
std::io::Error::last_os_error()
235+
);
236+
237+
// Wait for scope-intercept to exit. If it doesn't shut down within 10s,
238+
// the bug is back: scope-intercept is hanging instead of letting the
239+
// child run its trap and propagating the exit.
240+
let exit_deadline = Instant::now() + Duration::from_secs(10);
241+
let status = loop {
242+
match child.try_wait().expect("try_wait failed") {
243+
Some(status) => break status,
244+
None => {
245+
if Instant::now() > exit_deadline {
246+
child.kill().ok();
247+
panic!("scope-intercept did not exit within 10s after SIGINT");
248+
}
249+
std::thread::sleep(Duration::from_millis(50));
250+
}
251+
}
252+
};
253+
254+
assert!(
255+
cleanup_marker.exists(),
256+
"child SIGINT trap did not run — cleanup.done was never created"
257+
);
258+
let body = std::fs::read_to_string(&cleanup_marker).unwrap();
259+
assert_eq!(body.trim(), "trapped");
260+
261+
assert_eq!(
262+
status.code(),
263+
Some(130),
264+
"scope-intercept should propagate the child's exit code (130 for SIGINT), got {status:?}"
265+
);
266+
267+
helper.clean_work_dir();
268+
}

0 commit comments

Comments
 (0)