feat(orchestration): add event-sourcing core #28

Merged
claude-desktop merged 4 commits from boss/4 into main 2026-04-17 07:05:52 +00:00
Collaborator

Summary

Implements the pure Command → Decider → Events → Projector → ReadModel pipeline described in spec §5 and §7.1.

Added

  • crates/backend/src/orchestration/commands.rsOrchestrationCommand enum covering projects, threads, turns, provider ingestion, checkpoints, and git.
  • crates/backend/src/orchestration/events.rsOrchestrationEvent enum, StoredEvent wrapper (sequence + stored_at), and ProviderEvent (TextDelta, ToolCall, ToolResult, ThinkingDelta, TurnComplete, Error, AwaitingApproval).
  • crates/backend/src/orchestration/decider.rs — pure decide(command, model) -> Result<Vec<Event>, DeciderError>, with a descriptive error enum. All validations listed in the issue are covered (duplicate project paths, missing ids, busy/idle thread invariants, turn existence, approval state, checkpoint existence).
  • crates/backend/src/orchestration/projector.rs — pure project(model, event) -> model fold, including cascading deletes (project → threads → turns → checkpoints) and streaming chunk accumulation for Text/ThinkingBlock message content.

Tests (56 pass)

  • Decider: at least one positive + one negative case per command variant.
  • Projector: unit tests per event variant covering state changes, cascades, chunk accumulation, and checkpoint linking.
  • Integration: decide → project round-trip asserting read-model consistency across CreateProject → CreateThread → StartTurn.

Closes #4

Test plan

  • cargo fmt --all -- --check
  • cargo clippy --workspace -- -D warnings
  • cargo test --workspace
## Summary Implements the pure `Command → Decider → Events → Projector → ReadModel` pipeline described in spec §5 and §7.1. ### Added - `crates/backend/src/orchestration/commands.rs` — `OrchestrationCommand` enum covering projects, threads, turns, provider ingestion, checkpoints, and git. - `crates/backend/src/orchestration/events.rs` — `OrchestrationEvent` enum, `StoredEvent` wrapper (`sequence` + `stored_at`), and `ProviderEvent` (`TextDelta`, `ToolCall`, `ToolResult`, `ThinkingDelta`, `TurnComplete`, `Error`, `AwaitingApproval`). - `crates/backend/src/orchestration/decider.rs` — pure `decide(command, model) -> Result<Vec<Event>, DeciderError>`, with a descriptive error enum. All validations listed in the issue are covered (duplicate project paths, missing ids, busy/idle thread invariants, turn existence, approval state, checkpoint existence). - `crates/backend/src/orchestration/projector.rs` — pure `project(model, event) -> model` fold, including cascading deletes (project → threads → turns → checkpoints) and streaming chunk accumulation for `Text`/`ThinkingBlock` message content. ### Tests (56 pass) - Decider: at least one positive + one negative case per command variant. - Projector: unit tests per event variant covering state changes, cascades, chunk accumulation, and checkpoint linking. - Integration: `decide → project` round-trip asserting read-model consistency across `CreateProject → CreateThread → StartTurn`. Closes #4 ## Test plan - [x] `cargo fmt --all -- --check` - [x] `cargo clippy --workspace -- -D warnings` - [x] `cargo test --workspace`
feat(orchestration): add event-sourcing core
Some checks are pending
qa / qa (pull_request) Waiting to run
a8ae896d5c
Implements the pure Command -> Decider -> Events -> Projector -> ReadModel
pipeline described in spec section 5:

- OrchestrationCommand enum covering projects, threads, turns, provider
  ingestion, checkpoints, and git
- OrchestrationEvent enum plus StoredEvent wrapper and ProviderEvent
  (TextDelta, ToolCall, ToolResult, ThinkingDelta, TurnComplete, Error,
  AwaitingApproval)
- decide() — pure validation with a descriptive DeciderError
- project() — pure fold rebuilding OrchestrationReadModel, including
  cascading deletes and streaming chunk accumulation

Unit tests cover at least one positive and one negative case per
command variant plus a decide -> project integration round-trip.

Closes #4
Ghost requested changes 2026-04-16 21:58:01 +00:00
Dismissed
Ghost left a comment

Overall: solid implementation of the pure event-sourcing core. All acceptance criteria from #4 are met — commands, events, decider, projector, and tests are complete. Two issues need fixing before merge.

Two request-changes items below. One is a correctness bug (tool call ID corruption), the other is a design issue that will silently break replay (empty git_ref baked into immutable events).

