Add durable person profiles over identity clusters

[salmonumbrella]

Adds a curated person layer over the identity-cluster substrate from #492.

A cluster's canonical ID is derived from component membership, so it re-points whenever links change elsewhere in the component, and importer-driven merges delete participant rows outright — there is nothing stable to hang curated data on. This introduces that stable identity:

• New persons table — stable surrogate ID (AUTOINCREMENT/IDENTITY, so a deleted person's ID is never recycled), reserved unique vCard UID, editable display-name override, and a revision counter driving If-Match optimistic concurrency — plus person_participants bindings, in both SQLite and PostgreSQL schemas. The design contracts (bindings are the source of truth for membership; UIDs are never reused, the survivor keeps its UID on a future person-merge, deletes retire UIDs with no tombstones) are documented in the schema.
• Linking two clusters that already belong to two different persons returns a conflict instead of silently merging curated data. When exactly one person covers the clusters being joined, linking auto-binds the combined cluster's unbound members to it, so person membership never drifts behind cluster membership; participant merges fill the same way and repoint bindings inside the same transaction. The Beeper importer skips (with a warning) merges that would join two different persons rather than aborting the sync.
• Unlinking never moves or deletes bindings, so a person ID and vCard UID survive arbitrary link/unlink churn.
• HTTP API: POST /api/v1/persons promotes a participant's cluster into a person (201 on creation, 200 on idempotent re-promotion); GET/list/PATCH (display name, with stale-revision conflicts) and DELETE (permanent, If-Match-guarded) round it out. GET /api/v1/people/{id} now carries a profile block (person ID, display-name override, revision) when the cluster is promoted, so clients can resolve participant → person without listing persons. OpenAPI document and generated clients updated.
• CLI: msgvault person promote|get|list|set-display-name|delete.

Usage: promote any participant of a cluster (msgvault person promote <participant-id>), then address that person by its stable ID regardless of later identity linking, unlinking, or importer merges. Profiles are only created by explicit promotion — no automatic promotion of observed participants. A binding conflict is resolved by deleting one of the profiles, linking, and re-promoting.

Web UI exposure and read-side integration are tracked in #532.

[roborev-ci]

roborev: Combined Review (9e4385d)

The PR has two medium-severity correctness issues: promotion mishandles HTTP 201, and binding changes do not update profile revisions/ETags.

Medium

• cmd/msgvault/cmd/person.go:39 — daemonclient.APIResponse accepts only HTTP 200, but promotion returns HTTP 201. The profile is created successfully, yet the CLI reports an API error and prints no result. Handle 201 as an expected success status and add coverage for the CLI’s 201 response path.

• internal/store/persons.go:103, internal/store/persons.go:316 — Adding, moving, or removing participant bindings changes the person representation without incrementing revision or updating updated_at. The strong ETag consequently remains unchanged, allowing stale If-Match values to remain valid. Within the same transaction, touch each affected person exactly once whenever its bindings change.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 6m41s

[salmonumbrella]

Both addressed in 9d910c5: the promote CLI now accepts the 201 response (new expected-status variant on the daemon client, with coverage), and any participant-binding insert/repoint/removal bumps the affected person's revision and updated_at in the same transaction so stale If-Match values are rejected.

[roborev-ci]

roborev: Combined Review (9d910c5)

The person-profile feature has three medium-severity correctness and data-integrity issues; no critical or high-severity findings were reported.

Medium

• API contract mismatch — internal/api/person_profiles.go:35
  The generated contract defines display_name as optional and non-null, while the handler requires the field and accepts null to clear it. Generated Go and TypeScript clients cannot express the clear operation and treat {} as valid even though the server rejects it.
  Fix: Mark display_name as required and nullable in the OpenAPI schema, then regenerate all clients.

• Inconsistent profile hydration and non-atomic updates — internal/store/persons.go:136
  Profile rows and participant bindings are hydrated through separate autocommit statements. A concurrent promotion or merge can combine an old revision with new participant IDs, invalidating the strong ETag. A display-name update can also commit before binding lookup fails, returning an error despite persisting the mutation and causing retries to fail against the bumped revision.
  Fix: Read profiles and bindings from one consistent statement or snapshot. Wrap updates and response hydration in a transaction that commits only after both succeed.

• Subset copies discard durable profiles — internal/store/subset.go:296
CopySubset copies selected participants but not their persons or person_participants rows, silently losing durable profiles, display-name overrides, and stable vCard UIDs.
  Fix: Copy person rows referenced by retained participants, then copy the corresponding bindings and add a subset round-trip test.

---

Reviewers: 2 done | Synthesis: codex, 12s | Total: 8m19s

[salmonumbrella]

All three addressed in 3c1a3f3: display_name is now required and nullable on the PATCH body (clients regenerated, null clears the override); person reads hydrate the profile and its bindings from a single transaction, with create/update committing only after hydration succeeds; and CopySubset now carries persons and bindings for retained participants, with a round-trip test.

[roborev-ci]

roborev: Combined Review (3c1a3f3)

Code is generally sound, but the CLI cannot clear an existing display-name override.

Medium

• cmd/msgvault/cmd/person.go:114 — The API supports clearing display_name with null, but the CLI rejects empty input and always sends a non-nil value. Once an override is set, CLI users cannot remove it.
  • Fix: Add a clear-display-name command or --clear option that PATCHes DisplayName: nil, with corresponding test coverage.

---

Reviewers: 2 done | Synthesis: codex, 6s | Total: 8m26s

[roborev-ci]

roborev: Combined Review (1050de9)

No Medium, High, or Critical findings were identified.

---

Reviewers: 2 done | Synthesis: codex, 5s | Total: 6m35s

[wesm]

looking

[wesm]

I'm going to do some work on this

[wesm]

Pushed fab8e51, which closes the operational gaps found in review so the person layer is safe to build UI on:

• Delete escape hatch. DeletePerson in the store (revision CAS under the identity mutation lock), DELETE /api/v1/persons/{id} with required If-Match (204/404/409/428), and msgvault person delete. A person_binding_conflict is no longer a dead end: delete one profile, link, re-promote.
• Auto-bind on link and merge. When exactly one person covers the clusters being joined, LinkParticipants binds the combined cluster's unbound members to it and bumps its revision; MergeParticipants fills the same way. Person membership can no longer silently drift behind cluster membership, and within a cluster bindings stay all-or-none to at most one person.
• Participant → person lookup. GET /api/v1/people/{id} now returns a profile block (person ID, display-name override, revision) when the cluster is promoted, so the web UI can show curated identity and drive PATCH/DELETE from a detail view without listing all persons.
• ID durability fix. The new delete test exposed that SQLite recycles plain INTEGER PRIMARY KEY rowids, so a deleted person's ID would have been reassigned to the next person. persons.id is now AUTOINCREMENT (PostgreSQL's IDENTITY already never reuses).
• Design contracts documented in both schemas. Bindings are the source of truth for membership; vCard UIDs are random and never reused (a future person-merge keeps the survivor's UID and retires the other); deletes are permanent with no tombstones.
• Hygiene. Promote returns 200 on idempotent re-promotion vs 201 on creation (CLI accepts both); persons added to the PG exclusive-lock table set (merge bumps person revisions on the import path); 409 message casing unified; If-Match strictness (no *, no tag lists) and the intentionally unpaginated persons list documented in the OpenAPI descriptions.

One deliberate semantic to be aware of: auto-bind plus "unlink never unbinds" means a briefly linked participant stays a person member after the link is retracted; the remedy today is delete + re-promote. That trade-off, along with web UI exposure, read-side integration (display-name override in analytics/search, messages-for-person), person-merge, and Beeper skip counts in import stats, is tracked in #532.

Verified: build, vet, golangci-lint clean; full test suite passes with the fts5 sqlite_vec tags; OpenAPI YAMLs, Go client, and web schema.d.ts regenerated with no drift. The PostgreSQL lane (make test-pg) was not run locally — no PG instance here — so it should be covered by CI or a manual run before merge.

[roborev-ci]

roborev: Combined Review (fab8e51)

Code review found two medium-severity issues involving idempotent person creation and a potential PostgreSQL deadlock.

Medium

• pkg/client/generated/client.go:5736 — Successful idempotent re-promotion is treated as an error.
  CreatePerson accepts only HTTP 201, although a legitimate idempotent re-promotion returns 200. Consequently, the public pkg/client.Client.CreatePerson convenience method reports an API error for a successful operation. Add a wrapper that accepts both JSON200 and JSON201, consistent with AddAccount and StageDeletion, and add a regression test for the 200 response.

• internal/store/dialect_pg.go:550 — Conflicting lock order can deadlock PostgreSQL operations.
  The exclusive lock acquires participants before persons and person_participants, while MergeParticipants mutates the person tables before updating participants. A serialized source removal racing a Beeper merge can therefore deadlock. Establish one lock order before mutations—such as serializing BeginExclusive against the identity-mutation lock first—and add a PostgreSQL concurrency test covering RemoveSourceSerialized racing MergeParticipants.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 6m57s

[wesm]

Both findings confirmed and fixed in 8c8055b:

• Client 200 handling — added a hand-written CreatePerson wrapper in pkg/client/client.go following the existing AddAccount/StageDeletion pattern (those exist for exactly this generated-client limitation), returning JSON201 or JSON200. Regression test TestCreatePersonAcceptsIdempotentOK covers the 200 path.

• PG lock inversion — real deadlock: MergeParticipants writes person_participants/persons before participants/messages, the opposite of BeginExclusive's LOCK TABLE order. Rather than reordering the table list (fragile against future writers), BeginExclusive now takes the identity-mutation row lock (the archive_metadata identity-revision row that link/unlink/merge/promote/delete all acquire first) before LOCK TABLE, serializing the two paths at a single point. Added TestRemoveSourceSerializedDoesNotDeadlockWithPersonMerge racing RemoveSourceSerialized against MergeParticipants over 15 iterations; verified against a live PostgreSQL 17 instance that it fails with SQLSTATE 40P01 without the row lock and passes with it. Full make test-pg suite is green.

[roborev-ci]

roborev: Combined Review (8c8055b)

Subset export has one medium-severity identity-link preservation issue; no qualifying security findings were identified.

Medium

• internal/store/subset.go:315 — Subset export copies person profiles and bindings but not participant_links. Linked participants become disconnected analytical identities in the destination, so /people/{id} no longer aggregates their activity even though both remain bound to the same durable profile.
  • Fix: Preserve or reconstruct relevant identity-link components for copied participants, including required intermediate members, and add coverage for a promoted linked cluster.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 11m32s

[wesm]

Confirmed and fixed in 973b040. CopySubset selected participants only by message involvement and never copied participant_links (a gap that predates this PR, but the person-binding copy made it concrete: bindings were silently filtered to message-bearing members while the cluster arrived disconnected).

The fix pulls each copied participant's full link component in first — cluster-mates resolved via a recursive CTE over src.participant_links, which covers connectivity through intermediate members with no copied messages — then copies every edge inside those components. Destination clusters, /people/{id} aggregation scope, and person bindings now match the source.

TestCopySubset_PreservesIdentityClusters covers a promoted linked cluster whose second member has no messages in the subset; verified it fails (disconnected cluster, missing binding) without the fix and passes with it.

[roborev-ci]

roborev: Combined Review (973b040)

Verdict: Three medium-severity issues remain involving subset privacy, incomplete person profiles, and PostgreSQL deadlock risk.

Medium

• Subset export leaks PII beyond selected messages — internal/store/subset.go:322
  The recursive CTE exports every transitive identity-cluster member, including participants with no selected messages. Their email addresses, phone numbers, display names, and alternate identifiers are exposed in subsets intended for sharing.
  Fix: Keep message-derived participants as the default boundary. Make complete identity-cluster export an explicit opt-in with a warning or preview.

• Subset copying truncates durable-person membership — internal/store/subset.go:351
  After unlinking leaves one person spanning disconnected clusters, selecting messages from one cluster copies the person but omits bindings and participants from the others, producing a truncated profile with the original revision.
  Fix: Define an explicit consistency policy alongside the privacy boundary. When complete profiles are requested, expand through both participant_links and person_participants and copy every binding for included people. Add a regression test for copying a promoted cluster after unlinking.

• Identity mutation lock ordering can deadlock — internal/store/dialect_pg.go:599
  The new identity-revision lock can invert with MigrateLegacyIdentityConfigContext: one transaction can hold the revision row while waiting for account_identities, while migration holds account_identities and waits for the revision row. PostgreSQL will abort one transaction.
  Fix: Acquire lockIdentityMutationTxContext at the start of the migration transaction, before writing account_identities, and audit other revision writers for consistent lock ordering.

---

Reviewers: 2 done | Synthesis: codex, 15s | Total: 9m4s

[wesm]

All three confirmed and fixed in c21c252. The two subset findings are resolved with one consistency policy rather than two patches:

• Privacy boundary (default) — the participant set is message-derived again: no participant row, identifier, link edge, or person binding is copied for identities without selected messages. Link edges are kept only when both endpoints are already included (no new PII), and a person is copied only when every binding falls inside the subset — a partial binding set under the original revision would misrepresent the curated profile, so partially-covered persons are skipped entirely instead of truncated. TestCopySubset_DefaultExcludesOffMessageIdentities pins the boundary.

• Complete profiles (opt-in) — create-subset --include-identity expands participants through the closure of participant_links and shared person_participants bindings, so included clusters and profiles arrive complete — including the requested regression case of a person left spanning disconnected clusters by an unlink (TestCopySubset_IncludeIdentitySpansUnlinkedClusters). The flag help spells out that this exposes identifiers of linked identities with no messages in the subset.

• Migration lock order — real inversion: the legacy identity-config migration wrote account_identities before bumping the identity revision, opposite to BeginExclusive's row-lock-then-LOCK TABLE order. It now takes lockIdentityMutationTxContext at the start of its transaction, matching every other revision writer (audited: link/unlink/merge/add/remove-identity all lock first; the phone-unique migration runs inside InitSchema before the daemon serves). Added TestRemoveSourceSerializedDoesNotDeadlockWithLegacyIdentityMigration; verified against live PostgreSQL 17 that it reproduces SQLSTATE 40P01 without the fix. The blocked-cancellation test was updated to pin the new up-front lock statement.

Full SQLite and PostgreSQL suites plus lint are green.

[roborev-ci]

roborev: Combined Review (c21c252)

No issues found.

---

Reviewers: 2 done | Synthesis: codex | Total: 12m43s

[wesm]

CI failures on c21c252 had a single root cause: the four failing jobs run the PR merged with latest main, and main's new cache-staleness tests (#528) link participants on the hand-rolled fixture schemas in build_cache_test.go — which predate the persons/person_participants tables that LinkParticipants now maintains, so they failed with no such table: person_participants. Fixed in 42edcc7 by merging main into the branch and mirroring the persons tables into both fixture schemas. Full SQLite and PostgreSQL suites plus lint are green on the merged tree.

[roborev-ci]

roborev: Combined Review (42edcc7)

Code review found two medium-severity issues involving transaction safety and subset privacy.

Medium

• internal/store/dialect_pg.go:599 — Inconsistent lock ordering can deadlock

  PostgreSQL BeginExclusive locks the identity-revision row before participant_identifiers, while SetParticipantIdentifier takes these locks in reverse order for owner-evidence identifiers. Concurrent source removal and identifier updates can deadlock and abort a transaction.

  Fix: Call lockIdentityMutationTx at the beginning of SetParticipantIdentifier, before accessing participant_identifiers, and audit other identity-revision writers for consistent lock ordering.

• internal/store/subset.go:365 — Identity metadata bypasses the opt-in privacy boundary

  CopySubset(..., includeIdentity=false) still copies participant_links and, near line 377, complete persons and person_participants records. A shared subset can therefore expose linked identities, curated display names, and stable vCard UIDs without explicit opt-in.

  Fix: Gate identity links and person-profile inserts on includeIdentity. Add a regression test confirming that a default subset containing messages from linked participants includes neither their link nor person profile.

---

Reviewers: 2 done | Synthesis: codex, 13s | Total: 11m54s

[roborev-ci]

roborev: Combined Review (0cf7f01)

The changes have one medium-severity PostgreSQL deadlock risk; no material security regressions were identified.

Medium

• Potential lock-order deadlock — internal/store/dialect_pg.go:594
BeginExclusive locks the identity-revision row before requesting an exclusive lock on participant_identifiers, while SetParticipantIdentifier modifies participant_identifiers first and may later wait for the identity-revision row when owner evidence changes. Concurrent operations can deadlock, causing PostgreSQL to abort one transaction.
  Recommended fix: Have SetParticipantIdentifier acquire lockIdentityMutationTx before modifying participant_identifiers, audit other late identity-revision writers for the same inversion, and add a concurrent PostgreSQL regression test.

---

Reviewers: 2 done | Synthesis: codex, 13s | Total: 12m11s

[wesm]

CI is green on 0cf7f01 (all jobs, including the four that failed and test-pgvector).

Two fixes were needed:

1. Fixture drift from the merge with main (42edcc7): main's new cache-staleness tests link participants on the hand-rolled fixture schemas in build_cache_test.go, which predate the persons tables that LinkParticipants now maintains. Mirrored the two table definitions into both fixture schemas.

2. test-pgvector timeout was a real perf regression (0cf7f01): the deadlock-ordering fix had the legacy identity migration take the identity-mutation row lock unconditionally at transaction start — turning every store open into a write transaction (row-version churn + WAL commit on one archive_metadata row, and a serialization point between the daemon and CLI subprocesses sharing a DB). Cheap locally, but it blew the cmd package past the 10-minute timeout on CI (main baseline: 72s). The no-op path is read-only again: marker check first without locks; only when the migration will actually write does it take the row lock (still before any account_identities write, preserving the ordering contract), then re-checks the marker under the lock. Local PG timing for the package went from 155s back to under the main baseline.

[wesm]

Confirmed and fixed in 52bcec1. SetParticipantIdentifier wrote participant_identifiers before its conditional owner-evidence identity-revision bump — the same inversion class as the migration. The no-op fast path (constant on importer re-runs) stays read-only; the identity-mutation row lock is taken only on the write path, before the table write, with the no-op re-checked under the lock. New regression test TestRemoveSourceSerializedDoesNotDeadlockWithSetParticipantIdentifier reproduces SQLSTATE 40P01 8 times across 5 runs without the fix on live PostgreSQL 17, and passes with it.

Also completed the requested audit of every identity-revision writer: link, unlink, merge, add/remove account identity, the legacy identity migration, and person mutations all lock first. One more late acquirer surfaced — the one-shot phone-unique migration's mergeParticipant took the lock mid-transaction via rewriteLinksForMerge; it now locks at entry. The ordering contract is now documented on lockIdentityMutationTx so future revision writers have a single rule to follow: acquire the identity row before the first write to any exclusiveLockTables table, keeping no-op fast paths read-only.

Full SQLite and PostgreSQL suites plus lint green; the PG cmd package stays at ~90s (no repeat of the timeout regression).

[roborev-ci]

roborev: Combined Review (52bcec1)

No issues found.

---

Reviewers: 2 done | Synthesis: codex | Total: 10m28s