fix(script-executor): schedule member email deduplication sweep

[ramanathan1504]

Fixes #4484

findAndMergeMembersWithSameVerifiedEmailsInDifferentPlatforms is registered in workflows.ts but nothing triggers it — no schedule, no cron job, no infra reference — so duplicate members accumulate until someone runs a sweep by hand.

Change

• Registers the workflow as a weekly schedule (Sunday 03:00, SKIP overlap, since a full sweep can outlast the interval).
• Adds mergeMembersIfAllowed, which checks memberNoMerge first and carries the absorbed member's no-merge edges onto the survivor, so a separated pair cannot rejoin directly or transitively through a third member. Mirrors mergeIfAllowed in data_sink_worker; the existing mergeMembers is untouched for the other workflows.
• Isolates per-couple failures: merge() throws 409 while a member has an IN_PROGRESS merge action and finishMemberMerging clears that asynchronously, so in transitive duplicate groups one couple could abort the entire sweep.
• Adds a dryRun arg to log a full pass without merging before the schedule goes live.

Detection is unchanged — the query still requires verified identities on both sides, so this adds no new trust in self-asserted commit emails.

No JIRA key on the title, so that check will warn; happy to retitle.

[cursor]

PR Summary

High Risk
Automated member merges affect core identity data and run on a schedule; guards reduce bad merges but incorrect no-merge migration or cursor logic could skip or mis-merge pairs at scale.

Overview
Automated verified-email deduplication is wired up by registering findAndMergeMembersWithSameVerifiedEmailsInDifferentPlatforms on a weekly Temporal schedule (Sunday 03:00, overlap SKIP) from script_executor_worker startup.

The sweep workflow now uses mergeMembersIfAllowed instead of unconditional mergeMembers: it skips when either member is missing, when a memberNoMerge edge blocks the pair, or when merge returns 409 (merge already in progress), so one blocked couple does not abort the whole run. dryRun logs would-be merges without writing.

Merge correctness: CommonMemberService.merge calls new moveMemberNoMerge so no-merge relationships on the absorbed member are re-pointed to the survivor (direct and transitive blocks). findExistingMemberIds supports the existence check in the activity layer.

Pagination for duplicate pairs switches from hash-based afterHash to lexicographic afterHighMemberId / afterLowMemberId cursors in the repo query and continueAsNew args.

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

[CLAassistant]

All committers have signed the CLA.

[Copilot]

Pull request overview

Schedules the existing verified-email member deduplication workflow to run weekly.

Changes:

• Adds a Sunday 03:00 Temporal schedule using SKIP overlap behavior.
• Registers the schedule during worker initialization.

Reviewed changes

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

File	Description
scheduleMemberDeduplication.ts	Defines the recurring deduplication schedule.
main.ts	Registers the schedule at startup.

Suppressed comments (2)

services/apps/script_executor_worker/src/schedules/scheduleMemberDeduplication.ts:17

• The overlap-policy rationale is useful, but allowed comments are limited to two lines. Please condense this performance constraint while preserving why SKIP must not be changed casually.

        // The workflow walks the whole memberIdentities table via continueAsNew, so a single
        // sweep can outlast the interval. Skip an overdue run instead of buffering it, so
        // sweeps never stack up on top of each other.

services/apps/script_executor_worker/src/schedules/scheduleMemberDeduplication.ts:30

• This comment only narrates the empty initial argument and workflow paging behavior. That behavior is already clear from args: [{}] and the workflow contract, so this violates the repository's no-descriptive-comments guideline.

        // Start from the beginning of the hash ordering; the workflow pages itself from there.

---

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

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

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

Suppressed comments (2)

services/apps/script_executor_worker/src/schedules/scheduleMemberDeduplication.ts:29

• This comment only restates the empty initial cursor represented by the argument, which the repository guidelines explicitly disallow. The workflow and argument names already make this behavior discoverable, so please remove the comment.

        // Start from the beginning of the hash ordering; the workflow pages itself from there.

services/apps/script_executor_worker/src/schedules/scheduleMemberDeduplication.ts:16

• This allowed scheduling-constraint comment exceeds the repository's two-line maximum. Please condense it while preserving the reason for using SKIP.

This issue also appears on line 29 of the same file.

        // The workflow walks the whole memberIdentities table via continueAsNew, so a single
        // sweep can outlast the interval. Skip an overdue run instead of buffering it, so
        // sweeps never stack up on top of each other.

[Copilot]

Pull request overview

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

Suppressed comments (1)

services/apps/script_executor_worker/src/workflows/findAndMergeMembersWithSameVerifiedEmailsInDifferentPlatforms.ts:44

• Each call returns after starting an asynchronous finishMemberMerging workflow, so this 1,000-item page can enqueue up to 1,000 heavy merge workflows and immediately continue to the next page. The other bulk merge workflows explicitly cap running finishMemberMerging workflows at 20 (cleanup/duplicate-members.ts:19-32, processLLMVerifiedMerges.ts:20-33) to avoid overloading PostgreSQL. Add equivalent backpressure and use a bounded batch size before enabling this recurring sweep.

      await common.mergeMembersIfAllowed(couple.primaryMemberId, couple.secondaryMemberId)