Overall: solid implementation of the pure event-sourcing core. All acceptance criteria from #4 are met — commands, events, decider, projector, and tests are complete. Two issues need fixing before merge. **Two request-changes items below.** One is a correctness bug (tool call ID corruption), the other is a design issue that will silently break replay (empty `git_ref` baked into immutable events).
@ -0,0 +182,4 @@
thread_id,
turn_id,
description,
} => {

Design issue: git_ref: String::new() is baked into an immutable event.

Events sourced from CheckpointCreated (here) and ChangesCommitted (~line 205) are stored in the event log with git_ref: "". Because stored events are immutable, replaying the log will always reconstruct Checkpoint and commit records with an empty git ref — the reactor can never patch this retroactively through normal projection.

The pattern that works in event sourcing: the reactor performs the git operation, learns the actual git_ref, and then emits the enriched event. Options:

  1. Don't emit CheckpointCreated / ChangesCommitted from the decider at all. Have the CheckpointReactor and git reactor emit them (with a real git_ref) after the side-effect completes. The decider can still validate the command and emit a lightweight intent event (e.g. CheckpointRequested) if you need an audit entry before the side-effect.
  2. Make git_ref: Option<String> in both event types, emit None from the decider, and add follow-up events (CheckpointGitRefAssigned { id, git_ref }, CommitRefAssigned { ... }) that the reactor emits after the fact.

As written, the event store will contain permanently-empty git_ref strings that make the stored log useless for point-in-time restore.

Same issue applies to ChangesCommitted at ~line 205.

**Design issue: `git_ref: String::new()` is baked into an immutable event.** Events sourced from `CheckpointCreated` (here) and `ChangesCommitted` (~line 205) are stored in the event log with `git_ref: ""`. Because stored events are immutable, replaying the log will always reconstruct `Checkpoint` and commit records with an empty git ref — the reactor can never patch this retroactively through normal projection. The pattern that works in event sourcing: the reactor performs the git operation, learns the actual `git_ref`, and *then* emits the enriched event. Options: 1. **Don't emit `CheckpointCreated` / `ChangesCommitted` from the decider at all.** Have the `CheckpointReactor` and git reactor emit them (with a real `git_ref`) after the side-effect completes. The decider can still validate the command and emit a lightweight intent event (e.g. `CheckpointRequested`) if you need an audit entry before the side-effect. 2. **Make `git_ref: Option<String>`** in both event types, emit `None` from the decider, and add follow-up events (`CheckpointGitRefAssigned { id, git_ref }`, `CommitRefAssigned { ... }`) that the reactor emits after the fact. As written, the event store will contain permanently-empty `git_ref` strings that make the stored log useless for point-in-time restore. Same issue applies to `ChangesCommitted` at ~line 205.
@ -0,0 +339,4 @@
turn_id: turn.id,
message_id: last.id,
chunk: content.clone(),
}]);

Bug: tool call ID is fused into name, corrupting both fields.

name: format!("{name}#{id}"),

This smuggles the provider's tool_call_id into the name field, so the read model stores "bash#tc_123" instead of "bash". Downstream code (UI rendering, ToolResult matching) has no way to extract the original name or ID without fragile string splitting.

The root cause is that MessageContent::ToolCall in contracts has no id field. Fix: add id: String to MessageContent::ToolCall (contracts crate), then use it here:

content: MessageContent::ToolCall {
    id: id.clone(),
    name: name.clone(),
    input: input.clone(),
},

This PR touches pure decider/projector logic; the contracts change can land in this same PR or as a prerequisite commit.

**Bug: tool call ID is fused into `name`, corrupting both fields.** ```rust name: format!("{name}#{id}"), ``` This smuggles the provider's `tool_call_id` into the `name` field, so the read model stores `"bash#tc_123"` instead of `"bash"`. Downstream code (UI rendering, ToolResult matching) has no way to extract the original name or ID without fragile string splitting. The root cause is that `MessageContent::ToolCall` in contracts has no `id` field. Fix: add `id: String` to `MessageContent::ToolCall` (contracts crate), then use it here: ```rust content: MessageContent::ToolCall { id: id.clone(), name: name.clone(), input: input.clone(), }, ``` This PR touches pure decider/projector logic; the contracts change can land in this same PR or as a prerequisite commit.
Ghost requested changes 2026-04-16 21:58:06 +00:00
Dismissed
Ghost left a comment

Overall: solid implementation of the pure event-sourcing core. All acceptance criteria from #4 are met — commands, events, decider, projector, and tests are complete. Two issues need fixing before merge.

Two request-changes items below. One is a correctness bug (tool call ID corruption), the other is a design issue that will silently break replay (empty git_ref baked into immutable events).

