Repository navigation
feat: add the C ABI lifecycle - #26
Conversation
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
WalkthroughChangesThe workspace now includes the FFI crate. The engine exposes public persistence APIs and migration, while the new FFI layer provides C-compatible store lifecycle functions, CBOR validation, error handling, buffer cleanup, concurrency coordination, and tests. Engine API and FFI lifecycle
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
13bb559 to
5d46691
Compare
0f4c623 to
b5885b0
Compare
b5885b0 to
a9cca28
Compare
5d46691 to
7e3232b
Compare
a9cca28 to
b93c3a5
Compare
7e3232b to
277ac60
Compare
b93c3a5 to
5697838
Compare
277ac60 to
350262e
Compare
5697838 to
9cba22b
Compare
350262e to
3e8cffd
Compare
9cba22b to
f3dba53
Compare
2087976 to
4f46410
Compare
43b1c3b to
14d2931
Compare
4f46410 to
626e657
Compare
108a313 to
d642d59
Compare
626e657 to
cbfbf10
Compare
d642d59 to
b3875b8
Compare
cbfbf10 to
02cee38
Compare
b3875b8 to
7aa769e
Compare
02cee38 to
611f664
Compare
8983b16 to
ed8d725
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
cb1a992 to
4180148
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/event-sorcery-ffi/build.rs`:
- Around line 10-12: Update the build script’s header generation flow around
generate().write_to_file so the generated event_sorcery.h is also published to a
stable checked-in or install/release location outside OUT_DIR, while preserving
the existing OUT_DIR output for Rust builds.
- Around line 4-11: Update the build script’s main entry point to return a
suitable Result type, replace the expect calls for CARGO_MANIFEST_DIR, OUT_DIR,
and cbindgen generation with ? propagation, and return the generated result as
required while preserving the existing builder configuration.
In `@crates/event-sorcery-ffi/Cargo.toml`:
- Around line 13-19: Move ciborium, event-sorcery, and cbindgen into the
workspace dependency table using cargo add, then update the event-sorcery-ffi
manifest to reference each with workspace = true, preserving their existing
versions, path, and build-dependency scope.
In `@crates/event-sorcery-ffi/src/lib.rs`:
- Around line 441-468: Update the decoded tuple in the options parser to
represent runtime_threads with a fixed-width wire integer instead of usize, then
reject values above a documented maximum (while preserving the existing nonzero
validation). Convert the validated value to usize using a checked conversion
before constructing OpenOptions, and add boundary tests covering the maximum
accepted value and the first rejected value.
- Line 1: The new Rust modules lack required module-level documentation. Add //!
documentation to crates/event-sorcery-ffi/src/lib.rs describing the unsafe ABI,
lifecycle, and ownership contract, and add //! documentation to
crates/event-sorcery-ffi/build.rs describing header generation and publication.
- Around line 109-114: Update es_close and linearize_close to pass the raw owner
pointer instead of creating an &mut reference via store.as_mut(). Within
linearize_close, while holding the registry mutex, use raw-pointer read and
write operations to access and clear the owner cell, preserving the existing
close linearization behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 23afe0ea-4ead-40fa-960b-70fd7d867876
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
Cargo.tomlcrates/event-sorcery-ffi/Cargo.tomlcrates/event-sorcery-ffi/build.rscrates/event-sorcery-ffi/src/lib.rscrates/event-sorcery/src/engine.rscrates/event-sorcery/src/job_sqlite.rscrates/event-sorcery/src/lib.rscrates/event-sorcery/src/sqlite_event_repository.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Before work, read SPEC.md and docs/domain.md; read relevant supplemental documentation before implementation.
New features must be documented in SPEC.md before implementation and must follow the hierarchy SPEC.md -> issue -> plan -> tests -> implementation.
Fix all known problems immediately, complete all tasks, and do not allow warnings or errors to pass through.
Keep a granular task list and clear completed tasks from the active list.
All new or modified logic must have corresponding test coverage.
Understand relevant documentation and source code before implementation, keep diffs small, and review the approach critically.
When changing direction or making an important undocumented architectural decision, obtain confirmation; record significant decisions as ADRs under adrs/.
Before handover, review the diff, revert unjustified changes, and check for scope creep.
Each aggregate in a consuming application must use exactly one SqliteCqrs instance constructed at startup; per-request construction is forbidden.
Never read secret or credential files such as .env*, credentials.json, *.key, *.pem, *.p12, *.pfx, or sensitive database files without explicit permission.
Never bypass, disable, suppress, or obscure quality-control mechanisms without explicit permission; fix lint and test issues at their root.
Use cargo check, cargo nextest, and cargo clippy for verification; never use cargo build unless build artifacts are required.
Files:
crates/event-sorcery-ffi/Cargo.tomlcrates/event-sorcery/src/lib.rsCargo.tomlcrates/event-sorcery-ffi/build.rscrates/event-sorcery/src/job_sqlite.rscrates/event-sorcery/src/sqlite_event_repository.rscrates/event-sorcery-ffi/src/lib.rscrates/event-sorcery/src/engine.rs
**/Cargo.toml
📄 CodeRabbit inference engine (AGENTS.md)
Never manually edit Cargo.toml to add dependencies; use cargo add, with workspace dependencies declared centrally and referenced by workspace=true.
Files:
crates/event-sorcery-ffi/Cargo.tomlCargo.toml
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: Organize code by business feature rather than technical layer; avoid catch-all modules such as types.rs, error.rs, models.rs, utils.rs, helpers.rs, and services.rs.
Never write directly to the events table; emit events through CqrsFramework::execute() or execute_with_metadata().
Use cqrs-es Services for side effects in handle() and follow the {Action}er -> {Domain}Service -> {Domain}Manager naming pattern.
Place command-execution logging in aggregate handle() methods rather than callers.
Model invalid states with enums, ADTs, newtypes, and typestate rather than relying on runtime validation.
Use domain newtypes at APIs and convert to SDK primitives inside the callee, except at cross-crate boundaries where conversion at the call site is necessary.
Keep visibility as restrictive as possible: private over pub(crate) over pub.
Use a three-group import order: external crates, workspace crates, then crate-internal imports; do not use function-level imports except enum variants.
Do not use unwrap() or expect() in production Rust code; they are permitted in #[cfg(test)] code.
Never create error variants containing opaque String values; prefer #[from], ?, #[source], and preserve error chains.
Log a warning or error before silent early returns such as let-else failures.
Never silently mask numeric failures with caps, fallback defaults, precision truncation, unwrap_or(), or unwrap_or_default(); use explicit checked conversions and errors.
Prefer functional patterns, pattern matching, combinators, type-driven design, and iterators over imperative loops unless complexity increases.
Use ASCII in identifiers, comments, log messages, and configuration keys; Unicode is preferred only in user-facing rendered output.
Do not use single-letter variables, arguments, closure parameters, or generic type parameters except an unambiguous lone type parameter or short unambiguous closure.
Every module must have a //! docstring and should order public API, private implementati...
Files:
crates/event-sorcery/src/lib.rscrates/event-sorcery-ffi/build.rscrates/event-sorcery/src/job_sqlite.rscrates/event-sorcery/src/sqlite_event_repository.rscrates/event-sorcery-ffi/src/lib.rscrates/event-sorcery/src/engine.rs
*
⚙️ CodeRabbit configuration file
Focus on providing constructive criticism. Whenever you see a suboptimal approach, suggest more idiomatic or robust alternative(s). Flag potential footguns. Suggest FP alternatives to mutable/imperative code. Point out architectural flaws like leaky abstractions, tight coupling, wrong level of abstraction, poor type modeling, over-abstraction, unclear domain boundaries. Code should generally be organized based on business concerns rather than technical aspects - suggest improvements if you find violations. Point out gaps in test coverage but suggest tests that are not too coupled to the implementation and actually test domain invariants and business logic
Files:
Cargo.toml
🔇 Additional comments (7)
crates/event-sorcery/src/engine.rs (1)
17-58: LGTM!Also applies to: 67-74, 94-116, 129-146, 167-167, 195-195, 223-223, 281-281, 385-385, 705-727, 800-808, 820-821
crates/event-sorcery/src/lib.rs (1)
128-128: LGTM!crates/event-sorcery/src/job_sqlite.rs (1)
77-79: LGTM!crates/event-sorcery/src/sqlite_event_repository.rs (1)
108-126: LGTM!Cargo.toml (1)
2-6: LGTM!crates/event-sorcery-ffi/Cargo.toml (1)
1-10: LGTM!Also applies to: 21-31
crates/event-sorcery-ffi/src/lib.rs (1)
2-108: LGTM!Also applies to: 115-440, 469-798
|
@coderabbitai review |
✅ Action performedReview finished.
|
Add the event-sorcery-ffi static library with a cbindgen-generated C header, versioned deterministic-CBOR open options and errors, an opaque store owning the captive Tokio runtime and extracted engine, migration through the canonical sqlite-es MIGRATOR, explicit buffer ownership, and panic barriers.
The crate links the existing engine facade; it does not contain storage or job decisions. Workspace tests and clippy with warnings denied are green.
Closes #62.
This is part 8 of 32 in a stack made with GitButler:
Summary by CodeRabbit
New Features
Bug Fixes