feat(store): add structured person profile primitives

[salmonumbrella]

What changed

• add structured, historized person names, contact points, addresses, dates, categories, media, and keys
• add an open communication-service catalog plus reviewable scoped identity observations and match candidates
• expose typed profile, history, patch, and service-catalog APIs on SQLite and PostgreSQL

Why

Msgvault needs one durable person profile with multiple emails, phone numbers, service-scoped usernames, and source evidence. Independent rows preserve per-value history, provenance, normalized lookup, and stable vCard property identity.

Similar names or contact points create review candidates rather than an irreversible merge. This continues roadmap issue #534.

Usage

Use GET /api/v1/persons/{id}/profile to read the curated profile,
PATCH /api/v1/persons/{id}/profile with the current revision to apply an
atomic typed change, and GET /api/v1/persons/{id}/profile/history to inspect
prior values. Communication services are available under
/api/v1/communication-services.

[roborev-ci]

roborev: Combined Review (c7af818)

Review verdict: Changes requested—one High-severity data-loss bug and two Medium-severity API/data-integrity issues remain.

High

• internal/store/schema.sql:1108 — Participant observations are lost during merges.
  participant_contact_observations cascades on participant deletion, but MergeParticipants deletes the absorbed participant without repointing its observations. Deduplicate and reassign those observations to the surviving participant before deletion, with a regression test covering the merge.

Medium

• internal/store/communication_services.go:209 — New service slugs can shadow existing aliases.
  Creation checks the requested slug against existing slugs but not against another service’s aliases. Check input.Slug against communication_service_aliases under the existing name locks and return ErrServiceAliasConflict when claimed.

• internal/store/profile_values.go:32 — Generated PATCH contracts incorrectly require server-owned fields.
  Reusing ValueEnvelope for requests and responses causes OpenAPI to mark fields such as id, created_at, and updated_at as required inputs, despite the server ignoring them. Add a write-specific envelope containing only accepted input fields, use it for profile input types, and regenerate the clients.

---

Reviewers: 2 done | Synthesis: codex, 14s | Total: 10m11s

[roborev-ci]

roborev: Combined Review (ac6a574)

The review found three medium-severity issues; no critical or high-severity findings were reported.

Medium

• Canonical service slugs can collide with existing aliases — internal/store/communication_services.go:209
  Service creation checks aliases for conflicts but does not check whether the proposed canonical slug is already another service’s alias. For example, registering twitter can succeed even when it aliases x, causing later resolution to silently switch services.
  Fix: Check the proposed slug against communication_service_aliases before insertion and return ErrServiceAliasConflict.

• Partial-date support is inconsistent with the vCard parser — internal/store/partialdate.go:28
  The parser rejects valid forms already supported by internal/vcard, including compact month/day (--0412) and month-only (--04). Current validation and SQL constraints also prevent month-only dates from being stored.
  Fix: Align parsing, validation, rendering, and database constraints with the partial-date forms supported by internal/vcard.

• PATCH schemas incorrectly require server-generated fields — api/openapi.yaml:6186
  PATCH inputs reuse the response ValueEnvelope schema, causing generated clients to require fields such as id, created_at, and updated_at, although the server expects only writable input fields such as source.
  Fix: Add a request-specific envelope containing only writable fields, or mark response-managed properties appropriately, then regenerate clients.

---

Reviewers: 2 done | Synthesis: codex, 16s | Total: 12m31s

[roborev-ci]

roborev: Combined Review (2a3221d)

Code review found three medium-severity correctness issues in subset copying, history supersession, and profile ordering.

Medium

• internal/store/subset.go:407 — CopySubset preserves people and their revisions but omits the new structured-profile tables. Copied people consequently lose names, contact points, addresses, dates, categories, and media, violating the complete-profile invariant. Copy current and historical structured values for each included person, including required observations and communication-service dependencies, and add behavioral coverage.

• internal/store/profile_store_helpers.go:88 — Rows imported with an existing active_until cannot be superseded because validation and update logic require active_until IS NULL. These historical values can remain permanently active for transaction purposes and block replacements through vCard property-identity indexes. Permit supersession of any row where superseded_at IS NULL while preserving an existing active_until; apply the same correction to participant observations.

• internal/store/person_names.go:139 — Ordinal 0 is treated as both a valid position and the “unspecified” sentinel. Explicitly inserting or replacing at ordinal 0 when siblings exist silently assigns MAX(ordinal)+1, corrupting requested ordering across all six profile-value types. Use an optional ordinal representation, auto-assign only when absent, and reject negative values.

