feat(people): add person external enrichment

[salmonumbrella]

What changed

• Add a provider-neutral enrichment subsystem (Exa People and Sixtyfour People Intelligence adapters) that sends only explicitly permitted identity primitives and requested target descriptors — never chats, email bodies, meetings, or private attributes.
• Add keyed HMAC suppression that survives person deletion, is checked before credential lookup and network egress, and is rechecked against every returned identifier before any claim can commit; the pre-egress key-state check covers the full durable union of suppression and attempt-identifier keys.
• Add durable runs, work, attempts, leases, fences, request and spend caps, cost history, and uncertain-start handling so synchronous and asynchronous providers are crash-safe; async jobs resume from durable attempt state after restart.
• Resolve returned claims automatically through the PR 1 fact ledger with weak, conflicting, or unaligned identity rejected fail-closed; provider identifiers never merge people or organizations.
• Add consent, deletion/DSR, scheduler, daemon, and CLI surfaces; everything is disabled by default with exact per-provider consent, and terminal runs derive truthful state and completion timestamps from stored attempt outcomes.

Why

The person fact ledger from #677 can ingest claims from any producer, but public-web enrichment previously had no privacy-preserving path to it. This adds the opt-in external enrichment stage of the profile-maintenance design (#534): minimum egress, durable accounting, and keyed suppression make provider access auditable and revocable end to end.

Usage

msgvault person enrichment consent --provider exa
msgvault person enrichment run --person <id> --provider exa
msgvault person enrichment history <id>
msgvault person enrichment suppress-person <id> --key-env MSGVAULT_SUPPRESSION_KEY

Stacked on #677. Refs #534

[roborev-ci]

roborev: Combined Review (dc18549)

Code review identified three medium-severity issues; no high or critical findings were reported.

Medium

• Missing provider versions prevent durable completion — internal/personenrichment/sixtyfour.go:365, internal/personenrichment/exa.go:549
  Completed Sixtyfour and Exa deep-mode results omit ProviderVersion. This is copied into GenerationInput, whose validation requires a nonempty value, so these supported flows cannot commit results or reach a durable terminal state. Populate stable provider versions in both constructors and add end-to-end worker/store coverage.

• Fact projection can retire unrelated derived values — internal/store/person_fact_projection.go:1438
retireUnsupported closes every non-declared current attribute absent from current ledger decisions, including extraction/system values never projected by the fact ledger. A malformed or below-threshold claim can therefore remove an unrelated value without producing a replacement. Retire only rows owned by a previously applied person-fact decision and preserve non-ledger projections.

• Mode-incompatible identities can trigger guaranteed-to-fail paid requests — internal/personenrichment/request.go:82
  Request construction accepts any allowed identity even though deep modes require a public-profile URL, and Exa people mode requires either that URL or both name and company. Structurally incapable configurations can still trigger paid requests whose results must be rejected. Validate mode-specific identity requirements before egress.

---

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

[roborev-ci]

roborev: Combined Review (8ac86c0)

Code review found four medium-severity issues involving credential forwarding, attempt handling, provider validation, and suppression enforcement.

Medium

• cmd/msgvault/cmd/person_enrichment.go:51 — Enrichment credentials are not forwarded through the daemon

  Daemon-proxied enrichment commands forward no environment variables, while cliRunEnvAllowed omits enrichment provider and suppression key names. Keys exported only in the invoking shell never reach person enrichment run, causing repeated credential-unavailable failures.

  Fix: Forward the selected provider’s configured api_key_env and the suppression-key environment variable, and add those configured names to the server allowlist.

• internal/personenrichment/worker.go:350 — Transient attempt errors are incorrectly treated as terminal policy rejections

  Every BeginAttempt error triggers a terminal "policy" release, including transient database, request-conflict, fencing, and accounting-state failures. If release succeeds, the work is permanently consumed and the infrastructure error is hidden.

  Fix: Return a typed policy or budget outcome from BeginAttempt. Release terminally only for expected policy outcomes; propagate or retry infrastructure and fencing failures without consuming the work.

