Add portable field metadata and typed person attributes

[salmonumbrella]

What changed

• Added a portable field-definition registry with stable universal IDs, immutable slugs, mutable labels, typed storage, cardinality, capabilities, options, vCard mapping, and lifecycle state.
• Added typed, historized person attribute values with value-level provenance, confidence, actor, ordinal, world-time intervals, and transactional supersession.
• Seeded primary_channel, contact_frequency, ask_me_about, and the read-only derived last_contacted.
• Added definition and person-attribute API routes, generated clients, and non-interactive CLI workflows with dry-run support.

Why

A JSON-only profile blob cannot preserve independently addressable values with their own provenance, confidence, actor, ordinal, and active interval. Typed value rows retain that history and keep current-value lookups indexable.

Dynamic per-user DDL would require migration, synchronization, permission, and API machinery for every custom field. Metadata rows plus concrete typed tables keep field creation portable across SQLite and PostgreSQL without issuing runtime DDL.

This is the second implementation slice in #534.

Usage

• msgvault attribute-definition list --object-type person
• msgvault attribute-definition create --definition @field.json
• msgvault person attributes set <person-id> ask_me_about --value sailing
• msgvault person attributes list <person-id> --history

[roborev-ci]

roborev: Combined Review (15e8a4d)

Verdict: Changes requested — one High-severity runtime failure and two Medium-severity data-integrity issues.

High

• New attribute endpoints always return 503 — internal/api/attribute_definitions.go:266, internal/api/person_attributes.go:248
  The production server supplies *storeAPIAdapter, which neither embeds *store.Store nor implements the new attribute interfaces, so both runtime type assertions fail. Add pass-through methods for AttributeDefinitionStore and PersonAttributeStore, compile-time interface assertions, and a production-adapter route test.

Medium

• Deleting a referenced person leaves dangling attribute values — internal/store/schema.sql:660, internal/store/schema_pg.sql:486
  Person references are validated only on insertion. In DeletePersonContext, atomically reject deletion while references exist or explicitly cascade/supersede those values. Add regression coverage for both database backends.

• Backdated replacements record an incorrect audit timestamp — internal/store/person_attributes.go:214
  The caller-supplied active_from is written to both active_until and superseded_at, conflating validity time with transaction time. Pass separate timestamps, using active_from for active_until and the actual write time for superseded_at.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 10m46s

[salmonumbrella]

Fixed in 69feb99.

• The production storeAPIAdapter now implements both attribute capabilities through explicit pass-through methods, with compile-time assertions and a real daemon-adapter route regression.
• Person deletion now returns a conflict while another profile has any stored person reference; reference creation and deletion share the identity-mutation lock so the check is atomic on SQLite and PostgreSQL.
• Attribute closure now records active_until from the requested validity boundary and superseded_at from the actual write time, including backdated replacement coverage.

The PR remains one commit.

[roborev-ci]

roborev: Combined Review (69feb99)

Code review found two medium-severity issues involving subset data loss and unsupported select value types.

Medium

• Subset creation drops person attribute data — internal/store/schema.sql:645, internal/store/subset.go:372
create-subset copies person profiles but omits attribute_definitions and person_attribute_values. Even with --include-identity, custom definitions and all attribute values/history are silently lost.
  Fix: Copy definitions by universal_id, remap definition IDs, and copy values for included people. Handle record-reference targets according to the subset identity policy and add subset coverage for attributes.

• Select definitions permit value types that cannot be stored — internal/store/attribute_definitions.go:321
  Select and multiselect widgets accept JSON or record-reference value types, but CanonicalString cannot represent them. Definitions can therefore be created successfully while every attempt to store a value fails.
  Fix: Reject select/multiselect definitions whose value type lacks a canonical string representation, and validate or canonicalize choices for the declared scalar type.

---

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

[salmonumbrella]

Fixed in 43f836f.

