feat: add local contacts and vCard support (#81) - #89
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
pebble | 52350da | Commit Preview URL Branch Preview URL |
Aug 10 2026, 05:13 AM |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 372b30e184
ℹ️ 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".
| loop { | ||
| let page = self.list_contacts(None, false, 200, contacts.len() as i64)?; | ||
| let page_len = page.len(); | ||
| contacts.extend(page); |
There was a problem hiding this comment.
When a profile has more than 200 contacts and a contact is added, deleted, or renamed while an automatic or manual backup is paging, each list_contacts call runs as an independent read. An edit that moves rows before the current offset can therefore skip a contact or duplicate one in the backup; the latter can make the entire restore fail on the duplicate contact ID. Load all contact IDs through one read transaction/snapshot rather than advancing an offset across separate reads.
Useful? React with 👍 / 👎.
| let card_count = unfolded | ||
| .iter() | ||
| .filter(|line| line.eq_ignore_ascii_case("BEGIN:VCARD")) | ||
| .count(); |
There was a problem hiding this comment.
For an empty, unrelated, or malformed file with no BEGIN:VCARD, card_count is zero and this function returns an empty successful result. The UI consequently reports “vCard import complete” with zero invalid records, so users cannot distinguish a valid empty operation from selecting the wrong or corrupted file. Return a validation error when no card marker is found.
Useful? React with 👍 / 👎.
| WHERE (?1 = '' OR c.display_name LIKE ?2 ESCAPE '\\' COLLATE NOCASE | ||
| OR EXISTS ( | ||
| SELECT 1 FROM contact_emails ce | ||
| WHERE ce.contact_id = c.id | ||
| AND ce.address LIKE ?2 ESCAPE '\\' COLLATE NOCASE |
There was a problem hiding this comment.
SQLite's built-in NOCASE collation only folds ASCII characters, so a saved contact named Élodie is not returned for the natural lowercase query élodie. The same SQL pattern is used for saved compose suggestions, causing international contacts to disappear or be demoted to recent-history results. Normalize searchable values with Unicode-aware case folding, or perform the comparison through a Unicode-aware collation.
Useful? React with 👍 / 👎.
| if (notes.trim().length > 2000) { | ||
| return t("contacts.notesTooLong", "Notes must be 2000 characters or fewer"); |
There was a problem hiding this comment.
JavaScript length and the textarea's maxLength count UTF-16 code units, while the backend enforces the limit with Rust chars().count(). Consequently a backend-valid note containing 1,001–2,000 emoji or other supplementary characters cannot be entered or resaved in this dialog; this also makes an imported contact with such a note uneditable until text is removed. Apply the same character-count definition as the backend and avoid the conflicting native maxLength limit.
Useful? React with 👍 / 👎.
| if version.as_deref().is_some_and(|value| value != "3.0") { | ||
| return Err("Only vCard 3.0 is supported".to_string()); |
There was a problem hiding this comment.
A card with no VERSION property bypasses this condition and is imported as though it were supported vCard 3.0. Because VERSION is required and determines the syntax and encoding rules, a malformed or version-stripped card can be silently interpreted incorrectly instead of appearing in the partial-error report. Reject None as well as versions other than 3.0.
Useful? React with 👍 / 👎.
| || domain.starts_with('.') | ||
| || domain.ends_with('.') | ||
| || !domain.contains('.') |
There was a problem hiding this comment.
Addresses such as user@example..com pass both the frontend regex and this backend validation because the domain merely has to contain a dot and not start or end with one. Such a contact can therefore be saved or imported and offered by recipient autocomplete even though the address has an invalid empty domain label, causing failure only when the user tries to send mail. Validate each domain label rather than only the outer dots.
Useful? React with 👍 / 👎.
| const handleKeyDown = (event: KeyboardEvent) => { | ||
| if (event.key === "Escape") onClose(); | ||
| }; |
There was a problem hiding this comment.
When a save is slow, the close button and backdrop correctly prevent dismissal via isSaving, but this document-level Escape handler still closes the dialog. If the pending save then fails, the dialog that would display the error has already unmounted and the user's entered contact data is lost without feedback. Ignore Escape while saving, consistently with the other dismissal paths.
Useful? React with 👍 / 👎.
1 participant
Summary
Independent review
An independent sub-agent review found five issues around backup safety, pagination, accessibility, and quoted vCard parameters. All five were fixed with regression coverage. The final re-review reported no Critical, Important, or Minor findings.
Verification
cargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warningspnpm test -- --reporter=dot— 94 files, 360 tests passedpnpm build:frontendcargo test --workspace --exclude pebble-oauthgit diff --checkThe existing
pebble-oauthtesttoken_exchange_without_secret_uses_public_client_requeststill fails on the base branch. This PR does not modifycrates/pebble-oauth.Resolves #81