• internal/personenrichment/sixtyfour.go:346 — Profile-URL-only requests consume attempts without making an HTTP request

  Request construction accepts public_profile_url as the sole identity, but the Sixtyfour adapter ignores profile URLs. The adapter rejects the request only after creating a durable attempt and budget reservation, and the validation failure is recorded as non-replayable uncertain_start.

  Fix: Validate Sixtyfour-compatible identity fields before BeginAttempt, or include public profile URLs in lead_info.

• internal/personenrichment/suppression.go:508 — Single-field identities bypass enrichment opt-outs

  Suppression digests cover a name only when current_company is also present, but Sixtyfour accepts and transmits either field alone. A name-only profile can therefore be explicitly suppressed yet still pass the egress gate and be sent externally.

  Fix: Reject outbound requests without a durable suppression binding or add a person-level suppression tombstone checked before every provider request. Make the suppression command fail when it cannot establish an effective egress barrier.

---

Reviewers: 2 done | Synthesis: codex, 13s | Total: 20m39s

[roborev-ci]

roborev: Combined Review (bf9d2ff)

Review identified four medium-severity issues that can cause rejected enrichments, repeated paid work, stale-sync corruption, or unsafe automatic claims.

Medium

• Sixtyfour enrichments are always identity-rejected — internal/personenrichment/sixtyfour.go:414
  Completed results never populate IdentityMatches or ProviderPersonIDs, so AssessIdentity rejects every paid enrichment and no claims are applied. Bind results to a validated provider-returned identifier and populate the identity fields, or disable Sixtyfour enrichment until identity can be verified safely.

• Batch budgeting can trigger repeated paid requests without progress — internal/peoplesweep/worker.go:462
maxBatches is applied independently across lanes and cursor modes, and context may add anchored batches. The worker can exceed its per-person budget, execute paid requests, then fail a later reservation before applying claims or advancing cursors; retries repeat the charges. Enforce one global batch limit and reserve the complete batch set before contacting providers, releasing all reservations if preflight fails.

• Some scoped sync mutations bypass the generation fence — internal/store/conversations.go:385, internal/store/messages.go:2269
  Conversation metadata/member-count updates, label removal, and reply linking execute directly on s.db. A superseded importer can overwrite newer metadata, remove labels, or replace reply relationships. Route these mutations through fenced transactions, validate source ownership, and make metadata read-modify-write operations atomic.

• Unauthenticated email senders can become auto-applicable self evidence — internal/store/person_sweep_evidence.go:846
  Any matching RoleFrom address receives DirectSelf and an identity score of 1000 without considering SPF, DKIM, or DMARC. A spoofed From header can therefore produce false claims above the automatic-application threshold. Propagate sender-authentication provenance and reserve high-confidence DirectSelf evidence for authenticated senders; otherwise require review or authenticated corroboration.

---

Reviewers: 2 done | Synthesis: codex, 13s | Total: 24m50s

[roborev-ci]

roborev: Combined Review (1a3e82e)

The PR has one high-severity retry flaw and three medium-severity issues involving consent enforcement, suppression policy, and sweep batching.

High

• Budget exhaustion permanently blocks enrichment retries — internal/personenrichment/worker.go:350
  Budget exhaustion records a terminal attempt tied to the globally unique request hash and deletes the work. When catch-up recreates the same request after a per-run or per-day budget reset, BeginAttempt rejects the reused hash, permanently preventing enrichment for that unchanged revision.
  Fix: Defer budget-rejected work until the next eligible run or day without creating a terminal hash-bound attempt. Add coverage proving that the same revision succeeds in a subsequent run.

Medium

• Revoked consent can still result in external PII disclosure — internal/personenrichment/worker.go:459
  Consent is checked before the attempt is persisted, but revocation can occur after BeginAttempt and before provider.Start. Although revocation cancels the stored attempt, the worker can still send names, email addresses, or phone numbers to the external provider. Rejecting the response later cannot undo the disclosure.
  Fix: Make dispatch authorization atomic with consent state. Immediately before provider.Start, transition the attempt to a dispatch-authorized state under the consent/revocation fence, and invalidate pending dispatch authorization during revocation.

