M2: Build a real Contacts (CardDAV) engine, replacing the fabricated client #187

New Issue

2026-07-28T14:04:23Z

KofTwentyTwo commented

2026-07-28 14:04:23 +00:00

(Migrated from github.com)

Context

Story for milestone M2. Read docs/STORY-WORKFLOW.md first — it defines the process, gates, and patterns; this issue defines only the specific work.

crates/nuncio-contacts is genuinely fake at its core: CardDavClient::fetch_remote_vcards (crates/nuncio-contacts/src/carddav.rs:40-48) never makes a network call — it always returns exactly one hardcoded Contact::new("Google CardDAV Sync Contact", "carddav.sync@nuncio.mx"), no matter what server is configured. There is no vCard parser anywhere in the crate — only a generator (Contact::to_vcard, crates/nuncio-contacts/src/models.rs:74-104). There is no ContactsBackend trait or mock, unlike the equivalent nuncio_cal::CalendarBackend/MockCalendarBackend pair. The crate also isn't opted into the workspace's lint gate: crates/nuncio-contacts/Cargo.toml has no [lints] section at all (every other crate has [lints]\nworkspace = true), and lib.rs has no #![forbid(unsafe_code)].

Separately, nuncio-contacts::db::ContactsDatabase (crates/nuncio-contacts/src/db.rs) is a genuinely real, working SQLite store with FTS5 search — it is NOT fake — but it is its own disconnected SQLite pool, entirely separate from the daemon's single owned store (nuncio_store::db::DatabaseEngine, which already has real, tested calendar CRUD to mirror). nunciod (the daemon binary) does not depend on nuncio-contacts at all today. The CLI's Commands::Contact handlers (crates/nuncio-cli/src/runner.rs:371-434) only ever call ContactsDatabase::in_memory() — a fresh, throwaway in-memory database on every single CLI invocation — so nothing a user adds via nuncio contact add survives past that one process. That is the "nothing persists" problem this story must fix, per the architecture principle in CLAUDE.md that nunciod owns all state: persistence must move onto the daemon's single store, not just switch ContactsDatabase from in_memory() to a file path (that would still be a second, disconnected store).

Goal

Make nuncio-contacts a real CardDAV engine, mirroring nuncio-cal's shape exactly: a real reqwest-based CardDAV REPORT client (RFC 6352 addressbook-query/addressbook-multiget), a genuine vCard (RFC 6350) parser (not just the existing generator), a ContactsBackend trait + MockContactsBackend, and persistence through the daemon's own store (nuncio-store::db::DatabaseEngine) rather than nuncio-contacts's separate SQLite pool. This story does not wire anything into nunciod/gRPC/the CLI (that is M2-S2) — it is engine-only, exactly like nuncio-cal was fully real and tested before nunciod ever touched it. No fabricated contacts anywhere: a CardDAV failure must be a real, typed error, never a canned success.

