Skip to content

feat(flowctl): migrate rusqlite → libSQL async + native vectors#9

Merged
z23cc merged 9 commits intomainfrom
fn-19-migrate-flowctl-to-libsql-async-native
Apr 5, 2026
Merged

feat(flowctl): migrate rusqlite → libSQL async + native vectors#9
z23cc merged 9 commits intomainfrom
fn-19-migrate-flowctl-to-libsql-async-native

Conversation

@z23cc
Copy link
Copy Markdown
Owner

@z23cc z23cc commented Apr 5, 2026

Summary

Full data-layer migration from rusqlite (sync) to libsql 0.9 (async). Memory subsystem gains semantic search via libSQL's native F32_BLOB(384) vector column + libsql_vector_idx + fastembed (BGE-small, ONNX, in-process).

No backward compatibility — fresh schema, fresh DBs. No data migration from old flowctl.db.

What changed

  • Zero rusqlite in the workspace (Cargo.toml + Rust code)
  • Zero spawn_blocking in daemon handlers
  • Zero Arc<Mutex<Connection>> — libsql Connection is cheap Clone
  • ✅ Crate flowctl-db deleted (4,533 LOC removed)
  • ✅ New crate flowctl-db-lsql is the sole storage layer (async, all 12 repos, vector search)
  • ✅ Service + daemon + CLI fully async cascade
  • ✅ fastembed for semantic memory search (first-build downloads ~130MB model to .fastembed_cache/)

Architecture pivot

Initial strategy: additive migration keeping both rusqlite + libsql. Blocked by C-level static init collision ("Once instance has previously been poisoned") when both SQLite-family libs link into the same test binary. Pivoted to separate-crate strategy: flowctl-db-lsql as a sibling, each crate tests in its own binary. Clean.

Verification

cargo check --all --release              # green
cargo test --all --release               # 269 passed, 2 ignored
  (2 ignored are network-gated fastembed model download tests)

grep -rn "rusqlite" crates/ --toml --rs  # zero code refs
grep -rn "spawn_blocking" flowctl-daemon/ # zero
grep flowctl_db\b crates/ --rs            # zero (old crate name)

Per-crate test counts (release):

  • flowctl-core: 132 passed
  • flowctl-db-lsql: 37 passed, 2 ignored
  • flowctl-service: 2 passed
  • flowctl-daemon: 29 passed
  • flowctl-scheduler: 31 passed
  • flowctl-cli: 17 + 2 + 1 + 16 across integration tests

Migration log (7 tasks sequential)

  1. Foundation: new crate, async pool, schema with F32_BLOB
  2. Epic/Task/Dep/FileOwnership repos (async)
  3. Runtime/Evidence/FileLock/PhaseProgress repos
  4. Event log + stats + metrics
  5. Memory + fastembed + vector search
  6. Service + daemon + CLI async cascade (removed 5 spawn_blocking)
  7. Delete rusqlite stack + CLI shim + indexer port

Follow-ups (out of scope)

  • Rename flowctl-db-lsqlflowctl-db (mechanical, touches ~40 files)
  • Activate Turso cloud sync (code is ready, not wired)
  • Migrate CLI workflow scheduling/phase paths from sync shim to direct async (they currently go through db_shim.rs for syntactic stability)

Test plan

  • cargo test --all --release green
  • cargo build --release produces flowctl binary
  • flowctl init && flowctl epic create --title test --json works end-to-end
  • Frontend still works against daemon (no API contract changes)

Epic: fn-19-migrate-flowctl-to-libsql-async-native

z23cc and others added 9 commits April 5, 2026 16:25
…sts pass)

Task 1 of fn-19 migration: libsql 0.9 installed as workspace dep,
new pool_async.rs with async open_memory_async/open_async, schema
consolidated into schema_libsql.sql with F32_BLOB(384) on memory table
and native libsql_vector_idx for semantic search.

