Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bf6b5ad. Configure here.
| // for the child. This allows the child to handle signals and exit gracefully. | ||
| #[cfg(unix)] | ||
| let _guard = if self.signal_delegation { | ||
| Some(zpm_utils::IgnoreSigint::new()) |
There was a problem hiding this comment.
Signal guard set before spawn prevents child signal delivery
Low Severity
In run_script_inherited, the IgnoreSignals guard is created before cmd.status().await, which internally spawns the child. This means SIGINT and SIGTERM are set to SIG_IGN before the fork/exec, so the child inherits the ignored signal disposition and cannot receive these signals. This contradicts the documented constraint in explicit.rs: "We must spawn BEFORE setting SIG_IGN, otherwise the child inherits the ignored signal disposition and won't receive signals." The sibling method run_exec correctly creates the guard after cmd.spawn().
Reviewed by Cursor Bugbot for commit bf6b5ad. Configure here.
⏱️ Benchmark Resultsgatsby install-full-cold
📊 Raw benchmark data (gatsby install-full-cold)Base times: 4.420s, 4.397s, 4.422s, 4.441s, 4.414s, 4.500s, 4.358s, 4.490s, 4.373s, 4.519s, 4.230s, 4.365s, 4.432s, 4.420s, 4.269s, 4.408s, 4.362s, 4.380s, 4.386s, 4.333s, 4.369s, 4.301s, 4.413s, 4.348s, 4.431s, 4.345s, 4.431s, 4.495s, 4.429s, 4.357s Head times: 4.838s, 4.267s, 4.600s, 4.413s, 4.366s, 4.371s, 4.394s, 4.374s, 4.410s, 4.404s, 4.329s, 4.458s, 4.408s, 4.469s, 4.287s, 4.355s, 4.367s, 4.407s, 4.483s, 4.466s, 4.403s, 4.428s, 4.367s, 4.391s, 4.419s, 4.525s, 4.403s, 4.395s, 4.338s, 4.520s gatsby install-cache-only
📊 Raw benchmark data (gatsby install-cache-only)Base times: 1.267s, 1.243s, 1.227s, 1.247s, 1.240s, 1.250s, 1.268s, 1.250s, 1.217s, 1.236s, 1.253s, 1.238s, 1.248s, 1.220s, 1.245s, 1.254s, 1.236s, 1.244s, 1.247s, 1.247s, 1.253s, 1.238s, 1.239s, 1.254s, 1.243s, 1.232s, 1.241s, 1.261s, 1.237s, 1.232s Head times: 1.267s, 1.239s, 1.244s, 1.242s, 1.238s, 1.224s, 1.239s, 1.253s, 1.244s, 1.238s, 1.245s, 1.247s, 1.246s, 1.236s, 1.243s, 1.242s, 1.256s, 1.247s, 1.284s, 1.255s, 1.238s, 1.251s, 1.249s, 1.253s, 1.245s, 1.219s, 1.251s, 1.243s, 1.234s, 1.251s gatsby install-cache-and-lock (warm, with lockfile)
📊 Raw benchmark data (gatsby install-cache-and-lock (warm, with lockfile))Base times: 0.330s, 0.335s, 0.332s, 0.332s, 0.332s, 0.330s, 0.333s, 0.331s, 0.335s, 0.331s, 0.332s, 0.331s, 0.330s, 0.332s, 0.329s, 0.330s, 0.332s, 0.332s, 0.335s, 0.333s, 0.335s, 0.330s, 0.331s, 0.331s, 0.333s, 0.332s, 0.379s, 0.337s, 0.340s, 0.343s Head times: 0.333s, 0.333s, 0.329s, 0.333s, 0.331s, 0.332s, 0.338s, 0.334s, 0.332s, 0.332s, 0.338s, 0.337s, 0.333s, 0.334s, 0.334s, 0.336s, 0.333s, 0.331s, 0.338s, 0.335s, 0.337s, 0.333s, 0.339s, 0.340s, 0.338s, 0.336s, 0.338s, 0.334s, 0.336s, 0.334s |
merceyz
left a comment
merceyz
left a comment
There was a problem hiding this comment.
Tested this and can confirm it fixes the issue I was having.
2 participants
We were only catching SIGINT but not SIGTERM. Additionally, while Yarn Switch was catching the signals, Yarn wasn't when proxying commands.
Fixes #236
Note
Medium Risk
Changes Unix signal handling for proxied commands by ignoring SIGINT/SIGTERM in parent processes while waiting, which can affect termination behavior and exit-code propagation across multiple commands and environments.
Overview
Improves signal delegation for proxied executions so wrapper processes don’t die on SIGINT/SIGTERM and can correctly propagate the child process exit code.
Replaces the Unix-only
IgnoreSigintguard withIgnoreSignals(SIGINT+SIGTERM) and enablesenable_signal_delegation()acrosszpmcommand entrypoints (run,exec,node,python,dlx) andzpm-switchexplicit switching.Adds acceptance coverage to verify SIGINT and SIGTERM sent to the process group are handled by the child (including docker-style SIGTERM) and result in exit code 0.
Reviewed by Cursor Bugbot for commit bf6b5ad. Bugbot is set up for automated code reviews on this repo. Configure here.