feat(people): add organizations and employment records

[salmonumbrella]

What changed

• Add first-class organizations with versioned profiles, typed custom attributes, lifecycle and merge operations, and reviewable duplicate suggestions.
• Add current and historical employment records with primary-employment constraints and read-time person and vCard projections.
• Keep SQLite and PostgreSQL behavior aligned, and expose the model through daemon APIs, generated clients, and organization and employment CLI commands.

Why

People can have several current or historical roles, but copied company and title fields cannot preserve that history or represent one organization consistently across people. First-class records make employment changes queryable without losing source evidence.

Usage

msgvault organization create "Example Corp" --domain example.com
msgvault employment add --person 42 --organization 7 --title "Engineer" --start 2024-01 --primary

Refs #534

[roborev-ci]

roborev: Combined Review (538ed05)

The change is generally sound, but three medium-severity API and profile-reconciliation gaps should be addressed.

Medium

• Missing organization attribute clear endpoint — internal/api/organizations.go:281
  Organization attributes can be listed and replaced, but no route invokes SupersedeOrganizationAttributeValueContext. Clients therefore cannot clear an attribute without replacing it, and cannot remove values whose definitions are inactive. Add a DELETE/clear endpoint analogous to the person-attribute endpoint, including ordinal, expected-value, and dry-run handling, and expose it through the generated client and CLI.

• Profile reconciliation can discard metadata changes or violate uniqueness — internal/store/organization_profile.go:455
  Reconciliation treats rows as unchanged based only on their normalized business key, silently dropping changes to writable metadata such as pref, ordinal, source fields, and vCard identity. Changing a value while preserving its vCard property identity can also insert the replacement before superseding the old row, violating the active property-identity uniqueness constraint. Compare all writable fields, explicitly match durable vCard identities, and supersede changed or removed rows before inserting replacements.

• Organization profile request limit is too small for supported media — internal/api/organizations.go:491
  Profile PUT uses the generic 1 MiB decoder even though inline organization media supports up to 8 MiB. Base64 payloads larger than roughly 750 KiB are rejected before reaching the store-level limit. Use a dedicated decoder sized for an 8 MiB base64 payload, similar to the person-profile decoder, and return the appropriate oversized-request response.

---

Reviewers: 2 done | Synthesis: codex, 16s | Total: 11m42s

[roborev-ci]

roborev: Combined Review (7c31adf)

Medium-severity lifecycle and media round-trip gaps remain; no Critical or High findings were reported.

Medium

• internal/store/organization_attributes.go:179 — Organization attributes can reference people, but person deletion checks only person_attribute_values. Deleting a person referenced by a current organization attribute leaves a dangling value_record_id.

  • Fix: Include current organization_attribute_values references in DeletePersonContext and add a matching record-reference index.

• internal/store/organizations.go:382 — Deleting a merge survivor with no employments reaches the database delete, but losing organizations still reference it through merged_into_id ... ON DELETE RESTRICT. This produces an internal database error instead of a defined lifecycle conflict.

  • Fix: Detect inbound merged redirects and return a typed conflict, or explicitly repoint/remove those redirects during deletion.

• internal/api/organizations.go:143 — Inline organization media can be uploaded, but responses expose only metadata, with no content-read endpoint or stable existing-media reference accepted by replacement requests. Stored blob-only media therefore cannot be downloaded or preserved through a GET/modify/PUT round trip.

  • Fix: Add organization-media content retrieval analogous to person media and allow replacements to retain an existing media row without re-uploading its bytes.

---

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

[wesm]

Thanks — the core engineering here is solid (dual-backend concurrency, merge lock ordering, partial dates are all handled carefully, and the tests exercise real DB/HTTP paths). But there are two data-loss-class bugs to fix, and the PR is carrying roughly a third more code than the feature needs. Requesting changes.

Blocking bugs