ISSUE FOUND: libsql 0.9 cannot coexist with rusqlite in the same
test binary. 'Once instance has previously been poisoned' panic
when both init in same process. pool_async tests pass in isolation
(cargo test pool_async::) but fail when run with rusqlite tests.

Root cause: libsql bundles its own libsql-core (SQLite fork) and
rusqlite links libsqlite3. The two static inits collide.

Options for next step:
  A. Split into separate crate (flowctl-db-libsql)
  B. Feature-flag XOR (rusqlite OR libsql, never both)
  C. Big cutover: delete rusqlite entirely NOW, break everything, rebuild
  D. Use libsql-rusqlite compat shim instead of native libsql API

Committing scaffolding + schema + 6 passing tests as checkpoint.
Waiting for architectural decision before proceeding to task 2.

fn-19-migrate-flowctl-to-libsql-async-native.1
Task 1 of fn-19 migration (strategy pivot — separate crate, Option A).

Why separate crate: libsql 0.9 and rusqlite cannot coexist in the same
test binary due to C-level static init collision ('Once instance has
previously been poisoned'). Splitting into its own crate gives clean
test isolation — each crate's tests run in a separate binary without
the other's SQLite init running.

New crate contents:
- pool.rs: async open_async() + open_memory_async() + PRAGMAs
- schema.sql: all 14 tables + memory.embedding F32_BLOB(384) +
  libsql_vector_idx for native semantic search
- error.rs: self-contained DbError enum (libsql::Error + serde_json + etc.)
- lib.rs: re-exports Connection, Database

Test isolation verified:
- cargo test -p flowctl-db --release: 57 passed
- cargo test -p flowctl-db-lsql --release: 4 passed
- cargo test --all --release: all green (separate test binaries)

Next: tasks 2-5 add repos in flowctl-db-lsql. Task 6 migrates callers.
Task 7 deletes flowctl-db entirely.

fn-19-migrate-flowctl-to-libsql-async-native.1
…rshipRepo

Port core relational repos from flowctl-db (sync rusqlite) to
flowctl-db-lsql (async libsql). All methods take owned Connection
(cheap clone) and return DbError.

- EpicRepo: upsert/upsert_with_body/get/get_with_body/list/
  update_status/delete + epic_deps upsert
- TaskRepo: upsert/upsert_with_body/get/get_with_body/list_by_epic/
  list_all (status+domain filters)/list_by_status/update_status/
  delete + task_deps + file_ownership upsert
- DepRepo: add/remove/list for task_deps and epic_deps
- FileOwnershipRepo: add/remove/list_for_task/list_for_file
- parse_* helpers inlined at top of file
- 9 new async tests; empty-body preservation verified;
  NotFound mapping verified

All 13 tests pass (4 pool + 9 repo) on cargo test --release.
cargo check --all --release green.

fn-19-migrate-flowctl-to-libsql-async-native.2
…gress repos

Appends four async repos to flowctl-db-lsql, completing the runtime
state surface area (Task 3 of fn-19). Each repo owns a libsql::Connection
by value (cheap clone) and exposes async methods returning DbError.

- RuntimeRepo: upsert/get on runtime_state (Teams assignment + timing)
- EvidenceRepo: upsert/get with JSON commit/test arrays
- FileLockRepo: Teams-mode file locking — acquire returns
  DbError::Constraint on duplicate file_path (load-bearing for
  concurrency). libsql Error is string-matched on
  'unique constraint' / 'constraint failed' / 'primary key'
- PhaseProgressRepo: mark_done/get_completed/reset on phase_progress

Tests: 21 passing (13 existing + 8 new). Notable: file_lock_acquire_twice
verifies the Constraint error path.

Verified: cargo test -p flowctl-db-lsql --release passes;
cargo check --all --release green.

fn-19-migrate-flowctl-to-libsql-async-native.3

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Port events.rs, metrics.rs, and EventRepo from flowctl-db to async libSQL:

- events.rs: EventLog with query/query_by_type/record_token_usage/
  tokens_by_task/tokens_by_epic/count_by_type. Exports EventRow,
  TaskTokenSummary, TokenRecord, TokenUsageRow.
- metrics.rs: StatsQuery with summary/epic_stats/weekly_trends/
  token_breakdown/bottlenecks/dora_metrics/domain_duration_stats/
  generate_monthly_rollups.
- repo.rs: append EventRepo + EventRow (async, owned Connection)
  with insert/list_by_epic/list_by_type using last_insert_rowid.
- lib.rs: module decls + re-exports of new types.

All methods take owned libsql::Connection (cheap Clone). Uses the
async .query(..).await + rows.next().await pattern consistent with
Tasks 2/3. 11 new tests added (32 total, up from 21).

fn-19-migrate-flowctl-to-libsql-async-native.4
Semantic memory search via BGE-small embeddings + libSQL native
vector_top_k. Offline fallback to literal substring search.

- New `memory` module with MemoryRepo, MemoryEntry, MemoryFilter
- Methods: add, get, list, search_literal, search_semantic,
  delete, increment_refs
- fastembed 5.12 lazy-initialized via OnceCell + Mutex, CPU work
  pushed to blocking pool; first call downloads ~130MB BGE-small
- Embedding failures are non-fatal: add() still inserts with NULL
  embedding and logs a warning; search_semantic() returns
  DbError::Schema so callers can fall back to search_literal
- Vector storage via SQL `vector32(?1)` literal; semantic search
  uses `vector_top_k('memory_emb_idx', ...)` native libSQL index
- 5 new unit tests (CRUD, literal search, filter, refs, hash dedup)
  + 2 gated tests (#[ignore]) for model download + semantic lookup
- Verified: `cargo test -p flowctl-db-lsql --release` → 37 passing,
  2 ignored; `cargo check --all --release` → green
- Ignored tests verified manually against real model:
  `test_embedder_loads` (384-dim output), `test_search_semantic`
  (query "javascript frontend framework" → React entry)

fn-19-migrate-flowctl-to-libsql-async-native.5
Task 6 of fn-19: migrate service + daemon layers to async libsql.

Service layer (crates/flowctl-service):
- Swap flowctl-db → flowctl-db-lsql, add libsql + tokio deps
- All lifecycle functions (start/done/block/fail/restart) become async
- Accept Option<&libsql::Connection> instead of Option<&rusqlite::Connection>
- All repo calls go through flowctl_db_lsql with .await
- ServiceError wraps flowctl_db_lsql::DbError
- connection.rs: drop ConnectionProvider trait (rusqlite-only), provide
  async open_async() + FileConnectionProvider::connect() async method

Daemon layer (crates/flowctl-daemon):
- Swap flowctl-db → flowctl-db-lsql, add libsql; drop rusqlite
- AppState.db: libsql::Connection (owned, cheap Clone) — no Arc<Mutex<>>
- Remove db_lock() — callers clone via state.db.clone()
- Remove all 5 tokio::task::spawn_blocking wrappers in handlers/task.rs
  (start/start_rest/done_rest/block_rest/restart_rest/done/block/restart)
- handlers/epic.rs, dag.rs, mod.rs: async repo/query cascade
- create_state() becomes async (opens db via open_async)
- AppError: add NotFound variant (HTTP 404), map DbError variants to
  appropriate HTTP status (NotFound→404, Constraint/InvalidInput→400)
- Migrate in-test inserts from state.db.lock() to state.db.execute().await

CLI (crates/flowctl-cli):
- Add flowctl-db-lsql + libsql deps, make tokio required (was optional)
- Add try_open_lsql_conn() + block_on() helpers in workflow/mod.rs
- workflow/lifecycle.rs: wrap each service call in block_on(...)
- mcp.rs: open libsql conn via tokio runtime, wrap service calls
- main.rs: .await on create_state()
- tests/parity_test.rs: 3 tests stubbed with #[ignore] (need rewrite with
  libsql async; tracked for fn-19 task 7)

flowctl-db-lsql drive-by fix:
- apply_pragmas(): use conn.query() instead of conn.execute() for PRAGMAs
  that return rows (journal_mode = WAL returns the resulting mode, which
  caused "Execute returned rows" errors on file-backed DBs). Fixes
  daemon + service file-backed test setup.

Workspace:
- Add flowctl-db-lsql to workspace deps in root Cargo.toml

Verification:
- cargo check --all --release: green
- cargo test -p flowctl-db-lsql --release: 37 passed (unchanged)
- cargo test -p flowctl-service --release: 2 passed
- cargo test -p flowctl-daemon --features daemon --release: 29 passed
- cargo build --release -p flowctl-daemon --features daemon: binary builds

Deferred to task 7:
- 3 parity tests in flowctl-cli/tests/parity_test.rs (stubbed + #[ignore])
- CLI workflow scheduling.rs/phase.rs still use flowctl-db (rusqlite) via
  try_open_db() helper — unchanged, only lifecycle.rs migrated
- flowctl-db rusqlite crate itself remains for CLI read paths

fn-19-migrate-flowctl-to-libsql-async-native.6
Completes fn-19 migration. flowctl-db (rusqlite) crate deleted;
all 14 CLI command files migrated to flowctl-db-lsql via a small
sync shim (crates/flowctl-cli/src/commands/db_shim.rs) that wraps
the async repos in per-call current-thread tokio runtimes, keeping
call sites syntactically unchanged.

Changes:
- crates/flowctl-db/ deleted (968-line indexer included)
- crates/flowctl-db-lsql/src/indexer.rs: ported from old crate
  (async reindex + Python-legacy .json/.md fallback parsers)
- crates/flowctl-db-lsql/src/pool.rs: added cleanup(), resolve_db_path()
- crates/flowctl-cli/src/commands/db_shim.rs: new sync shim module
- flowctl-scheduler: dropped unused flowctl-db workspace dep
- workspace Cargo.toml: removed rusqlite, rusqlite_migration, include_dir
  shared deps; removed flowctl-db path dep; removed crate from members
- tests: 3 ignored parity placeholders removed; export_import_test.rs
  and parity_test.rs rewritten with async libsql (tokio::test)

Verification:
- grep rusqlite     crates/ --include '*.toml' --include '*.rs' → 0 code refs
- grep flowctl_db\\b crates/ --include '*.rs'                   → 0 code refs
- grep spawn_blocking crates/flowctl-daemon/                    → 0
- cargo test --all --release                                    → 269 passed

Follow-up: renaming flowctl-db-lsql → flowctl-db left as a follow-up
(would touch 6 Cargo.toml + ~40 .rs files and is mechanically safe
but not required for functional completeness).

fn-19-migrate-flowctl-to-libsql-async-native.7
@z23cc z23cc marked this pull request as ready for review April 5, 2026 09:37
Copilot AI review requested due to automatic review settings April 5, 2026 09:37
@z23cc z23cc merged commit 0f33d9a into main Apr 5, 2026
1 check failed
@z23cc z23cc deleted the fn-19-migrate-flowctl-to-libsql-async-native branch April 5, 2026 09:38
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes a full migration of flowctl’s storage layer from sync rusqlite to async libsql (v0.9), introducing a new flowctl-db-lsql crate (including native vector embeddings for semantic memory search) and updating the service/daemon/CLI layers to use the new async DB APIs.

Changes:

  • Remove the legacy flowctl-db (rusqlite + migrations) implementation and all workspace dependencies on it.
  • Add flowctl-db-lsql with embedded schema application, async repos/indexer/metrics/events, and semantic search (native F32_BLOB(384) + vector index + fastembed).
  • Cascade async DB usage through flowctl-service and daemon handlers; add a sync CLI shim (db_shim) to preserve existing sync call sites.

Reviewed changes

Copilot reviewed 63 out of 64 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
flowctl/crates/flowctl-service/src/lifecycle.rs Converts lifecycle operations to async libSQL repos and awaits DB calls.
flowctl/crates/flowctl-service/src/lib.rs Updates crate docs/exports for async libSQL connection management.
flowctl/crates/flowctl-service/src/error.rs Switches service DB error type to flowctl_db_lsql::DbError.
flowctl/crates/flowctl-service/src/connection.rs Reworks connection opening for libSQL and adds async tests.
flowctl/crates/flowctl-service/Cargo.toml Replaces rusqlite deps with libsql/flowctl-db-lsql and adds tokio.
flowctl/crates/flowctl-scheduler/Cargo.toml Drops dependency on removed flowctl-db.
flowctl/crates/flowctl-db/src/pool.rs Deletes legacy rusqlite connection pool implementation.
flowctl/crates/flowctl-db/src/migrations/04-memory-table/up.sql Deletes legacy migration (memory table).
flowctl/crates/flowctl-db/src/migrations/03-body-column/up.sql Deletes legacy migration (body columns).
flowctl/crates/flowctl-db/src/migrations/03-body-column/down.sql Deletes legacy down migration.
flowctl/crates/flowctl-db/src/migrations/02-retry-count/up.sql Deletes legacy migration (retry_count).
flowctl/crates/flowctl-db/src/migrations/02-retry-count/down.sql Deletes legacy down migration.
flowctl/crates/flowctl-db/src/migrations/01-initial/up.sql Deletes legacy initial schema migration.
flowctl/crates/flowctl-db/src/migrations/01-initial/down.sql Deletes legacy schema teardown migration.
flowctl/crates/flowctl-db/src/migration.rs Deletes legacy runtime-state migration code.
flowctl/crates/flowctl-db/src/lib.rs Deletes legacy flowctl-db crate root exports.
flowctl/crates/flowctl-db/src/events.rs Deletes legacy rusqlite events/token-usage module.
flowctl/crates/flowctl-db/src/error.rs Deletes legacy rusqlite DbError type.
flowctl/crates/flowctl-db-lsql/src/schema.sql Defines consolidated “fresh schema” + vector index + trigger.
flowctl/crates/flowctl-db-lsql/src/pool.rs Adds async open/cleanup/memory-open and schema+pragma application.
flowctl/crates/flowctl-db-lsql/src/metrics.rs Ports stats queries to async libSQL.
flowctl/crates/flowctl-db-lsql/src/memory.rs Adds semantic memory repo using fastembed + libSQL vector search.
flowctl/crates/flowctl-db-lsql/src/lib.rs New crate root exports and architecture docs.
flowctl/crates/flowctl-db-lsql/src/indexer.rs Ports reindexer (Markdown/legacy JSON scan) to async libSQL.
flowctl/crates/flowctl-db-lsql/src/events.rs Ports event log + token usage to async libSQL.
flowctl/crates/flowctl-db-lsql/src/error.rs New DbError type for libSQL-based layer.
flowctl/crates/flowctl-db-lsql/Cargo.toml Renames crate and swaps deps to libSQL/tokio/fastembed.
flowctl/crates/flowctl-daemon/src/server.rs Makes daemon state creation async and uses libSQL connection in state.
flowctl/crates/flowctl-daemon/src/handlers/task.rs Removes spawn_blocking and migrates task handlers to async repos/service calls.
flowctl/crates/flowctl-daemon/src/handlers/mod.rs Switches list/token/memory/stats endpoints to async libSQL queries.
flowctl/crates/flowctl-daemon/src/handlers/epic.rs Migrates epic handlers (create/set plan/start work) to async libSQL.
flowctl/crates/flowctl-daemon/src/handlers/dag.rs Migrates DAG handlers/mutations to async libSQL and async touch_updated_at.
flowctl/crates/flowctl-daemon/src/handlers/common.rs Removes rusqlite mutex/lock path; maps DbError variants to HTTP errors.
flowctl/crates/flowctl-daemon/Cargo.toml Replaces rusqlite/flowctl-db deps with libSQL/flowctl-db-lsql.
flowctl/crates/flowctl-cli/tests/parity_test.rs Removes rusqlite parity tests; adds libSQL smoke/parity check helper.
flowctl/crates/flowctl-cli/tests/export_import_test.rs Converts export/import tests to async libSQL + async reindex.
flowctl/crates/flowctl-cli/src/main.rs Awaits async daemon create_state call.
flowctl/crates/flowctl-cli/src/commands/workflow/phase.rs Routes phase progress DB access through the new sync db_shim.
flowctl/crates/flowctl-cli/src/commands/workflow/mod.rs Adds helpers to open libSQL conn and block_on async service calls.
flowctl/crates/flowctl-cli/src/commands/workflow/lifecycle.rs Uses block_on to call async service lifecycle from sync CLI commands.
flowctl/crates/flowctl-cli/src/commands/task/query.rs Switches direct DB writes to db_shim repos.
flowctl/crates/flowctl-cli/src/commands/task/mutate.rs Switches direct DB writes to db_shim repos.
flowctl/crates/flowctl-cli/src/commands/task/mod.rs Switches DB reads/writes to db_shim repos.
flowctl/crates/flowctl-cli/src/commands/task/create.rs Switches DB upsert to db_shim repo.
flowctl/crates/flowctl-cli/src/commands/stats.rs Switches stats/cleanup paths to db_shim.
flowctl/crates/flowctl-cli/src/commands/query.rs Switches query/runtime/lock paths to db_shim repos.
flowctl/crates/flowctl-cli/src/commands/mod.rs Adds the new db_shim module.
flowctl/crates/flowctl-cli/src/commands/mcp.rs Updates MCP direct service calls to libSQL + async service functions.
flowctl/crates/flowctl-cli/src/commands/epic.rs Switches epic/task DB paths to db_shim.
flowctl/crates/flowctl-cli/src/commands/dep.rs Replaces raw SQL dep writes with db_shim DepRepo.
flowctl/crates/flowctl-cli/src/commands/db_shim.rs New sync shim that adapts async flowctl-db-lsql APIs to sync CLI usage.
flowctl/crates/flowctl-cli/src/commands/checkpoint.rs Switches state/db path resolution to db_shim.
flowctl/crates/flowctl-cli/src/commands/admin/status.rs Switches status/doctor DB paths to db_shim.
flowctl/crates/flowctl-cli/src/commands/admin/exchange.rs Switches import/export/reindex path to db_shim.
flowctl/crates/flowctl-cli/src/commands/admin/config.rs Switches state dir resolution to db_shim.
flowctl/crates/flowctl-cli/Cargo.toml Replaces rusqlite/flowctl-db deps with libSQL/flowctl-db-lsql; makes tokio non-optional.
flowctl/Cargo.toml Swaps workspace members/deps to flowctl-db-lsql; adds libSQL/fastembed workspace deps.
flowctl/.gitignore Ignores .fastembed_cache/ directories.
CLAUDE.md Documents libSQL-only storage runtime and fastembed cache behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +33 to 41
/// Open a new libSQL connection asynchronously.
pub async fn connect(&self) -> ServiceResult<Connection> {
let db = flowctl_db_lsql::open_async(&self.working_dir)
.await
.map_err(ServiceError::from)?;
db.connect().map_err(|e| {
ServiceError::DbError(flowctl_db_lsql::DbError::LibSql(e))
})
}
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flowctl_db_lsql::open_async() applies PRAGMAs on the connection it opens internally, but this method then calls db.connect() to create a new connection and returns it. That returned connection will miss per-connection PRAGMAs like busy_timeout / foreign_keys, contradicting the pool’s documented behavior. Consider adding an API that returns a fully-initialized Connection (or exposing an apply_pragmas() helper) and use that here instead of db.connect() directly.

Copilot uses AI. Check for mistakes.
Comment on lines +44 to +51
/// Open a connection asynchronously (convenience wrapper around
/// `flowctl_db_lsql::open_async`).
pub async fn open_async(working_dir: &Path) -> ServiceResult<Connection> {
let db = flowctl_db_lsql::open_async(working_dir)
.await
.map_err(ServiceError::from)?;
db.connect()
.map_err(|e| ServiceError::DbError(flowctl_db_lsql::DbError::LibSql(e)))
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as in the provider: open_async() returns a Database with schema/pragmas applied on an internal connection, but then db.connect() creates a new connection that won’t have per-connection PRAGMAs set. It would be safer to return a connection that has had the PRAGMAs applied, or add a flowctl_db_lsql::connect_with_pragmas() helper and use that here.

Copilot uses AI. Check for mistakes.
Comment on lines +21 to 36
pub async fn create_state(runtime: DaemonRuntime, event_bus: flowctl_scheduler::EventBus) -> Result<(AppState, tokio_util::sync::CancellationToken)> {
// Derive the project root from .flow/.state/ → parent of .flow/
let working_dir = runtime.paths.state_dir
.parent() // .flow/
.and_then(|p| p.parent()) // project root
.context("cannot resolve project root from state_dir")?;
let conn = flowctl_db::open(working_dir)
let db = flowctl_db_lsql::open_async(working_dir)
.await
.with_context(|| format!("failed to open db in {}", working_dir.display()))?;
let conn = db.connect().context("failed to connect to libsql db")?;
let cancel = runtime.cancel.clone();
let state = Arc::new(DaemonState {
runtime,
event_bus,
db: Arc::new(std::sync::Mutex::new(conn)),
db: conn,
});
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flowctl_db_lsql::open_async() applies schema + PRAGMAs using a connection created inside open_async(), but create_state then calls db.connect() and stores that new connection in state. That new connection will miss per-connection PRAGMAs (e.g., busy_timeout, foreign_keys). Prefer an API that returns a fully-initialized Connection, or re-apply PRAGMAs after db.connect() (ideally via a shared helper).

Copilot uses AI. Check for mistakes.
Comment on lines +47 to +58
/// Try to open a libSQL async DB connection (for service-layer calls).
pub(crate) fn try_open_lsql_conn() -> Option<libsql::Connection> {
let cwd = env::current_dir().ok()?;
let rt = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.ok()?;
rt.block_on(async {
let db = flowctl_db_lsql::open_async(&cwd).await.ok()?;
db.connect().ok()
})
}
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper opens the DB via flowctl_db_lsql::open_async() and then calls db.connect(); the returned connection will not have per-connection PRAGMAs applied (since PRAGMAs were applied to the connection created inside open_async()). Consider switching to a flowctl_db_lsql helper that returns a ready-to-use Connection (or re-applying PRAGMAs) so CLI service calls run with the same settings as the rest of the app.

Copilot uses AI. Check for mistakes.
Comment on lines +53 to +61
pub fn open(working_dir: &Path) -> Result<Connection, DbError> {
block_on(async {
let db = flowctl_db_lsql::open_async(working_dir).await?;
let conn = db.connect()?;
// Leak the Database handle to keep it alive for the process lifetime.
// (libsql Database drop closes the file.)
std::mem::forget(db);
Ok(Connection { conn })
})
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open() intentionally leaks the Database handle via std::mem::forget(db), which is a permanent memory leak for the CLI process. If a live Database is actually required to keep the connection usable, store it inside the Connection wrapper (e.g., struct Connection { _db: Database, conn: Connection }) rather than leaking; if it’s not required, drop the forget and the comment (and consider adding a regression test that the connection remains usable after db is dropped).

Copilot uses AI. Check for mistakes.
Comment on lines +147 to +156
pub async fn open_memory_async() -> Result<(Database, Connection), DbError> {
let db = Builder::new_local(":memory:")
.build()
.await
.map_err(|e| DbError::Schema(format!("libsql open_memory: {e}")))?;

let conn = db.connect()?;
apply_pragmas(&conn).await.ok();
apply_schema(&conn).await?;

Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open_memory_async() ignores errors from apply_pragmas() (.await.ok()), which can mask real failures (e.g., PRAGMA not supported, permission issues) and lead to tests passing with a misconfigured connection. Unless there’s a known benign failure mode here, it’s safer to propagate the error (like open_async() does) or log explicitly when PRAGMA application fails.

Copilot uses AI. Check for mistakes.
Comment on lines +418 to +425
pub async fn start_task(
conn: Option<&Connection>,
flow_dir: &Path,
req: StartTaskRequest,
) -> ServiceResult<StartTaskResponse> {
validate_task_id(&req.task_id)?;

let task = load_task(conn, flow_dir, &req.task_id).ok_or_else(|| {
let task = load_task(conn, flow_dir, &req.task_id).await.ok_or_else(|| {
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lifecycle entrypoints are now async, but they still perform blocking filesystem I/O via std::fs (e.g., markdown reads/writes and read_dir in the helper paths). When called from the daemon, this can block Tokio worker threads and degrade API responsiveness under load. Consider moving file I/O to tokio::fs (or isolating it behind spawn_blocking) so the async service layer remains non-blocking.

Copilot uses AI. Check for mistakes.
Comment on lines +95 to +114
/// Open a file-backed libSQL database with schema applied.
pub async fn open_async(working_dir: &Path) -> Result<Database, DbError> {
let db_path = resolve_libsql_path(working_dir)?;

if let Some(parent) = db_path.parent() {
std::fs::create_dir_all(parent).map_err(|e| {
DbError::StateDir(format!("failed to create {}: {e}", parent.display()))
})?;
}

let db = Builder::new_local(&db_path)
.build()
.await
.map_err(|e| DbError::Schema(format!("libsql open: {e}")))?;

let conn = db.connect()?;
apply_pragmas(&conn).await?;
apply_schema(&conn).await?;

Ok(db)
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open_async() applies PRAGMAs on a connection it creates internally, but then returns only the Database. Most call sites then do db.connect() and end up with a new connection that never had PRAGMAs applied. Consider changing this API (or adding a sibling like open_connection_async()) to return a fully-initialized Connection (and optionally the Database when the handle must be kept alive), so callers can’t accidentally bypass the per-connection setup.

Copilot uses AI. Check for mistakes.
z23cc added a commit that referenced this pull request Apr 9, 2026
P0 fixes (state loss — root cause of 5 issues):
- get_flow_dir() now walks up directory tree (FLOW_STATE_DIR env → walk-up → CWD)
  Fixes: #1 state loss, #3 state not persistent, #5 worker parallel fail,
  #9 .flow symlink issues. Same pattern as git finding .git.
- flowctl recover --epic <id> [--dry-run]: rebuilds task completion status
  from git log. Fixes #11 no recovery after state loss.

P1 fixes (guard + review):
- Guard graceful fallback: missing tools → "skipped" (not "failed").
  Only actual failures block pipeline. Fixes #8.
- Review-backend availability check: if rp-cli/codex not in PATH,
  auto-fallback to "none" with warning. Fixes #7.

P2 fixes (UX):
- Slug max length 40→20 chars. "Django+React platform with account
  management" → "fn-3-django-react-plat" not 40-char monster. Fixes #2 #12.
- Brainstorm auto-skip: trivial tasks (≤10 words, contains "fix"/"typo"/etc)
  skip brainstorm entirely. Fixes #6.
- --interactive flag: pause at key decisions. Fixes #10.

370 tests pass. Zero new dependencies.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants