Fix Windows serve without signal support#92
Conversation
📝 WalkthroughWalkthroughAdds a ChangesRuntime capability detection and serve wiring
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ServeCommand
participant ServeRuntimeCapabilities
participant EventLoop
User->>ServeCommand: run serve command
ServeCommand->>ServeRuntimeCapabilities: supportsWorkerPool()
ServeRuntimeCapabilities-->>ServeCommand: true/false
alt supportsWorkerPool true
ServeCommand->>ServeCommand: fork worker pool
else false
ServeCommand->>ServeCommand: run single server process
end
ServeCommand->>ServeRuntimeCapabilities: supportsEventLoopSignals()
ServeRuntimeCapabilities-->>ServeCommand: true/false
alt supportsEventLoopSignals true
ServeCommand->>EventLoop: register SIGINT/SIGTERM handlers
end
Related issues: Suggested labels: windows, bug, enhancement Suggested reviewers: samdark, vjik 🐰 A rabbit hops through PCNTL's maze, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes yiipress serve on Windows by detecting missing PCNTL/signal support at runtime and conditionally disabling prefork workers and ReactPHP signal handlers, preventing crashes like “Undefined constant SIGINT”. It also updates documentation and marks the roadmap item as complete.
Changes:
- Introduce
ServeRuntimeCapabilitiesto centralize runtime feature checks (PCNTL, POSIX signals, Windows separators). - Gate prefork worker mode and event-loop signal hooks in
ServeCommandbehind capability checks. - Add unit tests and update docs/roadmap to reflect Windows single-process behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Console/ServeRuntimeCapabilitiesTest.php | Adds unit coverage for capability combinations (Windows-like, missing signal constants, missing PCNTL pieces). |
| src/Console/ServeRuntimeCapabilities.php | New runtime capability helper used to decide whether to enable signals and worker pool. |
| src/Console/ServeCommand.php | Uses runtime capabilities to avoid SIGINT/SIGTERM and prefork logic on unsupported runtimes. |
| roadmap.md | Marks the Windows-compatible serve roadmap item as completed. |
| docs/preview.md | Documents Windows single-process preview behavior and conditional use of workers/signals. |
| docs/commands.md | Clarifies --workers behavior on POSIX vs Windows/unsupported runtimes. |
| docs/binaries-phar-docker.md | Updates packaging/runtime docs to describe single-process Windows behavior and conditional preforking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/Unit/Console/ServeRuntimeCapabilitiesTest.php (1)
14-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test isolating the
directorySeparatorcheck insupportsWorkerPool().This test combines
directorySeparator: '\\'withsigintDefined: false/sigtermDefined: false. SincesupportsEventLoopSignals()already returnsfalsefrom the signal flags alone,supportsWorkerPool()returnsfalseregardless of the directory-separator branch — the actual$this->directorySeparator !== '\\'short-circuit (the core Windows-detection logic this PR introduces) is never exercised in isolation.As per coding guidelines,
**/*Test.php: "For each piece of code add a test using phpunit."✅ Suggested additional test
#[Test] public function windowsDirectorySeparatorAloneDisablesWorkerPool(): void { $capabilities = new ServeRuntimeCapabilities( directorySeparator: '\\', sigintDefined: true, sigtermDefined: true, pcntlAsyncSignalsAvailable: true, pcntlForkAvailable: true, pcntlSignalDispatchAvailable: true, pcntlSignalAvailable: true, pcntlWaitAvailable: true, pcntlWaitStatusAvailable: true, posixKillAvailable: true, ); self::assertFalse($capabilities->supportsWorkerPool()); self::assertTrue($capabilities->supportsEventLoopSignals()); }🤖 Prompt for 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. In `@tests/Unit/Console/ServeRuntimeCapabilitiesTest.php` around lines 14 - 31, Add a focused PHPUnit test for ServeRuntimeCapabilities::supportsWorkerPool() that isolates the directorySeparator check by keeping the signal flags enabled while setting directorySeparator to '\\'. The current windowsRuntimeDoesNotSupportWorkerPoolOrEventLoopSignals() test also disables event-loop signals, so it does not prove the Windows short-circuit in supportsWorkerPool(). Create a new test in ServeRuntimeCapabilitiesTest that asserts supportsWorkerPool() is false solely because of the Windows directory separator, and verify supportsEventLoopSignals() remains true in that setup.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@tests/Unit/Console/ServeRuntimeCapabilitiesTest.php`:
- Around line 14-31: Add a focused PHPUnit test for
ServeRuntimeCapabilities::supportsWorkerPool() that isolates the
directorySeparator check by keeping the signal flags enabled while setting
directorySeparator to '\\'. The current
windowsRuntimeDoesNotSupportWorkerPoolOrEventLoopSignals() test also disables
event-loop signals, so it does not prove the Windows short-circuit in
supportsWorkerPool(). Create a new test in ServeRuntimeCapabilitiesTest that
asserts supportsWorkerPool() is false solely because of the Windows directory
separator, and verify supportsEventLoopSignals() remains true in that setup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d1bcd25c-f364-4feb-aff7-8f5f4e52442f
📒 Files selected for processing (7)
docs/binaries-phar-docker.mddocs/commands.mddocs/preview.mdroadmap.mdsrc/Console/ServeCommand.phpsrc/Console/ServeRuntimeCapabilities.phptests/Unit/Console/ServeRuntimeCapabilitiesTest.php
Summary
Closes #87.
Tests
Summary by CodeRabbit
New Features
servenow adapts automatically to platform capabilities, supporting Windows with a single server process while using prefork workers where available.Bug Fixes
Documentation