• employment set builds the PATCH body from changed flags only, but the server PATCH is full-replace (cmd/msgvault/cmd/employment.go:63). Fixing a title on a past employment nulls role, department, and dates, and flips it back to is_current=true. organization set already does the correct read-merge-write; employment set should too. There's no test for set.
• PATCH /organizations/{id} un-retires on any field-only update: Retired is a non-pointer bool with full-replace semantics (internal/api/organizations.go:40). Renaming a retired org silently reactivates it. The CLI works around this internally, which shows the footgun is live for other clients. Make it *bool.
• Employment writes have no SQLite busy-retry, unlike attribute writes (which this PR generalized retryAttributeWrite for). Concurrent writes for one person surface raw SQLITE_BUSY as an untyped 500, and both concurrency regression tests skip on SQLite, so the default backend has no coverage here (internal/store/employments.go:423).

Behavior surprises to resolve

• An update with is_primary omitted can silently auto-promote that employment to primary, changing the person's derived company as a side effect of a typo fix (employments.go:453).
• is_current=true can coexist with a past end_date; nothing validates consistency (employments.go:397).
• Profile GET opens a write transaction with FOR UPDATE where the person analog uses a read snapshot; on Postgres, reads serialize behind merges (organization_profile.go:161).
• An invalid primary_domain is silently dropped instead of erroring; the profile-identifier path errors for the same condition (organizations.go:523).

Scope: please trim

