Limiting scope of signal handling based on environment

[bennothommo]

• Do not handle Linux signals if the PCNTL extension is unavailable. Artisan commands can still function without this extension - they just simply cannot handle termination signals. This prevents commands from throwing an error due to the missing SIGINT/SIGTERM/SIGQUIT constants.
• Prevent multiple registrations of the Windows signal handler if the method is called multiple times.
• Remove specific checks for Windows signals - the handler only accepts two types of signals (CTRL+C or CTRL+BREAK) and both are termination signals.

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Improved signal handler registration and cleanup mechanisms for more reliable application termination behavior across Windows and Unix-like systems.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[LukeTowers]

@bennothommo this looks fine to me, any issues with it going in 1.2.9? Also what's the PHPStan issue?

[bennothommo]

@LukeTowers no reason not to include it in 1.2.9 if you would prefer. The PHPStan issue seems to have occurred with all pushes recently to Storm - it doesn't seem to be able to correctly work out how our Singletons work. We can probably ignore it for now.

[github-actions]

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this is still being worked on, please respond and we will re-open this pull request.
If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

[coderabbitai]

Walkthrough

This change introduces defensive programming to the HandlesCleanup trait by adding state tracking for Windows signal handler registration and guarding signal operations with capability checks. A new windowsSignalHandlerRegistered flag prevents duplicate handler registration, while conditional checks ensure operations only proceed when required functions exist on the target platform.

Changes

Cohort / File(s)	Summary
Signal handler state tracking and guards src/Console/Traits/HandlesCleanup.php	Added protected flag to track Windows signal handler registration state; guarded handler registration with sapi_windows_set_ctrl_handler existence check; guarded Unix signal operations with pcntl_signal existence check; simplified cleanup invocation logic and changed exit behavior to unconditionally exit with code 0 after cleanup on Windows.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Areas requiring attention:

• Verify the state flag initialization and lifecycle management across signal handler registration/removal paths
• Confirm that guards for pcntl_signal and sapi_windows_set_ctrl_handler are placed in all necessary locations
• Test the impact of the changed exit behavior (unconditional exit code 0 after cleanup) on both platforms
• Validate that the simplified cleanup invocation logic doesn't introduce any unintended side effects on Windows

Poem

🐰 Windows signals now tracked with care,
No double handlers in the air!
Guards protect each sacred call,
Cleanup flows, then exit all. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding environment-based scope limiting to signal handling logic.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/sigint-missing

---

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: 0

🧹 Nitpick comments (1)

src/Console/Traits/HandlesCleanup.php (1)

71-87: LGTM!

The simplified logic treating all events as termination signals aligns with the PR objectives.

Consider prefixing the unused $event parameter with an underscore to signal intent:

-    public function handleWindowsSignal(int $event): void
+    public function handleWindowsSignal(int $_event): void

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between deb4304 and 9cb8c68.

📒 Files selected for processing (1)

• src/Console/Traits/HandlesCleanup.php (5 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: windows-latest / PHP 8.4
• GitHub Check: windows-latest / PHP 8.2
• GitHub Check: windows-latest / PHP 8.3
• GitHub Check: windows-latest / PHP 8.1

🔇 Additional comments (5)

src/Console/Traits/HandlesCleanup.php (5)

17-18: LGTM!

The new flag cleanly tracks Windows handler registration state and prevents duplicate registrations as intended.

---

32-35: LGTM!

Good defensive programming: the function existence check and idempotency flag correctly prevent duplicate handler registrations and handle environments where the Windows function is unavailable.

---

38-40: LGTM!

The guard correctly ensures signal constants are only referenced when the PCNTL extension is available, preventing undefined constant errors.

---

61-63: LGTM!

Good defensive check. While handleSignal shouldn't receive Unix signals when PCNTL is unavailable (since getSubscribedSignals returns an empty array), this guard ensures robustness if the method is ever called directly.

---

74-78: LGTM!

The flag-guarded removal logic is properly paired with the registration logic, ensuring the handler is only removed if it was previously registered.