---

Reviewers: 2 done | Synthesis: codex, 14s | Total: 10m33s

[roborev-ci]

roborev: Combined Review (58ac8d3)

Verdict: One high-severity privacy issue and four medium-severity data integrity/API issues require attention.

High

• Shared subsets expose structured profile data without opt-in — internal/store/subset.go:435
copyStructuredProfiles runs unconditionally, including in the default sharing workflow. It copies sensitive current and historical profile data, contact details, addresses, birthdays, categories, media blobs, observations, and source metadata even when identity or attributes were not requested.
  Fix: Gate this behind an explicit option such as IncludeProfiles. Default subsets should contain only the minimal identity data required by selected messages, and CLI help should clearly warn about the sensitive-data opt-in.

Medium

• New participant identifiers remain unclassified after migration — internal/store/messages.go:2958
  The one-shot migration classifies existing identifiers, but normal insertion paths omit service_id, scope_kind, and scope_value. New identifiers for services such as iMessage, WhatsApp, Matrix, Discord, and Synctech therefore remain permanently unclassified.
  Fix: Centralize classification and apply it to every participant-identifier insert/upsert as well as the migration.

• Legacy subset copies break after adding participant identifier columns — internal/store/subset.go:529
INSERT INTO participant_identifiers SELECT * supplies fewer values when copying from supported legacy schemas into the expanded destination table.
  Fix: Copy this table through copyByName, consistent with other schema-version-sensitive tables.

• Participant merges leave stale identity-match candidates — internal/store/messages.go:2855
  Merges repoint observations but not identity_match_candidates, leaving deleted endpoints, logical self-links, and potentially duplicate candidates or evidence.
  Fix: Rewrite and canonicalize candidate endpoints during merges, reconciling self-links, duplicates, and associated evidence.

• Uploaded profile media cannot be retrieved through the API — internal/api/person_profile_values.go:143
  The API accepts inline media bytes, but responses omit them and no download route exposes ReadPersonMediaDataContext. Clients can upload photos or keys but cannot retrieve them.
  Fix: Add an authenticated media-content endpoint that returns the stored media type, or disable inline-data writes until retrieval is supported.

---

Reviewers: 2 done | Synthesis: codex, 17s | Total: 11m39s

[roborev-ci]

roborev: Combined Review (b60d539)

Medium-severity data integrity and subset-export issues require fixes before merge.

Medium

• internal/store/subset.go:676 — Profile-only provenance sources are added before general label copying, causing unrelated labels from those sources to enter exported subsets and potentially expose unrelated metadata. Track message-selected sources separately and restrict label copying to them, or add profile provenance sources after labels are processed.

• internal/store/identity_match_candidates.go:162 — Candidate creation validates endpoint kinds but not endpoint existence. Because the polymorphic columns lack foreign keys, invalid IDs can be persisted and entity deletion can leave dangling candidates. Validate both endpoints against their corresponding tables within the transaction, and remove or rewrite candidates when referenced entities are deleted.

• internal/store/participant_observations.go:112 — Participant merging permanently deletes duplicate current observations, losing distinct provenance, timestamps, original values, and history. Supersede and repoint the absorbed observation while retaining only one current row.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 12m8s

[roborev-ci]

roborev: Combined Review (43ee9d9)

Code review found one medium-severity data-integrity issue.

Medium

• Dangling identity-match endpoints after source deletion — internal/store/schema.sql:1108, internal/store/schema_pg.sql:888
  Deleting a source cascades its contact observations, but identity-match candidates can still reference those observations through polymorphic endpoint IDs. This leaves dangling candidates and associated evidence. Before deleting the source, remove candidates whose observation endpoints belong to it, or add equivalent database-trigger cleanup. Include the candidate/evidence tables in PostgreSQL’s serialized deletion locking.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 11m35s

[roborev-ci]

roborev: Combined Review (d3a393f)

The review found three medium-severity data-integrity issues; no exploitable security regressions were identified.

Medium

• internal/store/sources.go:387 — stale identity conflicts after source removal

  Source removal deletes candidates whose endpoints are observations, but automatically generated observation conflicts use participant endpoints (participant_observations.go:267). These candidates survive after their supporting observations are cascade-deleted.

  Fix: Associate generated candidates with their supporting observations or source, then delete or recompute them when that source is removed. Add coverage using an automatically generated conflict.