[ramanathan1504]

On rollout — the schedule is registered live (args: [{}]), so the first execution after deploy sweeps the whole memberIdentities table and merges every match in one pass. That first run is the risky one, not the weekly steady state.

If you'd rather see the blast radius first, the workflow now accepts dryRun:

temporal workflow start \
  --task-queue script-executor \
  --type findAndMergeMembersWithSameVerifiedEmailsInDifferentPlatforms \
  --input '{"dryRun": true}'

It still pages through the full table via continueAsNew, but logs every couple it would merge without touching anything — so you get a count and a sample before the first Sunday run. Happy to set the schedule's args to {"dryRun": true} for the initial release and drop it in a follow-up, if you'd prefer that ordering.

Two notes:

• The branch is rebased on current main (so it carries fix: skip unverified identity inserts when another member owns it (CM-1365) #4486 and fix: tombstone public API work experience deletes (CM-1383) #4487).
• pnpm isn't available in my environment, so tsc-check, lint and prettier haven't been run locally — CI is the first real compile here. I'll fix whatever it flags.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Suppressed comments (1)

services/apps/script_executor_worker/src/activities/common.ts:61

• The expected in-progress 409 is rethrown across the activity boundary, so Temporal applies the configured six activity attempts before the workflow can skip this couple. With the 3× backoff, each transitive collision can add roughly two minutes; a page containing many overlapping couples can therefore stall the weekly sweep for hours. Handle this known conflict inside the activity and return a skipped result, while continuing to throw unexpected failures.

  } catch (error) {
    svc.log.error({ err: error, primaryMemberId, secondaryMemberId }, 'Failed to merge members')
    throw error

[Copilot]

Pull request overview

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

Suppressed comments (1)

services/libs/data-access-layer/src/member_merge/index.ts:64

• This predicate scans both orientations, but the only index on memberNoMerge is the primary key ("memberId", "noMergeId") (V1666966941__initial.sql:685-689); there is no index starting with noMergeId. Consequently each merge can sequentially scan the whole no-merge table here, after getMemberNoMerge already performed the same unindexed lookup for every sweep candidate. Add an index on "noMergeId" (ideally via a concurrent migration) before putting this path into the weekly sweep.

        from "memberNoMerge"
        where "memberId" = $(fromMemberId) or "noMergeId" = $(fromMemberId)

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

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

[Copilot]

Pull request overview

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

Suppressed comments (1)

services/libs/data-access-layer/src/old/apps/script_executor_worker/member.repo.ts:46

• The grouping fix still leaves pagination lossy: multiple canonical pairs can share the 32-bit hash, but the next page filters only with hash < afterHash. If LIMIT 1000 splits a collision, the remaining pairs are skipped. This is not reliably self-healing because no-merge and in-progress couples intentionally remain in the result set and can recreate the same boundary every week. Use (hash, leastMemberId, greatestMemberId) as the deterministic order and continuation cursor.

    group by
        Least(a."memberId", b."memberId"),
        Greatest(a."memberId", b."memberId"),
        hash

[Copilot]

Pull request overview

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

[ramanathan1504]

Verification update — I now have a working local toolchain, so the earlier "CI is the first real compile" caveat no longer applies.

Static checks, clean across all three touched workspaces (script_executor_worker, data-access-layer, common_services):

• tsc --noEmit — clean
• eslint --ext .ts src --max-warnings=0 — clean
• prettier --check on every changed path — clean

SQL executed against Postgres 16, on a fixture reproducing the identity shapes both integrations actually produce — a GitHub member with a verified email identity, a git member with a verified username identity whose value is the email plus an unverified email identity, a third member sharing the address case-insensitively, an unrelated pair, and a soft-deleted pair as a negative control:

• Detection query returns exactly the four expected pairs; the soft-deleted pair is correctly excluded. For the git↔GitHub couple the GitHub member comes back as primary, which is the direction this PR depends on.
• Keyset pagination with limit 2 returns pages [1,2] then [3,4] — no repeat, no skip, full coverage. That is the gap from the previous review round, now closed and demonstrated rather than argued.
• moveMemberNoMerge verified in both orderings: absorbing B into A converts B–C into A–C, absorbing C into A converts C–B into A–B. Edges survive the absorbed member's ON DELETE CASCADE in both directions.
• Self-edge guard returns INSERT 0 0, so an absorbed member cannot produce an A–A row.

One item from this round is deliberately not coded: the no-merge read/merge race. Serializing it properly needs a lock taken on both the sweep and the UI's addToNoMerge path, and an in-transaction check alone only narrows the window rather than closing it under READ COMMITTED. Reasoning is on that thread — happy to implement whichever way maintainers prefer.

[Copilot]

Pull request overview

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