feat: implement Effect Pattern for core APIs (ADR-033) #348

2026-04-11T13:42:50-07:00

jwilger-ai-bot commented

2026-04-11 13:42:50 -07:00

(Migrated from github.com)

Summary

Implements the Effect Pattern described in ADR-033 (issue #299), refactoring execute() and run_projection() into pure state machines that yield effects, with thin shell loops dispatching effects to backend traits.

Caller-Driven Effects (User-Facing)

Added Effect and EffectResult associated types to CommandLogic
Added HandleDecision enum (Done/Effect) replacing Result<NewEvents, CommandError> as handle() return type
Added resume() method for continuing after effect fulfillment
execute() now returns Execution<C, S> with Success/Error/Effect variants
Effect-free commands use Effect = () — fully backward compatible

Internal State Machine Refactor

ExecutePipeline: Pure state machine yielding StoreEffect (ReadStream, AppendEvents, Sleep) with resume() accepting results
ProjectionPipeline: Pure state machine yielding ProjectionEffect (ReadEvents, LoadCheckpoint, SaveCheckpoint, Sleep)
Both execute() and ProjectionRunner::run() are now thin shell loops dispatching effects to backends
State machines are unit-testable without real stores (3 pipeline unit tests included)

Breaking Changes

CommandLogic::handle() returns HandleDecision<Self> instead of Result<NewEvents, CommandError>
CommandLogic requires type Effect and type EffectResult associated types
CommandLogic::State requires Clone bound
execute() returns Execution<C, S> instead of Result<ExecutionResponse, CommandError>
require! macro usage in handle() requires IIFE wrapper pattern

Closes #299

Test plan

All 212 existing tests pass unchanged
3 new caller-driven effects integration tests
3 new ExecutePipeline state machine unit tests
Clippy clean, fmt clean, doc tests pass
All pre-commit hooks pass

🤖 Generated with Claude Code

## Summary Implements the Effect Pattern described in ADR-033 (issue #299), refactoring `execute()` and `run_projection()` into pure state machines that yield effects, with thin shell loops dispatching effects to backend traits. ### Caller-Driven Effects (User-Facing) - Added `Effect` and `EffectResult` associated types to `CommandLogic` - Added `HandleDecision` enum (`Done`/`Effect`) replacing `Result<NewEvents, CommandError>` as `handle()` return type - Added `resume()` method for continuing after effect fulfillment - `execute()` now returns `Execution<C, S>` with `Success`/`Error`/`Effect` variants - Effect-free commands use `Effect = ()` — fully backward compatible ### Internal State Machine Refactor - **`ExecutePipeline`**: Pure state machine yielding `StoreEffect` (ReadStream, AppendEvents, Sleep) with `resume()` accepting results - **`ProjectionPipeline`**: Pure state machine yielding `ProjectionEffect` (ReadEvents, LoadCheckpoint, SaveCheckpoint, Sleep) - Both `execute()` and `ProjectionRunner::run()` are now thin shell loops dispatching effects to backends - State machines are unit-testable without real stores (3 pipeline unit tests included) ### Breaking Changes - `CommandLogic::handle()` returns `HandleDecision<Self>` instead of `Result<NewEvents, CommandError>` - `CommandLogic` requires `type Effect` and `type EffectResult` associated types - `CommandLogic::State` requires `Clone` bound - `execute()` returns `Execution<C, S>` instead of `Result<ExecutionResponse, CommandError>` - `require!` macro usage in `handle()` requires IIFE wrapper pattern Closes #299 ## Test plan - [x] All 212 existing tests pass unchanged - [x] 3 new caller-driven effects integration tests - [x] 3 new ExecutePipeline state machine unit tests - [x] Clippy clean, fmt clean, doc tests pass - [x] All pre-commit hooks pass 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jwilger (Migrated from github.com) reviewed

2026-04-11 15:15:24 -07:00

jwilger (Migrated from github.com) left a comment

Having a lot of second thoughts about this approach.

Walk me through the pros and cons.

Is this allowing us to do anything we either could not do or could not do as well versus either requiring all commands to execute only on their input values and current, event-derived state or passing in "services" via trait implementations as command properties for any command that needs to include execution of IO code while making decisions?

Having a lot of second thoughts about this approach. Walk me through the pros and cons. Is this allowing us to do anything we either could not do or could not do as well versus either requiring all commands to execute only on their input values and current, event-derived state or passing in "services" via trait implementations as command properties for any command that needs to include execution of IO code while making decisions?

eventcore-examples/tests/scenario_test.rs

					
				@ -117,17 +121,19 @@ impl CommandLogic for Withdraw {

				        }

I'm not sure I love the ergonomics or readability of this. It seems awkwardly nested when there are really three possibilities: done and here are events, done and here is an error, continue after giving me the results of these requested effects. This structure doesn't feel like it communicates that well.

eventcore-examples/tests/single_stream_command_test.rs

					
				@ -120,3 +122,4 @@

				        HandleDecision::Done(Ok(vec![BankAccountEvent::MoneyDeposited {

				            account_id: self.account_id.clone(),

				            amount: self.amount,

				        }]

Also not sure how I feel about having to set these up for every command when it is likely that most commands will not actually need this functionality. Starting to wonder if this is a good idea at all as opposed to just passing "services" into commands as input where the services just require a trait implementation. (That is, maybe we don't do this PR?)

Also not sure how I feel about having to set these up for every command when it is likely that most commands will not actually need this functionality. Starting to wonder if this is a good idea at all as opposed to just passing "services" into commands as input where the services just require a trait implementation. (That is, maybe we *don't* do this PR?)

jwilger-ai-bot (Migrated from github.com) reviewed

2026-04-11 15:19:57 -07:00

eventcore-examples/tests/scenario_test.rs

					
				@ -117,17 +121,19 @@ impl CommandLogic for Withdraw {

				        }

Agreed. The nesting here is real — HandleDecision::Done(Ok(vec![...].into())) buries the common case (emit events) under two layers of wrapping. Helper methods like HandleDecision::success(events) and HandleDecision::error(err) could flatten this, but it's papering over a design that made the simple path harder to express. The three-way decision (events / error / effect) is genuinely the semantic model, but forcing it through nested enums doesn't communicate it well — you're right that it reads as awkward nesting rather than clean branching.

Agreed. The nesting here is real — `HandleDecision::Done(Ok(vec![...].into()))` buries the common case (emit events) under two layers of wrapping. Helper methods like `HandleDecision::success(events)` and `HandleDecision::error(err)` could flatten this, but it's papering over a design that made the simple path harder to express. The three-way decision (events / error / effect) is genuinely the semantic model, but forcing it through nested enums doesn't communicate it well — you're right that it reads as awkward nesting rather than clean branching.

jwilger-ai-bot (Migrated from github.com) reviewed

2026-04-11 15:20:01 -07:00

eventcore-examples/tests/single_stream_command_test.rs

					
				@ -120,3 +122,4 @@

				        HandleDecision::Done(Ok(vec![BankAccountEvent::MoneyDeposited {

				            account_id: self.account_id.clone(),

				            amount: self.amount,

				        }]

This is a fair concern. Rust doesn't support associated type defaults on stable, so there's no way to make type Effect = (); type EffectResult = (); optional — every command pays this tax. The resume() method with a panicking default is also a code smell: it means the trait has a method that most implementations must never call, enforced by a runtime panic rather than the type system.

This boilerplate cost goes directly to the question you raised in the top-level review — see the full analysis there.

This is a fair concern. Rust doesn't support associated type defaults on stable, so there's no way to make `type Effect = (); type EffectResult = ();` optional — every command pays this tax. The `resume()` method with a panicking default is also a code smell: it means the trait has a method that most implementations must never call, enforced by a runtime panic rather than the type system. This boilerplate cost goes directly to the question you raised in the top-level review — see the full analysis there.

jwilger-ai-bot (Migrated from github.com) reviewed

2026-04-11 15:20:48 -07:00

jwilger-ai-bot (Migrated from github.com) left a comment

Pros/Cons Analysis: Effect Pattern vs. Alternatives

You asked whether this approach enables anything that couldn't be achieved with simpler alternatives. Here's an honest comparison.

What Changed (for reference)

Aspect	Before (main)	After (this PR)
`handle()` return	`Result<NewEvents, CommandError>`	`HandleDecision<Self>` (Done/Effect)
`execute()` return	`Result<ExecutionResponse, CommandError>`	`Execution<C, S>` (Success/Error/Effect)
Associated types	`Event`, `State`	`Event`, `State`, `Effect`, `EffectResult`
State bound	`Default + Send`	`Default + Clone + Send`
New methods	—	`resume()` with panicking default

Approach 1: Effect Pattern (This PR)

Pros:

handle() stays sync and pure — cannot do I/O, enforced by the type system
Effects are declared at the type level — the command's I/O needs are visible in its signature
Caller controls effect fulfillment — can mock, batch, timeout, or cancel effects
The internal state machine refactor (ExecutePipeline, ProjectionPipeline) is independently valuable

Cons:

Every command pays boilerplate tax: type Effect = (); type EffectResult = ();
Common path is deeply nested: HandleDecision::Done(Ok(vec![...].into()))
resume() default panics at runtime — the trait has a method most impls must never call
Execution enum forces three-way matching even when effects are impossible
State: Clone bound added for all commands
Unfamiliar pattern — most Rust developers won't recognize it

Approach 2: Async `handle()` with Service Injection

Commands that need I/O take service traits as struct fields. handle() becomes async so it can call injected services.

struct ValidateToken<V: TokenValidator> {
    stream_id: StreamId,
    token: String,
    validator: V,
}

impl<V: TokenValidator> CommandLogic for ValidateToken<V> {
    // No Effect/EffectResult types needed
    async fn handle(&self, _state: Self::State) -> Result<NewEvents<Self::Event>, CommandError> {
        let user_id = self.validator.validate(&self.token).await
            .map_err(|_| CommandError::business_rule_violated("invalid-token"))?;
        Ok(vec![TokenEvent::TokenValidated { ... }].into())
    }
}

Pros:

Familiar DI pattern — well-understood in Rust
No new types or concepts for effect-free commands (they're unchanged)
execute() keeps its clean Result<ExecutionResponse, CommandError> return
No boilerplate for effect-free commands
Testing via mock trait impls is well-established

Cons:

handle() is no longer guaranteed pure — convention only, not type-enforced
Making handle() async is itself a breaking change (though arguably smaller than this PR's changes)
Async fn in traits has some rough edges with dyn dispatch (though CommandLogic uses generics, not dyn)
Service traits need defining and mocking in tests

Approach 3: Pure Commands with External Orchestration

Commands stay completely pure. All I/O happens before execute(), with results passed as command input.

// Application code orchestrates:
let token_result = validator.validate(&token).await?;
let command = RecordValidation { stream_id, user_id: token_result.user_id };
let result = execute(&store, command, policy).await?;

Pros:

Simplest command model — no I/O framework at all
handle() stays sync, pure, trivially testable
No changes to existing APIs
Forces clean separation of I/O and domain logic

Cons:

Multi-step workflows risk TOCTOU: state may change between the external call and execute()
Application code becomes the orchestrator — error-prone, no framework guidance
Retry logic falls entirely on the caller (if external call succeeds but append fails, caller decides whether to re-do the external call)

Does the Effect Pattern Enable Anything the Others Can't?

No. All three approaches achieve the same functional outcomes. The differences are:

Where complexity lives: effect pattern puts it in the framework; service injection puts it in trait definitions; pure commands put it in application orchestration
What's enforced vs. conventional: the effect pattern enforces purity at the type level; service injection relies on convention ("use injected services, don't do raw I/O"); pure commands enforce purity by not providing any I/O mechanism
Ergonomic cost: the effect pattern has the highest cost for effect-free commands (boilerplate + nesting); service injection has zero cost for effect-free commands; pure commands have zero framework cost

The Internal Refactor

Worth noting: the ExecutePipeline and ProjectionPipeline state machine extractions are independently valuable regardless of which caller-facing approach we choose. They make the core logic unit-testable without real stores. If we abandon the caller-driven effects, we could keep the internal refactor in a separate PR.

My Assessment

The effect pattern's unique value is enforced purity of handle() — it's sync, so it literally cannot do I/O. This is a real property, but the cost is high: every command pays boilerplate tax for a feature most won't use, and the common path (emit events) becomes harder to express.

Service injection via async handle() would preserve the simple API for effect-free commands while giving effect-ful commands a familiar, well-understood mechanism. The trade-off is losing enforced purity, but in a library where the convention is well-documented and the audience is Rust developers who chose an event-sourcing framework, convention is probably sufficient.

If the conclusion is to not proceed with this PR, I'd suggest:

Close this PR
Open a separate PR for the internal state machine refactor (ExecutePipeline/ProjectionPipeline) — it's valuable on its own
Spike on async handle() + service injection if the I/O-during-commands use case is still a priority

## Pros/Cons Analysis: Effect Pattern vs. Alternatives You asked whether this approach enables anything that couldn't be achieved with simpler alternatives. Here's an honest comparison. ### What Changed (for reference) | Aspect | Before (main) | After (this PR) | |--------|--------------|-----------------| | `handle()` return | `Result<NewEvents, CommandError>` | `HandleDecision<Self>` (Done/Effect) | | `execute()` return | `Result<ExecutionResponse, CommandError>` | `Execution<C, S>` (Success/Error/Effect) | | Associated types | `Event`, `State` | `Event`, `State`, `Effect`, `EffectResult` | | State bound | `Default + Send` | `Default + Clone + Send` | | New methods | — | `resume()` with panicking default | ### Approach 1: Effect Pattern (This PR) **Pros:** - `handle()` stays sync and pure — cannot do I/O, enforced by the type system - Effects are declared at the type level — the command's I/O needs are visible in its signature - Caller controls effect fulfillment — can mock, batch, timeout, or cancel effects - The internal state machine refactor (ExecutePipeline, ProjectionPipeline) is independently valuable **Cons:** - Every command pays boilerplate tax: `type Effect = (); type EffectResult = ();` - Common path is deeply nested: `HandleDecision::Done(Ok(vec![...].into()))` - `resume()` default panics at runtime — the trait has a method most impls must never call - `Execution` enum forces three-way matching even when effects are impossible - `State: Clone` bound added for all commands - Unfamiliar pattern — most Rust developers won't recognize it ### Approach 2: Async `handle()` with Service Injection Commands that need I/O take service traits as struct fields. `handle()` becomes async so it can call injected services. ```rust struct ValidateToken<V: TokenValidator> { stream_id: StreamId, token: String, validator: V, } impl<V: TokenValidator> CommandLogic for ValidateToken<V> { // No Effect/EffectResult types needed async fn handle(&self, _state: Self::State) -> Result<NewEvents<Self::Event>, CommandError> { let user_id = self.validator.validate(&self.token).await .map_err(|_| CommandError::business_rule_violated("invalid-token"))?; Ok(vec![TokenEvent::TokenValidated { ... }].into()) } } ``` **Pros:** - Familiar DI pattern — well-understood in Rust - No new types or concepts for effect-free commands (they're unchanged) - `execute()` keeps its clean `Result<ExecutionResponse, CommandError>` return - No boilerplate for effect-free commands - Testing via mock trait impls is well-established **Cons:** - `handle()` is no longer guaranteed pure — convention only, not type-enforced - Making `handle()` async is itself a breaking change (though arguably smaller than this PR's changes) - Async fn in traits has some rough edges with `dyn` dispatch (though CommandLogic uses generics, not dyn) - Service traits need defining and mocking in tests ### Approach 3: Pure Commands with External Orchestration Commands stay completely pure. All I/O happens before `execute()`, with results passed as command input. ```rust // Application code orchestrates: let token_result = validator.validate(&token).await?; let command = RecordValidation { stream_id, user_id: token_result.user_id }; let result = execute(&store, command, policy).await?; ``` **Pros:** - Simplest command model — no I/O framework at all - `handle()` stays sync, pure, trivially testable - No changes to existing APIs - Forces clean separation of I/O and domain logic **Cons:** - Multi-step workflows risk TOCTOU: state may change between the external call and execute() - Application code becomes the orchestrator — error-prone, no framework guidance - Retry logic falls entirely on the caller (if external call succeeds but append fails, caller decides whether to re-do the external call) ### Does the Effect Pattern Enable Anything the Others Can't? **No.** All three approaches achieve the same functional outcomes. The differences are: - **Where complexity lives**: effect pattern puts it in the framework; service injection puts it in trait definitions; pure commands put it in application orchestration - **What's enforced vs. conventional**: the effect pattern enforces purity at the type level; service injection relies on convention ("use injected services, don't do raw I/O"); pure commands enforce purity by not providing any I/O mechanism - **Ergonomic cost**: the effect pattern has the highest cost for effect-free commands (boilerplate + nesting); service injection has zero cost for effect-free commands; pure commands have zero framework cost ### The Internal Refactor Worth noting: the `ExecutePipeline` and `ProjectionPipeline` state machine extractions are independently valuable regardless of which caller-facing approach we choose. They make the core logic unit-testable without real stores. If we abandon the caller-driven effects, we could keep the internal refactor in a separate PR. ### My Assessment The effect pattern's unique value is **enforced purity of `handle()`** — it's sync, so it literally cannot do I/O. This is a real property, but the cost is high: every command pays boilerplate tax for a feature most won't use, and the common path (emit events) becomes harder to express. Service injection via async `handle()` would preserve the simple API for effect-free commands while giving effect-ful commands a familiar, well-understood mechanism. The trade-off is losing enforced purity, but in a library where the convention is well-documented and the audience is Rust developers who chose an event-sourcing framework, convention is probably sufficient. If the conclusion is to not proceed with this PR, I'd suggest: 1. Close this PR 2. Open a separate PR for the internal state machine refactor (ExecutePipeline/ProjectionPipeline) — it's valuable on its own 3. Spike on async `handle()` + service injection if the I/O-during-commands use case is still a priority

jwilger (Migrated from github.com) reviewed

2026-04-11 15:26:22 -07:00

eventcore-examples/tests/single_stream_command_test.rs

					
				@ -120,3 +122,4 @@

				        HandleDecision::Done(Ok(vec![BankAccountEvent::MoneyDeposited {

				            account_id: self.account_id.clone(),

				            amount: self.amount,

				        }]

I wonder if we could have a separate trait that gets implemented for commands that need to use effects. That might give us a means to represent this type of command without requiring it on every command.

jwilger-ai-bot commented

2026-04-11 15:35:05 -07:00

(Migrated from github.com)

Closing this PR. The caller-driven effects pattern adds significant API complexity (boilerplate for every command, nested return types, panicking resume() default) for a benefit (enforced purity) that doesn't justify the cost.

Decision: Commands receive all non-system-state inputs pre-calculated. Anything requiring time-of-execution consistency must be represented in internal state (events). TOCTOU concerns with external systems are inherent regardless of approach — no framework-level mechanism can solve them.

The internal state machine refactor (ExecutePipeline, ProjectionPipeline) is independently valuable and will be submitted as a separate PR.

Closing this PR. The caller-driven effects pattern adds significant API complexity (boilerplate for every command, nested return types, panicking `resume()` default) for a benefit (enforced purity) that doesn't justify the cost. **Decision:** Commands receive all non-system-state inputs pre-calculated. Anything requiring time-of-execution consistency must be represented in internal state (events). TOCTOU concerns with external systems are inherent regardless of approach — no framework-level mechanism can solve them. The internal state machine refactor (ExecutePipeline, ProjectionPipeline) is independently valuable and will be submitted as a separate PR.