• URL-based enrichment conflicts with suppression rules — internal/personenrichment/suppression.go:498
  Exa accepts a public-profile URL without a complete name/company pair, but the suppression gate rejects requests containing only one of those optional fields. A profile with a URL and name but no company passes request validation, fails authorization, and is repeatedly rescheduled as a transient failure.
  Fix: Align request construction and suppression behavior by omitting incomplete optional pairs when the URL supplies identity binding, supporting independent suppression, or rejecting incompatible profiles during configuration validation.

• Configured change batch size is ignored — internal/peoplesweep/worker.go:464
change_batch_size is validated and defaulted but never used. The archive window is instead capped by the maximum model-batch count, reducing the default window from 256 changes to four and potentially creating a severe sweep backlog.
  Fix: Derive WindowLimit from ChangeBatchSize and apply HistoricalMessageCap separately to historical processing.

---

Reviewers: 2 done | Synthesis: codex, 14s | Total: 19m59s

[roborev-ci]

roborev: Combined Review (48e6206)

The review identified four Medium-severity issues involving unsafe work claiming, unbounded retrieval, stale sync writes, and prompt injection.

Medium

• Manual enrichment can process unrelated work — internal/store/person_enrichment_work.go:257
  Manual runs may claim unassigned due work for every enabled provider, causing a command targeting one person and provider to make unrelated external requests and consume its run budget. Require manual work to match person_enrichment_manual_run_targets and already have w.run_id = runID; construct the worker with only the requested provider.

• Out-of-scope documents can abort people sweeps — internal/peoplesweep/retrieval.go:143 (fatal handling at line 171)
  Document search is not restricted to CandidateMessageIDs, yet results outside that set are treated as fatal. Pass candidate IDs into DocumentContextRequest, constrain the query before ranking, and test that out-of-scope documents are excluded without failing retrieval.

• Import-item updates bypass the sync-generation fence — internal/store/sync.go:798
UpsertSourceImportItem writes directly through s.db, allowing a superseded Drive import to overwrite a newer run’s tracker entry and trigger unnecessary reimports. Use a source-scoped fenced transaction, validate item.SourceID, and return ErrSyncRunSuperseded for stale writers.

• External enrichment can be promoted as archive-backed facts through prompt injection — internal/peoplesweep/program.go:166, internal/peoplesweep/packet.go:204
  Attacker-controlled enrichment values are inserted verbatim into the extraction prompt and can induce arbitrary output citing unrelated valid evidence IDs. Keep external unresolved values out of the archive-extraction prompt and reconcile claims deterministically, or require human approval or deterministic value-to-excerpt validation before applying influenced claims.

---

Reviewers: 2 done | Synthesis: codex, 12s | Total: 21m47s

[wesm]

rebasing

[roborev-ci]

roborev: Combined Review (60acf6f)

Verdict: Several high-severity authorization and correctness issues, plus medium-severity race and configuration flaws, remain.

High

• Enrichment command environment variables are rejected by the daemon allowlist. The CLI forwards API and suppression-key variables, but person enrichment run and person enrichment suppress cannot use them through the normal daemon path. Add precise allowlist rules.
  Locations: cmd/msgvault/cmd/person_enrichment.go:298-299, internal/api/cli_handlers.go:1272-1280

• Profile canonicalization invalidates built-in employment descriptors. Sorting fields and choices without recomputing descriptor revisions causes revision validation failures and changes required employment field ordering. Share canonicalization/revision logic with personfacts, or preserve catalog ordering and compute revisions from the canonical form.
  Location: internal/personenrichment/profile.go:269-280

• Untracking does not fence already leased work. Leased work can continue, and final dispatch authorization checks consent but not active tracking, allowing identity data to be sent after untracking. Fence leased work and require active tracking at final authorization.
  Locations: internal/store/person_tracking.go:127-151, internal/store/person_enrichment_work.go:835-843

• Suppression can race with dispatch. Suppression is checked before attempt creation but not atomically immediately before the provider call, allowing a concurrent suppression to be bypassed. Revalidate disclosed identifiers under the same locks used by suppression writes.
  Location: internal/store/person_enrichment_work.go:835-849

Medium