• organization_names copies 13 person-name columns the code never reads (organizations now have honorific_prefixes), and organization_schema_parity_test.go enforces the copy. Strip the unused columns and the test that freezes them in.
• The duplicate-suggestions + merge workflow (~1,000 lines across store/API/CLI) goes beyond "organizations and temporal employment associations" — the roadmap lists same-name suggestions as bonus research. Suggest a follow-up PR.
• Dead surface: UpdateOrganizationContext/RetireOrganizationContext/UnretireOrganizationContext have no non-test callers; PrimaryCurrentEmploymentsContext is uncalled; SupersedeOrganizationAttributeValueContext is wired in the serve adapter but no route calls it (so org attributes can't be cleared over HTTP); PUT /organizations/{id}/profile has no consumer; resolved_by is never written and tests assert it's always nil.
• organization_attributes.go is a ~90% copy of person_attributes.go, and the copies have already drifted (different error sentinels and locking for the same operation). The retryAttributeWrite extraction in this PR shows the parameterization works — please share the rest.
• CLI: about 10 of the 22 subcommands aren't needed for the advertised workflow (org attributes, the duplicates trio, merge, and organization employments, which duplicates employment list --organization).

Smaller items

• organization duplicates resolve derefs resp.JSON200.Status without the nil guard every other subcommand has (organization.go:476).
• getCLIOrganization has a fallback for "older daemons" that can't exist — this PR introduces the endpoint — and the only test for the delete/retire path exercises that dead fallback.
• Org endpoints reuse decodePersonRequest, so malformed org bodies say "Invalid person request".
• Flag inconsistency: --end on add/set vs --end-date on end.
• Merge leaves open duplicate suggestions pointing at the merged org, unconditionally nulls employments.address_id, and allows merging a live org into a retired one.

With the bugs fixed and the trims above, this drops from ~10.7k to roughly 6-7k hand-written lines without losing anything the description promises.

[wesm]

looking

[wesm]

Pushed ec139bd addressing the review (note: the PR head had moved to 7c31adf, which already added the org-attribute DELETE route and profile history endpoint — the fixes are rebased on that).

Fixed

• employment set now read-merge-writes; unspecified flags come from the fetched record, and --person/--organization no longer need re-stating. Covered by a new CLI test.
• PATCH /organizations/{id} retired is now a nullable boolean; omitting it preserves lifecycle state (regression test added for the rename-reactivates case).
• Employment writes retry on SQLite busy errors via the shared helper (renamed retryContendedWrite), with a concurrency test on the SQLite path.
• Updates no longer auto-promote to primary when is_primary is omitted; is_current=true + end_date is rejected; invalid primary_domain errors instead of vanishing; profile GET uses a read snapshot; merging into a retired org is rejected; org endpoints no longer emit "Invalid person request".

Trimmed

• Duplicate-suggestion workflow removed end to end (store, schema, API, CLI, ~1,900 lines with generated code) — better as a follow-up PR.
• The 13 person-only name columns dropped from organization_names; the parity test now asserts they are absent rather than required.
• Dead surface removed: UpdateOrganizationContext/RetireOrganizationContext/UnretireOrganizationContext, batch PrimaryCurrentEmploymentsContext, the bare-Organization CLI fallback (and the test that encoded it), and the redundant organization employments subcommand. --end-date unified to --end.

Kept, deliberately

• merge stays: merged_into_id redirects are woven through the schema, employment locking, and regression tests; removing it would be a much larger change than the review's intent.
• PUT /organizations/{id}/profile stays: on the current head the profile envelope is the GET representation with a history endpoint, so this is the model's only write path rather than dead surface.
• The organization_attributes.go/person_attributes.go unification is deferred: beyond the now-shared retry helper, the remaining diff includes real behavior differences (owner locking, error sentinels) and per-entity public types, so collapsing it means touching shipped person behavior — follow-up material.

Net effect: −2,153 lines against the previous head. go vet, the testify-helper check, and the store/api/cmd/vcardmap/daemonclient suites pass locally (the only failures are two FTS5 environment-dependent tests that fail identically on main).

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (ec139bd)

Changes need revision: four medium-severity issues may cause dangling references, unintended data changes, or inaccessible media.

Medium

• internal/store/organization_attributes.go:179 — DeletePersonContext checks only person_attribute_values. Deleting a person referenced by an active organization attribute leaves a dangling record reference. Include active organization_attribute_values references in the deletion guard and add a regression test.

• internal/store/employments.go:140 — Updating a historical employment with both is_current and end_date omitted recalculates is_current as true, silently resurrecting historical records without end dates. Preserve the stored current state when IsCurrent is nil, while still ending employment when a new end date is supplied.

• cmd/msgvault/cmd/employment.go:293 — employment set claims to preserve unspecified fields but omits address_id, source_ref, and confidence from its read-merge-write body. Editing a title or department therefore clears those values. Copy compatible fields from the fetched record, clearing them only when an explicit related change requires it, and extend the preservation test.

• internal/api/organizations.go:239 — The profile API accepts inline organization media, but responses expose only metadata and no organization-media content endpoint exists. Uploaded bytes cannot subsequently be downloaded through the HTTP API. Add an authenticated content endpoint and store capability analogous to the existing person-profile media endpoint.

---

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

[wesm]

Pushed 85f2271 addressing all four roborev findings — each verified against the code before fixing, and each has a regression test:

• Person deletion guard: DeletePersonContext now also counts active organization_attribute_values record references (value_record_type = 'person'), so a person referenced by an organization attribute cannot be deleted; superseded references still allow deletion. Test mirrors the existing person-side guard test.
• Historical employment resurrection: UpdateEmploymentContext preserves the stored is_current when both is_current and end_date are omitted; supplying an end date still ends the employment. Test updates a historical record's title and asserts it stays historical.
• employment set field preservation: the merge body now carries address_id, source_ref, and confidence from the fetched record. address_id is cleared only when --organization changes (the address belongs to the previous org), and source_ref/confidence only when --source changes (they describe the previous provenance). The preservation test now asserts all three round-trip.
• Organization media content: added GET /organizations/{id}/profile/media/{media_id}/content with ReadOrganizationMediaDataContext, mirroring the person profile media endpoint (same headers, 404 for URI-only values). Test round-trips inline logo bytes and covers the URI-only and missing-ID 404s.

OpenAPI specs, Go client, and web schema regenerated; store/api/cmd suites pass locally including the artifact-staleness checks.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (85f2271)

Medium-severity issues remain in organization profile updates and merges.

Medium

• internal/api/organizations.go:75 — OrganizationEnvelopeBody omits writable envelope metadata including type_label, type_tokens, confidence, and active_from. A profile PUT cannot preserve these fields, while reconciliation compares them, causing imported or store-created rows to be superseded and reinserted without their metadata.

  • Fix: Expose and map the missing envelope fields, then add an HTTP round-trip test proving an unrelated profile update preserves them.

• internal/store/organizations.go:113 — Merging organizations moves employments but retains only the losing root name. Profile values and typed attributes remain attached to the hidden, immutable redirect, so they never join the survivor and cannot be corrected through supported mutations. Active record-valued attributes may continue blocking referenced-person deletion with no way to clear them.

  • Fix: Before making the losing organization immutable, migrate or reconcile its active profile and attribute rows into the survivor with explicit conflict handling.

---

Reviewers: 2 done | Synthesis: codex, 14s | Total: 15m5s

[wesm]

Pushed d741dfa addressing both findings — verified against the code first:

• Envelope metadata round-trip: confirmed organizationEnvelopeMatches compares type_label, type_tokens, confidence, and active_from, none of which OrganizationEnvelopeBody could express — so any row carrying them was superseded and reinserted stripped of metadata on every PUT. The body now exposes all four (the doc comment states the invariant: the body must cover every field the reconciliation compares). The new HTTP test PUTs a name with all four fields, then makes an unrelated update (adds a category) re-sending the name, and asserts the row's envelope ID is unchanged with metadata intact.

• Values stranded on merge redirects: confirmed the redirect is immutable after merge (both attribute write paths reject merged organizations), so active profile rows and attribute values left on the losing organization could never be corrected, and an active person record reference would block that person's deletion permanently. MergeOrganizationsContext now supersedes the losing organization's active rows across all six profile tables (reusing supersedeOrganizationRowsTx) and its attribute values before marking it merged. I chose supersede over migrate deliberately: blindly moving the losing organization's facts onto the survivor risks planting wrong or duplicate data there, while superseded rows remain fully readable as history on the redirect and anything that belongs on the survivor can be curated there explicitly — same philosophy as the existing former-name retention. The test merges an organization holding a profile and a person record reference, then asserts the redirect has no active values, the history remains readable, and the referenced person is deletable.

OpenAPI specs, Go client, and web schema regenerated; store/api/cmd suites pass including artifact-staleness checks.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (d741dfa)

The change is generally sound, but two medium-severity completeness and error-handling issues remain.

Medium

• internal/store/subset.go:407 — Subset creation does not copy organizations or employments for included people, and omits organization profiles and attributes even when all relevant opt-ins are enabled. This silently produces incomplete subsets.

  • Fix: Define the subset boundary for employment data, copy related organizations and employments, and include organization profile/attribute rows under the appropriate opt-ins with behavioral coverage.

• internal/store/organizations.go:356 — Organization deletion checks employments but not inbound merged_into_id references. Deleting a referenced survivor reaches the ON DELETE RESTRICT constraint and surfaces as an internal-server error.

  • Fix: Detect organizations whose merged_into_id targets the requested organization and return a typed conflict or validation error before deletion.

---

Reviewers: 2 done | Synthesis: codex, 16s | Total: 14m29s

[wesm]

Pushed 28cdbff addressing both findings plus the CI failure:

• Subset completeness: employment data now crosses the subset boundary under the existing opt-ins. IncludeProfiles copies the employments of copied people, the organizations they reference, and those organizations' profile rows (contact points go through the communication-service remap). IncludeAttributes copies organization attribute definitions and values with the same universal_id mapping and person record-reference boundary as person attributes. Default subsets still exclude all of it — employment history is the same sensitivity class as structured profiles. Behavioral tests cover both the opted-in copy and the default exclusion. One boundary note: employments cannot reference merged organizations, so the copied organization set needs no merge-redirect closure.
• Survivor deletion: DeleteOrganizationContext now counts inbound merged_into_id redirects and returns a typed ErrOrganizationInvalid ("N merged organization(s) redirect here") instead of letting ON DELETE RESTRICT surface as a 500. Test extends the existing redirect-immutability test.
• CI: the run / test lint failure was a wastedassign on the retired declaration — fixed. The run / nix-build failure is not from this PR: it's the sandboxed web build issue fixed on main by fix(nix): make the sandboxed web build work on GitHub-hosted runners #607 (which landed about an hour after that run started); the new CI run against the updated merge picks up the fix.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (28cdbff)

One medium-severity issue should be fixed before merge.

Medium

• internal/store/organization_profile.go:531 — Profile replacement treats omitted media data as an empty hash. Updating an unrelated field from a GET-derived payload can supersede URI-backed media containing inline data with a URI-only row, removing the content from the active profile.
  • Fix: Distinguish omitted data from explicit removal or provide a retention identifier so the existing blob is preserved. Add an HTTP regression test for an unrelated update to a profile containing URI-plus-inline media.

---

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

[wesm]

Pushed 67cd031 addressing the media-retention finding — verified first: the reconciliation hashed only the request's inline bytes, and since reads expose media metadata without the bytes, any GET-derived PUT computed an empty hash, mismatched the stored row, and superseded it with a URI-only copy.

The fix is the retention-identifier approach: OrganizationMediaBody and OrganizationMediaInput now carry content_hash. A media row sent with content_hash and no data matches the stored row carrying that hash and keeps its bytes — and since GET already returns content_hash on every media row, a GET-derived payload round-trips naturally. Two guardrails close the ambiguity between omission and removal: a retention hash that matches no active row is rejected as invalid_organization (never inserted as a hash-without-content row), and data sent with a mismatching content_hash fails validation. Omitting both still means URI-only, so explicit removal keeps working.

The HTTP regression test writes URI-plus-inline media, re-sends the GET-shaped row alongside an unrelated category addition, and proves the row is retained (same envelope ID, has_data still true, bytes still downloadable via the content endpoint); a stale hash returns 400.

OpenAPI specs, Go client, and web schema regenerated; store/api/cmd suites pass including artifact-staleness checks.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (67cd031)

Medium-severity issue found: inline-only media cannot be preserved during organization profile updates.

Medium

• internal/store/organization_profile.go:353 — Profile validation rejects GET-derived inline-only media that has a content_hash but no new data or URI. As a result, an unrelated PUT using the returned profile can fail instead of preserving that media.
  • Suggested fix: Accept a non-empty ContentHash as valid media input and add a round-trip test for inline media without a URI.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 10m32s

[wesm]

Pushed 7188c61. Confirmed: the media validation required uri or data, so inline-only media — which reads back as content_hash with no bytes and no URI — was rejected on a GET-derived PUT. A non-empty content_hash now satisfies the requirement (the retention path then matches it against the stored row as before, and a hash matching no active row is still rejected at insert). The new HTTP test round-trips inline-only media through an unrelated update and proves the row and its downloadable content survive.

Store-side validation only — no API schema change, so no regeneration was needed.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (7188c61)

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

Medium

• internal/store/organization_profile.go:553 — content_hash retention only works when all media and envelope fields remain unchanged. Editing metadata, a URI, or another mutable field supersedes the active inline-media row, after which insertOrganizationMediaTx rejects the hash-only replacement because it lacks data. This prevents clients from editing inline-media metadata without downloading and resending the bytes.
  • Fix: Resolve the retention hash against an active media row before reconciliation and carry its stored bytes into the replacement. Reject only hashes with no active match.

---

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

[wesm]

Pushed 8deee64 (+ a gofmt follow-up). Confirmed the limitation: the retention hash only survived an exact match — any edit to media_type, uri, or an envelope field superseded the active row and the hash-only replacement then failed at insert for lacking data, so metadata edits required re-uploading the bytes.

As suggested, retention hashes are now resolved against the active media rows before reconciliation: the stored bytes are loaded into the input, so when an edit supersedes and reinserts the row, the replacement carries the content. A hash with no active match is still rejected as invalid_organization (the insert-time guard remains as defense in depth). Fingerprints stay consistent because the stored hash is by construction the hash of the loaded bytes.

The HTTP regression test edits an inline media row's type and URI sending only the retention hash, and proves the replacement row keeps has_data and its downloadable content.

Store-side only again — no API schema change, no regeneration needed. Store/api suites pass.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (6aee3f7)

Code is not clean: one medium-severity subset export issue should be fixed before merging.

Medium

• internal/store/subset.go:370 — Identity closure misses organization-attribute person references. With IncludeIdentity, IncludeProfiles, and IncludeAttributes enabled, organization attributes reached through copied people’s employment records do not expand references to off-message people, so referenced identities are silently omitted.
  • Fix: Add reference edges from copied people’s employment-linked organization attributes, and add a subset regression test covering this case.

---

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

[wesm]

Pushed 8c54234. Confirmed: the IncludeIdentity closure's reference edges only walked person_attribute_values, so a person referenced by an included person's employer (an organization attribute) never entered the identity set — and the org-attribute copy's dangling-reference filter then silently dropped the value.

The reference-edge CTE now adds a second edge: employments of included people → their organizations' person-valued attribute values → the target's participants. It's gated on the source schema having the employment tables (older sources skip it) and on both IncludeProfiles and IncludeAttributes, since organizations only cross the subset boundary through employment references with attributes opted in.

The regression test builds an off-message person referenced by an included person's employer: with all three opt-ins the target person and the referencing attribute value survive the copy; without IncludeIdentity the target stays excluded and the reference is dropped rather than dangling, matching the person-attribute policy.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (8c54234)

The PR has one medium-severity issue: primary employment updates can create an invalid historical-primary state.

Medium

• cmd/msgvault/cmd/employment.go:353 — Updating a primary employment with --not-current or a non-empty --end sends is_primary=true with is_current=false. The store rejects historical primary employments, so these updates fail unless users also pass --no-primary.
  • Fix: Demote the employment when its resulting state is non-current, and reject explicit conflicts such as --primary --not-current.

---

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

[wesm]

Force-pushed d14e7aa: all review follow-ups squashed into the single feature commit and rebased onto current main (15c8f2c), authored as the original PR author with co-author trailers.

Also fixes the last roborev finding: employment set with --not-current or --end on a primary employment now demotes it (matching the server-side end operation) instead of sending an invalid historical primary, and an explicit --primary combined with --not-current or --end is rejected as a usage error. Both cases have CLI tests.

Rebase notes — main's typed person relationships (#592) landed on the same seams, resolved by keeping both features:

• subset export now copies relationships and employment/organization data under IncludeProfiles
• the serve adapter carries both route sets, with the wiring tests for each
• APISchemaVersion moves to 1.40.0 (main took 1.39.0 for relationships); contract assertions updated
• a duplicate formatCLIPartialDate (added independently by both branches) deduplicated

OpenAPI specs, Go client, and web schema regenerated against the merged surface. On the rebased tree: go vet clean, and the store, api, cmd, vcardmap, and daemonclient suites pass (the only failure is the FTS5 environment test that fails identically on main).

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (d14e7aa)

Changes requested: two medium-severity issues could prevent reliable organization profile updates and merges.

Medium

• internal/store/organization_profile.go:297 — Duplicate checks for addresses, contact points, and media ignore envelope metadata and durable vCard identity. Rows with identical values but distinct PROP-ID, TYPE, or ordinal are rejected, preventing imported profiles from round-tripping through profile replacement. Distinguish rows by durable vCard identity or ordinal when available, and support multiple rows sharing a business-value key during reconciliation.

• internal/store/organizations.go:64 — Organization merging reads before its first write but lacks retry handling for SQLite snapshot-upgrade contention. A concurrent commit can cause SQLITE_BUSY or SQLITE_BUSY_SNAPSHOT, failing the merge rather than producing a revision conflict or retrying successfully. Wrap the complete merge transaction in a bounded busy/deadlock retry and reload both revisions on every attempt.

---

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

[wesm]

Pushed 4731c7c addressing both findings and the CI lint failure:

• Value duplicates with distinct identity: confirmed the prepare-time duplicate checks for addresses, contact points, and media keyed only on business value, and the reconciliation's business-key map was single-valued — so an imported profile carrying the same value under two PROP-IDs could not round-trip. The duplicate check now appends an envelope discriminator (durable vCard identity, ordinal, TYPE label/tokens), and reconciliation keeps a list of current rows per business value, matching each desired row to an unclaimed counterpart — preferring the one whose full envelope matches so every row keeps its own identity. The regression test round-trips two identical addresses under distinct PROP-IDs and two identical media blobs under distinct ordinals, asserting stable row IDs across an identical replacement. Names, identifiers, and categories intentionally keep strict uniqueness, matching their unique indexes.
• Merge busy-retry: MergeOrganizationsContext now runs under the shared bounded busy/deadlock retry (retryContendedWrite), same as employment and attribute writes. Each attempt is a fresh transaction that re-reads both roots, so a concurrent commit produces a typed revision conflict rather than a raw SQLITE_BUSY. One deviation from the suggested fix: revisions are not reloaded server-side across attempts — they're the caller's compare-and-swap intent, and refreshing them would silently merge over a concurrent change.
• CI lint (goconst): the merge with the relationships feature pushed the "ETag" and "int64" literals in internal/api over the threshold; both are now package constants.

Store and api suites pass, vet clean.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (4731c7c)

The change needs one medium-severity fix before approval.

Medium

• cmd/msgvault/cmd/employment.go:411 — Organization-scoped employment listings show the organization ID for every row and omit the person ID, preventing users from identifying employees. Use personScoped to render ORGANIZATION/OrganizationID for person listings and PERSON/PersonID for organization listings.

---

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

[wesm]

Pushed 7dec131. Confirmed: employment list --organization printed the organization ID on every row — the one value the caller already supplied — with no way to identify the employee. The counterpart column now follows the scope: person-scoped listings show ORGANIZATION, organization-scoped listings show PERSON. The new CLI test lists an organization with two employees and asserts the header and per-row person IDs.

On the CI failure: run / test died in the govulncheck step because vuln.go.dev returned 403 Forbidden while fetching the vulnerability database — transient infrastructure on Google's side, not a code issue. This push retriggers the run.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (7dec131)

The PR has one medium-severity pagination issue; no critical or high-severity findings were reported.

Medium

• cmd/msgvault/cmd/employment.go:205 — employment list does not send limit or offset, silently restricting results to the server’s first 200 records with no way to retrieve subsequent pages.
  • Fix: Add pagination flags and forward them to both scoped requests, or automatically fetch pages until exhausted.

---

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

[wesm]

Pushed b055a8a. Confirmed: both employment list endpoints accept limit/offset but the CLI never sent them, so listings silently capped at the server's default page of 200. employment list now has --limit and --offset, forwarded to both the person- and organization-scoped requests — same flag shape as organization list. The organization-scoped test now asserts the parameters reach the server.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (b055a8a)

Organization attributes are broadly sound, but two Medium-severity API/CLI gaps block supported temporal and multi-value operations.

Medium

• internal/api/organizations.go:188 — Organization attribute requests omit active_from and active_until, preventing the HTTP API from using the temporal behavior supported by OrganizationAttributeValueInput.

  • Fix: Add both fields to SetOrganizationAttributeBody, forward them in the handler, regenerate clients, and test backdated and scheduled writes.

• cmd/msgvault/cmd/organization.go:343 — organization attribute set cannot specify an ordinal. Multi-valued attributes can only be appended, and CAS updates using --expected-value-id target a new ordinal and conflict.

  • Fix: Add an --ordinal flag and forward it through SetOrganizationAttributeBody.Ordinal.

---

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

[wesm]

Pushed 456f75f addressing both findings — verified first: OrganizationAttributeValueInput supports active_from/active_until and the person-side API body already exposes them, but the organization body dropped both; and the body had ordinal while the CLI offered no way to set it.

• SetOrganizationAttributeBody now carries active_from and active_until, forwarded to the store. The HTTP test covers a backdated write (stored active_from matches the request, not the write time) and a fully historical write (active_from+active_until in the past) that lands as history without becoming the current value.
• organization attribute set gains --ordinal (validated non-negative, forwarded through the body), so multi-valued attributes can be updated in place and --expected-value-id targets the intended ordinal instead of conflicting against a fresh one. The CLI test asserts the ordinal reaches the request body.

OpenAPI specs, Go client, and web schema regenerated; api/cmd suites and the artifact-staleness checks pass.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (456f75f)

Verdict: One medium-severity CLI issue should be addressed before merge.

Medium

• Employment dates cannot be cleared or ended employment reactivated — cmd/msgvault/cmd/employment.go:351

  Passing --start= or --end= produces a non-nil empty string that ParsePartialDate rejects. There is also no inverse of --not-current, so an ended employment cannot be marked current again.

  Suggested fix: Add explicit date-clearing flags that emit nil date pointers, plus a --current flag that sets is_current=true.

---

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

[wesm]

Pushed b7f06be for the CLI finding, and the CI failure is a repo-wide issue with a separate fix:

CLI dates and reactivation: confirmed — an empty --start/--end produced a non-nil empty string the server rejects, and nothing could flip is_current back on. employment set now has --clear-start/--clear-end (omit the date from the full-replace body) and --current (reactivate an ended employment). Guard rails: empty --start/--end fails fast pointing at the clearing flag, --current conflicts with --not-current, and --current with a retained end date asks for --clear-end instead of sending a state the store rejects. Tests cover the reactivation body and all four rejection cases.

CI: this govulncheck failure is not from the PR — Go 1.26.6 shipped stdlib security fixes (GO-2026-6089/6090/6091/6218), so every branch building with 1.26.5 now fails the vuln check, including main's next run. Fix opened separately as #614 (go.mod toolchain bump + nix flake pin); once that merges, this PR's CI goes green on re-run.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (b7f06be)

No Medium, High, or Critical findings were identified.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 22m11s

[wesm]

I'm dropping the SQL enum CHECKs in favor of validating in Go (SQLite can't alter constraints like these without rebuilding the table), would have liked to have caught these earlier and prevented them from getting into the codebase at all

