fix: tombston in inferMemberOrganizationStintChanges (CM-1367)

[ulemons]

Summary

Fixes DE-1016/DE-1021: a human-deleted email-domain sourced org affiliation can reappear on a member profile once new matching activity comes in, because the infer-member-organization-stint-changes cron job only checked deletedAt and never deletedBy. PR #4476 (CM-1367) already fixed this for provider/enrichment-sourced affiliations, but the email-domain stint-inference path is a separate codepath that PR didn't touch. This PR closes that gap by making a human tombstone (deletedBy set) permanently block recreation of that org affiliation, regardless of whether the new activity date falls inside the deleted row's date range.

Changes

• Added deletedBy?: string to IMemberOrganization (services/libs/types/src/organizations.ts) so the field can flow through to the stint-inference logic.
• Added "deletedBy" to the SELECT column list in fetchMemberOrganizationsBySource (services/libs/data-access-layer/src/members/organizations.ts) — the cron job already fetches deleted rows via withDeleted: true, it just wasn't reading who deleted them.
• inferMemberOrganizationStintChanges (services/libs/common_services/src/services/member-organization.ts) now computes a tombstonedOrgIds set from rows where both deletedAt and deletedBy are set, and skips any stint insert/extend for those organizationIds outright. This is intentionally stronger than the existing deletedRows suppression, which only blocks recreation for activity dates that intersect the deleted row's own date range — a human delete should hold regardless of date, while a system/provider delete (no deletedBy) keeps the narrower, date-scoped suppression.

Type of change

• Bug fix
• New feature
• Refactor / cleanup
• Performance improvement
• Chore / dependency update
• Documentation

JIRA ticket

CM-13267

[cursor]

PR Summary

Medium Risk
Changes member-organization stint inference and cron queue processing for email-domain affiliations, including merge ID remapping and FK retry behavior—data correctness paths but not auth/security.

Overview
Fixes human-deleted email-domain org affiliations reappearing when new matching activity is queued. The stint-inference path now reads deletedBy and treats rows with both deletedAt and deletedBy as permanent tombstones, skipping any insert/extend for that organizationId—unlike the existing date-intersection suppression for system deletes.

The infer-member-organization-stint-changes cron job also reconciles queued org IDs before inference: merged secondaries are rewritten to their primary id via findMergedPrimaryIds, and dates for orgs that no longer exist are dropped with a warning.

Operational hardening on the same job: FK violation (23503) handling with Redis-backed retries (purge poisoned queue entries after three failures), batched affiliation-policy fetches for inserts, and cleanup of FK-retry keys on successful processing and purge.

^{Reviewed by Cursor Bugbot for commit 4b2128f. Bugbot is set up for automated code reviews on this repo. Configure here.}

[Copilot]

Pull request overview

Prevents manually deleted email-domain affiliations from being recreated during stint inference.

Changes:

• Propagates deletedBy through organization types and DAL queries.
• Skips all inferred changes for human-tombstoned organizations.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
services/libs/types/src/organizations.ts	Adds deletedBy to the affiliation type.
services/libs/data-access-layer/src/members/organizations.ts	Selects the tombstone actor field.
services/libs/common_services/src/services/member-organization.ts	Suppresses inference for tombstoned organizations.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[skwowet]

LGTM

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

[cursor]

Cursor Bugbot has reviewed your changes and found 3 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit ee67b60. Configure here.}

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Suppressed comments (1)

services/apps/cron_service/src/jobs/inferMemberOrganizationStintChanges.job.ts:164

• These lines only restate the function's behavior. Repository guidance disallows comments that merely describe what code does (CLAUDE.md:72-84); the function name already conveys the reconciliation. Remove the comment.

// Merged orgs are rewritten to their primary id instead of dropped; only genuinely
// deleted orgs are dropped, since those can never resolve to a valid target.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

services/apps/cron_service/src/jobs/inferMemberOrganizationStintChanges.job.ts:204

• A missing ID is not necessarily deleted: the organization merge workflow hard-deletes the secondary before it marks the merge action MERGED (services/apps/entity_merging_worker/src/workflows/all.ts:116-119). If this cron runs in that window, findMergedPrimaryIds returns nothing, this line classifies the ID as deleted, and the queued date is permanently acknowledged. Detect an in-progress merge and leave those values queued for retry instead of dropping them.

  const deletedOrgIds = new Set(missingOrgIds.filter((id) => !mergedPrimaryIds.has(id)))

services/apps/cron_service/src/jobs/inferMemberOrganizationStintChanges.job.ts:142

• Purging the whole member key after the third FK failure loses unrelated and concurrently added dates. The normal success path deliberately uses ackSetMembers so values added after sMembers survive, but purgeMember deletes the entire set; one poisoned organization can therefore discard valid activity for every organization on this member. Isolate/ack only the failing snapshot or offending organization and preserve newly queued values.

            await purgeMember(redis, memberId)

services/apps/cron_service/src/jobs/inferMemberOrganizationStintChanges.job.ts:179

• These lines only narrate the function's behavior and violate the repository rule that code should be self-explanatory and comments must be reserved for non-obvious invariants or external quirks (CLAUDE.md:72-84). The function name already communicates the reconciliation intent, so remove the comment.

// Merged orgs are rewritten to their primary id instead of dropped; only genuinely
// deleted orgs are dropped, since those can never resolve to a valid target.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

services/apps/cron_service/src/jobs/inferMemberOrganizationStintChanges.job.ts:196

• findOrgsByIds treats soft-deleted organizations as existing because its query has no deletedAt predicate (services/libs/data-access-layer/src/organizations/base.ts:119-132). A stale activity for such an organization therefore survives reconciliation and can create or extend an email-domain affiliation attached to a deleted profile. Query active organizations here and route soft-deleted IDs through the deleted/merged handling.

  const existingOrgIds = new Set((await findOrgsByIds(qx, orgIdsToVerify)).map((o) => o.id))

services/apps/cron_service/src/jobs/inferMemberOrganizationStintChanges.job.ts:218

• Only the first merge hop is resolved. For a valid chain such as A→B followed later by B→C, this maps queued dates for A to the now-deleted B; the insert then fails its FK and the new retry path eventually purges the dates. Resolve the chain to the current surviving primary before returning the mapping.

    .map((d) =>
      mergedPrimaryIds.has(d.organizationId)
        ? { ...d, organizationId: mergedPrimaryIds.get(d.organizationId) }
        : d,

services/apps/cron_service/src/jobs/inferMemberOrganizationStintChanges.job.ts:179

• This comment only restates the function's implementation. Project guidance in CLAUDE.md:72-84 explicitly prohibits comments that explain what code does; remove it and rely on the descriptive function and variable names.

// Merged orgs are rewritten to their primary id instead of dropped; only genuinely
// deleted orgs are dropped, since those can never resolve to a valid target.

services/apps/cron_service/src/jobs/inferMemberOrganizationStintChanges.job.ts:135

• The PR description lists only tombstone propagation, but this adds a separate failure policy that suppresses every FK violation and permanently discards the member's queued dates after three attempts. That is a significant behavioral/data-loss change and should be split into its own PR or explicitly documented with its intended constraints and validation.

        if ((err as { code?: string })?.code === '23503') {
          const constraint = (err as { constraint?: string })?.constraint
          const retryKey = fkViolationRetryKey(memberId)
          const retries = await redis.incr(retryKey)