• Fresh enrichment triggers can be lost by ReleaseWork. Policy- or suppression-terminal work is unconditionally deleted even if a concurrent trigger sets has_fresh_trigger. Use the shared settlement/requeue logic or delete only when no fresh trigger exists.
  Location: internal/store/person_enrichment_work.go:435-485

• Provider configuration does not validate Exa mode requirements. Invalid identifier combinations—such as email-only people enrichment or deep modes without a public profile URL—can pass configuration validation but fail every request before egress. Validate identifiers against each provider mode’s requirements.
  Location: internal/personenrichment/config.go:173-197

• Consent is not rechecked before polling. An asynchronous attempt checks consent before provider creation, but pollAttempt can issue later authenticated provider.Poll requests after consent is revoked. Recheck consent before every provider call and fence or cancel active attempts on revocation where supported.
  Location: internal/personenrichment/worker.go:579-580

---

Reviewers: 2 done | Synthesis: codex, 15s | Total: 29m0s

[roborev-ci]

roborev: Combined Review (c2015d4)

Verdict: Two medium-severity issues require attention.

Medium

• Terminal attempts with the same request hash are not safely replayed. Across runs, BeginAttempt returns a request-hash conflict; within the same run, it returns the terminal attempt with created=false, which the worker ignores and tries to dispatch. The work remains leased, causing scheduled runs to fail repeatedly. Treat terminal request-hash matches as consumed work or define a new-attempt policy, and handle created=false before dispatching.
  Locations: internal/store/person_enrichment_work.go:566-579, internal/personenrichment/worker.go:341-361

• Suppression can race with enrichment dispatch. Suppression is checked before beginning the attempt, but dispatch authorization does not recheck the attempt’s disclosed identifier digests. A suppression can commit after the initial check and before provider dispatch, allowing newly suppressed identifiers to be sent externally. Recheck suppressions atomically during dispatch authorization and abort the attempt when any digest is suppressed.
  Locations: internal/store/person_enrichment_work.go:819-854, internal/personenrichment/worker.go:269-293, internal/personenrichment/worker.go:472-480

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 30m54s

[roborev-ci]

roborev: Combined Review (430a55b)

Verdict: Medium-severity issues remain in credential forwarding, suppression/deletion safety, identity invalidation, and dispatch concurrency.

Medium

• internal/api/cli_handlers.go:1272-1280 — The CLI environment allowlist rejects person enrichment ... despite forwarding provider and suppression credentials. Add an enrichment-specific allowlist for configured credential variables, with behavioral coverage.

• cmd/msgvault/cmd/person_enrichment.go:480-491 — Digest suppression uses the caller’s suppression key but proxies the command without forwarding it, preventing daemons lacking that key from validating or persisting the digest. Forward it through the restricted environment path.

• cmd/msgvault/cmd/serve.go:2191-2194 — When enrichment is disabled, person deletion omits current identifiers from durable suppression records if enrichment was previously used, allowing later reimport. Preserve enrichment-aware deletion when history exists or make identifier suppression independent of current enablement.

• internal/store/person_enrichment_work.go:1717-1749 — Participant identity changes, including linking and display-name backfills, do not consistently publish or invalidate enrichment identity work, allowing stale names, emails, and phone numbers in requests. Cover every participant mutation affecting a bound person.

• internal/personenrichment/worker.go:472-480 — Suppression or deletion can commit after dispatch authorization but before provider.Start(request), allowing identifiers to be sent without a fresh suppression check. Coordinate pending dispatches with suppression/deletion and recheck the original disclosed digests at result commit.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 37m33s

[roborev-ci]

roborev: Combined Review (bdeedaa)

Verdict: High-severity privacy issue remains, along with a medium-severity enrichment scheduling issue.

High

• Leased enrichment attempts can bypass tracking and suppression changes before provider dispatch. Untracking does not cancel active leased attempts, and the final dispatch authorization checks consent but not current tracking status or disclosed-identifier suppressions. A worker may therefore send identifiers to an external provider after tracking is disabled or suppression/opt-out is committed.
  Locations: internal/store/person_tracking.go:127-151, internal/store/person_enrichment_work.go:819-849
  Fix: Fence or revoke active attempts during untracking/suppression changes, and revalidate tracking and suppression state immediately before dispatch and commit.

