Repository navigation
feat(session): sessions.db on the SQLite driver's native mode - #348
Conversation
Update the vendored tinystoragedrivers submodule to the latest upstream commit. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The session store now opens its database through the SQLite driver's native mode instead of caching its own rusqlite connection, so every handle the host opens on the same file shares one connection and lock. Pragmas and migrations still run once per path, and the driver dependency moved from dev-dependencies to a regular dependency. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The helper now accepts any displayable error rather than a concrete StorageError, so it can also wrap the driver's connection lock failure. A doc comment was added to state what the helper represents. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Wire up a cfg(test) module in the session store so the new store_tests.rs file is compiled and run with the crate's test suite. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Tiny Sweeper reviewTiny Sweeper reviewed this pull request across 6 lane(s) and found 0 active actionable finding(s). The change moves the session store onto the SQLite driver's native mode, pins tinystoragedrivers to 0.4.0, documents the durability implications in the crate README and module docs, adds panic cleanup in with_connection so a panicking call cannot poison the shared connection, and adds tests in store_tests.rs covering the new behaviour. Reviewers noted the diff was reviewed without code retrieval or memory tooling, and the vendored SqliteNative submodule was not checked out, so the driver-side durability claims (WAL with synchronous = NORMAL, driver-set busy timeout) are stated but not verified or pinned by tests. All lanes concluded the change looks safe to merge. State: Ready for maintainer review Review snapshot
Completeness: Complete What changedNo supported behavioral explanation was produced. Features
Tests
FindingsNo active actionable findings. Before mergeNone. Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5e0af5e55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0078 · 160,626 in / 10,141 out · 17,432 cached (11%) · gpt-5.6-luna, glm-5.3-flash, deepseek-v4.1-flash
critique: $0.0033 · 57,936 in / 3,481 out · 8,271 cached (14%) · gpt-5.6-luna
security: $0.0044 · 81,622 in / 3,495 out · 9,161 cached (11%) · gpt-5.6-luna
tests: $0.0001 · 7,474 in / 627 out · 0 cached (0%) · glm-5.3-flash
description: $0.0001 · 7,462 in / 203 out · 0 cached (0%) · glm-5.3-flash
…ing) Co-authored-by: Medulla <medulla@tinyhumans.ai>
The session store README and store module docs now describe that the database is opened through the tinystoragedrivers SQLite driver's native mode, with one shared connection per file per process, and that WAL runs with synchronous = NORMAL. This makes explicit that commits survive a process crash but recent transactions can be rolled back after an OS crash or power loss, so callers know the transcript files remain the Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 713bfc15f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A panic inside a session call used to unwind while the driver's connection lock was held, poisoning the shared connection for every later call in the process. The panic is now caught inside the lock, any open transaction is rolled back, and the panic is resumed afterwards so callers still see it. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The rollback-after-panic path in with_connection now uses a single let-chain condition instead of nested ifs, with no change in behaviour. The test closure gains an explicit return type so the panic path still type-checks. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1a8a029b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| native | ||
| .run_blocking(|conn| { | ||
| prepare_connection(conn)?; | ||
| migrations::apply(conn) | ||
| }) | ||
| .map_err(driver_error)??; |
There was a problem hiding this comment.
Release the cache lock before acquiring the driver lock
When a host uses SqliteNative directly, this introduces a lock-order inversion: cached_connection retains the process-wide cache mutex while run_blocking waits for a per-file driver lock. If one thread holds driver lock A and, from its callback, opens an uncached session DB B while another thread performs the first session access to A, the latter holds the cache mutex waiting for A and the former waits for the cache mutex, deadlocking both operations. Initialize the native connection without holding the global cache lock, then reconcile concurrent insertions afterward (or use per-path initialization state).
Useful? React with 👍 / 👎.
Summary
This moves
sessions.dbonto the tinystoragedrivers SQLite driver's native mode. The session crate keeps its own tables, SQL, FTS5 index and migrations exactly as they are, and the driver now owns the connection.store::with_connection/with_transaction(and through them every session op, the run ledger, the entry tree and retention) run onSqliteNative::run_blocking, the driver's shared per-file connection.migrations::applyonce per path.{workspace}/session_db/sessions.db, the same schema andschema_versionmarker, and no data migration.a_database_written_before_native_mode_opens_unchangedcreates a database the old way and reads it through the new path.db_path,with_connection,with_transaction, and every op signature.synchronous = NORMAL(with WAL). After a power loss, the last committed transactions may roll back, but the database stays consistent. WAL, the 5 s busy timeout andforeign_keys = ONare unchanged; the session still sets them itself.tinystoragedrivers-sqlitebecomes a normal dependency oftinyagents-session, through a new workspace entry. It was already a dev-dependency, andcargo tree -dshows a singlelibsqlite3-sys.Builds on tinyhumansai/tinystoragedrivers#6 (
SqliteNative::run_blocking), merged and released as v0.4.0.vendor/tinystoragedriversis pinned to that tag, and the workspace requirements are0.4.0.Tests
New
store_tests.rs:cargo test --workspace(default features) hit one failure:providers::claude_agent_sdk::tests::provider_pipes_large_request_to_cli_stdin, which spawns a fake CLI script. It passes on every rerun and is untouched here, so it's a load-dependent flake onmain.