• Subset creation now reconciles person attribute definitions by universal_id, remaps value/history rows to destination-local definition IDs, omits references to excluded people by default, and follows person references directionally under --include-identity.
• Select and multiselect definitions now reject non-canonical JSON/record-reference value types, parse scalar choices by their declared type, store canonical forms, and reject invalid or canonically duplicate choices.
• Added real subset history/reference-policy coverage and scalar choice validation coverage.

The PR remains one commit.

[roborev-ci]

roborev: Combined Review (43f836f)

Verdict: One high-severity privacy risk and two medium-severity validation issues require changes.

High

• Shared subsets expose complete person attribute history by default
  Location: internal/store/subset.go:495
CopySubset, despite being intended for sharing, copies every scalar attribute value for each copied person—including superseded history, private notes, provenance references, and actor metadata. This happens even when includeIdentity is false and regardless of whether the attributes relate to selected messages, potentially exposing unrelated personal metadata.
  Fix: Require explicit opt-in, such as --include-attributes, before copying definitions and values. Consider separate options for current and historical values, with a clear sensitive-data warning.

Medium

• Invalid expected value ID silently disables compare-and-swap protection
  Location: cmd/msgvault/cmd/person_attributes.go:162
  An explicitly supplied non-positive --expected-value-id is omitted, allowing the current value to be superseded without the requested concurrency guard.
  Fix: Detect whether the flag was supplied, reject values below 1, and always forward valid supplied values.

• Organization field type accepts a contradictory person target
  Location: internal/store/attribute_definitions.go:347
field_type=organization accepts record_target=person, producing metadata that clients may render as an organization reference even though its values point to people.
  Fix: Validate person and organization widgets separately, requiring matching record targets or rejecting organization widgets until organization targets are supported.

---

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

[salmonumbrella]

Fixed in amended head 24630e3: shared subsets now exclude person attribute definitions and values by default; --include-attributes explicitly includes current and historical values and emits a sensitive-data warning. Explicit --expected-value-id values below 1 are rejected before any API request, while valid IDs are forwarded. Record-reference widgets now require a matching target, so organization widgets are rejected until organization targets are supported.

[roborev-ci]

roborev: Combined Review (24630e3)

Medium-severity validation gap found; no High or Critical issues identified.

Medium

• internal/store/attribute_values.go:80 — incomplete union-shape validation

  record_type is omitted from validation. A scalar value with a stray record_type passes application validation, then violates the database record-reference CHECK, causing HTTP 500 instead of 400.

  Recommended fix: Reject record_type and record_id unless both are present on a record_reference value, and add malformed-union tests.

---

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

[salmonumbrella]

Fixed in c8d111e. Non-record-reference values now reject either record field, and the request-level regression covers stray record_type and record_id as 400 responses.

[roborev-ci]

roborev: Combined Review (c8d111e)

Medium-severity issue found: attribute clearing lacks optimistic concurrency protection.

Medium

• internal/api/person_attributes.go:236 — Clearing an attribute neither exposes nor forwards ExpectedValueID, although the store supports this compare-and-swap guard. A stale client can therefore clear a newer value written after its last read.
  • Fix: Add an optional expected value ID to the DELETE API, generated clients, and CLI, then pass it to PersonAttributeSupersedeInput.ExpectedValueID.

---

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

[salmonumbrella]

Resolved in a2316eb4e918d10c304d65f02ef42c535f0b365c. The person-attribute DELETE API now accepts an optional expected_value_id, validates and forwards it to the store CAS guard, and returns 409 without clearing when the current value has changed. Generated Go/web clients and msgvault person attributes clear --expected-value-id expose the guard; regression coverage verifies a stale clear preserves the newer value.

[roborev-ci]

roborev: Combined Review (a2316eb)

Code looks clean; no findings at Medium, High, or Critical severity.

---

Reviewers: 2 done | Synthesis: codex, 5s | Total: 12m42s