• internal/store/schema.sql:1185 — distinct candidate values collapse

  Candidate identity omits normalized_value from the unique index, lock key, and lookup query. Distinct matching values between the same endpoints, basis, service, and scope therefore collapse into one candidate, discarding subsequent evidence.

  Fix: Include normalized_value throughout candidate identity and merge-collision handling, or explicitly preserve each additional value as evidence.

• internal/store/participant_observations.go:193 — observation deduplication loses source provenance

  Deduplication ignores SourceID. Recording the same contact from a second source returns the first source’s row without retaining the second provenance. Removing the first source then cascade-deletes an observation still supported by the second source.

  Fix: Model provenance per source using source-specific observation rows or a separate observation-source association. Test identical observations from two sources followed by removal of either source.

---

Reviewers: 2 done | Synthesis: codex, 14s | Total: 12m43s

[roborev-ci]

roborev: Combined Review (714620d)

Changes requested: two medium-severity identity-conflict lifecycle issues need fixes.

Medium

• internal/store/sources.go:444 — Source cleanup recognizes service_scope_username conflicts only when address_kind = 'username', but candidate creation assigns that basis to all non-email/non-phone address kinds. Conflicts from social, IMPP, URL, and similar observations can survive deletion of their supporting source. Align cleanup with candidate creation’s address-kind mapping and test source deletion for every supported kind.

• internal/store/participant_observations.go:362 — Superseding an observation does not recompute automatically generated identity conflicts. A conflict may remain visible after an endpoint loses its current supporting observation; later source deletion cannot remove it because cleanup considers only current observations. During the supersede transaction, remove generated conflicts no longer supported by current observations on both endpoints.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 12m55s

[roborev-ci]

roborev: Combined Review (cf6541b)

The PR has two medium-severity correctness issues involving subset service references and incomplete participant conflict tracking.

Medium

• internal/store/subset.go:533 — Communication service references may be invalid in default subsets. Participant identifiers retain numeric service_id values, while referenced services are copied only when IncludeProfiles is enabled. Identifiers for custom services can therefore violate foreign keys or point to an unrelated destination service. Reconcile referenced services regardless of IncludeProfiles, map them by immutable slug, and rewrite destination foreign keys.

• internal/store/participant_observations.go:279 — Conflict edges are incomplete for shared identifiers. An observation creates an edge only to the first conflicting participant. With three participants sharing a value, removing or superseding the hub observation can eliminate all generated edges even though the remaining participants still conflict. Create edges for every conflicting participant or rebuild a representative conflict graph after supersession and source deletion; add a three-participant regression test.

---

Reviewers: 2 done | Synthesis: codex, 12s | Total: 10m22s

[roborev-ci]

roborev: Combined Review (2c65d15)

Code review found two medium-severity identity-conflict reconciliation issues; no critical or high-severity findings.

Medium

• Stale conflicts after provider-ID convergence — internal/store/participant_observations.go:228, internal/store/participant_observations.go:421, internal/store/sources.go:414
  Generated conflicts can remain after both participants resolve to the same stable provider ID. Provider-ID enrichment returns without reconciliation, while cleanup only checks matching address rows and ignores the equal-provider-ID rule used during conflict creation.
  Fix: Determine support using a pairwise conflicting-observation predicate, and run reconciliation after provider-ID enrichment, participant merges, supersession, and source removal.

• Neutral candidates are not promoted to conflicts — internal/store/identity_match_candidates.go:248
  A conflict upsert can return an existing neutral candidate without updating its state. If identity discovery created the candidate first, later observations with differing provider IDs report Conflicting, but the persisted record remains candidate, causing conflict-only consumers and cleanup to miss it.
  Fix: Reconcile duplicate upserts by recording the new conflict evidence and transitioning neutral candidates to conflict, while preserving explicit decision provenance.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 24m3s

[roborev-ci]

roborev: Combined Review (e39fab8)

Code review found two medium-severity data-loss issues requiring fixes.

Medium

• internal/store/subset.go:825 — Profile-enabled subset copies include contact observations but omit associated identity_match_candidates and identity_match_evidence, silently losing conflicts, evidence, and accepted/rejected decisions.

  • Fix: Copy candidates whose endpoints are both present, remap service IDs, and copy their evidence records. Add a subset test covering a conflict with evidence and a recorded decision.