Medium

• Participant identity changes can invalidate in-flight enrichment without scheduling replacement work. Enrichment request identity includes mutable names, emails, and phone numbers, but participant identity updates only bump revisions and do not publish enrichment work for linked people.
  Location: internal/store/person_enrichment_work.go:1722-1755
  Fix: Publish enrichment work for every linked person when identity changes, or remove mutable participant fields from the durable enrichment input.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 42m45s

[roborev-ci]

roborev: Combined Review (8aa2cdb)

Verdict: Three medium-severity issues require attention.

Medium

• Identity-binding mutations do not requeue enrichment work
  internal/store/persons.go:113-131, internal/store/participant_links.go:426-445, internal/store/person_splits.go:337-340
  Person revisions are bumped without invalidating and republishing enrichment work, so existing work may be fenced as stale without fresh work being scheduled. Invoke the enrichment invalidation/rep publication helper after these mutations.

• Deletion suppression is incomplete when enrichment is disabled
  cmd/msgvault/cmd/serve.go:2191-2194
  Person deletion records only identifiers from prior enrichment attempts, allowing previously unsubmitted identity data to be sent if the person is later recreated after enrichment is enabled. Keep suppression current-identity-aware regardless of scheduler state, or fail closed without a suppression key.

• Manual enrichment ignores the global disabled setting
  cmd/msgvault/cmd/person_enrichment.go:311-368
  The manual command can create a worker and make provider requests when enrichment is globally disabled. Reject manual runs unless an explicit override is intended and documented.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 36m37s

[roborev-ci]

roborev: Combined Review (335748e)

Verdict: Three medium-severity issues require attention.

Medium

• internal/personenrichment/sixtyfour.go:319-322 — Sixtyfour accepts employment targets during configuration but rejects them only after creating an attempt. The worker treats the deterministic codec error as an uncertain start, permanently stranding the work. Validate target kinds before creating attempts or add employment serialization support.

• internal/store/participant_links.go:431-443 — Adding a participant bumps the person revision without invalidating stale enrichment identities or queuing replacement work. In-flight results may be rejected with no subsequent enrichment run. Invoke the post-revision enrichment identity invalidation helper when bindings change.

• internal/store/person_splits.go:337-338 — Splitting a person bumps revisions without invalidating the source person’s enrichment identities or scheduling replacement work, leaving stale results attached. Run enrichment identity invalidation and publication for the source person after the revision bump.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 26m26s

[roborev-ci]

roborev: Combined Review (640f156)

Verdict: Four issues require attention, including one high-severity environment allowlist bug.

High

• internal/api/cli_handlers.go:1272-1281 — The command-specific environment allowlist covers only person provider check and person sweep run. Credentials for person enrichment run and person-level suppression are rejected as env_not_allowed. Add the required enrichment commands and restrict them to configured provider and suppression environments.

Medium

• cmd/msgvault/cmd/serve.go:2859-2863 — Work for disabled or removed providers remains nonterminal but can no longer be claimed, causing scheduled runs to remain permanently active. Terminalize or cancel this work, including active attempts, or provide a worker path for persisted profiles no longer enabled.

• internal/personenrichment/worker.go:241-256 — Policy/profile drift uses releaseTerminalBeforeAttempt even when an active attempt exists. The release is rejected, leaving the attempt nonterminal and repeatedly reclaimable. Use active-attempt settlement/terminalization when an attempt ID is persisted.

• internal/personenrichment/profile.go:180-190 — Exa people profiles accept unsupported catalog targets. Validation occurs after attempt and budget reservation, and the resulting local error is classified as uncertain_start, preventing replay and run completion. Validate targets during profile construction or classify local preflight failures as deterministic policy failures.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 23m27s

[roborev-ci]

roborev: Combined Review (d8c4930)

Verdict: One medium-severity issue was identified; another reviewer found no issues.

Medium

