-
-
Notifications
You must be signed in to change notification settings - Fork 2
feature(app): Implement RoadRunner
stack.
#132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughReplaces Docker-based setup with RoadRunner. Adds RoadRunner config and DI bindings, updates bootstrap to run a stateless Yii app under RoadRunner, and adjusts modules. Simplifies CI Codeception run with a RoadRunner hook. Removes dotenv, c3/coverage settings, and multiple Docker files. Updates README and tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant RR as RoadRunner HTTP
participant PSR as PSR-7 Worker
participant App as Yii StatelessApplication
participant C as Controller
User->>RR: HTTP request
RR->>PSR: Dispatch PSR-7 ServerRequest
PSR->>App: Handle request
App->>C: Invoke action
C-->>App: Response payload
App-->>PSR: PSR-7 Response
PSR-->>RR: Return response
RR-->>User: HTTP response
note right of RR: Configured via .rr.yaml (static/gzip, pool, ports)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
…g for `c3.php` in `index.php`.
…pBrowser URL in `Acceptance.suite.yml`.
… on all interfaces.
…`C3` file in index.php if `YII_C3` is set.
… from build files.
…pported web server section.
…SSL support mention.
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (10)
tests/Acceptance.suite.yml (1)
10-10
: Add trailing slash to base URL to avoid edge path joins.Some Codeception helpers expect a trailing slash for clean relative path resolution. This also matches common examples.
- url: http://127.0.0.1:8080 + url: http://127.0.0.1:8080/config/web/modules.php (1)
5-7
: Alias Gii Module to avoid ambiguous bareModule
import.Using
Module
unqualified is easy to confuse and collide with local symbols. Alias for clarity.-use yii\gii\Module; +use yii\gii\Module as GiiModule; ... - 'class' => Module::class, + 'class' => GiiModule::class,Also applies to: 15-15
.rr.yaml (4)
8-10
: Avoid enabling coverage flag and debug unconditionally.YII_C3=true implies c3 coverage, but CI disables coverage. Also hardcoding debug/dev can leak into prod if this file is reused.
- YII_C3: true - YII_DEBUG: true - YII_ENV: dev +# Uncomment locally as needed; keep CI/prod lean. +# YII_C3: true +# YII_DEBUG: true +# YII_ENV: devConfirm c3 bootstrap still exists; if removed, drop YII_C3 entirely.
18-21
: Middleware order OK; consider adding only when needed.Static first + gzip is fine. For prod, add/cache headers or CDN instead of global no-cache if you change that later.
31-35
: Remove Jobs plugin if unused to speed startup and reduce noise.-jobs: - pool: - num_workers: 2 - max_worker_memory: 100 - consume: { } +# jobs: # enable when you add pipelines +# ...
41-42
: Metrics port may conflict on shared runners.If not scraping metrics in CI, disable or change to an ephemeral port.
-metrics: - address: '127.0.0.1:2112' +# metrics: +# address: '127.0.0.1:2112'.github/workflows/build.yml (1)
27-29
: Coverage config is inconsistent (driver: none, yet pcov enabled).Either re-enable coverage or drop pcov to speed up CI. If you intend no coverage, remove pcov.
- codeception-command: vendor/bin/codecept run - coverage-driver: none + codeception-command: vendor/bin/codecept run + coverage-driver: none coverage-file: runtime/output/coverage.xml - extensions: gd, intl, pcov, sockets + extensions: gd, intl, socketsIf you want Codecov uploads, set coverage-driver to pcov and restore the Codeception coverage flags.
Also applies to: 30-30
composer.json (1)
41-41
: Consider using stable worker-debug version when available.The
yii2-extensions/worker-debug
package is set to^0.1
which appears to be an early version. Monitor for stable releases.README.md (2)
9-11
: Fix markdown emphasis usage.The static analysis correctly identified that emphasis is used instead of proper heading structure.
Based on the static analysis hint, consider whether this badge section should use proper heading markup if it represents a distinct section, or keep as emphasis if it's intentionally styled differently.
71-72
: Update installation command for clarity.The installation command references a specific development branch which may not be intuitive for users.
Consider updating the installation command to be more explicit about the RoadRunner version:
-composer create-project --prefer-dist --stability=dev yii2-extensions/app-basic:dev-road-runner app-basic +composer create-project --prefer-dist --stability=dev yii2-extensions/app-basic app-basicOr provide clearer documentation about when to use the specific branch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
composer.lock
is excluded by!**/*.lock
docker/ssl/localhost-key.pem
is excluded by!**/*.pem
docker/ssl/localhost.pem
is excluded by!**/*.pem
📒 Files selected for processing (39)
.env
(0 hunks).github/workflows/build.yml
(1 hunks).github/workflows/docker.yml
(0 hunks).rr.yaml
(1 hunks)README.md
(3 hunks)codeception.yml
(0 hunks)composer.json
(3 hunks)config/web/app.php
(2 hunks)config/web/container.php
(1 hunks)config/web/modules.php
(1 hunks)docker-compose.caddy.yml
(0 hunks)docker-compose.frankenphp.yml
(0 hunks)docker-compose.nginx.yml
(0 hunks)docker-compose.yml
(0 hunks)docker/apache/Dockerfile
(0 hunks)docker/apache/apache.conf
(0 hunks)docker/apache/vhost.conf
(0 hunks)docker/caddy/Caddyfile
(0 hunks)docker/caddy/Dockerfile
(0 hunks)docker/entrypoint.sh
(0 hunks)docker/frankenphp/Caddyfile
(0 hunks)docker/frankenphp/Dockerfile
(0 hunks)docker/init.sh
(0 hunks)docker/nginx/Dockerfile
(0 hunks)docker/nginx/default.conf
(0 hunks)docker/nginx/nginx.conf
(0 hunks)docker/php/php.ini
(0 hunks)docker/supervisor/conf.d/apache2.conf
(0 hunks)docker/supervisor/conf.d/caddy-php-fpm.conf
(0 hunks)docker/supervisor/conf.d/frankenphp.conf
(0 hunks)docker/supervisor/conf.d/nginx.conf
(0 hunks)docker/supervisor/conf.d/php-fpm.conf
(0 hunks)docker/supervisor/conf.d/queue.conf
(0 hunks)docker/supervisor/supervisord.conf
(0 hunks)src/usecase/site/SiteController.php
(2 hunks)tests/Acceptance.suite.yml
(1 hunks)tests/_envs/dockerized.yml
(0 hunks)tests/_envs/php-builtin.yml
(0 hunks)web/index.php
(1 hunks)
💤 Files with no reviewable changes (29)
- docker/frankenphp/Dockerfile
- docker-compose.yml
- docker/supervisor/conf.d/caddy-php-fpm.conf
- docker/supervisor/conf.d/nginx.conf
- docker/nginx/default.conf
- docker/supervisor/conf.d/queue.conf
- tests/_envs/php-builtin.yml
- docker/init.sh
- docker/supervisor/conf.d/php-fpm.conf
- codeception.yml
- docker/supervisor/conf.d/frankenphp.conf
- docker/supervisor/supervisord.conf
- docker-compose.frankenphp.yml
- tests/_envs/dockerized.yml
- .github/workflows/docker.yml
- docker/apache/vhost.conf
- docker-compose.nginx.yml
- docker/php/php.ini
- docker/entrypoint.sh
- docker/supervisor/conf.d/apache2.conf
- docker/frankenphp/Caddyfile
- docker/apache/apache.conf
- docker/apache/Dockerfile
- docker/nginx/Dockerfile
- docker/nginx/nginx.conf
- .env
- docker-compose.caddy.yml
- docker/caddy/Dockerfile
- docker/caddy/Caddyfile
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-30T20:31:14.913Z
Learnt from: terabytesoftw
PR: yii2-extensions/app-basic#129
File: tests/_envs/php-builtin.yml:4-4
Timestamp: 2025-08-30T20:31:14.913Z
Learning: Yii2 framework doesn't require separate router scripts for PHP's built-in server. Yii2 handles routing through URL rules in configuration files and controller/action mapping conventions, not separate route definition files. If tests pass with the built-in server setup, additional router scripts are unnecessary.
Applied to files:
web/index.php
🪛 markdownlint-cli2 (0.17.2)
README.md
75-75: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (12)
src/usecase/site/SiteController.php (1)
10-11
: Drop the deep-merge suggestion—parent::actions()
returns only a flat, one-level config, soarray_merge
is sufficient.Likely an incorrect or invalid review comment.
config/web/modules.php (1)
10-10
: LGTM: Debug module correctly wired for dev.WorkerDebugModule registration under the dev-only gate is appropriate.
.rr.yaml (3)
12-12
: Port matches tests; good alignment.
22-26
: Good: PHP files forbidden from static serving.
28-30
: Worker memory cap: verify units and headroom.100 likely means ~100 MiB. Ensure this is sufficient for Yii + debug in CI to avoid premature restarts.
config/web/container.php (1)
1-37
: LGTM! Well-structured DI container configuration for RoadRunner integration.The container definitions are properly structured with correct PSR-7 factory bindings and RoadRunner worker configuration. The use of
Instance::of()
for dependency injection ensures proper lazy loading of dependencies.composer.json (2)
13-13
: Verify and confirm dev stability dependencies
Therequire
section contains an@dev
constraint (yii2-extensions/road-runner: ^0.1.0@dev
), and yourcomposer.lock
lists dozens of packages at dev stability (e.g.cebe/markdown: dev-master
,symfony/console: 7.4.x-dev
,yiisoft/yii2: dev-master
, etc.). Ensure this is intentional for production or adjustminimum-stability
/prefer-stable
and pin stable releases accordingly.
17-17
: Confirm PSR-7 & RoadRunner compatibility
- httpsoft/http-message ^1.0 resolves up to v1.1.6 and supports PHP ^7.4|^8.0 (includes PHP 8.1+) (packagist.org)
- yii2-extensions/road-runner requires PHP >=8.1, ext-sockets, psr/http-factory ^1.1, psr/http-message ^2.0, spiral/roadrunner ^2025.1.2 and yii2-extensions/psr-bridge; ensure these deps are in composer.json and test under RoadRunner v3+ (packagist.org)
No further changes required.
README.md (1)
125-125
: LGTM! Good practice using final class.Declaring the controller as
final
prevents unintended inheritance and aligns with modern PHP best practices.web/index.php (1)
17-19
: LGTM! Clean RoadRunner integration.The RoadRunner bootstrap is properly implemented with the stateless application pattern, which is appropriate for worker-based environments.
config/web/app.php (2)
7-14
: LGTM! Improved type annotations and container integration.The PHPStan type annotations are properly updated to reflect the actual mixed content of configuration arrays, and the container integration is cleanly implemented.
30-30
: Verify container configuration compatibility with Yii2 DI container
Confirm that supplying your custom$container
inconfig/web/app.php:30
aligns withyii\base\Application::$container
expectations and doesn’t override or break Yii2’s built-in dependency definitions.
hook: | | ||
vendor/bin/rr get-binary | ||
./rr serve > /dev/null 2>&1 & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Wait for RoadRunner to be ready to avoid flaky tests.
Add a simple readiness check after starting the server.
hook: |
vendor/bin/rr get-binary
- ./rr serve > /dev/null 2>&1 &
+ ./rr serve > /dev/null 2>&1 &
+ for i in {1..30}; do
+ curl -fsS http://127.0.0.1:8080/ >/dev/null && break
+ sleep 1
+ done
+ curl -fsS http://127.0.0.1:8080/ >/dev/null
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
hook: | | |
vendor/bin/rr get-binary | |
./rr serve > /dev/null 2>&1 & | |
hook: | | |
vendor/bin/rr get-binary | |
./rr serve > /dev/null 2>&1 & | |
for i in {1..30}; do | |
curl -fsS http://127.0.0.1:8080/ >/dev/null && break | |
sleep 1 | |
done | |
curl -fsS http://127.0.0.1:8080/ >/dev/null |
🤖 Prompt for AI Agents
In .github/workflows/build.yml around lines 31 to 33, the workflow starts
RoadRunner but does not wait for it to become ready which causes flaky tests;
after starting the server (./rr serve > /dev/null 2>&1 &) add a simple readiness
loop that polls the RoadRunner health or HTTP endpoint (e.g., curl -sSf
http://127.0.0.1:8080/health or the configured port) with a short sleep/retry
and a total timeout, exiting with failure if the server does not become ready in
time, so subsequent steps run only after RoadRunner is confirmed healthy.
define('YII_DEBUG', getenv('YII_DEBUG') ?? false); | ||
define('YII_ENV', getenv('YII_ENV') ?? 'prod'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve environment variable handling.
The current implementation uses loose type coercion that could lead to unexpected behavior. String values like "false"
or "0"
will be truthy in PHP.
-define('YII_DEBUG', getenv('YII_DEBUG') ?? false);
-define('YII_ENV', getenv('YII_ENV') ?? 'prod');
+define('YII_DEBUG', filter_var(getenv('YII_DEBUG'), FILTER_VALIDATE_BOOLEAN));
+define('YII_ENV', getenv('YII_ENV') ?: 'prod');
This ensures YII_DEBUG
properly handles boolean string values and YII_ENV
uses a more explicit fallback.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
define('YII_DEBUG', getenv('YII_DEBUG') ?? false); | |
define('YII_ENV', getenv('YII_ENV') ?? 'prod'); | |
define('YII_DEBUG', filter_var(getenv('YII_DEBUG'), FILTER_VALIDATE_BOOLEAN)); | |
define('YII_ENV', getenv('YII_ENV') ?: 'prod'); |
🤖 Prompt for AI Agents
In web/index.php around lines 10-11, replace the loose getenv() ?? fallback with
explicit handling: for YII_DEBUG, check if getenv('YII_DEBUG') returns a
non-false value and convert that string to a boolean using filter_var(...,
FILTER_VALIDATE_BOOLEAN) (so strings like "false"/"0" become false); for
YII_ENV, check getenv('YII_ENV') !== false and use that value, otherwise fall
back to 'prod'. Ensure you do not rely on PHP's truthiness of strings and handle
the getenv false return explicitly.
Summary by CodeRabbit
New Features
Documentation
Refactor
Tests
Chores