From 5eba9176da17d18bdca11a0bfb34cc41c42bb297 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 18 Feb 2026 12:27:31 +0000 Subject: [PATCH 1/7] Add CLAUDE.md and AGENTS.md with codebase review and improvement roadmap Thorough review of all 42 source files and 18 test files to create project context (CLAUDE.md) and agent instructions (AGENTS.md). CLAUDE.md covers: architecture, namespace map, state machine, key contracts, conventions, builder patterns, and CI/CD pipeline. AGENTS.md covers: code review checklist, testing gaps, implementation patterns, and a prioritized improvement roadmap identifying 16 issues including unimplemented timeouts, missing retry logic, broken compensation/saga pattern, and Laravel helper coupling in a framework-agnostic package. https://claude.ai/code/session_013CdFJtmtMmSqBZFwdrmicG --- AGENTS.md | 259 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ CLAUDE.md | 163 ++++++++++++++++++++++++++++++++++ 2 files changed, 422 insertions(+) create mode 100644 AGENTS.md create mode 100644 CLAUDE.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..04f489b --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,259 @@ +# AGENTS.md + +Instructions for AI agents working on this codebase. Read CLAUDE.md first for project context. + +--- + +## General Rules + +1. **Read before you write.** Never modify a file you haven't read. Understand the existing patterns. +2. **Zero production dependencies.** This is a framework-agnostic library. Never add runtime `require` entries to composer.json. Dev dependencies are fine. +3. **PHP 8.3+ only.** Use readonly properties, backed enums, match expressions, named arguments, constructor promotion. No legacy patterns. +4. **Run the full quality pipeline** after any change: `composer ci` (pint:test + analyze + test). +5. **PHPStan level 6.** All new code must pass. Don't suppress errors unless truly unavoidable—and document why. +6. **Pest PHP for tests.** Use `test()` syntax, `expect()` assertions, `describe()` blocks. No raw PHPUnit `$this->assert*()`. +7. **Laravel Pint formatting.** Run `composer pint` before committing. The CI will reject improperly formatted code. + +--- + +## Agent: Code Review + +When reviewing PRs or code changes for this project: + +### What to Check + +- **Contract compliance**: Does new code implement `WorkflowAction`, `StorageAdapter`, `EventDispatcher`, or `Logger` correctly? +- **Immutability**: `WorkflowContext`, `ActionResult`, `WorkflowDefinition` are value objects. Don't add setters or mutation methods. +- **State transitions**: Any code that calls `setState()` must respect the transition rules in `WorkflowState::canTransitionTo()`. Invalid transitions are bugs. +- **Action instantiation**: Actions are created with `new $actionClass($config, $logger)`. If you change the `WorkflowAction` interface or `BaseAction` constructor, you must update `Executor::executeAction()`. +- **No framework coupling**: This package must not `use` any Laravel, Symfony, or other framework classes in `src/`. The only exception is `data_get()`/`data_set()` helpers (which need to be replaced—see Known Issues). +- **Exception hierarchy**: All exceptions must extend `WorkflowException`. Use the static factory methods (e.g., `InvalidWorkflowDefinitionException::invalidName()`), not raw `new` constructors. +- **Event consistency**: Every state-changing operation should dispatch the appropriate event. Check that `WorkflowStarted`, `WorkflowCompleted`, `WorkflowFailed`, `WorkflowCancelled`, `StepCompleted`, `StepFailed` are dispatched at the right moments. + +### What to Reject + +- Adding `declare(strict_types=1)` piecemeal—if we add it, it goes in every file at once. +- Magic string config keys without constants or typed config objects. +- Suppressing PHPStan errors without a clear comment explaining why. +- Tests that use `assertTrue(true)` or other vacuous assertions. +- New public API methods without PHPDoc `@param`, `@return`, and `@throws` tags. + +--- + +## Agent: Testing + +### Test Structure + +``` +tests/ +├── Unit/ # Isolated class tests (no I/O, no storage) +├── Integration/ # Multi-step workflow execution through the engine +├── RealWorld/ # Complex production-like scenarios +├── Actions/ECommerce/ # Custom action fixtures used by RealWorld tests +├── Support/ # Test helpers (InMemoryStorage) +├── TestCase.php # Base class: provides $this->engine + $this->storage +├── Pest.php # Pest config +├── ArchTest.php # Architecture constraints +└── ExampleTest.php # Sanity check +``` + +### Writing Tests + +```php +// Good: descriptive, focused, uses Pest syntax +test('workflow transitions to failed state when action throws', function () { + $definition = [ + 'name' => 'failing-workflow', + 'steps' => [ + ['id' => 'bad_step', 'action' => NonExistentAction::class], + ], + ]; + + expect(fn () => $this->engine->start('test-fail', $definition)) + ->toThrow(ActionNotFoundException::class); +}); + +// Good: grouped with describe +describe('WorkflowBuilder', function () { + test('validates step IDs', function () { ... }); + test('rejects empty workflow names', function () { ... }); +}); +``` + +### Test Gaps to Fill + +These areas currently lack test coverage. Prioritize them when writing new tests: + +1. **Event dispatch verification** — No tests confirm events are actually dispatched. Create a `SpyEventDispatcher` that records dispatched events and assert against it. +2. **Retry logic** — `Step` supports `retryAttempts` but the `Executor` never actually retries. When retry is implemented, add tests for 0, 1, and max retries. +3. **HTTP/Email actions** — `HttpAction` and `EmailAction` have no unit tests. Mock the underlying operations and test config validation, error handling, and result mapping. +4. **Storage adapter edge cases** — Only `InMemoryStorage` is tested. Add tests for: loading a non-existent instance, concurrent saves, findInstances with various filter combinations. +5. **Condition evaluation** — The regex-based condition parser in `Step::evaluateCondition()` silently returns `true` for unparseable conditions. Test edge cases: nested properties, numeric comparisons, boolean values, empty strings. +6. **Compensation actions** — `Step` supports `compensationAction` but nothing executes it. When implemented, test rollback sequences. +7. **Pause/resume cycles** — Test multiple pause → resume → pause sequences and verify state consistency. + +--- + +## Agent: Implementation + +### Before Writing Code + +1. Check if the feature touches any contract interface. Interface changes require updates to all implementations (including `InMemoryStorage`, `NullLogger`, `NullEventDispatcher`). +2. Check if the change affects the builder API. Builder changes should maintain backward compatibility—add new methods, don't change existing signatures. +3. Check if new exceptions are needed. Use the existing hierarchy and static factory pattern. + +### Patterns to Follow + +**Creating a new Action:** +```php +class MyAction extends BaseAction +{ + protected function doExecute(WorkflowContext $context): ActionResult + { + $value = $this->getConfig('key'); + // ... business logic ... + return ActionResult::success(['output' => $result]); + } + + public function getName(): string + { + return 'My Action'; + } + + public function getDescription(): string + { + return 'Does something specific'; + } +} +``` + +**Creating a new Exception:** +```php +class MyException extends WorkflowException +{ + public static function specificCase(string $id): self + { + return new self( + message: "Technical: thing failed for {$id}", + userMessage: "The operation could not be completed.", + suggestions: ['Check the configuration', 'Verify the ID exists'], + context: ['id' => $id] + ); + } +} +``` + +**Creating a new Event:** +```php +class MyEvent +{ + public function __construct( + public readonly string $workflowId, + public readonly string $detail, + public readonly \DateTimeInterface $occurredAt = new \DateTime(), + ) {} +} +``` + +--- + +## Known Issues and Improvement Roadmap + +These are real issues found through code review, ordered by impact. This is not a wishlist—these are bugs, missing implementations, and design problems that need fixing before v1.0. + +### Critical — Blocks Production Use + +**1. Timeout is configured but never enforced** +`Step` accepts a `timeout` parameter. `WorkflowBuilder` validates it. But `Executor::executeStep()` never checks it. A misbehaving action can hang the entire process indefinitely. The TODO comment at `Executor.php:230` confirms this is known. + +*Fix:* Implement timeout enforcement in `Executor::executeStep()` using `pcntl_alarm()` or a wrapper that throws `StepExecutionException` on timeout. Add a `TimeoutException` to the exception hierarchy. + +**2. Retry logic is declared but never executed** +`Step::getRetryAttempts()` returns a value, but `Executor` catches exceptions and immediately fails. No retry loop exists anywhere in the execution path. + +*Fix:* Add a retry loop in `Executor::executeStep()` with exponential backoff. Dispatch `StepRetryEvent` on each retry attempt. Track attempt count on the `WorkflowInstance`. + +**3. Compensation actions are defined but never called** +`Step` supports `compensationAction` and `hasCompensation()`. Nothing in the codebase ever calls a compensation action. The Saga pattern is advertised but not implemented. + +*Fix:* When a step fails, walk backward through completed steps and execute their compensation actions. Add a `CompensationExecutedEvent`. This is a significant feature — design it before implementing. + +**4. `data_get()` / `data_set()` Laravel helper dependency** +`Step::evaluateCondition()` and `WorkflowDefinition::evaluateCondition()` call `data_get()`, a Laravel helper. This function doesn't exist in non-Laravel environments. The package advertises itself as framework-agnostic but will throw a fatal error without Laravel's helpers. + +*Fix:* Implement a simple `Support\Arr::get()` utility that handles dot-notation access, or inline the logic. Remove the PHPStan suppression for `data_get`/`data_set`. + +### High — Correctness Issues + +**5. Duplicate condition evaluation logic** +`Step::evaluateCondition()` (line 221) and `WorkflowDefinition::evaluateCondition()` contain identical regex-based condition parsing. This violates DRY and means bug fixes must be applied in two places. + +*Fix:* Extract to a `Support\ConditionEvaluator` class. Both `Step` and `WorkflowDefinition` should delegate to it. + +**6. Silent condition parsing failures** +`Step::evaluateCondition()` returns `true` when it can't parse a condition (line 244). This means a typo in a condition expression (`user.plan = "premium"` instead of `===`) will silently pass, executing steps that should be skipped. + +*Fix:* Throw `InvalidWorkflowDefinitionException` for unparseable conditions, or at minimum log a warning. Never silently succeed. + +**7. Inconsistent event class naming** +Some events end with `Event` (`StepCompletedEvent`, `WorkflowCompletedEvent`, `WorkflowFailedEvent`) and some don't (`WorkflowStarted`, `WorkflowCancelled`). Pick one convention and stick with it. + +*Fix:* Rename all to `*Event` suffix for consistency: `WorkflowStartedEvent`, `WorkflowCancelledEvent`. + +**8. Duplicate API methods on WorkflowEngine** +`getInstance()` and `getWorkflow()` do the same thing (lines 203 and 265). `getInstances()` and `listWorkflows()` do the same thing (lines 238 and 294). This confuses consumers and doubles the API surface. + +*Fix:* Deprecate `getWorkflow()` and `listWorkflows()`. Keep `getInstance()` and `getInstances()` as the canonical API. Remove the deprecated methods before v1.0. + +### Medium — Design Improvements + +**9. Action constructor not enforced by contract** +`Executor::executeAction()` (line 300) does `new $actionClass($config, $logger)`. The `WorkflowAction` interface doesn't define a constructor, so custom actions with different constructors will fail at runtime with a cryptic error. + +*Fix:* Either document the constructor contract clearly, add a static factory method to the interface (`WorkflowAction::make($config, $logger)`), or use an `ActionFactory` that can be overridden for DI containers. + +**10. `WorkflowInstance` does too much** +`WorkflowInstance` handles state tracking, progress calculation, step management, data merging, serialization, and error recording. It's a god object. + +*Fix:* Extract `WorkflowProgress` (progress calculation, step completion tracking) and `WorkflowSerializer` (toArray/fromArray) into separate concerns. Keep `WorkflowInstance` focused on identity and state. + +**11. No middleware/pipeline for cross-cutting concerns** +Retry, timeout, logging, and metrics all need to wrap step execution. Currently there's no clean way to add these behaviors without modifying `Executor` directly. + +*Fix:* Implement a `StepMiddleware` interface and a pipeline in `Executor` that chains middleware around action execution. Ship `RetryMiddleware`, `TimeoutMiddleware`, and `LoggingMiddleware` as built-in implementations. + +**12. Condition evaluator is too limited** +The regex-based condition parser only supports simple binary comparisons. No AND/OR, no grouping, no function calls. This limits real-world usefulness significantly. + +*Fix:* Consider a simple expression parser or adopt a lightweight expression language. At minimum, support AND (`&&`) and OR (`||`) operators. + +### Low — Polish + +**13. Missing `declare(strict_types=1)`** +No source file declares strict types. This allows implicit type coercion which can hide bugs. + +**14. PHPStan could be stricter** +Level 6 is good but not maximum. Several errors are suppressed in `phpstan.neon.dist`. Work toward level 8 by fixing the underlying type issues rather than suppressing them. + +**15. Excessive inline documentation** +Some classes (e.g., `WorkflowContext`) have 600+ lines with extensive code examples in PHPDoc. This makes files hard to navigate. Move examples to a `docs/` directory or the README. + +**16. No integration test for event dispatching** +The event system is a core feature but no test verifies that events are dispatched. Add a `SpyEventDispatcher` to the test support classes and use it in integration tests. + +--- + +## PR Checklist + +Before approving any PR, verify: + +- [ ] `composer ci` passes (pint:test + phpstan + pest) +- [ ] New public methods have `@param`, `@return`, and `@throws` PHPDoc +- [ ] No new PHPStan suppressions without justification +- [ ] No framework-specific imports in `src/` +- [ ] State transitions respect `WorkflowState::canTransitionTo()` +- [ ] New features have corresponding tests in the appropriate directory +- [ ] Exception messages are helpful (use static factory methods with context) +- [ ] Events are dispatched for state-changing operations +- [ ] No `dd()`, `dump()`, `var_dump()`, or `ray()` calls (ArchTest enforces this) +- [ ] Builder API changes are backward-compatible diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..f782a9b --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,163 @@ +# CLAUDE.md + +Project context for Claude Code and AI-assisted development. + +## Project Overview + +**workflow-engine-core** is a framework-agnostic PHP workflow engine. Zero production dependencies. PHP 8.3+. MIT licensed. + +Status: **v0.0.2-alpha** — active development, not production-ready. + +Related package: `solution-forest/workflow-engine-laravel` (Laravel integration layer). + +## Quick Reference + +```bash +# Run tests +composer test + +# Run tests with coverage +composer test:coverage + +# Static analysis (PHPStan level 6) +composer analyze + +# Code formatting (Laravel Pint) +composer pint + +# Check formatting without changes (CI mode) +composer pint:test + +# Full quality check: format + analyze + test +composer quality + +# CI pipeline: check format + analyze + test +composer ci +``` + +## Architecture + +``` +WorkflowBuilder → WorkflowDefinition → WorkflowEngine → Executor → Actions + ↓ + StateManager → StorageAdapter + ↓ + EventDispatcher +``` + +### Namespace Map + +| Namespace | Purpose | +|-----------|---------| +| `Core\` | Engine, Builder, Executor, StateManager, Step, WorkflowInstance, WorkflowDefinition, WorkflowContext, ActionResult | +| `Actions\` | Built-in actions: Log, Email, Http, Delay, Condition (all extend BaseAction) | +| `Contracts\` | Interfaces: WorkflowAction, StorageAdapter, EventDispatcher, Logger | +| `Attributes\` | PHP 8 attributes: WorkflowStep, Retry, Timeout, Condition | +| `Events\` | WorkflowStarted, WorkflowCompleted, WorkflowFailed, WorkflowCancelled, StepCompleted, StepFailed | +| `Exceptions\` | WorkflowException (base), InvalidWorkflowDefinition, InvalidWorkflowState, ActionNotFound, StepExecution, WorkflowInstanceNotFound | +| `Support\` | NullLogger, NullEventDispatcher, SimpleWorkflow, Uuid | + +### State Machine + +``` +PENDING → RUNNING → COMPLETED + ↓ ↑ + WAITING + ↓ ↑ + PAUSED + ↓ + FAILED + +CANCELLED ← (any non-terminal state) +``` + +Terminal states: COMPLETED, FAILED, CANCELLED. + +### Key Contracts + +Every custom integration implements one of these: + +- **`WorkflowAction`** — `execute(WorkflowContext): ActionResult` + `canExecute(WorkflowContext): bool` +- **`StorageAdapter`** — `save()`, `load()`, `findInstances()`, `delete()`, `exists()`, `updateState()` +- **`EventDispatcher`** — `dispatch(object $event): void` +- **`Logger`** — PSR-3 style: `info()`, `error()`, `warning()`, `debug()` + +### How Actions Work + +Actions are instantiated by `Executor` with `new $actionClass($config, $logger)`. They receive a `WorkflowContext` containing workflowId, stepId, data, config, and the instance reference. They return `ActionResult::success($data)` or `ActionResult::failure($message)`. + +## Conventions + +### Code Style +- **Laravel Pint** with Laravel preset +- Alphabetically ordered imports +- Short array syntax `[]` +- No trailing commas in single-line arrays +- PHPDoc left-aligned + +### Testing +- **Pest PHP 2.0** with PHPUnit 10 base +- Test structure: `tests/Unit/`, `tests/Integration/`, `tests/RealWorld/` +- Base test class: `TestCase` (provides `$this->engine` and `$this->storage` via `InMemoryStorage`) +- Use `test('description', function () { ... })` syntax +- Use `describe()` blocks for grouping related tests +- Use `expect()` fluent assertions, not `$this->assert*()` +- Architecture tests in `tests/ArchTest.php` — no `dd`, `dump`, `ray` calls + +### Naming +- Step IDs: `snake_case`, must match `/^[a-zA-Z][a-zA-Z0-9_-]*$/` +- Workflow names: `kebab-case` (e.g., `user-onboarding`, `order-processing`) +- Action classes: PascalCase ending in `Action` (e.g., `ProcessPaymentAction`) +- Event classes: PascalCase ending in `Event` (e.g., `StepCompletedEvent`) +- Exception classes: PascalCase ending in `Exception` + +### PHP Features Used +- PHP 8.3 readonly properties and classes +- Backed enums with methods (`WorkflowState`) +- Named arguments throughout builder API +- Match expressions instead of switch +- Union types (`string|WorkflowAction`) +- Constructor promotion + +### Builder API Pattern + +```php +// Fluent builder (primary API) +$workflow = WorkflowBuilder::create('order-flow') + ->description('Process customer orders') + ->addStep('validate', ValidateOrderAction::class) + ->when('order.total > 1000', function ($builder) { + $builder->addStep('fraud_check', FraudCheckAction::class); + }) + ->addStep('payment', ProcessPaymentAction::class, timeout: 300, retryAttempts: 3) + ->build(); + +// Quick templates +$workflow = WorkflowBuilder::quick()->userOnboarding(); +$workflow = WorkflowBuilder::quick()->orderProcessing(); +``` + +### Running a Workflow + +```php +$engine = new WorkflowEngine($storageAdapter, $eventDispatcher); +$instanceId = $engine->start('my-workflow', $definition->toArray(), ['key' => 'value']); +$instance = $engine->getInstance($instanceId); +$engine->cancel($instanceId, 'reason'); +``` + +## CI/CD + +GitHub Actions workflows: +- `run-tests.yml` — Matrix: PHP 8.3/8.4 × prefer-lowest/prefer-stable +- `phpstan.yml` — Static analysis on .php changes +- `fix-php-code-style-issues.yml` — Auto-format with Pint on push +- `update-changelog.yml` — Auto-update CHANGELOG on release +- `dependabot-auto-merge.yml` — Auto-merge minor/patch dependency updates + +## File Counts + +- 42 source files in `src/` +- 18 test files in `tests/` +- 40+ test cases, 160+ assertions +- PHPStan level 6 compliance From 32cb01b9118e2ccd84df11b8475b7fd942e78ede Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 19 Feb 2026 03:45:02 +0000 Subject: [PATCH 2/7] Implement Phases 1, 3, 4, 7: Remove Laravel coupling, normalize events, clean API, add event spy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 — Remove Laravel coupling: - Add Support\Arr helper for dot-notation array access - Replace data_get() calls in Step and WorkflowDefinition with Arr::get() - Remove getNestedValue() from WorkflowDefinition - Fix HttpAction to use curl instead of Laravel Http facade - Remove Laravel-specific PHPStan suppressions Phase 3 — Normalize event naming: - Rename WorkflowStarted → WorkflowStartedEvent - Rename WorkflowCancelled → WorkflowCancelledEvent - Standardize constructors to accept WorkflowInstance Phase 4 — Clean up duplicate API methods: - Remove getWorkflow() (duplicate of getInstance()) - Remove listWorkflows() (duplicate of getInstances()) - Add WorkflowState enum conversion to getInstances() Phase 7 — Add SpyEventDispatcher: - Create tests/Support/SpyEventDispatcher for event verification - Add EventDispatchTest integration tests - Add ArrTest unit tests https://claude.ai/code/session_013CdFJtmtMmSqBZFwdrmicG --- PLAN.md | 454 ++++++++++++++++++++++++ phpstan.neon.dist | 2 - src/Actions/HttpAction.php | 96 ++++- src/Core/Step.php | 4 +- src/Core/WorkflowDefinition.php | 27 +- src/Core/WorkflowEngine.php | 57 +-- src/Events/WorkflowCancelled.php | 12 - src/Events/WorkflowCancelledEvent.php | 23 ++ src/Events/WorkflowStarted.php | 15 - src/Events/WorkflowStartedEvent.php | 26 ++ src/Support/Arr.php | 49 +++ tests/Integration/EventDispatchTest.php | 150 ++++++++ tests/Support/SpyEventDispatcher.php | 59 +++ tests/Unit/Support/ArrTest.php | 66 ++++ tests/Unit/WorkflowEngineTest.php | 16 +- 15 files changed, 930 insertions(+), 126 deletions(-) create mode 100644 PLAN.md delete mode 100644 src/Events/WorkflowCancelled.php create mode 100644 src/Events/WorkflowCancelledEvent.php delete mode 100644 src/Events/WorkflowStarted.php create mode 100644 src/Events/WorkflowStartedEvent.php create mode 100644 src/Support/Arr.php create mode 100644 tests/Integration/EventDispatchTest.php create mode 100644 tests/Support/SpyEventDispatcher.php create mode 100644 tests/Unit/Support/ArrTest.php diff --git a/PLAN.md b/PLAN.md new file mode 100644 index 0000000..5cb31a9 --- /dev/null +++ b/PLAN.md @@ -0,0 +1,454 @@ +# Implementation Plan: workflow-engine-core Improvements + +## Philosophy + +Fix the foundation before adding features. Every change below makes the engine +more honest — the public API already promises these behaviors, we just need the +internals to deliver. Ordered by dependency graph: earlier phases unblock later ones. + +--- + +## Phase 1: Remove Laravel Coupling (Critical) + +The package claims "zero production dependencies" but calls `data_get()` which +only exists in Laravel. This is a fatal error in any non-Laravel environment. + +### 1a. Create `Support\Arr` helper + +**New file:** `src/Support/Arr.php` + +```php +final class Arr +{ + public static function get(array $array, string $key, mixed $default = null): mixed + { + // Handle direct key match first + if (array_key_exists($key, $array)) { + return $array[$key]; + } + + // Dot-notation traversal + foreach (explode('.', $key) as $segment) { + if (!is_array($array) || !array_key_exists($segment, $array)) { + return $default; + } + $array = $array[$segment]; + } + + return $array; + } + + public static function set(array &$array, string $key, mixed $value): void + { + $keys = explode('.', $key); + $current = &$array; + + foreach ($keys as $i => $segment) { + if ($i === count($keys) - 1) { + $current[$segment] = $value; + } else { + if (!isset($current[$segment]) || !is_array($current[$segment])) { + $current[$segment] = []; + } + $current = &$current[$segment]; + } + } + } +} +``` + +### 1b. Replace all `data_get()` calls + +- `src/Core/Step.php:229` — replace `data_get($data, $key)` with `Arr::get($data, $key)` +- Remove `WorkflowDefinition::getNestedValue()` private method, replace its calls with `Arr::get()` + +### 1c. Update PHPStan config + +- Remove the suppression: `'#Function (data_get|data_set|class_basename) not found#'` +- Remove the suppression: `'#Call to static method timeout\(\) on an unknown class Illuminate\\Support\\Facades\\Http#'` + (This is in `HttpAction` — fix the underlying code to not reference `Http` facade) + +### 1d. Fix `HttpAction` Laravel facade usage + +- `src/Actions/HttpAction.php` references `Illuminate\Support\Facades\Http` +- Replace with a simple `curl` wrapper or make HTTP client injectable +- This action should work without Laravel installed + +**Files changed:** `src/Support/Arr.php` (new), `src/Core/Step.php`, `src/Core/WorkflowDefinition.php`, `src/Actions/HttpAction.php`, `phpstan.neon.dist` + +**Tests:** `tests/Unit/Support/ArrTest.php` (new) — dot notation, nested arrays, missing keys, default values + +--- + +## Phase 2: Deduplicate Condition Evaluation (High) + +Two identical regex-based condition parsers exist. Fix DRY violation and the +silent-success bug at the same time. + +### 2a. Create `Support\ConditionEvaluator` + +**New file:** `src/Support/ConditionEvaluator.php` + +```php +final class ConditionEvaluator +{ + public static function evaluate(string $condition, array $data): bool + { + if (!preg_match('/(\w+(?:\.\w+)*)\s*(===|!==|>=|<=|==|!=|>|<)\s*(.+)/', $condition, $matches)) { + throw InvalidWorkflowDefinitionException::invalidCondition( + $condition, + 'Condition must be in format: "key operator value" (e.g., "user.plan === premium")' + ); + } + + $key = $matches[1]; + $operator = $matches[2]; + $value = trim($matches[3], '"\''); + + $dataValue = Arr::get($data, $key); + + return match ($operator) { + '===' => $dataValue === $value, + '!==' => $dataValue !== $value, + '>=' => $dataValue >= $value, + '<=' => $dataValue <= $value, + '==' => $dataValue == $value, + '!=' => $dataValue != $value, + '>' => $dataValue > $value, + '<' => $dataValue < $value, + default => false, + }; + } +} +``` + +### 2b. Refactor callers + +- `Step::evaluateCondition()` → delegate to `ConditionEvaluator::evaluate()` +- `WorkflowDefinition::evaluateCondition()` → delegate to `ConditionEvaluator::evaluate()` +- Remove `WorkflowDefinition::getNestedValue()` (already replaced by `Arr::get` in Phase 1) + +### 2c. Decide on silent failure behavior + +- `Step::evaluateCondition()` currently returns `true` on parse failure (line 244) — dangerous +- `WorkflowDefinition::evaluateCondition()` returns `false` on parse failure (line 346) — inconsistent +- **Decision:** Throw `InvalidWorkflowDefinitionException` on unparseable conditions. Fail loud. +- This is a breaking change for anyone relying on malformed conditions silently passing. Acceptable at v0.0.2-alpha. + +**Files changed:** `src/Support/ConditionEvaluator.php` (new), `src/Core/Step.php`, `src/Core/WorkflowDefinition.php` + +**Tests:** `tests/Unit/Support/ConditionEvaluatorTest.php` (new) — valid conditions, invalid format, dot notation, type coercion edge cases, all 8 operators + +--- + +## Phase 3: Normalize Event Naming (High) + +Inconsistent: `WorkflowStarted`, `WorkflowCancelled` vs `WorkflowCompletedEvent`, `StepCompletedEvent`. + +### 3a. Rename events to consistent `*Event` suffix + +| Current | New | +|---------|-----| +| `WorkflowStarted` | `WorkflowStartedEvent` | +| `WorkflowCancelled` | `WorkflowCancelledEvent` | +| `WorkflowCompletedEvent` | _(no change)_ | +| `WorkflowFailedEvent` | _(no change)_ | +| `StepCompletedEvent` | _(no change)_ | +| `StepFailedEvent` | _(no change)_ | + +### 3b. Update all dispatchers + +- `src/Core/WorkflowEngine.php:135` — `new WorkflowStarted(...)` → `new WorkflowStartedEvent(...)` +- `src/Core/WorkflowEngine.php:253` — `new WorkflowCancelled(...)` → `new WorkflowCancelledEvent(...)` + +### 3c. Normalize constructor signatures + +Currently `WorkflowStartedEvent` and `WorkflowCancelledEvent` take primitive strings, +while `WorkflowCompletedEvent` takes a `WorkflowInstance`. Standardize: + +- All workflow-level events should accept `WorkflowInstance` + optional extra context +- This gives event listeners access to the full instance, not just the ID + +```php +// Standardized pattern for all workflow events: +final readonly class WorkflowStartedEvent +{ + public function __construct( + public WorkflowInstance $instance, + public array $initialContext = [], + ) {} +} +``` + +**Files changed:** `src/Events/WorkflowStarted.php` (rename + refactor), `src/Events/WorkflowCancelled.php` (rename + refactor), `src/Core/WorkflowEngine.php` + +**Tests:** Update any test referencing old class names + +--- + +## Phase 4: Clean Up Duplicate API Methods (High) + +`WorkflowEngine` has redundant method pairs that confuse consumers. + +### 4a. Remove duplicates + +- Remove `getWorkflow()` (line 265) — duplicate of `getInstance()` +- Remove `listWorkflows()` (line 294) — duplicate of `getInstances()` +- Move the `WorkflowState` enum-to-string conversion from `listWorkflows()` into `getInstances()` + +### 4b. Keep `getStatus()` but simplify + +`getStatus()` returns a formatted array — this is useful and distinct from `getInstance()`. +Keep it, but make it delegate to `getInstance()` internally (it already does via `getWorkflow()`). + +**Files changed:** `src/Core/WorkflowEngine.php` + +**Tests:** Update any test calling `getWorkflow()` or `listWorkflows()` to use `getInstance()` / `getInstances()` + +--- + +## Phase 5: Implement Retry Logic (Critical) + +`Step::getRetryAttempts()` returns a value but `Executor` never retries. + +### 5a. Add retry loop in `Executor::executeStep()` + +```php +private function executeStep(WorkflowInstance $instance, Step $step): void +{ + $maxAttempts = $step->getRetryAttempts() + 1; // +1 for initial attempt + + for ($attempt = 1; $attempt <= $maxAttempts; $attempt++) { + try { + $this->doExecuteStep($instance, $step, $attempt); + return; // Success — exit retry loop + } catch (\Exception $e) { + if ($attempt === $maxAttempts) { + throw $e; // Final attempt failed — propagate + } + + $this->logger->warning('Step failed, retrying', [ + 'step_id' => $step->getId(), + 'attempt' => $attempt, + 'max_attempts' => $maxAttempts, + 'error' => $e->getMessage(), + ]); + + // Exponential backoff: 1s, 2s, 4s... + $backoffMs = (int) (1000 * pow(2, $attempt - 1)); + usleep($backoffMs * 1000); + } + } +} +``` + +### 5b. Add `StepRetriedEvent` + +**New file:** `src/Events/StepRetriedEvent.php` + +```php +final readonly class StepRetriedEvent +{ + public function __construct( + public WorkflowInstance $instance, + public Step $step, + public int $attempt, + public int $maxAttempts, + public \Throwable $lastError, + ) {} +} +``` + +Dispatch from the retry loop's catch block. + +**Files changed:** `src/Core/Executor.php`, `src/Events/StepRetriedEvent.php` (new) + +**Tests:** `tests/Unit/ExecutorRetryTest.php` (new) — 0 retries (fail immediately), 1 retry (fail then succeed), max retries exhausted, backoff timing, event dispatch verification + +--- + +## Phase 6: Implement Timeout Enforcement (Critical) + +`Step` accepts timeout but `Executor` never enforces it. + +### 6a. Add timeout wrapper in `Executor` + +Use `pcntl_alarm` on Linux or a simpler approach with `set_time_limit` per step. +Since this is a library (not a web app), use a process-level approach: + +```php +private function executeWithTimeout(callable $callback, int $timeoutSeconds): mixed +{ + if (!function_exists('pcntl_alarm')) { + // Fallback: just execute without timeout (log warning) + $this->logger->warning('pcntl extension not available, timeout not enforced'); + return $callback(); + } + + $previousHandler = pcntl_signal(SIGALRM, function () use ($timeoutSeconds) { + throw StepExecutionException::timeout($timeoutSeconds); + }); + + pcntl_alarm($timeoutSeconds); + + try { + $result = $callback(); + pcntl_alarm(0); // Cancel alarm + return $result; + } catch (\Exception $e) { + pcntl_alarm(0); // Cancel alarm on error too + throw $e; + } finally { + // Restore previous handler + if ($previousHandler !== null) { + pcntl_signal(SIGALRM, $previousHandler); + } + } +} +``` + +### 6b. Parse timeout string in Executor + +The `Step::getTimeout()` returns a string like `"300"` or `"5m"`. Reuse the +`WorkflowBuilder::parseTimeoutString()` logic — extract it to a `Support\Timeout` helper. + +### 6c. Wire into step execution + +In `executeStep()`, wrap the action execution call: + +```php +if ($step->getTimeout() !== null) { + $seconds = Timeout::toSeconds($step->getTimeout()); + $this->executeWithTimeout(fn () => $this->executeAction($instance, $step), $seconds); +} else { + $this->executeAction($instance, $step); +} +``` + +**Files changed:** `src/Core/Executor.php`, `src/Support/Timeout.php` (new) + +**Tests:** `tests/Unit/Support/TimeoutTest.php` (parsing), `tests/Integration/TimeoutTest.php` (actual enforcement — skip if pcntl unavailable) + +--- + +## Phase 7: Add SpyEventDispatcher for Tests (Medium) + +No test currently verifies events are dispatched. This blocks testing of Phases 3, 5, 6. + +### 7a. Create test spy + +**New file:** `tests/Support/SpyEventDispatcher.php` + +```php +class SpyEventDispatcher implements EventDispatcher +{ + public array $dispatched = []; + + public function dispatch(object $event): void + { + $this->dispatched[] = $event; + } + + public function assertDispatched(string $eventClass, ?callable $callback = null): void + { + // Find matching events, optionally filtered by callback + } + + public function assertNotDispatched(string $eventClass): void { ... } + public function assertDispatchedCount(string $eventClass, int $count): void { ... } +} +``` + +### 7b. Add event dispatch tests + +- Test `WorkflowStartedEvent` dispatched on `$engine->start()` +- Test `WorkflowCompletedEvent` dispatched when all steps finish +- Test `WorkflowCancelledEvent` dispatched on `$engine->cancel()` +- Test `StepCompletedEvent` dispatched after each step +- Test `StepRetriedEvent` dispatched on retries (Phase 5) + +**Files changed:** `tests/Support/SpyEventDispatcher.php` (new), `tests/Integration/EventDispatchTest.php` (new) + +--- + +## Phase 8: State Transition Enforcement (Medium) + +`WorkflowState::canTransitionTo()` exists but `WorkflowInstance::setState()` doesn't call it. +Invalid transitions are silently accepted. + +### 8a. Enforce transitions in `WorkflowInstance::setState()` + +```php +public function setState(WorkflowState $state): void +{ + if (!$this->state->canTransitionTo($state)) { + throw InvalidWorkflowStateException::fromInstanceTransition( + $this->id, $this->state, $state + ); + } + + $this->state = $state; + $this->updatedAt = new \DateTime(); +} +``` + +### 8b. Audit all `setState()` callers + +Ensure every call site transitions legally: +- `Executor::processWorkflow()` — PENDING → RUNNING (valid) +- `Executor::processWorkflow()` — RUNNING → COMPLETED (valid) +- `StateManager::setError()` — any → FAILED (need to verify PENDING → FAILED is allowed, currently it's not — add it to `canTransitionTo()`) +- `WorkflowEngine::cancel()` — any → CANCELLED (verify all non-terminal → CANCELLED is allowed) + +### 8c. Update state machine + +Add missing transitions that the engine actually needs: +- `PENDING → FAILED` (engine can fail before running if definition is bad post-start) + +**Files changed:** `src/Core/WorkflowInstance.php`, `src/Core/WorkflowState.php` + +**Tests:** `tests/Unit/WorkflowStateTransitionTest.php` (new) — test every valid and invalid transition pair + +--- + +## Execution Order + +``` +Phase 1 (Laravel coupling) — no dependencies, do first +Phase 2 (Condition evaluator) — depends on Phase 1 (uses Arr::get) +Phase 3 (Event naming) — independent +Phase 4 (API cleanup) — independent +Phase 7 (SpyEventDispatcher) — independent, but needed by Phase 5 tests +Phase 5 (Retry logic) — depends on Phase 7 for testing +Phase 6 (Timeout enforcement) — depends on Phase 7 for testing +Phase 8 (State enforcement) — independent, but test after Phases 5-6 +``` + +Phases 1, 3, 4, 7 can be done in parallel. +Phases 2, 5, 6, 8 are sequential after their dependencies. + +## New Files Summary + +| File | Type | +|------|------| +| `src/Support/Arr.php` | Source | +| `src/Support/ConditionEvaluator.php` | Source | +| `src/Support/Timeout.php` | Source | +| `src/Events/WorkflowStartedEvent.php` | Source (rename) | +| `src/Events/WorkflowCancelledEvent.php` | Source (rename) | +| `src/Events/StepRetriedEvent.php` | Source | +| `tests/Support/SpyEventDispatcher.php` | Test support | +| `tests/Unit/Support/ArrTest.php` | Test | +| `tests/Unit/Support/ConditionEvaluatorTest.php` | Test | +| `tests/Unit/Support/TimeoutTest.php` | Test | +| `tests/Unit/ExecutorRetryTest.php` | Test | +| `tests/Unit/WorkflowStateTransitionTest.php` | Test | +| `tests/Integration/EventDispatchTest.php` | Test | +| `tests/Integration/TimeoutTest.php` | Test | + +## Quality Gate + +After all phases, the full CI pipeline must pass: +- `composer pint:test` — formatting +- `composer analyze` — PHPStan level 6 with fewer suppressions +- `composer test` — all existing + new tests green diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 6287813..1e03da1 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -4,8 +4,6 @@ parameters: - src tmpDir: build/phpstan ignoreErrors: - - '#Function (data_get|data_set|class_basename) not found#' - - '#Call to static method timeout\(\) on an unknown class Illuminate\\Support\\Facades\\Http#' - '#has no value type specified in iterable type array#' - '#Match arm comparison between .+ and .+ is always true#' - '#has parameter .+ with no type specified#' diff --git a/src/Actions/HttpAction.php b/src/Actions/HttpAction.php index a6e42df..6e7e6fc 100644 --- a/src/Actions/HttpAction.php +++ b/src/Actions/HttpAction.php @@ -2,12 +2,12 @@ namespace SolutionForest\WorkflowEngine\Actions; -use Illuminate\Support\Facades\Http; use SolutionForest\WorkflowEngine\Attributes\Retry; use SolutionForest\WorkflowEngine\Attributes\Timeout; use SolutionForest\WorkflowEngine\Attributes\WorkflowStep; use SolutionForest\WorkflowEngine\Core\ActionResult; use SolutionForest\WorkflowEngine\Core\WorkflowContext; +use SolutionForest\WorkflowEngine\Support\Arr; /** * HTTP request action with PHP 8.3+ features @@ -48,30 +48,90 @@ protected function doExecute(WorkflowContext $context): ActionResult $data = $this->processArrayTemplates($data, $context->getData()); try { - $response = match ($method) { - 'GET' => Http::timeout($timeout)->withHeaders($headers)->get($url, $data), - 'POST' => Http::timeout($timeout)->withHeaders($headers)->post($url, $data), - 'PUT' => Http::timeout($timeout)->withHeaders($headers)->put($url, $data), - 'PATCH' => Http::timeout($timeout)->withHeaders($headers)->patch($url, $data), - 'DELETE' => Http::timeout($timeout)->withHeaders($headers)->delete($url, $data), - default => throw new \InvalidArgumentException("Unsupported HTTP method: {$method}") - }; - - if ($response->successful()) { + // Initialize cURL + $ch = curl_init(); + + // Build query string for GET requests + if ($method === 'GET' && ! empty($data)) { + $url .= '?' . http_build_query($data); + } + + curl_setopt($ch, CURLOPT_URL, $url); + curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); + curl_setopt($ch, CURLOPT_TIMEOUT, $timeout); + curl_setopt($ch, CURLOPT_FOLLOWLOCATION, true); + + // Set HTTP method and body + if ($method !== 'GET') { + curl_setopt($ch, CURLOPT_CUSTOMREQUEST, $method); + if (! empty($data)) { + $jsonData = json_encode($data); + curl_setopt($ch, CURLOPT_POSTFIELDS, $jsonData); + $headers['Content-Type'] = 'application/json'; + $headers['Content-Length'] = strlen($jsonData); + } + } + + // Set headers + if (! empty($headers)) { + $headerArray = []; + foreach ($headers as $key => $value) { + $headerArray[] = "{$key}: {$value}"; + } + curl_setopt($ch, CURLOPT_HTTPHEADER, $headerArray); + } + + // Capture response headers + $responseHeaders = []; + curl_setopt($ch, CURLOPT_HEADERFUNCTION, function ($curl, $header) use (&$responseHeaders) { + $len = strlen($header); + $header = explode(':', $header, 2); + if (count($header) >= 2) { + $responseHeaders[strtolower(trim($header[0]))] = trim($header[1]); + } + + return $len; + }); + + // Execute request + $responseBody = curl_exec($ch); + $statusCode = curl_getinfo($ch, CURLINFO_HTTP_CODE); + $error = curl_error($ch); + curl_close($ch); + + if ($error) { + return ActionResult::failure( + "HTTP request failed: {$error}", + [ + 'error' => $error, + 'url' => $url, + 'method' => $method, + ] + ); + } + + // Parse JSON response + $responseData = json_decode($responseBody, true); + if (json_last_error() !== JSON_ERROR_NONE) { + $responseData = $responseBody; + } + + // Check if successful (2xx status code) + if ($statusCode >= 200 && $statusCode < 300) { return ActionResult::success([ - 'status_code' => $response->status(), - 'response_data' => $response->json(), - 'headers' => $response->headers(), + 'status_code' => $statusCode, + 'response_data' => $responseData, + 'headers' => $responseHeaders, 'url' => $url, 'method' => $method, ]); } return ActionResult::failure( - "HTTP request failed with status {$response->status()}: {$response->body()}", + "HTTP request failed with status {$statusCode}", [ - 'status_code' => $response->status(), - 'response_body' => $response->body(), + 'status_code' => $statusCode, + 'response_body' => $responseBody, 'url' => $url, 'method' => $method, ] @@ -95,7 +155,7 @@ protected function doExecute(WorkflowContext $context): ActionResult private function processTemplate(string $template, array $data): string { return preg_replace_callback('/\{\{\s*([^}]+)\s*\}\}/', function ($matches) use ($data) { - return data_get($data, trim($matches[1]), $matches[0]); + return Arr::get($data, trim($matches[1]), $matches[0]); }, $template); } diff --git a/src/Core/Step.php b/src/Core/Step.php index 142a5b8..49329c3 100644 --- a/src/Core/Step.php +++ b/src/Core/Step.php @@ -2,6 +2,8 @@ namespace SolutionForest\WorkflowEngine\Core; +use SolutionForest\WorkflowEngine\Support\Arr; + /** * Represents a single step in a workflow with complete configuration and execution metadata. * @@ -226,7 +228,7 @@ private function evaluateCondition(string $condition, array $data): bool $operator = $matches[2]; $value = trim($matches[3], '"\''); - $dataValue = data_get($data, $key); + $dataValue = Arr::get($data, $key); return match ($operator) { '===' => $dataValue === $value, diff --git a/src/Core/WorkflowDefinition.php b/src/Core/WorkflowDefinition.php index 2c7b0b9..a06ba6b 100644 --- a/src/Core/WorkflowDefinition.php +++ b/src/Core/WorkflowDefinition.php @@ -3,6 +3,7 @@ namespace SolutionForest\WorkflowEngine\Core; use SolutionForest\WorkflowEngine\Exceptions\InvalidWorkflowDefinitionException; +use SolutionForest\WorkflowEngine\Support\Arr; /** * Represents a complete workflow definition with steps, transitions, and metadata. @@ -328,7 +329,7 @@ private function evaluateCondition(string $condition, array $data): bool $operator = $matches[2]; $value = trim($matches[3], '"\''); - $dataValue = $this->getNestedValue($data, $key); + $dataValue = Arr::get($data, $key); return match ($operator) { '===' => $dataValue === $value, @@ -395,28 +396,4 @@ public static function fromArray(array $data): static ); } - /** - * Get a nested value from an array using dot notation. - * - * @param array $array - */ - private function getNestedValue(array $array, string $key, $default = null) - { - if (isset($array[$key])) { - return $array[$key]; - } - - // Handle dot notation for nested arrays - $keys = explode('.', $key); - $value = $array; - - foreach ($keys as $nestedKey) { - if (! is_array($value) || ! array_key_exists($nestedKey, $value)) { - return $default; - } - $value = $value[$nestedKey]; - } - - return $value; - } } diff --git a/src/Core/WorkflowEngine.php b/src/Core/WorkflowEngine.php index cce1591..33bb5fa 100644 --- a/src/Core/WorkflowEngine.php +++ b/src/Core/WorkflowEngine.php @@ -4,8 +4,8 @@ use SolutionForest\WorkflowEngine\Contracts\EventDispatcher; use SolutionForest\WorkflowEngine\Contracts\StorageAdapter; -use SolutionForest\WorkflowEngine\Events\WorkflowCancelled; -use SolutionForest\WorkflowEngine\Events\WorkflowStarted; +use SolutionForest\WorkflowEngine\Events\WorkflowCancelledEvent; +use SolutionForest\WorkflowEngine\Events\WorkflowStartedEvent; use SolutionForest\WorkflowEngine\Exceptions\InvalidWorkflowDefinitionException; use SolutionForest\WorkflowEngine\Exceptions\InvalidWorkflowStateException; use SolutionForest\WorkflowEngine\Exceptions\WorkflowInstanceNotFoundException; @@ -132,9 +132,8 @@ public function start(string $workflowId, array $definition, array $context = [] $this->stateManager->save($instance); // Dispatch start event - $this->dispatchEvent(new WorkflowStarted( - $instance->getId(), - $instance->getDefinition()->getName(), + $this->dispatchEvent(new WorkflowStartedEvent( + $instance, $context )); @@ -237,6 +236,11 @@ public function getInstance(string $instanceId): WorkflowInstance */ public function getInstances(array $filters = []): array { + // Convert WorkflowState enum to string value for storage layer + if (isset($filters['state']) && $filters['state'] instanceof WorkflowState) { + $filters['state'] = $filters['state']->value; + } + return $this->storage->findInstances($filters); } @@ -250,29 +254,20 @@ public function cancel(string $instanceId, string $reason = ''): WorkflowInstanc $this->stateManager->save($instance); // Dispatch cancel event - $this->dispatchEvent(new WorkflowCancelled( - $instance->getId(), - $instance->getDefinition()->getName(), + $this->dispatchEvent(new WorkflowCancelledEvent( + $instance, $reason )); return $instance; } - /** - * Get workflow instance by ID - */ - public function getWorkflow(string $workflowId): WorkflowInstance - { - return $this->stateManager->load($workflowId); - } - /** * Get workflow status */ public function getStatus(string $workflowId): array { - $instance = $this->getWorkflow($workflowId); + $instance = $this->getInstance($workflowId); return [ 'workflow_id' => $instance->getId(), @@ -285,34 +280,6 @@ public function getStatus(string $workflowId): array ]; } - /** - * List workflows with optional filters - * - * @param array $filters - * @return array> - */ - public function listWorkflows(array $filters = []): array - { - // Convert WorkflowState enum to string value for storage layer - if (isset($filters['state']) && $filters['state'] instanceof WorkflowState) { - $filters['state'] = $filters['state']->value; - } - - $instances = $this->storage->findInstances($filters); - - return array_map(function (WorkflowInstance $instance) { - return [ - 'workflow_id' => $instance->getId(), - 'name' => $instance->getDefinition()->getName(), - 'state' => $instance->getState()->value, - 'current_step' => $instance->getCurrentStepId(), - 'progress' => $instance->getProgress(), - 'created_at' => $instance->getCreatedAt(), - 'updated_at' => $instance->getUpdatedAt(), - ]; - }, $instances); - } - /** * Safely dispatch an event if event dispatcher is available */ diff --git a/src/Events/WorkflowCancelled.php b/src/Events/WorkflowCancelled.php deleted file mode 100644 index c9e8b06..0000000 --- a/src/Events/WorkflowCancelled.php +++ /dev/null @@ -1,12 +0,0 @@ -instance->getId(); + } + + public function getWorkflowName(): string + { + return $this->instance->getDefinition()->getName(); + } +} diff --git a/src/Events/WorkflowStarted.php b/src/Events/WorkflowStarted.php deleted file mode 100644 index b1852f7..0000000 --- a/src/Events/WorkflowStarted.php +++ /dev/null @@ -1,15 +0,0 @@ - $context - */ - public function __construct( - public readonly string $workflowId, - public readonly string $name, - public readonly array $context = [] - ) {} -} diff --git a/src/Events/WorkflowStartedEvent.php b/src/Events/WorkflowStartedEvent.php new file mode 100644 index 0000000..24c333d --- /dev/null +++ b/src/Events/WorkflowStartedEvent.php @@ -0,0 +1,26 @@ + $initialContext + */ + public function __construct( + public WorkflowInstance $instance, + public array $initialContext = [], + ) {} + + public function getWorkflowId(): string + { + return $this->instance->getId(); + } + + public function getWorkflowName(): string + { + return $this->instance->getDefinition()->getName(); + } +} diff --git a/src/Support/Arr.php b/src/Support/Arr.php new file mode 100644 index 0000000..6d42053 --- /dev/null +++ b/src/Support/Arr.php @@ -0,0 +1,49 @@ + $array + */ + public static function get(array $array, string $key, mixed $default = null): mixed + { + if (array_key_exists($key, $array)) { + return $array[$key]; + } + + foreach (explode('.', $key) as $segment) { + if (! is_array($array) || ! array_key_exists($segment, $array)) { + return $default; + } + $array = $array[$segment]; + } + + return $array; + } + + /** + * Set a value in a nested array using dot notation. + * + * @param array $array + */ + public static function set(array &$array, string $key, mixed $value): void + { + $keys = explode('.', $key); + $current = &$array; + + foreach ($keys as $i => $segment) { + if ($i === count($keys) - 1) { + $current[$segment] = $value; + } else { + if (! isset($current[$segment]) || ! is_array($current[$segment])) { + $current[$segment] = []; + } + $current = &$current[$segment]; + } + } + } +} diff --git a/tests/Integration/EventDispatchTest.php b/tests/Integration/EventDispatchTest.php new file mode 100644 index 0000000..b526da3 --- /dev/null +++ b/tests/Integration/EventDispatchTest.php @@ -0,0 +1,150 @@ + 'test-workflow', + 'version' => '1.0', + 'steps' => [ + [ + 'id' => 'step1', + 'action' => 'log', + 'parameters' => ['message' => 'hello', 'level' => 'info'], + ], + ], + ]; + + $engine->start('event-test-1', $definition, ['key' => 'value']); + + // Check that at least one event was dispatched + expect($spy->dispatched)->not->toBeEmpty(); + + // First event should be workflow started + $startEvents = array_filter($spy->dispatched, fn ($e) => str_contains(get_class($e), 'WorkflowStarted') + ); + expect($startEvents)->not->toBeEmpty(); + }); + + test('dispatches step completed event after step execution', function () { + $storage = new InMemoryStorage(); + $spy = new SpyEventDispatcher(); + $engine = new WorkflowEngine($storage, $spy); + + $definition = [ + 'name' => 'test-workflow', + 'version' => '1.0', + 'steps' => [ + [ + 'id' => 'step1', + 'action' => 'log', + 'parameters' => ['message' => 'test', 'level' => 'info'], + ], + ], + ]; + + $engine->start('event-test-2', $definition); + + $stepEvents = array_filter($spy->dispatched, fn ($e) => str_contains(get_class($e), 'StepCompleted') + ); + expect($stepEvents)->not->toBeEmpty(); + }); + + test('dispatches workflow completed event when all steps finish', function () { + $storage = new InMemoryStorage(); + $spy = new SpyEventDispatcher(); + $engine = new WorkflowEngine($storage, $spy); + + $definition = [ + 'name' => 'test-workflow', + 'version' => '1.0', + 'steps' => [ + [ + 'id' => 'step1', + 'action' => 'log', + 'parameters' => ['message' => 'test', 'level' => 'info'], + ], + ], + ]; + + $engine->start('event-test-3', $definition); + + $completedEvents = array_filter($spy->dispatched, fn ($e) => str_contains(get_class($e), 'WorkflowCompleted') + ); + expect($completedEvents)->not->toBeEmpty(); + }); + + test('dispatches cancelled event on cancel', function () { + $storage = new InMemoryStorage(); + $spy = new SpyEventDispatcher(); + $engine = new WorkflowEngine($storage, $spy); + + $definition = [ + 'name' => 'test-workflow', + 'version' => '1.0', + 'steps' => [ + [ + 'id' => 'step1', + 'action' => 'log', + 'parameters' => ['message' => 'test', 'level' => 'info'], + ], + [ + 'id' => 'step2', + 'action' => 'log', + 'parameters' => ['message' => 'test2', 'level' => 'info'], + ], + ], + 'transitions' => [ + ['from' => 'step1', 'to' => 'step2'], + ], + ]; + + $id = $engine->start('event-test-4', $definition); + + $spy->reset(); + $engine->cancel($id, 'testing cancellation'); + + $cancelEvents = array_filter($spy->dispatched, fn ($e) => str_contains(get_class($e), 'WorkflowCancelled') || str_contains(get_class($e), 'Cancelled') + ); + expect($cancelEvents)->not->toBeEmpty(); + }); + + test('dispatches events in correct order for full workflow', function () { + $storage = new InMemoryStorage(); + $spy = new SpyEventDispatcher(); + $engine = new WorkflowEngine($storage, $spy); + + $definition = [ + 'name' => 'test-workflow', + 'version' => '1.0', + 'steps' => [ + [ + 'id' => 'step1', + 'action' => 'log', + 'parameters' => ['message' => 'first', 'level' => 'info'], + ], + ], + ]; + + $engine->start('event-test-5', $definition); + + // Should have at least: WorkflowStarted, StepCompleted, WorkflowCompleted + expect(count($spy->dispatched))->toBeGreaterThanOrEqual(3); + + // First event should be start, last should be workflow completed + $classNames = array_map(fn ($e) => get_class($e), $spy->dispatched); + + // First is WorkflowStarted(Event) + expect($classNames[0])->toContain('WorkflowStarted'); + + // Last is WorkflowCompleted + expect(end($classNames))->toContain('WorkflowCompleted'); + }); +}); diff --git a/tests/Support/SpyEventDispatcher.php b/tests/Support/SpyEventDispatcher.php new file mode 100644 index 0000000..a64a5c0 --- /dev/null +++ b/tests/Support/SpyEventDispatcher.php @@ -0,0 +1,59 @@ + */ + public array $dispatched = []; + + public function dispatch(object $event): void + { + $this->dispatched[] = $event; + } + + /** + * Get all dispatched events of a given class. + * + * @template T of object + * @param class-string $eventClass + * @return array + */ + public function getDispatched(string $eventClass): array + { + return array_values(array_filter( + $this->dispatched, + fn (object $event) => $event instanceof $eventClass + )); + } + + /** + * Check if an event of the given class was dispatched. + * + * @param class-string $eventClass + */ + public function hasDispatched(string $eventClass): bool + { + return count($this->getDispatched($eventClass)) > 0; + } + + /** + * Get the count of dispatched events of a given class. + * + * @param class-string $eventClass + */ + public function countDispatched(string $eventClass): int + { + return count($this->getDispatched($eventClass)); + } + + /** + * Reset the dispatched events list. + */ + public function reset(): void + { + $this->dispatched = []; + } +} diff --git a/tests/Unit/Support/ArrTest.php b/tests/Unit/Support/ArrTest.php new file mode 100644 index 0000000..d35e397 --- /dev/null +++ b/tests/Unit/Support/ArrTest.php @@ -0,0 +1,66 @@ + 'John', 'age' => 30]; + expect(Arr::get($data, 'name'))->toBe('John'); + }); + + test('returns value for dot notation key', function () { + $data = ['user' => ['profile' => ['name' => 'John']]]; + expect(Arr::get($data, 'user.profile.name'))->toBe('John'); + }); + + test('returns default for missing key', function () { + expect(Arr::get([], 'missing', 'default'))->toBe('default'); + }); + + test('returns default for missing nested key', function () { + $data = ['user' => ['name' => 'John']]; + expect(Arr::get($data, 'user.email', 'none'))->toBe('none'); + }); + + test('returns null as default when no default provided', function () { + expect(Arr::get([], 'missing'))->toBeNull(); + }); + + test('handles deeply nested values', function () { + $data = ['a' => ['b' => ['c' => ['d' => 'deep']]]]; + expect(Arr::get($data, 'a.b.c.d'))->toBe('deep'); + }); + + test('returns full array value for intermediate key', function () { + $data = ['user' => ['name' => 'John', 'age' => 30]]; + expect(Arr::get($data, 'user'))->toBe(['name' => 'John', 'age' => 30]); + }); + }); + + describe('set', function () { + test('sets simple key', function () { + $data = []; + Arr::set($data, 'name', 'John'); + expect($data)->toBe(['name' => 'John']); + }); + + test('sets nested key with dot notation', function () { + $data = []; + Arr::set($data, 'user.name', 'John'); + expect($data)->toBe(['user' => ['name' => 'John']]); + }); + + test('sets deeply nested key', function () { + $data = []; + Arr::set($data, 'a.b.c', 'value'); + expect($data)->toBe(['a' => ['b' => ['c' => 'value']]]); + }); + + test('overwrites existing value', function () { + $data = ['name' => 'John']; + Arr::set($data, 'name', 'Jane'); + expect($data['name'])->toBe('Jane'); + }); + }); +}); diff --git a/tests/Unit/WorkflowEngineTest.php b/tests/Unit/WorkflowEngineTest.php index 379af91..738dc1f 100644 --- a/tests/Unit/WorkflowEngineTest.php +++ b/tests/Unit/WorkflowEngineTest.php @@ -30,7 +30,7 @@ expect($workflowId)->not->toBeEmpty(); // Verify the workflow instance was created - $instance = $this->engine->getWorkflow($workflowId); + $instance = $this->engine->getInstance($workflowId); expect($instance)->toBeInstanceOf(WorkflowInstance::class); expect($instance->getState())->toBe(WorkflowState::COMPLETED); // Log action completes immediately expect($instance->getName())->toBe('Test Workflow'); @@ -52,7 +52,7 @@ $context = ['name' => 'John']; $workflowId = $this->engine->start('test-workflow', $definition, $context); - $instance = $this->engine->getWorkflow($workflowId); + $instance = $this->engine->getInstance($workflowId); $workflowData = $instance->getContext()->getData(); // Should contain original context plus any data added by actions @@ -94,7 +94,7 @@ // Resume it $this->engine->resume($workflowId); - $instance = $this->engine->getWorkflow($workflowId); + $instance = $this->engine->getInstance($workflowId); // After resume, it should be completed since we have simple log actions expect($instance->getState())->toBe(WorkflowState::COMPLETED); }); @@ -115,7 +115,7 @@ $workflowId = $this->engine->start('test-workflow', $definition); $this->engine->cancel($workflowId, 'User cancelled'); - $instance = $this->engine->getWorkflow($workflowId); + $instance = $this->engine->getInstance($workflowId); expect($instance->getState())->toBe(WorkflowState::CANCELLED); }); @@ -151,7 +151,7 @@ })->throws(InvalidWorkflowDefinitionException::class, 'Required field \'name\' is missing from workflow definition'); test('it throws exception for nonexistent workflow', function () { - $this->engine->getWorkflow('nonexistent'); + $this->engine->getInstance('nonexistent'); })->throws(WorkflowInstanceNotFoundException::class, 'Workflow instance \'nonexistent\' was not found'); test('it can list workflows', function () { @@ -170,11 +170,11 @@ $workflowId1 = $this->engine->start('test-workflow-1', $definition); $workflowId2 = $this->engine->start('test-workflow-2', $definition); - $workflows = $this->engine->listWorkflows(); + $workflows = $this->engine->getInstances(); expect($workflows)->toHaveCount(2); - expect(array_column($workflows, 'workflow_id'))->toContain($workflowId1); - expect(array_column($workflows, 'workflow_id'))->toContain($workflowId2); + expect(array_map(fn ($w) => $w->getId(), $workflows))->toContain($workflowId1); + expect(array_map(fn ($w) => $w->getId(), $workflows))->toContain($workflowId2); }); test('it can filter workflows by state', function () { From e9fdb228bcb933fa1a593b871259519a04678791 Mon Sep 17 00:00:00 2001 From: lam0819 <68211972+lam0819@users.noreply.github.com> Date: Thu, 19 Feb 2026 03:45:28 +0000 Subject: [PATCH 3/7] Fix styling --- src/Actions/HttpAction.php | 2 +- src/Core/WorkflowDefinition.php | 1 - tests/Integration/EventDispatchTest.php | 20 ++++++++++---------- tests/Support/SpyEventDispatcher.php | 1 + 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Actions/HttpAction.php b/src/Actions/HttpAction.php index 6e7e6fc..1cfd889 100644 --- a/src/Actions/HttpAction.php +++ b/src/Actions/HttpAction.php @@ -53,7 +53,7 @@ protected function doExecute(WorkflowContext $context): ActionResult // Build query string for GET requests if ($method === 'GET' && ! empty($data)) { - $url .= '?' . http_build_query($data); + $url .= '?'.http_build_query($data); } curl_setopt($ch, CURLOPT_URL, $url); diff --git a/src/Core/WorkflowDefinition.php b/src/Core/WorkflowDefinition.php index a06ba6b..da98e12 100644 --- a/src/Core/WorkflowDefinition.php +++ b/src/Core/WorkflowDefinition.php @@ -395,5 +395,4 @@ public static function fromArray(array $data): static metadata: $data['metadata'] ?? [] ); } - } diff --git a/tests/Integration/EventDispatchTest.php b/tests/Integration/EventDispatchTest.php index b526da3..8bc63d1 100644 --- a/tests/Integration/EventDispatchTest.php +++ b/tests/Integration/EventDispatchTest.php @@ -6,8 +6,8 @@ describe('Event Dispatching', function () { test('dispatches workflow started event on start', function () { - $storage = new InMemoryStorage(); - $spy = new SpyEventDispatcher(); + $storage = new InMemoryStorage; + $spy = new SpyEventDispatcher; $engine = new WorkflowEngine($storage, $spy); $definition = [ @@ -34,8 +34,8 @@ }); test('dispatches step completed event after step execution', function () { - $storage = new InMemoryStorage(); - $spy = new SpyEventDispatcher(); + $storage = new InMemoryStorage; + $spy = new SpyEventDispatcher; $engine = new WorkflowEngine($storage, $spy); $definition = [ @@ -58,8 +58,8 @@ }); test('dispatches workflow completed event when all steps finish', function () { - $storage = new InMemoryStorage(); - $spy = new SpyEventDispatcher(); + $storage = new InMemoryStorage; + $spy = new SpyEventDispatcher; $engine = new WorkflowEngine($storage, $spy); $definition = [ @@ -82,8 +82,8 @@ }); test('dispatches cancelled event on cancel', function () { - $storage = new InMemoryStorage(); - $spy = new SpyEventDispatcher(); + $storage = new InMemoryStorage; + $spy = new SpyEventDispatcher; $engine = new WorkflowEngine($storage, $spy); $definition = [ @@ -117,8 +117,8 @@ }); test('dispatches events in correct order for full workflow', function () { - $storage = new InMemoryStorage(); - $spy = new SpyEventDispatcher(); + $storage = new InMemoryStorage; + $spy = new SpyEventDispatcher; $engine = new WorkflowEngine($storage, $spy); $definition = [ diff --git a/tests/Support/SpyEventDispatcher.php b/tests/Support/SpyEventDispatcher.php index a64a5c0..2091e1d 100644 --- a/tests/Support/SpyEventDispatcher.php +++ b/tests/Support/SpyEventDispatcher.php @@ -18,6 +18,7 @@ public function dispatch(object $event): void * Get all dispatched events of a given class. * * @template T of object + * * @param class-string $eventClass * @return array */ From 50b439d4a7b1a1cf8c5def013689b5d1e12873be Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 19 Feb 2026 03:53:58 +0000 Subject: [PATCH 4/7] Implement Phases 2, 5, 6, 8: ConditionEvaluator, retry, timeout, state enforcement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 — Deduplicate condition evaluation: - Extract ConditionEvaluator to Support namespace - Both Step and WorkflowDefinition delegate to it - Throw on unparseable conditions instead of silently passing Phase 5 — Retry logic: - Add executeActionWithRetry() to Executor with exponential backoff - Dispatch StepRetriedEvent on each retry attempt - Respects Step::getRetryAttempts() configuration Phase 6 — Timeout enforcement: - Add executeWithTimeout() using pcntl_alarm (graceful fallback) - Add Timeout helper for parsing duration strings (30s, 5m, 2h, 1d) - Wrap action execution with timeout in Executor Phase 8 — State transition enforcement: - WorkflowInstance::setState() now validates via canTransitionTo() - Add PENDING→FAILED and PAUSED→FAILED transitions - Executor handles PAUSED→RUNNING and WAITING→RUNNING on resume - WorkflowEngine::cancel() guards against terminal states Additional fixes: - Replace ALL remaining data_get/data_set/class_basename calls (WorkflowContext, ActionResult, ConditionAction, SimpleWorkflow, ActionNotFoundException, StepExecutionException) - Add Arr::classBasename() helper - Clean up stale PHPStan ignore patterns - Fix DocumentApproval test conditions to proper format CI: 93 tests, 224 assertions, all green. PHPStan level 6 clean. https://claude.ai/code/session_013CdFJtmtMmSqBZFwdrmicG --- phpstan.neon.dist | 2 - src/Actions/ConditionAction.php | 3 +- src/Core/ActionResult.php | 4 +- src/Core/DefinitionParser.php | 4 +- src/Core/Executor.php | 126 +++++++++++++++- src/Core/Step.php | 25 +--- src/Core/WorkflowBuilder.php | 2 +- src/Core/WorkflowContext.php | 9 +- src/Core/WorkflowDefinition.php | 25 +--- src/Core/WorkflowEngine.php | 13 +- src/Core/WorkflowInstance.php | 11 ++ src/Core/WorkflowState.php | 8 +- src/Events/StepRetriedEvent.php | 27 ++++ src/Exceptions/ActionNotFoundException.php | 3 +- .../InvalidWorkflowDefinitionException.php | 5 +- .../InvalidWorkflowStateException.php | 22 +++ src/Exceptions/StepExecutionException.php | 3 +- src/Support/Arr.php | 10 ++ src/Support/ConditionEvaluator.php | 45 ++++++ src/Support/SimpleWorkflow.php | 2 +- src/Support/Timeout.php | 44 ++++++ tests/Integration/EventDispatchTest.php | 11 +- tests/Integration/WorkflowIntegrationTest.php | 34 +++-- .../DocumentApprovalWorkflowTest.php | 4 +- tests/Unit/ExecutorRetryTest.php | 116 +++++++++++++++ tests/Unit/Support/ConditionEvaluatorTest.php | 65 +++++++++ tests/Unit/Support/TimeoutTest.php | 41 ++++++ tests/Unit/WorkflowEngineTest.php | 56 ++++++-- tests/Unit/WorkflowStateTransitionTest.php | 136 ++++++++++++++++++ 29 files changed, 757 insertions(+), 99 deletions(-) create mode 100644 src/Events/StepRetriedEvent.php create mode 100644 src/Support/ConditionEvaluator.php create mode 100644 src/Support/Timeout.php create mode 100644 tests/Unit/ExecutorRetryTest.php create mode 100644 tests/Unit/Support/ConditionEvaluatorTest.php create mode 100644 tests/Unit/Support/TimeoutTest.php create mode 100644 tests/Unit/WorkflowStateTransitionTest.php diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 1e03da1..7be727a 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -6,7 +6,5 @@ parameters: ignoreErrors: - '#has no value type specified in iterable type array#' - '#Match arm comparison between .+ and .+ is always true#' - - '#has parameter .+ with no type specified#' - - '#has no return type specified#' - '#PHPDoc tag @param for parameter .+ with type .+ is not subtype of native type#' - '#Parameter .+ has invalid type#' diff --git a/src/Actions/ConditionAction.php b/src/Actions/ConditionAction.php index 58533d0..3ec8d2d 100644 --- a/src/Actions/ConditionAction.php +++ b/src/Actions/ConditionAction.php @@ -6,6 +6,7 @@ use SolutionForest\WorkflowEngine\Attributes\WorkflowStep; use SolutionForest\WorkflowEngine\Core\ActionResult; use SolutionForest\WorkflowEngine\Core\WorkflowContext; +use SolutionForest\WorkflowEngine\Support\Arr; /** * Condition evaluation action with advanced expression parsing @@ -116,7 +117,7 @@ private function getValue(string $expression, array $data): mixed 'true', 'yes' => true, 'false', 'no' => false, 'null', 'empty' => null, - default => data_get($data, $expression) + default => Arr::get($data, $expression) }; } } diff --git a/src/Core/ActionResult.php b/src/Core/ActionResult.php index 73af1ac..19006df 100644 --- a/src/Core/ActionResult.php +++ b/src/Core/ActionResult.php @@ -2,6 +2,8 @@ namespace SolutionForest\WorkflowEngine\Core; +use SolutionForest\WorkflowEngine\Support\Arr; + /** * Represents the result of a workflow action execution. * @@ -375,7 +377,7 @@ public function toArray(): array */ public function get(string $key, $default = null) { - return data_get($this->data, $key, $default); + return Arr::get($this->data, $key, $default); } /** diff --git a/src/Core/DefinitionParser.php b/src/Core/DefinitionParser.php index bb4d55c..2ff5966 100644 --- a/src/Core/DefinitionParser.php +++ b/src/Core/DefinitionParser.php @@ -396,7 +396,7 @@ private function validateStep(string $stepId, array $stepData, array $fullDefini foreach ($stepData['conditions'] as $conditionIndex => $condition) { if (! is_string($condition) || empty(trim($condition))) { - throw InvalidWorkflowDefinitionException::invalidCondition($condition); + throw InvalidWorkflowDefinitionException::invalidCondition($condition, 'Condition cannot be empty.'); } } } @@ -512,7 +512,7 @@ private function validateTransition(array $transition, array $steps, int $transi // Validate optional condition field if (isset($transition['condition'])) { if (! is_string($transition['condition']) || empty(trim($transition['condition']))) { - throw InvalidWorkflowDefinitionException::invalidCondition($transition['condition']); + throw InvalidWorkflowDefinitionException::invalidCondition($transition['condition'], 'Condition cannot be empty.'); } } diff --git a/src/Core/Executor.php b/src/Core/Executor.php index 9b8772b..89f4477 100644 --- a/src/Core/Executor.php +++ b/src/Core/Executor.php @@ -8,12 +8,14 @@ use SolutionForest\WorkflowEngine\Contracts\WorkflowAction; use SolutionForest\WorkflowEngine\Events\StepCompletedEvent; use SolutionForest\WorkflowEngine\Events\StepFailedEvent; +use SolutionForest\WorkflowEngine\Events\StepRetriedEvent; use SolutionForest\WorkflowEngine\Events\WorkflowCompletedEvent; use SolutionForest\WorkflowEngine\Events\WorkflowFailedEvent; use SolutionForest\WorkflowEngine\Exceptions\ActionNotFoundException; use SolutionForest\WorkflowEngine\Exceptions\StepExecutionException; use SolutionForest\WorkflowEngine\Support\NullEventDispatcher; use SolutionForest\WorkflowEngine\Support\NullLogger; +use SolutionForest\WorkflowEngine\Support\Timeout; /** * Workflow executor responsible for running workflow steps and managing execution flow. @@ -150,8 +152,8 @@ public function execute(WorkflowInstance $instance): void */ private function processWorkflow(WorkflowInstance $instance): void { - // If workflow is not running, start it - if ($instance->getState() === WorkflowState::PENDING) { + // If workflow is not running, transition it to running + if (in_array($instance->getState(), [WorkflowState::PENDING, WorkflowState::PAUSED, WorkflowState::WAITING])) { $instance->setState(WorkflowState::RUNNING); $this->stateManager->save($instance); } @@ -217,7 +219,7 @@ private function executeStep(WorkflowInstance $instance, Step $step): void try { if ($step->hasAction()) { - $this->executeAction($instance, $step); + $this->executeActionWithRetry($instance, $step); } // Mark step as completed @@ -227,7 +229,6 @@ private function executeStep(WorkflowInstance $instance, Step $step): void $this->logger->info('Workflow step completed successfully', [ 'workflow_id' => $instance->getId(), 'step_id' => $step->getId(), - 'step_duration' => 'calculated_in_future_version', // TODO: Add timing ]); // Continue execution recursively @@ -268,6 +269,112 @@ private function executeStep(WorkflowInstance $instance, Step $step): void } } + /** + * Execute a step's action with retry logic. + * + * @param WorkflowInstance $instance The workflow instance + * @param Step $step The step to execute + * + * @throws ActionNotFoundException If the action class doesn't exist + * @throws StepExecutionException If all retry attempts are exhausted + */ + private function executeActionWithRetry(WorkflowInstance $instance, Step $step): void + { + $maxAttempts = $step->getRetryAttempts() + 1; // +1 for initial attempt + + if ($maxAttempts <= 1) { + // No retries configured, execute directly + $this->executeAction($instance, $step); + + return; + } + + $lastException = null; + + for ($attempt = 1; $attempt <= $maxAttempts; $attempt++) { + try { + $this->executeAction($instance, $step); + + return; // Success — exit retry loop + } catch (\Exception $e) { + $lastException = $e; + + if ($attempt === $maxAttempts) { + $this->logger->error('Step failed after all retry attempts', [ + 'workflow_id' => $instance->getId(), + 'step_id' => $step->getId(), + 'attempts' => $attempt, + 'max_attempts' => $maxAttempts, + 'error' => $e->getMessage(), + ]); + + throw $e; // Final attempt failed — propagate + } + + $this->logger->warning('Step failed, retrying', [ + 'workflow_id' => $instance->getId(), + 'step_id' => $step->getId(), + 'attempt' => $attempt, + 'max_attempts' => $maxAttempts, + 'error' => $e->getMessage(), + ]); + + $this->eventDispatcher->dispatch(new StepRetriedEvent( + $instance, + $step, + $attempt, + $maxAttempts, + $e + )); + + // Exponential backoff: 100ms, 200ms, 400ms... (keep short for a library) + $backoffMicroseconds = (int) (100000 * pow(2, $attempt - 1)); + usleep($backoffMicroseconds); + } + } + } + + /** + * Execute a callback with a timeout constraint. + * + * Uses pcntl_alarm when available, otherwise logs a warning and executes without timeout. + * + * @param callable $callback The callback to execute + * @param int $timeoutSeconds Maximum execution time in seconds + * @return mixed The callback's return value + * + * @throws StepExecutionException If the timeout is exceeded + */ + private function executeWithTimeout(callable $callback, int $timeoutSeconds): mixed + { + if (! function_exists('pcntl_alarm') || ! function_exists('pcntl_signal')) { + $this->logger->warning('pcntl extension not available, timeout not enforced', [ + 'timeout_seconds' => $timeoutSeconds, + ]); + + return $callback(); + } + + pcntl_signal(SIGALRM, function () use ($timeoutSeconds) { + throw new \RuntimeException("Step execution timed out after {$timeoutSeconds} seconds"); + }); + + pcntl_alarm($timeoutSeconds); + + try { + $result = $callback(); + pcntl_alarm(0); + + return $result; + } catch (\Exception $e) { + pcntl_alarm(0); + + throw $e; + } finally { + pcntl_signal(SIGALRM, SIG_DFL); + } + } + /** * Execute the action associated with a workflow step. * @@ -319,7 +426,16 @@ private function executeAction(WorkflowInstance $instance, Step $step): void instance: $instance ); - $result = $action->execute($context); + $timeoutValue = $step->getTimeout(); + if ($timeoutValue !== null) { + $timeoutSeconds = Timeout::toSeconds($timeoutValue); + $result = $this->executeWithTimeout( + fn () => $action->execute($context), + $timeoutSeconds + ); + } else { + $result = $action->execute($context); + } if ($result->isSuccess()) { // Merge any output data from the action diff --git a/src/Core/Step.php b/src/Core/Step.php index 49329c3..68e078e 100644 --- a/src/Core/Step.php +++ b/src/Core/Step.php @@ -2,7 +2,7 @@ namespace SolutionForest\WorkflowEngine\Core; -use SolutionForest\WorkflowEngine\Support\Arr; +use SolutionForest\WorkflowEngine\Support\ConditionEvaluator; /** * Represents a single step in a workflow with complete configuration and execution metadata. @@ -222,28 +222,7 @@ public function canExecute(array $data): bool */ private function evaluateCondition(string $condition, array $data): bool { - // Enhanced condition evaluation with support for more operators - if (preg_match('/(\w+(?:\.\w+)*)\s*(===|!==|>=|<=|==|!=|>|<)\s*(.+)/', $condition, $matches)) { - $key = $matches[1]; - $operator = $matches[2]; - $value = trim($matches[3], '"\''); - - $dataValue = Arr::get($data, $key); - - return match ($operator) { - '===' => $dataValue === $value, - '!==' => $dataValue !== $value, - '>=' => $dataValue >= $value, - '<=' => $dataValue <= $value, - '==' => $dataValue == $value, - '!=' => $dataValue != $value, - '>' => $dataValue > $value, - '<' => $dataValue < $value, - default => false, - }; - } - - return true; // Default to true if condition cannot be parsed + return ConditionEvaluator::evaluate($condition, $data); } /** diff --git a/src/Core/WorkflowBuilder.php b/src/Core/WorkflowBuilder.php index 5a6f489..80902c3 100644 --- a/src/Core/WorkflowBuilder.php +++ b/src/Core/WorkflowBuilder.php @@ -281,7 +281,7 @@ public function then( public function when(string $condition, callable $callback): self { if (empty(trim($condition))) { - throw InvalidWorkflowDefinitionException::invalidCondition($condition); + throw InvalidWorkflowDefinitionException::invalidCondition($condition, 'Condition cannot be empty.'); } $originalStepsCount = count($this->steps); diff --git a/src/Core/WorkflowContext.php b/src/Core/WorkflowContext.php index 3ed8535..d098038 100644 --- a/src/Core/WorkflowContext.php +++ b/src/Core/WorkflowContext.php @@ -3,6 +3,7 @@ namespace SolutionForest\WorkflowEngine\Core; use DateTime; +use SolutionForest\WorkflowEngine\Support\Arr; /** * Immutable workflow execution context using PHP 8.3+ readonly properties. @@ -111,7 +112,7 @@ public function getData(?string $key = null, mixed $default = null): mixed return $this->data; } - return data_get($this->data, $key, $default); + return Arr::get($this->data, $key, $default); } /** @@ -173,7 +174,7 @@ public function withData(array $newData): static public function with(string $key, mixed $value): static { $newData = $this->data; - data_set($newData, $key, $value); + Arr::set($newData, $key, $value); return new self( workflowId: $this->workflowId, @@ -206,7 +207,7 @@ public function with(string $key, mixed $value): static */ public function hasData(string $key): bool { - return data_get($this->data, $key) !== null; + return Arr::get($this->data, $key) !== null; } /** @@ -229,7 +230,7 @@ public function getConfig(?string $key = null, mixed $default = null): mixed return $this->config; } - return data_get($this->config, $key, $default); + return Arr::get($this->config, $key, $default); } /** diff --git a/src/Core/WorkflowDefinition.php b/src/Core/WorkflowDefinition.php index da98e12..d5804aa 100644 --- a/src/Core/WorkflowDefinition.php +++ b/src/Core/WorkflowDefinition.php @@ -3,7 +3,7 @@ namespace SolutionForest\WorkflowEngine\Core; use SolutionForest\WorkflowEngine\Exceptions\InvalidWorkflowDefinitionException; -use SolutionForest\WorkflowEngine\Support\Arr; +use SolutionForest\WorkflowEngine\Support\ConditionEvaluator; /** * Represents a complete workflow definition with steps, transitions, and metadata. @@ -323,28 +323,7 @@ private function processSteps(array $stepsData): array */ private function evaluateCondition(string $condition, array $data): bool { - // Enhanced condition evaluation with comprehensive operator support - if (preg_match('/(\w+(?:\.\w+)*)\s*(===|!==|>=|<=|==|!=|>|<)\s*(.+)/', $condition, $matches)) { - $key = $matches[1]; - $operator = $matches[2]; - $value = trim($matches[3], '"\''); - - $dataValue = Arr::get($data, $key); - - return match ($operator) { - '===' => $dataValue === $value, - '!==' => $dataValue !== $value, - '>=' => $dataValue >= $value, - '<=' => $dataValue <= $value, - '==' => $dataValue == $value, - '!=' => $dataValue != $value, - '>' => $dataValue > $value, - '<' => $dataValue < $value, - default => false, - }; - } - - return false; // Default to false if condition cannot be parsed + return ConditionEvaluator::evaluate($condition, $data); } /** diff --git a/src/Core/WorkflowEngine.php b/src/Core/WorkflowEngine.php index 33bb5fa..d992caf 100644 --- a/src/Core/WorkflowEngine.php +++ b/src/Core/WorkflowEngine.php @@ -6,7 +6,6 @@ use SolutionForest\WorkflowEngine\Contracts\StorageAdapter; use SolutionForest\WorkflowEngine\Events\WorkflowCancelledEvent; use SolutionForest\WorkflowEngine\Events\WorkflowStartedEvent; -use SolutionForest\WorkflowEngine\Exceptions\InvalidWorkflowDefinitionException; use SolutionForest\WorkflowEngine\Exceptions\InvalidWorkflowStateException; use SolutionForest\WorkflowEngine\Exceptions\WorkflowInstanceNotFoundException; @@ -95,7 +94,7 @@ public function __construct( * @param array $context Initial context data for the workflow * @return string The workflow instance ID * - * @throws InvalidWorkflowDefinitionException If the workflow definition is invalid + * @throws \SolutionForest\WorkflowEngine\Exceptions\InvalidWorkflowDefinitionException If the workflow definition is invalid * @throws \RuntimeException If the workflow cannot be started due to system issues * * @example Starting a simple workflow @@ -250,6 +249,16 @@ public function getInstances(array $filters = []): array public function cancel(string $instanceId, string $reason = ''): WorkflowInstance { $instance = $this->stateManager->load($instanceId); + + if ($instance->getState()->isFinished()) { + throw new InvalidWorkflowStateException( + "Cannot cancel workflow '{$instanceId}' because it is in '{$instance->getState()->value}' state", + $instance->getState(), + WorkflowState::CANCELLED, + $instanceId + ); + } + $instance->setState(WorkflowState::CANCELLED); $this->stateManager->save($instance); diff --git a/src/Core/WorkflowInstance.php b/src/Core/WorkflowInstance.php index bcb68bf..31ab20c 100644 --- a/src/Core/WorkflowInstance.php +++ b/src/Core/WorkflowInstance.php @@ -2,6 +2,8 @@ namespace SolutionForest\WorkflowEngine\Core; +use SolutionForest\WorkflowEngine\Exceptions\InvalidWorkflowStateException; + /** * Represents an active instance of a workflow execution. * @@ -175,6 +177,15 @@ public function getState(): WorkflowState */ public function setState(WorkflowState $state): void { + if (! $this->state->canTransitionTo($state)) { + throw new InvalidWorkflowStateException( + "Cannot transition workflow '{$this->id}' from '{$this->state->value}' to '{$state->value}'", + $this->state, + $state, + $this->id + ); + } + $this->state = $state; $this->updatedAt = new \DateTime; } diff --git a/src/Core/WorkflowState.php b/src/Core/WorkflowState.php index 6e6ec44..3e46678 100644 --- a/src/Core/WorkflowState.php +++ b/src/Core/WorkflowState.php @@ -242,8 +242,8 @@ public function icon(): string public function canTransitionTo(self $state): bool { return match ($this) { - // From PENDING: can start running or be cancelled - self::PENDING => in_array($state, [self::RUNNING, self::CANCELLED]), + // From PENDING: can start running, fail, or be cancelled + self::PENDING => in_array($state, [self::RUNNING, self::FAILED, self::CANCELLED]), // From RUNNING: can wait, pause, complete, fail, or be cancelled self::RUNNING => in_array($state, [ @@ -257,8 +257,8 @@ public function canTransitionTo(self $state): bool // From WAITING: can resume running, fail, or be cancelled self::WAITING => in_array($state, [self::RUNNING, self::FAILED, self::CANCELLED]), - // From PAUSED: can resume running or be cancelled - self::PAUSED => in_array($state, [self::RUNNING, self::CANCELLED]), + // From PAUSED: can resume running, fail, or be cancelled + self::PAUSED => in_array($state, [self::RUNNING, self::FAILED, self::CANCELLED]), // Terminal states cannot transition to other states default => false, diff --git a/src/Events/StepRetriedEvent.php b/src/Events/StepRetriedEvent.php new file mode 100644 index 0000000..a57e6c3 --- /dev/null +++ b/src/Events/StepRetriedEvent.php @@ -0,0 +1,27 @@ +instance->getId(); + } + + public function getStepId(): string + { + return $this->step->getId(); + } +} diff --git a/src/Exceptions/ActionNotFoundException.php b/src/Exceptions/ActionNotFoundException.php index e8e4dd1..fe09a20 100644 --- a/src/Exceptions/ActionNotFoundException.php +++ b/src/Exceptions/ActionNotFoundException.php @@ -4,6 +4,7 @@ use SolutionForest\WorkflowEngine\Core\Step; use SolutionForest\WorkflowEngine\Core\WorkflowContext; +use SolutionForest\WorkflowEngine\Support\Arr; /** * Thrown when a workflow action class cannot be found or loaded. @@ -108,7 +109,7 @@ public function getSuggestions(): array if (! class_exists($this->actionClass)) { $suggestions[] = "Ensure the action class '{$this->actionClass}' exists and is properly autoloaded"; - $suggested = $this->suggestNamespace(class_basename($this->actionClass)); + $suggested = $this->suggestNamespace(Arr::classBasename($this->actionClass)); if ($suggested) { $suggestions[] = "Did you mean '{$suggested}'?"; } diff --git a/src/Exceptions/InvalidWorkflowDefinitionException.php b/src/Exceptions/InvalidWorkflowDefinitionException.php index 544a272..111b596 100644 --- a/src/Exceptions/InvalidWorkflowDefinitionException.php +++ b/src/Exceptions/InvalidWorkflowDefinitionException.php @@ -219,11 +219,12 @@ public static function invalidName(string $name, ?string $reason = null): static * Create exception for invalid condition expression. * * @param string $condition The invalid condition expression + * @param string $reason The reason why the condition is invalid */ - public static function invalidCondition(string $condition): static + public static function invalidCondition(string $condition, string $reason): static { return new self( - message: "Invalid condition expression: '{$condition}'. Condition cannot be empty.", + message: "Invalid condition expression: '{$condition}'. {$reason}", definition: ['provided_condition' => $condition], validationErrors: [ 'Use valid condition expressions with comparison operators', diff --git a/src/Exceptions/InvalidWorkflowStateException.php b/src/Exceptions/InvalidWorkflowStateException.php index 4c5d573..6a4dd44 100644 --- a/src/Exceptions/InvalidWorkflowStateException.php +++ b/src/Exceptions/InvalidWorkflowStateException.php @@ -211,4 +211,26 @@ public static function fromInstanceTransition( $instance->getId() ); } + + /** + * Create an exception for an invalid state transition. + * + * @param string $instanceId The workflow instance ID + * @param WorkflowState $currentState The current state + * @param WorkflowState $attemptedState The attempted target state + */ + public static function invalidTransition( + string $instanceId, + WorkflowState $currentState, + WorkflowState $attemptedState + ): static { + $message = "Cannot transition workflow '{$instanceId}' from '{$currentState->value}' to '{$attemptedState->value}'"; + + return new self( + $message, + $currentState, + $attemptedState, + $instanceId + ); + } } diff --git a/src/Exceptions/StepExecutionException.php b/src/Exceptions/StepExecutionException.php index cbed8c4..55ef961 100644 --- a/src/Exceptions/StepExecutionException.php +++ b/src/Exceptions/StepExecutionException.php @@ -4,6 +4,7 @@ use SolutionForest\WorkflowEngine\Core\Step; use SolutionForest\WorkflowEngine\Core\WorkflowContext; +use SolutionForest\WorkflowEngine\Support\Arr; /** * Thrown when a workflow step fails during execution. @@ -81,7 +82,7 @@ public function canRetry(): bool public function getUserMessage(): string { $stepName = $this->step->getId(); - $actionClass = class_basename($this->step->getActionClass()); + $actionClass = Arr::classBasename($this->step->getActionClass()); return "The workflow step '{$stepName}' (using {$actionClass}) failed to execute. ". 'This may be due to invalid input data, external service issues, or configuration problems.'; diff --git a/src/Support/Arr.php b/src/Support/Arr.php index 6d42053..3efd3a3 100644 --- a/src/Support/Arr.php +++ b/src/Support/Arr.php @@ -25,6 +25,16 @@ public static function get(array $array, string $key, mixed $default = null): mi return $array; } + /** + * Get the class "basename" of a class string (without namespace). + */ + public static function classBasename(string $class): string + { + $parts = explode('\\', $class); + + return end($parts); + } + /** * Set a value in a nested array using dot notation. * diff --git a/src/Support/ConditionEvaluator.php b/src/Support/ConditionEvaluator.php new file mode 100644 index 0000000..72a7739 --- /dev/null +++ b/src/Support/ConditionEvaluator.php @@ -0,0 +1,45 @@ + $data Workflow data to evaluate against + * @return bool True if condition evaluates to true + * + * @throws InvalidWorkflowDefinitionException If condition format is invalid + */ + public static function evaluate(string $condition, array $data): bool + { + if (! preg_match('/(\w+(?:\.\w+)*)\s*(===|!==|>=|<=|==|!=|>|<)\s*(.+)/', $condition, $matches)) { + throw InvalidWorkflowDefinitionException::invalidCondition( + $condition, + 'Condition must be in format: "key operator value" (e.g., "user.plan === premium")' + ); + } + + $key = $matches[1]; + $operator = $matches[2]; + $value = trim($matches[3], '"\''); + + $dataValue = Arr::get($data, $key); + + return match ($operator) { + '===' => $dataValue === $value, + '!==' => $dataValue !== $value, + '>=' => $dataValue >= $value, + '<=' => $dataValue <= $value, + '==' => $dataValue == $value, + '!=' => $dataValue != $value, + '>' => $dataValue > $value, + '<' => $dataValue < $value, + default => false, + }; + } +} diff --git a/src/Support/SimpleWorkflow.php b/src/Support/SimpleWorkflow.php index bcb3a72..c4c27d4 100644 --- a/src/Support/SimpleWorkflow.php +++ b/src/Support/SimpleWorkflow.php @@ -108,7 +108,7 @@ public function sequential(string $name, array $actions, array $context = []): s public function runAction(string $actionClass, array $context = []): string { return $this->sequential( - 'single_action_'.class_basename($actionClass), + 'single_action_'.Arr::classBasename($actionClass), [$actionClass], $context ); diff --git a/src/Support/Timeout.php b/src/Support/Timeout.php new file mode 100644 index 0000000..5670b8a --- /dev/null +++ b/src/Support/Timeout.php @@ -0,0 +1,44 @@ + $value, + 'm' => $value * 60, + 'h' => $value * 3600, + 'd' => $value * 86400, + }; + } +} diff --git a/tests/Integration/EventDispatchTest.php b/tests/Integration/EventDispatchTest.php index 8bc63d1..45e822d 100644 --- a/tests/Integration/EventDispatchTest.php +++ b/tests/Integration/EventDispatchTest.php @@ -106,7 +106,16 @@ ], ]; - $id = $engine->start('event-test-4', $definition); + // Create a workflow in RUNNING state so it can be cancelled + $parser = new \SolutionForest\WorkflowEngine\Core\DefinitionParser; + $workflowDef = $parser->parse($definition); + $id = 'event-test-4'; + $instance = new \SolutionForest\WorkflowEngine\Core\WorkflowInstance( + id: $id, + definition: $workflowDef, + state: \SolutionForest\WorkflowEngine\Core\WorkflowState::RUNNING, + ); + $storage->save($instance); $spy->reset(); $engine->cancel($id, 'testing cancellation'); diff --git a/tests/Integration/WorkflowIntegrationTest.php b/tests/Integration/WorkflowIntegrationTest.php index fa017b3..40317cc 100644 --- a/tests/Integration/WorkflowIntegrationTest.php +++ b/tests/Integration/WorkflowIntegrationTest.php @@ -103,8 +103,16 @@ ], ]; - // Start workflow - $workflowId = $this->engine->start('cancellable-workflow', $definition); + // Create a workflow in RUNNING state so it can be cancelled + $parser = new \SolutionForest\WorkflowEngine\Core\DefinitionParser; + $workflowDef = $parser->parse($definition); + $workflowId = 'cancellable-workflow'; + $instance = new \SolutionForest\WorkflowEngine\Core\WorkflowInstance( + id: $workflowId, + definition: $workflowDef, + state: \SolutionForest\WorkflowEngine\Core\WorkflowState::RUNNING, + ); + $this->storage->save($instance); // Cancel workflow $this->engine->cancel($workflowId, 'User requested cancellation'); @@ -131,25 +139,35 @@ // Start two workflows $workflow1Id = $this->engine->start('list-test-1', $definition1); - $workflow2Id = $this->engine->start('list-test-2', $definition2); + + // For the second one, create it in RUNNING state so we can cancel it + $parser = new \SolutionForest\WorkflowEngine\Core\DefinitionParser; + $workflowDef = $parser->parse($definition2); + $workflow2Id = 'list-test-2'; + $instance = new \SolutionForest\WorkflowEngine\Core\WorkflowInstance( + id: $workflow2Id, + definition: $workflowDef, + state: \SolutionForest\WorkflowEngine\Core\WorkflowState::RUNNING, + ); + $this->storage->save($instance); // Cancel one $this->engine->cancel($workflow2Id); // List all workflows - $allWorkflows = $this->engine->listWorkflows(); + $allWorkflows = $this->engine->getInstances(); expect(count($allWorkflows))->toBeGreaterThanOrEqual(2); // Filter by state - $completedWorkflows = $this->engine->listWorkflows(['state' => WorkflowState::COMPLETED]); - $cancelledWorkflows = $this->engine->listWorkflows(['state' => WorkflowState::CANCELLED]); + $completedWorkflows = $this->engine->getInstances(['state' => WorkflowState::COMPLETED]); + $cancelledWorkflows = $this->engine->getInstances(['state' => WorkflowState::CANCELLED]); expect(count($completedWorkflows))->toBeGreaterThanOrEqual(1); expect(count($cancelledWorkflows))->toBeGreaterThanOrEqual(1); // Verify specific workflows exist in filtered results - $completedIds = array_column($completedWorkflows, 'workflow_id'); - $cancelledIds = array_column($cancelledWorkflows, 'workflow_id'); + $completedIds = array_map(fn ($w) => $w->getId(), $completedWorkflows); + $cancelledIds = array_map(fn ($w) => $w->getId(), $cancelledWorkflows); expect($completedIds)->toContain($workflow1Id); expect($cancelledIds)->toContain($workflow2Id); diff --git a/tests/RealWorld/DocumentApprovalWorkflowTest.php b/tests/RealWorld/DocumentApprovalWorkflowTest.php index 7fa9ea5..f212792 100644 --- a/tests/RealWorld/DocumentApprovalWorkflowTest.php +++ b/tests/RealWorld/DocumentApprovalWorkflowTest.php @@ -338,8 +338,8 @@ ], 'transitions' => [ ['from' => 'submit_document', 'to' => 'manager_review'], - ['from' => 'manager_review', 'to' => 'director_escalation', 'condition' => 'timeout_occurred'], - ['from' => 'director_escalation', 'to' => 'executive_override', 'condition' => 'escalation_required'], + ['from' => 'manager_review', 'to' => 'director_escalation', 'condition' => 'timeout_occurred === "true"'], + ['from' => 'director_escalation', 'to' => 'executive_override', 'condition' => 'escalation_required === "true"'], ], 'escalation_rules' => [ 'timeout_escalation' => true, diff --git a/tests/Unit/ExecutorRetryTest.php b/tests/Unit/ExecutorRetryTest.php new file mode 100644 index 0000000..b37daf0 --- /dev/null +++ b/tests/Unit/ExecutorRetryTest.php @@ -0,0 +1,116 @@ +getConfig('fail_count', 0); + + if (self::$callCount <= $failCount) { + throw new \RuntimeException('Intentional failure #'.self::$callCount); + } + + return ActionResult::success(['attempts' => self::$callCount]); + } + + public function getName(): string + { + return 'Fail N Times'; + } + + public function getDescription(): string + { + return 'Test action that fails a configurable number of times'; + } +} + +// A test action that always fails +class AlwaysFailAction extends BaseAction +{ + protected function doExecute(WorkflowContext $context): ActionResult + { + throw new \RuntimeException('Always fails'); + } + + public function getName(): string + { + return 'Always Fail'; + } + + public function getDescription(): string + { + return 'Test action that always fails'; + } +} + +describe('Executor Retry Logic', function () { + beforeEach(function () { + FailNTimesAction::reset(); + }); + + test('retries and succeeds after transient failure', function () { + $storage = new InMemoryStorage; + $spy = new SpyEventDispatcher; + $engine = new WorkflowEngine($storage, $spy); + + $definition = WorkflowBuilder::create('retry-test') + ->addStep('flaky_step', FailNTimesAction::class, ['fail_count' => 2], retryAttempts: 3) + ->build(); + + $id = $engine->start('retry-success', $definition->toArray(), []); + $instance = $engine->getInstance($id); + + expect($instance->getState()->value)->toBe('completed'); + + // Should have dispatched StepRetried events + $retryEvents = array_filter($spy->dispatched, fn ($e) => str_contains(get_class($e), 'StepRetried')); + expect(count($retryEvents))->toBe(2); // Failed twice before succeeding + }); + + test('fails after exhausting all retry attempts', function () { + $storage = new InMemoryStorage; + $spy = new SpyEventDispatcher; + $engine = new WorkflowEngine($storage, $spy); + + $definition = WorkflowBuilder::create('retry-exhaust') + ->addStep('always_fail', AlwaysFailAction::class, retryAttempts: 2) + ->build(); + + expect(fn () => $engine->start('retry-fail', $definition->toArray(), [])) + ->toThrow(StepExecutionException::class); + }); + + test('does not retry when retryAttempts is 0', function () { + $storage = new InMemoryStorage; + $spy = new SpyEventDispatcher; + $engine = new WorkflowEngine($storage, $spy); + + $definition = WorkflowBuilder::create('no-retry') + ->addStep('fail_once', AlwaysFailAction::class, retryAttempts: 0) + ->build(); + + expect(fn () => $engine->start('no-retry', $definition->toArray(), [])) + ->toThrow(StepExecutionException::class); + + $retryEvents = array_filter($spy->dispatched, fn ($e) => str_contains(get_class($e), 'StepRetried')); + expect(count($retryEvents))->toBe(0); + }); +}); diff --git a/tests/Unit/Support/ConditionEvaluatorTest.php b/tests/Unit/Support/ConditionEvaluatorTest.php new file mode 100644 index 0000000..40121af --- /dev/null +++ b/tests/Unit/Support/ConditionEvaluatorTest.php @@ -0,0 +1,65 @@ + 'premium']; + expect(ConditionEvaluator::evaluate('plan === "premium"', $data))->toBeTrue(); + expect(ConditionEvaluator::evaluate('plan === "basic"', $data))->toBeFalse(); + }); + + test('evaluates strict inequality', function () { + $data = ['plan' => 'premium']; + expect(ConditionEvaluator::evaluate('plan !== "basic"', $data))->toBeTrue(); + expect(ConditionEvaluator::evaluate('plan !== "premium"', $data))->toBeFalse(); + }); + + test('evaluates loose equality', function () { + $data = ['count' => '5']; + expect(ConditionEvaluator::evaluate('count == 5', $data))->toBeTrue(); + }); + + test('evaluates greater than', function () { + $data = ['score' => '90']; + expect(ConditionEvaluator::evaluate('score > 50', $data))->toBeTrue(); + expect(ConditionEvaluator::evaluate('score > 100', $data))->toBeFalse(); + }); + + test('evaluates less than', function () { + $data = ['score' => '30']; + expect(ConditionEvaluator::evaluate('score < 50', $data))->toBeTrue(); + }); + + test('evaluates greater than or equal', function () { + $data = ['score' => '50']; + expect(ConditionEvaluator::evaluate('score >= 50', $data))->toBeTrue(); + }); + + test('evaluates less than or equal', function () { + $data = ['score' => '50']; + expect(ConditionEvaluator::evaluate('score <= 50', $data))->toBeTrue(); + }); + + test('handles dot notation for nested data', function () { + $data = ['user' => ['profile' => ['tier' => 'gold']]]; + expect(ConditionEvaluator::evaluate('user.profile.tier === "gold"', $data))->toBeTrue(); + expect(ConditionEvaluator::evaluate('user.profile.tier === "silver"', $data))->toBeFalse(); + }); + + test('throws on unparseable condition', function () { + expect(fn () => ConditionEvaluator::evaluate('not a valid condition', ['key' => 'val'])) + ->toThrow(InvalidWorkflowDefinitionException::class); + }); + + test('throws on empty condition', function () { + expect(fn () => ConditionEvaluator::evaluate('', [])) + ->toThrow(InvalidWorkflowDefinitionException::class); + }); + + test('handles missing data key gracefully', function () { + $data = ['name' => 'John']; + expect(ConditionEvaluator::evaluate('age === "30"', $data))->toBeFalse(); + }); +}); diff --git a/tests/Unit/Support/TimeoutTest.php b/tests/Unit/Support/TimeoutTest.php new file mode 100644 index 0000000..9d9e5b4 --- /dev/null +++ b/tests/Unit/Support/TimeoutTest.php @@ -0,0 +1,41 @@ +toBe(30); + expect(Timeout::toSeconds(0))->toBe(0); + }); + + test('parses numeric string', function () { + expect(Timeout::toSeconds('300'))->toBe(300); + }); + + test('parses seconds suffix', function () { + expect(Timeout::toSeconds('30s'))->toBe(30); + }); + + test('parses minutes suffix', function () { + expect(Timeout::toSeconds('5m'))->toBe(300); + }); + + test('parses hours suffix', function () { + expect(Timeout::toSeconds('2h'))->toBe(7200); + }); + + test('parses days suffix', function () { + expect(Timeout::toSeconds('1d'))->toBe(86400); + }); + + test('throws on invalid format', function () { + expect(fn () => Timeout::toSeconds('invalid')) + ->toThrow(InvalidWorkflowDefinitionException::class); + }); + + test('throws on unsupported unit', function () { + expect(fn () => Timeout::toSeconds('5w')) + ->toThrow(InvalidWorkflowDefinitionException::class); + }); +}); diff --git a/tests/Unit/WorkflowEngineTest.php b/tests/Unit/WorkflowEngineTest.php index 738dc1f..8a4642d 100644 --- a/tests/Unit/WorkflowEngineTest.php +++ b/tests/Unit/WorkflowEngineTest.php @@ -8,8 +8,8 @@ use SolutionForest\WorkflowEngine\Tests\Support\InMemoryStorage; beforeEach(function () { - $storage = new InMemoryStorage; - $this->engine = new WorkflowEngine($storage); + $this->storage = new InMemoryStorage; + $this->engine = new WorkflowEngine($this->storage); }); test('it can start a workflow', function () { @@ -81,16 +81,20 @@ ], ]; - $workflowId = $this->engine->start('test-workflow', $definition); + // Create a paused workflow manually (bypass auto-execution) + $parser = new \SolutionForest\WorkflowEngine\Core\DefinitionParser; + $workflowDef = $parser->parse($definition); + $workflowId = 'test-workflow'; + $instance = new WorkflowInstance( + id: $workflowId, + definition: $workflowDef, + state: WorkflowState::PAUSED, + ); + $this->storage->save($instance); // Verify workflow was created expect($workflowId)->toBe('test-workflow'); - // Get instance and manually pause it using the same storage instance - $instance = $this->engine->getInstance($workflowId); - $instance->setState(WorkflowState::PAUSED); - $this->storage->save($instance); - // Resume it $this->engine->resume($workflowId); @@ -112,7 +116,17 @@ ], ]; - $workflowId = $this->engine->start('test-workflow', $definition); + // Create a workflow in RUNNING state (so it can be cancelled) + $parser = new \SolutionForest\WorkflowEngine\Core\DefinitionParser; + $workflowDef = $parser->parse($definition); + $workflowId = 'test-workflow'; + $instance = new WorkflowInstance( + id: $workflowId, + definition: $workflowDef, + state: WorkflowState::RUNNING, + ); + $this->storage->save($instance); + $this->engine->cancel($workflowId, 'User cancelled'); $instance = $this->engine->getInstance($workflowId); @@ -190,16 +204,28 @@ ], ]; + // Create a workflow that completes $completedId = $this->engine->start('completed-workflow', $definition); - $cancelledId = $this->engine->start('cancelled-workflow', $definition); + // Create a workflow in RUNNING state, then cancel it + $parser = new \SolutionForest\WorkflowEngine\Core\DefinitionParser; + $workflowDef = $parser->parse($definition); + $cancelledId = 'cancelled-workflow'; + $instance = new WorkflowInstance( + id: $cancelledId, + definition: $workflowDef, + state: WorkflowState::RUNNING, // Create in RUNNING state so we can cancel it + ); + $this->storage->save($instance); + + // Now cancel it $this->engine->cancel($cancelledId); - $completedWorkflows = $this->engine->listWorkflows(['state' => WorkflowState::COMPLETED]); - $cancelledWorkflows = $this->engine->listWorkflows(['state' => WorkflowState::CANCELLED]); + $completedWorkflows = $this->engine->getInstances(['state' => WorkflowState::COMPLETED]); + $cancelledWorkflows = $this->engine->getInstances(['state' => WorkflowState::CANCELLED]); // Debug: check if workflows exist - $allWorkflows = $this->engine->listWorkflows(); + $allWorkflows = $this->engine->getInstances(); expect($completedWorkflows)->toHaveCount(1); expect($cancelledWorkflows)->toHaveCount(1); @@ -207,7 +233,7 @@ // Find the workflows we created $found = false; foreach ($completedWorkflows as $workflow) { - if ($workflow['workflow_id'] === 'completed-workflow') { + if ($workflow->getId() === 'completed-workflow') { $found = true; break; } @@ -216,7 +242,7 @@ $found = false; foreach ($cancelledWorkflows as $workflow) { - if ($workflow['workflow_id'] === 'cancelled-workflow') { + if ($workflow->getId() === 'cancelled-workflow') { $found = true; break; } diff --git a/tests/Unit/WorkflowStateTransitionTest.php b/tests/Unit/WorkflowStateTransitionTest.php new file mode 100644 index 0000000..4b392ae --- /dev/null +++ b/tests/Unit/WorkflowStateTransitionTest.php @@ -0,0 +1,136 @@ + 'step1', 'action' => 'log', 'parameters' => ['message' => 'test', 'level' => 'info']]], + ); + + return new WorkflowInstance( + id: 'test-'.uniqid(), + definition: $definition, + state: $state, + ); + } + + describe('valid transitions', function () { + test('PENDING can transition to RUNNING', function () { + $instance = createInstance(WorkflowState::PENDING); + $instance->setState(WorkflowState::RUNNING); + expect($instance->getState())->toBe(WorkflowState::RUNNING); + }); + + test('PENDING can transition to FAILED', function () { + $instance = createInstance(WorkflowState::PENDING); + $instance->setState(WorkflowState::FAILED); + expect($instance->getState())->toBe(WorkflowState::FAILED); + }); + + test('PENDING can transition to CANCELLED', function () { + $instance = createInstance(WorkflowState::PENDING); + $instance->setState(WorkflowState::CANCELLED); + expect($instance->getState())->toBe(WorkflowState::CANCELLED); + }); + + test('RUNNING can transition to COMPLETED', function () { + $instance = createInstance(WorkflowState::PENDING); + $instance->setState(WorkflowState::RUNNING); + $instance->setState(WorkflowState::COMPLETED); + expect($instance->getState())->toBe(WorkflowState::COMPLETED); + }); + + test('RUNNING can transition to FAILED', function () { + $instance = createInstance(WorkflowState::PENDING); + $instance->setState(WorkflowState::RUNNING); + $instance->setState(WorkflowState::FAILED); + expect($instance->getState())->toBe(WorkflowState::FAILED); + }); + + test('RUNNING can transition to PAUSED', function () { + $instance = createInstance(WorkflowState::PENDING); + $instance->setState(WorkflowState::RUNNING); + $instance->setState(WorkflowState::PAUSED); + expect($instance->getState())->toBe(WorkflowState::PAUSED); + }); + + test('RUNNING can transition to WAITING', function () { + $instance = createInstance(WorkflowState::PENDING); + $instance->setState(WorkflowState::RUNNING); + $instance->setState(WorkflowState::WAITING); + expect($instance->getState())->toBe(WorkflowState::WAITING); + }); + + test('RUNNING can transition to CANCELLED', function () { + $instance = createInstance(WorkflowState::PENDING); + $instance->setState(WorkflowState::RUNNING); + $instance->setState(WorkflowState::CANCELLED); + expect($instance->getState())->toBe(WorkflowState::CANCELLED); + }); + + test('PAUSED can transition to RUNNING', function () { + $instance = createInstance(WorkflowState::PENDING); + $instance->setState(WorkflowState::RUNNING); + $instance->setState(WorkflowState::PAUSED); + $instance->setState(WorkflowState::RUNNING); + expect($instance->getState())->toBe(WorkflowState::RUNNING); + }); + + test('WAITING can transition to RUNNING', function () { + $instance = createInstance(WorkflowState::PENDING); + $instance->setState(WorkflowState::RUNNING); + $instance->setState(WorkflowState::WAITING); + $instance->setState(WorkflowState::RUNNING); + expect($instance->getState())->toBe(WorkflowState::RUNNING); + }); + }); + + describe('invalid transitions', function () { + test('COMPLETED cannot transition to RUNNING', function () { + $instance = createInstance(WorkflowState::PENDING); + $instance->setState(WorkflowState::RUNNING); + $instance->setState(WorkflowState::COMPLETED); + + expect(fn () => $instance->setState(WorkflowState::RUNNING)) + ->toThrow(InvalidWorkflowStateException::class); + }); + + test('FAILED cannot transition to RUNNING', function () { + $instance = createInstance(WorkflowState::PENDING); + $instance->setState(WorkflowState::FAILED); + + expect(fn () => $instance->setState(WorkflowState::RUNNING)) + ->toThrow(InvalidWorkflowStateException::class); + }); + + test('CANCELLED cannot transition to any state', function () { + $instance = createInstance(WorkflowState::PENDING); + $instance->setState(WorkflowState::CANCELLED); + + expect(fn () => $instance->setState(WorkflowState::RUNNING)) + ->toThrow(InvalidWorkflowStateException::class); + }); + + test('PENDING cannot transition to COMPLETED directly', function () { + $instance = createInstance(WorkflowState::PENDING); + + expect(fn () => $instance->setState(WorkflowState::COMPLETED)) + ->toThrow(InvalidWorkflowStateException::class); + }); + + test('PENDING cannot transition to PAUSED directly', function () { + $instance = createInstance(WorkflowState::PENDING); + + expect(fn () => $instance->setState(WorkflowState::PAUSED)) + ->toThrow(InvalidWorkflowStateException::class); + }); + }); +}); From fb57997b9dd67cff5d0a9d09ae481ed66f78eaf0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 19 Feb 2026 06:00:17 +0000 Subject: [PATCH 5/7] docs: update README.md and CLAUDE.md with accurate project state Sync documentation with actual codebase after recent improvements: - Update file counts (46 src, 25 test files) and test stats (93 tests, 224+ assertions) - Fix Quick Start code examples to use the actual WorkflowEngine API - Add complete namespace map with all classes including DefinitionParser, ActionResolver, StepRetriedEvent, ConditionEvaluator, Arr, Timeout - Document enforced state transitions with valid transition rules - Update StorageAdapter example with all 6 interface methods - Fix EventDispatcher example with correct class names and modern PHP syntax - Add debug() to Logger example, matching actual contract - Document quick template for documentApproval() - Add new quality standards: architecture tests, state transition validation, CI matrix https://claude.ai/code/session_013CdFJtmtMmSqBZFwdrmicG --- CLAUDE.md | 41 ++++--- README.md | 334 +++++++++++++++++++++++------------------------------- 2 files changed, 165 insertions(+), 210 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f782a9b..2941c2d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -49,29 +49,34 @@ WorkflowBuilder → WorkflowDefinition → WorkflowEngine → Executor → Actio | Namespace | Purpose | |-----------|---------| -| `Core\` | Engine, Builder, Executor, StateManager, Step, WorkflowInstance, WorkflowDefinition, WorkflowContext, ActionResult | -| `Actions\` | Built-in actions: Log, Email, Http, Delay, Condition (all extend BaseAction) | -| `Contracts\` | Interfaces: WorkflowAction, StorageAdapter, EventDispatcher, Logger | -| `Attributes\` | PHP 8 attributes: WorkflowStep, Retry, Timeout, Condition | -| `Events\` | WorkflowStarted, WorkflowCompleted, WorkflowFailed, WorkflowCancelled, StepCompleted, StepFailed | -| `Exceptions\` | WorkflowException (base), InvalidWorkflowDefinition, InvalidWorkflowState, ActionNotFound, StepExecution, WorkflowInstanceNotFound | -| `Support\` | NullLogger, NullEventDispatcher, SimpleWorkflow, Uuid | +| `Core\` | WorkflowEngine, WorkflowBuilder, Executor, StateManager, WorkflowInstance, WorkflowDefinition, WorkflowContext, ActionResult, Step, DefinitionParser, ActionResolver | +| `Actions\` | BaseAction, LogAction, EmailAction, HttpAction, DelayAction, ConditionAction | +| `Contracts\` | WorkflowAction, StorageAdapter, EventDispatcher, Logger | +| `Attributes\` | WorkflowStep, Retry, Timeout, Condition | +| `Events\` | WorkflowStartedEvent, WorkflowCompletedEvent, WorkflowFailedEvent, WorkflowCancelledEvent, StepCompletedEvent, StepFailedEvent, StepRetriedEvent | +| `Exceptions\` | WorkflowException (base), InvalidWorkflowDefinitionException, InvalidWorkflowStateException, ActionNotFoundException, StepExecutionException, WorkflowInstanceNotFoundException | +| `Support\` | NullLogger, NullEventDispatcher, SimpleWorkflow, Uuid, Timeout, ConditionEvaluator, Arr | ### State Machine ``` PENDING → RUNNING → COMPLETED - ↓ ↑ - WAITING - ↓ ↑ - PAUSED - ↓ - FAILED - + ↓ ↓ ↑ + FAILED WAITING + ↑ ↓ ↑ + FAILED ← PAUSED + ↑ CANCELLED ← (any non-terminal state) ``` -Terminal states: COMPLETED, FAILED, CANCELLED. +**Valid transitions (enforced at runtime):** +- `PENDING` → `RUNNING`, `FAILED`, `CANCELLED` +- `RUNNING` → `WAITING`, `PAUSED`, `COMPLETED`, `FAILED`, `CANCELLED` +- `WAITING` → `RUNNING`, `FAILED`, `CANCELLED` +- `PAUSED` → `RUNNING`, `FAILED`, `CANCELLED` +- Terminal states (`COMPLETED`, `FAILED`, `CANCELLED`) → no transitions allowed + +Invalid transitions throw `InvalidWorkflowStateException`. ### Key Contracts @@ -157,7 +162,7 @@ GitHub Actions workflows: ## File Counts -- 42 source files in `src/` -- 18 test files in `tests/` -- 40+ test cases, 160+ assertions +- 46 source files in `src/` +- 25 test files in `tests/` +- 93 tests, 224+ assertions - PHPStan level 6 compliance diff --git a/README.md b/README.md index b30f6a7..9d2ce27 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ A powerful, framework-agnostic workflow engine for PHP applications. This core l - **⏱️ Timeouts**: Step-level timeout controls for reliable execution - **📋 Conditions**: Conditional workflow execution based on runtime data - **🎯 Events**: Rich event system for monitoring and integration -- **🧪 Well Tested**: Comprehensive test suite with 160+ assertions +- **🧪 Well Tested**: Comprehensive test suite with 93 tests and 224+ assertions ## 📦 Installation @@ -64,6 +64,7 @@ composer ci use SolutionForest\WorkflowEngine\Core\WorkflowBuilder; use SolutionForest\WorkflowEngine\Core\WorkflowEngine; use SolutionForest\WorkflowEngine\Core\WorkflowContext; +use SolutionForest\WorkflowEngine\Core\ActionResult; use SolutionForest\WorkflowEngine\Actions\BaseAction; // Define custom actions @@ -72,237 +73,194 @@ class ValidateOrderAction extends BaseAction public function execute(WorkflowContext $context): ActionResult { $orderId = $context->getData('order_id'); - + // Your validation logic here if ($this->isValidOrder($orderId)) { return ActionResult::success(['validated' => true]); } - + return ActionResult::failure('Invalid order'); } } -// Create a workflow -$workflow = WorkflowBuilder::create('order-processing') +// Build a workflow definition +$definition = WorkflowBuilder::create('order-processing') + ->description('Process customer orders') ->addStep('validate', ValidateOrderAction::class) - ->addStep('payment', ProcessPaymentAction::class) + ->addStep('payment', ProcessPaymentAction::class, timeout: 300, retryAttempts: 3) ->addStep('fulfillment', FulfillOrderAction::class) - ->addTransition('validate', 'payment') - ->addTransition('payment', 'fulfillment') ->build(); -// Execute the workflow -$engine = new WorkflowEngine(); -$context = new WorkflowContext( - workflowId: 'order-processing', - stepId: 'validate', - data: ['order_id' => 123, 'customer_id' => 456] +// Create engine with storage adapter and event dispatcher +$engine = new WorkflowEngine($storageAdapter, $eventDispatcher); + +// Start and run the workflow +$instanceId = $engine->start( + 'order-processing', + $definition->toArray(), + ['order_id' => 123, 'customer_id' => 456] ); -$instance = $engine->start($workflow, $context); -$result = $engine->executeStep($instance, $context); +// Check the result +$instance = $engine->getInstance($instanceId); +echo $instance->getState()->value; // "completed" ``` -### Advanced Features +### Advanced Workflow Builder -#### Conditional Steps ```php -use SolutionForest\WorkflowEngine\Attributes\Condition; +use SolutionForest\WorkflowEngine\Core\WorkflowBuilder; -class ConditionalAction extends BaseAction -{ - #[Condition("data.amount > 1000")] - public function execute(WorkflowContext $context): ActionResult - { - // This action only executes if amount > 1000 - return ActionResult::success(); - } -} +$workflow = WorkflowBuilder::create('order-flow') + ->description('Process customer orders') + ->addStep('validate', ValidateOrderAction::class) + ->when('order.total > 1000', function ($builder) { + $builder->addStep('fraud_check', FraudCheckAction::class); + }) + ->addStep('payment', ProcessPaymentAction::class, timeout: 300, retryAttempts: 3) + ->email('order-confirmation', 'customer@example.com', 'Order Confirmed') + ->build(); + +// Quick templates for common patterns +$workflow = WorkflowBuilder::quick()->userOnboarding(); +$workflow = WorkflowBuilder::quick()->orderProcessing(); +$workflow = WorkflowBuilder::quick()->documentApproval(); ``` -#### Retry Logic +### Retry, Timeout & Conditions + ```php -use SolutionForest\WorkflowEngine\Attributes\Retry; +// Steps with retry and timeout configured via builder +$workflow = WorkflowBuilder::create('reliable-flow') + ->addStep('fetch_data', FetchDataAction::class, timeout: 30, retryAttempts: 3) + ->addStep('process', ProcessAction::class, timeout: 60) + ->build(); -class ReliableAction extends BaseAction -{ - #[Retry(maxAttempts: 3, delay: 1000)] - public function execute(WorkflowContext $context): ActionResult - { - // This action will retry up to 3 times with 1 second delay - return ActionResult::success(); - } -} +// Conditional steps evaluated at runtime +$workflow = WorkflowBuilder::create('conditional-flow') + ->addStep('validate', ValidateAction::class) + ->when('order.total > 1000', function ($builder) { + $builder->addStep('fraud_check', FraudCheckAction::class); + }) + ->addStep('complete', CompleteAction::class) + ->build(); ``` -#### Timeouts -```php -use SolutionForest\WorkflowEngine\Attributes\Timeout; +### Workflow Lifecycle Management -class TimedAction extends BaseAction -{ - #[Timeout(seconds: 30)] - public function execute(WorkflowContext $context): ActionResult - { - // This action will timeout after 30 seconds - return ActionResult::success(); - } -} +```php +// Start, pause, resume, cancel +$instanceId = $engine->start('my-workflow', $definition->toArray(), ['key' => 'value']); +$instance = $engine->getInstance($instanceId); +$engine->cancel($instanceId, 'No longer needed'); + +// Query instances +$instances = $engine->getInstances(['state' => 'running']); +$status = $engine->getStatus('my-workflow'); ``` ## 🏗️ Architecture -The workflow engine follows a clean architecture pattern with clear separation of concerns: +The workflow engine follows a clean architecture with clear separation of concerns: + +``` +WorkflowBuilder → WorkflowDefinition → WorkflowEngine → Executor → Actions + ↓ + StateManager → StorageAdapter + ↓ + EventDispatcher +``` + +### Core Components + +| Component | Purpose | +|-----------|---------| +| **WorkflowBuilder** | Fluent API for constructing workflow definitions with `addStep()`, `when()`, `email()`, `delay()`, `http()` | +| **WorkflowDefinition** | Immutable blueprint containing steps, transitions, conditions, and metadata | +| **WorkflowEngine** | Central orchestrator — `start()`, `resume()`, `cancel()`, `getInstance()`, `getInstances()`, `getStatus()` | +| **Executor** | Runs steps sequentially with retry logic, timeout enforcement, and condition evaluation | +| **StateManager** | Coordinates persistence through StorageAdapter | +| **EventDispatcher** | Broadcasts 7 event types during workflow lifecycle | + +### State Machine ``` -┌─────────────────┐ -│ Workflow │ -│ Builder │ -└─────────────────┘ - │ - ▼ -┌─────────────────┐ ┌─────────────────┐ -│ Workflow │◄───│ Workflow │ -│ Definition │ │ Engine │ -└─────────────────┘ └─────────────────┘ - │ │ - ▼ ▼ -┌─────────────────┐ ┌─────────────────┐ -│ Steps │ │ Executor │ -│ & Actions │ │ │ -└─────────────────┘ └─────────────────┘ - │ │ - ▼ ▼ -┌─────────────────┐ ┌─────────────────┐ -│ State │ │ Events │ -│ Manager │ │ Dispatcher │ -└─────────────────┘ └─────────────────┘ +PENDING → RUNNING → COMPLETED + ↓ ↓ ↑ + FAILED WAITING + ↑ ↓ ↑ + FAILED ← PAUSED + ↑ +CANCELLED ← (any non-terminal state) ``` +**Valid transitions:** +- `PENDING` → `RUNNING`, `FAILED`, `CANCELLED` +- `RUNNING` → `WAITING`, `PAUSED`, `COMPLETED`, `FAILED`, `CANCELLED` +- `WAITING` → `RUNNING`, `FAILED`, `CANCELLED` +- `PAUSED` → `RUNNING`, `FAILED`, `CANCELLED` +- Terminal states (`COMPLETED`, `FAILED`, `CANCELLED`) → no further transitions -#### 📝 **Workflow Builder** -- **Purpose**: Fluent interface for creating workflow definitions -- **Responsibilities**: - - Provides method chaining (`.addStep()`, `.when()`, `.email()`, etc.) - - Validates workflow structure during construction - - Creates immutable workflow definitions - - Supports conditional steps and common patterns -- **Example**: `WorkflowBuilder::create('user-onboarding')->addStep(...)->build()` - -#### 📋 **Workflow Definition** -- **Purpose**: Immutable data structure representing a complete workflow -- **Responsibilities**: - - Contains workflow metadata (name, description, version) - - Stores all steps and their relationships - - Defines step execution order and conditions - - Serves as a blueprint for workflow execution -- **Key data**: Steps, transitions, conditions, metadata - -#### ⚡ **Workflow Engine** -- **Purpose**: Central orchestrator that manages workflow execution -- **Responsibilities**: - - Starts new workflow instances from definitions - - Manages workflow lifecycle (start, pause, resume, cancel) - - Coordinates between different components - - Provides API for workflow operations -- **Main methods**: `start()`, `pause()`, `resume()`, `cancel()`, `getInstance()` - -#### 🎯 **Steps & Actions** -- **Purpose**: Individual workflow tasks and their implementations -- **Responsibilities**: - - **Steps**: Define what should happen (metadata, config, conditions) - - **Actions**: Implement the actual business logic (`execute()` method) - - Handle step-specific configuration (timeout, retry, conditions) - - Support compensation actions for rollback scenarios -- **Examples**: `SendEmailAction`, `CreateUserAction`, `ValidateOrderAction` - -#### 🎬 **Executor** -- **Purpose**: Runtime engine that executes individual workflow steps -- **Responsibilities**: - - Executes actions in the correct sequence - - Handles conditional execution based on workflow context - - Manages timeouts and retry logic - - Processes step transitions and flow control - - Handles errors and compensation - -#### 🗄️ **State Manager** -- **Purpose**: Component responsible for workflow instance state persistence -- **Responsibilities**: - - Saves/loads workflow instances to/from storage - - Tracks workflow execution state (running, paused, completed, failed) - - Manages workflow context data - - Handles state transitions and validation - - Supports different storage adapters (database, file, memory) - -#### 📡 **Events Dispatcher** -- **Purpose**: Event system for monitoring and integration -- **Responsibilities**: - - Fires events during workflow execution - - Enables workflow monitoring and logging - - Supports custom event listeners - - Provides hooks for external system integration - - Events: `WorkflowStarted`, `StepCompleted`, `WorkflowFailed`, etc. - -### 🔄 **Data Flow** -1. **Builder** → creates → **Definition** -2. **Engine** → uses **Definition** to create instances -3. **Engine** → delegates to **Executor** for step execution -4. **Executor** → runs → **Steps & Actions** -5. **State Manager** → persists → workflow state -6. **Events Dispatcher** → broadcasts → execution events - -### ✅ **Architecture Benefits** -- **Separation of concerns** - each component has a single responsibility -- **Extensibility** - you can swap out storage adapters, add custom actions -- **Testability** - each component can be tested independently -- **Framework agnostic** - no dependencies on specific frameworks -- **Type safety** - full PHP 8.3+ type hints throughout +State transitions are validated at runtime — invalid transitions throw `InvalidWorkflowStateException`. + +### Namespace Map + +| Namespace | Contents | +|-----------|----------| +| `Core\` | WorkflowEngine, WorkflowBuilder, Executor, StateManager, WorkflowInstance, WorkflowDefinition, WorkflowContext, ActionResult, Step, DefinitionParser, ActionResolver | +| `Actions\` | BaseAction, LogAction, EmailAction, HttpAction, DelayAction, ConditionAction | +| `Contracts\` | WorkflowAction, StorageAdapter, EventDispatcher, Logger | +| `Attributes\` | WorkflowStep, Retry, Timeout, Condition | +| `Events\` | WorkflowStartedEvent, WorkflowCompletedEvent, WorkflowFailedEvent, WorkflowCancelledEvent, StepCompletedEvent, StepFailedEvent, StepRetriedEvent | +| `Exceptions\` | WorkflowException, InvalidWorkflowDefinitionException, InvalidWorkflowStateException, ActionNotFoundException, StepExecutionException, WorkflowInstanceNotFoundException | +| `Support\` | NullLogger, NullEventDispatcher, SimpleWorkflow, Uuid, Timeout, ConditionEvaluator, Arr | ## 🔧 Configuration ### Storage Adapters -Implement the `StorageAdapter` interface for custom storage: +Implement the `StorageAdapter` interface for custom persistence: ```php use SolutionForest\WorkflowEngine\Contracts\StorageAdapter; -class CustomStorageAdapter implements StorageAdapter +class DatabaseStorageAdapter implements StorageAdapter { - public function save(WorkflowInstance $instance): void - { - // Save workflow instance to your storage - } - - public function load(string $instanceId): ?WorkflowInstance - { - // Load workflow instance from your storage - } - - public function delete(string $instanceId): void - { - // Delete workflow instance from your storage - } + public function save(WorkflowInstance $instance): void { /* ... */ } + public function load(string $id): WorkflowInstance { /* ... */ } + public function findInstances(array $criteria = []): array { /* ... */ } + public function delete(string $id): void { /* ... */ } + public function exists(string $id): bool { /* ... */ } + public function updateState(string $id, array $updates): void { /* ... */ } } ``` ### Event Handling -Listen to workflow events: +Listen to workflow events — 7 event types are dispatched during execution: ```php use SolutionForest\WorkflowEngine\Contracts\EventDispatcher; +use SolutionForest\WorkflowEngine\Events\WorkflowStartedEvent; +use SolutionForest\WorkflowEngine\Events\WorkflowCompletedEvent; +use SolutionForest\WorkflowEngine\Events\WorkflowFailedEvent; +use SolutionForest\WorkflowEngine\Events\WorkflowCancelledEvent; +use SolutionForest\WorkflowEngine\Events\StepCompletedEvent; +use SolutionForest\WorkflowEngine\Events\StepFailedEvent; +use SolutionForest\WorkflowEngine\Events\StepRetriedEvent; class CustomEventDispatcher implements EventDispatcher { public function dispatch(object $event): void { - // Handle workflow events - match (get_class($event)) { - 'SolutionForest\WorkflowEngine\Events\WorkflowStarted' => $this->onWorkflowStarted($event), - 'SolutionForest\WorkflowEngine\Events\StepCompletedEvent' => $this->onStepCompleted($event), - 'SolutionForest\WorkflowEngine\Events\WorkflowCompletedEvent' => $this->onWorkflowCompleted($event), + match ($event::class) { + WorkflowStartedEvent::class => $this->onWorkflowStarted($event), + WorkflowCompletedEvent::class => $this->onWorkflowCompleted($event), + WorkflowFailedEvent::class => $this->onWorkflowFailed($event), + StepCompletedEvent::class => $this->onStepCompleted($event), + StepFailedEvent::class => $this->onStepFailed($event), + StepRetriedEvent::class => $this->onStepRetried($event), default => null, }; } @@ -311,27 +269,17 @@ class CustomEventDispatcher implements EventDispatcher ### Logging -Provide custom logging implementation: +Provide a custom logging implementation (PSR-3 style): ```php use SolutionForest\WorkflowEngine\Contracts\Logger; class CustomLogger implements Logger { - public function info(string $message, array $context = []): void - { - // Log info messages - } - - public function error(string $message, array $context = []): void - { - // Log error messages - } - - public function warning(string $message, array $context = []): void - { - // Log warning messages - } + public function info(string $message, array $context = []): void { /* ... */ } + public function warning(string $message, array $context = []): void { /* ... */ } + public function error(string $message, array $context = []): void { /* ... */ } + public function debug(string $message, array $context = []): void { /* ... */ } } ``` @@ -392,11 +340,13 @@ composer pint && composer analyze && composer test We maintain high code quality through: -- **100% PHPStan Level 6** - Static analysis with no errors +- **100% PHPStan Level 6** - Static analysis with zero errors across 46 source files - **Laravel Pint** - Consistent code formatting following Laravel standards -- **Comprehensive Testing** - 40 tests with 160+ assertions covering all core functionality -- **Type Safety** - Full PHP 8.3+ type declarations and documentation -- **Continuous Integration** - Automated quality checks on every commit +- **Comprehensive Testing** - 93 tests with 224+ assertions covering unit, integration, and real-world scenarios +- **Architecture Tests** - Automated checks preventing debug functions in source code +- **State Transition Validation** - Runtime enforcement of valid workflow state transitions +- **Type Safety** - Full PHP 8.3+ type declarations throughout +- **Continuous Integration** - Automated quality checks on every commit (PHP 8.3/8.4 matrix) ## 📚 Framework Integrations From 42189731bce763ffc71f8ed35c662e7e8e81b034 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 19 Feb 2026 06:01:48 +0000 Subject: [PATCH 6/7] docs: restore PHP 8.3+ attribute examples in README Add back documentation for PHP 8 attributes (#[Retry], #[Timeout], #[Condition], #[WorkflowStep]) that were inadvertently removed. These are real features using readonly classes with native PHP attributes. https://claude.ai/code/session_013CdFJtmtMmSqBZFwdrmicG --- README.md | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 9d2ce27..97cea2e 100644 --- a/README.md +++ b/README.md @@ -127,7 +127,72 @@ $workflow = WorkflowBuilder::quick()->orderProcessing(); $workflow = WorkflowBuilder::quick()->documentApproval(); ``` -### Retry, Timeout & Conditions +### PHP 8.3+ Attributes + +Use native PHP attributes to configure actions with retry, timeout, and conditions: + +#### Retry Logic +```php +use SolutionForest\WorkflowEngine\Attributes\Retry; + +#[Retry(attempts: 3, backoff: 'exponential', delay: 1000)] +class ReliableApiAction extends BaseAction +{ + public function execute(WorkflowContext $context): ActionResult + { + // Retries up to 3 times with exponential backoff starting at 1s + return ActionResult::success(); + } +} +``` + +#### Timeouts +```php +use SolutionForest\WorkflowEngine\Attributes\Timeout; + +#[Timeout(seconds: 30)] +class TimedAction extends BaseAction +{ + public function execute(WorkflowContext $context): ActionResult + { + // Will timeout after 30 seconds + return ActionResult::success(); + } +} +``` + +#### Conditional Execution +```php +use SolutionForest\WorkflowEngine\Attributes\Condition; + +#[Condition('order.amount > 100')] +class PremiumProcessingAction extends BaseAction +{ + public function execute(WorkflowContext $context): ActionResult + { + // Only executes when order.amount > 100 + return ActionResult::success(); + } +} +``` + +#### Step Metadata +```php +use SolutionForest\WorkflowEngine\Attributes\WorkflowStep; + +#[WorkflowStep(id: 'send_email', name: 'Send Welcome Email', description: 'Sends a welcome email to the new user')] +class SendWelcomeEmailAction extends BaseAction +{ + public function execute(WorkflowContext $context): ActionResult + { + return ActionResult::success(); + } +} +``` + +### Retry, Timeout & Conditions via Builder + +These features can also be configured through the fluent builder API: ```php // Steps with retry and timeout configured via builder From 4f54059ea666ba853f1bb48c71cef2a8eaf284eb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 19 Feb 2026 06:41:01 +0000 Subject: [PATCH 7/7] refactor: standardize event classes and improve README documentation - Standardize WorkflowCompletedEvent and WorkflowFailedEvent to use `final readonly class` with constructor promotion, matching all other event classes. Added helper methods (getWorkflowId, getWorkflowName, getErrorMessage) for consistency. - README improvements: - Remove broken /docs link that pointed to non-existent directory - Document SimpleWorkflow helper with usage examples - Document getInstances() filter options (state, definition_name, created_after, created_before, limit, offset) - Document getProgress() and getStatusSummary() on WorkflowInstance - Add built-in actions table (LogAction, ConditionAction, etc.) - Add WorkflowState helper methods section (isActive, color, icon, etc.) - Clarify PHP 8.3+ attributes are metadata annotations, not yet auto-parsed by the engine at runtime https://claude.ai/code/session_013CdFJtmtMmSqBZFwdrmicG --- README.md | 93 ++++++++++++++++++++++++--- src/Events/WorkflowCompletedEvent.php | 15 +++-- src/Events/WorkflowFailedEvent.php | 22 +++++-- 3 files changed, 111 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 97cea2e..3ebd6eb 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,9 @@ $workflow = WorkflowBuilder::quick()->documentApproval(); ### PHP 8.3+ Attributes -Use native PHP attributes to configure actions with retry, timeout, and conditions: +> **Note:** Attributes are currently metadata annotations for documentation and tooling. They are not yet auto-parsed by the engine at runtime — use the builder API (`timeout:`, `retryAttempts:`, `when()`) or step config to apply these behaviors. Attribute-driven execution is planned for a future release. + +Use native PHP attributes to annotate actions with retry, timeout, and conditions: #### Retry Logic ```php @@ -214,14 +216,57 @@ $workflow = WorkflowBuilder::create('conditional-flow') ### Workflow Lifecycle Management ```php -// Start, pause, resume, cancel +// Start, resume, cancel $instanceId = $engine->start('my-workflow', $definition->toArray(), ['key' => 'value']); $instance = $engine->getInstance($instanceId); +$engine->resume($instanceId); $engine->cancel($instanceId, 'No longer needed'); -// Query instances -$instances = $engine->getInstances(['state' => 'running']); -$status = $engine->getStatus('my-workflow'); +// Track progress +$progress = $instance->getProgress(); // 0.0 to 100.0 +$summary = $instance->getStatusSummary(); + +// Query instances with filters +$instances = $engine->getInstances([ + 'state' => 'running', + 'definition_name' => 'order-processing', + 'created_after' => new \DateTime('-7 days'), + 'limit' => 50, + 'offset' => 0, +]); +``` + +### SimpleWorkflow Helper + +For quick workflow execution without manual engine setup: + +```php +use SolutionForest\WorkflowEngine\Support\SimpleWorkflow; + +$simple = new SimpleWorkflow($storageAdapter); + +// Run actions sequentially +$instanceId = $simple->sequential('user-onboarding', [ + SendWelcomeEmailAction::class, + CreateUserProfileAction::class, + AssignDefaultRoleAction::class, +], ['user_id' => 123]); + +// Run a single action as a workflow +$instanceId = $simple->runAction(SendEmailAction::class, [ + 'to' => 'user@example.com', + 'subject' => 'Welcome!', +]); + +// Execute from a builder +$builder = WorkflowBuilder::create('custom-flow') + ->addStep('validate', ValidateAction::class) + ->addStep('process', ProcessAction::class); +$instanceId = $simple->executeBuilder($builder, $context); + +// Check status +$status = $simple->getStatus($instanceId); +// Returns: id, state, current_step, progress, completed_steps, failed_steps, error_message, ... ``` ## 🏗️ Architecture @@ -280,6 +325,37 @@ State transitions are validated at runtime — invalid transitions throw `Invali | `Exceptions\` | WorkflowException, InvalidWorkflowDefinitionException, InvalidWorkflowStateException, ActionNotFoundException, StepExecutionException, WorkflowInstanceNotFoundException | | `Support\` | NullLogger, NullEventDispatcher, SimpleWorkflow, Uuid, Timeout, ConditionEvaluator, Arr | +### Built-in Actions + +Six ready-to-use actions are included: + +| Action | Purpose | Config Keys | +|--------|---------|-------------| +| **LogAction** | Log messages with placeholder replacement (`{user.name}`) | `message`, `level` (debug/info/warning/error) | +| **EmailAction** | Mock email sending with template support | `to`, `subject`, `body`, `template` | +| **HttpAction** | HTTP requests with `{{ variable }}` template variables | `url`, `method`, `headers`, `body` | +| **DelayAction** | Pause execution for a specified duration | `seconds`, `minutes`, `hours` | +| **ConditionAction** | Evaluate boolean expressions and branch (`on_true`/`on_false`) | `condition`, `on_true`, `on_false` | +| **BaseAction** | Abstract base class for custom actions | — | + +### WorkflowState Helpers + +The `WorkflowState` enum provides utility methods for UI and logic: + +```php +$state = $instance->getState(); + +$state->isActive(); // true for PENDING, RUNNING, WAITING, PAUSED +$state->isFinished(); // true for COMPLETED, FAILED, CANCELLED +$state->isSuccessful(); // true for COMPLETED +$state->isError(); // true for FAILED +$state->label(); // "Running" +$state->description(); // "The workflow is actively executing steps..." +$state->color(); // "blue" (gray, blue, yellow, orange, green, red, purple) +$state->icon(); // "▶️" +$state->canTransitionTo(WorkflowState::COMPLETED); // bool +$state->getValidTransitions(); // [WorkflowState::WAITING, ...] +``` ## 🔧 Configuration @@ -436,7 +512,6 @@ This project is licensed under the MIT License - see the [LICENSE](LICENSE) file ## 🔗 Links -- [Documentation](https://github.com/solution-forest/workflow-engine-core/docs) -- [Issues](https://github.com/solution-forest/workflow-engine-core/issues) -- [Changelog](https://github.com/solution-forest/workflow-engine-core/blob/main/CHANGELOG.md) -- [Laravel Integration](https://github.com/solution-forest/workflow-engine-laravel) +- [Issues](https://github.com/solutionforest/workflow-engine-core/issues) +- [Changelog](https://github.com/solutionforest/workflow-engine-core/blob/main/CHANGELOG.md) +- [Laravel Integration](https://github.com/solutionforest/workflow-engine-laravel) diff --git a/src/Events/WorkflowCompletedEvent.php b/src/Events/WorkflowCompletedEvent.php index 42014d2..d19e325 100644 --- a/src/Events/WorkflowCompletedEvent.php +++ b/src/Events/WorkflowCompletedEvent.php @@ -4,12 +4,19 @@ use SolutionForest\WorkflowEngine\Core\WorkflowInstance; -class WorkflowCompletedEvent +final readonly class WorkflowCompletedEvent { - public WorkflowInstance $instance; + public function __construct( + public WorkflowInstance $instance, + ) {} - public function __construct(WorkflowInstance $instance) + public function getWorkflowId(): string { - $this->instance = $instance; + return $this->instance->getId(); + } + + public function getWorkflowName(): string + { + return $this->instance->getDefinition()->getName(); } } diff --git a/src/Events/WorkflowFailedEvent.php b/src/Events/WorkflowFailedEvent.php index 0598d69..1cdd44a 100644 --- a/src/Events/WorkflowFailedEvent.php +++ b/src/Events/WorkflowFailedEvent.php @@ -4,15 +4,25 @@ use SolutionForest\WorkflowEngine\Core\WorkflowInstance; -class WorkflowFailedEvent +final readonly class WorkflowFailedEvent { - public WorkflowInstance $instance; + public function __construct( + public WorkflowInstance $instance, + public \Exception $exception, + ) {} - public \Exception $exception; + public function getWorkflowId(): string + { + return $this->instance->getId(); + } + + public function getWorkflowName(): string + { + return $this->instance->getDefinition()->getName(); + } - public function __construct(WorkflowInstance $instance, \Exception $exception) + public function getErrorMessage(): string { - $this->instance = $instance; - $this->exception = $exception; + return $this->exception->getMessage(); } }