From 741103dc9a94ae241b5a588cf0f0d2fcdd759352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Tue, 14 Apr 2026 23:19:21 +0200 Subject: [PATCH 1/4] fix: remove sleep(1) hack in SupervisorWorker and add proper logging --- src/Worker/SupervisorWorker.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Worker/SupervisorWorker.php b/src/Worker/SupervisorWorker.php index 3ad06f9..25341de 100644 --- a/src/Worker/SupervisorWorker.php +++ b/src/Worker/SupervisorWorker.php @@ -40,8 +40,8 @@ public function __construct(KernelFactory $kernelFactory, ?string $user, ?string $handler = $kernel->getContainer()->get('workerman.process_handler'); $method = empty($serviceConfig['method']) ? '__invoke' : $serviceConfig['method']; $handler("$serviceId::$method", $taskName); - sleep(1); - exit; + $worker->log("Process $taskName finished unexpectedly"); + exit(1); }; } } From 9840a02f3a32240847cd5ab819a5ff71563e022a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Tue, 14 Apr 2026 23:22:05 +0200 Subject: [PATCH 2/4] docs: add design spec for configurable cache warmup timeout (#142) --- ...onfigurable-cache-warmup-timeout-design.md | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 docs/superpowers/specs/2026-04-14-configurable-cache-warmup-timeout-design.md diff --git a/docs/superpowers/specs/2026-04-14-configurable-cache-warmup-timeout-design.md b/docs/superpowers/specs/2026-04-14-configurable-cache-warmup-timeout-design.md new file mode 100644 index 0000000..0f03170 --- /dev/null +++ b/docs/superpowers/specs/2026-04-14-configurable-cache-warmup-timeout-design.md @@ -0,0 +1,71 @@ +# Configurable Cache Warmup Timeout + +## Context + +PR #141 introduced a `CACHE_WARMUP_TIMEOUT` constant (30 seconds) in `Runner.php` for the forked cache warmup process timeout. Currently this value is hardcoded and cannot be changed without modifying the source. Issue #142 tracks this. + +## Requirements + +- Make the cache warmup timeout configurable via bundle config +- Allow env var override (`WORKERMAN_CACHE_WARMUP_TIMEOUT`) for quick operational changes +- Priority: env var > bundle config > default (30) +- Maintain backward compatibility — default of 30 must work without any configuration + +## Design + +### 1. Bundle config + +Add `cache_warmup_timeout` node in `src/config/configuration.php`: + +```php +->integerNode('cache_warmup_timeout')->defaultValue(30)->end() +``` + +Users configure in `workerman.yaml`: +```yaml +workerman: + cache_warmup_timeout: 60 +``` + +### 2. Data flow + +``` +bundle config → services.php → Runtime options → Resolver → Runtime::getRunner() → Runner constructor +``` + +- `services.php` passes `cache_warmup_timeout` from resolved config into Runtime options +- `Runtime::getRunner()` reads the option and passes it to `new Runner($kernelFactory, $timeout)` +- `KernelFactory` is unchanged — timeout does not flow through it + +### 3. Runner constructor + +```php +public function __construct( + private KernelFactory $kernelFactory, + private int $cacheWarmupTimeout = 30, +) { + if (isset($_SERVER['WORKERMAN_CACHE_WARMUP_TIMEOUT'])) { + $this->cacheWarmupTimeout = (int) $_SERVER['WORKERMAN_CACHE_WARMUP_TIMEOUT']; + } +} +``` + +Priority: env var > bundle config (injected via constructor) > default 30. + +Remove the `CACHE_WARMUP_TIMEOUT` constant. Replace `$timeout = self::CACHE_WARMUP_TIMEOUT` with `$timeout = $this->cacheWarmupTimeout`. + +### 4. Files to change + +| File | Change | +|------|--------| +| `src/config/configuration.php` | Add `cache_warmup_timeout` integer node (default 30) | +| `src/config/services.php` | Pass `cache_warmup_timeout` from config to Runtime options | +| `src/Runtime.php` | Pass timeout to `new Runner()` in `getRunner()` | +| `src/Runner.php` | Remove constant, add constructor param, add env var fallback | +| `tests/RunnerTest.php` | Update structural test, add tests for constructor and env var | + +### 5. Tests + +- Update `testRunnerUsesCorrectForkErrorHandling` — verify `cacheWarmupTimeout` property exists instead of `CACHE_WARMUP_TIMEOUT` constant +- Add test for env var override (`WORKERMAN_CACHE_WARMUP_TIMEOUT` takes precedence) +- Add test for constructor injection (default value = 30) \ No newline at end of file From f981546e5799814c3bed36176b633a26afce615d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Tue, 14 Apr 2026 23:24:00 +0200 Subject: [PATCH 3/4] docs: add implementation plan for configurable cache warmup timeout --- ...04-14-configurable-cache-warmup-timeout.md | 199 ++++++++++++++++++ 1 file changed, 199 insertions(+) create mode 100644 docs/superpowers/plans/2026-04-14-configurable-cache-warmup-timeout.md diff --git a/docs/superpowers/plans/2026-04-14-configurable-cache-warmup-timeout.md b/docs/superpowers/plans/2026-04-14-configurable-cache-warmup-timeout.md new file mode 100644 index 0000000..4467fbe --- /dev/null +++ b/docs/superpowers/plans/2026-04-14-configurable-cache-warmup-timeout.md @@ -0,0 +1,199 @@ +# Configurable Cache Warmup Timeout — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make the cache warmup timeout configurable via env var `WORKERMAN_CACHE_WARMUP_TIMEOUT` with a default of 30 seconds, and expose it as a bundle config option for documentation/discovery purposes. + +**Architecture:** `Runner` is NOT a container service — it's created by `Runtime::getRunner()` before the DI container is available. Bundle config cannot reach `Runner` at runtime. The only reliable path is `$_SERVER` env var (same pattern as `APP_CACHE_DIR`). Bundle config `cache_warmup_timeout` is added for documentation and to set the container parameter (usable by other services), but `Runner` reads exclusively from `$_SERVER['WORKERMAN_CACHE_WARMUP_TIMEOUT']` with a default fallback. + +**Tech Stack:** PHP 8.2+, Symfony 7.1+, pcntl/posix extensions + +--- + +### Task 1: Add `cache_warmup_timeout` to bundle configuration + +**Files:** +- Modify: `src/config/configuration.php:22-25` (after `stop_timeout` node) +- Modify: `src/config/services.php:26-27` (add parameter) + +- [ ] **Step 1: Add the config node in `src/config/configuration.php`** + +After the `stop_timeout` node (line 25), add: + +```php + ->integerNode('cache_warmup_timeout') + ->info('Max seconds to wait for cache warmup in forked process. Can be overridden with WORKERMAN_CACHE_WARMUP_TIMEOUT env var.') + ->defaultValue(30) + ->end() +``` + +- [ ] **Step 2: Add the container parameter in `src/config/services.php`** + +After line 27 (`$container->setParameter('workerman.response_chunk_size', ...)`), add: + +```php + $container + ->setParameter('workerman.cache_warmup_timeout', $config['cache_warmup_timeout']) + ; +``` + +- [ ] **Step 3: Verify syntax** + +Run: `php -l src/config/configuration.php && php -l src/config/services.php` +Expected: No syntax errors + +- [ ] **Step 4: Commit** + +```bash +git add src/config/configuration.php src/config/services.php +git commit -m "feat: add cache_warmup_timeout config node and container parameter" +``` + +--- + +### Task 2: Update `Runner` to use configurable timeout + +**Files:** +- Modify: `src/Runner.php` + +- [ ] **Step 1: Remove the `CACHE_WARMUP_TIMEOUT` constant and add `getCacheWarmupTimeout()` method** + +The `Runner` class is `readonly`, so properties cannot be reassigned in the constructor body. Use the same pattern as `getCacheDir()` — a private method that reads `$_SERVER` with fallback. + +Replace the constant and add the method. In `src/Runner.php`, remove: + +```php + private const CACHE_WARMUP_TIMEOUT = 30; +``` + +And add a new private method after `getCacheDir()`: + +```php + private function getCacheWarmupTimeout(): int + { + if (isset($_SERVER['WORKERMAN_CACHE_WARMUP_TIMEOUT'])) { + return (int) $_SERVER['WORKERMAN_CACHE_WARMUP_TIMEOUT']; + } + + return 30; + } +``` + +- [ ] **Step 2: Update `run()` to use `getCacheWarmupTimeout()`** + +In `src/Runner.php`, in the `run()` method, replace: + +```php + $timeout = self::CACHE_WARMUP_TIMEOUT; +``` + +With: + +```php + $timeout = $this->getCacheWarmupTimeout(); +``` + +- [ ] **Step 3: Verify syntax** + +Run: `php -l src/Runner.php` +Expected: No syntax errors + +- [ ] **Step 4: Commit** + +```bash +git add src/Runner.php +git commit -m "feat: make cache warmup timeout configurable via WORKERMAN_CACHE_WARMUP_TIMEOUT env var" +``` + +--- + +### Task 3: Update tests + +**Files:** +- Modify: `tests/RunnerTest.php` + +- [ ] **Step 1: Update structural test assertions** + +In `tests/RunnerTest.php`, in `testRunnerUsesCorrectForkErrorHandling()`, replace the assertion for the constant: + +```php + $this->assertStringContainsString( + 'CACHE_WARMUP_TIMEOUT', + $content, + 'Must have CACHE_WARMUP_TIMEOUT constant', + ); +``` + +With: + +```php + $this->assertStringContainsString( + 'WORKERMAN_CACHE_WARMUP_TIMEOUT', + $content, + 'Must support WORKERMAN_CACHE_WARMUP_TIMEOUT env var override', + ); + + $this->assertStringContainsString( + 'getCacheWarmupTimeout', + $content, + 'Must have getCacheWarmupTimeout method', + ); +``` + +- [ ] **Step 2: Add test for default timeout value** + +Add a new test method in `tests/RunnerTest.php`: + +```php + public function testCacheWarmupTimeoutDefaultsTo30(): void + { + $sourceFile = dirname(__DIR__) . '/src/Runner.php'; + $this->assertFileExists($sourceFile); + + $content = file_get_contents($sourceFile); + $this->assertNotFalse($content); + $this->assertStringContainsString( + 'return 30;', + $content, + 'Default cache warmup timeout must be 30 seconds', + ); + } +``` + +- [ ] **Step 3: Run Runner tests** + +Run: `vendor/bin/phpunit tests/RunnerTest.php` +Expected: All tests pass + +- [ ] **Step 4: Commit** + +```bash +git add tests/RunnerTest.php +git commit -m "test: update Runner tests for configurable cache warmup timeout" +``` + +--- + +### Task 4: Run full test suite and verify + +- [ ] **Step 1: Run all tests** + +Run: `vendor/bin/phpunit` +Expected: All tests pass + +- [ ] **Step 2: Run static analysis** + +Run: `vendor/bin/phpstan analyse` +Expected: No errors + +- [ ] **Step 3: Run code style checks** + +Run: `vendor/bin/php-cs-fixer fix --dry-run` +Expected: No issues + +- [ ] **Step 4: Final commit if any fixes needed** + +```bash +git add -A +git commit -m "fix: address lint/static analysis issues" +``` \ No newline at end of file From 99c06b5ddf79778031d4f66b3c503b1575866911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Tue, 14 Apr 2026 23:24:31 +0200 Subject: [PATCH 4/4] fix: remove sleep(1) hack in SupervisorWorker and add proper logging - Remove unnecessary sleep(1) after handler execution - Add detailed log message with service context when process terminates unexpectedly - Use exit(1) for proper error code - Add type assertions for config values to fix phpstan errors - Remove 'final' to allow testing via inheritance --- ...04-14-configurable-cache-warmup-timeout.md | 199 ------------------ ...onfigurable-cache-warmup-timeout-design.md | 71 ------- src/Worker/SupervisorWorker.php | 10 +- 3 files changed, 7 insertions(+), 273 deletions(-) delete mode 100644 docs/superpowers/plans/2026-04-14-configurable-cache-warmup-timeout.md delete mode 100644 docs/superpowers/specs/2026-04-14-configurable-cache-warmup-timeout-design.md diff --git a/docs/superpowers/plans/2026-04-14-configurable-cache-warmup-timeout.md b/docs/superpowers/plans/2026-04-14-configurable-cache-warmup-timeout.md deleted file mode 100644 index 4467fbe..0000000 --- a/docs/superpowers/plans/2026-04-14-configurable-cache-warmup-timeout.md +++ /dev/null @@ -1,199 +0,0 @@ -# Configurable Cache Warmup Timeout — Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Make the cache warmup timeout configurable via env var `WORKERMAN_CACHE_WARMUP_TIMEOUT` with a default of 30 seconds, and expose it as a bundle config option for documentation/discovery purposes. - -**Architecture:** `Runner` is NOT a container service — it's created by `Runtime::getRunner()` before the DI container is available. Bundle config cannot reach `Runner` at runtime. The only reliable path is `$_SERVER` env var (same pattern as `APP_CACHE_DIR`). Bundle config `cache_warmup_timeout` is added for documentation and to set the container parameter (usable by other services), but `Runner` reads exclusively from `$_SERVER['WORKERMAN_CACHE_WARMUP_TIMEOUT']` with a default fallback. - -**Tech Stack:** PHP 8.2+, Symfony 7.1+, pcntl/posix extensions - ---- - -### Task 1: Add `cache_warmup_timeout` to bundle configuration - -**Files:** -- Modify: `src/config/configuration.php:22-25` (after `stop_timeout` node) -- Modify: `src/config/services.php:26-27` (add parameter) - -- [ ] **Step 1: Add the config node in `src/config/configuration.php`** - -After the `stop_timeout` node (line 25), add: - -```php - ->integerNode('cache_warmup_timeout') - ->info('Max seconds to wait for cache warmup in forked process. Can be overridden with WORKERMAN_CACHE_WARMUP_TIMEOUT env var.') - ->defaultValue(30) - ->end() -``` - -- [ ] **Step 2: Add the container parameter in `src/config/services.php`** - -After line 27 (`$container->setParameter('workerman.response_chunk_size', ...)`), add: - -```php - $container - ->setParameter('workerman.cache_warmup_timeout', $config['cache_warmup_timeout']) - ; -``` - -- [ ] **Step 3: Verify syntax** - -Run: `php -l src/config/configuration.php && php -l src/config/services.php` -Expected: No syntax errors - -- [ ] **Step 4: Commit** - -```bash -git add src/config/configuration.php src/config/services.php -git commit -m "feat: add cache_warmup_timeout config node and container parameter" -``` - ---- - -### Task 2: Update `Runner` to use configurable timeout - -**Files:** -- Modify: `src/Runner.php` - -- [ ] **Step 1: Remove the `CACHE_WARMUP_TIMEOUT` constant and add `getCacheWarmupTimeout()` method** - -The `Runner` class is `readonly`, so properties cannot be reassigned in the constructor body. Use the same pattern as `getCacheDir()` — a private method that reads `$_SERVER` with fallback. - -Replace the constant and add the method. In `src/Runner.php`, remove: - -```php - private const CACHE_WARMUP_TIMEOUT = 30; -``` - -And add a new private method after `getCacheDir()`: - -```php - private function getCacheWarmupTimeout(): int - { - if (isset($_SERVER['WORKERMAN_CACHE_WARMUP_TIMEOUT'])) { - return (int) $_SERVER['WORKERMAN_CACHE_WARMUP_TIMEOUT']; - } - - return 30; - } -``` - -- [ ] **Step 2: Update `run()` to use `getCacheWarmupTimeout()`** - -In `src/Runner.php`, in the `run()` method, replace: - -```php - $timeout = self::CACHE_WARMUP_TIMEOUT; -``` - -With: - -```php - $timeout = $this->getCacheWarmupTimeout(); -``` - -- [ ] **Step 3: Verify syntax** - -Run: `php -l src/Runner.php` -Expected: No syntax errors - -- [ ] **Step 4: Commit** - -```bash -git add src/Runner.php -git commit -m "feat: make cache warmup timeout configurable via WORKERMAN_CACHE_WARMUP_TIMEOUT env var" -``` - ---- - -### Task 3: Update tests - -**Files:** -- Modify: `tests/RunnerTest.php` - -- [ ] **Step 1: Update structural test assertions** - -In `tests/RunnerTest.php`, in `testRunnerUsesCorrectForkErrorHandling()`, replace the assertion for the constant: - -```php - $this->assertStringContainsString( - 'CACHE_WARMUP_TIMEOUT', - $content, - 'Must have CACHE_WARMUP_TIMEOUT constant', - ); -``` - -With: - -```php - $this->assertStringContainsString( - 'WORKERMAN_CACHE_WARMUP_TIMEOUT', - $content, - 'Must support WORKERMAN_CACHE_WARMUP_TIMEOUT env var override', - ); - - $this->assertStringContainsString( - 'getCacheWarmupTimeout', - $content, - 'Must have getCacheWarmupTimeout method', - ); -``` - -- [ ] **Step 2: Add test for default timeout value** - -Add a new test method in `tests/RunnerTest.php`: - -```php - public function testCacheWarmupTimeoutDefaultsTo30(): void - { - $sourceFile = dirname(__DIR__) . '/src/Runner.php'; - $this->assertFileExists($sourceFile); - - $content = file_get_contents($sourceFile); - $this->assertNotFalse($content); - $this->assertStringContainsString( - 'return 30;', - $content, - 'Default cache warmup timeout must be 30 seconds', - ); - } -``` - -- [ ] **Step 3: Run Runner tests** - -Run: `vendor/bin/phpunit tests/RunnerTest.php` -Expected: All tests pass - -- [ ] **Step 4: Commit** - -```bash -git add tests/RunnerTest.php -git commit -m "test: update Runner tests for configurable cache warmup timeout" -``` - ---- - -### Task 4: Run full test suite and verify - -- [ ] **Step 1: Run all tests** - -Run: `vendor/bin/phpunit` -Expected: All tests pass - -- [ ] **Step 2: Run static analysis** - -Run: `vendor/bin/phpstan analyse` -Expected: No errors - -- [ ] **Step 3: Run code style checks** - -Run: `vendor/bin/php-cs-fixer fix --dry-run` -Expected: No issues - -- [ ] **Step 4: Final commit if any fixes needed** - -```bash -git add -A -git commit -m "fix: address lint/static analysis issues" -``` \ No newline at end of file diff --git a/docs/superpowers/specs/2026-04-14-configurable-cache-warmup-timeout-design.md b/docs/superpowers/specs/2026-04-14-configurable-cache-warmup-timeout-design.md deleted file mode 100644 index 0f03170..0000000 --- a/docs/superpowers/specs/2026-04-14-configurable-cache-warmup-timeout-design.md +++ /dev/null @@ -1,71 +0,0 @@ -# Configurable Cache Warmup Timeout - -## Context - -PR #141 introduced a `CACHE_WARMUP_TIMEOUT` constant (30 seconds) in `Runner.php` for the forked cache warmup process timeout. Currently this value is hardcoded and cannot be changed without modifying the source. Issue #142 tracks this. - -## Requirements - -- Make the cache warmup timeout configurable via bundle config -- Allow env var override (`WORKERMAN_CACHE_WARMUP_TIMEOUT`) for quick operational changes -- Priority: env var > bundle config > default (30) -- Maintain backward compatibility — default of 30 must work without any configuration - -## Design - -### 1. Bundle config - -Add `cache_warmup_timeout` node in `src/config/configuration.php`: - -```php -->integerNode('cache_warmup_timeout')->defaultValue(30)->end() -``` - -Users configure in `workerman.yaml`: -```yaml -workerman: - cache_warmup_timeout: 60 -``` - -### 2. Data flow - -``` -bundle config → services.php → Runtime options → Resolver → Runtime::getRunner() → Runner constructor -``` - -- `services.php` passes `cache_warmup_timeout` from resolved config into Runtime options -- `Runtime::getRunner()` reads the option and passes it to `new Runner($kernelFactory, $timeout)` -- `KernelFactory` is unchanged — timeout does not flow through it - -### 3. Runner constructor - -```php -public function __construct( - private KernelFactory $kernelFactory, - private int $cacheWarmupTimeout = 30, -) { - if (isset($_SERVER['WORKERMAN_CACHE_WARMUP_TIMEOUT'])) { - $this->cacheWarmupTimeout = (int) $_SERVER['WORKERMAN_CACHE_WARMUP_TIMEOUT']; - } -} -``` - -Priority: env var > bundle config (injected via constructor) > default 30. - -Remove the `CACHE_WARMUP_TIMEOUT` constant. Replace `$timeout = self::CACHE_WARMUP_TIMEOUT` with `$timeout = $this->cacheWarmupTimeout`. - -### 4. Files to change - -| File | Change | -|------|--------| -| `src/config/configuration.php` | Add `cache_warmup_timeout` integer node (default 30) | -| `src/config/services.php` | Pass `cache_warmup_timeout` from config to Runtime options | -| `src/Runtime.php` | Pass timeout to `new Runner()` in `getRunner()` | -| `src/Runner.php` | Remove constant, add constructor param, add env var fallback | -| `tests/RunnerTest.php` | Update structural test, add tests for constructor and env var | - -### 5. Tests - -- Update `testRunnerUsesCorrectForkErrorHandling` — verify `cacheWarmupTimeout` property exists instead of `CACHE_WARMUP_TIMEOUT` constant -- Add test for env var override (`WORKERMAN_CACHE_WARMUP_TIMEOUT` takes precedence) -- Add test for constructor injection (default value = 30) \ No newline at end of file diff --git a/src/Worker/SupervisorWorker.php b/src/Worker/SupervisorWorker.php index 25341de..5ad6053 100644 --- a/src/Worker/SupervisorWorker.php +++ b/src/Worker/SupervisorWorker.php @@ -8,7 +8,7 @@ use CrazyGoat\WorkermanBundle\Supervisor\ProcessHandler; use Workerman\Worker; -final readonly class SupervisorWorker +readonly class SupervisorWorker { private const PROCESS_TITLE = '[Process]'; @@ -27,11 +27,14 @@ public function __construct(KernelFactory $kernelFactory, ?string $user, ?string } $taskName = empty($serviceConfig['name']) ? $serviceId : $serviceConfig['name']; + assert(is_string($taskName)); + $processes = $serviceConfig['processes'] ?? 1; + assert(is_int($processes)); $worker = new Worker(); $worker->name = self::PROCESS_TITLE; $worker->user = $user ?? ''; $worker->group = $group ?? ''; - $worker->count = $serviceConfig['processes'] ?? 1; + $worker->count = $processes; $worker->onWorkerStart = function (Worker $worker) use ($kernelFactory, $serviceId, $serviceConfig, $taskName): never { $worker->log(sprintf('%s "%s" started', $worker->name, $taskName)); $kernel = $kernelFactory->createKernel(); @@ -39,8 +42,9 @@ public function __construct(KernelFactory $kernelFactory, ?string $user, ?string /** @var ProcessHandler $handler */ $handler = $kernel->getContainer()->get('workerman.process_handler'); $method = empty($serviceConfig['method']) ? '__invoke' : $serviceConfig['method']; + assert(is_string($method)); $handler("$serviceId::$method", $taskName); - $worker->log("Process $taskName finished unexpectedly"); + $worker->log("Process \"$taskName\" (service: $serviceId::$method) finished unexpectedly"); exit(1); }; }