• internal/store/person_addresses.go:136 — Addresses containing only an accepted non-postal value (FreeText, GeoURI, or PlaceURI) pass validation, but omitting OriginalValue causes the fallback to store ;;;;;;, discarding the supplied representation.

  • Fix: Derive OriginalValue from the available accepted representation, or require it for non-postal variants. Add behavioral tests for these variants.

---

Reviewers: 2 done | Synthesis: codex, 20s | Total: 11m23s

[roborev-ci]

roborev: Combined Review (699df8e)

Changes need fixes for two medium-severity data-integrity issues.

Medium

• Invalid supersession intervals — internal/store/profile_store_helpers.go:79, internal/store/participant_observations.go:395
  Superseding without an explicit close time assigns the database’s current time to active_until. For future-dated records, this creates an invalid interval where active_until < active_from; HTTP profile patches cannot avoid it because supersede operations accept only IDs. Default active_until to the later of active_from and the current time, consistent with the participant-merge path, and add behavioral tests for future-dated values and observations.

• Cleanup can delete promoted user/system candidates — internal/store/participant_observations.go:429
  Stale-conflict cleanup deletes every conflict whose current source is archive_observation. However, upsertIdentityMatchCandidateTx can promote a pre-existing user/system candidate to a conflict and overwrite its source. When observation support disappears, cleanup then deletes the original candidate, including review notes and evidence. Persist whether a conflict was generated from observations or promoted; delete only generated rows, and restore or demote promoted candidates while preserving their provenance and evidence.

---

Reviewers: 2 done | Synthesis: codex, 12s | Total: 12m18s

[roborev-ci]

roborev: Combined Review (cdf7bac)

The PR has one medium-severity client behavior issue; no security vulnerabilities were identified.

Medium

• pkg/client/generated/client.go:3784 — CreateCommunicationService treats every response other than 201 Created as an error, but re-registering an existing slug intentionally returns 200 OK. This causes the generated convenience client to report a successful idempotent operation as a failure.
  • Fix: Standardize the endpoint on one success status, or update the generated client/wrapper to accept and decode both 200 and 201.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 15m19s

[roborev-ci]

roborev: Combined Review (3e3f965)

Medium-severity issue found in serialized source removal; no security boundary regressions were identified.

Medium

• internal/store/sources.go:292 — RemoveSourceSerialized deletes the source but does not call deleteUnsupportedObservationIdentityConflictsContext, unlike RemoveSource. As a result, promoted observation conflicts can remain marked as conflicts after their supporting observations are cascade-deleted. Run the shared conflict cleanup after deleting the source and before committing, and add coverage for promoted conflicts through both removal paths.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 18m8s

[roborev-ci]

roborev: Combined Review (e7dd2f9)

The changes are generally sound, but one medium-severity API contract mismatch should be addressed.

Medium

• internal/api/person_profile_values.go:75 — Runtime request handling diverges from the OpenAPI schema. The schema requires original_value for names, addresses, dates, and media, and requires date for dates. However, the raw decoder permits those fields to be omitted while the store accepts fallback forms—for example, URI/inline media without original_value or a text-only date without date. Generated clients therefore reject requests the server accepts.
  • Fix: Make fallback-backed fields optional in the request DTO/schema and regenerate clients, or reject omissions at the API boundary so runtime behavior matches the published contract.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 28m1s

[roborev-ci]

roborev: Combined Review (c1343e2)

High-severity wiring issue blocks the new structured-profile and communication-service endpoints in production.

High

• Production store adapter does not implement required interfaces — internal/api/person_profile_values.go:334, internal/api/communication_services.go:118
  The routes assert the configured store implements PersonProfileValueStore and CommunicationServiceStore, but production supplies storeAPIAdapter, which implements neither. As a result, every structured-profile and communication-service endpoint returns HTTP 503 in the daemon, while tests using *store.Store directly still pass.
  Fix: Add adapter pass-through methods and compile-time interface assertions in cmd/msgvault/cmd/serve.go, plus an HTTP wiring test exercising these routes through storeAPIAdapter.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 19m28s

[roborev-ci]

roborev: Combined Review (32ba084)

Code is clean: no Medium, High, or Critical findings were reported.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 19m16s

[salmonumbrella]

@wesm all you

[wesm]

Thanks, I'll rebase this and review

[roborev-ci]

roborev: Combined Review (6f58f7d)

One medium-severity issue found; no material security regressions identified.

Medium

