Skip to content

fix(contacts): add view action, show phones in list, preserve fields on update#5748

Open
holden093 wants to merge 2 commits into
odysseus-dev:devfrom
holden093:fix/manage-contact-list-phones-reopen
Open

fix(contacts): add view action, show phones in list, preserve fields on update#5748
holden093 wants to merge 2 commits into
odysseus-dev:devfrom
holden093:fix/manage-contact-list-phones-reopen

Conversation

@holden093

Copy link
Copy Markdown
Contributor

Note: Re-opening of #4748, auto-closed 2026-07-23 during the odysseus-dev org transfer (the dev history rewrite force-closed all open PRs). Same change, cleanly rebased onto the current dev. Original review thread: #4748.

Summary

Fixes three gaps in the manage_contacts agent tool that made phone-number workflows unreliable: list now includes phone numbers, a new view action returns full contact details by UID, and update preserves existing emails/phones when the caller doesn't pass them (preventing data loss on partial edits like renaming).

Target branch

  • This PR targets dev, not main.

Linked Issue

Fixes #4747

Type of Change

  • Bug fix (non-breaking — fixes a confirmed issue)

Checklist

  • I searched open issues and open PRs — this is not a duplicate.
  • This PR targets dev
  • My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
  • I actually ran the app (docker compose up or uvicorn app:app) and verified the change works end-to-end. Type-checks and unit tests are not enough.

How to Test

  1. Add a contact with name, email, and phone number via the UI or CardDAV.
  2. Ask the agent to "list my contacts" — verify phone numbers appear in the output.
  3. Ask the agent to "view contact [name]" — verify full details (phones, address, UID) are shown.
  4. Ask the agent to "rename [contact] to [new name]" — verify the phone and email are preserved after the update.

Visual / UI changes

None — backend-only change.

…on update

- Add 'view' action to manage_contact: fetch a single contact by UID and
  display name, UID, emails, phones, and address.
- Show phone numbers in 'list' output alongside emails.
- Preserve existing emails/phones during partial update (rename-only,
  single-field updates) by fetching the current contact when either
  field is missing. Uses force=True on _fetch_contacts to avoid a stale
  cache masking a just-added contact.
- Fix the preserve guard: use OR instead of AND so single-field updates
  like {uid, emails:[new]} don't wipe phones and vice versa.
@github-actions github-actions Bot added the ready for review Description complete — ready for maintainer review label Jul 25, 2026
@holden093

Copy link
Copy Markdown
Contributor Author

Heads-up: the failing Python tests (pytest) check is a pre-existing dev bug, not this PR

The pytest job fails at test collection, before any of this PR's changes are exercised:

NameError: name 'Any' is not defined

Root cause is on dev itself: src/agent_loop.py annotates with dict[str, Any] (lines 1503 and 1877) but only imports AsyncGenerator, List, Dict, Optional, Set — no Any, and no from __future__ import annotations. So importing the module raises NameError, which breaks collection for every test that transitively imports agent_loop (hence the repo-wide failure).

This PR doesn't touch agent_loop.py; every other check (syntax, CodeQL, lint, security scans) passes here, and this PR's own tests pass locally.

One-line fix (add Any to the typing import) is included in #5751. Once dev carries that fix, this PR's pytest will go green with no changes needed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Description complete — ready for maintainer review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(contacts): manage_contacts tool hides phone numbers and has no view action

1 participant