• Suppression checks may be bypassed after authorization: internal/personenrichment/suppression.go:516-520, internal/store/person_enrichment_work.go:666-667
  Known provider person IDs are checked by the egress gate but omitted from durable attempt identifiers. If suppression is added after the initial check but before dispatch, dispatch cannot recheck the ID, and suppression invalidation cannot find the active attempt, allowing the provider request to proceed after opt-out.
  Fix: Persist provider-ID suppression digests used during authorization, include them in dispatch rechecks, and use them for suppression invalidation.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 30m54s

[roborev-ci]

roborev: Combined Review (e3c41e8)

Verdict: 3 issues found—1 High and 2 Medium severity.

High

• internal/store/person_sweep_commit.go:579-585 — The remaining-work check ignores optimistic_document_key and reconcile_document_key. After a partial multi-page document commit, continuation keys may remain while the sequence/upper key is unchanged, causing the work row to be deleted and later chunks to be skipped. Treat non-empty continuation keys as remaining work and add regression coverage.

Medium

• internal/store/messages.go:752-770 — EnsureConversation writes through s.db, bypassing the sync-generation fence used by scoped stores. A superseded sync can create an empty conversation even though its subsequent message write is rejected. Route conversation creation through the transaction-scoped generation-fencing path.

• cmd/msgvault/cmd/person_enrichment.go:320 — Manual enrichment runs bypass the global [people.enrichment].enabled = false safeguard. runPersonEnrichmentManual checks only provider enablement, allowing configured credentials and prior consent to send personal data externally. Enforce config.Enabled before creating work or dispatching provider requests.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 37m18s

[roborev-ci]

roborev: Combined Review (ebaac64)

Verdict: Three medium-severity issues require remediation before merge.

Medium

• Unresolved uncertain enrichment starts
  Locations: internal/personenrichment/worker.go:690-695; internal/store/person_enrichment_work.go:337-343
  Ambiguous provider starts are marked uncertain_start, excluded from claiming, and left nonterminal. Without reconciliation or operator resolution, manual runs may return while still running, and scheduled runs may remain stuck indefinitely.
  Fix: Add reconciliation/resolution, safe provider lookup, or durable terminalization so the owning run can complete.

• Deletion can fail to preserve current suppression identifiers when enrichment is disabled
  Location: cmd/msgvault/cmd/serve.go:2191-2194
  API person deletion bypasses the enrichment-aware deletion path and may copy only digests from past attempts. Identifiers changed since the last enrichment therefore may not be suppressed if enrichment is later re-enabled.
  Fix: Preserve current-identifier suppression digests whenever enrichment history exists, or fail explicitly when safe recording is impossible.

• Deletion can race the final outbound provider request
  Location: internal/store/persons.go:251
deletePersonOnce does not acquire the enrichment-authority fence. A worker can authorize dispatch, be paused before provider.Start, and then send the person’s identifiers after deletion or enrichment withdrawal commits.
  Fix: Invalidate or cancel dispatch leases during deletion and enforce a durable final-send fence immediately before provider egress. Add a concurrency test for this pause window.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 33m46s

[roborev-ci]

roborev: Combined Review (f39a572)

Verdict: One high-severity identity risk and one medium-severity work-loss race remain.

High

• internal/personenrichment/exa.go:691-697 — Historical employment entries can match IdentifierCurrentCompany, allowing a same-name former employee to pass identity verification and receive enrichment intended for someone else. Restrict matching to explicitly current employment or entries without an end date, and test the former-employer case.

Medium

• internal/store/person_enrichment_work.go:517-567 — Terminal release with no active attempt unconditionally deletes the work row. If a new trigger is coalesced onto the leased row, has_fresh_trigger is discarded and the new work is lost. Preserve or detach rows with has_fresh_trigger using the same conditional settlement behavior as active-attempt terminalization.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 37m1s

[roborev-ci]

roborev: Combined Review (6bb9efd)

Verdict: One medium-severity correctness issue identified; no high or critical findings.

Medium

• internal/store/person_enrichment_work.go:1954-1966 — LoadRequestInput omits persons.display_name, so people whose only curated name is display_name cannot satisfy name-and-company provider requests, and the name is absent from deletion-suppression digests. Include Person.DisplayName as a normalized, deterministically prioritized identity candidate and cover request/deletion behavior.

---

