Skip to content

Improves signal handling#293

Merged
arcanis merged 2 commits into
mainfrom
mael/signals
May 7, 2026
Merged

Improves signal handling#293
arcanis merged 2 commits into
mainfrom
mael/signals

Conversation

@arcanis
Copy link
Copy Markdown
Member

@arcanis arcanis commented May 1, 2026

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 multiple command execution paths, which can affect how long-running processes terminate and what exit codes are propagated (especially under Ctrl-C or container shutdown). Scope is limited and covered by new acceptance tests, but regressions could impact CLI usability.

Overview
Improves signal delegation so wrapper/parent processes ignore both SIGINT and SIGTERM while waiting on spawned children, allowing the child process to handle shutdown and ensuring the child’s exit code is propagated.

This replaces IgnoreSigint with IgnoreSignals and enables enable_signal_delegation() across more execution entrypoints (e.g. dlx, exec, node, python, run), plus adjusts inherited-stdio script execution to wait on the spawned child consistently.

Adds acceptance tests for switch proxy to verify process-group SIGINT and SIGTERM are correctly delegated and result in clean exit code 0.

Reviewed by Cursor Bugbot for commit fbb67f4. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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.

Comment thread packages/zpm/src/script.rs
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

⏱️ Benchmark Results

gatsby install-full-cold

Metric Base Head Difference
Mean 4.464s 4.471s +0.15% ⚠️
Median 4.458s 4.460s +0.04% ⚠️
Min 4.290s 4.356s
Max 4.574s 4.626s
Std Dev 0.061s 0.067s
📊 Raw benchmark data (gatsby install-full-cold)

Base times: 4.491s, 4.495s, 4.507s, 4.393s, 4.449s, 4.440s, 4.512s, 4.435s, 4.412s, 4.457s, 4.486s, 4.485s, 4.492s, 4.454s, 4.431s, 4.342s, 4.456s, 4.390s, 4.416s, 4.457s, 4.574s, 4.538s, 4.550s, 4.523s, 4.509s, 4.495s, 4.533s, 4.290s, 4.459s, 4.450s

Head times: 4.533s, 4.377s, 4.452s, 4.429s, 4.409s, 4.462s, 4.483s, 4.413s, 4.458s, 4.447s, 4.504s, 4.417s, 4.418s, 4.356s, 4.453s, 4.393s, 4.386s, 4.571s, 4.486s, 4.558s, 4.437s, 4.538s, 4.397s, 4.571s, 4.543s, 4.487s, 4.485s, 4.549s, 4.626s, 4.486s


gatsby install-cache-only

Metric Base Head Difference
Mean 1.269s 1.287s +1.42% ⚠️
Median 1.266s 1.287s +1.65% ⚠️
Min 1.227s 1.247s
Max 1.306s 1.323s
Std Dev 0.019s 0.019s
📊 Raw benchmark data (gatsby install-cache-only)

Base times: 1.292s, 1.303s, 1.267s, 1.227s, 1.262s, 1.261s, 1.268s, 1.281s, 1.277s, 1.288s, 1.250s, 1.275s, 1.267s, 1.291s, 1.284s, 1.306s, 1.305s, 1.255s, 1.256s, 1.278s, 1.257s, 1.250s, 1.263s, 1.252s, 1.269s, 1.259s, 1.265s, 1.240s, 1.251s, 1.265s

Head times: 1.290s, 1.272s, 1.282s, 1.262s, 1.263s, 1.288s, 1.304s, 1.302s, 1.311s, 1.298s, 1.302s, 1.323s, 1.286s, 1.291s, 1.309s, 1.295s, 1.316s, 1.303s, 1.309s, 1.284s, 1.288s, 1.273s, 1.285s, 1.261s, 1.271s, 1.270s, 1.256s, 1.247s, 1.283s, 1.282s


gatsby install-cache-and-lock (warm, with lockfile)

Metric Base Head Difference
Mean 0.340s 0.351s +3.10% ⚠️
Median 0.340s 0.349s +2.61% ⚠️
Min 0.329s 0.338s
Max 0.355s 0.376s
Std Dev 0.007s 0.010s
📊 Raw benchmark data (gatsby install-cache-and-lock (warm, with lockfile))

Base times: 0.340s, 0.354s, 0.337s, 0.336s, 0.336s, 0.335s, 0.336s, 0.344s, 0.347s, 0.355s, 0.343s, 0.345s, 0.352s, 0.347s, 0.340s, 0.344s, 0.341s, 0.348s, 0.332s, 0.329s, 0.334s, 0.332s, 0.337s, 0.337s, 0.335s, 0.343s, 0.339s, 0.333s, 0.341s, 0.342s

Head times: 0.348s, 0.342s, 0.344s, 0.349s, 0.351s, 0.347s, 0.341s, 0.341s, 0.338s, 0.338s, 0.340s, 0.339s, 0.346s, 0.342s, 0.347s, 0.358s, 0.342s, 0.349s, 0.355s, 0.355s, 0.358s, 0.353s, 0.363s, 0.364s, 0.371s, 0.358s, 0.359s, 0.358s, 0.376s, 0.362s

Copy link
Copy Markdown
Member

@merceyz merceyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this and can confirm it fixes the issue I was having.

@arcanis arcanis merged commit df93cf0 into main May 7, 2026
19 of 20 checks passed
@arcanis arcanis deleted the mael/signals branch May 7, 2026 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Yarn Switch terminates before the child process does

2 participants