• Decision metadata can be lost during participant merge — internal/store/identity_match_candidates.go:653

  An observation-generated conflict can take precedence over an accepted or rejected candidate with the same resulting edge. Because decision metadata is copied only from candidates already in the selected conflict state, decided_by, decided_at, and notes are discarded. The merged row is also marked promoted, allowing later observation cleanup to demote it to an undecided candidate and permanently erase the user’s decision.

  Suggested fix: Preserve terminal decisions and their metadata when reconciling them with observation-derived conflicts, or persist the underlying pre-conflict state so cleanup can restore it.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 18m0s

[roborev-ci]

roborev: Combined Review (84f7eba)

Code review found one medium-severity identity-matching issue; no high or critical findings.

Medium

• Cross-kind address conflicts can overwrite or incorrectly support each other — internal/store/participant_observations.go:646, internal/store/sources.go:454

  All address kinds other than email and phone are collapsed into service_scope_username, while candidate identity and cleanup omit the original address_kind. As a result, distinct URL, social, calendar, IMPP, and username conflicts may overwrite one another or remain supported by mismatched cross-kind observations.

  Recommended fix: Preserve the exact address kind in generated candidates, include it in uniqueness and lookup keys, and require it in cleanup support predicates.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 15m57s

[roborev-ci]

roborev: Combined Review (3373f03)

The PR is generally sound, but one medium-severity identity-candidate validation issue should be fixed before merging.

Medium

• internal/store/identity_match_candidates.go:172 — Candidate upserts bypass service-scope validation and accept blank normalized values. A required-scope service can therefore create fragmented identity keys, while a stable-provider candidate with NormalizedValue: new("") can pass the non-user acceptance guard at line 421 without a corroborating ID.
  • Fix: Call ValidateServiceScope after resolving the service, normalize blank values to nil, and require a nonblank normalized value for automatic stable-provider acceptance.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 13m51s

[roborev-ci]

roborev: Combined Review (a6a2d45)

Medium-severity correctness issues remain in identity conflict handling and provider ID normalization.

Medium

• internal/store/identity_match_candidates.go:258 — Observation conflicts only promote candidates in the candidate state. An already accepted edge that later receives contradictory stable provider IDs remains accepted, concealing evidence that the participants may be distinct. Promote accepted edges to conflict, preserve accepted in pre_conflict_state, and restore it when contradictory observation support disappears.

• internal/store/identity_match_candidates.go:666 — Merging a manual conflict with an observation-generated conflict marks the result as promoted. Removing observation support then demotes the combined row and incorrectly discards the manually asserted conflict. If any merged conflict has no observation origin, keep the merged origin NULL so observation cleanup cannot demote it.

• internal/store/participant_observations.go:158 — Blank ProviderUserID values remain non-NULL, so two &"" observations are treated as sharing a stable provider ID and conflict generation is suppressed. Normalize blank IDs to NULL before comparison and storage, and test that blanks are treated as unknown evidence.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 20m45s

[roborev-ci]

roborev: Combined Review (bbb397b)

Code review found three medium-severity identity consistency issues; no high or critical findings.

Medium

• internal/store/participant_observations.go:158 — ProviderUserID is neither trimmed nor converted from blank to nil. Blank IDs become non-NULL stable identifiers, potentially merging different participants under one provider account and suppressing genuine identity conflicts. Normalize with trimmedOrNil before comparison and persistence, and test blank IDs as equivalent to absent IDs.

• internal/store/identity_match_candidates.go:431 — Manually confirming an automatically generated conflict leaves observation_conflict_origin as generated or promoted. If supporting observations later disappear, cleanup may delete or demote the candidate, losing the explicit decision and metadata. Clear the automatic origin when a user explicitly retains the conflict, and test that cleanup preserves it.

• internal/store/communication_services.go:263 — Changing a service’s normalization strategy or version does not re-normalize existing contact points and observations. Old and new rows can use incompatible identity representations, causing duplicate contact points or missed conflicts. Either make referenced normalization settings immutable or atomically migrate and reconcile dependent values and candidates.

---

Reviewers: 2 done | Synthesis: codex, 17s | Total: 16m28s

[roborev-ci]

roborev: Combined Review (f8f8e54)

One medium-severity correctness issue found in participant conflict detection.

Medium

• internal/store/participant_observations.go:260 — Enriching an existing observation with a provider ID returns before checking other participants. If another participant has the same address but a different provider ID, the result incorrectly reports Conflicting: false and omits the conflicting candidate ID. Continue through the normal conflict-discovery/upsert path after enrichment, and add tests covering matching and differing provider IDs.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 25m35s