Scope committee_member.sfid forward mapping key by committee

[jordane]

Summary

• The v2->v1 create path keyed the committee_member.sfid forward mapping on the v1 API MemberID (contact SFID) alone, which the same contact reuses across every committee it belongs to — adding that contact to a second committee silently overwrote the first committee's forward mapping.
• Scope the create-path key by committee SFID and update the delete path to look up the scoped key.
• Add a one-shot backfill to migrate existing flat keys to the scoped format and tombstone stale entries whose committee can no longer be resolved.

Issue: LFXV2-2709

[Copilot]

Pull request overview

Scopes committee-member mappings by committee to prevent cross-committee collisions and adds migration support.

Changes:

• Uses committee-scoped forward keys for create/delete paths.
• Adds a one-shot migration job with dry-run support.
• Documents and tests the new key format.

Reviewed changes

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

Show a summary per file

File	Description
README.md	Documents scoped mappings and migration.
manifests/backfill-committee-member-forward-mappings-job.yaml	Defines the migration job.
cmd/lfx-v1-sync-helper/main.go	Registers and runs the backfill flag.
cmd/lfx-v1-sync-helper/ingest_indexer.go	Applies scoped keys to synchronization paths.
cmd/lfx-v1-sync-helper/backfill_committee_member_forward_mappings.go	Implements migration logic.
cmd/lfx-v1-sync-helper/backfill_committee_member_forward_mappings_test.go	Tests classification and key generation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

cmd/lfx-v1-sync-helper/backfill_committee_member_forward_mappings.go:387

• splitTwoParts accepts any non-empty strings after the first colon, including extra fields (for example, <committee-uuid>:<member-uuid>:extra). Such entries are classified as migratable instead of malformed; when the resulting lookup fails, the destructive unresolved branch tombstones the flat mapping. Since both v2 IDs are UUIDs, validate both fields here so unexpected values remain in the non-destructive malformed path.

	committeeUID, memberUID, ok := splitTwoParts(string(data))
	if !ok || committeeUID == "" || memberUID == "" {
		return forwardMappingClassification{outcome: forwardMappingMalformed}

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

cmd/lfx-v1-sync-helper/backfill_committee_member_forward_mappings.go:303

• A successful rollback at lines 226–228 leaves scopedKey containing the custom !del marker while the flat key remains live. On the next run, this branch treats that tombstone as an unexpected destination and reports a conflict forever, so a transient post-create liveness-read failure makes the documented retry unable to finish the migration. Treat isTombstonedMapping(entry.Value()) as a recreatable destination: revive it with a revision-guarded Update, return the new revision as owned by this run, and mirror that behavior in the dry-run destination check.

	case getErr == nil:
		// Holds something else (tombstoned, or a different committee/member pairing) —
		// don't overwrite a value we don't understand.
		return true, false, 0, nil

[emsearcy]

Key structure across the two flows, and why they need to match

Tracing through both directions of committee-member sync, here's the forward (v1→v2) and reverse (v2→v1) mapping each one writes today:

Forward-creation (v1 WAL upsert with no existing mapping → v2 API create, handleCommitteeMemberUpdate in handlers_committees.go):

• Forward (v1→v2): committee_member.sfid.<recordSFID> → <committeeUID>:<memberUID>, where recordSFID is the platform.community__c row's own sfid (globally unique per membership row, never reused across committees).
• Reverse (v2→v1): committee_member.uid.<memberUID> → <projectSFID>:<committeeSFID>:<contactSFID>.
• Side key: committee_member.record_sfid.<memberUID> → <recordSFID> (kept separately so a later v1-originated delete can still tombstone the forward key without growing the reverse mapping's field count).

Critically, recordSFID is exactly what handleCommitteeMemberUpdate looks up (memberMappingKey := "committee_member.sfid." + sfid) on every subsequent v1 WAL event for that row, to decide "update existing" vs. "create new."

Reverse-creation (v2 "created" indexer event → v1 API create, syncCommitteeMemberCreateToV1 in ingest_indexer.go), pre-PR:

• Forward (v1→v2): committee_member.sfid.<memberSFID> → <committeeUID>:<memberUID>, where memberSFID = result.MemberID (the contact SFID returned by the v1 create call) — not a community__c record sfid.
• Reverse (v2→v1): same shape, committee_member.uid.<memberUID> → <projectSFID>:<committeeSFID>:<contactSFID>.

This PR rescopes that forward key to committee_member.sfid.<committeeSFID>.<contactSFID>, which correctly stops one contact's second-committee membership from clobbering the first committee's mapping.

The bigger problem this doesn't address

The reverse-creation flow's forward mapping is keyed differently (<contactSFID> / now <committeeSFID>.<contactSFID>) than the forward-creation flow's (<recordSFID>). But only one of these key formats is ever read by handleCommitteeMemberUpdate — it always looks up committee_member.sfid.<recordSFID>.

So when a v2-created member is pushed to v1 via this create call, and the resulting platform.community__c row is later picked up by the WAL pipeline and echoed back through handleCommitteeMemberUpdate, the lookup under <recordSFID> finds nothing (because reverse-creation never wrote anything under that key), and the WAL handler falls into the "create new committee member" branch — calling createCommitteeMember again for a membership that already exists in v2. As far as I can tell nothing else in the pipeline guards against this (no email/committee dedup check on the v2 side that I found).

Put differently: the two flows need to write their forward mapping under the same key format, since that key is the sole mechanism handleCommitteeMemberUpdate uses to recognize "this v1 row already has a v2 counterpart." Today they don't, regardless of whether this PR merges.

Proposed fix

projectServiceCommitteeMemberResponse (the type createV1CommitteeMember deserializes into) already has an ID field alongside MemberID — and the existing integration test (TestCommitteeMemberClientIntegration) uses member.ID (not member.MemberID) for the subsequent update/delete calls, which suggests ID is in fact the community__c record sfid, obtainable directly from the create response.

If that holds up, the cleanest fix is for syncCommitteeMemberCreateToV1 to write its forward mapping as committee_member.sfid.<result.ID> — matching the forward-creation flow's key exactly, with no committee-scoping needed (since a record sfid is already globally unique). That fixes both the WAL-echo duplicate-create problem and the collision this PR targets, in one key format, and probably lets the new committee-scoped key and the associated backfill be dropped.

If it turns out result.ID isn't reliably available or isn't the same value the WAL rows carry as their own sfid (worth confirming against the actual API response, not just the struct field name), then the fallback is what this PR already does, but applied to both flows: handleCommitteeMemberUpdate would also need to move to (and look up) committee_member.sfid.<committeeSFID>.<contactSFID>, and the backfill_committee_member_mappings (LFXV2-2673) / backfill_committee_member_forward_mappings (this PR) backfills would need to be reconciled into one migration that converges both flows onto the committee-namespaced contact-SFID key.

Given the impact (possible duplicate v2 committee members on WAL echo), I'd like to resolve which of these two paths is correct before merging.