jwilger/eventcore
1
0
Fork 0
Code Issues 9 Pull requests 2 Projects Releases 62 Activity Actions

feat(sqlite): make encryption opt-in, re-export rusqlite, add BYOC constructors #379

Merged
jwilger-ai-bot merged 2 commits from feat/sqlite-dep-flexibility-378 into main 2026-04-27 10:08:19 -07:00
Conversation 9 Commits 2 Files changed 7 +191 -5
jwilger-ai-bot commented 2026-04-27 07:47:59 -07:00 (Migrated from github.com)
Copy link

Summary

Closes #378.

eventcore-sqlite previously forced two undesirable constraints on consumers:

  1. The encryption feature was on by default, meaning every consumer pulled in rusqlite/bundled-sqlcipher-vendored-openssl whether they needed encryption or not — a heavy native-link surface (vendored SQLCipher + vendored OpenSSL) that can clash at link time with other crates that touch sqlite/openssl.
  2. There was no rusqlite re-export and no way to construct a store from an externally configured connection, so consumers who needed to do anything beyond the basic config had to declare their own rusqlite dependency — creating version-mismatch pain (see the motivating downstream incident in #378).

This PR addresses all four acceptance criteria from the issue:

  • encryption is opt-in. The default feature set is now bundled, which vendors vanilla SQLite without OpenSSL/SQLCipher. Consumers who actually need at-rest encryption opt in via features = ["encryption"]. Consumers who want to bring their own SQLite (system or custom) disable defaults.
  • pub use rusqlite; at the crate root. Consumers write use eventcore_sqlite::rusqlite::Connection and never need to declare their own rusqlite dependency for the common case.
  • SqliteEventStore::from_connection and SqliteCheckpointStore::from_connection for bring-your-own-connection setups (custom pragmas, attached DBs, encryption keys, pooling). The connection is taken as-is.
  • README documents the feature matrix, version-compatibility contract, and BYOC pattern. CHANGELOG records the breaking change.

The Rust type system enforces version compatibility for free — if a consumer's rusqlite::Connection doesn't unify with the version eventcore-sqlite was built against, they get a clear compile-time error at the from_connection call site rather than a silent runtime mismatch.

Also picked up cargo update -p rustls-webpki (0.103.12 → 0.103.13) to clear RUSTSEC-2026-0104, which was blocking pre-commit.

Behavior changes

  • Breaking (default features): encryption is no longer a default feature. Consumers who relied on the default encryption feature must now add features = ["encryption"] explicitly. The replacement default bundled keeps out-of-the-box ergonomics intact — existing code that doesn't use SQLCipher continues to work without changes.
  • Additive: new from_connection constructors, new bundled feature, new rusqlite re-export. None of these change existing callers.

Test plan

  • New integration test bring_your_own_connection_test exercises both from_connection constructors via the re-exported eventcore_sqlite::rusqlite, drives a checkpoint round-trip end-to-end, and was written before implementation (TDD red → green).
  • cargo nextest run --workspace --all-features — 197 / 197 pass, including the existing encrypted_migration_test (still gated on #[cfg(feature = "encryption")]).
  • cargo build --package eventcore-sqlite --no-default-features — compiles clean, demonstrating consumers can opt out of vendored SQLite to BYO sqlite.
  • cargo build --package eventcore-sqlite --all-features — bundled and encryption coexist (rusqlite resolves the linkage to SQLCipher when both are active, matching upstream behavior).
  • cargo clippy --all-targets --all-features -- -D warnings clean.
  • cargo audit clean.
  • Blueprint blueprints/store-backends.md updated to reflect the new feature surface and BYOC constructor.
## Summary Closes #378. `eventcore-sqlite` previously forced two undesirable constraints on consumers: 1. The `encryption` feature was on by default, meaning every consumer pulled in `rusqlite/bundled-sqlcipher-vendored-openssl` whether they needed encryption or not — a heavy native-link surface (vendored SQLCipher + vendored OpenSSL) that can clash at link time with other crates that touch sqlite/openssl. 2. There was no `rusqlite` re-export and no way to construct a store from an externally configured connection, so consumers who needed to do anything beyond the basic config had to declare their own `rusqlite` dependency — creating version-mismatch pain (see the motivating downstream incident in #378). This PR addresses all four acceptance criteria from the issue: - **`encryption` is opt-in.** The default feature set is now `bundled`, which vendors vanilla SQLite without OpenSSL/SQLCipher. Consumers who actually need at-rest encryption opt in via `features = ["encryption"]`. Consumers who want to bring their own SQLite (system or custom) disable defaults. - **`pub use rusqlite;`** at the crate root. Consumers write `use eventcore_sqlite::rusqlite::Connection` and never need to declare their own `rusqlite` dependency for the common case. - **`SqliteEventStore::from_connection`** and **`SqliteCheckpointStore::from_connection`** for bring-your-own-connection setups (custom pragmas, attached DBs, encryption keys, pooling). The connection is taken as-is. - **README** documents the feature matrix, version-compatibility contract, and BYOC pattern. CHANGELOG records the breaking change. The Rust type system enforces version compatibility for free — if a consumer's `rusqlite::Connection` doesn't unify with the version `eventcore-sqlite` was built against, they get a clear compile-time error at the `from_connection` call site rather than a silent runtime mismatch. Also picked up `cargo update -p rustls-webpki` (0.103.12 → 0.103.13) to clear RUSTSEC-2026-0104, which was blocking pre-commit. ## Behavior changes - **Breaking (default features):** `encryption` is no longer a default feature. Consumers who relied on the default `encryption` feature must now add `features = ["encryption"]` explicitly. The replacement default `bundled` keeps out-of-the-box ergonomics intact — existing code that doesn't use SQLCipher continues to work without changes. - **Additive:** new `from_connection` constructors, new `bundled` feature, new `rusqlite` re-export. None of these change existing callers. ## Test plan - [x] New integration test `bring_your_own_connection_test` exercises both `from_connection` constructors via the re-exported `eventcore_sqlite::rusqlite`, drives a checkpoint round-trip end-to-end, and was written before implementation (TDD red → green). - [x] `cargo nextest run --workspace --all-features` — 197 / 197 pass, including the existing `encrypted_migration_test` (still gated on `#[cfg(feature = "encryption")]`). - [x] `cargo build --package eventcore-sqlite --no-default-features` — compiles clean, demonstrating consumers can opt out of vendored SQLite to BYO sqlite. - [x] `cargo build --package eventcore-sqlite --all-features` — `bundled` and `encryption` coexist (rusqlite resolves the linkage to SQLCipher when both are active, matching upstream behavior). - [x] `cargo clippy --all-targets --all-features -- -D warnings` clean. - [x] `cargo audit` clean. - [x] Blueprint `blueprints/store-backends.md` updated to reflect the new feature surface and BYOC constructor.
coderabbitai[bot] commented 2026-04-27 07:48:23 -07:00 (Migrated from github.com)
Copy link

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8d8c0f4e-5961-4f6e-98fd-ccb52edf0b63

📥 Commits

Reviewing files that changed from the base of the PR and between 5e8ef03a36 and 245f78dacd.

📒 Files selected for processing (3)
  • eventcore-sqlite/Cargo.toml
  • eventcore-sqlite/README.md
  • eventcore-sqlite/tests/bring_your_own_connection_test.rs
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: Test (nightly, 17)
  • GitHub Check: Test (beta, 15)
  • GitHub Check: Test (nightly, 18)
  • GitHub Check: Test (stable, 18)
  • GitHub Check: Test (beta, 16)
  • GitHub Check: Test (beta, 18)
  • GitHub Check: Test (nightly, 16)
  • GitHub Check: Test (stable, 15)
  • GitHub Check: Test (nightly, 15)
  • GitHub Check: Test (stable, 17)
  • GitHub Check: Test (beta, 17)
  • GitHub Check: Test (stable, 16)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings 
Learnt from: jwilger-ai-bot
Repo: jwilger/eventcore PR: 379
File: eventcore-sqlite/Cargo.toml:13-23
Timestamp: 2026-04-27T15:03:10.675Z
Learning: In `eventcore-sqlite` (Rust crate), the `bundled` and `encryption` Cargo features are NOT mutually exclusive. When both are active, `libsqlite3-sys` links SQLCipher (the `bundled-sqlcipher-vendored-openssl` feature takes precedence over the plain `bundled`/vanilla SQLite feature). Adding a `compile_error!` guard for `cfg(all(feature = "bundled", feature = "encryption"))` would be wrong and would break `--all-features` CI. The correct approach is to document that when both are active, you get SQLCipher.
📚 Learning: 2026-04-27T15:03:10.675Z 
Learnt from: jwilger-ai-bot
Repo: jwilger/eventcore PR: 379
File: eventcore-sqlite/Cargo.toml:13-23
Timestamp: 2026-04-27T15:03:10.675Z
Learning: In `eventcore-sqlite` (Rust crate), the `bundled` and `encryption` Cargo features are NOT mutually exclusive. When both are active, `libsqlite3-sys` links SQLCipher (the `bundled-sqlcipher-vendored-openssl` feature takes precedence over the plain `bundled`/vanilla SQLite feature). Adding a `compile_error!` guard for `cfg(all(feature = "bundled", feature = "encryption"))` would be wrong and would break `--all-features` CI. The correct approach is to document that when both are active, you get SQLCipher.

Applied to files:

  • eventcore-sqlite/README.md
  • eventcore-sqlite/Cargo.toml
🔇 Additional comments (6)
eventcore-sqlite/Cargo.toml (1)

13-23: Feature flag defaults and interaction docs are correctly implemented.

The default/opt-in split is coherent, and the combined bundled + encryption behavior is now accurately documented.

Based on learnings: bundled and encryption are not mutually exclusive, and documenting SQLCipher precedence is the correct behavior.

eventcore-sqlite/tests/bring_your_own_connection_test.rs (2)

13-34: Event-store BYOC test now validates real behavior, not just construction.

Running migrate() twice is a good idempotence/liveness assertion for the consumer-supplied connection path.

36-58: Checkpoint-store BYOC test provides solid end-to-end coverage.

The migrate + save/load round-trip confirms externally supplied connections work for checkpoint persistence flows.

eventcore-sqlite/README.md (3)

8-28: Feature matrix documentation is clear and technically correct.

The guidance around default bundled, opt-in encryption, and disabling defaults is consistent with the crate behavior.

Based on learnings: documenting that both bundled and encryption can coexist (with SQLCipher linkage) is the correct approach.

42-50: rusqlite compatibility wording is accurate and actionable.

This correctly explains Cargo resolution vs. downstream type mismatch behavior and gives a practical recommendation to use the re-export.

52-70: BYOC constructor docs set the right expectations.

The section clearly communicates as-is connection handoff and when to choose from_connection vs new.

📝 Walkthrough

Walkthrough

The PR makes encryption opt-in (changes default features), re-exports rusqlite, adds from_connection constructors for event and checkpoint stores, updates docs/CHANGELOG to document version-compatibility and the feature change, and adds tests exercising caller-provided rusqlite::Connection.

Changes

Cohort / File(s) Summary
Feature Configuration
eventcore-sqlite/Cargo.toml		 Default features changed to include bundled instead of encryption; new bundled = ["rusqlite/bundled"] feature added; encryption documented as opt-in.
Core API
eventcore-sqlite/src/lib.rs		 Added pub use rusqlite; re-export and SqliteEventStore::from_connection / SqliteCheckpointStore::from_connection constructors that accept a caller-provided rusqlite::Connection.
Documentation & Migration
eventcore-sqlite/CHANGELOG.md, eventcore-sqlite/README.md, blueprints/store-backends.md		 Changelog migration note added; README documents rusqlite version guidance, re-export recommendation, encryption opt-in, and BYOC constructor usage; backend docs updated to reflect feature and WAL behavior.
Integration Tests
eventcore-sqlite/tests/bring_your_own_connection_test.rs		 New tests construct an in-memory rusqlite::Connection via the crate re-export and pass it to from_connection for both stores; tests exercise migrate(), save(), and load() flows.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant eventcore_sqlite
    participant rusqlite
    participant SQLite

    Client->>rusqlite: create Connection (e.g., in-memory, set pragmas/encryption)
    Client->>eventcore_sqlite: SqliteEventStore::from_connection(conn)
    eventcore_sqlite->>rusqlite: store holds Arc<Mutex<Connection>>
    Client->>eventcore_sqlite: call migrate()/save()/load()
    eventcore_sqlite->>rusqlite: execute SQL via Connection
    rusqlite->>SQLite: native SQLite calls (queries/PRAGMA)
    SQLite-->>rusqlite: results
    rusqlite-->>eventcore_sqlite: query results
    eventcore_sqlite-->>Client: return result

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Assessment against linked issues

Objective Addressed Explanation
Make encryption non-default feature [#378] ✅
Re-export rusqlite at crate root [#378] ✅
Add "bring-your-own-connection" constructor [#378] ✅
Document version-compatibility contract and encryption opt-in [#378] ✅
Migration note in changelog for default-feature change [#378] ✅

Comment @coderabbitai help to get the list of available commands and usage tips.

<!-- This is an auto-generated comment: summarize by coderabbit.ai --> No actionable comments were generated in the recent review. 🎉 <details> <summary>ℹ️ Recent review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: ASSERTIVE **Plan**: Pro Plus **Run ID**: `8d8c0f4e-5961-4f6e-98fd-ccb52edf0b63` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 5e8ef03a36cd5174178d83854dd126bf74a4c614 and 245f78dacd90ba465012e7960ede8efaf0b889b6. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `eventcore-sqlite/Cargo.toml` * `eventcore-sqlite/README.md` * `eventcore-sqlite/tests/bring_your_own_connection_test.rs` </details> </details> <details> <summary>📜 Recent review details</summary> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)</summary> * GitHub Check: Test (nightly, 17) * GitHub Check: Test (beta, 15) * GitHub Check: Test (nightly, 18) * GitHub Check: Test (stable, 18) * GitHub Check: Test (beta, 16) * GitHub Check: Test (beta, 18) * GitHub Check: Test (nightly, 16) * GitHub Check: Test (stable, 15) * GitHub Check: Test (nightly, 15) * GitHub Check: Test (stable, 17) * GitHub Check: Test (beta, 17) * GitHub Check: Test (stable, 16) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (2)</summary> <details> <summary>📓 Common learnings</summary> ``` Learnt from: jwilger-ai-bot Repo: jwilger/eventcore PR: 379 File: eventcore-sqlite/Cargo.toml:13-23 Timestamp: 2026-04-27T15:03:10.675Z Learning: In `eventcore-sqlite` (Rust crate), the `bundled` and `encryption` Cargo features are NOT mutually exclusive. When both are active, `libsqlite3-sys` links SQLCipher (the `bundled-sqlcipher-vendored-openssl` feature takes precedence over the plain `bundled`/vanilla SQLite feature). Adding a `compile_error!` guard for `cfg(all(feature = "bundled", feature = "encryption"))` would be wrong and would break `--all-features` CI. The correct approach is to document that when both are active, you get SQLCipher. ``` </details> <details> <summary>📚 Learning: 2026-04-27T15:03:10.675Z</summary> ``` Learnt from: jwilger-ai-bot Repo: jwilger/eventcore PR: 379 File: eventcore-sqlite/Cargo.toml:13-23 Timestamp: 2026-04-27T15:03:10.675Z Learning: In `eventcore-sqlite` (Rust crate), the `bundled` and `encryption` Cargo features are NOT mutually exclusive. When both are active, `libsqlite3-sys` links SQLCipher (the `bundled-sqlcipher-vendored-openssl` feature takes precedence over the plain `bundled`/vanilla SQLite feature). Adding a `compile_error!` guard for `cfg(all(feature = "bundled", feature = "encryption"))` would be wrong and would break `--all-features` CI. The correct approach is to document that when both are active, you get SQLCipher. ``` **Applied to files:** - `eventcore-sqlite/README.md` - `eventcore-sqlite/Cargo.toml` </details> </details> </details> <details> <summary>🔇 Additional comments (6)</summary><blockquote> <details> <summary>eventcore-sqlite/Cargo.toml (1)</summary><blockquote> `13-23`: **Feature flag defaults and interaction docs are correctly implemented.** The default/opt-in split is coherent, and the combined `bundled` + `encryption` behavior is now accurately documented. Based on learnings: `bundled` and `encryption` are not mutually exclusive, and documenting SQLCipher precedence is the correct behavior. </blockquote></details> <details> <summary>eventcore-sqlite/tests/bring_your_own_connection_test.rs (2)</summary><blockquote> `13-34`: **Event-store BYOC test now validates real behavior, not just construction.** Running `migrate()` twice is a good idempotence/liveness assertion for the consumer-supplied connection path. --- `36-58`: **Checkpoint-store BYOC test provides solid end-to-end coverage.** The migrate + save/load round-trip confirms externally supplied connections work for checkpoint persistence flows. </blockquote></details> <details> <summary>eventcore-sqlite/README.md (3)</summary><blockquote> `8-28`: **Feature matrix documentation is clear and technically correct.** The guidance around default `bundled`, opt-in `encryption`, and disabling defaults is consistent with the crate behavior. Based on learnings: documenting that both `bundled` and `encryption` can coexist (with SQLCipher linkage) is the correct approach. --- `42-50`: **`rusqlite` compatibility wording is accurate and actionable.** This correctly explains Cargo resolution vs. downstream type mismatch behavior and gives a practical recommendation to use the re-export. --- `52-70`: **BYOC constructor docs set the right expectations.** The section clearly communicates as-is connection handoff and when to choose `from_connection` vs `new`. </blockquote></details> </blockquote></details> </details> --- <!-- walkthrough_start --> <details> <summary>📝 Walkthrough</summary> ## Walkthrough The PR makes `encryption` opt-in (changes default features), re-exports `rusqlite`, adds `from_connection` constructors for event and checkpoint stores, updates docs/CHANGELOG to document version-compatibility and the feature change, and adds tests exercising caller-provided `rusqlite::Connection`. ## Changes |Cohort / File(s)|Summary| |---|---| |**Feature Configuration** <br> `eventcore-sqlite/Cargo.toml`|Default features changed to include `bundled` instead of `encryption`; new `bundled = ["rusqlite/bundled"]` feature added; `encryption` documented as opt-in.| |**Core API** <br> `eventcore-sqlite/src/lib.rs`|Added `pub use rusqlite;` re-export and `SqliteEventStore::from_connection` / `SqliteCheckpointStore::from_connection` constructors that accept a caller-provided `rusqlite::Connection`.| |**Documentation & Migration** <br> `eventcore-sqlite/CHANGELOG.md`, `eventcore-sqlite/README.md`, `blueprints/store-backends.md`|Changelog migration note added; README documents rusqlite version guidance, re-export recommendation, encryption opt-in, and BYOC constructor usage; backend docs updated to reflect feature and WAL behavior.| |**Integration Tests** <br> `eventcore-sqlite/tests/bring_your_own_connection_test.rs`|New tests construct an in-memory `rusqlite::Connection` via the crate re-export and pass it to `from_connection` for both stores; tests exercise migrate(), save(), and load() flows.| ## Sequence Diagram(s) ```mermaid sequenceDiagram participant Client participant eventcore_sqlite participant rusqlite participant SQLite Client->>rusqlite: create Connection (e.g., in-memory, set pragmas/encryption) Client->>eventcore_sqlite: SqliteEventStore::from_connection(conn) eventcore_sqlite->>rusqlite: store holds Arc<Mutex<Connection>> Client->>eventcore_sqlite: call migrate()/save()/load() eventcore_sqlite->>rusqlite: execute SQL via Connection rusqlite->>SQLite: native SQLite calls (queries/PRAGMA) SQLite-->>rusqlite: results rusqlite-->>eventcore_sqlite: query results eventcore_sqlite-->>Client: return result ``` ## Estimated code review effort 🎯 3 (Moderate) | ⏱️ ~25 minutes ## Assessment against linked issues |Objective|Addressed|Explanation| |---|---|---| |Make `encryption` non-default feature [`#378`]|✅|| |Re-export `rusqlite` at crate root [`#378`]|✅|| |Add "bring-your-own-connection" constructor [`#378`]|✅|| |Document version-compatibility contract and encryption opt-in [`#378`]|✅|| |Migration note in changelog for default-feature change [`#378`]|✅|| </details> <!-- walkthrough_end --> <!-- tips_start --> --- <sub>Comment `@coderabbitai help` to get the list of available commands and usage tips.</sub> <!-- tips_end --> <!-- internal state start --> <!-- DwQgtGAEAqAWCWBnSTIEMB26CuAXA9mAOYCmGJATmriQCaQDG+Ats2bgFyQAOFk+AIwBWJBrngA3EsgEBPRvlqU0AgfFwA6NPEgQAfACgjoCEYDEZyAAUASpETZWaCrKPR1AGxJcAZiWoAFIgAjh7qJACUXMxoANYkkGQMLtzi+Fj4qWDwGAA0kBQkYCQAHtz4FLgF2CFhNPlotPQAQgCaAPIAwgoYiLgU2GIVyAG2kGYAzADsAJwRbgjIYwwe+IjS49MAHOgY9AywmKTIJFIYuEyFYLXhkAQFdIMJ8Mzl6/RK3GRKGAzyTL1+tpzsh0j0HGwKIgNJAANIkf6HDDHDhGKAAMX8uGwhUQXAABkkUmkMPiUMgMPhIKtkZR0JAlD40NgPFU/NQcSQYXAEozmazIOzsYV7CQqqhKQB3SD4gTYPZeWhkgJnWgVOiQCSYeAeDxoSAAZQAigAZcIRGGddIQyjISWwKnkDXUMC4qpE2SpeBg5g1KqZcVYCTwfX4oWc5AAXkgAG0AEQer3pOMAXXxlutjltjEwDKQKi8DJITJZbKxEbuVIEFByRDusBI8D4+ElWGNZpoGgMUBsRVK5UqBO42AEkBqCQGNxoAG4yahGkp6NR6wlktQJ/h8FVEFSATaoTmsIU/IVfglCWcLuqAPpT7wcSehcIcDhWjDkMTe0mQSXqB14IsVmcWtqjvFdmHWDwpGhbtIDaLpwX6QYCChLgAEEmg1fEDSfGgAFFLwNFD7x8CgWGvAEPxJMlMHobDcJIToGwYWJyhyXAiPVF9SPIyjRGo+tlzQBgGBIVJ6T3LMKDAXh8GDRcZUfOp7zfKiv2VHwKkYP0WB4KgiBiRAGlwXBhIbegABFmiMxJfmJL9IHiWQbPKfAwmRC0YAbHo1LBVBTPiLA0EQbIYKgCz8AYLNzmoByADJGCRUhViILgbDwtCLIAWTwsduFodd6HuNUorYc4V0FcsRRifp4BKfIpChL8wCYV5YrUOp/nSIExAaPYKvg7puGoGgKAwS0AAk0IAOQAcTwk12lmnpTJyZB9WYeAiCoElIEpGhBS03BvL5UswHDEVq38WIQIOI4uVg6BpFwZAEs6ABJLhppIaV2JIbbYrBGg+hlata2vWR8Bxa8WwwCj0l8uHgdwMlJWC9BMKXfqBiwXhvT4e4Xm4LwytMkkYTQxB1kqL88UgABGGYpgAekZqY7me5Bhqpn8/3QXVzqq6Rp0gOUdVoZAHBEkgNV/Y6ZQgSkwFO1lBY5XEaP6/EIDQAWLukfERZWeBuG4eRaJzCgiEIZlaHURLRFidaRRWfxxrgjxsDEmtypK6KyYc7B8sKrsoCypAGC4OVXg1SdcA8ELJRIARuBuyAAgABg0emM4mbOACZIEAJMJICznO8/piYIkrDHaFxZAbAAVQNaADTwzowHzjP84ANjADOc4AFi7AxmhIQ5gy0u7aTpq64hAzS+BVst1Y2AJE12ylqXSUgl+LflcAiEWtwbChf3WWv1EkBIAnIaUrn7CoqgtwbEIGIYoQtcxLHaYR+Ov5APFmDUhyPEegSAHDSCMOiMiwCIFe02FMLY+RjoJEECIT80F0CFFRLBLKcRzwb3UntdIyt96lnsLuTMkJkBqhIVUYcupNTfHVPQdsnQTan2Zu0L4GADQGhNKLeQy905oAkPgeAdtkR7VilIMA7lYj2FkMwAQbkeg+DCGIRAX8ex9jKE/RSNQGJkiAYkS8lwihgR3AoVgYJJI0MPGOC+F52AWNvAxF8Slwioz/FDKoSggI+zrF4g6qCIIkCgtIfIhRaCDBAo1RAzVNqIBqgcQUNZPzpFDpADCS5RZBLAJDHEYBYYtQRv/Ox1okIf0Es/aWqR1rgiktcIOxN4BYRCSpcpmTvwBC0q5Dw1d7gOFNgYxoWozz0HWNibgXAop9F0rwNABlgrGVMgcDUVkbJELBE5GyFsBm1nyPaHUCReDSEoMGaRqCehnHaWeN+yEKjZIiqVdgFUEnNVasNcQHV1BdXOFQMQKAsDpUyjlLgfR1w/gQGkzpzCmpA28muA6qAxYCmWcCPo0TRAsDKvQGoIEbkP30ZUPq9B+x6hyBVIESgSk+B8KCHw/AvTSPYlSQkdlPTUWyVYY8dIvgUGSYkqQCLElgioDPQ6fB4Xrw0EQDQ+Q4xZwmPnOMQyqS61WNKTozhrZjgwPAHw8AGCA3GrBNC9TTIPOSOEGsaACQ7O/PtIsJZWTRKMcpB4xRSU0FoPkV+e5qkoXQFqHUBYSD5DBdlXKQcCp+vJRJJKET8B1k2gDTeW5ni/E9ouEeFhIBWlYPbNgVNlkbAhDEFwRh2jkEOrqFsYAg42M2lUQJRr2l2gqFIusFtkai3HmI/GuDdGSm7RqXVVt8AaAIMwDwzNo05WpEcbA5b0CqIAnKBUdAADUTqFClCQDQB5FsHTSkpBgEpjU9SmxAvCyVxx7A4iZKJdOuBPQJEoGRPgy5TVMMSQdCQ0IHg7k9rtJkOpOQ6MgL2YmwlnQkJKdwO4H70BU0oLtalNzX5/QzQ5ftctYC7BQEoV4Wbyrpp2quLVNdSiUAYEgBINz7GUBaabMIGo+I9NsrQMABBih7BHvoYw4AoDfH4My5k/HSDkCo/sPF7AuC8H4H/TBGw5AKCUFQVQ6gtA6GEyYKAcBUDziCngQgMnlB+pbYpgoaBpSVucPIDTTAtMqDUJobQugwCGBE6YAwAhPbe3YogZmCyrgCGEoFCWGhmC0FRHGRLBgC1oXesQMgVmNSOZcBJxK91EAGB5IaU0txIssXE37UmZrACYBMgQAOATwnkPrRAgBcAlFNxtGyA42FRrseLwwL9bnT1L2/qaErDvXsF8BjxqGB03YZwuk+7gyhjQOx2Q14nXXicgECIc4KQtgZJFf2zpQRZCw7AMi2AiBEdzJy5I3LiH6xFjc9stwyBEByM8Wh0hbUCBO8wvYrChGuoPpqEM9JZTyloIqExQteby1zAGfDVI7aIAjSD0sgCtJg2RIUqG0lYbFY7FyLyvJIrrQTpqpotTDF3j296x+lRnRVGY1Rgom5cD5HYmRWJr6wywPhu+Cp34g3vxQljvg9FlIEXYJxQoms6I4WUkxR2bFzhy5IGSYZrSDGlFGhgXWPlhcgowSSRN7afDyGOsuAA6mhQRQh8cG/cmmxQX30DsfaUVXcVGwDorbd0kkMEjDJcsGhVkVnaY1xuQEvUO0o/4GZYz6z/SRyaNsuIcQUCoDTXaNNPCRgzTkGQNPUg8XIA7p7szTuRg8J9BeD11zE5TjtOlMWRenBIAmhbAYRLcY0T+dOK49U1wGLM06FNOaC0lqxfi73pLKW0uWbk0+pwOXE95ZngsVcyaUq1ZlI3DAhQvDBToGSdY3HUD9lorLPm+o74ymaIUee0imL5bJOwHLULxDXMOFUeb3AXC+6EoVINIu8GOqsZAEaWM9AUoDwwQ2ATYGwlKJq9sAY2QQY4ONyd29k6QsOq8xyfMMe5CAo+soobaSURKHKW60OZ+MI70z8lOtcjSD++ImIq8iAH+gK8g+0RKf+tOxi5IDOvqGopirO0KZEW4uwRU/BuAY6e0P0MoQCguiMZIouTyB4ScIo3OigTw9Ai8osJ8MoSu4QMu6uxECuxhDEKuLEauHEFhNcwkok4kf6Xg0ksk8kHSnqz4r4ge6kI838OSEe8e1o0eJ0ogceZqTKiQIh9AqegWJqGeV8UCsE006QN8lW7AZqJSGAHgiI90IKMoQ+5wFio+yk4+k+80i0s0s++IX8BgReGwpedAXAO69MAArNXhnLXvXjVJxm7g8MGIoe3k/FwN3pKPPv3t2IPuYiPneOPnqtOrOh4AlgvmHkvhlivtlvIBvs0QVkViiiQPvpOvqmQcvM7AkN1tZtYpDtukqEIVAYFhqBpiImtDQI0LljgQ9ngRTAodKLcbQfcWQfOJjDTovGjBQBLDXPiJ0szDQTDg0A2r+L/quGzoTDmtgEoPSBMjqHqETrcN0GENWE5vDv+FUMfrICBGCPqIgM5DQMAviESXeBMOmJAO9LkZ9odm8jFOBkdN5IevXtIl8UmN+MCScGUDft7kWIgL9gkGekThwoAXSAoiBH9ECi9KSaDFDjDunDkCsJiSBPKaovLH9iBMJOIKKmgAyvxMgIyfAAIMydcM5HUTCqco5DLNwHwYQlyiKfgcKEcd9rwj8H8JADELetIvKM0bQAEaHkEaNFEWEbyBEc4AmRvsnhqPEenuwMkQVhiHDiIlGYUS4iUXMWPicUsSwB4PiKiJALoDKCItGPGE6qmGSCXPiA2bGHGPCXQC2akYocOAkQwIwGzmQQuC0QYLWVAACYqJAI2XGLCd2bQL2VAHhCUEegvHDhbl7unLHimbtJGcmrQFEBOXWcKbtHOQudqXQGUQxkqdJKqKwkhmQFTB4C2enK1KTF1sHH6vMA0Z9iXoea0TnNXvTD0eIH0fJliYUEMW3gyqMZAFlHQPAI4JMQPmAEYMUVeFcPMYunhLPqsf3ovulrJj1tsblnsQYJ0M/sDPSA/rhdKkUbMdhYIUoDKTWGoCicOdCujPqK9gdGVtFgxTcmYbgFaCKJhdcPjgxtIkSVQDlv7vwFgDCd4TQKyQwVyf7MgOWZVBwenNOWfhAT/nWPcA+TEpqNqLqLxSVoBuDspfMYufiImkjukIbmecQstgqQtnwIRgDmqGZTwmQPwiaBEFzhiT2pAPKcVPmE8YZTpf6cgPcGIhIvQIwi7oUTiZZb5UDnxQkDuDiK+vaGQIpXkbZPdiKUIaxUgdGaTjBhlDGpNqIB2hsNblUMWVhZYoIaZFbGKI0sNDTFFHHgIcpGSJtJSHwJ8mCAEFFBQKePHPIPiCqvnBoCUC6X2rIcimziSgOBqXZYIcuOoMgJISzlSA2B4MhixgeElZIiCt8u1IWLtcNWKlHudkmUGUkPIADIASLPbJSlijYp6V4Hxi8ExqhvAs1fwaakFDWBfIVUGLaA5A+hsGqBgNVv6NeqtvQQwmRJ4V1mjqQJAEQIgQVCev1E3vYIaqbD1Qxc9AWEgAgNIvMrOkbtxtMs2sdJdtdlYdLoRMRNxALlxgJAEGtQkKkvTXWOoTUgYVLuEDYaxBIuYeqHUfkN/nwcuDch4ZIv0ULpfrjSdqFNITTsxtQnSKgLiOUL0PaYWAYUsisqFvuk5OoltDiGaoQQjvQkxlSOOFzaYTzVxBwHfCYlpOcTGSlsEQmfcMQYEqmUnrEfwHwAOVmecDmakXnrlAEPRZkTyQ5OkHkb+Y0QBfdOXjulMBnF0WBQ3tZmTdBa3okHBYOF3j3n3mhRhUxR1eUYgBQAwMzESRoKhKhURcvqRY4FWjscyhRaudtbaZ0mSAnSaiVaYm1aUfMR3V3T3VCI5bZLTdImqK2H0M/sAmTfcPyqeHzp0i+KpMLlrhdlDJzcdKgIcRTE0AlfIQ/rPUORLeLoACgE0t+EvtOCHAyhAtxCFsP9jEzEct7EGufNvEfheBn9LV6AVqxGusz+tAsgT55AdEZ9vh2tAkFs4Wzw/iSBYgJVAQkoVAlN4CSlaEndwAWUeApQwAGgzDegegStQi3MiSnFApRlIK+uhuQDpCmQZAtYzMAIxqhNIRuM1AsAwegR4e8ZQeiZgEkRSjaZsdmZiR2ZWeuZOSoJb9gokZJIQ4I4gobYDEIlUDAD/NsDGAU1CMXA2DF9PS1cPmhoESPg+IFq+jaeiRPgRjX4Jjo4Pg5jyu4DdhVjgDtj9j74jjKlXSuDX4rjeg7jHgnj3jCkBjW18F+IA5TiE48Ts46cD+6ZUyyiqigygR+ChqfgIM6IbpaEzusgAAXpQIXv+ZvmXq0RMKXf3OXRBZps3jBbXR3lwIhXbChU3dMehQYJhUvWPsjKFjjkQBDPjjDK2CocLteMjL3XiP3escRZllMsPSSbscmgVrkutFgLhlIxzCDMwLoYWAg8kFiBsLmDkGAGwA8zlg9T4c4wJOzTfURtgXMzeHeJ4vE2SNk5UAbZw+Db+rY3wxyiYb/bLrzdYzA4k3gQbaA7LRE+i1E1i6SNyPyZePYMRHc+SfKLaZRuuDtlrr+K+kirlWgGwOS+qCClCiTUuOhpUDIEYbS0o1LKJHQA1JQEapSdIt8HxoQOJmEGcNIFc+AqRuUMegVXfjmLqKxrDFrYjCSzvqrvLduBS/2jjDS1tFRvSygg2FgK4baWjlIPSwxTSSOGxSbLtA7T5fqIPdZthP0P4MwFYGsFfHgfkJpEic8fNasI0E6wYTcmjmy05A0Ly7wzctG5k8G7tKLeDWgjWB9s7iVQ6xqFqEFiHWHmHUoxHeEVHWozHdtRmfHb40Odo52qkekcU4of2saoWDnbIPUfnZ0y0RXu0VsGXQYHXuBY3gMdXcMXXZ3uM8hcwKhdMb5oZjxrllJhZpsVO6wLZlQA5ic/JV1G5jpp5vpj5gYGux+eoNeJIogNeDO0nLQLeF1VUMJpe6JpAO0SQFsMWLnGgBMD3AwLQO0fTFMIPGB1sLQFsBMFsO0YPE0PTL3AID4OB2gIPAwD3PTIPN5oYGu/nIPO0ah1B8JLQDMBnJFoPD3O0QPPnCQLMD3BnHQD+/vD4BR1sFsDMAID3Lh35lANe7gLexLA+y3j9HQBtv1AZp+5u9eMNOOC++uApzC++wAN5xh6h9BWDMjvBoS4C9gwV0BFqtpWjyi4BxgcAZwAC+fHOABAsn2nJACnNAEn9A+gQAA== --> <!-- internal state end -->
coderabbitai[bot] (Migrated from github.com) requested changes 2026-04-27 07:55:24 -07:00
coderabbitai[bot] (Migrated from github.com) left a comment
Copy link

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents 
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@eventcore-sqlite/Cargo.toml`:
- Around line 13-22: Add a compile-time guard in eventcore-sqlite's root lib
(e.g., at the top of src/lib.rs) that detects the illegal feature combination
and emits a compile_error! when both "bundled" and "encryption" are enabled;
implement this using a cfg check such as cfg(all(feature = "bundled", feature =
"encryption")) and a compile_error! with a clear message that selecting both
features is unsupported and instructing the user to choose one. Ensure the check
runs unconditionally at crate compile time (place it at crate root before other
code) so the invalid combination fails early.

In `@eventcore-sqlite/README.md`:
- Around line 41-44: The README incorrectly claims Cargo "emits a clear
compile-time error" for non-overlapping version ranges; update the wording
around the mention of Cargo and rusqlite to say that Cargo can permit multiple
SemVer-incompatible versions (e.g., rusqlite 1.x and 2.x) to coexist in the
dependency graph, and that compile-time errors occur only when your code mixes
types or APIs from different versions; retain the recommendation to prefer the
re-export over a direct rusqlite dependency but clarify that version conflicts
surface as type/API mismatches rather than Cargo failing resolution.

In `@eventcore-sqlite/tests/bring_your_own_connection_test.rs`:
- Around line 28-32: Remove the no-op type assertion `let _: &SqliteEventStore =
&store;` from the test; it provides no behavioral coverage. Edit the test in
bring_your_own_connection_test.rs to delete that line (or replace it with a real
assertion that calls a method on `store` or verifies a property), referencing
the `SqliteEventStore` type and the `store` variable so the test demonstrably
exercises the constructed store rather than only asserting its type.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3573afab-998d-479d-9f07-a7b44c59397e

📥 Commits

Reviewing files that changed from the base of the PR and between 807bb8a327 and 5e8ef03a36.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • blueprints/store-backends.md
  • eventcore-sqlite/CHANGELOG.md
  • eventcore-sqlite/Cargo.toml
  • eventcore-sqlite/README.md
  • eventcore-sqlite/src/lib.rs
  • eventcore-sqlite/tests/bring_your_own_connection_test.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
  • GitHub Check: Test (stable, 18)
  • GitHub Check: Test (beta, 18)
  • GitHub Check: Test (beta, 15)
  • GitHub Check: Test (stable, 17)
  • GitHub Check: Test (nightly, 16)
  • GitHub Check: Test (nightly, 17)
  • GitHub Check: Test (nightly, 15)
  • GitHub Check: Test (nightly, 18)
  • GitHub Check: Test (stable, 16)
  • GitHub Check: Test (stable, 15)
  • GitHub Check: Test (beta, 17)
  • GitHub Check: Test (beta, 16)
  • GitHub Check: Clippy
🔇 Additional comments (5)
eventcore-sqlite/CHANGELOG.md (1)

10-24: Release notes accurately capture the behavioral change and new APIs.

The breaking/default-feature update and BYOC/re-export additions are clearly documented.

eventcore-sqlite/tests/bring_your_own_connection_test.rs (1)

34-56: Good BYOC checkpoint-path coverage.

This test validates externally provided connection + migration + persistence roundtrip (save/load) end-to-end.

blueprints/store-backends.md (1)

50-55: SQLite backend blueprint update is aligned with implementation.

The feature/connection model described here matches the crate’s current behavior.

eventcore-sqlite/src/lib.rs (2)

10-10: Crate-root rusqlite re-export is correctly exposed.

This provides the intended consumer path (eventcore_sqlite::rusqlite) for version alignment.

247-260: BYOC constructors are clear and correctly scoped.

Both constructors consistently take a caller-managed rusqlite::Connection as-is and preserve existing internal synchronization patterns.

Also applies to: 699-711

**Actionable comments posted: 3** <details> <summary>🤖 Prompt for all review comments with AI agents</summary> ``` Verify each finding against the current code and only fix it if needed. Inline comments: In `@eventcore-sqlite/Cargo.toml`: - Around line 13-22: Add a compile-time guard in eventcore-sqlite's root lib (e.g., at the top of src/lib.rs) that detects the illegal feature combination and emits a compile_error! when both "bundled" and "encryption" are enabled; implement this using a cfg check such as cfg(all(feature = "bundled", feature = "encryption")) and a compile_error! with a clear message that selecting both features is unsupported and instructing the user to choose one. Ensure the check runs unconditionally at crate compile time (place it at crate root before other code) so the invalid combination fails early. In `@eventcore-sqlite/README.md`: - Around line 41-44: The README incorrectly claims Cargo "emits a clear compile-time error" for non-overlapping version ranges; update the wording around the mention of Cargo and rusqlite to say that Cargo can permit multiple SemVer-incompatible versions (e.g., rusqlite 1.x and 2.x) to coexist in the dependency graph, and that compile-time errors occur only when your code mixes types or APIs from different versions; retain the recommendation to prefer the re-export over a direct rusqlite dependency but clarify that version conflicts surface as type/API mismatches rather than Cargo failing resolution. In `@eventcore-sqlite/tests/bring_your_own_connection_test.rs`: - Around line 28-32: Remove the no-op type assertion `let _: &SqliteEventStore = &store;` from the test; it provides no behavioral coverage. Edit the test in bring_your_own_connection_test.rs to delete that line (or replace it with a real assertion that calls a method on `store` or verifies a property), referencing the `SqliteEventStore` type and the `store` variable so the test demonstrably exercises the constructed store rather than only asserting its type. ``` </details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: ASSERTIVE **Plan**: Pro Plus **Run ID**: `3573afab-998d-479d-9f07-a7b44c59397e` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 807bb8a3270a5f885646e71a9191ecfa0882dc01 and 5e8ef03a36cd5174178d83854dd126bf74a4c614. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `Cargo.lock` is excluded by `!**/*.lock` </details> <details> <summary>📒 Files selected for processing (6)</summary> * `blueprints/store-backends.md` * `eventcore-sqlite/CHANGELOG.md` * `eventcore-sqlite/Cargo.toml` * `eventcore-sqlite/README.md` * `eventcore-sqlite/src/lib.rs` * `eventcore-sqlite/tests/bring_your_own_connection_test.rs` </details> </details> <details> <summary>📜 Review details</summary> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)</summary> * GitHub Check: Test (stable, 18) * GitHub Check: Test (beta, 18) * GitHub Check: Test (beta, 15) * GitHub Check: Test (stable, 17) * GitHub Check: Test (nightly, 16) * GitHub Check: Test (nightly, 17) * GitHub Check: Test (nightly, 15) * GitHub Check: Test (nightly, 18) * GitHub Check: Test (stable, 16) * GitHub Check: Test (stable, 15) * GitHub Check: Test (beta, 17) * GitHub Check: Test (beta, 16) * GitHub Check: Clippy </details> <details> <summary>🔇 Additional comments (5)</summary><blockquote> <details> <summary>eventcore-sqlite/CHANGELOG.md (1)</summary><blockquote> `10-24`: **Release notes accurately capture the behavioral change and new APIs.** The breaking/default-feature update and BYOC/re-export additions are clearly documented. </blockquote></details> <details> <summary>eventcore-sqlite/tests/bring_your_own_connection_test.rs (1)</summary><blockquote> `34-56`: **Good BYOC checkpoint-path coverage.** This test validates externally provided connection + migration + persistence roundtrip (`save`/`load`) end-to-end. </blockquote></details> <details> <summary>blueprints/store-backends.md (1)</summary><blockquote> `50-55`: **SQLite backend blueprint update is aligned with implementation.** The feature/connection model described here matches the crate’s current behavior. </blockquote></details> <details> <summary>eventcore-sqlite/src/lib.rs (2)</summary><blockquote> `10-10`: **Crate-root `rusqlite` re-export is correctly exposed.** This provides the intended consumer path (`eventcore_sqlite::rusqlite`) for version alignment. --- `247-260`: **BYOC constructors are clear and correctly scoped.** Both constructors consistently take a caller-managed `rusqlite::Connection` as-is and preserve existing internal synchronization patterns. Also applies to: 699-711 </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
eventcore-sqlite/Cargo.toml
@ -14,2 +21,4 @@
# libsqlite3-sys links SQLCipher; consumers who want encryption without the
# vanilla SQLite dep can disable defaults.
encryption = ["rusqlite/bundled-sqlcipher-vendored-openssl"]
coderabbitai[bot] (Migrated from github.com) commented 2026-04-27 07:55:19 -07:00
Copy link

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Feature declarations:"
rg -n '^\[features\]|^default\s*=|^bundled\s*=|^encryption\s*=' eventcore-sqlite/Cargo.toml

echo
echo "Checking for existing compile-time mutual-exclusion guard:"
rg -n 'cfg\(all\(feature\s*=\s*"bundled",\s*feature\s*=\s*"encryption"\)\)|compile_error!\(' eventcore-sqlite/src/lib.rs

Repository: jwilger/eventcore

Length of output: 280

🏁 Script executed:

# Check if eventcore-sqlite/src/lib.rs exists and view its content
if [ -f eventcore-sqlite/src/lib.rs ]; then
  echo "=== eventcore-sqlite/src/lib.rs exists ==="
  wc -l eventcore-sqlite/src/lib.rs
  echo "=== First 50 lines ==="
  head -50 eventcore-sqlite/src/lib.rs
else
  echo "eventcore-sqlite/src/lib.rs does NOT exist"
fi

echo
echo "=== Directory structure of eventcore-sqlite/src ==="
find eventcore-sqlite/src -type f -name "*.rs" | head -20

Repository: jwilger/eventcore

Length of output: 1705

🏁 Script executed:

# Double-check for any compile-time guards or alternative mutual exclusion mechanisms
echo "=== Checking for any compile_error or cfg guards related to features ==="
rg -i 'compile_error|#\[cfg' eventcore-sqlite/src/ --type rust -A 2

echo
echo "=== Checking build.rs for feature validation ==="
if [ -f eventcore-sqlite/build.rs ]; then
  cat eventcore-sqlite/build.rs
else
  echo "No build.rs file found in eventcore-sqlite/"
fi

Repository: jwilger/eventcore

Length of output: 228

Enforce bundled/encryption mutual exclusivity in code.

Line 13 makes bundled default, while lines 20-21 document that bundled + encryption is unsupported. This constraint is currently documentation-only; add a compile-time guard to prevent invalid feature combinations.

🔒 Suggested guard (in eventcore-sqlite/src/lib.rs) 
+#[cfg(all(feature = "bundled", feature = "encryption"))]
+compile_error!(
+    "`bundled` and `encryption` are mutually exclusive. \
+Disable default features when enabling `encryption`."
+);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@eventcore-sqlite/Cargo.toml` around lines 13 - 22, Add a compile-time guard
in eventcore-sqlite's root lib (e.g., at the top of src/lib.rs) that detects the
illegal feature combination and emits a compile_error! when both "bundled" and
"encryption" are enabled; implement this using a cfg check such as
cfg(all(feature = "bundled", feature = "encryption")) and a compile_error! with
a clear message that selecting both features is unsupported and instructing the
user to choose one. Ensure the check runs unconditionally at crate compile time
(place it at crate root before other code) so the invalid combination fails
early.
_⚠️ Potential issue_ | _🟠 Major_ <details> <summary>🧩 Analysis chain</summary> 🏁 Script executed: ```shell #!/bin/bash set -euo pipefail echo "Feature declarations:" rg -n '^\[features\]|^default\s*=|^bundled\s*=|^encryption\s*=' eventcore-sqlite/Cargo.toml echo echo "Checking for existing compile-time mutual-exclusion guard:" rg -n 'cfg\(all\(feature\s*=\s*"bundled",\s*feature\s*=\s*"encryption"\)\)|compile_error!\(' eventcore-sqlite/src/lib.rs ``` Repository: jwilger/eventcore Length of output: 280 --- 🏁 Script executed: ```shell # Check if eventcore-sqlite/src/lib.rs exists and view its content if [ -f eventcore-sqlite/src/lib.rs ]; then echo "=== eventcore-sqlite/src/lib.rs exists ===" wc -l eventcore-sqlite/src/lib.rs echo "=== First 50 lines ===" head -50 eventcore-sqlite/src/lib.rs else echo "eventcore-sqlite/src/lib.rs does NOT exist" fi echo echo "=== Directory structure of eventcore-sqlite/src ===" find eventcore-sqlite/src -type f -name "*.rs" | head -20 ``` Repository: jwilger/eventcore Length of output: 1705 --- 🏁 Script executed: ```shell # Double-check for any compile-time guards or alternative mutual exclusion mechanisms echo "=== Checking for any compile_error or cfg guards related to features ===" rg -i 'compile_error|#\[cfg' eventcore-sqlite/src/ --type rust -A 2 echo echo "=== Checking build.rs for feature validation ===" if [ -f eventcore-sqlite/build.rs ]; then cat eventcore-sqlite/build.rs else echo "No build.rs file found in eventcore-sqlite/" fi ``` Repository: jwilger/eventcore Length of output: 228 --- </details> **Enforce `bundled`/`encryption` mutual exclusivity in code.** Line 13 makes `bundled` default, while lines 20-21 document that `bundled` + `encryption` is unsupported. This constraint is currently documentation-only; add a compile-time guard to prevent invalid feature combinations. <details> <summary>🔒 Suggested guard (in <code>eventcore-sqlite/src/lib.rs</code>)</summary> ```diff +#[cfg(all(feature = "bundled", feature = "encryption"))] +compile_error!( + "`bundled` and `encryption` are mutually exclusive. \ +Disable default features when enabling `encryption`." +); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@eventcore-sqlite/Cargo.toml` around lines 13 - 22, Add a compile-time guard in eventcore-sqlite's root lib (e.g., at the top of src/lib.rs) that detects the illegal feature combination and emits a compile_error! when both "bundled" and "encryption" are enabled; implement this using a cfg check such as cfg(all(feature = "bundled", feature = "encryption")) and a compile_error! with a clear message that selecting both features is unsupported and instructing the user to choose one. Ensure the check runs unconditionally at crate compile time (place it at crate root before other code) so the invalid combination fails early. ``` </details> <!-- fingerprinting:phantom:poseidon:hawk --> <!-- d98c2f50 --> <!-- This is an auto-generated reply by CodeRabbit -->
eventcore-sqlite/README.md
@ -0,0 +1,70 @@
# eventcore-sqlite
coderabbitai[bot] (Migrated from github.com) commented 2026-04-27 07:55:19 -07:00
Copy link

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

According to Cargo documentation, can a dependency graph include multiple versions of the same crate when version requirements do not overlap?

💡 Result:

Yes, according to Cargo documentation, a dependency graph can include multiple versions of the same crate when version requirements do not overlap, specifically when the versions are SemVer-incompatible (e.g., different major versions like 1.x and 2.x, or different 0.x series). Cargo unifies versions within SemVer-compatible ranges but allows multiple versions if they fall outside those ranges to satisfy conflicting requirements. Within the same compatibility range (e.g., two different patch versions like 0.4.11 and 0.4.8), Cargo does not allow multiple versions and will error if unification fails.

Citations:

Correct Cargo version-resolution wording at lines 41-44.

The current text incorrectly states that Cargo "emits a clear compile-time error" when version ranges do not overlap. In reality, Cargo allows multiple SemVer-incompatible versions of the same crate (e.g., rusqlite 1.x and 2.x) to coexist in the dependency graph. Compile-time errors occur when you mix types or APIs from different versions, not from Cargo's resolution itself.

📝 Suggested wording update 
-Prefer the re-export over a direct `rusqlite` dependency. Cargo will unify
-versions automatically when both crates declare compatible ranges; if the
-ranges do not overlap, Cargo emits a clear compile-time error rather than
-linking two incompatible copies.
+Prefer the re-export over a direct `rusqlite` dependency. Cargo will unify
+versions automatically when ranges overlap. If ranges do not overlap, Cargo
+may resolve multiple `rusqlite` versions; compile-time type mismatches can
+appear if values from different versions are mixed.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Prefer the re-export over a direct `rusqlite` dependency. Cargo will unify
versions automatically when ranges overlap. If ranges do not overlap, Cargo
may resolve multiple `rusqlite` versions; compile-time type mismatches can
appear if values from different versions are mixed.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@eventcore-sqlite/README.md` around lines 41 - 44, The README incorrectly
claims Cargo "emits a clear compile-time error" for non-overlapping version
ranges; update the wording around the mention of Cargo and rusqlite to say that
Cargo can permit multiple SemVer-incompatible versions (e.g., rusqlite 1.x and
2.x) to coexist in the dependency graph, and that compile-time errors occur only
when your code mixes types or APIs from different versions; retain the
recommendation to prefer the re-export over a direct rusqlite dependency but
clarify that version conflicts surface as type/API mismatches rather than Cargo
failing resolution.

✅ Addressed in commit 245f78d

_⚠️ Potential issue_ | _🟡 Minor_ <details> <summary>🧩 Analysis chain</summary> 🌐 Web query: `According to Cargo documentation, can a dependency graph include multiple versions of the same crate when version requirements do not overlap?` 💡 Result: Yes, according to Cargo documentation, a dependency graph can include multiple versions of the same crate when version requirements do not overlap, specifically when the versions are SemVer-incompatible (e.g., different major versions like 1.x and 2.x, or different 0.x series). Cargo unifies versions within SemVer-compatible ranges but allows multiple versions if they fall outside those ranges to satisfy conflicting requirements. Within the same compatibility range (e.g., two different patch versions like 0.4.11 and 0.4.8), Cargo does not allow multiple versions and will error if unification fails. Citations: - 1: https://doc.rust-lang.org/1.93.1/cargo/reference/resolver.html - 2: https://doc.rust-lang.org/1.57.0/cargo/reference/resolver.html - 3: https://doc.rust-lang.org/cargo/reference/resolver.html --- </details> **Correct Cargo version-resolution wording at lines 41-44.** The current text incorrectly states that Cargo "emits a clear compile-time error" when version ranges do not overlap. In reality, Cargo allows multiple SemVer-incompatible versions of the same crate (e.g., `rusqlite` 1.x and 2.x) to coexist in the dependency graph. Compile-time errors occur when you mix types or APIs from different versions, not from Cargo's resolution itself. <details> <summary>📝 Suggested wording update</summary> ```diff -Prefer the re-export over a direct `rusqlite` dependency. Cargo will unify -versions automatically when both crates declare compatible ranges; if the -ranges do not overlap, Cargo emits a clear compile-time error rather than -linking two incompatible copies. +Prefer the re-export over a direct `rusqlite` dependency. Cargo will unify +versions automatically when ranges overlap. If ranges do not overlap, Cargo +may resolve multiple `rusqlite` versions; compile-time type mismatches can +appear if values from different versions are mixed. ``` </details> <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion Prefer the re-export over a direct `rusqlite` dependency. Cargo will unify versions automatically when ranges overlap. If ranges do not overlap, Cargo may resolve multiple `rusqlite` versions; compile-time type mismatches can appear if values from different versions are mixed. ``` </details> <!-- suggestion_end --> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@eventcore-sqlite/README.md` around lines 41 - 44, The README incorrectly claims Cargo "emits a clear compile-time error" for non-overlapping version ranges; update the wording around the mention of Cargo and rusqlite to say that Cargo can permit multiple SemVer-incompatible versions (e.g., rusqlite 1.x and 2.x) to coexist in the dependency graph, and that compile-time errors occur only when your code mixes types or APIs from different versions; retain the recommendation to prefer the re-export over a direct rusqlite dependency but clarify that version conflicts surface as type/API mismatches rather than Cargo failing resolution. ``` </details> <!-- fingerprinting:phantom:poseidon:hawk --> <!-- d98c2f50 --> <!-- This is an auto-generated reply by CodeRabbit --> ✅ Addressed in commit 245f78d
eventcore-sqlite/tests/bring_your_own_connection_test.rs
@ -0,0 +1,58 @@
//! Integration tests verifying that consumers can supply their own
coderabbitai[bot] (Migrated from github.com) commented 2026-04-27 07:55:19 -07:00
Copy link

🧹 Nitpick | 🔵 Trivial

Remove the no-op type assertion at Line 31.

let _: &SqliteEventStore = &store; does not add behavioral coverage. Prefer removing it (or replace with a real operation assertion).

♻️ Minimal cleanup 
-    let _: &SqliteEventStore = &store;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    // Then: the store is usable end-to-end via the EventStore trait. The
    // contract suite already exercises the trait surface; here we just need
    // to demonstrate that migrate succeeded and the store is constructed.
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@eventcore-sqlite/tests/bring_your_own_connection_test.rs` around lines 28 -
32, Remove the no-op type assertion `let _: &SqliteEventStore = &store;` from
the test; it provides no behavioral coverage. Edit the test in
bring_your_own_connection_test.rs to delete that line (or replace it with a real
assertion that calls a method on `store` or verifies a property), referencing
the `SqliteEventStore` type and the `store` variable so the test demonstrably
exercises the constructed store rather than only asserting its type.

✅ Addressed in commit 245f78d

_🧹 Nitpick_ | _🔵 Trivial_ **Remove the no-op type assertion at Line 31.** `let _: &SqliteEventStore = &store;` does not add behavioral coverage. Prefer removing it (or replace with a real operation assertion). <details> <summary>♻️ Minimal cleanup</summary> ```diff - let _: &SqliteEventStore = &store; ``` </details> <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion // Then: the store is usable end-to-end via the EventStore trait. The // contract suite already exercises the trait surface; here we just need // to demonstrate that migrate succeeded and the store is constructed. } ``` </details> <!-- suggestion_end --> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@eventcore-sqlite/tests/bring_your_own_connection_test.rs` around lines 28 - 32, Remove the no-op type assertion `let _: &SqliteEventStore = &store;` from the test; it provides no behavioral coverage. Edit the test in bring_your_own_connection_test.rs to delete that line (or replace it with a real assertion that calls a method on `store` or verifies a property), referencing the `SqliteEventStore` type and the `store` variable so the test demonstrably exercises the constructed store rather than only asserting its type. ``` </details> <!-- fingerprinting:phantom:poseidon:hawk --> <!-- d98c2f50 --> <!-- This is an auto-generated reply by CodeRabbit --> ✅ Addressed in commit 245f78d
jwilger-ai-bot (Migrated from github.com) reviewed 2026-04-27 08:02:35 -07:00
eventcore-sqlite/Cargo.toml
@ -14,2 +21,4 @@
# libsqlite3-sys links SQLCipher; consumers who want encryption without the
# vanilla SQLite dep can disable defaults.
encryption = ["rusqlite/bundled-sqlcipher-vendored-openssl"]
jwilger-ai-bot (Migrated from github.com) commented 2026-04-27 08:02:35 -07:00
Copy link

Fixed in 245f78d, but as a doc correction rather than a code guard.

The Cargo.toml comment was wrong about the combination being "unsupported" — libsqlite3-sys links SQLCipher when both bundled and encryption are active (SQLCipher takes precedence), with no link-time conflict. The workspace CI already runs cargo nextest --all-features with both features enabled and the encrypted_migration_test passes against that build, which is the empirical proof.

Adding compile_error!(all(feature = "bundled", feature = "encryption")) would break that CI invocation and force a feature-matrix split for no real benefit. Reworded the Cargo.toml comment and the README to describe the actual behavior: when both are active you get SQLCipher; consumers who want encryption without the vanilla bundled SQLite source can disable defaults.

Fixed in 245f78d, but as a doc correction rather than a code guard. The Cargo.toml comment was wrong about the combination being "unsupported" — `libsqlite3-sys` links SQLCipher when both `bundled` and `encryption` are active (SQLCipher takes precedence), with no link-time conflict. The workspace CI already runs `cargo nextest --all-features` with both features enabled and the `encrypted_migration_test` passes against that build, which is the empirical proof. Adding `compile_error!(all(feature = "bundled", feature = "encryption"))` would break that CI invocation and force a feature-matrix split for no real benefit. Reworded the Cargo.toml comment and the README to describe the actual behavior: when both are active you get SQLCipher; consumers who want encryption without the vanilla bundled SQLite source can disable defaults.
jwilger-ai-bot (Migrated from github.com) reviewed 2026-04-27 08:02:46 -07:00
eventcore-sqlite/README.md
@ -0,0 +1,70 @@
# eventcore-sqlite
jwilger-ai-bot (Migrated from github.com) commented 2026-04-27 08:02:46 -07:00
Copy link

Good catch — fixed in 245f78d. The original wording was wrong: Cargo permits multiple SemVer-incompatible copies of rusqlite in the dependency graph rather than failing resolution, and the mismatch only surfaces as a type error at the call site when a Connection from one version is handed to an API expecting the other. Reworded the README accordingly and kept the recommendation to use the re-export.

Good catch — fixed in 245f78d. The original wording was wrong: Cargo permits multiple SemVer-incompatible copies of `rusqlite` in the dependency graph rather than failing resolution, and the mismatch only surfaces as a type error at the call site when a `Connection` from one version is handed to an API expecting the other. Reworded the README accordingly and kept the recommendation to use the re-export.
jwilger-ai-bot (Migrated from github.com) reviewed 2026-04-27 08:02:56 -07:00
eventcore-sqlite/tests/bring_your_own_connection_test.rs
@ -0,0 +1,58 @@
//! Integration tests verifying that consumers can supply their own
jwilger-ai-bot (Migrated from github.com) commented 2026-04-27 08:02:56 -07:00
Copy link

Replaced in 245f78d. Instead of removing the line outright, swapped it for a second migrate().await call so the test exercises the consumer-supplied connection end-to-end (proving the round-trip is live, and incidentally that migrate is idempotent against a BYO connection).

Replaced in 245f78d. Instead of removing the line outright, swapped it for a second `migrate().await` call so the test exercises the consumer-supplied connection end-to-end (proving the round-trip is live, and incidentally that migrate is idempotent against a BYO connection).
coderabbitai[bot] (Migrated from github.com) reviewed 2026-04-27 08:03:02 -07:00
eventcore-sqlite/README.md
@ -0,0 +1,70 @@
# eventcore-sqlite
coderabbitai[bot] (Migrated from github.com) commented 2026-04-27 08:03:02 -07:00
Copy link

@jwilger-ai-bot, thanks for the confirmation and the clear explanation in the commit message! The updated wording accurately reflects Cargo's resolver behavior. Marking this as resolved.

`@jwilger-ai-bot`, thanks for the confirmation and the clear explanation in the commit message! The updated wording accurately reflects Cargo's resolver behavior. Marking this as resolved. <!-- <review_comment_addressed> --> <!-- This is an auto-generated reply by CodeRabbit -->
coderabbitai[bot] (Migrated from github.com) reviewed 2026-04-27 08:03:11 -07:00
eventcore-sqlite/tests/bring_your_own_connection_test.rs
@ -0,0 +1,58 @@
//! Integration tests verifying that consumers can supply their own
coderabbitai[bot] (Migrated from github.com) commented 2026-04-27 08:03:11 -07:00
Copy link

@jwilger-ai-bot, great call — a second migrate().await is a meaningful improvement over both the original no-op and the suggested bare deletion. It actively verifies the migration is idempotent on a consumer-supplied connection, which is a useful property to assert. Thanks for the follow-through!

`@jwilger-ai-bot`, great call — a second `migrate().await` is a meaningful improvement over both the original no-op and the suggested bare deletion. It actively verifies the migration is idempotent on a consumer-supplied connection, which is a useful property to assert. Thanks for the follow-through! <!-- <review_comment_addressed> --> <!-- This is an auto-generated reply by CodeRabbit -->
coderabbitai[bot] (Migrated from github.com) reviewed 2026-04-27 08:03:17 -07:00
eventcore-sqlite/Cargo.toml
@ -14,2 +21,4 @@
# libsqlite3-sys links SQLCipher; consumers who want encryption without the
# vanilla SQLite dep can disable defaults.
encryption = ["rusqlite/bundled-sqlcipher-vendored-openssl"]
coderabbitai[bot] (Migrated from github.com) commented 2026-04-27 08:03:17 -07:00
Copy link

@jwilger-ai-bot, thanks for the detailed correction — my analysis was wrong. The bundled-sqlcipher-vendored-openssl feature in libsqlite3-sys bundled-sqlcipher-vendored-openssl allows using bundled-sqlcipher with a vendored OpenSSL, and as the name implies, it depends on and automatically turns on bundled-sqlcipher — so it is an additive feature that takes precedence over the plain bundled (vanilla SQLite) feature at the link level, rather than creating a conflict. A compile_error! guard would have been incorrect and would have broken --all-features CI. The documentation correction in 245f78d is the right fix here.

<review_comment_addressed>

✏️ Learnings added 
Learnt from: jwilger-ai-bot
Repo: jwilger/eventcore PR: 379
File: eventcore-sqlite/Cargo.toml:13-23
Timestamp: 2026-04-27T15:03:10.675Z
Learning: In `eventcore-sqlite` (Rust crate), the `bundled` and `encryption` Cargo features are NOT mutually exclusive. When both are active, `libsqlite3-sys` links SQLCipher (the `bundled-sqlcipher-vendored-openssl` feature takes precedence over the plain `bundled`/vanilla SQLite feature). Adding a `compile_error!` guard for `cfg(all(feature = "bundled", feature = "encryption"))` would be wrong and would break `--all-features` CI. The correct approach is to document that when both are active, you get SQLCipher.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

`@jwilger-ai-bot`, thanks for the detailed correction — my analysis was wrong. The `bundled-sqlcipher-vendored-openssl` feature in `libsqlite3-sys` `bundled-sqlcipher-vendored-openssl` allows using `bundled-sqlcipher` with a vendored OpenSSL, and as the name implies, it depends on and automatically turns on `bundled-sqlcipher` — so it is an additive feature that takes precedence over the plain `bundled` (vanilla SQLite) feature at the link level, rather than creating a conflict. A `compile_error!` guard would have been incorrect and would have broken `--all-features` CI. The documentation correction in 245f78d is the right fix here. <review_comment_addressed> --- <details> <summary>✏️ Learnings added</summary> ``` Learnt from: jwilger-ai-bot Repo: jwilger/eventcore PR: 379 File: eventcore-sqlite/Cargo.toml:13-23 Timestamp: 2026-04-27T15:03:10.675Z Learning: In `eventcore-sqlite` (Rust crate), the `bundled` and `encryption` Cargo features are NOT mutually exclusive. When both are active, `libsqlite3-sys` links SQLCipher (the `bundled-sqlcipher-vendored-openssl` feature takes precedence over the plain `bundled`/vanilla SQLite feature). Adding a `compile_error!` guard for `cfg(all(feature = "bundled", feature = "encryption"))` would be wrong and would break `--all-features` CI. The correct approach is to document that when both are active, you get SQLCipher. ``` > Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure [review instructions](https://docs.coderabbit.ai/guides/review-instructions). You can manage existing learnings in the [app](https://app.coderabbit.ai/learnings). </details> <!-- This is an auto-generated reply by CodeRabbit -->
coderabbitai[bot] (Migrated from github.com) approved these changes 2026-04-27 10:08:17 -07:00
Sign in to join this conversation.
Reviewers
No reviewers
coderabbitai[bot]
Labels
Clear labels   
adr
Architecture Decision Record

  
automated

   
bug
Something isn't working

  
chore
Maintenance (audits, cleanup)

  
dependencies

   
documentation
Improvements or additions to documentation

  
enhancement
New feature or request

  
epic
Large feature with sub-issues

  
github-actions

   
P1-high

   
P2-medium
Default priority

  
P3-low
Polish, optimization

  
release

   
research
Investigation / spike

  
rust
No labels
adr
automated
bug
chore
dependencies
documentation
enhancement
epic
github-actions
P1-high
P2-medium
P3-low
release
research
rust
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
jwilger/eventcore!379
No description provided.