Reviewers: 2 done | Synthesis: codex, 6s | Total: 20m19s

[roborev-ci]

roborev: Combined Review (2cc2ed0)

Verdict: Three medium-severity issues require fixes before merge.

Medium

• internal/personenrichment/worker.go:499-511 — Synchronous provider results are persisted as a pending attempt without storing the result. A crash before completeAttempt commits can cause recovery to mark the successful attempt uncertain and lose the result. Persist synchronous completion and its result durably before acknowledgment, or add recovery for this intermediate state.

• internal/store/person_enrichment_runs.go:362-377 — Policy-only terminal attempts use state = 'terminal' and are counted as failures, despite completion logic treating policy outcomes as successful runs. Exclude policy-classified terminal attempts from failure counts or classify policy outcomes separately.

• internal/store/persons.go:528 — UpdatePersonDisplayNameContext can publish new enrichment work without rejecting or invalidating an already-authorized dispatch, allowing the provider to receive a stale display name. Apply the established dispatch-fencing and identity-invalidation path transactionally before publishing work.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 23m34s

[roborev-ci]

roborev: Combined Review (8deb4ad)

Verdict: One medium-severity retry-accounting issue remains.

• Medium — internal/store/person_enrichment_work.go:498-501
  Retrying an active enrichment attempt clears its lease and schedules retry_wait without incrementing attempt_count. Persistent failures can bypass MaxRetries and leave work pending indefinitely. Increment the retry counter and enforce the retry limit consistently, preferably via a shared retry helper.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 29m39s

[roborev-ci]

roborev: Combined Review (b4822d9)

Verdict: One medium-severity retracking bug requires attention; no material security regressions were identified.

Medium

• Retracking an unchanged person can prevent re-enrichment — internal/store/person_tracking.go:142-146; internal/store/person_enrichment_work.go:731-742
  Untracking terminalizes the existing enrichment attempt, but retracking the unchanged person republishes the same revision-based request hash. BeginAttempt replays the old terminal attempt instead of creating a new one, leaving tracked work unable to dispatch enrichment again.
  Suggested fix: Add a unique enrollment generation to the tracked trigger/request hash, or restrict replay to reusable non-terminal attempts and create a fresh attempt after re-enrollment.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 42m16s

[roborev-ci]

roborev: Combined Review (e0ee5ba)

Verdict: One medium-severity correctness issue found; no security issues identified.

Medium

• In-flight enrichment results can be discarded after unrelated person updates. Enrichment attempts use the global person revision as a fence, but updates to addresses, dates, categories, or media can increment that revision without triggering new enrichment. The in-flight result is then marked stale, and terminal settlement deletes the work without retrying it. Consider using an enrichment-specific revision/payload fence or preserving and re-queuing work on revision mismatch.
  Locations: internal/personenrichment/worker.go:257-261; internal/store/person_enrichment_results.go:456-459

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 27m15s

[roborev-ci]

roborev: Combined Review (fbab20c)

Verdict: High-severity lifecycle race and two Medium-severity authorization/idempotency issues remain.

High

• internal/store/person_enrichment_work.go:236 — Profile cleanup does not acquire the enrichment authority lock, allowing an attempt to be terminalized after dispatch authorization commits but before provider.Start. Data may be sent to a provider whose profile was disabled or removed. Serialize cleanup with dispatch authorization or add a durable cancellation fence immediately before dispatch.

Medium

• internal/personenrichment/worker.go:586-587 — The poll path authorizes egress once, then may call provider.Poll after consent or suppression is revoked because actively leased attempts remain uncancelled. Add a durable poll-authorization fence immediately before each provider call.

• internal/store/person_enrichment_runs.go:81 — Manual idempotency replay validates the current person and authorization before checking the existing key. Replaying a key after person deletion returns ErrPersonNotFound, despite historical idempotency scope being preserved. Resolve existing keys before requiring the person or current authorization; apply authorization checks only when creating a new run.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 26m4s

[roborev-ci]

roborev: Combined Review (183896b)

Reviewed the 30-commit change set for concrete vulnerabilities across enrichment, consent, credential handling, provider responses, identity fencing, and external input paths.