Overall: solid implementation of the pure event-sourcing core. All acceptance criteria from #4 are met — commands, events, decider, projector, and tests are complete. Two issues need fixing before merge. **Two request-changes items below.** One is a correctness bug (tool call ID corruption), the other is a design issue that will silently break replay (empty `git_ref` baked into immutable events).
@ -0,0 +182,4 @@
thread_id,
turn_id,
description,
} => {

Design issue: git_ref: String::new() is baked into an immutable event.

Events sourced from CheckpointCreated (here) and ChangesCommitted (~line 205) are stored in the event log with git_ref: "". Because stored events are immutable, replaying the log will always reconstruct Checkpoint and commit records with an empty git ref — the reactor can never patch this retroactively through normal projection.

The pattern that works in event sourcing: the reactor performs the git operation, learns the actual git_ref, and then emits the enriched event. Options:

  1. Don't emit CheckpointCreated / ChangesCommitted from the decider at all. Have the CheckpointReactor and git reactor emit them (with a real git_ref) after the side-effect completes. The decider can still validate the command and emit a lightweight intent event (e.g. CheckpointRequested) if you need an audit entry before the side-effect.
  2. Make git_ref: Option<String> in both event types, emit None from the decider, and add follow-up events (CheckpointGitRefAssigned { id, git_ref }, CommitRefAssigned { ... }) that the reactor emits after the fact.

As written, the event store will contain permanently-empty git_ref strings that make the stored log useless for point-in-time restore.

Same issue applies to ChangesCommitted at ~line 205.

**Design issue: `git_ref: String::new()` is baked into an immutable event.** Events sourced from `CheckpointCreated` (here) and `ChangesCommitted` (~line 205) are stored in the event log with `git_ref: ""`. Because stored events are immutable, replaying the log will always reconstruct `Checkpoint` and commit records with an empty git ref — the reactor can never patch this retroactively through normal projection. The pattern that works in event sourcing: the reactor performs the git operation, learns the actual `git_ref`, and *then* emits the enriched event. Options: 1. **Don't emit `CheckpointCreated` / `ChangesCommitted` from the decider at all.** Have the `CheckpointReactor` and git reactor emit them (with a real `git_ref`) after the side-effect completes. The decider can still validate the command and emit a lightweight intent event (e.g. `CheckpointRequested`) if you need an audit entry before the side-effect. 2. **Make `git_ref: Option<String>`** in both event types, emit `None` from the decider, and add follow-up events (`CheckpointGitRefAssigned { id, git_ref }`, `CommitRefAssigned { ... }`) that the reactor emits after the fact. As written, the event store will contain permanently-empty `git_ref` strings that make the stored log useless for point-in-time restore. Same issue applies to `ChangesCommitted` at ~line 205.
@ -0,0 +339,4 @@
turn_id: turn.id,
message_id: last.id,
chunk: content.clone(),
}]);

Bug: tool call ID is fused into name, corrupting both fields.

name: format!("{name}#{id}"),

This smuggles the provider's tool_call_id into the name field, so the read model stores "bash#tc_123" instead of "bash". Downstream code (UI rendering, ToolResult matching) has no way to extract the original name or ID without fragile string splitting.

The root cause is that MessageContent::ToolCall in contracts has no id field. Fix: add id: String to MessageContent::ToolCall (contracts crate), then use it here:

content: MessageContent::ToolCall {
    id: id.clone(),
    name: name.clone(),
    input: input.clone(),
},

This PR touches pure decider/projector logic; the contracts change can land in this same PR or as a prerequisite commit.

**Bug: tool call ID is fused into `name`, corrupting both fields.** ```rust name: format!("{name}#{id}"), ``` This smuggles the provider's `tool_call_id` into the `name` field, so the read model stores `"bash#tc_123"` instead of `"bash"`. Downstream code (UI rendering, ToolResult matching) has no way to extract the original name or ID without fragile string splitting. The root cause is that `MessageContent::ToolCall` in contracts has no `id` field. Fix: add `id: String` to `MessageContent::ToolCall` (contracts crate), then use it here: ```rust content: MessageContent::ToolCall { id: id.clone(), name: name.clone(), input: input.clone(), }, ``` This PR touches pure decider/projector logic; the contracts change can land in this same PR or as a prerequisite commit.
charles dismissed Ghost's review 2026-04-16 22:00:43 +00:00
- Add `id: String` to `MessageContent::ToolCall` so the tool call ID
  is stored separately instead of being fused into the name field
- Change `git_ref` from `String` to `Option<String>` in CheckpointCreated,
  ChangesCommitted events and the Checkpoint struct — decider emits None,
  reactor will enrich with the real ref after the side-effect completes

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
test(orchestration): add test for ToolCall id/name separation
Some checks are pending
qa / qa (pull_request) Waiting to run
a4987c3d12
Verifies that IngestProviderEvent::ToolCall preserves the tool call ID
as a separate field rather than concatenating it into the name.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix(ci): add libsqlite3-dev to system dependencies
All checks were successful
qa / qa (pull_request) Successful in 10m17s
0e90d636bc
Required by libsqlite3-sys for sqlx compilation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign in to join this conversation.
No description provided.