Skip to content

feat(infra): T-205 add LibsqlProjectRepo adapter (closes #73)#97

Merged
mpiton merged 1 commit into
mainfrom
feat/t-205-projects-infra
May 12, 2026
Merged

feat(infra): T-205 add LibsqlProjectRepo adapter (closes #73)#97
mpiton merged 1 commit into
mainfrom
feat/t-205-projects-infra

Conversation

@mpiton
Copy link
Copy Markdown
Owner

@mpiton mpiton commented May 11, 2026

Summary

  • Wire LibsqlProjectRepo for the ProjectRepository port from T-204, mirroring LibsqlTaskRepo (per-call connection, PRAGMA foreign_keys = ON, INSERT … RETURNING id, epoch-ms round-trip).
  • Map UNIQUE constraint failed: projects.pathDomainError::Conflict; all other libsql errors → DomainError::IllegalState.
  • Expose project_repo: Arc<dyn ProjectRepository> on AppContainer via application/di.rs.

Why

T-204 landed the projects domain context with a port doc that mandates Conflict on a UNIQUE violation. T-205 is the libsql adapter that fulfils that contract so the in-process pre-check in create_project and the storage-layer enforcement converge on the same DomainError variant.

Contract note (issue text vs. port doc)

Issue #73 acceptance text says the UNIQUE violation should map to DomainError::IllegalState. The committed port contract in src-tauri/src/domain/projects/ports.rs (shipped with T-204) explicitly states:

the libsql adapter relies on the UNIQUE constraint on projects.path and surfaces DomainError::Conflict when the constraint fires

I deferred to the port-doc contract — it is the long-term reference future implementers will read, and semantically a duplicate path is a Conflict, not an internal Illegal state. Test asserts Conflict; a companion non_unique_db_errors_still_map_to_illegal_state test (NOT NULL violation on projects.name) guards the UNIQUE substring match from accidentally widening.

Changes

  • src-tauri/src/infrastructure/persistence/projects_repo.rs — new adapter, 10 integration tests against tempdir libsql DB.
  • src-tauri/src/infrastructure/persistence/mod.rs — register module.
  • src-tauri/src/application/di.rs — wire project_repo on AppContainer + new wiring test (build_inner_exposes_project_repo_backed_by_same_database).
  • CHANGELOG.md — Unreleased/Added entry.

Test plan

  • cargo test --manifest-path src-tauri/Cargo.toml --workspace — 223/223 green
  • cargo clippy --manifest-path src-tauri/Cargo.toml --workspace --all-targets -- -D warnings — clean
  • cargo fmt --manifest-path src-tauri/Cargo.toml --check — clean
  • cargo test --manifest-path src-tauri/Cargo.toml --test architecture — 2/2 pass (no infra imports leaked into domain/)
  • UNIQUE(path) violation surfaces as DomainError::Conflict
  • NOT NULL violation still maps to DomainError::IllegalState
  • get returns None for unknown id
  • AppContainer.project_repo and AppContainer.db share the same Arc<Database>

Summary by CodeRabbit

  • New Features
    • Added project repository infrastructure enabling persistent storage and management of projects with support for creation, listing, and retrieval operations.

Review Change Stack

Wire `LibsqlProjectRepo` for the `ProjectRepository` port introduced by
T-204, mirroring `LibsqlTaskRepo`: per-call connection with `PRAGMA
foreign_keys = ON`, `INSERT … RETURNING id` for the auto-increment
rowid, epoch-ms round-trip via the shared time helpers.

`map_db_err` routes `UNIQUE constraint failed: projects.path` to
`DomainError::Conflict` (honouring the port-doc TOCTOU contract from
`domain/projects/ports.rs`) and folds every other libsql error into
`DomainError::IllegalState`. Issue #73 said `IllegalState` for the
UNIQUE case, but the committed port contract wins as the long-term
reference — the test was updated to assert `Conflict` and a companion
test on a `NOT NULL` violation guards the substring match from
widening into unrelated infrastructure failures.

`AppContainer` now exposes `project_repo: Arc<dyn ProjectRepository>`
wired from the same `Arc<Database>` as `task_repo`. 10 integration
tests against a tempdir libsql DB plus a `di.rs` wiring test that
seeds through `container.db` and reads back through `project_repo`.

cargo test --workspace 223/223 green, clippy/fmt/architecture clean.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 50d435ea-8bbe-48bc-bde2-aee4697db21b

📥 Commits

Reviewing files that changed from the base of the PR and between e27daea and 78d56d5.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • src-tauri/src/application/di.rs
  • src-tauri/src/infrastructure/persistence/mod.rs
  • src-tauri/src/infrastructure/persistence/projects_repo.rs

📝 Walkthrough

Walkthrough

This PR introduces a libsql-backed ProjectRepository adapter that implements CRUD operations (list, get, create) for the projects bounded context, with error mapping from database constraints to domain errors, and wires it into the DI container via AppContainer for application access.

Changes

ProjectRepository Adapter and DI Integration

Layer / File(s) Summary
ProjectRepository Adapter Implementation
src-tauri/src/infrastructure/persistence/projects_repo.rs
LibsqlProjectRepo implements ProjectRepository with SQL constants for list/get/insert, per-call connection creation with foreign key enforcement, async list (ordered by created_at desc), get (optional return by id), create (INSERT RETURNING) methods, error mapping (UNIQUE constraintConflict, others → IllegalState), row-to-project conversion with epoch-ms parsing, and comprehensive test suite validating CRUD, ordering, conflict handling, optional fields, and persistence.
Module Export
src-tauri/src/infrastructure/persistence/mod.rs
Exports projects_repo submodule from persistence layer.
DI Container Wiring
src-tauri/src/application/di.rs
Imports ProjectRepository and LibsqlProjectRepo; adds project_repo field to AppContainer; wires LibsqlProjectRepo::new() from shared Arc<Database> in build_inner; includes async test confirming repo accesses same migrated database and can list seeded projects.
Documentation
CHANGELOG.md
Documents libsql adapter, DI exposure, repository methods, error mapping, integration tests, and verification results.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • mpiton/forgent#73: Main issue tracking the implementation and wiring of LibsqlProjectRepo adapter, repository methods, DI integration, and test coverage for the projects bounded context.

Possibly related PRs

  • mpiton/forgent#50: Earlier PR that introduced the AppContainer composition root and task_repo wiring pattern that is extended here with the new project_repo field and libsql integration.
  • mpiton/forgent#96: Complementary PR that introduces the projects bounded-context domain model and ProjectRepository trait that this PR implements and wires into the DI container.

Poem

🐰 A new repo hops into place,
Projects listed at database pace,
Libsql binds with grace and care,
DI wires it everywhere—
Tests confirm the schemas share! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding the LibsqlProjectRepo adapter infrastructure. It references the task ID (T-205) and the closed issue (#73), directly matching the changeset's primary purpose.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/t-205-projects-infra

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

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Add LibsqlProjectRepo adapter and wire ProjectRepository port (T-205)

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Implement LibsqlProjectRepo adapter for ProjectRepository domain port
• Wire project_repo on AppContainer backed by shared Arc<Database>
• Map UNIQUE constraint violations to DomainError::Conflict per port contract
• Add 10 integration tests + 1 DI wiring test with 100% coverage
Diagram
flowchart LR
  A["ProjectRepository<br/>Domain Port"] -->|"implements"| B["LibsqlProjectRepo<br/>Adapter"]
  B -->|"uses"| C["Arc&lt;Database&gt;<br/>Shared Connection"]
  D["AppContainer"] -->|"exposes"| E["project_repo:<br/>Arc&lt;dyn ProjectRepository&gt;"]
  E -->|"backed by"| C
  F["Error Mapping"] -->|"UNIQUE path<br/>→ Conflict"| G["DomainError"]
  F -->|"other errors<br/>→ IllegalState"| G
Loading

Grey Divider

File Changes

1. src-tauri/src/infrastructure/persistence/projects_repo.rs ✨ Enhancement +340/-0

New libsql adapter for ProjectRepository port

• New 340-line libsql adapter implementing ProjectRepository trait with list(), get(id), and
 create(&Project) methods
• Per-call connection pattern with PRAGMA foreign_keys = ON enforcement matching LibsqlTaskRepo
 design
• Error mapping routes UNIQUE constraint failed: projects.path to DomainError::Conflict and all
 other errors to DomainError::IllegalState
• 10 integration tests covering happy paths, constraint violations, optional field round-trips,
 ordering, and error cases

src-tauri/src/infrastructure/persistence/projects_repo.rs


2. src-tauri/src/infrastructure/persistence/mod.rs ⚙️ Configuration changes +1/-0

Register projects_repo module

• Register new projects_repo module in persistence layer exports

src-tauri/src/infrastructure/persistence/mod.rs


3. src-tauri/src/application/di.rs ✨ Enhancement +44/-0

Wire ProjectRepository on AppContainer

• Add project_repo: Arc<dyn ProjectRepository + Send + Sync> field to AppContainer
• Wire LibsqlProjectRepo in build_inner() using cloned Arc<Database> from pool
• Add build_inner_exposes_project_repo_backed_by_same_database test verifying shared database
 backing

src-tauri/src/application/di.rs


View more (1)
4. CHANGELOG.md 📝 Documentation +1/-0

Document T-205 ProjectRepository adapter implementation

• Document T-205 implementation with comprehensive details on adapter design, error mapping, test
 coverage, and integration with existing infrastructure
• Record UNIQUE constraint to Conflict mapping decision and rationale (port contract precedence
 over issue acceptance text)
• List all 10 integration tests and 1 DI wiring test with their coverage purposes

CHANGELOG.md


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented May 11, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (1)

Context used

Grey Divider


Action required

1. map_db_err returns Conflict 📎 Requirement gap ≡ Correctness
Description
map_db_err in the LibsqlProjectRepo adapter maps the projects.path UNIQUE constraint failure
to DomainError::Conflict even though the compliance checklist requires it to be
DomainError::IllegalState, and the accompanying integration test reinforces the wrong behavior by
asserting Conflict. This violates the adapter contract and can lead to inconsistent
infrastructure-to-domain error semantics.
Code

src-tauri/src/infrastructure/persistence/projects_repo.rs[R120-126]

+fn map_db_err(e: libsql::Error) -> DomainError {
+    let msg = e.to_string();
+    if msg.contains("UNIQUE constraint failed: projects.path") {
+        DomainError::Conflict(format!("project path already registered: {msg}"))
+    } else {
+        DomainError::IllegalState(format!("db: {msg}"))
+    }
Evidence
PR Compliance ID 1 states that libsql::Error in this adapter must be converted to
DomainError::IllegalState, yet the newly added map_db_err function explicitly returns
DomainError::Conflict when the error message contains UNIQUE constraint failed: projects.path.
PR Compliance ID 3 further requires a test that verifies a UNIQUE(path) violation maps to
DomainError::IllegalState, but the added integration test
create_with_duplicate_path_violates_unique_and_maps_to_conflict instead asserts `matches!(err,
DomainError::Conflict(_))`, directly contradicting the required mapping.

Implement LibsqlProjectRepo adapter following LibsqlTaskRepo pattern
src-tauri/src/infrastructure/persistence/projects_repo.rs[120-126]
src-tauri/src/infrastructure/persistence/projects_repo.rs[239-256]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `LibsqlProjectRepo` adapter currently special-cases the `UNIQUE constraint failed: projects.path` database error and maps it to `DomainError::Conflict`, but the compliance checklist requires that `libsql::Error` in this adapter maps to `DomainError::IllegalState`. The integration test for a duplicate `projects.path` violation also asserts `DomainError::Conflict`, which enshrines behavior that contradicts the compliance-required mapping.

## Issue Context
- PR Compliance ID 1 success criteria requires converting `libsql::Error` in this adapter to `DomainError::IllegalState` (i.e., avoid special-case mappings away from `IllegalState`).
- PR Compliance ID 3 success criteria requires a test verifying a `UNIQUE(path)` violation maps to `DomainError::IllegalState`, but the current test asserts `Conflict`.

## Fix Focus Areas
- src-tauri/src/infrastructure/persistence/projects_repo.rs[120-126]
- src-tauri/src/infrastructure/persistence/projects_repo.rs[239-256]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Strict UNIQUE error match 🐞 Bug ☼ Reliability
Description
LibsqlProjectRepo::map_db_err only maps a duplicate path to DomainError::Conflict when the libsql
error string contains the exact substring "UNIQUE constraint failed: projects.path", so any message
formatting change would incorrectly map duplicates to IllegalState and violate the ProjectRepository
contract.
Code

src-tauri/src/infrastructure/persistence/projects_repo.rs[R120-126]

+fn map_db_err(e: libsql::Error) -> DomainError {
+    let msg = e.to_string();
+    if msg.contains("UNIQUE constraint failed: projects.path") {
+        DomainError::Conflict(format!("project path already registered: {msg}"))
+    } else {
+        DomainError::IllegalState(format!("db: {msg}"))
+    }
Evidence
The port contract states implementations must surface DomainError::Conflict when the UNIQUE
constraint on projects.path fires, but the adapter’s discriminator is a single exact substring match
which can fail if the error string format changes.

src-tauri/src/domain/projects/ports.rs[11-28]
src-tauri/src/infrastructure/persistence/projects_repo.rs[114-127]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`map_db_err` relies on a single hard-coded substring (`"UNIQUE constraint failed: projects.path"`) to detect duplicates. If libsql/SQLite ever prefixes/suffixes the message (e.g., adds error codes or schema qualifiers), duplicate-path inserts would map to `DomainError::IllegalState`, violating the domain port contract that requires `DomainError::Conflict`.

## Issue Context
The domain port documentation explicitly requires `Conflict` when the `projects.path` UNIQUE constraint fires.

## Fix Focus Areas
- src-tauri/src/infrastructure/persistence/projects_repo.rs[114-127]
- src-tauri/src/domain/projects/ports.rs[11-28]

## Suggested fix
Update detection to be tolerant to message formatting while still being scoped to this constraint, e.g.:
- `if msg.contains("UNIQUE constraint failed") && msg.contains("projects.path") { ... }`

Optionally add/adjust a unit/integration test that validates mapping still works if the error string contains a prefix/suffix around the SQLite message (guarding against future libsql formatting changes).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

3. Raw DB details sent to IPC 🐞 Bug ⛨ Security
Description
LibsqlProjectRepo::map_db_err embeds the full libsql error string into DomainError, and
AppError::from(DomainError) forwards that Display string to the IPC boundary, exposing internal
database wording/schema details instead of a stable domain message.
Code

src-tauri/src/infrastructure/persistence/projects_repo.rs[R121-126]

+    let msg = e.to_string();
+    if msg.contains("UNIQUE constraint failed: projects.path") {
+        DomainError::Conflict(format!("project path already registered: {msg}"))
+    } else {
+        DomainError::IllegalState(format!("db: {msg}"))
+    }
Evidence
The adapter constructs DomainError strings from libsql::Error::to_string(), and AppError’s
From<DomainError> implementation forwards DomainError’s Display string directly to IPC as
AppError::Domain.

src-tauri/src/infrastructure/persistence/projects_repo.rs[120-126]
src-tauri/src/error.rs[51-55]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`map_db_err` formats `DomainError` values with the raw `libsql::Error` string, and `AppError::from(DomainError)` serializes that string to the frontend. This makes client-visible errors depend on DB engine wording and can expose internal DB details.

## Issue Context
`AppError::from(DomainError)` uses `Display` and the result is serialized as `{ kind: "Domain", detail: "..." }`.

## Fix Focus Areas
- src-tauri/src/infrastructure/persistence/projects_repo.rs[120-126]
- src-tauri/src/error.rs[51-55]

## Suggested fix
- Keep user-facing messages stable and domain-centric (e.g., `Conflict("project path already registered")`).
- For `IllegalState`, avoid embedding raw DB strings in the returned error; instead, emit the raw error via logging/tracing at the infrastructure boundary and return a generic `IllegalState("db error")` (or similarly stable text).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq Bot commented May 11, 2026

Merging this PR will not alter performance

✅ 7 untouched benchmarks


Comparing feat/t-205-projects-infra (78d56d5) with main (e27daea)

Open in CodSpeed

Comment thread src-tauri/src/infrastructure/persistence/projects_repo.rs
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 4 files

@mpiton
Copy link
Copy Markdown
Owner Author

mpiton commented May 11, 2026

@qodo-code-review on finding #2 (Strict UNIQUE error match, projects_repo.rs:120-126)

map_db_err relies on a single hard-coded substring ("UNIQUE constraint failed: projects.path") to detect duplicates. If libsql/SQLite ever prefixes/suffixes the message (e.g., adds error codes or schema qualifiers), duplicate-path inserts would map to DomainError::IllegalState

Fixed in e47e6b9. Discriminator now matches UNIQUE constraint failed and projects.path as two independent substrings:

if msg.contains("UNIQUE constraint failed") && msg.contains("projects.path") {
    DomainError::Conflict(...)
} else {
    DomainError::IllegalState(...)
}

A libsql wrapper-shape change around the SQLite phrase can no longer silently downgrade the duplicate-path case to IllegalState. The existing happy-path test create_with_duplicate_path_violates_unique_and_maps_to_conflict still passes against the real libsql output, and non_unique_db_errors_still_map_to_illegal_state continues to guard the match from widening over unrelated infrastructure failures.

@mpiton
Copy link
Copy Markdown
Owner Author

mpiton commented May 11, 2026

@qodo-code-review on finding #3 (Raw DB details sent to IPC, projects_repo.rs:120-126 + error.rs:51-55)

LibsqlProjectRepo::map_db_err embeds the full libsql error string into DomainError, and AppError::from(DomainError) forwards that Display string to the IPC boundary, exposing internal database wording/schema details

Deferring as out of scope for T-205. The same format!("db: {e}") pattern lives in src-tauri/src/infrastructure/persistence/tasks_repo.rs:149-151 and presumably every other libsql adapter that lands later this sprint (runs_repo, phase_events_repo, …). Redacting the IPC surface needs to be a cross-cutting decision covering:

  1. Which DomainError variants carry stable user-facing strings vs. dev-only strings.
  2. Where the raw libsql text is logged via tracing at the infrastructure boundary so debuggers do not lose it.
  3. How AppError::Domain serialises to the frontend (today: { kind: "Domain", detail: "<Display>" } per error.rs:51-55).

Touching only projects_repo.rs here would create an inconsistency the next adapter would copy back the wrong way. Will file a follow-up issue scoped to all repo adapters + error.rs.

@mpiton
Copy link
Copy Markdown
Owner Author

mpiton commented May 11, 2026

Follow-up for Qodo finding #3 filed as #98.

@mpiton mpiton merged commit 73cf872 into main May 12, 2026
16 checks passed
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.

1 participant