feat(postgres): fix call sites and add tests for postgres build#2384
Merged
feat(postgres): fix call sites and add tests for postgres build#2384
Conversation
github-actions
bot
added
enhancement
#2377, #2378) Introduce `resolve_db_url(config)` helper that returns `database_url` when set and non-empty, otherwise falls back to `sqlite_path`. This mirrors the logic already present in `handle_db_migrate` (which is now also updated to use the helper instead of inlining the same logic). Replace hardcoded `config.memory.sqlite_path` references across six files so all call sites resolve the correct URL under both sqlite and postgres feature builds: - src/commands/skill.rs (single extraction point, 8 indirect uses) - src/commands/memory.rs - src/commands/sessions.rs - src/runner.rs - src/acp.rs (3 sites) - src/scheduler.rs (2 sites) The `cargo check` postgres target still fails in `zeph-memory` due to pre-existing Phase 2 debt (Pool<Postgres> vs Sqlite type mismatches) — no new errors are introduced by this PR. Tests added: - src/commands/db.rs: cfg-gated unit tests for `is_postgres_url` predicate under both postgres and sqlite features (#2377) - src/init.rs: unit tests for `build_config` postgres path — verifies `database_url` propagates and `sqlite_path` defaults correctly (#2378)
bug-ops
force-pushed
the
feat-postgres-fix-sqlitestore
branch
from
7347269 to
60af647
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2374
Closes #2377
Closes #2378
Summary
resolve_db_url(config)helper insrc/db_url.rsthat returnsdatabase_urlwhen set and non-empty, falling back tosqlite_path. Eliminates the duplicated inline logic that was inhandle_db_migrate.config.memory.sqlite_pathacross 6 files (skill.rs,memory.rs,sessions.rs,runner.rs,acp.rs,scheduler.rs) so all call sites work correctly under both--features sqliteand--features postgres.is_postgres_urlpredicate insrc/commands/db.rs(test(db): add unit tests for db migrate URL validation branches #2377).build_configunit tests for the postgres backend path insrc/init.rs(test(init): add build_config test for postgres backend path #2378).Pre-existing postgres check failures
cargo check --features postgres,...still fails with 27E0271errors inzeph-memory(Pool vs Sqlite type mismatches from Phase 2, PR #2372). Zero new errors are introduced by this PR — verified by checking that all failing file paths are incrates/zeph-memory/.Test plan
cargo +nightly fmt --check— passcargo clippy --workspace --features full -- -D warnings— passcargo nextest run --workspace --features full --lib --bins— 7195 passed, 22 skippedcargo check --features postgres,... 2>&1 | grep "^error\[" | grep -v "zeph-memory"— zero output (no new errors)