Design decision (this issue makes the call so you don't have to guess)

nuncio-contacts::models::Contact (rich: given_name/family_name/organization/job_title/emails: Vec<ContactEmail>/phones: Vec<ContactPhone>) is the domain entity this story builds around — the same role nuncio_core::model::CalendarEvent plays for Calendar. Do not migrate onto nuncio_core::model::Contact (the sparse id/name/email/phone type in crates/nuncio-core/src/model.rs:125-135): that type has zero callers anywhere in the tree today (it's dead/orphaned) and lacks the fields needed for real vCard round-trip fidelity. Leave it untouched; it is out of scope.

Files to touch / patterns to mirror

crates/nuncio-contacts/Cargo.toml — add [lints]\nworkspace = true (mirror every other crate, e.g. crates/nuncio-cal/Cargo.toml:9-10). Add nuncio-store = { path = "../nuncio-store" } as a new dependency (needed for the store-CRUD step below).
crates/nuncio-contacts/src/lib.rs — add #![forbid(unsafe_code)] (mirror crates/nuncio-cal/src/lib.rs:3). Fix whatever the new lint gate surfaces (a scan of src/ today found no production unwrap()/expect()/panic! outside #[cfg(test)], so this should be low-risk, but verify).
crates/nuncio-contacts/src/parser.rs (new file) — a real vCard parser, mirroring nuncio_cal::parser::IcalParserAdapter (crates/nuncio-cal/src/parser.rs) in shape and rigor: parse FN/N/ORG/TITLE/EMAIL;TYPE=.../TEL;TYPE=... back into a Contact, with a real error type (CardDavError or a new VCardError) for malformed input — never a fabricated/zeroed field. Round-tripping a Contact through to_vcard() then this new parser must reproduce an equivalent Contact (this is the story's core "vCard parse→model→regenerate round-trip" proof) — add that exact round-trip test.
crates/nuncio-contacts/src/backend.rs (new file) — ContactsBackend trait mirroring nuncio_cal::CalendarBackend (crates/nuncio-cal/src/backend.rs) exactly: #[async_trait] pub trait ContactsBackend: Send + Sync { async fn fetch_contacts(&self, account_id: &str) -> Result<Vec<Contact>, CardDavError>; }.
crates/nuncio-contacts/src/carddav.rs — replace fetch_remote_vcards's hardcoded return (lines 40-48) with a real CardDAV REPORT request, mirroring nuncio_cal::CalDavClient::fetch_remote_events (crates/nuncio-cal/src/caldav.rs:116-157) almost exactly: build an addressbook-query/addressbook-multiget XML body (mirror CalDavClient::build_report_query, lines 47-66, adapted to urn:ietf:params:xml:ns:carddav and <card:address-data/>), issue it via reqwest with Depth: 1 + basic auth, accept 207/2xx, extract each <card:address-data> block from the multistatus response, and parse each through the new vCard parser. Expand CardDavError (currently just Http/SyncError, crates/nuncio-contacts/src/carddav.rs:7-13) with ParseFailed/TransportFailed variants mirroring nuncio_cal::CalendarError (crates/nuncio-cal/src/parser.rs:11-29), so a transport failure and a parse failure are distinguishable and neither is ever silently swallowed into a fabricated empty/hardcoded result. Implement ContactsBackend for CardDavClient.
crates/nuncio-contacts/src/mock.rs (new file) — MockContactsBackend, mirroring nuncio_cal::mock::MockCalendarBackend (crates/nuncio-cal/src/mock.rs) exactly: in-memory Vec<Contact> behind a Mutex, add_contact, set_should_fail, implementing ContactsBackend.
crates/nuncio-contacts/tests/wiremock_carddav_test.rs (new file) — mirror crates/nuncio-cal/tests/wiremock_caldav_test.rs exactly: a wiremock server returning a canned multistatus response with 1-2 real vCards, driven through CardDavClient::fetch_remote_vcards/ContactsBackend::fetch_contacts, asserting the parsed fields; plus a server-error-surfaces-as-transport-failure test mirroring wiremock_caldav_report_server_error_surfaces_as_transport_failure (lines 95-119). Add wiremock as a dev-dependency in Cargo.toml if not already present (it is not, today).
crates/nuncio-store/src/db.rs — add contact CRUD mirroring the calendar CRUD shape exactly (save_calendar_event/list_calendar_events/get_calendar_event, lines 918-1035): save_contact, list_contacts, get_contact, operating on nuncio_contacts::Contact. Add a contacts table + FTS mirror in DatabaseEngine::migrate (crates/nuncio-store/src/db.rs:348+), mirroring the contacts/contacts_fts schema already proven to work in crates/nuncio-contacts/src/db.rs:37-73 (JSON columns for emails/phones, trigram FTS). This is the one deliberate exception to "the store only depends on nuncio-core": nuncio-store gains a narrow, explicit dependency on nuncio-contacts::Contact here because that type is not (yet) promoted to nuncio-core the way CalendarEvent/Email are — call this out plainly in a doc comment on the new methods so it isn't mistaken for an accident.
crates/nuncio-contacts/src/db.rs — leave ContactsDatabase as-is (do not delete it or its tests); it becomes dead code from the daemon's perspective after this story but the CLI's existing Commands::Contact handlers (runner.rs:371-434) still use it directly and are explicitly out of scope for this story (they get rewired onto the new store + a Contacts gRPC service in M2-S2, the same way mail sync/mail list moved off the CLI's own ephemeral DB onto connect_mail_client in the Mail vertical). Note this as a known, intentional gap, not a regression, mirroring the comment style at runner.rs:453-464.

Implementation steps

Add [lints]/#![forbid(unsafe_code)]; fix any resulting violations.
Write the vCard parser + its round-trip test against the existing to_vcard().
Add ContactsBackend + MockContactsBackend.
Rewrite CardDavClient::fetch_remote_vcards as a real REPORT client implementing ContactsBackend; add the wiremock tests.
Add contact CRUD to nuncio-store::db::DatabaseEngine + its migration; test directly against an ephemeral DatabaseEngine.
Run cargo verify.

Acceptance criteria

CardDavClient::fetch_remote_vcards performs a real HTTP REPORT request and parses a real multistatus/vCard response; the old hardcoded "Google CardDAV Sync Contact" return is gone.
A real vCard parser exists and round-trips through Contact::to_vcard() losslessly for at least FN/N/ORG/TITLE/primary EMAIL/TEL, proven by a test.
ContactsBackend trait + MockContactsBackend exist, mirroring CalendarBackend/MockCalendarBackend.
A CardDAV transport failure and a malformed-vCard parse failure are distinguishable typed errors, never a fabricated empty/hardcoded success.
nuncio-store::db::DatabaseEngine gains real, tested save_contact/list_contacts/get_contact backed by a genuine SQLite table (not nuncio-contacts's separate in-process pool).
crates/nuncio-contacts/Cargo.toml has [lints]\nworkspace = true; lib.rs has #![forbid(unsafe_code)].
Offline E2E (wiremock) proves the real CardDAV REPORT → parse → ContactsBackend path end to end; no live network.
cargo verify green (fmt + clippy -D warnings all-targets + test-all), 0 ignored tests.

Definition of done & process

Follow docs/STORY-WORKFLOW.md: no fabricated data; comment convention (intent-only, no story/GH/phase refs in .rs/.proto); mock-only tests. Branch story/m2-contacts-engine off dev; PR to dev with the gate output pasted as evidence; the PO reviews before the issue is closed.

Dependencies / notes

Not a [PROTO] story — no proto changes, and it does not touch nunciod or the CLI. It can run in parallel with any proto-touching story.
M2-S2 (Contacts API vertical) depends on this story: it wires ContactsBackend/MockContactsBackend and the new store CRUD into a real nuncio.v1.Contacts gRPC service, the way M1-S1 wires nuncio-cal into nunciod.
Size: L.

## Context Story for milestone M2. **Read [`docs/STORY-WORKFLOW.md`](https://github.com/KofTwentyTwo/nuncio/blob/dev/docs/STORY-WORKFLOW.md) first** — it defines the process, gates, and patterns; this issue defines only the specific work. `crates/nuncio-contacts` is genuinely fake at its core: `CardDavClient::fetch_remote_vcards` (`crates/nuncio-contacts/src/carddav.rs:40-48`) never makes a network call — it always returns exactly one hardcoded `Contact::new("Google CardDAV Sync Contact", "carddav.sync@nuncio.mx")`, no matter what server is configured. There is no vCard *parser* anywhere in the crate — only a generator (`Contact::to_vcard`, `crates/nuncio-contacts/src/models.rs:74-104`). There is no `ContactsBackend` trait or mock, unlike the equivalent `nuncio_cal::CalendarBackend`/`MockCalendarBackend` pair. The crate also isn't opted into the workspace's lint gate: `crates/nuncio-contacts/Cargo.toml` has no `[lints]` section at all (every other crate has `[lints]\nworkspace = true`), and `lib.rs` has no `#![forbid(unsafe_code)]`. Separately, `nuncio-contacts::db::ContactsDatabase` (`crates/nuncio-contacts/src/db.rs`) is a genuinely real, working SQLite store with FTS5 search — it is NOT fake — but it is its own disconnected SQLite pool, entirely separate from the daemon's single owned store (`nuncio_store::db::DatabaseEngine`, which already has real, tested calendar CRUD to mirror). `nunciod` (the daemon binary) does not depend on `nuncio-contacts` at all today. The CLI's `Commands::Contact` handlers (`crates/nuncio-cli/src/runner.rs:371-434`) only ever call `ContactsDatabase::in_memory()` — a fresh, throwaway in-memory database on every single CLI invocation — so nothing a user adds via `nuncio contact add` survives past that one process. That is the "nothing persists" problem this story must fix, per the architecture principle in `CLAUDE.md` that `nunciod` owns all state: persistence must move onto the daemon's single store, not just switch `ContactsDatabase` from `in_memory()` to a file path (that would still be a second, disconnected store). ## Goal Make `nuncio-contacts` a real CardDAV engine, mirroring `nuncio-cal`'s shape exactly: a real `reqwest`-based CardDAV `REPORT` client (RFC 6352 `addressbook-query`/`addressbook-multiget`), a genuine vCard (RFC 6350) parser (not just the existing generator), a `ContactsBackend` trait + `MockContactsBackend`, and persistence through the daemon's own store (`nuncio-store::db::DatabaseEngine`) rather than `nuncio-contacts`'s separate SQLite pool. This story does **not** wire anything into `nunciod`/gRPC/the CLI (that is **M2-S2**) — it is engine-only, exactly like `nuncio-cal` was fully real and tested before `nunciod` ever touched it. No fabricated contacts anywhere: a CardDAV failure must be a real, typed error, never a canned success. ## Design decision (this issue makes the call so you don't have to guess) `nuncio-contacts::models::Contact` (rich: `given_name`/`family_name`/`organization`/`job_title`/`emails: Vec<ContactEmail>`/`phones: Vec<ContactPhone>`) is the domain entity this story builds around — the same role `nuncio_core::model::CalendarEvent` plays for Calendar. Do **not** migrate onto `nuncio_core::model::Contact` (the sparse `id`/`name`/`email`/`phone` type in `crates/nuncio-core/src/model.rs:125-135`): that type has zero callers anywhere in the tree today (it's dead/orphaned) and lacks the fields needed for real vCard round-trip fidelity. Leave it untouched; it is out of scope. ## Files to touch / patterns to mirror - `crates/nuncio-contacts/Cargo.toml` — add `[lints]\nworkspace = true` (mirror every other crate, e.g. `crates/nuncio-cal/Cargo.toml:9-10`). Add `nuncio-store = { path = "../nuncio-store" }` as a new dependency (needed for the store-CRUD step below). - `crates/nuncio-contacts/src/lib.rs` — add `#![forbid(unsafe_code)]` (mirror `crates/nuncio-cal/src/lib.rs:3`). Fix whatever the new lint gate surfaces (a scan of `src/` today found no production `unwrap()`/`expect()`/`panic!` outside `#[cfg(test)]`, so this should be low-risk, but verify). - `crates/nuncio-contacts/src/parser.rs` (**new file**) — a real vCard parser, mirroring `nuncio_cal::parser::IcalParserAdapter` (`crates/nuncio-cal/src/parser.rs`) in shape and rigor: parse `FN`/`N`/`ORG`/`TITLE`/`EMAIL;TYPE=...`/`TEL;TYPE=...` back into a `Contact`, with a real error type (`CardDavError` or a new `VCardError`) for malformed input — never a fabricated/zeroed field. Round-tripping a `Contact` through `to_vcard()` then this new parser must reproduce an equivalent `Contact` (this is the story's core "vCard parse→model→regenerate round-trip" proof) — add that exact round-trip test. - `crates/nuncio-contacts/src/backend.rs` (**new file**) — `ContactsBackend` trait mirroring `nuncio_cal::CalendarBackend` (`crates/nuncio-cal/src/backend.rs`) exactly: `#[async_trait] pub trait ContactsBackend: Send + Sync { async fn fetch_contacts(&self, account_id: &str) -> Result<Vec<Contact>, CardDavError>; }`. - `crates/nuncio-contacts/src/carddav.rs` — replace `fetch_remote_vcards`'s hardcoded return (lines 40-48) with a real CardDAV `REPORT` request, mirroring `nuncio_cal::CalDavClient::fetch_remote_events` (`crates/nuncio-cal/src/caldav.rs:116-157`) almost exactly: build an `addressbook-query`/`addressbook-multiget` XML body (mirror `CalDavClient::build_report_query`, lines 47-66, adapted to `urn:ietf:params:xml:ns:carddav` and `<card:address-data/>`), issue it via `reqwest` with `Depth: 1` + basic auth, accept `207`/2xx, extract each `<card:address-data>` block from the multistatus response, and parse each through the new vCard parser. Expand `CardDavError` (currently just `Http`/`SyncError`, `crates/nuncio-contacts/src/carddav.rs:7-13`) with `ParseFailed`/`TransportFailed` variants mirroring `nuncio_cal::CalendarError` (`crates/nuncio-cal/src/parser.rs:11-29`), so a transport failure and a parse failure are distinguishable and neither is ever silently swallowed into a fabricated empty/hardcoded result. Implement `ContactsBackend` for `CardDavClient`. - `crates/nuncio-contacts/src/mock.rs` (**new file**) — `MockContactsBackend`, mirroring `nuncio_cal::mock::MockCalendarBackend` (`crates/nuncio-cal/src/mock.rs`) exactly: in-memory `Vec<Contact>` behind a `Mutex`, `add_contact`, `set_should_fail`, implementing `ContactsBackend`. - `crates/nuncio-contacts/tests/wiremock_carddav_test.rs` (**new file**) — mirror `crates/nuncio-cal/tests/wiremock_caldav_test.rs` exactly: a `wiremock` server returning a canned multistatus response with 1-2 real vCards, driven through `CardDavClient::fetch_remote_vcards`/`ContactsBackend::fetch_contacts`, asserting the parsed fields; plus a server-error-surfaces-as-transport-failure test mirroring `wiremock_caldav_report_server_error_surfaces_as_transport_failure` (lines 95-119). Add `wiremock` as a dev-dependency in `Cargo.toml` if not already present (it is not, today). - `crates/nuncio-store/src/db.rs` — add contact CRUD mirroring the calendar CRUD shape exactly (`save_calendar_event`/`list_calendar_events`/`get_calendar_event`, lines 918-1035): `save_contact`, `list_contacts`, `get_contact`, operating on `nuncio_contacts::Contact`. Add a `contacts` table + FTS mirror in `DatabaseEngine::migrate` (`crates/nuncio-store/src/db.rs:348+`), mirroring the `contacts`/`contacts_fts` schema already proven to work in `crates/nuncio-contacts/src/db.rs:37-73` (JSON columns for `emails`/`phones`, trigram FTS). This is the one deliberate exception to "the store only depends on `nuncio-core`": `nuncio-store` gains a narrow, explicit dependency on `nuncio-contacts::Contact` here because that type is not (yet) promoted to `nuncio-core` the way `CalendarEvent`/`Email` are — call this out plainly in a doc comment on the new methods so it isn't mistaken for an accident. - `crates/nuncio-contacts/src/db.rs` — leave `ContactsDatabase` as-is (do not delete it or its tests); it becomes dead code from the daemon's perspective after this story but the CLI's existing `Commands::Contact` handlers (`runner.rs:371-434`) still use it directly and are explicitly out of scope for this story (they get rewired onto the new store + a `Contacts` gRPC service in **M2-S2**, the same way `mail sync`/`mail list` moved off the CLI's own ephemeral DB onto `connect_mail_client` in the Mail vertical). Note this as a known, intentional gap, not a regression, mirroring the comment style at `runner.rs:453-464`. ## Implementation steps 1. Add `[lints]`/`#![forbid(unsafe_code)]`; fix any resulting violations. 2. Write the vCard parser + its round-trip test against the existing `to_vcard()`. 3. Add `ContactsBackend` + `MockContactsBackend`. 4. Rewrite `CardDavClient::fetch_remote_vcards` as a real REPORT client implementing `ContactsBackend`; add the wiremock tests. 5. Add contact CRUD to `nuncio-store::db::DatabaseEngine` + its migration; test directly against an ephemeral `DatabaseEngine`. 6. Run `cargo verify`. ## Acceptance criteria - [ ] `CardDavClient::fetch_remote_vcards` performs a real HTTP `REPORT` request and parses a real multistatus/vCard response; the old hardcoded "Google CardDAV Sync Contact" return is gone. - [ ] A real vCard parser exists and round-trips through `Contact::to_vcard()` losslessly for at least `FN`/`N`/`ORG`/`TITLE`/primary `EMAIL`/`TEL`, proven by a test. - [ ] `ContactsBackend` trait + `MockContactsBackend` exist, mirroring `CalendarBackend`/`MockCalendarBackend`. - [ ] A CardDAV transport failure and a malformed-vCard parse failure are distinguishable typed errors, never a fabricated empty/hardcoded success. - [ ] `nuncio-store::db::DatabaseEngine` gains real, tested `save_contact`/`list_contacts`/`get_contact` backed by a genuine SQLite table (not `nuncio-contacts`'s separate in-process pool). - [ ] `crates/nuncio-contacts/Cargo.toml` has `[lints]\nworkspace = true`; `lib.rs` has `#![forbid(unsafe_code)]`. - [ ] Offline E2E (wiremock) proves the real CardDAV REPORT → parse → `ContactsBackend` path end to end; no live network. - [ ] `cargo verify` green (fmt + clippy `-D warnings` all-targets + test-all), 0 ignored tests. ## Definition of done & process Follow [`docs/STORY-WORKFLOW.md`](https://github.com/KofTwentyTwo/nuncio/blob/dev/docs/STORY-WORKFLOW.md): no fabricated data; comment convention (intent-only, no story/GH/phase refs in `.rs`/`.proto`); mock-only tests. Branch `story/m2-contacts-engine` off `dev`; PR to `dev` with the gate output pasted as evidence; the PO reviews before the issue is closed. ## Dependencies / notes - Not a `[PROTO]` story — no proto changes, and it does not touch `nunciod` or the CLI. It can run in parallel with any proto-touching story. - **M2-S2** (Contacts API vertical) depends on this story: it wires `ContactsBackend`/`MockContactsBackend` and the new store CRUD into a real `nuncio.v1.Contacts` gRPC service, the way **M1-S1** wires `nuncio-cal` into `nunciod`. - Size: L.

KofTwentyTwo commented

2026-07-29 22:53:57 +00:00

(Migrated from github.com)

Delivered via PR #217 (merged to dev). Review PASS — real CardDAV client + vCard round-trip + ContactsBackend + persistent store CRUD; FTS triggers correct, no fabrication, no dependency cycle. Known minor (non-blocking): is_primary fidelity for multi-email/phone contacts isn't preserved on round-trip (to_vcard emits no PREF); primary email/tel fidelity holds. Fold into a future contacts refinement if needed.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#187