[wesm]

Pushed 3a679fb: dropped the seven-value source IN (...) enum CHECK from all twenty tables per backend (person, organization, employment, relationship, and identity tables alike — this PR's tables mirrored the pattern, so the drop is repo-wide for consistency).

Rationale: a schema CHECK is a compatibility ceiling on SQLite — CREATE TABLE IF NOT EXISTS never updates constraints on existing archives and ALTER can't modify them, so extending the provenance vocabulary would mean rebuilding every table on every archive. The kinds vocabularies and the communication-service catalog already avoid database enums for exactly this reason, and provenance is validated at the Go boundary (ParseProvenance/Provenance.Valid plus the API enum tags). Kept: NOT NULL and the cross-field invariants (confidence range and declared-source scope, partial-date shape, active windows, merge self-reference).

The four tests that pinned the frozen vocabulary now prove the intended contract instead: the database accepts future provenance values while the store boundary still rejects them.

One legacy caveat: person_attribute_values shipped its CHECK in v0.19.1, so archives created on v0.19.1–v0.19.3 keep the frozen vocabulary on that single table until a rebuild migration; everything else lands open.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (3a679fb)

One medium-severity disclosure warning issue should be addressed before merging.

Medium

• Incomplete subset-export privacy warning — internal/store/subset.go:485, cmd/msgvault/cmd/create_subset.go:48
--include-profiles now exports employment histories plus referenced organizations’ profiles, contacts, and media, while --include-attributes can export organization attributes. The CLI help and runtime warning still describe only person data, so users may unknowingly disclose substantially more information when sharing subsets. Update both to explicitly list employment history, referenced organizations and their profiles/media, and organization attributes. Consider also reporting exported organization and employment counts for auditing.

---

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

[wesm]

Pushed 91e7010. Confirmed: the create-subset flag help and runtime warnings described only person data, understating what the opt-ins now export.

• --include-profiles help and warning now name employment history and the referenced organizations' profiles, contacts, and media; --include-attributes now says person and organization attribute values.
• For auditing, CopyResult gains Organizations/Employments counts, printed in the summary when --include-profiles is set. The employment subset test asserts both counts.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (91e7010)

No Medium, High, or Critical findings were identified.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 17m45s