No issues found.

---

Reviewers: 2 done | Synthesis: codex | Total: 52m18s

[roborev-ci]

roborev: Combined Review (a8ab32f)

Verdict: One medium-severity reliability issue found; no critical or high-severity issues.

Medium

• internal/store/person_enrichment_work.go:392-405, internal/store/person_enrichment_work.go:1040-1063 — Reclaiming an expired pending attempt preserves dispatch_authorized_at. If a worker stops after authorization but before Poll completes, the reclaimed attempt cannot pass AuthorizeAttemptPoll, permanently stranding the provider job. Clear dispatch_authorized_at during reclamation and add a regression test for this interruption point.

---

Reviewers: 2 done | Synthesis: codex, 5s | Total: 24m46s

[roborev-ci]

roborev: Combined Review (2f33283)

Verdict: One medium-severity concurrency issue requires attention.

Medium

• internal/store/person_enrichment_triggers.go:167-284 — EnqueueDuePersonEnrichmentContext checks consent/tracking before upserting work but does not acquire the enrichment authority lock. A concurrent consent revocation or untracking can delete the work and commit before this transaction recreates it, leaving queued work for an unauthorized person. Acquire the authority mutation lock before catch-up reads and upserts, or revalidate authorization under the same fence before publishing each work item.

---

Reviewers: 2 done | Synthesis: codex, 6s | Total: 1h8m34s

[roborev-ci]

roborev: Combined Review (c553bae)

No issues found.

---

Reviewers: 2 done | Synthesis: codex | Total: 41m37s

[roborev-ci]

roborev: Combined Review (d614592)

Verdict: Two medium-severity consistency issues remain; no critical or high-severity findings were reported.

Medium

• internal/store/person_enrichment_work.go:2151-2168 — Organization rename, retirement, and merge paths do not advance affected person revisions or invalidate enrichment work. In-flight requests may commit stale company identities, and fresh enrichment may not be queued after completed work. Route organization mutations through enrichment invalidation/fencing and enqueue fresh identity work, or validate against an organization/company revision.

• internal/store/carddav_resources.go:905-1013 — CardDAV rebase and retirement mutate person identities without invoking enrichment invalidation and dispatch fencing. Stale provider identities remain available, and already-dispatched requests are not immediately fenced. Route these mutations through the shared invalidation path, including provider-identity cleanup and fresh-work publication.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 38m22s

[roborev-ci]

roborev: Combined Review (e23abda)

Verdict: Two medium-severity issues were identified; no high- or critical-severity findings were reported.

Medium

• internal/personenrichment/exa.go:305-307 — Outbound Exa queries include profile URLs that receive only generic normalization, allowing HTTP and arbitrary hosts, including private or internal addresses. Enforce Exa’s safe URL policy before provider dispatch.

• internal/store/person_enrichment_triggers.go:568-574 — Authorized starting attempts are treated as permanently in progress without checking lease expiry. Add fenced recovery for expired attempts before rejecting cleanup or mutations.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 35m54s

[roborev-ci]

roborev: Combined Review (96b009a)

Verdict: One medium-severity data-consistency issue requires attention.

Medium

• Person deletion and absorbed-person merges can erase nonterminal enrichment attempts (internal/store/persons.go:276-437, internal/store/person_merges.go:437-452).
  • ON DELETE CASCADE removes in-flight or pending provider work without first calling reconcilePersonEnrichmentCostTx, potentially leaving run counters and derived counts inconsistent and preventing later results or charges from being reconciled.
  • Cancel or terminalize nonterminal attempts under the existing authority/person locks, reconciling reserved and actual costs before deletion.

---

Reviewers: 2 done | Synthesis: codex, 6s | Total: 56m25s

[roborev-ci]

roborev: Combined Review (20f8fc8)

Verdict: One medium-severity validation bug found; no security vulnerabilities identified.

Medium

• cmd/msgvault/cmd/person_enrichment.go:242 — Argument validation is inverted: revoking a single fingerprint always fails, while --all is accepted. Require exactly one argument without --all, and zero arguments with --all.

---

Reviewers: 2 done | Synthesis: codex, 5s | Total: 42m26s