fix(openInEditor): disable spawnSync signal forwarding for editor helper

[fallintoplace]

What changed

• Add a Unix forward_signals option to sync spawn options, defaulting to true.
• Guard the POSIX sync-spawn signal forwarding state behind that option.
• Disable signal forwarding for the detached Bun.openInEditor() editor helper spawn.
• Add a regression test that verifies a JS SIGUSR2 handler still runs while the editor helper is waiting.

Why

Bun.openInEditor() uses sync::spawn() only as a blocking implementation detail inside a detached helper thread. It should not install the foreground bun run / bunx signal-forwarding handlers process-wide while user JS continues running.

Fixes #31194

Testing

• git diff --check
• cargo fmt --all --check
• npx --yes prettier@3.6.2 --check --config .prettierrc test/js/bun/util/open-in-editor-gc.test.ts

Not run locally:

• bun bd test test/js/bun/util/open-in-editor-gc.test.ts because no bun binary is installed in this checkout.
• cargo check -p bun_spawn --keep-going because this plain checkout does not have the generated/fetched vendor/lolhtml/c-api path dependency.

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fe1c6e82-73f0-4e75-998e-b36f41d47bae

📥 Commits

Reviewing files that changed from the base of the PR and between 661705d and f91f9cb.

📒 Files selected for processing (3)

• src/runtime/cli/open.rs
• src/spawn/process.rs
• test/js/bun/util/open-in-editor-gc.test.ts

---

Walkthrough

This PR adds a forward_signals option to Unix sync spawn that gates signal-handler registration. openInEditor() passes forward_signals: false to prevent its detached helper thread from installing process-global signal-forwarding handlers that would interfere with user code. A new test verifies user signal handlers are not stolen.

Changes

Signal forwarding gate for sync spawn

Layer / File(s)	Summary
Signal forwarding option definition and conditional gating src/spawn/process.rs	sync::Options gains a Unix-only forward_signals: bool field defaulting to true. Three conditional blocks in spawn_posix gate signal-forwarding initialization, PID tracking, and pending-signal dispatch based on this option.
Disable signal forwarding in openInEditor src/runtime/cli/open.rs	The auto_close function in Editor::open passes forward_signals: false to the sync::spawn call that waits for the editor process, preventing signal-handler installation in the detached helper thread.
Signal handler preservation test test/js/bun/util/open-in-editor-gc.test.ts	A new Linux-only test verifies that Bun.openInEditor does not steal process signal handlers: a subprocess registers a SIGUSR2 handler, invokes openInEditor with a temporary editor script, sends the signal, and asserts the handler was invoked and the process exits cleanly.

Possibly related PRs

• oven-sh/bun#31183: Adjusts spawnSync signal-forwarding to exclude SIGPWR, complementing the signal-forwarding behavior gating introduced here.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately describes the main change: disabling signal forwarding for the openInEditor helper's spawnSync.
Description check	✅ Passed	The description covers the changes, rationale, and testing approach, though it does not precisely follow the repository template structure.
Linked Issues check	✅ Passed	The code changes directly address the objectives from issue #31194 by adding the forward_signals option and disabling it for the openInEditor helper.
Out of Scope Changes check	✅ Passed	All changes are scoped to the stated objectives: Unix signal forwarding control in sync spawn and a regression test for openInEditor.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/bun/util/open-in-editor-gc.test.ts`:
- Around line 68-71: The helper script exits immediately because the final line
uses exec ${shellQuote(sleepPath)} "$1" where $1 is a temp-file path
(non-numeric), so sleep returns and the fake editor dies right after printing
ready; update the script template in open-in-editor-gc.test.ts to keep the fake
editor alive after printing ready by exec'ing sleep with a long numeric duration
(e.g., exec ${shellQuote(sleepPath)} 9999999) or another blocking command,
ensuring references to sleepPath and ready in the template are preserved.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ba9017af-5776-48a0-9e24-a72c0196cbdc

📥 Commits

Reviewing files that changed from the base of the PR and between 832edbc and 661705d.

📒 Files selected for processing (3)

• src/runtime/cli/open.rs
• src/spawn/process.rs
• test/js/bun/util/open-in-editor-gc.test.ts

[robobun]

Thanks for the report and the fix, and for the test scenario. I consolidated the three open fixes for #31194 into #31297. It takes a slightly different route: instead of adding a forward_signals option to the sync spawn options, the editor thread stops using the spawnSync machinery altogether and goes through bun_core::spawn_sync_inherit (plain spawn and wait, which is what the pre-Rust implementation did). That keeps the forwarding code with exactly one caller, and as a side effect also fixes the PATH lookup of the xdg-open opener and the per-open event loop allocation on Windows. This branch had also picked up a conflict in src/spawn/process.rs in the meantime.

Your SIGUSR2 listener scenario is in #31297 as the "process.on signal handler still runs while an editor is up" test, with a Co-authored-by credit to you on that commit. Closing this one in favor of #31297.