Skip to content

fix!: correct silent-success defects and release v2.0.0 - #58

Merged
lam0819 merged 2 commits into
mainfrom
fix/v0.1.0-release-blockers
Sep 12, 2026
Merged

fix!: correct silent-success defects and release v2.0.0#58
lam0819 merged 2 commits into
mainfrom
fix/v0.1.0-release-blockers

Conversation

@lam0819

@lam0819 lam0819 commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Fixes the five release blockers found in the pre-v0.1.0 review, plus the
should-fix items, and prepares the v0.1.0 release.

The common thread: several paths let the engine do the wrong thing and still
report success
— the failure mode a workflow engine can least afford, because
nothing in the test suite or a user's logs would catch it. Each was reproduced
against the real API before being fixed, and each now has a regression test.

Blockers

1. Compound conditions took the wrong branch, silently. ConditionEvaluator
had no &&/|| support, but rather than rejecting them it swallowed the
operator into the right-hand side and fell back to a string comparison:

// before: true, for a 500 dollar order  ("500" > "1000 && ..." compares "5" > "1")
ConditionEvaluator::evaluate('order.total > 1000 && order.vip === true', ['order' => ['total' => 500]]);

Now supports &&, ||, parentheses and negated groups, with && binding
tighter. A malformed expression throws instead of collapsing to a boolean.

2. resume() on a failed workflow was broken, and masked its own error.
The documented recovery path threw Cannot transition workflow from 'failed' to 'failed'. Three defects chained: the executor never lifted FAILED back to
RUNNING; the final FAILED -> COMPLETED hop was rejected; and that exception
was raised inside the failure handler, destroying the original cause.
FAILED -> RUNNING is now legal, resume retries the step that failed, and the
handler can never mask the real error.

3. Multi-root workflows dropped a branch and reported completed at 66.67%
progress — state and progress disagreeing, with no error. Every root now runs.

4. EmailAction sent nothing but reported 'status' => 'sent'. Renamed to
FakeEmailAction; the payload is now explicitly 'sent' => false, 'mock' => true
and it logs a warning.

5. CI never ran on pull requests (push: only), while dependabot-auto-merge
auto-merges minor/patch bumps with contents: write. Both workflows now run on
pull_request.

Also fixed

  • Relational comparisons against a missing key are false (null coerces to 0,
    so missing.key < 1000 was true).
  • Steps reachable only from each other are rejected at parse time.
  • start() throws instead of silently overwriting an existing instance.
  • Workflows blocked on unmet prerequisites park in WAITING instead of sitting
    in RUNNING indistinguishable from one still executing.
  • DelayAction honours the documented minutes/hours keys — they were never
    read, so delay(hours: 2) paused for the one second default.
  • HttpAction reports missing ext-curl as a step failure, not a fatal error.
  • Storage\InMemoryStorage ships with the package; the only adapter used to
    live in tests/, so the library could not run a workflow out of the box.
  • README action table corrected (it documented a body key neither action read),
    attribute examples no longer claim retries/timeouts the engine does not perform,
    and the condition grammar is documented.

Breaking changes

All pre-1.0 corrections; each has a migration note in CHANGELOG.md.

Change Migration
EmailActionFakeEmailAction Implement WorkflowAction with a real mail transport
WorkflowBuilder::email()fakeEmail() Rename the call
ConditionAction drops on_true/on_false Branch with a condition on the transition
ConditionAction uses the shared grammar Use ===/==/!= instead of =/is
FAILED is no longer terminal Update code asserting failed workflows are immutable

Verification

Full gate green locally on PHP 8.3 and 8.4 (both matrix versions):

  • 161 tests / 363 assertions pass (was 116/277)
  • Pint clean, PHPStan level 6 clean, composer validate --strict OK
  • Also removed a stale PHPStan ignoreErrors pattern that no longer matched

Two pre-existing tests asserted the old broken behaviour (FAILED cannot
transition to RUNNING) and were updated to encode the new contract.

🤖 Generated with Claude Code

lam0819 and others added 2 commits September 12, 2026 13:35
Several paths let the engine do the wrong thing and still report success,
which nothing in the suite or a user's logs would have caught.

Fixed:
- Compound conditions took the wrong branch. ConditionEvaluator had no
  && / || support but swallowed the operator into the right-hand side and
  string-compared, so 'order.total > 1000 && order.vip === true' was true
  for a 500 dollar order. Boolean operators, parentheses and negated
  groups are now supported; a malformed expression throws instead of
  collapsing to a boolean.
- resume() on a failed workflow threw "Cannot transition from 'failed' to
  'failed'" from inside the failure handler, destroying the original
  cause. FAILED -> RUNNING is now legal, resume retries the failed step,
  and the handler can no longer mask the real error.
- Multi-root workflows executed one root and still reported COMPLETED.
- Steps reachable only from each other are rejected at parse time.
- Relational comparisons against a missing key are false (null coerces
  to 0, so 'missing.key < 1000' used to be true).
- start() threw instead of overwriting an existing instance.
- Workflows blocked on unmet prerequisites park in WAITING.
- DelayAction honours the documented minutes/hours keys.
- HttpAction reports missing ext-curl as a step failure.

Breaking (pre-1.0, each with a migration note in CHANGELOG.md):
- EmailAction -> FakeEmailAction; it never sent mail but reported
  'status' => 'sent'. Payload is now 'sent' => false, 'mock' => true.
- WorkflowBuilder::email() -> fakeEmail().
- ConditionAction drops the non-functional on_true/on_false and uses the
  shared condition grammar instead of its own '=' / 'is' parser.
- FAILED is no longer terminal.

Added:
- Storage\InMemoryStorage now ships with the package; previously the only
  adapter lived in tests/ so the library could not run out of the box.
- CI runs on pull requests, not just pushes.
- SECURITY.md; corrected README action table and attribute examples.
- Regression tests for every issue above (116 -> 161 tests).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Now installable there since #57 widened the dev tooling constraints. The
library accepts php: ^8.3, which admits 8.5, so it should be tested there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lam0819
lam0819 force-pushed the fix/v0.1.0-release-blockers branch from 122c5f3 to ed03cd0 Compare September 12, 2026 05:35
@lam0819

lam0819 commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main after #57 landed, and renumbered the release to v2.0.0.

v1.0.0 was tagged and released while this branch was in progress. That makes
v0.1.0 unusable as a version number — it sorts below what is already
published, so Composer would never resolve it for anyone. And because v1.0.0
is a stable release, the corrections here (public class renames, FAILED no
longer terminal, the ConditionAction grammar) are breaking changes under
semver and need a major bump rather than a minor one.

Note that v1.0.0 shipped with every defect this PR fixes — it was tagged as
"no behavioural change from v0.0.4-alpha", which is accurate, but it means the
silent-success bugs are now in a release marked stable.

Also picked up from the rebase: PHP 8.5 added to the test matrix. #57 widened
the dev tooling so the suite can finally install there, and the library's
php: ^8.3 constraint admits 8.5, so it should be covered.

Verified on PHP 8.3, 8.4 and 8.5 with Pest 4 / PHPUnit 12: 161 tests / 363
assertions pass, Pint clean, PHPStan level 6 clean, composer validate --strict OK.

@lam0819 lam0819 changed the title fix: correct silent-success defects and prepare the v0.1.0 release fix!: correct silent-success defects and release v2.0.0 Sep 12, 2026
@lam0819
lam0819 merged commit 64d495f into main Sep 12, 2026
16 checks passed
@lam0819
lam0819 deleted the fix/v0.1.0-release-blockers branch September 12, 2026 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant